Move approvals notifications to Sidekiq

ee/app/services/merge_requests/approval_service.rb

@@ -10,7 +10,7 @@ module MergeRequests
         mark_pending_todos_as_done(merge_request)
 
         if merge_request.approvals_left.zero?
-          notification_service.approve_mr(merge_request, current_user)
+          notification_service.async.approve_mr(merge_request, current_user)
           execute_hooks(merge_request, 'approved')
         end
       end

ee/app/services/merge_requests/remove_approval_service.rb

@@ -14,7 +14,7 @@ module MergeRequests
         create_note(merge_request)
 
         if currently_approved
-          notification_service.unapprove_mr(merge_request, current_user)
+          notification_service.async.unapprove_mr(merge_request, current_user)
           execute_hooks(merge_request, 'unapproved')
         end
       end

ee/spec/services/merge_requests/approval_service_spec.rb

@@ -71,9 +71,8 @@ describe MergeRequests::ApprovalService do
 
         before do
           expect(merge_request).to receive(:approvals_left).and_return(0)
-          allow(service).to receive(:notification_service).and_return(notification_service)
           allow(service).to receive(:execute_hooks)
-          allow(notification_service).to receive(:approve_mr)
+          allow(service).to receive(:notification_service).and_return(notification_service)
         end
 
         it 'fires a webhook' do
@@ -83,7 +82,7 @@ describe MergeRequests::ApprovalService do
         end
 
         it 'sends an email' do
-          expect(notification_service).to receive(:approve_mr).with(merge_request, user)
+          expect(notification_service).to receive_message_chain(:async, :approve_mr).with(merge_request, user)
 
           service.execute(merge_request)
         end

ee/spec/services/merge_requests/remove_approval_service_spec.rb

@@ -3,8 +3,8 @@ require 'rails_helper'
 describe MergeRequests::RemoveApprovalService do
   describe '#execute' do
     let(:user) { create(:user) }
-    let(:merge_request) { create(:merge_request) }
-    let(:project) { merge_request.project }
+    let(:project) { create(:project, approvals_before_merge: 1) }
+    let(:merge_request) { create(:merge_request, source_project: project) }
 
     subject(:service) { described_class.new(project, user) }
 
@@ -14,6 +14,8 @@ describe MergeRequests::RemoveApprovalService do
 
     context 'with a user who has approved' do
       before do
+        project.add_developer(create(:user))
+        merge_request.update!(approvals_before_merge: 2)
         merge_request.approvals.create(user: user)
       end
 
@@ -30,7 +32,7 @@ describe MergeRequests::RemoveApprovalService do
       end
 
       it 'does not send a notification' do
-        expect(Notify).not_to receive(:unapprove_mr)
+        expect(service).not_to receive(:notification_service)
 
         execute!
       end
@@ -43,16 +45,16 @@ describe MergeRequests::RemoveApprovalService do
     end
 
     context 'with an approved merge request' do
-      let(:notify) { Object.new }
+      let(:notification_service) { NotificationService.new }
 
       before do
-        merge_request.update_attribute :approvals_before_merge, 1
         merge_request.approvals.create(user: user)
-        allow(service).to receive(:notification_service).and_return(notify)
+        allow(service).to receive(:notification_service).and_return(notification_service)
       end
 
       it 'sends a notification' do
-        expect(notify).to receive(:unapprove_mr)
+        expect(notification_service).to receive_message_chain(:async, :unapprove_mr).with(merge_request, user)
+
         execute!
       end
     end