-
Notifications
You must be signed in to change notification settings - Fork 320
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
enterprise feature: project-level membership #8569
base: main
Are you sure you want to change the base?
Conversation
|
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.
Code + click test are looking good!
One thing I noticed is that non-admins still have access to some of the admin UI even though the page doesn't really work for them. For example, if I'm a project-level member, I can still access /w/${workspaceId}
even though I see an error.
Think it would be worthwhile to only define those routes for admins inside AppRouter
?
I also think we might be linking to some admin-only pages for members too. e.g. LogsOverageCard
, the session overage CTA, the default settings page redirect, and possibly a few more (I tried to audit the places I saw /w/${
in the app.
@@ -455,23 +431,71 @@ func (r *Resolver) addAdminMembership(ctx context.Context, workspaceId int, invi | |||
return &admin.ID, nil | |||
} | |||
|
|||
func (r *Resolver) DeleteAdminAssociation(ctx context.Context, obj interface{}, adminID int) (*int, error) { | |||
func (r *Resolver) isUserInWorkspaceReadOnly(ctx context.Context, workspaceID int) (*model.Workspace, error) { | |||
span, ctx := util.StartSpanFromContext(ctx, "isAdminInWorkspace", util.ResourceName("resolver.internal.auth")) |
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.
Nit, should we update the span name?
span, ctx := util.StartSpanFromContext(ctx, "isAdminInWorkspace", util.ResourceName("resolver.internal.auth")) | |
span, ctx := util.StartSpanFromContext(ctx, "isUserInWorkspace", util.ResourceName("resolver.internal.auth")) |
Summary
https://www.loom.com/share/8b4de7ae89b44e13ad0c88055f2bec13
How did you test this change?
Projects
resolverAre there any deployment considerations?
Does this work require review from our design team?