Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Workspace]Refactor workspace form UI #7133

Merged
merged 36 commits into from
Jul 12, 2024
Merged
Show file tree
Hide file tree
Changes from 29 commits
Commits
Show all changes
36 commits
Select commit Hold shift + click to select a range
3edb425
Make create workspace and update workspace full width
wanglam Jun 14, 2024
d8c1709
Refactor user permissions input
wanglam Jun 14, 2024
bd6751c
Add workspace form call out
wanglam Jun 14, 2024
8dad0e0
Fix permissions input unit tests
wanglam Jun 17, 2024
293fec3
Update gaps
wanglam Jun 19, 2024
04d5532
Update error callout
wanglam Jun 20, 2024
7e456e6
Update user permission current user and number of changes
wanglam Jun 21, 2024
6f6a8dc
Fix changes
wanglam Jun 21, 2024
a43a45a
Fix owner order
wanglam Jun 21, 2024
380b3f2
Add ut for form error callout
wanglam Jun 24, 2024
5ee654a
Fix unit tests in workspace
wanglam Jun 24, 2024
c43014b
Mark first user row required
wanglam Jun 24, 2024
47ed32e
Update section title
wanglam Jun 27, 2024
be4d8ba
Add validate for owner missing
wanglam Jun 27, 2024
8463a2b
Changeset file for PR #7133 created/updated
opensearch-changeset-bot[bot] Jul 1, 2024
83da744
Fix unit tests for workspace form utils
wanglam Jul 2, 2024
d41f618
Fix unit tests for form error callout
wanglam Jul 2, 2024
1c47632
Add unit test for transfer current user placeholder
wanglam Jul 2, 2024
8169847
Fix unit tests in workspace permission setting panel
wanglam Jul 2, 2024
d24eac7
Fix unit test in useWorkspaceForm
wanglam Jul 2, 2024
c7de1f0
Merge branch 'main' into refactor-workspace-form-ui
wanglam Jul 3, 2024
392dc72
Add missing unit tests for workspace form utils
wanglam Jul 3, 2024
d49a234
Add unit tests for getNumberOfErrors
wanglam Jul 4, 2024
4486233
Add more ut for workspace form error callout
wanglam Jul 4, 2024
c911b30
Merge remote-tracking branch 'origin/main' into refactor-workspace-fo…
wanglam Jul 5, 2024
6c096a7
Merge branch 'main' into refactor-workspace-form-ui
wanglam Jul 8, 2024
b1ab397
Fix error code
wanglam Jul 8, 2024
8a17062
Merge branch 'main' into refactor-workspace-form-ui
wanglam Jul 8, 2024
cb07ad3
Fix failed unit test
wanglam Jul 8, 2024
2930293
Add back color picker
wanglam Jul 9, 2024
ce9b527
Merge branch 'main' into refactor-workspace-form-ui
wanglam Jul 9, 2024
bcbc36b
Merge branch 'main' into refactor-workspace-form-ui
wanglam Jul 11, 2024
7e2f036
Address UX comments
wanglam Jul 11, 2024
9c42390
Fix empty user no workspace owner
wanglam Jul 11, 2024
7e28c7d
Change to Associate data source
wanglam Jul 11, 2024
a7b3d1d
Merge branch 'main' into refactor-workspace-form-ui
wanglam Jul 11, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions changelogs/fragments/7133.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
feat:
- [Workspace] Refactor workspace form UI ([#7133](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/7133))
2 changes: 2 additions & 0 deletions src/plugins/workspace/common/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -182,3 +182,5 @@ export const WORKSPACE_USE_CASES = Object.freeze({
] as string[],
},
});

export const CURRENT_USER_PLACEHOLDER = '%current_user%';
Original file line number Diff line number Diff line change
Expand Up @@ -182,22 +182,21 @@ describe('WorkspaceCreator', () => {
fireEvent.input(descriptionInput, {
target: { value: 'test workspace description' },
});
const colorSelector = getByTestId(
'euiColorPickerAnchor workspaceForm-workspaceDetails-colorPicker'
);
fireEvent.input(colorSelector, {
target: { value: '#000000' },
});
fireEvent.click(getByTestId('workspaceUseCase-observability'));
fireEvent.click(getByTestId('workspaceForm-bottomBar-createButton'));
expect(workspaceClientCreate).toHaveBeenCalledWith(
expect.objectContaining({
name: 'test workspace name',
color: '#000000',
description: 'test workspace description',
features: expect.arrayContaining(['use-case-observability']),
}),
{ dataSources: [], permissions: undefined }
{
dataSources: [],
permissions: {
library_write: { users: ['%current_user%'] },
write: { users: ['%current_user%'] },
},
}
);
await waitFor(() => {
expect(notificationToastsAddSuccess).toHaveBeenCalled();
Expand Down Expand Up @@ -247,8 +246,8 @@ describe('WorkspaceCreator', () => {
expect(notificationToastsAddSuccess).not.toHaveBeenCalled();
});

it('create workspace with customized permissions', async () => {
const { getByTestId, getAllByText } = render(
it('create workspace with current user', async () => {
const { getByTestId } = render(
<WorkspaceCreator
workspaceConfigurableApps$={new BehaviorSubject([...PublicAPPInfoMap.values()])}
/>
Expand All @@ -259,12 +258,6 @@ describe('WorkspaceCreator', () => {
});
fireEvent.click(getByTestId('workspaceUseCase-observability'));
fireEvent.click(getByTestId('workspaceForm-permissionSettingPanel-user-addNew'));
const userIdInput = getAllByText('Select')[0];
fireEvent.click(userIdInput);
fireEvent.input(getByTestId('comboBoxSearchInput'), {
target: { value: 'test user id' },
});
fireEvent.blur(getByTestId('comboBoxSearchInput'));
fireEvent.click(getByTestId('workspaceForm-bottomBar-createButton'));
expect(workspaceClientCreate).toHaveBeenCalledWith(
expect.objectContaining({
Expand All @@ -273,11 +266,11 @@ describe('WorkspaceCreator', () => {
{
dataSources: [],
permissions: {
read: {
users: ['test user id'],
write: {
users: ['%current_user%'],
},
library_read: {
users: ['test user id'],
library_write: {
users: ['%current_user%'],
},
},
}
Expand Down Expand Up @@ -313,7 +306,14 @@ describe('WorkspaceCreator', () => {
}),
{
dataSources: ['id1'],
permissions: undefined,
permissions: {
library_write: {
users: ['%current_user%'],
},
write: {
users: ['%current_user%'],
},
},
}
);
await waitFor(() => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
*/

import React, { useCallback } from 'react';
import { EuiPage, EuiPageBody, EuiPageHeader, EuiPageContent, EuiSpacer } from '@elastic/eui';
import { EuiPage, EuiPageBody, EuiPageHeader, EuiPageContent } from '@elastic/eui';
import { i18n } from '@osd/i18n';
import { useObservable } from 'react-use';
import { BehaviorSubject, of } from 'rxjs';
Expand Down Expand Up @@ -80,21 +80,15 @@ export const WorkspaceCreator = (props: WorkspaceCreatorProps) => {
);

return (
<EuiPage paddingSize="none">
<EuiPage>
<EuiPageBody>
<EuiPageHeader restrictWidth pageTitle="Create a workspace" />
<EuiSpacer />
<EuiPageHeader pageTitle="Create a workspace" />
<EuiPageContent
verticalPosition="center"
horizontalPosition="center"
paddingSize="none"
color="subdued"
hasShadow={false}
/**
* Since above EuiPageHeader has a maxWidth: 1000 style,
* add maxWidth: 1000 below to align with the above page header
**/
style={{ width: '100%', maxWidth: 1000 }}
>
{application && savedObjects && (
<WorkspaceForm
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
* SPDX-License-Identifier: Apache-2.0
*/

import { i18n } from '@osd/i18n';
import { WorkspacePermissionMode } from '../../../common/constants';

export enum WorkspaceOperationType {
Expand All @@ -19,33 +18,9 @@ export enum WorkspacePermissionItemType {
export enum PermissionModeId {
Read = 'read',
ReadAndWrite = 'read+write',
Admin = 'admin',
Owner = 'owner',
}

export const permissionModeOptions = [
{
id: PermissionModeId.Read,
label: i18n.translate('workspace.form.permissionSettingPanel.permissionModeOptions.read', {
defaultMessage: 'Read',
}),
},
{
id: PermissionModeId.ReadAndWrite,
label: i18n.translate(
'workspace.form.permissionSettingPanel.permissionModeOptions.readAndWrite',
{
defaultMessage: 'Read & Write',
}
),
},
{
id: PermissionModeId.Admin,
label: i18n.translate('workspace.form.permissionSettingPanel.permissionModeOptions.admin', {
defaultMessage: 'Admin',
}),
},
];

export const optionIdToWorkspacePermissionModesMap: {
[key: string]: WorkspacePermissionMode[];
} = {
Expand All @@ -54,5 +29,5 @@ export const optionIdToWorkspacePermissionModesMap: {
WorkspacePermissionMode.LibraryWrite,
WorkspacePermissionMode.Read,
],
[PermissionModeId.Admin]: [WorkspacePermissionMode.LibraryWrite, WorkspacePermissionMode.Write],
[PermissionModeId.Owner]: [WorkspacePermissionMode.LibraryWrite, WorkspacePermissionMode.Write],
};
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,10 @@ import { i18n } from '@osd/i18n';
import { SavedObjectsStart } from '../../../../../core/public';
import { getDataSourcesList } from '../../utils';
import { DataSource } from '../../../common/types';
import { WorkspaceFormError } from './types';

export interface SelectDataSourcePanelProps {
errors?: { [key: number]: string };
errors?: { [key: number]: WorkspaceFormError };
savedObjects: SavedObjectsStart;
selectedDataSources: DataSource[];
onChange: (value: DataSource[]) => void;
Expand Down Expand Up @@ -96,7 +97,7 @@ export const SelectDataSourcePanel = ({
</EuiText>
<EuiSpacer size="s" />
{selectedDataSources.map(({ id, title }, index) => (
<EuiFormRow key={index} isInvalid={!!errors?.[index]} error={errors?.[index]}>
<EuiFormRow key={index} isInvalid={!!errors?.[index]} error={errors?.[index].message}>
<EuiFlexGroup gutterSize="l">
<EuiFlexItem grow={false}>
<EuiComboBox
Expand Down
64 changes: 46 additions & 18 deletions src/plugins/workspace/public/components/workspace_form/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,25 +12,28 @@ import type { WorkspacePermissionMode } from '../../../common/constants';
import type { WorkspaceOperationType, WorkspacePermissionItemType } from './constants';
import { DataSource } from '../../../common/types';

export interface WorkspaceUserPermissionSetting {
id: number;
type: WorkspacePermissionItemType.User;
userId: string;
modes: WorkspacePermissionMode[];
}

export interface WorkspaceUserGroupPermissionSetting {
id: number;
type: WorkspacePermissionItemType.Group;
group: string;
modes: WorkspacePermissionMode[];
}

export type WorkspacePermissionSetting =
| {
id: number;
type: WorkspacePermissionItemType.User;
userId: string;
modes: WorkspacePermissionMode[];
}
| {
id: number;
type: WorkspacePermissionItemType.Group;
group: string;
modes: WorkspacePermissionMode[];
};
| WorkspaceUserPermissionSetting
| WorkspaceUserGroupPermissionSetting;

export interface WorkspaceFormSubmitData {
name: string;
description?: string;
features?: string[];
color?: string;
permissionSettings?: WorkspacePermissionSetting[];
selectedDataSources?: DataSource[];
}
Expand All @@ -40,20 +43,45 @@ export interface WorkspaceFormData extends WorkspaceFormSubmitData {
reserved?: boolean;
}

export enum WorkspaceFormErrorCode {
InvalidWorkspaceName,
WorkspaceNameMissing,
UseCaseMissing,
InvalidPermissionType,
InvalidPermissionModes,
PermissionUserIdMissing,
PermissionUserGroupMissing,
DuplicateUserIdPermissionSetting,
DuplicateUserGroupPermissionSetting,
PermissionSettingOwnerMissing,
InvalidDataSource,
DuplicateDataSource,
}

export interface WorkspaceFormError {
message: string;
code: WorkspaceFormErrorCode;
}

export type WorkspaceFormErrors = {
[key in keyof Omit<WorkspaceFormData, 'permissionSettings' | 'selectedDataSources'>]?: string;
[key in keyof Omit<
WorkspaceFormData,
'permissionSettings' | 'description' | 'selectedDataSources'
>]?: WorkspaceFormError;
} & {
permissionSettings?: { [key: number]: string };
selectedDataSources?: { [key: number]: string };
permissionSettings?: {
overall?: WorkspaceFormError;
fields?: { [key: number]: WorkspaceFormError };
};
selectedDataSources?: { [key: number]: WorkspaceFormError };
};

export interface WorkspaceFormProps {
application: ApplicationStart;
savedObjects: SavedObjectsStart;
onSubmit?: (formData: WorkspaceFormSubmitData) => void;
defaultValues?: WorkspaceFormData;
operationType?: WorkspaceOperationType;
operationType: WorkspaceOperationType;
workspaceConfigurableApps?: PublicAppInfo[];
permissionEnabled?: boolean;
permissionLastAdminItemDeletable?: boolean;
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@
import { renderHook, act } from '@testing-library/react-hooks';

import { applicationServiceMock } from '../../../../../core/public/mocks';
import { WorkspaceFormData } from './types';
import { WorkspaceOperationType } from './constants';
import { WorkspaceFormData, WorkspaceFormErrorCode } from './types';
import { useWorkspaceForm } from './use_workspace_form';

const setup = (defaultValues?: WorkspaceFormData) => {
Expand All @@ -16,6 +17,7 @@ const setup = (defaultValues?: WorkspaceFormData) => {
application: applicationServiceMock.createStartContract(),
defaultValues,
onSubmit: onSubmitMock,
operationType: WorkspaceOperationType.Create,
},
});
return {
Expand All @@ -25,7 +27,7 @@ const setup = (defaultValues?: WorkspaceFormData) => {
};

describe('useWorkspaceForm', () => {
it('should return "Invalid workspace name" and not call onSubmit when invalid name', async () => {
it('should return invalid workspace name error and not call onSubmit when invalid name', async () => {
const { renderResult, onSubmitMock } = setup({
id: 'foo',
name: '~',
Expand All @@ -37,7 +39,10 @@ describe('useWorkspaceForm', () => {
});
expect(renderResult.result.current.formErrors).toEqual(
expect.objectContaining({
name: 'Invalid workspace name',
name: {
code: WorkspaceFormErrorCode.InvalidWorkspaceName,
message: 'Name is invalid. Enter a valid name.',
},
})
);
expect(onSubmitMock).not.toHaveBeenCalled();
Expand All @@ -54,7 +59,10 @@ describe('useWorkspaceForm', () => {
});
expect(renderResult.result.current.formErrors).toEqual(
expect.objectContaining({
features: 'Use case is required. Select a use case.',
features: {
code: WorkspaceFormErrorCode.UseCaseMissing,
message: 'Use case is required. Select a use case.',
},
})
);
expect(onSubmitMock).not.toHaveBeenCalled();
Expand Down
Loading
Loading