From eb3ee35e1ca5ab14aa8de2f7dcdc192da9068437 Mon Sep 17 00:00:00 2001 From: Levente Balogh Date: Mon, 7 Nov 2022 15:19:31 +0100 Subject: [PATCH] Frontend Routing: Always render standalone plugin pages using the `` (#57771) * chore: fix go lint issues * feat(Routing): route standalone plugin pages to the `AppRoutePage` * feat(plugin.json): introduce a new field called `isCorePage` for `includes` * chore: add explanatory comments for types * refactor(AppRootPage): receive the `pluginId` and `pluginSection` through the props Now we are able to receive these as props as the pluginId is defined on navLinks that are registered by plugins. * chore: update teests for AppRootPage * fix: remove rebase issue * tests(applinks): add a test for checking isCorePage plugin page setting * refactor(applinks): update tests to use FindById() and be more resilient to changes * fix: Go lint issues * refactor(routes): use cleaner types when working with plugin nav nodes Co-authored-by: Marcus Andersson * chore: fix linting issues * t: remove `isCorePage` field from includes Co-authored-by: Marcus Andersson --- .betterer.results | 3 - packages/grafana-data/src/types/plugin.ts | 7 +- pkg/services/navtree/navtreeimpl/applinks.go | 10 +- .../navtree/navtreeimpl/applinks_test.go | 181 ++++++++++++++---- .../plugins/components/AppRootPage.test.tsx | 26 ++- .../plugins/components/AppRootPage.tsx | 48 +++-- public/app/features/plugins/routes.tsx | 34 ++++ public/app/features/plugins/utils.test.ts | 51 +---- public/app/features/plugins/utils.ts | 46 +---- public/app/routes/routes.tsx | 19 +- 10 files changed, 249 insertions(+), 176 deletions(-) create mode 100644 public/app/features/plugins/routes.tsx diff --git a/.betterer.results b/.betterer.results index d6008a4036f..e0890b5e1b6 100644 --- a/.betterer.results +++ b/.betterer.results @@ -4431,9 +4431,6 @@ exports[`better eslint`] = { "public/app/features/plugins/components/AppRootPage.test.tsx:5381": [ [0, 0, 0, "Unexpected any. Specify a different type.", "0"] ], - "public/app/features/plugins/components/AppRootPage.tsx:5381": [ - [0, 0, 0, "Do not use any type assertions.", "0"] - ], "public/app/features/plugins/datasource_srv.ts:5381": [ [0, 0, 0, "Do not use any type assertions.", "0"], [0, 0, 0, "Unexpected any. Specify a different type.", "1"], diff --git a/packages/grafana-data/src/types/plugin.ts b/packages/grafana-data/src/types/plugin.ts index 0d36d4ce3d9..0d718e92fa7 100644 --- a/packages/grafana-data/src/types/plugin.ts +++ b/packages/grafana-data/src/types/plugin.ts @@ -110,8 +110,11 @@ export interface PluginInclude { path?: string; icon?: string; - role?: string; // "Viewer", Admin, editor??? - addToNav?: boolean; // Show in the sidebar... only if type=page? + // "Admin", "Editor" or "Viewer". If set then the include will only show up in the navigation if the user has the required roles. + role?: string; + + // Adds the "page" or "dashboard" type includes to the navigation if set to `true`. + addToNav?: boolean; // Angular app pages component?: string; diff --git a/pkg/services/navtree/navtreeimpl/applinks.go b/pkg/services/navtree/navtreeimpl/applinks.go index b67bd8f0208..35d4230133d 100644 --- a/pkg/services/navtree/navtreeimpl/applinks.go +++ b/pkg/services/navtree/navtreeimpl/applinks.go @@ -86,7 +86,7 @@ func (s *ServiceImpl) processAppPlugin(plugin plugins.PluginDTO, c *models.ReqCo continue } - if include.Type == "page" && include.AddToNav { + if include.Type == "page" { link := &navtree.NavLink{ Text: include.Name, Icon: include.Icon, @@ -95,7 +95,7 @@ func (s *ServiceImpl) processAppPlugin(plugin plugins.PluginDTO, c *models.ReqCo if len(include.Path) > 0 { link.Url = s.cfg.AppSubURL + include.Path - if include.DefaultNav { + if include.DefaultNav && include.AddToNav { appLink.Url = link.Url } } else { @@ -127,7 +127,9 @@ func (s *ServiceImpl) processAppPlugin(plugin plugins.PluginDTO, c *models.ReqCo sectionForPage.Children = append(sectionForPage.Children, link) } } - } else { + + // Register the page under the app + } else if include.AddToNav { appLink.Children = append(appLink.Children, link) } } @@ -169,7 +171,7 @@ func (s *ServiceImpl) processAppPlugin(plugin plugins.PluginDTO, c *models.ReqCo // Handle moving apps into specific navtree sections alertingNode := treeRoot.FindById(navtree.NavIDAlerting) - sectionID := "apps" + sectionID := navtree.NavIDApps if navConfig, hasOverride := s.navigationAppConfig[plugin.ID]; hasOverride { appLink.SortWeight = navConfig.SortWeight diff --git a/pkg/services/navtree/navtreeimpl/applinks_test.go b/pkg/services/navtree/navtreeimpl/applinks_test.go index 82b791090ad..149805ae096 100644 --- a/pkg/services/navtree/navtreeimpl/applinks_test.go +++ b/pkg/services/navtree/navtreeimpl/applinks_test.go @@ -72,12 +72,24 @@ func TestAddAppLinks(t *testing.T) { Type: plugins.App, Includes: []*plugins.Includes{ { - Name: "Hello", - Path: "/connections/connect-data", + Name: "Default page", + Path: "/a/test-app3/default", Type: "page", AddToNav: true, DefaultNav: true, }, + { + Name: "Random page", + Path: "/a/test-app3/random-page", + Type: "page", + AddToNav: true, + }, + { + Name: "Connect data", + Path: "/connections/connect-data", + Type: "page", + AddToNav: false, + }, }, }, } @@ -113,20 +125,27 @@ func TestAddAppLinks(t *testing.T) { treeRoot := navtree.NavTreeRoot{} err := service.addAppLinks(&treeRoot, reqCtx) require.NoError(t, err) - require.Equal(t, "Apps", treeRoot.Children[0].Text) - require.Equal(t, "Test app1 name", treeRoot.Children[0].Children[0].Text) + + appsNode := treeRoot.FindById(navtree.NavIDApps) + require.NotNil(t, appsNode) + require.Equal(t, "Apps", appsNode.Text) + require.Len(t, appsNode.Children, 3) + require.Equal(t, testApp1.Name, appsNode.Children[0].Text) }) - t.Run("Should remove add default nav child when topnav is enabled", func(t *testing.T) { + t.Run("Should remove the default nav child (DefaultNav=true) when topnav is enabled and should set its URL to the plugin nav root", func(t *testing.T) { service.features = featuremgmt.WithFeatures(featuremgmt.FlagTopnav) treeRoot := navtree.NavTreeRoot{} err := service.addAppLinks(&treeRoot, reqCtx) require.NoError(t, err) - require.Equal(t, "Apps", treeRoot.Children[0].Text) - require.Equal(t, "Test app1 name", treeRoot.Children[0].Children[0].Text) - require.Equal(t, "Page2", treeRoot.Children[0].Children[0].Children[0].Text) + + app1Node := treeRoot.FindById("plugin-page-test-app1") + require.Len(t, app1Node.Children, 1) // The page include with DefaultNav=true gets removed + require.Equal(t, "/a/test-app1/catalog", app1Node.Url) + require.Equal(t, "Page2", app1Node.Children[0].Text) }) + // 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.features = featuremgmt.WithFeatures(featuremgmt.FlagTopnav) service.navigationAppConfig = map[string]NavigationAppConfig{ @@ -140,76 +159,158 @@ func TestAddAppLinks(t *testing.T) { err := service.addAppLinks(&treeRoot, reqCtx) require.NoError(t, err) - require.Equal(t, "plugin-page-test-app1", treeRoot.Children[0].Children[0].Id) + + // Check if the plugin gets moved over to the "Admin" section + adminNode := treeRoot.FindById(navtree.NavIDAdmin) + require.NotNil(t, adminNode) + require.Len(t, adminNode.Children, 1) + require.Equal(t, "plugin-page-test-app1", adminNode.Children[0].Id) + + // Check if it is not under the "Apps" section anymore + appsNode := treeRoot.FindById(navtree.NavIDApps) + require.NotNil(t, appsNode) + require.Len(t, appsNode.Children, 2) + require.Equal(t, "plugin-page-test-app2", appsNode.Children[0].Id) + require.Equal(t, "plugin-page-test-app3", appsNode.Children[1].Id) }) - t.Run("Should add monitoring section if plugin exists that wants to live there", func(t *testing.T) { + t.Run("Should only add a 'Monitoring' section if a plugin exists that wants to live there", func(t *testing.T) { service.features = featuremgmt.WithFeatures(featuremgmt.FlagTopnav) + service.navigationAppConfig = map[string]NavigationAppConfig{} + + // Check if the Monitoring section is not there if no apps try to register to it + treeRoot := navtree.NavTreeRoot{} + err := service.addAppLinks(&treeRoot, reqCtx) + require.NoError(t, err) + monitoringNode := treeRoot.FindById(navtree.NavIDMonitoring) + require.Nil(t, monitoringNode) + + // It should appear and once an app tries to register to it + treeRoot = navtree.NavTreeRoot{} service.navigationAppConfig = map[string]NavigationAppConfig{ "test-app1": {SectionID: navtree.NavIDMonitoring}, } - - treeRoot := navtree.NavTreeRoot{} - - err := service.addAppLinks(&treeRoot, reqCtx) + err = service.addAppLinks(&treeRoot, reqCtx) require.NoError(t, err) - require.Equal(t, "Monitoring", treeRoot.Children[0].Text) - require.Equal(t, "Test app1 name", treeRoot.Children[0].Children[0].Text) + monitoringNode = treeRoot.FindById(navtree.NavIDMonitoring) + require.NotNil(t, monitoringNode) + require.Len(t, monitoringNode.Children, 1) + require.Equal(t, "Test app1 name", monitoringNode.Children[0].Text) }) - t.Run("Should add Alerts and incidents section if plugin exists that wants to live there", func(t *testing.T) { + t.Run("Should add a 'Alerts and Incidents' section if a plugin exists that wants to live there", func(t *testing.T) { service.features = featuremgmt.WithFeatures(featuremgmt.FlagTopnav) + service.navigationAppConfig = map[string]NavigationAppConfig{} + + // Check if the 'Alerts and Incidents' section is not there if no apps try to register to it + treeRoot := navtree.NavTreeRoot{} + err := service.addAppLinks(&treeRoot, reqCtx) + require.NoError(t, err) + alertsAndIncidentsNode := treeRoot.FindById(navtree.NavIDAlertsAndIncidents) + require.Nil(t, alertsAndIncidentsNode) + + // If there is no 'Alerting' node in the navigation (= alerting not enabled) then we don't auto-create the 'Alerts and Incidents' section + treeRoot = navtree.NavTreeRoot{} service.navigationAppConfig = map[string]NavigationAppConfig{ "test-app1": {SectionID: navtree.NavIDAlertsAndIncidents}, } - - treeRoot := navtree.NavTreeRoot{} - treeRoot.AddSection(&navtree.NavLink{Id: navtree.NavIDAlerting, Text: "Alerting"}) - - err := service.addAppLinks(&treeRoot, reqCtx) + err = service.addAppLinks(&treeRoot, reqCtx) require.NoError(t, err) - require.Equal(t, "Alerts & incidents", treeRoot.Children[0].Text) - require.Equal(t, "Alerting", treeRoot.Children[0].Children[0].Text) - require.Equal(t, "Test app1 name", treeRoot.Children[0].Children[1].Text) + alertsAndIncidentsNode = treeRoot.FindById(navtree.NavIDAlertsAndIncidents) + require.Nil(t, alertsAndIncidentsNode) + + // It should appear and once an app tries to register to it and the `Alerting` nav node is present + treeRoot = navtree.NavTreeRoot{} + treeRoot.AddSection(&navtree.NavLink{Id: navtree.NavIDAlerting, Text: "Alerting"}) + service.navigationAppConfig = map[string]NavigationAppConfig{ + "test-app1": {SectionID: navtree.NavIDAlertsAndIncidents}, + } + err = service.addAppLinks(&treeRoot, reqCtx) + require.NoError(t, err) + alertsAndIncidentsNode = treeRoot.FindById(navtree.NavIDAlertsAndIncidents) + require.NotNil(t, alertsAndIncidentsNode) + require.Len(t, alertsAndIncidentsNode.Children, 2) + require.Equal(t, "Alerting", alertsAndIncidentsNode.Children[0].Text) + require.Equal(t, "Test app1 name", alertsAndIncidentsNode.Children[1].Text) }) - t.Run("Should be able to control app sort order with SortWeight", func(t *testing.T) { + t.Run("Should be able to control app sort order with SortWeight (smaller SortWeight displayed first)", func(t *testing.T) { service.features = featuremgmt.WithFeatures(featuremgmt.FlagTopnav) service.navigationAppConfig = map[string]NavigationAppConfig{ - "test-app2": {SectionID: navtree.NavIDMonitoring, SortWeight: 1}, - "test-app1": {SectionID: navtree.NavIDMonitoring, SortWeight: 2}, + "test-app2": {SectionID: navtree.NavIDMonitoring, SortWeight: 2}, + "test-app1": {SectionID: navtree.NavIDMonitoring, SortWeight: 3}, + "test-app3": {SectionID: navtree.NavIDMonitoring, SortWeight: 1}, } treeRoot := navtree.NavTreeRoot{} - err := service.addAppLinks(&treeRoot, reqCtx) - treeRoot.Sort() + monitoringNode := treeRoot.FindById(navtree.NavIDMonitoring) require.NoError(t, err) - require.Equal(t, "Test app2 name", treeRoot.Children[0].Children[0].Text) - require.Equal(t, "Test app1 name", treeRoot.Children[0].Children[1].Text) + require.Equal(t, "Test app3 name", monitoringNode.Children[0].Text) + require.Equal(t, "Test app2 name", monitoringNode.Children[1].Text) + require.Equal(t, "Test app1 name", monitoringNode.Children[2].Text) }) t.Run("Should replace page from plugin", func(t *testing.T) { service.features = featuremgmt.WithFeatures(featuremgmt.FlagTopnav, featuremgmt.FlagDataConnectionsConsole) + service.navigationAppConfig = map[string]NavigationAppConfig{} 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) + connectionsNode := treeRoot.FindById("connections") + require.Equal(t, "Connections", connectionsNode.Text) + require.Equal(t, "Connect Data", connectionsNode.Children[1].Text) + require.Equal(t, "connections-connect-data", connectionsNode.Children[1].Id) // Original "Connect Data" page + require.Equal(t, "", connectionsNode.Children[1].PluginID) err := service.addAppLinks(&treeRoot, reqCtx) + + // Check if the standalone plugin page appears under the section where we registered it 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) + require.Equal(t, "Connections", connectionsNode.Text) + require.Equal(t, "Connect Data", connectionsNode.Children[1].Text) + require.Equal(t, "standalone-plugin-page-/connections/connect-data", connectionsNode.Children[1].Id) // Overridden "Connect Data" page + require.Equal(t, "test-app3", connectionsNode.Children[1].PluginID) + + // Check if the standalone plugin page does not appear under the app section anymore + // (Also checking if the Default Page got removed) + app3Node := treeRoot.FindById("plugin-page-test-app3") + require.NotNil(t, app3Node) + require.Len(t, app3Node.Children, 1) + require.Equal(t, "Random page", app3Node.Children[0].Text) + + // The plugin item should take the URL of the Default Nav + require.Equal(t, "/a/test-app3/default", app3Node.Url) + }) + + t.Run("Should not register pages under the app plugin section unless AddToNav=true", func(t *testing.T) { + service.features = featuremgmt.WithFeatures(featuremgmt.FlagTopnav, featuremgmt.FlagDataConnectionsConsole) + service.navigationAppPathConfig = map[string]NavigationAppConfig{} // We don't configure it as a standalone plugin page + + treeRoot := navtree.NavTreeRoot{} + treeRoot.AddSection(service.buildDataConnectionsNavLink(reqCtx)) + err := service.addAppLinks(&treeRoot, reqCtx) + require.NoError(t, err) + + // The original core page should exist under the section + connectDataNode := treeRoot.FindById("connections-connect-data") + require.Equal(t, "connections-connect-data", connectDataNode.Id) + require.Equal(t, "", connectDataNode.PluginID) + + // The standalone plugin page should not be found in the navtree at all (as we didn't configure it) + standaloneConnectDataNode := treeRoot.FindById("standalone-plugin-page-/connections/connect-data") + require.Nil(t, standaloneConnectDataNode) + + // Only the pages that have `AddToNav=true` appear under the plugin navigation + app3Node := treeRoot.FindById("plugin-page-test-app3") + require.NotNil(t, app3Node) + require.Len(t, app3Node.Children, 1) // It should only have a single child now + require.Equal(t, "Random page", app3Node.Children[0].Text) }) } diff --git a/public/app/features/plugins/components/AppRootPage.test.tsx b/public/app/features/plugins/components/AppRootPage.test.tsx index bb2bfe9408e..0be95818648 100644 --- a/public/app/features/plugins/components/AppRootPage.test.tsx +++ b/public/app/features/plugins/components/AppRootPage.test.tsx @@ -65,8 +65,32 @@ class RootComponent extends Component { } function renderUnderRouter() { + const appPluginNavItem: NavModelItem = { + text: 'App', + id: 'plugin-page-app', + url: '/a/plugin-page-app', + children: [ + { + text: 'Page 1', + url: '/a/plugin-page-app/page-1', + }, + { + text: 'Page 2', + url: '/a/plugin-page-app/page-2', + }, + ], + }; + + const appsSection = { + text: 'apps', + id: 'apps', + children: [appPluginNavItem], + }; + + appPluginNavItem.parentItem = appsSection; + const store = configureStore(); - const route = { component: AppRootPage }; + const route = { component: () => }; locationService.push('/a/my-awesome-plugin'); render( diff --git a/public/app/features/plugins/components/AppRootPage.tsx b/public/app/features/plugins/components/AppRootPage.tsx index 8469224964f..40e465cd1d9 100644 --- a/public/app/features/plugins/components/AppRootPage.tsx +++ b/public/app/features/plugins/components/AppRootPage.tsx @@ -2,16 +2,14 @@ import { AnyAction, createSlice, PayloadAction } from '@reduxjs/toolkit'; import React, { useCallback, useEffect, useMemo, useReducer } from 'react'; import { createHtmlPortalNode, InPortal, OutPortal } from 'react-reverse-portal'; -import { createSelector } from 'reselect'; +import { useLocation, useRouteMatch, useParams } from 'react-router-dom'; -import { AppEvents, AppPlugin, AppPluginMeta, KeyValue, NavModel, PluginType } from '@grafana/data'; +import { AppEvents, AppPlugin, AppPluginMeta, NavModel, NavModelItem, PluginType } from '@grafana/data'; 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 PageLoader from 'app/core/components/PageLoader/PageLoader'; import { appEvents } from 'app/core/core'; -import { GrafanaRouteComponentProps } from 'app/core/navigation/types'; -import { StoreState, useSelector } from 'app/types'; import { getPluginSettings } from '../pluginSettings'; import { importAppPlugin } from '../plugin_loader'; @@ -19,47 +17,49 @@ import { buildPluginSectionNav } from '../utils'; import { buildPluginPageContext, PluginPageContext } from './PluginPageContext'; -interface RouteParams { +interface Props { + // The ID of the plugin we would like to load and display pluginId: string; + // The root navModelItem for the plugin (root = lives directly under 'home') + pluginNavSection: NavModelItem; } -interface Props extends GrafanaRouteComponentProps {} - interface State { loading: boolean; plugin?: AppPlugin | null; + // Used to display a tab navigation (used before the new Top Nav) pluginNav: NavModel | null; } const initialState: State = { loading: true, pluginNav: null, plugin: null }; -export function AppRootPage({ match, queryParams, location }: Props) { +export function AppRootPage({ pluginId, pluginNavSection }: Props) { + const match = useRouteMatch(); + const queryParams = useParams(); + const location = useLocation(); const [state, dispatch] = useReducer(stateSlice.reducer, initialState); const portalNode = useMemo(() => createHtmlPortalNode(), []); + const currentUrl = config.appSubUrl + location.pathname + location.search; const { plugin, loading, pluginNav } = state; - const sectionNav = useSelector( - createSelector(getNavIndex, (navIndex) => - buildPluginSectionNav(location, pluginNav, navIndex, match.params.pluginId) - ) - ); - const context = useMemo(() => buildPluginPageContext(sectionNav), [sectionNav]); + const navModel = buildPluginSectionNav(pluginNavSection, pluginNav, currentUrl); + const context = useMemo(() => buildPluginPageContext(navModel), [navModel]); useEffect(() => { - loadAppPlugin(match.params.pluginId, dispatch); - }, [match.params.pluginId]); + loadAppPlugin(pluginId, dispatch); + }, [pluginId]); const onNavChanged = useCallback( (newPluginNav: NavModel) => dispatch(stateSlice.actions.changeNav(newPluginNav)), [] ); - if (!plugin || match.params.pluginId !== plugin.meta.id) { - return {loading && }; + if (!plugin || pluginId !== plugin.meta.id) { + return {loading && }; } if (!plugin.root) { return ( - +
No root app page component found
 ); @@ -70,7 +70,7 @@ export function AppRootPage({ match, queryParams, location }: Props) { meta={plugin.meta} basename={match.url} onNavChanged={onNavChanged} - query={queryParams as KeyValue} + query={queryParams} path={location.pathname} /> ); @@ -82,8 +82,8 @@ export function AppRootPage({ match, queryParams, location }: Props) { return ( <> {pluginRoot} - {sectionNav ? ( - + {navModel ? ( + @@ -144,10 +144,6 @@ async function loadAppPlugin(pluginId: string, dispatch: React.Dispatch id.startsWith('standalone-plugin-page-/'); + const isPluginNavModelItem = (model: NavModelItem): model is PluginNavModelItem => + 'pluginId' in model && 'id' in model; + + return Object.values(navIndex) + .filter(isPluginNavModelItem) + .map((navItem) => { + const pluginNavSection = getRootSectionForNode(navItem); + const appPluginUrl = `/a/${navItem.pluginId}`; + const path = isStandalonePluginPage(navItem.id) ? navItem.url || appPluginUrl : appPluginUrl; // Only standalone pages can use core URLs, otherwise we fall back to "/a/:pluginId" + + return { + path, + exact: false, // route everything under this path to the plugin, so it can define more routes under this path + component: () => , + }; + }); +} + +interface PluginNavModelItem extends Omit { + pluginId: string; + id: string; +} diff --git a/public/app/features/plugins/utils.test.ts b/public/app/features/plugins/utils.test.ts index dd6e16f75b1..bca1665ffb6 100644 --- a/public/app/features/plugins/utils.test.ts +++ b/public/app/features/plugins/utils.test.ts @@ -1,6 +1,4 @@ -import { Location as HistoryLocation } from 'history'; - -import { NavIndex, NavModelItem } from '@grafana/data'; +import { NavModelItem } from '@grafana/data'; import { config } from '@grafana/runtime'; import { HOME_NAV_ID } from 'app/core/reducers/navModel'; @@ -52,73 +50,36 @@ describe('buildPluginSectionNav', () => { app1.parentItem = appsSection; - const navIndex: NavIndex = { - apps: appsSection, - [app1.id!]: appsSection.children[0], - [standalonePluginPage.id]: standalonePluginPage, - [HOME_NAV_ID]: home, - }; - it('Should return pluginNav if topnav is disabled', () => { config.featureToggles.topnav = false; - const result = buildPluginSectionNav({} as HistoryLocation, pluginNav, {}, 'app1'); + const result = buildPluginSectionNav(appsSection, pluginNav, '/a/plugin1/page1'); expect(result).toBe(pluginNav); }); it('Should return return section nav if topnav is enabled', () => { config.featureToggles.topnav = true; - const result = buildPluginSectionNav({} as HistoryLocation, pluginNav, navIndex, 'app1'); + const result = buildPluginSectionNav(appsSection, pluginNav, '/a/plugin1/page1'); expect(result?.main.text).toBe('apps'); }); it('Should set active page', () => { config.featureToggles.topnav = true; - const result = buildPluginSectionNav( - { pathname: '/a/plugin1/page2', search: '' } as HistoryLocation, - null, - navIndex, - 'app1' - ); + const result = buildPluginSectionNav(appsSection, null, '/a/plugin1/page2'); expect(result?.main.children![0].children![1].active).toBe(true); expect(result?.node.text).toBe('page2'); }); it('Should set app section to active', () => { config.featureToggles.topnav = true; - const result = buildPluginSectionNav( - { pathname: '/a/plugin1', search: '' } as HistoryLocation, - null, - navIndex, - 'app1' - ); + const result = buildPluginSectionNav(appsSection, null, '/a/plugin1'); expect(result?.main.children![0].active).toBe(true); expect(result?.node.text).toBe('App1'); }); it('Should handle standalone page', () => { config.featureToggles.topnav = true; - const result = buildPluginSectionNav( - { pathname: '/a/app2/config', search: '' } as HistoryLocation, - pluginNav, - navIndex, - 'app2' - ); + const result = buildPluginSectionNav(adminSection, pluginNav, '/a/app2/config'); expect(result?.main.text).toBe('Admin'); expect(result?.node.text).toBe('Standalone page'); }); - - 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 result = buildPluginSectionNav({} as HistoryLocation, pluginNav, navIndex, 'app3'); - expect(result?.main.id).toBe(HOME_NAV_ID); - }); - - it('Should throw error if app has no section', () => { - config.featureToggles.topnav = true; - app1.parentItem = undefined; - const action = () => { - buildPluginSectionNav({} as HistoryLocation, pluginNav, navIndex, 'app1'); - }; - expect(action).toThrowError(); - }); }); diff --git a/public/app/features/plugins/utils.ts b/public/app/features/plugins/utils.ts index 2bdf2885b7c..c28a58e70df 100644 --- a/public/app/features/plugins/utils.ts +++ b/public/app/features/plugins/utils.ts @@ -1,9 +1,5 @@ -import { Location as HistoryLocation } from 'history'; - -import { GrafanaPlugin, NavIndex, NavModel, NavModelItem, PanelPluginMeta, PluginType } from '@grafana/data'; +import { GrafanaPlugin, NavModel, NavModelItem, PanelPluginMeta, PluginType } from '@grafana/data'; import { config } from '@grafana/runtime'; -import { HOME_NAV_ID } from 'app/core/reducers/navModel'; -import { getRootSectionForNode } from 'app/core/selectors/navModel'; import { importPanelPluginFromMeta } from './importPanelPlugin'; import { getPluginSettings } from './pluginSettings'; @@ -35,26 +31,17 @@ export async function loadPlugin(pluginId: string): Promise { } export function buildPluginSectionNav( - location: HistoryLocation, + pluginNavSection: NavModelItem, pluginNav: NavModel | null, - navIndex: NavIndex, - pluginId: string + currentUrl: string ): NavModel | undefined { // When topnav is disabled we only just show pluginNav like before if (!config.featureToggles.topnav) { return pluginNav ?? undefined; } - 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 copiedPluginNavSection = { ...pluginNavSection }; let activePage: NavModelItem | undefined; function setPageToActive(page: NavModelItem, currentUrl: string): NavModelItem { @@ -75,7 +62,7 @@ export function buildPluginSectionNav( } // Find and set active page - section.children = (section?.children ?? []).map((child) => { + copiedPluginNavSection.children = (copiedPluginNavSection?.children ?? []).map((child) => { if (child.children) { return { ...setPageToActive(child, currentUrl), @@ -86,26 +73,5 @@ export function buildPluginSectionNav( return setPageToActive(child, currentUrl); }); - return { main: section, node: activePage ?? section }; -} - -// TODO make work for sub pages -export function getPluginSection(location: HistoryLocation, navIndex: NavIndex, pluginId: string): NavModelItem { - // First check if this page exist in navIndex using path, some plugin pages are not under their own section - const byPath = navIndex[`standalone-plugin-page-${location.pathname}`]; - if (byPath) { - return getRootSectionForNode(byPath); - } - - // 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) { - return navIndex[HOME_NAV_ID]; - } - - if (!navTreeNodeForPlugin.parentItem) { - throw new Error('Could not find plugin section'); - } - - return navTreeNodeForPlugin.parentItem; + return { main: copiedPluginNavSection, node: activePage ?? copiedPluginNavSection }; } diff --git a/public/app/routes/routes.tsx b/public/app/routes/routes.tsx index 8179c38099c..938a38b7e0d 100644 --- a/public/app/routes/routes.tsx +++ b/public/app/routes/routes.tsx @@ -13,6 +13,7 @@ import { getRoutes as getDataConnectionsRoutes } from 'app/features/connections/ import { DATASOURCES_ROUTES } from 'app/features/datasources/constants'; import { getLiveRoutes } from 'app/features/live/pages/routes'; import { getRoutes as getPluginCatalogRoutes } from 'app/features/plugins/admin/routes'; +import { getAppPluginRoutes } from 'app/features/plugins/routes'; import { getProfileRoutes } from 'app/features/profile/routes'; import { AccessControlAction, DashboardRoutes } from 'app/types'; @@ -37,17 +38,13 @@ export function getAppRoutes(): RouteDescriptor[] { path: '/monitoring', component: () => , }, - { - path: '/a/:pluginId', - exact: true, - component: SafeDynamicImport( - () => import(/* webpackChunkName: "AppRootPage" */ 'app/features/plugins/components/AppRootPage') - ), - }, ] : []; return [ + // Based on the Grafana configuration standalone plugin pages can even override and extend existing core pages, or they can register new routes under existing ones. + // In order to make it possible we need to register them first due to how `` is evaluating routes. (This will be unnecessary once/when we upgrade to React Router v6 and start using `` instead.) + ...getAppPluginRoutes(), { path: '/', pageClass: 'page-dashboard', @@ -208,14 +205,6 @@ export function getAppRoutes(): RouteDescriptor[] { ), }, ...topnavRoutes, - { - path: '/a/:pluginId', - exact: false, - // Someday * and will get a ReactRouter under that path! - component: SafeDynamicImport( - () => import(/* webpackChunkName: "AppRootPage" */ 'app/features/plugins/components/AppRootPage') - ), - }, { path: '/org', component: SafeDynamicImport(