[8.x] [Index Management] Fix unhandled error in ds data retention mod…

…al (#196524) (#197481) # Backport This will backport the following commits from `main` to `8.x`: - [[Index Management] Fix unhandled error in ds data retention modal (#196524)](#196524)  ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport) \r\nClick \"Manage\" --> Click \"Edit data retention\"\r\n4. Disable the toggle \"Keep data up to maximum retention period\"\r\n5. Verify that the field is enabled correctly, there is not endless\r\nspinner, and no console error.\r\n\r\n\r\n\r\nhttps://github.com/user-attachments/assets/957e0869-ee23-46d9-8f20-134937f6f8cf\r\n\r\n---------\r\n\r\nCo-authored-by: Matthew Kime <[email protected]>","sha":"d8fa996c5052d10da2f438b93723437f5631bbde","branchLabelMapping":{"^v9.0.0$":"main","^v8.17.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["Feature:Index Management","Team:Kibana Management","release_note:skip","v9.0.0","backport:prev-minor","v8.17.0"],"title":"[Index Management] Fix unhandled error in ds data retention modal","number":196524,"url":"https://github.com/elastic/kibana/pull/196524","mergeCommit":{"message":"[Index Management] Fix unhandled error in ds data retention modal (#196524)\n\nFixes https://github.com/elastic/kibana/issues/196331\r\n\r\n## Summary\r\n\r\nThis PR fixes the bug in the Edit ds data retention modal where we were\r\ncomparing the max retention period with an undefined `value` (now the\r\ncomparison happens only if `value` is defined). Also, the PR makes the\r\ndata retention field get re-validated only when the time unit changes\r\n(otherwise, when we switch off the toggle to enable to data retention\r\nfield, the field would get validated and would immediately show an error\r\n\"A data retention value is required.\" which is not great UX).\r\n\r\n### How to test:\r\n1. Start serverless ES with `yarn es serverless --projectType=security\r\n-E data_streams.lifecycle.retention.max=200d` and kibana with `yarn\r\nserverless-security`\r\n2. Navigate to Kibana, create a data stream using Dev Tools:\r\n```\r\nPUT _index_template/ds\r\n{\r\n \"index_patterns\": [\"ds\"],\r\n \"data_stream\": {}\r\n}\r\n\r\nPOST ds/_doc\r\n{\r\n \"@timestamp\": \"2020-01-27\"\r\n}\r\n```\r\n3. Navigate to Index Management. Find the data stream and select it -->\r\nClick \"Manage\" --> Click \"Edit data retention\"\r\n4. Disable the toggle \"Keep data up to maximum retention period\"\r\n5. Verify that the field is enabled correctly, there is not endless\r\nspinner, and no console error.\r\n\r\n\r\n\r\nhttps://github.com/user-attachments/assets/957e0869-ee23-46d9-8f20-134937f6f8cf\r\n\r\n---------\r\n\r\nCo-authored-by: Matthew Kime <[email protected]>","sha":"d8fa996c5052d10da2f438b93723437f5631bbde"}},"sourceBranch":"main","suggestedTargetBranches":["8.x"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/196524","number":196524,"mergeCommit":{"message":"[Index Management] Fix unhandled error in ds data retention modal (#196524)\n\nFixes https://github.com/elastic/kibana/issues/196331\r\n\r\n## Summary\r\n\r\nThis PR fixes the bug in the Edit ds data retention modal where we were\r\ncomparing the max retention period with an undefined `value` (now the\r\ncomparison happens only if `value` is defined). Also, the PR makes the\r\ndata retention field get re-validated only when the time unit changes\r\n(otherwise, when we switch off the toggle to enable to data retention\r\nfield, the field would get validated and would immediately show an error\r\n\"A data retention value is required.\" which is not great UX).\r\n\r\n### How to test:\r\n1. Start serverless ES with `yarn es serverless --projectType=security\r\n-E data_streams.lifecycle.retention.max=200d` and kibana with `yarn\r\nserverless-security`\r\n2. Navigate to Kibana, create a data stream using Dev Tools:\r\n```\r\nPUT _index_template/ds\r\n{\r\n \"index_patterns\": [\"ds\"],\r\n \"data_stream\": {}\r\n}\r\n\r\nPOST ds/_doc\r\n{\r\n \"@timestamp\": \"2020-01-27\"\r\n}\r\n```\r\n3. Navigate to Index Management. Find the data stream and select it -->\r\nClick \"Manage\" --> Click \"Edit data retention\"\r\n4. Disable the toggle \"Keep data up to maximum retention period\"\r\n5. Verify that the field is enabled correctly, there is not endless\r\nspinner, and no console error.\r\n\r\n\r\n\r\nhttps://github.com/user-attachments/assets/957e0869-ee23-46d9-8f20-134937f6f8cf\r\n\r\n---------\r\n\r\nCo-authored-by: Matthew Kime <[email protected]>","sha":"d8fa996c5052d10da2f438b93723437f5631bbde"}},{"branch":"8.x","label":"v8.17.0","branchLabelMappingKey":"^v8.17.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}] BACKPORT--> Co-authored-by: Elena Stoeva <[email protected]>
elastic · Oct 23, 2024 · a91f488 · a91f488
1 parent 083d618
commit a91f488
Show file tree

Hide file tree

Showing 3 changed files with 161 additions and 62 deletions.
diff --git a/...on/sections/home/data_stream_list/edit_data_retention_modal/edit_data_retention_modal.tsx b/...on/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,
@@ -94,50 +67,62 @@ const configurationFormSchema: FormSchema = {
     formatters: [fieldFormatters.toInt],
     validations: [
       {
-        validator: ({ value, formData, customData }) => {
-          // If infiniteRetentionPeriod is set, we dont need to validate the data retention field
-          if (formData.infiniteRetentionPeriod) {
-            return undefined;
+        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.',
+                }
+              ),
+            };
           }
-
-          // 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)) {
-              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) },
-                  }
-                ),
-              };
+        },
+      },
+      {
+        validator: ({ value, formData, customData }) => {
+          // 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);
           }
         },
       },
@@ -258,11 +243,11 @@ export const EditDataRetentionModal: React.FunctionComponent<Props> = ({
   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 timeUnit field changes, we need to re-validate
+  // the dataRetention field
   useEffect(() => {
     form.validateFields(['dataRetention']);
-  }, [formData, form]);
+  }, [formData.timeUnit, form]);
 
   const onSubmitForm = async () => {
     const { isValid, data } = await form.submit();

diff --git a/.../application/sections/home/data_stream_list/edit_data_retention_modal/validations.test.ts b/.../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('', '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)', () => {
+    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/...ublic/application/sections/home/data_stream_list/edit_data_retention_modal/validations.ts b/...ublic/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: string | number,
+  retentionTimeUnit: string,
+  globalMaxRetention: string
+) => {
+  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;
+};