From d2428b5f0cd0847453d1e5d1911e0c743910ed12 Mon Sep 17 00:00:00 2001 From: Yuliia Naumenko Date: Fri, 20 Mar 2020 07:18:54 -0700 Subject: [PATCH] Implemented ability to clear and properly validate alert interval (#60571) * Implemented ability to clear and properly validate alert interval * Fixed due to comments * Fixed additional request for the last field * Fixed failing test --- .../alerting/common/parse_duration.test.ts | 32 ++++++++++++++++- .../plugins/alerting/common/parse_duration.ts | 9 +++++ .../threshold/expression.tsx | 4 +-- .../sections/alert_form/alert_form.test.tsx | 4 +-- .../sections/alert_form/alert_form.tsx | 34 +++++++++++++------ .../expression_items/for_the_last.test.tsx | 2 +- .../common/expression_items/for_the_last.tsx | 14 ++++---- .../apps/triggers_actions_ui/alerts.ts | 1 - 8 files changed, 75 insertions(+), 25 deletions(-) diff --git a/x-pack/plugins/alerting/common/parse_duration.test.ts b/x-pack/plugins/alerting/common/parse_duration.test.ts index ccdddd8ecf5f40..41d3ab5868c9ea 100644 --- a/x-pack/plugins/alerting/common/parse_duration.test.ts +++ b/x-pack/plugins/alerting/common/parse_duration.test.ts @@ -4,7 +4,7 @@ * you may not use this file except in compliance with the Elastic License. */ -import { parseDuration } from './parse_duration'; +import { parseDuration, getDurationNumberInItsUnit, getDurationUnitValue } from './parse_duration'; test('parses seconds', () => { const result = parseDuration('10s'); @@ -52,3 +52,33 @@ test('throws error when 0 based', () => { `"Invalid duration \\"0d\\". Durations must be of the form {number}x. Example: 5s, 5m, 5h or 5d\\""` ); }); + +test('getDurationNumberInItsUnit days', () => { + const result = getDurationNumberInItsUnit('10d'); + expect(result).toEqual(10); +}); + +test('getDurationNumberInItsUnit minutes', () => { + const result = getDurationNumberInItsUnit('1m'); + expect(result).toEqual(1); +}); + +test('getDurationNumberInItsUnit seconds', () => { + const result = getDurationNumberInItsUnit('123s'); + expect(result).toEqual(123); +}); + +test('getDurationUnitValue minutes', () => { + const result = getDurationUnitValue('1m'); + expect(result).toEqual('m'); +}); + +test('getDurationUnitValue days', () => { + const result = getDurationUnitValue('23d'); + expect(result).toEqual('d'); +}); + +test('getDurationUnitValue hours', () => { + const result = getDurationUnitValue('100h'); + expect(result).toEqual('h'); +}); diff --git a/x-pack/plugins/alerting/common/parse_duration.ts b/x-pack/plugins/alerting/common/parse_duration.ts index 4e35a4c4cb0cf6..c271035f012e58 100644 --- a/x-pack/plugins/alerting/common/parse_duration.ts +++ b/x-pack/plugins/alerting/common/parse_duration.ts @@ -25,6 +25,15 @@ export function parseDuration(duration: string): number { ); } +export function getDurationNumberInItsUnit(duration: string): number { + return parseInt(duration.replace(/[^0-9.]/g, ''), 0); +} + +export function getDurationUnitValue(duration: string): string { + const durationNumber = getDurationNumberInItsUnit(duration); + return duration.replace(durationNumber.toString(), ''); +} + export function validateDurationSchema(duration: string) { if (duration.match(SECONDS_REGEX)) { return; diff --git a/x-pack/plugins/triggers_actions_ui/public/application/components/builtin_alert_types/threshold/expression.tsx b/x-pack/plugins/triggers_actions_ui/public/application/components/builtin_alert_types/threshold/expression.tsx index 728418bf3c3368..fa26e8b11bfec3 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/components/builtin_alert_types/threshold/expression.tsx +++ b/x-pack/plugins/triggers_actions_ui/public/application/components/builtin_alert_types/threshold/expression.tsx @@ -399,8 +399,8 @@ export const IndexThresholdAlertTypeExpression: React.FunctionComponent setAlertParams('timeWindowSize', selectedWindowSize) diff --git a/x-pack/plugins/triggers_actions_ui/public/application/sections/alert_form/alert_form.test.tsx b/x-pack/plugins/triggers_actions_ui/public/application/sections/alert_form/alert_form.test.tsx index b31be7ecb9a79d..b87aaacb3ec0e2 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/sections/alert_form/alert_form.test.tsx +++ b/x-pack/plugins/triggers_actions_ui/public/application/sections/alert_form/alert_form.test.tsx @@ -86,7 +86,7 @@ describe('alert_form', () => { uiSettings: deps!.uiSettings, }} > - {}} errors={{ name: [] }} /> + {}} errors={{ name: [], interval: [] }} /> ); @@ -165,7 +165,7 @@ describe('alert_form', () => { uiSettings: deps!.uiSettings, }} > - {}} errors={{ name: [] }} /> + {}} errors={{ name: [], interval: [] }} /> ); diff --git a/x-pack/plugins/triggers_actions_ui/public/application/sections/alert_form/alert_form.tsx b/x-pack/plugins/triggers_actions_ui/public/application/sections/alert_form/alert_form.tsx index 1fa620c5394a12..8382cbe825da32 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/sections/alert_form/alert_form.tsx +++ b/x-pack/plugins/triggers_actions_ui/public/application/sections/alert_form/alert_form.tsx @@ -24,6 +24,10 @@ import { EuiButtonIcon, EuiHorizontalRule, } from '@elastic/eui'; +import { + getDurationNumberInItsUnit, + getDurationUnitValue, +} from '../../../../../alerting/common/parse_duration'; import { loadAlertTypes } from '../../lib/alert_api'; import { actionVariablesFromAlertType } from '../../lib/action_variables'; import { AlertReducerAction } from './alert_reducer'; @@ -48,7 +52,7 @@ export function validateBaseProperties(alertObject: Alert) { }) ); } - if (!alertObject.schedule.interval) { + if (alertObject.schedule.interval.length < 2) { errors.interval.push( i18n.translate('xpack.triggersActionsUI.sections.alertForm.error.requiredIntervalText', { defaultMessage: 'Check interval is required.', @@ -81,17 +85,17 @@ export const AlertForm = ({ alert, canChangeTrigger = true, dispatch, errors }: ); const [alertTypesIndex, setAlertTypesIndex] = useState(undefined); - const [alertInterval, setAlertInterval] = useState( - alert.schedule.interval ? parseInt(alert.schedule.interval.replace(/^[A-Za-z]+$/, ''), 0) : 1 + const [alertInterval, setAlertInterval] = useState( + alert.schedule.interval ? getDurationNumberInItsUnit(alert.schedule.interval) : undefined ); const [alertIntervalUnit, setAlertIntervalUnit] = useState( - alert.schedule.interval ? alert.schedule.interval.replace(alertInterval.toString(), '') : 'm' + alert.schedule.interval ? getDurationUnitValue(alert.schedule.interval) : 'm' ); const [alertThrottle, setAlertThrottle] = useState( - alert.throttle ? parseInt(alert.throttle.replace(/^[A-Za-z]+$/, ''), 0) : null + alert.throttle ? getDurationNumberInItsUnit(alert.throttle) : null ); const [alertThrottleUnit, setAlertThrottleUnit] = useState( - alert.throttle ? alert.throttle.replace((alertThrottle ?? '').toString(), '') : 'm' + alert.throttle ? getDurationUnitValue(alert.throttle) : 'm' ); const [defaultActionGroupId, setDefaultActionGroupId] = useState(undefined); @@ -344,19 +348,27 @@ export const AlertForm = ({ alert, canChangeTrigger = true, dispatch, errors }: - + 0} + error={errors.interval} + > 0} compressed - value={alertInterval} + value={alertInterval || ''} name="interval" data-test-subj="intervalInput" onChange={e => { - const interval = e.target.value !== '' ? parseInt(e.target.value, 10) : null; - setAlertInterval(interval ?? 1); + const interval = + e.target.value !== '' ? parseInt(e.target.value, 10) : undefined; + setAlertInterval(interval); setScheduleProperty('interval', `${e.target.value}${alertIntervalUnit}`); }} /> @@ -366,7 +378,7 @@ export const AlertForm = ({ alert, canChangeTrigger = true, dispatch, errors }: fullWidth compressed value={alertIntervalUnit} - options={getTimeOptions(alertInterval)} + options={getTimeOptions(alertInterval ?? 1)} onChange={e => { setAlertIntervalUnit(e.target.value); setScheduleProperty('interval', `${alertInterval}${e.target.value}`); diff --git a/x-pack/plugins/triggers_actions_ui/public/common/expression_items/for_the_last.test.tsx b/x-pack/plugins/triggers_actions_ui/public/common/expression_items/for_the_last.test.tsx index 6ae3056001c8f5..e66bb1e7b4b9a9 100644 --- a/x-pack/plugins/triggers_actions_ui/public/common/expression_items/for_the_last.test.tsx +++ b/x-pack/plugins/triggers_actions_ui/public/common/expression_items/for_the_last.test.tsx @@ -36,7 +36,7 @@ describe('for the last expression', () => { /> ); wrapper.simulate('click'); - expect(wrapper.find('[value=1]').length > 0).toBeTruthy(); + expect(wrapper.find('[value=""]').length > 0).toBeTruthy(); expect(wrapper.find('[value="s"]').length > 0).toBeTruthy(); expect( wrapper.contains( diff --git a/x-pack/plugins/triggers_actions_ui/public/common/expression_items/for_the_last.tsx b/x-pack/plugins/triggers_actions_ui/public/common/expression_items/for_the_last.tsx index 844551de3171de..673391dd9cbad9 100644 --- a/x-pack/plugins/triggers_actions_ui/public/common/expression_items/for_the_last.tsx +++ b/x-pack/plugins/triggers_actions_ui/public/common/expression_items/for_the_last.tsx @@ -25,7 +25,7 @@ interface ForLastExpressionProps { timeWindowSize?: number; timeWindowUnit?: string; errors: { [key: string]: string[] }; - onChangeWindowSize: (selectedWindowSize: number | '') => void; + onChangeWindowSize: (selectedWindowSize: number | undefined) => void; onChangeWindowUnit: (selectedWindowUnit: string) => void; popupPosition?: | 'upCenter' @@ -43,7 +43,7 @@ interface ForLastExpressionProps { } export const ForLastExpression = ({ - timeWindowSize = 1, + timeWindowSize, timeWindowUnit = 's', errors, onChangeWindowSize, @@ -64,7 +64,7 @@ export const ForLastExpression = ({ )} value={`${timeWindowSize} ${getTimeUnitLabel( timeWindowUnit as TIME_UNITS, - timeWindowSize.toString() + (timeWindowSize ?? '').toString() )}`} isActive={alertDurationPopoverOpen} onClick={() => { @@ -97,11 +97,11 @@ export const ForLastExpression = ({ 0 && timeWindowSize !== undefined} - min={1} - value={timeWindowSize} + min={0} + value={timeWindowSize || ''} onChange={e => { const { value } = e.target; - const timeWindowSizeVal = value !== '' ? parseInt(value, 10) : value; + const timeWindowSizeVal = value !== '' ? parseInt(value, 10) : undefined; onChangeWindowSize(timeWindowSizeVal); }} /> @@ -114,7 +114,7 @@ export const ForLastExpression = ({ onChange={e => { onChangeWindowUnit(e.target.value); }} - options={getTimeOptions(timeWindowSize)} + options={getTimeOptions(timeWindowSize ?? 1)} /> diff --git a/x-pack/test/functional_with_es_ssl/apps/triggers_actions_ui/alerts.ts b/x-pack/test/functional_with_es_ssl/apps/triggers_actions_ui/alerts.ts index 79448fa535370c..d7df541433ebca 100644 --- a/x-pack/test/functional_with_es_ssl/apps/triggers_actions_ui/alerts.ts +++ b/x-pack/test/functional_with_es_ssl/apps/triggers_actions_ui/alerts.ts @@ -63,7 +63,6 @@ export default ({ getPageObjects, getService }: FtrProviderContext) => { await fieldOptions[1].click(); // need this two out of popup clicks to close them await nameInput.click(); - await testSubjects.click('intervalInput'); await testSubjects.click('.slack-ActionTypeSelectOption'); await testSubjects.click('createActionConnectorButton');