Skip to content

Extend MergeRequestResetApprovalsWorker to accept checks override

What does this MR do and why?

From #371408 (closed)

MergeRequestResetApprovalsWorker relies on MergeRequests::ResetApprovalsService for resetting approvals, but is focused on being reactive to events as a side-effect -- like resetting approvals DUE to a push. It does this through gating via MergeRequests::ResetApprovalsService#reset_approvals? which checks project settings and the state of the MR.

This prevents us from resetting approvals as a deliberative choice. !95525 (merged) adds this, at least for bots, but causes us to have to directly invoke the code that ultimately resets the approvals (merge_request.approvals.delete_all) rather than keep our logic in a SSoT by going through MergeRequests::ResetApprovalsService

We can allow for a consolidated logic by extending MergeRequests::ResetApprovalsService to accept some kind of param to flag "skip that check and delete anyways"

By adding this option, we can implement approval deletions without side effects, in situations where we may not have access to specific rev information. The primary use case is !95525 (merged), and the goal is to keep our API and UI logic in one single location.

Screenshots or screen recordings

Screenshots are required for UI changes, and strongly recommended for all other merge requests.

How to set up and validate locally

Numbered steps to set up and validate the change are strongly suggested.

MR acceptance checklist

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

Edited by Kerri Miller

Merge request reports