From ef795700d62903e86abc1661d56a474a35506c9d Mon Sep 17 00:00:00 2001 From: Salih Candir Date: Wed, 14 Dec 2022 11:27:47 +0100 Subject: [PATCH 1/4] fix: use '../' instead of '../../' for relative paths With `../../audit/` we navigate from `//audit/` to `/audit/` but this is wrong because the base path is missing. Therefore we use `../audit/` and this will navigate to the right url with respect to the base path. Signed-off-by: Salih Candir --- .../src/components/AuditView/index.test.tsx | 22 +++++++++++++++++++ .../src/components/AuditView/index.tsx | 4 ++-- 2 files changed, 24 insertions(+), 2 deletions(-) diff --git a/plugins/lighthouse/src/components/AuditView/index.test.tsx b/plugins/lighthouse/src/components/AuditView/index.test.tsx index c68daddc67..d19543d679 100644 --- a/plugins/lighthouse/src/components/AuditView/index.test.tsx +++ b/plugins/lighthouse/src/components/AuditView/index.test.tsx @@ -149,6 +149,28 @@ describe('AuditView', () => { ).toHaveAttribute('href', `/audit/${a.id}`); }); }); + + it('navigates to the next report with respect to the base path', async () => { + // TODO: I would need help here. How to set the base path for the test? + const basePath = `/lighthouse`; + const rendered = render( + wrapInTestApp( + + + , + { routeEntries: [basePath] }, + ), + ); + + await rendered.findByTestId('audit-sidebar'); + + websiteResponse.audits.forEach(a => { + expect( + rendered.getByText(formatTime(a.timeCreated)).parentElement + ?.parentElement, + ).toHaveAttribute('href', `${basePath}/audits/${a.id}`); + }); + }); }); describe('when the request for the website by id is pending', () => { diff --git a/plugins/lighthouse/src/components/AuditView/index.tsx b/plugins/lighthouse/src/components/AuditView/index.tsx index e377894c90..52a087fd8e 100644 --- a/plugins/lighthouse/src/components/AuditView/index.tsx +++ b/plugins/lighthouse/src/components/AuditView/index.tsx @@ -80,7 +80,7 @@ const AuditLinkList = ({ audits = [], selectedId }: AuditLinkListProps) => ( button component={Link} replace - to={resolvePath(generatePath('audit/:id', { id: audit.id }), '../../')} + to={resolvePath(generatePath('audit/:id', { id: audit.id }), '../')} > @@ -178,7 +178,7 @@ export const AuditViewContent = () => { From 2f1b283de2b8062d2e308981b90b8307128e0e2b Mon Sep 17 00:00:00 2001 From: Salih Candir Date: Wed, 14 Dec 2022 13:40:23 +0100 Subject: [PATCH 2/4] docs: add .changeset Signed-off-by: Salih Candir --- .changeset/ninety-bags-turn.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/ninety-bags-turn.md diff --git a/.changeset/ninety-bags-turn.md b/.changeset/ninety-bags-turn.md new file mode 100644 index 0000000000..27b9a94ad2 --- /dev/null +++ b/.changeset/ninety-bags-turn.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-lighthouse': patch +--- + +Fixed bug in Lighthouse Plugin where audit list item and create audit button navigated to a wrong URL. From 1bb26305637e3aa8a81688fb69f6db5bc1cde6c8 Mon Sep 17 00:00:00 2001 From: Salih Candir Date: Wed, 21 Dec 2022 11:37:08 +0100 Subject: [PATCH 3/4] fix: use right url path Signed-off-by: Salih Candir Signed-off-by: Salih Candir --- plugins/lighthouse/src/components/AuditView/index.test.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/lighthouse/src/components/AuditView/index.test.tsx b/plugins/lighthouse/src/components/AuditView/index.test.tsx index d19543d679..d58c0d1ce4 100644 --- a/plugins/lighthouse/src/components/AuditView/index.test.tsx +++ b/plugins/lighthouse/src/components/AuditView/index.test.tsx @@ -168,7 +168,7 @@ describe('AuditView', () => { expect( rendered.getByText(formatTime(a.timeCreated)).parentElement ?.parentElement, - ).toHaveAttribute('href', `${basePath}/audits/${a.id}`); + ).toHaveAttribute('href', `${basePath}/audit/${a.id}`); }); }); }); From 1c0fcca4b857572e9eec84d351b3abac5c34d180 Mon Sep 17 00:00:00 2001 From: Salih Candir Date: Fri, 23 Dec 2022 17:08:10 +0100 Subject: [PATCH 4/4] refactor: use route ref instead of relative paths Using `useRouteRef` allows us to make the component testable, with a relative path we cannot use `mountedRoutes` within `wrapInTestApp` Signed-off-by: Salih Candir --- .../src/components/AuditView/index.test.tsx | 43 ++++++++++---- .../src/components/AuditView/index.tsx | 58 +++++++++++-------- 2 files changed, 65 insertions(+), 36 deletions(-) diff --git a/plugins/lighthouse/src/components/AuditView/index.test.tsx b/plugins/lighthouse/src/components/AuditView/index.test.tsx index d58c0d1ce4..d555133f3e 100644 --- a/plugins/lighthouse/src/components/AuditView/index.test.tsx +++ b/plugins/lighthouse/src/components/AuditView/index.test.tsx @@ -16,6 +16,8 @@ /* eslint-disable jest/no-disabled-tests */ +import { configApiRef } from '@backstage/core-plugin-api'; + jest.mock('react-router-dom', () => { const actual = jest.requireActual('react-router-dom'); const mockNavigation = jest.fn(); @@ -28,6 +30,7 @@ jest.mock('react-router-dom', () => { import { setupRequestMockHandlers, + TestApiProvider, TestApiRegistry, wrapInTestApp, } from '@backstage/test-utils'; @@ -39,13 +42,18 @@ import { Audit, lighthouseApiRef, LighthouseRestApi, Website } from '../../api'; import { formatTime } from '../../utils'; import * as data from '../../__fixtures__/website-response.json'; import AuditView from './index'; -import { ApiProvider } from '@backstage/core-app-api'; +import { ApiProvider, ConfigReader } from '@backstage/core-app-api'; +import { rootRouteRef } from '../../plugin'; const { useParams }: { useParams: jest.Mock } = jest.requireMock('react-router-dom'); const websiteResponse = data as Website; describe('AuditView', () => { + const lighthouseRestApiMock = new LighthouseRestApi('https://lighthouse'); + const testAppOptions = { + mountedRoutes: { '/': rootRouteRef }, + }; let apis: TestApiRegistry; let id: string; @@ -59,10 +67,7 @@ describe('AuditView', () => { ), ); - apis = TestApiRegistry.from([ - lighthouseApiRef, - new LighthouseRestApi('https://lighthouse'), - ]); + apis = TestApiRegistry.from([lighthouseApiRef, lighthouseRestApiMock]); id = websiteResponse.audits.find(a => a.status === 'COMPLETED') ?.id as string; useParams.mockReturnValue({ id }); @@ -74,6 +79,7 @@ describe('AuditView', () => { , + testAppOptions, ), ); @@ -91,6 +97,7 @@ describe('AuditView', () => { , + testAppOptions, ), ); @@ -109,6 +116,7 @@ describe('AuditView', () => { , + testAppOptions, ), ); @@ -137,6 +145,7 @@ describe('AuditView', () => { , + testAppOptions, ), ); @@ -151,14 +160,22 @@ describe('AuditView', () => { }); it('navigates to the next report with respect to the base path', async () => { - // TODO: I would need help here. How to set the base path for the test? - const basePath = `/lighthouse`; + const configApiMock = new ConfigReader({ + app: { baseUrl: `http://localhost:3000/example` }, + }); const rendered = render( wrapInTestApp( - + - , - { routeEntries: [basePath] }, + , + { + mountedRoutes: { [`/example/lighthouse`]: rootRouteRef }, + }, ), ); @@ -168,7 +185,7 @@ describe('AuditView', () => { expect( rendered.getByText(formatTime(a.timeCreated)).parentElement ?.parentElement, - ).toHaveAttribute('href', `${basePath}/audit/${a.id}`); + ).toHaveAttribute('href', `/example/lighthouse/audit/${a.id}`); }); }); }); @@ -181,6 +198,7 @@ describe('AuditView', () => { , + testAppOptions, ), ); expect(await rendered.findByTestId('progress')).toBeInTheDocument(); @@ -199,6 +217,7 @@ describe('AuditView', () => { , + testAppOptions, ), ); expect(await rendered.findByText(/failed to fetch/)).toBeInTheDocument(); @@ -216,6 +235,7 @@ describe('AuditView', () => { , + testAppOptions, ), ); @@ -236,6 +256,7 @@ describe('AuditView', () => { , + testAppOptions, ), ); diff --git a/plugins/lighthouse/src/components/AuditView/index.tsx b/plugins/lighthouse/src/components/AuditView/index.tsx index 52a087fd8e..ade485b09a 100644 --- a/plugins/lighthouse/src/components/AuditView/index.tsx +++ b/plugins/lighthouse/src/components/AuditView/index.tsx @@ -47,7 +47,8 @@ import { Page, Progress, } from '@backstage/core-components'; -import { useApi } from '@backstage/core-plugin-api'; +import { useApi, useRouteRef } from '@backstage/core-plugin-api'; +import { rootRouteRef } from '../../plugin'; // TODO(freben): move all of this out of index @@ -67,29 +68,35 @@ interface AuditLinkListProps { audits?: Audit[]; selectedId: string; } -const AuditLinkList = ({ audits = [], selectedId }: AuditLinkListProps) => ( - - {audits.map(audit => ( - - - - - - - ))} - -); +const AuditLinkList = ({ audits = [], selectedId }: AuditLinkListProps) => { + const fromPath = useRouteRef(rootRouteRef)?.() ?? '../'; + return ( + + {audits.map(audit => ( + + + + + + + ))} + + ); +}; const AuditView = ({ audit }: { audit?: Audit }) => { const classes = useStyles(); @@ -119,6 +126,7 @@ const AuditView = ({ audit }: { audit?: Audit }) => { export const AuditViewContent = () => { const lighthouseApi = useApi(lighthouseApiRef); + const fromPath = useRouteRef(rootRouteRef)?.() ?? '../'; const params = useParams() as { id: string }; const classes = useStyles(); const navigate = useNavigate(); @@ -178,7 +186,7 @@ export const AuditViewContent = () => {