forked from elastic/kibana
-
Notifications
You must be signed in to change notification settings - Fork 0
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
[Index Management] Fix unhandled error in ds data retention modal (el…
…astic#196524) Fixes elastic#196331 ## Summary This PR fixes the bug in the Edit ds data retention modal where we were comparing the max retention period with an undefined `value` (now the comparison happens only if `value` is defined). Also, the PR makes the data retention field get re-validated only when the time unit changes (otherwise, when we switch off the toggle to enable to data retention field, the field would get validated and would immediately show an error "A data retention value is required." which is not great UX). ### How to test: 1. Start serverless ES with `yarn es serverless --projectType=security -E data_streams.lifecycle.retention.max=200d` and kibana with `yarn serverless-security` 2. Navigate to Kibana, create a data stream using Dev Tools: ``` PUT _index_template/ds { "index_patterns": ["ds"], "data_stream": {} } POST ds/_doc { "@timestamp": "2020-01-27" } ``` 3. Navigate to Index Management. Find the data stream and select it --> Click "Manage" --> Click "Edit data retention" 4. Disable the toggle "Keep data up to maximum retention period" 5. Verify that the field is enabled correctly, there is not endless spinner, and no console error. https://github.com/user-attachments/assets/957e0869-ee23-46d9-8f20-134937f6f8cf --------- Co-authored-by: Matthew Kime <[email protected]>
- Loading branch information
1 parent
68b328d
commit d8fa996
Showing
3 changed files
with
161 additions
and
62 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
53 changes: 53 additions & 0 deletions
53
.../application/sections/home/data_stream_list/edit_data_retention_modal/validations.test.ts
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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'); | ||
| }); | ||
| }); |
61 changes: 61 additions & 0 deletions
61
...ublic/application/sections/home/data_stream_list/edit_data_retention_modal/validations.ts
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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; | ||
| }; |