enterprise grid support #11
Conversation
| @@ -102,7 +116,7 @@ func (o *workspaceResourceType) Entitlements(ctx context.Context, resource *v2.R | |||
| } | |||
|
|
|||
| func (o *workspaceResourceType) Grants(ctx context.Context, resource *v2.Resource, pt *pagination.Token) ([]*v2.Grant, string, annotations.Annotations, error) { | |||
| users, err := o.client.GetUsersContext(ctx) | |||
| users, err := o.client.GetUsersContext(ctx, slack.GetUsersOptionTeamID(resource.Id.Resource)) | |||
There was a problem hiding this comment.
If we're not an org client does this still work? Does passing the param where it's not needed "just work" ?
There was a problem hiding this comment.
yep it will ignore it :)
| @@ -84,7 +100,7 @@ func (o *userGroupResourceType) Entitlements(ctx context.Context, resource *v2.R | |||
| } | |||
|
|
|||
| func (o *userGroupResourceType) Grants(ctx context.Context, resource *v2.Resource, pt *pagination.Token) ([]*v2.Grant, string, annotations.Annotations, error) { | |||
| groupMembers, err := o.client.GetUserGroupMembers(resource.Id.Resource) | |||
| groupMembers, err := o.enterpriseClient.GetUserGroupMembers(ctx, resource.Id.Resource, resource.ParentResourceId.Resource) | |||
There was a problem hiding this comment.
Will this always work or does it need treatment like the code on lines 61-72 checking o.enterpriseId? Does the enterprise client work for non-enterprise workspaces?
There was a problem hiding this comment.
yeah it works even for non-enterprise workspaces, however I should probably rename it, I initially created it to work with enterprise stuff, but turned out I needed to add more web api stuff in here as well
| if !ok { | ||
| return nil, fmt.Errorf("invalid system or organization roleID: %s", roleID) | ||
| } else { | ||
| roleName = orgRoleName | ||
| } |
There was a problem hiding this comment.
| if !ok { | |
| return nil, fmt.Errorf("invalid system or organization roleID: %s", roleID) | |
| } else { | |
| roleName = orgRoleName | |
| } | |
| if !ok { | |
| return nil, fmt.Errorf("invalid system or organization roleID: %s", roleID) | |
| } | |
| roleName = orgRoleName |
|
@loganintech sorry, I just got to the comments this morning. Glad it works as expected! Can I merge it in that case? I will do the fixes in following PR that will handle SCIM api and provisioning, along with any other stuff that might be needed for full enterprise grid experience :) |
|
No worries, time differences are a thing. The only reason I didn't merge it myself is if the were any other things you wanted to add or change before we did. The code looks good and my testing was good so if you're happy with it I say go go go. |
|
alright, in that case let's merge it since it has the fix for the customers, and I can add any additional stuff in follow up PR :) thank you! |
No description provided.