Do not show revoke button if revoke_path is absent
What does this MR do and why?
AccessTokenTableApp component assumed that revoke_path in the access
token is always present. However, the property revoke_path is
optional.
When the revoke_path is absent or falsy, it doesn't display the
revoke button action.
Screenshots or screen recordings
Before
After
How to set up and validate locally
- Create a personal access token:
- See the revoke button is present
- Apply patch:
diff --git a/app/assets/javascripts/access_tokens/index.js b/app/assets/javascripts/access_tokens/index.js
index f0c1b415157..c12b45bf2b1 100644
--- a/app/assets/javascripts/access_tokens/index.js
+++ b/app/assets/javascripts/access_tokens/index.js
@@ -43,7 +43,7 @@ export const initAccessTokenTableApp = () => {
provide: {
accessTokenType,
accessTokenTypePlural,
- initialActiveAccessTokens,
+ initialActiveAccessTokens: initialActiveAccessTokens.map(({ revokePath, ...rest }) => rest),
noActiveTokensMessage,
showRole,
},- Reload the page and observe that the button is not present.
MR acceptance checklist
This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.
-
I have evaluated the MR acceptance checklist for this MR.
Merge request reports
Activity
changed milestone to %15.4
requested review from @eduardosanz
assigned to @eduardosanz
Suggested Reviewers (beta)
The individuals below may be good candidates to participate in the review based on various factors.
You can use slash commands in comments to quickly assign
/assign_reviewer @user1.Suggested Reviewers @dmoraBerlin,@iamphill,@peterhegman,@cngo,@bsandlinIf you do not believe these suggestions are useful, please apply the label Bad Suggested Reviewer. You can also provide feedback for this feature on this issue:
https://gitlab.com/gitlab-org/gitlab/-/issues/357923.Automatically generated by Suggested Reviewers Bot - an experimental ML-based recommendation engine created by ~"group::applied ml".
Reviewer roulette
Changes that require review have been detected!
Please refer to the table below for assigning reviewers and maintainers suggested by Danger in the specified category:
Category Reviewer Maintainer frontend Doug Stull ( @dstull) (UTC-4, 6 hours behind@eduardosanz)Paul Slaughter ( @pslaughter) (UTC-5, 7 hours behind@eduardosanz)UX Sascha Eggenberger ( @seggenberger) (UTC+2, same timezone as@eduardosanz)Maintainer review is optional for UX ~"group::authentication and authorization" Reviewer review is optional for ~"group::authentication and authorization" Drew Blessing ( @dblessing) (UTC-5, 7 hours behind@eduardosanz)To spread load more evenly across eligible reviewers, Danger has picked a candidate for each review slot, based on their timezone. Feel free to override these selections if you think someone else would be better-suited or use the GitLab Review Workload Dashboard to find other available reviewers.
To read more on how to use the reviewer roulette, please take a look at the Engineering workflow and code review guidelines. Please consider assigning a reviewer or maintainer who is a domain expert in the area of the merge request.
Once you've decided who will review this merge request, assign them as a reviewer! Danger does not automatically notify them for you.
If needed, you can retry the
danger-reviewjob that generated this comment.Generated by
Bundle size analysis [beta]
This compares changes in bundle size for entry points between the commits 335b8cbb and 1080be8f
Entrypoint / Name Size before Size after Diff Diff in percent average 3.51 MB 3.51 MB - 0.0 % mainChunk 1.91 MB 1.91 MB - 0.0 %
Note: We do not have exact data for 335b8cbb. So we have used data from: f09082eb.
The intended commit has no webpack pipeline, so we chose the last commit with one before it.Please look at the full report for more details
Read more about how this report works.
Generated by
Resolved by Paul Slaughter@agulina, please could you review this MR? Thanks!
- Last reply by Paul Slaughter
requested review from @agulina
removed review request for @eduardosanz
Setting label(s) devopsmanage sectiondev based on ~"group::authentication and authorization".
added devopsmanage sectiondev labels
-
This nudge was added by this triage-ops policy.
removed review request for @agulina
added UX label
-
Please wait for Reviewer Roulette to suggest a designer for UX review, and then assign them as Reviewer. This helps evenly distribute reviews across UX.
This message was generated automatically. You're welcome to improve it.
requested review from @agulina
added 565 commits
-
7de821ec...072ec49c - 563 commits from branch
master - ed772b46 - Do not show revoke button if revoke_path is absent
- 04ad4e70 - Apply feedback from code review
-
7de821ec...072ec49c - 563 commits from branch
-
@dblessing, would you mind reviewing this MR in behave of the ~"group::authentication and authorization"? Thanks!
It is frontend only, but we still need the approval of somebody from the group.
requested review from @dblessing
-
Hey @pslaughter,
would you have time to look at this merge request, please? Thanks!
requested review from @pslaughter
-
@agulina, thanks for approving this merge request.This is the first time the merge request is approved. To ensure full test coverage, a new pipeline has been started.
For more info, please refer to the following links:
requested review from @seggenberger
Allure report
allure-report-publishergenerated test report!review-qa-blocking:
expand test summary
+-----------------------------------------------------------------------------------------+ | suites summary | +------------------------------------+--------+--------+---------+-------+-------+--------+ | | passed | failed | skipped | flaky | total | result | +------------------------------------+--------+--------+---------+-------+-------+--------+ | Create | 28 | 0 | 1 | 5 | 29 | ❗ | | Verify | 12 | 0 | 1 | 5 | 13 | ❗ | | Plan | 47 | 0 | 1 | 6 | 48 | ❗ | | Feature flag handler sanity checks | 9 | 0 | 0 | 0 | 9 | ✅ | | Manage | 46 | 0 | 3 | 14 | 49 | ❗ | | Configure | 0 | 0 | 1 | 0 | 1 | ➖ | | Version sanity check | 0 | 0 | 1 | 0 | 1 | ➖ | | Package | 0 | 0 | 1 | 0 | 1 | ➖ | | Secure | 2 | 0 | 0 | 2 | 2 | ❗ | | Protect | 2 | 0 | 0 | 1 | 2 | ❗ | +------------------------------------+--------+--------+---------+-------+-------+--------+ | Total | 146 | 0 | 9 | 33 | 155 | ❗ | +------------------------------------+--------+--------+---------+-------+-------+--------+e2e-review-qa-blocking:
expand test summary
+-----------------------------------------------------------------------------------------+ | suites summary | +------------------------------------+--------+--------+---------+-------+-------+--------+ | | passed | failed | skipped | flaky | total | result | +------------------------------------+--------+--------+---------+-------+-------+--------+ | Plan | 47 | 0 | 1 | 47 | 48 | ❗ | | Version sanity check | 0 | 0 | 1 | 0 | 1 | ➖ | | Create | 28 | 0 | 1 | 27 | 29 | ❗ | | Manage | 57 | 0 | 3 | 54 | 60 | ❗ | | Verify | 12 | 0 | 1 | 10 | 13 | ❗ | | Feature flag handler sanity checks | 9 | 0 | 0 | 0 | 9 | ✅ | | Configure | 0 | 0 | 1 | 0 | 1 | ➖ | | Protect | 2 | 0 | 0 | 2 | 2 | ❗ | | Package | 0 | 0 | 1 | 0 | 1 | ➖ | | Secure | 2 | 0 | 0 | 2 | 2 | ❗ | +------------------------------------+--------+--------+---------+-------+-------+--------+ | Total | 157 | 0 | 9 | 142 | 166 | ❗ | +------------------------------------+--------+--------+---------+-------+-------+--------+removed review request for @dblessing
-
Thanks for everything here @eduardosanz! I left you a very small testing suggestion for your consideration:
Then we should be good to go
Back to you!
removed review request for @pslaughter
requested review from @pslaughter
-
Looks like pipeline failure is unrelated and possible fixed by this commit:
Let me try a
/rebase
added 589 commits
-
6f19649f...b0d79b7f - 586 commits from branch
master - 32cfe3cf - Do not show revoke button if revoke_path is absent
- 1a9abec6 - Apply feedback from code review
- 1080be8f - Apply feedback from code review
Toggle commit list-
6f19649f...b0d79b7f - 586 commits from branch
-
Thanks for this MR @eduardosanz! Changes LGTM!
Approving...
todo: For the sake of completeness, let's get an official UX
removed review request for @pslaughter
requested review from @pslaughter
mentioned in merge request !96549 (merged)
enabled an automatic merge when the pipeline for 5f9b593f succeeds
mentioned in commit e21429e7
added workflowstaging-canary label
added workflowcanary label and removed workflowstaging-canary label
added workflowstaging label and removed workflowcanary label
added workflowproduction label and removed workflowstaging label
added workflowpost-deploy-db-staging label and removed workflowproduction label
added workflowpost-deploy-db-production label and removed workflowpost-deploy-db-staging label
added releasedcandidate label
mentioned in merge request kubitus-project/kubitus-installer!1453 (merged)
mentioned in merge request !99464 (merged)
added releasedpublished label and removed releasedcandidate label
