From 1886a59a12a64878d270676cddfa7d1dbf230318 Mon Sep 17 00:00:00 2001 From: Konrad Lalik Date: Wed, 16 Feb 2022 11:03:53 +0100 Subject: [PATCH] Fix notification routes editing when filters are applied (#45380) * Fix route editing when filters are applied * Fix route delete operation, reset expanded item when filters change * Refactor edit and delete functions, add tests * Fix comment --- .../components/amroutes/AmRoutesTable.test.ts | 78 ++++++++++++++++++- .../components/amroutes/AmRoutesTable.tsx | 45 ++++++----- 2 files changed, 103 insertions(+), 20 deletions(-) diff --git a/public/app/features/alerting/unified/components/amroutes/AmRoutesTable.test.ts b/public/app/features/alerting/unified/components/amroutes/AmRoutesTable.test.ts index af54659548d..a7fcee29adf 100644 --- a/public/app/features/alerting/unified/components/amroutes/AmRoutesTable.test.ts +++ b/public/app/features/alerting/unified/components/amroutes/AmRoutesTable.test.ts @@ -1,7 +1,7 @@ import { MatcherOperator } from 'app/plugins/datasource/alertmanager/types'; import { FormAmRoute } from '../../types/amroutes'; import { MatcherFieldValue } from '../../types/silence-form'; -import { getFilteredRoutes } from './AmRoutesTable'; +import { deleteRoute, getFilteredRoutes, updatedRoute } from './AmRoutesTable'; const defaultAmRoute: FormAmRoute = { id: '', @@ -110,3 +110,79 @@ describe('getFilteredRoutes', () => { expect(filteredRoutes).toContain(routes[2]); }); }); + +describe('updatedRoute', () => { + it('Should update an item of the same id', () => { + // Arrange + const routes: FormAmRoute[] = [buildAmRoute({ id: '1' }), buildAmRoute({ id: '2' }), buildAmRoute({ id: '3' })]; + + const routeUpdate: FormAmRoute = { + ...routes[1], + object_matchers: [buildMatcher('severity', 'critical', MatcherOperator.equal)], + }; + + // Act + const updatedRoutes = updatedRoute(routes, routeUpdate); + + // Assert + expect(updatedRoutes).toHaveLength(3); + const changedRoute = updatedRoutes[1]; + + expect(changedRoute.object_matchers).toHaveLength(1); + expect(changedRoute.object_matchers[0].name).toBe('severity'); + expect(changedRoute.object_matchers[0].value).toBe('critical'); + expect(changedRoute.object_matchers[0].operator).toBe(MatcherOperator.equal); + }); + + it('Should not update any element when an element of matching id not found', () => { + // Arrange + const routes: FormAmRoute[] = [buildAmRoute({ id: '1' }), buildAmRoute({ id: '2' }), buildAmRoute({ id: '3' })]; + + const routeUpdate: FormAmRoute = { + ...routes[1], + id: '-1', + object_matchers: [buildMatcher('severity', 'critical', MatcherOperator.equal)], + }; + + // Act + const updatedRoutes = updatedRoute(routes, routeUpdate); + + // Assert + expect(updatedRoutes).toHaveLength(3); + + updatedRoutes.forEach((route) => { + expect(route.object_matchers).toHaveLength(0); + }); + }); +}); + +describe('deleteRoute', () => { + it('Should delete an element of the same id', () => { + // Arrange + const routes: FormAmRoute[] = [buildAmRoute({ id: '1' }), buildAmRoute({ id: '2' }), buildAmRoute({ id: '3' })]; + + const routeToDelete = routes[1]; + + // Act + const updatedRoutes = deleteRoute(routes, routeToDelete); + + // Assert + expect(updatedRoutes).toHaveLength(2); + expect(updatedRoutes[0].id).toBe('1'); + expect(updatedRoutes[1].id).toBe('3'); + }); + + it('Should not delete anything when an element of matching id not found', () => { + // Arrange + const routes: FormAmRoute[] = [buildAmRoute({ id: '1' }), buildAmRoute({ id: '2' }), buildAmRoute({ id: '3' })]; + + // Act + const updatedRoutes = deleteRoute(routes, buildAmRoute({ id: '-1' })); + + // Assert + expect(updatedRoutes).toHaveLength(3); + expect(updatedRoutes[0].id).toBe('1'); + expect(updatedRoutes[1].id).toBe('2'); + expect(updatedRoutes[2].id).toBe('3'); + }); +}); diff --git a/public/app/features/alerting/unified/components/amroutes/AmRoutesTable.tsx b/public/app/features/alerting/unified/components/amroutes/AmRoutesTable.tsx index b76807394e8..e5cb1df5d46 100644 --- a/public/app/features/alerting/unified/components/amroutes/AmRoutesTable.tsx +++ b/public/app/features/alerting/unified/components/amroutes/AmRoutesTable.tsx @@ -44,6 +44,23 @@ export const getFilteredRoutes = (routes: FormAmRoute[], labelMatcherQuery?: str return filteredRoutes; }; +export const updatedRoute = (routes: FormAmRoute[], updatedRoute: FormAmRoute): FormAmRoute[] => { + const newRoutes = [...routes]; + const editIndex = newRoutes.findIndex((route) => route.id === updatedRoute.id); + + if (editIndex >= 0) { + newRoutes[editIndex] = { + ...newRoutes[editIndex], + ...updatedRoute, + }; + } + return newRoutes; +}; + +export const deleteRoute = (routes: FormAmRoute[], routeToRemove: FormAmRoute): FormAmRoute[] => { + return routes.filter((route) => route.id !== routeToRemove.id); +}; + export const AmRoutesTable: FC = ({ isAddMode, onCancelAdd, @@ -92,7 +109,7 @@ export const AmRoutesTable: FC = ({ id: 'actions', label: 'Actions', // eslint-disable-next-line react/display-name - renderCell: (item, index) => { + renderCell: (item) => { if (item.renderExpandedContent) { return null; } @@ -118,10 +135,7 @@ export const AmRoutesTable: FC = ({ aria-label="Delete route" name="trash-alt" onClick={() => { - const newRoutes = [...routes]; - - newRoutes.splice(index, 1); - + const newRoutes = deleteRoute(routes, item.data); onChange(newRoutes); }} type="button" @@ -144,11 +158,14 @@ export const AmRoutesTable: FC = ({ [isAddMode, routes, filteredRoutes] ); - // expand the last item when adding + // expand the last item when adding or reset when the length changed useEffect(() => { if (isAddMode && dynamicTableRoutes.length) { setExpandedId(dynamicTableRoutes[dynamicTableRoutes.length - 1].id); } + if (!isAddMode && dynamicTableRoutes.length) { + setExpandedId(undefined); + } }, [isAddMode, dynamicTableRoutes]); if (routes.length > 0 && filteredRoutes.length === 0) { @@ -168,7 +185,7 @@ export const AmRoutesTable: FC = ({ onCollapse={collapseItem} onExpand={expandItem} isExpanded={(item) => expandedId === item.id} - renderExpandedContent={(item: RouteTableItemProps, index) => + renderExpandedContent={(item: RouteTableItemProps) => isAddMode || editMode ? ( { @@ -178,12 +195,8 @@ export const AmRoutesTable: FC = ({ setEditMode(false); }} onSave={(data) => { - const newRoutes = [...routes]; + const newRoutes = updatedRoute(routes, data); - newRoutes[index] = { - ...newRoutes[index], - ...data, - }; setEditMode(false); onChange(newRoutes); }} @@ -193,13 +206,7 @@ export const AmRoutesTable: FC = ({ ) : ( { - const newRoutes = [...routes]; - - newRoutes[index] = { - ...item.data, - ...data, - }; - + const newRoutes = updatedRoute(routes, data); onChange(newRoutes); }} receivers={receivers}