diff --git a/ee/app/services/merge_requests/approval_service.rb b/ee/app/services/merge_requests/approval_service.rb index 8122d7042590f8501d47e00d38824b841fe65c8e..3bad19262e0c6002234455f43e3af95aab8fe0fa 100644 --- a/ee/app/services/merge_requests/approval_service.rb +++ b/ee/app/services/merge_requests/approval_service.rb @@ -14,6 +14,8 @@ def execute(merge_request) 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 diff --git a/ee/app/services/merge_requests/remove_approval_service.rb b/ee/app/services/merge_requests/remove_approval_service.rb index 355e34547b4ec3b4cf26ff13ee28aa74754b5085..a353a25922e2234c91376299c8ee9975919ed031 100644 --- a/ee/app/services/merge_requests/remove_approval_service.rb +++ b/ee/app/services/merge_requests/remove_approval_service.rb @@ -13,12 +13,13 @@ def execute(merge_request) 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 diff --git a/ee/changelogs/unreleased/approval-unapproval-webhooks.yml b/ee/changelogs/unreleased/approval-unapproval-webhooks.yml new file mode 100644 index 0000000000000000000000000000000000000000..34074bbbb71e4c9e2e2f17b593a6acdd587654cd --- /dev/null +++ b/ee/changelogs/unreleased/approval-unapproval-webhooks.yml @@ -0,0 +1,5 @@ +--- +title: Add approval and unapproval webhooks +merge_request: 8742 +author: +type: added diff --git a/ee/spec/services/merge_requests/approval_service_spec.rb b/ee/spec/services/merge_requests/approval_service_spec.rb index 97aa9337362eb2bd2e0544cdea57b6eb59e831e4..a971c146d0cc25aec5a023e934703d508b39dbc3 100644 --- a/ee/spec/services/merge_requests/approval_service_spec.rb +++ b/ee/spec/services/merge_requests/approval_service_spec.rb @@ -51,9 +51,9 @@ 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 @@ 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) diff --git a/ee/spec/services/merge_requests/remove_approval_service_spec.rb b/ee/spec/services/merge_requests/remove_approval_service_spec.rb index 770b1cdffceb4092ee147b5abe9456ed5fcf3364..92a5d9c42b714b92771fbe3022f36d9f77f24251 100644 --- a/ee/spec/services/merge_requests/remove_approval_service_spec.rb +++ b/ee/spec/services/merge_requests/remove_approval_service_spec.rb @@ -35,6 +35,12 @@ def execute! 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 @@ def execute! 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)