Skip to content
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

[graphql] add permissions field to WorkspaceLocationEntryStatus #21946

Conversation

alangenfeld
Copy link
Member

Fetching these off the workspace directly blocks them on loading the whole workspace, and in some environments that is costly.
Make it available off of the other code location scoped object for faster access.

How I Tested These Changes

updated test

Copy link
Member Author

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @alangenfeld and the rest of your teammates on Graphite Graphite

Comment on lines 34 to 59
query WorkspacePermissionsQuery {
locationStatusesOrError {
__typename
... on WorkspaceLocationStatusEntries {
entries {
permissions {
permission
value
disabledReason
}
}
}
}
}
Copy link
Member Author

Choose a reason for hiding this comment

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

can update this to just test both

Comment on lines +157 to +159
def resolve_permissions(self, graphene_info):
permissions = graphene_info.context.permissions_for_location(location_name=self.name)
return [GraphenePermission(permission, value) for permission, value in permissions.items()]
Copy link
Member Author

Choose a reason for hiding this comment

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

should i deprecate or remove the other one?

Copy link
Member

Choose a reason for hiding this comment

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

I could imagine a world in which the current one is being accessed, I have seen people asking about programatic access to the users page in cloud for example

@alangenfeld alangenfeld force-pushed the al/05-17-_graphql_add_permissions_field_to_workspacelocationentrystatus branch from e10d2db to 8ee84d7 Compare May 17, 2024 21:45
Copy link
Member

@gibsondan gibsondan left a comment

Choose a reason for hiding this comment

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

testing both seems prudent, if only for push safety

Comment on lines +157 to +159
def resolve_permissions(self, graphene_info):
permissions = graphene_info.context.permissions_for_location(location_name=self.name)
return [GraphenePermission(permission, value) for permission, value in permissions.items()]
Copy link
Member

Choose a reason for hiding this comment

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

I could imagine a world in which the current one is being accessed, I have seen people asking about programatic access to the users page in cloud for example

@alangenfeld alangenfeld force-pushed the al/05-17-_graphql_add_permissions_field_to_workspacelocationentrystatus branch from 8ee84d7 to 4dc6e70 Compare May 22, 2024 17:13
@alangenfeld alangenfeld force-pushed the al/05-17-_graphql_add_permissions_field_to_workspacelocationentrystatus branch from 4dc6e70 to 8894587 Compare May 22, 2024 17:20
@alangenfeld alangenfeld merged commit b387a1c into master May 22, 2024
1 of 2 checks passed
@alangenfeld alangenfeld deleted the al/05-17-_graphql_add_permissions_field_to_workspacelocationentrystatus branch May 22, 2024 17:53
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.

None yet

2 participants