Refactor how we call `MergeRequests::BaseService#reset_approvals` and `MergeRequests::BaseService#delete_approvals`
Background
We are no longer calling MergeRequests::BaseService#reset_approvals
in other services aside from MergeRequests::ResetApprovalsService
but it's still in MergeRequests::BaseService
.
We are also calling MergeRequests::BaseService#delete_approvals
in MergeRequests::UpdateService#create_branch_change_note
which is not really its responsibility. We do it because we only want to delete approvals in EE when target branch changes.
Proposal
- Move
MergeRequests::BaseService#reset_approvals
toMergeRequests::ResetApprovalsService
. - Create a
MergeRequests::UpdateService#delete_approvals_on_target_branch_change
, call it inMergeRequests::UpdateService#handle_target_branch_change
after calling#create_branch_change_note
. Then in EE, override#delete_approvals_on_target_branch_change
to calldelete_approvals(merge_request) if reset_approvals?(merge_request, nil)
.
Edited by Patrick Bajao