Navigation: refactor RemoveEmptySection... logic into main navtree code (#66878)

refactor RemoveEmptySection logic into main navtree code

pkg/api/index.go

@@ -166,8 +166,7 @@ func (hs *HTTPServer) setIndexViewData(c *contextmodel.ReqContext) (*dtos.IndexV
 
 	hs.HooksService.RunIndexDataHooks(&data, c)
 
-	// This will remove empty cfg or admin sections and move sections around
+	data.NavTree.ApplyAdminIA()
-	data.NavTree.RemoveEmptySectionsAndApplyNewInformationArchitecture()
 	data.NavTree.Sort()
 
 	return &data, nil

pkg/services/navtree/models.go

@@ -30,10 +30,8 @@ const (
 
 const (
 	NavIDRoot               = "root"
-	NavIDDashboards         = "dashboards"
+	NavIDDashboards         = "dashboards/browse"
-	NavIDDashboardsBrowse   = "dashboards/browse"
 	NavIDCfg                = "cfg" // NavIDCfg is the id for org configuration navigation node
-	NavIDAdmin              = "admin"
 	NavIDAlertsAndIncidents = "alerts-and-incidents"
 	NavIDAlerting           = "alerting"
 	NavIDAlertingLegacy     = "alerting-legacy"
@@ -90,44 +88,6 @@ func (root *NavTreeRoot) FindById(id string) *NavLink {
 	return FindById(root.Children, id)
 }
 
-func (root *NavTreeRoot) RemoveEmptySectionsAndApplyNewInformationArchitecture() {
-	// Remove server admin node if it has no children or set the url to first child
-	if node := root.FindById(NavIDAdmin); node != nil {
-		if len(node.Children) == 0 {
-			root.RemoveSection(node)
-		} else {
-			node.Url = node.Children[0].Url
-		}
-	}
-
-	ApplyAdminIA(root)
-
-	// Move reports into dashboards
-	if reports := root.FindById(NavIDReporting); reports != nil {
-		if dashboards := root.FindById(NavIDDashboards); dashboards != nil {
-			reports.SortWeight = 0
-			dashboards.Children = append(dashboards.Children, reports)
-			root.RemoveSection(reports)
-		}
-	}
-
-	// Change id of dashboards
-	if dashboards := root.FindById(NavIDDashboards); dashboards != nil {
-		dashboards.Id = "dashboards/browse"
-	}
-
-	// Remove top level cfg / administration node if it has no children
-	if node := root.FindById(NavIDCfg); node != nil {
-		if len(node.Children) == 0 {
-			root.RemoveSection(node)
-		}
-	}
-
-	if len(root.Children) < 1 {
-		root.Children = make([]*NavLink, 0)
-	}
-}
-
 func (root *NavTreeRoot) Sort() {
 	Sort(root.Children)
 }
@@ -155,28 +115,19 @@ func Sort(nodes []*NavLink) {
 	}
 }
 
-func ApplyAdminIA(root *NavTreeRoot) {
+func (root *NavTreeRoot) ApplyAdminIA() {
 	orgAdminNode := root.FindById(NavIDCfg)
 
 	if orgAdminNode != nil {
-		orgAdminNode.Url = "/admin"
-		orgAdminNode.Text = "Administration"
-
 		adminNodeLinks := []*NavLink{}
 
 		adminNodeLinks = AppendIfNotNil(adminNodeLinks, root.FindById("datasources"))
 		adminNodeLinks = AppendIfNotNil(adminNodeLinks, root.FindById("plugins"))
-		if globalUsers := root.FindById("global-users"); globalUsers != nil {
+		adminNodeLinks = AppendIfNotNil(adminNodeLinks, root.FindById("global-users"))
-			globalUsers.Text = "Users"
-			adminNodeLinks = append(adminNodeLinks, globalUsers)
-		}
 		adminNodeLinks = AppendIfNotNil(adminNodeLinks, root.FindById("teams"))
 		adminNodeLinks = AppendIfNotNil(adminNodeLinks, root.FindById("serviceaccounts"))
 		adminNodeLinks = AppendIfNotNil(adminNodeLinks, root.FindById("apikeys"))
-		if orgSettings := root.FindById("org-settings"); orgSettings != nil {
+		adminNodeLinks = AppendIfNotNil(adminNodeLinks, root.FindById("org-settings"))
-			orgSettings.Text = "Default preferences"
-			adminNodeLinks = append(adminNodeLinks, orgSettings)
-		}
 		adminNodeLinks = AppendIfNotNil(adminNodeLinks, root.FindById("authentication"))
 		adminNodeLinks = AppendIfNotNil(adminNodeLinks, root.FindById("server-settings"))
 		adminNodeLinks = AppendIfNotNil(adminNodeLinks, root.FindById("global-orgs"))
@@ -197,10 +148,6 @@ func ApplyAdminIA(root *NavTreeRoot) {
 			root.RemoveSection(orgAdminNode)
 		}
 	}
-
-	if serverAdminNode := root.FindById(NavIDAdmin); serverAdminNode != nil {
-		root.RemoveSection(serverAdminNode)
-	}
 }
 
 func AppendIfNotNil(children []*NavLink, newChild *NavLink) []*NavLink {

pkg/services/navtree/models_test.go

@@ -7,45 +7,6 @@ import (
 )
 
 func TestNavTreeRoot(t *testing.T) {
-	t.Run("Should remove empty admin and server admin sections", func(t *testing.T) {
-		treeRoot := NavTreeRoot{
-			Children: []*NavLink{
-				{Id: NavIDCfg},
-				{Id: NavIDAdmin},
-			},
-		}
-
-		treeRoot.RemoveEmptySectionsAndApplyNewInformationArchitecture()
-
-		require.Equal(t, 0, len(treeRoot.Children))
-	})
-
-	t.Run("Should create 3 new sections in the Admin node", func(t *testing.T) {
-		treeRoot := NavTreeRoot{
-			Children: []*NavLink{
-				{Id: NavIDCfg},
-				{Id: NavIDAdmin, Children: []*NavLink{{Id: "upgrading"}, {Id: "plugins"}, {Id: "teams"}}},
-			},
-		}
-
-		treeRoot.RemoveEmptySectionsAndApplyNewInformationArchitecture()
-
-		require.Equal(t, "Administration", treeRoot.Children[0].Text)
-	})
-
-	t.Run("Should move reports into Dashboards", func(t *testing.T) {
-		treeRoot := NavTreeRoot{
-			Children: []*NavLink{
-				{Id: NavIDDashboards},
-				{Id: NavIDReporting},
-			},
-		}
-
-		treeRoot.RemoveEmptySectionsAndApplyNewInformationArchitecture()
-
-		require.Equal(t, NavIDReporting, treeRoot.Children[0].Children[0].Id)
-	})
-
 	t.Run("Sorting by index", func(t *testing.T) {
 		treeRoot := NavTreeRoot{
 			Children: []*NavLink{

pkg/services/navtree/navtreeimpl/admin.go

@@ -12,10 +12,13 @@ import (
 	"github.com/grafana/grafana/pkg/services/serviceaccounts"
 )
 
-func (s *ServiceImpl) getOrgAdminNode(c *contextmodel.ReqContext) (*navtree.NavLink, error) {
+func (s *ServiceImpl) getAdminNode(c *contextmodel.ReqContext) (*navtree.NavLink, error) {
 	var configNodes []*navtree.NavLink
-
 	hasAccess := ac.HasAccess(s.accessControl, c)
+	hasGlobalAccess := ac.HasGlobalAccess(s.accessControl, s.accesscontrolService, c)
+	orgsAccessEvaluator := ac.EvalPermission(ac.ActionOrgsRead)
+	authConfigUIAvailable := s.license.FeatureEnabled("saml") && s.features.IsEnabled(featuremgmt.FlagAuthenticationConfigUI)
+
 	if hasAccess(ac.ReqOrgAdmin, datasources.ConfigurationPageAccess) {
 		configNodes = append(configNodes, &navtree.NavLink{
 			Text:     "Data sources",
@@ -26,26 +29,6 @@ func (s *ServiceImpl) getOrgAdminNode(c *contextmodel.ReqContext) (*navtree.NavL
 		})
 	}
 
-	if s.features.IsEnabled(featuremgmt.FlagCorrelations) && hasAccess(ac.ReqOrgAdmin, correlations.ConfigurationPageAccess) {
-		configNodes = append(configNodes, &navtree.NavLink{
-			Text:     "Correlations",
-			Icon:     "gf-glue",
-			SubTitle: "Add and configure correlations",
-			Id:       "correlations",
-			Url:      s.cfg.AppSubURL + "/datasources/correlations",
-		})
-	}
-
-	if hasAccess(s.ReqCanAdminTeams, ac.TeamsAccessEvaluator) {
-		configNodes = append(configNodes, &navtree.NavLink{
-			Text:     "Teams",
-			Id:       "teams",
-			SubTitle: "Groups of users that have common dashboard and permission needs",
-			Icon:     "users-alt",
-			Url:      s.cfg.AppSubURL + "/org/teams",
-		})
-	}
-
 	// FIXME: while we don't have a permissions for listing plugins the legacy check has to stay as a default
 	if pluginaccesscontrol.ReqCanAdminPlugins(s.cfg)(c) || hasAccess(pluginaccesscontrol.ReqCanAdminPlugins(s.cfg), pluginaccesscontrol.AdminAccessEvaluator) {
 		configNodes = append(configNodes, &navtree.NavLink{
@@ -57,13 +40,29 @@ func (s *ServiceImpl) getOrgAdminNode(c *contextmodel.ReqContext) (*navtree.NavL
 		})
 	}
 
-	if hasAccess(ac.ReqOrgAdmin, ac.OrgPreferencesAccessEvaluator) {
+	if hasAccess(ac.ReqSignedIn, ac.EvalAny(ac.EvalPermission(ac.ActionOrgUsersRead), ac.EvalPermission(ac.ActionUsersRead, ac.ScopeGlobalUsersAll))) {
 		configNodes = append(configNodes, &navtree.NavLink{
-			Text:     "Preferences",
+			Text: "Users", SubTitle: "Manage users in Grafana", Id: "global-users", Url: s.cfg.AppSubURL + "/admin/users", Icon: "user",
-			Id:       "org-settings",
+		})
-			SubTitle: "Manage preferences across an organization",
+	}
-			Icon:     "sliders-v-alt",
+
-			Url:      s.cfg.AppSubURL + "/org",
+	if hasAccess(s.ReqCanAdminTeams, ac.TeamsAccessEvaluator) {
+		configNodes = append(configNodes, &navtree.NavLink{
+			Text:     "Teams",
+			Id:       "teams",
+			SubTitle: "Groups of users that have common dashboard and permission needs",
+			Icon:     "users-alt",
+			Url:      s.cfg.AppSubURL + "/org/teams",
+		})
+	}
+
+	if enableServiceAccount(s, c) {
+		configNodes = append(configNodes, &navtree.NavLink{
+			Text:     "Service accounts",
+			Id:       "serviceaccounts",
+			SubTitle: "Use service accounts to run automated workloads in Grafana",
+			Icon:     "gf-service-account",
+			Url:      s.cfg.AppSubURL + "/org/serviceaccounts",
 		})
 	}
 
@@ -81,43 +80,18 @@ func (s *ServiceImpl) getOrgAdminNode(c *contextmodel.ReqContext) (*navtree.NavL
 		})
 	}
 
-	if enableServiceAccount(s, c) {
+	if hasAccess(ac.ReqOrgAdmin, ac.OrgPreferencesAccessEvaluator) {
 		configNodes = append(configNodes, &navtree.NavLink{
-			Text:     "Service accounts",
+			Text:     "Default preferences",
-			Id:       "serviceaccounts",
+			Id:       "org-settings",
-			SubTitle: "Use service accounts to run automated workloads in Grafana",
+			SubTitle: "Manage preferences across an organization",
-			Icon:     "gf-service-account",
+			Icon:     "sliders-v-alt",
-			Url:      s.cfg.AppSubURL + "/org/serviceaccounts",
+			Url:      s.cfg.AppSubURL + "/org",
 		})
 	}
 
-	configNode := &navtree.NavLink{
-		Id:         navtree.NavIDCfg,
-		Text:       "Configuration",
-		SubTitle:   "Organization: " + c.OrgName,
-		Icon:       "cog",
-		SortWeight: navtree.WeightConfig,
-		Children:   configNodes,
-	}
-
-	return configNode, nil
-}
-
-func (s *ServiceImpl) getServerAdminNode(c *contextmodel.ReqContext) *navtree.NavLink {
-	hasAccess := ac.HasAccess(s.accessControl, c)
-	hasGlobalAccess := ac.HasGlobalAccess(s.accessControl, s.accesscontrolService, c)
-	orgsAccessEvaluator := ac.EvalPermission(ac.ActionOrgsRead)
-	adminNavLinks := []*navtree.NavLink{}
-
-	if hasAccess(ac.ReqSignedIn, ac.EvalAny(ac.EvalPermission(ac.ActionOrgUsersRead), ac.EvalPermission(ac.ActionUsersRead, ac.ScopeGlobalUsersAll))) {
-		adminNavLinks = append(adminNavLinks, &navtree.NavLink{
-			Text: "Users", SubTitle: "Manage users in Grafana", Id: "global-users", Url: s.cfg.AppSubURL + "/admin/users", Icon: "user",
-		})
-	}
-
-	authConfigUIAvailable := s.license.FeatureEnabled("saml") && s.features.IsEnabled(featuremgmt.FlagAuthenticationConfigUI)
 	if authConfigUIAvailable && hasAccess(ac.ReqGrafanaAdmin, evalAuthenticationSettings()) {
-		adminNavLinks = append(adminNavLinks, &navtree.NavLink{
+		configNodes = append(configNodes, &navtree.NavLink{
 			Text:     "Authentication",
 			Id:       "authentication",
 			SubTitle: "Manage your auth settings and configure single sign-on",
@@ -126,15 +100,31 @@ func (s *ServiceImpl) getServerAdminNode(c *contextmodel.ReqContext) *navtree.Na
 		})
 	}
 
+	if hasAccess(ac.ReqGrafanaAdmin, ac.EvalPermission(ac.ActionSettingsRead)) {
+		configNodes = append(configNodes, &navtree.NavLink{
+			Text: "Settings", SubTitle: "View the settings defined in your Grafana config", Id: "server-settings", Url: s.cfg.AppSubURL + "/admin/settings", Icon: "sliders-v-alt",
+		})
+	}
+
 	if hasGlobalAccess(ac.ReqGrafanaAdmin, orgsAccessEvaluator) {
-		adminNavLinks = append(adminNavLinks, &navtree.NavLink{
+		configNodes = append(configNodes, &navtree.NavLink{
 			Text: "Organizations", SubTitle: "Isolated instances of Grafana running on the same server", Id: "global-orgs", Url: s.cfg.AppSubURL + "/admin/orgs", Icon: "building",
 		})
 	}
 
-	if hasAccess(ac.ReqGrafanaAdmin, ac.EvalPermission(ac.ActionSettingsRead)) {
+	if s.features.IsEnabled(featuremgmt.FlagCorrelations) && hasAccess(ac.ReqOrgAdmin, correlations.ConfigurationPageAccess) {
-		adminNavLinks = append(adminNavLinks, &navtree.NavLink{
+		configNodes = append(configNodes, &navtree.NavLink{
-			Text: "Settings", SubTitle: "View the settings defined in your Grafana config", Id: "server-settings", Url: s.cfg.AppSubURL + "/admin/settings", Icon: "sliders-v-alt",
+			Text:     "Correlations",
+			Icon:     "gf-glue",
+			SubTitle: "Add and configure correlations",
+			Id:       "correlations",
+			Url:      s.cfg.AppSubURL + "/datasources/correlations",
+		})
+	}
+
+	if s.cfg.LDAPAuthEnabled && hasAccess(ac.ReqGrafanaAdmin, ac.EvalPermission(ac.ActionLDAPStatusRead)) {
+		configNodes = append(configNodes, &navtree.NavLink{
+			Text: "LDAP", Id: "ldap", Url: s.cfg.AppSubURL + "/admin/ldap", Icon: "book",
 		})
 	}
 
@@ -146,28 +136,20 @@ func (s *ServiceImpl) getServerAdminNode(c *contextmodel.ReqContext) *navtree.Na
 			Icon:     "cube",
 			Url:      s.cfg.AppSubURL + "/admin/storage",
 		}
-		adminNavLinks = append(adminNavLinks, storage)
+		configNodes = append(configNodes, storage)
 	}
 
-	if s.cfg.LDAPAuthEnabled && hasAccess(ac.ReqGrafanaAdmin, ac.EvalPermission(ac.ActionLDAPStatusRead)) {
+	configNode := &navtree.NavLink{
-		adminNavLinks = append(adminNavLinks, &navtree.NavLink{
+		Id:         navtree.NavIDCfg,
-			Text: "LDAP", Id: "ldap", Url: s.cfg.AppSubURL + "/admin/ldap", Icon: "book",
+		Text:       "Administration",
-		})
+		SubTitle:   "Organization: " + c.OrgName,
+		Icon:       "cog",
+		SortWeight: navtree.WeightConfig,
+		Children:   configNodes,
+		Url:        "/admin",
 	}
 
-	adminNode := &navtree.NavLink{
+	return configNode, nil
-		Text:       "Server admin",
-		Id:         navtree.NavIDAdmin,
-		Icon:       "shield",
-		SortWeight: navtree.WeightAdmin,
-		Children:   adminNavLinks,
-	}
-
-	if len(adminNavLinks) > 0 {
-		adminNode.Url = adminNavLinks[0].Url
-	}
-
-	return adminNode
 }
 
 func (s *ServiceImpl) ReqCanAdminTeams(c *contextmodel.ReqContext) bool {

pkg/services/navtree/navtreeimpl/applinks_test.go

@@ -178,19 +178,19 @@ func TestAddAppLinks(t *testing.T) {
 	// This can be done by using `[navigation.app_sections]` in the INI config
 	t.Run("Should move apps that have specific nav id configured to correct section", func(t *testing.T) {
 		service.navigationAppConfig = map[string]NavigationAppConfig{
-			"test-app1": {SectionID: navtree.NavIDAdmin},
+			"test-app1": {SectionID: navtree.NavIDCfg},
 		}
 
 		treeRoot := navtree.NavTreeRoot{}
 		treeRoot.AddSection(&navtree.NavLink{
-			Id: navtree.NavIDAdmin,
+			Id: navtree.NavIDCfg,
 		})
 
 		err := service.addAppLinks(&treeRoot, reqCtx)
 		require.NoError(t, err)
 
 		// Check if the plugin gets moved over to the "Admin" section
-		adminNode := treeRoot.FindById(navtree.NavIDAdmin)
+		adminNode := treeRoot.FindById(navtree.NavIDCfg)
 		require.NotNil(t, adminNode)
 		require.Len(t, adminNode.Children, 1)
 		require.Equal(t, "plugin-page-test-app1", adminNode.Children[0].Id)

pkg/services/navtree/navtreeimpl/navtree.go

@@ -149,7 +149,7 @@ func (s *ServiceImpl) GetNavTree(c *contextmodel.ReqContext, hasEditPerm bool, p
 		}
 	}
 
-	orgAdminNode, err := s.getOrgAdminNode(c)
+	orgAdminNode, err := s.getAdminNode(c)
 
 	if orgAdminNode != nil {
 		treeRoot.AddSection(orgAdminNode)
@@ -157,12 +157,6 @@ func (s *ServiceImpl) GetNavTree(c *contextmodel.ReqContext, hasEditPerm bool, p
 		return nil, err
 	}
 
-	serverAdminNode := s.getServerAdminNode(c)
-
-	if serverAdminNode != nil {
-		treeRoot.AddSection(serverAdminNode)
-	}
-
 	s.addHelpLinks(treeRoot, c)
 
 	if err := s.addAppLinks(treeRoot, c); err != nil {