Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Update NotFoundError to have login button #3804

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
66 changes: 54 additions & 12 deletions src/layouts/shared/NetworkErrorBoundary/NetworkErrorBoundary.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -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'

Expand Down Expand Up @@ -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"
Expand All @@ -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"
Expand All @@ -109,13 +110,40 @@ export const NetworkErrorMessage = () => {
our docs
</A>{' '}
for common support.
</>
)
}

DocErrorMessage.propTypes = {
capitalize: PropTypes.bool,
}
Comment on lines +117 to +119
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤢


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
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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>
)
}
Expand All @@ -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>
)
Expand Down
176 changes: 160 additions & 16 deletions src/layouts/shared/NetworkErrorBoundary/NetworkErrorBoundary.test.jsx
Original file line number Diff line number Diff line change
@@ -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'
Expand All @@ -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)
Expand All @@ -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,
Expand Down Expand Up @@ -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() }
}

Expand Down Expand Up @@ -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', () => {
Expand All @@ -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()
})

Expand All @@ -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()
})
})
})
})

Expand Down Expand Up @@ -366,7 +510,7 @@ describe('NetworkErrorBoundary', () => {
global.fetch = vi.fn(() =>
Promise.resolve({
ok: true,
json: () => Promise.resolve({}),
json: () => Promise.resolve({ data: nullUser }),
})
)

Expand Down