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 && (