Refactor MergeRequests::ApprovalService to return reason
Summary
As of this writing, when MergeRequests::ApprovalService#execute
is called and it fails, it returns nil
or false
and the consumer doesn't know what's the reason for it.
This was initially brought up in #372972 (comment 1094005013).
Improvements
Once !97541 (merged) is merged, we can refactor the MergeRequests::ApprovalService
Risks
Approval failure handling at consumer level (controller and API endpoint) may break. This can be mitigated by making sure failure cases are covered in tests of specific consumers.
Involved components
Service
app/services/merge_requests/approval_service.rb
ee/app/services/ee/merge_requests/approval_service.rb
Consumers
app/controllers/projects/merge_requests/drafts_controller.rb
lib/api/merge_request_approvals.rb
lib/gitlab/quick_actions/merge_request_actions.rb