Add new hooks for all approvals and unapprovals, not just when the threshold...

Add new hooks for all approvals and unapprovals, not just when the threshold is met. The new hooks are using 'approval' and 'unapproval' to ensure backwards compatibility.

ee/app/services/merge_requests/approval_service.rb

@@ -14,6 +14,8 @@ module MergeRequests
         if merge_request.approvals_left.zero?
           notification_service.async.approve_mr(merge_request, current_user)
           execute_hooks(merge_request, 'approved')
+        else
+          execute_hooks(merge_request, 'approval')
         end
       end
     end

ee/app/services/merge_requests/remove_approval_service.rb

@@ -13,12 +13,13 @@ module MergeRequests
 
       if approval.destroy_all # rubocop: disable DestroyAll
         merge_request.reset_approval_cache!
-
         create_note(merge_request)
 
         if currently_approved
           notification_service.async.unapprove_mr(merge_request, current_user)
           execute_hooks(merge_request, 'unapproved')
+        else
+          execute_hooks(merge_request, 'unapproval')
         end
       end
     end

ee/changelogs/unreleased/approval-unapproval-webhooks.yml

+---
+title: Add approval and unapproval webhooks
+merge_request: 8742
+author:
+type: added

ee/spec/services/merge_requests/approval_service_spec.rb

@@ -51,9 +51,9 @@ describe MergeRequests::ApprovalService do
       end
 
       context 'with remaining approvals' do
-        it 'does not fire a webhook' do
+        it 'fires an approval webhook' do
           expect(merge_request).to receive(:approvals_left).and_return(5)
-          expect(service).not_to receive(:execute_hooks)
+          expect(service).to receive(:execute_hooks).with(merge_request, 'approval')
 
           service.execute(merge_request)
         end
@@ -75,7 +75,7 @@ describe MergeRequests::ApprovalService do
           allow(service).to receive(:notification_service).and_return(notification_service)
         end
 
-        it 'fires a webhook' do
+        it 'fires an approved webhook' do
           expect(service).to receive(:execute_hooks).with(merge_request, 'approved')
 
           service.execute(merge_request)

ee/spec/services/merge_requests/remove_approval_service_spec.rb

@@ -35,6 +35,12 @@ describe MergeRequests::RemoveApprovalService do
         execute!
       end
 
+      it 'fires an unapproval webhook' do
+        expect(service).to receive(:execute_hooks).with(merge_request, 'unapproval')
+
+        execute!
+      end
+
       it 'does not send a notification' do
         expect(service).not_to receive(:notification_service)
 
@@ -56,6 +62,12 @@ describe MergeRequests::RemoveApprovalService do
         allow(service).to receive(:notification_service).and_return(notification_service)
       end
 
+      it 'fires an unapproved webhook' do
+        expect(service).to receive(:execute_hooks).with(merge_request, 'unapproved')
+
+        execute!
+      end
+
       it 'sends a notification' do
         expect(notification_service).to receive_message_chain(:async, :unapprove_mr).with(merge_request, user)