From 5641498fbb7600f3658d44eb79a85b478325048f Mon Sep 17 00:00:00 2001 From: Pablo Machado Date: Tue, 13 Jul 2021 10:06:53 +0200 Subject: [PATCH] Fix error when validating the form with non blocking validations (#103629) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Fix error when validating the form with non blocking validations issue: https://github.com/elastic/kibana/issues/102338 * Add a flag to only validate blocking validation when validating the form * Refactor bypass validation Co-authored-by: SeĢbastien Loix Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> --- .../hook_form_lib/hooks/use_field.test.tsx | 140 ++++++++++++++++++ .../forms/hook_form_lib/hooks/use_field.ts | 38 +++-- .../hook_form_lib/hooks/use_form.test.tsx | 75 +++++++++- .../forms/hook_form_lib/hooks/use_form.ts | 7 +- .../static/forms/hook_form_lib/types.ts | 5 +- 5 files changed, 250 insertions(+), 15 deletions(-) create mode 100644 src/plugins/es_ui_shared/static/forms/hook_form_lib/hooks/use_field.test.tsx diff --git a/src/plugins/es_ui_shared/static/forms/hook_form_lib/hooks/use_field.test.tsx b/src/plugins/es_ui_shared/static/forms/hook_form_lib/hooks/use_field.test.tsx new file mode 100644 index 00000000000000..0e5dd7eb82e720 --- /dev/null +++ b/src/plugins/es_ui_shared/static/forms/hook_form_lib/hooks/use_field.test.tsx @@ -0,0 +1,140 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +import { act } from 'react-dom/test-utils'; +import { registerTestBed } from '../shared_imports'; + +import { Form, UseField } from '../components'; +import React from 'react'; +import { useForm } from '.'; +import { emptyField } from '../../helpers/field_validators'; +import { FieldHook, FieldValidateResponse, VALIDATION_TYPES } from '..'; + +describe('useField() hook', () => { + describe('field.validate()', () => { + const EMPTY_VALUE = ' '; + + test('It should not invalidate a field with arrayItem validation when isBlocking is false', async () => { + let fieldHook: FieldHook; + + const TestField = ({ field }: { field: FieldHook }) => { + fieldHook = field; + return null; + }; + + const TestForm = () => { + const { form } = useForm(); + + return ( +
+ + + ); + }; + + registerTestBed(TestForm)(); + + let validateResponse: FieldValidateResponse; + + await act(async () => { + validateResponse = await fieldHook!.validate({ + value: EMPTY_VALUE, + validationType: VALIDATION_TYPES.ARRAY_ITEM, + }); + }); + + // validation fails for ARRAY_ITEM with a non-blocking validation error + expect(validateResponse!).toEqual({ + isValid: false, + errors: [ + { + code: 'ERR_FIELD_MISSING', + path: 'test-path', + message: 'error-message', + __isBlocking__: false, + validationType: 'arrayItem', + }, + ], + }); + + // expect the field to be valid because the validation error is non-blocking + expect(fieldHook!.isValid).toBe(true); + }); + + test('It should invalidate an arrayItem field when isBlocking is true', async () => { + let fieldHook: FieldHook; + + const TestField = ({ field }: { field: FieldHook }) => { + fieldHook = field; + return null; + }; + + const TestForm = () => { + const { form } = useForm(); + + return ( +
+ + + ); + }; + + registerTestBed(TestForm)(); + + let validateResponse: FieldValidateResponse; + + await act(async () => { + validateResponse = await fieldHook!.validate({ + value: EMPTY_VALUE, + validationType: VALIDATION_TYPES.ARRAY_ITEM, + }); + }); + + // validation fails for ARRAY_ITEM with a blocking validation error + expect(validateResponse!).toEqual({ + isValid: false, + errors: [ + { + code: 'ERR_FIELD_MISSING', + path: 'test-path', + message: 'error-message', + __isBlocking__: true, + validationType: 'arrayItem', + }, + ], + }); + + // expect the field to be invalid because the validation error is blocking + expect(fieldHook!.isValid).toBe(false); + }); + }); +}); diff --git a/src/plugins/es_ui_shared/static/forms/hook_form_lib/hooks/use_field.ts b/src/plugins/es_ui_shared/static/forms/hook_form_lib/hooks/use_field.ts index 0cf1bb36016671..77bb17d7b9e601 100644 --- a/src/plugins/es_ui_shared/static/forms/hook_form_lib/hooks/use_field.ts +++ b/src/plugins/es_ui_shared/static/forms/hook_form_lib/hooks/use_field.ts @@ -15,6 +15,7 @@ import { FieldValidateResponse, ValidationError, FormData, + ValidationConfig, } from '../types'; import { FIELD_TYPES, VALIDATION_TYPES } from '../constants'; @@ -189,10 +190,12 @@ export const useField = ( { formData, value: valueToValidate, + onlyBlocking: runOnlyBlockingValidations, validationTypeToValidate, }: { formData: any; value: I; + onlyBlocking: boolean; validationTypeToValidate?: string; }, clearFieldErrors: FieldHook['clearErrors'] @@ -203,10 +206,31 @@ export const useField = ( // By default, for fields that have an asynchronous validation // we will clear the errors as soon as the field value changes. - clearFieldErrors([VALIDATION_TYPES.FIELD, VALIDATION_TYPES.ASYNC]); + clearFieldErrors([ + validationTypeToValidate ?? VALIDATION_TYPES.FIELD, + VALIDATION_TYPES.ASYNC, + ]); cancelInflightValidation(); + const doByPassValidation = ({ + type: validationType, + isBlocking, + }: ValidationConfig) => { + if ( + typeof validationTypeToValidate !== 'undefined' && + validationType !== validationTypeToValidate + ) { + return true; + } + + if (runOnlyBlockingValidations && isBlocking === false) { + return true; + } + + return false; + }; + const runAsync = async () => { const validationErrors: ValidationError[] = []; @@ -219,10 +243,7 @@ export const useField = ( type: validationType = VALIDATION_TYPES.FIELD, } = validation; - if ( - typeof validationTypeToValidate !== 'undefined' && - validationType !== validationTypeToValidate - ) { + if (doByPassValidation(validation)) { continue; } @@ -265,10 +286,7 @@ export const useField = ( type: validationType = VALIDATION_TYPES.FIELD, } = validation; - if ( - typeof validationTypeToValidate !== 'undefined' && - validationType !== validationTypeToValidate - ) { + if (doByPassValidation(validation)) { continue; } @@ -344,6 +362,7 @@ export const useField = ( formData = __getFormData$().value, value: valueToValidate = value, validationType, + onlyBlocking = false, } = validationData; setIsValidated(true); @@ -377,6 +396,7 @@ export const useField = ( formData, value: valueToValidate, validationTypeToValidate: validationType, + onlyBlocking, }, clearErrors ); diff --git a/src/plugins/es_ui_shared/static/forms/hook_form_lib/hooks/use_form.test.tsx b/src/plugins/es_ui_shared/static/forms/hook_form_lib/hooks/use_form.test.tsx index 40fc179c73c3bb..92a9876f1cd301 100644 --- a/src/plugins/es_ui_shared/static/forms/hook_form_lib/hooks/use_form.test.tsx +++ b/src/plugins/es_ui_shared/static/forms/hook_form_lib/hooks/use_form.test.tsx @@ -10,7 +10,7 @@ import React, { useEffect } from 'react'; import { act } from 'react-dom/test-utils'; import { registerTestBed, getRandomString, TestBed } from '../shared_imports'; - +import { emptyField } from '../../helpers/field_validators'; import { Form, UseField } from '../components'; import { FormSubmitHandler, @@ -18,7 +18,8 @@ import { FormHook, ValidationFunc, FieldConfig, -} from '../types'; + VALIDATION_TYPES, +} from '..'; import { useForm } from './use_form'; interface MyForm { @@ -501,4 +502,74 @@ describe('useForm() hook', () => { expect(isValid).toBeUndefined(); // Make sure it is "undefined" and not "false". }); }); + + describe('form.validate()', () => { + test('should not invalidate a field with arrayItem validation when validating a form', async () => { + const TestComp = () => { + const { form } = useForm(); + formHook = form; + + return ( +
+ + + ); + }; + + registerTestBed(TestComp)(); + + let isValid: boolean = false; + + await act(async () => { + isValid = await formHook!.validate(); + }); + + 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 ( +
+ + + ); + }; + + registerTestBed(TestComp)(); + + let isValid: boolean = false; + + await act(async () => { + isValid = await formHook!.validate(); + }); + + expect(isValid).toBe(false); + }); + }); }); diff --git a/src/plugins/es_ui_shared/static/forms/hook_form_lib/hooks/use_form.ts b/src/plugins/es_ui_shared/static/forms/hook_form_lib/hooks/use_form.ts index 11c73076a1b358..dcf2cb37d65429 100644 --- a/src/plugins/es_ui_shared/static/forms/hook_form_lib/hooks/use_form.ts +++ b/src/plugins/es_ui_shared/static/forms/hook_form_lib/hooks/use_form.ts @@ -151,14 +151,14 @@ export function useForm( }, [fieldsToArray]); const validateFields: FormHook['__validateFields'] = useCallback( - async (fieldNames) => { + async (fieldNames, onlyBlocking = false) => { const fieldsToValidate = fieldNames .map((name) => fieldsRefs.current[name]) .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, onlyBlocking })) ); if (isMounted.current === false) { @@ -310,7 +310,8 @@ export function useForm( if (fieldsToValidate.length === 0) { isFormValid = fieldsArray.every(isFieldValid); } else { - ({ isFormValid } = await validateFields(fieldsToValidate.map((field) => field.path))); + const fieldPathsToValidate = fieldsToValidate.map((field) => field.path); + ({ isFormValid } = await validateFields(fieldPathsToValidate, true)); } setIsValid(isFormValid); diff --git a/src/plugins/es_ui_shared/static/forms/hook_form_lib/types.ts b/src/plugins/es_ui_shared/static/forms/hook_form_lib/types.ts index b08cba79e98773..4e9ff29f0cdd36 100644 --- a/src/plugins/es_ui_shared/static/forms/hook_form_lib/types.ts +++ b/src/plugins/es_ui_shared/static/forms/hook_form_lib/types.ts @@ -55,7 +55,9 @@ export interface FormHook __addField: (field: FieldHook) => void; __removeField: (fieldNames: string | string[]) => void; __validateFields: ( - fieldNames: string[] + fieldNames: string[], + /** Run only blocking validations */ + onlyBlocking?: boolean ) => Promise<{ areFieldsValid: boolean; isFormValid: boolean | undefined }>; __updateFormDataAt: (field: string, value: unknown) => void; __updateDefaultValueAt: (field: string, value: unknown) => void; @@ -138,6 +140,7 @@ export interface FieldHook { formData?: any; value?: I; validationType?: string; + onlyBlocking?: boolean; }) => FieldValidateResponse | Promise; reset: (options?: { resetValue?: boolean; defaultValue?: T }) => unknown | undefined; // Flag to indicate if the field value will be included in the form data outputted