Merge branch '346495-fix-github-diff-note-discussion-id' into 'master'

Fix missing discussion_id in GitHub imports

See merge request gitlab-org/gitlab!80584

lib/gitlab/github_import/representation/diff_note.rb

@@ -129,17 +129,7 @@ module Gitlab
 
         def 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
+            (in_reply_to_id.present? && current_discussion_id) || generate_discussion_id
           end
         end
 
@@ -160,6 +150,16 @@ module Gitlab
           side == 'RIGHT'
         end
 
+        def generate_discussion_id
+          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
+
         def cache_discussion_id(discussion_id)
           Gitlab::Cache::Import::Caching.write(discussion_id_cache_key(note_id), discussion_id)
         end

spec/lib/gitlab/github_import/representation/diff_note_spec.rb

@@ -2,7 +2,7 @@
 
 require 'spec_helper'
 
-RSpec.describe Gitlab::GithubImport::Representation::DiffNote, :clean_gitlab_redis_shared_state do
+RSpec.describe Gitlab::GithubImport::Representation::DiffNote, :clean_gitlab_redis_cache do
   let(:hunk) do
     '@@ -1 +1 @@
     -Hello
@@ -166,6 +166,23 @@ RSpec.describe Gitlab::GithubImport::Representation::DiffNote, :clean_gitlab_red
             expect(new_discussion_note.discussion_id)
               .to eq('SECOND_DISCUSSION_ID')
           end
+
+          context 'when cached value does not exist' do
+            it 'falls back to generating a new discussion_id' do
+              expect(Discussion)
+                .to receive(:discussion_id)
+                .and_return('NEW_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
+
+              expect(reply_note.discussion_id).to eq('NEW_DISCUSSION_ID')
+            end
+          end
         end
       end