Add latest changes from gitlab-org/gitlab@14-5-stable-eev14.5.0-rc42

9 files changed, 508 insertions, 347 deletions

diff --git a/spec/lib/gitlab/github_import/bulk_importing_spec.rb b/spec/lib/gitlab/github_import/bulk_importing_spec.rb
index 6c94973b5a8..e170496ff7b 100644
--- a/spec/lib/gitlab/github_import/bulk_importing_spec.rb
+++ b/spec/lib/gitlab/github_import/bulk_importing_spec.rb
@@ -116,13 +116,13 @@ RSpec.describe Gitlab::GithubImport::BulkImporting do
           value: 5
         )
 
-      expect(Gitlab::Database.main)
-        .to receive(:bulk_insert)
+      expect(ApplicationRecord)
+        .to receive(:legacy_bulk_insert)
         .ordered
         .with('kittens', rows.first(5))
 
-      expect(Gitlab::Database.main)
-        .to receive(:bulk_insert)
+      expect(ApplicationRecord)
+        .to receive(:legacy_bulk_insert)
         .ordered
         .with('kittens', rows.last(5))
 
diff --git a/spec/lib/gitlab/github_import/importer/diff_note_importer_spec.rb b/spec/lib/gitlab/github_import/importer/diff_note_importer_spec.rb
index 3dc15c7c059..0448ada6bca 100644
--- a/spec/lib/gitlab/github_import/importer/diff_note_importer_spec.rb
+++ b/spec/lib/gitlab/github_import/importer/diff_note_importer_spec.rb
@@ -2,156 +2,226 @@
 
 require 'spec_helper'
 
-RSpec.describe Gitlab::GithubImport::Importer::DiffNoteImporter do
-  let(:project) { create(:project) }
-  let(:client) { double(:client) }
-  let(:user) { create(:user) }
-  let(:created_at) { Time.new(2017, 1, 1, 12, 00) }
-  let(:updated_at) { Time.new(2017, 1, 1, 12, 15) }
+RSpec.describe Gitlab::GithubImport::Importer::DiffNoteImporter, :aggregate_failures do
+  let_it_be(:project) { create(:project, :repository) }
+  let_it_be(:user) { create(:user) }
 
-  let(:hunk) do
-    '@@ -1 +1 @@
+  let(:client) { double(:client) }
+  let(:discussion_id) { 'b0fa404393eeebb4e82becb8104f238812bb1fe6' }
+  let(:created_at) { Time.new(2017, 1, 1, 12, 00).utc }
+  let(:updated_at) { Time.new(2017, 1, 1, 12, 15).utc }
+  let(:note_body) { 'Hello' }
+  let(:file_path) { 'files/ruby/popen.rb' }
+
+  let(:diff_hunk) do
+    '@@ -14 +14 @@
     -Hello
     +Hello world'
   end
 
-  let(:note) do
+  let(:note_representation) do
     Gitlab::GithubImport::Representation::DiffNote.new(
       noteable_type: 'MergeRequest',
       noteable_id: 1,
       commit_id: '123abc',
       original_commit_id: 'original123abc',
-      file_path: 'README.md',
-      diff_hunk: hunk,
-      author: Gitlab::GithubImport::Representation::User
-        .new(id: user.id, login: user.username),
-      note: 'Hello',
+      file_path: file_path,
+      author: Gitlab::GithubImport::Representation::User.new(id: user.id, login: user.username),
+      note: note_body,
       created_at: created_at,
       updated_at: updated_at,
-      github_id: 1
+      start_line: nil,
+      end_line: 15,
+      github_id: 1,
+      diff_hunk: diff_hunk,
+      side: 'RIGHT'
     )
   end
 
-  let(:importer) { described_class.new(note, project, client) }
+  subject(:importer) { described_class.new(note_representation, project, client) }
+
+  shared_examples 'diff notes without suggestion' do
+    it 'imports the note as legacy diff note' do
+      stub_user_finder(user.id, true)
+
+      expect { subject.execute }
+        .to change(LegacyDiffNote, :count)
+        .by(1)
+
+      note = project.notes.diff_notes.take
+      expect(note).to be_valid
+      expect(note.author_id).to eq(user.id)
+      expect(note.commit_id).to eq('original123abc')
+      expect(note.created_at).to eq(created_at)
+      expect(note.diff).to be_an_instance_of(Gitlab::Git::Diff)
+      expect(note.discussion_id).to eq(discussion_id)
+      expect(note.line_code).to eq(note_representation.line_code)
+      expect(note.note).to eq('Hello')
+      expect(note.noteable_id).to eq(merge_request.id)
+      expect(note.noteable_type).to eq('MergeRequest')
+      expect(note.project_id).to eq(project.id)
+      expect(note.st_diff).to eq(note_representation.diff_hash)
+      expect(note.system).to eq(false)
+      expect(note.type).to eq('LegacyDiffNote')
+      expect(note.updated_at).to eq(updated_at)
+    end
+
+    it 'adds a "created by:" note when the author cannot be found' do
+      stub_user_finder(project.creator_id, false)
+
+      expect { subject.execute }
+        .to change(LegacyDiffNote, :count)
+        .by(1)
+
+      note = project.notes.diff_notes.take
+      expect(note).to be_valid
+      expect(note.author_id).to eq(project.creator_id)
+      expect(note.note).to eq("*Created by: #{user.username}*\n\nHello")
+    end
+
+    it 'does not import the note when a foreign key error is raised' do
+      stub_user_finder(project.creator_id, false)
+
+      expect(ApplicationRecord)
+        .to receive(:legacy_bulk_insert)
+        .and_raise(ActiveRecord::InvalidForeignKey, 'invalid foreign key')
+
+      expect { subject.execute }
+        .not_to change(LegacyDiffNote, :count)
+    end
+  end
 
   describe '#execute' do
     context 'when the merge request no longer exists' do
       it 'does not import anything' do
-        expect(Gitlab::Database.main).not_to receive(:bulk_insert)
+        expect(ApplicationRecord).not_to receive(:legacy_bulk_insert)
 
-        importer.execute
+        expect { subject.execute }
+          .to not_change(DiffNote, :count)
+          .and not_change(LegacyDiffNote, :count)
       end
     end
 
     context 'when the merge request exists' do
-      let!(:merge_request) do
+      let_it_be(:merge_request) do
         create(:merge_request, source_project: project, target_project: project)
       end
 
       before do
-        allow(importer)
-          .to receive(:find_merge_request_id)
-          .and_return(merge_request.id)
+        expect_next_instance_of(Gitlab::GithubImport::IssuableFinder) do |finder|
+          expect(finder)
+            .to receive(:database_id)
+            .and_return(merge_request.id)
+        end
+
+        expect(Discussion)
+          .to receive(:discussion_id)
+          .and_return(discussion_id)
       end
 
-      it 'imports the note' do
-        allow(importer.user_finder)
-          .to receive(:author_id_for)
-          .and_return([user.id, true])
-
-        expect(Gitlab::Database.main)
-          .to receive(:bulk_insert)
-          .with(
-            LegacyDiffNote.table_name,
-            [
-              {
-                discussion_id: anything,
-                noteable_type: 'MergeRequest',
-                noteable_id: merge_request.id,
-                project_id: project.id,
-                author_id: user.id,
-                note: 'Hello',
-                system: false,
-                commit_id: 'original123abc',
-                line_code: note.line_code,
-                type: 'LegacyDiffNote',
-                created_at: created_at,
-                updated_at: updated_at,
-                st_diff: note.diff_hash.to_yaml
-              }
-            ]
-          )
-          .and_call_original
-
-        importer.execute
+      context 'when github_importer_use_diff_note_with_suggestions is disabled' do
+        before do
+          stub_feature_flags(github_importer_use_diff_note_with_suggestions: false)
+        end
+
+        it_behaves_like 'diff notes without suggestion'
+
+        context 'when the note has suggestions' do
+          let(:note_body) do
+            <<~EOB
+            Suggestion:
+            ```suggestion
+            what do you think to do it like this
+            ```
+            EOB
+          end
+
+          it 'imports the note' do
+            stub_user_finder(user.id, true)
+
+            expect { subject.execute }
+              .to change(LegacyDiffNote, :count)
+              .and not_change(DiffNote, :count)
+
+            note = project.notes.diff_notes.take
+            expect(note).to be_valid
+            expect(note.note)
+              .to eq <<~NOTE
+              Suggestion:
+              ```suggestion:-0+0
+              what do you think to do it like this
+              ```
+              NOTE
+          end
+        end
       end
 
-      it 'imports the note when the author could not be found' do
-        allow(importer.user_finder)
-          .to receive(:author_id_for)
-          .and_return([project.creator_id, false])
-
-        expect(Gitlab::Database.main)
-          .to receive(:bulk_insert)
-          .with(
-            LegacyDiffNote.table_name,
-            [
-              {
-                discussion_id: anything,
-                noteable_type: 'MergeRequest',
-                noteable_id: merge_request.id,
-                project_id: project.id,
-                author_id: project.creator_id,
-                note: "*Created by: #{user.username}*\n\nHello",
-                system: false,
-                commit_id: 'original123abc',
-                line_code: note.line_code,
-                type: 'LegacyDiffNote',
-                created_at: created_at,
-                updated_at: updated_at,
-                st_diff: note.diff_hash.to_yaml
-              }
-            ]
-          )
-          .and_call_original
-
-        importer.execute
-      end
-
-      it 'produces a valid LegacyDiffNote' do
-        allow(importer.user_finder)
-          .to receive(:author_id_for)
-          .and_return([user.id, true])
-
-        importer.execute
-
-        note = project.notes.diff_notes.take
-
-        expect(note).to be_valid
-        expect(note.diff).to be_an_instance_of(Gitlab::Git::Diff)
-      end
-
-      it 'does not import the note when a foreign key error is raised' do
-        allow(importer.user_finder)
-          .to receive(:author_id_for)
-          .and_return([project.creator_id, false])
-
-        expect(Gitlab::Database.main)
-          .to receive(:bulk_insert)
-          .and_raise(ActiveRecord::InvalidForeignKey, 'invalid foreign key')
-
-        expect { importer.execute }.not_to raise_error
+      context 'when github_importer_use_diff_note_with_suggestions is enabled' do
+        before do
+          stub_feature_flags(github_importer_use_diff_note_with_suggestions: true)
+        end
+
+        it_behaves_like 'diff notes without suggestion'
+
+        context 'when the note has suggestions' do
+          let(:note_body) do
+            <<~EOB
+            Suggestion:
+            ```suggestion
+            what do you think to do it like this
+            ```
+            EOB
+          end
+
+          it 'imports the note as diff note' do
+            stub_user_finder(user.id, true)
+
+            expect { subject.execute }
+              .to change(DiffNote, :count)
+              .by(1)
+
+            note = project.notes.diff_notes.take
+            expect(note).to be_valid
+            expect(note.noteable_type).to eq('MergeRequest')
+            expect(note.noteable_id).to eq(merge_request.id)
+            expect(note.project_id).to eq(project.id)
+            expect(note.author_id).to eq(user.id)
+            expect(note.system).to eq(false)
+            expect(note.discussion_id).to eq(discussion_id)
+            expect(note.commit_id).to eq('original123abc')
+            expect(note.line_code).to eq(note_representation.line_code)
+            expect(note.type).to eq('DiffNote')
+            expect(note.created_at).to eq(created_at)
+            expect(note.updated_at).to eq(updated_at)
+            expect(note.position.to_h).to eq({
+              base_sha: merge_request.diffs.diff_refs.base_sha,
+              head_sha: merge_request.diffs.diff_refs.head_sha,
+              start_sha: merge_request.diffs.diff_refs.start_sha,
+              new_line: 15,
+              old_line: nil,
+              new_path: file_path,
+              old_path: file_path,
+              position_type: 'text',
+              line_range: nil
+            })
+            expect(note.note)
+              .to eq <<~NOTE
+              Suggestion:
+              ```suggestion:-0+0
+              what do you think to do it like this
+              ```
+              NOTE
+          end
+        end
       end
     end
   end
 
-  describe '#find_merge_request_id' do
-    it 'returns a merge request ID' do
-      expect_next_instance_of(Gitlab::GithubImport::IssuableFinder) do |instance|
-        expect(instance).to receive(:database_id).and_return(10)
-      end
-
-      expect(importer.find_merge_request_id).to eq(10)
+  def stub_user_finder(user, found)
+    expect_next_instance_of(Gitlab::GithubImport::UserFinder) do |finder|
+      expect(finder)
+        .to receive(:author_id_for)
+        .and_return([user, found])
     end
   end
 end
diff --git a/spec/lib/gitlab/github_import/importer/diff_notes_importer_spec.rb b/spec/lib/gitlab/github_import/importer/diff_notes_importer_spec.rb
index be4fc3cbf16..1c7b35ed928 100644
--- a/spec/lib/gitlab/github_import/importer/diff_notes_importer_spec.rb
+++ b/spec/lib/gitlab/github_import/importer/diff_notes_importer_spec.rb
@@ -19,7 +19,9 @@ RSpec.describe Gitlab::GithubImport::Importer::DiffNotesImporter do
       updated_at: Time.zone.now,
       line: 23,
       start_line: nil,
+      in_reply_to_id: nil,
       id: 1,
+      side: 'RIGHT',
       body: <<~BODY
         Hello World
 
diff --git a/spec/lib/gitlab/github_import/importer/issue_importer_spec.rb b/spec/lib/gitlab/github_import/importer/issue_importer_spec.rb
index 0926000428c..4287c32b947 100644
--- a/spec/lib/gitlab/github_import/importer/issue_importer_spec.rb
+++ b/spec/lib/gitlab/github_import/importer/issue_importer_spec.rb
@@ -190,8 +190,8 @@ RSpec.describe Gitlab::GithubImport::Importer::IssueImporter, :clean_gitlab_redi
         .with(issue.assignees[1])
         .and_return(5)
 
-      expect(Gitlab::Database.main)
-        .to receive(:bulk_insert)
+      expect(ApplicationRecord)
+        .to receive(:legacy_bulk_insert)
         .with(
           IssueAssignee.table_name,
           [{ issue_id: 1, user_id: 4 }, { issue_id: 1, user_id: 5 }]
diff --git a/spec/lib/gitlab/github_import/importer/label_links_importer_spec.rb b/spec/lib/gitlab/github_import/importer/label_links_importer_spec.rb
index 241a0fef600..e68849755b2 100644
--- a/spec/lib/gitlab/github_import/importer/label_links_importer_spec.rb
+++ b/spec/lib/gitlab/github_import/importer/label_links_importer_spec.rb
@@ -39,8 +39,8 @@ RSpec.describe Gitlab::GithubImport::Importer::LabelLinksImporter do
         .and_return(1)
 
       freeze_time do
-        expect(Gitlab::Database.main)
-          .to receive(:bulk_insert)
+        expect(ApplicationRecord)
+          .to receive(:legacy_bulk_insert)
           .with(
             LabelLink.table_name,
             [
@@ -64,8 +64,8 @@ RSpec.describe Gitlab::GithubImport::Importer::LabelLinksImporter do
         .with('bug')
         .and_return(nil)
 
-      expect(Gitlab::Database.main)
-        .to receive(:bulk_insert)
+      expect(ApplicationRecord)
+        .to receive(:legacy_bulk_insert)
         .with(LabelLink.table_name, [])
 
       importer.create_labels
diff --git a/spec/lib/gitlab/github_import/importer/note_importer_spec.rb b/spec/lib/gitlab/github_import/importer/note_importer_spec.rb
index 820f46c7286..96d8acbd3de 100644
--- a/spec/lib/gitlab/github_import/importer/note_importer_spec.rb
+++ b/spec/lib/gitlab/github_import/importer/note_importer_spec.rb
@@ -41,8 +41,8 @@ RSpec.describe Gitlab::GithubImport::Importer::NoteImporter do
             .with(github_note)
             .and_return([user.id, true])
 
-          expect(Gitlab::Database.main)
-            .to receive(:bulk_insert)
+          expect(ApplicationRecord)
+            .to receive(:legacy_bulk_insert)
             .with(
               Note.table_name,
               [
@@ -71,8 +71,8 @@ RSpec.describe Gitlab::GithubImport::Importer::NoteImporter do
             .with(github_note)
             .and_return([project.creator_id, false])
 
-          expect(Gitlab::Database.main)
-            .to receive(:bulk_insert)
+          expect(ApplicationRecord)
+            .to receive(:legacy_bulk_insert)
             .with(
               Note.table_name,
               [
@@ -115,7 +115,7 @@ RSpec.describe Gitlab::GithubImport::Importer::NoteImporter do
 
     context 'when the noteable does not exist' do
       it 'does not import the note' do
-        expect(Gitlab::Database.main).not_to receive(:bulk_insert)
+        expect(ApplicationRecord).not_to receive(:legacy_bulk_insert)
 
         importer.execute
       end
@@ -134,8 +134,8 @@ RSpec.describe Gitlab::GithubImport::Importer::NoteImporter do
           .with(github_note)
           .and_return([user.id, true])
 
-        expect(Gitlab::Database.main)
-          .to receive(:bulk_insert)
+        expect(ApplicationRecord)
+          .to receive(:legacy_bulk_insert)
           .and_raise(ActiveRecord::InvalidForeignKey, 'invalid foreign key')
 
         expect { importer.execute }.not_to raise_error
diff --git a/spec/lib/gitlab/github_import/importer/pull_requests_merged_by_importer_spec.rb b/spec/lib/gitlab/github_import/importer/pull_requests_merged_by_importer_spec.rb
index 4a47d103cde..b6c162aafa9 100644
--- a/spec/lib/gitlab/github_import/importer/pull_requests_merged_by_importer_spec.rb
+++ b/spec/lib/gitlab/github_import/importer/pull_requests_merged_by_importer_spec.rb
@@ -4,7 +4,8 @@ require 'spec_helper'
 
 RSpec.describe Gitlab::GithubImport::Importer::PullRequestsMergedByImporter do
   let(:client) { double }
-  let(:project) { create(:project, import_source: 'http://somegithub.com') }
+
+  let_it_be(:project) { create(:project, import_source: 'http://somegithub.com') }
 
   subject { described_class.new(project, client) }
 
@@ -27,14 +28,11 @@ RSpec.describe Gitlab::GithubImport::Importer::PullRequestsMergedByImporter do
   end
 
   describe '#each_object_to_import', :clean_gitlab_redis_cache do
-    it 'fetchs the merged pull requests data' do
-      create(
-        :merged_merge_request,
-        iid: 999,
-        source_project: project,
-        target_project: project
-      )
+    let!(:merge_request) do
+      create(:merged_merge_request, iid: 999, source_project: project, target_project: project)
+    end
 
+    it 'fetches the merged pull requests data' do
       pull_request = double
 
       allow(client)
@@ -48,5 +46,16 @@ RSpec.describe Gitlab::GithubImport::Importer::PullRequestsMergedByImporter do
 
       subject.each_object_to_import {}
     end
+
+    it 'skips cached merge requests' do
+      Gitlab::Cache::Import::Caching.set_add(
+        "github-importer/already-imported/#{project.id}/pull_requests_merged_by",
+        merge_request.id
+      )
+
+      expect(client).not_to receive(:pull_request)
+
+      subject.each_object_to_import {}
+    end
   end
 end
diff --git a/spec/lib/gitlab/github_import/representation/diff_note_spec.rb b/spec/lib/gitlab/github_import/representation/diff_note_spec.rb
index 81722c0eba7..63834cfdb94 100644
--- a/spec/lib/gitlab/github_import/representation/diff_note_spec.rb
+++ b/spec/lib/gitlab/github_import/representation/diff_note_spec.rb
@@ -2,23 +2,44 @@
 
 require 'spec_helper'
 
-RSpec.describe Gitlab::GithubImport::Representation::DiffNote do
+RSpec.describe Gitlab::GithubImport::Representation::DiffNote, :clean_gitlab_redis_shared_state do
   let(:hunk) do
     '@@ -1 +1 @@
     -Hello
     +Hello world'
   end
 
+  let(:merge_request) do
+    double(
+      :merge_request,
+      id: 54,
+      diff_refs: double(
+        :refs,
+        base_sha: 'base',
+        start_sha: 'start',
+        head_sha: 'head'
+      )
+    )
+  end
+
+  let(:project) { double(:project, id: 836) }
+  let(:note_id) { 1 }
+  let(:in_reply_to_id) { nil }
+  let(:start_line) { nil }
+  let(:end_line) { 23 }
+  let(:note_body) { 'Hello world' }
+  let(:user_data) { { 'id' => 4, 'login' => 'alice' } }
+  let(:side) { 'RIGHT' }
   let(:created_at) { Time.new(2017, 1, 1, 12, 00) }
   let(:updated_at) { Time.new(2017, 1, 1, 12, 15) }
 
-  shared_examples 'a DiffNote' do
+  shared_examples 'a DiffNote representation' do
     it 'returns an instance of DiffNote' do
       expect(note).to be_an_instance_of(described_class)
     end
 
     context 'the returned DiffNote' do
-      it 'includes the number of the note' do
+      it 'includes the number of the merge request' do
         expect(note.noteable_id).to eq(42)
       end
 
@@ -30,18 +51,6 @@ RSpec.describe Gitlab::GithubImport::Representation::DiffNote do
         expect(note.commit_id).to eq('123abc')
       end
 
-      it 'includes the user details' do
-        expect(note.author)
-          .to be_an_instance_of(Gitlab::GithubImport::Representation::User)
-
-        expect(note.author.id).to eq(4)
-        expect(note.author.login).to eq('alice')
-      end
-
-      it 'includes the note body' do
-        expect(note.note).to eq('Hello world')
-      end
-
       it 'includes the created timestamp' do
         expect(note.created_at).to eq(created_at)
       end
@@ -51,209 +60,250 @@ RSpec.describe Gitlab::GithubImport::Representation::DiffNote do
       end
 
       it 'includes the GitHub ID' do
-        expect(note.note_id).to eq(1)
+        expect(note.note_id).to eq(note_id)
       end
 
       it 'returns the noteable type' do
         expect(note.noteable_type).to eq('MergeRequest')
       end
-    end
-  end
-
-  describe '.from_api_response' do
-    let(:response) do
-      double(
-        :response,
-        html_url: 'https://github.com/foo/bar/pull/42',
-        path: 'README.md',
-        commit_id: '123abc',
-        original_commit_id: 'original123abc',
-        diff_hunk: hunk,
-        user: double(:user, id: 4, login: 'alice'),
-        body: 'Hello world',
-        created_at: created_at,
-        updated_at: updated_at,
-        line: 23,
-        start_line: nil,
-        id: 1
-      )
-    end
-
-    it_behaves_like 'a DiffNote' do
-      let(:note) { described_class.from_api_response(response) }
-    end
-
-    it 'does not set the user if the response did not include a user' do
-      allow(response)
-        .to receive(:user)
-        .and_return(nil)
-
-      note = described_class.from_api_response(response)
-
-      expect(note.author).to be_nil
-    end
-
-    it 'formats a suggestion in the note body' do
-      allow(response)
-        .to receive(:body)
-        .and_return <<~BODY
-      ```suggestion
-      Hello World
-      ```
-      BODY
-
-      note = described_class.from_api_response(response)
-
-      expect(note.note).to eq <<~BODY
-      ```suggestion:-0+0
-      Hello World
-      ```
-      BODY
-    end
-  end
-
-  describe '.from_json_hash' do
-    let(:hash) do
-      {
-        'noteable_type' => 'MergeRequest',
-        'noteable_id' => 42,
-        'file_path' => 'README.md',
-        'commit_id' => '123abc',
-        'original_commit_id' => 'original123abc',
-        'diff_hunk' => hunk,
-        'author' => { 'id' => 4, 'login' => 'alice' },
-        'note' => 'Hello world',
-        'created_at' => created_at.to_s,
-        'updated_at' => updated_at.to_s,
-        'note_id' => 1
-      }
-    end
 
-    it_behaves_like 'a DiffNote' do
-      let(:note) { described_class.from_json_hash(hash) }
-    end
-
-    it 'does not convert the author if it was not specified' do
-      hash.delete('author')
-
-      note = described_class.from_json_hash(hash)
+      describe '#diff_hash' do
+        it 'returns a Hash containing the diff details' do
+          expect(note.diff_hash).to eq(
+            diff: hunk,
+            new_path: 'README.md',
+            old_path: 'README.md',
+            a_mode: '100644',
+            b_mode: '100644',
+            new_file: false
+          )
+        end
+      end
 
-      expect(note.author).to be_nil
-    end
+      describe '#diff_position' do
+        before do
+          note.merge_request = double(
+            :merge_request,
+            diff_refs: double(
+              :refs,
+              base_sha: 'base',
+              start_sha: 'start',
+              head_sha: 'head'
+            )
+          )
+        end
+
+        context 'when the diff is an addition' do
+          it 'returns a Gitlab::Diff::Position' do
+            expect(note.diff_position.to_h).to eq(
+              base_sha: 'base',
+              head_sha: 'head',
+              line_range: nil,
+              new_line: 23,
+              new_path: 'README.md',
+              old_line: nil,
+              old_path: 'README.md',
+              position_type: 'text',
+              start_sha: 'start'
+            )
+          end
+        end
+
+        context 'when the diff is an deletion' do
+          let(:side) { 'LEFT' }
+
+          it 'returns a Gitlab::Diff::Position' do
+            expect(note.diff_position.to_h).to eq(
+              base_sha: 'base',
+              head_sha: 'head',
+              line_range: nil,
+              old_line: 23,
+              new_path: 'README.md',
+              new_line: nil,
+              old_path: 'README.md',
+              position_type: 'text',
+              start_sha: 'start'
+            )
+          end
+        end
+      end
 
-    it 'formats a suggestion in the note body' do
-      hash['note'] = <<~BODY
-      ```suggestion
-      Hello World
-      ```
-      BODY
+      describe '#discussion_id' do
+        before do
+          note.project = project
+          note.merge_request = merge_request
+        end
+
+        context 'when the note is a reply to a discussion' do
+          it 'uses the cached value as the discussion_id only when responding an existing discussion' do
+            expect(Discussion)
+              .to receive(:discussion_id)
+              .and_return('FIRST_DISCUSSION_ID', 'SECOND_DISCUSSION_ID')
+
+            # Creates the first discussion id and caches its value
+            expect(note.discussion_id)
+              .to eq('FIRST_DISCUSSION_ID')
+
+            reply_note = described_class.from_json_hash(
+              'note_id' => note.note_id + 1,
+              'in_reply_to_id' => note.note_id
+            )
+            reply_note.project = project
+            reply_note.merge_request = merge_request
+
+            # Reading from the cached value
+            expect(reply_note.discussion_id)
+              .to eq('FIRST_DISCUSSION_ID')
+
+            new_discussion_note = described_class.from_json_hash(
+              'note_id' => note.note_id + 2,
+              'in_reply_to_id' => nil
+            )
+            new_discussion_note.project = project
+            new_discussion_note.merge_request = merge_request
+
+            # Because it's a new discussion, it must not use the cached value
+            expect(new_discussion_note.discussion_id)
+              .to eq('SECOND_DISCUSSION_ID')
+          end
+        end
+      end
 
-      note = described_class.from_json_hash(hash)
+      describe '#github_identifiers' do
+        it 'returns a hash with needed identifiers' do
+          expect(note.github_identifiers).to eq(
+            noteable_id: 42,
+            noteable_type: 'MergeRequest',
+            note_id: 1
+          )
+        end
+      end
 
-      expect(note.note).to eq <<~BODY
-      ```suggestion:-0+0
-      Hello World
-      ```
-      BODY
-    end
-  end
+      describe '#line_code' do
+        it 'generates the proper line code' do
+          note = described_class.new(diff_hunk: hunk, file_path: 'README.md')
 
-  describe '#line_code' do
-    it 'returns a String' do
-      note = described_class.new(diff_hunk: hunk, file_path: 'README.md')
+          expect(note.line_code).to eq('8ec9a00bfd09b3190ac6b22251dbb1aa95a0579d_2_2')
+        end
+      end
 
-      expect(note.line_code).to be_an_instance_of(String)
+      describe '#note and #contains_suggestion?' do
+        it 'includes the note body' do
+          expect(note.note).to eq('Hello world')
+          expect(note.contains_suggestion?).to eq(false)
+        end
+
+        context 'when the note have a suggestion' do
+          let(:note_body) do
+            <<~BODY
+            ```suggestion
+            Hello World
+            ```
+            BODY
+          end
+
+          it 'returns the suggestion formatted in the note' do
+            expect(note.note).to eq <<~BODY
+            ```suggestion:-0+0
+            Hello World
+            ```
+            BODY
+            expect(note.contains_suggestion?).to eq(true)
+          end
+        end
+
+        context 'when the note have a multiline suggestion' do
+          let(:start_line) { 20 }
+          let(:end_line) { 23 }
+          let(:note_body) do
+            <<~BODY
+            ```suggestion
+            Hello World
+            ```
+            BODY
+          end
+
+          it 'returns the multi-line suggestion formatted in the note' do
+            expect(note.note).to eq <<~BODY
+            ```suggestion:-3+0
+            Hello World
+            ```
+            BODY
+            expect(note.contains_suggestion?).to eq(true)
+          end
+        end
+
+        describe '#author' do
+          it 'includes the user details' do
+            expect(note.author).to be_an_instance_of(
+              Gitlab::GithubImport::Representation::User
+            )
+
+            expect(note.author.id).to eq(4)
+            expect(note.author.login).to eq('alice')
+          end
+
+          context 'when the author is empty' do
+            let(:user_data) { nil }
+
+            it 'does not set the user if the response did not include a user' do
+              expect(note.author).to be_nil
+            end
+          end
+        end
+      end
     end
   end
 
-  describe '#diff_hash' do
-    it 'returns a Hash containing the diff details' do
-      note = described_class.from_json_hash(
-        'noteable_type' => 'MergeRequest',
-        'noteable_id' => 42,
-        'file_path' => 'README.md',
-        'commit_id' => '123abc',
-        'original_commit_id' => 'original123abc',
-        'diff_hunk' => hunk,
-        'author' => { 'id' => 4, 'login' => 'alice' },
-        'note' => 'Hello world',
-        'created_at' => created_at.to_s,
-        'updated_at' => updated_at.to_s,
-        'note_id' => 1
-      )
-
-      expect(note.diff_hash).to eq(
-        diff: hunk,
-        new_path: 'README.md',
-        old_path: 'README.md',
-        a_mode: '100644',
-        b_mode: '100644',
-        new_file: false
-      )
-    end
-  end
+  describe '.from_api_response' do
+    it_behaves_like 'a DiffNote representation' do
+      let(:response) do
+        double(
+          :response,
+          id: note_id,
+          html_url: 'https://github.com/foo/bar/pull/42',
+          path: 'README.md',
+          commit_id: '123abc',
+          original_commit_id: 'original123abc',
+          side: side,
+          user: user_data && double(:user, user_data),
+          diff_hunk: hunk,
+          body: note_body,
+          created_at: created_at,
+          updated_at: updated_at,
+          line: end_line,
+          start_line: start_line,
+          in_reply_to_id: in_reply_to_id
+        )
+      end
 
-  describe '#github_identifiers' do
-    it 'returns a hash with needed identifiers' do
-      github_identifiers = {
-        noteable_id: 42,
-        noteable_type: 'MergeRequest',
-        note_id: 1
-      }
-      other_attributes = { something_else: '_something_else_' }
-      note = described_class.new(github_identifiers.merge(other_attributes))
-
-      expect(note.github_identifiers).to eq(github_identifiers)
+      subject(:note) { described_class.from_api_response(response) }
     end
   end
 
-  describe '#note' do
-    it 'returns the given note' do
-      hash = {
-        'note': 'simple text'
-      }
-
-      note = described_class.new(hash)
-
-      expect(note.note).to eq 'simple text'
-    end
-
-    it 'returns the suggestion formatted in the note' do
-      hash = {
-        'note': <<~BODY
-        ```suggestion
-        Hello World
-        ```
-        BODY
-      }
-
-      note = described_class.new(hash)
-
-      expect(note.note).to eq <<~BODY
-      ```suggestion:-0+0
-      Hello World
-      ```
-      BODY
-    end
+  describe '.from_json_hash' do
+    it_behaves_like 'a DiffNote representation' do
+      let(:hash) do
+        {
+          'note_id' => note_id,
+          'noteable_type' => 'MergeRequest',
+          'noteable_id' => 42,
+          'file_path' => 'README.md',
+          'commit_id' => '123abc',
+          'original_commit_id' => 'original123abc',
+          'side' => side,
+          'author' => user_data,
+          'diff_hunk' => hunk,
+          'note' => note_body,
+          'created_at' => created_at.to_s,
+          'updated_at' => updated_at.to_s,
+          'end_line' => end_line,
+          'start_line' => start_line,
+          'in_reply_to_id' => in_reply_to_id
+        }
+      end
 
-    it 'returns the multi-line suggestion formatted in the note' do
-      hash = {
-        'start_line': 20,
-        'end_line': 23,
-        'note': <<~BODY
-        ```suggestion
-        Hello World
-        ```
-        BODY
-      }
-
-      note = described_class.new(hash)
-
-      expect(note.note).to eq <<~BODY
-      ```suggestion:-3+0
-      Hello World
-      ```
-      BODY
+      subject(:note) { described_class.from_json_hash(hash) }
     end
   end
 end
diff --git a/spec/lib/gitlab/github_import/representation/diff_notes/suggestion_formatter_spec.rb b/spec/lib/gitlab/github_import/representation/diff_notes/suggestion_formatter_spec.rb
index 2ffd5f50d3b..bcb8575bdbf 100644
--- a/spec/lib/gitlab/github_import/representation/diff_notes/suggestion_formatter_spec.rb
+++ b/spec/lib/gitlab/github_import/representation/diff_notes/suggestion_formatter_spec.rb
@@ -9,13 +9,19 @@ RSpec.describe Gitlab::GithubImport::Representation::DiffNotes::SuggestionFormat
     ```
     BODY
 
-    expect(described_class.formatted_note_for(note: note)).to eq(note)
+    note_formatter = described_class.new(note: note)
+
+    expect(note_formatter.formatted_note).to eq(note)
+    expect(note_formatter.contains_suggestion?).to eq(false)
   end
 
   it 'handles nil value for note' do
     note = nil
 
-    expect(described_class.formatted_note_for(note: note)).to eq(note)
+    note_formatter = described_class.new(note: note)
+
+    expect(note_formatter.formatted_note).to eq(note)
+    expect(note_formatter.contains_suggestion?).to eq(false)
   end
 
   it 'does not allow over 3 leading spaces for valid suggestion' do
@@ -26,7 +32,10 @@ RSpec.describe Gitlab::GithubImport::Representation::DiffNotes::SuggestionFormat
       ```
     BODY
 
-    expect(described_class.formatted_note_for(note: note)).to eq(note)
+    note_formatter = described_class.new(note: note)
+
+    expect(note_formatter.formatted_note).to eq(note)
+    expect(note_formatter.contains_suggestion?).to eq(false)
   end
 
   it 'allows up to 3 leading spaces' do
@@ -44,7 +53,10 @@ RSpec.describe Gitlab::GithubImport::Representation::DiffNotes::SuggestionFormat
       ```
     BODY
 
-    expect(described_class.formatted_note_for(note: note)).to eq(expected)
+    note_formatter = described_class.new(note: note)
+
+    expect(note_formatter.formatted_note).to eq(expected)
+    expect(note_formatter.contains_suggestion?).to eq(true)
   end
 
   it 'does nothing when there is any text without space after the suggestion tag' do
@@ -53,7 +65,10 @@ RSpec.describe Gitlab::GithubImport::Representation::DiffNotes::SuggestionFormat
     ```
     BODY
 
-    expect(described_class.formatted_note_for(note: note)).to eq(note)
+    note_formatter = described_class.new(note: note)
+
+    expect(note_formatter.formatted_note).to eq(note)
+    expect(note_formatter.contains_suggestion?).to eq(false)
   end
 
   it 'formats single-line suggestions' do
@@ -71,7 +86,10 @@ RSpec.describe Gitlab::GithubImport::Representation::DiffNotes::SuggestionFormat
       ```
     BODY
 
-    expect(described_class.formatted_note_for(note: note)).to eq(expected)
+    note_formatter = described_class.new(note: note)
+
+    expect(note_formatter.formatted_note).to eq(expected)
+    expect(note_formatter.contains_suggestion?).to eq(true)
   end
 
   it 'ignores text after suggestion tag on the same line' do
@@ -89,7 +107,10 @@ RSpec.describe Gitlab::GithubImport::Representation::DiffNotes::SuggestionFormat
     ```
     BODY
 
-    expect(described_class.formatted_note_for(note: note)).to eq(expected)
+    note_formatter = described_class.new(note: note)
+
+    expect(note_formatter.formatted_note).to eq(expected)
+    expect(note_formatter.contains_suggestion?).to eq(true)
   end
 
   it 'formats multiple single-line suggestions' do
@@ -115,7 +136,10 @@ RSpec.describe Gitlab::GithubImport::Representation::DiffNotes::SuggestionFormat
       ```
     BODY
 
-    expect(described_class.formatted_note_for(note: note)).to eq(expected)
+    note_formatter = described_class.new(note: note)
+
+    expect(note_formatter.formatted_note).to eq(expected)
+    expect(note_formatter.contains_suggestion?).to eq(true)
   end
 
   it 'formats multi-line suggestions' do
@@ -133,7 +157,10 @@ RSpec.describe Gitlab::GithubImport::Representation::DiffNotes::SuggestionFormat
       ```
     BODY
 
-    expect(described_class.formatted_note_for(note: note, start_line: 6, end_line: 8)).to eq(expected)
+    note_formatter = described_class.new(note: note, start_line: 6, end_line: 8)
+
+    expect(note_formatter.formatted_note).to eq(expected)
+    expect(note_formatter.contains_suggestion?).to eq(true)
   end
 
   it 'formats multiple multi-line suggestions' do
@@ -159,6 +186,9 @@ RSpec.describe Gitlab::GithubImport::Representation::DiffNotes::SuggestionFormat
       ```
     BODY
 
-    expect(described_class.formatted_note_for(note: note, start_line: 6, end_line: 8)).to eq(expected)
+    note_formatter = described_class.new(note: note, start_line: 6, end_line: 8)
+
+    expect(note_formatter.formatted_note).to eq(expected)
+    expect(note_formatter.contains_suggestion?).to eq(true)
   end
 end