From c4900f7daa050ce4a27888d5176be82ccb79f468 Mon Sep 17 00:00:00 2001 From: Bill McConaghy Date: Wed, 12 Dec 2018 12:25:45 -0500 Subject: [PATCH] fixing some validation issues with edit policy (#27045) * fixing validation issues with editing existing policy * preventing less than 1 values for max size and max age and adding tests --- .../__jest__/components/edit_policy.test.js | 44 +++++++++++++++++++ .../public/store/selectors/lifecycle.js | 17 +++++-- .../public/store/selectors/policies.js | 1 - 3 files changed, 58 insertions(+), 4 deletions(-) diff --git a/x-pack/plugins/index_lifecycle_management/__jest__/components/edit_policy.test.js b/x-pack/plugins/index_lifecycle_management/__jest__/components/edit_policy.test.js index c39940940813..5dee81591a0f 100644 --- a/x-pack/plugins/index_lifecycle_management/__jest__/components/edit_policy.test.js +++ b/x-pack/plugins/index_lifecycle_management/__jest__/components/edit_policy.test.js @@ -171,6 +171,50 @@ describe('edit policy', () => { expectedErrorMessages(rendered, [policyNameStartsWithUnderscoreErrorMessage]); }); }); + describe('hot phase', () => { + test('should show errors when trying to save with no max size and no max age', () => { + const rendered = mountWithIntl(component); + setPolicyName(rendered, 'mypolicy'); + save(rendered); + expectedErrorMessages(rendered, [maximumSizeRequiredMessage, maximumAgeRequiredMessage]); + }); + test('should show number above 0 required error when trying to save with -1 for max size', () => { + const rendered = mountWithIntl(component); + setPolicyName(rendered, 'mypolicy'); + const maxSizeInput = rendered.find(`input#hot-selectedMaxSizeStored`); + maxSizeInput.simulate('change', { target: { value: -1 } }); + rendered.update(); + save(rendered); + expectedErrorMessages(rendered, [positiveNumbersAboveZeroErrorMessage]); + }); + test('should show number above 0 required error when trying to save with 0 for max size', () => { + const rendered = mountWithIntl(component); + setPolicyName(rendered, 'mypolicy'); + const maxSizeInput = rendered.find(`input#hot-selectedMaxSizeStored`); + maxSizeInput.simulate('change', { target: { value: 0 } }); + rendered.update(); + save(rendered); + expectedErrorMessages(rendered, [positiveNumbersAboveZeroErrorMessage]); + }); + test('should show number above 0 required error when trying to save with -1 for max age', () => { + const rendered = mountWithIntl(component); + setPolicyName(rendered, 'mypolicy'); + const maxSizeInput = rendered.find(`input#hot-selectedMaxAge`); + maxSizeInput.simulate('change', { target: { value: -1 } }); + rendered.update(); + save(rendered); + expectedErrorMessages(rendered, [positiveNumbersAboveZeroErrorMessage]); + }); + test('should show number above 0 required error when trying to save with 0 for max age', () => { + const rendered = mountWithIntl(component); + setPolicyName(rendered, 'mypolicy'); + const maxSizeInput = rendered.find(`input#hot-selectedMaxAge`); + maxSizeInput.simulate('change', { target: { value: 0 } }); + rendered.update(); + save(rendered); + expectedErrorMessages(rendered, [positiveNumbersAboveZeroErrorMessage]); + }); + }); describe('warm phase', () => { test('should show number required error when trying to save empty warm phase', () => { const rendered = mountWithIntl(component); diff --git a/x-pack/plugins/index_lifecycle_management/public/store/selectors/lifecycle.js b/x-pack/plugins/index_lifecycle_management/public/store/selectors/lifecycle.js index 5265590c6d92..7a71d46ad9d2 100644 --- a/x-pack/plugins/index_lifecycle_management/public/store/selectors/lifecycle.js +++ b/x-pack/plugins/index_lifecycle_management/public/store/selectors/lifecycle.js @@ -60,8 +60,9 @@ export const validatePhase = (type, phase, errors) => { for (const numberedAttribute of PHASE_ATTRIBUTES_THAT_ARE_NUMBERS_VALIDATE) { if (phase.hasOwnProperty(numberedAttribute)) { - // If WARM_PHASE_ON_ROLLOVER there is no need to validate this - if (numberedAttribute === PHASE_ROLLOVER_MINIMUM_AGE && phase[WARM_PHASE_ON_ROLLOVER]) { + // If WARM_PHASE_ON_ROLLOVER or PHASE_HOT there is no need to validate this + if (numberedAttribute === PHASE_ROLLOVER_MINIMUM_AGE + && (phase[WARM_PHASE_ON_ROLLOVER] || type === PHASE_HOT)) { continue; } // If shrink is disabled, there is no need to validate this @@ -100,6 +101,16 @@ export const validatePhase = (type, phase, errors) => { maximumSizeRequiredMessage ]; } + if (isNumber(phase[PHASE_ROLLOVER_MAX_AGE]) && phase[PHASE_ROLLOVER_MAX_AGE] < 1) { + phaseErrors[PHASE_ROLLOVER_MAX_AGE] = [ + positiveNumbersAboveZeroErrorMessage + ]; + } + if (isNumber(phase[PHASE_ROLLOVER_MAX_SIZE_STORED]) && phase[PHASE_ROLLOVER_MAX_SIZE_STORED] < 1) { + phaseErrors[PHASE_ROLLOVER_MAX_SIZE_STORED] = [ + positiveNumbersAboveZeroErrorMessage + ]; + } } if (phase[PHASE_SHRINK_ENABLED]) { if (!isNumber(phase[PHASE_PRIMARY_SHARD_COUNT])) { @@ -172,7 +183,7 @@ export const validateLifecycle = state => { if (getSaveAsNewPolicy(state) && getSelectedOriginalPolicyName(state) === getSelectedPolicyName(state)) { errors[STRUCTURE_POLICY_NAME].push(policyNameMustBeDifferentErrorMessage); - } else { + } else if (getSelectedOriginalPolicyName(state) !== getSelectedPolicyName(state)) { const policyNames = getPolicies(state).map(policy => policy.name); if (policyNames.includes(getSelectedPolicyName(state))) { errors[STRUCTURE_POLICY_NAME].push(policyNameAlreadyUsedErrorMessage); diff --git a/x-pack/plugins/index_lifecycle_management/public/store/selectors/policies.js b/x-pack/plugins/index_lifecycle_management/public/store/selectors/policies.js index 169230c9a260..29f792a01617 100644 --- a/x-pack/plugins/index_lifecycle_management/public/store/selectors/policies.js +++ b/x-pack/plugins/index_lifecycle_management/public/store/selectors/policies.js @@ -268,6 +268,5 @@ export const phaseToES = (state, phase) => { number_of_shards: phase[PHASE_PRIMARY_SHARD_COUNT] }; } - console.log("PHASE", phase); return esPhase; };