Resolve "Add "No approval required" state to approval rules MR component"
What does this MR do?
The previous implementation of multiple blocking approval rules #1979 (closed) did not include the "No approvals required" view. This MR adds this feature to the new set of MR widget approvals components.
Please note:
To test this feature you will need to enable the feature flag approval_rules
.
What are the relevant issue numbers?
Closes #9963 (closed)
Screenshots
Does this MR meet the acceptance criteria?
-
Changelog entry added, if necessary -
Documentation created/updated via this MR -
Documentation reviewed by technical writer or follow-up review issue created -
Tests added for this feature/bug -
Tested in all supported browsers -
Conforms to the code review guidelines -
Conforms to the merge request performance guidelines -
Conforms to the style guides -
Conforms to the database guides -
Link to e2e tests MR added if this MR has Requires e2e tests label. See the Test Planning Process. -
EE specific content should be in the top level /ee
folder -
For a paid feature, have we considered GitLab.com plans, how it works for groups, and is there a design for promoting it to users who aren't on the correct plan? -
Security reports checked/validated by reviewer
Merge request reports
Activity
changed milestone to %11.9
@sbigelow, do you mind giving this a pre-maintainer review? Thanks so much!
assigned to @sbigelow
1 Warning The title of this merge request is longer than 72 characters and would violate our commit message rules when using the Squash on Merge feature. Please consider adjusting the title, or rebase the commits manually and don’t use Squash on Merge. Reviewer roulette
Changes that require review have been detected! A merge request is normally reviewed by both a reviewer and a maintainer in its primary category (e.g. frontend or backend), and by a maintainer in all other categories.
To spread load more evenly across eligible reviewers, Danger has randomly picked a candidate for each review slot. Feel free to override this selection if you think someone else would be better-suited, or the chosen person is unavailable.
Once you've decided who will review this merge request, mention them as you normally would! Danger does not (yet?) automatically notify them for you.
Category Reviewer Maintainer frontend Enrique Alcántara ( @ealcantara
)Filipa Lacerda ( @filipa
)Generated by
Danger Edited by 🤖 GitLab Bot 🤖- Resolved by Paul Slaughter
- Resolved by Phil Hughes
- Resolved by Paul Slaughter
- Resolved by Paul Slaughter
- Resolved by Paul Slaughter
- Resolved by Paul Slaughter
- Resolved by Paul Slaughter
@pslaughter I was thinking about this later and wanted to give it some more attention. At a better look, I basically just noticed some places that could use more null checking. This lead me to thinking it may be worth just a computed
approvals
property that could default values onthis.mr.approvals
if it is undefined.I could be off here, so they are just mentioned as questions.
assigned to @pslaughter
added 1 commit
- ef2a70ae - Add default approvals obj to rules mr component