From baa9f9d1a597f55d90adb29d9e99af8b75494edc Mon Sep 17 00:00:00 2001 From: nicholas-codecov Date: Mon, 3 Feb 2025 09:19:55 -0400 Subject: [PATCH 1/3] migrate hook over to queryOptions --- ...tsx => SunburstCoverageQueryOpts.test.tsx} | 58 ++++++++------- .../charts/SunburstCoverageQueryOpts.ts | 72 +++++++++++++++++++ src/services/charts/index.ts | 1 - src/services/charts/useSunburstCoverage.ts | 45 ------------ 4 files changed, 103 insertions(+), 73 deletions(-) rename src/services/charts/{useSunburstCoverage.test.tsx => SunburstCoverageQueryOpts.test.tsx} (80%) create mode 100644 src/services/charts/SunburstCoverageQueryOpts.ts delete mode 100644 src/services/charts/index.ts delete mode 100644 src/services/charts/useSunburstCoverage.ts diff --git a/src/services/charts/useSunburstCoverage.test.tsx b/src/services/charts/SunburstCoverageQueryOpts.test.tsx similarity index 80% rename from src/services/charts/useSunburstCoverage.test.tsx rename to src/services/charts/SunburstCoverageQueryOpts.test.tsx index ca8658968c..a53662387d 100644 --- a/src/services/charts/useSunburstCoverage.test.tsx +++ b/src/services/charts/SunburstCoverageQueryOpts.test.tsx @@ -1,23 +1,27 @@ -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 { http, HttpResponse } from 'msw' import { setupServer } from 'msw/node' import { MemoryRouter, Route } from 'react-router-dom' -import { useSunburstCoverage } from './index' +import { SunburstCoverageQueryOpts } from './SunburstCoverageQueryOpts' -const queryClient = new QueryClient({ +const queryClientV5 = new QueryClientV5({ defaultOptions: { queries: { retry: false } }, }) const wrapper = (initialEntries = '/gh'): React.FC => ({ children }) => ( - + {children} - + ) const server = setupServer() @@ -27,7 +31,7 @@ beforeAll(() => { afterEach(() => { server.resetHandlers() - queryClient.clear() + queryClientV5.clear() }) afterAll(() => { @@ -110,15 +114,15 @@ describe('useSunburstCoverage', () => { it('returns chart data', async () => { const { result } = renderHook( () => - useSunburstCoverage({ - provider: 'github', - owner: 'critical role', - repo: 'c3', - query: {}, - }), - { - wrapper: wrapper(), - } + useQueryV5( + SunburstCoverageQueryOpts({ + provider: 'github', + owner: 'critical role', + repo: 'c3', + query: {}, + }) + ), + { wrapper: wrapper() } ) await waitFor(() => !result.current.isFetching) @@ -163,18 +167,18 @@ describe('useSunburstCoverage', () => { it('returns filtered data', async () => { const { result } = renderHook( () => - useSunburstCoverage({ - provider: 'github', - owner: 'critical role', - repo: 'c3', - query: { - flags: ['flag1', 'flag2'], - components: ['c1'], - }, - }), - { - wrapper: wrapper(), - } + useQueryV5( + SunburstCoverageQueryOpts({ + provider: 'github', + owner: 'critical role', + repo: 'c3', + query: { + flags: ['flag1', 'flag2'], + components: ['c1'], + }, + }) + ), + { wrapper: wrapper() } ) await waitFor(() => !result.current.isFetching) diff --git a/src/services/charts/SunburstCoverageQueryOpts.ts b/src/services/charts/SunburstCoverageQueryOpts.ts new file mode 100644 index 0000000000..768dd9fd2a --- /dev/null +++ b/src/services/charts/SunburstCoverageQueryOpts.ts @@ -0,0 +1,72 @@ +import { queryOptions as queryOptionsV5 } from '@tanstack/react-queryV5' +import { z } from 'zod' + +import Api from 'shared/api' +import { Provider, rejectNetworkError } from 'shared/api/helpers' +import { providerToInternalProvider } from 'shared/utils/provider' + +function getSunburstCoverage({ provider, owner, repo }: SunburstCoverageArgs) { + const internalProvider = providerToInternalProvider(provider) + return `/${internalProvider}/${owner}/${repo}/coverage/tree` +} + +const baseResponseSchema = z.object({ + name: z.string(), + fullPath: z.string(), + coverage: z.number(), + lines: z.number(), + hits: z.number(), + partials: z.number(), + misses: z.number(), +}) + +type SunburstResponse = z.infer & { + children?: SunburstResponse[] +} + +const SunburstSchema: z.ZodType = baseResponseSchema.extend({ + children: z.lazy(() => SunburstSchema.array()).optional(), +}) + +const ResponseSchema = z.array(SunburstSchema) + +interface SunburstCoverageArgs { + provider: Provider + owner: string + repo: string + query?: { + branch?: string + flags?: string[] + components?: string[] + } + signal?: AbortSignal +} + +export function SunburstCoverageQueryOpts({ + provider, + owner, + repo, + query, +}: SunburstCoverageArgs) { + return queryOptionsV5({ + queryKey: ['organization', 'coverage', provider, owner, repo, query], + queryFn: ({ signal }) => { + const path = getSunburstCoverage({ provider, owner, repo }) + return Api.get({ path, provider, query, signal }).then((res) => { + const parsedRes = ResponseSchema.safeParse(res) + + if (!parsedRes.success) { + console.error(parsedRes.error) + return rejectNetworkError({ + status: 404, + data: {}, + dev: 'SunburstCoverageQueryOpts - 404 Failed to parse data', + error: parsedRes.error, + }) + } + + return parsedRes.data + }) + }, + }) +} diff --git a/src/services/charts/index.ts b/src/services/charts/index.ts deleted file mode 100644 index 540a262ab2..0000000000 --- a/src/services/charts/index.ts +++ /dev/null @@ -1 +0,0 @@ -export * from './useSunburstCoverage' diff --git a/src/services/charts/useSunburstCoverage.ts b/src/services/charts/useSunburstCoverage.ts deleted file mode 100644 index 67a88d3495..0000000000 --- a/src/services/charts/useSunburstCoverage.ts +++ /dev/null @@ -1,45 +0,0 @@ -import { useQuery } from '@tanstack/react-query' - -import Api from 'shared/api' -import { Provider } from 'shared/api/helpers' -import { providerToInternalProvider } from 'shared/utils/provider' - -interface SunburstCoverageArgs { - provider: Provider - owner: string - repo: string - query?: { - branch?: string - flags?: string[] - components?: string[] - } - signal?: AbortSignal -} - -function getSunburstCoverage({ provider, owner, repo }: SunburstCoverageArgs) { - const internalProvider = providerToInternalProvider(provider) - return `/${internalProvider}/${owner}/${repo}/coverage/tree` -} - -function fetchSunburstCoverage({ - provider, - owner, - repo, - query, - signal, -}: SunburstCoverageArgs) { - const path = getSunburstCoverage({ provider, owner, repo }) - return Api.get({ path, provider, query, signal }) -} - -export function useSunburstCoverage( - { provider, owner, repo, query }: SunburstCoverageArgs, - opts = {} -) { - return useQuery({ - queryKey: ['organization', 'coverage', provider, owner, repo, query], - queryFn: ({ signal }) => - fetchSunburstCoverage({ provider, owner, query, repo, signal }), - ...opts, - }) -} From 38ba23a1ef9d232c6001585fe64a7b333cdee3ae Mon Sep 17 00:00:00 2001 From: nicholas-codecov Date: Mon, 3 Feb 2025 09:20:26 -0400 Subject: [PATCH 2/3] refactor useSunburstChart to TS and use new query opts --- .../Sunburst/hooks/useSunburstChart.js | 43 ------ ...art.test.jsx => useSunburstChart.test.tsx} | 128 +++++++++--------- .../Sunburst/hooks/useSunburstChart.ts | 51 +++++++ 3 files changed, 114 insertions(+), 108 deletions(-) delete mode 100644 src/pages/RepoPage/CoverageTab/OverviewTab/subroute/Sunburst/hooks/useSunburstChart.js rename src/pages/RepoPage/CoverageTab/OverviewTab/subroute/Sunburst/hooks/{useSunburstChart.test.jsx => useSunburstChart.test.tsx} (68%) create mode 100644 src/pages/RepoPage/CoverageTab/OverviewTab/subroute/Sunburst/hooks/useSunburstChart.ts diff --git a/src/pages/RepoPage/CoverageTab/OverviewTab/subroute/Sunburst/hooks/useSunburstChart.js b/src/pages/RepoPage/CoverageTab/OverviewTab/subroute/Sunburst/hooks/useSunburstChart.js deleted file mode 100644 index 68cd8c194d..0000000000 --- a/src/pages/RepoPage/CoverageTab/OverviewTab/subroute/Sunburst/hooks/useSunburstChart.js +++ /dev/null @@ -1,43 +0,0 @@ -import qs from 'qs' -import { useLocation, useParams } from 'react-router-dom' - -import { useSunburstCoverage } from 'services/charts' -import { useRepoOverview } from 'services/repo' - -const useSunburstChart = () => { - const { provider, owner, repo, branch } = useParams() - const { data: overview } = useRepoOverview({ - provider, - repo, - owner, - }) - - const location = useLocation() - const queryParams = qs.parse(location.search, { - ignoreQueryPrefix: true, - depth: 1, - }) - - const flags = queryParams?.flags - const components = queryParams?.components - - const currentBranch = branch - ? decodeURIComponent(branch) - : overview?.defaultBranch - - return useSunburstCoverage( - { - provider, - owner, - repo, - query: { branch: currentBranch, flags, components }, - }, - { - enabled: !!currentBranch, - suspense: false, - select: (data) => data && { name: repo, children: data }, - } - ) -} - -export default useSunburstChart diff --git a/src/pages/RepoPage/CoverageTab/OverviewTab/subroute/Sunburst/hooks/useSunburstChart.test.jsx b/src/pages/RepoPage/CoverageTab/OverviewTab/subroute/Sunburst/hooks/useSunburstChart.test.tsx similarity index 68% rename from src/pages/RepoPage/CoverageTab/OverviewTab/subroute/Sunburst/hooks/useSunburstChart.test.jsx rename to src/pages/RepoPage/CoverageTab/OverviewTab/subroute/Sunburst/hooks/useSunburstChart.test.tsx index 17dd51a25c..ff3bfdf660 100644 --- a/src/pages/RepoPage/CoverageTab/OverviewTab/subroute/Sunburst/hooks/useSunburstChart.test.jsx +++ b/src/pages/RepoPage/CoverageTab/OverviewTab/subroute/Sunburst/hooks/useSunburstChart.test.tsx @@ -1,4 +1,8 @@ import { QueryClient, QueryClientProvider } from '@tanstack/react-query' +import { + QueryClientProvider as QueryClientProviderV5, + QueryClient as QueryClientV5, +} from '@tanstack/react-queryV5' import { renderHook, waitFor } from '@testing-library/react' import { graphql, http, HttpResponse } from 'msw' import { setupServer } from 'msw/node' @@ -8,37 +12,56 @@ import { MemoryRouter, Route } from 'react-router-dom' import useSunburstChart from './useSunburstChart' const queryClient = new QueryClient({ - defaultOptions: { - queries: { - retry: false, - }, - }, + defaultOptions: { queries: { retry: false } }, +}) +const queryClientV5 = new QueryClientV5({ + defaultOptions: { queries: { retry: false } }, }) -const server = setupServer() const wrapper = - (initialEntries = ['/gh/codecov/cool-repo/tree/main']) => + ( + initialEntries = ['/gh/codecov/cool-repo/tree/main'] + ): React.FC => ({ children }) => ( - - - {children} - {children} - - + + + + {children} + {children} + + + ) +const server = setupServer() beforeAll(() => { server.listen({ onUnhandledRequest: 'warn' }) }) + afterEach(() => { + vi.clearAllMocks() queryClient.clear() + queryClientV5.clear() server.resetHandlers() }) + afterAll(() => { server.close() }) -const treeMock = { name: 'repoName', children: [] } +const treeMock = [ + { + name: 'repoName', + fullPath: 'repoName', + coverage: 100, + lines: 100, + hits: 100, + partials: 0, + misses: 0, + children: [], + }, +] + const overviewMock = { owner: { isCurrentUserActivated: true, @@ -55,19 +78,19 @@ const overviewMock = { }, } +interface SetupArgs { + coverageTreeStatus?: number +} + describe('useSunburstChart', () => { const mockDetectedBranch = vi.fn() const mockDetectedFlags = vi.fn() const mockDetectedComponents = vi.fn() - function setup({ - repoOverviewData, - coverageTreeRes, - coverageTreeStatus = 200, - }) { + function setup({ coverageTreeStatus = 200 }: SetupArgs) { server.use( graphql.query('GetRepoOverview', () => { - return HttpResponse.json({ data: repoOverviewData }) + return HttpResponse.json({ data: overviewMock }) }), http.get('/internal/:provider/:owner/:repo/coverage/tree', (info) => { const searchParams = new URL(info.request.url).searchParams @@ -76,24 +99,16 @@ describe('useSunburstChart', () => { mockDetectedFlags(searchParams.getAll('flags')) mockDetectedComponents(searchParams.get('components')) - return HttpResponse.json( - { data: coverageTreeRes }, - { status: coverageTreeStatus } - ) + return HttpResponse.json(treeMock, { + status: coverageTreeStatus, + }) }) ) } describe('successful call', () => { - beforeEach(() => { - setup({ - repoOverviewData: overviewMock, - coverageTreeRes: treeMock, - coverageTreeStatus: 200, - }) - }) - it('renders something', async () => { + setup({ coverageTreeStatus: 200 }) const { result } = renderHook(() => useSunburstChart(), { wrapper: wrapper(), }) @@ -101,7 +116,18 @@ describe('useSunburstChart', () => { await waitFor(() => expect(result.current.data).toMatchObject({ name: 'cool-repo', - children: { data: { name: 'repoName', children: [] } }, + children: [ + { + name: 'repoName', + fullPath: 'repoName', + coverage: 100, + lines: 100, + hits: 100, + partials: 0, + misses: 0, + children: [], + }, + ], }) ) }) @@ -110,14 +136,10 @@ describe('useSunburstChart', () => { describe('unsuccessful call', () => { beforeEach(() => { vi.spyOn(console, 'error') - setup({ - repoOverviewData: overviewMock, - coverageTreeStatus: 500, - }) }) - afterEach(() => vi.clearAllMocks()) it('returns undefined data if no data is received from the server', async () => { + setup({ coverageTreeStatus: 500 }) const { result } = renderHook(() => useSunburstChart(), { wrapper: wrapper(), }) @@ -129,16 +151,8 @@ describe('useSunburstChart', () => { }) describe('if no branch provided in url', () => { - beforeEach(() => { - setup({ - repoOverviewData: overviewMock, - coverageTreeRes: treeMock, - coverageTreeStatus: 200, - }) - }) - afterEach(() => vi.clearAllMocks()) - it('query using default branch', async () => { + setup({ coverageTreeStatus: 200 }) renderHook(() => useSunburstChart(), { wrapper: wrapper(['/critical-role/c3/bells-hells']), }) @@ -151,16 +165,8 @@ describe('useSunburstChart', () => { }) describe('if branch is in the url', () => { - beforeEach(() => { - setup({ - repoOverviewData: overviewMock, - coverageTreeRes: treeMock, - coverageTreeStatus: 200, - }) - }) - afterEach(() => vi.clearAllMocks()) - it('query uses current branch', async () => { + setup({ coverageTreeStatus: 200 }) renderHook(() => useSunburstChart(), { wrapper: wrapper(['/critical-role/c3/bells-hells/tree/something']), }) @@ -173,16 +179,8 @@ describe('useSunburstChart', () => { }) describe('if flags and components are in the url', () => { - beforeEach(() => { - setup({ - repoOverviewData: overviewMock, - coverageTreeRes: treeMock, - coverageTreeStatus: 200, - }) - }) - afterEach(() => vi.clearAllMocks()) - it('query uses flags and components', async () => { + setup({ coverageTreeStatus: 200 }) const queryString = qs.stringify( { flags: ['flag-1', 'flag-2'], components: ['components-1'] }, { addQueryPrefix: true } diff --git a/src/pages/RepoPage/CoverageTab/OverviewTab/subroute/Sunburst/hooks/useSunburstChart.ts b/src/pages/RepoPage/CoverageTab/OverviewTab/subroute/Sunburst/hooks/useSunburstChart.ts new file mode 100644 index 0000000000..64056daae8 --- /dev/null +++ b/src/pages/RepoPage/CoverageTab/OverviewTab/subroute/Sunburst/hooks/useSunburstChart.ts @@ -0,0 +1,51 @@ +import { useQuery } from '@tanstack/react-queryV5' +import qs from 'qs' +import { useLocation, useParams } from 'react-router-dom' + +import { SunburstCoverageQueryOpts } from 'services/charts/SunburstCoverageQueryOpts' +import { useRepoOverview } from 'services/repo' +import { Provider } from 'shared/api/helpers' + +interface URLParams { + provider: Provider + owner: string + repo: string + branch: string +} + +const useSunburstChart = () => { + const { provider, owner, repo, branch } = useParams() + const { data: overview } = useRepoOverview({ + provider, + repo, + owner, + }) + + const location = useLocation() + const queryParams = qs.parse(location.search, { + ignoreQueryPrefix: true, + depth: 1, + }) + + const flags = queryParams?.flags as string[] | undefined + const components = queryParams?.components as string[] | undefined + + const currentBranch = branch + ? decodeURIComponent(branch) + : overview?.defaultBranch + ? overview?.defaultBranch + : '' + + return useQuery({ + ...SunburstCoverageQueryOpts({ + provider, + owner, + repo, + query: { branch: currentBranch, flags, components }, + }), + enabled: !!currentBranch, + select: (data) => (data ? { name: repo, children: data } : null), + }) +} + +export default useSunburstChart From dff687a3dc6595729199a2587a1e77f3beda87f0 Mon Sep 17 00:00:00 2001 From: nicholas-codecov Date: Mon, 3 Feb 2025 09:21:51 -0400 Subject: [PATCH 3/3] update Sunbrst tests --- .../subroute/Sunburst/Sunburst.test.jsx | 75 ++++++++++--------- 1 file changed, 40 insertions(+), 35 deletions(-) diff --git a/src/pages/RepoPage/CoverageTab/OverviewTab/subroute/Sunburst/Sunburst.test.jsx b/src/pages/RepoPage/CoverageTab/OverviewTab/subroute/Sunburst/Sunburst.test.jsx index a1b55482e0..e9c4472f36 100644 --- a/src/pages/RepoPage/CoverageTab/OverviewTab/subroute/Sunburst/Sunburst.test.jsx +++ b/src/pages/RepoPage/CoverageTab/OverviewTab/subroute/Sunburst/Sunburst.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 } from '@testing-library/react' import { graphql, http, HttpResponse } from 'msw' import { setupServer } from 'msw/node' @@ -9,34 +13,50 @@ import Sunburst, { getPathsToDisplay } from './Sunburst' vi.mock('ui/SunburstChart', () => ({ default: () => 'Chart Mocked' })) const queryClient = new QueryClient({ - defaultOptions: { - queries: { - retry: false, - }, - }, + defaultOptions: { queries: { retry: false } }, +}) +const queryClientV5 = new QueryClientV5({ + defaultOptions: { queries: { retry: false } }, }) -const server = setupServer() const wrapper = ({ children }) => ( - - - {children} - - + + + + {children} + + + ) +const server = setupServer() beforeAll(() => { server.listen({ onUnhandledRequest: 'warn' }) }) + afterEach(() => { queryClient.clear() + queryClientV5.clear() server.resetHandlers() }) + afterAll(() => { server.close() }) -const treeMock = { name: 'repoName', children: [] } +const treeMock = [ + { + name: 'repoName', + fullPath: 'repoName', + coverage: 100, + lines: 100, + hits: 100, + partials: 0, + misses: 0, + children: [], + }, +] + const overviewMock = { owner: { isCurrentUserActivated: true, @@ -52,6 +72,7 @@ const overviewMock = { }, }, } + const repoConfigMock = { owner: { repository: { @@ -64,37 +85,25 @@ const repoConfigMock = { } describe('Sunburst', () => { - function setup({ - repoOverviewData, - coverageTreeRes, - coverageTreeStatus = 200, - }) { + function setup({ coverageTreeStatus = 200 }) { server.use( graphql.query('GetRepoOverview', () => { - return HttpResponse.json({ data: repoOverviewData }) + return HttpResponse.json({ data: overviewMock }) }), graphql.query('RepoConfig', () => { return HttpResponse.json({ data: repoConfigMock }) }), http.get('/internal/:provider/:owner/:repo/coverage/tree', () => { - return HttpResponse.json( - { data: coverageTreeRes }, - { status: coverageTreeStatus } - ) + return HttpResponse.json(treeMock, { + status: coverageTreeStatus, + }) }) ) } describe('successful call', () => { - beforeEach(() => { - setup({ - repoOverviewData: overviewMock, - coverageTreeRes: treeMock, - coverageTreeStatus: 200, - }) - }) - it('renders something', async () => { + setup({ coverageTreeStatus: 200 }) render(, { wrapper }) const chart = await screen.findByText('Chart Mocked') @@ -107,14 +116,10 @@ describe('Sunburst', () => { beforeEach(() => { // disable intentional error in vi log vi.spyOn(console, 'error').mockImplementation(() => {}) - - setup({ - repoOverviewData: overviewMock, - coverageTreeStatus: 500, - }) }) it('renders something', async () => { + setup({ repoOverviewData: overviewMock, coverageTreeStatus: 500 }) render(, { wrapper }) const chart = await screen.findByText(