Navigation: Add pluginId to standalone plugin page NavLinks (#57769)

* feat(Navigation): add `pluginId` to NavLink and override sibling navlinks with the same URL

* test replacing page from plugin

* chore: fix go lint issues

* fix(NavLink): change `PluginId` to `PluginID`

Co-authored-by: Torkel Ödegaard <torkel@grafana.com>

* fix(NavLink): make the `PluginId` -> `PluginID` change everywhere

* chore(navModel.ts): update explanatory comment for `pluginId`

Co-authored-by: Miklós Tolnai <miklos.tolnai@grafana.com>
Co-authored-by: Torkel Ödegaard <torkel@grafana.com>

packages/grafana-data/src/types/navModel.ts

@@ -26,6 +26,8 @@ export interface NavLinkDTO {
   children?: NavLinkDTO[];
   highlightText?: string;
   emptyMessageId?: string;
+  // The ID of the plugin that registered the page (in case it was registered by a plugin, otherwise left empty)
+  pluginId?: string;
 }
 
 export interface NavModelItem extends NavLinkDTO {

pkg/services/navtree/models.go

@@ -67,6 +67,7 @@ type NavLink struct {
 	HighlightText    string     `json:"highlightText,omitempty"`
 	HighlightID      string     `json:"highlightId,omitempty"`
 	EmptyMessageId   string     `json:"emptyMessageId,omitempty"`
+	PluginID         string     `json:"pluginId,omitempty"` // (Optional) The ID of the plugin that registered nav link (e.g. as a standalone plugin page)
 }
 
 func (node *NavLink) Sort() {

pkg/services/navtree/navtreeimpl/applinks.go

@@ -72,6 +72,7 @@ func (s *ServiceImpl) processAppPlugin(plugin plugins.PluginDTO, c *models.ReqCo
 		Section:    navtree.NavSectionPlugin,
 		SortWeight: navtree.WeightPlugin,
 		IsSection:  true,
+		PluginID:   plugin.ID,
 	}
 
 	if topNavEnabled {
@@ -89,6 +90,7 @@ func (s *ServiceImpl) processAppPlugin(plugin plugins.PluginDTO, c *models.ReqCo
 			link := &navtree.NavLink{
 				Text:     include.Name,
 				Icon:     include.Icon,
+				PluginID: plugin.ID,
 			}
 
 			if len(include.Path) > 0 {
@@ -100,12 +102,31 @@ func (s *ServiceImpl) processAppPlugin(plugin plugins.PluginDTO, c *models.ReqCo
 				link.Url = s.cfg.AppSubURL + "/plugins/" + plugin.ID + "/page/" + include.Slug
 			}
 
+			// Register standalone plugin pages to certain sections using the Grafana config
 			if pathConfig, ok := s.navigationAppPathConfig[include.Path]; ok {
 				if sectionForPage := treeRoot.FindById(pathConfig.SectionID); sectionForPage != nil {
 					link.Id = "standalone-plugin-page-" + include.Path
 					link.SortWeight = pathConfig.SortWeight
+
+					// Check if the section already has a page with the same URL, and in that case override it
+					// (This only happens if it is explicitly set by `navigation.app_standalone_pages` in the INI config)
+					isOverridingCorePage := false
+					for _, child := range sectionForPage.Children {
+						if child.Url == link.Url {
+							child.Id = link.Id
+							child.SortWeight = link.SortWeight
+							child.PluginID = link.PluginID
+							child.Children = []*navtree.NavLink{}
+							isOverridingCorePage = true
+							break
+						}
+					}
+
+					// Append the page to the section
+					if !isOverridingCorePage {
 						sectionForPage.Children = append(sectionForPage.Children, link)
 					}
+				}
 			} else {
 				appLink.Children = append(appLink.Children, link)
 			}
@@ -117,6 +138,7 @@ func (s *ServiceImpl) processAppPlugin(plugin plugins.PluginDTO, c *models.ReqCo
 				link := &navtree.NavLink{
 					Url:      path.Join(s.cfg.AppSubURL, dboardURL),
 					Text:     include.Name,
+					PluginID: plugin.ID,
 				}
 				appLink.Children = append(appLink.Children, link)
 			}

pkg/services/navtree/navtreeimpl/applinks_test.go

@@ -65,9 +65,27 @@ func TestAddAppLinks(t *testing.T) {
 		},
 	}
 
+	testApp3 := plugins.PluginDTO{
+		JSONData: plugins.JSONData{
+			ID:   "test-app3",
+			Name: "Test app3 name",
+			Type: plugins.App,
+			Includes: []*plugins.Includes{
+				{
+					Name:       "Hello",
+					Path:       "/connections/connect-data",
+					Type:       "page",
+					AddToNav:   true,
+					DefaultNav: true,
+				},
+			},
+		},
+	}
+
 	pluginSettings := pluginsettings.FakePluginSettings{Plugins: map[string]*pluginsettings.DTO{
 		testApp1.ID: {ID: 0, OrgID: 1, PluginID: testApp1.ID, PluginVersion: "1.0.0", Enabled: true},
 		testApp2.ID: {ID: 0, OrgID: 1, PluginID: testApp2.ID, PluginVersion: "1.0.0", Enabled: true},
+		testApp3.ID: {ID: 0, OrgID: 1, PluginID: testApp3.ID, PluginVersion: "1.0.0", Enabled: true},
 	}}
 
 	service := ServiceImpl{
@@ -77,7 +95,7 @@ func TestAddAppLinks(t *testing.T) {
 		pluginSettings: &pluginSettings,
 		features:       featuremgmt.WithFeatures(),
 		pluginStore: plugins.FakePluginStore{
-			PluginList: []plugins.PluginDTO{testApp1, testApp2},
+			PluginList: []plugins.PluginDTO{testApp1, testApp2, testApp3},
 		},
 	}
 
@@ -172,6 +190,27 @@ func TestAddAppLinks(t *testing.T) {
 		require.Equal(t, "Test app2 name", treeRoot.Children[0].Children[0].Text)
 		require.Equal(t, "Test app1 name", treeRoot.Children[0].Children[1].Text)
 	})
+
+	t.Run("Should replace page from plugin", func(t *testing.T) {
+		service.features = featuremgmt.WithFeatures(featuremgmt.FlagTopnav, featuremgmt.FlagDataConnectionsConsole)
+		service.navigationAppPathConfig = map[string]NavigationAppConfig{
+			"/connections/connect-data": {SectionID: "connections"},
+		}
+
+		treeRoot := navtree.NavTreeRoot{}
+		treeRoot.AddSection(service.buildDataConnectionsNavLink(reqCtx))
+		require.Equal(t, "Connections", treeRoot.Children[0].Text)
+		require.Equal(t, "Connect Data", treeRoot.Children[0].Children[1].Text)
+		require.Equal(t, "connections-connect-data", treeRoot.Children[0].Children[1].Id)
+		require.Equal(t, "", treeRoot.Children[0].Children[1].PluginID)
+
+		err := service.addAppLinks(&treeRoot, reqCtx)
+		require.NoError(t, err)
+		require.Equal(t, "Connections", treeRoot.Children[0].Text)
+		require.Equal(t, "Connect Data", treeRoot.Children[0].Children[1].Text)
+		require.Equal(t, "standalone-plugin-page-/connections/connect-data", treeRoot.Children[0].Children[1].Id)
+		require.Equal(t, "test-app3", treeRoot.Children[0].Children[1].PluginID)
+	})
 }
 
 func TestReadingNavigationSettings(t *testing.T) {