Repository navigation
Show the remove-duplicate button to non-admins as disabled - #5503
Merged
ViniciustCosta merged 3 commits intoSep 30, 2026
Merged
ViniciustCosta merged 3 commits into
ViniciustCosta merged 3 commits into
Conversation
The button that removes a testcase's duplicate status was wrapped in a dom-if on info.is_admin, so non-admins saw nothing at all and had no way to tell the functionality existed. Render it unconditionally and disable it for non-admins instead, using the existing disabled$ idiom. The tooltip moves onto a wrapping span so it describes the action as admin-only for admins and non-admins alike: paper-icon-button sets pointer-events: none while disabled, so a title on the button itself would never surface for the readers who most need it. Server-side enforcement is unchanged; remove_duplicate.Handler.post still requires check_admin_access. Addresses google#5452.
Collaborator
|
/gcbrun |
dylanjew
reviewed
Sep 29, 2026
dylanjew
left a comment
dylanjew
left a comment
Collaborator
There was a problem hiding this comment.
Thanks, appreciate you picking this up! I pushed this to our dev environment and confirmed the button is disabled but still shows up for non-admins.
LGTM, but want to get approval from the core GOSST team.
Collaborator
Verified the tooltip shows up correctly |
dylanjew
added
the
ready for GOSST review
Contributor
Author
Looks as expected |
Collaborator
|
/gcbrun |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The "remove duplicate status" button in the testcase detail page is wrapped in a
dom-ifoninfo.is_admin, so non-admins see nothing at all and cannot tell the functionality exists (#5452).This renders the button unconditionally and disables it for non-admins instead, using the
disabled$idiom already used elsewhere in the components. The tooltip moves onto a wrappingspanso it describes the action as admin-only for admins and non-admins alike:paper-icon-buttonsetspointer-events: nonewhile disabled, so atitleon the button itself would never surface for non-admins, who are exactly the readers the text is for.Server-side authorization is unchanged:
remove_duplicate.Handler.poststill requirescheck_admin_access.This covers the first two points of the issue. Whether removing a false dedup should be an admin-only action at all is a product decision and is left for maintainers.
Verified by running
butler.py run_serverlocally and confirmingpolymer_bundlerregeneratessrc/appengine/templates/testcase-detail.htmlwith the new markup.Addresses #5452.