Merge branch 'georgekoltsov/63955-fix-import-with-source-branch-deleted' into 'master'

Always return MR diff_refs if importing

See merge request gitlab-org/gitlab-ce!30630

app/models/merge_request.rb

@@ -588,7 +588,11 @@ class MergeRequest < ApplicationRecord
   end
 
   def diff_refs
-    persisted? ? merge_request_diff.diff_refs : repository_diff_refs
+    if importing? || persisted?
+      merge_request_diff.diff_refs
+    else
+      repository_diff_refs
+    end
   end
 
   # Instead trying to fetch the

changelogs/unreleased/georgekoltsov-63955-fix-import-with-source-branch-deleted.yml

@@ -0,0 +1,5 @@
+---
+title: Fix a bug that prevented projects containing merge request diff comments from being imported
+merge_request: 30630
+author:
+type: fixed

lib/gitlab/import_export/attribute_cleaner.rb

@@ -3,7 +3,7 @@
 module Gitlab
   module ImportExport
     class AttributeCleaner
-      ALLOWED_REFERENCES = RelationFactory::PROJECT_REFERENCES + RelationFactory::USER_REFERENCES + ['group_id']
+      ALLOWED_REFERENCES = RelationFactory::PROJECT_REFERENCES + RelationFactory::USER_REFERENCES + %w[group_id commit_id]
       PROHIBITED_REFERENCES = Regexp.union(/\Acached_markdown_version\Z/, /_id\Z/, /_html\Z/).freeze
 
       def self.clean(*args)

spec/lib/gitlab/import_export/attribute_cleaner_spec.rb

@@ -22,7 +22,9 @@ describe Gitlab::ImportExport::AttributeCleaner do
       'some_html' => '<p>dodgy html</p>',
       'legit_html' => '<p>legit html</p>',
       '_html' => '<p>perfectly ordinary html</p>',
-      'cached_markdown_version' => 12345
+      'cached_markdown_version' => 12345,
+      'group_id' => 99,
+      'commit_id' => 99
     }
   end
 
@@ -31,7 +33,9 @@ describe Gitlab::ImportExport::AttributeCleaner do
       'project_id' => 99,
       'user_id' => 99,
       'random_id_in_the_middle' => 99,
-      'notid' => 99
+      'notid' => 99,
+      'group_id' => 99,
+      'commit_id' => 99
     }
   end
 
@@ -59,6 +63,6 @@ describe Gitlab::ImportExport::AttributeCleaner do
   it 'does not remove excluded key if not listed' do
     parsed_hash = described_class.clean(relation_hash: unsafe_hash, relation_class: relation_class)
 
-    expect(parsed_hash.keys).to eq post_safe_hash.keys + excluded_keys
+    expect(parsed_hash.keys).to match_array post_safe_hash.keys + excluded_keys
   end
 end

spec/models/merge_request_spec.rb

@@ -2454,6 +2454,13 @@ describe MergeRequest do
   describe "#diff_refs" do
     context "with diffs" do
       subject { create(:merge_request, :with_diffs) }
+      let(:expected_diff_refs) do
+        Gitlab::Diff::DiffRefs.new(
+          base_sha:  subject.merge_request_diff.base_commit_sha,
+          start_sha: subject.merge_request_diff.start_commit_sha,
+          head_sha:  subject.merge_request_diff.head_commit_sha
+        )
+      end
 
       it "does not touch the repository" do
         subject # Instantiate the object
@@ -2464,14 +2471,18 @@ describe MergeRequest do
       end
 
       it "returns expected diff_refs" do
-        expected_diff_refs = Gitlab::Diff::DiffRefs.new(
-          base_sha:  subject.merge_request_diff.base_commit_sha,
-          start_sha: subject.merge_request_diff.start_commit_sha,
-          head_sha:  subject.merge_request_diff.head_commit_sha
-        )
-
         expect(subject.diff_refs).to eq(expected_diff_refs)
       end
+
+      context 'when importing' do
+        before do
+          subject.importing = true
+        end
+
+        it "returns MR diff_refs" do
+          expect(subject.diff_refs).to eq(expected_diff_refs)
+        end
+      end
     end
   end