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.
This commit is contained in:
Alejandro Fraenkel
@@ -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)
|
||||
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user