warehouse
warehouse copied to clipboard
pypi
Multi project token
This PR adds support for multi project tokens. The backend was mostly ready for this it seems, I just had to make the forms and other UI components compatible wit it.
Thanks to @ip4368 for the help with the CSS so that the multi select doesn't look as if it were from the 90s.
Screen capture: https://www.dropbox.com/s/ukyqp5ag0ocrbd2/multi-proj-token.mov?dl=0
Closes: #6292
CI seems to have some transient issues with apt get but can't find a way of retrying it myself :/
HI @Sterbic - thank you for this PR!
I have a suggestion regarding the user interface. Instead of throwing an error when a user makes a mistake with the token scope, we can prevent it by not allowing users to select their entire account and a project at the same time.
I was thinking something like this would work well:
For accounts with fewer than 10 projects
- No radio selected by default
- Checkboxes disabled unless "by project" is selected.
For accounts with more than 10 projects
- No radio selected by default
- Select disabled unless "by project" is selected
Thanks @nlhkabu, that design looks pretty neat! Would you be ok with it going into a separate pull request since it would be touching only the UI? This PR has all the backend logic and tests so I'd rather merge it as is so it's easier to review the UI changes afterwards.
Hi @Sterbic - my strong preference is to include the UI improvements in this PR, rather than ship as is and fix it later. My reason: there is no guarantee we'll actually "fix it later".
Did you want some help with the HTML/CSS/JS here?
Fair enough, I'll amend this PR. Is there a similar example in the codebase for such a UI? I have zero JS experience tbh.
Agreed w/ @nlhkabu.
As an alternative to adding the commits here, someone else could create a PR against your branch.
We use Stimulus for our JS framework, this should be a pretty straightforward controller.
Instead of throwing an error when a user makes a mistake with the token scope, we can prevent it by not allowing users to select their entire account and a project at the same time.
Just a nitpick: we still need to perform this validation on the backend, since a user could use curl or another client to bypass the client-side restriction on what they're allowed to select. But it'll be good to also have it on the frontend :smile:
@Sterbic do you want to have a go at the JavaScript for this or should I tackle it myself?
Have been a bit swamped at work since I started working on this at PyCon AU but definitely want to finish it.
Note: If / when this PR is re-reviewed, we will need to ensure that the template is appropriately translated.
@yeraydiazdiaz, if you want to take a stab at the JS part go for it. I have a fire at work and won't have any cycles to spend learning JS any time soon 😭 I can rebase this PR on master if it helps.
Hey @Sterbic no worries, I can rebase myself and take it from here. Thanks so much for starting this PR 🌟
@Sterbic, I created a PR on your fork merging master onto your branch to bring this PR up to date. I was planning on creating an additional one with the JavaScripts changes as suggested by @di.
If you don't have time or don't want the noise let me know. 🙂
I've implemented the changes suggested by @nlhkabu, here's the capture using checkboxes:
And another using multi-select, note this should only happen if the number of projects it greater than 10, I changed the code in the template to use it in this example:
Once the PR in @Sterbic's repo is merged this should be ready for review.
@Sterbic has merged https://github.com/Sterbic/warehouse/pull/2 so this is now ready for review.
#Triage This PR currently has merge conflicts that would need to be addressed before proceeding.
@Sterbic are you still interested in pursuing this effort? It's okay if not!
