Required approval rule “Eligible users” can result in misleading required approvals count
Summary
When adding CODEOWNERES to a repo and requiring that approval must be obtained from at least one (1) CODEOWNER, the approval workflow does not work correctly.
Steps to reproduce
- Running
v12.2.5-eecreate a repo and invite the CODEOWNERS group to it under Settings / Members / Invite Group - Create a CODEOWNERS file in the repo that contains
* @group-referenced-hereas a default global code owner - Enable settings requiring code owner approval for merge under Settings / General / Merge Request Approval
- Change the number of approvals required to 1
- tick the box 'require approval from code owners' and save changes.
- Push a branch with committed code and start an MR
- Observe as it says two (2) members are required (when you only indicated one (1)
- Approve the request and see the UI show approval being satisfied but the merge button is greyed out.
What is the current bug behavior?
Unable to use CODEOWNERS feature
What is the expected correct behavior?
That one (1) of the members identified in the group for default code ownership is ALL that is required to approve the MR
Relevant logs and/or screenshots
Proposal
- In the following scenario:
- The “Eligible users” rule has a required number of approvals…
- AND there are other required approval rules OR Code Owner rules…
- …we should discount the number of required approvals from the “Eligible users” rule.
Here are some examples:
-
Eligible users rule requires
5approvals, UX rule requires1approval, BE rule requires1approval: the summary should readRequires 5 approvals from eligible users, UX, and BE.- Today this scenario incorrectly reads
Requires 7 more approvals from UX and BE.
- Today this scenario incorrectly reads
-
Eligible users rule requires
1approval, UX rule requires1approval, BE rule requires1approval: the summary should readRequires 2 approvals from UX and BE.- Today this scenario incorrectly reads
Requires 3 more approvals from UX and BE.
- Today this scenario incorrectly reads
-
Eligible users rule requires
0approvals, UX rule requires1approval, BE rule requires1approval: the summary should readRequires 2 approvals from UX and BE.- Today this scenario already reads
Requires 2 more approvals from UX and BE.
- Today this scenario already reads
-
Eligible users rule requires
1approval:Requires 1 approval from eligible users.- Today this scenario reads
Requires approval.
- Today this scenario reads
Actionable insight
This was also noted as an actionable insight from the UX Department: MRs experience async critique research (insight).
Edited by Pedro Moreira da Silva

