From a5d240751dc437c58b4a572417a9f6610f752270 Mon Sep 17 00:00:00 2001 From: Alejandro Fraenkel Date: Mon, 12 Jan 2026 14:14:30 +0100 Subject: [PATCH] refactor(alerting): rename V2 nav function to main name for easier future cleanup Make the V2 navigation implementation the main buildAlertNavLinks() function, and keep buildAlertNavLinksLegacy() with its descriptive name. This makes future cleanup trivial: - To remove legacy support: just delete buildAlertNavLinksLegacy() and the feature flag check - No need to rename functions later - Main function already has the desired implementation Changes: - Inline V2 implementation into buildAlertNavLinks() - Delete buildAlertNavLinksV2() function (now redundant) - Update tests to call buildAlertNavLinks() directly - Inverted feature flag check (!enabled instead of enabled) All tests pass with identical coverage. --- pkg/services/navtree/navtreeimpl/navtree.go | 238 +++++++++--------- .../navtreeimpl/navtree_alerting_test.go | 8 +- 2 files changed, 122 insertions(+), 124 deletions(-) diff --git a/pkg/services/navtree/navtreeimpl/navtree.go b/pkg/services/navtree/navtreeimpl/navtree.go index 7b4325d8dec..837cbe3ad81 100644 --- a/pkg/services/navtree/navtreeimpl/navtree.go +++ b/pkg/services/navtree/navtreeimpl/navtree.go @@ -434,128 +434,11 @@ func (s *ServiceImpl) buildDashboardNavLinks(c *contextmodel.ReqContext) []*navt func (s *ServiceImpl) buildAlertNavLinks(c *contextmodel.ReqContext) *navtree.NavLink { //nolint:staticcheck // not yet migrated to OpenFeature - if s.features.IsEnabled(c.Req.Context(), featuremgmt.FlagAlertingNavigationV2) { - return s.buildAlertNavLinksV2(c) - } - return s.buildAlertNavLinksLegacy(c) -} - -func (s *ServiceImpl) buildAlertNavLinksLegacy(c *contextmodel.ReqContext) *navtree.NavLink { - hasAccess := ac.HasAccess(s.accessControl, c) - var alertChildNavs []*navtree.NavLink - - //nolint:staticcheck // not yet migrated to OpenFeature - if s.features.IsEnabled(c.Req.Context(), featuremgmt.FlagAlertingTriage) { - if hasAccess(ac.EvalAny(ac.EvalPermission(ac.ActionAlertingRuleRead), ac.EvalPermission(ac.ActionAlertingRuleExternalRead))) { - alertChildNavs = append(alertChildNavs, &navtree.NavLink{ - Text: "Alert activity", SubTitle: "Visualize active and pending alerts", Id: "alert-alerts", Url: s.cfg.AppSubURL + "/alerting/alerts", Icon: "bell", IsNew: true, - }) - } + if !s.features.IsEnabled(c.Req.Context(), featuremgmt.FlagAlertingNavigationV2) { + return s.buildAlertNavLinksLegacy(c) } - if hasAccess(ac.EvalAny(ac.EvalPermission(ac.ActionAlertingRuleRead), ac.EvalPermission(ac.ActionAlertingRuleExternalRead))) { - alertChildNavs = append(alertChildNavs, &navtree.NavLink{ - Text: "Alert rules", SubTitle: "Rules that determine whether an alert will fire", Id: "alert-list", Url: s.cfg.AppSubURL + "/alerting/list", Icon: "list-ul", - }) - } - - contactPointsPerms := []ac.Evaluator{ - ac.EvalPermission(ac.ActionAlertingNotificationsRead), - ac.EvalPermission(ac.ActionAlertingNotificationsExternalRead), - - ac.EvalPermission(ac.ActionAlertingReceiversRead), - ac.EvalPermission(ac.ActionAlertingReceiversReadSecrets), - ac.EvalPermission(ac.ActionAlertingReceiversCreate), - - ac.EvalPermission(ac.ActionAlertingNotificationsTemplatesRead), - ac.EvalPermission(ac.ActionAlertingNotificationsTemplatesWrite), - ac.EvalPermission(ac.ActionAlertingNotificationsTemplatesDelete), - } - - if hasAccess(ac.EvalAny(contactPointsPerms...)) { - alertChildNavs = append(alertChildNavs, &navtree.NavLink{ - Text: "Contact points", SubTitle: "Choose how to notify your contact points when an alert instance fires", Id: "receivers", Url: s.cfg.AppSubURL + "/alerting/notifications", - Icon: "comment-alt-share", - }) - } - - if hasAccess(ac.EvalAny( - ac.EvalPermission(ac.ActionAlertingNotificationsRead), - ac.EvalPermission(ac.ActionAlertingNotificationsExternalRead), - ac.EvalPermission(ac.ActionAlertingRoutesRead), - ac.EvalPermission(ac.ActionAlertingRoutesWrite), - ac.EvalPermission(ac.ActionAlertingNotificationsTimeIntervalsRead), - ac.EvalPermission(ac.ActionAlertingNotificationsTimeIntervalsWrite), - )) { - alertChildNavs = append(alertChildNavs, &navtree.NavLink{Text: "Notification policies", SubTitle: "Determine how alerts are routed to contact points", Id: "am-routes", Url: s.cfg.AppSubURL + "/alerting/routes", Icon: "sitemap"}) - } - - if hasAccess(ac.EvalAny( - ac.EvalPermission(ac.ActionAlertingInstanceRead), - ac.EvalPermission(ac.ActionAlertingInstancesExternalRead), - ac.EvalPermission(ac.ActionAlertingSilencesRead), - )) { - alertChildNavs = append(alertChildNavs, &navtree.NavLink{Text: "Silences", SubTitle: "Stop notifications from one or more alerting rules", Id: "silences", Url: s.cfg.AppSubURL + "/alerting/silences", Icon: "bell-slash"}) - } - - if hasAccess(ac.EvalAny(ac.EvalPermission(ac.ActionAlertingInstanceRead), ac.EvalPermission(ac.ActionAlertingInstancesExternalRead))) { - alertChildNavs = append(alertChildNavs, &navtree.NavLink{Text: "Alert groups", SubTitle: "See grouped alerts with active notifications", Id: "groups", Url: s.cfg.AppSubURL + "/alerting/groups", Icon: "layer-group"}) - } - - //nolint:staticcheck // not yet migrated to OpenFeature - if s.features.IsEnabled(c.Req.Context(), featuremgmt.FlagAlertingCentralAlertHistory) { - if hasAccess(ac.EvalAny(ac.EvalPermission(ac.ActionAlertingRuleRead))) { - alertChildNavs = append(alertChildNavs, &navtree.NavLink{ - Text: "History", - SubTitle: "View a history of all alert events generated by your Grafana-managed alert rules. All alert events are displayed regardless of whether silences or mute timings are set.", - Id: "alerts-history", - Url: s.cfg.AppSubURL + "/alerting/history", - Icon: "history", - }) - } - } - //nolint:staticcheck // not yet migrated to OpenFeature - if c.GetOrgRole() == org.RoleAdmin && s.features.IsEnabled(c.Req.Context(), featuremgmt.FlagAlertRuleRestore) && s.features.IsEnabled(c.Req.Context(), featuremgmt.FlagAlertingRuleRecoverDeleted) { - alertChildNavs = append(alertChildNavs, &navtree.NavLink{ - Text: "Recently deleted", - SubTitle: "Any items listed here for more than 30 days will be automatically deleted.", - Id: "alerts/recently-deleted", - Url: s.cfg.AppSubURL + "/alerting/recently-deleted", - }) - } - - if c.GetOrgRole() == org.RoleAdmin { - alertChildNavs = append(alertChildNavs, &navtree.NavLink{ - Text: "Settings", Id: "alerting-admin", Url: s.cfg.AppSubURL + "/alerting/admin", - Icon: "cog", - }) - } - - if hasAccess(ac.EvalAny(ac.EvalPermission(ac.ActionAlertingRuleCreate), ac.EvalPermission(ac.ActionAlertingRuleExternalWrite))) { - alertChildNavs = append(alertChildNavs, &navtree.NavLink{ - Text: "Create alert rule", SubTitle: "Create an alert rule", Id: "alert", - Icon: "plus", Url: s.cfg.AppSubURL + "/alerting/new", HideFromTabs: true, IsCreateAction: true, - }) - } - - if len(alertChildNavs) > 0 { - var alertNav = navtree.NavLink{ - Text: "Alerting", - SubTitle: "Learn about problems in your systems moments after they occur", - Id: navtree.NavIDAlerting, - Icon: "bell", - Children: alertChildNavs, - SortWeight: navtree.WeightAlerting, - Url: s.cfg.AppSubURL + "/alerting", - } - - return &alertNav - } - - return nil -} - -func (s *ServiceImpl) buildAlertNavLinksV2(c *contextmodel.ReqContext) *navtree.NavLink { + // V2 Navigation - New grouped structure hasAccess := ac.HasAccess(s.accessControl, c) var alertChildNavs []*navtree.NavLink @@ -757,6 +640,121 @@ func (s *ServiceImpl) buildAlertNavLinksV2(c *contextmodel.ReqContext) *navtree. return nil } +func (s *ServiceImpl) buildAlertNavLinksLegacy(c *contextmodel.ReqContext) *navtree.NavLink { + hasAccess := ac.HasAccess(s.accessControl, c) + var alertChildNavs []*navtree.NavLink + + //nolint:staticcheck // not yet migrated to OpenFeature + if s.features.IsEnabled(c.Req.Context(), featuremgmt.FlagAlertingTriage) { + if hasAccess(ac.EvalAny(ac.EvalPermission(ac.ActionAlertingRuleRead), ac.EvalPermission(ac.ActionAlertingRuleExternalRead))) { + alertChildNavs = append(alertChildNavs, &navtree.NavLink{ + Text: "Alert activity", SubTitle: "Visualize active and pending alerts", Id: "alert-alerts", Url: s.cfg.AppSubURL + "/alerting/alerts", Icon: "bell", IsNew: true, + }) + } + } + + if hasAccess(ac.EvalAny(ac.EvalPermission(ac.ActionAlertingRuleRead), ac.EvalPermission(ac.ActionAlertingRuleExternalRead))) { + alertChildNavs = append(alertChildNavs, &navtree.NavLink{ + Text: "Alert rules", SubTitle: "Rules that determine whether an alert will fire", Id: "alert-list", Url: s.cfg.AppSubURL + "/alerting/list", Icon: "list-ul", + }) + } + + contactPointsPerms := []ac.Evaluator{ + ac.EvalPermission(ac.ActionAlertingNotificationsRead), + ac.EvalPermission(ac.ActionAlertingNotificationsExternalRead), + + ac.EvalPermission(ac.ActionAlertingReceiversRead), + ac.EvalPermission(ac.ActionAlertingReceiversReadSecrets), + ac.EvalPermission(ac.ActionAlertingReceiversCreate), + + ac.EvalPermission(ac.ActionAlertingNotificationsTemplatesRead), + ac.EvalPermission(ac.ActionAlertingNotificationsTemplatesWrite), + ac.EvalPermission(ac.ActionAlertingNotificationsTemplatesDelete), + } + + if hasAccess(ac.EvalAny(contactPointsPerms...)) { + alertChildNavs = append(alertChildNavs, &navtree.NavLink{ + Text: "Contact points", SubTitle: "Choose how to notify your contact points when an alert instance fires", Id: "receivers", Url: s.cfg.AppSubURL + "/alerting/notifications", + Icon: "comment-alt-share", + }) + } + + if hasAccess(ac.EvalAny( + ac.EvalPermission(ac.ActionAlertingNotificationsRead), + ac.EvalPermission(ac.ActionAlertingNotificationsExternalRead), + ac.EvalPermission(ac.ActionAlertingRoutesRead), + ac.EvalPermission(ac.ActionAlertingRoutesWrite), + ac.EvalPermission(ac.ActionAlertingNotificationsTimeIntervalsRead), + ac.EvalPermission(ac.ActionAlertingNotificationsTimeIntervalsWrite), + )) { + alertChildNavs = append(alertChildNavs, &navtree.NavLink{Text: "Notification policies", SubTitle: "Determine how alerts are routed to contact points", Id: "am-routes", Url: s.cfg.AppSubURL + "/alerting/routes", Icon: "sitemap"}) + } + + if hasAccess(ac.EvalAny( + ac.EvalPermission(ac.ActionAlertingInstanceRead), + ac.EvalPermission(ac.ActionAlertingInstancesExternalRead), + ac.EvalPermission(ac.ActionAlertingSilencesRead), + )) { + alertChildNavs = append(alertChildNavs, &navtree.NavLink{Text: "Silences", SubTitle: "Stop notifications from one or more alerting rules", Id: "silences", Url: s.cfg.AppSubURL + "/alerting/silences", Icon: "bell-slash"}) + } + + if hasAccess(ac.EvalAny(ac.EvalPermission(ac.ActionAlertingInstanceRead), ac.EvalPermission(ac.ActionAlertingInstancesExternalRead))) { + alertChildNavs = append(alertChildNavs, &navtree.NavLink{Text: "Alert groups", SubTitle: "See grouped alerts with active notifications", Id: "groups", Url: s.cfg.AppSubURL + "/alerting/groups", Icon: "layer-group"}) + } + + //nolint:staticcheck // not yet migrated to OpenFeature + if s.features.IsEnabled(c.Req.Context(), featuremgmt.FlagAlertingCentralAlertHistory) { + if hasAccess(ac.EvalAny(ac.EvalPermission(ac.ActionAlertingRuleRead))) { + alertChildNavs = append(alertChildNavs, &navtree.NavLink{ + Text: "History", + SubTitle: "View a history of all alert events generated by your Grafana-managed alert rules. All alert events are displayed regardless of whether silences or mute timings are set.", + Id: "alerts-history", + Url: s.cfg.AppSubURL + "/alerting/history", + Icon: "history", + }) + } + } + //nolint:staticcheck // not yet migrated to OpenFeature + if c.GetOrgRole() == org.RoleAdmin && s.features.IsEnabled(c.Req.Context(), featuremgmt.FlagAlertRuleRestore) && s.features.IsEnabled(c.Req.Context(), featuremgmt.FlagAlertingRuleRecoverDeleted) { + alertChildNavs = append(alertChildNavs, &navtree.NavLink{ + Text: "Recently deleted", + SubTitle: "Any items listed here for more than 30 days will be automatically deleted.", + Id: "alerts/recently-deleted", + Url: s.cfg.AppSubURL + "/alerting/recently-deleted", + }) + } + + if c.GetOrgRole() == org.RoleAdmin { + alertChildNavs = append(alertChildNavs, &navtree.NavLink{ + Text: "Settings", Id: "alerting-admin", Url: s.cfg.AppSubURL + "/alerting/admin", + Icon: "cog", + }) + } + + if hasAccess(ac.EvalAny(ac.EvalPermission(ac.ActionAlertingRuleCreate), ac.EvalPermission(ac.ActionAlertingRuleExternalWrite))) { + alertChildNavs = append(alertChildNavs, &navtree.NavLink{ + Text: "Create alert rule", SubTitle: "Create an alert rule", Id: "alert", + Icon: "plus", Url: s.cfg.AppSubURL + "/alerting/new", HideFromTabs: true, IsCreateAction: true, + }) + } + + if len(alertChildNavs) > 0 { + var alertNav = navtree.NavLink{ + Text: "Alerting", + SubTitle: "Learn about problems in your systems moments after they occur", + Id: navtree.NavIDAlerting, + Icon: "bell", + Children: alertChildNavs, + SortWeight: navtree.WeightAlerting, + Url: s.cfg.AppSubURL + "/alerting", + } + + return &alertNav + } + + return nil +} + func (s *ServiceImpl) buildDataConnectionsNavLink(c *contextmodel.ReqContext) *navtree.NavLink { hasAccess := ac.HasAccess(s.accessControl, c) diff --git a/pkg/services/navtree/navtreeimpl/navtree_alerting_test.go b/pkg/services/navtree/navtreeimpl/navtree_alerting_test.go index 69b2d5b81fa..96c7a223d49 100644 --- a/pkg/services/navtree/navtreeimpl/navtree_alerting_test.go +++ b/pkg/services/navtree/navtreeimpl/navtree_alerting_test.go @@ -165,7 +165,7 @@ func TestBuildAlertNavLinks_V2(t *testing.T) { service := setupTestService(fullPermissions(), allFeatureFlags...) t.Run("Should have correct parent structure in V2 navigation", func(t *testing.T) { - navLink := service.buildAlertNavLinksV2(reqCtx) + navLink := service.buildAlertNavLinks(reqCtx) require.NotNil(t, navLink) require.NotEmpty(t, navLink.Children) @@ -179,7 +179,7 @@ func TestBuildAlertNavLinks_V2(t *testing.T) { }) t.Run("Should have correct tabs under each parent", func(t *testing.T) { - navLink := service.buildAlertNavLinksV2(reqCtx) + navLink := service.buildAlertNavLinks(reqCtx) require.NotNil(t, navLink) // Table-driven test for tab verification @@ -208,7 +208,7 @@ func TestBuildAlertNavLinks_V2(t *testing.T) { } limitedService := setupTestService(limitedPermissions, "alertingNavigationV2") - navLink := limitedService.buildAlertNavLinksV2(reqCtx) + navLink := limitedService.buildAlertNavLinks(reqCtx) require.NotNil(t, navLink) // Should not have notification-config without notification permissions @@ -216,7 +216,7 @@ func TestBuildAlertNavLinks_V2(t *testing.T) { }) t.Run("Should exclude future items from V2 navigation", func(t *testing.T) { - navLink := service.buildAlertNavLinksV2(reqCtx) + navLink := service.buildAlertNavLinks(reqCtx) require.NotNil(t, navLink) // Verify future items are not present