Dashboards: Switch to useParams hook (#94060)

* Update DashboardScenePage

* Update SoloPanelPage

* Update DashboardPage

* Cleanup

* Switch to useLocation

* Do not use location from history
This commit is contained in:
Alex Khomenko 2024-10-07 12:11:57 +03:00 committed by GitHub
parent 4bc7a35f56
commit 9680722b78
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
8 changed files with 80 additions and 91 deletions

View File

@ -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<GrafanaRouteComponentProps> } = {}) {
@ -55,12 +61,6 @@ function setup({ routeProps }: { routeProps?: Partial<GrafanaRouteComponentProps
const defaultRouteProps = getRouteComponentProps();
const props: Props = {
...defaultRouteProps,
match: {
...defaultRouteProps.match,
params: {
uid: 'my-dash-uid',
},
},
...routeProps,
};
@ -163,7 +163,7 @@ describe('DashboardScenePage', () => {
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 <div>{props.options.content}</div>;
}
async function waitForDashbordToRender() {
async function waitForDashboardToRender() {
expect(await screen.findByText('Last 6 hours')).toBeInTheDocument();
expect(await screen.findByTitle('Panel A')).toBeInTheDocument();
}

View File

@ -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<DashboardPageRouteParams, DashboardPageRouteSearchParams> {}
export interface Props
extends Omit<GrafanaRouteComponentProps<DashboardPageRouteParams, DashboardPageRouteSearchParams>, '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<DashboardDTO>(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;
}

View File

@ -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<DashboardPageRoutePara
/**
* Used for iframe embedding and image rendering of single panels
*/
export function SoloPanelPage({ match, queryParams }: Props) {
export function SoloPanelPage({ queryParams }: Props) {
const stateManager = getDashboardScenePageStateManager();
const { dashboard } = stateManager.useState();
const { uid = '' } = useParams();
useEffect(() => {
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 <EntityNotFound entity="Panel" />;

View File

@ -100,9 +100,9 @@ function setup(propOverrides?: Partial<Props>) {
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(() => {

View File

@ -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<DashboardPageRouteParams, DashboardPageRouteSearchParams> &
ConnectedProps<typeof connector>;
Omit<GrafanaRouteComponentProps<DashboardPageRouteParams, DashboardPageRouteSearchParams>, 'match'> &
// The params returned from useParams are all optional, so we need to match that type here
ConnectedProps<typeof connector> & { params: Partial<DashboardPageParams> };
export interface State {
editPanel: PanelModel | null;
@ -138,7 +140,7 @@ export class UnthemedDashboardPage extends PureComponent<Props, State> {
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<Props, State> {
}
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<Props, State> {
}
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;
}

View File

@ -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<DashboardPageProxyProps>) {
jest.mock('react-router-dom-v5-compat', () => ({
...jest.requireActual('react-router-dom-v5-compat'),
useParams: jest.fn().mockReturnValue({}),
}));
function setup(props: Partial<DashboardPageProxyProps> & { uid?: string }) {
(useParams as jest.Mock).mockReturnValue({ uid: props.uid });
return render(
<DashboardPageProxy
location={locationService.getLocation()}
history={locationService.getHistory()}
queryParams={{}}
route={{ routeName: DashboardRoutes.Home, component: () => 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: '/',
},
});
});
@ -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(() => {

View File

@ -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<DashboardPageRouteParams, DashboardPageRouteSearchParams>,
'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<DashboardPageParams>();
const location = useLocation();
if (forceScenes || (config.featureToggles.dashboardScene && !forceOld)) {
return <DashboardScenePage {...props} />;
@ -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 <DashboardPage {...props} />;
return <DashboardPage {...props} params={params} location={location} />;
}
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 <DashboardScenePage {...props} />;
} else {
return <DashboardPage {...props} />;
return <DashboardPage {...props} params={params} location={location} />;
}
}

View File

@ -72,12 +72,6 @@ function soloPanelPageScenario(description: string, scenarioFn: (ctx: ScenarioCo
mount: (propOverrides?: Partial<Props>) => {
const props: Props = {
...getRouteComponentProps({
match: {
params: { slug: 'my-dash', uid: '11' },
isExact: false,
path: '',
url: '',
},
queryParams: {
panelId: '1',
},