[Workspace]Refactor workspace form UI (#7133)

* Make create workspace and update workspace full width Signed-off-by: Lin Wang <[email protected]> * Refactor user permissions input Signed-off-by: Lin Wang <[email protected]> * Add workspace form call out Signed-off-by: Lin Wang <[email protected]> * Fix permissions input unit tests Signed-off-by: Lin Wang <[email protected]> * Update gaps Signed-off-by: Lin Wang <[email protected]> * Update error callout Signed-off-by: Lin Wang <[email protected]> * Update user permission current user and number of changes Signed-off-by: Lin Wang <[email protected]> * Fix changes Signed-off-by: Lin Wang <[email protected]> * Fix owner order Signed-off-by: Lin Wang <[email protected]> * Add ut for form error callout Signed-off-by: Lin Wang <[email protected]> * Fix unit tests in workspace Signed-off-by: Lin Wang <[email protected]> * Mark first user row required Signed-off-by: Lin Wang <[email protected]> * Update section title Signed-off-by: Lin Wang <[email protected]> * Add validate for owner missing Signed-off-by: Lin Wang <[email protected]> * Changeset file for PR #7133 created/updated * Fix unit tests for workspace form utils Signed-off-by: Lin Wang <[email protected]> * Fix unit tests for form error callout Signed-off-by: Lin Wang <[email protected]> * Add unit test for transfer current user placeholder Signed-off-by: Lin Wang <[email protected]> * Fix unit tests in workspace permission setting panel Signed-off-by: Lin Wang <[email protected]> * Fix unit test in useWorkspaceForm Signed-off-by: Lin Wang <[email protected]> * Add missing unit tests for workspace form utils Signed-off-by: Lin Wang <[email protected]> * Add unit tests for getNumberOfErrors Signed-off-by: Lin Wang <[email protected]> * Add more ut for workspace form error callout Signed-off-by: Lin Wang <[email protected]> * Fix error code Signed-off-by: Lin Wang <[email protected]> * Fix failed unit test Signed-off-by: Lin Wang <[email protected]> * Add back color picker Signed-off-by: Lin Wang <[email protected]> * Address UX comments Signed-off-by: Lin Wang <[email protected]> * Fix empty user no workspace owner Signed-off-by: Lin Wang <[email protected]> * Change to Associate data source Signed-off-by: Lin Wang <[email protected]> --------- Signed-off-by: Lin Wang <[email protected]> Co-authored-by: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com> (cherry picked from commit c5946b9) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
opensearch-project · Jul 12, 2024 · e1c12da · e1c12da
1 parent a2b10fc
commit e1c12da
Show file tree

Hide file tree

Showing 26 changed files with 1,462 additions and 513 deletions.
diff --git a/changelogs/fragments/7133.yml b/changelogs/fragments/7133.yml
@@ -0,0 +1,2 @@
+feat:
+- [Workspace] Refactor workspace form UI ([#7133](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/7133))
diff --git a/src/plugins/workspace/common/constants.ts b/src/plugins/workspace/common/constants.ts
@@ -182,3 +182,5 @@ export const WORKSPACE_USE_CASES = Object.freeze({
     ] as string[],
   },
 });
+
+export const CURRENT_USER_PLACEHOLDER = '%me%';
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
@@ -197,7 +197,13 @@ describe('WorkspaceCreator', () => {
         description: 'test workspace description',
         features: expect.arrayContaining(['use-case-observability']),
       }),
-      { dataSources: [], permissions: undefined }
+      {
+        dataSources: [],
+        permissions: {
+          library_write: { users: ['%me%'] },
+          write: { users: ['%me%'] },
+        },
+      }
     );
     await waitFor(() => {
       expect(notificationToastsAddSuccess).toHaveBeenCalled();
@@ -247,8 +253,8 @@ describe('WorkspaceCreator', () => {
     expect(notificationToastsAddSuccess).not.toHaveBeenCalled();
   });
 
-  it('create workspace with customized permissions', async () => {
-    const { getByTestId, getAllByText } = render(
+  it('create workspace with current user', async () => {
+    const { getByTestId } = render(
       <WorkspaceCreator
         workspaceConfigurableApps$={new BehaviorSubject([...PublicAPPInfoMap.values()])}
       />
@@ -259,12 +265,6 @@ describe('WorkspaceCreator', () => {
     });
     fireEvent.click(getByTestId('workspaceUseCase-observability'));
     fireEvent.click(getByTestId('workspaceForm-permissionSettingPanel-user-addNew'));
-    const userIdInput = getAllByText('Select')[0];
-    fireEvent.click(userIdInput);
-    fireEvent.input(getByTestId('comboBoxSearchInput'), {
-      target: { value: 'test user id' },
-    });
-    fireEvent.blur(getByTestId('comboBoxSearchInput'));
     fireEvent.click(getByTestId('workspaceForm-bottomBar-createButton'));
     expect(workspaceClientCreate).toHaveBeenCalledWith(
       expect.objectContaining({
@@ -273,11 +273,11 @@ describe('WorkspaceCreator', () => {
       {
         dataSources: [],
         permissions: {
-          read: {
-            users: ['test user id'],
+          write: {
+            users: ['%me%'],
           },
-          library_read: {
-            users: ['test user id'],
+          library_write: {
+            users: ['%me%'],
           },
         },
       }
@@ -313,7 +313,14 @@ describe('WorkspaceCreator', () => {
       }),
       {
         dataSources: ['id1'],
-        permissions: undefined,
+        permissions: {
+          library_write: {
+            users: ['%me%'],
+          },
+          write: {
+            users: ['%me%'],
+          },
+        },
       }
     );
     await waitFor(() => {

diff --git a/src/plugins/workspace/public/components/workspace_creator/workspace_creator.tsx b/src/plugins/workspace/public/components/workspace_creator/workspace_creator.tsx
@@ -4,7 +4,7 @@
  */
 
 import React, { useCallback } from 'react';
-import { EuiPage, EuiPageBody, EuiPageHeader, EuiPageContent, EuiSpacer } from '@elastic/eui';
+import { EuiPage, EuiPageBody, EuiPageHeader, EuiPageContent } from '@elastic/eui';
 import { i18n } from '@osd/i18n';
 import { useObservable } from 'react-use';
 import { BehaviorSubject, of } from 'rxjs';
@@ -80,21 +80,15 @@ export const WorkspaceCreator = (props: WorkspaceCreatorProps) => {
   );
 
   return (
-    <EuiPage paddingSize="none">
+    <EuiPage>
       <EuiPageBody>
-        <EuiPageHeader restrictWidth pageTitle="Create a workspace" />
-        <EuiSpacer />
+        <EuiPageHeader pageTitle="Create a workspace" />
         <EuiPageContent
           verticalPosition="center"
           horizontalPosition="center"
           paddingSize="none"
           color="subdued"
           hasShadow={false}
-          /**
-           * Since above EuiPageHeader has a maxWidth: 1000 style,
-           * add maxWidth: 1000 below to align with the above page header
-           **/
-          style={{ width: '100%', maxWidth: 1000 }}
         >
           {application && savedObjects && (
             <WorkspaceForm

diff --git a/src/plugins/workspace/public/components/workspace_form/constants.ts b/src/plugins/workspace/public/components/workspace_form/constants.ts
@@ -3,7 +3,6 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
-import { i18n } from '@osd/i18n';
 import { WorkspacePermissionMode } from '../../../common/constants';
 
 export enum WorkspaceOperationType {
@@ -19,33 +18,9 @@ export enum WorkspacePermissionItemType {
 export enum PermissionModeId {
   Read = 'read',
   ReadAndWrite = 'read+write',
-  Admin = 'admin',
+  Owner = 'owner',
 }
 
-export const permissionModeOptions = [
-  {
-    id: PermissionModeId.Read,
-    label: i18n.translate('workspace.form.permissionSettingPanel.permissionModeOptions.read', {
-      defaultMessage: 'Read',
-    }),
-  },
-  {
-    id: PermissionModeId.ReadAndWrite,
-    label: i18n.translate(
-      'workspace.form.permissionSettingPanel.permissionModeOptions.readAndWrite',
-      {
-        defaultMessage: 'Read & Write',
-      }
-    ),
-  },
-  {
-    id: PermissionModeId.Admin,
-    label: i18n.translate('workspace.form.permissionSettingPanel.permissionModeOptions.admin', {
-      defaultMessage: 'Admin',
-    }),
-  },
-];
-
 export const optionIdToWorkspacePermissionModesMap: {
   [key: string]: WorkspacePermissionMode[];
 } = {
@@ -54,5 +29,5 @@ export const optionIdToWorkspacePermissionModesMap: {
     WorkspacePermissionMode.LibraryWrite,
     WorkspacePermissionMode.Read,
   ],
-  [PermissionModeId.Admin]: [WorkspacePermissionMode.LibraryWrite, WorkspacePermissionMode.Write],
+  [PermissionModeId.Owner]: [WorkspacePermissionMode.LibraryWrite, WorkspacePermissionMode.Write],
 };
diff --git a/src/plugins/workspace/public/components/workspace_form/select_data_source_panel.tsx b/src/plugins/workspace/public/components/workspace_form/select_data_source_panel.tsx
@@ -19,9 +19,10 @@ import { i18n } from '@osd/i18n';
 import { SavedObjectsStart } from '../../../../../core/public';
 import { getDataSourcesList } from '../../utils';
 import { DataSource } from '../../../common/types';
+import { WorkspaceFormError } from './types';
 
 export interface SelectDataSourcePanelProps {
-  errors?: { [key: number]: string };
+  errors?: { [key: number]: WorkspaceFormError };
   savedObjects: SavedObjectsStart;
   selectedDataSources: DataSource[];
   onChange: (value: DataSource[]) => void;
@@ -96,7 +97,7 @@ export const SelectDataSourcePanel = ({
       </EuiText>
       <EuiSpacer size="s" />
       {selectedDataSources.map(({ id, title }, index) => (
-        <EuiFormRow key={index} isInvalid={!!errors?.[index]} error={errors?.[index]}>
+        <EuiFormRow key={index} isInvalid={!!errors?.[index]} error={errors?.[index].message}>
           <EuiFlexGroup gutterSize="l">
             <EuiFlexItem grow={false}>
               <EuiComboBox

diff --git a/src/plugins/workspace/public/components/workspace_form/types.ts b/src/plugins/workspace/public/components/workspace_form/types.ts
@@ -12,19 +12,23 @@ import type { WorkspacePermissionMode } from '../../../common/constants';
 import type { WorkspaceOperationType, WorkspacePermissionItemType } from './constants';
 import { DataSource } from '../../../common/types';
 
+export interface WorkspaceUserPermissionSetting {
+  id: number;
+  type: WorkspacePermissionItemType.User;
+  userId: string;
+  modes: WorkspacePermissionMode[];
+}
+
+export interface WorkspaceUserGroupPermissionSetting {
+  id: number;
+  type: WorkspacePermissionItemType.Group;
+  group: string;
+  modes: WorkspacePermissionMode[];
+}
+
 export type WorkspacePermissionSetting =
-  | {
-      id: number;
-      type: WorkspacePermissionItemType.User;
-      userId: string;
-      modes: WorkspacePermissionMode[];
-    }
-  | {
-      id: number;
-      type: WorkspacePermissionItemType.Group;
-      group: string;
-      modes: WorkspacePermissionMode[];
-    };
+  | WorkspaceUserPermissionSetting
+  | WorkspaceUserGroupPermissionSetting;
 
 export interface WorkspaceFormSubmitData {
   name: string;
@@ -40,20 +44,45 @@ export interface WorkspaceFormData extends WorkspaceFormSubmitData {
   reserved?: boolean;
 }
 
+export enum WorkspaceFormErrorCode {
+  InvalidWorkspaceName,
+  WorkspaceNameMissing,
+  UseCaseMissing,
+  InvalidPermissionType,
+  InvalidPermissionModes,
+  PermissionUserIdMissing,
+  PermissionUserGroupMissing,
+  DuplicateUserIdPermissionSetting,
+  DuplicateUserGroupPermissionSetting,
+  PermissionSettingOwnerMissing,
+  InvalidDataSource,
+  DuplicateDataSource,
+}
+
+export interface WorkspaceFormError {
+  message: string;
+  code: WorkspaceFormErrorCode;
+}
+
 export type WorkspaceFormErrors = {
-  [key in keyof Omit<WorkspaceFormData, 'permissionSettings' | 'selectedDataSources'>]?: string;
+  [key in keyof Omit<
+    WorkspaceFormData,
+    'permissionSettings' | 'description' | 'selectedDataSources'
+  >]?: WorkspaceFormError;
 } & {
-  permissionSettings?: { [key: number]: string };
-  selectedDataSources?: { [key: number]: string };
+  permissionSettings?: {
+    overall?: WorkspaceFormError;
+    fields?: { [key: number]: WorkspaceFormError };
+  };
+  selectedDataSources?: { [key: number]: WorkspaceFormError };
 };
 
 export interface WorkspaceFormProps {
   application: ApplicationStart;
   savedObjects: SavedObjectsStart;
   onSubmit?: (formData: WorkspaceFormSubmitData) => void;
   defaultValues?: WorkspaceFormData;
-  operationType?: WorkspaceOperationType;
+  operationType: WorkspaceOperationType;
   workspaceConfigurableApps?: PublicAppInfo[];
   permissionEnabled?: boolean;
-  permissionLastAdminItemDeletable?: boolean;
 }
diff --git a/src/plugins/workspace/public/components/workspace_form/use_workspace_form.test.ts b/src/plugins/workspace/public/components/workspace_form/use_workspace_form.test.ts
@@ -6,16 +6,20 @@
 import { renderHook, act } from '@testing-library/react-hooks';
 
 import { applicationServiceMock } from '../../../../../core/public/mocks';
-import { WorkspaceFormData } from './types';
+import { WorkspacePermissionMode } from '../../../common/constants';
+import { WorkspaceOperationType, WorkspacePermissionItemType } from './constants';
+import { WorkspaceFormData, WorkspaceFormErrorCode } from './types';
 import { useWorkspaceForm } from './use_workspace_form';
 
-const setup = (defaultValues?: WorkspaceFormData) => {
+const setup = (defaultValues?: WorkspaceFormData, permissionEnabled = false) => {
   const onSubmitMock = jest.fn();
   const renderResult = renderHook(useWorkspaceForm, {
     initialProps: {
       application: applicationServiceMock.createStartContract(),
       defaultValues,
       onSubmit: onSubmitMock,
+      operationType: WorkspaceOperationType.Create,
+      permissionEnabled,
     },
   });
   return {
@@ -25,7 +29,7 @@ const setup = (defaultValues?: WorkspaceFormData) => {
 };
 
 describe('useWorkspaceForm', () => {
-  it('should return "Invalid workspace name" and not call onSubmit when invalid name', async () => {
+  it('should return invalid workspace name error and not call onSubmit when invalid name', async () => {
     const { renderResult, onSubmitMock } = setup({
       id: 'foo',
       name: '~',
@@ -37,7 +41,10 @@ describe('useWorkspaceForm', () => {
     });
     expect(renderResult.result.current.formErrors).toEqual(
       expect.objectContaining({
-        name: 'Invalid workspace name',
+        name: {
+          code: WorkspaceFormErrorCode.InvalidWorkspaceName,
+          message: 'Name is invalid. Enter a valid name.',
+        },
       })
     );
     expect(onSubmitMock).not.toHaveBeenCalled();
@@ -54,7 +61,50 @@ describe('useWorkspaceForm', () => {
     });
     expect(renderResult.result.current.formErrors).toEqual(
       expect.objectContaining({
-        features: 'Use case is required. Select a use case.',
+        features: {
+          code: WorkspaceFormErrorCode.UseCaseMissing,
+          message: 'Use case is required. Select a use case.',
+        },
+      })
+    );
+    expect(onSubmitMock).not.toHaveBeenCalled();
+  });
+  it('should return "Add workspace owner." and not call onSubmit', async () => {
+    const { renderResult, onSubmitMock } = setup(
+      {
+        id: 'foo',
+        name: 'test-workspace-name',
+      },
+      true
+    );
+    expect(renderResult.result.current.formErrors).toEqual({});
+
+    act(() => {
+      renderResult.result.current.setPermissionSettings([
+        {
+          id: 0,
+          modes: [WorkspacePermissionMode.LibraryWrite, WorkspacePermissionMode.Write],
+          type: WorkspacePermissionItemType.User,
+        },
+        {
+          id: 1,
+          modes: [WorkspacePermissionMode.LibraryWrite, WorkspacePermissionMode.Write],
+          type: WorkspacePermissionItemType.Group,
+        },
+      ]);
+    });
+    act(() => {
+      renderResult.result.current.handleFormSubmit({ preventDefault: jest.fn() });
+    });
+
+    expect(renderResult.result.current.formErrors).toEqual(
+      expect.objectContaining({
+        permissionSettings: {
+          overall: {
+            code: WorkspaceFormErrorCode.PermissionSettingOwnerMissing,
+            message: 'Add a workspace owner.',
+          },
+        },
       })
     );
     expect(onSubmitMock).not.toHaveBeenCalled();
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1,2 @@
		feat:
		- [Workspace] Refactor workspace form UI ([#7133](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/7133))
-Original file line number
+Diff line change
@@ Expand Up / @@ -182,3 +182,5 @@ export const WORKSPACE_USE_CASES = Object.freeze({ @@
         ] as string[],
       },
     });
+    export const CURRENT_USER_PLACEHOLDER = '%me%';