From cc8fde56469a29e352e854ea6a67e8a0481b7082 Mon Sep 17 00:00:00 2001 From: yuye-aws Date: Thu, 19 Oct 2023 14:41:26 +0800 Subject: [PATCH 01/21] track usnaved changes Signed-off-by: yuye-aws --- .../workspace_bottom_bar.tsx | 10 +- .../workspace_creator/workspace_form.tsx | 400 ++++++++++++------ .../workspace_permission_setting_panel.tsx | 2 +- 3 files changed, 289 insertions(+), 123 deletions(-) diff --git a/src/plugins/workspace/public/components/workspace_creator/workspace_bottom_bar.tsx b/src/plugins/workspace/public/components/workspace_creator/workspace_bottom_bar.tsx index 79f1f92c8685..aa248ea45c44 100644 --- a/src/plugins/workspace/public/components/workspace_creator/workspace_bottom_bar.tsx +++ b/src/plugins/workspace/public/components/workspace_creator/workspace_bottom_bar.tsx @@ -21,7 +21,8 @@ import { WorkspaceCancelModal } from './workspace_cancel_modal'; interface WorkspaceBottomBarProps { formId: string; opType?: string; - numberOfErrors: number; + errorsCount: number; + unsavedChangesCount: number; application: ApplicationStart; } @@ -29,7 +30,8 @@ interface WorkspaceBottomBarProps { export const WorkspaceBottomBar = ({ formId, opType, - numberOfErrors, + errorsCount, + unsavedChangesCount, application, }: WorkspaceBottomBarProps) => { const [isCancelModalVisible, setIsCancelModalVisible] = useState(false); @@ -47,13 +49,13 @@ export const WorkspaceBottomBar = ({ {opType === WORKSPACE_OP_TYPE_UPDATE ? ( {i18n.translate('workspace.form.bottomBar.unsavedChanges', { - defaultMessage: '1 Unsaved change(s)', + defaultMessage: `${unsavedChangesCount} Unsaved change(s)`, })} ) : ( {i18n.translate('workspace.form.bottomBar.errors', { - defaultMessage: `${numberOfErrors} Error(s)`, + defaultMessage: `${errorsCount} Error(s)`, })} )} diff --git a/src/plugins/workspace/public/components/workspace_creator/workspace_form.tsx b/src/plugins/workspace/public/components/workspace_creator/workspace_form.tsx index b2ff40855a9d..0da66d841eae 100644 --- a/src/plugins/workspace/public/components/workspace_creator/workspace_form.tsx +++ b/src/plugins/workspace/public/components/workspace_creator/workspace_form.tsx @@ -35,9 +35,14 @@ import { ApplicationStart, DEFAULT_APP_CATEGORIES, MANAGEMENT_WORKSPACE_ID, + WorkspacePermissionMode, } from '../../../../../core/public'; import { useApplications } from '../../hooks'; -import { DEFAULT_CHECKED_FEATURES_IDS } from '../../../common/constants'; +import { + DEFAULT_CHECKED_FEATURES_IDS, + WORKSPACE_OP_TYPE_CREATE, + WORKSPACE_OP_TYPE_UPDATE, +} from '../../../common/constants'; import { isFeatureDependBySelectedFeatures, getFinalFeatureIdsByDependency, @@ -46,6 +51,7 @@ import { import { WorkspaceBottomBar } from './workspace_bottom_bar'; import { WorkspaceIconSelector } from './workspace_icon_selector'; import { + getPermissionModeId, WorkspacePermissionSetting, WorkspacePermissionItemType, WorkspacePermissionSettingPanel, @@ -54,6 +60,7 @@ import { featureMatchesConfig } from '../../utils'; enum WorkspaceFormTabs { NotSelected, + WorkspaceSettings, FeatureVisibility, UsersAndPermissions, } @@ -116,18 +123,129 @@ const isValidNameOrDescription = (input?: string) => { return regex.test(input); }; -const getNumberOfErrors = (formErrors: WorkspaceFormErrors) => { - let numberOfErrors = 0; +const getErrorsCount = (formErrors: WorkspaceFormErrors) => { + let errorsCount = 0; if (formErrors.name) { - numberOfErrors += 1; + errorsCount += 1; } if (formErrors.description) { - numberOfErrors += 1; + errorsCount += 1; } if (formErrors.permissions) { - numberOfErrors += formErrors.permissions.length; + errorsCount += formErrors.permissions.length; } - return numberOfErrors; + return errorsCount; +}; + +// when editing, attributes could be undefined in workspace form +type WorkspaceFormEditingData = Partial< + Omit & { + permissions: Array>; + } +>; + +type UserOrGroupPermissionEditingData = Array< + Partial<{ id: string; modes: WorkspacePermissionMode[] }> +>; + +const getUserAndGroupPermissions = (permissions: Array>) => { + const userPermissions: UserOrGroupPermissionEditingData = []; + const groupPermissions: UserOrGroupPermissionEditingData = []; + if (permissions) { + for (const permission of permissions) { + if (permission.type === WorkspacePermissionItemType.User) { + userPermissions.push({ id: permission.userId, modes: permission.modes }); + } + if (permission.type === WorkspacePermissionItemType.Group) { + groupPermissions.push({ id: permission.group, modes: permission.modes }); + } + } + } + return [userPermissions, groupPermissions]; +}; + +const getUnsavedUserOrGroupPermissionChangesCount = ( + initialPermissions: UserOrGroupPermissionEditingData, + currentPermissions: UserOrGroupPermissionEditingData +) => { + // for user or group permissions, unsaved changes is the sum of + // # deleted permissions, # added permissions and # edited permissions + let addedPermissions = 0; + let editedPermissions = 0; + const initialPermissionMap = new Map(); + for (const permission of initialPermissions) { + if (permission.id) { + initialPermissionMap.set(permission.id, permission.modes ?? []); + } + } + + for (const permission of currentPermissions) { + if (!permission.id) { + addedPermissions += 1; // added permissions + } else { + const permissionModes = initialPermissionMap.get(permission.id); + if (!permissionModes) { + addedPermissions += 1; + } else if ( + getPermissionModeId(permissionModes) !== getPermissionModeId(permission.modes ?? []) + ) { + editedPermissions += 1; // added or edited permissions + } + } + } + + // currentPermissions.length = initialPermissions.length + # added permissions - # deleted permissions + const deletedPermissions = + addedPermissions + initialPermissions.length - currentPermissions.length; + + return addedPermissions + editedPermissions + deletedPermissions; +}; + +const getUnsavedChangesCount = ( + initialFormData: WorkspaceFormEditingData, + currentFormData: WorkspaceFormEditingData +) => { + let unsavedChangesCount = 0; + if (initialFormData.name !== currentFormData.name) { + unsavedChangesCount += 1; + } + // initial and current description could be undefined + const initialDescription = initialFormData.description ?? ''; + const currentDescription = currentFormData.description ?? ''; + if (initialDescription !== currentDescription) { + unsavedChangesCount += 1; + } + if (initialFormData.color !== currentFormData.color) { + unsavedChangesCount += 1; + } + if (initialFormData.icon !== currentFormData.icon) { + unsavedChangesCount += 1; + } + if (initialFormData.defaultVISTheme !== currentFormData.defaultVISTheme) { + unsavedChangesCount += 1; + } + const featureIntersectionCount = ( + initialFormData.features?.filter((feature) => currentFormData.features?.includes(feature)) ?? [] + ).length; + // for features, unsaved changes is the sum of # deleted features and # added features + unsavedChangesCount += (initialFormData.features?.length ?? 0) - featureIntersectionCount; + unsavedChangesCount += (currentFormData.features?.length ?? 0) - featureIntersectionCount; + // for permissions, unsaved changes is the sum of # unsaved user permissions and # unsaved group permissions + const [initialUserPermissions, initialGroupPermissions] = getUserAndGroupPermissions( + initialFormData.permissions ?? [] + ); + const [currentUserPermissions, currentGroupPermissions] = getUserAndGroupPermissions( + currentFormData.permissions ?? [] + ); + unsavedChangesCount += getUnsavedUserOrGroupPermissionChangesCount( + initialUserPermissions, + currentUserPermissions + ); + unsavedChangesCount += getUnsavedUserOrGroupPermissionChangesCount( + initialGroupPermissions, + currentGroupPermissions + ); + return unsavedChangesCount; }; const isUserOrGroupPermissionSettingDuplicated = ( @@ -183,7 +301,7 @@ export const WorkspaceForm = ({ ? WorkspaceFormTabs.UsersAndPermissions : WorkspaceFormTabs.NotSelected ); - const [numberOfErrors, setNumberOfErrors] = useState(0); + const [errorsCount, setErrorsCount] = useState(0); // The matched feature id list based on original feature config, // the feature category will be expanded to list of feature ids const defaultFeatures = useMemo(() => { @@ -211,6 +329,18 @@ export const WorkspaceForm = ({ : [] ); + const libraryCategoryLabel = i18n.translate('core.ui.libraryNavList.label', { + defaultMessage: 'Library', + }); + const categoryToDescription: { [key: string]: string } = { + [libraryCategoryLabel]: i18n.translate( + 'workspace.form.featureVisibility.libraryCategory.Description', + { + defaultMessage: 'Workspace-owned library items', + } + ), + }; + const [formErrors, setFormErrors] = useState({}); const formIdRef = useRef(); const getFormData = () => ({ @@ -225,6 +355,28 @@ export const WorkspaceForm = ({ const getFormDataRef = useRef(getFormData); getFormDataRef.current = getFormData; + const unsavedChangesCount = useMemo(() => { + const currentFormData = { + name, + description, + features: selectedFeatureIds, + color, + icon, + defaultVISTheme, + permissions: permissionSettings, + }; + return getUnsavedChangesCount(defaultValues ?? {}, currentFormData); + }, [ + defaultValues, + name, + description, + color, + icon, + defaultVISTheme, + selectedFeatureIds, + permissionSettings, + ]); + const featureOrGroups = useMemo(() => { const transformedApplications = applications.map((app) => { if (app.category?.id === DEFAULT_APP_CATEGORIES.opensearchDashboards.id) { @@ -232,9 +384,7 @@ export const WorkspaceForm = ({ ...app, category: { ...app.category, - label: i18n.translate('core.ui.libraryNavList.label', { - defaultMessage: 'Library', - }), + label: libraryCategoryLabel, }, }; } @@ -271,7 +421,7 @@ export const WorkspaceForm = ({ }, ]; }, []); - }, [applications]); + }, [applications, libraryCategoryLabel]); const allFeatures = useMemo( () => @@ -432,10 +582,10 @@ export const WorkspaceForm = ({ permissions: permissionErrors, }; } - const currentNumberOfErrors = getNumberOfErrors(currentFormErrors); + const currentErrorsCount = getErrorsCount(currentFormErrors); setFormErrors(currentFormErrors); - setNumberOfErrors(currentNumberOfErrors); - if (currentNumberOfErrors > 0) { + setErrorsCount(currentErrorsCount); + if (currentErrorsCount > 0) { return; } @@ -483,6 +633,10 @@ export const WorkspaceForm = ({ setSelectedTab(WorkspaceFormTabs.UsersAndPermissions); }, []); + const handleTabWorkspaceSettingsClick = useCallback(() => { + setSelectedTab(WorkspaceFormTabs.WorkspaceSettings); + }, []); + const onDefaultVISThemeChange = (e: React.ChangeEvent) => { setDefaultVISTheme(e.target.value); }; @@ -493,114 +647,111 @@ export const WorkspaceForm = ({ const featureVisibilityTitle = i18n.translate('workspace.form.featureVisibility.title', { defaultMessage: 'Feature Visibility', }); + const workspaceSettingsTitle = i18n.translate('workspace.form.workspaceSettings.title', { + defaultMessage: 'Workspace Settings', + }); const usersAndPermissionsTitle = i18n.translate('workspace.form.usersAndPermissions.title', { defaultMessage: 'Users & Permissions', }); - const libraryCategoryLabel = i18n.translate('core.ui.libraryNavList.label', { - defaultMessage: 'Library', - }); - const categoryToDescription: { [key: string]: string } = { - [libraryCategoryLabel]: i18n.translate( - 'workspace.form.featureVisibility.libraryCategory.Description', - { - defaultMessage: 'Workspace-owned library items', - } - ), - }; - return ( - - - -

{workspaceDetailsTitle}

- - - - - - - - Description - optional - - } - helpText={i18n.translate('workspace.form.workspaceDetails.description.helpText', { - defaultMessage: - 'Valid characters are a-z, A-Z, 0-9, (), [], _ (underscore), - (hyphen) and (space).', - })} - isInvalid={!!formErrors.description} - error={formErrors.description} - > - - - -
- - {i18n.translate('workspace.form.workspaceDetails.color.helpText', { - defaultMessage: 'Accent color for your workspace', - })} - - - -
- - - - - - + +

+ {opType === WORKSPACE_OP_TYPE_UPDATE ? workspaceSettingsTitle : workspaceDetailsTitle} +

+ + + + + + + + Description - optional + + } + helpText={i18n.translate('workspace.form.workspaceDetails.description.helpText', { + defaultMessage: + 'Valid characters are a-z, A-Z, 0-9, (), [], _ (underscore), - (hyphen) and (space).', + })} + isInvalid={!!formErrors.description} + error={formErrors.description} + > + + + +
+ + {i18n.translate('workspace.form.workspaceDetails.color.helpText', { + defaultMessage: 'Accent color for your workspace', + })} + + + - - - +
+ + + + + + + + + ); + return ( + + {opType === WORKSPACE_OP_TYPE_CREATE && workspaceInfoTab} + {opType === WORKSPACE_OP_TYPE_CREATE && } {!isEditingManagementWorkspace && ( {featureVisibilityTitle} )} + {opType === WORKSPACE_OP_TYPE_UPDATE && ( + + {workspaceSettingsTitle} + + )} {permissionEnabled && ( + {opType === WORKSPACE_OP_TYPE_UPDATE && + selectedTab === WorkspaceFormTabs.WorkspaceSettings && + workspaceInfoTab} + {selectedTab === WorkspaceFormTabs.FeatureVisibility && ( @@ -719,7 +882,8 @@ export const WorkspaceForm = ({ opType={opType} formId={formIdRef.current} application={application} - numberOfErrors={numberOfErrors} + errorsCount={errorsCount} + unsavedChangesCount={unsavedChangesCount} /> ); diff --git a/src/plugins/workspace/public/components/workspace_creator/workspace_permission_setting_panel.tsx b/src/plugins/workspace/public/components/workspace_creator/workspace_permission_setting_panel.tsx index d95b062db3c4..20c1a6d03815 100644 --- a/src/plugins/workspace/public/components/workspace_creator/workspace_permission_setting_panel.tsx +++ b/src/plugins/workspace/public/components/workspace_creator/workspace_permission_setting_panel.tsx @@ -81,7 +81,7 @@ const generateWorkspacePermissionItemKey = ( ].join('-'); // default permission mode is read -const getPermissionModeId = (modes: WorkspacePermissionMode[]) => { +export const getPermissionModeId = (modes: WorkspacePermissionMode[]) => { for (const key in optionIdToWorkspacePermissionModesMap) { if (optionIdToWorkspacePermissionModesMap[key].every((mode) => modes?.includes(mode))) { return key; From 2522c2441b66f52cccbbe9309f3d52f05f413340 Mon Sep 17 00:00:00 2001 From: yuye-aws Date: Thu, 19 Oct 2023 15:12:56 +0800 Subject: [PATCH 02/21] implement overview section Signed-off-by: yuye-aws --- .../workspace_creator/workspace_form.tsx | 81 ++++++++++++++++--- .../workspace_updater.test.tsx | 0 2 files changed, 71 insertions(+), 10 deletions(-) create mode 100644 src/plugins/workspace/public/components/workspace_updater/workspace_updater.test.tsx diff --git a/src/plugins/workspace/public/components/workspace_creator/workspace_form.tsx b/src/plugins/workspace/public/components/workspace_creator/workspace_form.tsx index 0da66d841eae..170b04c456dc 100644 --- a/src/plugins/workspace/public/components/workspace_creator/workspace_form.tsx +++ b/src/plugins/workspace/public/components/workspace_creator/workspace_form.tsx @@ -15,6 +15,7 @@ import { EuiSelect, EuiText, EuiFlexItem, + EuiFlexGrid, htmlIdGenerator, EuiCheckbox, EuiCheckboxGroup, @@ -23,7 +24,6 @@ import { EuiFieldTextProps, EuiColorPicker, EuiColorPickerProps, - EuiHorizontalRule, EuiFlexGroup, EuiTab, EuiTabs, @@ -641,6 +641,9 @@ export const WorkspaceForm = ({ setDefaultVISTheme(e.target.value); }; + const workspaceOverviewTitle = i18n.translate('workspace.form.overview.title', { + defaultMessage: 'Overview', + }); const workspaceDetailsTitle = i18n.translate('workspace.form.workspaceDetails.title', { defaultMessage: 'Workspace Details', }); @@ -654,15 +657,73 @@ export const WorkspaceForm = ({ defaultMessage: 'Users & Permissions', }); - const workspaceInfoTab = ( + const workspaceOverviewSection = ( + + +

{workspaceOverviewTitle}

+ + + + + <> + + + {i18n.translate('workspace.form.overview.workspaceNameTitle', { + defaultMessage: 'Name', + })} + + + {defaultValues?.name} + + + + <> + + + {i18n.translate('workspace.form.overview.lastUpdatedTimeTitle', { + defaultMessage: 'Last Updated', + })} + + + {defaultValues?.name} + + + + <> + + + {i18n.translate('workspace.form.overview.createdTimeTitle', { + defaultMessage: 'Created', + })} + + + {defaultValues?.name} + + + + <> + + + {i18n.translate('workspace.form.overview.workspaceDescriptionTitle', { + defaultMessage: 'Workspace Description', + })} + + + {defaultValues?.description} + + + + + ); + + const workspaceInfoSection = (

{opType === WORKSPACE_OP_TYPE_UPDATE ? workspaceSettingsTitle : workspaceDetailsTitle}

- - + - {opType === WORKSPACE_OP_TYPE_CREATE && workspaceInfoTab} - {opType === WORKSPACE_OP_TYPE_CREATE && } + {opType === WORKSPACE_OP_TYPE_UPDATE && workspaceOverviewSection} + {opType === WORKSPACE_OP_TYPE_CREATE && workspaceInfoSection} + {!isEditingManagementWorkspace && (

{featureVisibilityTitle}

 - - + {featureOrGroups.map((featureOrGroup) => { const features = isWorkspaceFeatureGroup(featureOrGroup) ? featureOrGroup.features : []; const selectedIds = selectedFeatureIds.filter((id) => @@ -867,7 +928,7 @@ export const WorkspaceForm = ({

{usersAndPermissionsTitle}

 - + Date: Wed, 25 Oct 2023 17:10:42 +0800 Subject: [PATCH 03/21] refactor: add utils, types and constants under workspace creator folder Signed-off-by: yuye-aws --- .../components/workspace_creator/constants.ts | 23 +++ .../components/workspace_creator/types.ts | 57 +++++ .../components/workspace_creator/utils.ts | 147 +++++++++++++ .../workspace_creator/workspace_form.tsx | 194 ++---------------- .../workspace_permission_setting_panel.tsx | 51 +---- 5 files changed, 248 insertions(+), 224 deletions(-) create mode 100644 src/plugins/workspace/public/components/workspace_creator/constants.ts create mode 100644 src/plugins/workspace/public/components/workspace_creator/types.ts create mode 100644 src/plugins/workspace/public/components/workspace_creator/utils.ts diff --git a/src/plugins/workspace/public/components/workspace_creator/constants.ts b/src/plugins/workspace/public/components/workspace_creator/constants.ts new file mode 100644 index 000000000000..a5e31ed9b8f0 --- /dev/null +++ b/src/plugins/workspace/public/components/workspace_creator/constants.ts @@ -0,0 +1,23 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +import { WorkspacePermissionMode } from '../../../../../core/public'; + +export enum PermissionModeId { + Read = 'read', + ReadAndWrite = 'read+write', + Admin = 'admin', +} + +export const OptionIdToWorkspacePermissionModesMap: { + [key: string]: WorkspacePermissionMode[]; +} = { + [PermissionModeId.Read]: [WorkspacePermissionMode.LibraryRead, WorkspacePermissionMode.Read], + [PermissionModeId.ReadAndWrite]: [ + WorkspacePermissionMode.LibraryWrite, + WorkspacePermissionMode.Read, + ], + [PermissionModeId.Admin]: [WorkspacePermissionMode.LibraryWrite, WorkspacePermissionMode.Write], +}; diff --git a/src/plugins/workspace/public/components/workspace_creator/types.ts b/src/plugins/workspace/public/components/workspace_creator/types.ts new file mode 100644 index 000000000000..fef4186aef92 --- /dev/null +++ b/src/plugins/workspace/public/components/workspace_creator/types.ts @@ -0,0 +1,57 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +import { App, WorkspacePermissionMode } from '../../../../../core/public'; + +export interface WorkspaceFeature extends Pick { + id: string; + name: string; +} + +export interface WorkspaceFeatureGroup { + name: string; + features: WorkspaceFeature[]; +} +export interface WorkspaceFormSubmitData { + name: string; + description?: string; + features?: string[]; + color?: string; + icon?: string; + defaultVISTheme?: string; + permissions: WorkspacePermissionSetting[]; +} + +export interface WorkspaceFormData extends WorkspaceFormSubmitData { + id: string; + reserved?: boolean; +} + +export type WorkspaceFormErrors = Omit< + { [key in keyof WorkspaceFormData]?: string }, + 'permissions' +> & { + permissions?: string[]; +}; + +// when editing, attributes could be undefined in workspace form +export type WorkspaceFormEditingData = Partial< + Omit & { + permissions: Array>; + } +>; + +export type UserOrGroupPermissionEditingData = Array< + Partial<{ id: string; modes: WorkspacePermissionMode[] }> +>; + +export type WorkspacePermissionSetting = + | { type: WorkspacePermissionItemType.User; userId: string; modes: WorkspacePermissionMode[] } + | { type: WorkspacePermissionItemType.Group; group: string; modes: WorkspacePermissionMode[] }; + +export enum WorkspacePermissionItemType { + User = 'user', + Group = 'group', +} diff --git a/src/plugins/workspace/public/components/workspace_creator/utils.ts b/src/plugins/workspace/public/components/workspace_creator/utils.ts new file mode 100644 index 000000000000..c815b1f894d0 --- /dev/null +++ b/src/plugins/workspace/public/components/workspace_creator/utils.ts @@ -0,0 +1,147 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +import { WorkspacePermissionMode } from '../../../../../core/public'; +import { + UserOrGroupPermissionEditingData, + WorkspaceFormEditingData, + WorkspaceFormErrors, + WorkspacePermissionSetting, + WorkspacePermissionItemType, +} from './types'; +import { OptionIdToWorkspacePermissionModesMap, PermissionModeId } from './constants'; + +export const isValidWorkspacePermissionSetting = ( + setting: Partial +): setting is WorkspacePermissionSetting => + !!setting.modes && + setting.modes.length > 0 && + ((setting.type === WorkspacePermissionItemType.User && !!setting.userId) || + (setting.type === WorkspacePermissionItemType.Group && !!setting.group)); + +export const getErrorsCount = (formErrors: WorkspaceFormErrors) => { + let errorsCount = 0; + if (formErrors.name) { + errorsCount += 1; + } + if (formErrors.description) { + errorsCount += 1; + } + if (formErrors.permissions) { + errorsCount += formErrors.permissions.length; + } + return errorsCount; +}; +export const getUserAndGroupPermissions = ( + permissions: Array> +) => { + const userPermissions: UserOrGroupPermissionEditingData = []; + const groupPermissions: UserOrGroupPermissionEditingData = []; + if (permissions) { + for (const permission of permissions) { + if (permission.type === WorkspacePermissionItemType.User) { + userPermissions.push({ id: permission.userId, modes: permission.modes }); + } + if (permission.type === WorkspacePermissionItemType.Group) { + groupPermissions.push({ id: permission.group, modes: permission.modes }); + } + } + } + return [userPermissions, groupPermissions]; +}; + +const getUnsavedUserOrGroupPermissionChangesCount = ( + initialPermissions: UserOrGroupPermissionEditingData, + currentPermissions: UserOrGroupPermissionEditingData +) => { + // for user or group permissions, unsaved changes is the sum of + // # deleted permissions, # added permissions and # edited permissions + let addedPermissions = 0; + let editedPermissions = 0; + const initialPermissionMap = new Map(); + for (const permission of initialPermissions) { + if (permission.id) { + initialPermissionMap.set(permission.id, permission.modes ?? []); + } + } + + for (const permission of currentPermissions) { + if (!permission.id) { + addedPermissions += 1; // added permissions + } else { + const permissionModes = initialPermissionMap.get(permission.id); + if (!permissionModes) { + addedPermissions += 1; + } else if ( + getPermissionModeId(permissionModes) !== getPermissionModeId(permission.modes ?? []) + ) { + editedPermissions += 1; // added or edited permissions + } + } + } + + // currentPermissions.length = initialPermissions.length + # added permissions - # deleted permissions + const deletedPermissions = + addedPermissions + initialPermissions.length - currentPermissions.length; + + return addedPermissions + editedPermissions + deletedPermissions; +}; + +export const getUnsavedChangesCount = ( + initialFormData: WorkspaceFormEditingData, + currentFormData: WorkspaceFormEditingData +) => { + let unsavedChangesCount = 0; + if (initialFormData.name !== currentFormData.name) { + unsavedChangesCount += 1; + } + // initial and current description could be undefined + const initialDescription = initialFormData.description ?? ''; + const currentDescription = currentFormData.description ?? ''; + if (initialDescription !== currentDescription) { + unsavedChangesCount += 1; + } + if (initialFormData.color !== currentFormData.color) { + unsavedChangesCount += 1; + } + if (initialFormData.icon !== currentFormData.icon) { + unsavedChangesCount += 1; + } + if (initialFormData.defaultVISTheme !== currentFormData.defaultVISTheme) { + unsavedChangesCount += 1; + } + const featureIntersectionCount = ( + initialFormData.features?.filter((feature) => currentFormData.features?.includes(feature)) ?? [] + ).length; + // for features, unsaved changes is the sum of # deleted features and # added features + unsavedChangesCount += (initialFormData.features?.length ?? 0) - featureIntersectionCount; + unsavedChangesCount += (currentFormData.features?.length ?? 0) - featureIntersectionCount; + // for permissions, unsaved changes is the sum of # unsaved user permissions and # unsaved group permissions + const [initialUserPermissions, initialGroupPermissions] = getUserAndGroupPermissions( + initialFormData.permissions ?? [] + ); + const [currentUserPermissions, currentGroupPermissions] = getUserAndGroupPermissions( + currentFormData.permissions ?? [] + ); + unsavedChangesCount += getUnsavedUserOrGroupPermissionChangesCount( + initialUserPermissions, + currentUserPermissions + ); + unsavedChangesCount += getUnsavedUserOrGroupPermissionChangesCount( + initialGroupPermissions, + currentGroupPermissions + ); + return unsavedChangesCount; +}; + +// default permission mode is read +export const getPermissionModeId = (modes: WorkspacePermissionMode[]) => { + for (const key in OptionIdToWorkspacePermissionModesMap) { + if (OptionIdToWorkspacePermissionModesMap[key].every((mode) => modes?.includes(mode))) { + return key; + } + } + return PermissionModeId.Read; +}; diff --git a/src/plugins/workspace/public/components/workspace_creator/workspace_form.tsx b/src/plugins/workspace/public/components/workspace_creator/workspace_form.tsx index 170b04c456dc..21ea647e1501 100644 --- a/src/plugins/workspace/public/components/workspace_creator/workspace_form.tsx +++ b/src/plugins/workspace/public/components/workspace_creator/workspace_form.tsx @@ -30,12 +30,10 @@ import { } from '@elastic/eui'; import { i18n } from '@osd/i18n'; import { - App, AppNavLinkStatus, ApplicationStart, DEFAULT_APP_CATEGORIES, MANAGEMENT_WORKSPACE_ID, - WorkspacePermissionMode, } from '../../../../../core/public'; import { useApplications } from '../../hooks'; import { @@ -50,13 +48,18 @@ import { } from '../utils/feature'; import { WorkspaceBottomBar } from './workspace_bottom_bar'; import { WorkspaceIconSelector } from './workspace_icon_selector'; +import { WorkspacePermissionSettingPanel } from './workspace_permission_setting_panel'; +import { featureMatchesConfig } from '../../utils'; +import { getErrorsCount, getUnsavedChangesCount, isValidWorkspacePermissionSetting } from './utils'; import { - getPermissionModeId, + WorkspaceFeature, + WorkspaceFeatureGroup, + WorkspaceFormData, + WorkspaceFormErrors, + WorkspaceFormSubmitData, WorkspacePermissionSetting, WorkspacePermissionItemType, - WorkspacePermissionSettingPanel, -} from './workspace_permission_setting_panel'; -import { featureMatchesConfig } from '../../utils'; +} from './types'; enum WorkspaceFormTabs { NotSelected, @@ -65,47 +68,10 @@ enum WorkspaceFormTabs { UsersAndPermissions, } -interface WorkspaceFeature extends Pick { - id: string; - name: string; -} - -interface WorkspaceFeatureGroup { - name: string; - features: WorkspaceFeature[]; -} - -export interface WorkspaceFormSubmitData { - name: string; - description?: string; - features?: string[]; - color?: string; - icon?: string; - defaultVISTheme?: string; - permissions: WorkspacePermissionSetting[]; -} - -export interface WorkspaceFormData extends WorkspaceFormSubmitData { - id: string; - reserved?: boolean; -} - -type WorkspaceFormErrors = Omit<{ [key in keyof WorkspaceFormData]?: string }, 'permissions'> & { - permissions?: string[]; -}; - const isWorkspaceFeatureGroup = ( featureOrGroup: WorkspaceFeature | WorkspaceFeatureGroup ): featureOrGroup is WorkspaceFeatureGroup => 'features' in featureOrGroup; -const isValidWorkspacePermissionSetting = ( - setting: Partial -): setting is WorkspacePermissionSetting => - !!setting.modes && - setting.modes.length > 0 && - ((setting.type === WorkspacePermissionItemType.User && !!setting.userId) || - (setting.type === WorkspacePermissionItemType.Group && !!setting.group)); - const isDefaultCheckedFeatureId = (id: string) => { return DEFAULT_CHECKED_FEATURES_IDS.indexOf(id) > -1; }; @@ -123,145 +89,6 @@ const isValidNameOrDescription = (input?: string) => { return regex.test(input); }; -const getErrorsCount = (formErrors: WorkspaceFormErrors) => { - let errorsCount = 0; - if (formErrors.name) { - errorsCount += 1; - } - if (formErrors.description) { - errorsCount += 1; - } - if (formErrors.permissions) { - errorsCount += formErrors.permissions.length; - } - return errorsCount; -}; - -// when editing, attributes could be undefined in workspace form -type WorkspaceFormEditingData = Partial< - Omit & { - permissions: Array>; - } ->; - -type UserOrGroupPermissionEditingData = Array< - Partial<{ id: string; modes: WorkspacePermissionMode[] }> ->; - -const getUserAndGroupPermissions = (permissions: Array>) => { - const userPermissions: UserOrGroupPermissionEditingData = []; - const groupPermissions: UserOrGroupPermissionEditingData = []; - if (permissions) { - for (const permission of permissions) { - if (permission.type === WorkspacePermissionItemType.User) { - userPermissions.push({ id: permission.userId, modes: permission.modes }); - } - if (permission.type === WorkspacePermissionItemType.Group) { - groupPermissions.push({ id: permission.group, modes: permission.modes }); - } - } - } - return [userPermissions, groupPermissions]; -}; - -const getUnsavedUserOrGroupPermissionChangesCount = ( - initialPermissions: UserOrGroupPermissionEditingData, - currentPermissions: UserOrGroupPermissionEditingData -) => { - // for user or group permissions, unsaved changes is the sum of - // # deleted permissions, # added permissions and # edited permissions - let addedPermissions = 0; - let editedPermissions = 0; - const initialPermissionMap = new Map(); - for (const permission of initialPermissions) { - if (permission.id) { - initialPermissionMap.set(permission.id, permission.modes ?? []); - } - } - - for (const permission of currentPermissions) { - if (!permission.id) { - addedPermissions += 1; // added permissions - } else { - const permissionModes = initialPermissionMap.get(permission.id); - if (!permissionModes) { - addedPermissions += 1; - } else if ( - getPermissionModeId(permissionModes) !== getPermissionModeId(permission.modes ?? []) - ) { - editedPermissions += 1; // added or edited permissions - } - } - } - - // currentPermissions.length = initialPermissions.length + # added permissions - # deleted permissions - const deletedPermissions = - addedPermissions + initialPermissions.length - currentPermissions.length; - - return addedPermissions + editedPermissions + deletedPermissions; -}; - -const getUnsavedChangesCount = ( - initialFormData: WorkspaceFormEditingData, - currentFormData: WorkspaceFormEditingData -) => { - let unsavedChangesCount = 0; - if (initialFormData.name !== currentFormData.name) { - unsavedChangesCount += 1; - } - // initial and current description could be undefined - const initialDescription = initialFormData.description ?? ''; - const currentDescription = currentFormData.description ?? ''; - if (initialDescription !== currentDescription) { - unsavedChangesCount += 1; - } - if (initialFormData.color !== currentFormData.color) { - unsavedChangesCount += 1; - } - if (initialFormData.icon !== currentFormData.icon) { - unsavedChangesCount += 1; - } - if (initialFormData.defaultVISTheme !== currentFormData.defaultVISTheme) { - unsavedChangesCount += 1; - } - const featureIntersectionCount = ( - initialFormData.features?.filter((feature) => currentFormData.features?.includes(feature)) ?? [] - ).length; - // for features, unsaved changes is the sum of # deleted features and # added features - unsavedChangesCount += (initialFormData.features?.length ?? 0) - featureIntersectionCount; - unsavedChangesCount += (currentFormData.features?.length ?? 0) - featureIntersectionCount; - // for permissions, unsaved changes is the sum of # unsaved user permissions and # unsaved group permissions - const [initialUserPermissions, initialGroupPermissions] = getUserAndGroupPermissions( - initialFormData.permissions ?? [] - ); - const [currentUserPermissions, currentGroupPermissions] = getUserAndGroupPermissions( - currentFormData.permissions ?? [] - ); - unsavedChangesCount += getUnsavedUserOrGroupPermissionChangesCount( - initialUserPermissions, - currentUserPermissions - ); - unsavedChangesCount += getUnsavedUserOrGroupPermissionChangesCount( - initialGroupPermissions, - currentGroupPermissions - ); - return unsavedChangesCount; -}; - -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) - ); - const workspaceHtmlIdGenerator = htmlIdGenerator(); const defaultVISThemeOptions = [{ value: 'categorical', text: 'Categorical' }]; @@ -819,6 +646,7 @@ export const WorkspaceForm = ({ {featureVisibilityTitle} @@ -827,6 +655,7 @@ export const WorkspaceForm = ({ {workspaceSettingsTitle} @@ -835,6 +664,7 @@ export const WorkspaceForm = ({ {usersAndPermissionsTitle} diff --git a/src/plugins/workspace/public/components/workspace_creator/workspace_permission_setting_panel.tsx b/src/plugins/workspace/public/components/workspace_creator/workspace_permission_setting_panel.tsx index 20c1a6d03815..de400d328d26 100644 --- a/src/plugins/workspace/public/components/workspace_creator/workspace_permission_setting_panel.tsx +++ b/src/plugins/workspace/public/components/workspace_creator/workspace_permission_setting_panel.tsx @@ -17,21 +17,9 @@ import { } from '@elastic/eui'; import { i18n } from '@osd/i18n'; import { WorkspacePermissionMode } from '../../../../../core/public'; - -export enum WorkspacePermissionItemType { - User = 'user', - Group = 'group', -} - -export type WorkspacePermissionSetting = - | { type: WorkspacePermissionItemType.User; userId: string; modes: WorkspacePermissionMode[] } - | { type: WorkspacePermissionItemType.Group; group: string; modes: WorkspacePermissionMode[] }; - -enum PermissionModeId { - Read = 'read', - ReadAndWrite = 'read+write', - Admin = 'admin', -} +import { WorkspacePermissionSetting, WorkspacePermissionItemType } from './types'; +import { PermissionModeId, OptionIdToWorkspacePermissionModesMap } from './constants'; +import { getPermissionModeId } from './utils'; const permissionModeOptions = [ { @@ -57,17 +45,6 @@ const permissionModeOptions = [ }, ]; -const optionIdToWorkspacePermissionModesMap: { - [key: string]: WorkspacePermissionMode[]; -} = { - [PermissionModeId.Read]: [WorkspacePermissionMode.LibraryRead, WorkspacePermissionMode.Read], - [PermissionModeId.ReadAndWrite]: [ - WorkspacePermissionMode.LibraryWrite, - WorkspacePermissionMode.Read, - ], - [PermissionModeId.Admin]: [WorkspacePermissionMode.LibraryWrite, WorkspacePermissionMode.Write], -}; - const generateWorkspacePermissionItemKey = ( item: Partial, index?: number @@ -80,16 +57,6 @@ const generateWorkspacePermissionItemKey = ( index, ].join('-'); -// default permission mode is read -export const getPermissionModeId = (modes: WorkspacePermissionMode[]) => { - for (const key in optionIdToWorkspacePermissionModesMap) { - if (optionIdToWorkspacePermissionModesMap[key].every((mode) => modes?.includes(mode))) { - return key; - } - } - return PermissionModeId.Read; -}; - interface WorkspacePermissionSettingInputProps { index: number; deletable: boolean; @@ -150,8 +117,8 @@ const WorkspacePermissionSettingInput = ({ const handlePermissionModeOptionChange = useCallback( (id: string) => { - if (optionIdToWorkspacePermissionModesMap[id]) { - onPermissionModesChange([...optionIdToWorkspacePermissionModesMap[id]], index); + if (OptionIdToWorkspacePermissionModesMap[id]) { + onPermissionModesChange([...OptionIdToWorkspacePermissionModesMap[id]], index); } }, [index, onPermissionModesChange] @@ -247,11 +214,11 @@ const UserOrGroupSection = ({ * if one settings includes all permission modes in a specific option, * add these permission modes to the result array. */ - for (const key in optionIdToWorkspacePermissionModesMap) { - if (!Object.prototype.hasOwnProperty.call(optionIdToWorkspacePermissionModesMap, key)) { + for (const key in OptionIdToWorkspacePermissionModesMap) { + if (!Object.prototype.hasOwnProperty.call(OptionIdToWorkspacePermissionModesMap, key)) { continue; } - const modesForCertainPermissionId = optionIdToWorkspacePermissionModesMap[key]; + const modesForCertainPermissionId = OptionIdToWorkspacePermissionModesMap[key]; if (modesForCertainPermissionId.every((mode) => valueItem.modes?.includes(mode))) { result.push({ ...valueItem, modes: modesForCertainPermissionId }); } @@ -264,7 +231,7 @@ const UserOrGroupSection = ({ const handleAddNewOne = useCallback(() => { onChange?.([ ...(transformedValue ?? []), - { type, modes: optionIdToWorkspacePermissionModesMap[PermissionModeId.Read] }, + { type, modes: OptionIdToWorkspacePermissionModesMap[PermissionModeId.Read] }, ]); }, [onChange, type, transformedValue]); From ea273dcafa7a58fab457923c70bdb7adc7b8bc5d Mon Sep 17 00:00:00 2001 From: yuye-aws Date: Thu, 26 Oct 2023 14:03:48 +0800 Subject: [PATCH 04/21] bug fix: permission errors count and display Signed-off-by: yuye-aws --- .../components/workspace_creator/types.ts | 34 ++++-- .../workspace_creator/utils.test.ts | 0 .../workspace_creator/workspace_form.tsx | 105 ++++++++---------- .../workspace_permission_setting_panel.tsx | 61 +++++----- 4 files changed, 99 insertions(+), 101 deletions(-) create mode 100644 src/plugins/workspace/public/components/workspace_creator/utils.test.ts diff --git a/src/plugins/workspace/public/components/workspace_creator/types.ts b/src/plugins/workspace/public/components/workspace_creator/types.ts index fef4186aef92..a2550e2a7561 100644 --- a/src/plugins/workspace/public/components/workspace_creator/types.ts +++ b/src/plugins/workspace/public/components/workspace_creator/types.ts @@ -33,25 +33,37 @@ export type WorkspaceFormErrors = Omit< { [key in keyof WorkspaceFormData]?: string }, 'permissions' > & { - permissions?: string[]; + userPermissions?: string[]; + groupPermissions?: string[]; }; +export enum WorkspacePermissionItemType { + User = 'user', + Group = 'group', +} + +export interface UserPermissionSetting { + type: WorkspacePermissionItemType.User; + userId: string; + modes: WorkspacePermissionMode[]; +} + +export interface GroupPermissionSetting { + type: WorkspacePermissionItemType.Group; + group: string; + modes: WorkspacePermissionMode[]; +} + +export type WorkspacePermissionSetting = UserPermissionSetting | GroupPermissionSetting; + // when editing, attributes could be undefined in workspace form export type WorkspaceFormEditingData = Partial< Omit & { - permissions: Array>; + userPermissions: Array>; + groupPermissions: Array>; } >; export type UserOrGroupPermissionEditingData = Array< Partial<{ id: string; modes: WorkspacePermissionMode[] }> >; - -export type WorkspacePermissionSetting = - | { type: WorkspacePermissionItemType.User; userId: string; modes: WorkspacePermissionMode[] } - | { type: WorkspacePermissionItemType.Group; group: string; modes: WorkspacePermissionMode[] }; - -export enum WorkspacePermissionItemType { - User = 'user', - Group = 'group', -} diff --git a/src/plugins/workspace/public/components/workspace_creator/utils.test.ts b/src/plugins/workspace/public/components/workspace_creator/utils.test.ts new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/src/plugins/workspace/public/components/workspace_creator/workspace_form.tsx b/src/plugins/workspace/public/components/workspace_creator/workspace_form.tsx index 21ea647e1501..c5b28870a7be 100644 --- a/src/plugins/workspace/public/components/workspace_creator/workspace_form.tsx +++ b/src/plugins/workspace/public/components/workspace_creator/workspace_form.tsx @@ -50,15 +50,21 @@ import { WorkspaceBottomBar } from './workspace_bottom_bar'; import { WorkspaceIconSelector } from './workspace_icon_selector'; import { WorkspacePermissionSettingPanel } from './workspace_permission_setting_panel'; import { featureMatchesConfig } from '../../utils'; -import { getErrorsCount, getUnsavedChangesCount, isValidWorkspacePermissionSetting } from './utils'; +import { + getErrorsCount, + getPermissionErrors, + getUnsavedChangesCount, + getUserAndGroupPermissions, + isValidWorkspacePermissionSetting, +} from './utils'; import { WorkspaceFeature, WorkspaceFeatureGroup, WorkspaceFormData, WorkspaceFormErrors, WorkspaceFormSubmitData, - WorkspacePermissionSetting, - WorkspacePermissionItemType, + UserPermissionSetting, + GroupPermissionSetting, } from './types'; enum WorkspaceFormTabs { @@ -148,13 +154,17 @@ export const WorkspaceForm = ({ const [selectedFeatureIds, setSelectedFeatureIds] = useState( appendDefaultFeatureIds(defaultFeatures) ); - const [permissionSettings, setPermissionSettings] = useState< - Array> - >( + const [initialUserPermissions, initialGroupPermissions] = getUserAndGroupPermissions( defaultValues?.permissions && defaultValues.permissions.length > 0 ? defaultValues.permissions : [] ); + const [userPermissions, setUserPermissions] = useState>>( + initialUserPermissions + ); + const [groupPermissions, setGroupPermissions] = useState>>( + initialGroupPermissions + ); const libraryCategoryLabel = i18n.translate('core.ui.libraryNavList.label', { defaultMessage: 'Library', @@ -177,7 +187,8 @@ export const WorkspaceForm = ({ color, icon, defaultVISTheme, - permissions: permissionSettings, + userPermissions, + groupPermissions, }); const getFormDataRef = useRef(getFormData); getFormDataRef.current = getFormData; @@ -190,18 +201,20 @@ export const WorkspaceForm = ({ color, icon, defaultVISTheme, - permissions: permissionSettings, + userPermissions, + groupPermissions, }; - return getUnsavedChangesCount(defaultValues ?? {}, currentFormData); + return getUnsavedChangesCount(defaultValues ?? ({} as any), currentFormData); }, [ - defaultValues, name, description, + selectedFeatureIds, color, icon, defaultVISTheme, - selectedFeatureIds, - permissionSettings, + userPermissions, + groupPermissions, + defaultValues, ]); const featureOrGroups = useMemo(() => { @@ -364,49 +377,18 @@ export const WorkspaceForm = ({ }), }; } - const permissionErrors: string[] = new Array(formData.permissions.length); - for (let i = 0; i < formData.permissions.length; i++) { - const permission = formData.permissions[i]; - if (isValidWorkspacePermissionSetting(permission)) { - if ( - isUserOrGroupPermissionSettingDuplicated(formData.permissions.slice(0, i), permission) - ) { - permissionErrors[i] = i18n.translate('workspace.form.permission.invalidate.group', { - defaultMessage: 'Duplicate permission setting', - }); - continue; - } - continue; - } - if (!permission.type) { - permissionErrors[i] = i18n.translate('workspace.form.permission.invalidate.type', { - defaultMessage: 'Invalid type', - }); - continue; - } - if (!permission.modes || permission.modes.length === 0) { - permissionErrors[i] = i18n.translate('workspace.form.permission.invalidate.modes', { - defaultMessage: 'Invalid permission modes', - }); - continue; - } - if (permission.type === WorkspacePermissionItemType.User && !permission.userId) { - permissionErrors[i] = i18n.translate('workspace.form.permission.invalidate.userId', { - defaultMessage: 'Invalid userId', - }); - continue; - } - if (permission.type === WorkspacePermissionItemType.Group && !permission.group) { - permissionErrors[i] = i18n.translate('workspace.form.permission.invalidate.group', { - defaultMessage: 'Invalid user group', - }); - continue; // this line is need for more conditions - } + const userPermissionErrors = getPermissionErrors(formData.userPermissions); + const groupPermissionErrors = getPermissionErrors(formData.groupPermissions); + if (userPermissionErrors.some((error) => !!error)) { + currentFormErrors = { + ...currentFormErrors, + userPermissions: userPermissionErrors, + }; } - if (permissionErrors.some((error) => !!error)) { + if (groupPermissionErrors.some((error) => !!error)) { currentFormErrors = { ...currentFormErrors, - permissions: permissionErrors, + groupPermissions: groupPermissionErrors, }; } const currentErrorsCount = getErrorsCount(currentFormErrors); @@ -430,8 +412,14 @@ export const WorkspaceForm = ({ formData.features = defaultValues?.features ?? []; } - const permissions = formData.permissions.filter(isValidWorkspacePermissionSetting); - onSubmit?.({ ...formData, name: formData.name!, permissions }); + onSubmit?.({ + ...formData, + name: formData.name!, + permissions: [ + ...formData.userPermissions.filter(isValidWorkspacePermissionSetting), + ...formData.groupPermissions.filter(isValidWorkspacePermissionSetting), + ] as any, + }); }, [defaultFeatures, onSubmit, defaultValues?.features] ); @@ -760,9 +748,12 @@ export const WorkspaceForm = ({ diff --git a/src/plugins/workspace/public/components/workspace_creator/workspace_permission_setting_panel.tsx b/src/plugins/workspace/public/components/workspace_creator/workspace_permission_setting_panel.tsx index de400d328d26..1e8ac23bd0e5 100644 --- a/src/plugins/workspace/public/components/workspace_creator/workspace_permission_setting_panel.tsx +++ b/src/plugins/workspace/public/components/workspace_creator/workspace_permission_setting_panel.tsx @@ -3,7 +3,7 @@ * SPDX-License-Identifier: Apache-2.0 */ -import React, { useCallback, useMemo, useState, useEffect } from 'react'; +import React, { useCallback, useMemo } from 'react'; import { EuiFlexGroup, EuiComboBox, @@ -17,7 +17,12 @@ import { } from '@elastic/eui'; import { i18n } from '@osd/i18n'; import { WorkspacePermissionMode } from '../../../../../core/public'; -import { WorkspacePermissionSetting, WorkspacePermissionItemType } from './types'; +import { + UserPermissionSetting, + GroupPermissionSetting, + WorkspacePermissionSetting, + WorkspacePermissionItemType, +} from './types'; import { PermissionModeId, OptionIdToWorkspacePermissionModesMap } from './constants'; import { getPermissionModeId } from './utils'; @@ -167,17 +172,22 @@ const WorkspacePermissionSettingInput = ({ }; interface WorkspacePermissionSettingPanelProps { - errors?: string[]; - lastAdminItemDeletable: boolean; - permissionSettings: Array>; - onChange?: (value: Array>) => void; + userErrors?: string[]; + groupErrors?: string[]; + lastAdminItemDeletable?: boolean; + userPermissionSettings: Array>; + groupPermissionSettings: Array>; + onUserPermissionChange: (value: Array>) => void; + onGroupPermissionChange: (value: Array>) => void; } -interface UserOrGroupSectionProps - extends Omit { +interface UserOrGroupSectionProps { title: string; + errors?: string[]; nonDeletableIndex: number; type: WorkspacePermissionItemType; + permissionSettings: Array>; + onChange: (value: Array>) => void; } const UserOrGroupSection = ({ @@ -307,29 +317,14 @@ const UserOrGroupSection = ({ }; export const WorkspacePermissionSettingPanel = ({ - errors, - onChange, - permissionSettings, + userErrors, + groupErrors, + onUserPermissionChange, + onGroupPermissionChange, + userPermissionSettings, + groupPermissionSettings, lastAdminItemDeletable, }: WorkspacePermissionSettingPanelProps) => { - const [userPermissionSettings, setUserPermissionSettings] = useState< - Array> - >( - permissionSettings?.filter( - (permissionSettingItem) => permissionSettingItem.type === WorkspacePermissionItemType.User - ) ?? [] - ); - const [groupPermissionSettings, setGroupPermissionSettings] = useState< - Array> - >( - permissionSettings?.filter( - (permissionSettingItem) => permissionSettingItem.type === WorkspacePermissionItemType.Group - ) ?? [] - ); - - useEffect(() => { - onChange?.([...userPermissionSettings, ...groupPermissionSettings]); - }, [onChange, userPermissionSettings, groupPermissionSettings]); const nonDeletableIndex = useMemo(() => { let userNonDeletableIndex = -1; @@ -362,8 +357,8 @@ export const WorkspacePermissionSettingPanel = ({ title={i18n.translate('workspace.form.permissionSettingPanel.userTitle', { defaultMessage: 'User', })} - errors={errors} - onChange={setUserPermissionSettings} + errors={userErrors} + onChange={onUserPermissionChange as any} nonDeletableIndex={userNonDeletableIndex} permissionSettings={userPermissionSettings} type={WorkspacePermissionItemType.User} @@ -373,8 +368,8 @@ export const WorkspacePermissionSettingPanel = ({ title={i18n.translate('workspace.form.permissionSettingPanel.userGroupTitle', { defaultMessage: 'User Groups', })} - errors={errors} - onChange={setGroupPermissionSettings} + errors={groupErrors} + onChange={onGroupPermissionChange as any} nonDeletableIndex={groupNonDeletableIndex} permissionSettings={groupPermissionSettings} type={WorkspacePermissionItemType.Group} From 6b91c94d631f7d52861a3e314eb5297a8d854db6 Mon Sep 17 00:00:00 2001 From: yuye-aws Date: Thu, 26 Oct 2023 14:59:18 +0800 Subject: [PATCH 05/21] add test for utils Signed-off-by: yuye-aws --- .../components/workspace_creator/index.tsx | 1 - .../workspace_creator/utils.test.ts | 228 +++++++++++++++ .../components/workspace_creator/utils.ts | 67 +++-- .../workspace_bottom_bar.tsx | 8 +- .../workspace_updater.test.tsx | 265 ++++++++++++++++++ 5 files changed, 551 insertions(+), 18 deletions(-) diff --git a/src/plugins/workspace/public/components/workspace_creator/index.tsx b/src/plugins/workspace/public/components/workspace_creator/index.tsx index 83d87debad6e..c8cdbfab65be 100644 --- a/src/plugins/workspace/public/components/workspace_creator/index.tsx +++ b/src/plugins/workspace/public/components/workspace_creator/index.tsx @@ -4,4 +4,3 @@ */ export { WorkspaceCreator } from './workspace_creator'; -export { WorkspacePermissionSetting } from './workspace_permission_setting_panel'; diff --git a/src/plugins/workspace/public/components/workspace_creator/utils.test.ts b/src/plugins/workspace/public/components/workspace_creator/utils.test.ts index e69de29bb2d1..132e984b8e78 100644 --- a/src/plugins/workspace/public/components/workspace_creator/utils.test.ts +++ b/src/plugins/workspace/public/components/workspace_creator/utils.test.ts @@ -0,0 +1,228 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +import { + isValidWorkspacePermissionSetting, + getErrorsCount, + getUserAndGroupPermissions, + getUnsavedChangesCount, + getPermissionModeId, + getPermissionErrors, +} from './utils'; +import { + UserPermissionSetting, + WorkspaceFormErrors, + WorkspacePermissionItemType, + WorkspacePermissionSetting, +} from './types'; +import { WorkspacePermissionMode } from '../../../../../core/public'; +import { PermissionModeId } from './constants'; + +describe('isValidWorkspacePermissionSetting', () => { + it('should return true with valid user permission setting', () => { + expect( + isValidWorkspacePermissionSetting({ + type: WorkspacePermissionItemType.User, + userId: 'test user id', + modes: [WorkspacePermissionMode.Write, WorkspacePermissionMode.LibraryWrite], + }) + ).toBe(true); + }); + + it('should return true with valid group permission setting', () => { + expect( + isValidWorkspacePermissionSetting({ + type: WorkspacePermissionItemType.Group, + group: 'test group id', + modes: [WorkspacePermissionMode.Write, WorkspacePermissionMode.LibraryWrite], + }) + ).toBe(true); + }); + + it('should return false with empty permission modes', () => { + expect( + isValidWorkspacePermissionSetting({ + type: WorkspacePermissionItemType.User, + userId: 'test user id', + modes: [], + }) + ).toBe(false); + }); + + it('should return false with incorrect permission type (expect user)', () => { + expect( + isValidWorkspacePermissionSetting({ + type: WorkspacePermissionItemType.Group, + userId: 'test user id', + modes: [], + } as any) + ).toBe(false); + }); + + it('should return false with incorrect permission type 2 (expect group)', () => { + expect( + isValidWorkspacePermissionSetting({ + type: WorkspacePermissionItemType.User, + group: 'test user id', + modes: [], + } as any) + ).toBe(false); + }); +}); + +describe('getErrorsCount', () => { + it('should return error count in name, description and permissions', () => { + const workspaceFormErrors: WorkspaceFormErrors = { + name: 'test name error', + description: 'test description error', + userPermissions: ['test user permission error 1'], + groupPermissions: ['test group permission error 1', 'test group permission error 2'], + }; + expect(getErrorsCount(workspaceFormErrors)).toBe(5); + }); +}); + +describe('getUserAndGroupPermissions', () => { + it('should split user and group permissions from all permissions', () => { + const permissions = [ + { modes: [] }, + { userId: 'test user id 1' }, + { group: 'test group id 1' }, + { type: WorkspacePermissionItemType.User, userId: 'test user id 2', modes: [] }, + { type: WorkspacePermissionItemType.Group, group: 'test group id 2', modes: [] }, + { type: WorkspacePermissionItemType.Group, group: 'test group id 3', modes: [] }, + ]; + expect(getUserAndGroupPermissions(permissions)).toStrictEqual([ + [{ id: 'test user id 2', modes: [] }], + [ + { id: 'test group id 2', modes: [] }, + { id: 'test group id 3', modes: [] }, + ], + ]); + }); +}); + +describe('getUnsavedChangesCount', () => { + it('should return number of unsaved changes in workspace metadata', () => { + const initialFormData = { + id: 'test workspace id', + name: 'test workspace name', + description: 'test workspace description', + permissions: [], + }; + const currentFormData = { + name: 'changed workspace name', + description: 'changed workspace description', + }; + expect(getUnsavedChangesCount(initialFormData, currentFormData)).toBe(2); + }); + + it('should return number of unsaved changes in workspace features', () => { + const initialFormData = { + id: 'test workspace id', + name: 'test workspace name', + permissions: [], + features: ['feature 1', 'feature 2', 'feature 3'], + }; + const currentFormData = { + name: 'test workspace name', + features: ['feature 1', 'feature 2', 'feature 4'], + }; + // 1 deleted feature and 1 added feature + expect(getUnsavedChangesCount(initialFormData, currentFormData)).toBe(2); + }); + + it('should return number of unsaved changes in workspace permissions', () => { + const initialFormData = { + id: 'test workspace id', + name: 'test workspace name', + permissions: [ + { + userId: 'test user id 1', + type: WorkspacePermissionItemType.User, + modes: [WorkspacePermissionMode.LibraryRead, WorkspacePermissionMode.Read], + }, + { + group: 'test group id', + type: WorkspacePermissionItemType.Group, + modes: [WorkspacePermissionMode.LibraryRead, WorkspacePermissionMode.Read], + }, + ] as WorkspacePermissionSetting[], + }; + const currentFormData = { + name: 'test workspace name', + userPermissions: [ + { + userId: 'test user id 1', + type: WorkspacePermissionItemType.User, + modes: [WorkspacePermissionMode.LibraryWrite, WorkspacePermissionMode.Write], + }, + ] as UserPermissionSetting[], + groupPermissions: [], + }; + // 1 deleted permission and 1 edited permission + expect(getUnsavedChangesCount(initialFormData, currentFormData)).toBe(false); + }); +}); + +describe('getPermissionModeId', () => { + it('should return Read with empty permission', () => { + expect(getPermissionModeId([])).toBe(PermissionModeId.Read); + }); + + it('should return Read with [LibraryRead, Read]', () => { + expect( + getPermissionModeId([WorkspacePermissionMode.LibraryRead, WorkspacePermissionMode.Read]) + ).toBe(PermissionModeId.Read); + }); + + it('should return ReadAndWrite with [LibraryWrite, Read],', () => { + expect( + getPermissionModeId([WorkspacePermissionMode.LibraryWrite, WorkspacePermissionMode.Read]) + ).toBe(PermissionModeId.ReadAndWrite); + }); + + it('should return ReadAndWrite with [LibraryWrite, Read]', () => { + expect( + getPermissionModeId([WorkspacePermissionMode.LibraryWrite, WorkspacePermissionMode.Write]) + ).toBe(PermissionModeId.Admin); + }); +}); + +describe('getPermissionErrors', () => { + it('should get permission errors for both users and groups', () => { + const permissions = [ + {}, + { type: WorkspacePermissionItemType.User }, + { + type: WorkspacePermissionItemType.User, + modes: [WorkspacePermissionMode.LibraryRead, WorkspacePermissionMode.Read], + }, + { + type: WorkspacePermissionItemType.Group, + modes: [WorkspacePermissionMode.LibraryRead, WorkspacePermissionMode.Read], + }, + { + userId: 'test user id', + type: WorkspacePermissionItemType.User, + modes: [WorkspacePermissionMode.LibraryRead, WorkspacePermissionMode.Read], + }, + { + group: 'test group id', + type: WorkspacePermissionItemType.Group, + modes: [WorkspacePermissionMode.LibraryRead, WorkspacePermissionMode.Read], + }, + ]; + const expectedPermissionErrors = [ + 'Invalid type', + 'Invalid permission modes', + 'Invalid userId', + 'Invalid user group', + ]; + expect(getPermissionErrors(permissions)).toEqual( + expect.arrayContaining(expectedPermissionErrors) + ); + }); +}); diff --git a/src/plugins/workspace/public/components/workspace_creator/utils.ts b/src/plugins/workspace/public/components/workspace_creator/utils.ts index c815b1f894d0..684195d11524 100644 --- a/src/plugins/workspace/public/components/workspace_creator/utils.ts +++ b/src/plugins/workspace/public/components/workspace_creator/utils.ts @@ -3,6 +3,7 @@ * SPDX-License-Identifier: Apache-2.0 */ +import { i18n } from '@osd/i18n'; import { WorkspacePermissionMode } from '../../../../../core/public'; import { UserOrGroupPermissionEditingData, @@ -10,6 +11,7 @@ import { WorkspaceFormErrors, WorkspacePermissionSetting, WorkspacePermissionItemType, + WorkspaceFormData, } from './types'; import { OptionIdToWorkspacePermissionModesMap, PermissionModeId } from './constants'; @@ -29,8 +31,11 @@ export const getErrorsCount = (formErrors: WorkspaceFormErrors) => { if (formErrors.description) { errorsCount += 1; } - if (formErrors.permissions) { - errorsCount += formErrors.permissions.length; + if (formErrors.userPermissions) { + errorsCount += formErrors.userPermissions.filter((permission) => !!permission).length; + } + if (formErrors.groupPermissions) { + errorsCount += formErrors.groupPermissions.filter((permission) => !!permission).length; } return errorsCount; }; @@ -39,14 +44,12 @@ export const getUserAndGroupPermissions = ( ) => { const userPermissions: UserOrGroupPermissionEditingData = []; const groupPermissions: UserOrGroupPermissionEditingData = []; - if (permissions) { - for (const permission of permissions) { - if (permission.type === WorkspacePermissionItemType.User) { - userPermissions.push({ id: permission.userId, modes: permission.modes }); - } - if (permission.type === WorkspacePermissionItemType.Group) { - groupPermissions.push({ id: permission.group, modes: permission.modes }); - } + for (const permission of permissions) { + if (permission.type === WorkspacePermissionItemType.User) { + userPermissions.push({ id: permission.userId, modes: permission.modes }); + } + if (permission.type === WorkspacePermissionItemType.Group) { + groupPermissions.push({ id: permission.group, modes: permission.modes }); } } return [userPermissions, groupPermissions]; @@ -90,7 +93,7 @@ const getUnsavedUserOrGroupPermissionChangesCount = ( }; export const getUnsavedChangesCount = ( - initialFormData: WorkspaceFormEditingData, + initialFormData: WorkspaceFormData, currentFormData: WorkspaceFormEditingData ) => { let unsavedChangesCount = 0; @@ -122,16 +125,13 @@ export const getUnsavedChangesCount = ( const [initialUserPermissions, initialGroupPermissions] = getUserAndGroupPermissions( initialFormData.permissions ?? [] ); - const [currentUserPermissions, currentGroupPermissions] = getUserAndGroupPermissions( - currentFormData.permissions ?? [] - ); unsavedChangesCount += getUnsavedUserOrGroupPermissionChangesCount( initialUserPermissions, - currentUserPermissions + currentFormData.userPermissions ?? [] ); unsavedChangesCount += getUnsavedUserOrGroupPermissionChangesCount( initialGroupPermissions, - currentGroupPermissions + currentFormData.groupPermissions ?? [] ); return unsavedChangesCount; }; @@ -145,3 +145,38 @@ export const getPermissionModeId = (modes: WorkspacePermissionMode[]) => { } return PermissionModeId.Read; }; + +export const getPermissionErrors = (permissions: Array>) => { + const permissionErrors: string[] = new Array(permissions.length); + for (let i = 0; i < permissions.length; i++) { + const permission = permissions[i]; + if (isValidWorkspacePermissionSetting(permission)) { + continue; + } + if (!permission.type) { + permissionErrors[i] = i18n.translate('workspace.form.permission.invalidate.type', { + defaultMessage: 'Invalid type', + }); + continue; + } + if (!permission.modes || permission.modes.length === 0) { + permissionErrors[i] = i18n.translate('workspace.form.permission.invalidate.modes', { + defaultMessage: 'Invalid permission modes', + }); + continue; + } + if (permission.type === WorkspacePermissionItemType.User && !permission.userId) { + permissionErrors[i] = i18n.translate('workspace.form.permission.invalidate.userId', { + defaultMessage: 'Invalid userId', + }); + continue; + } + if (permission.type === WorkspacePermissionItemType.Group && !permission.group) { + permissionErrors[i] = i18n.translate('workspace.form.permission.invalidate.group', { + defaultMessage: 'Invalid user group', + }); + continue; // this line is need for more conditions + } + } + return permissionErrors; +}; diff --git a/src/plugins/workspace/public/components/workspace_creator/workspace_bottom_bar.tsx b/src/plugins/workspace/public/components/workspace_creator/workspace_bottom_bar.tsx index aa248ea45c44..d889d844bce6 100644 --- a/src/plugins/workspace/public/components/workspace_creator/workspace_bottom_bar.tsx +++ b/src/plugins/workspace/public/components/workspace_creator/workspace_bottom_bar.tsx @@ -87,7 +87,13 @@ export const WorkspaceBottomBar = ({ )} {opType === WORKSPACE_OP_TYPE_UPDATE && ( - + {i18n.translate('workspace.form.bottomBar.saveChanges', { defaultMessage: 'Save changes', })} 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 e69de29bb2d1..9cd97e6e507b 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 @@ -0,0 +1,265 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +import React from 'react'; +import { PublicAppInfo, WorkspaceAttribute } from 'opensearch-dashboards/public'; +import { fireEvent, render, waitFor } from '@testing-library/react'; +import { BehaviorSubject, of } from 'rxjs'; +import { WorkspaceUpdater as WorkspaceUpdaterComponent } from './workspace_updater'; +import { coreMock } from '../../../../../core/public/mocks'; +import { createOpenSearchDashboardsReactContext } from '../../../../opensearch_dashboards_react/public'; +import { + WORKSPACE_UPDATE_APP_ID, + WORKSPACE_OVERVIEW_APP_ID, + DEFAULT_CHECKED_FEATURES_IDS, +} from '../../../common/constants'; + +const workspaceClientUpdate = jest + .fn() + .mockReturnValue({ result: { id: 'successResult' }, success: true }); + +const navigateToApp = jest.fn(); +const notificationToastsAddSuccess = jest.fn(); +const notificationToastsAddDanger = jest.fn(); + +// upon initialization on workspace settings page, two features (workspace overview and workspace settings) are automatically checked +const PublicAPPInfoMap = new Map([ + ['app1', { id: 'app1', title: 'app1' }], + ['app2', { id: 'app2', title: 'app2', category: { id: 'category1', label: 'category1' } }], + ['app3', { id: 'app3', category: { id: 'category1', label: 'category1' } }], + ['app4', { id: 'app4', category: { id: 'category2', label: 'category2' } }], + ['app5', { id: 'app5', category: { id: 'category2', label: 'category2' } }], + [WORKSPACE_UPDATE_APP_ID, { id: WORKSPACE_UPDATE_APP_ID, title: 'Workspace Settings' }], + [WORKSPACE_OVERVIEW_APP_ID, { id: WORKSPACE_OVERVIEW_APP_ID, title: 'Overview' }], +]); + +const mockCoreStart = coreMock.createStart(); + +const WorkspaceUpdater = (props: any) => { + const { Provider } = createOpenSearchDashboardsReactContext({ + ...mockCoreStart, + ...{ + application: { + ...mockCoreStart.application, + capabilities: { + ...mockCoreStart.application.capabilities, + workspaces: { + permissionEnabled: true, + }, + }, + navigateToApp, + getUrlForApp: jest.fn(), + applications$: new BehaviorSubject>(PublicAPPInfoMap as any), + }, + http: { + ...mockCoreStart.http, + basePath: { + ...mockCoreStart.http.basePath, + remove: jest.fn(), + prepend: jest.fn(), + }, + }, + notifications: { + ...mockCoreStart.notifications, + toasts: { + ...mockCoreStart.notifications.toasts, + addDanger: notificationToastsAddDanger, + addSuccess: notificationToastsAddSuccess, + }, + }, + workspaceClient: { + update: workspaceClientUpdate, + }, + workspaces: { + ...mockCoreStart.workspaces, + currentWorkspace$: new BehaviorSubject({ + id: 'test workspace id', + name: 'test workspace name', + features: DEFAULT_CHECKED_FEATURES_IDS, + }), + }, + }, + }); + + return ( + + + + ); +}; + +function clearMockedFunctions() { + workspaceClientUpdate.mockClear(); + notificationToastsAddDanger.mockClear(); + notificationToastsAddSuccess.mockClear(); +} + +describe('WorkspaceUpdater', () => { + beforeEach(() => clearMockedFunctions()); + const { location } = window; + const setHrefSpy = jest.fn((href) => href); + + beforeAll(() => { + if (window.location) { + // @ts-ignore + delete window.location; + } + window.location = {} as Location; + Object.defineProperty(window.location, 'href', { + get: () => 'http://localhost/', + set: setHrefSpy, + }); + }); + + afterAll(() => { + window.location = location; + }); + + it('cannot update workspace when name empty', async () => { + const { getByTestId } = render(); + fireEvent.click(getByTestId('workspaceForm-tabSelection-workspaceSettings')); + const nameInput = getByTestId('workspaceForm-workspaceDetails-nameInputText'); + fireEvent.input(nameInput, { + target: { value: '' }, + }); + fireEvent.click(getByTestId('workspaceForm-bottomBar-updateButton')); + expect(workspaceClientUpdate).not.toHaveBeenCalled(); + }); + + it('cannot update workspace with invalid name', async () => { + const { getByTestId } = render(); + fireEvent.click(getByTestId('workspaceForm-tabSelection-workspaceSettings')); + const nameInput = getByTestId('workspaceForm-workspaceDetails-nameInputText'); + fireEvent.input(nameInput, { + target: { value: '~' }, + }); + expect(workspaceClientUpdate).not.toHaveBeenCalled(); + }); + + it('cannot create workspace with invalid description', async () => { + const { getByTestId } = render(); + fireEvent.click(getByTestId('workspaceForm-tabSelection-workspaceSettings')); + const descriptionInput = getByTestId('workspaceForm-workspaceDetails-descriptionInputText'); + fireEvent.input(descriptionInput, { + target: { value: '~' }, + }); + expect(workspaceClientUpdate).not.toHaveBeenCalled(); + }); + + it('cancel update workspace', async () => { + const { findByText, getByTestId } = render(); + fireEvent.click(getByTestId('workspaceForm-bottomBar-cancelButton')); + await findByText('Discard changes?'); + fireEvent.click(getByTestId('confirmModalConfirmButton')); + expect(navigateToApp).toHaveBeenCalled(); + }); + + /* + it('create workspace with detailed information', async () => { + const { getByTestId } = render(); + const nameInput = getByTestId('workspaceForm-workspaceDetails-nameInputText'); + fireEvent.input(nameInput, { + target: { value: 'test workspace name' }, + }); + const descriptionInput = getByTestId('workspaceForm-workspaceDetails-descriptionInputText'); + fireEvent.input(descriptionInput, { + target: { value: 'test workspace description' }, + }); + const colorSelector = getByTestId( + 'euiColorPickerAnchor workspaceForm-workspaceDetails-colorPicker' + ); + fireEvent.input(colorSelector, { + target: { value: '#000000' }, + }); + const iconSelector = getByTestId('workspaceForm-workspaceDetails-iconSelector'); + fireEvent.click(iconSelector); + fireEvent.click(getByTestId('workspaceForm-workspaceDetails-iconSelector-Glasses')); + const defaultVISThemeSelector = getByTestId( + 'workspaceForm-workspaceDetails-defaultVISThemeSelector' + ); + fireEvent.click(defaultVISThemeSelector); + fireEvent.change(defaultVISThemeSelector, { target: { value: 'categorical' } }); + fireEvent.click(getByTestId('workspaceForm-bottomBar-createButton')); + expect(workspaceClientCreate).toHaveBeenCalledWith( + expect.objectContaining({ + name: 'test workspace name', + icon: 'Glasses', + color: '#000000', + description: 'test workspace description', + defaultVISTheme: 'categorical', + }) + ); + await waitFor(() => { + expect(notificationToastsAddSuccess).toHaveBeenCalled(); + }); + expect(notificationToastsAddDanger).not.toHaveBeenCalled(); + }); + + it('create workspace with customized features', async () => { + const { getByTestId } = render(); + const nameInput = getByTestId('workspaceForm-workspaceDetails-nameInputText'); + fireEvent.input(nameInput, { + target: { value: 'test workspace name' }, + }); + fireEvent.click(getByTestId('workspaceForm-workspaceFeatureVisibility-app1')); + fireEvent.click(getByTestId('workspaceForm-workspaceFeatureVisibility-category1')); + fireEvent.click(getByTestId('workspaceForm-bottomBar-createButton')); + expect(workspaceClientCreate).toHaveBeenCalledWith( + expect.objectContaining({ + name: 'test workspace name', + features: expect.arrayContaining(['app1', 'app2', 'app3']), + }) + ); + await waitFor(() => { + expect(notificationToastsAddSuccess).toHaveBeenCalled(); + }); + expect(notificationToastsAddDanger).not.toHaveBeenCalled(); + }); + + it('create workspace with customized permissions', async () => { + const { getByTestId, getByText } = render(); + const nameInput = getByTestId('workspaceForm-workspaceDetails-nameInputText'); + fireEvent.input(nameInput, { + target: { value: 'test workspace name' }, + }); + fireEvent.click(getByText('Users & Permissions')); + fireEvent.click(getByTestId('workspaceForm-permissionSettingPanel-user-addNew')); + const userIdInput = getByTestId('workspaceForm-permissionSettingPanel-0-userId'); + 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', + permissions: expect.arrayContaining([ + expect.objectContaining({ type: 'user', userId: 'test user id' }), + ]), + }) + ); + await waitFor(() => { + expect(notificationToastsAddSuccess).toHaveBeenCalled(); + }); + expect(notificationToastsAddDanger).not.toHaveBeenCalled(); + }); + + it('should show danger toasts after create workspace failed', async () => { + workspaceClientCreate.mockReturnValue({ result: { id: 'failResult' }, success: false }); + const { getByTestId } = render(); + const nameInput = getByTestId('workspaceForm-workspaceDetails-nameInputText'); + fireEvent.input(nameInput, { + target: { value: 'test workspace name' }, + }); + fireEvent.click(getByTestId('workspaceForm-bottomBar-createButton')); + expect(workspaceClientCreate).toHaveBeenCalled(); + await waitFor(() => { + expect(notificationToastsAddDanger).toHaveBeenCalled(); + }); + expect(notificationToastsAddSuccess).not.toHaveBeenCalled(); + }); + */ +}); From 1ae2330cebe1d24320b5405c9301ff239b362569 Mon Sep 17 00:00:00 2001 From: yuye-aws Date: Thu, 26 Oct 2023 15:05:52 +0800 Subject: [PATCH 06/21] refactor: move permission constants outside Signed-off-by: yuye-aws --- src/core/public/index.ts | 2 ++ src/core/utils/constants.ts | 17 ++++++++++++++ src/core/utils/index.ts | 2 ++ .../components/workspace_creator/constants.ts | 23 ------------------- .../workspace_creator/utils.test.ts | 3 +-- .../components/workspace_creator/utils.ts | 7 ++++-- .../workspace_creator/workspace_creator.tsx | 3 ++- .../workspace_permission_setting_panel.tsx | 7 ++++-- 8 files changed, 34 insertions(+), 30 deletions(-) delete mode 100644 src/plugins/workspace/public/components/workspace_creator/constants.ts diff --git a/src/core/public/index.ts b/src/core/public/index.ts index 08000926d7aa..7ecc5c1dc9fe 100644 --- a/src/core/public/index.ts +++ b/src/core/public/index.ts @@ -354,6 +354,8 @@ export { WorkspacesStart, WorkspacesSetup, WorkspacesService } from './workspace export { WorkspacePermissionMode, + OptionIdToWorkspacePermissionModesMap, + PermissionModeId, PUBLIC_WORKSPACE_ID, MANAGEMENT_WORKSPACE_ID, WORKSPACE_TYPE, diff --git a/src/core/utils/constants.ts b/src/core/utils/constants.ts index 1eecb44ae96f..061f1d151e38 100644 --- a/src/core/utils/constants.ts +++ b/src/core/utils/constants.ts @@ -14,6 +14,23 @@ export enum WorkspacePermissionMode { LibraryWrite = 'library_write', } +export enum PermissionModeId { + Read = 'read', + ReadAndWrite = 'read+write', + Admin = 'admin', +} + +export const OptionIdToWorkspacePermissionModesMap: { + [key: string]: WorkspacePermissionMode[]; +} = { + [PermissionModeId.Read]: [WorkspacePermissionMode.LibraryRead, WorkspacePermissionMode.Read], + [PermissionModeId.ReadAndWrite]: [ + WorkspacePermissionMode.LibraryWrite, + WorkspacePermissionMode.Read, + ], + [PermissionModeId.Admin]: [WorkspacePermissionMode.LibraryWrite, WorkspacePermissionMode.Write], +}; + export const PUBLIC_WORKSPACE_ID = 'public'; export const MANAGEMENT_WORKSPACE_ID = 'management'; diff --git a/src/core/utils/index.ts b/src/core/utils/index.ts index f2884c60581c..768bdc36a3c1 100644 --- a/src/core/utils/index.ts +++ b/src/core/utils/index.ts @@ -40,6 +40,8 @@ export { DEFAULT_APP_CATEGORIES } from './default_app_categories'; export { WORKSPACE_PATH_PREFIX, WorkspacePermissionMode, + PermissionModeId, + OptionIdToWorkspacePermissionModesMap, PUBLIC_WORKSPACE_ID, MANAGEMENT_WORKSPACE_ID, WORKSPACE_TYPE, diff --git a/src/plugins/workspace/public/components/workspace_creator/constants.ts b/src/plugins/workspace/public/components/workspace_creator/constants.ts deleted file mode 100644 index a5e31ed9b8f0..000000000000 --- a/src/plugins/workspace/public/components/workspace_creator/constants.ts +++ /dev/null @@ -1,23 +0,0 @@ -/* - * Copyright OpenSearch Contributors - * SPDX-License-Identifier: Apache-2.0 - */ - -import { WorkspacePermissionMode } from '../../../../../core/public'; - -export enum PermissionModeId { - Read = 'read', - ReadAndWrite = 'read+write', - Admin = 'admin', -} - -export const OptionIdToWorkspacePermissionModesMap: { - [key: string]: WorkspacePermissionMode[]; -} = { - [PermissionModeId.Read]: [WorkspacePermissionMode.LibraryRead, WorkspacePermissionMode.Read], - [PermissionModeId.ReadAndWrite]: [ - WorkspacePermissionMode.LibraryWrite, - WorkspacePermissionMode.Read, - ], - [PermissionModeId.Admin]: [WorkspacePermissionMode.LibraryWrite, WorkspacePermissionMode.Write], -}; diff --git a/src/plugins/workspace/public/components/workspace_creator/utils.test.ts b/src/plugins/workspace/public/components/workspace_creator/utils.test.ts index 132e984b8e78..6d4128885250 100644 --- a/src/plugins/workspace/public/components/workspace_creator/utils.test.ts +++ b/src/plugins/workspace/public/components/workspace_creator/utils.test.ts @@ -17,8 +17,7 @@ import { WorkspacePermissionItemType, WorkspacePermissionSetting, } from './types'; -import { WorkspacePermissionMode } from '../../../../../core/public'; -import { PermissionModeId } from './constants'; +import { WorkspacePermissionMode, PermissionModeId } from '../../../../../core/public'; describe('isValidWorkspacePermissionSetting', () => { it('should return true with valid user permission setting', () => { diff --git a/src/plugins/workspace/public/components/workspace_creator/utils.ts b/src/plugins/workspace/public/components/workspace_creator/utils.ts index 684195d11524..6e4e36f4b93a 100644 --- a/src/plugins/workspace/public/components/workspace_creator/utils.ts +++ b/src/plugins/workspace/public/components/workspace_creator/utils.ts @@ -4,7 +4,11 @@ */ import { i18n } from '@osd/i18n'; -import { WorkspacePermissionMode } from '../../../../../core/public'; +import { + WorkspacePermissionMode, + OptionIdToWorkspacePermissionModesMap, + PermissionModeId, +} from '../../../../../core/public'; import { UserOrGroupPermissionEditingData, WorkspaceFormEditingData, @@ -13,7 +17,6 @@ import { WorkspacePermissionItemType, WorkspaceFormData, } from './types'; -import { OptionIdToWorkspacePermissionModesMap, PermissionModeId } from './constants'; export const isValidWorkspacePermissionSetting = ( setting: Partial 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 22663aad1dc0..df2c315f8772 100644 --- a/src/plugins/workspace/public/components/workspace_creator/workspace_creator.tsx +++ b/src/plugins/workspace/public/components/workspace_creator/workspace_creator.tsx @@ -7,7 +7,8 @@ import React, { useCallback } from 'react'; import { EuiPage, EuiPageBody, EuiPageHeader, EuiPageContent, EuiSpacer } from '@elastic/eui'; import { i18n } from '@osd/i18n'; import { useOpenSearchDashboards } from '../../../../opensearch_dashboards_react/public'; -import { WorkspaceForm, WorkspaceFormSubmitData } from './workspace_form'; +import { WorkspaceForm } from './workspace_form'; +import { WorkspaceFormSubmitData } from './types'; import { WORKSPACE_OVERVIEW_APP_ID, WORKSPACE_OP_TYPE_CREATE } from '../../../common/constants'; import { formatUrlWithWorkspaceId } from '../../../../../core/public/utils'; import { WorkspaceClient } from '../../workspace_client'; diff --git a/src/plugins/workspace/public/components/workspace_creator/workspace_permission_setting_panel.tsx b/src/plugins/workspace/public/components/workspace_creator/workspace_permission_setting_panel.tsx index 1e8ac23bd0e5..76d7c9878bec 100644 --- a/src/plugins/workspace/public/components/workspace_creator/workspace_permission_setting_panel.tsx +++ b/src/plugins/workspace/public/components/workspace_creator/workspace_permission_setting_panel.tsx @@ -16,14 +16,17 @@ import { EuiSpacer, } from '@elastic/eui'; import { i18n } from '@osd/i18n'; -import { WorkspacePermissionMode } from '../../../../../core/public'; +import { + WorkspacePermissionMode, + PermissionModeId, + OptionIdToWorkspacePermissionModesMap, +} from '../../../../../core/public'; import { UserPermissionSetting, GroupPermissionSetting, WorkspacePermissionSetting, WorkspacePermissionItemType, } from './types'; -import { PermissionModeId, OptionIdToWorkspacePermissionModesMap } from './constants'; import { getPermissionModeId } from './utils'; const permissionModeOptions = [ From 86e20877e0a30c826dfb1b3acb97d352640341b0 Mon Sep 17 00:00:00 2001 From: yuye-aws Date: Thu, 26 Oct 2023 15:06:57 +0800 Subject: [PATCH 07/21] remove unnecessary test cases Signed-off-by: yuye-aws --- .../workspace_updater.test.tsx | 111 +----------------- 1 file changed, 2 insertions(+), 109 deletions(-) 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 9cd97e6e507b..8f0731ebcf96 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 @@ -5,8 +5,8 @@ import React from 'react'; import { PublicAppInfo, WorkspaceAttribute } from 'opensearch-dashboards/public'; -import { fireEvent, render, waitFor } from '@testing-library/react'; -import { BehaviorSubject, of } from 'rxjs'; +import { fireEvent, render } from '@testing-library/react'; +import { BehaviorSubject } from 'rxjs'; import { WorkspaceUpdater as WorkspaceUpdaterComponent } from './workspace_updater'; import { coreMock } from '../../../../../core/public/mocks'; import { createOpenSearchDashboardsReactContext } from '../../../../opensearch_dashboards_react/public'; @@ -155,111 +155,4 @@ describe('WorkspaceUpdater', () => { fireEvent.click(getByTestId('confirmModalConfirmButton')); expect(navigateToApp).toHaveBeenCalled(); }); - - /* - it('create workspace with detailed information', async () => { - const { getByTestId } = render(); - const nameInput = getByTestId('workspaceForm-workspaceDetails-nameInputText'); - fireEvent.input(nameInput, { - target: { value: 'test workspace name' }, - }); - const descriptionInput = getByTestId('workspaceForm-workspaceDetails-descriptionInputText'); - fireEvent.input(descriptionInput, { - target: { value: 'test workspace description' }, - }); - const colorSelector = getByTestId( - 'euiColorPickerAnchor workspaceForm-workspaceDetails-colorPicker' - ); - fireEvent.input(colorSelector, { - target: { value: '#000000' }, - }); - const iconSelector = getByTestId('workspaceForm-workspaceDetails-iconSelector'); - fireEvent.click(iconSelector); - fireEvent.click(getByTestId('workspaceForm-workspaceDetails-iconSelector-Glasses')); - const defaultVISThemeSelector = getByTestId( - 'workspaceForm-workspaceDetails-defaultVISThemeSelector' - ); - fireEvent.click(defaultVISThemeSelector); - fireEvent.change(defaultVISThemeSelector, { target: { value: 'categorical' } }); - fireEvent.click(getByTestId('workspaceForm-bottomBar-createButton')); - expect(workspaceClientCreate).toHaveBeenCalledWith( - expect.objectContaining({ - name: 'test workspace name', - icon: 'Glasses', - color: '#000000', - description: 'test workspace description', - defaultVISTheme: 'categorical', - }) - ); - await waitFor(() => { - expect(notificationToastsAddSuccess).toHaveBeenCalled(); - }); - expect(notificationToastsAddDanger).not.toHaveBeenCalled(); - }); - - it('create workspace with customized features', async () => { - const { getByTestId } = render(); - const nameInput = getByTestId('workspaceForm-workspaceDetails-nameInputText'); - fireEvent.input(nameInput, { - target: { value: 'test workspace name' }, - }); - fireEvent.click(getByTestId('workspaceForm-workspaceFeatureVisibility-app1')); - fireEvent.click(getByTestId('workspaceForm-workspaceFeatureVisibility-category1')); - fireEvent.click(getByTestId('workspaceForm-bottomBar-createButton')); - expect(workspaceClientCreate).toHaveBeenCalledWith( - expect.objectContaining({ - name: 'test workspace name', - features: expect.arrayContaining(['app1', 'app2', 'app3']), - }) - ); - await waitFor(() => { - expect(notificationToastsAddSuccess).toHaveBeenCalled(); - }); - expect(notificationToastsAddDanger).not.toHaveBeenCalled(); - }); - - it('create workspace with customized permissions', async () => { - const { getByTestId, getByText } = render(); - const nameInput = getByTestId('workspaceForm-workspaceDetails-nameInputText'); - fireEvent.input(nameInput, { - target: { value: 'test workspace name' }, - }); - fireEvent.click(getByText('Users & Permissions')); - fireEvent.click(getByTestId('workspaceForm-permissionSettingPanel-user-addNew')); - const userIdInput = getByTestId('workspaceForm-permissionSettingPanel-0-userId'); - 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', - permissions: expect.arrayContaining([ - expect.objectContaining({ type: 'user', userId: 'test user id' }), - ]), - }) - ); - await waitFor(() => { - expect(notificationToastsAddSuccess).toHaveBeenCalled(); - }); - expect(notificationToastsAddDanger).not.toHaveBeenCalled(); - }); - - it('should show danger toasts after create workspace failed', async () => { - workspaceClientCreate.mockReturnValue({ result: { id: 'failResult' }, success: false }); - const { getByTestId } = render(); - const nameInput = getByTestId('workspaceForm-workspaceDetails-nameInputText'); - fireEvent.input(nameInput, { - target: { value: 'test workspace name' }, - }); - fireEvent.click(getByTestId('workspaceForm-bottomBar-createButton')); - expect(workspaceClientCreate).toHaveBeenCalled(); - await waitFor(() => { - expect(notificationToastsAddDanger).toHaveBeenCalled(); - }); - expect(notificationToastsAddSuccess).not.toHaveBeenCalled(); - }); - */ }); From a2014f91dbde5931e2538c685062ca15dda8d0fc Mon Sep 17 00:00:00 2001 From: yuye-aws Date: Thu, 26 Oct 2023 15:09:50 +0800 Subject: [PATCH 08/21] implement errors count with use memo Signed-off-by: yuye-aws --- .../public/components/workspace_creator/workspace_form.tsx | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/plugins/workspace/public/components/workspace_creator/workspace_form.tsx b/src/plugins/workspace/public/components/workspace_creator/workspace_form.tsx index c5b28870a7be..9dc09ad2458a 100644 --- a/src/plugins/workspace/public/components/workspace_creator/workspace_form.tsx +++ b/src/plugins/workspace/public/components/workspace_creator/workspace_form.tsx @@ -134,7 +134,6 @@ export const WorkspaceForm = ({ ? WorkspaceFormTabs.UsersAndPermissions : WorkspaceFormTabs.NotSelected ); - const [errorsCount, setErrorsCount] = useState(0); // The matched feature id list based on original feature config, // the feature category will be expanded to list of feature ids const defaultFeatures = useMemo(() => { @@ -179,6 +178,7 @@ export const WorkspaceForm = ({ }; const [formErrors, setFormErrors] = useState({}); + const errorsCount = useMemo(() => getErrorsCount(formErrors), [formErrors]); const formIdRef = useRef(); const getFormData = () => ({ name, @@ -393,7 +393,6 @@ export const WorkspaceForm = ({ } const currentErrorsCount = getErrorsCount(currentFormErrors); setFormErrors(currentFormErrors); - setErrorsCount(currentErrorsCount); if (currentErrorsCount > 0) { return; } From 178a3e0ba8ef5a52ad57e47fa5b6bf6c5d538ea2 Mon Sep 17 00:00:00 2001 From: yuye-aws Date: Thu, 26 Oct 2023 15:31:38 +0800 Subject: [PATCH 09/21] resolve conflicts Signed-off-by: yuye-aws --- .../components/workspace_creator/workspace_form.tsx | 12 +++++++++--- .../workspace_permission_setting_panel.tsx | 1 - .../workspace_updater/workspace_updater.test.tsx | 4 ++-- 3 files changed, 11 insertions(+), 6 deletions(-) diff --git a/src/plugins/workspace/public/components/workspace_creator/workspace_form.tsx b/src/plugins/workspace/public/components/workspace_creator/workspace_form.tsx index 9dc09ad2458a..9ba89f71c340 100644 --- a/src/plugins/workspace/public/components/workspace_creator/workspace_form.tsx +++ b/src/plugins/workspace/public/components/workspace_creator/workspace_form.tsx @@ -411,12 +411,18 @@ export const WorkspaceForm = ({ formData.features = defaultValues?.features ?? []; } + // Create a new object without the specified properties + const { + ['userPermissions']: formDataUserPermissions, + ['groupPermissions']: formDataGroupPermissions, + ...formDataWithoutPermissions + } = formData; onSubmit?.({ - ...formData, + ...formDataWithoutPermissions, name: formData.name!, permissions: [ - ...formData.userPermissions.filter(isValidWorkspacePermissionSetting), - ...formData.groupPermissions.filter(isValidWorkspacePermissionSetting), + ...formDataUserPermissions.filter(isValidWorkspacePermissionSetting), + ...formDataGroupPermissions.filter(isValidWorkspacePermissionSetting), ] as any, }); }, diff --git a/src/plugins/workspace/public/components/workspace_creator/workspace_permission_setting_panel.tsx b/src/plugins/workspace/public/components/workspace_creator/workspace_permission_setting_panel.tsx index 76d7c9878bec..f3e9c52f082d 100644 --- a/src/plugins/workspace/public/components/workspace_creator/workspace_permission_setting_panel.tsx +++ b/src/plugins/workspace/public/components/workspace_creator/workspace_permission_setting_panel.tsx @@ -328,7 +328,6 @@ export const WorkspacePermissionSettingPanel = ({ groupPermissionSettings, lastAdminItemDeletable, }: WorkspacePermissionSettingPanelProps) => { - const nonDeletableIndex = useMemo(() => { let userNonDeletableIndex = -1; let groupNonDeletableIndex = -1; 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 8f0731ebcf96..b08809a792a9 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 @@ -4,7 +4,7 @@ */ import React from 'react'; -import { PublicAppInfo, WorkspaceAttribute } from 'opensearch-dashboards/public'; +import { PublicAppInfo, WorkspaceObject } from 'opensearch-dashboards/public'; import { fireEvent, render } from '@testing-library/react'; import { BehaviorSubject } from 'rxjs'; import { WorkspaceUpdater as WorkspaceUpdaterComponent } from './workspace_updater'; @@ -74,7 +74,7 @@ const WorkspaceUpdater = (props: any) => { }, workspaces: { ...mockCoreStart.workspaces, - currentWorkspace$: new BehaviorSubject({ + currentWorkspace$: new BehaviorSubject({ id: 'test workspace id', name: 'test workspace name', features: DEFAULT_CHECKED_FEATURES_IDS, From c4f433b6f78d36c5396d05791e42be84caf1b2e1 Mon Sep 17 00:00:00 2001 From: yuye-aws Date: Fri, 27 Oct 2023 15:03:46 +0800 Subject: [PATCH 10/21] fix permission panel bug Signed-off-by: yuye-aws --- .../public/workspace/workspaces_service.ts | 1 - .../components/workspace_creator/index.tsx | 2 + .../components/workspace_creator/types.ts | 13 +- .../workspace_creator/utils.test.ts | 88 ++++++--- .../components/workspace_creator/utils.ts | 52 +++-- .../workspace_creator.test.tsx | 2 +- .../workspace_creator/workspace_form.tsx | 24 +-- .../workspace_permission_setting_panel.tsx | 186 ++++++------------ .../workspace_updater/workspace_updater.tsx | 12 +- 9 files changed, 177 insertions(+), 203 deletions(-) diff --git a/src/core/public/workspace/workspaces_service.ts b/src/core/public/workspace/workspaces_service.ts index d9d6664582b7..f0ba51792cf9 100644 --- a/src/core/public/workspace/workspaces_service.ts +++ b/src/core/public/workspace/workspaces_service.ts @@ -53,7 +53,6 @@ export class WorkspacesService implements CoreService { if (workspaceInitialized) { const currentWorkspace = workspaceList.find((w) => w && w.id === currentWorkspaceId); - /** * Do a simple idempotent verification here */ diff --git a/src/plugins/workspace/public/components/workspace_creator/index.tsx b/src/plugins/workspace/public/components/workspace_creator/index.tsx index c8cdbfab65be..ffdd51601704 100644 --- a/src/plugins/workspace/public/components/workspace_creator/index.tsx +++ b/src/plugins/workspace/public/components/workspace_creator/index.tsx @@ -4,3 +4,5 @@ */ export { WorkspaceCreator } from './workspace_creator'; +export { WorkspaceForm } from './workspace_form'; +export { WorkspaceFormSubmitData, WorkspaceFormData } from './types'; diff --git a/src/plugins/workspace/public/components/workspace_creator/types.ts b/src/plugins/workspace/public/components/workspace_creator/types.ts index a2550e2a7561..65bcb3467b70 100644 --- a/src/plugins/workspace/public/components/workspace_creator/types.ts +++ b/src/plugins/workspace/public/components/workspace_creator/types.ts @@ -42,13 +42,18 @@ export enum WorkspacePermissionItemType { Group = 'group', } -export interface UserPermissionSetting { +export interface TypelessPermissionSetting { + id: string; + modes: WorkspacePermissionMode[]; +} + +interface UserPermissionSetting { type: WorkspacePermissionItemType.User; userId: string; modes: WorkspacePermissionMode[]; } -export interface GroupPermissionSetting { +interface GroupPermissionSetting { type: WorkspacePermissionItemType.Group; group: string; modes: WorkspacePermissionMode[]; @@ -59,8 +64,8 @@ export type WorkspacePermissionSetting = UserPermissionSetting | GroupPermission // when editing, attributes could be undefined in workspace form export type WorkspaceFormEditingData = Partial< Omit & { - userPermissions: Array>; - groupPermissions: Array>; + userPermissions: Array>; + groupPermissions: Array>; } >; diff --git a/src/plugins/workspace/public/components/workspace_creator/utils.test.ts b/src/plugins/workspace/public/components/workspace_creator/utils.test.ts index 6d4128885250..a9a5efd0e7c6 100644 --- a/src/plugins/workspace/public/components/workspace_creator/utils.test.ts +++ b/src/plugins/workspace/public/components/workspace_creator/utils.test.ts @@ -10,9 +10,10 @@ import { getUnsavedChangesCount, getPermissionModeId, getPermissionErrors, + formatPermissions, } from './utils'; import { - UserPermissionSetting, + TypelessPermissionSetting, WorkspaceFormErrors, WorkspacePermissionItemType, WorkspacePermissionSetting, @@ -86,20 +87,19 @@ describe('getErrorsCount', () => { describe('getUserAndGroupPermissions', () => { it('should split user and group permissions from all permissions', () => { const permissions = [ - { modes: [] }, - { userId: 'test user id 1' }, - { group: 'test group id 1' }, - { type: WorkspacePermissionItemType.User, userId: 'test user id 2', modes: [] }, + { type: WorkspacePermissionItemType.User, userId: 'test user id 1', modes: [] }, + { type: WorkspacePermissionItemType.Group, group: 'test group id 1', modes: [] }, { type: WorkspacePermissionItemType.Group, group: 'test group id 2', modes: [] }, - { type: WorkspacePermissionItemType.Group, group: 'test group id 3', modes: [] }, - ]; - expect(getUserAndGroupPermissions(permissions)).toStrictEqual([ - [{ id: 'test user id 2', modes: [] }], - [ - { id: 'test group id 2', modes: [] }, - { id: 'test group id 3', modes: [] }, - ], - ]); + ] as any; + expect(getUserAndGroupPermissions(permissions)).toEqual( + expect.arrayContaining([ + [{ id: 'test user id 1', modes: [] }], + [ + { id: 'test group id 1', modes: [] }, + { id: 'test group id 2', modes: [] }, + ], + ]) + ); }); }); @@ -154,15 +154,14 @@ describe('getUnsavedChangesCount', () => { name: 'test workspace name', userPermissions: [ { - userId: 'test user id 1', - type: WorkspacePermissionItemType.User, + id: 'test user id 1', modes: [WorkspacePermissionMode.LibraryWrite, WorkspacePermissionMode.Write], }, - ] as UserPermissionSetting[], + ] as any, groupPermissions: [], }; // 1 deleted permission and 1 edited permission - expect(getUnsavedChangesCount(initialFormData, currentFormData)).toBe(false); + expect(getUnsavedChangesCount(initialFormData, currentFormData)).toBe(2); }); }); @@ -194,34 +193,61 @@ describe('getPermissionErrors', () => { it('should get permission errors for both users and groups', () => { const permissions = [ {}, - { type: WorkspacePermissionItemType.User }, + { id: 'test permission id' }, + { id: 'test permission id', modes: [] }, { - type: WorkspacePermissionItemType.User, + id: 'test permission id', + modes: [WorkspacePermissionMode.LibraryRead, WorkspacePermissionMode.Read], + }, + ]; + const expectedPermissionErrors = [ + 'Invalid id', + 'Invalid permission modes', + 'Invalid permission modes', + ]; + expect(getPermissionErrors(permissions)).toEqual( + expect.arrayContaining(expectedPermissionErrors) + ); + }); +}); + +describe('formatPermissions', () => { + it('should get permission errors for both users and groups', () => { + const userPermissions: TypelessPermissionSetting[] = [ + { + id: 'read user', modes: [WorkspacePermissionMode.LibraryRead, WorkspacePermissionMode.Read], }, { - type: WorkspacePermissionItemType.Group, + id: 'admin user', + modes: [WorkspacePermissionMode.LibraryWrite, WorkspacePermissionMode.Write], + }, + ]; + const groupPermissions: TypelessPermissionSetting[] = [ + { + id: 'read group', modes: [WorkspacePermissionMode.LibraryRead, WorkspacePermissionMode.Read], }, + ]; + const expectedPermissions: WorkspacePermissionSetting[] = [ { - userId: 'test user id', + userId: 'read user', type: WorkspacePermissionItemType.User, modes: [WorkspacePermissionMode.LibraryRead, WorkspacePermissionMode.Read], }, { - group: 'test group id', + userId: 'admin user', + type: WorkspacePermissionItemType.User, + modes: [WorkspacePermissionMode.LibraryWrite, WorkspacePermissionMode.Write], + }, + { + group: 'read group', type: WorkspacePermissionItemType.Group, modes: [WorkspacePermissionMode.LibraryRead, WorkspacePermissionMode.Read], }, ]; - const expectedPermissionErrors = [ - 'Invalid type', - 'Invalid permission modes', - 'Invalid userId', - 'Invalid user group', - ]; - expect(getPermissionErrors(permissions)).toEqual( - expect.arrayContaining(expectedPermissionErrors) + expect(formatPermissions(userPermissions, groupPermissions)).toEqual( + expect.arrayContaining(expectedPermissions) ); }); }); diff --git a/src/plugins/workspace/public/components/workspace_creator/utils.ts b/src/plugins/workspace/public/components/workspace_creator/utils.ts index 6e4e36f4b93a..5c722698bbed 100644 --- a/src/plugins/workspace/public/components/workspace_creator/utils.ts +++ b/src/plugins/workspace/public/components/workspace_creator/utils.ts @@ -16,6 +16,7 @@ import { WorkspacePermissionSetting, WorkspacePermissionItemType, WorkspaceFormData, + TypelessPermissionSetting, } from './types'; export const isValidWorkspacePermissionSetting = ( @@ -42,11 +43,12 @@ export const getErrorsCount = (formErrors: WorkspaceFormErrors) => { } return errorsCount; }; + export const getUserAndGroupPermissions = ( - permissions: Array> -) => { - const userPermissions: UserOrGroupPermissionEditingData = []; - const groupPermissions: UserOrGroupPermissionEditingData = []; + permissions: WorkspacePermissionSetting[] +): TypelessPermissionSetting[][] => { + const userPermissions: TypelessPermissionSetting[] = []; + const groupPermissions: TypelessPermissionSetting[] = []; for (const permission of permissions) { if (permission.type === WorkspacePermissionItemType.User) { userPermissions.push({ id: permission.userId, modes: permission.modes }); @@ -149,16 +151,16 @@ export const getPermissionModeId = (modes: WorkspacePermissionMode[]) => { return PermissionModeId.Read; }; -export const getPermissionErrors = (permissions: Array>) => { +export const getPermissionErrors = (permissions: Array>) => { const permissionErrors: string[] = new Array(permissions.length); for (let i = 0; i < permissions.length; i++) { const permission = permissions[i]; if (isValidWorkspacePermissionSetting(permission)) { continue; } - if (!permission.type) { - permissionErrors[i] = i18n.translate('workspace.form.permission.invalidate.type', { - defaultMessage: 'Invalid type', + if (!permission.id) { + permissionErrors[i] = i18n.translate('workspace.form.permission.invalidate.id', { + defaultMessage: 'Invalid id', }); continue; } @@ -166,20 +168,30 @@ export const getPermissionErrors = (permissions: Array { + const permissions: WorkspacePermissionSetting[] = []; + for (const permission of userPermissions) { + permissions.push({ + userId: permission.id, + modes: permission.modes, + type: WorkspacePermissionItemType.User, + }); + } + for (const permission of groupPermissions) { + permissions.push({ + group: permission.id, + modes: permission.modes, + type: WorkspacePermissionItemType.Group, + }); + } + return permissions; +}; 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 9ba73b8e8b32..1bb4a3edb0b5 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 @@ -208,7 +208,7 @@ describe('WorkspaceCreator', () => { }); fireEvent.click(getByText('Users & Permissions')); fireEvent.click(getByTestId('workspaceForm-permissionSettingPanel-user-addNew')); - const userIdInput = getByTestId('workspaceForm-permissionSettingPanel-0-userId'); + const userIdInput = getByTestId('workspaceForm-permissionSettingPanel-user-0-id'); fireEvent.click(userIdInput); fireEvent.input(getByTestId('comboBoxSearchInput'), { target: { value: 'test user id' }, diff --git a/src/plugins/workspace/public/components/workspace_creator/workspace_form.tsx b/src/plugins/workspace/public/components/workspace_creator/workspace_form.tsx index 9ba89f71c340..b7fbd96f3f90 100644 --- a/src/plugins/workspace/public/components/workspace_creator/workspace_form.tsx +++ b/src/plugins/workspace/public/components/workspace_creator/workspace_form.tsx @@ -51,11 +51,11 @@ import { WorkspaceIconSelector } from './workspace_icon_selector'; import { WorkspacePermissionSettingPanel } from './workspace_permission_setting_panel'; import { featureMatchesConfig } from '../../utils'; import { + formatPermissions, getErrorsCount, getPermissionErrors, getUnsavedChangesCount, getUserAndGroupPermissions, - isValidWorkspacePermissionSetting, } from './utils'; import { WorkspaceFeature, @@ -63,8 +63,7 @@ import { WorkspaceFormData, WorkspaceFormErrors, WorkspaceFormSubmitData, - UserPermissionSetting, - GroupPermissionSetting, + TypelessPermissionSetting, } from './types'; enum WorkspaceFormTabs { @@ -158,12 +157,12 @@ export const WorkspaceForm = ({ ? defaultValues.permissions : [] ); - const [userPermissions, setUserPermissions] = useState>>( + const [userPermissions, setUserPermissions] = useState>>( initialUserPermissions ); - const [groupPermissions, setGroupPermissions] = useState>>( - initialGroupPermissions - ); + const [groupPermissions, setGroupPermissions] = useState< + Array> + >(initialGroupPermissions); const libraryCategoryLabel = i18n.translate('core.ui.libraryNavList.label', { defaultMessage: 'Library', @@ -407,11 +406,12 @@ export const WorkspaceForm = ({ // such as `['@management']` or `['*']`. The form value `formData.features` will be // expanded to array of individual feature id, if the feature hasn't changed, we will // set the feature config back to the original value so that category wildcard won't - // expanded to feature ids + // be expanded to feature ids formData.features = defaultValues?.features ?? []; } // Create a new object without the specified properties + // If there are no form errors, attributes are available in TypelessPermissionSetting const { ['userPermissions']: formDataUserPermissions, ['groupPermissions']: formDataGroupPermissions, @@ -420,10 +420,10 @@ export const WorkspaceForm = ({ onSubmit?.({ ...formDataWithoutPermissions, name: formData.name!, - permissions: [ - ...formDataUserPermissions.filter(isValidWorkspacePermissionSetting), - ...formDataGroupPermissions.filter(isValidWorkspacePermissionSetting), - ] as any, + permissions: formatPermissions( + formDataUserPermissions as TypelessPermissionSetting[], + formDataGroupPermissions as TypelessPermissionSetting[] + ), }); }, [defaultFeatures, onSubmit, defaultValues?.features] diff --git a/src/plugins/workspace/public/components/workspace_creator/workspace_permission_setting_panel.tsx b/src/plugins/workspace/public/components/workspace_creator/workspace_permission_setting_panel.tsx index f3e9c52f082d..61ab6afbdcde 100644 --- a/src/plugins/workspace/public/components/workspace_creator/workspace_permission_setting_panel.tsx +++ b/src/plugins/workspace/public/components/workspace_creator/workspace_permission_setting_panel.tsx @@ -22,10 +22,8 @@ import { OptionIdToWorkspacePermissionModesMap, } from '../../../../../core/public'; import { - UserPermissionSetting, - GroupPermissionSetting, - WorkspacePermissionSetting, WorkspacePermissionItemType, + TypelessPermissionSetting as PermissionSetting, } from './types'; import { getPermissionModeId } from './utils'; @@ -53,31 +51,15 @@ const permissionModeOptions = [ }, ]; -const generateWorkspacePermissionItemKey = ( - item: Partial, - index?: number -) => - [ - ...(item.type ?? []), - ...(item.type === WorkspacePermissionItemType.User ? [item.userId] : []), - ...(item.type === WorkspacePermissionItemType.Group ? [item.group] : []), - ...(item.modes ?? []), - index, - ].join('-'); +const generateWorkspacePermissionItemKey = (type: string, index: number) => [type, index].join('-'); interface WorkspacePermissionSettingInputProps { index: number; deletable: boolean; type: WorkspacePermissionItemType; - userId?: string; - group?: string; - modes?: WorkspacePermissionMode[]; - onGroupOrUserIdChange: ( - groupOrUserId: - | { type: WorkspacePermissionItemType.User; userId?: string } - | { type: WorkspacePermissionItemType.Group; group?: string }, - index: number - ) => void; + permissionId?: string; + permissionModes?: WorkspacePermissionMode[]; + onIdChange: (index: number, id?: string) => void; onPermissionModesChange: ( WorkspacePermissionMode: WorkspacePermissionMode[], index: number @@ -88,45 +70,42 @@ interface WorkspacePermissionSettingInputProps { const WorkspacePermissionSettingInput = ({ index, type, - userId, - group, - modes, + permissionModes, deletable, onDelete, - onGroupOrUserIdChange, + onIdChange, + permissionId, onPermissionModesChange, }: WorkspacePermissionSettingInputProps) => { - const groupOrUserIdSelectedOptions = useMemo( - () => (group || userId ? [{ label: (group || userId) as string }] : []), - [group, userId] + const idSelectedOptions = useMemo( + () => (permissionId ? [{ label: permissionId as string }] : []), + [permissionId] ); - const permissionModesSelectedId = useMemo(() => getPermissionModeId(modes ?? []), [modes]); - const handleGroupOrUserIdCreate = useCallback( - (groupOrUserId) => { - onGroupOrUserIdChange( - type === WorkspacePermissionItemType.Group - ? { type, group: groupOrUserId } - : { type, userId: groupOrUserId }, - index - ); + const permissionModesSelectedId = useMemo(() => getPermissionModeId(permissionModes ?? []), [ + permissionModes, + ]); + + const handleIdCreate = useCallback( + (createdId) => { + onIdChange(index, createdId); }, - [index, type, onGroupOrUserIdChange] + [index, onIdChange] ); - const handleGroupOrUserIdChange = useCallback( + const handleIdChange = useCallback( (options) => { if (options.length === 0) { - onGroupOrUserIdChange({ type }, index); + onIdChange(index); } }, - [index, type, onGroupOrUserIdChange] + [index, onIdChange] ); const handlePermissionModeOptionChange = useCallback( - (id: string) => { - if (OptionIdToWorkspacePermissionModesMap[id]) { - onPermissionModesChange([...OptionIdToWorkspacePermissionModesMap[id]], index); + (changedId: string) => { + if (OptionIdToWorkspacePermissionModesMap[changedId]) { + onPermissionModesChange([...OptionIdToWorkspacePermissionModesMap[changedId]], index); } }, [index, onPermissionModesChange] @@ -141,12 +120,12 @@ const WorkspacePermissionSettingInput = ({ @@ -178,10 +157,10 @@ interface WorkspacePermissionSettingPanelProps { userErrors?: string[]; groupErrors?: string[]; lastAdminItemDeletable?: boolean; - userPermissionSettings: Array>; - groupPermissionSettings: Array>; - onUserPermissionChange: (value: Array>) => void; - onGroupPermissionChange: (value: Array>) => void; + userPermissionSettings: Array>; + groupPermissionSettings: Array>; + onUserPermissionChange: (value: Array>) => void; + onGroupPermissionChange: (value: Array>) => void; } interface UserOrGroupSectionProps { @@ -189,8 +168,8 @@ interface UserOrGroupSectionProps { errors?: string[]; nonDeletableIndex: number; type: WorkspacePermissionItemType; - permissionSettings: Array>; - onChange: (value: Array>) => void; + permissionSettings: Array>; + onChange: (value: Array>) => void; } const UserOrGroupSection = ({ @@ -201,58 +180,19 @@ const UserOrGroupSection = ({ permissionSettings, nonDeletableIndex, }: UserOrGroupSectionProps) => { - const transformedValue = useMemo(() => { - if (!permissionSettings) { - return []; - } - const result: Array> = []; - /** - * One workspace permission setting may include multi setting options, - * for loop the workspace permission setting array to separate it to multi rows. - **/ - for (let i = 0; i < permissionSettings.length; i++) { - const valueItem = permissionSettings[i]; - // Incomplete workspace permission setting don't need to separate to multi rows - if ( - !valueItem.modes || - !valueItem.type || - (valueItem.type === 'user' && !valueItem.userId) || - (valueItem.type === 'group' && !valueItem.group) - ) { - result.push(valueItem); - continue; - } - /** - * For loop the option id to workspace permission modes map, - * if one settings includes all permission modes in a specific option, - * add these permission modes to the result array. - */ - for (const key in OptionIdToWorkspacePermissionModesMap) { - if (!Object.prototype.hasOwnProperty.call(OptionIdToWorkspacePermissionModesMap, key)) { - continue; - } - const modesForCertainPermissionId = OptionIdToWorkspacePermissionModesMap[key]; - if (modesForCertainPermissionId.every((mode) => valueItem.modes?.includes(mode))) { - result.push({ ...valueItem, modes: modesForCertainPermissionId }); - } - } - } - return result; - }, [permissionSettings]); - // default permission mode is read const handleAddNewOne = useCallback(() => { onChange?.([ - ...(transformedValue ?? []), - { type, modes: OptionIdToWorkspacePermissionModesMap[PermissionModeId.Read] }, + ...(permissionSettings ?? []), + { modes: OptionIdToWorkspacePermissionModesMap[PermissionModeId.Read] }, ]); - }, [onChange, type, transformedValue]); + }, [onChange, permissionSettings]); const handleDelete = useCallback( (index: number) => { - onChange?.((transformedValue ?? []).filter((_item, itemIndex) => itemIndex !== index)); + onChange?.((permissionSettings ?? []).filter((_item, itemIndex) => itemIndex !== index)); }, - [onChange, transformedValue] + [onChange, permissionSettings] ); const handlePermissionModesChange = useCallback< @@ -260,27 +200,23 @@ const UserOrGroupSection = ({ >( (modes, index) => { onChange?.( - (transformedValue ?? []).map((item, itemIndex) => + (permissionSettings ?? []).map((item, itemIndex) => index === itemIndex ? { ...item, modes } : item ) ); }, - [onChange, transformedValue] + [onChange, permissionSettings] ); - const handleGroupOrUserIdChange = useCallback< - WorkspacePermissionSettingInputProps['onGroupOrUserIdChange'] - >( - (userOrGroupIdWithType, index) => { + const handleIdChange = useCallback( + (index: number, id?: string) => { onChange?.( - (transformedValue ?? []).map((item, itemIndex) => - index === itemIndex - ? { ...userOrGroupIdWithType, ...(item.modes ? { modes: item.modes } : {}) } - : item + (permissionSettings ?? []).map((item, itemIndex) => + index === itemIndex ? { id, ...(item.modes ? { modes: item.modes } : {}) } : item ) ); }, - [onChange, transformedValue] + [onChange, permissionSettings] ); // assume that group items are always deletable @@ -290,16 +226,17 @@ const UserOrGroupSection = ({ {title} - {transformedValue?.map((item, index) => ( - + {permissionSettings?.map((permissionItem, index) => ( + @@ -337,15 +274,12 @@ export const WorkspacePermissionSettingPanel = ({ (permission) => getPermissionModeId(permission.modes ?? []) === PermissionModeId.Admin ); if (adminPermissionSettings.length === 1) { - if (adminPermissionSettings[0].type === WorkspacePermissionItemType.User) { - userNonDeletableIndex = userPermissionSettings.findIndex( - (permission) => getPermissionModeId(permission.modes ?? []) === PermissionModeId.Admin - ); - } else { - groupNonDeletableIndex = groupPermissionSettings.findIndex( - (permission) => getPermissionModeId(permission.modes ?? []) === PermissionModeId.Admin - ); - } + userNonDeletableIndex = userPermissionSettings.findIndex( + (permission) => getPermissionModeId(permission.modes ?? []) === PermissionModeId.Admin + ); + groupNonDeletableIndex = groupPermissionSettings.findIndex( + (permission) => getPermissionModeId(permission.modes ?? []) === PermissionModeId.Admin + ); } } return { userNonDeletableIndex, groupNonDeletableIndex }; @@ -360,7 +294,7 @@ export const WorkspacePermissionSettingPanel = ({ defaultMessage: 'User', })} errors={userErrors} - onChange={onUserPermissionChange as any} + onChange={onUserPermissionChange} nonDeletableIndex={userNonDeletableIndex} permissionSettings={userPermissionSettings} type={WorkspacePermissionItemType.User} @@ -371,7 +305,7 @@ export const WorkspacePermissionSettingPanel = ({ defaultMessage: 'User Groups', })} errors={groupErrors} - onChange={onGroupPermissionChange as any} + onChange={onGroupPermissionChange} nonDeletableIndex={groupNonDeletableIndex} permissionSettings={groupPermissionSettings} type={WorkspacePermissionItemType.Group} diff --git a/src/plugins/workspace/public/components/workspace_updater/workspace_updater.tsx b/src/plugins/workspace/public/components/workspace_updater/workspace_updater.tsx index 59268fa7d917..f2ae052a59dc 100644 --- a/src/plugins/workspace/public/components/workspace_updater/workspace_updater.tsx +++ b/src/plugins/workspace/public/components/workspace_updater/workspace_updater.tsx @@ -16,25 +16,21 @@ import { import { useObservable } from 'react-use'; import { i18n } from '@osd/i18n'; import { of } from 'rxjs'; -import { WorkspaceAttribute } from 'opensearch-dashboards/public'; +import { WorkspaceObject } from 'opensearch-dashboards/public'; import { useOpenSearchDashboards } from '../../../../opensearch_dashboards_react/public'; -import { - WorkspaceForm, - WorkspaceFormSubmitData, - WorkspaceFormData, -} from '../workspace_creator/workspace_form'; +import { WorkspaceForm, WorkspaceFormSubmitData, WorkspaceFormData } from '../workspace_creator/'; import { WORKSPACE_OVERVIEW_APP_ID, WORKSPACE_OP_TYPE_UPDATE } from '../../../common/constants'; import { DeleteWorkspaceModal } from '../delete_workspace_modal'; import { formatUrlWithWorkspaceId } from '../../../../../core/public/utils'; import { WorkspaceClient } from '../../workspace_client'; import { WorkspacePermissionSetting } from '../'; -interface WorkspaceWithPermission extends WorkspaceAttribute { +interface WorkspaceWithPermission extends WorkspaceObject { permissions?: WorkspacePermissionSetting[]; } function getFormDataFromWorkspace( - currentWorkspace: WorkspaceAttribute | null | undefined + currentWorkspace: WorkspaceObject | null | undefined ): WorkspaceFormData { const currentWorkspaceWithPermission = (currentWorkspace || {}) as WorkspaceWithPermission; return { From 3f9bb36f646163655f3b01957eb1449216a22456 Mon Sep 17 00:00:00 2001 From: yuye-aws Date: Fri, 27 Oct 2023 15:23:17 +0800 Subject: [PATCH 11/21] rename util function Signed-off-by: yuye-aws --- .../workspace/public/components/workspace_creator/utils.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/plugins/workspace/public/components/workspace_creator/utils.ts b/src/plugins/workspace/public/components/workspace_creator/utils.ts index 5c722698bbed..6d1dbda55f6c 100644 --- a/src/plugins/workspace/public/components/workspace_creator/utils.ts +++ b/src/plugins/workspace/public/components/workspace_creator/utils.ts @@ -60,7 +60,7 @@ export const getUserAndGroupPermissions = ( return [userPermissions, groupPermissions]; }; -const getUnsavedUserOrGroupPermissionChangesCount = ( +const getUnsavedPermissionsCount = ( initialPermissions: UserOrGroupPermissionEditingData, currentPermissions: UserOrGroupPermissionEditingData ) => { @@ -130,11 +130,11 @@ export const getUnsavedChangesCount = ( const [initialUserPermissions, initialGroupPermissions] = getUserAndGroupPermissions( initialFormData.permissions ?? [] ); - unsavedChangesCount += getUnsavedUserOrGroupPermissionChangesCount( + unsavedChangesCount += getUnsavedPermissionsCount( initialUserPermissions, currentFormData.userPermissions ?? [] ); - unsavedChangesCount += getUnsavedUserOrGroupPermissionChangesCount( + unsavedChangesCount += getUnsavedPermissionsCount( initialGroupPermissions, currentFormData.groupPermissions ?? [] ); From 26b14c8d4d207dd8d5227578a94df4eb5098edc1 Mon Sep 17 00:00:00 2001 From: yuye-aws Date: Fri, 27 Oct 2023 15:26:08 +0800 Subject: [PATCH 12/21] rename permission type Signed-off-by: yuye-aws --- .../workspace/public/components/workspace_creator/types.ts | 4 +--- .../workspace/public/components/workspace_creator/utils.ts | 6 +++--- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/src/plugins/workspace/public/components/workspace_creator/types.ts b/src/plugins/workspace/public/components/workspace_creator/types.ts index 65bcb3467b70..9cdde0cc9288 100644 --- a/src/plugins/workspace/public/components/workspace_creator/types.ts +++ b/src/plugins/workspace/public/components/workspace_creator/types.ts @@ -69,6 +69,4 @@ export type WorkspaceFormEditingData = Partial< } >; -export type UserOrGroupPermissionEditingData = Array< - Partial<{ id: string; modes: WorkspacePermissionMode[] }> ->; +export type PermissionEditingData = Array>; diff --git a/src/plugins/workspace/public/components/workspace_creator/utils.ts b/src/plugins/workspace/public/components/workspace_creator/utils.ts index 6d1dbda55f6c..f43327ccf49d 100644 --- a/src/plugins/workspace/public/components/workspace_creator/utils.ts +++ b/src/plugins/workspace/public/components/workspace_creator/utils.ts @@ -10,7 +10,7 @@ import { PermissionModeId, } from '../../../../../core/public'; import { - UserOrGroupPermissionEditingData, + PermissionEditingData, WorkspaceFormEditingData, WorkspaceFormErrors, WorkspacePermissionSetting, @@ -61,8 +61,8 @@ export const getUserAndGroupPermissions = ( }; const getUnsavedPermissionsCount = ( - initialPermissions: UserOrGroupPermissionEditingData, - currentPermissions: UserOrGroupPermissionEditingData + initialPermissions: PermissionEditingData, + currentPermissions: PermissionEditingData ) => { // for user or group permissions, unsaved changes is the sum of // # deleted permissions, # added permissions and # edited permissions From 2532c42ed5c55f6e47c5657c0359f618427d2d81 Mon Sep 17 00:00:00 2001 From: yuye-aws Date: Fri, 27 Oct 2023 15:34:00 +0800 Subject: [PATCH 13/21] rename permission type Signed-off-by: yuye-aws --- .../components/workspace_creator/types.ts | 10 +++++----- .../components/workspace_creator/utils.ts | 14 +++++++------- .../workspace_creator/workspace_form.tsx | 15 ++++++++------- .../workspace_permission_setting_panel.tsx | 17 +++++++---------- 4 files changed, 27 insertions(+), 29 deletions(-) diff --git a/src/plugins/workspace/public/components/workspace_creator/types.ts b/src/plugins/workspace/public/components/workspace_creator/types.ts index 9cdde0cc9288..8d3406e42f50 100644 --- a/src/plugins/workspace/public/components/workspace_creator/types.ts +++ b/src/plugins/workspace/public/components/workspace_creator/types.ts @@ -42,7 +42,7 @@ export enum WorkspacePermissionItemType { Group = 'group', } -export interface TypelessPermissionSetting { +export interface PermissionFieldData { id: string; modes: WorkspacePermissionMode[]; } @@ -61,12 +61,12 @@ interface GroupPermissionSetting { export type WorkspacePermissionSetting = UserPermissionSetting | GroupPermissionSetting; +export type PermissionEditingData = Array>; + // when editing, attributes could be undefined in workspace form export type WorkspaceFormEditingData = Partial< Omit & { - userPermissions: Array>; - groupPermissions: Array>; + userPermissions: PermissionEditingData; + groupPermissions: PermissionEditingData; } >; - -export type PermissionEditingData = Array>; diff --git a/src/plugins/workspace/public/components/workspace_creator/utils.ts b/src/plugins/workspace/public/components/workspace_creator/utils.ts index f43327ccf49d..59532dcf1242 100644 --- a/src/plugins/workspace/public/components/workspace_creator/utils.ts +++ b/src/plugins/workspace/public/components/workspace_creator/utils.ts @@ -16,7 +16,7 @@ import { WorkspacePermissionSetting, WorkspacePermissionItemType, WorkspaceFormData, - TypelessPermissionSetting, + PermissionFieldData, } from './types'; export const isValidWorkspacePermissionSetting = ( @@ -46,9 +46,9 @@ export const getErrorsCount = (formErrors: WorkspaceFormErrors) => { export const getUserAndGroupPermissions = ( permissions: WorkspacePermissionSetting[] -): TypelessPermissionSetting[][] => { - const userPermissions: TypelessPermissionSetting[] = []; - const groupPermissions: TypelessPermissionSetting[] = []; +): PermissionFieldData[][] => { + const userPermissions: PermissionFieldData[] = []; + const groupPermissions: PermissionFieldData[] = []; for (const permission of permissions) { if (permission.type === WorkspacePermissionItemType.User) { userPermissions.push({ id: permission.userId, modes: permission.modes }); @@ -151,7 +151,7 @@ export const getPermissionModeId = (modes: WorkspacePermissionMode[]) => { return PermissionModeId.Read; }; -export const getPermissionErrors = (permissions: Array>) => { +export const getPermissionErrors = (permissions: PermissionEditingData) => { const permissionErrors: string[] = new Array(permissions.length); for (let i = 0; i < permissions.length; i++) { const permission = permissions[i]; @@ -175,8 +175,8 @@ export const getPermissionErrors = (permissions: Array { const permissions: WorkspacePermissionSetting[] = []; for (const permission of userPermissions) { diff --git a/src/plugins/workspace/public/components/workspace_creator/workspace_form.tsx b/src/plugins/workspace/public/components/workspace_creator/workspace_form.tsx index b7fbd96f3f90..61c2cd5d2cac 100644 --- a/src/plugins/workspace/public/components/workspace_creator/workspace_form.tsx +++ b/src/plugins/workspace/public/components/workspace_creator/workspace_form.tsx @@ -63,7 +63,8 @@ import { WorkspaceFormData, WorkspaceFormErrors, WorkspaceFormSubmitData, - TypelessPermissionSetting, + PermissionFieldData, + PermissionEditingData, } from './types'; enum WorkspaceFormTabs { @@ -157,12 +158,12 @@ export const WorkspaceForm = ({ ? defaultValues.permissions : [] ); - const [userPermissions, setUserPermissions] = useState>>( + const [userPermissions, setUserPermissions] = useState( initialUserPermissions ); - const [groupPermissions, setGroupPermissions] = useState< - Array> - >(initialGroupPermissions); + const [groupPermissions, setGroupPermissions] = useState( + initialGroupPermissions + ); const libraryCategoryLabel = i18n.translate('core.ui.libraryNavList.label', { defaultMessage: 'Library', @@ -421,8 +422,8 @@ export const WorkspaceForm = ({ ...formDataWithoutPermissions, name: formData.name!, permissions: formatPermissions( - formDataUserPermissions as TypelessPermissionSetting[], - formDataGroupPermissions as TypelessPermissionSetting[] + formDataUserPermissions as PermissionFieldData[], + formDataGroupPermissions as PermissionFieldData[] ), }); }, diff --git a/src/plugins/workspace/public/components/workspace_creator/workspace_permission_setting_panel.tsx b/src/plugins/workspace/public/components/workspace_creator/workspace_permission_setting_panel.tsx index 61ab6afbdcde..b908bab7c591 100644 --- a/src/plugins/workspace/public/components/workspace_creator/workspace_permission_setting_panel.tsx +++ b/src/plugins/workspace/public/components/workspace_creator/workspace_permission_setting_panel.tsx @@ -21,10 +21,7 @@ import { PermissionModeId, OptionIdToWorkspacePermissionModesMap, } from '../../../../../core/public'; -import { - WorkspacePermissionItemType, - TypelessPermissionSetting as PermissionSetting, -} from './types'; +import { WorkspacePermissionItemType, PermissionEditingData } from './types'; import { getPermissionModeId } from './utils'; const permissionModeOptions = [ @@ -157,10 +154,10 @@ interface WorkspacePermissionSettingPanelProps { userErrors?: string[]; groupErrors?: string[]; lastAdminItemDeletable?: boolean; - userPermissionSettings: Array>; - groupPermissionSettings: Array>; - onUserPermissionChange: (value: Array>) => void; - onGroupPermissionChange: (value: Array>) => void; + userPermissionSettings: PermissionEditingData; + groupPermissionSettings: PermissionEditingData; + onUserPermissionChange: (value: PermissionEditingData) => void; + onGroupPermissionChange: (value: PermissionEditingData) => void; } interface UserOrGroupSectionProps { @@ -168,8 +165,8 @@ interface UserOrGroupSectionProps { errors?: string[]; nonDeletableIndex: number; type: WorkspacePermissionItemType; - permissionSettings: Array>; - onChange: (value: Array>) => void; + permissionSettings: PermissionEditingData; + onChange: (value: PermissionEditingData) => void; } const UserOrGroupSection = ({ From 5bc5d3d930cfec5964cfb3e4589fdb4f74a125b6 Mon Sep 17 00:00:00 2001 From: yuye-aws Date: Fri, 27 Oct 2023 17:42:05 +0800 Subject: [PATCH 14/21] move constants declaration to workspace plugin Signed-off-by: yuye-aws --- src/core/public/index.ts | 2 -- src/core/utils/constants.ts | 17 ----------------- src/core/utils/index.ts | 2 -- src/plugins/workspace/common/constants.ts | 19 +++++++++++++++++++ .../workspace_creator/utils.test.ts | 9 +++++---- .../components/workspace_creator/utils.ts | 7 ++----- .../workspace_permission_setting_panel.tsx | 7 ++----- 7 files changed, 28 insertions(+), 35 deletions(-) diff --git a/src/core/public/index.ts b/src/core/public/index.ts index 7ecc5c1dc9fe..08000926d7aa 100644 --- a/src/core/public/index.ts +++ b/src/core/public/index.ts @@ -354,8 +354,6 @@ export { WorkspacesStart, WorkspacesSetup, WorkspacesService } from './workspace export { WorkspacePermissionMode, - OptionIdToWorkspacePermissionModesMap, - PermissionModeId, PUBLIC_WORKSPACE_ID, MANAGEMENT_WORKSPACE_ID, WORKSPACE_TYPE, diff --git a/src/core/utils/constants.ts b/src/core/utils/constants.ts index 061f1d151e38..1eecb44ae96f 100644 --- a/src/core/utils/constants.ts +++ b/src/core/utils/constants.ts @@ -14,23 +14,6 @@ export enum WorkspacePermissionMode { LibraryWrite = 'library_write', } -export enum PermissionModeId { - Read = 'read', - ReadAndWrite = 'read+write', - Admin = 'admin', -} - -export const OptionIdToWorkspacePermissionModesMap: { - [key: string]: WorkspacePermissionMode[]; -} = { - [PermissionModeId.Read]: [WorkspacePermissionMode.LibraryRead, WorkspacePermissionMode.Read], - [PermissionModeId.ReadAndWrite]: [ - WorkspacePermissionMode.LibraryWrite, - WorkspacePermissionMode.Read, - ], - [PermissionModeId.Admin]: [WorkspacePermissionMode.LibraryWrite, WorkspacePermissionMode.Write], -}; - export const PUBLIC_WORKSPACE_ID = 'public'; export const MANAGEMENT_WORKSPACE_ID = 'management'; diff --git a/src/core/utils/index.ts b/src/core/utils/index.ts index 768bdc36a3c1..f2884c60581c 100644 --- a/src/core/utils/index.ts +++ b/src/core/utils/index.ts @@ -40,8 +40,6 @@ export { DEFAULT_APP_CATEGORIES } from './default_app_categories'; export { WORKSPACE_PATH_PREFIX, WorkspacePermissionMode, - PermissionModeId, - OptionIdToWorkspacePermissionModesMap, PUBLIC_WORKSPACE_ID, MANAGEMENT_WORKSPACE_ID, WORKSPACE_TYPE, diff --git a/src/plugins/workspace/common/constants.ts b/src/plugins/workspace/common/constants.ts index ef0821c88619..3ce208aa06bf 100644 --- a/src/plugins/workspace/common/constants.ts +++ b/src/plugins/workspace/common/constants.ts @@ -3,6 +3,8 @@ * SPDX-License-Identifier: Apache-2.0 */ +import { WorkspacePermissionMode } from '../../../core/public'; + export const WORKSPACE_CREATE_APP_ID = 'workspace_create'; export const WORKSPACE_LIST_APP_ID = 'workspace_list'; export const WORKSPACE_UPDATE_APP_ID = 'workspace_update'; @@ -19,3 +21,20 @@ export const PATHS = { export const WORKSPACE_OP_TYPE_CREATE = 'create'; export const WORKSPACE_OP_TYPE_UPDATE = 'update'; export const WORKSPACE_SAVED_OBJECTS_CLIENT_WRAPPER_ID = 'workspace'; + +export enum PermissionModeId { + Read = 'read', + ReadAndWrite = 'read+write', + Admin = 'admin', +} + +export const OptionIdToWorkspacePermissionModesMap: { + [key: string]: WorkspacePermissionMode[]; +} = { + [PermissionModeId.Read]: [WorkspacePermissionMode.LibraryRead, WorkspacePermissionMode.Read], + [PermissionModeId.ReadAndWrite]: [ + WorkspacePermissionMode.LibraryWrite, + WorkspacePermissionMode.Read, + ], + [PermissionModeId.Admin]: [WorkspacePermissionMode.LibraryWrite, WorkspacePermissionMode.Write], +}; diff --git a/src/plugins/workspace/public/components/workspace_creator/utils.test.ts b/src/plugins/workspace/public/components/workspace_creator/utils.test.ts index a9a5efd0e7c6..b6ce81d5da62 100644 --- a/src/plugins/workspace/public/components/workspace_creator/utils.test.ts +++ b/src/plugins/workspace/public/components/workspace_creator/utils.test.ts @@ -13,12 +13,13 @@ import { formatPermissions, } from './utils'; import { - TypelessPermissionSetting, + PermissionFieldData, WorkspaceFormErrors, WorkspacePermissionItemType, WorkspacePermissionSetting, } from './types'; -import { WorkspacePermissionMode, PermissionModeId } from '../../../../../core/public'; +import { WorkspacePermissionMode } from '../../../../../core/public'; +import { PermissionModeId } from '../../../common/constants'; describe('isValidWorkspacePermissionSetting', () => { it('should return true with valid user permission setting', () => { @@ -213,7 +214,7 @@ describe('getPermissionErrors', () => { describe('formatPermissions', () => { it('should get permission errors for both users and groups', () => { - const userPermissions: TypelessPermissionSetting[] = [ + const userPermissions: PermissionFieldData[] = [ { id: 'read user', modes: [WorkspacePermissionMode.LibraryRead, WorkspacePermissionMode.Read], @@ -223,7 +224,7 @@ describe('formatPermissions', () => { modes: [WorkspacePermissionMode.LibraryWrite, WorkspacePermissionMode.Write], }, ]; - const groupPermissions: TypelessPermissionSetting[] = [ + const groupPermissions: PermissionFieldData[] = [ { id: 'read group', modes: [WorkspacePermissionMode.LibraryRead, WorkspacePermissionMode.Read], diff --git a/src/plugins/workspace/public/components/workspace_creator/utils.ts b/src/plugins/workspace/public/components/workspace_creator/utils.ts index 59532dcf1242..5ef1fc31ee5a 100644 --- a/src/plugins/workspace/public/components/workspace_creator/utils.ts +++ b/src/plugins/workspace/public/components/workspace_creator/utils.ts @@ -4,11 +4,8 @@ */ import { i18n } from '@osd/i18n'; -import { - WorkspacePermissionMode, - OptionIdToWorkspacePermissionModesMap, - PermissionModeId, -} from '../../../../../core/public'; +import { WorkspacePermissionMode } from '../../../../../core/public'; +import { OptionIdToWorkspacePermissionModesMap, PermissionModeId } from '../../../common/constants'; import { PermissionEditingData, WorkspaceFormEditingData, diff --git a/src/plugins/workspace/public/components/workspace_creator/workspace_permission_setting_panel.tsx b/src/plugins/workspace/public/components/workspace_creator/workspace_permission_setting_panel.tsx index b908bab7c591..e7c3240a1f1a 100644 --- a/src/plugins/workspace/public/components/workspace_creator/workspace_permission_setting_panel.tsx +++ b/src/plugins/workspace/public/components/workspace_creator/workspace_permission_setting_panel.tsx @@ -16,11 +16,8 @@ import { EuiSpacer, } from '@elastic/eui'; import { i18n } from '@osd/i18n'; -import { - WorkspacePermissionMode, - PermissionModeId, - OptionIdToWorkspacePermissionModesMap, -} from '../../../../../core/public'; +import { WorkspacePermissionMode } from '../../../../../core/public'; +import { PermissionModeId, OptionIdToWorkspacePermissionModesMap } from '../../../common/constants'; import { WorkspacePermissionItemType, PermissionEditingData } from './types'; import { getPermissionModeId } from './utils'; From ac8f9fe011a326fb7026ed90778e23b9df72fbb3 Mon Sep 17 00:00:00 2001 From: yuye-aws Date: Fri, 27 Oct 2023 18:12:04 +0800 Subject: [PATCH 15/21] reimplement permission unsaved changes Signed-off-by: yuye-aws --- src/plugins/workspace/common/constants.ts | 7 ++++--- .../components/workspace_creator/utils.ts | 19 +++++++++---------- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/src/plugins/workspace/common/constants.ts b/src/plugins/workspace/common/constants.ts index 3ce208aa06bf..1a1efa2870d0 100644 --- a/src/plugins/workspace/common/constants.ts +++ b/src/plugins/workspace/common/constants.ts @@ -28,9 +28,10 @@ export enum PermissionModeId { Admin = 'admin', } -export const OptionIdToWorkspacePermissionModesMap: { - [key: string]: WorkspacePermissionMode[]; -} = { +export const OptionIdToWorkspacePermissionModesMap: Record< + PermissionModeId, + WorkspacePermissionMode[] +> = { [PermissionModeId.Read]: [WorkspacePermissionMode.LibraryRead, WorkspacePermissionMode.Read], [PermissionModeId.ReadAndWrite]: [ WorkspacePermissionMode.LibraryWrite, diff --git a/src/plugins/workspace/public/components/workspace_creator/utils.ts b/src/plugins/workspace/public/components/workspace_creator/utils.ts index 5ef1fc31ee5a..6f9a6b0ca98b 100644 --- a/src/plugins/workspace/public/components/workspace_creator/utils.ts +++ b/src/plugins/workspace/public/components/workspace_creator/utils.ts @@ -61,14 +61,14 @@ const getUnsavedPermissionsCount = ( initialPermissions: PermissionEditingData, currentPermissions: PermissionEditingData ) => { - // for user or group permissions, unsaved changes is the sum of + // for permission data being edited, unsaved changes is the sum of // # deleted permissions, # added permissions and # edited permissions let addedPermissions = 0; let editedPermissions = 0; - const initialPermissionMap = new Map(); + const initialPermissionMap = new Map(); for (const permission of initialPermissions) { if (permission.id) { - initialPermissionMap.set(permission.id, permission.modes ?? []); + initialPermissionMap.set(permission.id, getPermissionModeId(permission.modes ?? [])); } } @@ -76,12 +76,10 @@ const getUnsavedPermissionsCount = ( if (!permission.id) { addedPermissions += 1; // added permissions } else { - const permissionModes = initialPermissionMap.get(permission.id); - if (!permissionModes) { + const permissionMode = initialPermissionMap.get(permission.id); + if (!permissionMode) { addedPermissions += 1; - } else if ( - getPermissionModeId(permissionModes) !== getPermissionModeId(permission.modes ?? []) - ) { + } else if (permissionMode !== getPermissionModeId(permission.modes ?? [])) { editedPermissions += 1; // added or edited permissions } } @@ -139,8 +137,9 @@ export const getUnsavedChangesCount = ( }; // default permission mode is read -export const getPermissionModeId = (modes: WorkspacePermissionMode[]) => { - for (const key in OptionIdToWorkspacePermissionModesMap) { +export const getPermissionModeId = (modes: WorkspacePermissionMode[]): PermissionModeId => { + let key: PermissionModeId; + for (key in OptionIdToWorkspacePermissionModesMap) { if (OptionIdToWorkspacePermissionModesMap[key].every((mode) => modes?.includes(mode))) { return key; } From 7c4901935ab856ae4600b32855c49ca22371213f Mon Sep 17 00:00:00 2001 From: yuye-aws Date: Mon, 30 Oct 2023 11:45:46 +0800 Subject: [PATCH 16/21] support feature config match for unsaved changes count Signed-off-by: yuye-aws --- .../workspace_creator/utils.test.ts | 34 +++++++++++++++---- .../components/workspace_creator/utils.ts | 31 ++++++++++++----- .../workspace_creator/workspace_form.tsx | 3 +- 3 files changed, 53 insertions(+), 15 deletions(-) diff --git a/src/plugins/workspace/public/components/workspace_creator/utils.test.ts b/src/plugins/workspace/public/components/workspace_creator/utils.test.ts index b6ce81d5da62..a5e1cd8781f2 100644 --- a/src/plugins/workspace/public/components/workspace_creator/utils.test.ts +++ b/src/plugins/workspace/public/components/workspace_creator/utils.test.ts @@ -18,7 +18,7 @@ import { WorkspacePermissionItemType, WorkspacePermissionSetting, } from './types'; -import { WorkspacePermissionMode } from '../../../../../core/public'; +import { PublicAppInfo, WorkspacePermissionMode } from '../../../../../core/public'; import { PermissionModeId } from '../../../common/constants'; describe('isValidWorkspacePermissionSetting', () => { @@ -105,6 +105,13 @@ describe('getUserAndGroupPermissions', () => { }); describe('getUnsavedChangesCount', () => { + const allApplications = [ + { id: 'feature 1-1', category: { id: 'category 1' } }, + { id: 'feature 1-2', category: { id: 'category 1' } }, + { id: 'feature 2-1', category: { id: 'category 2' } }, + { id: 'feature 2-2', category: { id: 'category 2' } }, + ] as any; + it('should return number of unsaved changes in workspace metadata', () => { const initialFormData = { id: 'test workspace id', @@ -116,7 +123,7 @@ describe('getUnsavedChangesCount', () => { name: 'changed workspace name', description: 'changed workspace description', }; - expect(getUnsavedChangesCount(initialFormData, currentFormData)).toBe(2); + expect(getUnsavedChangesCount(initialFormData, currentFormData, allApplications)).toBe(2); }); it('should return number of unsaved changes in workspace features', () => { @@ -124,14 +131,29 @@ describe('getUnsavedChangesCount', () => { id: 'test workspace id', name: 'test workspace name', permissions: [], - features: ['feature 1', 'feature 2', 'feature 3'], + features: ['feature 1-1', 'feature 1-2', 'feature 2-1'], + }; + const currentFormData = { + name: 'test workspace name', + features: ['feature 1-1', 'feature 1-2', 'feature 2-2'], + }; + // 1 deleted feature and 1 added feature + expect(getUnsavedChangesCount(initialFormData, currentFormData, allApplications)).toBe(2); + }); + + it('should return number of unsaved changes in workspace features with feature configs', () => { + const initialFormData = { + id: 'test workspace id', + name: 'test workspace name', + permissions: [], + features: ['@category 1', '@category 2'], }; const currentFormData = { name: 'test workspace name', - features: ['feature 1', 'feature 2', 'feature 4'], + features: ['@category 1', '!@category 2'], }; // 1 deleted feature and 1 added feature - expect(getUnsavedChangesCount(initialFormData, currentFormData)).toBe(2); + expect(getUnsavedChangesCount(initialFormData, currentFormData, allApplications)).toBe(2); }); it('should return number of unsaved changes in workspace permissions', () => { @@ -162,7 +184,7 @@ describe('getUnsavedChangesCount', () => { groupPermissions: [], }; // 1 deleted permission and 1 edited permission - expect(getUnsavedChangesCount(initialFormData, currentFormData)).toBe(2); + expect(getUnsavedChangesCount(initialFormData, currentFormData, allApplications)).toBe(2); }); }); diff --git a/src/plugins/workspace/public/components/workspace_creator/utils.ts b/src/plugins/workspace/public/components/workspace_creator/utils.ts index 6f9a6b0ca98b..cdea021fb397 100644 --- a/src/plugins/workspace/public/components/workspace_creator/utils.ts +++ b/src/plugins/workspace/public/components/workspace_creator/utils.ts @@ -4,7 +4,8 @@ */ import { i18n } from '@osd/i18n'; -import { WorkspacePermissionMode } from '../../../../../core/public'; +import { PublicAppInfo, WorkspacePermissionMode } from '../../../../../core/public'; +import { featureMatchesConfig } from '../../utils'; import { OptionIdToWorkspacePermissionModesMap, PermissionModeId } from '../../../common/constants'; import { PermissionEditingData, @@ -57,6 +58,20 @@ export const getUserAndGroupPermissions = ( return [userPermissions, groupPermissions]; }; +const getUnsavedFeaturesCount = ( + initialFeatureConfig: string[], + currentFeatureConfig: string[], + allApplications: PublicAppInfo[] +) => { + // for features, unsaved changes is the sum of # deleted features and # added features + const initialFeatures = allApplications.filter(featureMatchesConfig(initialFeatureConfig)); + const currentFeatures = allApplications.filter(featureMatchesConfig(currentFeatureConfig)); + const featureIntersectionCount = ( + initialFeatures.filter((feature) => currentFeatures.includes(feature)) ?? [] + ).length; + return initialFeatures.length + currentFeatures.length - featureIntersectionCount * 2; +}; + const getUnsavedPermissionsCount = ( initialPermissions: PermissionEditingData, currentPermissions: PermissionEditingData @@ -94,7 +109,8 @@ const getUnsavedPermissionsCount = ( export const getUnsavedChangesCount = ( initialFormData: WorkspaceFormData, - currentFormData: WorkspaceFormEditingData + currentFormData: WorkspaceFormEditingData, + allApplications: PublicAppInfo[] ) => { let unsavedChangesCount = 0; if (initialFormData.name !== currentFormData.name) { @@ -115,12 +131,11 @@ export const getUnsavedChangesCount = ( if (initialFormData.defaultVISTheme !== currentFormData.defaultVISTheme) { unsavedChangesCount += 1; } - const featureIntersectionCount = ( - initialFormData.features?.filter((feature) => currentFormData.features?.includes(feature)) ?? [] - ).length; - // for features, unsaved changes is the sum of # deleted features and # added features - unsavedChangesCount += (initialFormData.features?.length ?? 0) - featureIntersectionCount; - unsavedChangesCount += (currentFormData.features?.length ?? 0) - featureIntersectionCount; + unsavedChangesCount += getUnsavedFeaturesCount( + initialFormData.features ?? [], + currentFormData.features ?? [], + allApplications + ); // for permissions, unsaved changes is the sum of # unsaved user permissions and # unsaved group permissions const [initialUserPermissions, initialGroupPermissions] = getUserAndGroupPermissions( initialFormData.permissions ?? [] diff --git a/src/plugins/workspace/public/components/workspace_creator/workspace_form.tsx b/src/plugins/workspace/public/components/workspace_creator/workspace_form.tsx index 61c2cd5d2cac..e59a1ebada6d 100644 --- a/src/plugins/workspace/public/components/workspace_creator/workspace_form.tsx +++ b/src/plugins/workspace/public/components/workspace_creator/workspace_form.tsx @@ -204,7 +204,7 @@ export const WorkspaceForm = ({ userPermissions, groupPermissions, }; - return getUnsavedChangesCount(defaultValues ?? ({} as any), currentFormData); + return getUnsavedChangesCount(defaultValues ?? ({} as any), currentFormData, applications); }, [ name, description, @@ -215,6 +215,7 @@ export const WorkspaceForm = ({ userPermissions, groupPermissions, defaultValues, + applications, ]); const featureOrGroups = useMemo(() => { From 13d2f85c2c30a65a4037524c7e11e57adf70fd91 Mon Sep 17 00:00:00 2001 From: yuye-aws Date: Mon, 30 Oct 2023 11:52:22 +0800 Subject: [PATCH 17/21] remove unused import Signed-off-by: yuye-aws --- .../workspace/public/components/workspace_creator/utils.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/plugins/workspace/public/components/workspace_creator/utils.test.ts b/src/plugins/workspace/public/components/workspace_creator/utils.test.ts index a5e1cd8781f2..6039ec99fb4a 100644 --- a/src/plugins/workspace/public/components/workspace_creator/utils.test.ts +++ b/src/plugins/workspace/public/components/workspace_creator/utils.test.ts @@ -18,7 +18,7 @@ import { WorkspacePermissionItemType, WorkspacePermissionSetting, } from './types'; -import { PublicAppInfo, WorkspacePermissionMode } from '../../../../../core/public'; +import { WorkspacePermissionMode } from '../../../../../core/public'; import { PermissionModeId } from '../../../common/constants'; describe('isValidWorkspacePermissionSetting', () => { From 85bb8834d3f53d6793c38d36890b0d23e21b5109 Mon Sep 17 00:00:00 2001 From: yuye-aws Date: Mon, 30 Oct 2023 15:15:19 +0800 Subject: [PATCH 18/21] avoid any when type casting Signed-off-by: yuye-aws --- .../components/workspace_creator/utils.test.ts | 12 ++++++------ .../components/workspace_creator/workspace_form.tsx | 6 +++++- 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/src/plugins/workspace/public/components/workspace_creator/utils.test.ts b/src/plugins/workspace/public/components/workspace_creator/utils.test.ts index 6039ec99fb4a..cc5c88347320 100644 --- a/src/plugins/workspace/public/components/workspace_creator/utils.test.ts +++ b/src/plugins/workspace/public/components/workspace_creator/utils.test.ts @@ -18,7 +18,7 @@ import { WorkspacePermissionItemType, WorkspacePermissionSetting, } from './types'; -import { WorkspacePermissionMode } from '../../../../../core/public'; +import { PublicAppInfo, WorkspacePermissionMode } from '../../../../../core/public'; import { PermissionModeId } from '../../../common/constants'; describe('isValidWorkspacePermissionSetting', () => { @@ -58,7 +58,7 @@ describe('isValidWorkspacePermissionSetting', () => { type: WorkspacePermissionItemType.Group, userId: 'test user id', modes: [], - } as any) + } as Partial) ).toBe(false); }); @@ -68,7 +68,7 @@ describe('isValidWorkspacePermissionSetting', () => { type: WorkspacePermissionItemType.User, group: 'test user id', modes: [], - } as any) + } as Partial) ).toBe(false); }); }); @@ -91,7 +91,7 @@ describe('getUserAndGroupPermissions', () => { { type: WorkspacePermissionItemType.User, userId: 'test user id 1', modes: [] }, { type: WorkspacePermissionItemType.Group, group: 'test group id 1', modes: [] }, { type: WorkspacePermissionItemType.Group, group: 'test group id 2', modes: [] }, - ] as any; + ] as WorkspacePermissionSetting[]; expect(getUserAndGroupPermissions(permissions)).toEqual( expect.arrayContaining([ [{ id: 'test user id 1', modes: [] }], @@ -110,7 +110,7 @@ describe('getUnsavedChangesCount', () => { { id: 'feature 1-2', category: { id: 'category 1' } }, { id: 'feature 2-1', category: { id: 'category 2' } }, { id: 'feature 2-2', category: { id: 'category 2' } }, - ] as any; + ] as PublicAppInfo[]; it('should return number of unsaved changes in workspace metadata', () => { const initialFormData = { @@ -180,7 +180,7 @@ describe('getUnsavedChangesCount', () => { id: 'test user id 1', modes: [WorkspacePermissionMode.LibraryWrite, WorkspacePermissionMode.Write], }, - ] as any, + ], groupPermissions: [], }; // 1 deleted permission and 1 edited permission diff --git a/src/plugins/workspace/public/components/workspace_creator/workspace_form.tsx b/src/plugins/workspace/public/components/workspace_creator/workspace_form.tsx index e59a1ebada6d..ecc80cf62d85 100644 --- a/src/plugins/workspace/public/components/workspace_creator/workspace_form.tsx +++ b/src/plugins/workspace/public/components/workspace_creator/workspace_form.tsx @@ -204,7 +204,11 @@ export const WorkspaceForm = ({ userPermissions, groupPermissions, }; - return getUnsavedChangesCount(defaultValues ?? ({} as any), currentFormData, applications); + return getUnsavedChangesCount( + defaultValues ?? ({} as WorkspaceFormData), + currentFormData, + applications + ); }, [ name, description, From 54a94924ba7aef24dd80fa7db49febf6d57ffdf6 Mon Sep 17 00:00:00 2001 From: yuye-aws Date: Mon, 30 Oct 2023 15:52:54 +0800 Subject: [PATCH 19/21] fix yarn start error Signed-off-by: yuye-aws --- src/plugins/workspace/common/constants.ts | 20 ---------------- .../workspace/common/permission/constants.ts | 24 +++++++++++++++++++ .../components/workspace_creator/utils.ts | 5 +++- .../workspace_permission_setting_panel.tsx | 12 +++++++--- 4 files changed, 37 insertions(+), 24 deletions(-) create mode 100644 src/plugins/workspace/common/permission/constants.ts diff --git a/src/plugins/workspace/common/constants.ts b/src/plugins/workspace/common/constants.ts index 1a1efa2870d0..ef0821c88619 100644 --- a/src/plugins/workspace/common/constants.ts +++ b/src/plugins/workspace/common/constants.ts @@ -3,8 +3,6 @@ * SPDX-License-Identifier: Apache-2.0 */ -import { WorkspacePermissionMode } from '../../../core/public'; - export const WORKSPACE_CREATE_APP_ID = 'workspace_create'; export const WORKSPACE_LIST_APP_ID = 'workspace_list'; export const WORKSPACE_UPDATE_APP_ID = 'workspace_update'; @@ -21,21 +19,3 @@ export const PATHS = { export const WORKSPACE_OP_TYPE_CREATE = 'create'; export const WORKSPACE_OP_TYPE_UPDATE = 'update'; export const WORKSPACE_SAVED_OBJECTS_CLIENT_WRAPPER_ID = 'workspace'; - -export enum PermissionModeId { - Read = 'read', - ReadAndWrite = 'read+write', - Admin = 'admin', -} - -export const OptionIdToWorkspacePermissionModesMap: Record< - PermissionModeId, - WorkspacePermissionMode[] -> = { - [PermissionModeId.Read]: [WorkspacePermissionMode.LibraryRead, WorkspacePermissionMode.Read], - [PermissionModeId.ReadAndWrite]: [ - WorkspacePermissionMode.LibraryWrite, - WorkspacePermissionMode.Read, - ], - [PermissionModeId.Admin]: [WorkspacePermissionMode.LibraryWrite, WorkspacePermissionMode.Write], -}; diff --git a/src/plugins/workspace/common/permission/constants.ts b/src/plugins/workspace/common/permission/constants.ts new file mode 100644 index 000000000000..8f7913853c5b --- /dev/null +++ b/src/plugins/workspace/common/permission/constants.ts @@ -0,0 +1,24 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +import { WorkspacePermissionMode } from '../../../../core/public'; + +export enum PermissionModeId { + Read = 'read', + ReadAndWrite = 'read+write', + Admin = 'admin', +} + +export const OptionIdToWorkspacePermissionModesMap: Record< + PermissionModeId, + WorkspacePermissionMode[] +> = { + [PermissionModeId.Read]: [WorkspacePermissionMode.LibraryRead, WorkspacePermissionMode.Read], + [PermissionModeId.ReadAndWrite]: [ + WorkspacePermissionMode.LibraryWrite, + WorkspacePermissionMode.Read, + ], + [PermissionModeId.Admin]: [WorkspacePermissionMode.LibraryWrite, WorkspacePermissionMode.Write], +}; diff --git a/src/plugins/workspace/public/components/workspace_creator/utils.ts b/src/plugins/workspace/public/components/workspace_creator/utils.ts index cdea021fb397..9e1a04a3c46c 100644 --- a/src/plugins/workspace/public/components/workspace_creator/utils.ts +++ b/src/plugins/workspace/public/components/workspace_creator/utils.ts @@ -6,7 +6,10 @@ import { i18n } from '@osd/i18n'; import { PublicAppInfo, WorkspacePermissionMode } from '../../../../../core/public'; import { featureMatchesConfig } from '../../utils'; -import { OptionIdToWorkspacePermissionModesMap, PermissionModeId } from '../../../common/constants'; +import { + OptionIdToWorkspacePermissionModesMap, + PermissionModeId, +} from '../../../common/permission/constants'; import { PermissionEditingData, WorkspaceFormEditingData, diff --git a/src/plugins/workspace/public/components/workspace_creator/workspace_permission_setting_panel.tsx b/src/plugins/workspace/public/components/workspace_creator/workspace_permission_setting_panel.tsx index e7c3240a1f1a..eaaaec7f1ba6 100644 --- a/src/plugins/workspace/public/components/workspace_creator/workspace_permission_setting_panel.tsx +++ b/src/plugins/workspace/public/components/workspace_creator/workspace_permission_setting_panel.tsx @@ -17,7 +17,10 @@ import { } from '@elastic/eui'; import { i18n } from '@osd/i18n'; import { WorkspacePermissionMode } from '../../../../../core/public'; -import { PermissionModeId, OptionIdToWorkspacePermissionModesMap } from '../../../common/constants'; +import { + PermissionModeId, + OptionIdToWorkspacePermissionModesMap, +} from '../../../common/permission/constants'; import { WorkspacePermissionItemType, PermissionEditingData } from './types'; import { getPermissionModeId } from './utils'; @@ -98,8 +101,11 @@ const WorkspacePermissionSettingInput = ({ const handlePermissionModeOptionChange = useCallback( (changedId: string) => { - if (OptionIdToWorkspacePermissionModesMap[changedId]) { - onPermissionModesChange([...OptionIdToWorkspacePermissionModesMap[changedId]], index); + if (OptionIdToWorkspacePermissionModesMap[changedId as PermissionModeId]) { + onPermissionModesChange( + [...OptionIdToWorkspacePermissionModesMap[changedId as PermissionModeId]], + index + ); } }, [index, onPermissionModesChange] From eb4f2feb683c27d8dd56ed7f47cf4ca296260c0b Mon Sep 17 00:00:00 2001 From: yuye-aws Date: Mon, 30 Oct 2023 16:01:15 +0800 Subject: [PATCH 20/21] fix import error Signed-off-by: yuye-aws --- .../workspace/public/components/workspace_creator/utils.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/plugins/workspace/public/components/workspace_creator/utils.test.ts b/src/plugins/workspace/public/components/workspace_creator/utils.test.ts index cc5c88347320..ec6e4e60e385 100644 --- a/src/plugins/workspace/public/components/workspace_creator/utils.test.ts +++ b/src/plugins/workspace/public/components/workspace_creator/utils.test.ts @@ -19,7 +19,7 @@ import { WorkspacePermissionSetting, } from './types'; import { PublicAppInfo, WorkspacePermissionMode } from '../../../../../core/public'; -import { PermissionModeId } from '../../../common/constants'; +import { PermissionModeId } from '../../../common/permission/constants'; describe('isValidWorkspacePermissionSetting', () => { it('should return true with valid user permission setting', () => { From 00502b99d90ad5a5e4a2e0f9fc01042ec03452b9 Mon Sep 17 00:00:00 2001 From: yuye-aws Date: Tue, 31 Oct 2023 07:27:18 +0800 Subject: [PATCH 21/21] optimize code Signed-off-by: yuye-aws --- src/core/public/workspace/workspaces_service.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/core/public/workspace/workspaces_service.ts b/src/core/public/workspace/workspaces_service.ts index f0ba51792cf9..d9d6664582b7 100644 --- a/src/core/public/workspace/workspaces_service.ts +++ b/src/core/public/workspace/workspaces_service.ts @@ -53,6 +53,7 @@ export class WorkspacesService implements CoreService { if (workspaceInitialized) { const currentWorkspace = workspaceList.find((w) => w && w.id === currentWorkspaceId); + /** * Do a simple idempotent verification here */