-
Notifications
You must be signed in to change notification settings - Fork 13
Sort IAM role assignments with groups first, then by name #1237
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
apiQueryClient.prefetchQuery('policyView', {}), | ||
// used to resolve user names | ||
apiQueryClient.prefetchQuery('userList', {}), | ||
apiQueryClient.prefetchQuery('groupList', {}), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is exactly the kind of bug I was concerned about in #1232
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tricky, tricky
apiQueryClient.prefetchQuery('projectPolicyView', { orgName, projectName }), | ||
// used to resolve user names | ||
apiQueryClient.prefetchQuery('userList', {}), | ||
apiQueryClient.prefetchQuery('groupList', {}), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this very much re-enforces the fact that this should only be a short term impl.
1610074
to
3b5265c
Compare
3b5265c
to
0d37bee
Compare
The Omicron commit I upgraded to is not merged yet, it's the latest commit on oxidecomputer/omicron#1830. Even if that PR changes further, the API spec probably won't. In any case I will wait until that's merged, then update this to point to omicron main, then merge. |
# Conflicts: # libs/api/__generated__/Api.ts # libs/api/__generated__/msw-handlers.ts # libs/api/__generated__/validate.ts
Closes #1221
Closes #1235