feat: Update NotFoundError to have login button (#3804)

Author: nicholas-codecov

src/layouts/shared/NetworkErrorBoundary/NetworkErrorBoundary.jsx

@@ -2,10 +2,11 @@ import { useQueryClient } from '@tanstack/react-query'
 import cs from 'classnames'
 import PropTypes from 'prop-types'
 import { Component, useEffect } from 'react'
-import { useHistory } from 'react-router-dom'
+import { useHistory, useLocation } from 'react-router-dom'
 
 import config from 'config'
 
+import { useUser } from 'services/user'
 import A from 'ui/A'
 import Button from 'ui/Button'
 
@@ -67,11 +68,11 @@ const graphQLErrorToUI = {
   },
 }
 
-export const NetworkErrorMessage = () => {
+export const DocErrorMessage = ({ capitalize = true }) => {
   if (config.IS_SELF_HOSTED) {
     return (
-      <p className="my-4 px-3 sm:px-0">
-        Please see{' '}
+      <>
+        {capitalize ? 'Please' : 'please'} see{' '}
         <A
           rel="noreferrer"
           className="text-ds-blue-default"
@@ -82,13 +83,13 @@ export const NetworkErrorMessage = () => {
           our docs
         </A>{' '}
         for common support.
-      </p>
+      </>
     )
   }
 
   return (
-    <p className="my-4 px-3 sm:px-0">
-      Check on{' '}
+    <>
+      {capitalize ? 'Check' : 'check'} on{' '}
       <A
         rel="noreferrer"
         className="text-ds-blue-default"
@@ -109,13 +110,40 @@ export const NetworkErrorMessage = () => {
         our docs
       </A>{' '}
       for common support.
+    </>
+  )
+}
+
+DocErrorMessage.propTypes = {
+  capitalize: PropTypes.bool,
+}
+
+export const NetworkErrorMessage = ({ statusCode }) => {
+  if (statusCode === 404) {
+    return (
+      <p className="my-4 max-w-2xl px-3 text-center sm:px-0">
+        To view private repositories ensure you are logged in, you can also{' '}
+        <DocErrorMessage capitalize={false} />
+      </p>
+    )
+  }
+
+  return (
+    <p className="my-4 max-w-2xl px-3 text-center sm:px-0">
+      <DocErrorMessage capitalize={true} />
     </p>
   )
 }
 
-function ResetHandler({ logoutUser = false, reset }) {
+NetworkErrorMessage.propTypes = {
+  statusCode: PropTypes.number,
+}
+
+function ResetHandler({ logoutUser = false, reset, statusCode }) {
   const queryClient = useQueryClient()
   const history = useHistory()
+  const location = useLocation()
+  const { data: user } = useUser()
 
   useEffect(() => {
     let unMounted = false
@@ -148,17 +176,27 @@ function ResetHandler({ logoutUser = false, reset }) {
   }
 
   return (
-    <div className="my-4">
+    <div className="my-4 flex items-center gap-2">
       <Button onClick={logoutUser ? handleSignOut : handleReset}>
         {logoutUser ? 'Return to login' : 'Return to previous page'}
       </Button>
+      {/* if the user is logged in, we don't want to show the login button */}
+      {statusCode === 404 && !user ? (
+        <Button
+          variant="primary"
+          to={{ pageName: 'login', options: { to: location.pathname } }}
+        >
+          Login
+        </Button>
+      ) : null}
     </div>
   )
 }
 
 ResetHandler.propTypes = {
   reset: PropTypes.func,
   logoutUser: PropTypes.bool,
+  statusCode: PropTypes.number,
 }
 
 class NetworkErrorBoundary extends Component {
@@ -211,8 +249,11 @@ class NetworkErrorBoundary extends Component {
           src={illustration}
         />
         <h1 className="mt-6 text-2xl">{title}</h1>
-        <NetworkErrorMessage />
-        <ResetHandler reset={this.resetErrorBoundary} />
+        <NetworkErrorMessage statusCode={this.state.error.status} />
+        <ResetHandler
+          reset={this.resetErrorBoundary}
+          statusCode={this.state.error.status}
+        />
       </article>
     )
   }
@@ -230,13 +271,14 @@ class NetworkErrorBoundary extends Component {
         />
         <h1 className="mt-6 text-2xl">{title}</h1>
         {description ? <p className="mt-2">{description(data)}</p> : null}
-        {showDocs ? <NetworkErrorMessage /> : null}
+        {showDocs ? <NetworkErrorMessage statusCode={status} /> : null}
         <p>
           <strong>Error {status}</strong>
         </p>
         <ResetHandler
           logoutUser={status === 429}
           reset={this.resetErrorBoundary}
+          statusCode={status}
         />
       </article>
     )

src/layouts/shared/NetworkErrorBoundary/NetworkErrorBoundary.test.jsx

@@ -1,8 +1,11 @@
 import { QueryClient, QueryClientProvider } from '@tanstack/react-query'
 import { render, screen, waitFor } from '@testing-library/react'
 import userEvent from '@testing-library/user-event'
-import { Component, useState } from 'react'
-import { MemoryRouter, useHistory } from 'react-router-dom'
+import { graphql, HttpResponse } from 'msw'
+import { setupServer } from 'msw/node'
+import qs from 'qs'
+import { Component, Suspense, useState } from 'react'
+import { MemoryRouter, Route, useHistory } from 'react-router-dom'
 import { vi } from 'vitest'
 
 import config from 'config'
@@ -13,15 +16,67 @@ import NetworkErrorBoundary from './NetworkErrorBoundary'
 vi.spyOn(console, 'error').mockImplementation(() => undefined)
 vi.mock('config')
 
-const queryClient = new QueryClient({
-  defaultOptions: { queries: { retry: false } },
+const mockUser = {
+  me: {
+    owner: {
+      defaultOrgUsername: 'codecov',
+    },
+    email: '[email protected]',
+    privateAccess: true,
+    onboardingCompleted: true,
+    businessEmail: '[email protected]',
+    termsAgreement: true,
+    user: {
+      name: 'Jane Doe',
+      username: 'janedoe',
+      avatarUrl: 'http://127.0.0.1/avatar-url',
+      avatar: 'http://127.0.0.1/avatar-url',
+      student: false,
+      studentCreatedAt: null,
+      studentUpdatedAt: null,
+    },
+    trackingMetadata: {
+      service: 'github',
+      ownerid: 123,
+      serviceId: '123',
+      plan: 'users-basic',
+      staff: false,
+      hasYaml: false,
+      bot: null,
+      delinquent: null,
+      didTrial: null,
+      planProvider: null,
+      planUserCount: 1,
+      createdAt: 'timestamp',
+      updatedAt: 'timestamp',
+      profile: {
+        createdAt: 'timestamp',
+        otherGoal: null,
+        typeProjects: [],
+        goals: [],
+      },
+    },
+  },
+}
+
+const nullUser = {
+  me: null,
+}
+
+const server = setupServer()
+beforeAll(() => {
+  server.listen()
 })
 
 afterEach(() => {
-  queryClient.clear()
+  server.resetHandlers()
   vi.clearAllMocks()
 })
 
+afterAll(() => {
+  server.close()
+})
+
 class TestErrorBoundary extends Component {
   constructor(props) {
     super(props)
@@ -48,9 +103,7 @@ function ErrorComponent({ status, detail, typename, dev, error }) {
   // eslint-disable-next-line no-throw-literal
   throw {
     status,
-    data: {
-      detail,
-    },
+    data: { detail },
     dev,
     error,
     __typename: typename,
@@ -105,16 +158,37 @@ function App({ status, detail, typename, dev, error }) {
 
 const wrapper =
   (initialEntries = ['/gh/codecov', '/gh']) =>
-  ({ children }) => (
-    <QueryClientProvider client={queryClient}>
-      <MemoryRouter initialEntries={initialEntries}>{children}</MemoryRouter>
-    </QueryClientProvider>
-  )
+  ({ children }) => {
+    const queryClient = new QueryClient({
+      defaultOptions: { queries: { retry: false, suspense: true } },
+    })
+
+    return (
+      <QueryClientProvider client={queryClient}>
+        <MemoryRouter initialEntries={initialEntries}>
+          <Route path="/:provider">
+            <Suspense fallback={<div>Loading</div>}>{children}</Suspense>
+          </Route>
+        </MemoryRouter>
+      </QueryClientProvider>
+    )
+  }
 
 describe('NetworkErrorBoundary', () => {
-  function setup({ isSelfHosted = false } = { isSelfHosted: false }) {
+  function setup(
+    { isSelfHosted = false, user = nullUser } = {
+      isSelfHosted: false,
+      user: nullUser,
+    }
+  ) {
     config.IS_SELF_HOSTED = isSelfHosted
 
+    server.use(
+      graphql.query('CurrentUser', () => {
+        return HttpResponse.json({ data: user })
+      })
+    )
+
     return { user: userEvent.setup() }
   }
 
@@ -310,6 +384,41 @@ describe('NetworkErrorBoundary', () => {
         const button = await screen.findByText('Return to previous page')
         expect(button).toBeInTheDocument()
       })
+
+      describe('user is not logged in', () => {
+        it('renders a login button', async () => {
+          const { user } = setup({ user: nullUser })
+          render(<App status={404} />, {
+            wrapper: wrapper(),
+          })
+
+          const textBox = await screen.findByRole('textbox')
+          await user.type(textBox, 'fail')
+
+          const queryString = qs.stringify({ to: '/gh/codecov' })
+          const loginButton = await screen.findByText(/Login/)
+          expect(loginButton).toBeInTheDocument()
+          expect(loginButton).toHaveAttribute('href', `/login?${queryString}`)
+        })
+      })
+
+      describe('user is logged in', () => {
+        it('does not render a login button', async () => {
+          const { user } = setup({ user: mockUser })
+          render(<App status={404} />, {
+            wrapper: wrapper(),
+          })
+
+          const textBox = await screen.findByRole('textbox')
+          await user.type(textBox, 'fail')
+
+          const notFound = await screen.findByText(/Not found/)
+          expect(notFound).toBeInTheDocument()
+
+          const loginButton = screen.queryByText(/Login/)
+          expect(loginButton).not.toBeInTheDocument()
+        })
+      })
     })
 
     describe('when running in self hosted mode', () => {
@@ -322,7 +431,7 @@ describe('NetworkErrorBoundary', () => {
         const textBox = await screen.findByRole('textbox')
         await user.type(textBox, 'fail')
 
-        const pleaseSee = await screen.findByText(/Please see/)
+        const pleaseSee = await screen.findByText(/please see/)
         expect(pleaseSee).toBeInTheDocument()
       })
 
@@ -338,6 +447,41 @@ describe('NetworkErrorBoundary', () => {
         const button = await screen.findByText('Return to previous page')
         expect(button).toBeInTheDocument()
       })
+
+      describe('user is not logged in', () => {
+        it('renders a login button', async () => {
+          const { user } = setup({ user: nullUser, isSelfHosted: true })
+          render(<App status={404} />, {
+            wrapper: wrapper(),
+          })
+
+          const textBox = await screen.findByRole('textbox')
+          await user.type(textBox, 'fail')
+
+          const queryString = qs.stringify({ to: '/gh/codecov' })
+          const loginButton = await screen.findByText(/Login/)
+          expect(loginButton).toBeInTheDocument()
+          expect(loginButton).toHaveAttribute('href', `/?${queryString}`)
+        })
+      })
+
+      describe('user is logged in', () => {
+        it('does not render a login button', async () => {
+          const { user } = setup({ user: mockUser, isSelfHosted: true })
+          render(<App status={404} />, {
+            wrapper: wrapper(),
+          })
+
+          const textBox = await screen.findByRole('textbox')
+          await user.type(textBox, 'fail')
+
+          const notFound = await screen.findByText(/Not found/)
+          expect(notFound).toBeInTheDocument()
+
+          const loginButton = screen.queryByText(/Login/)
+          expect(loginButton).not.toBeInTheDocument()
+        })
+      })
     })
   })
 
@@ -366,7 +510,7 @@ describe('NetworkErrorBoundary', () => {
       global.fetch = vi.fn(() =>
         Promise.resolve({
           ok: true,
-          json: () => Promise.resolve({}),
+          json: () => Promise.resolve({ data: nullUser }),
         })
       )