From 380ce19b1a7c710e2c312d326a8ee82f74f266aa Mon Sep 17 00:00:00 2001 From: Alex Khomenko Date: Wed, 9 Oct 2024 13:56:54 +0300 Subject: [PATCH] Routing: Remove nested silences routes (#94385) * Update SilencesTable * Remove nested routes * Update tests * Update component names * Update test * Remove unused code --- public/app/features/alerting/routes.tsx | 12 ++- .../alerting/unified/NewSilencePage.tsx | 51 +++++++++++++ .../alerting/unified/Silences.test.tsx | 47 +++++------- .../features/alerting/unified/Silences.tsx | 74 ------------------- .../components/silences/SilencesEditor.tsx | 30 ++++++-- .../components/silences/SilencesTable.tsx | 21 ++++-- .../unified/hooks/useSilenceNavData.test.tsx | 40 ---------- .../unified/hooks/useSilenceNavData.ts | 34 --------- 8 files changed, 119 insertions(+), 190 deletions(-) create mode 100644 public/app/features/alerting/unified/NewSilencePage.tsx delete mode 100644 public/app/features/alerting/unified/Silences.tsx delete mode 100644 public/app/features/alerting/unified/hooks/useSilenceNavData.test.tsx delete mode 100644 public/app/features/alerting/unified/hooks/useSilenceNavData.ts diff --git a/public/app/features/alerting/routes.tsx b/public/app/features/alerting/routes.tsx index ee8fe2bbb17..da52a55447f 100644 --- a/public/app/features/alerting/routes.tsx +++ b/public/app/features/alerting/routes.tsx @@ -72,7 +72,10 @@ export function getAlertingRoutes(cfg = config): RouteDescriptor[] { AccessControlAction.AlertingSilenceRead, ]), component: importAlertingComponent( - () => import(/* webpackChunkName: "AlertSilences" */ 'app/features/alerting/unified/Silences') + () => + import( + /* webpackChunkName: "SilencesTablePage" */ 'app/features/alerting/unified/components/silences/SilencesTable' + ) ), }, { @@ -84,13 +87,16 @@ export function getAlertingRoutes(cfg = config): RouteDescriptor[] { AccessControlAction.AlertingSilenceUpdate, ]), component: importAlertingComponent( - () => import(/* webpackChunkName: "AlertSilences" */ 'app/features/alerting/unified/Silences') + () => import(/* webpackChunkName: "NewSilencePage" */ 'app/features/alerting/unified/NewSilencePage') ), }, { path: '/alerting/silence/:id/edit', component: importAlertingComponent( - () => import(/* webpackChunkName: "AlertSilences" */ 'app/features/alerting/unified/Silences') + () => + import( + /* webpackChunkName: "ExistingSilenceEditorPage" */ 'app/features/alerting/unified/components/silences/SilencesEditor' + ) ), }, { diff --git a/public/app/features/alerting/unified/NewSilencePage.tsx b/public/app/features/alerting/unified/NewSilencePage.tsx new file mode 100644 index 00000000000..859b9c3875e --- /dev/null +++ b/public/app/features/alerting/unified/NewSilencePage.tsx @@ -0,0 +1,51 @@ +import { useLocation } from 'react-router-dom-v5-compat'; + +import { withErrorBoundary } from '@grafana/ui'; +import { + defaultsFromQuery, + getDefaultSilenceFormValues, +} from 'app/features/alerting/unified/components/silences/utils'; +import { MATCHER_ALERT_RULE_UID } from 'app/features/alerting/unified/utils/constants'; +import { parseQueryParamMatchers } from 'app/features/alerting/unified/utils/matchers'; + +import { AlertmanagerPageWrapper } from './components/AlertingPageWrapper'; +import { GrafanaAlertmanagerDeliveryWarning } from './components/GrafanaAlertmanagerDeliveryWarning'; +import { SilencesEditor } from './components/silences/SilencesEditor'; +import { useAlertmanager } from './state/AlertmanagerContext'; + +const SilencesEditorComponent = () => { + const location = useLocation(); + const queryParams = new URLSearchParams(location.search); + const { selectedAlertmanager = '' } = useAlertmanager(); + const potentialAlertRuleMatcher = parseQueryParamMatchers(queryParams.getAll('matcher')).find( + (m) => m.name === MATCHER_ALERT_RULE_UID + ); + + const potentialRuleUid = potentialAlertRuleMatcher?.value; + const formValues = getDefaultSilenceFormValues(defaultsFromQuery(queryParams)); + + return ( + <> + + + + ); +}; + +function NewSilencePage() { + const pageNav = { + id: 'silence-new', + text: 'Silence alert rule', + subTitle: 'Configure silences to stop notifications from a particular alert rule', + }; + return ( + + + + ); +} +export default withErrorBoundary(NewSilencePage, { style: 'page' }); diff --git a/public/app/features/alerting/unified/Silences.test.tsx b/public/app/features/alerting/unified/Silences.test.tsx index 009f39a8b7c..e2b2f134ca2 100644 --- a/public/app/features/alerting/unified/Silences.test.tsx +++ b/public/app/features/alerting/unified/Silences.test.tsx @@ -1,4 +1,4 @@ -import { useParams } from 'react-router-dom-v5-compat'; +import { Route, Routes } from 'react-router-dom-v5-compat'; import { render, screen, userEvent, waitFor, within } from 'test/test-utils'; import { byLabelText, byPlaceholderText, byRole, byTestId, byText } from 'testing-library-selector'; @@ -17,7 +17,9 @@ import { MATCHER_ALERT_RULE_UID } from 'app/features/alerting/unified/utils/cons import { MatcherOperator, SilenceState } from 'app/plugins/datasource/alertmanager/types'; import { AccessControlAction } from 'app/types'; -import Silences from './Silences'; +import NewSilencePage from './NewSilencePage'; +import ExistingSilenceEditorPage from './components/silences/SilencesEditor'; +import SilencesTablePage from './components/silences/SilencesTable'; import { MOCK_SILENCE_ID_EXISTING, MOCK_SILENCE_ID_EXISTING_ALERT_RULE_UID, @@ -30,21 +32,21 @@ import { grafanaRulerRule } from './mocks/grafanaRulerApi'; import { setupDataSources } from './testSetup/datasources'; import { DataSourceType, GRAFANA_RULES_SOURCE_NAME } from './utils/datasource'; -jest.mock('app/core/services/context_srv'); - -jest.mock('react-router-dom-v5-compat', () => ({ - ...jest.requireActual('react-router-dom-v5-compat'), - useParams: jest.fn(), -})); - const TEST_TIMEOUT = 60000; const renderSilences = (location = '/alerting/silences/') => { - return render(, { - historyOptions: { - initialEntries: [location], - }, - }); + return render( + + } /> + } /> + } /> + , + { + historyOptions: { + initialEntries: [location], + }, + } + ); }; const dataSources = { @@ -124,8 +126,7 @@ describe('Silences', () => { it( 'loads and shows silences', async () => { - const user = userEvent.setup(); - renderSilences(); + const { user } = renderSilences(); expect(await ui.notExpiredTable.find()).toBeInTheDocument(); @@ -174,8 +175,7 @@ describe('Silences', () => { it( 'filters silences by matchers', async () => { - const user = userEvent.setup(); - renderSilences(); + const { user } = renderSilences(); const queryBar = await ui.queryBar.find(); await user.type(queryBar, 'foo=bar'); @@ -260,8 +260,7 @@ describe('Silence create/edit', () => { it( 'creates a new silence', async () => { - const user = userEvent.setup(); - renderSilences(`${baseUrlPath}?alertmanager=${GRAFANA_RULES_SOURCE_NAME}`); + const { user } = renderSilences(`${baseUrlPath}?alertmanager=${GRAFANA_RULES_SOURCE_NAME}`); expect(await ui.editor.durationField.find()).toBeInTheDocument(); const postRequest = waitForServerRequest(silenceCreateHandler()); @@ -320,20 +319,17 @@ describe('Silence create/edit', () => { }); it('shows an error when existing silence cannot be found', async () => { - (useParams as jest.Mock).mockReturnValue({ id: 'foo-bar' }); renderSilences('/alerting/silence/foo-bar/edit'); expect(await ui.existingSilenceNotFound.find()).toBeInTheDocument(); }); it('shows an error when user cannot edit/recreate silence', async () => { - (useParams as jest.Mock).mockReturnValue({ id: MOCK_SILENCE_ID_LACKING_PERMISSIONS }); renderSilences(`/alerting/silence/${MOCK_SILENCE_ID_LACKING_PERMISSIONS}/edit`); expect(await ui.noPermissionToEdit.find()).toBeInTheDocument(); }); it('populates form with existing silence information', async () => { - (useParams as jest.Mock).mockReturnValue({ id: MOCK_SILENCE_ID_EXISTING }); renderSilences(`/alerting/silence/${MOCK_SILENCE_ID_EXISTING}/edit`); // Await the first value to be populated, after which we can expect that all of the other @@ -344,7 +340,6 @@ describe('Silence create/edit', () => { }); it('populates form with existing silence information that has __alert_rule_uid__', async () => { - (useParams as jest.Mock).mockReturnValue({ id: MOCK_SILENCE_ID_EXISTING_ALERT_RULE_UID }); mockAlertRuleApi(server).getAlertRule(MOCK_SILENCE_ID_EXISTING_ALERT_RULE_UID, grafanaRulerRule); renderSilences(`/alerting/silence/${MOCK_SILENCE_ID_EXISTING_ALERT_RULE_UID}/edit`); expect(await screen.findByLabelText(/alert rule/i)).toHaveValue(grafanaRulerRule.grafana_alert.title); @@ -358,11 +353,9 @@ describe('Silence create/edit', () => { it( 'silences page should contain alertmanager parameter after creating a silence', async () => { - const user = userEvent.setup(); - const postRequest = waitForServerRequest(silenceCreateHandler()); - renderSilences(`${baseUrlPath}?alertmanager=${GRAFANA_RULES_SOURCE_NAME}`); + const { user } = renderSilences(`${baseUrlPath}?alertmanager=${GRAFANA_RULES_SOURCE_NAME}`); await waitFor(() => expect(ui.editor.durationField.query()).not.toBeNull()); await enterSilenceLabel(0, 'foo', MatcherOperator.equal, 'bar'); diff --git a/public/app/features/alerting/unified/Silences.tsx b/public/app/features/alerting/unified/Silences.tsx deleted file mode 100644 index bc257e65a2e..00000000000 --- a/public/app/features/alerting/unified/Silences.tsx +++ /dev/null @@ -1,74 +0,0 @@ -import { Route, Switch } from 'react-router-dom'; -import { useLocation } from 'react-router-dom-v5-compat'; - -import { withErrorBoundary } from '@grafana/ui'; -import { - defaultsFromQuery, - getDefaultSilenceFormValues, -} from 'app/features/alerting/unified/components/silences/utils'; -import { MATCHER_ALERT_RULE_UID } from 'app/features/alerting/unified/utils/constants'; -import { parseQueryParamMatchers } from 'app/features/alerting/unified/utils/matchers'; - -import { AlertmanagerPageWrapper } from './components/AlertingPageWrapper'; -import { GrafanaAlertmanagerDeliveryWarning } from './components/GrafanaAlertmanagerDeliveryWarning'; -import ExistingSilenceEditor, { SilencesEditor } from './components/silences/SilencesEditor'; -import SilencesTable from './components/silences/SilencesTable'; -import { useSilenceNavData } from './hooks/useSilenceNavData'; -import { useAlertmanager } from './state/AlertmanagerContext'; - -const Silences = () => { - const { selectedAlertmanager } = useAlertmanager(); - - if (!selectedAlertmanager) { - return null; - } - - return ( - <> - - - - - - - - - - - - - - ); -}; - -function SilencesPage() { - const pageNav = useSilenceNavData(); - - return ( - - - - ); -} - -export default withErrorBoundary(SilencesPage, { style: 'page' }); - -type SilencesEditorComponentProps = { - selectedAlertmanager: string; -}; -const SilencesEditorComponent = ({ selectedAlertmanager }: SilencesEditorComponentProps) => { - const location = useLocation(); - const queryParams = new URLSearchParams(location.search); - - const potentialAlertRuleMatcher = parseQueryParamMatchers(queryParams.getAll('matcher')).find( - (m) => m.name === MATCHER_ALERT_RULE_UID - ); - - const potentialRuleUid = potentialAlertRuleMatcher?.value; - - const formValues = getDefaultSilenceFormValues(defaultsFromQuery(queryParams)); - - return ( - - ); -}; diff --git a/public/app/features/alerting/unified/components/silences/SilencesEditor.tsx b/public/app/features/alerting/unified/components/silences/SilencesEditor.tsx index e9ce5661d3c..f4164badfad 100644 --- a/public/app/features/alerting/unified/components/silences/SilencesEditor.tsx +++ b/public/app/features/alerting/unified/components/silences/SilencesEditor.tsx @@ -25,6 +25,7 @@ import { Stack, TextArea, useStyles2, + withErrorBoundary, } from '@grafana/ui'; import { alertSilencesApi, SilenceCreatedResponse } from 'app/features/alerting/unified/api/alertSilencesApi'; import { MATCHER_ALERT_RULE_UID } from 'app/features/alerting/unified/utils/constants'; @@ -32,26 +33,26 @@ import { getDatasourceAPIUid, GRAFANA_RULES_SOURCE_NAME } from 'app/features/ale import { MatcherOperator, SilenceCreatePayload } from 'app/plugins/datasource/alertmanager/types'; import { AlertmanagerAction, useAlertmanagerAbility } from '../../hooks/useAbilities'; +import { useAlertmanager } from '../../state/AlertmanagerContext'; import { SilenceFormFields } from '../../types/silence-form'; import { matcherFieldToMatcher } from '../../utils/alertmanager'; import { makeAMLink } from '../../utils/misc'; +import { AlertmanagerPageWrapper } from '../AlertingPageWrapper'; +import { GrafanaAlertmanagerDeliveryWarning } from '../GrafanaAlertmanagerDeliveryWarning'; import MatchersField from './MatchersField'; import { SilencePeriod } from './SilencePeriod'; import { SilencedInstancesPreview } from './SilencedInstancesPreview'; import { getDefaultSilenceFormValues, getFormFieldsForSilence } from './utils'; -interface Props { - alertManagerSourceName: string; -} - /** * Silences editor for editing an existing silence. * * Fetches silence details from API, based on `silenceId` */ -const ExistingSilenceEditor = ({ alertManagerSourceName }: Props) => { +const ExistingSilenceEditor = () => { const { id: silenceId = '' } = useParams(); + const { selectedAlertmanager: alertManagerSourceName = '' } = useAlertmanager(); const { data: silence, isLoading: getSilenceIsLoading, @@ -91,7 +92,10 @@ const ExistingSilenceEditor = ({ alertManagerSourceName }: Props) => { } return ( - + <> + + + ); }; @@ -279,4 +283,16 @@ const getStyles = (theme: GrafanaTheme2) => ({ }), }); -export default ExistingSilenceEditor; +function ExistingSilenceEditorPage() { + const pageNav = { + id: 'silence-edit', + text: 'Edit silence', + subTitle: 'Recreate existing silence to stop notifications from a particular alert rule', + }; + return ( + + + + ); +} +export default withErrorBoundary(ExistingSilenceEditorPage, { style: 'page' }); diff --git a/public/app/features/alerting/unified/components/silences/SilencesTable.tsx b/public/app/features/alerting/unified/components/silences/SilencesTable.tsx index 01b92427a8e..6eaf819d11a 100644 --- a/public/app/features/alerting/unified/components/silences/SilencesTable.tsx +++ b/public/app/features/alerting/unified/components/silences/SilencesTable.tsx @@ -12,6 +12,7 @@ import { LoadingPlaceholder, Stack, useStyles2, + withErrorBoundary, } from '@grafana/ui'; import { useQueryParams } from 'app/core/hooks/useQueryParams'; import { Trans } from 'app/core/internationalization'; @@ -23,10 +24,13 @@ import { AlertmanagerAlert, Silence, SilenceState } from 'app/plugins/datasource import { alertmanagerApi } from '../../api/alertmanagerApi'; import { AlertmanagerAction, useAlertmanagerAbility } from '../../hooks/useAbilities'; +import { useAlertmanager } from '../../state/AlertmanagerContext'; import { parsePromQLStyleMatcherLooseSafe } from '../../utils/matchers'; import { getSilenceFiltersFromUrlParams, makeAMLink, stringifyErrorLike } from '../../utils/misc'; +import { AlertmanagerPageWrapper } from '../AlertingPageWrapper'; import { Authorize } from '../Authorize'; import { DynamicTable, DynamicTableColumnProps, DynamicTableItemProps } from '../DynamicTable'; +import { GrafanaAlertmanagerDeliveryWarning } from '../GrafanaAlertmanagerDeliveryWarning'; import { Matchers } from './Matchers'; import { NoSilencesSplash } from './NoSilencesCTA'; @@ -40,13 +44,11 @@ export interface SilenceTableItem extends Silence { type SilenceTableColumnProps = DynamicTableColumnProps; type SilenceTableItemProps = DynamicTableItemProps; -interface Props { - alertManagerSourceName: string; -} const API_QUERY_OPTIONS = { pollingInterval: SILENCES_POLL_INTERVAL_MS, refetchOnFocus: true }; -const SilencesTable = ({ alertManagerSourceName }: Props) => { +const SilencesTable = () => { + const { selectedAlertmanager: alertManagerSourceName = '' } = useAlertmanager(); const [previewAlertsSupported, previewAlertsAllowed] = useAlertmanagerAbility( AlertmanagerAction.PreviewSilencedInstances ); @@ -135,6 +137,7 @@ const SilencesTable = ({ alertManagerSourceName }: Props) => { return (
+ {!!silences.length && ( @@ -382,4 +385,12 @@ function useColumns(alertManagerSourceName: string) { return columns; }, [alertManagerSourceName, expireSilence, isGrafanaFlavoredAlertmanager, updateAllowed, updateSupported]); } -export default SilencesTable; + +function SilencesTablePage() { + return ( + + + + ); +} +export default withErrorBoundary(SilencesTablePage, { style: 'page' }); diff --git a/public/app/features/alerting/unified/hooks/useSilenceNavData.test.tsx b/public/app/features/alerting/unified/hooks/useSilenceNavData.test.tsx deleted file mode 100644 index f06c47805d1..00000000000 --- a/public/app/features/alerting/unified/hooks/useSilenceNavData.test.tsx +++ /dev/null @@ -1,40 +0,0 @@ -import { render } from '@testing-library/react'; -import { useMatch } from 'react-router-dom-v5-compat'; - -import { useSilenceNavData } from './useSilenceNavData'; - -jest.mock('react-router-dom-v5-compat', () => ({ - ...jest.requireActual('react-router-dom-v5-compat'), - useMatch: jest.fn(), -})); - -const setup = () => { - let result: ReturnType; - function TestComponent() { - result = useSilenceNavData(); - return null; - } - - render(); - - return { result }; -}; -describe('useSilenceNavData', () => { - it('should return correct nav data when route is "/alerting/silence/new"', () => { - (useMatch as jest.Mock).mockImplementation((param) => param === '/alerting/silence/new'); - const { result } = setup(); - - expect(result).toMatchObject({ - text: 'Silence alert rule', - }); - }); - - it('should return correct nav data when route is "/alerting/silence/:id/edit"', () => { - (useMatch as jest.Mock).mockImplementation((param) => param === '/alerting/silence/:id/edit'); - const { result } = setup(); - - expect(result).toMatchObject({ - text: 'Edit silence', - }); - }); -}); diff --git a/public/app/features/alerting/unified/hooks/useSilenceNavData.ts b/public/app/features/alerting/unified/hooks/useSilenceNavData.ts deleted file mode 100644 index 9df734e8aa2..00000000000 --- a/public/app/features/alerting/unified/hooks/useSilenceNavData.ts +++ /dev/null @@ -1,34 +0,0 @@ -import { useEffect, useState } from 'react'; -import { useMatch } from 'react-router-dom-v5-compat'; - -import { NavModelItem } from '@grafana/data'; - -const defaultPageNav: Partial = { - icon: 'bell-slash', -}; - -export function useSilenceNavData() { - const [pageNav, setPageNav] = useState(); - const isNewPath = useMatch('/alerting/silence/new'); - const isEditPath = useMatch('/alerting/silence/:id/edit'); - - useEffect(() => { - if (isNewPath) { - setPageNav({ - ...defaultPageNav, - id: 'silence-new', - text: 'Silence alert rule', - subTitle: 'Configure silences to stop notifications from a particular alert rule', - }); - } else if (isEditPath) { - setPageNav({ - ...defaultPageNav, - id: 'silence-edit', - text: 'Edit silence', - subTitle: 'Recreate existing silence to stop notifications from a particular alert rule', - }); - } - }, [isEditPath, isNewPath]); - - return pageNav; -}