Restore 404 behavior.

Signed-off-by: Eric Peterson <ericpeterson@spotify.com>

.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.

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<{

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;
+}) => (
+  <ThemeProvider theme={lightTheme}>
+    <TestApiProvider apis={[[techdocsApiRef, techdocsApiMock]]}>
+      <TechDocsReaderPageProvider entityRef={entityRef}>
+        {children}
+      </TechDocsReaderPageProvider>
+    </TestApiProvider>
+  </ThemeProvider>
+);
+
+describe('<TechDocsReaderPageContent />', () => {
+  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(
+        <Wrapper>
+          <TechDocsReaderPageContent withSearch={false} />
+        </Wrapper>,
+      );
+
+      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(
+        <Wrapper>
+          <TechDocsReaderPageContent withSearch={false} />
+        </Wrapper>,
+      );
+
+      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(
+        <Wrapper>
+          <TechDocsReaderPageContent withSearch={false} />
+        </Wrapper>,
+      );
+
+      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(
+        <Wrapper>
+          <TechDocsReaderPageContent withSearch={false} />
+        </Wrapper>,
+      );
+
+      await waitFor(() => {
+        expect(
+          rendered.queryByTestId('techdocs-native-shadowroot'),
+        ).not.toBeInTheDocument();
+        expect(
+          rendered.getByText('ERROR 404: Documentation not found'),
+        ).toBeInTheDocument();
+      });
+    });
+  });
+});

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 <ErrorPage status="404" statusMessage="PAGE NOT FOUND" />;
+
+    // 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 (
         <Content>
-          <Progress />
+          <Grid container>
+            <Grid xs={12} item>
+              <TechDocsStateIndicator />
+            </Grid>
+          </Grid>
         </Content>
       );
     }

plugins/techdocs/src/reader/components/TechDocsReaderPageHeader/TechDocsReaderPageHeader.test.tsx

@@ -108,7 +108,7 @@ describe('<TechDocsReaderPageHeader />', () => {
     });
   });
 
-  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(
         <Wrapper>
@@ -126,6 +126,28 @@ describe('<TechDocsReaderPageHeader />', () => {
     });
   });
 
+  it('should not render a techdocs page header if entity metadata is missing', async () => {
+    getEntityMetadata.mockResolvedValue(undefined);
+
+    await act(async () => {
+      const rendered = await renderInTestApp(
+        <Wrapper>
+          <TechDocsReaderPageHeader />
+        </Wrapper>,
+        {
+          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);
 

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 (
     <Header
       type="Documentation"

plugins/techdocs/src/reader/components/TechDocsReaderPageSubheader/TechDocsReaderPageSubheader.tsx

@@ -21,6 +21,7 @@ import { Box, makeStyles, Toolbar, ToolbarProps } from '@material-ui/core';
 import {
   TechDocsAddonLocations as locations,
   useTechDocsAddons,
+  useTechDocsReaderPage,
 } from '@backstage/plugin-techdocs-react';
 
 const useStyles = makeStyles(theme => ({
@@ -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 (
     <Toolbar classes={classes} {...toolbarProps}>
       {subheaderAddons && (