Skip to content

Commit

Permalink
Add validation for workspace color
Browse files Browse the repository at this point in the history
Signed-off-by: Lin Wang <[email protected]>
  • Loading branch information
wanglam committed Aug 9, 2024
1 parent 389fd28 commit 6a4944f
Show file tree
Hide file tree
Showing 9 changed files with 149 additions and 5 deletions.
38 changes: 38 additions & 0 deletions src/plugins/workspace/common/__tests__/utils.test.ts
Original file line number Diff line number Diff line change
@@ -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);
});
});
8 changes: 8 additions & 0 deletions src/plugins/workspace/common/utils.ts
Original file line number Diff line number Diff line change
@@ -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);
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ export enum WorkspaceFormErrorCode {
PermissionSettingOwnerMissing,
InvalidDataSource,
DuplicateDataSource,
InvalidColor,
}

export interface WorkspaceFormError {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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({
Expand Down Expand Up @@ -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(
Expand Down
17 changes: 16 additions & 1 deletion src/plugins/workspace/public/components/workspace_form/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -62,6 +63,9 @@ export const getNumberOfErrors = (formErrors: WorkspaceFormErrors) => {
if (formErrors.features) {
numberOfErrors += 1;
}
if (formErrors.color) {
numberOfErrors += 1;
}
return numberOfErrors;
};

Expand Down Expand Up @@ -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 = {
Expand All @@ -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);
}
Expand Down Expand Up @@ -463,6 +475,9 @@ export const getNumberOfChanges = (
if (newFormData.description !== initialFormData.description) {
count++;
}
if (newFormData.color !== newFormData.color) {
count++;
}
if (
newFormData.features?.length !== initialFormData.features?.length ||
newFormData.features?.some((item) => !initialFormData.features?.includes(item))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down Expand Up @@ -106,6 +110,14 @@ export const WorkspaceFormErrorCallout = ({ errors }: WorkspaceFormErrorCalloutP
message={getSuggestionFromErrorCode(errors.name)}
/>
)}
{errors.color && (
<WorkspaceFormErrorCalloutItem
errorKey={i18n.translate('workspace.form.errorCallout.nameKey', {
defaultMessage: 'Color:',
})}
message={getSuggestionFromErrorCode(errors.color)}
/>
)}
{errors.features && (
<WorkspaceFormErrorCalloutItem
errorKey={i18n.translate('workspace.form.errorCallout.useCaseKey', {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ import { getFirstUseCaseOfFeatureConfigs } from '../../utils';
import { recentWorkspaceManager } from '../../recent_workspace_manager';
import { WorkspaceUseCase } from '../../types';
import { navigateToWorkspaceDetail } from '../utils/workspace';
import { validateWorkspaceColor } from '../../../common/utils';

const defaultHeaderName = i18n.translate('workspace.menu.defaultHeaderName', {
defaultMessage: 'Workspaces',
Expand Down Expand Up @@ -63,6 +64,9 @@ const manageWorkspacesButton = i18n.translate('workspace.menu.button.manageWorks
defaultMessage: 'Manage workspaces',
});

const getValidateWorkspaceColor = (color?: string) =>
validateWorkspaceColor(color) ? color : undefined;

interface Props {
coreStart: CoreStart;
registeredUseCases$: BehaviorSubject<WorkspaceUseCase[]>;
Expand Down Expand Up @@ -111,7 +115,7 @@ export const WorkspaceMenu = ({ coreStart, registeredUseCases$ }: Props) => {
size="s"
type="space"
name={currentWorkspace.name}
color={currentWorkspace.color}
color={getValidateWorkspaceColor(currentWorkspace.color)}
initialsLength={2}
/>
</EuiButtonEmpty>
Expand Down Expand Up @@ -148,7 +152,7 @@ export const WorkspaceMenu = ({ coreStart, registeredUseCases$ }: Props) => {
size="s"
type="space"
name={workspace.name}
color={workspace.color}
color={getValidateWorkspaceColor(workspace.color)}
initialsLength={2}
/>
}
Expand Down Expand Up @@ -196,7 +200,7 @@ export const WorkspaceMenu = ({ coreStart, registeredUseCases$ }: Props) => {
size="m"
type="space"
name={currentWorkspaceName}
color={currentWorkspace?.color}
color={getValidateWorkspaceColor(currentWorkspace?.color)}
initialsLength={2}
/>
</EuiFlexItem>
Expand Down
11 changes: 10 additions & 1 deletion src/plugins/workspace/server/routes/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -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()),
Expand Down

0 comments on commit 6a4944f

Please sign in to comment.