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()),