address PR comments and add annotations

Signed-off-by: Lin Wang <[email protected]>
wanglam · Sep 25, 2023 · d8aa1d1 · d8aa1d1
1 parent eb5629d
commit d8aa1d1
Show file tree

Hide file tree

Showing 2 changed files with 32 additions and 30 deletions.
diff --git a/...gins/workspace/public/components/workspace_creator/workspace_permission_setting_panel.tsx b/...gins/workspace/public/components/workspace_creator/workspace_permission_setting_panel.tsx
@@ -25,12 +25,7 @@ export type WorkspacePermissionSetting = (
   type: 'user' | 'group';
   userId?: string;
   group?: string;
-  modes: Array<
-    | WorkspacePermissionMode.LibraryRead
-    | WorkspacePermissionMode.LibraryWrite
-    | WorkspacePermissionMode.Read
-    | WorkspacePermissionMode.Write
-  >;
+  modes: WorkspacePermissionMode[];
 };
 
 enum PermissionModeId {
@@ -84,12 +79,7 @@ interface WorkspacePermissionSettingInputProps {
   type?: 'user' | 'group';
   userId?: string;
   group?: string;
-  modes?: Array<
-    | WorkspacePermissionMode.LibraryRead
-    | WorkspacePermissionMode.LibraryWrite
-    | WorkspacePermissionMode.Read
-    | WorkspacePermissionMode.Write
-  >;
+  modes?: WorkspacePermissionMode[];
   onTypeChange: (type: 'user' | 'group', index: number) => void;
   onGroupOrUserIdChange: (
     groupOrUserId:
@@ -99,12 +89,7 @@ interface WorkspacePermissionSettingInputProps {
     index: number
   ) => void;
   onPermissionModesChange: (
-    WorkspacePermissionMode: Array<
-      | WorkspacePermissionMode.LibraryRead
-      | WorkspacePermissionMode.LibraryWrite
-      | WorkspacePermissionMode.Read
-      | WorkspacePermissionMode.Write
-    >,
+    WorkspacePermissionMode: WorkspacePermissionMode[],
     index: number
   ) => void;
   onDelete: (index: number) => void;
@@ -239,13 +224,18 @@ export const WorkspacePermissionSettingPanel = ({
   onChange,
   firstRowDeletable,
 }: WorkspacePermissionSettingPanelProps) => {
-  const transferredValue = useMemo(() => {
+  const transformedValue = useMemo(() => {
     if (!value) {
       return [];
     }
     const result: Array<Partial<WorkspacePermissionSetting>> = [];
+    /**
+     * One workspace permission setting may includes multi setting options,
+     * for loop the workspace permission setting array to separate it to multi rows.
+     **/
     for (let i = 0; i < value.length; i++) {
       const valueItem = value[i];
+      // Incomplete workspace permission setting don't need to separate to multi rows
       if (
         !valueItem.modes ||
         !valueItem.type ||
@@ -255,6 +245,11 @@ export const WorkspacePermissionSettingPanel = ({
         result.push(valueItem);
         continue;
       }
+      /**
+       * For loop the option id to workspace permission modes map,
+       * if one settings includes all permission modes in a specific option,
+       * add these permission modes to the result array.
+       */
       for (const key in optionIdToWorkspacePermissionModesMap) {
         if (!Object.prototype.hasOwnProperty.call(optionIdToWorkspacePermissionModesMap, key)) {
           continue;
@@ -269,58 +264,58 @@ export const WorkspacePermissionSettingPanel = ({
   }, [value]);
 
   const handleAddNewOne = useCallback(() => {
-    onChange?.([...(transferredValue ?? []), {}]);
-  }, [onChange, transferredValue]);
+    onChange?.([...(transformedValue ?? []), {}]);
+  }, [onChange, transformedValue]);
 
   const handleDelete = useCallback(
     (index: number) => {
-      onChange?.((transferredValue ?? []).filter((_item, itemIndex) => itemIndex !== index));
+      onChange?.((transformedValue ?? []).filter((_item, itemIndex) => itemIndex !== index));
     },
-    [onChange, transferredValue]
+    [onChange, transformedValue]
   );
 
   const handlePermissionModesChange = useCallback<
     WorkspacePermissionSettingInputProps['onPermissionModesChange']
   >(
     (modes, index) => {
       onChange?.(
-        (transferredValue ?? []).map((item, itemIndex) =>
+        (transformedValue ?? []).map((item, itemIndex) =>
           index === itemIndex ? { ...item, modes } : item
         )
       );
     },
-    [onChange, transferredValue]
+    [onChange, transformedValue]
   );
 
   const handleTypeChange = useCallback<WorkspacePermissionSettingInputProps['onTypeChange']>(
     (type, index) => {
       onChange?.(
-        (transferredValue ?? []).map((item, itemIndex) =>
+        (transformedValue ?? []).map((item, itemIndex) =>
           index === itemIndex ? { ...item, type } : item
         )
       );
     },
-    [onChange, transferredValue]
+    [onChange, transformedValue]
   );
 
   const handleGroupOrUserIdChange = useCallback<
     WorkspacePermissionSettingInputProps['onGroupOrUserIdChange']
   >(
     (userOrGroupIdWithType, index) => {
       onChange?.(
-        (transferredValue ?? []).map((item, itemIndex) =>
+        (transformedValue ?? []).map((item, itemIndex) =>
           index === itemIndex
             ? { ...userOrGroupIdWithType, ...(item.modes ? { modes: item.modes } : {}) }
             : item
         )
       );
     },
-    [onChange, transferredValue]
+    [onChange, transformedValue]
   );
 
   return (
     <EuiDescribedFormGroup title={<h3>Users, User Groups & Groups</h3>}>
-      {transferredValue?.map((item, index) => (
+      {transformedValue?.map((item, index) => (
         <React.Fragment key={generateWorkspacePermissionItemKey(item, index)}>
           <EuiFormRow isInvalid={!!errors?.[index]} error={errors?.[index]}>
             <WorkspacePermissionSettingInput

diff --git a/src/plugins/workspace/server/saved_objects/workspace_saved_objects_client_wrapper.ts b/src/plugins/workspace/server/saved_objects/workspace_saved_objects_client_wrapper.ts
@@ -357,6 +357,13 @@ export class WorkspaceSavedObjectsClientWrapper {
             perPage: 999,
             ACLSearchParams: {
               principals,
+              /**
+               * The permitted workspace ids will be passed to the options.workspaces
+               * or options.ACLSearchParams.workspaces. These two were indicated the saved
+               * objects data inner specific workspaces. We use Library related permission here.
+               * For outside passed permission modes, it may contains other permissions. Add a intersection
+               * here to make sure only Library related permission modes will be used.
+               */
               permissionModes: options.ACLSearchParams.permissionModes
                 ? intersection(options.ACLSearchParams.permissionModes, [
                     WorkspacePermissionMode.LibraryRead,