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.

Sign up for GitHub

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

chore: Migrate useCommitPageData to TSQ V5 #3537

Merged
merged 5 commits into from
Dec 3, 2024
Merged

chore: Migrate useCommitPageData to TSQ V5 #3537

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
a116a56
update useCommitPageData to query options
nicholas-codecov Nov 28, 2024
94a8215
update CommitCoverage
nicholas-codecov Nov 28, 2024
a8174c3
update CommitDetailPage
nicholas-codecov Nov 28, 2024
027d582
update CommitBundleAnalysis
nicholas-codecov Nov 28, 2024
3dcc556
remove suspense option
nicholas-codecov Dec 3, 2024
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
36 changes: 22 additions & 14 deletions src/pages/CommitDetailPage/CommitBundleAnalysis/CommitBundleAnalysis.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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, HttpResponse } from 'msw'
import { setupServer } from 'msw/node'
Expand Down Expand Up @@ -127,23 +131,26 @@ const mockRepoOverview = ({
})

const queryClient = new QueryClient({
defaultOptions: {
queries: { retry: false, suspense: true },
},
defaultOptions: { queries: { retry: false, suspense: true } },
})
const queryClientV5 = new QueryClientV5({
defaultOptions: { queries: { retry: false } },
})

const wrapper: React.FC<React.PropsWithChildren> = ({ children }) => (
<QueryClientProvider client={queryClient}>
<MemoryRouter
initialEntries={[
'/gh/test-org/test-repo/commit/e736f78b3cb5c8abb1d6b2ec5e5102de455f98ed',
]}
>
<Route path="/:provider/:owner/:repo/commit/:commit">
<Suspense fallback={<p>Loading...</p>}>{children}</Suspense>
</Route>
</MemoryRouter>
</QueryClientProvider>
<QueryClientProviderV5 client={queryClientV5}>
<QueryClientProvider client={queryClient}>
<MemoryRouter
initialEntries={[
'/gh/test-org/test-repo/commit/e736f78b3cb5c8abb1d6b2ec5e5102de455f98ed',
]}
>
<Route path="/:provider/:owner/:repo/commit/:commit">
<Suspense fallback={<p>Loading</p>}>{children}</Suspense>
</Route>
</MemoryRouter>
</QueryClientProvider>
</QueryClientProviderV5>
)

const server = setupServer()
Expand All @@ -153,6 +160,7 @@ beforeAll(() => {

afterEach(() => {
queryClient.clear()
queryClientV5.clear()
server.resetHandlers()
})

Expand Down
20 changes: 13 additions & 7 deletions src/pages/CommitDetailPage/CommitBundleAnalysis/CommitBundleAnalysis.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { useSuspenseQuery as useSuspenseQueryV5 } from '@tanstack/react-queryV5'
import { lazy, Suspense } from 'react'
import { useParams } from 'react-router-dom'

Expand All @@ -9,7 +10,10 @@ import BundleMessage from './BundleMessage'
import EmptyTable from './EmptyTable'
import FirstPullBanner from './FirstPullBanner'

import { TBundleAnalysisComparisonResult, useCommitPageData } from '../hooks'
import {
CommitPageDataQueryOpts,
TBundleAnalysisComparisonResult,
} from '../queries/CommitPageDataQueryOpts'

const CommitBundleAnalysisTable = lazy(
() => import('./CommitBundleAnalysisTable')
Expand Down Expand Up @@ -63,12 +67,14 @@ const BundleContent: React.FC<BundleContentProps> = ({ bundleCompareType }) => {

const CommitBundleAnalysis: React.FC = () => {
const { provider, owner, repo, commit: commitSha } = useParams<URLParams>()
const { data: commitPageData } = useCommitPageData({
provider,
owner,
repo,
commitId: commitSha,
})
const { data: commitPageData } = useSuspenseQueryV5(
CommitPageDataQueryOpts({
provider,
owner,
repo,
commitId: commitSha,
})
)

const bundleCompareType =
commitPageData?.commit?.bundleAnalysis?.bundleAnalysisCompareWithParent
Expand Down
31 changes: 18 additions & 13 deletions src/pages/CommitDetailPage/CommitCoverage/CommitCoverage.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { useSuspenseQuery as useSuspenseQueryV5 } from '@tanstack/react-queryV5'
import isEmpty from 'lodash/isEmpty'
import { lazy, Suspense } from 'react'
import { Redirect, Switch, useParams } from 'react-router-dom'
Expand All @@ -22,7 +23,7 @@ import ErroredUploads from './ErroredUploads'
import FirstPullBanner from './FirstPullBanner'
import YamlErrorBanner from './YamlErrorBanner'

import { useCommitPageData } from '../hooks'
import { CommitPageDataQueryOpts } from '../queries/CommitPageDataQueryOpts'

const CommitDetailFileExplorer = lazy(
() => import('./routes/CommitDetailFileExplorer')
Expand All @@ -45,12 +46,14 @@ function CommitRoutes() {
const { provider, owner, repo, commit: commitSha } = useParams()
const { data: tierName } = useTier({ owner, provider })
const { data: overview } = useRepoOverview({ provider, owner, repo })
const { data: commitPageData } = useCommitPageData({
provider,
owner,
repo,
commitId: commitSha,
})
const { data: commitPageData } = useSuspenseQueryV5(
CommitPageDataQueryOpts({
provider,
owner,
repo,
commitId: commitSha,
Copy link
Contributor

Choose a reason for hiding this comment

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

Unrelated question - Do you happen to know how pervasively we allow sha and id to be interchanged like it is here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think it's too bad, but it is a symptom of the API argument for commit sha's being id, see:

query CommitPageData($owner: String!, $repo: String!, $commitId: String!) {
  owner(username: $owner) {
    isCurrentUserPartOfOrg
    repository(name: $repo) {
      __typename
      ... on Repository {
        private
        bundleAnalysisEnabled
        coverageEnabled
        commit(id: $commitId) { # <- should be sha imo
          commitid
          compareWithParent {
            __typename
          }
          bundleAnalysis {
            bundleAnalysisCompareWithParent {
              __typename
            }
          }
        }
      }
      ... on NotFoundError {
        message
      }
      ... on OwnerNotActivatedError {
        message
      }
    }
  }
}

Copy link
Contributor

Choose a reason for hiding this comment

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

Gotcha, yeah I guess the api accepts both as the key. Thanks for looking at it!

})
)

const compareTypeName = commitPageData?.commit?.compareWithParent?.__typename
const ErrorBannerComponent = (
Expand Down Expand Up @@ -174,12 +177,14 @@ function CommitCoverage() {
const { data: tierName } = useTier({ owner, provider })
const { data: overview } = useRepoOverview({ provider, owner, repo })
const { data: rateLimit } = useRepoRateLimitStatus({ provider, owner, repo })
const { data: commitPageData } = useCommitPageData({
provider,
owner,
repo,
commitId: commitSha,
})
const { data: commitPageData } = useSuspenseQueryV5(
CommitPageDataQueryOpts({
provider,
owner,
repo,
commitId: commitSha,
})
)

const showCommitSummary = !(overview.private && tierName === TierNames.TEAM)
const showFirstPullBanner =
Expand Down
Loading
Loading