diff --git a/frontend/src/utils/validateParameterValue.test.ts b/frontend/src/utils/validateParameterValue.test.ts index 0186607fec..c78f978955 100644 --- a/frontend/src/utils/validateParameterValue.test.ts +++ b/frontend/src/utils/validateParameterValue.test.ts @@ -1,39 +1,67 @@ import { validateParameterValue } from 'utils/validateParameterValue'; +const createStrategy = (type: string, required: boolean) => { + return { type, required }; +}; + test('validateParameterValue string', () => { const fn = validateParameterValue; - expect(fn({ type: 'string', required: false }, '')).toBeUndefined(); - expect(fn({ type: 'string', required: false }, 'a')).toBeUndefined(); - expect(fn({ type: 'string', required: true }, '')).not.toBeUndefined(); - expect(fn({ type: 'string', required: true }, 'a')).toBeUndefined(); + expect(fn(createStrategy('string', false), undefined)).toBeUndefined(); + expect(fn(createStrategy('string', false), '')).toBeUndefined(); + expect(fn(createStrategy('string', true), 'a')).toBeUndefined(); + expect(fn(createStrategy('string', true), undefined)).not.toBeUndefined(); + expect(fn(createStrategy('string', true), '')).not.toBeUndefined(); + expect(fn(createStrategy('string', true), 'a')).toBeUndefined(); }); test('validateParameterValue list', () => { const fn = validateParameterValue; - expect(fn({ type: 'list', required: false }, '')).toBeUndefined(); - expect(fn({ type: 'list', required: false }, 'a,b')).toBeUndefined(); - expect(fn({ type: 'list', required: true }, '')).not.toBeUndefined(); - expect(fn({ type: 'list', required: true }, 'a,b')).toBeUndefined(); + expect(fn(createStrategy('list', false), undefined)).toBeUndefined(); + expect(fn(createStrategy('list', false), '')).toBeUndefined(); + expect(fn(createStrategy('list', false), 'a,b')).toBeUndefined(); + expect(fn(createStrategy('list', true), undefined)).not.toBeUndefined(); + expect(fn(createStrategy('list', true), '')).not.toBeUndefined(); + expect(fn(createStrategy('list', true), 'a,b')).toBeUndefined(); }); test('validateParameterValue number', () => { const fn = validateParameterValue; - expect(fn({ type: 'number', required: false }, '')).toBeUndefined(); - expect(fn({ type: 'number', required: false }, 'a')).not.toBeUndefined(); - expect(fn({ type: 'number', required: false }, '1')).toBeUndefined(); - expect(fn({ type: 'number', required: true }, '')).not.toBeUndefined(); - expect(fn({ type: 'number', required: true }, 'a')).not.toBeUndefined(); - expect(fn({ type: 'number', required: true }, '1')).toBeUndefined(); + expect(fn(createStrategy('number', false), undefined)).toBeUndefined(); + expect(fn(createStrategy('number', false), '')).toBeUndefined(); + expect(fn(createStrategy('number', false), 'a')).not.toBeUndefined(); + expect(fn(createStrategy('number', false), '0')).toBeUndefined(); + expect(fn(createStrategy('number', false), '1')).toBeUndefined(); + expect(fn(createStrategy('number', true), undefined)).not.toBeUndefined(); + expect(fn(createStrategy('number', true), '')).not.toBeUndefined(); + expect(fn(createStrategy('number', true), 'a')).not.toBeUndefined(); + expect(fn(createStrategy('number', true), '0')).toBeUndefined(); + expect(fn(createStrategy('number', true), '1')).toBeUndefined(); }); test('validateParameterValue boolean', () => { const fn = validateParameterValue; - expect(fn({ type: 'boolean', required: false }, '')).toBeUndefined(); - expect(fn({ type: 'boolean', required: false }, 'true')).toBeUndefined(); - expect(fn({ type: 'boolean', required: false }, 'false')).toBeUndefined(); - expect(fn({ type: 'boolean', required: false }, 'a')).not.toBeUndefined(); - expect(fn({ type: 'boolean', required: true }, '')).toBeUndefined(); - expect(fn({ type: 'boolean', required: true }, 'true')).toBeUndefined(); - expect(fn({ type: 'boolean', required: true }, 'false')).toBeUndefined(); - expect(fn({ type: 'boolean', required: true }, 'a')).not.toBeUndefined(); + expect(fn(createStrategy('boolean', false), undefined)).toBeUndefined(); + expect(fn(createStrategy('boolean', false), '')).toBeUndefined(); + expect(fn(createStrategy('boolean', false), 'true')).toBeUndefined(); + expect(fn(createStrategy('boolean', false), 'false')).toBeUndefined(); + expect(fn(createStrategy('boolean', false), 'a')).toBeUndefined(); + expect(fn(createStrategy('boolean', true), undefined)).toBeUndefined(); + expect(fn(createStrategy('boolean', true), '')).toBeUndefined(); + expect(fn(createStrategy('boolean', true), 'true')).toBeUndefined(); + expect(fn(createStrategy('boolean', true), 'false')).toBeUndefined(); + expect(fn(createStrategy('boolean', true), 'a')).toBeUndefined(); +}); + +test('validateParameterValue percentage', () => { + const fn = validateParameterValue; + expect(fn(createStrategy('percentage', false), undefined)).toBeUndefined(); + expect(fn(createStrategy('percentage', false), '')).toBeUndefined(); + expect(fn(createStrategy('percentage', false), 'a')).toBeUndefined(); + expect(fn(createStrategy('percentage', false), '0')).toBeUndefined(); + expect(fn(createStrategy('percentage', false), '1')).toBeUndefined(); + expect(fn(createStrategy('percentage', true), undefined)).toBeUndefined(); + expect(fn(createStrategy('percentage', true), '')).toBeUndefined(); + expect(fn(createStrategy('percentage', true), 'a')).toBeUndefined(); + expect(fn(createStrategy('percentage', true), '0')).toBeUndefined(); + expect(fn(createStrategy('percentage', true), '1')).toBeUndefined(); }); diff --git a/frontend/src/utils/validateParameterValue.ts b/frontend/src/utils/validateParameterValue.ts index 6f011d93d7..50b9ae8c98 100644 --- a/frontend/src/utils/validateParameterValue.ts +++ b/frontend/src/utils/validateParameterValue.ts @@ -9,28 +9,27 @@ export const validateParameterValue = ( ): string | undefined => { const { type, required } = definition; - // The components for booleans and percentages can't yet show error messages. - // We should not enforce `required` until these errors can be shown in the UI. - const shouldValidateRequired = - type === 'string' || type === 'list' || type === 'number'; - if (shouldValidateRequired && required && value === '') { + // Some input components in the feature strategy form can't show error messages. + // We should not validate those fields until their errors can be shown in the UI. + if (type !== 'string' && type !== 'list' && type !== 'number') { + return; + } + + // If we're editing a feature strategy that has a newly added parameter, + // the value will be `undefined` until the field has been edited. + if (required && (typeof value === 'undefined' || value === '')) { return 'Field is required'; } - const shouldValidateNumeric = type === 'percentage' || type === 'number'; - if (shouldValidateNumeric && !isValidNumberOrEmpty(value)) { + if (type === 'number' && !isValidNumberOrEmpty(value)) { return 'Not a valid number.'; } - - if (type === 'boolean' && !isValidBooleanOrEmpty(value)) { - return 'Not a valid boolean.'; - } }; const isValidNumberOrEmpty = (value: string | number | undefined): boolean => { - return value === '' || /^\d+$/.test(String(value)); -}; - -const isValidBooleanOrEmpty = (value: string | number | undefined): boolean => { - return value === '' || value === 'true' || value === 'false'; + return ( + typeof value === 'undefined' || + value === '' || + /^\d+$/.test(String(value)) + ); };