Fix adding multiple users to buckets and objects #151
Conversation
Giving permissions to one user will no longer remove other users without permissions from the permissions list. For objects and buckets When user is added to permissions, they automatically have read perms Modified files: BucketPermission.vue BucketPermissionAddUser.vue ObjectPermission.vue ObjectPermissionAddUser.vue permissionStore.ts
wilwong89
requested review from
jujaga,
kyle1morel,
TimCsaky,
jatindersingh93 and
norrisng-bc
as code owners
Coverage Report (Application)
|
Coverage Report (Frontend)
|
There was a problem hiding this comment.
minor comments.
i guess the issue with empty rows being removed is that after a checbox is selected, the datatable always resets to whatever permissions are in the store.
Not sure how to solve that, maybe an array of added users (without a ticked permission) could be added to the component's local state and that is merged into the datatable data.
| const afterPerms = [...state.mappedObjectToUserPermissions.value]; | ||
|
|
||
| const results = initialPerms.filter(({ userId: id1 }) => !afterPerms.some(({ userId: id2 }) => id2 === id1)); | ||
| state.mappedObjectToUserPermissions.value = state.mappedObjectToUserPermissions.value.concat(results); |
There was a problem hiding this comment.
i wonder if this before and after comparison is necessary. i think line 91 await permissionService.objectAddPermissions(objectId, [{ userId, permCode: Permissions.READ }]); returns the full details of the permission added
There was a problem hiding this comment.
The data returned by line 91 does not return the full details, it returns a different object that is slightly different. For example, the data returned does not have the name or email fields and if the previous user permission did not already have the READ perm code and another (non-READ) perm code is added, the data returned suggests that only READ was updated.
| @@ -293,13 +309,23 @@ export const usePermissionStore = defineStore('permission', () => { | |||
| } catch (error: any) { | |||
| toast.error('Removing bucket user', error); | |||
| } finally { | |||
| await fetchBucketPermissions(); | |||
| await mapBucketToUserPermissions(bucketId); | |||
| await removeBucketUserPermsHandler(bucketId, userId); | |||
There was a problem hiding this comment.
same with these.. not sure if you need the new handler function.. maybe you can drop this permission from the store here.
line 307:
deletedPermission = await permissionService.bucketDeletePermission(bucketId, { userId, permCode: value });
state.mappedBucketToUserPermissions.value = state.mappedBucketToUserPermissions.value.filter(p => p.id !== deletedPermission.id)There was a problem hiding this comment.
Using your method to read the deleted permission and then dropping it from the local state requires storing an array of deleted permissions returned form line, iteratively checking that list and removing to remove the permissions from the local state, and then finally another check to see if all permissions were removed from an entry in the local list before removing the entry entirely. I think instead the current method of doing a simple diff from before and after is the better method.
Description
Giving permissions to one user will no longer remove other users without permissions from the permissions list. For objects and buckets
When user is added to permissions, they automatically have read perms
Modified files:
BucketPermission.vue
BucketPermissionAddUser.vue
ObjectPermission.vue
ObjectPermissionAddUser.vue
permissionStore.ts
Types of changes
Bug fix (non-breaking change which fixes an issue)
Checklist
Further comments