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 useSelfHostedSeatsAndLicense to TS Query V5 #3577

Merged
merged 4 commits into from
Dec 30, 2024
Merged

chore: Migrate useSelfHostedSeatsAndLicense to TS Query V5 #3577

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
8c8e41e
migrate useSelfHostedSeatsAndLicense to SelfHostedSeatsAndLicenseQuer…
nicholas-codecov Dec 12, 2024
e9a5a93
update usage in SelfHostedLicenseExpiration and refactor to TS
nicholas-codecov Dec 12, 2024
dfd3c00
add in comment about short circut
nicholas-codecov Dec 12, 2024
8597356
fix typo
nicholas-codecov Dec 12, 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
24 changes: 17 additions & 7 deletions ...ted/useSelfHostedSeatsAndLicense.test.tsx → ...lfHostedSeatsAndLicenseQueryOpts.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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 { type MockInstance } from 'vitest'

import { useSelfHostedSeatsAndLicense } from './useSelfHostedSeatsAndLicense'
import { SelfHostedSeatsAndLicenseQueryOpts } from './SelfHostedSeatsAndLicenseQueryOpts'

const mockSelfHostedLicense = {
config: {
Expand All @@ -16,12 +20,14 @@ const mockSelfHostedLicense = {

const mockUnsuccessfulParseError = {}

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

const wrapper: React.FC<React.PropsWithChildren> = ({ children }) => (
<QueryClientProvider client={queryClient}>{children}</QueryClientProvider>
<QueryClientProviderV5 client={queryClientV5}>
{children}
</QueryClientProviderV5>
)

const server = setupServer()
Expand All @@ -30,7 +36,7 @@ beforeAll(() => {
})

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

Expand Down Expand Up @@ -61,7 +67,10 @@ describe('useSelfHostedSeatsAndLicense', () => {
it('returns the license details', async () => {
setup({})
const { result } = renderHook(
() => useSelfHostedSeatsAndLicense({ provider: 'gh' }),
() =>
useQueryV5(
SelfHostedSeatsAndLicenseQueryOpts({ provider: 'gh' })
),
{ wrapper }
)

Expand Down Expand Up @@ -91,7 +100,8 @@ describe('useSelfHostedSeatsAndLicense', () => {
it('throws a 404', async () => {
setup({ isUnsuccessfulParseError: true })
const { result } = renderHook(
() => useSelfHostedSeatsAndLicense({ provider: 'gh' }),
() =>
useQueryV5(SelfHostedSeatsAndLicenseQueryOpts({ provider: 'gh' })),
{ wrapper }
)

Expand Down
46 changes: 20 additions & 26 deletions ...elfHosted/useSelfHostedSeatsAndLicense.ts → ...ted/SelfHostedSeatsAndLicenseQueryOpts.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import { useQuery } from '@tanstack/react-query'
import { queryOptions as queryOptionsV5 } from '@tanstack/react-queryV5'
import { z } from 'zod'

import Api from 'shared/api'
import { rejectNetworkError } from 'shared/api/helpers'

export const SelfHostedSeatsAndLicenseSchema = z
.object({
Expand All @@ -19,32 +20,24 @@ export const SelfHostedSeatsAndLicenseSchema = z
})
.nullable()

export interface UseSelfHostedSeatsAndLicenseArgs {
provider: string
opts?: {
enabled: boolean
}
}

const query = `
query SelfHostedSeatsAndLicense {
config {
seatsUsed
seatsLimit
selfHostedLicense {
expirationDate
}
const query = `query SelfHostedSeatsAndLicense {
config {
seatsUsed
seatsLimit
selfHostedLicense {
expirationDate
}
}
`
}`

export interface SelfHostedSeatsAndLicenseQueryArgs {
provider: string
}

export const useSelfHostedSeatsAndLicense = ({
export const SelfHostedSeatsAndLicenseQueryOpts = ({
provider,
opts = {
enabled: true,
},
}: UseSelfHostedSeatsAndLicenseArgs) =>
useQuery({
}: SelfHostedSeatsAndLicenseQueryArgs) =>
queryOptionsV5({
queryKey: ['SelfHostedSeatsAndLicense', provider, query],
queryFn: ({ signal }) =>
Api.graphql({
Expand All @@ -55,13 +48,14 @@ export const useSelfHostedSeatsAndLicense = ({
const parsedRes = SelfHostedSeatsAndLicenseSchema.safeParse(res?.data)

if (!parsedRes.success) {
return Promise.reject({
return rejectNetworkError({
status: 404,
data: null,
data: {},
dev: `SelfHostedSeatsAndLicenseQueryOpts - 404 Failed to parse`,
error: parsedRes.error,
})
}

return parsedRes.data?.config ?? null
}),
...(!!opts && opts),
})
1 change: 0 additions & 1 deletion src/services/selfHosted/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,2 +1 @@
export * from './useSelfHostedCurrentUser'
export * from './useSelfHostedSeatsAndLicense'
49 changes: 31 additions & 18 deletions src/shared/GlobalBanners/SelfHostedLicenseExpiration/SelfHostedLicenseExpiration.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, waitFor } from '@testing-library/react'
import userEvent from '@testing-library/user-event'
import { graphql, HttpResponse } from 'msw'
Expand All @@ -15,45 +19,54 @@ vi.mock('config')
const queryClient = new QueryClient({
defaultOptions: { queries: { retry: false, suspense: true } },
})
const server = setupServer()

const queryClientV5 = new QueryClientV5({
defaultOptions: { queries: { retry: false } },
})

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

const server = setupServer()
beforeAll(() => {
server.listen()
})

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

afterAll(() => {
server.close()
})

const wrapper =
(initialEntries = ['/gh/test-org']) =>
({ children }: { children: React.ReactNode }) => (
<QueryClientProvider client={queryClient}>
<MemoryRouter initialEntries={initialEntries}>
<Suspense fallback={<p>Loading</p>}>
<Route path="/:provider/:owner">{children}</Route>
</Suspense>
</MemoryRouter>
</QueryClientProvider>
)
interface SetupArgs {
isUndefined?: boolean
seatsLimit?: number | null
seatsUsed?: number | null
expirationDate?: string | null
}

describe('SelfHostedLicenseExpiration', () => {
function setup({
isUndefined = false,
seatsLimit = 50,
seatsUsed = 10,
expirationDate = '2020-05-09T00:00:00',
}: {
isUndefined?: boolean
seatsLimit?: number | null
seatsUsed?: number | null
expirationDate?: string | null
}) {
}: SetupArgs) {
const user = userEvent.setup({ delay: null })

server.use(
Expand Down
39 changes: 24 additions & 15 deletions src/shared/GlobalBanners/SelfHostedLicenseExpiration/SelfHostedLicenseExpiration.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
import { useSuspenseQuery as useSuspenseQueryV5 } from '@tanstack/react-queryV5'
import { differenceInCalendarDays } from 'date-fns'
import { useState } from 'react'
import { useParams } from 'react-router-dom'

import config from 'config'

import { useSelfHostedSeatsAndLicense } from 'services/selfHosted/useSelfHostedSeatsAndLicense'
import { SelfHostedSeatsAndLicenseQueryOpts } from 'services/selfHosted/SelfHostedSeatsAndLicenseQueryOpts'
import { Provider } from 'shared/api/helpers'
import Banner from 'ui/Banner'
import Button from 'ui/Button'
Expand Down Expand Up @@ -56,31 +57,25 @@ interface URLParams {
const SelfHostedLicenseExpiration = () => {
const { provider } = useParams<URLParams>()
const [isModalOpen, setIsModalOpen] = useState(false)
const isSelfHosted = !!config.IS_SELF_HOSTED
const isDedicatedNamespace = !!config.IS_DEDICATED_NAMESPACE
const { data } = useSelfHostedSeatsAndLicense({
provider,
opts: { enabled: !!provider && isSelfHosted && isDedicatedNamespace },
})
const { data } = useSuspenseQueryV5(
SelfHostedSeatsAndLicenseQueryOpts({
provider,
})
)

const licenseExpirationDate = data?.selfHostedLicense?.expirationDate
const seatsUsed = data?.seatsUsed
const seatsLimit = data?.seatsLimit

if (
!isSelfHosted ||
!isDedicatedNamespace ||
!licenseExpirationDate ||
!seatsUsed ||
!seatsLimit
) {
if (!licenseExpirationDate || !seatsUsed || !seatsLimit) {
return null
}

const dateDiff = differenceInCalendarDays(
new Date(licenseExpirationDate),
new Date()
)

const isSeatsLimitReached = seatsUsed === seatsLimit
const isLicenseExpired = dateDiff < 0
const isLicenseExpiringWithin30Days = dateDiff < 31 && dateDiff >= 0
Expand Down Expand Up @@ -122,4 +117,18 @@ const SelfHostedLicenseExpiration = () => {
)
}

export default SelfHostedLicenseExpiration
// This wrapper is just so we don't make a request to the API if we're not on a
// self-hosted instance, this is because we're useSuspenseQuery is not
// toggle'ble through a `enabled` field like useQuery
function SelfHostedLicenseExpirationWrapper() {
const isSelfHosted = !!config.IS_SELF_HOSTED
const isDedicatedNamespace = !!config.IS_DEDICATED_NAMESPACE

if (!isSelfHosted || !isDedicatedNamespace) {
return null
}

return <SelfHostedLicenseExpiration />
}

export default SelfHostedLicenseExpirationWrapper
Copy link
Contributor

Choose a reason for hiding this comment

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

I am probably missing it - where is this wrapper component meant to be used?

Was just wondering in case perhaps there's a way to create a generic "conditionalSuspense" or similar wrapper to handle if we have this scenario in other places

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So i played around a bit with this idea and came up with something like:

// ConditionalRenderer.tsx
export const ConditionalRenderer = ({ condition, children }) => {
  if (condition) {
    return <>{children}</>
  }

  return null
}

// SelfHostedLicenseExpiration.tsx
function SelfHostedLicenseExpirationWrapper() {
  const isSelfHosted = !!config.IS_SELF_HOSTED
  const isDedicatedNamespace = !!config.IS_DEDICATED_NAMESPACE

  return (
    <ConditionalRenderer condition={isSelfHosted && isDedicatedNamespace}>
      <SelfHostedLicenseExpiration />
    </ConditionalRenderer>
  )
}

export default SelfHostedLicenseExpirationWrapper

Because of the way TanStack suspense queries work, we're still required to creating this wrapping component to conditionally render the hook call, so the resulting abstraction becomes fairly simple. Because of this I'd rather avoid trying to abstract it, and just handle the conditions directly in the same file.

The main motivating factor behind keeping the conditional rendering in the same file, is that everything is defined up front right in front of the engineer, and if there are more complex condition cases we're not stuck trying to create a "one size fits all" conditional rendering component that is a pain to maintain and makes the app overall more fragile.

Loading