Skip to content

Commit

Permalink
Improve ComboBox arrayItem validations to take into account `isBlocki…
Browse files Browse the repository at this point in the history
…ng` param

It adrresses the folowing comment:

"We should also test that behaviour in the form.validate() block.
I think that currently that test would not pass (by setting the type: VALIDATION_TYPES.ARRAY_ITEM and isBlocking: true the form isValid should be false)."
  • Loading branch information
machadoum committed Jul 6, 2021
1 parent 9cf3d17 commit 0f19e23
Show file tree
Hide file tree
Showing 7 changed files with 68 additions and 16 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -43,12 +43,12 @@ export const ComboBoxField = ({ field, euiFieldProps = {}, idAria, ...rest }: Pr
const onCreateComboOption = (value: string) => {
// Note: for now, all validations for a comboBox array item have to be synchronous
// If there is a need to support asynchronous validation, we'll work on it (and will need to update the <EuiComboBox /> logic).
const { isValid } = field.validate({
const { errors } = field.validate({
value,
validationType: VALIDATION_TYPES.ARRAY_ITEM,
validationType: [VALIDATION_TYPES.ARRAY_ITEM],
}) as FieldValidateResponse;

if (!isValid) {
if (errors.length > 0) {
// Return false to explicitly reject the user's input.
return false;
}
Expand All @@ -68,6 +68,12 @@ export const ComboBoxField = ({ field, euiFieldProps = {}, idAria, ...rest }: Pr
}
};

const onBlur = () => {
// Execute field validations. These validation run on the value of the entire field and not on every item.
// Ex: emptyField, validates that the field can't be empty.
field.validate({ value: field.value, validationType: [VALIDATION_TYPES.FIELD] });
};

return (
<EuiFormRow
label={field.label}
Expand All @@ -80,6 +86,7 @@ export const ComboBoxField = ({ field, euiFieldProps = {}, idAria, ...rest }: Pr
{...rest}
>
<EuiComboBox
onBlur={onBlur}
noSuggestions
placeholder={i18n.translate('esUi.forms.comboBoxField.placeHolderText', {
defaultMessage: 'Type and then hit "ENTER"',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,13 +56,13 @@ describe('useField() hook', () => {
await act(async () => {
validateResponse = await fieldHook!.validate({
value: EMPTY_VALUE,
validationType: VALIDATION_TYPES.ARRAY_ITEM,
validationType: [VALIDATION_TYPES.ARRAY_ITEM],
});
});

// Expect a non-blocking validation error
expect(validateResponse!).toEqual({
isValid: false,
isValid: true,
errors: [
{
code: 'ERR_FIELD_MISSING',
Expand Down Expand Up @@ -115,7 +115,7 @@ describe('useField() hook', () => {
await act(async () => {
validateResponse = await fieldHook!.validate({
value: EMPTY_VALUE,
validationType: VALIDATION_TYPES.ARRAY_ITEM,
validationType: [VALIDATION_TYPES.ARRAY_ITEM],
});
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,7 @@ export const useField = <T, FormType = FormData, I = T>(
}: {
formData: any;
value: I;
validationTypeToValidate?: string;
validationTypeToValidate?: string[];
},
clearFieldErrors: FieldHook['clearErrors']
): ValidationError[] | Promise<ValidationError[]> => {
Expand Down Expand Up @@ -221,7 +221,7 @@ export const useField = <T, FormType = FormData, I = T>(

if (
typeof validationTypeToValidate !== 'undefined' &&
validationType !== validationTypeToValidate
!validationTypeToValidate.includes(validationType)
) {
continue;
}
Expand Down Expand Up @@ -267,7 +267,7 @@ export const useField = <T, FormType = FormData, I = T>(

if (
typeof validationTypeToValidate !== 'undefined' &&
validationType !== validationTypeToValidate
!validationTypeToValidate.includes(validationType)
) {
continue;
}
Expand Down Expand Up @@ -343,7 +343,7 @@ export const useField = <T, FormType = FormData, I = T>(
const {
formData = __getFormData$().value,
value: valueToValidate = value,
validationType = VALIDATION_TYPES.FIELD,
validationType = [VALIDATION_TYPES.FIELD],
} = validationData;

setIsValidated(true);
Expand All @@ -365,9 +365,11 @@ export const useField = <T, FormType = FormData, I = T>(
return [...filteredErrors, ..._validationErrors];
});
}
// Don't take into account non blocker validation. Some are just warning (like trying to add a wrong ComboBox item)
const isValid = _validationErrors.filter((e) => e.__isBlocking__ !== false).length === 0;

return {
isValid: _validationErrors.length === 0,
isValid,
errors: _validationErrors,
};
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -518,6 +518,7 @@ describe('useForm() hook', () => {
{
validator: emptyField('error-message'),
type: VALIDATION_TYPES.ARRAY_ITEM,
isBlocking: false,
},
],
}}
Expand All @@ -536,5 +537,39 @@ describe('useForm() hook', () => {

expect(isValid).toBe(true);
});

test('should invalidate a field with a blocking arrayItem validation when validating a form', async () => {
const TestComp = () => {
const { form } = useForm();
formHook = form;

return (
<Form form={form}>
<UseField
path="test-path"
config={{
validations: [
{
validator: emptyField('error-message'),
type: VALIDATION_TYPES.ARRAY_ITEM,
isBlocking: true,
},
],
}}
/>
</Form>
);
};

registerTestBed(TestComp)();

let isValid: boolean = false;

await act(async () => {
isValid = await formHook!.validate();
});

expect(isValid).toBe(false);
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import { set } from '@elastic/safer-lodash-set';

import { FormHook, FieldHook, FormData, FieldConfig, FieldsMap, FormConfig } from '../types';
import { mapFormFields, unflattenObject, Subject, Subscription } from '../lib';
import { VALIDATION_TYPES } from '..';

const DEFAULT_OPTIONS = {
valueChangeDebounceTime: 500,
Expand Down Expand Up @@ -157,8 +158,14 @@ export function useForm<T extends FormData = FormData, I extends FormData = T>(
.filter((field) => field !== undefined);

const formData = getFormData$().value;

const validationResult = await Promise.all(
fieldsToValidate.map((field) => field.validate({ formData }))
fieldsToValidate.map((field) =>
field.validate({
formData,
validationType: [VALIDATION_TYPES.FIELD, VALIDATION_TYPES.ARRAY_ITEM],
})
)
);

if (isMounted.current === false) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ export interface FieldHook<T = unknown, I = T> {
validate: (validateData?: {
formData?: any;
value?: I;
validationType?: string;
validationType?: string[];
}) => FieldValidateResponse | Promise<FieldValidateResponse>;
reset: (options?: { resetValue?: boolean; defaultValue?: T }) => unknown | undefined;
// Flag to indicate if the field value will be included in the form data outputted
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -97,12 +97,13 @@ export const ProcessorTypeField: FunctionComponent<Props> = ({ initialType }) =>
const onCreateComboOption = (value: string) => {
// Note: for now, all validations for a comboBox array item have to be synchronous
// If there is a need to support asynchronous validation, we'll work on it (and will need to update the <EuiComboBox /> logic).
const { isValid } = typeField.validate({

const { errors } = typeField.validate({
value,
validationType: VALIDATION_TYPES.ARRAY_ITEM,
validationType: [VALIDATION_TYPES.ARRAY_ITEM],
}) as FieldValidateResponse;

if (!isValid) {
if (errors.length > 0) {
// Return false to explicitly reject the user's input.
return false;
}
Expand Down

0 comments on commit 0f19e23

Please sign in to comment.