From efa37ef85a3603f27e4b06edb6d56fc1ca169738 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" Date: Mon, 12 Aug 2024 07:45:12 +0000 Subject: [PATCH] [Workspace]Fix page crash caused by invalid workspace color (#7671) * Add validation for workspace color Signed-off-by: Lin Wang * Changeset file for PR #7671 created/updated --------- Signed-off-by: Lin Wang Co-authored-by: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com> (cherry picked from commit 248c8ff0c3ef598c45f140bdf70f8964e6da2aa2) Signed-off-by: github-actions[bot] --- changelogs/fragments/7671.yml | 2 + .../workspace/common/__tests__/utils.test.ts | 38 ++++++++++++++++ src/plugins/workspace/common/utils.ts | 8 ++++ .../public/components/workspace_form/types.ts | 1 + .../components/workspace_form/utils.test.ts | 44 +++++++++++++++++++ .../public/components/workspace_form/utils.ts | 17 ++++++- .../workspace_form_error_callout.test.tsx | 13 ++++++ .../workspace_form_error_callout.tsx | 12 +++++ .../workspace_menu/workspace_menu.tsx | 10 +++-- src/plugins/workspace/server/routes/index.ts | 11 ++++- 10 files changed, 151 insertions(+), 5 deletions(-) create mode 100644 changelogs/fragments/7671.yml create mode 100644 src/plugins/workspace/common/__tests__/utils.test.ts create mode 100644 src/plugins/workspace/common/utils.ts diff --git a/changelogs/fragments/7671.yml b/changelogs/fragments/7671.yml new file mode 100644 index 000000000000..8d88fec48eee --- /dev/null +++ b/changelogs/fragments/7671.yml @@ -0,0 +1,2 @@ +fix: +- [Workspace]Fix page crash caused by invalid workspace color ([#7671](/~https://github.com/opensearch-project/OpenSearch-Dashboards/pull/7671)) \ No newline at end of file diff --git a/src/plugins/workspace/common/__tests__/utils.test.ts b/src/plugins/workspace/common/__tests__/utils.test.ts new file mode 100644 index 000000000000..c9369fe29f6b --- /dev/null +++ b/src/plugins/workspace/common/__tests__/utils.test.ts @@ -0,0 +1,38 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +import { validateWorkspaceColor } from '../utils'; + +describe('validateWorkspaceColor', () => { + it('should return true for a valid 6-digit hex color code', () => { + expect(validateWorkspaceColor('#ABCDEF')).toBe(true); + expect(validateWorkspaceColor('#123456')).toBe(true); + }); + + it('should return true for a valid 3-digit hex color code', () => { + expect(validateWorkspaceColor('#ABC')).toBe(true); + expect(validateWorkspaceColor('#DEF')).toBe(true); + }); + + it('should return false for an invalid color code', () => { + expect(validateWorkspaceColor('#GHI')).toBe(false); + expect(validateWorkspaceColor('#12345')).toBe(false); + expect(validateWorkspaceColor('#ABCDEFG')).toBe(false); + expect(validateWorkspaceColor('ABCDEF')).toBe(false); + }); + + it('should return false for an empty string', () => { + expect(validateWorkspaceColor('')).toBe(false); + }); + + it('should return false for undefined', () => { + expect(validateWorkspaceColor()).toBe(false); + }); + + it('should be case-insensitive', () => { + expect(validateWorkspaceColor('#abcdef')).toBe(true); + expect(validateWorkspaceColor('#ABC')).toBe(true); + }); +}); diff --git a/src/plugins/workspace/common/utils.ts b/src/plugins/workspace/common/utils.ts new file mode 100644 index 000000000000..8cc67a44644e --- /dev/null +++ b/src/plugins/workspace/common/utils.ts @@ -0,0 +1,8 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +// Reference /~https://github.com/opensearch-project/oui/blob/main/src/services/color/is_valid_hex.ts +export const validateWorkspaceColor = (color?: string) => + !!color && /(^#[0-9A-F]{6}$)|(^#[0-9A-F]{3}$)/i.test(color); diff --git a/src/plugins/workspace/public/components/workspace_form/types.ts b/src/plugins/workspace/public/components/workspace_form/types.ts index 0e2ab1631fc9..7b83bc36ddd3 100644 --- a/src/plugins/workspace/public/components/workspace_form/types.ts +++ b/src/plugins/workspace/public/components/workspace_form/types.ts @@ -55,6 +55,7 @@ export enum WorkspaceFormErrorCode { PermissionSettingOwnerMissing, InvalidDataSource, DuplicateDataSource, + InvalidColor, } export interface WorkspaceFormError { diff --git a/src/plugins/workspace/public/components/workspace_form/utils.test.ts b/src/plugins/workspace/public/components/workspace_form/utils.test.ts index 3256f255b0a4..3a45165044d7 100644 --- a/src/plugins/workspace/public/components/workspace_form/utils.test.ts +++ b/src/plugins/workspace/public/components/workspace_form/utils.test.ts @@ -179,6 +179,12 @@ describe('validateWorkspaceForm', () => { message: 'Name is invalid. Enter a valid name.', }); }); + it('should return error if color is invalid', () => { + expect(validateWorkspaceForm({ color: 'QWERTY' }, false).color).toEqual({ + code: WorkspaceFormErrorCode.InvalidColor, + message: 'Color is invalid. Enter a valid color.', + }); + }); it('should return error if use case is empty', () => { expect(validateWorkspaceForm({}, false).features).toEqual({ code: WorkspaceFormErrorCode.UseCaseMissing, @@ -393,6 +399,18 @@ describe('getNumberOfErrors', () => { }) ).toEqual(1); }); + + it('should return consistent color errors count', () => { + expect( + getNumberOfErrors({ + name: { + code: WorkspaceFormErrorCode.InvalidColor, + message: '', + }, + }) + ).toEqual(1); + }); + it('should return consistent permission settings errors count', () => { expect( getNumberOfErrors({ @@ -461,6 +479,32 @@ describe('getNumberOfChanges', () => { ) ).toEqual(1); }); + it('should return consistent color changes count', () => { + expect( + getNumberOfChanges( + { + name: 'foo', + color: '#000', + }, + { + name: 'foo', + color: '#000', + } + ) + ).toEqual(0); + expect( + getNumberOfChanges( + { + name: 'foo', + color: '#000', + }, + { + name: 'foo', + color: '#001', + } + ) + ).toEqual(1); + }); it('should return consistent features changes count', () => { expect( getNumberOfChanges( diff --git a/src/plugins/workspace/public/components/workspace_form/utils.ts b/src/plugins/workspace/public/components/workspace_form/utils.ts index 7588178d8c94..2ea76756d3d8 100644 --- a/src/plugins/workspace/public/components/workspace_form/utils.ts +++ b/src/plugins/workspace/public/components/workspace_form/utils.ts @@ -30,6 +30,7 @@ import { WorkspaceUserPermissionSetting, } from './types'; import { DataSource } from '../../../common/types'; +import { validateWorkspaceColor } from '../../../common/utils'; export const appendDefaultFeatureIds = (ids: string[]) => { // concat default checked ids and unique the result @@ -62,6 +63,9 @@ export const getNumberOfErrors = (formErrors: WorkspaceFormErrors) => { if (formErrors.features) { numberOfErrors += 1; } + if (formErrors.color) { + numberOfErrors += 1; + } return numberOfErrors; }; @@ -308,7 +312,7 @@ export const validateWorkspaceForm = ( isPermissionEnabled: boolean ) => { const formErrors: WorkspaceFormErrors = {}; - const { name, permissionSettings, features, selectedDataSources } = formData; + const { name, permissionSettings, color, features, selectedDataSources } = formData; if (name && name.trim()) { if (!isValidFormTextInput(name)) { formErrors.name = { @@ -334,6 +338,14 @@ export const validateWorkspaceForm = ( }), }; } + if (color && !validateWorkspaceColor(color)) { + formErrors.color = { + code: WorkspaceFormErrorCode.InvalidColor, + message: i18n.translate('workspace.form.features.empty', { + defaultMessage: 'Color is invalid. Enter a valid color.', + }), + }; + } if (isPermissionEnabled) { formErrors.permissionSettings = validatePermissionSetting(permissionSettings); } @@ -463,6 +475,9 @@ export const getNumberOfChanges = ( if (newFormData.description !== initialFormData.description) { count++; } + if (newFormData.color !== initialFormData.color) { + count++; + } if ( newFormData.features?.length !== initialFormData.features?.length || newFormData.features?.some((item) => !initialFormData.features?.includes(item)) diff --git a/src/plugins/workspace/public/components/workspace_form/workspace_form_error_callout.test.tsx b/src/plugins/workspace/public/components/workspace_form/workspace_form_error_callout.test.tsx index e248bf202257..d8a582ce97eb 100644 --- a/src/plugins/workspace/public/components/workspace_form/workspace_form_error_callout.test.tsx +++ b/src/plugins/workspace/public/components/workspace_form/workspace_form_error_callout.test.tsx @@ -48,6 +48,19 @@ describe('WorkspaceFormErrorCallout', () => { expect(renderResult.getByText('Name: Enter a valid name.')).toBeInTheDocument(); }); + it('should render color suggestion', () => { + const { renderResult } = setup({ + errors: { + color: { + code: WorkspaceFormErrorCode.InvalidColor, + message: '', + }, + }, + }); + + expect(renderResult.getByText('Color: Enter a valid color.')).toBeInTheDocument(); + }); + it('should render use case suggestion', () => { const { renderResult } = setup({ errors: { diff --git a/src/plugins/workspace/public/components/workspace_form/workspace_form_error_callout.tsx b/src/plugins/workspace/public/components/workspace_form/workspace_form_error_callout.tsx index e7388f1909b3..1d90235ed82c 100644 --- a/src/plugins/workspace/public/components/workspace_form/workspace_form_error_callout.tsx +++ b/src/plugins/workspace/public/components/workspace_form/workspace_form_error_callout.tsx @@ -42,6 +42,10 @@ const getSuggestionFromErrorCode = (error: WorkspaceFormError) => { return i18n.translate('workspace.form.errorCallout.permissionSettingOwnerMissing', { defaultMessage: 'Add a workspace owner.', }); + case WorkspaceFormErrorCode.InvalidColor: + return i18n.translate('workspace.form.errorCallout.invalidColor', { + defaultMessage: 'Enter a valid color.', + }); default: return error.message; } @@ -106,6 +110,14 @@ export const WorkspaceFormErrorCallout = ({ errors }: WorkspaceFormErrorCalloutP message={getSuggestionFromErrorCode(errors.name)} /> )} + {errors.color && ( + + )} {errors.features && ( + validateWorkspaceColor(color) ? color : undefined; + interface Props { coreStart: CoreStart; registeredUseCases$: BehaviorSubject; @@ -114,7 +118,7 @@ export const WorkspaceMenu = ({ coreStart, registeredUseCases$ }: Props) => { size="s" type="space" name={currentWorkspace.name} - color={currentWorkspace.color} + color={getValidWorkspaceColor(currentWorkspace.color)} initialsLength={2} /> @@ -151,7 +155,7 @@ export const WorkspaceMenu = ({ coreStart, registeredUseCases$ }: Props) => { size="s" type="space" name={workspace.name} - color={workspace.color} + color={getValidWorkspaceColor(workspace.color)} initialsLength={2} /> } @@ -199,7 +203,7 @@ export const WorkspaceMenu = ({ coreStart, registeredUseCases$ }: Props) => { size="m" type="space" name={currentWorkspaceName} - color={currentWorkspace?.color} + color={getValidWorkspaceColor(currentWorkspace?.color)} initialsLength={2} /> diff --git a/src/plugins/workspace/server/routes/index.ts b/src/plugins/workspace/server/routes/index.ts index b57b0a529d1c..4676a743f9d7 100644 --- a/src/plugins/workspace/server/routes/index.ts +++ b/src/plugins/workspace/server/routes/index.ts @@ -10,6 +10,7 @@ import { IWorkspaceClientImpl, WorkspaceAttributeWithPermission } from '../types import { SavedObjectsPermissionControlContract } from '../permission_control/client'; import { registerDuplicateRoute } from './duplicate'; import { transferCurrentUserInPermissions } from '../utils'; +import { validateWorkspaceColor } from '../../common/utils'; export const WORKSPACES_API_BASE_URL = '/api/workspaces'; @@ -40,7 +41,15 @@ const settingsSchema = schema.object({ const workspaceOptionalAttributesSchema = { description: schema.maybe(schema.string()), features: schema.maybe(schema.arrayOf(schema.string())), - color: schema.maybe(schema.string()), + color: schema.maybe( + schema.string({ + validate: (color) => { + if (!validateWorkspaceColor(color)) { + return 'invalid workspace color format'; + } + }, + }) + ), icon: schema.maybe(schema.string()), defaultVISTheme: schema.maybe(schema.string()), reserved: schema.maybe(schema.boolean()),