Plugins: Fix Default Nav URL for dashboard includes (#47143)

* Plugins: Fix Default Nav URL for dashboard includes

* update nav links

* PR comments

pkg/api/index.go

@@ -120,11 +120,14 @@ func (hs *HTTPServer) getAppLinks(c *models.ReqContext) ([]*dtos.NavLink, error)
 			}
 
 			if include.Type == "dashboard" && include.AddToNav {
-				link := &dtos.NavLink{
+				dboardURL := include.DashboardURLPath()
-					Url:  hs.Cfg.AppSubURL + include.GetSlugOrUIDLink(),
+				if dboardURL != "" {
-					Text: include.Name,
+					link := &dtos.NavLink{
+						Url:  path.Join(hs.Cfg.AppSubURL, dboardURL),
+						Text: include.Name,
+					}
+					appLink.Children = append(appLink.Children, link)
 				}
-				appLink.Children = append(appLink.Children, link)
 			}
 		}
 

pkg/plugins/manager/loader/loader.go

@@ -271,7 +271,13 @@ func setDefaultNavURL(p *plugins.Plugin) {
 			p.DefaultNavURL = path.Join("/plugins/", p.ID, "/page/", include.Slug)
 		}
 		if include.Type == "dashboard" {
-			p.DefaultNavURL = path.Join("/dashboard/db/", include.Slug)
+			dboardURL := include.DashboardURLPath()
+			if dboardURL == "" {
+				p.Logger().Warn("Included dashboard is missing a UID field")
+				continue
+			}
+
+			p.DefaultNavURL = dboardURL
 		}
 	}
 }

pkg/plugins/manager/loader/loader_test.go

@@ -378,7 +378,7 @@ func TestLoader_Load(t *testing.T) {
 						Plugins:           []plugins.Dependency{},
 					},
 					Includes: []*plugins.Includes{
-						{Name: "Nginx Memory", Path: "dashboards/memory.json", Type: "dashboard", Role: "Viewer", Slug: "nginx-memory", DefaultNav: true},
+						{Name: "Nginx Memory", Path: "dashboards/memory.json", Type: "dashboard", Role: "Viewer", Slug: "nginx-memory"},
 						{Name: "Root Page (react)", Type: "page", Role: "Viewer", Path: "/a/my-simple-app", DefaultNav: true, AddToNav: true, Slug: "root-page-react"},
 					},
 					Backend: false,
@@ -411,6 +411,60 @@ func TestLoader_Load(t *testing.T) {
 	}
 }
 
+func TestLoader_setDefaultNavURL(t *testing.T) {
+	t.Run("When including a dashboard with DefaultNav: true", func(t *testing.T) {
+		pluginWithDashboard := &plugins.Plugin{
+			JSONData: plugins.JSONData{Includes: []*plugins.Includes{
+				{
+					Type:       "dashboard",
+					DefaultNav: true,
+					UID:        "",
+				},
+			}},
+		}
+		logger := &fakeLogger{loggedLines: []string{}}
+		pluginWithDashboard.SetLogger(logger)
+
+		t.Run("Default nav URL is not set if dashboard UID field not is set", func(t *testing.T) {
+			setDefaultNavURL(pluginWithDashboard)
+			require.Equal(t, "", pluginWithDashboard.DefaultNavURL)
+			require.Equal(t, []string{"Included dashboard is missing a UID field"}, logger.loggedLines)
+		})
+
+		t.Run("Default nav URL is set if dashboard UID field is set", func(t *testing.T) {
+			pluginWithDashboard.Includes[0].UID = "a1b2c3"
+
+			setDefaultNavURL(pluginWithDashboard)
+			require.Equal(t, "/d/a1b2c3", pluginWithDashboard.DefaultNavURL)
+		})
+	})
+
+	t.Run("When including a page with DefaultNav: true", func(t *testing.T) {
+		pluginWithPage := &plugins.Plugin{
+			JSONData: plugins.JSONData{Includes: []*plugins.Includes{
+				{
+					Type:       "page",
+					DefaultNav: true,
+					Slug:       "testPage",
+				},
+			}},
+		}
+
+		t.Run("Default nav URL is set using slug", func(t *testing.T) {
+			setDefaultNavURL(pluginWithPage)
+			require.Equal(t, "/plugins/page/testPage", pluginWithPage.DefaultNavURL)
+		})
+
+		t.Run("Default nav URL is set using slugified Name field if Slug field is empty", func(t *testing.T) {
+			pluginWithPage.Includes[0].Slug = ""
+			pluginWithPage.Includes[0].Name = "My Test Page"
+
+			setDefaultNavURL(pluginWithPage)
+			require.Equal(t, "/plugins/page/my-test-page", pluginWithPage.DefaultNavURL)
+		})
+	})
+}
+
 func TestLoader_Load_MultiplePlugins(t *testing.T) {
 	parentDir, err := filepath.Abs("../")
 	if err != nil {
@@ -1126,20 +1180,22 @@ func (*fakeLicensingService) FeatureEnabled(feature string) bool {
 
 type fakeLogger struct {
 	log.Logger
+
+	loggedLines []string
 }
 
-func (fl fakeLogger) New(_ ...interface{}) *log.ConcreteLogger {
+func (fl *fakeLogger) New(_ ...interface{}) *log.ConcreteLogger {
 	return &log.ConcreteLogger{}
 }
 
-func (fl fakeLogger) Info(_ string, _ ...interface{}) {
+func (fl *fakeLogger) Info(l string, _ ...interface{}) {
-
+	fl.loggedLines = append(fl.loggedLines, l)
 }
 
-func (fl fakeLogger) Debug(_ string, _ ...interface{}) {
+func (fl *fakeLogger) Debug(l string, _ ...interface{}) {
-
+	fl.loggedLines = append(fl.loggedLines, l)
 }
 
-func (fl fakeLogger) Warn(_ string, _ ...interface{}) {
+func (fl *fakeLogger) Warn(l string, _ ...interface{}) {
-
+	fl.loggedLines = append(fl.loggedLines, l)
 }

pkg/plugins/manager/testdata/test-app-with-includes/plugin.json

@@ -20,8 +20,7 @@
     {
       "type": "dashboard",
       "name": "Nginx Memory",
-      "path": "dashboards/memory.json",
+      "path": "dashboards/memory.json"
-      "defaultNav": true
     },
     {
       "type": "page",

pkg/plugins/models.go

@@ -97,12 +97,11 @@ type Includes struct {
 	ID string `json:"-"`
 }
 
-func (e Includes) GetSlugOrUIDLink() string {
+func (e Includes) DashboardURLPath() string {
-	if len(e.UID) > 0 {
+	if e.Type != "dashboard" || len(e.UID) == 0 {
-		return "/d/" + e.UID
+		return ""
-	} else {
-		return "/dashboard/db/" + e.Slug
 	}
+	return "/d/" + e.UID
 }
 
 type Dependency struct {