From 1c1ac89aff9610421ae65c38359d07e0e2a0e013 Mon Sep 17 00:00:00 2001 From: Sonia Sanz Vivas Date: Thu, 24 Oct 2024 18:09:38 +0200 Subject: [PATCH] Create a common Int Validator and use it in ingest_pipelines and Index_lifecycle_management (#196527) Closes [#110417 ](https://github.com/elastic/kibana/issues/110417) ## Summary In the Ingest Node Pipelines section, when the users created a new pipeline selecting de Community ID processor the users could set a non-integer number in this field. Then, they received a server side error when tried to create a pipeline. For fixing this, a validation must be added in the client. We didn't have a reusable validation for this case, but we did have a custom validation for integer values in the Index lifecycle management plugin. We also had the necessary translation in that plugin. So I went forward with: * I created a new integer validator in the `es_ui_shared` package as it is a fairly common validation and we could take advantage of it in the future. Also added a couple of unit test there for this validator. * I reused in the `ingest_pipelines` plugin the strings that already existed in `index_lifecycle_management`. * I added the new validation in the Community ID form in the `ingest_pipelines` plugin. Also added some test verifying that the processor doesn't create when the seeds validation fails. * Changed the method in the `index_lifecycle_management` validator so now it uses the reusable one. Now the Ingest pipeline forms shows the validation when the number is not an integer: ![Screenshot 2024-10-16 at 12 16 47](https://github.com/user-attachments/assets/1db9ad22-b144-44a5-9012-d3ebd5a19b6f) And the `index_lifecycle_management` still shows the validations as expected: Screenshot 2024-10-16 at 11 49 53 (cherry picked from commit 5a42e5861dc53911e10a3140d8cb5366fea98ff7) --- .../forms/helpers/field_validators/index.ts | 1 + .../field_validators/is_integer.test.ts | 47 ++++++++++++++++++ .../helpers/field_validators/is_integer.ts | 27 +++++++++++ .../forms/helpers/field_validators/types.ts | 3 +- .../sections/edit_policy/form/schema.ts | 13 +++-- .../sections/edit_policy/form/validations.ts | 6 --- .../edit_data_retention_modal.tsx | 21 ++++---- .../__jest__/processors/community_id.test.tsx | 48 +++++++++++++++++++ .../processors/community_id.tsx | 10 +++- .../translations/translations/fr-FR.json | 1 + .../translations/translations/ja-JP.json | 1 + .../translations/translations/zh-CN.json | 1 + 12 files changed, 151 insertions(+), 28 deletions(-) create mode 100644 src/plugins/es_ui_shared/static/forms/helpers/field_validators/is_integer.test.ts create mode 100644 src/plugins/es_ui_shared/static/forms/helpers/field_validators/is_integer.ts diff --git a/src/plugins/es_ui_shared/static/forms/helpers/field_validators/index.ts b/src/plugins/es_ui_shared/static/forms/helpers/field_validators/index.ts index c3801edde7a06..32e4076d2dd9b 100644 --- a/src/plugins/es_ui_shared/static/forms/helpers/field_validators/index.ts +++ b/src/plugins/es_ui_shared/static/forms/helpers/field_validators/index.ts @@ -20,3 +20,4 @@ export * from './lowercase_string'; export * from './is_json'; export * from './number_greater_than'; export * from './number_smaller_than'; +export * from './is_integer'; diff --git a/src/plugins/es_ui_shared/static/forms/helpers/field_validators/is_integer.test.ts b/src/plugins/es_ui_shared/static/forms/helpers/field_validators/is_integer.test.ts new file mode 100644 index 0000000000000..1c01a9fe14ca9 --- /dev/null +++ b/src/plugins/es_ui_shared/static/forms/helpers/field_validators/is_integer.test.ts @@ -0,0 +1,47 @@ +/* + * 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", the "GNU Affero General Public License v3.0 only", 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", the "GNU Affero General Public + * License v3.0 only", or the "Server Side Public License, v 1". + */ + +import { ValidationFuncArg } from '../../hook_form_lib'; +import { isInteger } from './is_integer'; + +describe('isInteger', () => { + const message = 'test error message'; + const code = 'ERR_NOT_INT_NUMBER'; + + const validate = isInteger({ message }); + const validator = (value: unknown) => validate({ value } as ValidationFuncArg); + + test('should return undefined if value is integer number', () => { + expect(validator(5)).toBeUndefined(); + }); + + test('should return undefined if value string that can be parsed to integer', () => { + expect(validator('5')).toBeUndefined(); + }); + + test('should return Validation function if value is not integer number', () => { + expect(validator(5.3)).toMatchObject({ message, code }); + }); + + test('should return Validation function if value a string that can not be parsed to number but is not an integer', () => { + expect(validator('5.3')).toMatchObject({ message, code }); + }); + + test('should return Validation function if value a string that can not be parsed to number', () => { + expect(validator('test')).toMatchObject({ message, code }); + }); + + test('should return Validation function if value is boolean', () => { + expect(validator(false)).toMatchObject({ message, code }); + }); + + test('should return undefined if value is empty', () => { + expect(validator('')).toBeUndefined(); + }); +}); diff --git a/src/plugins/es_ui_shared/static/forms/helpers/field_validators/is_integer.ts b/src/plugins/es_ui_shared/static/forms/helpers/field_validators/is_integer.ts new file mode 100644 index 0000000000000..9e8c8cbfaef77 --- /dev/null +++ b/src/plugins/es_ui_shared/static/forms/helpers/field_validators/is_integer.ts @@ -0,0 +1,27 @@ +/* + * 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", the "GNU Affero General Public License v3.0 only", 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", the "GNU Affero General Public + * License v3.0 only", or the "Server Side Public License, v 1". + */ + +import { ValidationFunc } from '../../hook_form_lib'; +import { ERROR_CODE } from './types'; + +export const isInteger = + ({ message }: { message: string }) => + (...args: Parameters): ReturnType> => { + const [{ value }] = args; + + if ( + value === '' || + (typeof value === 'number' && Number.isInteger(value)) || + (typeof value === 'string' && Number.isInteger(Number(value))) + ) { + return undefined; + } + + return { message, code: 'ERR_NOT_INT_NUMBER' }; + }; diff --git a/src/plugins/es_ui_shared/static/forms/helpers/field_validators/types.ts b/src/plugins/es_ui_shared/static/forms/helpers/field_validators/types.ts index 7a41e09b2932a..9ad3f54896990 100644 --- a/src/plugins/es_ui_shared/static/forms/helpers/field_validators/types.ts +++ b/src/plugins/es_ui_shared/static/forms/helpers/field_validators/types.ts @@ -19,4 +19,5 @@ export type ERROR_CODE = | 'ERR_LOWERCASE_STRING' | 'ERR_JSON_FORMAT' | 'ERR_SMALLER_THAN_NUMBER' - | 'ERR_GREATER_THAN_NUMBER'; + | 'ERR_GREATER_THAN_NUMBER' + | 'ERR_NOT_INT_NUMBER'; 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 a4f5f92acc086..5b8c40e729424 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 @@ -15,7 +15,6 @@ import { i18nTexts } from '../i18n_texts'; import { ifExistsNumberGreaterThanZero, ifExistsNumberNonNegative, - integerValidator, minAgeGreaterThanPreviousPhase, rolloverThresholdsValidator, downsampleIntervalMultipleOfPreviousOne, @@ -23,7 +22,7 @@ import { const rolloverFormPaths = Object.values(ROLLOVER_FORM_PATHS); -const { emptyField, numberGreaterThanField } = fieldValidators; +const { emptyField, isInteger, numberGreaterThanField } = fieldValidators; const serializers = { stringToNumber: (v: string): any => (v != null ? parseInt(v, 10) : undefined), @@ -150,7 +149,7 @@ const getMinAgeField = (phase: PhaseWithTiming, defaultValue?: string) => ({ validator: ifExistsNumberNonNegative, }, { - validator: integerValidator, + validator: isInteger({ message: i18nTexts.editPolicy.errors.integerRequired }), }, { validator: minAgeGreaterThanPreviousPhase(phase), @@ -192,7 +191,7 @@ const getDownsampleSchema = (phase: PhaseWithDownsample): FormSchema['downsample validator: ifExistsNumberGreaterThanZero, }, { - validator: integerValidator, + validator: isInteger({ message: i18nTexts.editPolicy.errors.integerRequired }), }, { validator: downsampleIntervalMultipleOfPreviousOne(phase), @@ -381,7 +380,7 @@ export const getSchema = (isCloudEnabled: boolean): FormSchema => ({ validator: ifExistsNumberGreaterThanZero, }, { - validator: integerValidator, + validator: isInteger({ message: i18nTexts.editPolicy.errors.integerRequired }), }, ], fieldsToValidateOnChange: rolloverFormPaths, @@ -396,7 +395,7 @@ export const getSchema = (isCloudEnabled: boolean): FormSchema => ({ validator: ifExistsNumberGreaterThanZero, }, { - validator: integerValidator, + validator: isInteger({ message: i18nTexts.editPolicy.errors.integerRequired }), }, ], serializer: serializers.stringToNumber, @@ -424,7 +423,7 @@ export const getSchema = (isCloudEnabled: boolean): FormSchema => ({ validator: ifExistsNumberGreaterThanZero, }, { - validator: integerValidator, + validator: isInteger({ message: i18nTexts.editPolicy.errors.integerRequired }), }, ], serializer: serializers.stringToNumber, 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 5035071a1f2a1..3020c843b5516 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 @@ -101,12 +101,6 @@ export const rolloverThresholdsValidator: ValidationFunc = ({ form, path }) => { } }; -export const integerValidator: ValidationFunc = (arg) => { - if (!Number.isInteger(Number(arg.value ?? ''))) { - return { message: i18nTexts.editPolicy.errors.integerRequired }; - } -}; - export const createPolicyNameValidations = ({ policies, isClonedPolicy, 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 f747abca19f05..f5eee4671481a 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 @@ -67,19 +67,14 @@ 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: fieldValidators.isInteger({ + message: i18n.translate( + 'xpack.idxMgmt.dataStreamsDetailsPanel.editDataRetentionModal.dataRetentionFieldIntegerError', + { + defaultMessage: 'Only integers are allowed.', + } + ), + }), }, { validator: ({ value, formData, customData }) => { diff --git a/x-pack/plugins/ingest_pipelines/public/application/components/pipeline_editor/__jest__/processors/community_id.test.tsx b/x-pack/plugins/ingest_pipelines/public/application/components/pipeline_editor/__jest__/processors/community_id.test.tsx index b67b198eb0afa..72a8f8ec1a1ea 100644 --- a/x-pack/plugins/ingest_pipelines/public/application/components/pipeline_editor/__jest__/processors/community_id.test.tsx +++ b/x-pack/plugins/ingest_pipelines/public/application/components/pipeline_editor/__jest__/processors/community_id.test.tsx @@ -107,4 +107,52 @@ describe('Processor: Community id', () => { seed: 10, }); }); + + test('should not add a processor if the seedField is smaller than min_value', async () => { + const { + actions: { saveNewProcessor }, + form, + } = testBed; + + form.setInputValue('seedField.input', '-1'); + + // Save the field with new changes + await saveNewProcessor(); + + const processors = getProcessorValue(onUpdate, COMMUNITY_ID_TYPE); + + expect(processors).toHaveLength(0); + }); + + test('should not add a processor if the seedField is bigger than max_value', async () => { + const { + actions: { saveNewProcessor }, + form, + } = testBed; + + form.setInputValue('seedField.input', '65536'); + + // Save the field with new changes + await saveNewProcessor(); + + const processors = getProcessorValue(onUpdate, COMMUNITY_ID_TYPE); + + expect(processors).toHaveLength(0); + }); + + test('should not add a processor if the seedField is not an integer', async () => { + const { + actions: { saveNewProcessor }, + form, + } = testBed; + + form.setInputValue('seedField.input', '10.2'); + + // Save the field with new changes + await saveNewProcessor(); + + const processors = getProcessorValue(onUpdate, COMMUNITY_ID_TYPE); + + expect(processors).toHaveLength(0); + }); }); diff --git a/x-pack/plugins/ingest_pipelines/public/application/components/pipeline_editor/components/processor_form/processors/community_id.tsx b/x-pack/plugins/ingest_pipelines/public/application/components/pipeline_editor/components/processor_form/processors/community_id.tsx index 5a2aa91547c94..7a08a5c72b827 100644 --- a/x-pack/plugins/ingest_pipelines/public/application/components/pipeline_editor/components/processor_form/processors/community_id.tsx +++ b/x-pack/plugins/ingest_pipelines/public/application/components/pipeline_editor/components/processor_form/processors/community_id.tsx @@ -44,6 +44,14 @@ const seedValidator = { values: { minValue: SEED_MIN_VALUE }, }), }), + int: fieldValidators.isInteger({ + message: i18n.translate( + 'xpack.ingestPipelines.pipelineEditor.communityId.integerRequiredError', + { + defaultMessage: 'Only integers are allowed.', + } + ), + }), }; const fieldsConfig: FieldsConfig = { @@ -183,7 +191,7 @@ const fieldsConfig: FieldsConfig = { { validator: (field) => { if (field.value) { - return seedValidator.max(field) ?? seedValidator.min(field); + return seedValidator.max(field) ?? seedValidator.min(field) ?? seedValidator.int(field); } }, }, diff --git a/x-pack/plugins/translations/translations/fr-FR.json b/x-pack/plugins/translations/translations/fr-FR.json index 8761399b5d33a..9399120719fd3 100644 --- a/x-pack/plugins/translations/translations/fr-FR.json +++ b/x-pack/plugins/translations/translations/fr-FR.json @@ -24131,6 +24131,7 @@ "xpack.ingestPipelines.pipelineEditor.communityId.icmpCodeLabel": "Code ICMP (facultatif)", "xpack.ingestPipelines.pipelineEditor.communityId.icmpTypeHelpText": "Champ contenant le type ICMP de la destination. La valeur par défaut est {defaultValue}.", "xpack.ingestPipelines.pipelineEditor.communityId.icmpTypeLabel": "Type ICMP (facultatif)", + "xpack.ingestPipelines.pipelineEditor.communityId.integerRequiredError": "Seuls les entiers sont autorisés.", "xpack.ingestPipelines.pipelineEditor.communityId.seedHelpText": "Valeur initiale du hachage de l'ID de communauté. La valeur par défaut est {defaultValue}.", "xpack.ingestPipelines.pipelineEditor.communityId.seedLabel": "Valeur initiale (facultatif)", "xpack.ingestPipelines.pipelineEditor.communityId.seedMaxNumberError": "Ce nombre doit être inférieur ou égal à {maxValue}.", diff --git a/x-pack/plugins/translations/translations/ja-JP.json b/x-pack/plugins/translations/translations/ja-JP.json index e02a35a433251..59a6bdf9b149f 100644 --- a/x-pack/plugins/translations/translations/ja-JP.json +++ b/x-pack/plugins/translations/translations/ja-JP.json @@ -23878,6 +23878,7 @@ "xpack.ingestPipelines.pipelineEditor.communityId.icmpCodeLabel": "ICMPコード（任意）", "xpack.ingestPipelines.pipelineEditor.communityId.icmpTypeHelpText": "デスティネーションICMPタイプを含むフィールド。デフォルトは{defaultValue}です。", "xpack.ingestPipelines.pipelineEditor.communityId.icmpTypeLabel": "ICMPタイプ（任意）", + "xpack.ingestPipelines.pipelineEditor.communityId.integerRequiredError": "整数のみを使用できます。", "xpack.ingestPipelines.pipelineEditor.communityId.seedHelpText": "コミュニティIDハッシュのシード。デフォルトは{defaultValue}です。", "xpack.ingestPipelines.pipelineEditor.communityId.seedLabel": "シード（任意）", "xpack.ingestPipelines.pipelineEditor.communityId.seedMaxNumberError": "この数は{maxValue}以下でなければなりません。", diff --git a/x-pack/plugins/translations/translations/zh-CN.json b/x-pack/plugins/translations/translations/zh-CN.json index 8fbf954130039..5f58a61b928cf 100644 --- a/x-pack/plugins/translations/translations/zh-CN.json +++ b/x-pack/plugins/translations/translations/zh-CN.json @@ -23912,6 +23912,7 @@ "xpack.ingestPipelines.pipelineEditor.communityId.icmpCodeLabel": "ICMP 代码（可选）", "xpack.ingestPipelines.pipelineEditor.communityId.icmpTypeHelpText": "包含目标 ICMP 类型的字段。默认为 {defaultValue}。", "xpack.ingestPipelines.pipelineEditor.communityId.icmpTypeLabel": "ICMP 类型（可选）", + "xpack.ingestPipelines.pipelineEditor.communityId.integerRequiredError": "仅允许使用整数。", "xpack.ingestPipelines.pipelineEditor.communityId.seedHelpText": "社区 ID 哈希的种子。默认为 {defaultValue}。", "xpack.ingestPipelines.pipelineEditor.communityId.seedLabel": "种子（可选）", "xpack.ingestPipelines.pipelineEditor.communityId.seedMaxNumberError": "此数字必须等于或小于 {maxValue}。",