From e6e88aa528fa63eafcd9d6ec900dbc6d1904f52d Mon Sep 17 00:00:00 2001 From: Virginia Cepeda Date: Fri, 2 Jun 2023 11:18:26 -0300 Subject: [PATCH] Alerting: Display error if repeat interval is lower than group interval (#69413) * Display error if repeat interval is lower than group interval * Small change * Update tests --- .../EditDefaultPolicyForm.test.tsx | 26 +++++++++++++++++++ .../EditDefaultPolicyForm.tsx | 10 +++++-- .../EditNotificationPolicyForm.test.tsx | 26 +++++++++++++++++++ .../EditNotificationPolicyForm.tsx | 10 +++++-- .../alerting/unified/utils/amroutes.ts | 25 ++++++++++++++++++ 5 files changed, 93 insertions(+), 4 deletions(-) diff --git a/public/app/features/alerting/unified/components/notification-policies/EditDefaultPolicyForm.test.tsx b/public/app/features/alerting/unified/components/notification-policies/EditDefaultPolicyForm.test.tsx index 6203fe440dd..e6e046c66b0 100644 --- a/public/app/features/alerting/unified/components/notification-policies/EditDefaultPolicyForm.test.tsx +++ b/public/app/features/alerting/unified/components/notification-policies/EditDefaultPolicyForm.test.tsx @@ -78,6 +78,32 @@ describe('EditDefaultPolicyForm', function () { }); }); + it('should show an error if repeat interval is lower than group interval', async function () { + const user = userEvent.setup(); + + const onSubmit = jest.fn(); + renderRouteForm( + { + id: '0', + receiver: 'default', + }, + [{ value: 'default', label: 'Default' }], + onSubmit + ); + + await user.click(ui.timingOptionsBtn.get()); + + await user.type(ui.groupWaitInput.get(), '5m25s'); + await user.type(ui.groupIntervalInput.get(), '35m40s'); + await user.type(ui.repeatIntervalInput.get(), '30m'); + + await user.click(ui.submitBtn.get()); + + expect(ui.error.getAll()).toHaveLength(1); + expect(ui.error.get().textContent).toBe('Repeat interval should be higher or equal to Group interval'); + expect(onSubmit).not.toHaveBeenCalled(); + }); + it('should allow resetting existing timing options', async function () { const user = userEvent.setup(); diff --git a/public/app/features/alerting/unified/components/notification-policies/EditDefaultPolicyForm.tsx b/public/app/features/alerting/unified/components/notification-policies/EditDefaultPolicyForm.tsx index 192299b12ad..1812ad8bbb9 100644 --- a/public/app/features/alerting/unified/components/notification-policies/EditDefaultPolicyForm.tsx +++ b/public/app/features/alerting/unified/components/notification-policies/EditDefaultPolicyForm.tsx @@ -10,6 +10,7 @@ import { mapMultiSelectValueToStrings, mapSelectValueToString, promDurationValidator, + repeatIntervalValidator, stringsToSelectableValues, stringToSelectableValue, } from '../../utils/amroutes'; @@ -43,7 +44,7 @@ export const AmRootRouteForm = ({ return (
- {({ register, control, errors, setValue }) => ( + {({ register, control, errors, setValue, getValues }) => ( <> <> @@ -143,7 +144,12 @@ export const AmRootRouteForm = ({ data-testid="am-repeat-interval" > { + const groupInterval = getValues('groupIntervalValue'); + return repeatIntervalValidator(value, groupInterval); + }, + })} placeholder={TIMING_OPTIONS_DEFAULTS.repeat_interval} className={styles.promDurationInput} aria-label="Repeat interval" diff --git a/public/app/features/alerting/unified/components/notification-policies/EditNotificationPolicyForm.test.tsx b/public/app/features/alerting/unified/components/notification-policies/EditNotificationPolicyForm.test.tsx index b86c7f21097..38856103425 100644 --- a/public/app/features/alerting/unified/components/notification-policies/EditNotificationPolicyForm.test.tsx +++ b/public/app/features/alerting/unified/components/notification-policies/EditNotificationPolicyForm.test.tsx @@ -76,6 +76,32 @@ describe('EditNotificationPolicyForm', function () { }); }); + it('should show an error if repeat interval is lower than group interval', async function () { + const user = userEvent.setup(); + + const onSubmit = jest.fn(); + renderRouteForm( + { + id: '1', + receiver: 'default', + }, + [{ value: 'default', label: 'Default' }], + onSubmit + ); + + await user.click(ui.overrideTimingsCheckbox.get()); + + await user.type(ui.groupWaitInput.get(), '5m25s'); + await user.type(ui.groupIntervalInput.get(), '35m40s'); + await user.type(ui.repeatIntervalInput.get(), '30m'); + + await user.click(ui.submitBtn.get()); + + expect(ui.error.getAll()).toHaveLength(1); + expect(ui.error.get().textContent).toBe('Repeat interval should be higher or equal to Group interval'); + expect(onSubmit).not.toHaveBeenCalled(); + }); + it('should allow resetting existing timing options', async function () { const user = userEvent.setup(); diff --git a/public/app/features/alerting/unified/components/notification-policies/EditNotificationPolicyForm.tsx b/public/app/features/alerting/unified/components/notification-policies/EditNotificationPolicyForm.tsx index eff3d4a2c54..2ec875a0fcf 100644 --- a/public/app/features/alerting/unified/components/notification-policies/EditNotificationPolicyForm.tsx +++ b/public/app/features/alerting/unified/components/notification-policies/EditNotificationPolicyForm.tsx @@ -32,6 +32,7 @@ import { commonGroupByOptions, amRouteToFormAmRoute, promDurationValidator, + repeatIntervalValidator, } from '../../utils/amroutes'; import { AmRouteReceiver } from '../receivers/grafanaAppReceivers/types'; @@ -74,7 +75,7 @@ export const AmRoutesExpandedForm = ({ return ( - {({ control, register, errors, setValue, watch }) => ( + {({ control, register, errors, setValue, watch, getValues }) => ( <> {/* @ts-ignore-check: react-hook-form made me do this */} @@ -247,7 +248,12 @@ export const AmRoutesExpandedForm = ({ error={errors.repeatIntervalValue?.message} > { + const groupInterval = getValues('groupIntervalValue'); + return repeatIntervalValidator(value, groupInterval); + }, + })} aria-label="Repeat interval value" className={formStyles.promDurationInput} /> diff --git a/public/app/features/alerting/unified/utils/amroutes.ts b/public/app/features/alerting/unified/utils/amroutes.ts index 7fbb401e007..615ef277049 100644 --- a/public/app/features/alerting/unified/utils/amroutes.ts +++ b/public/app/features/alerting/unified/utils/amroutes.ts @@ -3,6 +3,7 @@ import { uniqueId } from 'lodash'; import { SelectableValue } from '@grafana/data'; import { MatcherOperator, ObjectMatcher, Route, RouteWithID } from 'app/plugins/datasource/alertmanager/types'; +import { safeParseDurationstr } from '../components/rules/EditRuleGroupModal'; import { FormAmRoute } from '../types/amroutes'; import { MatcherFieldValue } from '../types/silence-form'; @@ -225,3 +226,27 @@ export function promDurationValidator(duration: string) { return isValidPrometheusDuration(duration) || 'Invalid duration format. Must be {number}{time_unit}'; } + +export const repeatIntervalValidator = (repeatInterval: string, groupInterval: string) => { + if (repeatInterval.length === 0) { + return true; + } + + const validRepeatInterval = promDurationValidator(repeatInterval); + const validGroupInterval = promDurationValidator(groupInterval); + + if (validRepeatInterval !== true) { + return validRepeatInterval; + } + + if (validGroupInterval !== true) { + return validGroupInterval; + } + + const repeatDuration = safeParseDurationstr(repeatInterval); + const groupDuration = safeParseDurationstr(groupInterval); + + const isRepeatLowerThanGroupDuration = groupDuration !== 0 && repeatDuration < groupDuration; + + return isRepeatLowerThanGroupDuration ? 'Repeat interval should be higher or equal to Group interval' : true; +};