From 0833e73f000e0d0c28bec484a500b632a70fb3a1 Mon Sep 17 00:00:00 2001 From: nicholas-codecov Date: Fri, 29 Nov 2024 09:21:55 -0400 Subject: [PATCH 1/2] migrate hook to query options --- .../routes/ComponentsTab/hooks/index.ts | 1 - .../routes/ComponentsTab/hooks/query.ts | 57 ------- .../ComponentComparisonQueryOpts.test.tsx} | 144 +++++++++++------- .../ComponentComparisonQueryOpts.tsx} | 104 ++++++++++--- 4 files changed, 170 insertions(+), 136 deletions(-) delete mode 100644 src/pages/PullRequestPage/PullCoverage/routes/ComponentsTab/hooks/index.ts delete mode 100644 src/pages/PullRequestPage/PullCoverage/routes/ComponentsTab/hooks/query.ts rename src/pages/PullRequestPage/PullCoverage/routes/ComponentsTab/{hooks/useComponentComparison.test.tsx => queries/ComponentComparisonQueryOpts.test.tsx} (65%) rename src/pages/PullRequestPage/PullCoverage/routes/ComponentsTab/{hooks/useComponentComparison.tsx => queries/ComponentComparisonQueryOpts.tsx} (58%) diff --git a/src/pages/PullRequestPage/PullCoverage/routes/ComponentsTab/hooks/index.ts b/src/pages/PullRequestPage/PullCoverage/routes/ComponentsTab/hooks/index.ts deleted file mode 100644 index b19ad78b25..0000000000 --- a/src/pages/PullRequestPage/PullCoverage/routes/ComponentsTab/hooks/index.ts +++ /dev/null @@ -1 +0,0 @@ -export * from './useComponentComparison' diff --git a/src/pages/PullRequestPage/PullCoverage/routes/ComponentsTab/hooks/query.ts b/src/pages/PullRequestPage/PullCoverage/routes/ComponentsTab/hooks/query.ts deleted file mode 100644 index bc87cc1930..0000000000 --- a/src/pages/PullRequestPage/PullCoverage/routes/ComponentsTab/hooks/query.ts +++ /dev/null @@ -1,57 +0,0 @@ -export const query = ` - query PullComponentComparison($owner: String!, $repo: String!, $pullId: Int!, $filters: ComponentsFilters) { - owner(username: $owner) { - repository(name: $repo) { - __typename - ... on Repository { - pull(id: $pullId) { - compareWithBase { - __typename - ... on Comparison { - componentComparisons(filters: $filters) { - name - patchTotals { - percentCovered - } - headTotals { - percentCovered - } - baseTotals { - percentCovered - } - } - } - ... on FirstPullRequest { - message - } - ... on MissingBaseCommit { - message - } - ... on MissingHeadCommit { - message - } - ... on MissingComparison { - message - } - ... on MissingBaseReport { - message - } - ... on MissingHeadReport { - message - } - } - head { - branchName - } - } - } - ... on NotFoundError { - message - } - ... on OwnerNotActivatedError { - message - } - } - } - } -` diff --git a/src/pages/PullRequestPage/PullCoverage/routes/ComponentsTab/hooks/useComponentComparison.test.tsx b/src/pages/PullRequestPage/PullCoverage/routes/ComponentsTab/queries/ComponentComparisonQueryOpts.test.tsx similarity index 65% rename from src/pages/PullRequestPage/PullCoverage/routes/ComponentsTab/hooks/useComponentComparison.test.tsx rename to src/pages/PullRequestPage/PullCoverage/routes/ComponentsTab/queries/ComponentComparisonQueryOpts.test.tsx index e5d2193db8..b59bbf6b35 100644 --- a/src/pages/PullRequestPage/PullCoverage/routes/ComponentsTab/hooks/useComponentComparison.test.tsx +++ b/src/pages/PullRequestPage/PullCoverage/routes/ComponentsTab/queries/ComponentComparisonQueryOpts.test.tsx @@ -1,10 +1,14 @@ -import { QueryClient, QueryClientProvider } from '@tanstack/react-query' +import { + QueryClientProvider as QueryClientProviderV5, + QueryClient as QueryClientV5, + useQuery as useQueryV5, +} from '@tanstack/react-queryV5' import { renderHook, waitFor } from '@testing-library/react' import { graphql, HttpResponse } from 'msw' import { setupServer } from 'msw/node' import { MemoryRouter, Route } from 'react-router-dom' -import { useComponentComparison } from './useComponentComparison' +import { ComponentComparisonQueryOpts } from './ComponentComparisonQueryOpts' const mockResponse = { owner: { @@ -15,22 +19,14 @@ const mockResponse = { __typename: 'Comparison', componentComparisons: [ { - name: 'kevdak', - patchTotals: { - percentCovered: 31.46, - }, - headTotals: { - percentCovered: 71.46, - }, - baseTotals: { - percentCovered: 51.46, - }, + name: 'test-component', + patchTotals: { percentCovered: 31.46 }, + headTotals: { percentCovered: 71.46 }, + baseTotals: { percentCovered: 51.46 }, }, ], }, - head: { - branchName: 'abc', - }, + head: { branchName: 'abc' }, }, }, }, @@ -56,17 +52,19 @@ const mockOwnerNotActivatedError = { const mockUnsuccessfulParseError = {} -const queryClient = new QueryClient({ +const queryClientV5 = new QueryClientV5({ defaultOptions: { queries: { retry: false } }, }) const wrapper: React.FC = ({ children }) => ( - - - {children} - - + + + + {children} + + + ) const server = setupServer() @@ -74,7 +72,7 @@ const server = setupServer() beforeAll(() => server.listen()) beforeEach(() => { server.resetHandlers() - queryClient.clear() + queryClientV5.clear() }) afterAll(() => server.close()) @@ -118,9 +116,18 @@ describe('useComponentComparison', () => { it('returns data for the owner page', async () => { setup({}) - const { result } = renderHook(() => useComponentComparison(), { - wrapper, - }) + const { result } = renderHook( + () => + useQueryV5( + ComponentComparisonQueryOpts({ + provider: 'gh', + owner: 'test-org', + repo: 'test-repo', + pullId: '123', + }) + ), + { wrapper } + ) await waitFor(() => expect(result.current.data).toStrictEqual({ @@ -129,22 +136,14 @@ describe('useComponentComparison', () => { __typename: 'Comparison', componentComparisons: [ { - name: 'kevdak', - patchTotals: { - percentCovered: 31.46, - }, - headTotals: { - percentCovered: 71.46, - }, - baseTotals: { - percentCovered: 51.46, - }, + name: 'test-component', + patchTotals: { percentCovered: 31.46 }, + headTotals: { percentCovered: 71.46 }, + baseTotals: { percentCovered: 51.46 }, }, ], }, - head: { - branchName: 'abc', - }, + head: { branchName: 'abc' }, }, }) ) @@ -164,9 +163,18 @@ describe('useComponentComparison', () => { it('throws a 404', async () => { setup({ isNotFoundError: true }) - const { result } = renderHook(() => useComponentComparison(), { - wrapper, - }) + const { result } = renderHook( + () => + useQueryV5( + ComponentComparisonQueryOpts({ + provider: 'gh', + owner: 'test-org', + repo: 'test-repo', + pullId: '123', + }) + ), + { wrapper } + ) await waitFor(() => expect(result.current.isError).toBeTruthy()) await waitFor(() => @@ -192,9 +200,18 @@ describe('useComponentComparison', () => { it('throws a 403', async () => { setup({ isOwnerNotActivatedError: true }) - const { result } = renderHook(() => useComponentComparison(), { - wrapper, - }) + const { result } = renderHook( + () => + useQueryV5( + ComponentComparisonQueryOpts({ + provider: 'gh', + owner: 'test-org', + repo: 'test-repo', + pullId: '123', + }) + ), + { wrapper } + ) await waitFor(() => expect(result.current.isError).toBeTruthy()) await waitFor(() => @@ -220,9 +237,18 @@ describe('useComponentComparison', () => { it('throws a 404', async () => { setup({ isUnsuccessfulParseError: true }) - const { result } = renderHook(() => useComponentComparison(), { - wrapper, - }) + const { result } = renderHook( + () => + useQueryV5( + ComponentComparisonQueryOpts({ + provider: 'gh', + owner: 'test-org', + repo: 'test-repo', + pullId: '123', + }) + ), + { wrapper } + ) await waitFor(() => expect(result.current.isError).toBeTruthy()) await waitFor(() => expect(result.current.isError).toBeTruthy()) @@ -244,14 +270,18 @@ describe('useComponentComparison', () => { const { result } = renderHook( () => - useComponentComparison({ - filters: { - components: componentsFilter, - }, - }), - { - wrapper, - } + useQueryV5( + ComponentComparisonQueryOpts({ + provider: 'gh', + owner: 'test-org', + repo: 'test-repo', + pullId: '123', + filters: { + components: componentsFilter, + }, + }) + ), + { wrapper } ) await waitFor(() => result.current.isLoading) diff --git a/src/pages/PullRequestPage/PullCoverage/routes/ComponentsTab/hooks/useComponentComparison.tsx b/src/pages/PullRequestPage/PullCoverage/routes/ComponentsTab/queries/ComponentComparisonQueryOpts.tsx similarity index 58% rename from src/pages/PullRequestPage/PullCoverage/routes/ComponentsTab/hooks/useComponentComparison.tsx rename to src/pages/PullRequestPage/PullCoverage/routes/ComponentsTab/queries/ComponentComparisonQueryOpts.tsx index 89788acac5..f58cf09b08 100644 --- a/src/pages/PullRequestPage/PullCoverage/routes/ComponentsTab/hooks/useComponentComparison.tsx +++ b/src/pages/PullRequestPage/PullCoverage/routes/ComponentsTab/queries/ComponentComparisonQueryOpts.tsx @@ -1,6 +1,5 @@ -import { useQuery } from '@tanstack/react-query' +import { queryOptions as queryOptionsV5 } from '@tanstack/react-queryV5' import { type ParsedQs } from 'qs' -import { useParams } from 'react-router-dom' import { z } from 'zod' import { @@ -16,10 +15,9 @@ import { RepoOwnerNotActivatedErrorSchema, } from 'services/repo' import Api from 'shared/api' +import { rejectNetworkError } from 'shared/api/helpers' import A from 'ui/A' -import { query } from './query' - const ComponentsComparisonSchema = z .object({ name: z.string(), @@ -74,24 +72,86 @@ const ComponentComparisonSchema = z.object({ .nullable(), }) -interface URLParams { +const query = ` +query PullComponentComparison( + $owner: String! + $repo: String! + $pullId: Int! + $filters: ComponentsFilters +) { + owner(username: $owner) { + repository(name: $repo) { + __typename + ... on Repository { + pull(id: $pullId) { + compareWithBase { + __typename + ... on Comparison { + componentComparisons(filters: $filters) { + name + patchTotals { + percentCovered + } + headTotals { + percentCovered + } + baseTotals { + percentCovered + } + } + } + ... on FirstPullRequest { + message + } + ... on MissingBaseCommit { + message + } + ... on MissingHeadCommit { + message + } + ... on MissingComparison { + message + } + ... on MissingBaseReport { + message + } + ... on MissingHeadReport { + message + } + } + head { + branchName + } + } + } + ... on NotFoundError { + message + } + ... on OwnerNotActivatedError { + message + } + } + } +}` + +interface ComponentComparisonQueryArgs { provider: string owner: string repo: string pullId: string -} - -interface ComponentComparisonParams { filters?: { components?: string[] | ParsedQs[] } } -export function useComponentComparison({ +export function ComponentComparisonQueryOpts({ + provider, + owner, + repo, + pullId, filters, -}: ComponentComparisonParams = {}) { - const { provider, owner, repo, pullId } = useParams() - return useQuery({ +}: ComponentComparisonQueryArgs) { + return queryOptionsV5({ queryKey: [ 'PullComponentComparison', provider, @@ -114,26 +174,29 @@ export function useComponentComparison({ pullId: parseInt(pullId, 10), }, }).then((res) => { - const parsedData = ComponentComparisonSchema.safeParse(res?.data) + const parsedRes = ComponentComparisonSchema.safeParse(res?.data) - if (!parsedData.success) { - return Promise.reject({ + if (!parsedRes.success) { + return rejectNetworkError({ status: 404, data: {}, + dev: `ComponentComparisonQueryOpts - 404 Failed to parse`, + error: parsedRes.error, }) } - const data = parsedData.data + const data = parsedRes.data if (data?.owner?.repository?.__typename === 'NotFoundError') { - return Promise.reject({ + return rejectNetworkError({ status: 404, data: {}, + dev: `ComponentComparisonQueryOpts - 404 Not Found`, }) } if (data?.owner?.repository?.__typename === 'OwnerNotActivatedError') { - return Promise.reject({ + return rejectNetworkError({ status: 403, data: { detail: ( @@ -145,12 +208,11 @@ export function useComponentComparison({

), }, + dev: `ComponentComparisonQueryOpts - 403 Owner Not Activated`, }) } - return { - pull: data?.owner?.repository?.pull, - } + return { pull: data?.owner?.repository?.pull } }), }) } From 52cfd83652320df46c09c45901090e39ba75987f Mon Sep 17 00:00:00 2001 From: nicholas-codecov Date: Fri, 29 Nov 2024 09:22:34 -0400 Subject: [PATCH 2/2] update ComponentsTable and tests to use query V5 --- .../ComponentsTab/ComponentsTable.test.jsx | 113 ++++++++++-------- .../routes/ComponentsTab/ComponentsTable.tsx | 28 ++++- 2 files changed, 88 insertions(+), 53 deletions(-) diff --git a/src/pages/PullRequestPage/PullCoverage/routes/ComponentsTab/ComponentsTable.test.jsx b/src/pages/PullRequestPage/PullCoverage/routes/ComponentsTab/ComponentsTable.test.jsx index 8d44b3f734..267778eb05 100644 --- a/src/pages/PullRequestPage/PullCoverage/routes/ComponentsTab/ComponentsTable.test.jsx +++ b/src/pages/PullRequestPage/PullCoverage/routes/ComponentsTab/ComponentsTable.test.jsx @@ -1,4 +1,8 @@ import { QueryClient, QueryClientProvider } from '@tanstack/react-query' +import { + QueryClientProvider as QueryClientProviderV5, + QueryClient as QueryClientV5, +} from '@tanstack/react-queryV5' import { render, screen, waitFor } from '@testing-library/react' import { graphql, HttpResponse } from 'msw' import { setupServer } from 'msw/node' @@ -10,31 +14,23 @@ vi.mock('./ComponentsNotConfigured', () => ({ default: () => 'ComponentsNotConfigured', })) -const queryClient = new QueryClient() -const server = setupServer() - -const wrapper = - (initialEntries = '/gh/codecov/gazebo/pull/123/components') => - ({ children }) => ( - - - - {children} - - - - ) - -beforeAll(() => { - server.listen() -}) -afterEach(() => { - queryClient.clear() - server.resetHandlers() -}) -afterAll(() => { - server.close() -}) +const mockPullComponentsResponse = { + owner: { + repository: { + __typename: 'Repository', + pull: { + compareWithBase: { + __typename: 'Comparison', + componentComparisons: [ + { name: 'component-1' }, + { name: 'component-2' }, + { name: 'component-3' }, + ], + }, + }, + }, + }, +} const mockPull = { owner: { @@ -46,26 +42,52 @@ const mockPull = { componentComparisons: [ { name: 'secondTest', - headTotals: { - percentCovered: 82.71, - }, - baseTotals: { - percentCovered: 80.0, - }, - patchTotals: { - percentCovered: 59.0, - }, + headTotals: { percentCovered: 82.71 }, + baseTotals: { percentCovered: 80.0 }, + patchTotals: { percentCovered: 59.0 }, }, ], }, - head: { - branchName: 'abc', - }, + head: { branchName: 'abc' }, }, }, }, } +const server = setupServer() +const queryClient = new QueryClient({ + defaultOptions: { queries: { retry: false } }, +}) +const queryClientV5 = new QueryClientV5({ + defaultOptions: { queries: { retry: false } }, +}) + +const wrapper = + (initialEntries = '/gh/codecov/gazebo/pull/123/components') => + ({ children }) => ( + + + + + {children} + + + + + ) + +beforeAll(() => { + server.listen() +}) +afterEach(() => { + queryClient.clear() + queryClientV5.clear() + server.resetHandlers() +}) +afterAll(() => { + server.close() +}) + describe('ComponentsTable', () => { function setup(overrideData) { const componentsMock = vi.fn() @@ -79,8 +101,10 @@ describe('ComponentsTable', () => { if (overrideData) { return HttpResponse.json({ data: overrideData }) } - return HttpResponse.json({ data: mockPull }) + }), + graphql.query('PullComponentsSelector', () => { + return HttpResponse.json({ data: mockPullComponentsResponse }) }) ) @@ -88,13 +112,8 @@ describe('ComponentsTable', () => { } describe('when there are no components in the new tab', () => { - beforeEach(() => { - setup({ - owner: null, - }) - }) - it('will render card with no dismiss button', async () => { + setup({ owner: null }) render(, { wrapper: wrapper() }) const componentNotConfigured = await screen.findByText( @@ -105,11 +124,8 @@ describe('ComponentsTable', () => { }) describe('when rendered with populated data in the new tab', () => { - beforeEach(() => { - setup() - }) - it('shows title and body', async () => { + setup() render(, { wrapper: wrapper() }) const nameTableField = await screen.findByText(`Name`) @@ -158,6 +174,7 @@ describe('ComponentsTable', () => { describe('when loading', () => { it('renders spinner', () => { + setup() render(, { wrapper: wrapper() }) const spinner = screen.getByTestId('spinner') diff --git a/src/pages/PullRequestPage/PullCoverage/routes/ComponentsTab/ComponentsTable.tsx b/src/pages/PullRequestPage/PullCoverage/routes/ComponentsTab/ComponentsTable.tsx index ac8c8ddac4..3a945f1919 100644 --- a/src/pages/PullRequestPage/PullCoverage/routes/ComponentsTab/ComponentsTable.tsx +++ b/src/pages/PullRequestPage/PullCoverage/routes/ComponentsTab/ComponentsTable.tsx @@ -1,3 +1,4 @@ +import { useQuery as useQueryV5 } from '@tanstack/react-queryV5' import { createColumnHelper, flexRender, @@ -8,14 +9,17 @@ import cs from 'classnames' import isArray from 'lodash/isArray' import qs, { type ParsedQs } from 'qs' import { useMemo } from 'react' -import { useLocation } from 'react-router-dom' +import { useLocation, useParams } from 'react-router-dom' import A from 'ui/A' import Spinner from 'ui/Spinner' import TotalsNumber from 'ui/TotalsNumber' import ComponentsNotConfigured from './ComponentsNotConfigured' -import { ComponentsComparison, useComponentComparison } from './hooks' +import { + ComponentComparisonQueryOpts, + ComponentsComparison, +} from './queries/ComponentComparisonQueryOpts' import ComponentsSelector from '../ComponentsSelector' @@ -91,7 +95,15 @@ function getFilters({ components }: { components?: ParsedQs[] | string[] }) { } } +interface URLParams { + provider: string + owner: string + repo: string + pullId: string +} + export default function ComponentsTable() { + const { provider, owner, repo, pullId } = useParams() const location = useLocation() const queryParams = qs.parse(location.search, { ignoreQueryPrefix: true, @@ -104,9 +116,15 @@ export default function ComponentsTable() { } const filters = getFilters({ components }) - const { data, isLoading } = useComponentComparison({ - filters, - }) + const { data, isLoading } = useQueryV5( + ComponentComparisonQueryOpts({ + provider, + owner, + repo, + pullId, + filters, + }) + ) const tableData = useMemo(() => { if (