Add latest changes from gitlab-org/gitlab@master

8 files changed, 367 insertions, 58 deletions

diff --git a/spec/lib/bitbucket/representation/pull_request_comment_spec.rb b/spec/lib/bitbucket/representation/pull_request_comment_spec.rb
index e748cd7b955..6fdd1dfa561 100644
--- a/spec/lib/bitbucket/representation/pull_request_comment_spec.rb
+++ b/spec/lib/bitbucket/representation/pull_request_comment_spec.rb
@@ -2,7 +2,7 @@
 
 require 'fast_spec_helper'
 
-RSpec.describe Bitbucket::Representation::PullRequestComment do
+RSpec.describe Bitbucket::Representation::PullRequestComment, feature_category: :importers do
   describe '#iid' do
     it { expect(described_class.new('id' => 1).iid).to eq(1) }
   end
@@ -33,4 +33,10 @@ RSpec.describe Bitbucket::Representation::PullRequestComment do
     it { expect(described_class.new('parent' => {}).has_parent?).to be_truthy }
     it { expect(described_class.new({}).has_parent?).to be_falsey }
   end
+
+  describe '#deleted?' do
+    it { expect(described_class.new('deleted' => true).deleted?).to be_truthy }
+    it { expect(described_class.new('deleted' => false).deleted?).to be_falsey }
+    it { expect(described_class.new({}).deleted?).to be_falsey }
+  end
 end
diff --git a/spec/lib/gitlab/bitbucket_import/importers/issues_notes_importer_spec.rb b/spec/lib/gitlab/bitbucket_import/importers/issues_notes_importer_spec.rb
index 043cd7f17b9..1466b91fa8b 100644
--- a/spec/lib/gitlab/bitbucket_import/importers/issues_notes_importer_spec.rb
+++ b/spec/lib/gitlab/bitbucket_import/importers/issues_notes_importer_spec.rb
@@ -4,8 +4,6 @@ require 'spec_helper'
 
 RSpec.describe Gitlab::BitbucketImport::Importers::IssuesNotesImporter, feature_category: :importers do
   let_it_be(:project) { create(:project, :import_started) }
-  # let_it_be(:merge_request_1) { create(:merge_request, source_project: project) }
-  # let_it_be(:merge_request_2) { create(:merge_request, source_project: project, source_branch: 'other-branch') }
   let_it_be(:issue_1) { create(:issue, project: project) }
   let_it_be(:issue_2) { create(:issue, project: project) }
 
diff --git a/spec/lib/gitlab/bitbucket_import/importers/pull_request_notes_importer_spec.rb b/spec/lib/gitlab/bitbucket_import/importers/pull_request_notes_importer_spec.rb
index 4a30f225d66..332f6e5bd03 100644
--- a/spec/lib/gitlab/bitbucket_import/importers/pull_request_notes_importer_spec.rb
+++ b/spec/lib/gitlab/bitbucket_import/importers/pull_request_notes_importer_spec.rb
@@ -2,9 +2,9 @@
 
 require 'spec_helper'
 
-RSpec.describe Gitlab::BitbucketImport::Importers::PullRequestNotesImporter, feature_category: :importers do
+RSpec.describe Gitlab::BitbucketImport::Importers::PullRequestNotesImporter, :clean_gitlab_redis_cache, feature_category: :importers do
   let_it_be(:project) do
-    create(:project, :import_started,
+    create(:project, :repository, :import_started,
       import_data_attributes: {
         credentials: { 'base_uri' => 'http://bitbucket.org/', 'user' => 'bitbucket', 'password' => 'password' }
       }
@@ -12,28 +12,216 @@ RSpec.describe Gitlab::BitbucketImport::Importers::PullRequestNotesImporter, fea
   end
 
   let_it_be(:merge_request) { create(:merge_request, source_project: project) }
-
+  let_it_be(:merge_request_diff) { create(:merge_request_diff, :external, merge_request: merge_request) }
+  let_it_be(:bitbucket_user) { create(:user) }
+  let_it_be(:identity) { create(:identity, user: bitbucket_user, extern_uid: 'bitbucket_user', provider: :bitbucket) }
   let(:hash) { { iid: merge_request.iid } }
-  let(:importer_helper) { Gitlab::BitbucketImport::Importer.new(project) }
+  let(:client) { Bitbucket::Client.new({}) }
+  let(:ref_converter) { Gitlab::BitbucketImport::RefConverter.new(project) }
+  let(:user_finder) { Gitlab::BitbucketImport::UserFinder.new(project) }
+  let(:note_body) { 'body' }
+  let(:comments) { [Bitbucket::Representation::PullRequestComment.new(note_hash)] }
+  let(:created_at) { Date.today - 2.days }
+  let(:updated_at) { Date.today }
+  let(:note_hash) do
+    {
+      'id' => 12,
+      'user' => { 'nickname' => 'bitbucket_user' },
+      'content' => { 'raw' => note_body },
+      'created_on' => created_at,
+      'updated_on' => updated_at
+    }
+  end
 
   subject(:importer) { described_class.new(project, hash) }
 
   before do
-    allow(Gitlab::BitbucketImport::Importer).to receive(:new).and_return(importer_helper)
+    allow(Bitbucket::Client).to receive(:new).and_return(client)
+    allow(Gitlab::BitbucketImport::RefConverter).to receive(:new).and_return(ref_converter)
+    allow(Gitlab::BitbucketImport::UserFinder).to receive(:new).and_return(user_finder)
+    allow(client).to receive(:pull_request_comments).and_return(comments)
   end
 
   describe '#execute' do
-    it 'calls Importer.import_pull_request_comments' do
-      expect(importer_helper).to receive(:import_pull_request_comments).once
+    context 'for standalone pr comments' do
+      it 'calls RefConverter' do
+        expect(ref_converter).to receive(:convert_note).once.and_call_original
+
+        importer.execute
+      end
+
+      it 'creates a note with the correct attributes' do
+        expect { importer.execute }.to change { merge_request.notes.count }.from(0).to(1)
+
+        note = merge_request.notes.first
+
+        expect(note.note).to eq(note_body)
+        expect(note.author).to eq(bitbucket_user)
+        expect(note.created_at).to eq(created_at)
+        expect(note.updated_at).to eq(updated_at)
+      end
+
+      context 'when the author does not have a bitbucket identity' do
+        before do
+          identity.update!(provider: :github)
+        end
+
+        it 'sets the author to the project creator and adds the author to the note' do
+          importer.execute
+
+          note = merge_request.notes.first
+
+          expect(note.author).to eq(project.creator)
+          expect(note.note).to eq("*Created by: bitbucket_user*\n\nbody")
+        end
+      end
+
+      context 'when the note is deleted' do
+        let(:note_hash) do
+          {
+            'id' => 12,
+            'user' => { 'nickname' => 'bitbucket_user' },
+            'content' => { 'raw' => note_body },
+            'deleted' => true,
+            'created_on' => created_at,
+            'updated_on' => updated_at
+          }
+        end
+
+        it 'does not create a note' do
+          expect { importer.execute }.not_to change { merge_request.notes.count }
+        end
+      end
+    end
+
+    context 'for threaded inline comments' do
+      let(:path) { project.repository.commit.raw_diffs.first.new_path }
+      let(:reply_body) { 'Some reply' }
+      let(:comments) do
+        [
+          Bitbucket::Representation::PullRequestComment.new(pr_comment_1),
+          Bitbucket::Representation::PullRequestComment.new(pr_comment_2)
+        ]
+      end
 
-      importer.execute
+      let(:pr_comment_1) do
+        {
+          'id' => 14,
+          'inline' => {
+            'path' => path,
+            'from' => nil,
+            'to' => 1
+          },
+          'parent' => { 'id' => 13 },
+          'user' => { 'nickname' => 'bitbucket_user' },
+          'content' => { 'raw' => reply_body },
+          'created_on' => created_at,
+          'updated_on' => updated_at
+        }
+      end
+
+      let(:pr_comment_2) do
+        {
+          'id' => 13,
+          'inline' => {
+            'path' => path,
+            'from' => nil,
+            'to' => 1
+          },
+          'user' => { 'nickname' => 'non_existent_user' },
+          'content' => { 'raw' => note_body },
+          'created_on' => created_at,
+          'updated_on' => updated_at
+        }
+      end
+
+      it 'creates notes in the correct position with the right attributes' do
+        expect { importer.execute }.to change { merge_request.notes.count }.from(0).to(2)
+
+        expect(merge_request.notes.map(&:discussion_id).uniq.count).to eq(1)
+
+        notes = merge_request.notes.order(:id).to_a
+
+        start_note = notes.first
+        expect(start_note).to be_a(DiffNote)
+        expect(start_note.note).to eq("*Created by: non_existent_user*\n\n#{note_body}")
+        expect(start_note.author).to eq(project.creator)
+
+        reply_note = notes.last
+        expect(reply_note).to be_a(DiffNote)
+        expect(reply_note.note).to eq(reply_body)
+        expect(reply_note.author).to eq(bitbucket_user)
+      end
+
+      context 'when the comments are not part of the diff' do
+        let(:pr_comment_1) do
+          {
+            'id' => 14,
+            'inline' => {
+              'path' => path,
+              'from' => nil,
+              'to' => nil
+            },
+            'parent' => { 'id' => 13 },
+            'user' => { 'nickname' => 'bitbucket_user' },
+            'content' => { 'raw' => reply_body },
+            'created_on' => created_at,
+            'updated_on' => updated_at
+          }
+        end
+
+        let(:pr_comment_2) do
+          {
+            'id' => 13,
+            'inline' => {
+              'path' => path,
+              'from' => nil,
+              'to' => nil
+            },
+            'user' => { 'nickname' => 'bitbucket_user' },
+            'content' => { 'raw' => note_body },
+            'created_on' => created_at,
+            'updated_on' => updated_at
+          }
+        end
+
+        it 'creates them as normal notes' do
+          expect { importer.execute }.to change { merge_request.notes.count }.from(0).to(2)
+
+          notes = merge_request.notes.order(:id).to_a
+
+          first_note = notes.first
+          expect(first_note).not_to be_a(DiffNote)
+          expect(first_note.note).to eq("*Comment on*\n\n#{note_body}")
+          expect(first_note.author).to eq(bitbucket_user)
+
+          second_note = notes.last
+          expect(second_note).not_to be_a(DiffNote)
+          expect(second_note.note).to eq("*Comment on*\n\n#{reply_body}")
+          expect(second_note.author).to eq(bitbucket_user)
+        end
+      end
+
+      context 'when an error is raised for one note' do
+        before do
+          allow(user_finder).to receive(:gitlab_user_id).and_call_original
+          allow(user_finder).to receive(:gitlab_user_id).with(project, 'bitbucket_user').and_raise(StandardError)
+        end
+
+        it 'tracks the error and continues to import other notes' do
+          expect(Gitlab::ErrorTracking).to receive(:log_exception)
+            .with(anything, hash_including(comment_id: 14)).and_call_original
+
+          expect { importer.execute }.to change { merge_request.notes.count }.from(0).to(1)
+        end
+      end
     end
 
     context 'when the merge request does not exist' do
       let(:hash) { { iid: 'nonexistent' } }
 
-      it 'does not call Importer.import_pull_request_comments' do
-        expect(importer_helper).not_to receive(:import_pull_request_comments)
+      it 'does not call #import_pull_request_comments' do
+        expect(importer).not_to receive(:import_pull_request_comments)
 
         importer.execute
       end
@@ -46,8 +234,8 @@ RSpec.describe Gitlab::BitbucketImport::Importers::PullRequestNotesImporter, fea
         merge_request.update!(source_project: another_project, target_project: another_project)
       end
 
-      it 'does not call Importer.import_pull_request_comments' do
-        expect(importer_helper).not_to receive(:import_pull_request_comments)
+      it 'does not call #import_pull_request_comments' do
+        expect(importer).not_to receive(:import_pull_request_comments)
 
         importer.execute
       end
@@ -55,7 +243,7 @@ RSpec.describe Gitlab::BitbucketImport::Importers::PullRequestNotesImporter, fea
 
     context 'when an error is raised' do
       before do
-        allow(importer_helper).to receive(:import_pull_request_comments).and_raise(StandardError)
+        allow(importer).to receive(:import_pull_request_comments).and_raise(StandardError)
       end
 
       it 'tracks the failure and does not fail' do
diff --git a/spec/lib/gitlab/ci/build/context/build_spec.rb b/spec/lib/gitlab/ci/build/context/build_spec.rb
index fae02e140f2..9fdb4ee9393 100644
--- a/spec/lib/gitlab/ci/build/context/build_spec.rb
+++ b/spec/lib/gitlab/ci/build/context/build_spec.rb
@@ -41,16 +41,6 @@ RSpec.describe Gitlab::Ci::Build::Context::Build, feature_category: :pipeline_co
     it { expect(context.variables).to be_instance_of(Gitlab::Ci::Variables::Collection) }
 
     it_behaves_like 'variables collection'
-
-    context 'with FF disabled' do
-      before do
-        stub_feature_flags(reduced_build_attributes_list_for_rules: false)
-      end
-
-      it { expect(context.variables).to be_instance_of(Gitlab::Ci::Variables::Collection) }
-
-      it_behaves_like 'variables collection'
-    end
   end
 
   describe '#variables_hash' do
@@ -59,15 +49,5 @@ RSpec.describe Gitlab::Ci::Build::Context::Build, feature_category: :pipeline_co
     it { expect(context.variables_hash).to be_instance_of(ActiveSupport::HashWithIndifferentAccess) }
 
     it_behaves_like 'variables collection'
-
-    context 'with FF disabled' do
-      before do
-        stub_feature_flags(reduced_build_attributes_list_for_rules: false)
-      end
-
-      it { expect(context.variables_hash).to be_instance_of(ActiveSupport::HashWithIndifferentAccess) }
-
-      it_behaves_like 'variables collection'
-    end
   end
 end
diff --git a/spec/migrations/20230929155123_migrate_disable_merge_trains_value_spec.rb b/spec/migrations/20230929155123_migrate_disable_merge_trains_value_spec.rb
new file mode 100644
index 00000000000..ee011687bbb
--- /dev/null
+++ b/spec/migrations/20230929155123_migrate_disable_merge_trains_value_spec.rb
@@ -0,0 +1,83 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+require_migration! 'migrate_disable_merge_trains_value'
+
+RSpec.describe MigrateDisableMergeTrainsValue, schema: 20230929155123, feature_category: :continuous_integration do
+  let!(:feature_gates) { table(:feature_gates) }
+  let!(:projects) { table(:projects) }
+  let!(:project_ci_cd_settings) { table(:project_ci_cd_settings) }
+  let!(:namespace1) { table(:namespaces).create!(name: 'name1', path: 'path1') }
+  let!(:namespace2) { table(:namespaces).create!(name: 'name2', path: 'path2') }
+
+  let!(:project_with_flag_on) do
+    projects
+      .create!(
+        name: "project",
+        path: "project",
+        namespace_id: namespace1.id,
+        project_namespace_id: namespace1.id
+      )
+  end
+
+  let!(:project_with_flag_off) do
+    projects
+      .create!(
+        name: "project2",
+        path: "project2",
+        namespace_id: namespace2.id,
+        project_namespace_id: namespace2.id
+      )
+  end
+
+  let!(:settings_flag_on) do
+    project_ci_cd_settings.create!(
+      merge_trains_enabled: true,
+      project_id: project_with_flag_on.id
+    )
+  end
+
+  let!(:settings_flag_off) do
+    project_ci_cd_settings.create!(
+      merge_trains_enabled: true,
+      project_id: project_with_flag_off.id
+    )
+  end
+
+  let!(:migration) { described_class.new }
+
+  before do
+    # Enable the feature flag
+    feature_gates.create!(
+      feature_key: 'disable_merge_trains',
+      key: 'actors',
+      value: "Project:#{project_with_flag_on.id}"
+    )
+
+    migration.up
+  end
+
+  describe '#up' do
+    it 'migrates the flag value into the setting value' do
+      expect(
+        settings_flag_on.reload.merge_trains_enabled
+      ).to eq(false)
+      expect(
+        settings_flag_off.reload.merge_trains_enabled
+      ).to eq(true)
+    end
+  end
+
+  describe '#down' do
+    it 'reverts the migration' do
+      migration.down
+
+      expect(
+        settings_flag_on.reload.merge_trains_enabled
+      ).to eq(true)
+      expect(
+        settings_flag_off.reload.merge_trains_enabled
+      ).to eq(true)
+    end
+  end
+end
diff --git a/spec/models/ci/catalog/resource_spec.rb b/spec/models/ci/catalog/resource_spec.rb
index 4ce1433e015..f9723385111 100644
--- a/spec/models/ci/catalog/resource_spec.rb
+++ b/spec/models/ci/catalog/resource_spec.rb
@@ -10,7 +10,7 @@ RSpec.describe Ci::Catalog::Resource, feature_category: :pipeline_composition do
   let_it_be(:project) { create(:project, name: 'A') }
   let_it_be(:project_2) { build(:project, name: 'Z') }
   let_it_be(:project_3) { build(:project, name: 'L') }
-  let_it_be(:resource) { create(:ci_catalog_resource, project: project, latest_released_at: tomorrow) }
+  let_it_be_with_reload(:resource) { create(:ci_catalog_resource, project: project, latest_released_at: tomorrow) }
   let_it_be(:resource_2) { create(:ci_catalog_resource, project: project_2, latest_released_at: today) }
   let_it_be(:resource_3) { create(:ci_catalog_resource, project: project_3, latest_released_at: nil) }
 
@@ -95,4 +95,27 @@ RSpec.describe Ci::Catalog::Resource, feature_category: :pipeline_composition do
       expect(resource.state).to eq('draft')
     end
   end
+
+  describe '#unpublish!' do
+    context 'when the catalog resource is in published state' do
+      it 'updates the state to draft' do
+        resource.update!(state: :published)
+        expect(resource.state).to eq('published')
+
+        resource.unpublish!
+
+        expect(resource.reload.state).to eq('draft')
+      end
+    end
+
+    context 'when the catalog resource is already in draft state' do
+      it 'leaves the state as draft' do
+        expect(resource.state).to eq('draft')
+
+        resource.unpublish!
+
+        expect(resource.reload.state).to eq('draft')
+      end
+    end
+  end
 end
diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb
index 887ec48ec8f..0094f56d96e 100644
--- a/spec/models/ci/pipeline_spec.rb
+++ b/spec/models/ci/pipeline_spec.rb
@@ -5578,25 +5578,4 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep, feature_category:
       end
     end
   end
-
-  describe '#reduced_build_attributes_list_for_rules?' do
-    subject { pipeline.reduced_build_attributes_list_for_rules? }
-
-    let(:pipeline) { build_stubbed(:ci_pipeline, project: project, user: user) }
-
-    it { is_expected.to be_truthy }
-
-    it 'memoizes the result' do
-      expect { subject }
-        .to change { pipeline.strong_memoized?(:reduced_build_attributes_list_for_rules?) }
-    end
-
-    context 'with the FF disabled' do
-      before do
-        stub_feature_flags(reduced_build_attributes_list_for_rules: false)
-      end
-
-      it { is_expected.to be_falsey }
-    end
-  end
 end
diff --git a/spec/requests/api/graphql/mutations/ci/catalog/unpublish_spec.rb b/spec/requests/api/graphql/mutations/ci/catalog/unpublish_spec.rb
new file mode 100644
index 00000000000..07465777263
--- /dev/null
+++ b/spec/requests/api/graphql/mutations/ci/catalog/unpublish_spec.rb
@@ -0,0 +1,52 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe 'CatalogResourceUnpublish', feature_category: :pipeline_composition do
+  include GraphqlHelpers
+
+  let_it_be(:current_user) { create(:user) }
+  let_it_be_with_reload(:resource) { create(:ci_catalog_resource) }
+
+  let(:mutation) do
+    graphql_mutation(
+      :catalog_resource_unpublish,
+      id: resource.to_gid.to_s
+    )
+  end
+
+  subject(:post_query) { post_graphql_mutation(mutation, current_user: current_user) }
+
+  context 'when unauthorized' do
+    it_behaves_like 'a mutation that returns a top-level access error'
+  end
+
+  context 'when authorized' do
+    before_all do
+      resource.project.add_owner(current_user)
+    end
+
+    context 'when the catalog resource is in published state' do
+      it 'updates the state to draft' do
+        resource.update!(state: :published)
+        expect(resource.state).to eq('published')
+
+        post_query
+
+        expect(resource.reload.state).to eq('draft')
+        expect_graphql_errors_to_be_empty
+      end
+    end
+
+    context 'when the catalog resource is already in draft state' do
+      it 'leaves the state as draft' do
+        expect(resource.state).to eq('draft')
+
+        post_query
+
+        expect(resource.reload.state).to eq('draft')
+        expect_graphql_errors_to_be_empty
+      end
+    end
+  end
+end