From fcc18fb2033f7216ca51854900946339d53bc772 Mon Sep 17 00:00:00 2001 From: Lin Wang Date: Fri, 11 Oct 2024 17:35:24 +0800 Subject: [PATCH] [Workspace]Remove collaborators in workspace creation page and refactor with getDisplayedType (#8520) * Remove workspace collaborators section in workspace creation Signed-off-by: Lin Wang * Redirect to workspace detail collaborators tab after workspace created Signed-off-by: Lin Wang * Changeset file for PR #8520 created/updated * Add getDisplayedType in default collaborator types Signed-off-by: Lin Wang * Update detail type to ReactNode Signed-off-by: Lin Wang * Add learn more link Signed-off-by: Lin Wang * Remove permissionEnabled passed in WorkspaceCreatorForm Signed-off-by: Lin Wang * Redirect to use case landing page if permission not enabled Signed-off-by: Lin Wang * Use getDisplayedType and render — for empty Signed-off-by: Lin Wang * Fix incorrect unit tests Signed-off-by: Lin Wang * Assign current user as workspace admin Signed-off-by: Lin Wang --------- Signed-off-by: Lin Wang Co-authored-by: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com> --- changelogs/fragments/8520.yml | 2 + .../add_collaborators_modal.test.tsx | 6 +- .../add_collaborators_modal.tsx | 16 +- .../workspace_creator.test.tsx | 142 +++++----- .../workspace_creator/workspace_creator.tsx | 58 +++- .../workspace_creator_form.tsx | 63 +---- .../workspace_form_summary_panel.test.tsx | 52 +--- .../workspace_form_summary_panel.tsx | 50 +--- ...workspace_collaborator_table.test.tsx.snap | 258 +++++++++++++++--- .../public/components/workspace_form/index.ts | 7 +- .../workspace_form/use_workspace_form.ts | 21 +- .../public/components/workspace_form/utils.ts | 48 +--- .../workspace_collaborator_table.test.tsx | 51 +++- .../workspace_collaborator_table.tsx | 83 ++++-- .../workspace_form/workspace_detail_form.tsx | 10 +- ...gister_default_collaborator_types.test.tsx | 28 +- .../register_default_collaborator_types.tsx | 12 + 17 files changed, 550 insertions(+), 357 deletions(-) create mode 100644 changelogs/fragments/8520.yml diff --git a/changelogs/fragments/8520.yml b/changelogs/fragments/8520.yml new file mode 100644 index 000000000000..f0883088331f --- /dev/null +++ b/changelogs/fragments/8520.yml @@ -0,0 +1,2 @@ +feat: +- [Workspace]Remove collaborators in workspace creation page ([#8520](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/8520)) \ No newline at end of file diff --git a/src/plugins/workspace/public/components/add_collaborators_modal/add_collaborators_modal.test.tsx b/src/plugins/workspace/public/components/add_collaborators_modal/add_collaborators_modal.test.tsx index 17d0818f37b1..3cf1c9203760 100644 --- a/src/plugins/workspace/public/components/add_collaborators_modal/add_collaborators_modal.test.tsx +++ b/src/plugins/workspace/public/components/add_collaborators_modal/add_collaborators_modal.test.tsx @@ -12,7 +12,7 @@ describe('AddCollaboratorsModal', () => { title: 'Add Collaborators', inputLabel: 'Collaborator ID', addAnotherButtonLabel: 'Add Another', - permissionType: 'readOnly', + permissionType: 'user' as const, onClose: jest.fn(), onAddCollaborators: jest.fn(), }; @@ -46,7 +46,7 @@ describe('AddCollaboratorsModal', () => { fireEvent.click(addCollaboratorsButton); await waitFor(() => { expect(defaultProps.onAddCollaborators).toHaveBeenCalledWith([ - { collaboratorId: 'user1', accessLevel: 'readOnly', permissionType: 'readOnly' }, + { collaboratorId: 'user1', accessLevel: 'readOnly', permissionType: 'user' }, ]); }); }); @@ -68,10 +68,12 @@ describe('AddCollaboratorsModal', () => { const instruction = { title: 'Instructions', detail: 'Follow these instructions to add collaborators', + link: 'foo', }; const props = { ...defaultProps, instruction }; render(); expect(screen.getByText(instruction.title)).toBeInTheDocument(); expect(screen.getByText(instruction.detail)).toBeInTheDocument(); + expect(screen.getByText('Learn more in Documentation')).toBeInTheDocument(); }); }); diff --git a/src/plugins/workspace/public/components/add_collaborators_modal/add_collaborators_modal.tsx b/src/plugins/workspace/public/components/add_collaborators_modal/add_collaborators_modal.tsx index 6c69d17886a4..48ba9c62ed7d 100644 --- a/src/plugins/workspace/public/components/add_collaborators_modal/add_collaborators_modal.tsx +++ b/src/plugins/workspace/public/components/add_collaborators_modal/add_collaborators_modal.tsx @@ -6,6 +6,7 @@ import { EuiAccordion, EuiHorizontalRule, + EuiLink, EuiModal, EuiModalBody, EuiModalFooter, @@ -34,7 +35,7 @@ export interface AddCollaboratorsModalProps { inputPlaceholder?: string; instruction?: { title: string; - detail: string; + detail: React.ReactNode; link?: string; }; permissionType: WorkspaceCollaboratorPermissionType; @@ -91,6 +92,19 @@ export const AddCollaboratorsModal = ({ {instruction.detail} + {instruction.link && ( + <> + + + {i18n.translate( + 'workspace.addCollaboratorsModal.instruction.learnMoreLinkText', + { + defaultMessage: 'Learn more in Documentation', + } + )} + + + )} 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 95797ffe81c8..dc4bf6f8f9f6 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 @@ -18,6 +18,7 @@ import { import { DataSourceEngineType } from '../../../../data_source/common/data_sources'; import { DataSourceConnectionType } from '../../../common/types'; import * as utils from '../../utils'; +import * as workspaceUtilsExports from '../utils/workspace'; jest.mock('react-router-dom', () => ({ ...jest.requireActual('react-router-dom'), @@ -113,8 +114,15 @@ jest.spyOn(utils, 'fetchDataSourceConnections').mockImplementation(async (passed const WorkspaceCreator = ({ isDashboardAdmin = false, dataSourceEnabled = false, + isPermissionEnabled = true, ...props -}: Partial) => { +}: Partial< + WorkspaceCreatorProps & { + isDashboardAdmin: boolean; + dataSourceEnabled?: boolean; + isPermissionEnabled?: boolean; + } +>) => { const { Provider } = createOpenSearchDashboardsReactContext({ ...mockCoreStart, ...{ @@ -123,7 +131,7 @@ const WorkspaceCreator = ({ capabilities: { ...mockCoreStart.application.capabilities, workspaces: { - permissionEnabled: true, + permissionEnabled: isPermissionEnabled, }, dashboards: { isDashboardAdmin, @@ -273,8 +281,12 @@ describe('WorkspaceCreator', () => { dataSources: [], dataConnections: [], permissions: { - library_write: { users: ['%me%'] }, - write: { users: ['%me%'] }, + library_write: { + users: ['%me%'], + }, + write: { + users: ['%me%'], + }, }, } ); @@ -328,43 +340,6 @@ describe('WorkspaceCreator', () => { expect(notificationToastsAddSuccess).not.toHaveBeenCalled(); }); - it('create workspace with customized permissions', async () => { - const { getByTestId } = render(); - - // Ensure workspace create form rendered - await waitFor(() => { - expect(getByTestId('workspaceForm-bottomBar-createButton')).toBeInTheDocument(); - }); - const nameInput = getByTestId('workspaceForm-workspaceDetails-nameInputText'); - fireEvent.input(nameInput, { - target: { value: 'test workspace name' }, - }); - fireEvent.click(getByTestId('workspaceUseCase-observability')); - fireEvent.click(getByTestId('workspaceForm-permissionSettingPanel-addNew')); - fireEvent.click(getByTestId('workspaceForm-bottomBar-createButton')); - expect(workspaceClientCreate).toHaveBeenCalledWith( - expect.objectContaining({ - name: 'test workspace name', - }), - { - dataConnections: [], - dataSources: [], - permissions: { - write: { - users: ['%me%'], - }, - library_write: { - users: ['%me%'], - }, - }, - } - ); - await waitFor(() => { - expect(notificationToastsAddSuccess).toHaveBeenCalled(); - }); - expect(notificationToastsAddDanger).not.toHaveBeenCalled(); - }); - it('create workspace with customized selected dataSources', async () => { Object.defineProperty(HTMLElement.prototype, 'offsetHeight', { configurable: true, @@ -404,18 +379,10 @@ describe('WorkspaceCreator', () => { expect.objectContaining({ name: 'test workspace name', }), - { + expect.objectContaining({ dataConnections: [], dataSources: ['id1'], - permissions: { - library_write: { - users: ['%me%'], - }, - write: { - users: ['%me%'], - }, - }, - } + }) ); await waitFor(() => { expect(notificationToastsAddSuccess).toHaveBeenCalled(); @@ -462,17 +429,40 @@ describe('WorkspaceCreator', () => { expect.objectContaining({ name: 'test workspace name', }), - { + expect.objectContaining({ dataConnections: ['id3'], dataSources: [], - permissions: { - library_write: { - users: ['%me%'], - }, - write: { - users: ['%me%'], - }, - }, + }) + ); + await waitFor(() => { + expect(notificationToastsAddSuccess).toHaveBeenCalled(); + }); + expect(notificationToastsAddDanger).not.toHaveBeenCalled(); + }); + + it('should not include permissions parameter if permissions not enabled', async () => { + const { getByTestId } = render( + + ); + + // Ensure workspace create form rendered + await waitFor(() => { + expect(getByTestId('workspaceForm-bottomBar-createButton')).toBeInTheDocument(); + }); + const nameInput = getByTestId('workspaceForm-workspaceDetails-nameInputText'); + fireEvent.input(nameInput, { + target: { value: 'test workspace name' }, + }); + fireEvent.click(getByTestId('workspaceUseCase-observability')); + + fireEvent.click(getByTestId('workspaceForm-bottomBar-createButton')); + expect(workspaceClientCreate).toHaveBeenCalledWith( + expect.objectContaining({ + name: 'test workspace name', + }), + { + dataConnections: [], + dataSources: [], } ); await waitFor(() => { @@ -510,8 +500,8 @@ describe('WorkspaceCreator', () => { }); }); - it('should redirect to workspace use case landing page after created successfully', async () => { - const { getByTestId } = render(); + it('should redirect to workspace use case landing page if permission not enabled', async () => { + const { getByTestId } = render(); // Ensure workspace create form rendered await waitFor(() => { @@ -529,4 +519,32 @@ describe('WorkspaceCreator', () => { }); jest.useRealTimers(); }); + + it('should redirect to workspace setting collaborators tab if permission enabled', async () => { + const { getByTestId } = render(); + const navigateToWorkspaceDetailMock = jest.fn(); + jest + .spyOn(workspaceUtilsExports, 'navigateToWorkspaceDetail') + .mockImplementation(navigateToWorkspaceDetailMock); + + // Ensure workspace create form rendered + await waitFor(() => { + expect(getByTestId('workspaceForm-bottomBar-createButton')).toBeInTheDocument(); + }); + const nameInput = getByTestId('workspaceForm-workspaceDetails-nameInputText'); + fireEvent.input(nameInput, { + target: { value: 'test workspace name' }, + }); + fireEvent.click(getByTestId('workspaceForm-bottomBar-createButton')); + jest.useFakeTimers(); + jest.runAllTimers(); + await waitFor(() => { + expect(navigateToWorkspaceDetailMock).toHaveBeenCalledWith( + expect.anything(), + 'successResult', + 'collaborators' + ); + }); + jest.useRealTimers(); + }); }); 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 9b51ba22d0fe..061649304331 100644 --- a/src/plugins/workspace/public/components/workspace_creator/workspace_creator.tsx +++ b/src/plugins/workspace/public/components/workspace_creator/workspace_creator.tsx @@ -7,22 +7,31 @@ import React, { useCallback, useState, useMemo } from 'react'; import { EuiPage, EuiPageBody, EuiPageContent, euiPaletteColorBlind } from '@elastic/eui'; import { i18n } from '@osd/i18n'; import { BehaviorSubject } from 'rxjs'; - import { useLocation } from 'react-router-dom'; + import { useOpenSearchDashboards } from '../../../../opensearch_dashboards_react/public'; -import { WorkspaceFormSubmitData, WorkspaceOperationType } from '../workspace_form'; -import { WORKSPACE_DETAIL_APP_ID } from '../../../common/constants'; +import { PermissionModeId } from '../../../../../core/public'; +import { CURRENT_USER_PLACEHOLDER, WORKSPACE_DETAIL_APP_ID } from '../../../common/constants'; +import { + WorkspaceFormSubmitData, + WorkspaceOperationType, + DetailTab, + WorkspacePermissionItemType, + convertPermissionSettingsToPermissions, + WorkspacePermissionSetting, +} from '../workspace_form'; import { getUseCaseFeatureConfig } from '../../../common/utils'; import { formatUrlWithWorkspaceId } from '../../../../../core/public/utils'; import { WorkspaceClient } from '../../workspace_client'; -import { convertPermissionSettingsToPermissions } from '../workspace_form'; import { DataSourceManagementPluginSetup } from '../../../../../plugins/data_source_management/public'; import { WorkspaceUseCase } from '../../types'; import { getFirstUseCaseOfFeatureConfigs } from '../../utils'; import { useFormAvailableUseCases } from '../workspace_form/use_form_available_use_cases'; import { NavigationPublicPluginStart } from '../../../../../plugins/navigation/public'; import { DataSourceConnectionType } from '../../../common/types'; +import { navigateToWorkspaceDetail } from '../utils/workspace'; import { WorkspaceCreatorForm } from './workspace_creator_form'; +import { optionIdToWorkspacePermissionModesMap } from '../workspace_form/constants'; export interface WorkspaceCreatorProps { registeredUseCases$: BehaviorSubject; @@ -46,8 +55,8 @@ export const WorkspaceCreator = (props: WorkspaceCreatorProps) => { navigationUI: NavigationPublicPluginStart['ui']; }>(); const [isFormSubmitting, setIsFormSubmitting] = useState(false); - const isPermissionEnabled = application?.capabilities.workspaces.permissionEnabled; + const { isOnlyAllowEssential, availableUseCases } = useFormAvailableUseCases({ savedObjects, registeredUseCases$, @@ -73,8 +82,20 @@ export const WorkspaceCreator = (props: WorkspaceCreatorProps) => { features: [getUseCaseFeatureConfig(defaultSelectedUseCase.id)], } : {}), + ...(isPermissionEnabled + ? { + permissionSettings: [ + { + id: 1, + type: WorkspacePermissionItemType.User, + userId: CURRENT_USER_PLACEHOLDER, + modes: optionIdToWorkspacePermissionModesMap[PermissionModeId.Owner], + }, + ] as WorkspacePermissionSetting[], + } + : {}), }; - }, [location.search, availableUseCases]); + }, [location.search, availableUseCases, isPermissionEnabled]); const handleWorkspaceFormSubmit = useCallback( async (data: WorkspaceFormSubmitData) => { @@ -102,7 +123,11 @@ export const WorkspaceCreator = (props: WorkspaceCreatorProps) => { result = await workspaceClient.create(attributes, { dataSources: selectedDataSourceIds, dataConnections: selectedDataConnectionIds, - permissions: convertPermissionSettingsToPermissions(permissionSettings), + ...(isPermissionEnabled + ? { + permissions: convertPermissionSettingsToPermissions(permissionSettings), + } + : {}), }); if (result?.success) { notifications?.toasts.addSuccess({ @@ -117,6 +142,14 @@ export const WorkspaceCreator = (props: WorkspaceCreatorProps) => { ?.features[0].id; // Redirect page after one second, leave one second time to show create successful toast. window.setTimeout(() => { + if (isPermissionEnabled) { + navigateToWorkspaceDetail( + { application, http }, + newWorkspaceId, + DetailTab.Collaborators + ); + return; + } window.location.href = formatUrlWithWorkspaceId( application.getUrlForApp(useCaseLandingAppId || WORKSPACE_DETAIL_APP_ID, { absolute: true, @@ -142,7 +175,15 @@ export const WorkspaceCreator = (props: WorkspaceCreatorProps) => { setIsFormSubmitting(false); } }, - [notifications?.toasts, http, application, workspaceClient, isFormSubmitting, availableUseCases] + [ + notifications?.toasts, + http, + application, + workspaceClient, + isFormSubmitting, + availableUseCases, + isPermissionEnabled, + ] ); const isFormReadyToRender = @@ -177,7 +218,6 @@ export const WorkspaceCreator = (props: WorkspaceCreatorProps) => { savedObjects={savedObjects} onSubmit={handleWorkspaceFormSubmit} operationType={WorkspaceOperationType.Create} - permissionEnabled={isPermissionEnabled} dataSourceManagement={dataSourceManagement} availableUseCases={availableUseCases} defaultValues={defaultWorkspaceFormValues} diff --git a/src/plugins/workspace/public/components/workspace_creator/workspace_creator_form.tsx b/src/plugins/workspace/public/components/workspace_creator/workspace_creator_form.tsx index 52e2dd4e840e..16d53e798d62 100644 --- a/src/plugins/workspace/public/components/workspace_creator/workspace_creator_form.tsx +++ b/src/plugins/workspace/public/components/workspace_creator/workspace_creator_form.tsx @@ -3,24 +3,14 @@ * SPDX-License-Identifier: Apache-2.0 */ -import React, { useCallback, useRef } from 'react'; -import { - EuiSpacer, - EuiForm, - EuiText, - EuiFlexItem, - EuiFlexGroup, - EuiDescribedFormGroup, - EuiPanel, -} from '@elastic/eui'; +import React from 'react'; +import { EuiSpacer, EuiForm, EuiText, EuiFlexItem, EuiFlexGroup, EuiPanel } from '@elastic/eui'; import { i18n } from '@osd/i18n'; import { useWorkspaceForm, - WorkspacePermissionSettingPanel, WorkspaceUseCase, WorkspaceFormErrorCallout, SelectDataSourcePanel, - usersAndPermissionsCreatePageTitle, WorkspaceFormProps, } from '../workspace_form'; @@ -38,8 +28,6 @@ export const WorkspaceCreatorForm = (props: WorkspaceCreatorFormProps) => { const { application, savedObjects, - defaultValues, - permissionEnabled, dataSourceManagement: isDataSourceEnabled, availableUseCases, } = props; @@ -53,13 +41,9 @@ export const WorkspaceCreatorForm = (props: WorkspaceCreatorFormProps) => { handleFormSubmit, handleColorChange, handleUseCaseChange, - setPermissionSettings, setSelectedDataSourceConnections, } = useWorkspaceForm(props); - const disabledUserOrGroupInputIdsRef = useRef( - defaultValues?.permissionSettings?.map((item) => item.id) ?? [] - ); const isDashboardAdmin = application?.capabilities?.dashboards?.isDashboardAdmin ?? false; return ( @@ -123,50 +107,8 @@ export const WorkspaceCreatorForm = (props: WorkspaceCreatorFormProps) => { showDataSourceManagement={true} /> - - )} - {permissionEnabled && ( - - -

{usersAndPermissionsCreatePageTitle}

- - - {i18n.translate('workspace.creator.form.usersAndPermissionsDescription', { - defaultMessage: 'Manage access and permissions', - })} - - - - {i18n.translate('workspace.creator.collaborators.panel.fields.name.title', { - defaultMessage: 'Workspace access', - })} - - } - description={i18n.translate( - 'workspace.creator.collaborators.panel.fields.name.description', - { - defaultMessage: - 'You will be added as an owner to the workspace. Select additional users and user groups as workspace collaborators with different access levels.', - } - )} - > - - - - )} @@ -175,7 +117,6 @@ export const WorkspaceCreatorForm = (props: WorkspaceCreatorFormProps) => { { const formData = { @@ -38,25 +36,11 @@ describe('WorkspaceFormSummaryPanel', () => { type: '', connectionType: DataSourceConnectionType.OpenSearchConnection, }, - ], - permissionSettings: [ - { - id: 1, - type: WorkspacePermissionItemType.User, - userId: 'user1', - modes: [WorkspacePermissionMode.LibraryWrite, WorkspacePermissionMode.Write], - }, { - id: 2, - type: WorkspacePermissionItemType.Group, - group: 'group1', - modes: [WorkspacePermissionMode.LibraryRead, WorkspacePermissionMode.Read], - }, - { - id: 3, - type: WorkspacePermissionItemType.User, - userId: 'user2', - modes: [WorkspacePermissionMode.LibraryWrite, WorkspacePermissionMode.Read], + id: 'data-source-4', + name: 'Data Source 4', + type: '', + connectionType: DataSourceConnectionType.OpenSearchConnection, }, ], }; @@ -84,7 +68,6 @@ describe('WorkspaceFormSummaryPanel', () => { { expect(screen.getByText('Data Source 1')).toBeInTheDocument(); expect(screen.getByText('Data Source 2')).toBeInTheDocument(); expect(screen.getByText('Data Source 3')).toBeInTheDocument(); - expect(screen.getByText('user1')).toBeInTheDocument(); - expect(screen.getByText('Admin')).toBeInTheDocument(); - expect(screen.getByText('group1')).toBeInTheDocument(); - expect(screen.getByText('Read only')).toBeInTheDocument(); - expect(screen.getByText('+1 more')).toBeInTheDocument(); - expect(screen.queryByText('user2')).toBeNull(); expect(screen.getByText('Cancel')).toBeInTheDocument(); expect(screen.getByText('Create workspace')).toBeInTheDocument(); }); @@ -116,12 +93,10 @@ describe('WorkspaceFormSummaryPanel', () => { formData={{ name: '', selectedDataSourceConnections: [], - permissionSettings: [], features: [], useCase: undefined, }} availableUseCases={availableUseCases} - permissionEnabled formId="id" application={applicationMock} isSubmitting={false} @@ -150,12 +125,6 @@ describe('WorkspaceFormSummaryPanel', () => { 'workspaceFormRightSideBarSummary-dataSource-Value' ); expect(dataSourcesPlaceholder).toHaveTextContent('—'); - - // Permissions placeholder - const permissionsPlaceholder = screen.getByTestId( - 'workspaceFormRightSideBarSummary-collaborators-Value' - ); - expect(permissionsPlaceholder).toHaveTextContent('—'); }); it('renders all items when expanded and hide some items when click show less', () => { @@ -163,23 +132,23 @@ describe('WorkspaceFormSummaryPanel', () => { ); - expect(screen.getByText('user1')).toBeInTheDocument(); - expect(screen.getByText('group1')).toBeInTheDocument(); - expect(screen.queryByText('user2')).toBeNull(); + expect(screen.getByText('Data Source 1')).toBeInTheDocument(); + expect(screen.getByText('Data Source 2')).toBeInTheDocument(); + expect(screen.getByText('Data Source 3')).toBeInTheDocument(); + expect(screen.queryByText('Data Source 4')).toBeNull(); fireEvent.click(screen.getByText('+1 more')); - expect(screen.getByText('user2')).toBeInTheDocument(); + expect(screen.getByText('Data Source 4')).toBeInTheDocument(); expect(screen.getByText('Show less')).toBeInTheDocument(); fireEvent.click(screen.getByText('Show less')); - expect(screen.queryByText('user2')).toBeNull(); + expect(screen.queryByText('Data Source 4')).toBeNull(); }); it('should hide "Data sources" if data source not enabled', () => { @@ -187,7 +156,6 @@ describe('WorkspaceFormSummaryPanel', () => { { - return userAndGroups.map((userAndGroup) => { - return ( - - {userAndGroup.key} - {userAndGroup.modes && ( - - {getPermissionModeName(userAndGroup.modes)} - - )} - - ); - }); -}; - interface WorkspaceFormSummaryPanelProps { - formData: WorkspaceFormDataState; + formData: Omit; availableUseCases: WorkspaceUseCase[]; - permissionEnabled?: boolean; formId: string; application: ApplicationStart; isSubmitting: boolean; @@ -171,23 +144,12 @@ interface WorkspaceFormSummaryPanelProps { export const WorkspaceFormSummaryPanel = ({ formData, availableUseCases, - permissionEnabled, formId, application, isSubmitting, dataSourceEnabled, }: WorkspaceFormSummaryPanelProps) => { const useCase = availableUseCases.find((item) => item.id === formData.useCase); - const userAndGroups: UserAndGroups[] = formData.permissionSettings.flatMap((setting) => { - const modesExist = 'modes' in setting && !!setting.modes; - if ('userId' in setting && !!setting.userId && modesExist) { - return [{ key: setting.userId, modes: setting.modes }]; - } - if ('group' in setting && !!setting.group && modesExist) { - return [{ key: setting.group, modes: setting.modes }]; - } - return []; - }); const useCaseIcon = useCase?.icon || 'logoOpenSearch'; return ( @@ -229,16 +191,6 @@ export const WorkspaceFormSummaryPanel = ({ )} )} - {permissionEnabled && ( - - {userAndGroups.length > 0 && ( - - )} - - )}
- - User - + User
- - Admin - + Admin
Type +
+ Group +
+ + +
+ Access level +
+
+ Read only +
+ + +
+ Actions +
+
+
+
+ +
+
+
+ + + +
- - Group - + +
+
- Access level + ID
- Read only - + /> +
+ + +
+ Type +
+
+ — +
+ + +
+ Access level +
+
+ —
- - User - + User
- - Admin - + Admin
Type +
+ Group +
+ + +
+ Access level +
+
+ Read only +
+ + +
+ Actions +
+
+
+
+ +
+
+
+ + + +
- - Group - + +
+
- Access level + ID
- Read only - + /> +
+ + +
+ Type +
+
+ — +
+ + +
+ Access level +
+
+ —
(defaultValues?.features ?? []); const selectedUseCase = useMemo(() => getFirstUseCaseOfFeatureConfigs(featureConfigs), [ @@ -50,7 +41,7 @@ export const useWorkspaceForm = ({ ]); const [permissionSettings, setPermissionSettings] = useState< WorkspaceFormDataState['permissionSettings'] - >(initialPermissionSettingsRef.current); + >(defaultValues?.permissionSettings ?? []); const [selectedDataSourceConnections, setSelectedDataSourceConnections] = useState< DataSourceConnection[] @@ -77,11 +68,7 @@ export const useWorkspaceForm = ({ getFormDataRef.current = getFormData; const formData = getFormData(); const numberOfChanges = defaultValuesRef.current - ? getNumberOfChanges(formData, { - ...defaultValuesRef.current, - // The user form will insert some empty permission rows, should ignore these rows not treated as user new added. - permissionSettings: initialPermissionSettingsRef.current, - }) + ? getNumberOfChanges(formData, defaultValuesRef.current) : 0; if (!formIdRef.current) { @@ -163,7 +150,7 @@ export const useWorkspaceForm = ({ setDescription(resetValues?.description ?? ''); setColor(resetValues?.color); setFeatureConfigs(resetValues?.features ?? []); - setPermissionSettings(initialPermissionSettingsRef.current); + setPermissionSettings(defaultValuesRef.current?.permissionSettings ?? []); setFormErrors({}); setIsEditing(false); }, []); diff --git a/src/plugins/workspace/public/components/workspace_form/utils.ts b/src/plugins/workspace/public/components/workspace_form/utils.ts index a1fdb319e1aa..9d0e2a47cbc3 100644 --- a/src/plugins/workspace/public/components/workspace_form/utils.ts +++ b/src/plugins/workspace/public/components/workspace_form/utils.ts @@ -6,12 +6,11 @@ import { i18n } from '@osd/i18n'; import type { SavedObjectPermissions } from '../../../../../core/types'; -import { CURRENT_USER_PLACEHOLDER, WorkspacePermissionMode } from '../../../common/constants'; +import { WorkspacePermissionMode } from '../../../common/constants'; import { isUseCaseFeatureConfig } from '../../utils'; import { optionIdToWorkspacePermissionModesMap, permissionModeOptions, - WorkspaceOperationType, WorkspacePermissionItemType, } from './constants'; @@ -85,7 +84,7 @@ export const hasSameUserIdOrGroup = ( export const getPermissionModeId = (modes: WorkspacePermissionMode[]) => { for (const key in optionIdToWorkspacePermissionModesMap) { if (optionIdToWorkspacePermissionModesMap[key].every((mode) => modes?.includes(mode))) { - return key; + return key as PermissionModeId; } } return PermissionModeId.Read; @@ -382,49 +381,6 @@ export const generateNextPermissionSettingsId = (permissionSettings: Array<{ id: : Math.max(...permissionSettings.map(({ id }) => id)) + 1; }; -/** - * - * Generate permission settings state with provided operation type and permission settings, - * It will always return current user as an Owner and an empty user group permission settings - * when operation type is create or no users and groups in provided permission settings. - * It will append current user to result permission settings if no user in provided permission settings. - * It will append an empty permission group to result permission settings if no user group in provided permission settings. - * It will always return original permission settings if both user or user group in provided permission settings. - * - * @param operationType - * @param permissionSettings - * @returns - */ -export const generatePermissionSettingsState = ( - operationType: WorkspaceOperationType, - permissionSettings?: WorkspacePermissionSetting[] -): WorkspacePermissionSetting[] => { - const emptyUserPermission: WorkspaceUserPermissionSetting = { - id: 1, - type: WorkspacePermissionItemType.User, - userId: '', - modes: optionIdToWorkspacePermissionModesMap[PermissionModeId.Owner], - }; - const emptyUserGroupPermission: WorkspaceUserGroupPermissionSetting = { - id: 2, - type: WorkspacePermissionItemType.Group, - group: '', - modes: optionIdToWorkspacePermissionModesMap[PermissionModeId.Read], - }; - - if (operationType === WorkspaceOperationType.Create) { - return [ - { - ...emptyUserPermission, - userId: CURRENT_USER_PLACEHOLDER, - }, - emptyUserGroupPermission, - ]; - } - - return [...(permissionSettings ?? [])]; -}; - interface PermissionSettingLike extends Omit, 'type'>, Omit, 'type'> { diff --git a/src/plugins/workspace/public/components/workspace_form/workspace_collaborator_table.test.tsx b/src/plugins/workspace/public/components/workspace_form/workspace_collaborator_table.test.tsx index 1b4e514e62c8..dcb6c707e25c 100644 --- a/src/plugins/workspace/public/components/workspace_form/workspace_collaborator_table.test.tsx +++ b/src/plugins/workspace/public/components/workspace_form/workspace_collaborator_table.test.tsx @@ -6,11 +6,27 @@ import React from 'react'; import { fireEvent, render } from '@testing-library/react'; -import { WorkspaceCollaboratorTable } from './workspace_collaborator_table'; +import { WorkspaceCollaboratorTable, getDisplayedType } from './workspace_collaborator_table'; import { createOpenSearchDashboardsReactContext } from '../../../../opensearch_dashboards_react/public'; import { coreMock } from '../../../../../core/public/mocks'; const mockCoreStart = coreMock.createStart(); +const displayedCollaboratorTypes = [ + { + id: 'user', + name: 'User', + buttonLabel: 'Add Users', + onAdd: async () => {}, + getDisplayedType: ({ permissionType }) => (permissionType === 'user' ? 'User' : undefined), + }, + { + id: 'group', + name: 'Group', + buttonLabel: 'Add Groups', + onAdd: async () => {}, + getDisplayedType: ({ permissionType }) => (permissionType === 'group' ? 'Group' : undefined), + }, +]; const mockOverlays = { openModal: jest.fn(), @@ -21,9 +37,35 @@ const { Provider } = createOpenSearchDashboardsReactContext({ overlays: mockOverlays, }); +describe('getDisplayedTypes', () => { + it('should return undefined if not match any collaborator type', () => { + expect( + getDisplayedType(displayedCollaboratorTypes, { permissionType: 'unknown' }) + ).toBeUndefined(); + }); + it('should return "User"', () => { + expect( + getDisplayedType(displayedCollaboratorTypes, { + collaboratorId: 'foo', + permissionType: 'user', + accessLevel: 'readOnly', + }) + ).toEqual('User'); + }); + it('should return "Group"', () => { + expect( + getDisplayedType(displayedCollaboratorTypes, { + collaboratorId: 'foo', + permissionType: 'group', + accessLevel: 'readOnly', + }) + ).toEqual('Group'); + }); +}); + describe('WorkspaceCollaboratorTable', () => { const mockProps = { - displayedCollaboratorTypes: [], + displayedCollaboratorTypes, permissionSettings: [ { id: 0, @@ -37,6 +79,11 @@ describe('WorkspaceCollaboratorTable', () => { type: 'group', group: 'group', }, + { + id: 2, + modes: ['library_read', 'read'], + type: 'unknown', + }, ], handleSubmitPermissionSettings: jest.fn(), }; diff --git a/src/plugins/workspace/public/components/workspace_form/workspace_collaborator_table.tsx b/src/plugins/workspace/public/components/workspace_form/workspace_collaborator_table.tsx index b0f537c30c05..f87ee2e894c5 100644 --- a/src/plugins/workspace/public/components/workspace_form/workspace_collaborator_table.tsx +++ b/src/plugins/workspace/public/components/workspace_form/workspace_collaborator_table.tsx @@ -22,21 +22,55 @@ import { } from '@elastic/eui'; import { i18n } from '@osd/i18n'; import { WorkspacePermissionSetting } from './types'; -import { WorkspacePermissionItemType, permissionModeOptions, typeOptions } from './constants'; -import { getPermissionModeId } from './utils'; +import { WorkspacePermissionItemType } from './constants'; +import { getPermissionModeId, isWorkspacePermissionSetting } from './utils'; import { useOpenSearchDashboards } from '../../../../opensearch_dashboards_react/public'; +import { PermissionModeId } from '../../../../../core/public'; import { AddCollaboratorButton } from './add_collaborator_button'; import { WorkspaceCollaboratorType } from '../../services/workspace_collaborator_types_service'; import { WORKSPACE_ACCESS_LEVEL_NAMES, accessLevelNameToWorkspacePermissionModesMap, } from '../../constants'; -import { WorkspaceCollaboratorAccessLevel } from '../../types'; +import { WorkspaceCollaborator, WorkspaceCollaboratorAccessLevel } from '../../types'; import { BackgroundPic } from '../../assets/background_pic'; export type PermissionSetting = Pick & Partial; +// TODO: Update PermissionModeId to align with WorkspaceCollaboratorAccessLevel +const permissionModeId2WorkspaceAccessLevelMap: { + [key in PermissionModeId]: WorkspaceCollaboratorAccessLevel; +} = { + [PermissionModeId.Owner]: 'admin', + [PermissionModeId.Read]: 'readOnly', + [PermissionModeId.ReadAndWrite]: 'readAndWrite', +}; + +const convertPermissionSettingToWorkspaceCollaborator = ( + permissionSetting: WorkspacePermissionSetting +) => ({ + collaboratorId: + permissionSetting.type === WorkspacePermissionItemType.User + ? permissionSetting.userId + : permissionSetting.group, + permissionType: permissionSetting.type, + accessLevel: + permissionModeId2WorkspaceAccessLevelMap[getPermissionModeId(permissionSetting.modes)], +}); + +export const getDisplayedType = ( + supportCollaboratorTypes: WorkspaceCollaboratorType[], + collaborator: WorkspaceCollaborator +) => { + for (const collaboratorType of supportCollaboratorTypes) { + const displayedType = collaboratorType.getDisplayedType?.(collaborator); + if (displayedType) { + return displayedType; + } + } +}; + interface Props { permissionSettings: PermissionSetting[]; displayedCollaboratorTypes: WorkspaceCollaboratorType[]; @@ -53,15 +87,18 @@ export const WorkspaceCollaboratorTable = ({ const items = useMemo(() => { return permissionSettings.map((setting) => { + const collaborator = isWorkspacePermissionSetting(setting) + ? convertPermissionSettingToWorkspaceCollaborator(setting) + : undefined; const basicSettings = { ...setting, // This is used for table display and search match. - displayedType: - typeOptions.find((option) => option.value === setting.type)?.inputDisplay ?? '', - accessLevel: - permissionModeOptions.find( - (option) => option.value === getPermissionModeId(setting.modes ?? []) - )?.inputDisplay ?? '', + displayedType: collaborator + ? getDisplayedType(displayedCollaboratorTypes, collaborator) + : undefined, + accessLevel: collaborator + ? WORKSPACE_ACCESS_LEVEL_NAMES[collaborator.accessLevel] + : undefined, }; // Unique primary key and filter null value if (setting.type === WorkspacePermissionItemType.User) { @@ -78,7 +115,7 @@ export const WorkspaceCollaboratorTable = ({ } return basicSettings; }); - }, [permissionSettings]); + }, [permissionSettings, displayedCollaboratorTypes]); const emptyStateMessage = useMemo(() => { return ( @@ -192,24 +229,24 @@ export const WorkspaceCollaboratorTable = ({ field: 'displayedType', name: 'Type', multiSelect: false, - options: Array.from(new Set(items.map(({ displayedType }) => displayedType ?? ''))).map( - (item) => ({ - value: item, - name: item, - }) - ), + options: Array.from( + new Set(items.flatMap(({ displayedType }) => (!!displayedType ? [displayedType] : []))) + ).map((item) => ({ + value: item, + name: item, + })), }, { type: 'field_value_selection', field: 'accessLevel', name: 'Access level', multiSelect: false, - options: Array.from(new Set(items.map(({ accessLevel }) => accessLevel ?? ''))).map( - (item) => ({ - value: item, - name: item, - }) - ), + options: Array.from( + new Set(items.flatMap(({ accessLevel }) => (!!accessLevel ? [accessLevel] : []))) + ).map((item) => ({ + value: item, + name: item, + })), }, ], toolsLeft: renderToolsLeft(), @@ -224,10 +261,12 @@ export const WorkspaceCollaboratorTable = ({ { field: 'displayedType', name: 'Type', + render: (displayedType: string) => displayedType || <>—, }, { field: 'accessLevel', name: 'Access level', + render: (accessLevel: string) => accessLevel || <>—, }, { name: 'Actions', diff --git a/src/plugins/workspace/public/components/workspace_form/workspace_detail_form.tsx b/src/plugins/workspace/public/components/workspace_form/workspace_detail_form.tsx index 1466ef3bae66..84a84c677586 100644 --- a/src/plugins/workspace/public/components/workspace_form/workspace_detail_form.tsx +++ b/src/plugins/workspace/public/components/workspace_form/workspace_detail_form.tsx @@ -24,10 +24,15 @@ import { WorkspaceFormErrorCallout } from './workspace_form_error_callout'; import { useWorkspaceFormContext } from './workspace_form_context'; import { WorkspaceDetailFormDetails } from './workspace_detail_form_details'; import { WorkspaceCollaboratorTable } from './workspace_collaborator_table'; -import { WorkspaceCollaboratorTypesService } from '../../services/workspace_collaborator_types_service'; +import { + WorkspaceCollaboratorType, + WorkspaceCollaboratorTypesService, +} from '../../services/workspace_collaborator_types_service'; import { AddCollaboratorButton } from './add_collaborator_button'; import { useOpenSearchDashboards } from '../../../../opensearch_dashboards_react/public'; +const EMPTY_COLLABORATOR_TYPES: WorkspaceCollaboratorType[] = []; + interface WorkspaceDetailedFormProps extends Omit { detailTab?: DetailTab; detailTitle?: string; @@ -51,7 +56,8 @@ export const WorkspaceDetailForm = (props: WorkspaceDetailedFormProps) => { services: { collaboratorTypes }, } = useOpenSearchDashboards<{ collaboratorTypes: WorkspaceCollaboratorTypesService }>(); - const displayedCollaboratorTypes = useObservable(collaboratorTypes.getTypes$()) ?? []; + const displayedCollaboratorTypes = + useObservable(collaboratorTypes.getTypes$()) ?? EMPTY_COLLABORATOR_TYPES; return ( { title: 'Test Title', inputLabel: 'Test Input Label', addAnotherButtonLabel: 'Test Add Another Button Label', - permissionType: 'test', + permissionType: 'user' as const, }; beforeEach(() => { @@ -127,4 +127,30 @@ describe('registerDefaultCollaboratorTypes', () => { expect(groupType?.name).toBe('Group'); expect(groupType?.buttonLabel).toBe('Add Groups'); }); + + it('should return consistent displayed types', () => { + registerDefaultCollaboratorTypes({ collaboratorTypesService, getStartServices }); + + const registeredTypes = collaboratorTypesService.getTypes$().getValue(); + expect(registeredTypes).toHaveLength(2); + + const userType = registeredTypes.find((type) => type.id === 'user'); + const groupType = registeredTypes.find((type) => type.id === 'group'); + const adminUserCollaborator = { + permissionType: 'user' as const, + collaboratorId: '', + accessLevel: 'admin' as const, + }; + const adminGroupCollaborator = { + permissionType: 'group' as const, + collaboratorId: '', + accessLevel: 'admin' as const, + }; + + expect(userType?.getDisplayedType?.(adminUserCollaborator)).toBe('User'); + expect(userType?.getDisplayedType?.(adminGroupCollaborator)).toBeUndefined(); + + expect(groupType?.getDisplayedType?.(adminUserCollaborator)).toBeUndefined(); + expect(groupType?.getDisplayedType?.(adminGroupCollaborator)).toBe('Group'); + }); }); diff --git a/src/plugins/workspace/public/register_default_collaborator_types.tsx b/src/plugins/workspace/public/register_default_collaborator_types.tsx index 09fe96ae4db4..0f917acdeecd 100644 --- a/src/plugins/workspace/public/register_default_collaborator_types.tsx +++ b/src/plugins/workspace/public/register_default_collaborator_types.tsx @@ -55,6 +55,12 @@ export const registerDefaultCollaboratorTypes = ({ buttonLabel: i18n.translate('workspace.collaboratorType.defaultUser.buttonLabel', { defaultMessage: 'Add Users', }), + getDisplayedType: ({ permissionType }) => + permissionType === 'user' + ? i18n.translate('workspace.collaboratorType.defaultUser.displayedType', { + defaultMessage: 'User', + }) + : undefined, onAdd: generateOnAddCallback({ getStartServices, title: i18n.translate('workspace.collaboratorType.defaultUser.modalTitle', { @@ -86,6 +92,12 @@ export const registerDefaultCollaboratorTypes = ({ buttonLabel: i18n.translate('workspace.collaboratorType.defaultGroup.buttonLabel', { defaultMessage: 'Add Groups', }), + getDisplayedType: ({ permissionType }) => + permissionType === 'group' + ? i18n.translate('workspace.collaboratorType.defaultGroup.displayedType', { + defaultMessage: 'Group', + }) + : undefined, onAdd: generateOnAddCallback({ getStartServices, title: i18n.translate('workspace.collaboratorType.defaultGroup.modalTitle', {