From b4f73c9f09e738265c3ac9ef961fee4ed7b9b2c2 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Torkel=20=C3=96degaard?= <torkel@grafana.com>
Date: Thu, 29 Sep 2022 13:27:51 +0200
Subject: [PATCH] PluginPages: Support plugin pages that don't belong to a
 section (#55904)

* Fixing pages that don't exist in navtree

* Fix test

* fix lint warning

* Fixes
---
 pkg/services/navtree/navtreeimpl/applinks.go  |  2 +-
 public/app/core/components/PageNew/Page.tsx   |  2 +-
 .../core/components/PageNew/SectionNav.tsx    |  4 ++
 .../plugins/components/AppRootPage.tsx        | 15 +-----
 .../plugins/components/PluginPageContext.tsx  |  2 +-
 public/app/features/plugins/utils.test.ts     | 10 ++--
 public/app/features/plugins/utils.ts          | 54 +++++++++++--------
 7 files changed, 44 insertions(+), 45 deletions(-)

diff --git a/pkg/services/navtree/navtreeimpl/applinks.go b/pkg/services/navtree/navtreeimpl/applinks.go
index d6a8978f759..88b52e62b5b 100644
--- a/pkg/services/navtree/navtreeimpl/applinks.go
+++ b/pkg/services/navtree/navtreeimpl/applinks.go
@@ -85,7 +85,7 @@ func (s *ServiceImpl) processAppPlugin(plugin plugins.PluginDTO, c *models.ReqCo
 		SortWeight: navtree.WeightPlugin,
 	}
 
-	if s.features.IsEnabled(featuremgmt.FlagTopnav) {
+	if topNavEnabled {
 		appLink.Url = s.cfg.AppSubURL + "/a/" + plugin.ID
 	} else {
 		appLink.Url = path.Join(s.cfg.AppSubURL, plugin.DefaultNavURL)
diff --git a/public/app/core/components/PageNew/Page.tsx b/public/app/core/components/PageNew/Page.tsx
index 1ad41a503b4..4e39f9a8701 100644
--- a/public/app/core/components/PageNew/Page.tsx
+++ b/public/app/core/components/PageNew/Page.tsx
@@ -50,7 +50,7 @@ export const Page: PageType = ({
     <div className={cx(styles.wrapper, className)} {...otherProps}>
       {layout === PageLayoutType.Standard && (
         <div className={styles.panes}>
-          {navModel && navModel.main.children && <SectionNav model={navModel} />}
+          {navModel && <SectionNav model={navModel} />}
           <div className={styles.pageContent}>
             <CustomScrollbar autoHeightMin={'100%'} scrollTop={scrollTop} scrollRefCallback={scrollRef}>
               <div className={styles.pageInner}>
diff --git a/public/app/core/components/PageNew/SectionNav.tsx b/public/app/core/components/PageNew/SectionNav.tsx
index 59c31a5bde9..58d2fc57669 100644
--- a/public/app/core/components/PageNew/SectionNav.tsx
+++ b/public/app/core/components/PageNew/SectionNav.tsx
@@ -13,6 +13,10 @@ export interface Props {
 export function SectionNav({ model }: Props) {
   const styles = useStyles2(getStyles);
 
+  if (!Boolean(model.main?.children?.length)) {
+    return null;
+  }
+
   return (
     <nav className={styles.nav}>
       <CustomScrollbar showScrollIndicators>
diff --git a/public/app/features/plugins/components/AppRootPage.tsx b/public/app/features/plugins/components/AppRootPage.tsx
index 0e9771dad77..7f34d68b22e 100644
--- a/public/app/features/plugins/components/AppRootPage.tsx
+++ b/public/app/features/plugins/components/AppRootPage.tsx
@@ -8,7 +8,6 @@ import { AppEvents, AppPlugin, AppPluginMeta, KeyValue, NavModel, PluginType } f
 import { config } from '@grafana/runtime';
 import { getNotFoundNav, getWarningNav, getExceptionNav } from 'app/angular/services/nav_model_srv';
 import { Page } from 'app/core/components/Page/Page';
-import { PageProps } from 'app/core/components/Page/types';
 import PageLoader from 'app/core/components/PageLoader/PageLoader';
 import { appEvents } from 'app/core/core';
 import { GrafanaRouteComponentProps } from 'app/core/navigation/types';
@@ -55,7 +54,7 @@ export function AppRootPage({ match, queryParams, location }: Props) {
   );
 
   if (!plugin || match.params.pluginId !== plugin.meta.id) {
-    return <Page {...getLoadingPageProps(sectionNav)}>{loading && <PageLoader />}</Page>;
+    return <Page navModel={sectionNav}>{loading && <PageLoader />}</Page>;
   }
 
   if (!plugin.root) {
@@ -122,18 +121,6 @@ const stateSlice = createSlice({
   },
 });
 
-function getLoadingPageProps(sectionNav: NavModel | null): Partial<PageProps> {
-  if (config.featureToggles.topnav && sectionNav) {
-    return { navModel: sectionNav };
-  }
-
-  const loading = { text: 'Loading plugin' };
-
-  return {
-    navModel: { main: loading, node: loading },
-  };
-}
-
 async function loadAppPlugin(pluginId: string, dispatch: React.Dispatch<AnyAction>) {
   try {
     const app = await getPluginSettings(pluginId).then((info) => {
diff --git a/public/app/features/plugins/components/PluginPageContext.tsx b/public/app/features/plugins/components/PluginPageContext.tsx
index dbbca91cb1c..8f10677ab44 100644
--- a/public/app/features/plugins/components/PluginPageContext.tsx
+++ b/public/app/features/plugins/components/PluginPageContext.tsx
@@ -19,7 +19,7 @@ function getInitialPluginPageContext(): PluginPageContextType {
   };
 }
 
-export function buildPluginPageContext(sectionNav: NavModel | null): PluginPageContextType {
+export function buildPluginPageContext(sectionNav: NavModel | undefined): PluginPageContextType {
   return {
     sectionNav: sectionNav ?? getInitialPluginPageContext().sectionNav,
   };
diff --git a/public/app/features/plugins/utils.test.ts b/public/app/features/plugins/utils.test.ts
index 755fc884dd5..e034ea17d2b 100644
--- a/public/app/features/plugins/utils.test.ts
+++ b/public/app/features/plugins/utils.test.ts
@@ -86,12 +86,12 @@ describe('buildPluginSectionNav', () => {
     expect(result?.node.text).toBe('Standalone page');
   });
 
-  it('Should throw error if app not found in navtree', () => {
+  it('Should not throw error just return a root nav model without children for plugins that dont exist in navtree', () => {
     config.featureToggles.topnav = true;
-    const action = () => {
-      buildPluginSectionNav({} as HistoryLocation, pluginNav, navIndex, 'app3');
-    };
-    expect(action).toThrowError();
+    const result = buildPluginSectionNav({} as HistoryLocation, pluginNav, navIndex, 'app3');
+    expect(result?.main.id).toBe('root-plugin-page');
+    expect(result?.main.hideFromBreadcrumbs).toBe(true);
+    expect(result?.main.children?.length).toBe(0);
   });
 
   it('Should throw error if app has no section', () => {
diff --git a/public/app/features/plugins/utils.ts b/public/app/features/plugins/utils.ts
index 8a36d8066ff..b63ff766469 100644
--- a/public/app/features/plugins/utils.ts
+++ b/public/app/features/plugins/utils.ts
@@ -37,44 +37,51 @@ export function buildPluginSectionNav(
   pluginNav: NavModel | null,
   navIndex: NavIndex,
   pluginId: string
-) {
+): NavModel | undefined {
   // When topnav is disabled we only just show pluginNav like before
   if (!config.featureToggles.topnav) {
-    return pluginNav;
+    return pluginNav ?? undefined;
   }
 
-  const section = { ...getPluginSection(location, navIndex, pluginId) };
+  let section = getPluginSection(location, navIndex, pluginId);
+  if (!section) {
+    return undefined;
+  }
+
+  // shallow clone as we set active flag
+  section = { ...section };
 
   // If we have plugin nav don't set active page in section as it will cause double breadcrumbs
   const currentUrl = config.appSubUrl + location.pathname + location.search;
   let activePage: NavModelItem | undefined;
 
+  function setPageToActive(page: NavModelItem, currentUrl: string): NavModelItem {
+    if (!currentUrl.startsWith(page.url ?? '')) {
+      return page;
+    }
+
+    if (activePage && (activePage.url?.length ?? 0) > (page.url?.length ?? 0)) {
+      return page;
+    }
+
+    if (activePage) {
+      activePage.active = false;
+    }
+
+    activePage = { ...page, active: true };
+    return activePage;
+  }
+
   // Find and set active page
   section.children = (section?.children ?? []).map((child) => {
     if (child.children) {
       return {
         ...child,
-        children: child.children.map((pluginPage) => {
-          if (currentUrl.startsWith(pluginPage.url ?? '')) {
-            activePage = {
-              ...pluginPage,
-              active: true,
-            };
-            return activePage;
-          }
-          return pluginPage;
-        }),
+        children: child.children.map((pluginPage) => setPageToActive(pluginPage, currentUrl)),
       };
-    } else {
-      if (currentUrl.startsWith(child.url ?? '')) {
-        activePage = {
-          ...child,
-          active: true,
-        };
-        return activePage;
-      }
     }
-    return child;
+
+    return setPageToActive(child, currentUrl);
   });
 
   return { main: section, node: activePage ?? section };
@@ -90,9 +97,10 @@ export function getPluginSection(location: HistoryLocation, navIndex: NavIndex,
     return parent.parentItem ?? parent;
   }
 
+  // Some plugins like cloud home don't have any precense in the navtree so we need to allow those
   const navTreeNodeForPlugin = navIndex[`plugin-page-${pluginId}`];
   if (!navTreeNodeForPlugin) {
-    throw new Error('Plugin not found in navigation tree');
+    return { id: 'root-plugin-page', text: 'Root plugin page', hideFromBreadcrumbs: true };
   }
 
   if (!navTreeNodeForPlugin.parentItem) {