Filter approval rule approvers individually
What does this MR do and why?
Merge request approval policies support enforcing stricter approval settings through their approval_settings.prevent_approval_by_{commit_}author properties. A merge request approval rule originating from an approval policy that sets one of these attributes hence permits less approvers, because it excludes the author or committers. Likewise, a merge request approval rule originating from a policy that doesn't set these attributes should permit more approvers (assuming the target project permits author/committer approval).
Currently, approvers are filtered for author and committers globally for all rules in ApprovalState#filter_approvers. However this means that if there are two policy approval rules, and only one of them sets eg. prevent_approval_by_author, then the author gets filtered out as eligible approver for both rules, despite only one of the rules setting the property.
This merge request moves the filtering logic from ApprovalState into ApprovalWrappedRule, so that approvers are filtered depending on individual approval rules, not their aggregate.
References
Screenshots or screen recordings
| Before | After |
|---|---|
|
|
How to set up and validate locally
-
Enable the feature flag:
echo "Feature.enable(:approval_policy_rules_individual_approvers_filtering)" | rails c -
Create a new project (as
Owneruser) -
Add two Maintainers to the project:
Author,Committer -
Navigate to
Secure > Policiesand create the following Merge request approval policies:approval_policy: - name: Committer approval prevented enabled: true rules: - type: any_merge_request branch_type: protected commits: any actions: - type: require_approval approvals_required: 1 role_approvers: - maintainer - owner - type: send_bot_message enabled: true approval_settings: prevent_approval_by_commit_author: trueapproval_policy: - name: Author approval prevented enabled: true rules: - type: any_merge_request branch_type: protected commits: any actions: - type: require_approval approvals_required: 1 role_approvers: - maintainer - owner - type: send_bot_message enabled: true approval_settings: prevent_approval_by_author: trueapproval_policy: - name: Any Owner or Maintainer enabled: true rules: - type: any_merge_request branch_type: protected commits: any actions: - type: require_approval approvals_required: 1 role_approvers: - maintainer - owner - type: send_bot_message enabled: true -
Open an MR using
Authortargeting the default branch -
Add a commit to the MR source branch with
Committer -
Verify that the following rule/users combinations are approvable:
Owner Committer Author Committer approval prevented yes Author approval prevented yes yes Any Owner or Maintainer yes yes -
Navigate to
Settings > Merge requests, uncheckPrevent approval by merge request creatorandSave changes -
Verify that the following rule/users combinations are approvable:
Owner Committer Author Committer approval prevented yes Author approval prevented yes yes Any Owner or Maintainer yes yes yes -
Navigate to
Settings > Merge requests, checkPrevent approvals by users who add commitsandSave changes -
Verify that the following rule/users combinations are approvable:
Owner Committer Author Committer approval prevented yes Author approval prevented yes Any Owner or Maintainer yes -
Navigate to
Settings > Merge requests, uncheckPrevent approvals by users who add commitsandSave changes -
As administrator, navigate to
Admin > Push rulesand underMerge request approvals, checkPrevent approval by merge request creatorsandSave changes -
Verify that the following rule/users combinations are approvable:
Owner Committer Author Committer approval prevented yes Author approval prevented yes yes Any Owner or Maintainer yes yes -
As administrator, navigate to
Admin > Push rulesand underMerge request approvals, uncheckPrevent approval by merge request creatorsandSave changes -
Approve as
Committerand verify the MR approval state:-
Committer approval prevented: unapproved -
Author approval prevented: approved -
Any Owner or Maintainer: approved - Mergeable: no
-
-
Revoke the prior approval, then approve as
Authorand verify the MR approval state:-
Committer approval prevented: unapproved -
Author approval prevented: unapproved -
Any Owner or Maintainer: approved - Mergeable: no
-
-
Revoke the prior approval, then approve as
Ownerand verify the MR approval state:-
Committer approval prevented: approved -
Author approval prevented: approved -
Any Owner or Maintainer: approved - Mergeable: yes
-
MR acceptance checklist
Evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.
Related to #574854 (closed)

