From 60ea37505514c1785bfe9b2dc923345b5cdd19f7 Mon Sep 17 00:00:00 2001 From: Lin Wang Date: Tue, 8 Oct 2024 11:17:38 +0800 Subject: [PATCH 01/11] Remove workspace collaborators section in workspace creation Signed-off-by: Lin Wang --- .../workspace_creator.test.tsx | 57 ----------------- .../workspace_creator/workspace_creator.tsx | 5 +- .../workspace_creator_form.tsx | 63 +------------------ .../workspace_form_summary_panel.test.tsx | 52 +++------------ .../workspace_form_summary_panel.tsx | 50 +-------------- .../public/components/workspace_form/utils.ts | 8 +-- 6 files changed, 15 insertions(+), 220 deletions(-) diff --git a/src/plugins/workspace/public/components/workspace_creator/workspace_creator.test.tsx b/src/plugins/workspace/public/components/workspace_creator/workspace_creator.test.tsx index 95797ffe81c8..268e7a6104cb 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 @@ -272,10 +272,6 @@ describe('WorkspaceCreator', () => { { dataSources: [], dataConnections: [], - permissions: { - library_write: { users: ['%me%'] }, - write: { users: ['%me%'] }, - }, } ); await waitFor(() => { @@ -328,43 +324,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, @@ -407,14 +366,6 @@ describe('WorkspaceCreator', () => { { dataConnections: [], dataSources: ['id1'], - permissions: { - library_write: { - users: ['%me%'], - }, - write: { - users: ['%me%'], - }, - }, } ); await waitFor(() => { @@ -465,14 +416,6 @@ describe('WorkspaceCreator', () => { { dataConnections: ['id3'], dataSources: [], - permissions: { - library_write: { - users: ['%me%'], - }, - write: { - users: ['%me%'], - }, - }, } ); await waitFor(() => { diff --git a/src/plugins/workspace/public/components/workspace_creator/workspace_creator.tsx b/src/plugins/workspace/public/components/workspace_creator/workspace_creator.tsx index 9b51ba22d0fe..72c37ebcfda4 100644 --- a/src/plugins/workspace/public/components/workspace_creator/workspace_creator.tsx +++ b/src/plugins/workspace/public/components/workspace_creator/workspace_creator.tsx @@ -15,7 +15,6 @@ import { WORKSPACE_DETAIL_APP_ID } from '../../../common/constants'; 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'; @@ -47,7 +46,6 @@ export const WorkspaceCreator = (props: WorkspaceCreatorProps) => { }>(); const [isFormSubmitting, setIsFormSubmitting] = useState(false); - const isPermissionEnabled = application?.capabilities.workspaces.permissionEnabled; const { isOnlyAllowEssential, availableUseCases } = useFormAvailableUseCases({ savedObjects, registeredUseCases$, @@ -102,7 +100,6 @@ export const WorkspaceCreator = (props: WorkspaceCreatorProps) => { result = await workspaceClient.create(attributes, { dataSources: selectedDataSourceIds, dataConnections: selectedDataConnectionIds, - permissions: convertPermissionSettingsToPermissions(permissionSettings), }); if (result?.success) { notifications?.toasts.addSuccess({ @@ -177,7 +174,7 @@ export const WorkspaceCreator = (props: WorkspaceCreatorProps) => { savedObjects={savedObjects} onSubmit={handleWorkspaceFormSubmit} operationType={WorkspaceOperationType.Create} - permissionEnabled={isPermissionEnabled} + permissionEnabled={false} 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('Owner')).toBeInTheDocument(); - expect(screen.getByText('group1')).toBeInTheDocument(); - expect(screen.getByText('Read')).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 && ( - - )} - - )} Date: Tue, 8 Oct 2024 11:41:44 +0800 Subject: [PATCH 02/11] Redirect to workspace detail collaborators tab after workspace created Signed-off-by: Lin Wang --- .../workspace_creator.test.tsx | 16 ++++++++++------ .../workspace_creator/workspace_creator.tsx | 19 ++++++------------- 2 files changed, 16 insertions(+), 19 deletions(-) diff --git a/src/plugins/workspace/public/components/workspace_creator/workspace_creator.test.tsx b/src/plugins/workspace/public/components/workspace_creator/workspace_creator.test.tsx index 268e7a6104cb..9d0e1f1d61ca 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'), @@ -178,7 +179,6 @@ function clearMockedFunctions() { describe('WorkspaceCreator', () => { beforeEach(() => clearMockedFunctions()); const { location } = window; - const setHrefSpy = jest.fn((href) => href); beforeAll(() => { if (window.location) { @@ -186,10 +186,6 @@ describe('WorkspaceCreator', () => { delete window.location; } window.location = {} as Location; - Object.defineProperty(window.location, 'href', { - get: () => 'http://localhost/w/workspace/app/workspace_create', - set: setHrefSpy, - }); }); afterAll(() => { @@ -455,6 +451,10 @@ describe('WorkspaceCreator', () => { it('should redirect to workspace use case landing page after created successfully', async () => { const { getByTestId } = render(); + const navigateToWorkspaceDetailMock = jest.fn(); + jest + .spyOn(workspaceUtilsExports, 'navigateToWorkspaceDetail') + .mockImplementation(navigateToWorkspaceDetailMock); // Ensure workspace create form rendered await waitFor(() => { @@ -468,7 +468,11 @@ describe('WorkspaceCreator', () => { jest.useFakeTimers(); jest.runAllTimers(); await waitFor(() => { - expect(setHrefSpy).toHaveBeenCalledWith(expect.stringContaining('/app/discover')); + 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 72c37ebcfda4..a427d328e85b 100644 --- a/src/plugins/workspace/public/components/workspace_creator/workspace_creator.tsx +++ b/src/plugins/workspace/public/components/workspace_creator/workspace_creator.tsx @@ -10,17 +10,15 @@ 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 { WorkspaceFormSubmitData, WorkspaceOperationType, DetailTab } from '../workspace_form'; import { getUseCaseFeatureConfig } from '../../../common/utils'; -import { formatUrlWithWorkspaceId } from '../../../../../core/public/utils'; import { WorkspaceClient } from '../../workspace_client'; 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'; export interface WorkspaceCreatorProps { @@ -109,17 +107,12 @@ export const WorkspaceCreator = (props: WorkspaceCreatorProps) => { }); if (application && http) { const newWorkspaceId = result.result.id; - const useCaseId = getFirstUseCaseOfFeatureConfigs(attributes.features); - const useCaseLandingAppId = availableUseCases?.find(({ id }) => useCaseId === id) - ?.features[0].id; // Redirect page after one second, leave one second time to show create successful toast. window.setTimeout(() => { - window.location.href = formatUrlWithWorkspaceId( - application.getUrlForApp(useCaseLandingAppId || WORKSPACE_DETAIL_APP_ID, { - absolute: true, - }), + navigateToWorkspaceDetail( + { application, http }, newWorkspaceId, - http.basePath + DetailTab.Collaborators ); }, 1000); } @@ -139,7 +132,7 @@ export const WorkspaceCreator = (props: WorkspaceCreatorProps) => { setIsFormSubmitting(false); } }, - [notifications?.toasts, http, application, workspaceClient, isFormSubmitting, availableUseCases] + [notifications?.toasts, http, application, workspaceClient, isFormSubmitting] ); const isFormReadyToRender = From 2d601c7398965b5412952a33518e8aba20954023 Mon Sep 17 00:00:00 2001 From: "opensearch-changeset-bot[bot]" <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com> Date: Tue, 8 Oct 2024 03:52:22 +0000 Subject: [PATCH 03/11] Changeset file for PR #8520 created/updated --- changelogs/fragments/8520.yml | 2 ++ 1 file changed, 2 insertions(+) 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 From b8b979ea578d28a4434401666a22d8e4a7b9b63e Mon Sep 17 00:00:00 2001 From: Lin Wang Date: Tue, 8 Oct 2024 12:02:39 +0800 Subject: [PATCH 04/11] Add getDisplayedType in default collaborator types Signed-off-by: Lin Wang --- ...gister_default_collaborator_types.test.tsx | 28 ++++++++++++++++++- .../register_default_collaborator_types.tsx | 12 ++++++++ 2 files changed, 39 insertions(+), 1 deletion(-) diff --git a/src/plugins/workspace/public/register_default_collaborator_types.test.tsx b/src/plugins/workspace/public/register_default_collaborator_types.test.tsx index 3a711dc87a7e..5ebac9732015 100644 --- a/src/plugins/workspace/public/register_default_collaborator_types.test.tsx +++ b/src/plugins/workspace/public/register_default_collaborator_types.test.tsx @@ -48,7 +48,7 @@ describe('generateOnAddCallback', () => { 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', { From 00b8a773907867ad6b0deeb2f7799cef2f232cb8 Mon Sep 17 00:00:00 2001 From: Lin Wang Date: Tue, 8 Oct 2024 17:26:45 +0800 Subject: [PATCH 05/11] Update detail type to ReactNode Signed-off-by: Lin Wang --- .../add_collaborators_modal/add_collaborators_modal.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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..5362d1a41a4e 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 @@ -34,7 +34,7 @@ export interface AddCollaboratorsModalProps { inputPlaceholder?: string; instruction?: { title: string; - detail: string; + detail: React.ReactNode; link?: string; }; permissionType: WorkspaceCollaboratorPermissionType; From de863cb08cbc974569bf595e24f61525b6ead519 Mon Sep 17 00:00:00 2001 From: Lin Wang Date: Wed, 9 Oct 2024 10:23:56 +0800 Subject: [PATCH 06/11] Add learn more link Signed-off-by: Lin Wang --- .../add_collaborators_modal.test.tsx | 6 ++++-- .../add_collaborators_modal.tsx | 14 ++++++++++++++ 2 files changed, 18 insertions(+), 2 deletions(-) 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 5362d1a41a4e..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, @@ -91,6 +92,19 @@ export const AddCollaboratorsModal = ({ {instruction.detail} + {instruction.link && ( + <> + + + {i18n.translate( + 'workspace.addCollaboratorsModal.instruction.learnMoreLinkText', + { + defaultMessage: 'Learn more in Documentation', + } + )} + + + )} From 69c9794a364d367624750a977ba157041281f655 Mon Sep 17 00:00:00 2001 From: Lin Wang Date: Wed, 9 Oct 2024 13:56:33 +0800 Subject: [PATCH 07/11] Remove permissionEnabled passed in WorkspaceCreatorForm Signed-off-by: Lin Wang --- .../public/components/workspace_creator/workspace_creator.tsx | 1 - 1 file changed, 1 deletion(-) 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 a427d328e85b..585ec500ff07 100644 --- a/src/plugins/workspace/public/components/workspace_creator/workspace_creator.tsx +++ b/src/plugins/workspace/public/components/workspace_creator/workspace_creator.tsx @@ -167,7 +167,6 @@ export const WorkspaceCreator = (props: WorkspaceCreatorProps) => { savedObjects={savedObjects} onSubmit={handleWorkspaceFormSubmit} operationType={WorkspaceOperationType.Create} - permissionEnabled={false} dataSourceManagement={dataSourceManagement} availableUseCases={availableUseCases} defaultValues={defaultWorkspaceFormValues} From 9f96825038a896967118028d0934488a297d7e67 Mon Sep 17 00:00:00 2001 From: Lin Wang Date: Wed, 9 Oct 2024 16:40:45 +0800 Subject: [PATCH 08/11] Redirect to use case landing page if permission not enabled Signed-off-by: Lin Wang --- .../workspace_creator.test.tsx | 38 +++++++++++++++++-- .../workspace_creator/workspace_creator.tsx | 35 ++++++++++++++--- 2 files changed, 65 insertions(+), 8 deletions(-) diff --git a/src/plugins/workspace/public/components/workspace_creator/workspace_creator.test.tsx b/src/plugins/workspace/public/components/workspace_creator/workspace_creator.test.tsx index 9d0e1f1d61ca..7bbf7a7b63dd 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 @@ -114,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, ...{ @@ -124,7 +131,7 @@ const WorkspaceCreator = ({ capabilities: { ...mockCoreStart.application.capabilities, workspaces: { - permissionEnabled: true, + permissionEnabled: isPermissionEnabled, }, dashboards: { isDashboardAdmin, @@ -179,6 +186,7 @@ function clearMockedFunctions() { describe('WorkspaceCreator', () => { beforeEach(() => clearMockedFunctions()); const { location } = window; + const setHrefSpy = jest.fn((href) => href); beforeAll(() => { if (window.location) { @@ -186,6 +194,10 @@ describe('WorkspaceCreator', () => { delete window.location; } window.location = {} as Location; + Object.defineProperty(window.location, 'href', { + get: () => 'http://localhost/w/workspace/app/workspace_create', + set: setHrefSpy, + }); }); afterAll(() => { @@ -449,7 +461,27 @@ describe('WorkspaceCreator', () => { }); }); - it('should redirect to workspace use case landing page after created successfully', async () => { + it('should redirect to workspace use case landing page if permission 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('workspaceForm-bottomBar-createButton')); + jest.useFakeTimers(); + jest.runAllTimers(); + await waitFor(() => { + expect(setHrefSpy).toHaveBeenCalledWith(expect.stringContaining('/app/discover')); + }); + jest.useRealTimers(); + }); + + it('should redirect to workspace setting collaborators tab if permission enabled', async () => { const { getByTestId } = render(); const navigateToWorkspaceDetailMock = jest.fn(); jest 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 585ec500ff07..6939892e8d14 100644 --- a/src/plugins/workspace/public/components/workspace_creator/workspace_creator.tsx +++ b/src/plugins/workspace/public/components/workspace_creator/workspace_creator.tsx @@ -7,14 +7,17 @@ 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 { WORKSPACE_DETAIL_APP_ID } from '../../../common/constants'; import { WorkspaceFormSubmitData, WorkspaceOperationType, DetailTab } from '../workspace_form'; import { getUseCaseFeatureConfig } from '../../../common/utils'; +import { formatUrlWithWorkspaceId } from '../../../../../core/public/utils'; import { WorkspaceClient } from '../../workspace_client'; 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'; @@ -43,6 +46,7 @@ export const WorkspaceCreator = (props: WorkspaceCreatorProps) => { navigationUI: NavigationPublicPluginStart['ui']; }>(); const [isFormSubmitting, setIsFormSubmitting] = useState(false); + const isPermissionEnabled = application?.capabilities.workspaces.permissionEnabled; const { isOnlyAllowEssential, availableUseCases } = useFormAvailableUseCases({ savedObjects, @@ -107,12 +111,25 @@ export const WorkspaceCreator = (props: WorkspaceCreatorProps) => { }); if (application && http) { const newWorkspaceId = result.result.id; + const useCaseId = getFirstUseCaseOfFeatureConfigs(attributes.features); + const useCaseLandingAppId = availableUseCases?.find(({ id }) => useCaseId === id) + ?.features[0].id; // Redirect page after one second, leave one second time to show create successful toast. window.setTimeout(() => { - navigateToWorkspaceDetail( - { application, http }, + if (isPermissionEnabled) { + navigateToWorkspaceDetail( + { application, http }, + newWorkspaceId, + DetailTab.Collaborators + ); + return; + } + window.location.href = formatUrlWithWorkspaceId( + application.getUrlForApp(useCaseLandingAppId || WORKSPACE_DETAIL_APP_ID, { + absolute: true, + }), newWorkspaceId, - DetailTab.Collaborators + http.basePath ); }, 1000); } @@ -132,7 +149,15 @@ export const WorkspaceCreator = (props: WorkspaceCreatorProps) => { setIsFormSubmitting(false); } }, - [notifications?.toasts, http, application, workspaceClient, isFormSubmitting] + [ + notifications?.toasts, + http, + application, + workspaceClient, + isFormSubmitting, + availableUseCases, + isPermissionEnabled, + ] ); const isFormReadyToRender = From 4750e3e084f4d977ff94e3c096ed929edc0b5c5b Mon Sep 17 00:00:00 2001 From: Lin Wang Date: Wed, 9 Oct 2024 19:26:26 +0800 Subject: [PATCH 09/11] Use getDisplayedType and render — for empty Signed-off-by: Lin Wang --- ...workspace_collaborator_table.test.tsx.snap | 258 +++++++++++++++--- .../public/components/workspace_form/utils.ts | 2 +- .../workspace_collaborator_table.test.tsx | 50 +++- .../workspace_collaborator_table.tsx | 83 ++++-- .../workspace_form/workspace_detail_form.tsx | 10 +- 5 files changed, 336 insertions(+), 67 deletions(-) diff --git a/src/plugins/workspace/public/components/workspace_form/__snapshots__/workspace_collaborator_table.test.tsx.snap b/src/plugins/workspace/public/components/workspace_form/__snapshots__/workspace_collaborator_table.test.tsx.snap index caaa8456ecdb..2e83a4ebe437 100644 --- a/src/plugins/workspace/public/components/workspace_form/__snapshots__/workspace_collaborator_table.test.tsx.snap +++ b/src/plugins/workspace/public/components/workspace_form/__snapshots__/workspace_collaborator_table.test.tsx.snap @@ -316,13 +316,9 @@ Object { Type
- - 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 +
+
+ —
{ for (const key in optionIdToWorkspacePermissionModesMap) { if (optionIdToWorkspacePermissionModesMap[key].every((mode) => modes?.includes(mode))) { - return key; + return key as PermissionModeId; } } return PermissionModeId.Read; 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..c564c87d5b91 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,28 @@ 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'; +import { WorkspacePermissionItemType } from './constants'; 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 +38,33 @@ const { Provider } = createOpenSearchDashboardsReactContext({ overlays: mockOverlays, }); +describe('getDisplayedTypes', () => { + it('should return undefined if not match any collaborator type', () => { + expect(getDisplayedType(displayedCollaboratorTypes, { type: 'unknown' })).toBeUndefined(); + }); + it('should return "User"', () => { + expect( + getDisplayedType(displayedCollaboratorTypes, { + type: WorkspacePermissionItemType.User, + userId: 'foo', + id: 0, + }) + ).toBeUndefined(); + }); + it('should return "Group"', () => { + expect( + getDisplayedType(displayedCollaboratorTypes, { + type: WorkspacePermissionItemType.Group, + group: 'foo', + id: 0, + }) + ).toBeUndefined(); + }); +}); + describe('WorkspaceCollaboratorTable', () => { const mockProps = { - displayedCollaboratorTypes: [], + displayedCollaboratorTypes, permissionSettings: [ { id: 0, @@ -37,6 +78,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 0b510cb1f597..ce0a40a4b0b1 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 @@ -20,20 +20,54 @@ 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'; 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[]; @@ -50,15 +84,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) { @@ -75,7 +112,7 @@ export const WorkspaceCollaboratorTable = ({ } return basicSettings; }); - }, [permissionSettings]); + }, [permissionSettings, displayedCollaboratorTypes]); const emptyStateMessage = useMemo(() => { return ( @@ -182,24 +219,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(), @@ -214,10 +251,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 ( Date: Wed, 9 Oct 2024 22:51:05 +0800 Subject: [PATCH 10/11] Fix incorrect unit tests Signed-off-by: Lin Wang --- .../workspace_collaborator_table.test.tsx | 21 ++++++++++--------- 1 file changed, 11 insertions(+), 10 deletions(-) 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 c564c87d5b91..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 @@ -9,7 +9,6 @@ import { fireEvent, render } from '@testing-library/react'; import { WorkspaceCollaboratorTable, getDisplayedType } from './workspace_collaborator_table'; import { createOpenSearchDashboardsReactContext } from '../../../../opensearch_dashboards_react/public'; import { coreMock } from '../../../../../core/public/mocks'; -import { WorkspacePermissionItemType } from './constants'; const mockCoreStart = coreMock.createStart(); const displayedCollaboratorTypes = [ @@ -40,25 +39,27 @@ const { Provider } = createOpenSearchDashboardsReactContext({ describe('getDisplayedTypes', () => { it('should return undefined if not match any collaborator type', () => { - expect(getDisplayedType(displayedCollaboratorTypes, { type: 'unknown' })).toBeUndefined(); + expect( + getDisplayedType(displayedCollaboratorTypes, { permissionType: 'unknown' }) + ).toBeUndefined(); }); it('should return "User"', () => { expect( getDisplayedType(displayedCollaboratorTypes, { - type: WorkspacePermissionItemType.User, - userId: 'foo', - id: 0, + collaboratorId: 'foo', + permissionType: 'user', + accessLevel: 'readOnly', }) - ).toBeUndefined(); + ).toEqual('User'); }); it('should return "Group"', () => { expect( getDisplayedType(displayedCollaboratorTypes, { - type: WorkspacePermissionItemType.Group, - group: 'foo', - id: 0, + collaboratorId: 'foo', + permissionType: 'group', + accessLevel: 'readOnly', }) - ).toBeUndefined(); + ).toEqual('Group'); }); }); From 8255e82201dc5b2c150bd0b743775288cbcb3a8d Mon Sep 17 00:00:00 2001 From: Lin Wang Date: Fri, 11 Oct 2024 14:57:26 +0800 Subject: [PATCH 11/11] Assign current user as workspace admin Signed-off-by: Lin Wang --- .../workspace_creator.test.tsx | 45 +++++++++++++++++-- .../workspace_creator/workspace_creator.tsx | 32 +++++++++++-- .../public/components/workspace_form/index.ts | 7 ++- .../workspace_form/use_workspace_form.ts | 21 ++------- .../public/components/workspace_form/utils.ts | 40 +---------------- 5 files changed, 82 insertions(+), 63 deletions(-) diff --git a/src/plugins/workspace/public/components/workspace_creator/workspace_creator.test.tsx b/src/plugins/workspace/public/components/workspace_creator/workspace_creator.test.tsx index 7bbf7a7b63dd..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 @@ -280,6 +280,14 @@ describe('WorkspaceCreator', () => { { dataSources: [], dataConnections: [], + permissions: { + library_write: { + users: ['%me%'], + }, + write: { + users: ['%me%'], + }, + }, } ); await waitFor(() => { @@ -371,10 +379,10 @@ describe('WorkspaceCreator', () => { expect.objectContaining({ name: 'test workspace name', }), - { + expect.objectContaining({ dataConnections: [], dataSources: ['id1'], - } + }) ); await waitFor(() => { expect(notificationToastsAddSuccess).toHaveBeenCalled(); @@ -421,9 +429,40 @@ describe('WorkspaceCreator', () => { expect.objectContaining({ name: 'test workspace name', }), - { + expect.objectContaining({ dataConnections: ['id3'], dataSources: [], + }) + ); + 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(() => { 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 6939892e8d14..061649304331 100644 --- a/src/plugins/workspace/public/components/workspace_creator/workspace_creator.tsx +++ b/src/plugins/workspace/public/components/workspace_creator/workspace_creator.tsx @@ -10,8 +10,16 @@ import { BehaviorSubject } from 'rxjs'; import { useLocation } from 'react-router-dom'; import { useOpenSearchDashboards } from '../../../../opensearch_dashboards_react/public'; -import { WORKSPACE_DETAIL_APP_ID } from '../../../common/constants'; -import { WorkspaceFormSubmitData, WorkspaceOperationType, DetailTab } from '../workspace_form'; +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'; @@ -23,6 +31,7 @@ import { NavigationPublicPluginStart } from '../../../../../plugins/navigation/p 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; @@ -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,6 +123,11 @@ export const WorkspaceCreator = (props: WorkspaceCreatorProps) => { result = await workspaceClient.create(attributes, { dataSources: selectedDataSourceIds, dataConnections: selectedDataConnectionIds, + ...(isPermissionEnabled + ? { + permissions: convertPermissionSettingsToPermissions(permissionSettings), + } + : {}), }); if (result?.success) { notifications?.toasts.addSuccess({ diff --git a/src/plugins/workspace/public/components/workspace_form/index.ts b/src/plugins/workspace/public/components/workspace_form/index.ts index fef18d7c8c5e..48f1aec5e504 100644 --- a/src/plugins/workspace/public/components/workspace_form/index.ts +++ b/src/plugins/workspace/public/components/workspace_form/index.ts @@ -14,7 +14,12 @@ export { ConnectionTypeIcon } from './connection_type_icon'; export { DataSourceConnectionTable } from './data_source_connection_table'; export { WorkspaceUseCaseFlyout } from './workspace_use_case_flyout'; -export { WorkspaceFormSubmitData, WorkspaceFormProps, WorkspaceFormDataState } from './types'; +export { + WorkspaceFormSubmitData, + WorkspaceFormProps, + WorkspaceFormDataState, + WorkspacePermissionSetting, +} from './types'; export { WorkspaceOperationType, DetailTab, diff --git a/src/plugins/workspace/public/components/workspace_form/use_workspace_form.ts b/src/plugins/workspace/public/components/workspace_form/use_workspace_form.ts index a2c2cee12066..cac2c02bc414 100644 --- a/src/plugins/workspace/public/components/workspace_form/use_workspace_form.ts +++ b/src/plugins/workspace/public/components/workspace_form/use_workspace_form.ts @@ -16,12 +16,7 @@ import { WorkspacePermissionSetting, WorkspaceFormDataState, } from './types'; -import { - generatePermissionSettingsState, - getNumberOfChanges, - getNumberOfErrors, - validateWorkspaceForm, -} from './utils'; +import { getNumberOfChanges, getNumberOfErrors, validateWorkspaceForm } from './utils'; import { WorkspacePermissionItemType } from './constants'; const workspaceHtmlIdGenerator = htmlIdGenerator(); @@ -29,7 +24,6 @@ const workspaceHtmlIdGenerator = htmlIdGenerator(); export const useWorkspaceForm = ({ application, defaultValues, - operationType, onSubmit, permissionEnabled, onAppLeave, @@ -40,9 +34,6 @@ export const useWorkspaceForm = ({ const [color, setColor] = useState(defaultValues?.color); const defaultValuesRef = useRef(defaultValues); const [isEditing, setIsEditing] = useState(false); - const initialPermissionSettingsRef = useRef( - generatePermissionSettingsState(operationType, defaultValues?.permissionSettings) - ); const [featureConfigs, setFeatureConfigs] = useState(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) { @@ -161,7 +148,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 8b991fd22ac2..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'; @@ -382,43 +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 []; - } - - return [...(permissionSettings ?? [])]; -}; - interface PermissionSettingLike extends Omit, 'type'>, Omit, 'type'> {