Merge branch 'kassio/github-importer-fallback-failed-diffnote-suggestion' into 'master'

GithubImporter: Fallback to LegacyDiffNote when DiffNote fails

See merge request gitlab-org/gitlab!76376

lib/gitlab/github_import/importer/diff_note_importer.rb

@@ -31,6 +31,10 @@ module Gitlab
           else
             import_with_legacy_diff_note
           end
+        rescue ::DiffNote::NoteDiffFileCreationError => e
+          Logger.warn(message: e.message, 'error.class': e.class.name)
+
+          import_with_legacy_diff_note
         rescue ActiveRecord::InvalidForeignKey => e
           # It's possible the project and the issue have been deleted since
           # scheduling this job. In this case we'll just skip creating the note

lib/gitlab/github_import/representation/diff_note.rb

@@ -4,6 +4,7 @@ module Gitlab
   module GithubImport
     module Representation
       class DiffNote
+        include Gitlab::Utils::StrongMemoize
         include ToHash
         include ExposeAttribute
 
@@ -127,15 +128,17 @@ module Gitlab
         end
 
         def discussion_id
-          if in_reply_to_id.present?
-            current_discussion_id
-          else
-            Discussion.discussion_id(
-              Struct
-              .new(:noteable_id, :noteable_type)
-              .new(merge_request.id, NOTEABLE_TYPE)
-            ).tap do |discussion_id|
-              cache_discussion_id(discussion_id)
+          strong_memoize(:discussion_id) do
+            if in_reply_to_id.present?
+              current_discussion_id
+            else
+              Discussion.discussion_id(
+                Struct
+                .new(:noteable_id, :noteable_type)
+                .new(merge_request.id, NOTEABLE_TYPE)
+              ).tap do |discussion_id|
+                cache_discussion_id(discussion_id)
+              end
             end
           end
         end

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

@@ -173,9 +173,11 @@ RSpec.describe Gitlab::GithubImport::Importer::DiffNoteImporter, :aggregate_fail
             EOB
           end
 
-          it 'imports the note as diff note' do
+          before do
             stub_user_finder(user.id, true)
+          end
 
+          it 'imports the note as diff note' do
             expect { subject.execute }
               .to change(DiffNote, :count)
               .by(1)
@@ -212,6 +214,29 @@ RSpec.describe Gitlab::GithubImport::Importer::DiffNoteImporter, :aggregate_fail
               ```
               NOTE
           end
+
+          context 'when the note diff file creation fails' do
+            it 'falls back to the LegacyDiffNote' do
+              exception = ::DiffNote::NoteDiffFileCreationError.new('Failed to create diff note file')
+
+              expect_next_instance_of(::Import::Github::Notes::CreateService) do |service|
+                expect(service)
+                  .to receive(:execute)
+                  .and_raise(exception)
+              end
+
+              expect(Gitlab::GithubImport::Logger)
+                .to receive(:warn)
+                .with(
+                  message: 'Failed to create diff note file',
+                  'error.class': 'DiffNote::NoteDiffFileCreationError'
+                )
+
+              expect { subject.execute }
+                .to change(LegacyDiffNote, :count)
+                .and not_change(DiffNote, :count)
+            end
+          end
         end
       end
     end