[Workspace] fix: Fix workspace page hanging with none collaborators for non dashboard admin #9004
Conversation
raintygao
requested review from
ananzh,
kavilla,
AMoo-Miki,
ashwin-pc,
joshuarrrr,
abbyhu2000,
zengyan-amazon,
zhongnansu,
manasvinibs,
ZilongX,
Flyingliuhub,
curq,
bandinib-amzn,
SuZhou-Joe,
ruanyl,
BionIT,
xinruiba,
zhyuanqi,
mengweieric,
LDrago27,
virajsanghvi,
sejli,
joshuali925 and
huyaboo
as code owners
raintygao
changed the title
raintygao
changed the title
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #9004 +/- ##
==========================================
- Coverage 60.87% 60.86% -0.02%
==========================================
Files 3808 3808
Lines 91209 91210 +1
Branches 14410 14410
==========================================
- Hits 55526 55517 -9
- Misses 32142 32153 +11
+ Partials 3541 3540 -1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
| export const EMPTY_PERMISSIONS = { | ||
| library_read: {}, | ||
| library_write: {}, | ||
| read: {}, | ||
| write: {}, | ||
| }; | ||
|
|
There was a problem hiding this comment.
Is this a WorkspacePermissionSetting object? If so can we add the typing so that typescript can better infer the return type of convertPermissionSettingsToPermissions?
We can also define this as readonly to ensure no consumer can modify this.
Ex.
export const EMPTY_PERMISSIONS: WorkspacePermissionSetting = {
library_read: {},
library_write: {},
read: {},
write: {},
} as const;
There was a problem hiding this comment.
Sure. Its type is SavedObjectPermissions, have updated.
| return undefined; | ||
| // Workspace object should always have permissions, set it as an empty object here instead of undefined. | ||
| return EMPTY_PERMISSIONS; | ||
| } |
There was a problem hiding this comment.
does it has any impact when permission control is turned off?
There was a problem hiding this comment.
I supposed it may not as this is a specific object for workspace object.
There was a problem hiding this comment.
when permission control is off, permission field will not initialized and add to kibana index, i am not sure it will cause any issue when try to save a object with empty permission, we can have a test
There was a problem hiding this comment.
This means the value of permission field will not be consistent between permission is on/off, right? So it seems updating client wrapper will be a better solution?
Signed-off-by: tygao <[email protected]>
Signed-off-by: tygao <[email protected]>
…tail page Signed-off-by: tygao <[email protected]>
raintygao
force-pushed
the
fix-empty-coll
branch
from
58e4cbb to
b9b7edd
Compare
Description
Fix workspace page hanging with none collaborators for non dashboard admin
Previously the permission field of a workspace with none collaborators is undefined, which will pass the workspace permission wrapper and cause page hanging. Actually this field could be an empty ACL object as workspace type object should always have this field.
Screenshot
Before
After
Testing the changes
Changelog
Check List
yarn test:jestyarn test:jest_integration