From 0b8c2beaf4a7e12206a456a32312e9629e623eed Mon Sep 17 00:00:00 2001
From: Kassio Borges <kborges@gitlab.com>
Date: Tue, 7 Dec 2021 09:19:51 +0000
Subject: [PATCH] GithubImporter: Fallback to LegacyDiffNote when DiffNote
 fails

When `DiffNote` fails to create the diff note file with
`DiffNote::NoteDiffFileCreationError` exception, fallsback to use the
`LegacyDiffNote`.

Changelog: fixed
---
 .../importer/diff_note_importer.rb            |  4 +++
 .../github_import/representation/diff_note.rb | 21 ++++++++-------
 .../importer/diff_note_importer_spec.rb       | 27 ++++++++++++++++++-
 3 files changed, 42 insertions(+), 10 deletions(-)

diff --git a/lib/gitlab/github_import/importer/diff_note_importer.rb b/lib/gitlab/github_import/importer/diff_note_importer.rb
index 0aa0896aa57d6c..8a8d23401c1392 100644
--- a/lib/gitlab/github_import/importer/diff_note_importer.rb
+++ b/lib/gitlab/github_import/importer/diff_note_importer.rb
@@ -31,6 +31,10 @@ def execute
           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
diff --git a/lib/gitlab/github_import/representation/diff_note.rb b/lib/gitlab/github_import/representation/diff_note.rb
index fecff0644c2090..04f53accfeb7b7 100644
--- a/lib/gitlab/github_import/representation/diff_note.rb
+++ b/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 @@ def github_identifiers
         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
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 0448ada6bcaa7b..a0e78186caa5db 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
@@ -173,9 +173,11 @@
             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 @@
               ```
               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
-- 
GitLab