From 3edb4251671e991878ace875625ac762b302783a Mon Sep 17 00:00:00 2001 From: Lin Wang Date: Fri, 14 Jun 2024 14:56:31 +0800 Subject: [PATCH 01/29] Make create workspace and update workspace full width Signed-off-by: Lin Wang --- .../components/workspace_creator/workspace_creator.tsx | 9 ++------- .../components/workspace_updater/workspace_updater.tsx | 5 ++--- 2 files changed, 4 insertions(+), 10 deletions(-) diff --git a/src/plugins/workspace/public/components/workspace_creator/workspace_creator.tsx b/src/plugins/workspace/public/components/workspace_creator/workspace_creator.tsx index 61905572f628..f94c8ab09b40 100644 --- a/src/plugins/workspace/public/components/workspace_creator/workspace_creator.tsx +++ b/src/plugins/workspace/public/components/workspace_creator/workspace_creator.tsx @@ -76,9 +76,9 @@ export const WorkspaceCreator = (props: WorkspaceCreatorProps) => { ); return ( - + - + { 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 && ( { } return ( - + - {!props.hideTitle ? : null} + {!props.hideTitle ? : null} { paddingSize="none" color="subdued" hasShadow={false} - style={{ width: '100%', maxWidth: props.maxWidth ? props.maxWidth : 1000 }} > {application && ( Date: Fri, 14 Jun 2024 16:22:01 +0800 Subject: [PATCH 02/29] Refactor user permissions input Signed-off-by: Lin Wang --- .../components/workspace_form/constants.ts | 29 +---- .../workspace_permission_setting_input.tsx | 112 ++++++++++++++---- .../workspace_permission_setting_panel.tsx | 33 ++---- 3 files changed, 107 insertions(+), 67 deletions(-) diff --git a/src/plugins/workspace/public/components/workspace_form/constants.ts b/src/plugins/workspace/public/components/workspace_form/constants.ts index 073477b2ad89..1f4e4260a343 100644 --- 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/workspace_permission_setting_input.tsx b/src/plugins/workspace/public/components/workspace_form/workspace_permission_setting_input.tsx index 099ae33bc23c..605d7ea2301e 100644 --- a/src/plugins/workspace/public/components/workspace_form/workspace_permission_setting_input.tsx +++ b/src/plugins/workspace/public/components/workspace_form/workspace_permission_setting_input.tsx @@ -10,15 +10,54 @@ import { EuiFlexItem, EuiButtonIcon, EuiButtonGroup, + EuiFormRow, + EuiText, } from '@elastic/eui'; +import { i18n } from '@osd/i18n'; import { WorkspacePermissionMode } from '../../../common/constants'; import { WorkspacePermissionItemType, optionIdToWorkspacePermissionModesMap, - permissionModeOptions, + PermissionModeId, } from './constants'; import { getPermissionModeId } from './utils'; +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.Owner, + label: ( + + {i18n.translate('workspace.form.permissionSettingPanel.permissionModeOptions.owner', { + defaultMessage: 'Owner', + })} + + ), + }, +]; + export interface WorkspacePermissionSettingInputProps { index: number; deletable: boolean; @@ -91,26 +130,59 @@ export const WorkspacePermissionSettingInput = ({ }, [index, onDelete]); return ( - - - + + + + + - - + + + + - - {title} - - {permissionSettings.map((item, index) => ( - + ))} - {i18n.translate('workspace.form.permissionSettingPanel.addNew', { - defaultMessage: 'Add New', - })} + {type === WorkspacePermissionItemType.User + ? i18n.translate('workspace.form.permissionSettingPanel.addUser', { + defaultMessage: 'Add user', + }) + : i18n.translate('workspace.form.permissionSettingPanel.addUserGroup', { + defaultMessage: 'Add user group', + })} ); @@ -159,7 +158,7 @@ export const WorkspacePermissionSettingPanel = ({ // Permission setting can be deleted if there are more than one admin setting [...userPermissionSettings, ...groupPermissionSettings].filter( (permission) => - permission.modes && getPermissionModeId(permission.modes) === PermissionModeId.Admin + permission.modes && getPermissionModeId(permission.modes) === PermissionModeId.Owner ).length > 1 ) { return { userNonDeletableIndex: -1, groupNonDeletableIndex: -1 }; @@ -167,11 +166,11 @@ export const WorkspacePermissionSettingPanel = ({ return { userNonDeletableIndex: userPermissionSettings.findIndex( (permission) => - permission.modes && getPermissionModeId(permission.modes) === PermissionModeId.Admin + permission.modes && getPermissionModeId(permission.modes) === PermissionModeId.Owner ), groupNonDeletableIndex: groupPermissionSettings.findIndex( (permission) => - permission.modes && getPermissionModeId(permission.modes) === PermissionModeId.Admin + permission.modes && getPermissionModeId(permission.modes) === PermissionModeId.Owner ), }; }, [userPermissionSettings, groupPermissionSettings, lastAdminItemDeletable]); @@ -208,9 +207,6 @@ export const WorkspacePermissionSettingPanel = ({ return (
Date: Fri, 14 Jun 2024 18:10:19 +0800 Subject: [PATCH 03/29] Add workspace form call out Signed-off-by: Lin Wang --- .../workspace_form/workspace_form.tsx | 45 +++++++++++++-- .../workspace_form_error_callout.tsx | 56 +++++++++++++++++++ .../workspace_permission_setting_panel.tsx | 1 - 3 files changed, 95 insertions(+), 7 deletions(-) create mode 100644 src/plugins/workspace/public/components/workspace_form/workspace_form_error_callout.tsx diff --git a/src/plugins/workspace/public/components/workspace_form/workspace_form.tsx b/src/plugins/workspace/public/components/workspace_form/workspace_form.tsx index 645e1843c9ab..76947f0a0686 100644 --- a/src/plugins/workspace/public/components/workspace_form/workspace_form.tsx +++ b/src/plugins/workspace/public/components/workspace_form/workspace_form.tsx @@ -14,6 +14,9 @@ import { EuiText, EuiColorPicker, EuiTextArea, + EuiButton, + EuiFlexGroup, + EuiFlexItem, } from '@elastic/eui'; import { i18n } from '@osd/i18n'; @@ -22,6 +25,8 @@ import { WorkspaceFormProps } from './types'; import { useWorkspaceForm } from './use_workspace_form'; import { WorkspacePermissionSettingPanel } from './workspace_permission_setting_panel'; import { WorkspaceUseCase } from './workspace_use_case'; +import { WorkspaceOperationType } from './constants'; +import { WorkspaceFormErrorCallout } from './workspace_form_error_callout'; export const WorkspaceForm = (props: WorkspaceFormProps) => { const { @@ -50,6 +55,7 @@ export const WorkspaceForm = (props: WorkspaceFormProps) => { return ( +

{workspaceDetailsTitle}

@@ -173,12 +179,39 @@ export const WorkspaceForm = (props: WorkspaceFormProps) => { )} - + {operationType === WorkspaceOperationType.Create && ( + + + + {i18n.translate('workspace.form.bottomBar.cancel', { + defaultMessage: 'Cancel', + })} + + + + {operationType === WorkspaceOperationType.Create && ( + + {i18n.translate('workspace.form.bottomBar.createWorkspace', { + defaultMessage: 'Create workspace', + })} + + )} + + + )} + {operationType === WorkspaceOperationType.Update && ( + + )} ); }; diff --git a/src/plugins/workspace/public/components/workspace_form/workspace_form_error_callout.tsx b/src/plugins/workspace/public/components/workspace_form/workspace_form_error_callout.tsx new file mode 100644 index 000000000000..3d35fc5b24d3 --- /dev/null +++ b/src/plugins/workspace/public/components/workspace_form/workspace_form_error_callout.tsx @@ -0,0 +1,56 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +import React from 'react'; +import { i18n } from '@osd/i18n'; +import { EuiCallOut } from '@elastic/eui'; +import { WorkspaceFormErrors } from './types'; +import { getNumberOfErrors } from './utils'; + +interface WorkspaceFormErrorCalloutProps { + errors: WorkspaceFormErrors; +} + +export const WorkspaceFormErrorCallout = ({ errors }: WorkspaceFormErrorCalloutProps) => { + if (getNumberOfErrors(errors) === 0) { + return null; + } + return ( + + {errors.name && ( +

+ {i18n.translate('workspace.form.errorCallout.nameKey', { + defaultMessage: 'Name:', + })} +   + {errors.name} +

+ )} + {errors.features && ( +

+ {i18n.translate('workspace.form.errorCallout.nameKey', { + defaultMessage: 'Use case:', + })} +   + {errors.features} +

+ )} + {errors.permissionSettings && ( +

+ {i18n.translate('workspace.form.errorCallout.nameKey', { + defaultMessage: 'Permissions:', + })} +
+ {Object.keys(errors.permissionSettings).map((key) => ( +

+ {key} + {errors.permissionSettings[key]} +

+ ))} +

+ )} + + ); +}; diff --git a/src/plugins/workspace/public/components/workspace_form/workspace_permission_setting_panel.tsx b/src/plugins/workspace/public/components/workspace_form/workspace_permission_setting_panel.tsx index 9054899f1bdc..14f9ad33f58a 100644 --- a/src/plugins/workspace/public/components/workspace_form/workspace_permission_setting_panel.tsx +++ b/src/plugins/workspace/public/components/workspace_form/workspace_permission_setting_panel.tsx @@ -31,7 +31,6 @@ export interface WorkspacePermissionSettingPanelProps { interface UserOrGroupSectionProps extends Omit { - title: string; nonDeletableIndex: number; type: WorkspacePermissionItemType; nextIdGenerator: () => number; From 8dad0e041e22791ca80214cca26fefcb2f2685c3 Mon Sep 17 00:00:00 2001 From: Lin Wang Date: Mon, 17 Jun 2024 16:12:58 +0800 Subject: [PATCH 04/29] Fix permissions input unit tests Signed-off-by: Lin Wang --- .../workspace_permission_setting_input.test.tsx | 6 +++--- .../workspace_permission_setting_panel.test.tsx | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/plugins/workspace/public/components/workspace_form/workspace_permission_setting_input.test.tsx b/src/plugins/workspace/public/components/workspace_form/workspace_permission_setting_input.test.tsx index ea18692255e6..fc0f7ec6b44c 100644 --- a/src/plugins/workspace/public/components/workspace_form/workspace_permission_setting_input.test.tsx +++ b/src/plugins/workspace/public/components/workspace_form/workspace_permission_setting_input.test.tsx @@ -65,7 +65,7 @@ describe('WorkspacePermissionSettingInput', () => { const { renderResult, onGroupOrUserIdChangeMock } = setup(); expect(onGroupOrUserIdChangeMock).not.toHaveBeenCalled(); - fireEvent.click(renderResult.getByText('Select')); + fireEvent.click(renderResult.getByText('Select a user')); fireEvent.input(renderResult.getByTestId('comboBoxSearchInput'), { target: { value: 'user1' }, }); @@ -78,7 +78,7 @@ describe('WorkspacePermissionSettingInput', () => { }); expect(onGroupOrUserIdChangeMock).not.toHaveBeenCalled(); - fireEvent.click(renderResult.getByText('Select')); + fireEvent.click(renderResult.getByText('Select a user group')); fireEvent.input(renderResult.getByTestId('comboBoxSearchInput'), { target: { value: 'group' }, }); @@ -100,7 +100,7 @@ describe('WorkspacePermissionSettingInput', () => { const { renderResult, onPermissionModesChangeMock } = setup({}); expect(onPermissionModesChangeMock).not.toHaveBeenCalled(); - fireEvent.click(renderResult.getByText('Admin')); + fireEvent.click(renderResult.getByText('Owner')); expect(onPermissionModesChangeMock).toHaveBeenCalledWith(['library_write', 'write'], 0); }); diff --git a/src/plugins/workspace/public/components/workspace_form/workspace_permission_setting_panel.test.tsx b/src/plugins/workspace/public/components/workspace_form/workspace_permission_setting_panel.test.tsx index 8415405e8e98..a3c687ba5133 100644 --- a/src/plugins/workspace/public/components/workspace_form/workspace_permission_setting_panel.test.tsx +++ b/src/plugins/workspace/public/components/workspace_form/workspace_permission_setting_panel.test.tsx @@ -81,7 +81,7 @@ describe('WorkspacePermissionSettingInput', () => { const { renderResult, onChangeMock } = setup(); expect(onChangeMock).not.toHaveBeenCalled(); - fireEvent.click(renderResult.getAllByText('Admin')[1]); + fireEvent.click(renderResult.getAllByText('Owner')[1]); expect(onChangeMock).toHaveBeenCalledWith([ { id: 0, @@ -189,10 +189,10 @@ describe('WorkspacePermissionSettingInput', () => { const { renderResult } = setup({ errors: { '0': 'User permission setting error', '1': 'Group permission setting error' }, }); - expect(renderResult.container.querySelectorAll('.euiFormRow')[0]).toHaveTextContent( + expect(renderResult.container.querySelectorAll('.euiFormErrorText')[0]).toHaveTextContent( 'User permission setting error' ); - expect(renderResult.container.querySelectorAll('.euiFormRow')[1]).toHaveTextContent( + expect(renderResult.container.querySelectorAll('.euiFormErrorText')[1]).toHaveTextContent( 'Group permission setting error' ); }); From 293fec3f2da4459403bc5df5e2bf79aec56e9759 Mon Sep 17 00:00:00 2001 From: Lin Wang Date: Wed, 19 Jun 2024 14:51:37 +0800 Subject: [PATCH 05/29] Update gaps Signed-off-by: Lin Wang --- .../workspace_form/workspace_form.tsx | 1 + .../workspace_permission_setting_input.tsx | 72 +++++++------------ .../workspace_permission_setting_panel.tsx | 19 ++++- 3 files changed, 43 insertions(+), 49 deletions(-) diff --git a/src/plugins/workspace/public/components/workspace_form/workspace_form.tsx b/src/plugins/workspace/public/components/workspace_form/workspace_form.tsx index 76947f0a0686..6130e27b2306 100644 --- a/src/plugins/workspace/public/components/workspace_form/workspace_form.tsx +++ b/src/plugins/workspace/public/components/workspace_form/workspace_form.tsx @@ -169,6 +169,7 @@ export const WorkspaceForm = (props: WorkspaceFormProps) => { })} + - - - + /> - - - + + + + {type === WorkspacePermissionItemType.User + ? i18n.translate('workspaceForm.permissionSetting.userLabel', { + defaultMessage: 'User', + }) + : i18n.translate('workspaceForm.permissionSetting.groupLabel', { + defaultMessage: 'User group', + })} + + + {i18n.translate('workspaceForm.permissionSetting.permissionLabel', { + defaultMessage: 'Permissions', + })} + + + {permissionSettings.map((item, index) => ( From 04d55328cd74451a73188798b598bb8bc633080a Mon Sep 17 00:00:00 2001 From: Lin Wang Date: Thu, 20 Jun 2024 16:52:33 +0800 Subject: [PATCH 06/29] Update error callout Signed-off-by: Lin Wang --- .../workspace_creator/workspace_creator.tsx | 1 - .../public/components/workspace_form/types.ts | 21 ++- .../public/components/workspace_form/utils.ts | 107 ++++++++----- .../workspace_form/workspace_form.tsx | 16 +- .../workspace_form_error_callout.tsx | 147 +++++++++++++----- .../workspace_permission_setting_input.tsx | 1 - .../workspace_permission_setting_panel.tsx | 6 +- .../workspace_updater/workspace_updater.tsx | 1 - 8 files changed, 210 insertions(+), 90 deletions(-) diff --git a/src/plugins/workspace/public/components/workspace_creator/workspace_creator.tsx b/src/plugins/workspace/public/components/workspace_creator/workspace_creator.tsx index f94c8ab09b40..3255e82b45d5 100644 --- a/src/plugins/workspace/public/components/workspace_creator/workspace_creator.tsx +++ b/src/plugins/workspace/public/components/workspace_creator/workspace_creator.tsx @@ -79,7 +79,6 @@ export const WorkspaceCreator = (props: WorkspaceCreatorProps) => { - ]?: string; + [key in keyof Omit]?: WorkspaceFormError; } & { - permissionSettings?: { [key: number]: string }; + permissionSettings?: { [key: number]: WorkspaceFormError }; }; export interface WorkspaceFormProps { diff --git a/src/plugins/workspace/public/components/workspace_form/utils.ts b/src/plugins/workspace/public/components/workspace_form/utils.ts index 352cbe1aca1a..66e63b757448 100644 --- a/src/plugins/workspace/public/components/workspace_form/utils.ts +++ b/src/plugins/workspace/public/components/workspace_form/utils.ts @@ -14,7 +14,13 @@ import { WorkspacePermissionItemType, } from './constants'; -import { WorkspaceFormErrors, WorkspaceFormSubmitData, WorkspacePermissionSetting } from './types'; +import { + WorkspaceFormError, + WorkspaceFormErrorCode, + WorkspaceFormErrors, + WorkspaceFormSubmitData, + WorkspacePermissionSetting, +} from './types'; export const appendDefaultFeatureIds = (ids: string[]) => { // concat default checked ids and unique the result @@ -190,64 +196,87 @@ export const validateWorkspaceForm = ( const { name, permissionSettings, features } = formData; if (name) { if (!isValidFormTextInput(name)) { - formErrors.name = i18n.translate('workspace.form.detail.name.invalid', { - defaultMessage: 'Invalid workspace name', - }); + formErrors.name = { + code: WorkspaceFormErrorCode.InvalidWorkspaceName, + message: i18n.translate('workspace.form.detail.name.invalid', { + defaultMessage: 'Invalid workspace name', + }), + }; } } else { - formErrors.name = i18n.translate('workspace.form.detail.name.empty', { - defaultMessage: "Name can't be empty.", - }); + formErrors.name = { + code: WorkspaceFormErrorCode.WorkspaceNameMissing, + message: i18n.translate('workspace.form.detail.name.empty', { + defaultMessage: "Name can't be empty.", + }), + }; } if (!features || !features.some((featureConfig) => isUseCaseFeatureConfig(featureConfig))) { - formErrors.features = i18n.translate('workspace.form.features.empty', { - defaultMessage: 'Use case is required. Select a use case.', - }); + formErrors.features = { + code: WorkspaceFormErrorCode.UseCaseMissing, + message: i18n.translate('workspace.form.features.empty', { + defaultMessage: 'Use case is required. Select a use case.', + }), + }; } if (permissionSettings) { - const permissionSettingsErrors: { [key: number]: string } = {}; + const permissionSettingsErrors: { [key: number]: WorkspaceFormError } = {}; for (let i = 0; i < permissionSettings.length; i++) { const setting = permissionSettings[i]; if (!setting.type) { - permissionSettingsErrors[setting.id] = i18n.translate( - 'workspace.form.permission.invalidate.type', - { + permissionSettingsErrors[setting.id] = { + code: WorkspaceFormErrorCode.InvalidPermissionType, + message: i18n.translate('workspace.form.permission.invalidate.type', { defaultMessage: 'Invalid type', - } - ); + }), + }; } else if (!setting.modes || setting.modes.length === 0) { - permissionSettingsErrors[setting.id] = i18n.translate( - 'workspace.form.permission.invalidate.modes', - { + permissionSettingsErrors[setting.id] = { + code: WorkspaceFormErrorCode.InvalidPermissionModes, + message: i18n.translate('workspace.form.permission.invalidate.modes', { defaultMessage: 'Invalid permission modes', - } - ); + }), + }; } else if (setting.type === WorkspacePermissionItemType.User && !setting.userId) { - permissionSettingsErrors[setting.id] = i18n.translate( - 'workspace.form.permission.invalidate.userId', - { - defaultMessage: 'Invalid user id', - } - ); + permissionSettingsErrors[setting.id] = { + code: WorkspaceFormErrorCode.PermissionUserIdMissing, + message: i18n.translate('workspace.form.permission.invalidate.userId', { + defaultMessage: 'User is required. Enter a user.', + }), + }; } else if (setting.type === WorkspacePermissionItemType.Group && !setting.group) { - permissionSettingsErrors[setting.id] = i18n.translate( - 'workspace.form.permission.invalidate.group', - { - defaultMessage: 'Invalid user group', - } - ); + permissionSettingsErrors[setting.id] = { + code: WorkspaceFormErrorCode.PermissionUserGroupMissing, + message: i18n.translate('workspace.form.permission.invalidate.group', { + defaultMessage: 'User group is required. Enter a user group.', + }), + }; } else if ( isUserOrGroupPermissionSettingDuplicated( permissionSettings.slice(0, i), setting as WorkspacePermissionSetting ) ) { - permissionSettingsErrors[setting.id] = i18n.translate( - 'workspace.form.permission.invalidate.group', - { - defaultMessage: 'Duplicate permission setting', - } - ); + permissionSettingsErrors[setting.id] = { + code: + setting.type === WorkspacePermissionItemType.User + ? WorkspaceFormErrorCode.DuplicateUserPermissionSetting + : WorkspaceFormErrorCode.DuplicateUserGroupPermissionSetting, + message: + setting.type === WorkspacePermissionItemType.User + ? i18n.translate( + 'workspace.form.permission.invalidate.duplicateUserPermissionSetting', + { + defaultMessage: 'User must be unique. Enter a unique user.', + } + ) + : i18n.translate( + 'workspace.form.permission.invalidate.duplicateUserGroupPermissionSetting', + { + defaultMessage: 'User group must be unique. Enter a unique user group.', + } + ), + }; } } if (Object.keys(permissionSettingsErrors).length > 0) { diff --git a/src/plugins/workspace/public/components/workspace_form/workspace_form.tsx b/src/plugins/workspace/public/components/workspace_form/workspace_form.tsx index 6130e27b2306..0bb461919291 100644 --- a/src/plugins/workspace/public/components/workspace_form/workspace_form.tsx +++ b/src/plugins/workspace/public/components/workspace_form/workspace_form.tsx @@ -55,7 +55,13 @@ export const WorkspaceForm = (props: WorkspaceFormProps) => { return ( - + {numberOfErrors > 0 && ( + <> + + + + )} +

{workspaceDetailsTitle}

@@ -70,7 +76,7 @@ export const WorkspaceForm = (props: WorkspaceFormProps) => { 'Valid characters are a-z, A-Z, 0-9, (), [], _ (underscore), - (hyphen) and (space).', })} isInvalid={!!formErrors.name} - error={formErrors.name} + error={formErrors.name?.message} > { } isInvalid={!!formErrors.description} - error={formErrors.description} + error={formErrors.description?.message} > <> @@ -117,7 +123,7 @@ export const WorkspaceForm = (props: WorkspaceFormProps) => { defaultMessage: 'Color', })} isInvalid={!!formErrors.color} - error={formErrors.color} + error={formErrors.color?.message} >
@@ -149,7 +155,7 @@ export const WorkspaceForm = (props: WorkspaceFormProps) => { defaultMessage: 'Use case', })} isInvalid={!!formErrors.features} - error={formErrors.features} + error={formErrors.features?.message} fullWidth > { + switch (error.code) { + case WorkspaceFormErrorCode.WorkspaceNameMissing: + return i18n.translate('workspace.form.errorCallout.nameMissing', { + defaultMessage: 'Enter a workspace name.', + }); + case WorkspaceFormErrorCode.InvalidWorkspaceName: + return i18n.translate('workspace.form.errorCallout.nameInvalid', { + defaultMessage: 'Enter a valid workspace name.', + }); + case WorkspaceFormErrorCode.UseCaseMissing: + return i18n.translate('workspace.form.errorCallout.useCaseMissing', { + defaultMessage: 'Select a use case.', + }); + case WorkspaceFormErrorCode.PermissionUserIdMissing: + return i18n.translate('workspace.form.errorCallout.missingUser', { + defaultMessage: 'Enter a user.', + }); + case WorkspaceFormErrorCode.PermissionUserGroupMissing: + return i18n.translate('workspace.form.errorCallout.missingUserGroup', { + defaultMessage: 'Enter a user group.', + }); + case WorkspaceFormErrorCode.DuplicateUserPermissionSetting: + return i18n.translate('workspace.form.errorCallout.duplicatePermission', { + defaultMessage: 'Enter a unique user.', + }); + case WorkspaceFormErrorCode.DuplicateUserGroupPermissionSetting: + return i18n.translate('workspace.form.errorCallout.duplicatePermission', { + defaultMessage: 'Enter a unique user group.', + }); + default: + return error.message; + } +}; + +const WorkspaceFormErrorCalloutItem = ({ + errorKey, + message, +}: { + errorKey?: string; + message?: string; +}) => { + if (!errorKey || !message) { + return null; + } + return ( +
  • + {errorKey} {message} +
    • + ); +}; interface WorkspaceFormErrorCalloutProps { errors: WorkspaceFormErrors; } export const WorkspaceFormErrorCallout = ({ errors }: WorkspaceFormErrorCalloutProps) => { - if (getNumberOfErrors(errors) === 0) { - return null; - } - return ( - - {errors.name && ( -

    - {i18n.translate('workspace.form.errorCallout.nameKey', { - defaultMessage: 'Name:', - })} -   - {errors.name} -

    - )} - {errors.features && ( -

    - {i18n.translate('workspace.form.errorCallout.nameKey', { - defaultMessage: 'Use case:', + const renderPermissionSettingSuggestion = (errorCode: WorkspaceFormErrorCode) => { + if (!errors.permissionSettings) { + return null; + } + const findingError = Object.values(errors.permissionSettings).find( + (item) => item.code === errorCode + ); + + if (!findingError) { + return null; + } + + return ( +

  • + {(errorCode === WorkspaceFormErrorCode.DuplicateUserPermissionSetting || + errorCode === WorkspaceFormErrorCode.PermissionUserIdMissing) && + i18n.translate('workspace.form.errorCallout.userPermissionKey', { + defaultMessage: 'User:', })} -   - {errors.features} -

    - )} - {errors.permissionSettings && ( -

    - {i18n.translate('workspace.form.errorCallout.nameKey', { - defaultMessage: 'Permissions:', + {(errorCode === WorkspaceFormErrorCode.DuplicateUserGroupPermissionSetting || + errorCode === WorkspaceFormErrorCode.PermissionUserGroupMissing) && + i18n.translate('workspace.form.errorCallout.userGroupPermissionKey', { + defaultMessage: 'Group:', })} -
    - {Object.keys(errors.permissionSettings).map((key) => ( -

    - {key} - {errors.permissionSettings[key]} -

    - ))} -

    - )} +   + {getSuggestionFromErrorCode(findingError)} +
    • + ); + }; + return ( + + +
      + {errors.name && ( + + )} + {errors.features && ( + + )} + {renderPermissionSettingSuggestion(WorkspaceFormErrorCode.PermissionUserIdMissing)} + {renderPermissionSettingSuggestion(WorkspaceFormErrorCode.DuplicateUserPermissionSetting)} + {renderPermissionSettingSuggestion(WorkspaceFormErrorCode.PermissionUserGroupMissing)} + {renderPermissionSettingSuggestion( + WorkspaceFormErrorCode.DuplicateUserGroupPermissionSetting + )} +
    +     ); }; diff --git a/src/plugins/workspace/public/components/workspace_form/workspace_permission_setting_input.tsx b/src/plugins/workspace/public/components/workspace_form/workspace_permission_setting_input.tsx index f2ff87e44c30..083c7a91cba2 100644 --- a/src/plugins/workspace/public/components/workspace_form/workspace_permission_setting_input.tsx +++ b/src/plugins/workspace/public/components/workspace_form/workspace_permission_setting_input.tsx @@ -10,7 +10,6 @@ import { EuiFlexItem, EuiButtonIcon, EuiButtonGroup, - EuiFormRow, EuiText, } from '@elastic/eui'; import { i18n } from '@osd/i18n'; diff --git a/src/plugins/workspace/public/components/workspace_form/workspace_permission_setting_panel.tsx b/src/plugins/workspace/public/components/workspace_form/workspace_permission_setting_panel.tsx index ae96b7305abb..1a7cce308928 100644 --- a/src/plugins/workspace/public/components/workspace_form/workspace_permission_setting_panel.tsx +++ b/src/plugins/workspace/public/components/workspace_form/workspace_permission_setting_panel.tsx @@ -6,7 +6,7 @@ import React, { useCallback, useEffect, useMemo, useRef } from 'react'; import { EuiButton, EuiFlexGroup, EuiFlexItem, EuiFormRow, EuiSpacer } from '@elastic/eui'; import { i18n } from '@osd/i18n'; -import { WorkspacePermissionSetting } from './types'; +import { WorkspaceFormError, WorkspacePermissionSetting } from './types'; import { WorkspacePermissionItemType, optionIdToWorkspacePermissionModesMap, @@ -19,7 +19,7 @@ import { import { generateNextPermissionSettingsId, getPermissionModeId } from './utils'; export interface WorkspacePermissionSettingPanelProps { - errors?: { [key: number]: string }; + errors?: { [key: number]: WorkspaceFormError }; lastAdminItemDeletable: boolean; permissionSettings: Array< Pick & Partial @@ -116,7 +116,7 @@ const UserOrGroupSection = ({ {permissionSettings.map((item, index) => ( - + { {!props.hideTitle ? : null} - Date: Fri, 21 Jun 2024 17:20:07 +0800 Subject: [PATCH 07/29] Update user permission current user and number of changes Signed-off-by: Lin Wang --- src/plugins/workspace/common/constants.ts | 2 + .../public/components/workspace_form/types.ts | 30 +- .../workspace_form/use_workspace_form.ts | 58 +++- .../public/components/workspace_form/utils.ts | 258 ++++++++++++++---- .../workspace_form/workspace_bottom_bar.tsx | 67 ++--- .../workspace_create_action_panel.tsx | 56 ++++ .../workspace_form/workspace_form.tsx | 32 +-- .../workspace_permission_setting_input.tsx | 20 +- .../workspace_permission_setting_panel.tsx | 38 ++- src/plugins/workspace/server/routes/index.ts | 11 +- src/plugins/workspace/server/utils.ts | 23 ++ 11 files changed, 411 insertions(+), 184 deletions(-) create mode 100644 src/plugins/workspace/public/components/workspace_form/workspace_create_action_panel.tsx diff --git a/src/plugins/workspace/common/constants.ts b/src/plugins/workspace/common/constants.ts index 4380ce40a10b..cc1d0552a38e 100644 --- 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 = '%current_user%'; diff --git a/src/plugins/workspace/public/components/workspace_form/types.ts b/src/plugins/workspace/public/components/workspace_form/types.ts index 1578a193c221..a1298afe7d55 100644 --- a/src/plugins/workspace/public/components/workspace_form/types.ts +++ b/src/plugins/workspace/public/components/workspace_form/types.ts @@ -7,19 +7,23 @@ import type { ApplicationStart, PublicAppInfo } from '../../../../../core/public import type { WorkspacePermissionMode } from '../../../common/constants'; import type { WorkspaceOperationType, WorkspacePermissionItemType } from './constants'; +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; @@ -61,7 +65,7 @@ export interface WorkspaceFormProps { application: ApplicationStart; 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.ts b/src/plugins/workspace/public/components/workspace_form/use_workspace_form.ts index 4536e56d21bd..7fc88b755d56 100644 --- a/src/plugins/workspace/public/components/workspace_form/use_workspace_form.ts +++ b/src/plugins/workspace/public/components/workspace_form/use_workspace_form.ts @@ -19,17 +19,33 @@ import { } from '../../utils'; import { WorkspaceFormProps, WorkspaceFormErrors, WorkspacePermissionSetting } from './types'; -import { appendDefaultFeatureIds, getNumberOfErrors, validateWorkspaceForm } from './utils'; +import { + appendDefaultFeatureIds, + generatePermissionSettingsState, + getNumberOfChanges, + getNumberOfErrors, + validateWorkspaceForm, +} from './utils'; +import { WorkspacePermissionItemType } from './constants'; const workspaceHtmlIdGenerator = htmlIdGenerator(); const isNotNull = (value: T | null): value is T => !!value; -export const useWorkspaceForm = ({ application, defaultValues, onSubmit }: WorkspaceFormProps) => { +export const useWorkspaceForm = ({ + application, + defaultValues, + operationType, + onSubmit, +}: WorkspaceFormProps) => { const applications = useApplications(application); const [name, setName] = useState(defaultValues?.name); const [description, setDescription] = useState(defaultValues?.description); const [color, setColor] = useState(defaultValues?.color); + const defaultValuesRef = useRef(defaultValues); + const initialPermissionSettingsRef = useRef( + generatePermissionSettingsState(operationType, defaultValues?.permissionSettings) + ); const [featureConfigs, setFeatureConfigs] = useState( appendDefaultFeatureIds(defaultValues?.features ?? []) @@ -40,11 +56,7 @@ export const useWorkspaceForm = ({ application, defaultValues, onSubmit }: Works ); const [permissionSettings, setPermissionSettings] = useState< Array & Partial> - >( - defaultValues?.permissionSettings && defaultValues.permissionSettings.length > 0 - ? defaultValues.permissionSettings - : [] - ); + >(initialPermissionSettingsRef.current); const [formErrors, setFormErrors] = useState({}); const numberOfErrors = useMemo(() => getNumberOfErrors(formErrors), [formErrors]); @@ -59,6 +71,14 @@ export const useWorkspaceForm = ({ application, defaultValues, onSubmit }: Works }); const getFormDataRef = useRef(getFormData); getFormDataRef.current = getFormData; + const formData = getFormData(); + const numberOfChanges = defaultValuesRef.current + ? getNumberOfChanges(formData, { + ...defaultValuesRef.current, + // The user form will insert some empty permission rows, should ignore these rows not treated as user new added. + permissionSettings: initialPermissionSettingsRef.current, + }) + : 0; if (!formIdRef.current) { formIdRef.current = workspaceHtmlIdGenerator(); @@ -81,19 +101,26 @@ export const useWorkspaceForm = ({ application, defaultValues, onSubmit }: Works const handleFormSubmit = useCallback( (e) => { e.preventDefault(); - const formData = getFormDataRef.current(); - const currentFormErrors: WorkspaceFormErrors = validateWorkspaceForm(formData); + const currentFormData = getFormDataRef.current(); + const currentFormErrors: WorkspaceFormErrors = validateWorkspaceForm( + currentFormData, + defaultValuesRef.current + ); setFormErrors(currentFormErrors); if (getNumberOfErrors(currentFormErrors) > 0) { return; } onSubmit?.({ - name: formData.name!, - description: formData.description, - features: formData.features, - color: formData.color, - permissionSettings: formData.permissionSettings as WorkspacePermissionSetting[], + name: currentFormData.name!, + description: currentFormData.description, + features: currentFormData.features, + color: currentFormData.color, + permissionSettings: currentFormData.permissionSettings.filter( + (item) => + (item.type === WorkspacePermissionItemType.User && !!item.userId) || + (item.type === WorkspacePermissionItemType.Group && !!item.group) + ) as WorkspacePermissionSetting[], }); }, [onSubmit] @@ -113,10 +140,11 @@ export const useWorkspaceForm = ({ application, defaultValues, onSubmit }: Works return { formId: formIdRef.current, - formData: getFormData(), + formData, formErrors, applications, numberOfErrors, + numberOfChanges, handleFormSubmit, handleColorChange, handleUseCasesChange, diff --git a/src/plugins/workspace/public/components/workspace_form/utils.ts b/src/plugins/workspace/public/components/workspace_form/utils.ts index 66e63b757448..5c457fad323c 100644 --- a/src/plugins/workspace/public/components/workspace_form/utils.ts +++ b/src/plugins/workspace/public/components/workspace_form/utils.ts @@ -6,20 +6,28 @@ import { i18n } from '@osd/i18n'; import type { SavedObjectPermissions } from '../../../../../core/types'; -import { DEFAULT_SELECTED_FEATURES_IDS, WorkspacePermissionMode } from '../../../common/constants'; +import { + CURRENT_USER_PLACEHOLDER, + DEFAULT_SELECTED_FEATURES_IDS, + WorkspacePermissionMode, +} from '../../../common/constants'; import { isUseCaseFeatureConfig } from '../../utils'; import { optionIdToWorkspacePermissionModesMap, PermissionModeId, + WorkspaceOperationType, WorkspacePermissionItemType, } from './constants'; import { + WorkspaceFormData, WorkspaceFormError, WorkspaceFormErrorCode, WorkspaceFormErrors, WorkspaceFormSubmitData, WorkspacePermissionSetting, + WorkspaceUserGroupPermissionSetting, + WorkspaceUserPermissionSetting, } from './types'; export const appendDefaultFeatureIds = (ids: string[]) => { @@ -53,18 +61,23 @@ export const getNumberOfErrors = (formErrors: WorkspaceFormErrors) => { return numberOfErrors; }; +const isSamePermissionSetting = ( + a: Partial, + b: Partial +) => + (a.type === WorkspacePermissionItemType.User && + b.type === WorkspacePermissionItemType.User && + a.userId === b.userId) || + (a.type === WorkspacePermissionItemType.Group && + b.type === WorkspacePermissionItemType.Group && + a.group === b.group); + export const isUserOrGroupPermissionSettingDuplicated = ( permissionSettings: Array>, permissionSettingToCheck: WorkspacePermissionSetting ) => - permissionSettings.some( - (permissionSetting) => - (permissionSettingToCheck.type === WorkspacePermissionItemType.User && - permissionSetting.type === WorkspacePermissionItemType.User && - permissionSettingToCheck.userId === permissionSetting.userId) || - (permissionSettingToCheck.type === WorkspacePermissionItemType.Group && - permissionSetting.type === WorkspacePermissionItemType.Group && - permissionSettingToCheck.group === permissionSetting.group) + permissionSettings.some((permissionSetting) => + isSamePermissionSetting(permissionSetting, permissionSettingToCheck) ); /** @@ -185,12 +198,71 @@ export const convertPermissionsToPermissionSettings = (permissions: SavedObjectP return finalPermissionSettings; }; +const validateUserPermissionSetting = ( + setting: WorkspaceUserPermissionSetting, + required: boolean, + previousPermissionSettings: Array> +) => { + if (required && !setting.userId) { + return { + code: WorkspaceFormErrorCode.PermissionUserIdMissing, + message: i18n.translate('workspace.form.permission.invalidate.userId', { + defaultMessage: 'User is required. Enter a user.', + }), + }; + } + if ( + !!setting.userId && + isUserOrGroupPermissionSettingDuplicated(previousPermissionSettings, setting) + ) { + return { + code: WorkspaceFormErrorCode.DuplicateUserPermissionSetting, + message: i18n.translate( + 'workspace.form.permission.invalidate.duplicateUserPermissionSetting', + { + defaultMessage: 'User must be unique. Enter a unique user.', + } + ), + }; + } +}; + +const validateUserGroupPermissionSetting = ( + setting: WorkspaceUserGroupPermissionSetting, + required: boolean, + previousPermissionSettings: Array> +) => { + if (required && !setting.group) { + return { + code: WorkspaceFormErrorCode.PermissionUserGroupMissing, + message: i18n.translate('workspace.form.permission.invalidate.group', { + defaultMessage: 'User group is required. Enter a user group.', + }), + }; + } + if ( + !!setting.group && + isUserOrGroupPermissionSettingDuplicated(previousPermissionSettings, setting) + ) { + return { + code: WorkspaceFormErrorCode.DuplicateUserGroupPermissionSetting, + message: i18n.translate( + 'workspace.form.permission.invalidate.duplicateUserGroupPermissionSetting', + { + defaultMessage: 'User group must be unique. Enter a unique user group.', + } + ), + }; + } +}; + export const validateWorkspaceForm = ( formData: Omit, 'permissionSettings'> & { permissionSettings?: Array< Pick & Partial >; - } + }, + initialFormData?: WorkspaceFormSubmitData ) => { const formErrors: WorkspaceFormErrors = {}; const { name, permissionSettings, features } = formData; @@ -221,6 +293,9 @@ export const validateWorkspaceForm = ( } if (permissionSettings) { const permissionSettingsErrors: { [key: number]: WorkspaceFormError } = {}; + const requiredSettingIds = initialFormData?.permissionSettings?.map((item) => item.id); + const isUserIdOrGroupRequired = (settingId: number) => + !!requiredSettingIds?.includes(settingId); for (let i = 0; i < permissionSettings.length; i++) { const setting = permissionSettings[i]; if (!setting.type) { @@ -237,46 +312,24 @@ export const validateWorkspaceForm = ( defaultMessage: 'Invalid permission modes', }), }; - } else if (setting.type === WorkspacePermissionItemType.User && !setting.userId) { - permissionSettingsErrors[setting.id] = { - code: WorkspaceFormErrorCode.PermissionUserIdMissing, - message: i18n.translate('workspace.form.permission.invalidate.userId', { - defaultMessage: 'User is required. Enter a user.', - }), - }; - } else if (setting.type === WorkspacePermissionItemType.Group && !setting.group) { - permissionSettingsErrors[setting.id] = { - code: WorkspaceFormErrorCode.PermissionUserGroupMissing, - message: i18n.translate('workspace.form.permission.invalidate.group', { - defaultMessage: 'User group is required. Enter a user group.', - }), - }; - } else if ( - isUserOrGroupPermissionSettingDuplicated( - permissionSettings.slice(0, i), - setting as WorkspacePermissionSetting - ) - ) { - permissionSettingsErrors[setting.id] = { - code: - setting.type === WorkspacePermissionItemType.User - ? WorkspaceFormErrorCode.DuplicateUserPermissionSetting - : WorkspaceFormErrorCode.DuplicateUserGroupPermissionSetting, - message: - setting.type === WorkspacePermissionItemType.User - ? i18n.translate( - 'workspace.form.permission.invalidate.duplicateUserPermissionSetting', - { - defaultMessage: 'User must be unique. Enter a unique user.', - } - ) - : i18n.translate( - 'workspace.form.permission.invalidate.duplicateUserGroupPermissionSetting', - { - defaultMessage: 'User group must be unique. Enter a unique user group.', - } - ), - }; + } else if (setting.type === WorkspacePermissionItemType.User) { + const validateResult = validateUserPermissionSetting( + setting as WorkspaceUserPermissionSetting, + isUserIdOrGroupRequired(setting.id), + permissionSettings.slice(0, i) + ); + if (validateResult) { + permissionSettingsErrors[setting.id] = validateResult; + } + } else if (setting.type === WorkspacePermissionItemType.Group) { + const validateResult = validateUserGroupPermissionSetting( + setting as WorkspaceUserGroupPermissionSetting, + isUserIdOrGroupRequired(setting.id), + permissionSettings.slice(0, i) + ); + if (validateResult) { + permissionSettingsErrors[setting.id] = validateResult; + } } } if (Object.keys(permissionSettingsErrors).length > 0) { @@ -291,3 +344,108 @@ export const generateNextPermissionSettingsId = (permissionSettings: Array<{ id: ? 0 : Math.max(...permissionSettings.map(({ id }) => id)) + 1; }; + +/** + * + * Generate permission settings state with provided permission settings, + * If no permission settings provided, it will return current user with read permission and + * an empty user group permission setting. + * If permission settings provided, it will return original permission settings when user group permission settings exists. + * It will return permission setting with an empty user group permission setting when user group permission settings not exists. + * + * @param operationType + * @param permissionSettings + * @returns + */ +export const generatePermissionSettingsState = ( + operationType: WorkspaceOperationType, + permissionSettings?: WorkspacePermissionSetting[] +): WorkspacePermissionSetting[] => { + const emptyUserPermission: WorkspaceUserPermissionSetting = { + id: 1, + type: WorkspacePermissionItemType.User, + userId: '', + modes: optionIdToWorkspacePermissionModesMap[PermissionModeId.Owner], + }; + const emptyUserGroupPermission: WorkspaceUserGroupPermissionSetting = { + id: 2, + type: WorkspacePermissionItemType.Group, + group: '', + modes: optionIdToWorkspacePermissionModesMap[PermissionModeId.Read], + }; + + if (operationType === WorkspaceOperationType.Create) { + return [ + { + ...emptyUserPermission, + userId: CURRENT_USER_PLACEHOLDER, + }, + emptyUserGroupPermission, + ]; + } + + const finalPermissionSettings = [...(permissionSettings ?? [])]; + const userPermissionExists = finalPermissionSettings.find( + (setting) => setting.type === WorkspacePermissionItemType.User + ); + const groupPermissionExists = finalPermissionSettings.find( + (setting) => setting.type === WorkspacePermissionItemType.Group + ); + if (!userPermissionExists) { + finalPermissionSettings.push({ + ...emptyUserPermission, + id: generateNextPermissionSettingsId(finalPermissionSettings), + } as typeof finalPermissionSettings[0]); + } + if (!groupPermissionExists) { + finalPermissionSettings.push({ + ...emptyUserGroupPermission, + id: generateNextPermissionSettingsId(finalPermissionSettings), + } as typeof finalPermissionSettings[0]); + } + return finalPermissionSettings; +}; + +export const getNumberOfChanges = ( + newFormData: Omit, 'permissionSettings'> & { + permissionSettings?: Array< + Pick & Partial + >; + }, + initialFormData: Omit +) => { + let count = 0; + if (newFormData.name !== initialFormData.name) { + count++; + } + if (newFormData.description !== initialFormData.description) { + count++; + } + if ( + newFormData.features?.length !== initialFormData.features?.length || + newFormData.features?.some((item) => !initialFormData.features?.includes(item)) + ) { + count++; + } + // Count all new added permission settings + count += + newFormData.permissionSettings?.reduce((prevNewAddedCount, setting) => { + if (!initialFormData.permissionSettings?.find((item) => item.id === setting.id)) { + prevNewAddedCount += 1; + } + return prevNewAddedCount; + }, 0) ?? 0; + count += + initialFormData.permissionSettings?.reduce((prevDeletedAndModifiedCount, setting) => { + const newSetting = newFormData.permissionSettings?.find((item) => item.id === setting.id); + if (!newSetting) { + // Count all delete permission settings + prevDeletedAndModifiedCount += 1; + } else if (!isSamePermissionSetting(newSetting, setting)) { + // Count all modified permission settings + prevDeletedAndModifiedCount += 1; + } + return prevDeletedAndModifiedCount; + }, 0) ?? 0; + return count; +}; diff --git a/src/plugins/workspace/public/components/workspace_form/workspace_bottom_bar.tsx b/src/plugins/workspace/public/components/workspace_form/workspace_bottom_bar.tsx index 34d5ca6359e1..c189c937be45 100644 --- a/src/plugins/workspace/public/components/workspace_form/workspace_bottom_bar.tsx +++ b/src/plugins/workspace/public/components/workspace_form/workspace_bottom_bar.tsx @@ -13,29 +13,24 @@ import { EuiText, } from '@elastic/eui'; import { i18n } from '@osd/i18n'; -import React, { useState } from 'react'; +import React, { useState, useCallback } from 'react'; import { ApplicationStart } from 'opensearch-dashboards/public'; -import { WorkspaceOperationType } from '../workspace_form'; import { WorkspaceCancelModal } from './workspace_cancel_modal'; interface WorkspaceBottomBarProps { formId: string; - operationType?: WorkspaceOperationType; - numberOfErrors: number; application: ApplicationStart; - numberOfUnSavedChanges?: number; + numberOfChanges: number; } export const WorkspaceBottomBar = ({ formId, - operationType, - numberOfErrors, - numberOfUnSavedChanges, + numberOfChanges, application, }: WorkspaceBottomBarProps) => { const [isCancelModalVisible, setIsCancelModalVisible] = useState(false); - const closeCancelModal = () => setIsCancelModalVisible(false); - const showCancelModal = () => setIsCancelModalVisible(true); + const closeCancelModal = useCallback(() => setIsCancelModalVisible(false), []); + const showCancelModal = useCallback(() => setIsCancelModalVisible(true), []); return (
    @@ -45,21 +40,12 @@ export const WorkspaceBottomBar = ({ - {operationType === WorkspaceOperationType.Update ? ( + {numberOfChanges > 0 && ( {i18n.translate('workspace.form.bottomBar.unsavedChanges', { - defaultMessage: '{numberOfUnSavedChanges} Unsaved change(s)', + defaultMessage: '{numberOfChanges} Unsaved change(s)', values: { - numberOfUnSavedChanges, - }, - })} - - ) : ( - - {i18n.translate('workspace.form.bottomBar.errors', { - defaultMessage: '{numberOfErrors} Error(s)', - values: { - numberOfErrors, + numberOfChanges, }, })} @@ -78,32 +64,17 @@ export const WorkspaceBottomBar = ({ })} - {operationType === WorkspaceOperationType.Create && ( - - {i18n.translate('workspace.form.bottomBar.createWorkspace', { - defaultMessage: 'Create workspace', - })} - - )} - {operationType === WorkspaceOperationType.Update && ( - - {i18n.translate('workspace.form.bottomBar.saveChanges', { - defaultMessage: 'Save changes', - })} - - )} + + {i18n.translate('workspace.form.bottomBar.saveChanges', { + defaultMessage: 'Save changes', + })} + diff --git a/src/plugins/workspace/public/components/workspace_form/workspace_create_action_panel.tsx b/src/plugins/workspace/public/components/workspace_form/workspace_create_action_panel.tsx new file mode 100644 index 000000000000..f10a80395488 --- /dev/null +++ b/src/plugins/workspace/public/components/workspace_form/workspace_create_action_panel.tsx @@ -0,0 +1,56 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +import { EuiButton, EuiFlexGroup, EuiFlexItem } from '@elastic/eui'; +import { i18n } from '@osd/i18n'; +import React, { useState, useCallback } from 'react'; +import { ApplicationStart } from 'opensearch-dashboards/public'; +import { WorkspaceCancelModal } from './workspace_cancel_modal'; + +interface WorkspaceCreateActionPanelProps { + formId: string; + application: ApplicationStart; +} + +export const WorkspaceCreateActionPanel = ({ + formId, + application, +}: WorkspaceCreateActionPanelProps) => { + const [isCancelModalVisible, setIsCancelModalVisible] = useState(false); + const closeCancelModal = useCallback(() => setIsCancelModalVisible(false), []); + const showCancelModal = useCallback(() => setIsCancelModalVisible(true), []); + + return ( + <> + + + + {i18n.translate('workspace.form.bottomBar.cancel', { + defaultMessage: 'Cancel', + })} + + + + + {i18n.translate('workspace.form.bottomBar.createWorkspace', { + defaultMessage: 'Create workspace', + })} + + + + {isCancelModalVisible && ( + + )} + + ); +}; diff --git a/src/plugins/workspace/public/components/workspace_form/workspace_form.tsx b/src/plugins/workspace/public/components/workspace_form/workspace_form.tsx index 0bb461919291..6e9d60b8958f 100644 --- a/src/plugins/workspace/public/components/workspace_form/workspace_form.tsx +++ b/src/plugins/workspace/public/components/workspace_form/workspace_form.tsx @@ -14,9 +14,6 @@ import { EuiText, EuiColorPicker, EuiTextArea, - EuiButton, - EuiFlexGroup, - EuiFlexItem, } from '@elastic/eui'; import { i18n } from '@osd/i18n'; @@ -27,6 +24,7 @@ import { WorkspacePermissionSettingPanel } from './workspace_permission_setting_ import { WorkspaceUseCase } from './workspace_use_case'; import { WorkspaceOperationType } from './constants'; import { WorkspaceFormErrorCallout } from './workspace_form_error_callout'; +import { WorkspaceCreateActionPanel } from './workspace_create_action_panel'; export const WorkspaceForm = (props: WorkspaceFormProps) => { const { @@ -42,6 +40,7 @@ export const WorkspaceForm = (props: WorkspaceFormProps) => { formData, formErrors, numberOfErrors, + numberOfChanges, handleFormSubmit, handleColorChange, handleUseCasesChange, @@ -187,36 +186,13 @@ export const WorkspaceForm = (props: WorkspaceFormProps) => { )} {operationType === WorkspaceOperationType.Create && ( - - - - {i18n.translate('workspace.form.bottomBar.cancel', { - defaultMessage: 'Cancel', - })} - - - - {operationType === WorkspaceOperationType.Create && ( - - {i18n.translate('workspace.form.bottomBar.createWorkspace', { - defaultMessage: 'Create workspace', - })} - - )} - - + )} {operationType === WorkspaceOperationType.Update && ( )} diff --git a/src/plugins/workspace/public/components/workspace_form/workspace_permission_setting_input.tsx b/src/plugins/workspace/public/components/workspace_form/workspace_permission_setting_input.tsx index 083c7a91cba2..cd31e27e08a3 100644 --- a/src/plugins/workspace/public/components/workspace_form/workspace_permission_setting_input.tsx +++ b/src/plugins/workspace/public/components/workspace_form/workspace_permission_setting_input.tsx @@ -160,15 +160,17 @@ export const WorkspacePermissionSettingInput = ({ /> - + {deletable && ( + + )} ); diff --git a/src/plugins/workspace/public/components/workspace_form/workspace_permission_setting_panel.tsx b/src/plugins/workspace/public/components/workspace_form/workspace_permission_setting_panel.tsx index 1a7cce308928..af7811209cba 100644 --- a/src/plugins/workspace/public/components/workspace_form/workspace_permission_setting_panel.tsx +++ b/src/plugins/workspace/public/components/workspace_form/workspace_permission_setting_panel.tsx @@ -99,18 +99,28 @@ const UserOrGroupSection = ({
    - {type === WorkspacePermissionItemType.User - ? i18n.translate('workspaceForm.permissionSetting.userLabel', { - defaultMessage: 'User', - }) - : i18n.translate('workspaceForm.permissionSetting.groupLabel', { - defaultMessage: 'User group', - })} + + <> + - {i18n.translate('workspaceForm.permissionSetting.permissionLabel', { - defaultMessage: 'Permissions', - })} + + <> + @@ -121,7 +131,11 @@ const UserOrGroupSection = ({ {...item} type={type} index={index} - deletable={index !== nonDeletableIndex} + deletable={ + type === WorkspacePermissionItemType.Group || + index > 0 || + (!!item.modes && getPermissionModeId(item.modes) !== PermissionModeId.Owner) + } onDelete={handleDelete} onGroupOrUserIdChange={handleGroupOrUserIdChange} onPermissionModesChange={handlePermissionModesChange} @@ -230,7 +244,7 @@ export const WorkspacePermissionSettingPanel = ({ type={WorkspacePermissionItemType.User} nextIdGenerator={nextIdGenerator} /> - + { - if (!acl.hasPermission([permissionMode], { users: [currentUserId] })) { - acl.addPermission([permissionMode], { users: [currentUserId] }); - } - } - ); + const acl = new ACL(transferCurrentUserInPermissions(currentUserId, permissions)); createPayload.permissions = acl.getPermissions(); } } diff --git a/src/plugins/workspace/server/utils.ts b/src/plugins/workspace/server/utils.ts index 9037038f16af..d1f8ea080cd4 100644 --- a/src/plugins/workspace/server/utils.ts +++ b/src/plugins/workspace/server/utils.ts @@ -13,9 +13,11 @@ import { Principals, PrincipalType, SharedGlobalConfig, + Permissions, } from '../../../core/server'; import { AuthInfo } from './types'; import { updateWorkspaceState } from '../../../core/server/utils'; +import { CURRENT_USER_PLACEHOLDER } from '../common/constants'; /** * Generate URL friendly random ID @@ -89,3 +91,24 @@ export const getOSDAdminConfigFromYMLConfig = async ( return [groupsResult, usersResult]; }; + +export const transferCurrentUserInPermissions = ( + realUserId: string, + permissions: Permissions | undefined +) => { + if (!permissions) { + return permissions; + } + return Object.keys(permissions).reduce( + (previousPermissions, currentKey) => ({ + ...previousPermissions, + [currentKey]: { + ...permissions[currentKey], + users: permissions[currentKey].users?.map((user) => + user === CURRENT_USER_PLACEHOLDER ? realUserId : user + ), + }, + }), + {} + ); +}; From 6f6a8dc97f3e699083011195c323ac97408df22f Mon Sep 17 00:00:00 2001 From: Lin Wang Date: Fri, 21 Jun 2024 17:53:51 +0800 Subject: [PATCH 08/29] Fix changes Signed-off-by: Lin Wang --- .../public/components/workspace_form/types.ts | 1 - .../workspace_form/use_workspace_form.ts | 15 +----- .../public/components/workspace_form/utils.ts | 49 ++++++++++--------- .../workspace_form/workspace_form.tsx | 23 --------- 4 files changed, 28 insertions(+), 60 deletions(-) diff --git a/src/plugins/workspace/public/components/workspace_form/types.ts b/src/plugins/workspace/public/components/workspace_form/types.ts index a1298afe7d55..e0cb38e88ea2 100644 --- a/src/plugins/workspace/public/components/workspace_form/types.ts +++ b/src/plugins/workspace/public/components/workspace_form/types.ts @@ -29,7 +29,6 @@ export interface WorkspaceFormSubmitData { name: string; description?: string; features?: string[]; - color?: string; permissionSettings?: WorkspacePermissionSetting[]; } 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 index 7fc88b755d56..88bad388d742 100644 --- a/src/plugins/workspace/public/components/workspace_form/use_workspace_form.ts +++ b/src/plugins/workspace/public/components/workspace_form/use_workspace_form.ts @@ -4,12 +4,7 @@ */ import { useCallback, useState, FormEventHandler, useRef, useMemo } from 'react'; -import { - htmlIdGenerator, - EuiFieldTextProps, - EuiColorPickerProps, - EuiTextAreaProps, -} from '@elastic/eui'; +import { htmlIdGenerator, EuiFieldTextProps, EuiTextAreaProps } from '@elastic/eui'; import { useApplications } from '../../hooks'; import { @@ -41,7 +36,6 @@ export const useWorkspaceForm = ({ const applications = useApplications(application); const [name, setName] = useState(defaultValues?.name); const [description, setDescription] = useState(defaultValues?.description); - const [color, setColor] = useState(defaultValues?.color); const defaultValuesRef = useRef(defaultValues); const initialPermissionSettingsRef = useRef( generatePermissionSettingsState(operationType, defaultValues?.permissionSettings) @@ -66,7 +60,6 @@ export const useWorkspaceForm = ({ description, features: featureConfigs, useCases: selectedUseCases, - color, permissionSettings, }); const getFormDataRef = useRef(getFormData); @@ -115,7 +108,6 @@ export const useWorkspaceForm = ({ name: currentFormData.name!, description: currentFormData.description, features: currentFormData.features, - color: currentFormData.color, permissionSettings: currentFormData.permissionSettings.filter( (item) => (item.type === WorkspacePermissionItemType.User && !!item.userId) || @@ -134,10 +126,6 @@ export const useWorkspaceForm = ({ setDescription(e.target.value); }, []); - const handleColorChange = useCallback['onChange']>((text) => { - setColor(text); - }, []); - return { formId: formIdRef.current, formData, @@ -146,7 +134,6 @@ export const useWorkspaceForm = ({ numberOfErrors, numberOfChanges, handleFormSubmit, - handleColorChange, handleUseCasesChange, handleNameInputChange, setPermissionSettings, diff --git a/src/plugins/workspace/public/components/workspace_form/utils.ts b/src/plugins/workspace/public/components/workspace_form/utils.ts index 5c457fad323c..f4d90f748c91 100644 --- a/src/plugins/workspace/public/components/workspace_form/utils.ts +++ b/src/plugins/workspace/public/components/workspace_form/utils.ts @@ -61,23 +61,18 @@ export const getNumberOfErrors = (formErrors: WorkspaceFormErrors) => { return numberOfErrors; }; -const isSamePermissionSetting = ( - a: Partial, - b: Partial -) => - (a.type === WorkspacePermissionItemType.User && - b.type === WorkspacePermissionItemType.User && - a.userId === b.userId) || - (a.type === WorkspacePermissionItemType.Group && - b.type === WorkspacePermissionItemType.Group && - a.group === b.group); - -export const isUserOrGroupPermissionSettingDuplicated = ( +export const hasSameUserIdOrGroup = ( permissionSettings: Array>, permissionSettingToCheck: WorkspacePermissionSetting ) => - permissionSettings.some((permissionSetting) => - isSamePermissionSetting(permissionSetting, permissionSettingToCheck) + permissionSettings.some( + (permissionSetting) => + (permissionSetting.type === WorkspacePermissionItemType.User && + permissionSettingToCheck.type === WorkspacePermissionItemType.User && + permissionSetting.userId === permissionSettingToCheck.userId) || + (permissionSetting.type === WorkspacePermissionItemType.Group && + permissionSettingToCheck.type === WorkspacePermissionItemType.Group && + permissionSetting.group === permissionSettingToCheck.group) ); /** @@ -211,10 +206,7 @@ const validateUserPermissionSetting = ( }), }; } - if ( - !!setting.userId && - isUserOrGroupPermissionSettingDuplicated(previousPermissionSettings, setting) - ) { + if (!!setting.userId && hasSameUserIdOrGroup(previousPermissionSettings, setting)) { return { code: WorkspaceFormErrorCode.DuplicateUserPermissionSetting, message: i18n.translate( @@ -240,10 +232,7 @@ const validateUserGroupPermissionSetting = ( }), }; } - if ( - !!setting.group && - isUserOrGroupPermissionSettingDuplicated(previousPermissionSettings, setting) - ) { + if (!!setting.group && hasSameUserIdOrGroup(previousPermissionSettings, setting)) { return { code: WorkspaceFormErrorCode.DuplicateUserGroupPermissionSetting, message: i18n.translate( @@ -406,6 +395,22 @@ export const generatePermissionSettingsState = ( return finalPermissionSettings; }; +interface PermissionSettingLike + extends Omit, 'type'>, + Omit, 'type'> { + type?: string; +} +const isSamePermissionSetting = (a: PermissionSettingLike, b: PermissionSettingLike) => { + return ( + a.id === b.id && + a.type === b.type && + a.userId === b.userId && + a.group === b.group && + a.modes?.length === b.modes?.length && + a.modes?.every((mode) => b.modes?.includes(mode)) + ); +}; + export const getNumberOfChanges = ( newFormData: Omit, 'permissionSettings'> & { permissionSettings?: Array< diff --git a/src/plugins/workspace/public/components/workspace_form/workspace_form.tsx b/src/plugins/workspace/public/components/workspace_form/workspace_form.tsx index 6e9d60b8958f..97387ca37d7c 100644 --- a/src/plugins/workspace/public/components/workspace_form/workspace_form.tsx +++ b/src/plugins/workspace/public/components/workspace_form/workspace_form.tsx @@ -12,7 +12,6 @@ import { EuiFormRow, EuiFieldText, EuiText, - EuiColorPicker, EuiTextArea, } from '@elastic/eui'; import { i18n } from '@osd/i18n'; @@ -42,7 +41,6 @@ export const WorkspaceForm = (props: WorkspaceFormProps) => { numberOfErrors, numberOfChanges, handleFormSubmit, - handleColorChange, handleUseCasesChange, handleNameInputChange, setPermissionSettings, @@ -117,27 +115,6 @@ export const WorkspaceForm = (props: WorkspaceFormProps) => { /> - -
    - - {i18n.translate('workspace.form.workspaceDetails.color.helpText', { - defaultMessage: 'Accent color for your workspace', - })} - - - -
    -     From a43a45ad41102a67ace02360a7da54e5cbf2cfa4 Mon Sep 17 00:00:00 2001 From: Lin Wang Date: Fri, 21 Jun 2024 18:06:05 +0800 Subject: [PATCH 09/29] Fix owner order Signed-off-by: Lin Wang --- .../public/components/workspace_form/utils.ts | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/plugins/workspace/public/components/workspace_form/utils.ts b/src/plugins/workspace/public/components/workspace_form/utils.ts index f4d90f748c91..d481e988a9db 100644 --- a/src/plugins/workspace/public/components/workspace_form/utils.ts +++ b/src/plugins/workspace/public/components/workspace_form/utils.ts @@ -119,12 +119,6 @@ export const convertPermissionSettingsToPermissions = ( }, {}); }; -const isWorkspacePermissionMode = (test: string): test is WorkspacePermissionMode => - test === WorkspacePermissionMode.LibraryRead || - test === WorkspacePermissionMode.LibraryWrite || - test === WorkspacePermissionMode.Read || - test === WorkspacePermissionMode.Write; - export const convertPermissionsToPermissionSettings = (permissions: SavedObjectPermissions) => { const permissionSettings: WorkspacePermissionSetting[] = []; const finalPermissionSettings: WorkspacePermissionSetting[] = []; @@ -154,8 +148,14 @@ export const convertPermissionsToPermissionSettings = (permissions: SavedObjectP }); }; - Object.keys(permissions).forEach((mode) => { - if (isWorkspacePermissionMode(mode)) { + // Since owner should always be the first row of permissions, specific the process order let owner moved to the top + [ + WorkspacePermissionMode.Write, + WorkspacePermissionMode.LibraryWrite, + WorkspacePermissionMode.LibraryRead, + WorkspacePermissionMode.Read, + ].forEach((mode) => { + if (permissions[mode]) { processUsersOrGroups(permissions[mode].users, WorkspacePermissionItemType.User, mode); processUsersOrGroups(permissions[mode].groups, WorkspacePermissionItemType.Group, mode); } From 380b3f20d9af46b999e3c192a9d4db4397dda9ae Mon Sep 17 00:00:00 2001 From: Lin Wang Date: Mon, 24 Jun 2024 11:51:04 +0800 Subject: [PATCH 10/29] Add ut for form error callout Signed-off-by: Lin Wang --- .../public/components/workspace_form/types.ts | 2 +- .../components/workspace_form/utils.test.ts | 67 +++++++--- .../public/components/workspace_form/utils.ts | 4 +- .../workspace_form_error_callout.test.tsx | 125 ++++++++++++++++++ .../workspace_form_error_callout.tsx | 12 +- 5 files changed, 187 insertions(+), 23 deletions(-) create mode 100644 src/plugins/workspace/public/components/workspace_form/workspace_form_error_callout.test.tsx diff --git a/src/plugins/workspace/public/components/workspace_form/types.ts b/src/plugins/workspace/public/components/workspace_form/types.ts index e0cb38e88ea2..313739fd86f8 100644 --- a/src/plugins/workspace/public/components/workspace_form/types.ts +++ b/src/plugins/workspace/public/components/workspace_form/types.ts @@ -45,7 +45,7 @@ export enum WorkspaceFormErrorCode { InvalidPermissionModes, PermissionUserIdMissing, PermissionUserGroupMissing, - DuplicateUserPermissionSetting, + DuplicateUserIdPermissionSetting, DuplicateUserGroupPermissionSetting, } diff --git a/src/plugins/workspace/public/components/workspace_form/utils.test.ts b/src/plugins/workspace/public/components/workspace_form/utils.test.ts index 6935f84eda35..a90d0d38e485 100644 --- a/src/plugins/workspace/public/components/workspace_form/utils.test.ts +++ b/src/plugins/workspace/public/components/workspace_form/utils.test.ts @@ -10,6 +10,7 @@ import { } from './utils'; import { WorkspacePermissionMode } from '../../../common/constants'; import { WorkspacePermissionItemType } from './constants'; +import { WorkspaceFormErrorCode } from './types'; describe('convertPermissionSettingsToPermissions', () => { it('should return undefined if permission items not provided', () => { @@ -88,16 +89,16 @@ describe('convertPermissionsToPermissionSettings', () => { ).toEqual([ { id: 0, - type: WorkspacePermissionItemType.User, - userId: 'foo', - modes: [WorkspacePermissionMode.LibraryRead, WorkspacePermissionMode.Read], - }, - { - id: 1, type: WorkspacePermissionItemType.Group, group: 'bar', modes: [WorkspacePermissionMode.LibraryWrite, WorkspacePermissionMode.Write], }, + { + id: 1, + type: WorkspacePermissionItemType.User, + userId: 'foo', + modes: [WorkspacePermissionMode.LibraryRead, WorkspacePermissionMode.Read], + }, ]); }); it('should separate to multi permission settings', () => { @@ -153,13 +154,22 @@ describe('convertPermissionsToPermissionSettings', () => { describe('validateWorkspaceForm', () => { it('should return error if name is empty', () => { - expect(validateWorkspaceForm({}).name).toEqual("Name can't be empty."); + expect(validateWorkspaceForm({}).name).toEqual({ + code: WorkspaceFormErrorCode.WorkspaceNameMissing, + message: "Name can't be empty.", + }); }); it('should return error if name is invalid', () => { - expect(validateWorkspaceForm({ name: '~' }).name).toEqual('Invalid workspace name'); + expect(validateWorkspaceForm({ name: '~' }).name).toEqual({ + code: WorkspaceFormErrorCode.InvalidWorkspaceName, + message: 'Invalid workspace name', + }); }); it('should return error if use case is empty', () => { - expect(validateWorkspaceForm({}).features).toEqual('Use case is required. Select a use case.'); + expect(validateWorkspaceForm({}).features).toEqual({ + code: WorkspaceFormErrorCode.UseCaseMissing, + message: 'Use case is required. Select a use case.', + }); }); it('should return error if permission setting type is invalid', () => { expect( @@ -167,7 +177,9 @@ describe('validateWorkspaceForm', () => { name: 'test', permissionSettings: [{ id: 0 }], }).permissionSettings - ).toEqual({ 0: 'Invalid type' }); + ).toEqual({ + 0: { code: WorkspaceFormErrorCode.InvalidPermissionType, message: 'Invalid type' }, + }); }); it('should return error if permission setting modes is invalid', () => { expect( @@ -175,7 +187,12 @@ describe('validateWorkspaceForm', () => { name: 'test', permissionSettings: [{ id: 0, type: WorkspacePermissionItemType.User, modes: [] }], }).permissionSettings - ).toEqual({ 0: 'Invalid permission modes' }); + ).toEqual({ + 0: { + code: WorkspaceFormErrorCode.InvalidPermissionModes, + message: 'Invalid permission modes', + }, + }); }); it('should return error if permission setting user id is invalid', () => { expect( @@ -190,7 +207,12 @@ describe('validateWorkspaceForm', () => { }, ], }).permissionSettings - ).toEqual({ 0: 'Invalid user id' }); + ).toEqual({ + 0: { + code: WorkspaceFormErrorCode.PermissionUserIdMissing, + message: 'Invalid user id', + }, + }); }); it('should return error if permission setting group is invalid', () => { expect( @@ -205,7 +227,12 @@ describe('validateWorkspaceForm', () => { }, ], }).permissionSettings - ).toEqual({ 0: 'Invalid user group' }); + ).toEqual({ + 0: { + code: WorkspaceFormErrorCode.PermissionUserGroupMissing, + message: 'Invalid user group', + }, + }); }); it('should return error if permission setting is duplicate', () => { @@ -227,7 +254,12 @@ describe('validateWorkspaceForm', () => { }, ], }).permissionSettings - ).toEqual({ 1: 'Duplicate permission setting' }); + ).toEqual({ + 1: { + code: WorkspaceFormErrorCode.DuplicateUserIdPermissionSetting, + message: 'User must be unique. Enter a unique user.', + }, + }); expect( validateWorkspaceForm({ name: 'test', @@ -246,7 +278,12 @@ describe('validateWorkspaceForm', () => { }, ], }).permissionSettings - ).toEqual({ 1: 'Duplicate permission setting' }); + ).toEqual({ + 1: { + code: WorkspaceFormErrorCode.DuplicateUserGroupPermissionSetting, + message: 'Duplicate permission setting', + }, + }); }); it('should return empty object for valid for data', () => { diff --git a/src/plugins/workspace/public/components/workspace_form/utils.ts b/src/plugins/workspace/public/components/workspace_form/utils.ts index d481e988a9db..c68ff8433c78 100644 --- a/src/plugins/workspace/public/components/workspace_form/utils.ts +++ b/src/plugins/workspace/public/components/workspace_form/utils.ts @@ -208,9 +208,9 @@ const validateUserPermissionSetting = ( } if (!!setting.userId && hasSameUserIdOrGroup(previousPermissionSettings, setting)) { return { - code: WorkspaceFormErrorCode.DuplicateUserPermissionSetting, + code: WorkspaceFormErrorCode.DuplicateUserIdPermissionSetting, message: i18n.translate( - 'workspace.form.permission.invalidate.duplicateUserPermissionSetting', + 'workspace.form.permission.invalidate.DuplicateUserIdPermissionSetting', { defaultMessage: 'User must be unique. Enter a unique user.', } diff --git a/src/plugins/workspace/public/components/workspace_form/workspace_form_error_callout.test.tsx b/src/plugins/workspace/public/components/workspace_form/workspace_form_error_callout.test.tsx new file mode 100644 index 000000000000..701f62699cca --- /dev/null +++ b/src/plugins/workspace/public/components/workspace_form/workspace_form_error_callout.test.tsx @@ -0,0 +1,125 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +import React from 'react'; +import { render } from '@testing-library/react'; +import { + WorkspaceFormErrorCallout, + WorkspaceFormErrorCalloutProps, +} from './workspace_form_error_callout'; +import { WorkspaceFormErrorCode } from './types'; + +const setup = (options?: Partial) => { + const renderResult = render(); + return { + renderResult, + }; +}; + +describe('WorkspaceFormErrorCallout', () => { + it('should render error callout title', () => { + const { renderResult } = setup({}); + + expect( + renderResult.getByText('Address the following error(s) in the form') + ).toBeInTheDocument(); + }); + + it('should render workspace name suggestion', () => { + const { renderResult } = setup({ + errors: { + name: { + code: WorkspaceFormErrorCode.WorkspaceNameMissing, + message: '', + }, + }, + }); + + expect(renderResult.getByText('Name: Enter a workspace name.')).toBeInTheDocument(); + + renderResult.rerender( + + ); + + expect(renderResult.getByText('Name: Enter a valid workspace name.')).toBeInTheDocument(); + }); + + it('should render use case suggestion', () => { + const { renderResult } = setup({ + errors: { + name: { + code: WorkspaceFormErrorCode.WorkspaceNameMissing, + message: '', + }, + }, + }); + + expect(renderResult.getByText('Name: Enter a workspace name.')).toBeInTheDocument(); + }); + + it('should combine user permission settings suggestions', () => { + const { renderResult } = setup({ + errors: { + permissionSettings: { + 0: { + code: WorkspaceFormErrorCode.PermissionUserIdMissing, + message: '', + }, + 1: { + code: WorkspaceFormErrorCode.PermissionUserIdMissing, + message: '', + }, + 2: { + code: WorkspaceFormErrorCode.DuplicateUserIdPermissionSetting, + message: '', + }, + 3: { + code: WorkspaceFormErrorCode.DuplicateUserIdPermissionSetting, + message: '', + }, + }, + }, + }); + + expect(renderResult.getByText('User: Enter a user.')).toBeInTheDocument(); + expect(renderResult.getAllByText('User: Enter a user.')).toHaveLength(1); + + expect(renderResult.getByText('User: Enter a unique user.')).toBeInTheDocument(); + expect(renderResult.getAllByText('User: Enter a unique user.')).toHaveLength(1); + }); + + it('should combine user group permission settings suggestions', () => { + const { renderResult } = setup({ + errors: { + permissionSettings: { + 0: { + code: WorkspaceFormErrorCode.PermissionUserGroupMissing, + message: '', + }, + 1: { + code: WorkspaceFormErrorCode.PermissionUserGroupMissing, + message: '', + }, + 2: { + code: WorkspaceFormErrorCode.DuplicateUserGroupPermissionSetting, + message: '', + }, + 3: { + code: WorkspaceFormErrorCode.DuplicateUserGroupPermissionSetting, + message: '', + }, + }, + }, + }); + + expect(renderResult.getByText('User Group: Enter a user group.')).toBeInTheDocument(); + expect(renderResult.getAllByText('User Group: Enter a user group.')).toHaveLength(1); + + expect(renderResult.getByText('User Group: Enter a unique user group.')).toBeInTheDocument(); + expect(renderResult.getAllByText('User Group: Enter a unique user group.')).toHaveLength(1); + }); +}); diff --git a/src/plugins/workspace/public/components/workspace_form/workspace_form_error_callout.tsx b/src/plugins/workspace/public/components/workspace_form/workspace_form_error_callout.tsx index bb2f16bb9fe9..e51340102fa8 100644 --- a/src/plugins/workspace/public/components/workspace_form/workspace_form_error_callout.tsx +++ b/src/plugins/workspace/public/components/workspace_form/workspace_form_error_callout.tsx @@ -30,7 +30,7 @@ const getSuggestionFromErrorCode = (error: WorkspaceFormError) => { return i18n.translate('workspace.form.errorCallout.missingUserGroup', { defaultMessage: 'Enter a user group.', }); - case WorkspaceFormErrorCode.DuplicateUserPermissionSetting: + case WorkspaceFormErrorCode.DuplicateUserIdPermissionSetting: return i18n.translate('workspace.form.errorCallout.duplicatePermission', { defaultMessage: 'Enter a unique user.', }); @@ -60,7 +60,7 @@ const WorkspaceFormErrorCalloutItem = ({ ); }; -interface WorkspaceFormErrorCalloutProps { +export interface WorkspaceFormErrorCalloutProps { errors: WorkspaceFormErrors; } @@ -79,7 +79,7 @@ export const WorkspaceFormErrorCallout = ({ errors }: WorkspaceFormErrorCalloutP return (
  • - {(errorCode === WorkspaceFormErrorCode.DuplicateUserPermissionSetting || + {(errorCode === WorkspaceFormErrorCode.DuplicateUserIdPermissionSetting || errorCode === WorkspaceFormErrorCode.PermissionUserIdMissing) && i18n.translate('workspace.form.errorCallout.userPermissionKey', { defaultMessage: 'User:', @@ -87,7 +87,7 @@ export const WorkspaceFormErrorCallout = ({ errors }: WorkspaceFormErrorCalloutP {(errorCode === WorkspaceFormErrorCode.DuplicateUserGroupPermissionSetting || errorCode === WorkspaceFormErrorCode.PermissionUserGroupMissing) && i18n.translate('workspace.form.errorCallout.userGroupPermissionKey', { - defaultMessage: 'Group:', + defaultMessage: 'User Group:', })}   {getSuggestionFromErrorCode(findingError)} @@ -115,7 +115,9 @@ export const WorkspaceFormErrorCallout = ({ errors }: WorkspaceFormErrorCalloutP /> )} {renderPermissionSettingSuggestion(WorkspaceFormErrorCode.PermissionUserIdMissing)} - {renderPermissionSettingSuggestion(WorkspaceFormErrorCode.DuplicateUserPermissionSetting)} + {renderPermissionSettingSuggestion( + WorkspaceFormErrorCode.DuplicateUserIdPermissionSetting + )} {renderPermissionSettingSuggestion(WorkspaceFormErrorCode.PermissionUserGroupMissing)} {renderPermissionSettingSuggestion( WorkspaceFormErrorCode.DuplicateUserGroupPermissionSetting From 5ee654a2cd85c6daf55328ab53ff2f9912125063 Mon Sep 17 00:00:00 2001 From: Lin Wang Date: Mon, 24 Jun 2024 16:40:46 +0800 Subject: [PATCH 11/29] Fix unit tests in workspace Signed-off-by: Lin Wang --- .../workspace_creator.test.tsx | 27 +++-------- .../workspace_creator/workspace_creator.tsx | 2 +- .../workspace_form/use_workspace_form.test.ts | 12 +++-- .../components/workspace_form/utils.test.ts | 42 +--------------- ...orkspace_permission_setting_panel.test.tsx | 48 +++---------------- .../workspace_updater.test.tsx | 38 +++++++-------- 6 files changed, 42 insertions(+), 127 deletions(-) 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 index 1c1632f7cb4d..5e2e76447a4d 100644 --- a/src/plugins/workspace/public/components/workspace_creator/workspace_creator.test.tsx +++ b/src/plugins/workspace/public/components/workspace_creator/workspace_creator.test.tsx @@ -155,22 +155,15 @@ describe('WorkspaceCreator', () => { fireEvent.input(descriptionInput, { target: { value: 'test workspace description' }, }); - const colorSelector = getByTestId( - 'euiColorPickerAnchor workspaceForm-workspaceDetails-colorPicker' - ); - fireEvent.input(colorSelector, { - target: { value: '#000000' }, - }); fireEvent.click(getByTestId('workspaceUseCase-observability')); fireEvent.click(getByTestId('workspaceForm-bottomBar-createButton')); expect(workspaceClientCreate).toHaveBeenCalledWith( expect.objectContaining({ name: 'test workspace name', - color: '#000000', description: 'test workspace description', features: expect.arrayContaining(['use-case-observability']), }), - undefined + { library_write: { users: ['%current_user%'] }, write: { users: ['%current_user%'] } } ); await waitFor(() => { expect(notificationToastsAddSuccess).toHaveBeenCalled(); @@ -220,8 +213,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( @@ -232,23 +225,17 @@ 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({ name: 'test workspace name', }), { - read: { - users: ['test user id'], + write: { + users: ['%current_user%'], }, - library_read: { - users: ['test user id'], + library_write: { + users: ['%current_user%'], }, } ); diff --git a/src/plugins/workspace/public/components/workspace_creator/workspace_creator.tsx b/src/plugins/workspace/public/components/workspace_creator/workspace_creator.tsx index 3255e82b45d5..bce222c81096 100644 --- 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'; 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 index 9e67d49bd07d..86ddd22ca625 100644 --- 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,7 +6,7 @@ import { renderHook, act } from '@testing-library/react-hooks'; import { applicationServiceMock } from '../../../../../core/public/mocks'; -import { WorkspaceFormData } from './types'; +import { WorkspaceFormData, WorkspaceFormErrorCode } from './types'; import { useWorkspaceForm } from './use_workspace_form'; const setup = (defaultValues?: WorkspaceFormData) => { @@ -37,7 +37,10 @@ describe('useWorkspaceForm', () => { }); expect(renderResult.result.current.formErrors).toEqual( expect.objectContaining({ - name: 'Invalid workspace name', + name: { + code: WorkspaceFormErrorCode.InvalidWorkspaceName, + message: 'Invalid workspace name', + }, }) ); expect(onSubmitMock).not.toHaveBeenCalled(); @@ -54,7 +57,10 @@ 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(); diff --git a/src/plugins/workspace/public/components/workspace_form/utils.test.ts b/src/plugins/workspace/public/components/workspace_form/utils.test.ts index a90d0d38e485..83b2215140eb 100644 --- a/src/plugins/workspace/public/components/workspace_form/utils.test.ts +++ b/src/plugins/workspace/public/components/workspace_form/utils.test.ts @@ -194,46 +194,6 @@ describe('validateWorkspaceForm', () => { }, }); }); - it('should return error if permission setting user id is invalid', () => { - expect( - validateWorkspaceForm({ - name: 'test', - permissionSettings: [ - { - id: 0, - type: WorkspacePermissionItemType.User, - modes: [WorkspacePermissionMode.LibraryRead], - userId: '', - }, - ], - }).permissionSettings - ).toEqual({ - 0: { - code: WorkspaceFormErrorCode.PermissionUserIdMissing, - message: 'Invalid user id', - }, - }); - }); - it('should return error if permission setting group is invalid', () => { - expect( - validateWorkspaceForm({ - name: 'test', - permissionSettings: [ - { - id: 0, - type: WorkspacePermissionItemType.Group, - modes: [WorkspacePermissionMode.LibraryRead], - group: '', - }, - ], - }).permissionSettings - ).toEqual({ - 0: { - code: WorkspaceFormErrorCode.PermissionUserGroupMissing, - message: 'Invalid user group', - }, - }); - }); it('should return error if permission setting is duplicate', () => { expect( @@ -281,7 +241,7 @@ describe('validateWorkspaceForm', () => { ).toEqual({ 1: { code: WorkspaceFormErrorCode.DuplicateUserGroupPermissionSetting, - message: 'Duplicate permission setting', + message: 'User group must be unique. Enter a unique user group.', }, }); }); diff --git a/src/plugins/workspace/public/components/workspace_form/workspace_permission_setting_panel.test.tsx b/src/plugins/workspace/public/components/workspace_form/workspace_permission_setting_panel.test.tsx index a3c687ba5133..4b5057e22365 100644 --- a/src/plugins/workspace/public/components/workspace_form/workspace_permission_setting_panel.test.tsx +++ b/src/plugins/workspace/public/components/workspace_form/workspace_permission_setting_panel.test.tsx @@ -130,7 +130,7 @@ describe('WorkspacePermissionSettingInput', () => { ]); }); - it('should not able to delete last user admin permission setting', () => { + it('should hide first user permission setting delete button', () => { const { renderResult } = setup({ lastAdminItemDeletable: false, permissionSettings: [ @@ -143,51 +143,15 @@ describe('WorkspacePermissionSettingInput', () => { ], }); - expect(renderResult.getByLabelText('Delete permission setting')).toBeDisabled(); - }); - - it('should not able to delete last group admin permission setting', () => { - const { renderResult } = setup({ - lastAdminItemDeletable: false, - permissionSettings: [ - { - id: 0, - type: WorkspacePermissionItemType.Group, - group: 'bar', - modes: [WorkspacePermissionMode.LibraryWrite, WorkspacePermissionMode.Write], - }, - ], - }); - - expect(renderResult.getByLabelText('Delete permission setting')).toBeDisabled(); - }); - - it('should able to delete permission setting if more than one admin permission', () => { - const { renderResult } = setup({ - lastAdminItemDeletable: false, - permissionSettings: [ - { - id: 0, - type: WorkspacePermissionItemType.User, - userId: 'foo', - modes: [WorkspacePermissionMode.LibraryWrite, WorkspacePermissionMode.Write], - }, - { - id: 0, - type: WorkspacePermissionItemType.Group, - group: 'bar', - modes: [WorkspacePermissionMode.LibraryWrite, WorkspacePermissionMode.Write], - }, - ], - }); - - expect(renderResult.getAllByLabelText('Delete permission setting')[0]).not.toBeDisabled(); - expect(renderResult.getAllByLabelText('Delete permission setting')[1]).not.toBeDisabled(); + expect(renderResult.queryByLabelText('Delete permission setting')).toBeNull(); }); it('should render consistent errors', () => { const { renderResult } = setup({ - errors: { '0': 'User permission setting error', '1': 'Group permission setting error' }, + errors: { + '0': { code: 0, message: 'User permission setting error' }, + '1': { code: 0, message: 'Group permission setting error' }, + }, }); expect(renderResult.container.querySelectorAll('.euiFormErrorText')[0]).toHaveTextContent( 'User permission setting error' diff --git a/src/plugins/workspace/public/components/workspace_updater/workspace_updater.test.tsx b/src/plugins/workspace/public/components/workspace_updater/workspace_updater.test.tsx index 463db3591f9c..8f71a8bb1b11 100644 --- a/src/plugins/workspace/public/components/workspace_updater/workspace_updater.test.tsx +++ b/src/plugins/workspace/public/components/workspace_updater/workspace_updater.test.tsx @@ -7,7 +7,7 @@ import React from 'react'; import { PublicAppInfo, WorkspaceObject } from 'opensearch-dashboards/public'; import { fireEvent, render, waitFor } from '@testing-library/react'; import { BehaviorSubject } from 'rxjs'; -import { WorkspaceUpdater as WorkspaceCreatorComponent } from './workspace_updater'; +import { WorkspaceUpdater as WorkspaceUpdaterComponent } from './workspace_updater'; import { coreMock, workspacesServiceMock } from '../../../../../core/public/mocks'; import { createOpenSearchDashboardsReactContext } from '../../../../opensearch_dashboards_react/public'; @@ -22,14 +22,20 @@ const PublicAPPInfoMap = new Map([ ]); const createWorkspacesSetupContractMockWithValue = () => { const currentWorkspaceId$ = new BehaviorSubject('abljlsds'); - const currentWorkspace: WorkspaceObject = { + const currentWorkspace = { id: 'abljlsds', name: 'test1', description: 'test1', features: ['use-case-observability'], - color: '', - icon: '', reserved: false, + permissions: { + library_write: { + users: ['foo'], + }, + write: { + users: ['foo'], + }, + }, }; const workspaceList$ = new BehaviorSubject([currentWorkspace]); const currentWorkspace$ = new BehaviorSubject(currentWorkspace); @@ -79,7 +85,7 @@ const WorkspaceUpdater = (props: any) => { return ( - + ); }; @@ -148,7 +154,7 @@ describe('WorkspaceUpdater', () => { }); it('update workspace successfully', async () => { - const { getByTestId, getByText, getAllByText } = render( + const { getByTestId, getAllByTestId } = render( @@ -163,38 +169,30 @@ describe('WorkspaceUpdater', () => { target: { value: 'test workspace description' }, }); - const colorSelector = getByTestId( - 'euiColorPickerAnchor workspaceForm-workspaceDetails-colorPicker' - ); - fireEvent.input(colorSelector, { - target: { value: '#000000' }, - }); - fireEvent.click(getByTestId('workspaceUseCase-observability')); fireEvent.click(getByTestId('workspaceUseCase-analytics')); - fireEvent.click(getByTestId('workspaceForm-permissionSettingPanel-user-addNew')); - const userIdInput = getAllByText('Select')[0]; + const userIdInput = getAllByTestId('comboBoxSearchInput')[0]; fireEvent.click(userIdInput); - fireEvent.input(getByTestId('comboBoxSearchInput'), { + + fireEvent.input(userIdInput, { target: { value: 'test user id' }, }); - fireEvent.blur(getByTestId('comboBoxSearchInput')); + fireEvent.blur(userIdInput); fireEvent.click(getByTestId('workspaceForm-bottomBar-updateButton')); expect(workspaceClientUpdate).toHaveBeenCalledWith( expect.any(String), expect.objectContaining({ name: 'test workspace name', - color: '#000000', description: 'test workspace description', features: expect.arrayContaining(['use-case-analytics']), }), { - read: { + library_write: { users: ['test user id'], }, - library_read: { + write: { users: ['test user id'], }, } From c43014b2cfa7829f2e12133028ce8ddb4b953984 Mon Sep 17 00:00:00 2001 From: Lin Wang Date: Mon, 24 Jun 2024 16:53:45 +0800 Subject: [PATCH 12/29] Mark first user row required Signed-off-by: Lin Wang --- .../workspace/public/components/workspace_form/utils.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/plugins/workspace/public/components/workspace_form/utils.ts b/src/plugins/workspace/public/components/workspace_form/utils.ts index c68ff8433c78..084fdb437b88 100644 --- a/src/plugins/workspace/public/components/workspace_form/utils.ts +++ b/src/plugins/workspace/public/components/workspace_form/utils.ts @@ -285,6 +285,7 @@ export const validateWorkspaceForm = ( const requiredSettingIds = initialFormData?.permissionSettings?.map((item) => item.id); const isUserIdOrGroupRequired = (settingId: number) => !!requiredSettingIds?.includes(settingId); + let firstUserRequiredFlag = true; for (let i = 0; i < permissionSettings.length; i++) { const setting = permissionSettings[i]; if (!setting.type) { @@ -304,12 +305,13 @@ export const validateWorkspaceForm = ( } else if (setting.type === WorkspacePermissionItemType.User) { const validateResult = validateUserPermissionSetting( setting as WorkspaceUserPermissionSetting, - isUserIdOrGroupRequired(setting.id), + isUserIdOrGroupRequired(setting.id) || firstUserRequiredFlag, permissionSettings.slice(0, i) ); if (validateResult) { permissionSettingsErrors[setting.id] = validateResult; } + firstUserRequiredFlag = false; } else if (setting.type === WorkspacePermissionItemType.Group) { const validateResult = validateUserGroupPermissionSetting( setting as WorkspaceUserGroupPermissionSetting, From 47ed32e79d7209c72520edb0836bceaa68c94804 Mon Sep 17 00:00:00 2001 From: Lin Wang Date: Thu, 27 Jun 2024 10:44:46 +0800 Subject: [PATCH 13/29] Update section title Signed-off-by: Lin Wang --- .../workspace/public/components/workspace_form/utils.ts | 2 +- .../public/components/workspace_form/workspace_form.tsx | 2 +- .../workspace_form/workspace_form_error_callout.tsx | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/plugins/workspace/public/components/workspace_form/utils.ts b/src/plugins/workspace/public/components/workspace_form/utils.ts index 084fdb437b88..8b00f167b01f 100644 --- a/src/plugins/workspace/public/components/workspace_form/utils.ts +++ b/src/plugins/workspace/public/components/workspace_form/utils.ts @@ -260,7 +260,7 @@ export const validateWorkspaceForm = ( formErrors.name = { code: WorkspaceFormErrorCode.InvalidWorkspaceName, message: i18n.translate('workspace.form.detail.name.invalid', { - defaultMessage: 'Invalid workspace name', + defaultMessage: 'Name is invalid. Enter a valid name.', }), }; } diff --git a/src/plugins/workspace/public/components/workspace_form/workspace_form.tsx b/src/plugins/workspace/public/components/workspace_form/workspace_form.tsx index 97387ca37d7c..4281b75584bf 100644 --- a/src/plugins/workspace/public/components/workspace_form/workspace_form.tsx +++ b/src/plugins/workspace/public/components/workspace_form/workspace_form.tsx @@ -47,7 +47,7 @@ export const WorkspaceForm = (props: WorkspaceFormProps) => { handleDescriptionChange, } = useWorkspaceForm(props); const workspaceDetailsTitle = i18n.translate('workspace.form.workspaceDetails.title', { - defaultMessage: 'Enter Details', + defaultMessage: 'Enter details', }); return ( diff --git a/src/plugins/workspace/public/components/workspace_form/workspace_form_error_callout.tsx b/src/plugins/workspace/public/components/workspace_form/workspace_form_error_callout.tsx index e51340102fa8..56df3c45cdd3 100644 --- a/src/plugins/workspace/public/components/workspace_form/workspace_form_error_callout.tsx +++ b/src/plugins/workspace/public/components/workspace_form/workspace_form_error_callout.tsx @@ -12,11 +12,11 @@ const getSuggestionFromErrorCode = (error: WorkspaceFormError) => { switch (error.code) { case WorkspaceFormErrorCode.WorkspaceNameMissing: return i18n.translate('workspace.form.errorCallout.nameMissing', { - defaultMessage: 'Enter a workspace name.', + defaultMessage: 'Enter a name.', }); case WorkspaceFormErrorCode.InvalidWorkspaceName: return i18n.translate('workspace.form.errorCallout.nameInvalid', { - defaultMessage: 'Enter a valid workspace name.', + defaultMessage: 'Enter a valid name.', }); case WorkspaceFormErrorCode.UseCaseMissing: return i18n.translate('workspace.form.errorCallout.useCaseMissing', { From be4d8ba05b5d5cebda7f626340eb9cd383d16f88 Mon Sep 17 00:00:00 2001 From: Lin Wang Date: Thu, 27 Jun 2024 17:11:29 +0800 Subject: [PATCH 14/29] Add validate for owner missing Signed-off-by: Lin Wang --- .../public/components/workspace_form/types.ts | 7 +- .../workspace_form/use_workspace_form.ts | 5 +- .../public/components/workspace_form/utils.ts | 140 +++++++++--------- .../workspace_form/workspace_form.tsx | 10 +- .../workspace_form_error_callout.tsx | 16 +- .../workspace_permission_setting_input.tsx | 7 +- .../workspace_permission_setting_panel.tsx | 45 +----- 7 files changed, 114 insertions(+), 116 deletions(-) diff --git a/src/plugins/workspace/public/components/workspace_form/types.ts b/src/plugins/workspace/public/components/workspace_form/types.ts index 313739fd86f8..dd1192a170a3 100644 --- a/src/plugins/workspace/public/components/workspace_form/types.ts +++ b/src/plugins/workspace/public/components/workspace_form/types.ts @@ -47,6 +47,7 @@ export enum WorkspaceFormErrorCode { PermissionUserGroupMissing, DuplicateUserIdPermissionSetting, DuplicateUserGroupPermissionSetting, + PermissionSettingOwnerMissing, } export interface WorkspaceFormError { @@ -57,7 +58,10 @@ export interface WorkspaceFormError { export type WorkspaceFormErrors = { [key in keyof Omit]?: WorkspaceFormError; } & { - permissionSettings?: { [key: number]: WorkspaceFormError }; + permissionSettings?: { + overall?: WorkspaceFormError; + fields?: { [key: number]: WorkspaceFormError }; + }; }; export interface WorkspaceFormProps { @@ -67,5 +71,4 @@ export interface WorkspaceFormProps { operationType: WorkspaceOperationType; workspaceConfigurableApps?: PublicAppInfo[]; permissionEnabled?: boolean; - permissionLastAdminItemDeletable?: boolean; } 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 index 88bad388d742..6dad6f4e875a 100644 --- a/src/plugins/workspace/public/components/workspace_form/use_workspace_form.ts +++ b/src/plugins/workspace/public/components/workspace_form/use_workspace_form.ts @@ -32,6 +32,7 @@ export const useWorkspaceForm = ({ defaultValues, operationType, onSubmit, + permissionEnabled, }: WorkspaceFormProps) => { const applications = useApplications(application); const [name, setName] = useState(defaultValues?.name); @@ -97,7 +98,7 @@ export const useWorkspaceForm = ({ const currentFormData = getFormDataRef.current(); const currentFormErrors: WorkspaceFormErrors = validateWorkspaceForm( currentFormData, - defaultValuesRef.current + !!permissionEnabled ); setFormErrors(currentFormErrors); if (getNumberOfErrors(currentFormErrors) > 0) { @@ -115,7 +116,7 @@ export const useWorkspaceForm = ({ ) as WorkspacePermissionSetting[], }); }, - [onSubmit] + [onSubmit, permissionEnabled] ); const handleNameInputChange = useCallback['onChange']>((e) => { diff --git a/src/plugins/workspace/public/components/workspace_form/utils.ts b/src/plugins/workspace/public/components/workspace_form/utils.ts index 8b00f167b01f..d9779f352dd3 100644 --- a/src/plugins/workspace/public/components/workspace_form/utils.ts +++ b/src/plugins/workspace/public/components/workspace_form/utils.ts @@ -52,8 +52,11 @@ export const getNumberOfErrors = (formErrors: WorkspaceFormErrors) => { if (formErrors.description) { numberOfErrors += 1; } - if (formErrors.permissionSettings) { - numberOfErrors += Object.keys(formErrors.permissionSettings).length; + if (formErrors.permissionSettings?.fields) { + numberOfErrors += Object.keys(formErrors.permissionSettings.fields).length; + } + if (formErrors.permissionSettings?.overall) { + numberOfErrors += 1; } if (formErrors.features) { numberOfErrors += 1; @@ -195,17 +198,8 @@ export const convertPermissionsToPermissionSettings = (permissions: SavedObjectP const validateUserPermissionSetting = ( setting: WorkspaceUserPermissionSetting, - required: boolean, previousPermissionSettings: Array> ) => { - if (required && !setting.userId) { - return { - code: WorkspaceFormErrorCode.PermissionUserIdMissing, - message: i18n.translate('workspace.form.permission.invalidate.userId', { - defaultMessage: 'User is required. Enter a user.', - }), - }; - } if (!!setting.userId && hasSameUserIdOrGroup(previousPermissionSettings, setting)) { return { code: WorkspaceFormErrorCode.DuplicateUserIdPermissionSetting, @@ -221,17 +215,8 @@ const validateUserPermissionSetting = ( const validateUserGroupPermissionSetting = ( setting: WorkspaceUserGroupPermissionSetting, - required: boolean, previousPermissionSettings: Array> ) => { - if (required && !setting.group) { - return { - code: WorkspaceFormErrorCode.PermissionUserGroupMissing, - message: i18n.translate('workspace.form.permission.invalidate.group', { - defaultMessage: 'User group is required. Enter a user group.', - }), - }; - } if (!!setting.group && hasSameUserIdOrGroup(previousPermissionSettings, setting)) { return { code: WorkspaceFormErrorCode.DuplicateUserGroupPermissionSetting, @@ -245,13 +230,78 @@ const validateUserGroupPermissionSetting = ( } }; +const validatePermissionSetting = ( + permissionSettings?: Array< + Pick & Partial + > +) => { + const permissionSettingMissingError = { + code: WorkspaceFormErrorCode.PermissionSettingOwnerMissing, + message: i18n.translate('workspace.form.permission.setting.missing', { + defaultMessage: 'Permission setting missing', + }), + }; + if (!permissionSettings) { + return { + overall: permissionSettingMissingError, + }; + } + + const permissionSettingsErrors: { [key: number]: WorkspaceFormError } = {}; + for (let i = 0; i < permissionSettings.length; i++) { + const setting = permissionSettings[i]; + if (!setting.type) { + permissionSettingsErrors[setting.id] = { + code: WorkspaceFormErrorCode.InvalidPermissionType, + message: i18n.translate('workspace.form.permission.invalidate.type', { + defaultMessage: 'Invalid type', + }), + }; + } else if (!setting.modes || setting.modes.length === 0) { + permissionSettingsErrors[setting.id] = { + code: WorkspaceFormErrorCode.InvalidPermissionModes, + message: i18n.translate('workspace.form.permission.invalidate.modes', { + defaultMessage: 'Invalid permission modes', + }), + }; + } else if (setting.type === WorkspacePermissionItemType.User) { + const validateResult = validateUserPermissionSetting( + setting as WorkspaceUserPermissionSetting, + permissionSettings.slice(0, i) + ); + if (validateResult) { + permissionSettingsErrors[setting.id] = validateResult; + } + } else if (setting.type === WorkspacePermissionItemType.Group) { + const validateResult = validateUserGroupPermissionSetting( + setting as WorkspaceUserGroupPermissionSetting, + permissionSettings.slice(0, i) + ); + if (validateResult) { + permissionSettingsErrors[setting.id] = validateResult; + } + } + } + + return { + ...(!permissionSettings.some( + (setting) => setting.modes && getPermissionModeId(setting.modes) === PermissionModeId.Owner + ) + ? { overall: permissionSettingMissingError } + : {}), + ...(Object.keys(permissionSettingsErrors).length > 0 + ? { fields: permissionSettingsErrors } + : {}), + }; +}; + export const validateWorkspaceForm = ( formData: Omit, 'permissionSettings'> & { permissionSettings?: Array< Pick & Partial >; }, - initialFormData?: WorkspaceFormSubmitData + isPermissionEnabled: boolean ) => { const formErrors: WorkspaceFormErrors = {}; const { name, permissionSettings, features } = formData; @@ -280,52 +330,8 @@ export const validateWorkspaceForm = ( }), }; } - if (permissionSettings) { - const permissionSettingsErrors: { [key: number]: WorkspaceFormError } = {}; - const requiredSettingIds = initialFormData?.permissionSettings?.map((item) => item.id); - const isUserIdOrGroupRequired = (settingId: number) => - !!requiredSettingIds?.includes(settingId); - let firstUserRequiredFlag = true; - for (let i = 0; i < permissionSettings.length; i++) { - const setting = permissionSettings[i]; - if (!setting.type) { - permissionSettingsErrors[setting.id] = { - code: WorkspaceFormErrorCode.InvalidPermissionType, - message: i18n.translate('workspace.form.permission.invalidate.type', { - defaultMessage: 'Invalid type', - }), - }; - } else if (!setting.modes || setting.modes.length === 0) { - permissionSettingsErrors[setting.id] = { - code: WorkspaceFormErrorCode.InvalidPermissionModes, - message: i18n.translate('workspace.form.permission.invalidate.modes', { - defaultMessage: 'Invalid permission modes', - }), - }; - } else if (setting.type === WorkspacePermissionItemType.User) { - const validateResult = validateUserPermissionSetting( - setting as WorkspaceUserPermissionSetting, - isUserIdOrGroupRequired(setting.id) || firstUserRequiredFlag, - permissionSettings.slice(0, i) - ); - if (validateResult) { - permissionSettingsErrors[setting.id] = validateResult; - } - firstUserRequiredFlag = false; - } else if (setting.type === WorkspacePermissionItemType.Group) { - const validateResult = validateUserGroupPermissionSetting( - setting as WorkspaceUserGroupPermissionSetting, - isUserIdOrGroupRequired(setting.id), - permissionSettings.slice(0, i) - ); - if (validateResult) { - permissionSettingsErrors[setting.id] = validateResult; - } - } - } - if (Object.keys(permissionSettingsErrors).length > 0) { - formErrors.permissionSettings = permissionSettingsErrors; - } + if (isPermissionEnabled) { + formErrors.permissionSettings = validatePermissionSetting(permissionSettings); } return formErrors; }; diff --git a/src/plugins/workspace/public/components/workspace_form/workspace_form.tsx b/src/plugins/workspace/public/components/workspace_form/workspace_form.tsx index 4281b75584bf..0b9e18896a9f 100644 --- a/src/plugins/workspace/public/components/workspace_form/workspace_form.tsx +++ b/src/plugins/workspace/public/components/workspace_form/workspace_form.tsx @@ -3,7 +3,7 @@ * SPDX-License-Identifier: Apache-2.0 */ -import React from 'react'; +import React, { useRef } from 'react'; import { EuiPanel, EuiSpacer, @@ -32,7 +32,6 @@ export const WorkspaceForm = (props: WorkspaceFormProps) => { operationType, permissionEnabled, workspaceConfigurableApps, - permissionLastAdminItemDeletable, } = props; const { formId, @@ -49,6 +48,9 @@ export const WorkspaceForm = (props: WorkspaceFormProps) => { const workspaceDetailsTitle = i18n.translate('workspace.form.workspaceDetails.title', { defaultMessage: 'Enter details', }); + const disabledUserOrGroupInputIdsRef = useRef( + defaultValues?.permissionSettings?.map((item) => item.id) ?? [] + ); return ( @@ -153,10 +155,10 @@ export const WorkspaceForm = (props: WorkspaceFormProps) => { diff --git a/src/plugins/workspace/public/components/workspace_form/workspace_form_error_callout.tsx b/src/plugins/workspace/public/components/workspace_form/workspace_form_error_callout.tsx index 56df3c45cdd3..a68890a5e46d 100644 --- a/src/plugins/workspace/public/components/workspace_form/workspace_form_error_callout.tsx +++ b/src/plugins/workspace/public/components/workspace_form/workspace_form_error_callout.tsx @@ -38,6 +38,10 @@ const getSuggestionFromErrorCode = (error: WorkspaceFormError) => { return i18n.translate('workspace.form.errorCallout.duplicatePermission', { defaultMessage: 'Enter a unique user group.', }); + case WorkspaceFormErrorCode.PermissionSettingOwnerMissing: + return i18n.translate('workspace.form.errorCallout.permissionSettingOwnerMissing', { + defaultMessage: 'Enter an owner at user or user group permission setting.', + }); default: return error.message; } @@ -66,10 +70,10 @@ export interface WorkspaceFormErrorCalloutProps { export const WorkspaceFormErrorCallout = ({ errors }: WorkspaceFormErrorCalloutProps) => { const renderPermissionSettingSuggestion = (errorCode: WorkspaceFormErrorCode) => { - if (!errors.permissionSettings) { + if (!errors.permissionSettings?.fields) { return null; } - const findingError = Object.values(errors.permissionSettings).find( + const findingError = Object.values(errors.permissionSettings.fields).find( (item) => item.code === errorCode ); @@ -122,6 +126,14 @@ export const WorkspaceFormErrorCallout = ({ errors }: WorkspaceFormErrorCalloutP {renderPermissionSettingSuggestion( WorkspaceFormErrorCode.DuplicateUserGroupPermissionSetting )} + {errors.permissionSettings?.overall && ( + + )} diff --git a/src/plugins/workspace/public/components/workspace_form/workspace_permission_setting_input.tsx b/src/plugins/workspace/public/components/workspace_form/workspace_permission_setting_input.tsx index cd31e27e08a3..c43991f492e8 100644 --- a/src/plugins/workspace/public/components/workspace_form/workspace_permission_setting_input.tsx +++ b/src/plugins/workspace/public/components/workspace_form/workspace_permission_setting_input.tsx @@ -59,11 +59,12 @@ export const permissionModeOptions = [ export interface WorkspacePermissionSettingInputProps { index: number; - deletable: boolean; type: WorkspacePermissionItemType; userId?: string; group?: string; modes?: WorkspacePermissionMode[]; + deletable?: boolean; + userOrGroupDisabled: boolean; onGroupOrUserIdChange: ( groupOrUserId: | { type: WorkspacePermissionItemType.User; userId?: string } @@ -83,7 +84,8 @@ export const WorkspacePermissionSettingInput = ({ userId, group, modes, - deletable, + deletable = true, + userOrGroupDisabled, onDelete, onGroupOrUserIdChange, onPermissionModesChange, @@ -145,6 +147,7 @@ export const WorkspacePermissionSettingInput = ({ defaultMessage: 'Select a user group', }) } + isDisabled={userOrGroupDisabled} /> diff --git a/src/plugins/workspace/public/components/workspace_form/workspace_permission_setting_panel.tsx b/src/plugins/workspace/public/components/workspace_form/workspace_permission_setting_panel.tsx index af7811209cba..6edc78185f3a 100644 --- a/src/plugins/workspace/public/components/workspace_form/workspace_permission_setting_panel.tsx +++ b/src/plugins/workspace/public/components/workspace_form/workspace_permission_setting_panel.tsx @@ -16,11 +16,11 @@ import { WorkspacePermissionSettingInput, WorkspacePermissionSettingInputProps, } from './workspace_permission_setting_input'; -import { generateNextPermissionSettingsId, getPermissionModeId } from './utils'; +import { generateNextPermissionSettingsId } from './utils'; export interface WorkspacePermissionSettingPanelProps { errors?: { [key: number]: WorkspaceFormError }; - lastAdminItemDeletable: boolean; + disabledUserOrGroupInputIds: number[]; permissionSettings: Array< Pick & Partial >; @@ -29,9 +29,7 @@ export interface WorkspacePermissionSettingPanelProps { ) => void; } -interface UserOrGroupSectionProps - extends Omit { - nonDeletableIndex: number; +interface UserOrGroupSectionProps extends WorkspacePermissionSettingPanelProps { type: WorkspacePermissionItemType; nextIdGenerator: () => number; } @@ -42,7 +40,7 @@ const UserOrGroupSection = ({ onChange, nextIdGenerator, permissionSettings, - nonDeletableIndex, + disabledUserOrGroupInputIds, }: UserOrGroupSectionProps) => { // default permission mode is read const handleAddNewOne = useCallback(() => { @@ -131,11 +129,7 @@ const UserOrGroupSection = ({ {...item} type={type} index={index} - deletable={ - type === WorkspacePermissionItemType.Group || - index > 0 || - (!!item.modes && getPermissionModeId(item.modes) !== PermissionModeId.Owner) - } + userOrGroupDisabled={disabledUserOrGroupInputIds.includes(item.id)} onDelete={handleDelete} onGroupOrUserIdChange={handleGroupOrUserIdChange} onPermissionModesChange={handlePermissionModesChange} @@ -165,7 +159,7 @@ export const WorkspacePermissionSettingPanel = ({ errors, onChange, permissionSettings, - lastAdminItemDeletable, + disabledUserOrGroupInputIds, }: WorkspacePermissionSettingPanelProps) => { const userPermissionSettings = useMemo( () => @@ -182,29 +176,6 @@ export const WorkspacePermissionSettingPanel = ({ [permissionSettings] ); - const { userNonDeletableIndex, groupNonDeletableIndex } = useMemo(() => { - if ( - lastAdminItemDeletable || - // Permission setting can be deleted if there are more than one admin setting - [...userPermissionSettings, ...groupPermissionSettings].filter( - (permission) => - permission.modes && getPermissionModeId(permission.modes) === PermissionModeId.Owner - ).length > 1 - ) { - return { userNonDeletableIndex: -1, groupNonDeletableIndex: -1 }; - } - return { - userNonDeletableIndex: userPermissionSettings.findIndex( - (permission) => - permission.modes && getPermissionModeId(permission.modes) === PermissionModeId.Owner - ), - groupNonDeletableIndex: groupPermissionSettings.findIndex( - (permission) => - permission.modes && getPermissionModeId(permission.modes) === PermissionModeId.Owner - ), - }; - }, [userPermissionSettings, groupPermissionSettings, lastAdminItemDeletable]); - const nextIdRef = useRef(generateNextPermissionSettingsId(permissionSettings)); const handleUserPermissionSettingsChange = useCallback( @@ -239,19 +210,19 @@ export const WorkspacePermissionSettingPanel = ({
    • ); From 8463a2b0d0d8d6add38357bf04303b37753ed3a9 Mon Sep 17 00:00:00 2001 From: "opensearch-changeset-bot[bot]" <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com> Date: Mon, 1 Jul 2024 09:39:28 +0000 Subject: [PATCH 15/29] Changeset file for PR #7133 created/updated --- changelogs/fragments/7133.yml | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 changelogs/fragments/7133.yml diff --git a/changelogs/fragments/7133.yml b/changelogs/fragments/7133.yml new file mode 100644 index 000000000000..e1755da06839 --- /dev/null +++ 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)) \ No newline at end of file From 83da74445027c4d9bbe2db48d5062fc604fa7b64 Mon Sep 17 00:00:00 2001 From: Lin Wang Date: Tue, 2 Jul 2024 13:51:53 +0800 Subject: [PATCH 16/29] Fix unit tests for workspace form utils Signed-off-by: Lin Wang --- .../components/workspace_form/utils.test.ts | 131 ++++++++++-------- 1 file changed, 73 insertions(+), 58 deletions(-) diff --git a/src/plugins/workspace/public/components/workspace_form/utils.test.ts b/src/plugins/workspace/public/components/workspace_form/utils.test.ts index 83b2215140eb..63d2dca421b3 100644 --- a/src/plugins/workspace/public/components/workspace_form/utils.test.ts +++ b/src/plugins/workspace/public/components/workspace_form/utils.test.ts @@ -154,39 +154,45 @@ describe('convertPermissionsToPermissionSettings', () => { describe('validateWorkspaceForm', () => { it('should return error if name is empty', () => { - expect(validateWorkspaceForm({}).name).toEqual({ + expect(validateWorkspaceForm({}, false).name).toEqual({ code: WorkspaceFormErrorCode.WorkspaceNameMissing, message: "Name can't be empty.", }); }); it('should return error if name is invalid', () => { - expect(validateWorkspaceForm({ name: '~' }).name).toEqual({ + expect(validateWorkspaceForm({ name: '~' }, false).name).toEqual({ code: WorkspaceFormErrorCode.InvalidWorkspaceName, - message: 'Invalid workspace name', + message: 'Name is invalid. Enter a valid name.', }); }); it('should return error if use case is empty', () => { - expect(validateWorkspaceForm({}).features).toEqual({ + expect(validateWorkspaceForm({}, false).features).toEqual({ code: WorkspaceFormErrorCode.UseCaseMissing, message: 'Use case is required. Select a use case.', }); }); it('should return error if permission setting type is invalid', () => { expect( - validateWorkspaceForm({ - name: 'test', - permissionSettings: [{ id: 0 }], - }).permissionSettings + validateWorkspaceForm( + { + name: 'test', + permissionSettings: [{ id: 0 }], + }, + true + ).permissionSettings?.fields ).toEqual({ 0: { code: WorkspaceFormErrorCode.InvalidPermissionType, message: 'Invalid type' }, }); }); it('should return error if permission setting modes is invalid', () => { expect( - validateWorkspaceForm({ - name: 'test', - permissionSettings: [{ id: 0, type: WorkspacePermissionItemType.User, modes: [] }], - }).permissionSettings + validateWorkspaceForm( + { + name: 'test', + permissionSettings: [{ id: 0, type: WorkspacePermissionItemType.User, modes: [] }], + }, + true + ).permissionSettings?.fields ).toEqual({ 0: { code: WorkspaceFormErrorCode.InvalidPermissionModes, @@ -197,23 +203,26 @@ describe('validateWorkspaceForm', () => { it('should return error if permission setting is duplicate', () => { expect( - validateWorkspaceForm({ - name: 'test', - permissionSettings: [ - { - id: 0, - type: WorkspacePermissionItemType.User, - modes: [WorkspacePermissionMode.LibraryRead], - userId: 'foo', - }, - { - id: 1, - type: WorkspacePermissionItemType.User, - modes: [WorkspacePermissionMode.LibraryRead], - userId: 'foo', - }, - ], - }).permissionSettings + validateWorkspaceForm( + { + name: 'test', + permissionSettings: [ + { + id: 0, + type: WorkspacePermissionItemType.User, + modes: [WorkspacePermissionMode.LibraryRead], + userId: 'foo', + }, + { + id: 1, + type: WorkspacePermissionItemType.User, + modes: [WorkspacePermissionMode.LibraryRead], + userId: 'foo', + }, + ], + }, + true + ).permissionSettings?.fields ).toEqual({ 1: { code: WorkspaceFormErrorCode.DuplicateUserIdPermissionSetting, @@ -221,23 +230,26 @@ describe('validateWorkspaceForm', () => { }, }); expect( - validateWorkspaceForm({ - name: 'test', - permissionSettings: [ - { - id: 0, - type: WorkspacePermissionItemType.Group, - modes: [WorkspacePermissionMode.LibraryRead], - group: 'foo', - }, - { - id: 1, - type: WorkspacePermissionItemType.Group, - modes: [WorkspacePermissionMode.LibraryRead], - group: 'foo', - }, - ], - }).permissionSettings + validateWorkspaceForm( + { + name: 'test', + permissionSettings: [ + { + id: 0, + type: WorkspacePermissionItemType.Group, + modes: [WorkspacePermissionMode.LibraryRead], + group: 'foo', + }, + { + id: 1, + type: WorkspacePermissionItemType.Group, + modes: [WorkspacePermissionMode.LibraryRead], + group: 'foo', + }, + ], + }, + true + ).permissionSettings?.fields ).toEqual({ 1: { code: WorkspaceFormErrorCode.DuplicateUserGroupPermissionSetting, @@ -248,18 +260,21 @@ describe('validateWorkspaceForm', () => { it('should return empty object for valid for data', () => { expect( - validateWorkspaceForm({ - name: 'test', - permissionSettings: [ - { - id: 0, - type: WorkspacePermissionItemType.Group, - modes: [WorkspacePermissionMode.LibraryRead], - group: 'foo', - }, - ], - features: ['use-case-observability'], - }) + validateWorkspaceForm( + { + name: 'test', + permissionSettings: [ + { + id: 0, + type: WorkspacePermissionItemType.Group, + modes: [WorkspacePermissionMode.LibraryRead], + group: 'foo', + }, + ], + features: ['use-case-observability'], + }, + false + ) ).toEqual({}); }); }); From d41f61852265d542d8d9060d53b9cab2f5d05300 Mon Sep 17 00:00:00 2001 From: Lin Wang Date: Tue, 2 Jul 2024 14:24:27 +0800 Subject: [PATCH 17/29] Fix unit tests for form error callout Signed-off-by: Lin Wang --- .../workspace_form_error_callout.test.tsx | 74 ++++++++++--------- 1 file changed, 39 insertions(+), 35 deletions(-) diff --git a/src/plugins/workspace/public/components/workspace_form/workspace_form_error_callout.test.tsx b/src/plugins/workspace/public/components/workspace_form/workspace_form_error_callout.test.tsx index 701f62699cca..76f81fc9074d 100644 --- a/src/plugins/workspace/public/components/workspace_form/workspace_form_error_callout.test.tsx +++ b/src/plugins/workspace/public/components/workspace_form/workspace_form_error_callout.test.tsx @@ -37,7 +37,7 @@ describe('WorkspaceFormErrorCallout', () => { }, }); - expect(renderResult.getByText('Name: Enter a workspace name.')).toBeInTheDocument(); + expect(renderResult.getByText('Name: Enter a name.')).toBeInTheDocument(); renderResult.rerender( { /> ); - expect(renderResult.getByText('Name: Enter a valid workspace name.')).toBeInTheDocument(); + expect(renderResult.getByText('Name: Enter a valid name.')).toBeInTheDocument(); }); it('should render use case suggestion', () => { const { renderResult } = setup({ errors: { - name: { - code: WorkspaceFormErrorCode.WorkspaceNameMissing, + features: { + code: WorkspaceFormErrorCode.UseCaseMissing, message: '', }, }, }); - expect(renderResult.getByText('Name: Enter a workspace name.')).toBeInTheDocument(); + expect(renderResult.getByText('Use case: Select a use case.')).toBeInTheDocument(); }); it('should combine user permission settings suggestions', () => { const { renderResult } = setup({ errors: { permissionSettings: { - 0: { - code: WorkspaceFormErrorCode.PermissionUserIdMissing, - message: '', - }, - 1: { - code: WorkspaceFormErrorCode.PermissionUserIdMissing, - message: '', - }, - 2: { - code: WorkspaceFormErrorCode.DuplicateUserIdPermissionSetting, - message: '', - }, - 3: { - code: WorkspaceFormErrorCode.DuplicateUserIdPermissionSetting, - message: '', + fields: { + 0: { + code: WorkspaceFormErrorCode.PermissionUserIdMissing, + message: '', + }, + 1: { + code: WorkspaceFormErrorCode.PermissionUserIdMissing, + message: '', + }, + 2: { + code: WorkspaceFormErrorCode.DuplicateUserIdPermissionSetting, + message: '', + }, + 3: { + code: WorkspaceFormErrorCode.DuplicateUserIdPermissionSetting, + message: '', + }, }, }, }, @@ -96,21 +98,23 @@ describe('WorkspaceFormErrorCallout', () => { const { renderResult } = setup({ errors: { permissionSettings: { - 0: { - code: WorkspaceFormErrorCode.PermissionUserGroupMissing, - message: '', - }, - 1: { - code: WorkspaceFormErrorCode.PermissionUserGroupMissing, - message: '', - }, - 2: { - code: WorkspaceFormErrorCode.DuplicateUserGroupPermissionSetting, - message: '', - }, - 3: { - code: WorkspaceFormErrorCode.DuplicateUserGroupPermissionSetting, - message: '', + fields: { + 0: { + code: WorkspaceFormErrorCode.PermissionUserGroupMissing, + message: '', + }, + 1: { + code: WorkspaceFormErrorCode.PermissionUserGroupMissing, + message: '', + }, + 2: { + code: WorkspaceFormErrorCode.DuplicateUserGroupPermissionSetting, + message: '', + }, + 3: { + code: WorkspaceFormErrorCode.DuplicateUserGroupPermissionSetting, + message: '', + }, }, }, }, From 1c47632d067e79af25c6827b60e527b6cbf77405 Mon Sep 17 00:00:00 2001 From: Lin Wang Date: Tue, 2 Jul 2024 14:26:15 +0800 Subject: [PATCH 18/29] Add unit test for transfer current user placeholder Signed-off-by: Lin Wang --- src/plugins/workspace/server/utils.test.ts | 28 ++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/src/plugins/workspace/server/utils.test.ts b/src/plugins/workspace/server/utils.test.ts index 15e6783c2cab..1cfd6bb622b3 100644 --- a/src/plugins/workspace/server/utils.test.ts +++ b/src/plugins/workspace/server/utils.test.ts @@ -10,6 +10,7 @@ import { getOSDAdminConfigFromYMLConfig, getPrincipalsFromRequest, updateDashboardAdminStateForRequest, + transferCurrentUserInPermissions, } from './utils'; import { getWorkspaceState } from '../../../core/server/utils'; import { Observable, of } from 'rxjs'; @@ -151,4 +152,31 @@ describe('workspace utils', () => { expect(groups).toEqual([]); expect(users).toEqual([]); }); + + it('should transfer current user placeholder in permissions', () => { + expect(transferCurrentUserInPermissions('foo', undefined)).toBeUndefined(); + expect( + transferCurrentUserInPermissions('foo', { + library_write: { + users: ['%current_user%', 'bar'], + }, + write: { + users: ['%current_user%'], + }, + read: { + users: ['bar'], + }, + }) + ).toEqual({ + library_write: { + users: ['foo', 'bar'], + }, + write: { + users: ['foo'], + }, + read: { + users: ['bar'], + }, + }); + }); }); From 8169847ea638897bd4b1a5317bcdf5c164b022f5 Mon Sep 17 00:00:00 2001 From: Lin Wang Date: Tue, 2 Jul 2024 17:29:14 +0800 Subject: [PATCH 19/29] Fix unit tests in workspace permission setting panel Signed-off-by: Lin Wang --- ...orkspace_permission_setting_panel.test.tsx | 21 ++++++++++++++----- .../workspace_overview.test.tsx | 2 +- 2 files changed, 17 insertions(+), 6 deletions(-) diff --git a/src/plugins/workspace/public/components/workspace_form/workspace_permission_setting_panel.test.tsx b/src/plugins/workspace/public/components/workspace_form/workspace_permission_setting_panel.test.tsx index 4b5057e22365..6e8a3ffb62dc 100644 --- a/src/plugins/workspace/public/components/workspace_form/workspace_permission_setting_panel.test.tsx +++ b/src/plugins/workspace/public/components/workspace_form/workspace_permission_setting_panel.test.tsx @@ -30,7 +30,7 @@ const setup = (options?: Partial) => { ]; const renderResult = render( { ]); }); - it('should hide first user permission setting delete button', () => { + it('should not able to edit user or group when disabled', () => { const { renderResult } = setup({ - lastAdminItemDeletable: false, permissionSettings: [ { id: 0, type: WorkspacePermissionItemType.User, - userId: 'foo', + userId: 'user-1', + modes: [WorkspacePermissionMode.LibraryWrite, WorkspacePermissionMode.Write], + }, + { + id: 1, + type: WorkspacePermissionItemType.Group, + group: 'user-group-1', modes: [WorkspacePermissionMode.LibraryWrite, WorkspacePermissionMode.Write], }, ], + disabledUserOrGroupInputIds: [0, 1], }); - expect(renderResult.queryByLabelText('Delete permission setting')).toBeNull(); + expect(renderResult.getByText('user-1')?.closest('div[role="combobox"]')).toHaveClass( + 'euiComboBox-isDisabled' + ); + expect(renderResult.getByText('user-group-1')?.closest('div[role="combobox"]')).toHaveClass( + 'euiComboBox-isDisabled' + ); }); it('should render consistent errors', () => { diff --git a/src/plugins/workspace/public/components/workspace_overview/workspace_overview.test.tsx b/src/plugins/workspace/public/components/workspace_overview/workspace_overview.test.tsx index 8a87510f0d9e..e22e9a06ec2d 100644 --- a/src/plugins/workspace/public/components/workspace_overview/workspace_overview.test.tsx +++ b/src/plugins/workspace/public/components/workspace_overview/workspace_overview.test.tsx @@ -207,7 +207,7 @@ describe('WorkspaceOverview', () => { const workspaceService = createWorkspacesSetupContractMockWithValue(workspaceObject); const { getByText } = render(WorkspaceOverviewPage({ workspacesService: workspaceService })); fireEvent.click(getByText('Settings')); - expect(screen.queryByText('Enter Details')).not.toBeNull(); + expect(screen.queryByText('Enter details')).not.toBeNull(); // title is hidden expect(screen.queryByText('Update Workspace')).toBeNull(); }); From d24eac709120ed2fc8197ac17d0eb6cc6d32ccc4 Mon Sep 17 00:00:00 2001 From: Lin Wang Date: Tue, 2 Jul 2024 17:31:22 +0800 Subject: [PATCH 20/29] Fix unit test in useWorkspaceForm Signed-off-by: Lin Wang --- .../components/workspace_form/use_workspace_form.test.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) 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 index 86ddd22ca625..b664ec346562 100644 --- 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,6 +6,7 @@ import { renderHook, act } from '@testing-library/react-hooks'; import { applicationServiceMock } from '../../../../../core/public/mocks'; +import { WorkspaceOperationType } from './constants'; import { WorkspaceFormData, WorkspaceFormErrorCode } from './types'; import { useWorkspaceForm } from './use_workspace_form'; @@ -16,6 +17,7 @@ const setup = (defaultValues?: WorkspaceFormData) => { application: applicationServiceMock.createStartContract(), defaultValues, onSubmit: onSubmitMock, + operationType: WorkspaceOperationType.Create, }, }); return { @@ -25,7 +27,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: '~', @@ -39,7 +41,7 @@ describe('useWorkspaceForm', () => { expect.objectContaining({ name: { code: WorkspaceFormErrorCode.InvalidWorkspaceName, - message: 'Invalid workspace name', + message: 'Name is invalid. Enter a valid name.', }, }) ); From 392dc720761bacb4c3d97e4db0affa5dd63af157 Mon Sep 17 00:00:00 2001 From: Lin Wang Date: Wed, 3 Jul 2024 16:50:24 +0800 Subject: [PATCH 21/29] Add missing unit tests for workspace form utils Signed-off-by: Lin Wang --- .../components/workspace_form/utils.test.ts | 198 +++++++++++++++++- 1 file changed, 197 insertions(+), 1 deletion(-) diff --git a/src/plugins/workspace/public/components/workspace_form/utils.test.ts b/src/plugins/workspace/public/components/workspace_form/utils.test.ts index 63d2dca421b3..323762c7cc0c 100644 --- a/src/plugins/workspace/public/components/workspace_form/utils.test.ts +++ b/src/plugins/workspace/public/components/workspace_form/utils.test.ts @@ -7,6 +7,7 @@ import { validateWorkspaceForm, convertPermissionSettingsToPermissions, convertPermissionsToPermissionSettings, + getNumberOfChanges, } from './utils'; import { WorkspacePermissionMode } from '../../../common/constants'; import { WorkspacePermissionItemType } from './constants'; @@ -258,7 +259,41 @@ describe('validateWorkspaceForm', () => { }); }); - it('should return empty object for valid for data', () => { + it('should return error if owner is missing in permission settings', () => { + expect( + validateWorkspaceForm( + { + name: 'test', + }, + true + ).permissionSettings?.overall + ).toEqual({ + code: WorkspaceFormErrorCode.PermissionSettingOwnerMissing, + message: 'Permission setting missing', + }); + + expect( + validateWorkspaceForm( + { + name: 'test', + permissionSettings: [ + { + id: 0, + type: WorkspacePermissionItemType.User, + modes: [WorkspacePermissionMode.LibraryRead], + userId: 'foo', + }, + ], + }, + true + ).permissionSettings?.overall + ).toEqual({ + code: WorkspaceFormErrorCode.PermissionSettingOwnerMissing, + message: 'Permission setting missing', + }); + }); + + it('should return empty object for valid form data', () => { expect( validateWorkspaceForm( { @@ -278,3 +313,164 @@ describe('validateWorkspaceForm', () => { ).toEqual({}); }); }); + +describe('getNumberOfChanges', () => { + it('should return consistent name changes count', () => { + expect( + getNumberOfChanges( + { + name: 'foo', + }, + { + name: 'foo', + } + ) + ).toEqual(0); + expect( + getNumberOfChanges( + { + name: 'foo1', + }, + { + name: 'foo', + } + ) + ).toEqual(1); + }); + it('should return consistent description changes count', () => { + expect( + getNumberOfChanges( + { + name: 'foo', + description: 'bar', + }, + { + name: 'foo', + description: 'bar', + } + ) + ).toEqual(0); + expect( + getNumberOfChanges( + { + name: 'foo', + }, + { + name: 'foo', + description: 'bar', + } + ) + ).toEqual(1); + }); + it('should return consistent features changes count', () => { + expect( + getNumberOfChanges( + { + name: 'foo', + features: ['bar'], + }, + { + name: 'foo', + features: ['bar'], + } + ) + ).toEqual(0); + expect( + getNumberOfChanges( + { + name: 'foo', + features: [], + }, + { + name: 'foo', + features: ['bar'], + } + ) + ).toEqual(1); + expect( + getNumberOfChanges( + { + name: 'foo', + features: ['bar'], + }, + { + name: 'foo', + features: [], + } + ) + ).toEqual(1); + }); + it('should return consistent permission settings changes count', () => { + expect( + getNumberOfChanges( + { + name: 'foo', + permissionSettings: [ + { + id: 0, + type: WorkspacePermissionItemType.User, + userId: 'user-1', + modes: [WorkspacePermissionMode.Write, WorkspacePermissionMode.LibraryWrite], + }, + ], + }, + { + name: 'foo', + permissionSettings: [ + { + id: 0, + type: WorkspacePermissionItemType.User, + userId: 'user-1', + modes: [WorkspacePermissionMode.Write, WorkspacePermissionMode.LibraryWrite], + }, + ], + } + ) + ).toEqual(0); + // for remove permission setting + expect( + getNumberOfChanges( + { + name: 'foo', + permissionSettings: [ + { + id: 0, + type: WorkspacePermissionItemType.User, + userId: 'user-1', + modes: [WorkspacePermissionMode.Write, WorkspacePermissionMode.LibraryWrite], + }, + { + id: 1, + type: WorkspacePermissionItemType.Group, + group: 'group-1', + modes: [WorkspacePermissionMode.Write, WorkspacePermissionMode.LibraryWrite], + }, + ], + }, + { + name: 'foo', + /** + * These include three changes: + * 1.Remove permission setting#0 + * 2.Modify permission setting#1 + * 3.Add permission setting#2 + */ + permissionSettings: [ + { + id: 1, + type: WorkspacePermissionItemType.Group, + group: 'group-1', + modes: [WorkspacePermissionMode.Read, WorkspacePermissionMode.LibraryWrite], + }, + { + id: 2, + type: WorkspacePermissionItemType.User, + userId: 'user-1', + modes: [WorkspacePermissionMode.Write, WorkspacePermissionMode.LibraryWrite], + }, + ], + } + ) + ).toEqual(3); + }); +}); From d49a2348823c98a7213795767861acf7582e469e Mon Sep 17 00:00:00 2001 From: Lin Wang Date: Thu, 4 Jul 2024 12:18:33 +0800 Subject: [PATCH 22/29] Add unit tests for getNumberOfErrors Signed-off-by: Lin Wang --- .../public/components/workspace_form/types.ts | 2 +- .../components/workspace_form/utils.test.ts | 35 +++++++++++++++++++ .../public/components/workspace_form/utils.ts | 3 -- .../workspace_form/workspace_form.tsx | 2 -- 4 files changed, 36 insertions(+), 6 deletions(-) diff --git a/src/plugins/workspace/public/components/workspace_form/types.ts b/src/plugins/workspace/public/components/workspace_form/types.ts index dd1192a170a3..548086504945 100644 --- a/src/plugins/workspace/public/components/workspace_form/types.ts +++ b/src/plugins/workspace/public/components/workspace_form/types.ts @@ -56,7 +56,7 @@ export interface WorkspaceFormError { } export type WorkspaceFormErrors = { - [key in keyof Omit]?: WorkspaceFormError; + [key in keyof Omit]?: WorkspaceFormError; } & { permissionSettings?: { overall?: WorkspaceFormError; diff --git a/src/plugins/workspace/public/components/workspace_form/utils.test.ts b/src/plugins/workspace/public/components/workspace_form/utils.test.ts index 323762c7cc0c..aef1211be6a5 100644 --- a/src/plugins/workspace/public/components/workspace_form/utils.test.ts +++ b/src/plugins/workspace/public/components/workspace_form/utils.test.ts @@ -8,6 +8,7 @@ import { convertPermissionSettingsToPermissions, convertPermissionsToPermissionSettings, getNumberOfChanges, + getNumberOfErrors, } from './utils'; import { WorkspacePermissionMode } from '../../../common/constants'; import { WorkspacePermissionItemType } from './constants'; @@ -474,3 +475,37 @@ describe('getNumberOfChanges', () => { ).toEqual(3); }); }); + +describe('getNumberOfErrors', () => { + it('should return zero if errors is empty', () => { + expect(getNumberOfErrors({})).toEqual(0); + }); + it('should return consistent name errors count', () => { + expect( + getNumberOfErrors({ + name: { + code: WorkspaceFormErrorCode.WorkspaceNameMissing, + message: '', + }, + }) + ).toEqual(1); + }); + it('should return consistent permission settings errors count', () => { + expect( + getNumberOfErrors({ + permissionSettings: { + overall: { + code: WorkspaceFormErrorCode.PermissionSettingOwnerMissing, + message: '', + }, + fields: { + 1: { + code: WorkspaceFormErrorCode.DuplicateUserIdPermissionSetting, + message: '', + }, + }, + }, + }) + ).toEqual(2); + }); +}); diff --git a/src/plugins/workspace/public/components/workspace_form/utils.ts b/src/plugins/workspace/public/components/workspace_form/utils.ts index d9779f352dd3..0ae87657eddb 100644 --- a/src/plugins/workspace/public/components/workspace_form/utils.ts +++ b/src/plugins/workspace/public/components/workspace_form/utils.ts @@ -49,9 +49,6 @@ export const getNumberOfErrors = (formErrors: WorkspaceFormErrors) => { if (formErrors.name) { numberOfErrors += 1; } - if (formErrors.description) { - numberOfErrors += 1; - } if (formErrors.permissionSettings?.fields) { numberOfErrors += Object.keys(formErrors.permissionSettings.fields).length; } diff --git a/src/plugins/workspace/public/components/workspace_form/workspace_form.tsx b/src/plugins/workspace/public/components/workspace_form/workspace_form.tsx index 0b9e18896a9f..d0a1aa0ab652 100644 --- a/src/plugins/workspace/public/components/workspace_form/workspace_form.tsx +++ b/src/plugins/workspace/public/components/workspace_form/workspace_form.tsx @@ -93,8 +93,6 @@ export const WorkspaceForm = (props: WorkspaceFormProps) => { Description - optional } - isInvalid={!!formErrors.description} - error={formErrors.description?.message} > <> From 44862339111d5e5bd93d3a040bacff0b55214358 Mon Sep 17 00:00:00 2001 From: Lin Wang Date: Thu, 4 Jul 2024 13:41:15 +0800 Subject: [PATCH 23/29] Add more ut for workspace form error callout Signed-off-by: Lin Wang --- .../workspace_form_error_callout.test.tsx | 32 +++++++++++++++++ .../workspace_form_error_callout.tsx | 34 ++++++++----------- 2 files changed, 47 insertions(+), 19 deletions(-) diff --git a/src/plugins/workspace/public/components/workspace_form/workspace_form_error_callout.test.tsx b/src/plugins/workspace/public/components/workspace_form/workspace_form_error_callout.test.tsx index 76f81fc9074d..f31d6c28dda0 100644 --- a/src/plugins/workspace/public/components/workspace_form/workspace_form_error_callout.test.tsx +++ b/src/plugins/workspace/public/components/workspace_form/workspace_form_error_callout.test.tsx @@ -126,4 +126,36 @@ describe('WorkspaceFormErrorCallout', () => { expect(renderResult.getByText('User Group: Enter a unique user group.')).toBeInTheDocument(); expect(renderResult.getAllByText('User Group: Enter a unique user group.')).toHaveLength(1); }); + + it('should render permission settings overall suggestions', () => { + const { renderResult } = setup({ + errors: { + permissionSettings: { + overall: { + code: WorkspaceFormErrorCode.PermissionSettingOwnerMissing, + message: '', + }, + }, + }, + }); + + expect( + renderResult.getByText( + 'Permission setting: Enter an owner at user or user group permission setting.' + ) + ).toBeInTheDocument(); + }); + + it('should render original message if code not recognized', () => { + const { renderResult } = setup({ + errors: { + name: { + code: 'unknown' as any, + message: 'Original name error message', + }, + }, + }); + + expect(renderResult.getByText('Name: Original name error message.')).toBeInTheDocument(); + }); }); diff --git a/src/plugins/workspace/public/components/workspace_form/workspace_form_error_callout.tsx b/src/plugins/workspace/public/components/workspace_form/workspace_form_error_callout.tsx index a68890a5e46d..96a33df6bbcf 100644 --- a/src/plugins/workspace/public/components/workspace_form/workspace_form_error_callout.tsx +++ b/src/plugins/workspace/public/components/workspace_form/workspace_form_error_callout.tsx @@ -51,12 +51,9 @@ const WorkspaceFormErrorCalloutItem = ({ errorKey, message, }: { - errorKey?: string; - message?: string; + errorKey: string; + message: string; }) => { - if (!errorKey || !message) { - return null; - } return (
  • {errorKey} {message} @@ -82,20 +79,19 @@ export const WorkspaceFormErrorCallout = ({ errors }: WorkspaceFormErrorCalloutP } return ( -
  • - {(errorCode === WorkspaceFormErrorCode.DuplicateUserIdPermissionSetting || - errorCode === WorkspaceFormErrorCode.PermissionUserIdMissing) && - i18n.translate('workspace.form.errorCallout.userPermissionKey', { - defaultMessage: 'User:', - })} - {(errorCode === WorkspaceFormErrorCode.DuplicateUserGroupPermissionSetting || - errorCode === WorkspaceFormErrorCode.PermissionUserGroupMissing) && - i18n.translate('workspace.form.errorCallout.userGroupPermissionKey', { - defaultMessage: 'User Group:', - })} -   - {getSuggestionFromErrorCode(findingError)} -
    • + ); }; return ( From b1ab397b0cdf5f0e9f198a50ebf04980e2bc40a2 Mon Sep 17 00:00:00 2001 From: Lin Wang Date: Mon, 8 Jul 2024 17:46:28 +0800 Subject: [PATCH 24/29] Fix error code Signed-off-by: Lin Wang --- .../public/components/workspace_form/utils.ts | 13 +++++++------ .../workspace_permission_setting_input.tsx | 2 +- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/src/plugins/workspace/public/components/workspace_form/utils.ts b/src/plugins/workspace/public/components/workspace_form/utils.ts index 3be8b5bbff23..9ec989bba3e9 100644 --- a/src/plugins/workspace/public/components/workspace_form/utils.ts +++ b/src/plugins/workspace/public/components/workspace_form/utils.ts @@ -351,7 +351,7 @@ export const validateWorkspaceForm = ( }; } else if (isSelectedDataSourcesDuplicated(selectedDataSources.slice(0, i), row)) { dataSourcesErrors[i] = { - code: WorkspaceFormErrorCode.InvalidDataSource, + code: WorkspaceFormErrorCode.DuplicateDataSource, message: i18n.translate('workspace.form.permission.invalidate.group', { defaultMessage: 'Duplicate data sources', }), @@ -373,11 +373,12 @@ export const generateNextPermissionSettingsId = (permissionSettings: Array<{ id: /** * - * Generate permission settings state with provided permission settings, - * If no permission settings provided, it will return current user with read permission and - * an empty user group permission setting. - * If permission settings provided, it will return original permission settings when user group permission settings exists. - * It will return permission setting with an empty user group permission setting when user group permission settings not exists. + * Generate permission settings state with provided operation type and permission settings, + * It will always return current user as an Owner and an empty user group permission settings + * when operation type is create or no users and groups in provided permission settings. + * It will append current user to result permission settings if no user in provided permission settings. + * It will append an empty permission group to result permission settings if no user group in provided permission settings. + * It will always return original permission settings if both user or user group in provided permission settings. * * @param operationType * @param permissionSettings diff --git a/src/plugins/workspace/public/components/workspace_form/workspace_permission_setting_input.tsx b/src/plugins/workspace/public/components/workspace_form/workspace_permission_setting_input.tsx index c43991f492e8..a33fddbc3a3c 100644 --- a/src/plugins/workspace/public/components/workspace_form/workspace_permission_setting_input.tsx +++ b/src/plugins/workspace/public/components/workspace_form/workspace_permission_setting_input.tsx @@ -21,7 +21,7 @@ import { } from './constants'; import { getPermissionModeId } from './utils'; -export const permissionModeOptions = [ +const permissionModeOptions = [ { id: PermissionModeId.Read, label: ( From cb07ad349d30149a4a3d85ac2a2c60a32a906510 Mon Sep 17 00:00:00 2001 From: Lin Wang Date: Mon, 8 Jul 2024 18:45:05 +0800 Subject: [PATCH 25/29] Fix failed unit test Signed-off-by: Lin Wang --- .../workspace/public/components/workspace_form/utils.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/plugins/workspace/public/components/workspace_form/utils.test.ts b/src/plugins/workspace/public/components/workspace_form/utils.test.ts index 10d661f02672..5e3cda676764 100644 --- a/src/plugins/workspace/public/components/workspace_form/utils.test.ts +++ b/src/plugins/workspace/public/components/workspace_form/utils.test.ts @@ -352,7 +352,7 @@ describe('validateWorkspaceForm', () => { false ).selectedDataSources ).toEqual({ - '1': { code: WorkspaceFormErrorCode.InvalidDataSource, message: 'Duplicate data sources' }, + '1': { code: WorkspaceFormErrorCode.DuplicateDataSource, message: 'Duplicate data sources' }, }); }); }); From 2930293a266927530db00231e1a0735592c5569d Mon Sep 17 00:00:00 2001 From: Lin Wang Date: Tue, 9 Jul 2024 13:36:36 +0800 Subject: [PATCH 26/29] Add back color picker Signed-off-by: Lin Wang --- .../workspace_creator.test.tsx | 7 ++++++ .../public/components/workspace_form/types.ts | 1 + .../workspace_form/use_workspace_form.ts | 15 +++++++++++- .../workspace_form/workspace_form.tsx | 23 +++++++++++++++++++ .../workspace_updater.test.tsx | 7 ++++++ 5 files changed, 52 insertions(+), 1 deletion(-) 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 index 44a30227e4c4..e15143054cfb 100644 --- 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,11 +182,18 @@ describe('WorkspaceCreator', () => { fireEvent.input(descriptionInput, { target: { value: 'test workspace description' }, }); + const colorSelector = getByTestId( + 'euiColorPickerAnchor workspaceForm-workspaceDetails-colorPicker' + ); + fireEvent.input(colorSelector, { + target: { value: '#000000' }, + }); fireEvent.click(getByTestId('workspaceUseCase-observability')); fireEvent.click(getByTestId('workspaceForm-bottomBar-createButton')); expect(workspaceClientCreate).toHaveBeenCalledWith( expect.objectContaining({ name: 'test workspace name', + color: '#000000', description: 'test workspace description', features: expect.arrayContaining(['use-case-observability']), }), diff --git a/src/plugins/workspace/public/components/workspace_form/types.ts b/src/plugins/workspace/public/components/workspace_form/types.ts index 26af43ba9be9..c84198f1a204 100644 --- a/src/plugins/workspace/public/components/workspace_form/types.ts +++ b/src/plugins/workspace/public/components/workspace_form/types.ts @@ -34,6 +34,7 @@ export interface WorkspaceFormSubmitData { name: string; description?: string; features?: string[]; + color?: string; permissionSettings?: WorkspacePermissionSetting[]; selectedDataSources?: DataSource[]; } 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 index 10ecf0093f01..ac1dfa1adf7a 100644 --- a/src/plugins/workspace/public/components/workspace_form/use_workspace_form.ts +++ b/src/plugins/workspace/public/components/workspace_form/use_workspace_form.ts @@ -4,7 +4,12 @@ */ import { useCallback, useState, FormEventHandler, useRef, useMemo } from 'react'; -import { htmlIdGenerator, EuiFieldTextProps, EuiTextAreaProps } from '@elastic/eui'; +import { + htmlIdGenerator, + EuiFieldTextProps, + EuiTextAreaProps, + EuiColorPickerProps, +} from '@elastic/eui'; import { useApplications } from '../../hooks'; import { @@ -37,6 +42,7 @@ export const useWorkspaceForm = ({ const applications = useApplications(application); const [name, setName] = useState(defaultValues?.name); const [description, setDescription] = useState(defaultValues?.description); + const [color, setColor] = useState(defaultValues?.color); const defaultValuesRef = useRef(defaultValues); const initialPermissionSettingsRef = useRef( generatePermissionSettingsState(operationType, defaultValues?.permissionSettings) @@ -67,6 +73,7 @@ export const useWorkspaceForm = ({ description, features: featureConfigs, useCases: selectedUseCases, + color, permissionSettings, selectedDataSources, }); @@ -115,6 +122,7 @@ export const useWorkspaceForm = ({ onSubmit?.({ name: currentFormData.name!, description: currentFormData.description, + color: currentFormData.color, features: currentFormData.features, permissionSettings: currentFormData.permissionSettings.filter( (item) => @@ -135,6 +143,10 @@ export const useWorkspaceForm = ({ setDescription(e.target.value); }, []); + const handleColorChange = useCallback['onChange']>((text) => { + setColor(text); + }, []); + return { formId: formIdRef.current, formData, @@ -143,6 +155,7 @@ export const useWorkspaceForm = ({ numberOfErrors, numberOfChanges, handleFormSubmit, + handleColorChange, handleUseCasesChange, handleNameInputChange, setPermissionSettings, diff --git a/src/plugins/workspace/public/components/workspace_form/workspace_form.tsx b/src/plugins/workspace/public/components/workspace_form/workspace_form.tsx index 521e86c0a02c..45a1d1585b9a 100644 --- a/src/plugins/workspace/public/components/workspace_form/workspace_form.tsx +++ b/src/plugins/workspace/public/components/workspace_form/workspace_form.tsx @@ -13,6 +13,7 @@ import { EuiFieldText, EuiText, EuiTextArea, + EuiColorPicker, } from '@elastic/eui'; import { i18n } from '@osd/i18n'; @@ -42,6 +43,7 @@ export const WorkspaceForm = (props: WorkspaceFormProps) => { numberOfErrors, numberOfChanges, handleFormSubmit, + handleColorChange, handleUseCasesChange, handleNameInputChange, setPermissionSettings, @@ -118,6 +120,27 @@ export const WorkspaceForm = (props: WorkspaceFormProps) => { /> + +
    + + {i18n.translate('workspace.form.workspaceDetails.color.helpText', { + defaultMessage: 'Accent color for your workspace', + })} + + + +
    +     diff --git a/src/plugins/workspace/public/components/workspace_updater/workspace_updater.test.tsx b/src/plugins/workspace/public/components/workspace_updater/workspace_updater.test.tsx index 462d8b048dd7..ac7bb3d0401b 100644 --- a/src/plugins/workspace/public/components/workspace_updater/workspace_updater.test.tsx +++ b/src/plugins/workspace/public/components/workspace_updater/workspace_updater.test.tsx @@ -203,6 +203,12 @@ describe('WorkspaceUpdater', () => { fireEvent.input(descriptionInput, { target: { value: 'test workspace description' }, }); + const colorSelector = getByTestId( + 'euiColorPickerAnchor workspaceForm-workspaceDetails-colorPicker' + ); + fireEvent.input(colorSelector, { + target: { value: '#000000' }, + }); fireEvent.click(getByTestId('workspaceUseCase-observability')); fireEvent.click(getByTestId('workspaceUseCase-analytics')); @@ -224,6 +230,7 @@ describe('WorkspaceUpdater', () => { expect.any(String), expect.objectContaining({ name: 'test workspace name', + color: '#000000', description: 'test workspace description', features: expect.arrayContaining(['use-case-analytics']), }), From 7e2f03642110ae7766e6cccac2c4fc112ebe9545 Mon Sep 17 00:00:00 2001 From: Lin Wang Date: Thu, 11 Jul 2024 10:59:53 +0800 Subject: [PATCH 27/29] Address UX comments Signed-off-by: Lin Wang --- src/plugins/workspace/common/constants.ts | 2 +- .../workspace_creator/workspace_creator.test.tsx | 12 ++++++------ .../public/components/workspace_form/utils.test.ts | 6 +++--- .../public/components/workspace_form/utils.ts | 13 ++++++------- .../workspace_form_error_callout.test.tsx | 4 +--- .../workspace_form/workspace_form_error_callout.tsx | 6 +++--- src/plugins/workspace/server/utils.test.ts | 4 ++-- 7 files changed, 22 insertions(+), 25 deletions(-) diff --git a/src/plugins/workspace/common/constants.ts b/src/plugins/workspace/common/constants.ts index cc1d0552a38e..a1f9e7d38e40 100644 --- a/src/plugins/workspace/common/constants.ts +++ b/src/plugins/workspace/common/constants.ts @@ -183,4 +183,4 @@ export const WORKSPACE_USE_CASES = Object.freeze({ }, }); -export const CURRENT_USER_PLACEHOLDER = '%current_user%'; +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 index e15143054cfb..076596de5aaf 100644 --- a/src/plugins/workspace/public/components/workspace_creator/workspace_creator.test.tsx +++ b/src/plugins/workspace/public/components/workspace_creator/workspace_creator.test.tsx @@ -200,8 +200,8 @@ describe('WorkspaceCreator', () => { { dataSources: [], permissions: { - library_write: { users: ['%current_user%'] }, - write: { users: ['%current_user%'] }, + library_write: { users: ['%me%'] }, + write: { users: ['%me%'] }, }, } ); @@ -274,10 +274,10 @@ describe('WorkspaceCreator', () => { dataSources: [], permissions: { write: { - users: ['%current_user%'], + users: ['%me%'], }, library_write: { - users: ['%current_user%'], + users: ['%me%'], }, }, } @@ -315,10 +315,10 @@ describe('WorkspaceCreator', () => { dataSources: ['id1'], permissions: { library_write: { - users: ['%current_user%'], + users: ['%me%'], }, write: { - users: ['%current_user%'], + users: ['%me%'], }, }, } diff --git a/src/plugins/workspace/public/components/workspace_form/utils.test.ts b/src/plugins/workspace/public/components/workspace_form/utils.test.ts index 5e3cda676764..1f8b7ee04fd2 100644 --- a/src/plugins/workspace/public/components/workspace_form/utils.test.ts +++ b/src/plugins/workspace/public/components/workspace_form/utils.test.ts @@ -158,7 +158,7 @@ describe('validateWorkspaceForm', () => { it('should return error if name is empty', () => { expect(validateWorkspaceForm({}, false).name).toEqual({ code: WorkspaceFormErrorCode.WorkspaceNameMissing, - message: "Name can't be empty.", + message: 'Name is required. Enter a name.', }); }); it('should return error if name is invalid', () => { @@ -270,7 +270,7 @@ describe('validateWorkspaceForm', () => { ).permissionSettings?.overall ).toEqual({ code: WorkspaceFormErrorCode.PermissionSettingOwnerMissing, - message: 'Permission setting missing', + message: 'Add a workspace owner.', }); expect( @@ -290,7 +290,7 @@ describe('validateWorkspaceForm', () => { ).permissionSettings?.overall ).toEqual({ code: WorkspaceFormErrorCode.PermissionSettingOwnerMissing, - message: 'Permission setting missing', + message: 'Add a workspace owner.', }); }); diff --git a/src/plugins/workspace/public/components/workspace_form/utils.ts b/src/plugins/workspace/public/components/workspace_form/utils.ts index 9ec989bba3e9..9e714a268614 100644 --- a/src/plugins/workspace/public/components/workspace_form/utils.ts +++ b/src/plugins/workspace/public/components/workspace_form/utils.ts @@ -236,15 +236,15 @@ const validatePermissionSetting = ( Pick & Partial > ) => { - const permissionSettingMissingError = { + const permissionSettingOwnerMissingError = { code: WorkspaceFormErrorCode.PermissionSettingOwnerMissing, - message: i18n.translate('workspace.form.permission.setting.missing', { - defaultMessage: 'Permission setting missing', + message: i18n.translate('workspace.form.permission.setting.owner.missing', { + defaultMessage: 'Add a workspace owner.', }), }; if (!permissionSettings) { return { - overall: permissionSettingMissingError, + overall: permissionSettingOwnerMissingError, }; } @@ -283,12 +283,11 @@ const validatePermissionSetting = ( } } } - return { ...(!permissionSettings.some( (setting) => setting.modes && getPermissionModeId(setting.modes) === PermissionModeId.Owner ) - ? { overall: permissionSettingMissingError } + ? { overall: permissionSettingOwnerMissingError } : {}), ...(Object.keys(permissionSettingsErrors).length > 0 ? { fields: permissionSettingsErrors } @@ -323,7 +322,7 @@ export const validateWorkspaceForm = ( formErrors.name = { code: WorkspaceFormErrorCode.WorkspaceNameMissing, message: i18n.translate('workspace.form.detail.name.empty', { - defaultMessage: "Name can't be empty.", + defaultMessage: 'Name is required. Enter a name.', }), }; } diff --git a/src/plugins/workspace/public/components/workspace_form/workspace_form_error_callout.test.tsx b/src/plugins/workspace/public/components/workspace_form/workspace_form_error_callout.test.tsx index 4d789ceb1ed2..e248bf202257 100644 --- a/src/plugins/workspace/public/components/workspace_form/workspace_form_error_callout.test.tsx +++ b/src/plugins/workspace/public/components/workspace_form/workspace_form_error_callout.test.tsx @@ -140,9 +140,7 @@ describe('WorkspaceFormErrorCallout', () => { }); expect( - renderResult.getByText( - 'Permission setting: Enter an owner at user or user group permission setting.' - ) + renderResult.getByText('Manage access and permissions: Add a workspace owner.') ).toBeInTheDocument(); }); diff --git a/src/plugins/workspace/public/components/workspace_form/workspace_form_error_callout.tsx b/src/plugins/workspace/public/components/workspace_form/workspace_form_error_callout.tsx index 96a33df6bbcf..e7388f1909b3 100644 --- a/src/plugins/workspace/public/components/workspace_form/workspace_form_error_callout.tsx +++ b/src/plugins/workspace/public/components/workspace_form/workspace_form_error_callout.tsx @@ -40,7 +40,7 @@ const getSuggestionFromErrorCode = (error: WorkspaceFormError) => { }); case WorkspaceFormErrorCode.PermissionSettingOwnerMissing: return i18n.translate('workspace.form.errorCallout.permissionSettingOwnerMissing', { - defaultMessage: 'Enter an owner at user or user group permission setting.', + defaultMessage: 'Add a workspace owner.', }); default: return error.message; @@ -124,8 +124,8 @@ export const WorkspaceFormErrorCallout = ({ errors }: WorkspaceFormErrorCalloutP )} {errors.permissionSettings?.overall && ( diff --git a/src/plugins/workspace/server/utils.test.ts b/src/plugins/workspace/server/utils.test.ts index c4855af43ad8..95db93e5b97a 100644 --- a/src/plugins/workspace/server/utils.test.ts +++ b/src/plugins/workspace/server/utils.test.ts @@ -163,10 +163,10 @@ describe('workspace utils', () => { expect( transferCurrentUserInPermissions('foo', { library_write: { - users: ['%current_user%', 'bar'], + users: ['%me%', 'bar'], }, write: { - users: ['%current_user%'], + users: ['%me%'], }, read: { users: ['bar'], From 9c42390a19eaf6c694cf7d50877d7162e40d336e Mon Sep 17 00:00:00 2001 From: Lin Wang Date: Thu, 11 Jul 2024 11:28:15 +0800 Subject: [PATCH 28/29] Fix empty user no workspace owner Signed-off-by: Lin Wang --- .../workspace_form/use_workspace_form.test.ts | 46 ++++++++++++++++++- .../workspace_form/use_workspace_form.ts | 11 +++-- 2 files changed, 50 insertions(+), 7 deletions(-) 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 index b664ec346562..095ba67a2897 100644 --- 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,11 +6,12 @@ import { renderHook, act } from '@testing-library/react-hooks'; import { applicationServiceMock } from '../../../../../core/public/mocks'; -import { WorkspaceOperationType } from './constants'; +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: { @@ -18,6 +19,7 @@ const setup = (defaultValues?: WorkspaceFormData) => { defaultValues, onSubmit: onSubmitMock, operationType: WorkspaceOperationType.Create, + permissionEnabled, }, }); return { @@ -67,6 +69,46 @@ describe('useWorkspaceForm', () => { ); 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(); + }); it('should call onSubmit with workspace name and features', async () => { const { renderResult, onSubmitMock } = setup({ id: 'foo', 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 index ac1dfa1adf7a..b15a6e6a670d 100644 --- a/src/plugins/workspace/public/components/workspace_form/use_workspace_form.ts +++ b/src/plugins/workspace/public/components/workspace_form/use_workspace_form.ts @@ -110,6 +110,11 @@ export const useWorkspaceForm = ({ (e) => { e.preventDefault(); const currentFormData = getFormDataRef.current(); + currentFormData.permissionSettings = currentFormData.permissionSettings.filter( + (item) => + (item.type === WorkspacePermissionItemType.User && !!item.userId) || + (item.type === WorkspacePermissionItemType.Group && !!item.group) + ); const currentFormErrors: WorkspaceFormErrors = validateWorkspaceForm( currentFormData, !!permissionEnabled @@ -124,11 +129,7 @@ export const useWorkspaceForm = ({ description: currentFormData.description, color: currentFormData.color, features: currentFormData.features, - permissionSettings: currentFormData.permissionSettings.filter( - (item) => - (item.type === WorkspacePermissionItemType.User && !!item.userId) || - (item.type === WorkspacePermissionItemType.Group && !!item.group) - ) as WorkspacePermissionSetting[], + permissionSettings: currentFormData.permissionSettings as WorkspacePermissionSetting[], selectedDataSources: currentFormData.selectedDataSources, }); }, From 7e28c7d9218043c83a71426ba50362e7d536ef65 Mon Sep 17 00:00:00 2001 From: Lin Wang Date: Thu, 11 Jul 2024 11:33:09 +0800 Subject: [PATCH 29/29] Change to Associate data source Signed-off-by: Lin Wang --- .../public/components/workspace_form/workspace_form.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/plugins/workspace/public/components/workspace_form/workspace_form.tsx b/src/plugins/workspace/public/components/workspace_form/workspace_form.tsx index 45a1d1585b9a..28c3d66ee6ec 100644 --- a/src/plugins/workspace/public/components/workspace_form/workspace_form.tsx +++ b/src/plugins/workspace/public/components/workspace_form/workspace_form.tsx @@ -192,7 +192,7 @@ export const WorkspaceForm = (props: WorkspaceFormProps) => {

    {i18n.translate('workspace.form.selectDataSource.title', { - defaultMessage: 'Select Data Sources', + defaultMessage: 'Associate data source', })}