Follow-up from "Added approval checkbox to submit review dropdown"

Summary

approve_merge_request access rights can be checked via 2 methods:

Ability.allowed?(current_user, :approve_merge_request, merge_request) merge_request.can_approve?(current_user)

which has different implementation. This dualism creates additional complexity and possible mistakes. it would be great to have can_approve? merged into a policy so the policy becomes the only source of truth.

There is also ApprovalState#can_be_approved_by? method which duplicates merge_request.can_approve?(current_user) and adds more logic.

Original discussion

The following discussion from !90004 (merged) should be addressed:

• @pshutsin started a discussion:

  that's weird we have 2 sources of truth here.

  can?(current_user, :approve_merge_request, merge_request)

  merge_request.can_approve?(current_user)

  And as I see they are out of sync. We should create an issue to unify it.

@iamphill I'm not sure whom to assign this to for scheduling so assigning to you. Could you please reroute it?