From 212bdb44005b0529a99d81a623c1f15801da033a Mon Sep 17 00:00:00 2001 From: Alejandro Fraenkel Date: Thu, 8 Jan 2026 15:56:36 +0100 Subject: [PATCH] fix(alerting): fix AlertmanagerContext error in TimeIntervalsPage - Move useAlertmanager hook call inside AlertmanagerPageWrapper context - Create TimeIntervalsPageContent component that uses the context - Fixes 'useAlertmanager must be used within a AlertmanagerContext' error - Revert incorrect changes to Templates.tsx (error was in TimeIntervalsPage) --- pkg/services/navtree/navtreeimpl/navtree.go | 23 ++- .../app/core/components/Breadcrumbs/utils.ts | 13 +- .../features/alerting/unified/AlertGroups.tsx | 4 +- .../alerting/unified/TimeIntervalsPage.tsx | 18 +- .../navigation/useAlertActivityNav.test.tsx | 181 ++++++++++++++++++ .../unified/navigation/useAlertActivityNav.ts | 93 +++++++++ .../alerting/unified/triage/Triage.tsx | 14 +- 7 files changed, 326 insertions(+), 20 deletions(-) create mode 100644 public/app/features/alerting/unified/navigation/useAlertActivityNav.test.tsx create mode 100644 public/app/features/alerting/unified/navigation/useAlertActivityNav.ts diff --git a/pkg/services/navtree/navtreeimpl/navtree.go b/pkg/services/navtree/navtreeimpl/navtree.go index a5c57cbbe93..cec97115b11 100644 --- a/pkg/services/navtree/navtreeimpl/navtree.go +++ b/pkg/services/navtree/navtreeimpl/navtree.go @@ -559,12 +559,31 @@ func (s *ServiceImpl) buildAlertNavLinksV2(c *contextmodel.ReqContext) *navtree. hasAccess := ac.HasAccess(s.accessControl, c) var alertChildNavs []*navtree.NavLink - // 1. Alert activity (renamed from "Alerts") + // 1. Alert activity (parent with tabs: Alerts, Active notifications) //nolint:staticcheck // not yet migrated to OpenFeature + var alertActivityChildren []*navtree.NavLink if s.features.IsEnabled(c.Req.Context(), featuremgmt.FlagAlertingTriage) { + // Alerts tab if hasAccess(ac.EvalAny(ac.EvalPermission(ac.ActionAlertingRuleRead), ac.EvalPermission(ac.ActionAlertingRuleExternalRead))) { + alertActivityChildren = append(alertActivityChildren, &navtree.NavLink{ + Text: "Alerts", SubTitle: "Visualize active and pending alerts", Id: "alert-activity-alerts", Url: s.cfg.AppSubURL + "/alerting/alerts", Icon: "bell", + }) + } + // Active notifications tab + if hasAccess(ac.EvalAny(ac.EvalPermission(ac.ActionAlertingInstanceRead), ac.EvalPermission(ac.ActionAlertingInstancesExternalRead))) { + alertActivityChildren = append(alertActivityChildren, &navtree.NavLink{ + Text: "Active notifications", SubTitle: "See grouped alerts with active notifications", Id: "alert-activity-groups", Url: s.cfg.AppSubURL + "/alerting/groups", Icon: "layer-group", + }) + } + if len(alertActivityChildren) > 0 { alertChildNavs = append(alertChildNavs, &navtree.NavLink{ - Text: "Alert activity", SubTitle: "Visualize active and pending alerts", Id: "alert-activity", Url: s.cfg.AppSubURL + "/alerting/alerts", Icon: "bell", IsNew: true, + Text: "Alert activity", + SubTitle: "Visualize active and pending alerts", + Id: "alert-activity", + Url: s.cfg.AppSubURL + "/alerting/alerts", + Icon: "bell", + IsNew: true, + Children: alertActivityChildren, }) } } diff --git a/public/app/core/components/Breadcrumbs/utils.ts b/public/app/core/components/Breadcrumbs/utils.ts index e46b37bb964..b4e46f8304f 100644 --- a/public/app/core/components/Breadcrumbs/utils.ts +++ b/public/app/core/components/Breadcrumbs/utils.ts @@ -35,11 +35,18 @@ export function buildBreadcrumbs(sectionNav: NavModelItem, pageNav?: NavModelIte if (shouldAddCrumb) { const activeChildIndex = node.children?.findIndex((child) => child.active) ?? -1; - // Add tab to breadcrumbs if it's not the first active child - if (activeChildIndex > 0) { + // Add active tab to breadcrumbs if it exists and its URL is different from the node's URL + // This ensures tabs show in breadcrumbs (including the first tab) while preventing duplication + if (activeChildIndex >= 0) { const activeChild = node.children?.[activeChildIndex]; if (activeChild) { - crumbs.unshift({ text: activeChild.text, href: activeChild.url ?? '' }); + // Only add the active child if its URL doesn't match the node's URL + // This prevents duplication when the pageNav is the active tab + const nodeUrl = node.url?.split('?')[0] ?? ''; + const childUrl = activeChild.url?.split('?')[0] ?? ''; + if (nodeUrl !== childUrl) { + crumbs.unshift({ text: activeChild.text, href: activeChild.url ?? '' }); + } } } crumbs.unshift({ text: node.text, href: node.url ?? '' }); diff --git a/public/app/features/alerting/unified/AlertGroups.tsx b/public/app/features/alerting/unified/AlertGroups.tsx index 0ca50b2c7c9..2cea9145320 100644 --- a/public/app/features/alerting/unified/AlertGroups.tsx +++ b/public/app/features/alerting/unified/AlertGroups.tsx @@ -14,6 +14,7 @@ import { AlertGroupFilter } from './components/alert-groups/AlertGroupFilter'; import { useFilteredAmGroups } from './hooks/useFilteredAmGroups'; import { useGroupedAlerts } from './hooks/useGroupedAlerts'; import { useUnifiedAlertingSelector } from './hooks/useUnifiedAlertingSelector'; +import { useAlertActivityNav } from './navigation/useAlertActivityNav'; import { useAlertmanager } from './state/AlertmanagerContext'; import { fetchAlertGroupsAction } from './state/actions'; import { NOTIFICATIONS_POLL_INTERVAL_MS } from './utils/constants'; @@ -113,8 +114,9 @@ const AlertGroups = () => { }; function AlertGroupsPage() { + const { navId, pageNav } = useAlertActivityNav(); return ( - + ); diff --git a/public/app/features/alerting/unified/TimeIntervalsPage.tsx b/public/app/features/alerting/unified/TimeIntervalsPage.tsx index 8a1b53398a0..319f96a0ac9 100644 --- a/public/app/features/alerting/unified/TimeIntervalsPage.tsx +++ b/public/app/features/alerting/unified/TimeIntervalsPage.tsx @@ -6,17 +6,29 @@ import { useNotificationConfigNav } from './navigation/useNotificationConfigNav' import { useAlertmanager } from './state/AlertmanagerContext'; import { withPageErrorBoundary } from './withPageErrorBoundary'; +// Content component that uses AlertmanagerContext +// This must be rendered within AlertmanagerPageWrapper +function TimeIntervalsPageContent() { + const { selectedAlertmanager } = useAlertmanager(); + + return ( + <> + + + + ); +} + function TimeIntervalsPage() { const useV2Nav = shouldUseAlertingNavigationV2(); const { navId, pageNav } = useNotificationConfigNav(); - const { selectedAlertmanager } = useAlertmanager(); // In V2 mode, wrap with page wrapper for proper navigation + // AlertmanagerPageWrapper provides AlertmanagerContext, so TimeIntervalsPageContent can use useAlertmanager if (useV2Nav) { return ( - - + ); } diff --git a/public/app/features/alerting/unified/navigation/useAlertActivityNav.test.tsx b/public/app/features/alerting/unified/navigation/useAlertActivityNav.test.tsx new file mode 100644 index 00000000000..87bb406b936 --- /dev/null +++ b/public/app/features/alerting/unified/navigation/useAlertActivityNav.test.tsx @@ -0,0 +1,181 @@ +import { renderHook } from '@testing-library/react'; +import { getWrapper } from 'test/test-utils'; + +import { config } from '@grafana/runtime'; + +import { useAlertActivityNav } from './useAlertActivityNav'; + +describe('useAlertActivityNav', () => { + const mockNavIndex = { + 'alert-activity': { + id: 'alert-activity', + text: 'Alert activity', + url: '/alerting/alerts', + }, + 'alert-activity-alerts': { + id: 'alert-activity-alerts', + text: 'Alerts', + url: '/alerting/alerts', + }, + 'alert-activity-groups': { + id: 'alert-activity-groups', + text: 'Active notifications', + url: '/alerting/groups', + }, + groups: { + id: 'groups', + text: 'Alert groups', + url: '/alerting/groups', + }, + 'alert-alerts': { + id: 'alert-alerts', + text: 'Alerts', + url: '/alerting/alerts', + }, + }; + + const defaultPreloadedState = { + navIndex: mockNavIndex, + }; + + beforeEach(() => { + config.featureToggles.alertingNavigationV2 = false; + }); + + it('should return legacy navId when feature flag is off for /alerting/groups', () => { + const wrapper = getWrapper({ + preloadedState: defaultPreloadedState, + renderWithRouter: true, + historyOptions: { + initialEntries: ['/alerting/groups'], + }, + }); + + const { result } = renderHook(() => useAlertActivityNav(), { wrapper }); + + expect(result.current.navId).toBe('groups'); + expect(result.current.pageNav).toBeUndefined(); + }); + + it('should return legacy navId when feature flag is off for /alerting/alerts', () => { + const wrapper = getWrapper({ + preloadedState: defaultPreloadedState, + renderWithRouter: true, + historyOptions: { + initialEntries: ['/alerting/alerts'], + }, + }); + + const { result } = renderHook(() => useAlertActivityNav(), { wrapper }); + + expect(result.current.navId).toBe('alert-alerts'); + expect(result.current.pageNav).toBeUndefined(); + }); + + it('should return V2 navigation when feature flag is on for Alerts tab', () => { + config.featureToggles.alertingNavigationV2 = true; + const wrapper = getWrapper({ + preloadedState: defaultPreloadedState, + renderWithRouter: true, + historyOptions: { + initialEntries: ['/alerting/alerts'], + }, + }); + + const { result } = renderHook(() => useAlertActivityNav(), { wrapper }); + + expect(result.current.navId).toBe('alert-activity'); + expect(result.current.pageNav).toBeDefined(); + // eslint-disable-next-line testing-library/no-node-access + expect(result.current.pageNav?.children).toBeDefined(); + // The pageNav should represent Alert Activity (not the active tab) for consistent title + expect(result.current.pageNav?.text).toBe('Alert activity'); + }); + + it('should return V2 navigation when feature flag is on for Active notifications tab', () => { + config.featureToggles.alertingNavigationV2 = true; + const wrapper = getWrapper({ + preloadedState: defaultPreloadedState, + renderWithRouter: true, + historyOptions: { + initialEntries: ['/alerting/groups'], + }, + }); + + const { result } = renderHook(() => useAlertActivityNav(), { wrapper }); + + expect(result.current.navId).toBe('alert-activity'); + expect(result.current.pageNav).toBeDefined(); + // eslint-disable-next-line testing-library/no-node-access + expect(result.current.pageNav?.children).toBeDefined(); + // The pageNav should represent Alert Activity (not the active tab) for consistent title + expect(result.current.pageNav?.text).toBe('Alert activity'); + }); + + it('should set active tab based on current path', () => { + config.featureToggles.alertingNavigationV2 = true; + const wrapper = getWrapper({ + preloadedState: defaultPreloadedState, + renderWithRouter: true, + historyOptions: { + initialEntries: ['/alerting/groups'], + }, + }); + + const { result } = renderHook(() => useAlertActivityNav(), { wrapper }); + + // eslint-disable-next-line testing-library/no-node-access + const activeNotificationsTab = result.current.pageNav?.children?.find((tab) => tab.id === 'alert-activity-groups'); + expect(activeNotificationsTab?.active).toBe(true); + + // eslint-disable-next-line testing-library/no-node-access + const alertsTab = result.current.pageNav?.children?.find((tab) => tab.id === 'alert-activity-alerts'); + expect(alertsTab?.active).toBe(false); + }); + + it('should filter tabs based on permissions', () => { + config.featureToggles.alertingNavigationV2 = true; + const limitedNavIndex = { + 'alert-activity': mockNavIndex['alert-activity'], + 'alert-activity-alerts': mockNavIndex['alert-activity-alerts'], + // Missing 'alert-activity-groups' - user doesn't have permission + }; + const wrapper = getWrapper({ + preloadedState: { + navIndex: limitedNavIndex, + }, + renderWithRouter: true, + historyOptions: { + initialEntries: ['/alerting/alerts'], + }, + }); + + const { result } = renderHook(() => useAlertActivityNav(), { wrapper }); + + // eslint-disable-next-line testing-library/no-node-access + expect(result.current.pageNav?.children?.length).toBe(1); + // eslint-disable-next-line testing-library/no-node-access + expect(result.current.pageNav?.children?.[0].id).toBe('alert-activity-alerts'); + }); + + it('should fallback to legacy when alert-activity nav is missing', () => { + config.featureToggles.alertingNavigationV2 = true; + const wrapper = getWrapper({ + preloadedState: { + navIndex: { + groups: mockNavIndex.groups, + 'alert-alerts': mockNavIndex['alert-alerts'], + }, + }, + renderWithRouter: true, + historyOptions: { + initialEntries: ['/alerting/groups'], + }, + }); + + const { result } = renderHook(() => useAlertActivityNav(), { wrapper }); + + expect(result.current.navId).toBe('groups'); + expect(result.current.pageNav).toBeUndefined(); + }); +}); diff --git a/public/app/features/alerting/unified/navigation/useAlertActivityNav.ts b/public/app/features/alerting/unified/navigation/useAlertActivityNav.ts new file mode 100644 index 00000000000..cfd84d11346 --- /dev/null +++ b/public/app/features/alerting/unified/navigation/useAlertActivityNav.ts @@ -0,0 +1,93 @@ +import { useLocation } from 'react-router-dom-v5-compat'; + +import { NavModelItem } from '@grafana/data'; +import { t } from '@grafana/i18n'; +import { useSelector } from 'app/types/store'; + +import { shouldUseAlertingNavigationV2 } from '../featureToggles'; + +export function useAlertActivityNav() { + const location = useLocation(); + const navIndex = useSelector((state) => state.navIndex); + const useV2Nav = shouldUseAlertingNavigationV2(); + + // If V2 navigation is not enabled, return legacy navId + if (!useV2Nav) { + if (location.pathname === '/alerting/groups') { + return { + navId: 'groups', + pageNav: undefined, + }; + } + if (location.pathname === '/alerting/alerts') { + return { + navId: 'alert-alerts', + pageNav: undefined, + }; + } + return { + navId: undefined, + pageNav: undefined, + }; + } + + const alertActivityNav = navIndex['alert-activity']; + if (!alertActivityNav) { + // Fallback to legacy + if (location.pathname === '/alerting/groups') { + return { + navId: 'groups', + pageNav: undefined, + }; + } + if (location.pathname === '/alerting/alerts') { + return { + navId: 'alert-alerts', + pageNav: undefined, + }; + } + return { + navId: undefined, + pageNav: undefined, + }; + } + + // All available tabs + const allTabs = [ + { + id: 'alert-activity-alerts', + text: t('alerting.navigation.alerts', 'Alerts'), + url: '/alerting/alerts', + active: location.pathname === '/alerting/alerts', + icon: 'bell', + parentItem: alertActivityNav, + }, + { + id: 'alert-activity-groups', + text: t('alerting.navigation.active-notifications', 'Active notifications'), + url: '/alerting/groups', + active: location.pathname === '/alerting/groups', + icon: 'layer-group', + parentItem: alertActivityNav, + }, + ].filter((tab) => { + // Filter based on permissions - if nav item doesn't exist, user doesn't have permission + const navItem = navIndex[tab.id]; + return navItem !== undefined; + }); + + // Create pageNav structure following the same pattern as useNotificationConfigNav + // Keep "Alert Activity" as the pageNav (not the active tab) so the title and subtitle stay consistent + // The tabs are children, and the breadcrumb utility will add the active tab to breadcrumbs + // (including the first tab, after our fix to the breadcrumb utility) + const pageNav: NavModelItem = { + ...alertActivityNav, + // eslint-disable-next-line @typescript-eslint/consistent-type-assertions + children: allTabs as NavModelItem[], + }; + + return { + navId: 'alert-activity', + pageNav, + }; +} diff --git a/public/app/features/alerting/unified/triage/Triage.tsx b/public/app/features/alerting/unified/triage/Triage.tsx index 90afe238e06..dea96b7d3d8 100644 --- a/public/app/features/alerting/unified/triage/Triage.tsx +++ b/public/app/features/alerting/unified/triage/Triage.tsx @@ -3,23 +3,15 @@ import { UrlSyncContextProvider } from '@grafana/scenes'; import { withErrorBoundary } from '@grafana/ui'; import { AlertingPageWrapper } from '../components/AlertingPageWrapper'; -import { shouldUseAlertingNavigationV2 } from '../featureToggles'; +import { useAlertActivityNav } from '../navigation/useAlertActivityNav'; import { TriageScene, triageScene } from './scene/TriageScene'; export const TriagePage = () => { - const useV2Nav = shouldUseAlertingNavigationV2(); - const navId = useV2Nav ? 'alert-activity' : 'alert-alerts'; + const { navId, pageNav } = useAlertActivityNav(); return ( - t('alerting.pages.triage.title', 'Alert Activity')} - > +