Skip to content

Refactor how reset_approvals and delete_approvals are called

Patrick Bajao requested to merge 428407-refactor-reset-approvals into master

What does this MR do and why?

Nothing else calls MergeRequests::BaseService#reset_approvals aside from MergeRequests::ResetApprovalsService so we move the method to that service along with other methods it needs that are also not called elsewhere.

This also includes changes to how we call #delete_approvals in MergeRequests::UpdateService. Outside EE, we don't really reset approvals when target branch changes but we are currently overriding #create_branch_change_note to add that behavior in EE which is not really the responsibility of that said method.

To improve this, we introduce a new method called #delete_approvals_on_target_branch_change and we call it in #handle_target_branch_change. It doesn't do anything outside EE and we override it to delete approvals when target branch changes in EE.

MR acceptance checklist

This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.

Related to #428407 (closed)

Merge request reports