From ace5b2058d05112eafe63b1cd8834bd6281c3734 Mon Sep 17 00:00:00 2001 From: Matthew Jacobson Date: Thu, 2 Jun 2022 07:45:21 -0400 Subject: [PATCH] Alerting: Fix notification policy "Override grouping" form save (#50031) --- .../components/amroutes/AmRootRouteForm.tsx | 2 +- .../amroutes/AmRoutesExpandedForm.tsx | 9 +- .../components/amroutes/AmRoutesTable.test.ts | 1 + .../components/amroutes/AmRoutesTable.tsx | 2 +- .../alerting/unified/types/amroutes.ts | 1 + .../alerting/unified/utils/amroutes.test.ts | 91 +++++++++++++++++++ .../alerting/unified/utils/amroutes.ts | 8 +- 7 files changed, 104 insertions(+), 10 deletions(-) create mode 100644 public/app/features/alerting/unified/utils/amroutes.test.ts diff --git a/public/app/features/alerting/unified/components/amroutes/AmRootRouteForm.tsx b/public/app/features/alerting/unified/components/amroutes/AmRootRouteForm.tsx index 5375476a99a..3a98e732783 100644 --- a/public/app/features/alerting/unified/components/amroutes/AmRootRouteForm.tsx +++ b/public/app/features/alerting/unified/components/amroutes/AmRootRouteForm.tsx @@ -36,7 +36,7 @@ export const AmRootRouteForm: FC = ({ const [groupByOptions, setGroupByOptions] = useState(stringsToSelectableValues(routes.groupBy)); return ( -
+ {({ control, errors, setValue }) => ( <> diff --git a/public/app/features/alerting/unified/components/amroutes/AmRoutesExpandedForm.tsx b/public/app/features/alerting/unified/components/amroutes/AmRoutesExpandedForm.tsx index d2f28c173a9..d95f984a09c 100644 --- a/public/app/features/alerting/unified/components/amroutes/AmRoutesExpandedForm.tsx +++ b/public/app/features/alerting/unified/components/amroutes/AmRoutesExpandedForm.tsx @@ -44,7 +44,6 @@ export interface AmRoutesExpandedFormProps { export const AmRoutesExpandedForm: FC = ({ onCancel, onSave, receivers, routes }) => { const styles = useStyles2(getStyles); const formStyles = useStyles2(getFormStyles); - const [overrideGrouping, setOverrideGrouping] = useState(routes.groupBy.length > 0); const [groupByOptions, setGroupByOptions] = useState(stringsToSelectableValues(routes.groupBy)); const muteTimingOptions = useMuteTimingOptions(); @@ -159,13 +158,9 @@ export const AmRoutesExpandedForm: FC = ({ onCancel, - setOverrideGrouping((overrideGrouping) => !overrideGrouping)} - /> + - {overrideGrouping && ( + {watch().overrideGrouping && ( = ({ { id: 'groupBy', label: 'Group by', - renderCell: (item) => item.data.groupBy.join(', ') || '-', + renderCell: (item) => (item.data.overrideGrouping && item.data.groupBy.join(', ')) || '-', size: 5, }, { diff --git a/public/app/features/alerting/unified/types/amroutes.ts b/public/app/features/alerting/unified/types/amroutes.ts index 0e15feb8e77..b52b56e489c 100644 --- a/public/app/features/alerting/unified/types/amroutes.ts +++ b/public/app/features/alerting/unified/types/amroutes.ts @@ -5,6 +5,7 @@ export interface FormAmRoute { object_matchers: MatcherFieldValue[]; continue: boolean; receiver: string; + overrideGrouping: boolean; groupBy: string[]; overrideTimings: boolean; groupWaitValue: string; diff --git a/public/app/features/alerting/unified/utils/amroutes.test.ts b/public/app/features/alerting/unified/utils/amroutes.test.ts new file mode 100644 index 00000000000..20448a0a9de --- /dev/null +++ b/public/app/features/alerting/unified/utils/amroutes.test.ts @@ -0,0 +1,91 @@ +import { Route } from 'app/plugins/datasource/alertmanager/types'; + +import { FormAmRoute } from '../types/amroutes'; + +import { amRouteToFormAmRoute, emptyRoute, formAmRouteToAmRoute } from './amroutes'; + +const emptyAmRoute: Route = { + receiver: '', + group_by: [], + continue: false, + object_matchers: [], + matchers: [], + match: {}, + match_re: {}, + group_wait: '', + group_interval: '', + repeat_interval: '', + routes: [], + mute_time_intervals: [], +}; + +const buildAmRoute = (override: Partial = {}): Route => { + return { ...emptyAmRoute, ...override }; +}; + +const buildFormAmRoute = (override: Partial = {}): FormAmRoute => { + return { ...emptyRoute, ...override }; +}; + +describe('formAmRouteToAmRoute', () => { + describe('when called with overrideGrouping=false', () => { + it('Should not set groupBy', () => { + // Arrange + const route: FormAmRoute = buildFormAmRoute({ id: '1', overrideGrouping: false, groupBy: ['SHOULD NOT BE SET'] }); + + // Act + const amRoute = formAmRouteToAmRoute('test', route, {}); + + // Assert + expect(amRoute.group_by).toStrictEqual([]); + }); + }); + + describe('when called with overrideGrouping=true', () => { + it('Should set groupBy', () => { + // Arrange + const route: FormAmRoute = buildFormAmRoute({ id: '1', overrideGrouping: true, groupBy: ['SHOULD BE SET'] }); + + // Act + const amRoute = formAmRouteToAmRoute('test', route, {}); + + // Assert + expect(amRoute.group_by).toStrictEqual(['SHOULD BE SET']); + }); + }); +}); + +describe('amRouteToFormAmRoute', () => { + describe('when called with empty group_by', () => { + it.each` + group_by + ${[]} + ${null} + ${undefined} + `("when group_by is '$group_by', should set overrideGrouping false", ({ group_by }) => { + // Arrange + const amRoute: Route = buildAmRoute({ group_by: group_by }); + + // Act + const [formRoute] = amRouteToFormAmRoute(amRoute); + + // Assert + expect(formRoute.groupBy).toStrictEqual([]); + expect(formRoute.overrideGrouping).toBe(false); + }); + }); + + describe('when called with non-empty group_by', () => { + it('Should set overrideGrouping true and groupBy', () => { + // Arrange + const amRoute: Route = buildAmRoute({ group_by: ['SHOULD BE SET'] }); + + // Act + const [formRoute] = amRouteToFormAmRoute(amRoute); + + // Assert + expect(formRoute.groupBy).toStrictEqual(['SHOULD BE SET']); + expect(formRoute.overrideGrouping).toBe(true); + }); + }); +}); diff --git a/public/app/features/alerting/unified/utils/amroutes.ts b/public/app/features/alerting/unified/utils/amroutes.ts index 58102b86367..7d5a2bb8e6e 100644 --- a/public/app/features/alerting/unified/utils/amroutes.ts +++ b/public/app/features/alerting/unified/utils/amroutes.ts @@ -61,6 +61,7 @@ export const emptyArrayFieldMatcher: MatcherFieldValue = { export const emptyRoute: FormAmRoute = { id: '', + overrideGrouping: false, groupBy: [], object_matchers: [], routes: [], @@ -114,6 +115,7 @@ export const amRouteToFormAmRoute = (route: Route | undefined): [FormAmRoute, Re ], continue: route.continue ?? false, receiver: route.receiver ?? '', + overrideGrouping: Array.isArray(route.group_by) && route.group_by.length !== 0, groupBy: route.group_by ?? [], overrideTimings: [groupWaitValue, groupIntervalValue, repeatIntervalValue].some(Boolean), groupWaitValue, @@ -137,6 +139,8 @@ export const formAmRouteToAmRoute = ( const existing: Route | undefined = id2ExistingRoute[formAmRoute.id]; const { + overrideGrouping, + groupBy, overrideTimings, groupWaitValue, groupWaitValueType, @@ -146,6 +150,8 @@ export const formAmRouteToAmRoute = ( repeatIntervalValueType, } = formAmRoute; + const group_by = overrideGrouping && groupBy ? groupBy : []; + const overrideGroupWait = overrideTimings && groupWaitValue; const group_wait = overrideGroupWait ? `${groupWaitValue}${groupWaitValueType}` : undefined; @@ -158,7 +164,7 @@ export const formAmRouteToAmRoute = ( const amRoute: Route = { ...(existing ?? {}), continue: formAmRoute.continue, - group_by: formAmRoute.groupBy, + group_by: group_by, object_matchers: formAmRoute.object_matchers.length ? formAmRoute.object_matchers.map((matcher) => [matcher.name, matcher.operator, matcher.value]) : undefined,