Add latest changes from gitlab-org/gitlab@master

16 files changed, 147 insertions, 90 deletions

diff --git a/spec/lib/banzai/reference_parser/external_issue_parser_spec.rb b/spec/lib/banzai/reference_parser/external_issue_parser_spec.rb
index 5f92eb42e74..0c1b98e5ec3 100644
--- a/spec/lib/banzai/reference_parser/external_issue_parser_spec.rb
+++ b/spec/lib/banzai/reference_parser/external_issue_parser_spec.rb
@@ -21,7 +21,7 @@ RSpec.describe Banzai::ReferenceParser::ExternalIssueParser do
 
       levels.each do |level|
         it "creates reference when the feature is #{level}" do
-          project.project_feature.update(issues_access_level: level)
+          project.project_feature.update!(issues_access_level: level)
 
           visible_nodes = subject.nodes_visible_to_user(user, [link])
 
diff --git a/spec/lib/banzai/reference_redactor_spec.rb b/spec/lib/banzai/reference_redactor_spec.rb
index 668e427cfa2..78cceedd0e5 100644
--- a/spec/lib/banzai/reference_redactor_spec.rb
+++ b/spec/lib/banzai/reference_redactor_spec.rb
@@ -64,7 +64,7 @@ RSpec.describe Banzai::ReferenceRedactor do
       let(:redactor) { described_class.new(Banzai::RenderContext.new(project, user)) }
 
       before do
-        project.update(pending_delete: true)
+        project.update!(pending_delete: true)
       end
 
       it 'redacts an issue attached' do
diff --git a/spec/lib/gitlab/analytics/cycle_analytics/stage_events/code_stage_start_spec.rb b/spec/lib/gitlab/analytics/cycle_analytics/stage_events/code_stage_start_spec.rb
index 52e9f2d9846..b6f9c8106c9 100644
--- a/spec/lib/gitlab/analytics/cycle_analytics/stage_events/code_stage_start_spec.rb
+++ b/spec/lib/gitlab/analytics/cycle_analytics/stage_events/code_stage_start_spec.rb
@@ -15,7 +15,7 @@ RSpec.describe Gitlab::Analytics::CycleAnalytics::StageEvents::CodeStageStart do
 
     other_merge_request = create(:merge_request, source_project: project, source_branch: 'a', target_branch: 'master')
 
-    records = subject.apply_query_customization(MergeRequest.all).where('merge_requests_closing_issues.issue_id IS NOT NULL')
+    records = subject.apply_query_customization(MergeRequest.all).where.not('merge_requests_closing_issues.issue_id' => nil)
     expect(records).to eq([merge_request])
     expect(records).not_to include(other_merge_request)
   end
diff --git a/spec/lib/gitlab/usage/metric_definition_spec.rb b/spec/lib/gitlab/usage/metric_definition_spec.rb
index 8b592838f5d..f98cc4cbcb7 100644
--- a/spec/lib/gitlab/usage/metric_definition_spec.rb
+++ b/spec/lib/gitlab/usage/metric_definition_spec.rb
@@ -16,7 +16,8 @@ RSpec.describe Gitlab::Usage::MetricDefinition do
       time_frame: 'none',
       data_source: 'database',
       distribution: %w(ee ce),
-      tier: %w(free starter premium ultimate bronze silver gold)
+      tier: %w(free starter premium ultimate bronze silver gold),
+      name: 'count_boards'
     }
   end
 
@@ -53,6 +54,7 @@ RSpec.describe Gitlab::Usage::MetricDefinition do
       :distribution       | nil
       :distribution       | 'test'
       :tier               | %w(test ee)
+      :name               | 'count_<adjective_describing>_boards'
     end
 
     with_them do
diff --git a/spec/migrations/clean_up_noteable_id_for_notes_on_commits_spec.rb b/spec/migrations/clean_up_noteable_id_for_notes_on_commits_spec.rb
index 531c1dbb76a..268fadee0af 100644
--- a/spec/migrations/clean_up_noteable_id_for_notes_on_commits_spec.rb
+++ b/spec/migrations/clean_up_noteable_id_for_notes_on_commits_spec.rb
@@ -25,7 +25,7 @@ RSpec.describe CleanUpNoteableIdForNotesOnCommits do
   end
 
   def dirty_notes_on_commits
-    notes.where(noteable_type: 'Commit').where('noteable_id IS NOT NULL')
+    notes.where(noteable_type: 'Commit').where.not(noteable_id: nil)
   end
 
   def other_notes
diff --git a/spec/migrations/migrate_bot_type_to_user_type_spec.rb b/spec/migrations/migrate_bot_type_to_user_type_spec.rb
index 2b85f2a7f69..fcd7f1ebcb8 100644
--- a/spec/migrations/migrate_bot_type_to_user_type_spec.rb
+++ b/spec/migrations/migrate_bot_type_to_user_type_spec.rb
@@ -15,6 +15,6 @@ RSpec.describe MigrateBotTypeToUserType, :migration do
 
     migrate!
 
-    expect(users.where('user_type IS NOT NULL').map(&:user_type)).to match_array([1, 2, 3])
+    expect(users.where.not(user_type: nil).map(&:user_type)).to match_array([1, 2, 3])
   end
 end
diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb
index aa3cc0800a8..fa0e72e7ad4 100644
--- a/spec/models/ci/build_spec.rb
+++ b/spec/models/ci/build_spec.rb
@@ -3582,10 +3582,10 @@ RSpec.describe Ci::Build do
   end
 
   describe 'state transition when build fails' do
-    let(:service) { MergeRequests::AddTodoWhenBuildFailsService.new(project, user) }
+    let(:service) { ::MergeRequests::AddTodoWhenBuildFailsService.new(project, user) }
 
     before do
-      allow(MergeRequests::AddTodoWhenBuildFailsService).to receive(:new).and_return(service)
+      allow(::MergeRequests::AddTodoWhenBuildFailsService).to receive(:new).and_return(service)
       allow(service).to receive(:close)
     end
 
@@ -3670,15 +3670,42 @@ RSpec.describe Ci::Build do
         subject.drop!
       end
 
-      it 'creates a todo' do
-        project.add_developer(user)
+      context 'when async_add_build_failure_todo flag enabled' do
+        it 'creates a todo async', :sidekiq_inline do
+          project.add_developer(user)
+
+          expect_next_instance_of(TodoService) do |todo_service|
+            expect(todo_service)
+              .to receive(:merge_request_build_failed).with(merge_request)
+          end
 
-        expect_next_instance_of(TodoService) do |todo_service|
-          expect(todo_service)
-            .to receive(:merge_request_build_failed).with(merge_request)
+          subject.drop!
         end
 
-        subject.drop!
+        it 'does not create a sync todo' do
+          project.add_developer(user)
+
+          expect(TodoService).not_to receive(:new)
+
+          subject.drop!
+        end
+      end
+
+      context 'when async_add_build_failure_todo flag disabled' do
+        before do
+          stub_feature_flags(async_add_build_failure_todo: false)
+        end
+
+        it 'creates a todo sync' do
+          project.add_developer(user)
+
+          expect_next_instance_of(TodoService) do |todo_service|
+            expect(todo_service)
+              .to receive(:merge_request_build_failed).with(merge_request)
+          end
+
+          subject.drop!
+        end
       end
     end
 
diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb
index 9498aa75289..ac92311e132 100644
--- a/spec/models/user_spec.rb
+++ b/spec/models/user_spec.rb
@@ -1790,28 +1790,14 @@ RSpec.describe User do
 
     context 'when user has running CI pipelines' do
       let(:service) { double }
+      let(:pipelines) { build_list(:ci_pipeline, 3, :running) }
 
-      context 'with abort_user_pipelines_on_block feature enabled' do
-        let(:pipelines) { build_list(:ci_pipeline, 3, :running) }
+      it 'aborts all running pipelines and related jobs' do
+        expect(user).to receive(:pipelines).and_return(pipelines)
+        expect(Ci::AbortPipelinesService).to receive(:new).and_return(service)
+        expect(service).to receive(:execute).with(pipelines)
 
-        it 'aborts all running pipelines and related jobs' do
-          stub_feature_flags(abort_user_pipelines_on_block: true)
-          expect(user).to receive(:pipelines).and_return(pipelines)
-          expect(Ci::AbortPipelinesService).to receive(:new).and_return(service)
-          expect(service).to receive(:execute).with(pipelines)
-
-          user.block
-        end
-      end
-
-      context 'with abort_user_pipelines_on_block feature disabled' do
-        it 'cancels all running pipelines and related jobs' do
-          stub_feature_flags(abort_user_pipelines_on_block: false)
-          expect(Ci::CancelUserPipelinesService).to receive(:new).and_return(service)
-          expect(service).to receive(:execute).with(user)
-
-          user.block
-        end
+        user.block
       end
     end
 
diff --git a/spec/serializers/service_event_entity_spec.rb b/spec/serializers/service_event_entity_spec.rb
index 09bb8bca43b..34ffd93ea0c 100644
--- a/spec/serializers/service_event_entity_spec.rb
+++ b/spec/serializers/service_event_entity_spec.rb
@@ -17,7 +17,7 @@ RSpec.describe ServiceEventEntity do
       let(:event) { 'push' }
 
       it 'exposes correct attributes' do
-        expect(subject[:description]).to eq('Event will be triggered by a push to the repository')
+        expect(subject[:description]).to eq('Event triggered when someone pushes to the repository.')
         expect(subject[:name]).to eq('push_events')
         expect(subject[:title]).to eq('push')
         expect(subject[:value]).to be(true)
@@ -29,7 +29,7 @@ RSpec.describe ServiceEventEntity do
       let(:event) { 'note' }
 
       it 'exposes correct attributes' do
-        expect(subject[:description]).to eq('Event will be triggered when someone adds a comment')
+        expect(subject[:description]).to eq('Event triggered when someone adds a comment.')
         expect(subject[:name]).to eq('note_events')
         expect(subject[:title]).to eq('note')
         expect(subject[:value]).to eq(false)
diff --git a/spec/services/ci/cancel_user_pipelines_service_spec.rb b/spec/services/ci/cancel_user_pipelines_service_spec.rb
deleted file mode 100644
index 8491242dfd5..00000000000
--- a/spec/services/ci/cancel_user_pipelines_service_spec.rb
+++ /dev/null
@@ -1,35 +0,0 @@
-# frozen_string_literal: true
-
-require 'spec_helper'
-
-RSpec.describe Ci::CancelUserPipelinesService do
-  describe '#execute' do
-    let(:user) { create(:user) }
-
-    subject { described_class.new.execute(user) }
-
-    context 'when user has running CI pipelines' do
-      let(:pipeline) { create(:ci_pipeline, :running, user: user) }
-      let!(:build) { create(:ci_build, :running, pipeline: pipeline) }
-
-      it 'cancels all running pipelines and related jobs', :sidekiq_might_not_need_inline do
-        subject
-
-        expect(pipeline.reload).to be_canceled
-        expect(build.reload).to be_canceled
-      end
-    end
-
-    context 'when an error ocurrs' do
-      it 'raises a service level error' do
-        service = double(execute: ServiceResponse.error(message: 'Error canceling pipeline'))
-        allow(::Ci::CancelUserPipelinesService).to receive(:new).and_return(service)
-
-        result = subject
-
-        expect(result).to be_a(ServiceResponse)
-        expect(result).to be_error
-      end
-    end
-  end
-end
diff --git a/spec/services/ci/retry_build_service_spec.rb b/spec/services/ci/retry_build_service_spec.rb
index bdf60bb3fdc..7dd3d963e56 100644
--- a/spec/services/ci/retry_build_service_spec.rb
+++ b/spec/services/ci/retry_build_service_spec.rb
@@ -181,7 +181,7 @@ RSpec.describe Ci::RetryBuildService do
       end
 
       it 'resolves todos for old build that failed' do
-        expect(MergeRequests::AddTodoWhenBuildFailsService)
+        expect(::MergeRequests::AddTodoWhenBuildFailsService)
           .to receive_message_chain(:new, :close)
 
         service.execute(build)
diff --git a/spec/services/ci/retry_pipeline_service_spec.rb b/spec/services/ci/retry_pipeline_service_spec.rb
index 3c6a99efbf8..3e2e9f07723 100644
--- a/spec/services/ci/retry_pipeline_service_spec.rb
+++ b/spec/services/ci/retry_pipeline_service_spec.rb
@@ -272,7 +272,7 @@ RSpec.describe Ci::RetryPipelineService, '#execute' do
     end
 
     it 'closes all todos about failed jobs for pipeline' do
-      expect(MergeRequests::AddTodoWhenBuildFailsService)
+      expect(::MergeRequests::AddTodoWhenBuildFailsService)
         .to receive_message_chain(:new, :close_all)
 
       service.execute(pipeline)
diff --git a/spec/services/merge_requests/add_todo_when_build_fails_service_spec.rb b/spec/services/merge_requests/add_todo_when_build_fails_service_spec.rb
index 3c81ad6722d..6edaa91b8b2 100644
--- a/spec/services/merge_requests/add_todo_when_build_fails_service_spec.rb
+++ b/spec/services/merge_requests/add_todo_when_build_fails_service_spec.rb
@@ -2,7 +2,7 @@
 
 require 'spec_helper'
 
-RSpec.describe MergeRequests::AddTodoWhenBuildFailsService do
+RSpec.describe ::MergeRequests::AddTodoWhenBuildFailsService do
   let(:user) { create(:user) }
   let(:project) { create(:project, :repository) }
   let(:sha) { '1234567890abcdef1234567890abcdef12345678' }
@@ -24,8 +24,8 @@ RSpec.describe MergeRequests::AddTodoWhenBuildFailsService do
 
   before do
     allow_any_instance_of(MergeRequest)
-      .to receive(:head_pipeline)
-      .and_return(pipeline)
+      .to receive(:head_pipeline_id)
+      .and_return(pipeline.id)
 
     allow(service).to receive(:todo_service).and_return(todo_service)
   end
diff --git a/spec/services/namespaces/in_product_marketing_emails_service_spec.rb b/spec/services/namespaces/in_product_marketing_emails_service_spec.rb
index 3aa4d0b2f39..43c41b5c99d 100644
--- a/spec/services/namespaces/in_product_marketing_emails_service_spec.rb
+++ b/spec/services/namespaces/in_product_marketing_emails_service_spec.rb
@@ -218,7 +218,7 @@ RSpec.describe Namespaces::InProductMarketingEmailsService, '#execute' do
       stub_const("#{described_class}::TRACKS", { bar: :git_write })
     end
 
-    it { expect { subject }.to raise_error(NotImplementedError, 'Track foo not defined') }
+    it { expect { subject }.to raise_error(ArgumentError, 'Track foo not defined') }
   end
 
   context 'when group is a sub-group' do
diff --git a/spec/workers/build_finished_worker_spec.rb b/spec/workers/build_finished_worker_spec.rb
index 6d040f83dc7..638bf8ab12b 100644
--- a/spec/workers/build_finished_worker_spec.rb
+++ b/spec/workers/build_finished_worker_spec.rb
@@ -6,10 +6,8 @@ RSpec.describe BuildFinishedWorker do
   subject { described_class.new.perform(build.id) }
 
   describe '#perform' do
-    let(:build) { create(:ci_build, :success, pipeline: create(:ci_pipeline)) }
-
     context 'when build exists' do
-      let!(:build) { create(:ci_build) }
+      let_it_be(:build) { create(:ci_build, :success, pipeline: create(:ci_pipeline)) }
 
       before do
         expect(Ci::Build).to receive(:find_by).with(id: build.id).and_return(build)
@@ -30,6 +28,42 @@ RSpec.describe BuildFinishedWorker do
 
         subject
       end
+
+      context 'when build is failed' do
+        before do
+          build.update!(status: :failed)
+        end
+
+        it 'adds a todo' do
+          expect(::Ci::MergeRequests::AddTodoWhenBuildFailsWorker).to receive(:perform_async)
+
+          subject
+        end
+
+        context 'when async_add_build_failure_todo disabled' do
+          before do
+            stub_feature_flags(async_add_build_failure_todo: false)
+          end
+
+          it 'does not add a todo' do
+            expect(::Ci::MergeRequests::AddTodoWhenBuildFailsWorker).not_to receive(:perform_async)
+
+            subject
+          end
+        end
+      end
+
+      context 'when build has a chat' do
+        before do
+          build.pipeline.update!(source: :chat)
+        end
+
+        it 'schedules a ChatNotification job' do
+          expect(ChatNotificationWorker).to receive(:perform_async).with(build.id)
+
+          subject
+        end
+      end
     end
 
     context 'when build does not exist' do
@@ -38,15 +72,5 @@ RSpec.describe BuildFinishedWorker do
           .not_to raise_error
       end
     end
-
-    context 'when build has a chat' do
-      let(:build) { create(:ci_build, :success, pipeline: create(:ci_pipeline, source: :chat)) }
-
-      it 'schedules a ChatNotification job' do
-        expect(ChatNotificationWorker).to receive(:perform_async).with(build.id)
-
-        subject
-      end
-    end
   end
 end
diff --git a/spec/workers/ci/merge_requests/add_todo_when_build_fails_worker_spec.rb b/spec/workers/ci/merge_requests/add_todo_when_build_fails_worker_spec.rb
new file mode 100644
index 00000000000..4690c73d121
--- /dev/null
+++ b/spec/workers/ci/merge_requests/add_todo_when_build_fails_worker_spec.rb
@@ -0,0 +1,53 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Ci::MergeRequests::AddTodoWhenBuildFailsWorker do
+  describe '#perform' do
+    let_it_be(:project) { create(:project) }
+    let_it_be(:pipeline) { create(:ci_pipeline, :detached_merge_request_pipeline) }
+    let_it_be(:job) { create(:ci_build, project: project, pipeline: pipeline, status: :failed) }
+
+    let(:job_args) { job.id }
+
+    subject(:perform_twice) { perform_multiple(job_args, exec_times: 2) }
+
+    include_examples 'an idempotent worker' do
+      it 'executes todo service' do
+        service = double
+        expect(::MergeRequests::AddTodoWhenBuildFailsService).to receive(:new).with(project, nil).and_return(service).twice
+        expect(service).to receive(:execute).with(job).twice
+
+        perform_twice
+      end
+    end
+
+    context 'when job does not exist' do
+      let(:job_args) { 0 }
+
+      it 'returns nil' do
+        expect(described_class.new.perform(job_args)).to eq(nil)
+      end
+    end
+
+    context 'when project does not exist' do
+      before do
+        job.update!(project_id: nil)
+      end
+
+      it 'returns nil' do
+        expect(described_class.new.perform(job_args)).to eq(nil)
+      end
+    end
+
+    context 'when pipeline does not exist' do
+      before do
+        job.update_attribute('pipeline_id', nil)
+      end
+
+      it 'returns nil' do
+        expect(described_class.new.perform(job_args)).to eq(nil)
+      end
+    end
+  end
+end