From 9680722b78b0cde593019778878c087c5299271a Mon Sep 17 00:00:00 2001 From: Alex Khomenko Date: Mon, 7 Oct 2024 12:11:57 +0300 Subject: [PATCH] Dashboards: Switch to useParams hook (#94060) * Update DashboardScenePage * Update SoloPanelPage * Update DashboardPage * Cleanup * Switch to useLocation * Do not use location from history --- .../pages/DashboardScenePage.test.tsx | 34 ++++++++----------- .../pages/DashboardScenePage.tsx | 33 ++++++++---------- .../dashboard-scene/solo/SoloPanelPage.tsx | 8 +++-- .../containers/DashboardPage.test.tsx | 6 ++-- .../dashboard/containers/DashboardPage.tsx | 28 ++++++++------- .../containers/DashboardPageProxy.test.tsx | 30 ++++++++-------- .../containers/DashboardPageProxy.tsx | 26 +++++++------- .../containers/SoloPanelPage.test.tsx | 6 ---- 8 files changed, 80 insertions(+), 91 deletions(-) diff --git a/public/app/features/dashboard-scene/pages/DashboardScenePage.test.tsx b/public/app/features/dashboard-scene/pages/DashboardScenePage.test.tsx index 2dd6ebcba26..fa18446d6b2 100644 --- a/public/app/features/dashboard-scene/pages/DashboardScenePage.test.tsx +++ b/public/app/features/dashboard-scene/pages/DashboardScenePage.test.tsx @@ -1,6 +1,7 @@ import { act, fireEvent, render, screen, waitFor } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import { cloneDeep } from 'lodash'; +import { useParams } from 'react-router-dom-v5-compat'; import { TestProvider } from 'test/helpers/TestProvider'; import { getGrafanaContextMock } from 'test/mocks/getGrafanaContextMock'; @@ -48,6 +49,11 @@ jest.mock('@grafana/runtime', () => ({ }), })); +jest.mock('react-router-dom-v5-compat', () => ({ + ...jest.requireActual('react-router-dom-v5-compat'), + useParams: jest.fn().mockReturnValue({ uid: 'my-dash-uid' }), +})); + const getPluginLinkExtensionsMock = jest.mocked(getPluginLinkExtensions); function setup({ routeProps }: { routeProps?: Partial } = {}) { @@ -55,12 +61,6 @@ function setup({ routeProps }: { routeProps?: Partial { it('Can render dashboard', async () => { setup(); - await waitForDashbordToRender(); + await waitForDashboardToRender(); expect(await screen.findByTitle('Panel A')).toBeInTheDocument(); expect(await screen.findByText('Content A')).toBeInTheDocument(); @@ -175,7 +175,7 @@ describe('DashboardScenePage', () => { it('routeReloadCounter should trigger reload', async () => { const { rerender, props } = setup(); - await waitForDashbordToRender(); + await waitForDashboardToRender(); expect(await screen.findByTitle('Panel A')).toBeInTheDocument(); @@ -196,7 +196,7 @@ describe('DashboardScenePage', () => { it('Can inspect panel', async () => { setup(); - await waitForDashbordToRender(); + await waitForDashboardToRender(); expect(screen.queryByText('Inspect: Panel B')).not.toBeInTheDocument(); @@ -218,7 +218,7 @@ describe('DashboardScenePage', () => { it('Can view panel in fullscreen', async () => { setup(); - await waitForDashbordToRender(); + await waitForDashboardToRender(); expect(await screen.findByTitle('Panel A')).toBeInTheDocument(); @@ -239,7 +239,7 @@ describe('DashboardScenePage', () => { it('shows and hides empty state when panels are added and removed', async () => { setup(); - await waitForDashbordToRender(); + await waitForDashboardToRender(); expect(await screen.queryByText('Start your new dashboard by adding a visualization')).not.toBeInTheDocument(); @@ -272,7 +272,7 @@ describe('DashboardScenePage', () => { setup(); - await waitForDashbordToRender(); + await waitForDashboardToRender(); const panelAMenu = await screen.findByLabelText('Menu for panel with title Panel A'); expect(panelAMenu).toBeInTheDocument(); @@ -283,21 +283,17 @@ describe('DashboardScenePage', () => { describe('home page', () => { it('should render the dashboard when the route is home', async () => { + (useParams as jest.Mock).mockReturnValue({}); setup({ routeProps: { route: { ...getRouteComponentProps().route, routeName: DashboardRoutes.Home, }, - match: { - ...getRouteComponentProps().match, - path: '/', - params: {}, - }, }, }); - await waitForDashbordToRender(); + await waitForDashboardToRender(); expect(await screen.findByTitle('Panel A')).toBeInTheDocument(); expect(await screen.findByText('Content A')).toBeInTheDocument(); @@ -328,7 +324,7 @@ function CustomVizPanel(props: VizProps) { return
{props.options.content}
; } -async function waitForDashbordToRender() { +async function waitForDashboardToRender() { expect(await screen.findByText('Last 6 hours')).toBeInTheDocument(); expect(await screen.findByTitle('Panel A')).toBeInTheDocument(); } diff --git a/public/app/features/dashboard-scene/pages/DashboardScenePage.tsx b/public/app/features/dashboard-scene/pages/DashboardScenePage.tsx index ada4df7acac..59b0f8bd8b5 100644 --- a/public/app/features/dashboard-scene/pages/DashboardScenePage.tsx +++ b/public/app/features/dashboard-scene/pages/DashboardScenePage.tsx @@ -1,5 +1,6 @@ // Libraries import { useEffect, useMemo } from 'react'; +import { useParams } from 'react-router-dom-v5-compat'; import { usePrevious } from 'react-use'; import { PageLayoutType } from '@grafana/data'; @@ -17,13 +18,15 @@ import { DashboardPrompt } from '../saving/DashboardPrompt'; import { getDashboardScenePageStateManager } from './DashboardScenePageStateManager'; -export interface Props extends GrafanaRouteComponentProps {} +export interface Props + extends Omit, 'match'> {} -export function DashboardScenePage({ match, route, queryParams, history }: Props) { - const prevMatch = usePrevious(match); +export function DashboardScenePage({ route, queryParams, history }: Props) { + const params = useParams(); + const { type, slug, uid } = params; + const prevMatch = usePrevious({ params }); const stateManager = getDashboardScenePageStateManager(); const { dashboard, isLoading, loadError } = stateManager.useState(); - // After scene migration is complete and we get rid of old dashboard we should refactor dashboardWatcher so this route reload is not need const routeReloadCounter = (history.location.state as any)?.routeReloadCounter; @@ -31,14 +34,14 @@ export function DashboardScenePage({ match, route, queryParams, history }: Props const comingFromExplore = useMemo(() => { return Boolean(store.getObject(DASHBOARD_FROM_LS_KEY)); // eslint-disable-next-line react-hooks/exhaustive-deps - }, [match.params.uid, match.params.slug, match.params.type]); + }, [uid, slug, type]); useEffect(() => { - if (route.routeName === DashboardRoutes.Normal && match.params.type === 'snapshot') { - stateManager.loadSnapshot(match.params.slug!); + if (route.routeName === DashboardRoutes.Normal && type === 'snapshot') { + stateManager.loadSnapshot(slug!); } else { stateManager.loadDashboard({ - uid: match.params.uid ?? '', + uid: uid ?? '', route: route.routeName as DashboardRoutes, urlFolderUid: queryParams.folderUid, keepDashboardFromExploreInLocalStorage: false, @@ -48,15 +51,7 @@ export function DashboardScenePage({ match, route, queryParams, history }: Props return () => { stateManager.clearState(); }; - }, [ - stateManager, - match.params.uid, - route.routeName, - queryParams.folderUid, - routeReloadCounter, - match.params.slug, - match.params.type, - ]); + }, [stateManager, uid, route.routeName, queryParams.folderUid, routeReloadCounter, slug, type]); // Effect that handles explore->dashboards workflow useEffect(() => { @@ -84,9 +79,9 @@ export function DashboardScenePage({ match, route, queryParams, history }: Props } // Do not render anything when transitioning from one dashboard to another - // A bit tricky for transition to or from Home dashbord that does not have a uid in the url (but could have it in the dashboard model) + // A bit tricky for transition to or from Home dashboard that does not have a uid in the url (but could have it in the dashboard model) // if prevMatch is undefined we are going from normal route to home route or vice versa - if (match.params.type !== 'snapshot' && (!prevMatch || match.params.uid !== prevMatch?.params.uid)) { + if (type !== 'snapshot' && (!prevMatch || uid !== prevMatch?.params.uid)) { console.log('skipping rendering'); return null; } diff --git a/public/app/features/dashboard-scene/solo/SoloPanelPage.tsx b/public/app/features/dashboard-scene/solo/SoloPanelPage.tsx index 6fe49965f4d..6747828b974 100644 --- a/public/app/features/dashboard-scene/solo/SoloPanelPage.tsx +++ b/public/app/features/dashboard-scene/solo/SoloPanelPage.tsx @@ -1,6 +1,7 @@ // Libraries import { css } from '@emotion/css'; import { useEffect } from 'react'; +import { useParams } from 'react-router-dom-v5-compat'; import { GrafanaTheme2 } from '@grafana/data'; import { Alert, Spinner, useStyles2 } from '@grafana/ui'; @@ -20,14 +21,15 @@ export interface Props extends GrafanaRouteComponentProps { - stateManager.loadDashboard({ uid: match.params.uid!, route: DashboardRoutes.Embedded }); + stateManager.loadDashboard({ uid, route: DashboardRoutes.Embedded }); return () => stateManager.clearState(); - }, [stateManager, match, queryParams]); + }, [stateManager, queryParams, uid]); if (!queryParams.panelId) { return ; diff --git a/public/app/features/dashboard/containers/DashboardPage.test.tsx b/public/app/features/dashboard/containers/DashboardPage.test.tsx index f5557c434fa..a773197e7b1 100644 --- a/public/app/features/dashboard/containers/DashboardPage.test.tsx +++ b/public/app/features/dashboard/containers/DashboardPage.test.tsx @@ -100,9 +100,9 @@ function setup(propOverrides?: Partial) { const props: Props = { ...getRouteComponentProps({ - match: { params: { slug: 'my-dash', uid: '11' }, isExact: false, path: '', url: '' }, route: { routeName: DashboardRoutes.Normal } as RouteDescriptor, }), + params: { slug: 'my-dash', uid: '11' }, navIndex: { 'dashboards/browse': { text: 'Dashboards', @@ -166,9 +166,9 @@ describe('DashboardPage', () => { it('only calls initDashboard once when wrapped in AppChrome', async () => { const props: Props = { ...getRouteComponentProps({ - match: { params: { slug: 'my-dash', uid: '11' }, isExact: true, path: '', url: '' }, route: { routeName: DashboardRoutes.Normal } as RouteDescriptor, }), + params: { slug: 'my-dash', uid: '11' }, navIndex: { 'dashboards/browse': { text: 'Dashboards', @@ -270,7 +270,7 @@ describe('DashboardPage', () => { const { rerender } = setup(); rerender({ dashboard: getTestDashboard() }); rerender({ - match: { params: { uid: 'new-uid' }, isExact: false, path: '', url: '' }, + params: { uid: 'new-uid' }, dashboard: getTestDashboard({ title: 'Another dashboard' }), }); await waitFor(() => { diff --git a/public/app/features/dashboard/containers/DashboardPage.tsx b/public/app/features/dashboard/containers/DashboardPage.tsx index 51df6b61d11..cf838fa2aca 100644 --- a/public/app/features/dashboard/containers/DashboardPage.tsx +++ b/public/app/features/dashboard/containers/DashboardPage.tsx @@ -66,9 +66,11 @@ const mapDispatchToProps = { const connector = connect(mapStateToProps, mapDispatchToProps); +export type DashboardPageParams = { slug: string; uid: string; type: string; accessToken: string }; export type Props = Themeable2 & - GrafanaRouteComponentProps & - ConnectedProps; + Omit, 'match'> & + // The params returned from useParams are all optional, so we need to match that type here + ConnectedProps & { params: Partial }; export interface State { editPanel: PanelModel | null; @@ -138,7 +140,7 @@ export class UnthemedDashboardPage extends PureComponent { componentDidMount() { this.initDashboard(); - this.forceRouteReloadCounter = (this.props.history.location.state as any)?.routeReloadCounter || 0; + this.forceRouteReloadCounter = (this.props.location.state as any)?.routeReloadCounter || 0; } componentWillUnmount() { @@ -151,21 +153,21 @@ export class UnthemedDashboardPage extends PureComponent { } initDashboard() { - const { dashboard, match, queryParams } = this.props; + const { dashboard, params, queryParams } = this.props; if (dashboard) { this.closeDashboard(); } this.props.initDashboard({ - urlSlug: match.params.slug, - urlUid: match.params.uid, - urlType: match.params.type, + urlSlug: params.slug, + urlUid: params.uid, + urlType: params.type, urlFolderUid: queryParams.folderUid, panelType: queryParams.panelType, routeName: this.props.route.routeName, fixUrl: true, - accessToken: match.params.accessToken, + accessToken: params.accessToken, keybindingSrv: this.context.keybindings, }); @@ -174,15 +176,15 @@ export class UnthemedDashboardPage extends PureComponent { } componentDidUpdate(prevProps: Props, prevState: State) { - const { dashboard, match, templateVarsChangedInUrl } = this.props; - const routeReloadCounter = (this.props.history.location.state as any)?.routeReloadCounter; + const { dashboard, params, templateVarsChangedInUrl } = this.props; + const routeReloadCounter = (this.props.location.state as any)?.routeReloadCounter; if (!dashboard) { return; } if ( - prevProps.match.params.uid !== match.params.uid || + prevProps.params.uid !== params.uid || (routeReloadCounter !== undefined && this.forceRouteReloadCounter !== routeReloadCounter) ) { this.initDashboard(); @@ -529,7 +531,7 @@ function updateStatePageNavFromProps(props: Props, state: State): State { if (!pageNav || dashboard.title !== pageNav.text || dashboard.meta.folderUrl !== pageNav.parentItem?.url) { pageNav = { text: dashboard.title, - url: locationUtil.getUrlForPartial(props.history.location, { + url: locationUtil.getUrlForPartial(props.location, { editview: null, editPanel: null, viewPanel: null, @@ -539,7 +541,7 @@ function updateStatePageNavFromProps(props: Props, state: State): State { if (props.route.routeName === DashboardRoutes.Path) { sectionNav = getRootContentNavModel(); - const pageNav = getPageNavFromSlug(props.match.params.slug!); + const pageNav = getPageNavFromSlug(props.params.slug!); if (pageNav?.parentItem) { pageNav.parentItem = pageNav.parentItem; } diff --git a/public/app/features/dashboard/containers/DashboardPageProxy.test.tsx b/public/app/features/dashboard/containers/DashboardPageProxy.test.tsx index 178c4b96cef..54f6b0c81fe 100644 --- a/public/app/features/dashboard/containers/DashboardPageProxy.test.tsx +++ b/public/app/features/dashboard/containers/DashboardPageProxy.test.tsx @@ -1,4 +1,5 @@ import { act, screen, waitFor } from '@testing-library/react'; +import { useParams } from 'react-router-dom-v5-compat'; import { Props } from 'react-virtualized-auto-sizer'; import { render } from 'test/test-utils'; @@ -80,14 +81,19 @@ jest.mock('app/features/dashboard/api/dashboard_api', () => ({ }), })); -function setup(props: Partial) { +jest.mock('react-router-dom-v5-compat', () => ({ + ...jest.requireActual('react-router-dom-v5-compat'), + useParams: jest.fn().mockReturnValue({}), +})); + +function setup(props: Partial & { uid?: string }) { + (useParams as jest.Mock).mockReturnValue({ uid: props.uid }); return render( null, path: '/' }} - match={{ params: {}, isExact: true, path: '/', url: '/' }} {...props} /> ); @@ -104,7 +110,6 @@ describe('DashboardPageProxy', () => { act(() => { setup({ route: { routeName: DashboardRoutes.Home, component: () => null, path: '/' }, - match: { params: {}, isExact: true, path: '/', url: '/' }, }); }); @@ -119,7 +124,7 @@ describe('DashboardPageProxy', () => { act(() => { setup({ route: { routeName: DashboardRoutes.Normal, component: () => null, path: '/' }, - match: { params: { uid: 'abc-def' }, isExact: true, path: '/', url: '/' }, + uid: 'abc-def', }); }); @@ -140,7 +145,7 @@ describe('DashboardPageProxy', () => { act(() => { setup({ route: { routeName: DashboardRoutes.Home, component: () => null, path: '/' }, - match: { params: { uid: '' }, isExact: true, path: '/', url: '/' }, + uid: '', }); }); @@ -154,7 +159,7 @@ describe('DashboardPageProxy', () => { act(() => { setup({ route: { routeName: DashboardRoutes.Normal, component: () => null, path: '/' }, - match: { params: { uid: 'abc-def' }, isExact: true, path: '/', url: '/' }, + uid: 'abc-def', }); }); await waitFor(() => { @@ -169,14 +174,7 @@ describe('DashboardPageProxy', () => { act(() => { setup({ route: { routeName: DashboardRoutes.Home, component: () => null, path: '/' }, - match: { - params: { - uid: '', - }, - isExact: true, - path: '/', - url: '/', - }, + uid: '', }); }); @@ -190,7 +188,7 @@ describe('DashboardPageProxy', () => { act(() => { setup({ route: { routeName: DashboardRoutes.Normal, component: () => null, path: '/' }, - match: { params: { uid: 'uid' }, isExact: true, path: '/', url: '/' }, + uid: 'uid', }); }); await waitFor(() => { @@ -203,7 +201,7 @@ describe('DashboardPageProxy', () => { act(() => { setup({ route: { routeName: DashboardRoutes.Normal, component: () => null, path: '/' }, - match: { params: { uid: 'wrongUID' }, isExact: true, path: '/', url: '/' }, + uid: 'wrongUID', }); }); await waitFor(() => { diff --git a/public/app/features/dashboard/containers/DashboardPageProxy.tsx b/public/app/features/dashboard/containers/DashboardPageProxy.tsx index 9a00b266ba0..e45bb442aee 100644 --- a/public/app/features/dashboard/containers/DashboardPageProxy.tsx +++ b/public/app/features/dashboard/containers/DashboardPageProxy.tsx @@ -1,3 +1,4 @@ +import { useLocation, useParams } from 'react-router-dom-v5-compat'; import { useAsync } from 'react-use'; import { config } from '@grafana/runtime'; @@ -6,12 +7,12 @@ import DashboardScenePage from 'app/features/dashboard-scene/pages/DashboardScen import { getDashboardScenePageStateManager } from 'app/features/dashboard-scene/pages/DashboardScenePageStateManager'; import { DashboardRoutes } from 'app/types'; -import DashboardPage from './DashboardPage'; +import DashboardPage, { DashboardPageParams } from './DashboardPage'; import { DashboardPageRouteParams, DashboardPageRouteSearchParams } from './types'; -export type DashboardPageProxyProps = GrafanaRouteComponentProps< - DashboardPageRouteParams, - DashboardPageRouteSearchParams +export type DashboardPageProxyProps = Omit< + GrafanaRouteComponentProps, + 'match' >; // This proxy component is used for Dashboard -> Scenes migration. @@ -19,6 +20,8 @@ export type DashboardPageProxyProps = GrafanaRouteComponentProps< function DashboardPageProxy(props: DashboardPageProxyProps) { const forceScenes = props.queryParams.scenes === true; const forceOld = props.queryParams.scenes === false; + const params = useParams(); + const location = useLocation(); if (forceScenes || (config.featureToggles.dashboardScene && !forceOld)) { return ; @@ -26,34 +29,33 @@ function DashboardPageProxy(props: DashboardPageProxyProps) { const stateManager = getDashboardScenePageStateManager(); const isScenesSupportedRoute = Boolean( - props.route.routeName === DashboardRoutes.Home || - (props.route.routeName === DashboardRoutes.Normal && props.match.params.uid) + props.route.routeName === DashboardRoutes.Home || (props.route.routeName === DashboardRoutes.Normal && params.uid) ); // We pre-fetch dashboard to render dashboard page component depending on dashboard permissions. // To avoid querying single dashboard multiple times, stateManager.fetchDashboard uses a simple, short-lived cache. // eslint-disable-next-line react-hooks/rules-of-hooks const dashboard = useAsync(async () => { - if (props.match.params.type === 'snapshot') { + if (params.type === 'snapshot') { return null; } return stateManager.fetchDashboard({ route: props.route.routeName as DashboardRoutes, - uid: props.match.params.uid ?? '', + uid: params.uid ?? '', keepDashboardFromExploreInLocalStorage: true, }); - }, [props.match.params.uid, props.route.routeName]); + }, [params.uid, props.route.routeName]); if (!config.featureToggles.dashboardSceneForViewers) { - return ; + return ; } if (dashboard.loading) { return null; } - if (dashboard?.value?.dashboard?.uid !== props.match.params.uid && dashboard.value?.meta?.isNew !== true) { + if (dashboard?.value?.dashboard?.uid !== params.uid && dashboard.value?.meta?.isNew !== true) { return null; } @@ -64,7 +66,7 @@ function DashboardPageProxy(props: DashboardPageProxyProps) { ) { return ; } else { - return ; + return ; } } diff --git a/public/app/features/dashboard/containers/SoloPanelPage.test.tsx b/public/app/features/dashboard/containers/SoloPanelPage.test.tsx index 29753f6275a..0d4d7840f38 100644 --- a/public/app/features/dashboard/containers/SoloPanelPage.test.tsx +++ b/public/app/features/dashboard/containers/SoloPanelPage.test.tsx @@ -72,12 +72,6 @@ function soloPanelPageScenario(description: string, scenarioFn: (ctx: ScenarioCo mount: (propOverrides?: Partial) => { const props: Props = { ...getRouteComponentProps({ - match: { - params: { slug: 'my-dash', uid: '11' }, - isExact: false, - path: '', - url: '', - }, queryParams: { panelId: '1', },