Skip to content

Conversation

@charliepark
Copy link
Contributor

@charliepark charliepark commented Dec 2, 2025

This PR splits the Silo Access and Project Access pages into tabs, so a viewer can see the roles broken down, showing just Users, just Groups, or All with access (and their respective roles). This also updates the forms on each tab's page, to show just "Add user", "Add group", or "Add user or group", depending on what tab it's shown on.

This is part of #2887, and will be useful for more detailed views of group member counts, user time_modified values (as in oxidecomputer/omicron#9494), more comprehensive lists of users, etc. I wanted to get these changes in first, though, to keep future PRs tighter.

Screenshot 2025-12-07 at 4 21 43 PM Screenshot 2025-12-07 at 4 21 33 PM

The forms are slightly modified to make them more contextually-relevant as well. (Ignore the weird scaling difference due to the screenshot dimensions.)

Screenshot 2025-12-08 at 9 20 29 AM Screenshot 2025-12-08 at 9 20 42 AM

@vercel
Copy link

vercel bot commented Dec 2, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Review Updated (UTC)
console Ready Ready Preview Dec 30, 2025 9:30pm

@charliepark
Copy link
Contributor Author

I wonder if, since we show GROUPS higher on the ALL list than USERS, if we should order the tabs as ALL | GROUPS | USERS

@charliepark
Copy link
Contributor Author

Here's the modal that shows the list of group members:
Screenshot 2025-12-12 at 10 16 29 AM

@charliepark
Copy link
Contributor Author

I backed out the change to show the modal and the members of the group, as well as the members count column, until we have the updated API with the members count in it, so we can lazy-load the members list and avoid an n+1 on the table.

@david-crespo
Copy link
Collaborator

Merged main to pull in #2998 and moved the eslint config change over to the oxlint config.

label={capitalize(entityLabel)}
required
control={form.control}
/>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be good to use placeholder to make this say "select a user" "select a group" as appropriate

Image

@david-crespo
Copy link
Collaborator

david-crespo commented Dec 30, 2025

Not sure we need the All tab if we have groups and users. I guess it can be nice to see them next to each other, but in practice I expect nearly all role assignments are on groups anyway — almost none are on users. Getting rid of the All tab would also eliminate the awkwardness of the "Add user or group" button, which feels redundant given the "Add user" and "Add group" buttons on the other tabs. I thought about just taking the button off the All tab, but that might be even worse.

@david-crespo
Copy link
Collaborator

Thinking more about it, I can see the All view is useful in the typical case of a small number of role assignments to groups or users, and the main reason I can think of for wanting the other tabs is if there are a lot of group assignments and you want to make sure the user assignments list is empty, or vice versa. It's kind of a stopgap for the lack of filtering — instead of tabs, you could imagine a single table with a type filter. This would actually be a good table to experiment with that in a future PR because there is no pagination in role policy responses.

This is a useful change but doesn't really have much to do with #2887 because in this view you can only see groups or users with explicit role assignments. If a user gets their roles through groups (which is the typical case), they will never appear in this view. That issue is about seeing the list of all groups or all users.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants