Add latest changes from gitlab-org/gitlab@14-8-stable-eev14.8.0-rc42

4 files changed, 106 insertions, 108 deletions

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 a0e78186caa..c8e744ab262 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
@@ -119,123 +119,80 @@ RSpec.describe Gitlab::GithubImport::Importer::DiffNoteImporter, :aggregate_fail
           .and_return(discussion_id)
       end
 
-      context 'when github_importer_use_diff_note_with_suggestions is disabled' do
-        before do
-          stub_feature_flags(github_importer_use_diff_note_with_suggestions: false)
+      it_behaves_like 'diff notes without suggestion'
+
+      context 'when the note has suggestions' do
+        let(:note_body) do
+          <<~EOB
+          Suggestion:
+          ```suggestion
+          what do you think to do it like this
+          ```
+          EOB
         end
 
-        it_behaves_like 'diff notes without suggestion'
+        before do
+          stub_user_finder(user.id, true)
+        end
 
-        context 'when the note has suggestions' do
-          let(:note_body) do
-            <<~EOB
+        it 'imports the note as diff note' do
+          expect { subject.execute }
+            .to change(DiffNote, :count)
+            .by(1)
+
+          note = project.notes.diff_notes.take
+          expect(note).to be_valid
+          expect(note.noteable_type).to eq('MergeRequest')
+          expect(note.noteable_id).to eq(merge_request.id)
+          expect(note.project_id).to eq(project.id)
+          expect(note.author_id).to eq(user.id)
+          expect(note.system).to eq(false)
+          expect(note.discussion_id).to eq(discussion_id)
+          expect(note.commit_id).to eq('original123abc')
+          expect(note.line_code).to eq(note_representation.line_code)
+          expect(note.type).to eq('DiffNote')
+          expect(note.created_at).to eq(created_at)
+          expect(note.updated_at).to eq(updated_at)
+          expect(note.position.to_h).to eq({
+            base_sha: merge_request.diffs.diff_refs.base_sha,
+            head_sha: merge_request.diffs.diff_refs.head_sha,
+            start_sha: merge_request.diffs.diff_refs.start_sha,
+            new_line: 15,
+            old_line: nil,
+            new_path: file_path,
+            old_path: file_path,
+            position_type: 'text',
+            line_range: nil
+          })
+          expect(note.note)
+            .to eq <<~NOTE
             Suggestion:
-            ```suggestion
+            ```suggestion:-0+0
             what do you think to do it like this
             ```
-            EOB
-          end
-
-          it 'imports the note' do
-            stub_user_finder(user.id, true)
-
-            expect { subject.execute }
-              .to change(LegacyDiffNote, :count)
-              .and not_change(DiffNote, :count)
-
-            note = project.notes.diff_notes.take
-            expect(note).to be_valid
-            expect(note.note)
-              .to eq <<~NOTE
-              Suggestion:
-              ```suggestion:-0+0
-              what do you think to do it like this
-              ```
-              NOTE
-          end
-        end
-      end
-
-      context 'when github_importer_use_diff_note_with_suggestions is enabled' do
-        before do
-          stub_feature_flags(github_importer_use_diff_note_with_suggestions: true)
+            NOTE
         end
 
-        it_behaves_like 'diff notes without suggestion'
+        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')
 
-        context 'when the note has suggestions' do
-          let(:note_body) do
-            <<~EOB
-            Suggestion:
-            ```suggestion
-            what do you think to do it like this
-            ```
-            EOB
-          end
+            expect_next_instance_of(::Import::Github::Notes::CreateService) do |service|
+              expect(service)
+                .to receive(:execute)
+                .and_raise(exception)
+            end
 
-          before do
-            stub_user_finder(user.id, true)
-          end
+            expect(Gitlab::GithubImport::Logger)
+              .to receive(:warn)
+              .with(
+                message: 'Failed to create diff note file',
+                'error.class': 'DiffNote::NoteDiffFileCreationError'
+              )
 
-          it 'imports the note as diff note' do
             expect { subject.execute }
-              .to change(DiffNote, :count)
-              .by(1)
-
-            note = project.notes.diff_notes.take
-            expect(note).to be_valid
-            expect(note.noteable_type).to eq('MergeRequest')
-            expect(note.noteable_id).to eq(merge_request.id)
-            expect(note.project_id).to eq(project.id)
-            expect(note.author_id).to eq(user.id)
-            expect(note.system).to eq(false)
-            expect(note.discussion_id).to eq(discussion_id)
-            expect(note.commit_id).to eq('original123abc')
-            expect(note.line_code).to eq(note_representation.line_code)
-            expect(note.type).to eq('DiffNote')
-            expect(note.created_at).to eq(created_at)
-            expect(note.updated_at).to eq(updated_at)
-            expect(note.position.to_h).to eq({
-              base_sha: merge_request.diffs.diff_refs.base_sha,
-              head_sha: merge_request.diffs.diff_refs.head_sha,
-              start_sha: merge_request.diffs.diff_refs.start_sha,
-              new_line: 15,
-              old_line: nil,
-              new_path: file_path,
-              old_path: file_path,
-              position_type: 'text',
-              line_range: nil
-            })
-            expect(note.note)
-              .to eq <<~NOTE
-              Suggestion:
-              ```suggestion:-0+0
-              what do you think to do it like this
-              ```
-              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
+              .to change(LegacyDiffNote, :count)
+              .and not_change(DiffNote, :count)
           end
         end
       end
diff --git a/spec/lib/gitlab/github_import/importer/releases_importer_spec.rb b/spec/lib/gitlab/github_import/importer/releases_importer_spec.rb
index 1a25824bc8a..6b3d18f20e9 100644
--- a/spec/lib/gitlab/github_import/importer/releases_importer_spec.rb
+++ b/spec/lib/gitlab/github_import/importer/releases_importer_spec.rb
@@ -52,6 +52,12 @@ RSpec.describe Gitlab::GithubImport::Importer::ReleasesImporter do
 
       expect { importer.execute }.to change { Release.count }.by(1)
     end
+
+    it 'is idempotent' do
+      allow(importer).to receive(:each_release).and_return([github_release])
+      expect { importer.execute }.to change { Release.count }.by(1)
+      expect { importer.execute }.to change { Release.count }.by(0) # Idempotency check
+    end
   end
 
   describe '#build_releases' do
@@ -79,6 +85,24 @@ RSpec.describe Gitlab::GithubImport::Importer::ReleasesImporter do
 
       expect(release[:description]).to eq('Release for tag 1.0')
     end
+
+    it 'does not create releases that have a NULL tag' do
+      null_tag_release = double(
+        name: 'NULL Test',
+        tag_name: nil
+      )
+
+      expect(importer).to receive(:each_release).and_return([null_tag_release])
+      expect(importer.build_releases).to be_empty
+    end
+
+    it 'does not create duplicate release tags' do
+      expect(importer).to receive(:each_release).and_return([github_release, github_release])
+
+      releases = importer.build_releases
+      expect(releases.length).to eq(1)
+      expect(releases[0][:description]).to eq('This is my release')
+    end
   end
 
   describe '#build' do
diff --git a/spec/lib/gitlab/github_import/importer/repository_importer_spec.rb b/spec/lib/gitlab/github_import/importer/repository_importer_spec.rb
index 58a8fb1b7e4..f2730ba74ec 100644
--- a/spec/lib/gitlab/github_import/importer/repository_importer_spec.rb
+++ b/spec/lib/gitlab/github_import/importer/repository_importer_spec.rb
@@ -264,8 +264,8 @@ RSpec.describe Gitlab::GithubImport::Importer::RepositoryImporter do
     it 'sets the timestamp for when the cloning process finished' do
       freeze_time do
         expect(project)
-          .to receive(:update_column)
-          .with(:last_repository_updated_at, Time.zone.now)
+          .to receive(:touch)
+          .with(:last_repository_updated_at)
 
         importer.update_clone_time
       end
diff --git a/spec/lib/gitlab/github_import/representation/diff_note_spec.rb b/spec/lib/gitlab/github_import/representation/diff_note_spec.rb
index 63834cfdb94..fe3040c102b 100644
--- a/spec/lib/gitlab/github_import/representation/diff_note_spec.rb
+++ b/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