Add latest changes from gitlab-org/gitlab@master

4 files changed, 141 insertions, 81 deletions

diff --git a/spec/services/draft_notes/publish_service_spec.rb b/spec/services/draft_notes/publish_service_spec.rb
index f83e91b683f..f93622dc25a 100644
--- a/spec/services/draft_notes/publish_service_spec.rb
+++ b/spec/services/draft_notes/publish_service_spec.rb
@@ -229,7 +229,7 @@ RSpec.describe DraftNotes::PublishService do
     expect(DraftNote.count).to eq(2)
   end
 
-  context 'with quick actions' do
+  context 'with quick actions', :sidekiq_inline do
     it 'performs quick actions' do
       other_user = create(:user)
       project.add_developer(other_user)
diff --git a/spec/services/merge_requests/handle_assignees_change_service_spec.rb b/spec/services/merge_requests/handle_assignees_change_service_spec.rb
new file mode 100644
index 00000000000..cc595aab04b
--- /dev/null
+++ b/spec/services/merge_requests/handle_assignees_change_service_spec.rb
@@ -0,0 +1,114 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe MergeRequests::HandleAssigneesChangeService do
+  let_it_be(:project) { create(:project, :repository) }
+  let_it_be(:user) { create(:user) }
+  let_it_be(:assignee) { create(:user) }
+  let_it_be(:merge_request) { create(:merge_request, author: user, source_project: project, assignees: [assignee]) }
+  let_it_be(:old_assignees) { create_list(:user, 3) }
+
+  let(:options) { {} }
+  let(:service) { described_class.new(project, user) }
+
+  before_all do
+    project.add_maintainer(user)
+    project.add_developer(assignee)
+
+    old_assignees.each do |old_assignee|
+      project.add_developer(old_assignee)
+    end
+  end
+
+  describe '#async_execute' do
+    def async_execute
+      service.async_execute(merge_request, old_assignees, options)
+    end
+
+    it 'performs MergeRequests::HandleAssigneesChangeWorker asynchronously' do
+      expect(MergeRequests::HandleAssigneesChangeWorker)
+        .to receive(:perform_async)
+        .with(
+          merge_request.id,
+          user.id,
+          old_assignees.map(&:id),
+          options
+        )
+
+      async_execute
+    end
+
+    context 'when async_handle_merge_request_assignees_change feature is disabled' do
+      before do
+        stub_feature_flags(async_handle_merge_request_assignees_change: false)
+      end
+
+      it 'calls #execute' do
+        expect(service).to receive(:execute).with(merge_request, old_assignees, options)
+
+        async_execute
+      end
+    end
+  end
+
+  describe '#execute' do
+    def execute
+      service.execute(merge_request, old_assignees, options)
+    end
+
+    it 'creates assignee note' do
+      execute
+
+      note = merge_request.notes.last
+
+      expect(note).not_to be_nil
+      expect(note.note).to include "assigned to #{assignee.to_reference} and unassigned #{old_assignees.map(&:to_reference).to_sentence}"
+    end
+
+    it 'sends email notifications to old and new assignees', :mailer, :sidekiq_inline do
+      perform_enqueued_jobs do
+        execute
+      end
+
+      should_email(assignee)
+      old_assignees.each do |old_assignee|
+        should_email(old_assignee)
+      end
+    end
+
+    it 'creates pending todo for assignee' do
+      execute
+
+      todo = assignee.todos.last
+
+      expect(todo).to be_pending
+    end
+
+    it 'tracks users assigned event' do
+      expect(Gitlab::UsageDataCounters::MergeRequestActivityUniqueCounter)
+        .to receive(:track_users_assigned_to_mr).once.with(users: [assignee])
+
+      execute
+    end
+
+    it 'tracks assignees changed event' do
+      expect(Gitlab::UsageDataCounters::MergeRequestActivityUniqueCounter)
+        .to receive(:track_assignees_changed_action).once.with(user: user)
+
+      execute
+    end
+
+    context 'when execute_hooks option is set to true' do
+      let(:options) { { execute_hooks: true } }
+
+      it 'execute hooks and services' do
+        expect(merge_request.project).to receive(:execute_hooks).with(anything, :merge_request_hooks)
+        expect(merge_request.project).to receive(:execute_services).with(anything, :merge_request_hooks)
+        expect(service).to receive(:enqueue_jira_connect_messages_for).with(merge_request)
+
+        execute
+      end
+    end
+  end
+end
diff --git a/spec/services/merge_requests/update_assignees_service_spec.rb b/spec/services/merge_requests/update_assignees_service_spec.rb
index aa6aad854a6..de03aab5418 100644
--- a/spec/services/merge_requests/update_assignees_service_spec.rb
+++ b/spec/services/merge_requests/update_assignees_service_spec.rb
@@ -37,9 +37,11 @@ RSpec.describe MergeRequests::UpdateAssigneesService do
 
     context 'when the parameters are valid' do
       it 'updates the MR, and queues the more expensive work for later' do
-        expect(MergeRequests::AssigneesChangeWorker)
-          .to receive(:perform_async)
-          .with(merge_request.id, user.id, [user3.id])
+        expect_next(MergeRequests::HandleAssigneesChangeService, project, user) do |service|
+          expect(service)
+            .to receive(:async_execute)
+            .with(merge_request, [user3], execute_hooks: true)
+        end
 
         expect { update_merge_request }
           .to change(merge_request, :assignees).to([user2])
@@ -54,9 +56,11 @@ RSpec.describe MergeRequests::UpdateAssigneesService do
       end
 
       it 'is more efficient than using the full update-service' do
-        allow(MergeRequests::AssigneesChangeWorker)
-          .to receive(:perform_async)
-          .with(merge_request.id, user.id, [user3.id])
+        allow_next(MergeRequests::HandleAssigneesChangeService, project, user) do |service|
+          expect(service)
+            .to receive(:async_execute)
+            .with(merge_request, [user3], execute_hooks: true)
+        end
 
         other_mr = create(:merge_request, :simple, :unique_branches,
                           title: merge_request.title,
@@ -72,17 +76,4 @@ RSpec.describe MergeRequests::UpdateAssigneesService do
       end
     end
   end
-
-  describe '#handle_assignee_changes' do
-    subject { service.handle_assignee_changes(merge_request, [user2]) }
-
-    it 'calls UpdateService#handle_assignee_changes and executes hooks' do
-      expect(service).to receive(:handle_assignees_change).with(merge_request, [user2])
-      expect(merge_request.project).to receive(:execute_hooks).with(anything, :merge_request_hooks)
-      expect(merge_request.project).to receive(:execute_services).with(anything, :merge_request_hooks)
-      expect(service).to receive(:enqueue_jira_connect_messages_for).with(merge_request)
-
-      subject
-    end
-  end
 end
diff --git a/spec/services/merge_requests/update_service_spec.rb b/spec/services/merge_requests/update_service_spec.rb
index 579d274d910..24b7c3b933b 100644
--- a/spec/services/merge_requests/update_service_spec.rb
+++ b/spec/services/merge_requests/update_service_spec.rb
@@ -205,30 +205,6 @@ RSpec.describe MergeRequests::UpdateService, :mailer do
           MergeRequests::UpdateService.new(project, user, opts).execute(merge_request)
         end
 
-        context 'assignees' do
-          context 'when assignees changed' do
-            it 'tracks assignees changed event' do
-              expect(Gitlab::UsageDataCounters::MergeRequestActivityUniqueCounter)
-                .to receive(:track_assignees_changed_action).once.with(user: user)
-
-              opts[:assignees] = [user2]
-
-              MergeRequests::UpdateService.new(project, user, opts).execute(merge_request)
-            end
-          end
-
-          context 'when assignees did not change' do
-            it 'does not track assignees changed event' do
-              expect(Gitlab::UsageDataCounters::MergeRequestActivityUniqueCounter)
-                .not_to receive(:track_assignees_changed_action)
-
-              opts[:assignees] = merge_request.assignees
-
-              MergeRequests::UpdateService.new(project, user, opts).execute(merge_request)
-            end
-          end
-        end
-
         context 'reviewers' do
           context 'when reviewers changed' do
             it 'tracks reviewers changed event' do
@@ -326,21 +302,6 @@ RSpec.describe MergeRequests::UpdateService, :mailer do
           )
       end
 
-      it 'sends email to user2 about assign of new merge request and email to user3 about merge request unassignment', :sidekiq_might_not_need_inline do
-        deliveries = ActionMailer::Base.deliveries
-        email = deliveries.last
-        recipients = deliveries.last(2).flat_map(&:to)
-        expect(recipients).to include(user2.email, user3.email)
-        expect(email.subject).to include(merge_request.title)
-      end
-
-      it 'creates system note about merge_request reassign' do
-        note = find_note('assigned to')
-
-        expect(note).not_to be_nil
-        expect(note.note).to include "assigned to #{user.to_reference} and unassigned #{user3.to_reference}"
-      end
-
       context 'with reviewers' do
         let(:opts) { { reviewer_ids: [user2.id] } }
 
@@ -664,20 +625,6 @@ RSpec.describe MergeRequests::UpdateService, :mailer do
         it 'marks previous assignee pending todos as done' do
           expect(pending_todo.reload).to be_done
         end
-
-        it 'creates a pending todo for new assignee' do
-          attributes = {
-            project: project,
-            author: user,
-            user: user2,
-            target_id: merge_request.id,
-            target_type: merge_request.class.name,
-            action: Todo::ASSIGNED,
-            state: :pending
-          }
-
-          expect(Todo.where(attributes).count).to eq 1
-        end
       end
 
       context 'when reviewers gets changed' do
@@ -999,6 +946,8 @@ RSpec.describe MergeRequests::UpdateService, :mailer do
       it 'does not update assignee when assignee_id is invalid' do
         merge_request.update!(assignee_ids: [user.id])
 
+        expect(MergeRequests::HandleAssigneesChangeService).not_to receive(:new)
+
         update_merge_request(assignee_ids: [-1])
 
         expect(merge_request.reload.assignees).to eq([user])
@@ -1007,29 +956,35 @@ RSpec.describe MergeRequests::UpdateService, :mailer do
       it 'unassigns assignee when user id is 0' do
         merge_request.update!(assignee_ids: [user.id])
 
+        expect_next_instance_of(MergeRequests::HandleAssigneesChangeService, project, user) do |service|
+          expect(service)
+            .to receive(:async_execute)
+            .with(merge_request, [user])
+        end
+
         update_merge_request(assignee_ids: [0])
 
         expect(merge_request.assignee_ids).to be_empty
       end
 
       it 'saves assignee when user id is valid' do
+        expect_next_instance_of(MergeRequests::HandleAssigneesChangeService, project, user) do |service|
+          expect(service)
+            .to receive(:async_execute)
+            .with(merge_request, [user3])
+        end
+
         update_merge_request(assignee_ids: [user.id])
 
         expect(merge_request.assignee_ids).to eq([user.id])
       end
 
-      it 'updates the tracking when user ids are valid' do
-        expect(Gitlab::UsageDataCounters::MergeRequestActivityUniqueCounter)
-          .to receive(:track_users_assigned_to_mr)
-          .with(users: [user])
-
-        update_merge_request(assignee_ids: [user.id])
-      end
-
       it 'does not update assignee_id when user cannot read issue' do
         non_member = create(:user)
         original_assignees = merge_request.assignees
 
+        expect(MergeRequests::HandleAssigneesChangeService).not_to receive(:new)
+
         update_merge_request(assignee_ids: [non_member.id])
 
         expect(merge_request.reload.assignees).to eq(original_assignees)