From 521a5966ee7eeb33bc888fe9807732757a8b6d30 Mon Sep 17 00:00:00 2001 From: nicholas-codecov Date: Mon, 15 Apr 2024 17:36:40 -0300 Subject: [PATCH] fix: Replace processing status indicator (#2776) * update CIStatus to display No Status instead of processing * update pull default header * update team pull header --- .../HeaderDefault/HeaderDefault.spec.jsx | 87 ------- .../HeaderDefault/HeaderDefault.spec.tsx | 208 ++++++++++++++++ .../{HeaderDefault.jsx => HeaderDefault.tsx} | 28 ++- .../HeaderDefault/hooks/usePullHeadData.tsx | 8 +- .../HeaderDefault/{index.js => index.ts} | 0 .../Header/HeaderTeam/HeaderTeam.spec.jsx | 106 --------- .../Header/HeaderTeam/HeaderTeam.spec.tsx | 223 ++++++++++++++++++ .../{HeaderTeam.jsx => HeaderTeam.tsx} | 38 +-- .../HeaderTeam/hooks/usePullHeadDataTeam.tsx | 8 +- .../Header/HeaderTeam/{index.js => index.ts} | 0 src/ui/CIStatus/CIStatus.jsx | 6 +- src/ui/CIStatus/CIStatus.spec.jsx | 4 +- src/ui/CIStatus/CIStatus.stories.jsx | 4 +- 13 files changed, 491 insertions(+), 229 deletions(-) delete mode 100644 src/pages/PullRequestPage/Header/HeaderDefault/HeaderDefault.spec.jsx create mode 100644 src/pages/PullRequestPage/Header/HeaderDefault/HeaderDefault.spec.tsx rename src/pages/PullRequestPage/Header/HeaderDefault/{HeaderDefault.jsx => HeaderDefault.tsx} (76%) rename src/pages/PullRequestPage/Header/HeaderDefault/{index.js => index.ts} (100%) delete mode 100644 src/pages/PullRequestPage/Header/HeaderTeam/HeaderTeam.spec.jsx create mode 100644 src/pages/PullRequestPage/Header/HeaderTeam/HeaderTeam.spec.tsx rename src/pages/PullRequestPage/Header/HeaderTeam/{HeaderTeam.jsx => HeaderTeam.tsx} (73%) rename src/pages/PullRequestPage/Header/HeaderTeam/{index.js => index.ts} (100%) diff --git a/src/pages/PullRequestPage/Header/HeaderDefault/HeaderDefault.spec.jsx b/src/pages/PullRequestPage/Header/HeaderDefault/HeaderDefault.spec.jsx deleted file mode 100644 index 9beaaf94b0..0000000000 --- a/src/pages/PullRequestPage/Header/HeaderDefault/HeaderDefault.spec.jsx +++ /dev/null @@ -1,87 +0,0 @@ -import { QueryClient, QueryClientProvider } from '@tanstack/react-query' -import { render, screen, waitFor } from '@testing-library/react' -import { graphql } from 'msw' -import { setupServer } from 'msw/node' -import { MemoryRouter, Route } from 'react-router-dom' - -import Header from './HeaderDefault' - -const mockPullData = { - owner: { - repository: { - __typename: 'Repository', - pull: { - pullId: 1, - title: 'Cool Pull Request', - state: 'OPEN', - author: { - username: 'cool-user', - }, - head: { - branchName: 'cool-branch', - ciPassed: true, - }, - updatestamp: '2020-01-01T12:00:00.000000', - }, - }, - }, -} - -const queryClient = new QueryClient({ - defaultOptions: { queries: { retry: false } }, -}) -const server = setupServer() -const wrapper = ({ children }) => ( - - - {children} - - -) - -beforeAll(() => { - server.listen() -}) - -afterEach(() => { - queryClient.clear() - server.resetHandlers() -}) - -afterAll(() => { - server.close() -}) - -describe('Header', () => { - function setup() { - server.use( - graphql.query('PullHeadData', (req, res, ctx) => - res(ctx.status(200), ctx.data(mockPullData)) - ) - ) - } - - describe('when rendered', () => { - beforeEach(() => { - setup() - }) - - it('renders the pr overview', async () => { - render(
, { wrapper }) - - await waitFor(() => queryClient.isFetching) - await waitFor(() => !queryClient.isFetching) - - const heading = await screen.findByRole('heading', { - name: /Cool Pull Request/, - }) - expect(heading).toBeInTheDocument() - - const open = await screen.findByText(/open/i) - expect(open).toBeInTheDocument() - - const prNumber = await screen.findByText(/#1/i) - expect(prNumber).toBeInTheDocument() - }) - }) -}) diff --git a/src/pages/PullRequestPage/Header/HeaderDefault/HeaderDefault.spec.tsx b/src/pages/PullRequestPage/Header/HeaderDefault/HeaderDefault.spec.tsx new file mode 100644 index 0000000000..6ba48a3573 --- /dev/null +++ b/src/pages/PullRequestPage/Header/HeaderDefault/HeaderDefault.spec.tsx @@ -0,0 +1,208 @@ +import { QueryClient, QueryClientProvider } from '@tanstack/react-query' +import { render, screen, waitFor } from '@testing-library/react' +import { graphql } from 'msw' +import { setupServer } from 'msw/node' +import { MemoryRouter, Route } from 'react-router-dom' + +import Header from './HeaderDefault' + +const mockPullData = ({ + pullState = 'OPEN', + ciStatus = true, +}: { + pullState: 'OPEN' | 'CLOSED' | 'MERGED' | undefined + ciStatus: boolean | null +}) => ({ + owner: { + repository: { + __typename: 'Repository', + pull: { + pullId: 1, + title: 'Cool Pull Request', + state: pullState, + author: { + username: 'cool-user', + }, + head: { + branchName: 'cool-branch', + ciPassed: ciStatus, + }, + updatestamp: '2020-01-01T12:00:00.000000', + }, + }, + }, +}) + +const queryClient = new QueryClient({ + defaultOptions: { queries: { retry: false } }, +}) +const server = setupServer() +const wrapper: React.FC = ({ children }) => ( + + + {children} + + +) + +beforeAll(() => { + server.listen() +}) + +afterEach(() => { + queryClient.clear() + server.resetHandlers() +}) + +afterAll(() => { + server.close() +}) + +interface SetupArgs { + pullState?: 'OPEN' | 'CLOSED' | 'MERGED' | undefined + ciStatus?: boolean | null + nullPull?: boolean +} + +describe('Header', () => { + function setup({ + pullState = 'OPEN', + ciStatus = true, + nullPull = false, + }: SetupArgs) { + server.use( + graphql.query('PullHeadData', (req, res, ctx) => { + if (nullPull) { + return res(ctx.status(200), ctx.data({ owner: { repository: null } })) + } + + return res( + ctx.status(200), + ctx.data(mockPullData({ pullState, ciStatus })) + ) + }) + ) + } + + describe('when rendered', () => { + it('renders PR title', async () => { + setup({}) + render(
, { wrapper }) + + const heading = await screen.findByRole('heading', { + name: /Cool Pull Request/, + }) + expect(heading).toBeInTheDocument() + }) + + describe('rendering PR status', () => { + describe('when PR is open', () => { + it('renders open PR status', async () => { + setup({ pullState: 'OPEN' }) + render(
, { wrapper }) + + const open = await screen.findByText(/open/i) + expect(open).toBeInTheDocument() + expect(open).toHaveClass('bg-ds-primary-green') + }) + }) + + describe('when PR is closed', () => { + it('renders closed PR status', async () => { + setup({ pullState: 'CLOSED' }) + render(
, { wrapper }) + + const closed = await screen.findByText(/closed/i) + expect(closed).toBeInTheDocument() + expect(closed).toHaveClass('bg-ds-primary-red') + }) + }) + + describe('when PR is merged', () => { + it('renders merged PR status', async () => { + setup({ pullState: 'MERGED' }) + render(
, { wrapper }) + + const merged = await screen.findByText(/merged/i) + expect(merged).toBeInTheDocument() + expect(merged).toHaveClass('bg-ds-primary-purple') + }) + }) + + describe('when PR status is undefined', () => { + it('does not render', async () => { + setup({ nullPull: true }) + render(
, { wrapper }) + + await waitFor(() => queryClient.isFetching) + await waitFor(() => !queryClient.isFetching) + + const open = screen.queryByText(/open/i) + expect(open).not.toBeInTheDocument() + + const closed = screen.queryByText(/closed/i) + expect(closed).not.toBeInTheDocument() + + const merged = screen.queryByText(/merged/i) + expect(merged).not.toBeInTheDocument() + }) + }) + }) + + it('renders the author username', async () => { + setup({}) + render(
, { wrapper }) + + const author = await screen.findByText(/cool-user/i) + expect(author).toBeInTheDocument() + }) + + it('renders the pr id', async () => { + setup({}) + render(
, { wrapper }) + + const prNumber = await screen.findByText(/#1/i) + expect(prNumber).toBeInTheDocument() + }) + + describe('rendering CI status', () => { + describe('when CI status is true', () => { + it('renders CI passed status', async () => { + setup({ ciStatus: true }) + render(
, { wrapper }) + + const ciStatus = await screen.findByText(/CI Passed/i) + expect(ciStatus).toBeInTheDocument() + }) + }) + + describe('when CI status is false', () => { + it('renders CI failed status', async () => { + setup({ ciStatus: false }) + render(
, { wrapper }) + + const ciStatus = await screen.findByText(/CI Failed/i) + expect(ciStatus).toBeInTheDocument() + }) + }) + + describe('when CI status is null', () => { + it('renders no status', async () => { + setup({ ciStatus: null }) + render(
, { wrapper }) + + const ciStatus = await screen.findByText(/No Status/i) + expect(ciStatus).toBeInTheDocument() + }) + }) + }) + + it('renders the branch name', async () => { + setup({}) + render(
, { wrapper }) + + const branchName = await screen.findByText(/cool-branch/i) + expect(branchName).toBeInTheDocument() + }) + }) +}) diff --git a/src/pages/PullRequestPage/Header/HeaderDefault/HeaderDefault.jsx b/src/pages/PullRequestPage/Header/HeaderDefault/HeaderDefault.tsx similarity index 76% rename from src/pages/PullRequestPage/Header/HeaderDefault/HeaderDefault.jsx rename to src/pages/PullRequestPage/Header/HeaderDefault/HeaderDefault.tsx index 18c9d51460..eddb52502c 100644 --- a/src/pages/PullRequestPage/Header/HeaderDefault/HeaderDefault.jsx +++ b/src/pages/PullRequestPage/Header/HeaderDefault/HeaderDefault.tsx @@ -12,8 +12,15 @@ import { usePullHeadData } from './hooks' import { pullStateToColor } from '../constants' +interface URLParams { + provider: string + owner: string + repo: string + pullId: string +} + function HeaderDefault() { - const { provider, owner, repo, pullId } = useParams() + const { provider, owner, repo, pullId } = useParams() const { data } = usePullHeadData({ provider, owner, repo, pullId }) const pull = data?.pull @@ -23,20 +30,23 @@ function HeaderDefault() {

{pull?.title} - - {capitalize(pull?.state)} - + {pull?.state ? ( + + {capitalize(pull?.state)} + + ) : null}

{pull?.updatestamp && formatTimeToNow(pull?.updatestamp)}{' '} {pull?.author?.username} authored{' '} {pull?.pullId && ( + // @ts-expect-error - A needs to be updated to TS ( - - - {children} - - -) - -beforeAll(() => { - server.listen() -}) - -afterEach(() => { - queryClient.clear() - server.resetHandlers() -}) - -afterAll(() => { - server.close() -}) - -describe('Header', () => { - function setup() { - server.use( - graphql.query('PullHeadDataTeam', (req, res, ctx) => - res(ctx.status(200), ctx.data(mockPullData)) - ) - ) - } - - describe('when rendered', () => { - beforeEach(() => { - setup() - }) - - it('renders the pr overview', async () => { - render(

, { wrapper }) - - await waitFor(() => queryClient.isFetching) - await waitFor(() => !queryClient.isFetching) - - const heading = await screen.findByRole('heading', { - name: /Cool Pull Request/, - }) - expect(heading).toBeInTheDocument() - - const open = await screen.findByText(/open/i) - expect(open).toBeInTheDocument() - - const prNumber = await screen.findByText(/#1/i) - expect(prNumber).toBeInTheDocument() - }) - - it('renders the patch coverage', async () => { - render(
, { wrapper }) - - await waitFor(() => queryClient.isFetching) - await waitFor(() => !queryClient.isFetching) - - const patchCoverage = await screen.findByText(/Patch Coverage/) - expect(patchCoverage).toBeInTheDocument() - - const patchCoverageValue = await screen.findByText(/35.45/) - expect(patchCoverageValue).toBeInTheDocument() - }) - }) -}) diff --git a/src/pages/PullRequestPage/Header/HeaderTeam/HeaderTeam.spec.tsx b/src/pages/PullRequestPage/Header/HeaderTeam/HeaderTeam.spec.tsx new file mode 100644 index 0000000000..04087f3350 --- /dev/null +++ b/src/pages/PullRequestPage/Header/HeaderTeam/HeaderTeam.spec.tsx @@ -0,0 +1,223 @@ +import { QueryClient, QueryClientProvider } from '@tanstack/react-query' +import { render, screen, waitFor } from '@testing-library/react' +import { graphql } from 'msw' +import { setupServer } from 'msw/node' +import { MemoryRouter, Route } from 'react-router-dom' + +import Header from './HeaderTeam' + +const mockPullData = ({ + pullState = 'OPEN', + ciStatus = true, +}: { + pullState: 'OPEN' | 'CLOSED' | 'MERGED' | undefined + ciStatus: boolean | null +}) => ({ + owner: { + repository: { + __typename: 'Repository', + pull: { + pullId: 1, + title: 'Cool Pull Request', + state: pullState, + author: { + username: 'cool-user', + }, + head: { + branchName: 'cool-branch', + ciPassed: ciStatus, + }, + updatestamp: '2020-01-01T12:00:00.000000', + compareWithBase: { + __typename: 'Comparison', + patchTotals: { + percentCovered: 35.45, + }, + }, + }, + }, + }, +}) + +const queryClient = new QueryClient({ + defaultOptions: { queries: { retry: false } }, +}) +const server = setupServer() +const wrapper: React.FC = ({ children }) => ( + + + {children} + + +) + +beforeAll(() => { + server.listen() +}) + +afterEach(() => { + queryClient.clear() + server.resetHandlers() +}) + +afterAll(() => { + server.close() +}) + +interface SetupArgs { + pullState?: 'OPEN' | 'CLOSED' | 'MERGED' | undefined + ciStatus?: boolean | null + nullPull?: boolean +} + +describe('Header', () => { + function setup({ + pullState = 'OPEN', + ciStatus = true, + nullPull = false, + }: SetupArgs) { + server.use( + graphql.query('PullHeadDataTeam', (req, res, ctx) => { + if (nullPull) { + return res(ctx.status(200), ctx.data({ owner: { repository: null } })) + } + + return res( + ctx.status(200), + ctx.data(mockPullData({ pullState, ciStatus })) + ) + }) + ) + } + + it('renders the title', async () => { + setup({}) + render(
, { wrapper }) + + const heading = await screen.findByRole('heading', { + name: /Cool Pull Request/, + }) + expect(heading).toBeInTheDocument() + }) + + describe('rendering PR status', () => { + describe('when PR is open', () => { + it('renders open PR status', async () => { + setup({ pullState: 'OPEN' }) + render(
, { wrapper }) + + const open = await screen.findByText(/open/i) + expect(open).toBeInTheDocument() + expect(open).toHaveClass('bg-ds-primary-green') + }) + }) + + describe('when PR is closed', () => { + it('renders PR closed status', async () => { + setup({ pullState: 'CLOSED' }) + render(
, { wrapper }) + + const closed = await screen.findByText(/closed/i) + expect(closed).toBeInTheDocument() + expect(closed).toHaveClass('bg-ds-primary-red') + }) + }) + + describe('when PR is merged', () => { + it('renders merged PR status', async () => { + setup({ pullState: 'MERGED' }) + render(
, { wrapper }) + + const merged = await screen.findByText(/merged/i) + expect(merged).toBeInTheDocument() + expect(merged).toHaveClass('bg-ds-primary-purple') + }) + }) + + describe('when PR status is undefined', () => { + it('does not render PR status', async () => { + setup({ nullPull: true }) + render(
, { wrapper }) + + await waitFor(() => queryClient.isFetching) + await waitFor(() => !queryClient.isFetching) + + const open = screen.queryByText(/open/i) + expect(open).not.toBeInTheDocument() + + const closed = screen.queryByText(/closed/i) + expect(closed).not.toBeInTheDocument() + + const merged = screen.queryByText(/merged/i) + expect(merged).not.toBeInTheDocument() + }) + }) + }) + + it('renders the authors username', async () => { + setup({}) + render(
, { wrapper }) + + const authorUsername = await screen.findByText(/cool-user/i) + expect(authorUsername).toBeInTheDocument() + }) + + it('renders the pull request id', async () => { + setup({}) + render(
, { wrapper }) + + const prNumber = await screen.findByText(/#1/i) + expect(prNumber).toBeInTheDocument() + }) + + describe('rendering CI status', () => { + describe('when CI status is true', () => { + it('renders CI passed status', async () => { + setup({ ciStatus: true }) + render(
, { wrapper }) + + const ciStatus = await screen.findByText(/CI Passed/i) + expect(ciStatus).toBeInTheDocument() + }) + }) + + describe('when CI status is false', () => { + it('renders CI failed status', async () => { + setup({ ciStatus: false }) + render(
, { wrapper }) + + const ciStatus = await screen.findByText(/CI Failed/i) + expect(ciStatus).toBeInTheDocument() + }) + }) + + describe('when CI status is null', () => { + it('renders no status', async () => { + setup({ ciStatus: null }) + render(
, { wrapper }) + + const ciStatus = await screen.findByText(/No Status/i) + expect(ciStatus).toBeInTheDocument() + }) + }) + }) + + it('renders the branch name', async () => { + setup({}) + render(
, { wrapper }) + + const branchName = await screen.findByText(/cool-branch/i) + expect(branchName).toBeInTheDocument() + }) + + it('renders the patch coverage', async () => { + setup({}) + render(
, { wrapper }) + + const patchCoverage = await screen.findByText(/Patch Coverage/) + expect(patchCoverage).toBeInTheDocument() + + const patchCoverageValue = await screen.findByText(/35.45/) + expect(patchCoverageValue).toBeInTheDocument() + }) +}) diff --git a/src/pages/PullRequestPage/Header/HeaderTeam/HeaderTeam.jsx b/src/pages/PullRequestPage/Header/HeaderTeam/HeaderTeam.tsx similarity index 73% rename from src/pages/PullRequestPage/Header/HeaderTeam/HeaderTeam.jsx rename to src/pages/PullRequestPage/Header/HeaderTeam/HeaderTeam.tsx index 9ecaea131c..fc2b0e337f 100644 --- a/src/pages/PullRequestPage/Header/HeaderTeam/HeaderTeam.jsx +++ b/src/pages/PullRequestPage/Header/HeaderTeam/HeaderTeam.tsx @@ -13,11 +13,22 @@ import { usePullHeadDataTeam } from './hooks' import { pullStateToColor } from '../constants' +interface URLParams { + provider: string + owner: string + repo: string + pullId: string +} + function HeaderTeam() { - const { provider, owner, repo, pullId } = useParams() + const { provider, owner, repo, pullId } = useParams() const { data } = usePullHeadDataTeam({ provider, owner, repo, pullId }) const pull = data?.pull + let percentCovered = null + if (pull?.compareWithBase?.__typename === 'Comparison') { + percentCovered = pull?.compareWithBase?.patchTotals?.percentCovered + } return (
@@ -25,20 +36,23 @@ function HeaderTeam() {

{pull?.title} - - {capitalize(pull?.state)} - + {pull?.state ? ( + + {capitalize(pull?.state)} + + ) : null}

{pull?.updatestamp && formatTimeToNow(pull?.updatestamp)}{' '} {pull?.author?.username} authored{' '} {pull?.pullId && ( + // @ts-expect-error - A needs to be updated to TS Patch Coverage - +

diff --git a/src/pages/PullRequestPage/Header/HeaderTeam/hooks/usePullHeadDataTeam.tsx b/src/pages/PullRequestPage/Header/HeaderTeam/hooks/usePullHeadDataTeam.tsx index 9be82dabc2..e8855f5858 100644 --- a/src/pages/PullRequestPage/Header/HeaderTeam/hooks/usePullHeadDataTeam.tsx +++ b/src/pages/PullRequestPage/Header/HeaderTeam/hooks/usePullHeadDataTeam.tsx @@ -41,9 +41,11 @@ const RepositorySchema = z.object({ .object({ pullId: z.number(), title: z.string().nullable(), - state: z - .union([z.literal('OPEN'), z.literal('CLOSED'), z.literal('MERGED')]) - .nullable(), + state: z.union([ + z.literal('OPEN'), + z.literal('CLOSED'), + z.literal('MERGED'), + ]), author: z .object({ username: z.string().nullable(), diff --git a/src/pages/PullRequestPage/Header/HeaderTeam/index.js b/src/pages/PullRequestPage/Header/HeaderTeam/index.ts similarity index 100% rename from src/pages/PullRequestPage/Header/HeaderTeam/index.js rename to src/pages/PullRequestPage/Header/HeaderTeam/index.ts diff --git a/src/ui/CIStatus/CIStatus.jsx b/src/ui/CIStatus/CIStatus.jsx index b2d4213be7..46db6375f1 100644 --- a/src/ui/CIStatus/CIStatus.jsx +++ b/src/ui/CIStatus/CIStatus.jsx @@ -8,10 +8,10 @@ export default function CIStatusLabel({ ciPassed }) { if (isNil(ciPassed)) { return ( - - + + - Processing + No Status ) } diff --git a/src/ui/CIStatus/CIStatus.spec.jsx b/src/ui/CIStatus/CIStatus.spec.jsx index 144890c032..ab024a3644 100644 --- a/src/ui/CIStatus/CIStatus.spec.jsx +++ b/src/ui/CIStatus/CIStatus.spec.jsx @@ -18,9 +18,9 @@ describe('CIStatus', () => { expect(screen.getByText(/Failed/)).toBeInTheDocument() }) - it('shows ci failed if no status is given', () => { + it('shows no status if no status is given', () => { setup({}) - expect(screen.getByText(/Processing/)).toBeInTheDocument() + expect(screen.getByText(/No Status/)).toBeInTheDocument() }) }) }) diff --git a/src/ui/CIStatus/CIStatus.stories.jsx b/src/ui/CIStatus/CIStatus.stories.jsx index 3dc6c85357..a2faed7fe8 100644 --- a/src/ui/CIStatus/CIStatus.stories.jsx +++ b/src/ui/CIStatus/CIStatus.stories.jsx @@ -12,8 +12,8 @@ Failing.args = { ciPassed: false, } -export const Processing = Template.bind({}) -Processing.args = { +export const NoStatus = Template.bind({}) +NoStatus.args = { ciPassed: null, }