From d3fb0af7c6253f15fd37fc592690c5772ce7211e Mon Sep 17 00:00:00 2001 From: Anton Dosov Date: Wed, 14 Sep 2022 12:35:47 +0200 Subject: [PATCH 1/3] fix trigger validation when field is removed --- .../static/forms/hook_form_lib/hooks/use_field.ts | 8 +++++++- .../form_validation/downsample_interval.test.ts | 9 ++++++--- 2 files changed, 13 insertions(+), 4 deletions(-) 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 206db21b50a0b..6ce084cb750b4 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 @@ -602,8 +602,14 @@ export const useField = ( // We only remove the field from the form "fieldsRefs" map when its path // changes (which in practice never occurs) or whenever the unmounts __removeField(path); + + // We also have to trigger validation of dependant fields + const dependantFields = fieldsToValidateOnChange?.filter((f) => f !== path); + if (dependantFields?.length) { + validateFields(dependantFields); + } }; - }, [path, __removeField]); + }, [path, __removeField, fieldsToValidateOnChange, validateFields]); // Value change: notify prop listener ( {...}}) // We have a separate useEffect for this as the "onChange" handler pass through prop diff --git a/x-pack/plugins/index_lifecycle_management/__jest__/client_integration/edit_policy/form_validation/downsample_interval.test.ts b/x-pack/plugins/index_lifecycle_management/__jest__/client_integration/edit_policy/form_validation/downsample_interval.test.ts index b24a6d1a32cb7..8ad72ca3d0a82 100644 --- a/x-pack/plugins/index_lifecycle_management/__jest__/client_integration/edit_policy/form_validation/downsample_interval.test.ts +++ b/x-pack/plugins/index_lifecycle_management/__jest__/client_integration/edit_policy/form_validation/downsample_interval.test.ts @@ -112,10 +112,13 @@ describe(' downsample interval validation', () => { 'cold' ); - // disable warm phase; + // disable warm phase, check that we now get an error because of the hot phase; await actions.togglePhase('warm'); - // TODO: there is a bug that disabling a phase doesn't trigger downsample validation in other phases, - // users can work around it by changing the value + actions.errors.waitForValidation(); + actions.errors.expectMessages( + ['Must be greater than and a multiple of the hot phase value (60m)'], + 'cold' + ); await actions.cold.downsample.setDownsampleInterval('120', 'm'); actions.errors.waitForValidation(); actions.errors.expectMessages([], 'cold'); From 5f85cbac57b17a5cfab54f2e1254a6f9c8eb2f70 Mon Sep 17 00:00:00 2001 From: Anton Dosov Date: Wed, 14 Sep 2022 13:34:52 +0200 Subject: [PATCH 2/3] add a unit test --- .../hook_form_lib/hooks/use_field.test.tsx | 86 +++++++++++++++---- 1 file changed, 69 insertions(+), 17 deletions(-) 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 index c5e24a578bd86..98050a81ade9a 100644 --- 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 @@ -16,8 +16,6 @@ import { emptyField } from '../../helpers/field_validators'; import { FieldHook, FieldValidateResponse, VALIDATION_TYPES, FieldConfig } from '..'; describe('useField() hook', () => { - let fieldHook: FieldHook; - beforeAll(() => { jest.useFakeTimers(); }); @@ -26,22 +24,23 @@ describe('useField() hook', () => { jest.useRealTimers(); }); - const TestField = ({ field }: { field: FieldHook }) => { - fieldHook = field; - return null; - }; - - const getTestForm = (config?: FieldConfig) => () => { - const { form } = useForm(); - - return ( -
- - - ); - }; - describe('field.validate()', () => { + let fieldHook: FieldHook; + const TestField = ({ field }: { field: FieldHook }) => { + fieldHook = field; + return null; + }; + + const getTestForm = (config?: FieldConfig) => () => { + const { form } = useForm(); + + return ( +
+ + + ); + }; + const EMPTY_VALUE = ' '; test('it should not invalidate a field with arrayItem validation when isBlocking is false', async () => { @@ -149,4 +148,57 @@ describe('useField() hook', () => { expect(validatorFn).toBeCalledTimes(0); }); }); + + describe('fieldsToValidateOnChange', () => { + let fieldHook1: FieldHook; + let fieldHook2: FieldHook; + const TestField1 = ({ field }: { field: FieldHook }) => { + fieldHook1 = field; + return null; + }; + + const TestField2 = ({ field }: { field: FieldHook }) => { + fieldHook2 = field; + return null; + }; + + const getTestForm = + (configField1?: FieldConfig, configField2?: FieldConfig) => + ({ showField1, showField2 }: { showField1: boolean; showField2: boolean }) => { + const { form } = useForm(); + + return ( +
+ {showField1 && } + {showField2 && } + + ); + }; + + test('validates dependent fields on unmount', async () => { + const field2ValidatorFn = jest.fn(); + const TestForm = getTestForm( + { + fieldsToValidateOnChange: ['field1', 'field2'], + }, + { + validations: [ + { + validator: field2ValidatorFn, + }, + ], + } + ); + + const wrapper = registerTestBed(TestForm, { + memoryRouter: { wrapComponent: false }, + })({ showField1: true, showField2: true }); + expect(field2ValidatorFn).toBeCalledTimes(0); + + await act(async () => { + wrapper.setProps({ showField1: false }); + }); + expect(field2ValidatorFn).toBeCalledTimes(1); + }); + }); }); From 75b4fcd9706cda39889e520ca563c523292f5ad0 Mon Sep 17 00:00:00 2001 From: Anton Dosov Date: Wed, 14 Sep 2022 13:59:26 +0200 Subject: [PATCH 3/3] fix lint --- .../forms/hook_form_lib/hooks/use_field.test.tsx | 16 ++-------------- 1 file changed, 2 insertions(+), 14 deletions(-) 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 index 98050a81ade9a..71aafa6376884 100644 --- 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 @@ -150,18 +150,6 @@ describe('useField() hook', () => { }); describe('fieldsToValidateOnChange', () => { - let fieldHook1: FieldHook; - let fieldHook2: FieldHook; - const TestField1 = ({ field }: { field: FieldHook }) => { - fieldHook1 = field; - return null; - }; - - const TestField2 = ({ field }: { field: FieldHook }) => { - fieldHook2 = field; - return null; - }; - const getTestForm = (configField1?: FieldConfig, configField2?: FieldConfig) => ({ showField1, showField2 }: { showField1: boolean; showField2: boolean }) => { @@ -169,8 +157,8 @@ describe('useField() hook', () => { return (
- {showField1 && } - {showField2 && } + {showField1 && null} config={configField1} />} + {showField2 && null} config={configField2} />} ); };