[Workspace] Add permission tab to workspace create update page (opens…

…earch-project#6378) * Allow workspace update with partial attirbutes Signed-off-by: Lin Wang <[email protected]> * Add permissions tab for workspace creator and update page Signed-off-by: Lin Wang <[email protected]> * Add change log for adding permission tab Signed-off-by: Lin Wang <[email protected]> * Optimize permissions to permissions settings convertation Signed-off-by: Lin Wang <[email protected]> * Address PR comments Signed-off-by: Lin Wang <[email protected]> * Update comments for getPermissionModeId Signed-off-by: Lin Wang <[email protected]> --------- Signed-off-by: Lin Wang <[email protected]>
wanglam · Apr 17, 2024 · ca14f60 · ca14f60
1 parent 32fab61
commit ca14f60
Show file tree

Hide file tree

Showing 23 changed files with 1,239 additions and 428 deletions.
diff --git a/src/plugins/workspace/common/constants.ts b/src/plugins/workspace/common/constants.ts
@@ -8,7 +8,7 @@ export const WORKSPACE_LIST_APP_ID = 'workspace_list';
 export const WORKSPACE_UPDATE_APP_ID = 'workspace_update';
 export const WORKSPACE_OVERVIEW_APP_ID = 'workspace_overview';
 // These features will be checked and disabled in checkbox on default.
-export const DEFAULT_CHECKED_FEATURES_IDS = [WORKSPACE_UPDATE_APP_ID, WORKSPACE_OVERVIEW_APP_ID];
+export const DEFAULT_SELECTED_FEATURES_IDS = [WORKSPACE_UPDATE_APP_ID, WORKSPACE_OVERVIEW_APP_ID];
 export const WORKSPACE_FATAL_ERROR_APP_ID = 'workspace_fatal_error';
 export const WORKSPACE_SAVED_OBJECTS_CLIENT_WRAPPER_ID = 'workspace';
 export const WORKSPACE_CONFLICT_CONTROL_SAVED_OBJECTS_CLIENT_WRAPPER_ID =

diff --git a/src/plugins/workspace/public/components/workspace_creator/workspace_creator.test.tsx b/src/plugins/workspace/public/components/workspace_creator/workspace_creator.test.tsx
@@ -182,15 +182,47 @@ describe('WorkspaceCreator', () => {
     expect(notificationToastsAddDanger).not.toHaveBeenCalled();
   });
 
+  it('should show danger toasts after create workspace failed', async () => {
+    workspaceClientCreate.mockReturnValueOnce({ result: { id: 'failResult' }, success: false });
+    const { getByTestId } = render(<WorkspaceCreator />);
+    const nameInput = getByTestId('workspaceForm-workspaceDetails-nameInputText');
+    fireEvent.input(nameInput, {
+      target: { value: 'test workspace name' },
+    });
+    fireEvent.click(getByTestId('workspaceForm-bottomBar-createButton'));
+    expect(workspaceClientCreate).toHaveBeenCalled();
+    await waitFor(() => {
+      expect(notificationToastsAddDanger).toHaveBeenCalled();
+    });
+    expect(notificationToastsAddSuccess).not.toHaveBeenCalled();
+  });
+
+  it('should show danger toasts after call create workspace API failed', async () => {
+    workspaceClientCreate.mockImplementationOnce(async () => {
+      throw new Error();
+    });
+    const { getByTestId } = render(<WorkspaceCreator />);
+    const nameInput = getByTestId('workspaceForm-workspaceDetails-nameInputText');
+    fireEvent.input(nameInput, {
+      target: { value: 'test workspace name' },
+    });
+    fireEvent.click(getByTestId('workspaceForm-bottomBar-createButton'));
+    expect(workspaceClientCreate).toHaveBeenCalled();
+    await waitFor(() => {
+      expect(notificationToastsAddDanger).toHaveBeenCalled();
+    });
+    expect(notificationToastsAddSuccess).not.toHaveBeenCalled();
+  });
+
   it('create workspace with customized permissions', async () => {
-    const { getByTestId, getByText } = render(<WorkspaceCreator />);
+    const { getByTestId, getByText, getAllByText } = render(<WorkspaceCreator />);
     const nameInput = getByTestId('workspaceForm-workspaceDetails-nameInputText');
     fireEvent.input(nameInput, {
       target: { value: 'test workspace name' },
     });
     fireEvent.click(getByText('Users & Permissions'));
     fireEvent.click(getByTestId('workspaceForm-permissionSettingPanel-user-addNew'));
-    const userIdInput = getByTestId('workspaceForm-permissionSettingPanel-0-userId');
+    const userIdInput = getAllByText('Select')[0];
     fireEvent.click(userIdInput);
     fireEvent.input(getByTestId('comboBoxSearchInput'), {
       target: { value: 'test user id' },
@@ -215,19 +247,4 @@ describe('WorkspaceCreator', () => {
     });
     expect(notificationToastsAddDanger).not.toHaveBeenCalled();
   });
-
-  it('should show danger toasts after create workspace failed', async () => {
-    workspaceClientCreate.mockReturnValue({ result: { id: 'failResult' }, success: false });
-    const { getByTestId } = render(<WorkspaceCreator />);
-    const nameInput = getByTestId('workspaceForm-workspaceDetails-nameInputText');
-    fireEvent.input(nameInput, {
-      target: { value: 'test workspace name' },
-    });
-    fireEvent.click(getByTestId('workspaceForm-bottomBar-createButton'));
-    expect(workspaceClientCreate).toHaveBeenCalled();
-    await waitFor(() => {
-      expect(notificationToastsAddDanger).toHaveBeenCalled();
-    });
-    expect(notificationToastsAddSuccess).not.toHaveBeenCalled();
-  });
 });
diff --git a/src/plugins/workspace/public/components/workspace_creator/workspace_creator.tsx b/src/plugins/workspace/public/components/workspace_creator/workspace_creator.tsx
@@ -11,14 +11,14 @@ import { WorkspaceForm, WorkspaceFormSubmitData, WorkspaceOperationType } from '
 import { WORKSPACE_OVERVIEW_APP_ID } from '../../../common/constants';
 import { formatUrlWithWorkspaceId } from '../../../../../core/public/utils';
 import { WorkspaceClient } from '../../workspace_client';
-import { convertPermissionSettingsToPermissions } from '../workspace_form/utils';
+import { convertPermissionSettingsToPermissions } from '../workspace_form';
 
 export const WorkspaceCreator = () => {
   const {
     services: { application, notifications, http, workspaceClient },
   } = useOpenSearchDashboards<{ workspaceClient: WorkspaceClient }>();
-
   const isPermissionEnabled = application?.capabilities.workspaces.permissionEnabled;
+
   const handleWorkspaceFormSubmit = useCallback(
     async (data: WorkspaceFormSubmitData) => {
       let result;
@@ -28,6 +28,29 @@ export const WorkspaceCreator = () => {
           attributes,
           convertPermissionSettingsToPermissions(permissionSettings)
         );
+        if (result?.success) {
+          notifications?.toasts.addSuccess({
+            title: i18n.translate('workspace.create.success', {
+              defaultMessage: 'Create workspace successfully',
+            }),
+          });
+          if (application && http) {
+            const newWorkspaceId = result.result.id;
+            // Redirect page after one second, leave one second time to show create successful toast.
+            window.setTimeout(() => {
+              window.location.href = formatUrlWithWorkspaceId(
+                application.getUrlForApp(WORKSPACE_OVERVIEW_APP_ID, {
+                  absolute: true,
+                }),
+                newWorkspaceId,
+                http.basePath
+              );
+            }, 1000);
+          }
+          return;
+        } else {
+          throw new Error(result?.error ? result?.error : 'create workspace failed');
+        }
       } catch (error) {
         notifications?.toasts.addDanger({
           title: i18n.translate('workspace.create.failed', {
@@ -37,33 +60,6 @@ export const WorkspaceCreator = () => {
         });
         return;
       }
-      if (result?.success) {
-        notifications?.toasts.addSuccess({
-          title: i18n.translate('workspace.create.success', {
-            defaultMessage: 'Create workspace successfully',
-          }),
-        });
-        if (application && http) {
-          const newWorkspaceId = result.result.id;
-          // Redirect page after one second, leave one second time to show create successful toast.
-          window.setTimeout(() => {
-            window.location.href = formatUrlWithWorkspaceId(
-              application.getUrlForApp(WORKSPACE_OVERVIEW_APP_ID, {
-                absolute: true,
-              }),
-              newWorkspaceId,
-              http.basePath
-            );
-          }, 1000);
-        }
-        return;
-      }
-      notifications?.toasts.addDanger({
-        title: i18n.translate('workspace.create.failed', {
-          defaultMessage: 'Failed to create workspace',
-        }),
-        text: result?.error,
-      });
     },
     [notifications?.toasts, http, application, workspaceClient]
   );

diff --git a/src/plugins/workspace/public/components/workspace_form/constants.ts b/src/plugins/workspace/public/components/workspace_form/constants.ts
@@ -6,6 +6,17 @@
 import { i18n } from '@osd/i18n';
 import { WorkspacePermissionMode } from '../../../common/constants';
 
+export enum WorkspaceOperationType {
+  Create = 'create',
+  Update = 'update',
+}
+
+export enum WorkspaceFormTabs {
+  NotSelected,
+  FeatureVisibility,
+  UsersAndPermissions,
+}
+
 export enum WorkspacePermissionItemType {
   User = 'user',
   Group = 'group',
@@ -51,14 +62,3 @@ export const optionIdToWorkspacePermissionModesMap: {
   ],
   [PermissionModeId.Admin]: [WorkspacePermissionMode.LibraryWrite, WorkspacePermissionMode.Write],
 };
-
-export enum WorkspaceOperationType {
-  Create = 'create',
-  Update = 'update',
-}
-
-export enum WorkspaceFormTabs {
-  NotSelected,
-  FeatureVisibility,
-  UsersAndPermissions,
-}
diff --git a/src/plugins/workspace/public/components/workspace_form/index.ts b/src/plugins/workspace/public/components/workspace_form/index.ts
@@ -6,3 +6,7 @@
 export { WorkspaceForm } from './workspace_form';
 export { WorkspaceFormSubmitData } from './types';
 export { WorkspaceOperationType } from './constants';
+export {
+  convertPermissionsToPermissionSettings,
+  convertPermissionSettingsToPermissions,
+} from './utils';
diff --git a/src/plugins/workspace/public/components/workspace_form/types.ts b/src/plugins/workspace/public/components/workspace_form/types.ts
@@ -3,21 +3,29 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
-import type { WorkspacePermissionItemType, WorkspaceOperationType } from './constants';
-import type { WorkspacePermissionMode } from '../../../common/constants';
 import type { ApplicationStart } from '../../../../../core/public';
+import type { WorkspacePermissionMode } from '../../../common/constants';
+import type { WorkspaceOperationType, WorkspacePermissionItemType } from './constants';
 
 export type WorkspacePermissionSetting =
-  | { type: WorkspacePermissionItemType.User; userId: string; modes: WorkspacePermissionMode[] }
-  | { type: WorkspacePermissionItemType.Group; group: string; modes: WorkspacePermissionMode[] };
+  | {
+      id: number;
+      type: WorkspacePermissionItemType.User;
+      userId: string;
+      modes: WorkspacePermissionMode[];
+    }
+  | {
+      id: number;
+      type: WorkspacePermissionItemType.Group;
+      group: string;
+      modes: WorkspacePermissionMode[];
+    };
 
 export interface WorkspaceFormSubmitData {
   name: string;
   description?: string;
   features?: string[];
   color?: string;
-  icon?: string;
-  defaultVISTheme?: string;
   permissionSettings?: WorkspacePermissionSetting[];
 }
 
@@ -36,11 +44,10 @@ export interface WorkspaceFeatureGroup {
   features: WorkspaceFeature[];
 }
 
-export type WorkspaceFormErrors = Omit<
-  { [key in keyof WorkspaceFormData]?: string },
-  'permissions'
-> & {
-  permissions?: string[];
+export type WorkspaceFormErrors = {
+  [key in keyof Omit<WorkspaceFormData, 'permissionSettings'>]?: string;
+} & {
+  permissionSettings?: { [key: number]: string };
 };
 
 export interface WorkspaceFormProps {

diff --git a/src/plugins/workspace/public/components/workspace_form/use_workspace_form.ts b/src/plugins/workspace/public/components/workspace_form/use_workspace_form.ts
@@ -5,19 +5,12 @@
 
 import { useCallback, useState, FormEventHandler, useRef, useMemo, useEffect } from 'react';
 import { htmlIdGenerator, EuiFieldTextProps, EuiColorPickerProps } from '@elastic/eui';
-import { i18n } from '@osd/i18n';
 import { useApplications } from '../../hooks';
 import { featureMatchesConfig } from '../../utils';
 
-import { WorkspacePermissionItemType, WorkspaceFormTabs } from './constants';
-import { WorkspacePermissionSetting, WorkspaceFormProps, WorkspaceFormErrors } from './types';
-import {
-  appendDefaultFeatureIds,
-  getNumberOfErrors,
-  isUserOrGroupPermissionSettingDuplicated,
-  isValidNameOrDescription,
-  isValidWorkspacePermissionSetting,
-} from './utils';
+import { WorkspaceFormTabs } from './constants';
+import { WorkspaceFormProps, WorkspaceFormErrors, WorkspacePermissionSetting } from './types';
+import { appendDefaultFeatureIds, getNumberOfErrors, validateWorkspaceForm } from './utils';
 
 const workspaceHtmlIdGenerator = htmlIdGenerator();
 
@@ -28,7 +21,6 @@ export const useWorkspaceForm = ({ application, defaultValues, onSubmit }: Works
   const [color, setColor] = useState(defaultValues?.color);
 
   const [selectedTab, setSelectedTab] = useState(WorkspaceFormTabs.FeatureVisibility);
-  const [numberOfErrors, setNumberOfErrors] = useState(0);
   // The matched feature id list based on original feature config,
   // the feature category will be expanded to list of feature ids
   const defaultFeatures = useMemo(() => {
@@ -44,14 +36,15 @@ export const useWorkspaceForm = ({ application, defaultValues, onSubmit }: Works
     appendDefaultFeatureIds(defaultFeatures)
   );
   const [permissionSettings, setPermissionSettings] = useState<
-    Array<Partial<WorkspacePermissionSetting>>
+    Array<Pick<WorkspacePermissionSetting, 'id'> & Partial<WorkspacePermissionSetting>>
   >(
     defaultValues?.permissionSettings && defaultValues.permissionSettings.length > 0
       ? defaultValues.permissionSettings
       : []
   );
 
   const [formErrors, setFormErrors] = useState<WorkspaceFormErrors>({});
+  const numberOfErrors = useMemo(() => getNumberOfErrors(formErrors), [formErrors]);
   const formIdRef = useRef<string>();
   const getFormData = () => ({
     name,
@@ -70,84 +63,10 @@ export const useWorkspaceForm = ({ application, defaultValues, onSubmit }: Works
   const handleFormSubmit = useCallback<FormEventHandler>(
     (e) => {
       e.preventDefault();
-      let currentFormErrors: WorkspaceFormErrors = {};
       const formData = getFormDataRef.current();
-      if (!formData.name) {
-        currentFormErrors = {
-          ...currentFormErrors,
-          name: i18n.translate('workspace.form.detail.name.empty', {
-            defaultMessage: "Name can't be empty.",
-          }),
-        };
-      }
-      if (!isValidNameOrDescription(formData.name)) {
-        currentFormErrors = {
-          ...currentFormErrors,
-          name: i18n.translate('workspace.form.detail.name.invalid', {
-            defaultMessage: 'Invalid workspace name',
-          }),
-        };
-      }
-      if (!isValidNameOrDescription(formData.description)) {
-        currentFormErrors = {
-          ...currentFormErrors,
-          description: i18n.translate('workspace.form.detail.description.invalid', {
-            defaultMessage: 'Invalid workspace description',
-          }),
-        };
-      }
-      const permissionErrors: string[] = new Array(formData.permissionSettings.length);
-      for (let i = 0; i < formData.permissionSettings.length; i++) {
-        const permission = formData.permissionSettings[i];
-        if (isValidWorkspacePermissionSetting(permission)) {
-          if (
-            isUserOrGroupPermissionSettingDuplicated(
-              formData.permissionSettings.slice(0, i),
-              permission
-            )
-          ) {
-            permissionErrors[i] = i18n.translate('workspace.form.permission.invalidate.group', {
-              defaultMessage: 'Duplicate permission setting',
-            });
-            continue;
-          }
-          continue;
-        }
-        if (!permission.type) {
-          permissionErrors[i] = i18n.translate('workspace.form.permission.invalidate.type', {
-            defaultMessage: 'Invalid type',
-          });
-          continue;
-        }
-        if (!permission.modes || permission.modes.length === 0) {
-          permissionErrors[i] = i18n.translate('workspace.form.permission.invalidate.modes', {
-            defaultMessage: 'Invalid permission modes',
-          });
-          continue;
-        }
-        if (permission.type === WorkspacePermissionItemType.User && !permission.userId) {
-          permissionErrors[i] = i18n.translate('workspace.form.permission.invalidate.userId', {
-            defaultMessage: 'Invalid userId',
-          });
-          continue;
-        }
-        if (permission.type === WorkspacePermissionItemType.Group && !permission.group) {
-          permissionErrors[i] = i18n.translate('workspace.form.permission.invalidate.group', {
-            defaultMessage: 'Invalid user group',
-          });
-          continue; // this line is need for more conditions
-        }
-      }
-      if (permissionErrors.some((error) => !!error)) {
-        currentFormErrors = {
-          ...currentFormErrors,
-          permissions: permissionErrors,
-        };
-      }
-      const currentNumberOfErrors = getNumberOfErrors(currentFormErrors);
+      const currentFormErrors: WorkspaceFormErrors = validateWorkspaceForm(formData);
       setFormErrors(currentFormErrors);
-      setNumberOfErrors(currentNumberOfErrors);
-      if (currentNumberOfErrors > 0) {
+      if (getNumberOfErrors(currentFormErrors) > 0) {
         return;
       }
 
@@ -168,7 +87,7 @@ export const useWorkspaceForm = ({ application, defaultValues, onSubmit }: Works
       onSubmit?.({
         ...formData,
         name: formData.name!,
-        permissionSettings: formData.permissionSettings.filter(isValidWorkspacePermissionSetting),
+        permissionSettings: formData.permissionSettings as WorkspacePermissionSetting[],
       });
     },
     [defaultFeatures, onSubmit, defaultValues?.features]