From a719654f2745fd6f13668358715cc8dad1425885 Mon Sep 17 00:00:00 2001 From: Elena Stoeva Date: Wed, 16 Oct 2024 11:21:10 +0100 Subject: [PATCH 1/4] [Index Management] Fix unhandled error in ds data retention modal --- .../edit_data_retention_modal.tsx | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/x-pack/plugins/index_management/public/application/sections/home/data_stream_list/edit_data_retention_modal/edit_data_retention_modal.tsx b/x-pack/plugins/index_management/public/application/sections/home/data_stream_list/edit_data_retention_modal/edit_data_retention_modal.tsx index ca3612bdab168..becfad348fac8 100644 --- a/x-pack/plugins/index_management/public/application/sections/home/data_stream_list/edit_data_retention_modal/edit_data_retention_modal.tsx +++ b/x-pack/plugins/index_management/public/application/sections/home/data_stream_list/edit_data_retention_modal/edit_data_retention_modal.tsx @@ -103,8 +103,12 @@ const configurationFormSchema: FormSchema = { // If project level data retention is enabled, we need to enforce the global max retention const { globalMaxRetention, enableProjectLevelRetentionChecks } = customData.value as any; if (enableProjectLevelRetentionChecks) { - const currentValue = `${value}${formData.timeUnit}`; - if (globalMaxRetention && isRetentionBiggerThan(currentValue, globalMaxRetention)) { + if ( + value && + formData.timeUnit && + globalMaxRetention && + isRetentionBiggerThan(`${value}${formData.timeUnit}`, globalMaxRetention) + ) { return { message: i18n.translate( 'xpack.idxMgmt.dataStreamsDetailsPanel.editDataRetentionModal.dataRetentionFieldMaxError', @@ -258,11 +262,11 @@ export const EditDataRetentionModal: React.FunctionComponent = ({ const formHasErrors = form.getErrors().length > 0; const disableSubmit = formHasErrors || !isDirty || form.isValid === false; - // Whenever the formData changes, we need to re-validate the dataRetention field - // as it depends on the timeUnit field. + // Whenever the dataRetention or timeUnit field changes, we need to re-validate + // the dataRetention field useEffect(() => { form.validateFields(['dataRetention']); - }, [formData, form]); + }, [formData.dataRetention, formData.timeUnit, form]); const onSubmitForm = async () => { const { isValid, data } = await form.submit(); From 893cca24aa8752e086d44121856d3b4d9d585aa6 Mon Sep 17 00:00:00 2001 From: Elena Stoeva Date: Mon, 21 Oct 2024 23:44:06 +0100 Subject: [PATCH 2/4] Refactor validators and add tests --- .../edit_data_retention_modal.tsx | 96 ++++++------------- .../validations.test.ts | 53 ++++++++++ .../edit_data_retention_modal/validations.ts | 61 ++++++++++++ 3 files changed, 145 insertions(+), 65 deletions(-) create mode 100644 x-pack/plugins/index_management/public/application/sections/home/data_stream_list/edit_data_retention_modal/validations.test.ts create mode 100644 x-pack/plugins/index_management/public/application/sections/home/data_stream_list/edit_data_retention_modal/validations.ts diff --git a/x-pack/plugins/index_management/public/application/sections/home/data_stream_list/edit_data_retention_modal/edit_data_retention_modal.tsx b/x-pack/plugins/index_management/public/application/sections/home/data_stream_list/edit_data_retention_modal/edit_data_retention_modal.tsx index becfad348fac8..12b7789f39051 100644 --- a/x-pack/plugins/index_management/public/application/sections/home/data_stream_list/edit_data_retention_modal/edit_data_retention_modal.tsx +++ b/x-pack/plugins/index_management/public/application/sections/home/data_stream_list/edit_data_retention_modal/edit_data_retention_modal.tsx @@ -23,6 +23,7 @@ import { has } from 'lodash'; import { ScopedHistory } from '@kbn/core/public'; import { i18n } from '@kbn/i18n'; import { FormattedMessage } from '@kbn/i18n-react'; +import { isBiggerThanGlobalMaxRetention } from './validations'; import { useForm, useFormData, @@ -34,6 +35,7 @@ import { UseField, ToggleField, NumericField, + fieldValidators, } from '../../../../../shared_imports'; import { reactRouterNavigate } from '../../../../../shared_imports'; @@ -53,35 +55,6 @@ interface Props { onClose: (data?: { hasUpdatedDataRetention: boolean }) => void; } -const convertToMinutes = (value: string) => { - const { size, unit } = splitSizeAndUnits(value); - const sizeNum = parseInt(size, 10); - - switch (unit) { - case 'd': - // days to minutes - return sizeNum * 24 * 60; - case 'h': - // hours to minutes - return sizeNum * 60; - case 'm': - // minutes to minutes - return sizeNum; - case 's': - // seconds to minutes - return sizeNum / 60; - default: - throw new Error(`Unknown unit: ${unit}`); - } -}; - -const isRetentionBiggerThan = (valueA: string, valueB: string) => { - const minutesA = convertToMinutes(valueA); - const minutesB = convertToMinutes(valueB); - - return minutesA > minutesB; -}; - const configurationFormSchema: FormSchema = { dataRetention: { type: FIELD_TYPES.TEXT, @@ -95,53 +68,46 @@ const configurationFormSchema: FormSchema = { validations: [ { validator: ({ value, formData, customData }) => { - // If infiniteRetentionPeriod is set, we dont need to validate the data retention field - if (formData.infiniteRetentionPeriod) { - return undefined; - } - - // If project level data retention is enabled, we need to enforce the global max retention - const { globalMaxRetention, enableProjectLevelRetentionChecks } = customData.value as any; - if (enableProjectLevelRetentionChecks) { - if ( - value && - formData.timeUnit && - globalMaxRetention && - isRetentionBiggerThan(`${value}${formData.timeUnit}`, globalMaxRetention) - ) { - return { - message: i18n.translate( - 'xpack.idxMgmt.dataStreamsDetailsPanel.editDataRetentionModal.dataRetentionFieldMaxError', - { - defaultMessage: - 'Maximum data retention period on this project is {maxRetention} days.', - // Remove the unit from the globalMaxRetention value - values: { maxRetention: globalMaxRetention.slice(0, -1) }, - } - ), - }; + // We only need to validate the data retention field if infiniteRetentionPeriod is set to false + if (!formData.infiniteRetentionPeriod) { + // If project level data retention is enabled, we need to enforce the global max retention + const { globalMaxRetention, enableProjectLevelRetentionChecks } = + customData.value as any; + if (enableProjectLevelRetentionChecks) { + return isBiggerThanGlobalMaxRetention(value, formData.timeUnit, globalMaxRetention); } } - - if (!value) { - return { - message: i18n.translate( + }, + }, + { + validator: (args) => { + // We only need to validate the data retention field if infiniteRetentionPeriod is set to false + if (!args.formData.infiniteRetentionPeriod) { + return fieldValidators.emptyField( + i18n.translate( 'xpack.idxMgmt.dataStreamsDetailsPanel.editDataRetentionModal.dataRetentionFieldRequiredError', { defaultMessage: 'A data retention value is required.', } - ), - }; + ) + )(args); } - if (value <= 0) { - return { + }, + }, + { + validator: (args) => { + // We only need to validate the data retention field if infiniteRetentionPeriod is set to false + if (!args.formData.infiniteRetentionPeriod) { + return fieldValidators.numberGreaterThanField({ + than: 0, + allowEquality: false, message: i18n.translate( 'xpack.idxMgmt.dataStreamsDetailsPanel.editDataRetentionModal.dataRetentionFieldNonNegativeError', { defaultMessage: `A positive value is required.`, } ), - }; + })(args); } }, }, @@ -262,11 +228,11 @@ export const EditDataRetentionModal: React.FunctionComponent = ({ const formHasErrors = form.getErrors().length > 0; const disableSubmit = formHasErrors || !isDirty || form.isValid === false; - // Whenever the dataRetention or timeUnit field changes, we need to re-validate + // Whenever the timeUnit field changes, we need to re-validate // the dataRetention field useEffect(() => { form.validateFields(['dataRetention']); - }, [formData.dataRetention, formData.timeUnit, form]); + }, [formData.timeUnit, form]); const onSubmitForm = async () => { const { isValid, data } = await form.submit(); diff --git a/x-pack/plugins/index_management/public/application/sections/home/data_stream_list/edit_data_retention_modal/validations.test.ts b/x-pack/plugins/index_management/public/application/sections/home/data_stream_list/edit_data_retention_modal/validations.test.ts new file mode 100644 index 0000000000000..f8ea18a10479d --- /dev/null +++ b/x-pack/plugins/index_management/public/application/sections/home/data_stream_list/edit_data_retention_modal/validations.test.ts @@ -0,0 +1,53 @@ +/* + * 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; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import { isBiggerThanGlobalMaxRetention } from './validations'; + +describe('isBiggerThanGlobalMaxRetention', () => { + it('should return undefined if any argument is missing', () => { + expect(isBiggerThanGlobalMaxRetention(undefined, 'd', '30d')).toBeUndefined(); + expect(isBiggerThanGlobalMaxRetention(10, undefined, '30d')).toBeUndefined(); + expect(isBiggerThanGlobalMaxRetention(10, 'd', undefined)).toBeUndefined(); + }); + + it('should return an error message if retention is bigger than global max retention (in days)', () => { + const result = isBiggerThanGlobalMaxRetention(40, 'd', '30d'); + expect(result).toEqual({ + message: 'Maximum data retention period on this project is 30 days.', + }); + }); + + it('should return undefined if retention is smaller than or equal to global max retention (in days)', () => { + expect(isBiggerThanGlobalMaxRetention(30, 'd', '30d')).toBeUndefined(); + expect(isBiggerThanGlobalMaxRetention(25, 'd', '30d')).toBeUndefined(); + }); + + it('should correctly compare retention in different time units against days', () => { + expect(isBiggerThanGlobalMaxRetention(24, 'h', '1d')).toBeUndefined(); + expect(isBiggerThanGlobalMaxRetention(23, 'h', '1d')).toBeUndefined(); + // 30 days = 720 hours + expect(isBiggerThanGlobalMaxRetention(800, 'h', '30d')).toEqual({ + message: 'Maximum data retention period on this project is 30 days.', + }); + + // 1 day = 1440 minutes + expect(isBiggerThanGlobalMaxRetention(1440, 'm', '1d')).toBeUndefined(); + expect(isBiggerThanGlobalMaxRetention(3000, 'm', '2d')).toEqual({ + message: 'Maximum data retention period on this project is 2 days.', + }); + + // 1 day = 86400 seconds + expect(isBiggerThanGlobalMaxRetention(1000, 's', '1d')).toBeUndefined(); + expect(isBiggerThanGlobalMaxRetention(87000, 's', '1d')).toEqual({ + message: 'Maximum data retention period on this project is 1 days.', + }); + }); + + it('should throw an error for unknown time units', () => { + expect(() => isBiggerThanGlobalMaxRetention(10, 'x', '30d')).toThrow('Unknown unit: x'); + }); +}); diff --git a/x-pack/plugins/index_management/public/application/sections/home/data_stream_list/edit_data_retention_modal/validations.ts b/x-pack/plugins/index_management/public/application/sections/home/data_stream_list/edit_data_retention_modal/validations.ts new file mode 100644 index 0000000000000..afaf236edf49b --- /dev/null +++ b/x-pack/plugins/index_management/public/application/sections/home/data_stream_list/edit_data_retention_modal/validations.ts @@ -0,0 +1,61 @@ +/* + * 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; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import { i18n } from '@kbn/i18n'; +import { splitSizeAndUnits } from '../../../../../../common'; + +const convertToMinutes = (value: string) => { + const { size, unit } = splitSizeAndUnits(value); + const sizeNum = parseInt(size, 10); + + switch (unit) { + case 'd': + // days to minutes + return sizeNum * 24 * 60; + case 'h': + // hours to minutes + return sizeNum * 60; + case 'm': + // minutes to minutes + return sizeNum; + case 's': + // seconds to minutes (round up if any remainder) + return Math.ceil(sizeNum / 60); + default: + throw new Error(`Unknown unit: ${unit}`); + } +}; + +const isRetentionBiggerThan = (valueA: string, valueB: string) => { + const minutesA = convertToMinutes(valueA); + const minutesB = convertToMinutes(valueB); + + return minutesA > minutesB; +}; + +export const isBiggerThanGlobalMaxRetention = ( + retentionValue, + retentionTimeUnit, + globalMaxRetention +) => { + if (!retentionValue || !retentionTimeUnit || !globalMaxRetention) { + return undefined; + } + + return isRetentionBiggerThan(`${retentionValue}${retentionTimeUnit}`, globalMaxRetention) + ? { + message: i18n.translate( + 'xpack.idxMgmt.dataStreamsDetailsPanel.editDataRetentionModal.dataRetentionFieldMaxError', + { + defaultMessage: 'Maximum data retention period on this project is {maxRetention} days.', + // Remove the unit from the globalMaxRetention value + values: { maxRetention: globalMaxRetention.slice(0, -1) }, + } + ), + } + : undefined; +}; From b0718ce3bc6eca87871bfb85298b6e0585690e9b Mon Sep 17 00:00:00 2001 From: Elena Stoeva Date: Tue, 22 Oct 2024 11:46:58 +0100 Subject: [PATCH 3/4] Add validation for integer --- .../edit_data_retention_modal.tsx | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/x-pack/plugins/index_management/public/application/sections/home/data_stream_list/edit_data_retention_modal/edit_data_retention_modal.tsx b/x-pack/plugins/index_management/public/application/sections/home/data_stream_list/edit_data_retention_modal/edit_data_retention_modal.tsx index 12b7789f39051..f747abca19f05 100644 --- a/x-pack/plugins/index_management/public/application/sections/home/data_stream_list/edit_data_retention_modal/edit_data_retention_modal.tsx +++ b/x-pack/plugins/index_management/public/application/sections/home/data_stream_list/edit_data_retention_modal/edit_data_retention_modal.tsx @@ -66,6 +66,21 @@ const configurationFormSchema: FormSchema = { ), formatters: [fieldFormatters.toInt], validations: [ + { + validator: ({ value }) => { + // TODO: Replace with validator added in https://github.com/elastic/kibana/pull/196527/ + if (!Number.isInteger(Number(value ?? ''))) { + return { + message: i18n.translate( + 'xpack.idxMgmt.dataStreamsDetailsPanel.editDataRetentionModal.dataRetentionFieldIntegerError', + { + defaultMessage: 'Only integers are allowed.', + } + ), + }; + } + }, + }, { validator: ({ value, formData, customData }) => { // We only need to validate the data retention field if infiniteRetentionPeriod is set to false From 82a6197bea16d7f511f923c95c770d6656ba9dd2 Mon Sep 17 00:00:00 2001 From: Elena Stoeva Date: Tue, 22 Oct 2024 12:53:20 +0100 Subject: [PATCH 4/4] Fix type errors --- .../edit_data_retention_modal/validations.test.ts | 6 +++--- .../edit_data_retention_modal/validations.ts | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/x-pack/plugins/index_management/public/application/sections/home/data_stream_list/edit_data_retention_modal/validations.test.ts b/x-pack/plugins/index_management/public/application/sections/home/data_stream_list/edit_data_retention_modal/validations.test.ts index f8ea18a10479d..0768d0990cdc0 100644 --- a/x-pack/plugins/index_management/public/application/sections/home/data_stream_list/edit_data_retention_modal/validations.test.ts +++ b/x-pack/plugins/index_management/public/application/sections/home/data_stream_list/edit_data_retention_modal/validations.test.ts @@ -9,9 +9,9 @@ import { isBiggerThanGlobalMaxRetention } from './validations'; describe('isBiggerThanGlobalMaxRetention', () => { it('should return undefined if any argument is missing', () => { - expect(isBiggerThanGlobalMaxRetention(undefined, 'd', '30d')).toBeUndefined(); - expect(isBiggerThanGlobalMaxRetention(10, undefined, '30d')).toBeUndefined(); - expect(isBiggerThanGlobalMaxRetention(10, 'd', undefined)).toBeUndefined(); + expect(isBiggerThanGlobalMaxRetention('', 'd', '30d')).toBeUndefined(); + expect(isBiggerThanGlobalMaxRetention(10, '', '30d')).toBeUndefined(); + expect(isBiggerThanGlobalMaxRetention(10, 'd', '')).toBeUndefined(); }); it('should return an error message if retention is bigger than global max retention (in days)', () => { diff --git a/x-pack/plugins/index_management/public/application/sections/home/data_stream_list/edit_data_retention_modal/validations.ts b/x-pack/plugins/index_management/public/application/sections/home/data_stream_list/edit_data_retention_modal/validations.ts index afaf236edf49b..831ac2f4c26b9 100644 --- a/x-pack/plugins/index_management/public/application/sections/home/data_stream_list/edit_data_retention_modal/validations.ts +++ b/x-pack/plugins/index_management/public/application/sections/home/data_stream_list/edit_data_retention_modal/validations.ts @@ -38,9 +38,9 @@ const isRetentionBiggerThan = (valueA: string, valueB: string) => { }; export const isBiggerThanGlobalMaxRetention = ( - retentionValue, - retentionTimeUnit, - globalMaxRetention + retentionValue: string | number, + retentionTimeUnit: string, + globalMaxRetention: string ) => { if (!retentionValue || !retentionTimeUnit || !globalMaxRetention) { return undefined;