From 26c3a991d470a1cd73a8c47ebafcbad20ea53120 Mon Sep 17 00:00:00 2001 From: Jean-Louis Leysens Date: Tue, 8 Dec 2020 16:20:10 +0100 Subject: [PATCH 1/4] pivot to different rollover validation mechanism --- .../components/phases/hot_phase/hot_phase.tsx | 2 +- .../sections/edit_policy/form/schema.ts | 6 +++ .../sections/edit_policy/form/validations.ts | 40 +++++++++---------- 3 files changed, 25 insertions(+), 23 deletions(-) diff --git a/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/components/phases/hot_phase/hot_phase.tsx b/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/components/phases/hot_phase/hot_phase.tsx index e86bbd9e747bc..74191a8cc861a 100644 --- a/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/components/phases/hot_phase/hot_phase.tsx +++ b/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/components/phases/hot_phase/hot_phase.tsx @@ -143,7 +143,7 @@ export const HotPhase: FunctionComponent = () => { {(field) => { const showErrorCallout = field.errors.some( - (e) => e.validationType === ROLLOVER_EMPTY_VALIDATION + (e) => e.code === ROLLOVER_EMPTY_VALIDATION ); if (showErrorCallout !== showEmptyRolloverFieldsError) { setShowEmptyRolloverFieldsError(showErrorCallout); diff --git a/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/form/schema.ts b/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/form/schema.ts index cedf1cdb4d9fe..d3e9be166cd29 100644 --- a/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/form/schema.ts +++ b/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/form/schema.ts @@ -8,6 +8,9 @@ import { i18n } from '@kbn/i18n'; import { FormSchema, fieldValidators } from '../../../../shared_imports'; import { defaultSetPriority, defaultPhaseIndexPriority } from '../../../constants'; +import { ROLLOVER_FORM_PATHS } from '../constants'; + +const rolloverFormPaths = Object.values(ROLLOVER_FORM_PATHS); import { FormInternal } from '../types'; @@ -127,6 +130,7 @@ export const schema: FormSchema = { validator: ifExistsNumberGreaterThanZero, }, ], + fieldsToValidateOnChange: rolloverFormPaths, }, max_docs: { label: i18n.translate('xpack.indexLifecycleMgmt.hotPhase.maximumDocumentsLabel', { @@ -141,6 +145,7 @@ export const schema: FormSchema = { }, ], serializer: serializers.stringToNumber, + fieldsToValidateOnChange: rolloverFormPaths, }, max_size: { label: i18n.translate('xpack.indexLifecycleMgmt.hotPhase.maximumIndexSizeLabel', { @@ -154,6 +159,7 @@ export const schema: FormSchema = { validator: ifExistsNumberGreaterThanZero, }, ], + fieldsToValidateOnChange: rolloverFormPaths, }, }, forcemerge: { diff --git a/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/form/validations.ts b/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/form/validations.ts index f2e26a552efc9..8641b36e3bf69 100644 --- a/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/form/validations.ts +++ b/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/form/validations.ts @@ -56,33 +56,29 @@ export const ROLLOVER_EMPTY_VALIDATION = 'ROLLOVER_EMPTY_VALIDATION'; * This validator checks that and updates form values by setting errors states imperatively to * indicate this error state. */ -export const rolloverThresholdsValidator: ValidationFunc = ({ form }) => { +export const rolloverThresholdsValidator: ValidationFunc = ({ form, path }) => { const fields = form.getFields(); if ( !( - fields[ROLLOVER_FORM_PATHS.maxAge].value || - fields[ROLLOVER_FORM_PATHS.maxDocs].value || - fields[ROLLOVER_FORM_PATHS.maxSize].value + fields[ROLLOVER_FORM_PATHS.maxAge]?.value || + fields[ROLLOVER_FORM_PATHS.maxDocs]?.value || + fields[ROLLOVER_FORM_PATHS.maxSize]?.value ) ) { - fields[ROLLOVER_FORM_PATHS.maxAge].setErrors([ - { - validationType: ROLLOVER_EMPTY_VALIDATION, - message: i18nTexts.editPolicy.errors.maximumAgeRequiredMessage, - }, - ]); - fields[ROLLOVER_FORM_PATHS.maxDocs].setErrors([ - { - validationType: ROLLOVER_EMPTY_VALIDATION, - message: i18nTexts.editPolicy.errors.maximumDocumentsRequiredMessage, - }, - ]); - fields[ROLLOVER_FORM_PATHS.maxSize].setErrors([ - { - validationType: ROLLOVER_EMPTY_VALIDATION, - message: i18nTexts.editPolicy.errors.maximumSizeRequiredMessage, - }, - ]); + return path === ROLLOVER_FORM_PATHS.maxAge + ? { + code: ROLLOVER_EMPTY_VALIDATION, + message: i18nTexts.editPolicy.errors.maximumAgeRequiredMessage, + } + : path === ROLLOVER_FORM_PATHS.maxDocs + ? { + code: ROLLOVER_EMPTY_VALIDATION, + message: i18nTexts.editPolicy.errors.maximumDocumentsRequiredMessage, + } + : { + code: ROLLOVER_EMPTY_VALIDATION, + message: i18nTexts.editPolicy.errors.maximumSizeRequiredMessage, + }; } else { fields[ROLLOVER_FORM_PATHS.maxAge].clearErrors(ROLLOVER_EMPTY_VALIDATION); fields[ROLLOVER_FORM_PATHS.maxDocs].clearErrors(ROLLOVER_EMPTY_VALIDATION); From a0c382ac258985e6ee3e6b02bfdc000276e204de Mon Sep 17 00:00:00 2001 From: Jean-Louis Leysens Date: Tue, 8 Dec 2020 16:45:12 +0100 Subject: [PATCH 2/4] implement stakeholder feedback to show forcemerge in hot --- .../edit_policy/components/phases/hot_phase/hot_phase.tsx | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/components/phases/hot_phase/hot_phase.tsx b/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/components/phases/hot_phase/hot_phase.tsx index 74191a8cc861a..5ce4fae596e8e 100644 --- a/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/components/phases/hot_phase/hot_phase.tsx +++ b/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/components/phases/hot_phase/hot_phase.tsx @@ -24,7 +24,7 @@ import { useFormData, UseField, SelectField, NumericField } from '../../../../.. import { i18nTexts } from '../../../i18n_texts'; -import { ROLLOVER_EMPTY_VALIDATION, useConfigurationIssues } from '../../../form'; +import { ROLLOVER_EMPTY_VALIDATION } from '../../../form'; import { useEditPolicyContext } from '../../../edit_policy_context'; @@ -51,8 +51,6 @@ export const HotPhase: FunctionComponent = () => { const isRolloverEnabled = get(formData, useRolloverPath); const [showEmptyRolloverFieldsError, setShowEmptyRolloverFieldsError] = useState(false); - const { isUsingSearchableSnapshotInHotPhase } = useConfigurationIssues(); - return ( <> { {isRolloverEnabled && ( <> + {} {license.canUseSearchableSnapshot() && } - {!isUsingSearchableSnapshotInHotPhase && } )} From 9293e1efa6c3ab7694eb6ec42bbb5dbc3f7369bf Mon Sep 17 00:00:00 2001 From: Jean-Louis Leysens Date: Wed, 9 Dec 2020 10:24:37 +0100 Subject: [PATCH 3/4] replace ternary with if..else statements --- .../sections/edit_policy/form/validations.ts | 30 ++++++++++--------- 1 file changed, 16 insertions(+), 14 deletions(-) diff --git a/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/form/validations.ts b/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/form/validations.ts index 8641b36e3bf69..a5d7d68d21915 100644 --- a/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/form/validations.ts +++ b/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/form/validations.ts @@ -65,20 +65,22 @@ export const rolloverThresholdsValidator: ValidationFunc = ({ form, path }) => { fields[ROLLOVER_FORM_PATHS.maxSize]?.value ) ) { - return path === ROLLOVER_FORM_PATHS.maxAge - ? { - code: ROLLOVER_EMPTY_VALIDATION, - message: i18nTexts.editPolicy.errors.maximumAgeRequiredMessage, - } - : path === ROLLOVER_FORM_PATHS.maxDocs - ? { - code: ROLLOVER_EMPTY_VALIDATION, - message: i18nTexts.editPolicy.errors.maximumDocumentsRequiredMessage, - } - : { - code: ROLLOVER_EMPTY_VALIDATION, - message: i18nTexts.editPolicy.errors.maximumSizeRequiredMessage, - }; + if (path === ROLLOVER_FORM_PATHS.maxAge) { + return { + code: ROLLOVER_EMPTY_VALIDATION, + message: i18nTexts.editPolicy.errors.maximumAgeRequiredMessage, + }; + } else if (path === ROLLOVER_FORM_PATHS.maxDocs) { + return { + code: ROLLOVER_EMPTY_VALIDATION, + message: i18nTexts.editPolicy.errors.maximumDocumentsRequiredMessage, + }; + } else { + return { + code: ROLLOVER_EMPTY_VALIDATION, + message: i18nTexts.editPolicy.errors.maximumSizeRequiredMessage, + }; + } } else { fields[ROLLOVER_FORM_PATHS.maxAge].clearErrors(ROLLOVER_EMPTY_VALIDATION); fields[ROLLOVER_FORM_PATHS.maxDocs].clearErrors(ROLLOVER_EMPTY_VALIDATION); From 1f02f172bd1b7df45f8e60992c3fe38f143fcf97 Mon Sep 17 00:00:00 2001 From: Jean-Louis Leysens Date: Wed, 9 Dec 2020 10:30:17 +0100 Subject: [PATCH 4/4] make rollover validation test more comprehensive --- .../__jest__/components/edit_policy.test.tsx | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/x-pack/plugins/index_lifecycle_management/__jest__/components/edit_policy.test.tsx b/x-pack/plugins/index_lifecycle_management/__jest__/components/edit_policy.test.tsx index 65952e81ae0ff..32964ab2ce84d 100644 --- a/x-pack/plugins/index_lifecycle_management/__jest__/components/edit_policy.test.tsx +++ b/x-pack/plugins/index_lifecycle_management/__jest__/components/edit_policy.test.tsx @@ -324,7 +324,7 @@ describe('edit policy', () => { }); }); describe('hot phase', () => { - test('should show errors when trying to save with no max size and no max age', async () => { + test('should show errors when trying to save with no max size, no max age and no max docs', async () => { const rendered = mountWithIntl(component); expect(findTestSubject(rendered, 'rolloverSettingsRequired').exists()).toBeFalsy(); await setPolicyName(rendered, 'mypolicy'); @@ -338,6 +338,11 @@ describe('edit policy', () => { maxAgeInput.simulate('change', { target: { value: '' } }); }); waitForFormLibValidation(rendered); + const maxDocsInput = findTestSubject(rendered, 'hot-selectedMaxDocuments'); + await act(async () => { + maxDocsInput.simulate('change', { target: { value: '' } }); + }); + waitForFormLibValidation(rendered); await save(rendered); expect(findTestSubject(rendered, 'rolloverSettingsRequired').exists()).toBeTruthy(); });