fix: delete acknowledgments for project removal - #20531
Open
miketheman wants to merge 17 commits into
Open
miketheman wants to merge 17 commits into
miketheman wants to merge 17 commits into
Conversation
The login-and-TOTP dance is copied into every functional test that needs an authenticated user. Move the admin conftest's version up a level so the rest of tests/functional can reach it, and convert the manage project tests. The shared copy reuses the IpAddress row the request already inserted rather than colliding with it.
The checkboxes sat outside the modal and gated an anchor carrying a disabled attribute, which browsers ignore. Only `pointer-events: none` ever blocked the click, and that went away in pypi#14244, so the boxes have been decorative since. Rendering them inside the modal lets the existing confirm controller gate a real submit button, and delete_confirm can go. Wrapping each box in a label makes its text a click target and ties the two together for screen readers. confirm_button's additional_attributes is how disabled reached an anchor in the first place; nothing passes it now. Seven acknowledgments plus a warning and a text field need the room, so these modals opt into the modal--wide the stylesheet already implemented.
The checkboxes carried no name, so they never reached the server and a crafted POST could delete a project or release without them. Enforce them the way confirm_project already enforces the typed name. Three templates render these modals against the two views, so the functional tests assert each rendered form carries exactly the names the view checks.
Contributor
|
Hi @miketheman Wondering why the delete file modal on the release detail page is out of scope here? These checkboxes also appear not to carry a name: The modal styling is also different: 
|
The file delete modal on the release page had the same unnamed checkboxes as the project and release modals. Name them through a shared macro and have delete_project_release_file enforce them.
Member
Author
It wasn't explicitly out of scope, it was overlooked - thanks! |
This branch has not been deployed
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
2 participants
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 checkboxes carried no name, so they never reached the server and a
crafted POST could delete a project or release without them. Enforce them
the way confirm_project already enforces the typed name.
Three templates render these modals against the two views, so the functional
tests assert each rendered form carries exactly the names the view checks.
Includes other refactors - see each commit for easier review.