From 898450729186b0cf24ecb2e2d6ecfc57ac309d65 Mon Sep 17 00:00:00 2001 From: Ashley Harrison Date: Mon, 3 Oct 2022 15:05:19 +0100 Subject: [PATCH] Navigation: show breadcrumbs correctly when on the home page (#55759) * show breadcrumbs correctly when on the home page * adjust breadcrumb unit tests * update betterer * fix backend tests * update getSectionRoot to look at the home nav id * remove redundant setting of home dashboard * construct a home navmodelitem in the backend * fix cases when the feature toggle is off * fix unit test * fix more unit tests * refactor how buildBreadcrumbs works * use HOME_NAV_ID * move homeNav useSelector into NavToolbar * remove unnecesary cloneDeep * don't need locationUtil here * restore using getUrlForPartial in DashboardPage * special case for the editview query param * remove commented out code * add comment to clarify splice behaviour * slightly cleaner syntax --- pkg/api/dashboard.go | 1 - pkg/api/dashboard_test.go | 1 - pkg/api/dtos/dashboard.go | 1 - pkg/services/navtree/models.go | 3 +- pkg/services/navtree/navtreeimpl/navtree.go | 34 +++++- .../core/components/AppChrome/NavToolbar.tsx | 5 +- .../core/components/Breadcrumbs/utils.test.ts | 100 ++++++++++++++---- .../app/core/components/Breadcrumbs/utils.ts | 29 +++-- .../components/MegaMenu/MegaMenu.test.tsx | 1 - .../app/core/components/MegaMenu/MegaMenu.tsx | 15 +-- public/app/core/reducers/navModel.ts | 11 +- public/app/core/selectors/navModel.ts | 4 +- .../dashboard/containers/DashboardPage.tsx | 3 +- .../app/features/teams/TeamMembers.test.tsx | 1 + 14 files changed, 158 insertions(+), 51 deletions(-) diff --git a/pkg/api/dashboard.go b/pkg/api/dashboard.go index 40d865e3a99..a0cb5fe7732 100644 --- a/pkg/api/dashboard.go +++ b/pkg/api/dashboard.go @@ -538,7 +538,6 @@ func (hs *HTTPServer) GetHomeDashboard(c *models.ReqContext) response.Response { }() dash := dtos.DashboardFullWithMeta{} - dash.Meta.IsHome = true dash.Meta.CanEdit = c.SignedInUser.HasRole(org.RoleEditor) dash.Meta.FolderTitle = "General" dash.Dashboard = simplejson.New() diff --git a/pkg/api/dashboard_test.go b/pkg/api/dashboard_test.go index 09ec76b5e59..c41c9be06a6 100644 --- a/pkg/api/dashboard_test.go +++ b/pkg/api/dashboard_test.go @@ -78,7 +78,6 @@ func TestGetHomeDashboard(t *testing.T) { for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { dash := dtos.DashboardFullWithMeta{} - dash.Meta.IsHome = true dash.Meta.FolderTitle = "General" homeDashJSON, err := os.ReadFile(tc.expectedDashboardPath) diff --git a/pkg/api/dtos/dashboard.go b/pkg/api/dtos/dashboard.go index aaf43fc59cd..3918374cf7f 100644 --- a/pkg/api/dtos/dashboard.go +++ b/pkg/api/dtos/dashboard.go @@ -8,7 +8,6 @@ import ( type DashboardMeta struct { IsStarred bool `json:"isStarred,omitempty"` - IsHome bool `json:"isHome,omitempty"` IsSnapshot bool `json:"isSnapshot,omitempty"` Type string `json:"type,omitempty"` CanSave bool `json:"canSave"` diff --git a/pkg/services/navtree/models.go b/pkg/services/navtree/models.go index be3bb89b889..a1cd112313a 100644 --- a/pkg/services/navtree/models.go +++ b/pkg/services/navtree/models.go @@ -11,7 +11,8 @@ const ( // are negative to ensure that the default items are placed above // any items with default weight. - WeightSavedItems = (iota - 20) * 100 + WeightHome = (iota - 20) * 100 + WeightSavedItems WeightCreate WeightDashboard WeightExplore diff --git a/pkg/services/navtree/navtreeimpl/navtree.go b/pkg/services/navtree/navtreeimpl/navtree.go index 888f7d29b92..c280fa05e9e 100644 --- a/pkg/services/navtree/navtreeimpl/navtree.go +++ b/pkg/services/navtree/navtreeimpl/navtree.go @@ -69,8 +69,12 @@ func (s *ServiceImpl) GetNavTree(c *models.ReqContext, hasEditPerm bool, prefs * hasAccess := ac.HasAccess(s.accessControl, c) treeRoot := &navtree.NavTreeRoot{} + if s.features.IsEnabled(featuremgmt.FlagTopnav) { + treeRoot.AddSection(s.getHomeNode(c, prefs)) + } + if hasAccess(ac.ReqSignedIn, ac.EvalPermission(dashboards.ActionDashboardsRead)) { - starredItemsLinks, err := s.buildStarredItemsNavLinks(c, prefs) + starredItemsLinks, err := s.buildStarredItemsNavLinks(c) if err != nil { return nil, err } @@ -186,6 +190,32 @@ func (s *ServiceImpl) GetNavTree(c *models.ReqContext, hasEditPerm bool, prefs * return treeRoot, nil } +func (s *ServiceImpl) getHomeNode(c *models.ReqContext, prefs *pref.Preference) *navtree.NavLink { + homeUrl := s.cfg.AppSubURL + "/" + homePage := s.cfg.HomePage + + if prefs.HomeDashboardID == 0 && len(homePage) > 0 { + homeUrl = homePage + } + + if prefs.HomeDashboardID != 0 { + slugQuery := models.GetDashboardRefByIdQuery{Id: prefs.HomeDashboardID} + err := s.dashboardService.GetDashboardUIDById(c.Req.Context(), &slugQuery) + if err == nil { + homeUrl = models.GetDashboardUrl(slugQuery.Result.Uid, slugQuery.Result.Slug) + } + } + + return &navtree.NavLink{ + Text: "Home", + Id: "home", + Url: homeUrl, + Icon: "home-alt", + Section: navtree.NavSectionCore, + SortWeight: navtree.WeightHome, + } +} + func (s *ServiceImpl) addHelpLinks(treeRoot *navtree.NavTreeRoot, c *models.ReqContext) { if setting.HelpEnabled { helpVersion := fmt.Sprintf(`%s v%s (%s)`, setting.ApplicationName, setting.BuildVersion, setting.BuildCommit) @@ -256,7 +286,7 @@ func (s *ServiceImpl) getProfileNode(c *models.ReqContext) *navtree.NavLink { } } -func (s *ServiceImpl) buildStarredItemsNavLinks(c *models.ReqContext, prefs *pref.Preference) ([]*navtree.NavLink, error) { +func (s *ServiceImpl) buildStarredItemsNavLinks(c *models.ReqContext) ([]*navtree.NavLink, error) { starredItemsChildNavs := []*navtree.NavLink{} query := star.GetUserStarsQuery{ diff --git a/public/app/core/components/AppChrome/NavToolbar.tsx b/public/app/core/components/AppChrome/NavToolbar.tsx index ef4ead80e06..12c4c46d481 100644 --- a/public/app/core/components/AppChrome/NavToolbar.tsx +++ b/public/app/core/components/AppChrome/NavToolbar.tsx @@ -3,6 +3,8 @@ import React from 'react'; import { GrafanaTheme2, NavModelItem } from '@grafana/data'; import { Icon, IconButton, ToolbarButton, useStyles2 } from '@grafana/ui'; +import { HOME_NAV_ID } from 'app/core/reducers/navModel'; +import { useSelector } from 'app/types'; import { Breadcrumbs } from '../Breadcrumbs/Breadcrumbs'; import { buildBreadcrumbs } from '../Breadcrumbs/utils'; @@ -29,8 +31,9 @@ export function NavToolbar({ onToggleSearchBar, onToggleKioskMode, }: Props) { + const homeNav = useSelector((state) => state.navIndex)[HOME_NAV_ID]; const styles = useStyles2(getStyles); - const breadcrumbs = buildBreadcrumbs(sectionNav, pageNav); + const breadcrumbs = buildBreadcrumbs(homeNav, sectionNav, pageNav); return (
diff --git a/public/app/core/components/Breadcrumbs/utils.test.ts b/public/app/core/components/Breadcrumbs/utils.test.ts index 2c9fa635af3..c84ca5d0c6d 100644 --- a/public/app/core/components/Breadcrumbs/utils.test.ts +++ b/public/app/core/components/Breadcrumbs/utils.test.ts @@ -2,26 +2,20 @@ import { NavModelItem } from '@grafana/data'; import { buildBreadcrumbs } from './utils'; +const mockHomeNav: NavModelItem = { + text: 'Home', + url: '/home', + id: 'home', +}; + describe('breadcrumb utils', () => { describe('buildBreadcrumbs', () => { - it('includes the home breadcrumb at the root', () => { - const sectionNav: NavModelItem = { - text: 'My section', - url: '/my-section', - }; - const result = buildBreadcrumbs(sectionNav); - expect(result[0]).toEqual({ href: '/', text: 'Home' }); - }); - it('includes breadcrumbs for the section nav', () => { const sectionNav: NavModelItem = { text: 'My section', url: '/my-section', }; - expect(buildBreadcrumbs(sectionNav)).toEqual([ - { href: '/', text: 'Home' }, - { text: 'My section', href: '/my-section' }, - ]); + expect(buildBreadcrumbs(mockHomeNav, sectionNav)).toEqual([{ text: 'My section', href: '/my-section' }]); }); it('includes breadcrumbs for the page nav', () => { @@ -34,8 +28,7 @@ describe('breadcrumb utils', () => { text: 'My page', url: '/my-page', }; - expect(buildBreadcrumbs(sectionNav, pageNav)).toEqual([ - { href: '/', text: 'Home' }, + expect(buildBreadcrumbs(mockHomeNav, sectionNav, pageNav)).toEqual([ { text: 'My section', href: '/my-section' }, { text: 'My page', href: '/my-page' }, ]); @@ -50,8 +43,7 @@ describe('breadcrumb utils', () => { url: '/my-parent-section', }, }; - expect(buildBreadcrumbs(sectionNav)).toEqual([ - { href: '/', text: 'Home' }, + expect(buildBreadcrumbs(mockHomeNav, sectionNav)).toEqual([ { text: 'My parent section', href: '/my-parent-section' }, { text: 'My section', href: '/my-section' }, ]); @@ -74,13 +66,83 @@ describe('breadcrumb utils', () => { url: '/my-parent-section', }, }; - expect(buildBreadcrumbs(sectionNav, pageNav)).toEqual([ - { href: '/', text: 'Home' }, + expect(buildBreadcrumbs(mockHomeNav, sectionNav, pageNav)).toEqual([ { text: 'My parent section', href: '/my-parent-section' }, { text: 'My section', href: '/my-section' }, { text: 'My parent page', href: '/my-parent-page' }, { text: 'My page', href: '/my-page' }, ]); }); + + it('shortcircuits if the home nav is found early', () => { + const pageNav: NavModelItem = { + text: 'My page', + url: '/my-page', + parentItem: { + text: 'My parent page', + url: '/home', + }, + }; + const sectionNav: NavModelItem = { + text: 'My section', + url: '/my-section', + parentItem: { + text: 'My parent section', + url: '/my-parent-section', + }, + }; + expect(buildBreadcrumbs(mockHomeNav, sectionNav, pageNav)).toEqual([ + { text: 'Home', href: '/home' }, + { text: 'My page', href: '/my-page' }, + ]); + }); + + it('matches the home nav ignoring query parameters', () => { + const pageNav: NavModelItem = { + text: 'My page', + url: '/my-page', + parentItem: { + text: 'My parent page', + url: '/home?orgId=1', + }, + }; + const sectionNav: NavModelItem = { + text: 'My section', + url: '/my-section', + parentItem: { + text: 'My parent section', + url: '/my-parent-section', + }, + }; + expect(buildBreadcrumbs(mockHomeNav, sectionNav, pageNav)).toEqual([ + { text: 'Home', href: '/home?orgId=1' }, + { text: 'My page', href: '/my-page' }, + ]); + }); + + it('does not match the home nav if the editview param is different', () => { + const pageNav: NavModelItem = { + text: 'My page', + url: '/my-page', + parentItem: { + text: 'My parent page', + url: '/home?orgId=1&editview=settings', + }, + }; + const sectionNav: NavModelItem = { + text: 'My section', + url: '/my-section', + parentItem: { + text: 'My parent section', + url: '/my-parent-section', + }, + }; + expect(buildBreadcrumbs(mockHomeNav, sectionNav, pageNav)).toEqual([ + { text: 'My parent section', href: '/my-parent-section' }, + { text: 'My section', href: '/my-section' }, + { text: 'My parent page', href: '/home?orgId=1&editview=settings' }, + { text: 'My page', href: '/my-page' }, + ]); + }); }); }); diff --git a/public/app/core/components/Breadcrumbs/utils.ts b/public/app/core/components/Breadcrumbs/utils.ts index a5ec30b3bc2..4c881c5d644 100644 --- a/public/app/core/components/Breadcrumbs/utils.ts +++ b/public/app/core/components/Breadcrumbs/utils.ts @@ -2,24 +2,37 @@ import { NavModelItem } from '@grafana/data'; import { Breadcrumb } from './types'; -export function buildBreadcrumbs(sectionNav: NavModelItem, pageNav?: NavModelItem) { - const crumbs: Breadcrumb[] = [{ href: '/', text: 'Home' }]; +export function buildBreadcrumbs(homeNav: NavModelItem, sectionNav: NavModelItem, pageNav?: NavModelItem) { + const crumbs: Breadcrumb[] = []; + let foundHome = false; function addCrumbs(node: NavModelItem) { + // construct the URL to match + // we want to ignore query params except for the editview query param + const urlSearchParams = new URLSearchParams(node.url?.split('?')[1]); + let urlToMatch = `${node.url?.split('?')[0]}`; + if (urlSearchParams.has('editview')) { + urlToMatch += `?editview=${urlSearchParams.get('editview')}`; + } + if (!foundHome && !node.hideFromBreadcrumbs) { + if (urlToMatch === homeNav.url) { + crumbs.unshift({ text: homeNav.text, href: node.url ?? '' }); + foundHome = true; + } else { + crumbs.unshift({ text: node.text, href: node.url ?? '' }); + } + } + if (node.parentItem) { addCrumbs(node.parentItem); } - - if (!node.hideFromBreadcrumbs) { - crumbs.push({ text: node.text, href: node.url ?? '' }); - } } - addCrumbs(sectionNav); - if (pageNav) { addCrumbs(pageNav); } + addCrumbs(sectionNav); + return crumbs; } diff --git a/public/app/core/components/MegaMenu/MegaMenu.test.tsx b/public/app/core/components/MegaMenu/MegaMenu.test.tsx index c8cdd3b531c..f06ae94136a 100644 --- a/public/app/core/components/MegaMenu/MegaMenu.test.tsx +++ b/public/app/core/components/MegaMenu/MegaMenu.test.tsx @@ -56,7 +56,6 @@ describe('MegaMenu', () => { setup(); expect(await screen.findByTestId('navbarmenu')).toBeInTheDocument(); - expect(await screen.findByRole('link', { name: 'Home' })).toBeInTheDocument(); expect(await screen.findByRole('link', { name: 'Section name' })).toBeInTheDocument(); }); diff --git a/public/app/core/components/MegaMenu/MegaMenu.tsx b/public/app/core/components/MegaMenu/MegaMenu.tsx index 4aa604a3175..a6115e7988f 100644 --- a/public/app/core/components/MegaMenu/MegaMenu.tsx +++ b/public/app/core/components/MegaMenu/MegaMenu.tsx @@ -3,8 +3,7 @@ import { cloneDeep } from 'lodash'; import React from 'react'; import { useLocation } from 'react-router-dom'; -import { GrafanaTheme2, NavModelItem, NavSection } from '@grafana/data'; -import { config } from '@grafana/runtime'; +import { GrafanaTheme2, NavSection } from '@grafana/data'; import { useTheme2 } from '@grafana/ui'; import { useSelector } from 'app/types'; @@ -23,16 +22,6 @@ export const MegaMenu = React.memo(({ onClose, searchBarHidden }) => { const styles = getStyles(theme); const location = useLocation(); - const homeItem: NavModelItem = enrichWithInteractionTracking( - { - id: 'home', - text: 'Home', - url: config.appSubUrl || '/', - icon: 'home-alt', - }, - true - ); - const navTree = cloneDeep(navBarTree); const coreItems = navTree @@ -46,7 +35,7 @@ export const MegaMenu = React.memo(({ onClose, searchBarHidden }) => { location ).map((item) => enrichWithInteractionTracking(item, true)); - const navItems = [homeItem, ...coreItems, ...pluginItems, ...configItems]; + const navItems = [...coreItems, ...pluginItems, ...configItems]; const activeItem = getActiveItem(navItems, location.pathname); diff --git a/public/app/core/reducers/navModel.ts b/public/app/core/reducers/navModel.ts index 16ec84daffb..b341176e2ad 100644 --- a/public/app/core/reducers/navModel.ts +++ b/public/app/core/reducers/navModel.ts @@ -4,10 +4,19 @@ import { cloneDeep } from 'lodash'; import { NavIndex, NavModel, NavModelItem } from '@grafana/data'; import config from 'app/core/config'; +export const HOME_NAV_ID = 'home'; + export function buildInitialState(): NavIndex { const navIndex: NavIndex = {}; const rootNodes = cloneDeep(config.bootData.navTree as NavModelItem[]); - buildNavIndex(navIndex, rootNodes); + const homeNav = rootNodes.find((node) => node.id === HOME_NAV_ID); + + // set home as parent for the rootNodes + buildNavIndex(navIndex, rootNodes, homeNav); + // remove circular parent reference on the home node + if (navIndex[HOME_NAV_ID]) { + delete navIndex[HOME_NAV_ID].parentItem; + } return navIndex; } diff --git a/public/app/core/selectors/navModel.ts b/public/app/core/selectors/navModel.ts index 53e4259eaea..67a345946ba 100644 --- a/public/app/core/selectors/navModel.ts +++ b/public/app/core/selectors/navModel.ts @@ -1,5 +1,7 @@ import { NavModel, NavModelItem, NavIndex } from '@grafana/data'; +import { HOME_NAV_ID } from '../reducers/navModel'; + const getNotFoundModel = (): NavModel => { const node: NavModelItem = { id: 'not-found', @@ -35,7 +37,7 @@ export const getNavModel = (navIndex: NavIndex, id: string, fallback?: NavModel, }; function getSectionRoot(node: NavModelItem): NavModelItem { - return node.parentItem ? getSectionRoot(node.parentItem) : node; + return node.parentItem && node.parentItem.id !== HOME_NAV_ID ? getSectionRoot(node.parentItem) : node; } function enrichNodeWithActiveState(node: NavModelItem, activeId: string): NavModelItem { diff --git a/public/app/features/dashboard/containers/DashboardPage.tsx b/public/app/features/dashboard/containers/DashboardPage.tsx index da6e4507253..885607f5a0b 100644 --- a/public/app/features/dashboard/containers/DashboardPage.tsx +++ b/public/app/features/dashboard/containers/DashboardPage.tsx @@ -2,7 +2,7 @@ import { cx } from '@emotion/css'; import React, { PureComponent } from 'react'; import { connect, ConnectedProps } from 'react-redux'; -import { locationUtil, NavModel, NavModelItem, TimeRange, PageLayoutType } from '@grafana/data'; +import { NavModel, NavModelItem, TimeRange, PageLayoutType, locationUtil } from '@grafana/data'; import { selectors } from '@grafana/e2e-selectors'; import { config, locationService } from '@grafana/runtime'; import { Themeable2, withTheme2 } from '@grafana/ui'; @@ -459,6 +459,7 @@ function updateStatePageNavFromProps(props: Props, state: State): State { ...pageNav, text: `${state.editPanel ? 'Edit' : 'View'} panel`, parentItem: pageNav, + url: undefined, }; } diff --git a/public/app/features/teams/TeamMembers.test.tsx b/public/app/features/teams/TeamMembers.test.tsx index 4900e8e3a57..16cc52c1a3b 100644 --- a/public/app/features/teams/TeamMembers.test.tsx +++ b/public/app/features/teams/TeamMembers.test.tsx @@ -21,6 +21,7 @@ jest.mock('@grafana/runtime', () => ({ get: jest.fn().mockResolvedValue([{ userId: 1, login: 'Test' }]), }), config: { + ...jest.requireActual('@grafana/runtime').config, bootData: { navTree: [], user: {} }, }, }));