From bed0d64ce9c5badb911572a7a679446bcaa37aaf Mon Sep 17 00:00:00 2001 From: Eric Peterson Date: Wed, 20 Apr 2022 13:58:50 +0200 Subject: [PATCH 1/2] Restore 404 behavior. Signed-off-by: Eric Peterson --- .changeset/techdocs-changeset-not-found.md | 5 + plugins/techdocs/api-report.md | 2 +- .../TechDocsReaderPageContent.test.tsx | 181 ++++++++++++++++++ .../TechDocsReaderPageContent.tsx | 22 ++- .../TechDocsReaderPageHeader.test.tsx | 24 ++- .../TechDocsReaderPageHeader.tsx | 6 +- .../TechDocsReaderPageSubheader.tsx | 7 + 7 files changed, 240 insertions(+), 7 deletions(-) create mode 100644 .changeset/techdocs-changeset-not-found.md create mode 100644 plugins/techdocs/src/reader/components/TechDocsReaderPageContent/TechDocsReaderPageContent.test.tsx diff --git a/.changeset/techdocs-changeset-not-found.md b/.changeset/techdocs-changeset-not-found.md new file mode 100644 index 0000000000..7ee5ec2b85 --- /dev/null +++ b/.changeset/techdocs-changeset-not-found.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-techdocs': patch +--- + +Fixed bugs that prevented a 404 error from being shown when it should have been. diff --git a/plugins/techdocs/api-report.md b/plugins/techdocs/api-report.md index 936df8991c..a7c287a4f7 100644 --- a/plugins/techdocs/api-report.md +++ b/plugins/techdocs/api-report.md @@ -322,7 +322,7 @@ export type TechDocsReaderPageContentProps = { // @public export const TechDocsReaderPageHeader: ( props: TechDocsReaderPageHeaderProps, -) => JSX.Element; +) => JSX.Element | null; // @public @deprecated export type TechDocsReaderPageHeaderProps = PropsWithChildren<{ diff --git a/plugins/techdocs/src/reader/components/TechDocsReaderPageContent/TechDocsReaderPageContent.test.tsx b/plugins/techdocs/src/reader/components/TechDocsReaderPageContent/TechDocsReaderPageContent.test.tsx new file mode 100644 index 0000000000..975a58f332 --- /dev/null +++ b/plugins/techdocs/src/reader/components/TechDocsReaderPageContent/TechDocsReaderPageContent.test.tsx @@ -0,0 +1,181 @@ +/* + * Copyright 2022 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +import React from 'react'; +import { act, waitFor } from '@testing-library/react'; + +import { ThemeProvider } from '@material-ui/core'; + +import { lightTheme } from '@backstage/theme'; +import { CompoundEntityRef } from '@backstage/catalog-model'; +import { + techdocsApiRef, + TechDocsReaderPageProvider, +} from '@backstage/plugin-techdocs-react'; +import { renderInTestApp, TestApiProvider } from '@backstage/test-utils'; + +const useTechDocsReaderDom = jest.fn(); +jest.mock('./dom', () => ({ + ...jest.requireActual('./dom'), + useTechDocsReaderDom, +})); +const useReaderState = jest.fn(); +jest.mock('../useReaderState', () => ({ + ...jest.requireActual('../useReaderState'), + useReaderState, +})); + +import { TechDocsReaderPageContent } from './TechDocsReaderPageContent'; + +const mockEntityMetadata = { + locationMetadata: { + type: 'github', + target: 'https://example.com/', + }, + apiVersion: 'v1', + kind: 'test', + metadata: { + name: 'test-name', + namespace: 'test-namespace', + }, + spec: { + owner: 'test', + }, +}; + +const mockTechDocsMetadata = { + site_name: 'test-site-name', + site_description: 'test-site-desc', +}; + +const getEntityMetadata = jest.fn(); +const getTechDocsMetadata = jest.fn(); + +const techdocsApiMock = { + getEntityMetadata, + getTechDocsMetadata, +}; + +const Wrapper = ({ + entityRef = { + kind: mockEntityMetadata.kind, + name: mockEntityMetadata.metadata.name, + namespace: mockEntityMetadata.metadata.namespace!!, + }, + children, +}: { + entityRef?: CompoundEntityRef; + children: React.ReactNode; +}) => ( + + + + {children} + + + +); + +describe('', () => { + it('should render techdocs page content', async () => { + getEntityMetadata.mockResolvedValue(mockEntityMetadata); + getTechDocsMetadata.mockResolvedValue(mockTechDocsMetadata); + useTechDocsReaderDom.mockReturnValue(document.createElement('html')); + useReaderState.mockReturnValue({ state: 'cached' }); + + await act(async () => { + const rendered = await renderInTestApp( + + + , + ); + + await waitFor(() => { + expect( + rendered.getByTestId('techdocs-native-shadowroot'), + ).toBeInTheDocument(); + }); + }); + }); + + it('should render progress if there is no dom and reader state is checking', async () => { + getEntityMetadata.mockResolvedValue(mockEntityMetadata); + getTechDocsMetadata.mockResolvedValue(mockTechDocsMetadata); + useTechDocsReaderDom.mockReturnValue(undefined); + useReaderState.mockReturnValue({ state: 'CHECKING' }); + + await act(async () => { + const rendered = await renderInTestApp( + + + , + ); + + await waitFor(() => { + expect( + rendered.queryByTestId('techdocs-native-shadowroot'), + ).not.toBeInTheDocument(); + expect(rendered.getByRole('progressbar')).toBeInTheDocument(); + }); + }); + }); + + it('should not render techdocs content if entity metadata is missing', async () => { + getEntityMetadata.mockResolvedValue(undefined); + useTechDocsReaderDom.mockReturnValue(document.createElement('html')); + useReaderState.mockReturnValue({ state: 'cached' }); + + await act(async () => { + const rendered = await renderInTestApp( + + + , + ); + + await waitFor(() => { + expect( + rendered.queryByTestId('techdocs-native-shadowroot'), + ).not.toBeInTheDocument(); + expect( + rendered.getByText('ERROR 404: PAGE NOT FOUND'), + ).toBeInTheDocument(); + }); + }); + }); + + it('should render 404 if there is no dom and reader state is not found', async () => { + getEntityMetadata.mockResolvedValue(mockEntityMetadata); + getTechDocsMetadata.mockResolvedValue(mockTechDocsMetadata); + useTechDocsReaderDom.mockReturnValue(undefined); + useReaderState.mockReturnValue({ state: 'CONTENT_NOT_FOUND' }); + + await act(async () => { + const rendered = await renderInTestApp( + + + , + ); + + await waitFor(() => { + expect( + rendered.queryByTestId('techdocs-native-shadowroot'), + ).not.toBeInTheDocument(); + expect( + rendered.getByText('ERROR 404: Documentation not found'), + ).toBeInTheDocument(); + }); + }); + }); +}); diff --git a/plugins/techdocs/src/reader/components/TechDocsReaderPageContent/TechDocsReaderPageContent.tsx b/plugins/techdocs/src/reader/components/TechDocsReaderPageContent/TechDocsReaderPageContent.tsx index c34b04ab8c..fbab933571 100644 --- a/plugins/techdocs/src/reader/components/TechDocsReaderPageContent/TechDocsReaderPageContent.tsx +++ b/plugins/techdocs/src/reader/components/TechDocsReaderPageContent/TechDocsReaderPageContent.tsx @@ -26,7 +26,7 @@ import { useTechDocsReaderPage, } from '@backstage/plugin-techdocs-react'; import { CompoundEntityRef } from '@backstage/catalog-model'; -import { Content, Progress } from '@backstage/core-components'; +import { Content, ErrorPage } from '@backstage/core-components'; import { TechDocsSearch } from '../../../search'; import { TechDocsStateIndicator } from '../TechDocsStateIndicator'; @@ -72,7 +72,12 @@ export const TechDocsReaderPageContent = withTechDocsReaderProvider( const { withSearch = true, onReady } = props; const classes = useStyles(); const addons = useTechDocsAddons(); - const { entityRef, shadowRoot, setShadowRoot } = useTechDocsReaderPage(); + const { + entityMetadata: { value: entityMetadata, loading: entityMetadataLoading }, + entityRef, + shadowRoot, + setShadowRoot, + } = useTechDocsReaderPage(); const dom = useTechDocsReaderDom(entityRef); const [jss, setJss] = useState( @@ -121,11 +126,20 @@ export const TechDocsReaderPageContent = withTechDocsReaderProvider( const secondarySidebarAddonLocation = document.createElement('div'); secondarySidebarElement?.prepend(secondarySidebarAddonLocation); - // do not return content until dom is ready + // No entity metadata = 404. Don't render content at all. + if (entityMetadataLoading === false && !entityMetadata) + return ; + + // Do not return content until dom is ready; instead, render a state + // indicator, which handles progress and content errors on our behalf. if (!dom) { return ( - + + + + + ); } diff --git a/plugins/techdocs/src/reader/components/TechDocsReaderPageHeader/TechDocsReaderPageHeader.test.tsx b/plugins/techdocs/src/reader/components/TechDocsReaderPageHeader/TechDocsReaderPageHeader.test.tsx index 288e3bec82..71456a6942 100644 --- a/plugins/techdocs/src/reader/components/TechDocsReaderPageHeader/TechDocsReaderPageHeader.test.tsx +++ b/plugins/techdocs/src/reader/components/TechDocsReaderPageHeader/TechDocsReaderPageHeader.test.tsx @@ -108,7 +108,7 @@ describe('', () => { }); }); - it('should render a techdocs page header even if metadata is missing', async () => { + it('should render a techdocs page header even if metadata is not loaded', async () => { await act(async () => { const rendered = await renderInTestApp( @@ -126,6 +126,28 @@ describe('', () => { }); }); + it('should not render a techdocs page header if entity metadata is missing', async () => { + getEntityMetadata.mockResolvedValue(undefined); + + await act(async () => { + const rendered = await renderInTestApp( + + + , + { + mountedRoutes: { + '/catalog/:namespace/:kind/:name/*': entityRouteRef, + '/docs': rootRouteRef, + }, + }, + ); + + await waitFor(() => { + expect(rendered.container.innerHTML).not.toContain('header'); + }); + }); + }); + it('should render a link back to the component page', async () => { getTechDocsMetadata.mockResolvedValue(mockTechDocsMetadata); diff --git a/plugins/techdocs/src/reader/components/TechDocsReaderPageHeader/TechDocsReaderPageHeader.tsx b/plugins/techdocs/src/reader/components/TechDocsReaderPageHeader/TechDocsReaderPageHeader.tsx index 5835121d2e..c0ef1fa95a 100644 --- a/plugins/techdocs/src/reader/components/TechDocsReaderPageHeader/TechDocsReaderPageHeader.tsx +++ b/plugins/techdocs/src/reader/components/TechDocsReaderPageHeader/TechDocsReaderPageHeader.tsx @@ -72,7 +72,7 @@ export const TechDocsReaderPageHeader = ( setSubtitle, entityRef, metadata: { value: metadata }, - entityMetadata: { value: entityMetadata }, + entityMetadata: { value: entityMetadata, loading: entityMetadataLoading }, } = useTechDocsReaderPage(); useEffect(() => { @@ -146,6 +146,10 @@ export const TechDocsReaderPageHeader = ( ); + // If there is no entity metadata, there's no reason to show the header. + if (entityMetadataLoading === false && entityMetadata === undefined) + return null; + return (
({ @@ -43,6 +44,9 @@ export const TechDocsReaderPageSubheader = ({ toolbarProps?: ToolbarProps; }) => { const classes = useStyles(); + const { + entityMetadata: { value: entityMetadata, loading: entityMetadataLoading }, + } = useTechDocsReaderPage(); const addons = useTechDocsAddons(); const subheaderAddons = addons.renderComponentsByLocation( locations.Subheader, @@ -50,6 +54,9 @@ export const TechDocsReaderPageSubheader = ({ if (!subheaderAddons) return null; + // No entity metadata = 404. Don't render subheader on 404. + if (entityMetadataLoading === false && !entityMetadata) return null; + return ( {subheaderAddons && ( From f3dc243432bb50f7ace732c49d287bdedb2f05d1 Mon Sep 17 00:00:00 2001 From: Eric Peterson Date: Mon, 25 Apr 2022 09:52:02 +0200 Subject: [PATCH 2/2] Account for case when builder is external, entity exists, but docs do not. Co-authored-by: Jeremy Guarini Signed-off-by: Eric Peterson --- .../TechDocsReaderPageHeader.test.tsx | 22 +++++++++++++++++++ .../TechDocsReaderPageHeader.tsx | 10 +++++---- 2 files changed, 28 insertions(+), 4 deletions(-) diff --git a/plugins/techdocs/src/reader/components/TechDocsReaderPageHeader/TechDocsReaderPageHeader.test.tsx b/plugins/techdocs/src/reader/components/TechDocsReaderPageHeader/TechDocsReaderPageHeader.test.tsx index 71456a6942..da193430d5 100644 --- a/plugins/techdocs/src/reader/components/TechDocsReaderPageHeader/TechDocsReaderPageHeader.test.tsx +++ b/plugins/techdocs/src/reader/components/TechDocsReaderPageHeader/TechDocsReaderPageHeader.test.tsx @@ -148,6 +148,28 @@ describe('', () => { }); }); + it('should not render a techdocs page header if techdocs metadata is missing', async () => { + getTechDocsMetadata.mockResolvedValue(undefined); + + await act(async () => { + const rendered = await renderInTestApp( + + + , + { + mountedRoutes: { + '/catalog/:namespace/:kind/:name/*': entityRouteRef, + '/docs': rootRouteRef, + }, + }, + ); + + await waitFor(() => { + expect(rendered.container.innerHTML).not.toContain('header'); + }); + }); + }); + it('should render a link back to the component page', async () => { getTechDocsMetadata.mockResolvedValue(mockTechDocsMetadata); diff --git a/plugins/techdocs/src/reader/components/TechDocsReaderPageHeader/TechDocsReaderPageHeader.tsx b/plugins/techdocs/src/reader/components/TechDocsReaderPageHeader/TechDocsReaderPageHeader.tsx index c0ef1fa95a..204ac835e4 100644 --- a/plugins/techdocs/src/reader/components/TechDocsReaderPageHeader/TechDocsReaderPageHeader.tsx +++ b/plugins/techdocs/src/reader/components/TechDocsReaderPageHeader/TechDocsReaderPageHeader.tsx @@ -71,7 +71,7 @@ export const TechDocsReaderPageHeader = ( subtitle, setSubtitle, entityRef, - metadata: { value: metadata }, + metadata: { value: metadata, loading: metadataLoading }, entityMetadata: { value: entityMetadata, loading: entityMetadataLoading }, } = useTechDocsReaderPage(); @@ -146,9 +146,11 @@ export const TechDocsReaderPageHeader = ( ); - // If there is no entity metadata, there's no reason to show the header. - if (entityMetadataLoading === false && entityMetadata === undefined) - return null; + // If there is no entity or techdocs metadata, there's no reason to show the + // header (hides the header on 404 error pages). + const noEntMetadata = !entityMetadataLoading && entityMetadata === undefined; + const noTdMetadata = !metadataLoading && metadata === undefined; + if (noEntMetadata || noTdMetadata) return null; return (