Skip to content

chore: Convert to TS - services selfHosted, access, pull, nav #3439

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.

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

Merged
merged 19 commits into from
Nov 7, 2024
Merged
Show file tree
Hide file tree
Changes from 15 commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,6 @@ const BundleSelector = forwardRef<any, BranchSelectorProps>(
history.push(
bundlesLink.path({
branch,
// @ts-expect-error - useNavLinks needs to be typed
bundle: encodeURIComponent(name),
})
)
Expand Down
34 changes: 0 additions & 34 deletions src/services/access/useGenerateUserToken.js

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,15 @@ import { QueryClient, QueryClientProvider } from '@tanstack/react-query'
import { renderHook, waitFor } from '@testing-library/react'
import { graphql, HttpResponse } from 'msw'
import { setupServer } from 'msw/node'
import { PropsWithChildren } from 'react'
import { MemoryRouter, Route } from 'react-router-dom'

import { useGenerateUserToken } from './index'

const queryClient = new QueryClient({
defaultOptions: { queries: { retry: false } },
})
const wrapper = ({ children }) => (
const wrapper: React.FC<PropsWithChildren> = ({ children }) => (
<MemoryRouter initialEntries={['/gh']}>
<Route path="/:provider">
<QueryClientProvider client={queryClient}>{children}</QueryClientProvider>
Expand All @@ -34,25 +35,32 @@ afterAll(() => {
})

describe('useGenerateUserToken', () => {
function setup(dataReturned) {
function setup() {
server.use(
graphql.mutation(`CreateUserToken`, (info) => {
return HttpResponse.json({ data: dataReturned })
return HttpResponse.json({
data: {
createUserToken: {
error: null,
fullToken: 'some-token',
},
},
})
})
)
}

describe('when called', () => {
describe('when calling the mutation', () => {
it('returns success', async () => {
setup({ me: null })
setup()

const { result } = renderHook(
() => useGenerateUserToken({ provider }),
{ wrapper }
)

result.current.mutate({ sessionid: 1 })
result.current.mutate({ name: '1' })
Copy link
Contributor Author

Choose a reason for hiding this comment

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

test was wrong before


await waitFor(() => expect(result.current.isSuccess).toBeTruthy())
})
Expand Down
65 changes: 65 additions & 0 deletions src/services/access/useGenerateUserToken.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
import { useMutation, useQueryClient } from '@tanstack/react-query'
import { z } from 'zod'

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

import { USER_TOKEN_TYPE } from './constants'

const UseGenerateTokenResponseSchema = z.object({
createUserToken: z
.object({
error: z
.union([
Copy link
Contributor

Choose a reason for hiding this comment

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

One small nit, can we move this to a .discriminatedUnion(), you can read a bit more about them here, but they're a super awesome way to union on a single field and makes checking for said field super easy.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Gah totally missed that here - sorry you had to point it out both in the other PR and here - will make sure not to miss that next time!

z.object({
__typename: z.literal('ValidationError'),
message: z.string(),
}),
z.object({
__typename: z.literal('UnauthenticatedError'),
message: z.string(),
}),
])
.nullable(),
fullToken: z.string().nullable(),
})
.nullable(),
})

export function useGenerateUserToken({ provider }: { provider: string }) {
const queryClient = useQueryClient()
return useMutation({
mutationFn: ({ name }: { name: string }) => {
const query = `
mutation CreateUserToken($input: CreateUserTokenInput!) {
createUserToken(input: $input) {
error {
__typename
}
fullToken
}
}
`
const variables = { input: { name, tokenType: USER_TOKEN_TYPE.API } }
return Api.graphqlMutation({
provider,
query,
variables,
mutationPath: 'createUserToken',
})
},
useErrorBoundary: true,
onSuccess: ({ data }) => {
queryClient.invalidateQueries(['sessions'])

const parsedData = UseGenerateTokenResponseSchema.safeParse(data)
if (!parsedData.success) {
return Promise.reject({
status: 404,
data: {},
dev: 'useGenerateUserToken - 404 failed to parse',
} satisfies NetworkErrorObject)
}
},
})
}
File renamed without changes.
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ export const ApiFilterEnum = Object.freeze({
})

// Help convert useForm data to our API
export function normalizeFormData(data) {
export function normalizeFormData(data: Record<string, unknown>) {
return mapValues(data, (value) => {
if (!value) return ''
return value
Expand Down
Original file line number Diff line number Diff line change
@@ -1,16 +1,16 @@
import { act, renderHook } from '@testing-library/react'
import { createMemoryHistory } from 'history'
import { createMemoryHistory, Location, MemoryHistory } from 'history'
import { Route, Router } from 'react-router-dom'

import { useLocationParams } from './useLocationParams'

describe('useLocationParams', () => {
let testLocation
let history
let wrapper
let testLocation: Location | string
let history: MemoryHistory
let wrapper: React.FC<React.PropsWithChildren>

function setup({ location } = {}) {
testLocation = location
function setup({ location }: { location?: Location | string } = {}) {
testLocation = location!
history = createMemoryHistory()

wrapper = ({ children }) => (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import omitBy from 'lodash/omitBy'
import qs from 'qs'
import { useHistory, useLocation } from 'react-router-dom'

export function useLocationParams(defaultParams = {}) {
export function useLocationParams(defaultParams: Record<string, unknown> = {}) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if we can use some TS generics here so that the default params that we pass in are typed to the return value of params

Copy link
Contributor

Choose a reason for hiding this comment

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

And i wonder if we can utilize these same generics to the update functions etc. so that when we call them we know the keys, and their value types.

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 think I had it most of the way there in this file but the changes it touched started to explode to 10+ other files and fixes needed. Spun it into this ticket and I can take another stab later - codecov/engineering-team#2843

Copy link
Contributor

Choose a reason for hiding this comment

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

Okay sounds good

Copy link
Contributor

Choose a reason for hiding this comment

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

If it's gonna be a significant investment, we may want to skip it, and wait until we move routers to save time ... and potentially lots of hair pulling.

const { push } = useHistory()
const { pathname, search, state } = useLocation()
const params = state || {
Expand All @@ -12,7 +12,7 @@ export function useLocationParams(defaultParams = {}) {
}),
}

function updateWindowLocation(params) {
function updateWindowLocation(params: Record<string, unknown>) {
const locationParams = omitBy(
params,
(value, key) => value === defaultParams[key]
Expand All @@ -22,12 +22,12 @@ export function useLocationParams(defaultParams = {}) {
}

// Create new state
function setParams(newParams) {
function setParams(newParams: Record<string, unknown>) {
updateWindowLocation(newParams)
}

// Retain previous state
function updateParams(newParams) {
function updateParams(newParams: Record<string, unknown>) {
updateWindowLocation({
...params,
...newParams,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
import { renderHook } from '@testing-library/react'
import { PropsWithChildren } from 'react'
import { MemoryRouter, Route } from 'react-router-dom'

import { useNavLinks } from './useNavLinks'

const wrapper =
(location) =>
(location: string): React.FC<PropsWithChildren> =>
({ children }) => (
<MemoryRouter initialEntries={[location]} initialIndex={0}>
<Route path="/:provider">{children}</Route>
Expand Down
Loading
Loading