Merge branch...

Merge branch '352277-failures-in-manage-project-import-imports-large-github-repo-via-api-spec' into 'master'

Create LegacyDiffNote when GitHub suggestion is outdated

See merge request gitlab-org/gitlab!81014

lib/gitlab/github_import/importer/diff_note_importer.rb

@@ -4,6 +4,8 @@ module Gitlab
   module GithubImport
     module Importer
       class DiffNoteImporter
+        DiffNoteCreationError = Class.new(ActiveRecord::RecordInvalid)
+
         # note - An instance of `Gitlab::GithubImport::Representation::DiffNote`
         # project - An instance of `Project`
         # client - An instance of `Gitlab::GithubImport::Client`
@@ -31,7 +33,7 @@ module Gitlab
           else
             import_with_legacy_diff_note
           end
-        rescue ::DiffNote::NoteDiffFileCreationError => e
+        rescue ::DiffNote::NoteDiffFileCreationError, DiffNoteCreationError => e
           Logger.warn(message: e.message, 'error.class': e.class.name)
 
           import_with_legacy_diff_note
@@ -84,7 +86,7 @@ module Gitlab
         def import_with_diff_note
           log_diff_note_creation('DiffNote')
 
-          ::Import::Github::Notes::CreateService.new(project, author, {
+          record = ::Import::Github::Notes::CreateService.new(project, author, {
             noteable_type: note.noteable_type,
             system: false,
             type: 'DiffNote',
@@ -97,6 +99,8 @@ module Gitlab
             updated_at: note.updated_at,
             position: note.diff_position
           }).execute
+
+          raise DiffNoteCreationError, record unless record.persisted?
         end
 
         def note_body

spec/lib/gitlab/github_import/importer/diff_note_importer_spec.rb

@@ -12,6 +12,7 @@ RSpec.describe Gitlab::GithubImport::Importer::DiffNoteImporter, :aggregate_fail
   let(:updated_at) { Time.new(2017, 1, 1, 12, 15).utc }
   let(:note_body) { 'Hello' }
   let(:file_path) { 'files/ruby/popen.rb' }
+  let(:end_line) { 15 }
 
   let(:diff_hunk) do
     '@@ -14 +14 @@
@@ -31,7 +32,7 @@ RSpec.describe Gitlab::GithubImport::Importer::DiffNoteImporter, :aggregate_fail
       created_at: created_at,
       updated_at: updated_at,
       start_line: nil,
-      end_line: 15,
+      end_line: end_line,
       github_id: 1,
       diff_hunk: diff_hunk,
       side: 'RIGHT'
@@ -173,7 +174,24 @@ RSpec.describe Gitlab::GithubImport::Importer::DiffNoteImporter, :aggregate_fail
             NOTE
         end
 
-        context 'when the note diff file creation fails' do
+        context 'when the note diff file creation fails with DiffNoteCreationError due to outdated suggestion' do
+          let(:end_line) { nil }
+
+          it 'falls back to the LegacyDiffNote' do
+            expect(Gitlab::GithubImport::Logger)
+              .to receive(:warn)
+                    .with(
+                      message: "Validation failed: Line code can't be blank, Line code must be a valid line code, Position is incomplete",
+                      'error.class': 'Gitlab::GithubImport::Importer::DiffNoteImporter::DiffNoteCreationError'
+                    )
+
+            expect { subject.execute }
+              .to change(LegacyDiffNote, :count)
+                    .and not_change(DiffNote, :count)
+          end
+        end
+
+        context 'when the note diff file creation fails with NoteDiffFileCreationError' do
           it 'falls back to the LegacyDiffNote' do
             exception = ::DiffNote::NoteDiffFileCreationError.new('Failed to create diff note file')