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.

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

fix: Seperate org name from error banner on 404 page #3525

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
aee8355
add in new query options for grabbing data for the Navigator
nicholas-codecov Nov 25, 2024
30970af
update logic in Navigator to handle all cases
nicholas-codecov Nov 25, 2024
ccff770
pass props through to Navigator
nicholas-codecov Nov 25, 2024
e10d58e
update BaseLayout to fetch data for Navigator
nicholas-codecov Nov 25, 2024
641168d
add in v5 query client to app tests and navigator data mock
nicholas-codecov Nov 25, 2024
f2f550b
wrap each banner in their own silent network error wrapper
nicholas-codecov Nov 25, 2024
3336b43
remove case where user doesn't belong to org as it removes the escape…
nicholas-codecov Nov 25, 2024
fe3f496
render the context switcher to the owner page if the user does not ha…
nicholas-codecov Nov 25, 2024
60dd83e
rework logic a bit more
nicholas-codecov Nov 25, 2024
5c0ab53
update tests as there were some issues with the routing in the wrapper
nicholas-codecov Nov 25, 2024
5a2997a
render fallback only if the owner exists
nicholas-codecov Nov 25, 2024
2ae45bd
add in test for failing to parse
nicholas-codecov Nov 25, 2024
501c32b
add some extra context in comment
nicholas-codecov Nov 25, 2024
074001d
add tests where use belongs to org but doesn't have access to a repo
nicholas-codecov Nov 25, 2024
5bb65c4
add in test for repo page user not being a vistor
nicholas-codecov Nov 28, 2024
662ce1b
update ContextSwitcher to read owner param
nicholas-codecov Dec 2, 2024
4a29705
render context select always even if there's no owner
nicholas-codecov Dec 2, 2024
76a076e
Merge branch 'main' into gh-eng-2830-seperate-org-name-from-error-ban…
nicholas-codecov Dec 2, 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
54 changes: 36 additions & 18 deletions src/App.test.tsx
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 { graphql, http, HttpResponse } from 'msw'
import { setupServer } from 'msw/node'
Expand Down Expand Up @@ -121,32 +125,43 @@ const mockRepoOverview = {
},
}

const queryClient = new QueryClient({
defaultOptions: {
queries: {
retry: false,
const mockNavigatorData = {
owner: {
isCurrentUserPartOfOrg: true,
repository: {
__typename: 'Repository',
name: 'test-repo',
},
},
}

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

let testLocation: ReturnType<typeof useLocation>
const wrapper =
(initialEntries = ['']): React.FC<React.PropsWithChildren> =>
({ children }) => (
<QueryClientProvider client={queryClient}>
<MemoryRouter initialEntries={initialEntries}>
<Suspense fallback={<p>Loading</p>}>
{children}
<Route
path="*"
render={({ location }) => {
testLocation = location
return null
}}
/>
</Suspense>
</MemoryRouter>
</QueryClientProvider>
<QueryClientProviderV5 client={queryClientV5}>
<QueryClientProvider client={queryClient}>
<MemoryRouter initialEntries={initialEntries}>
<Suspense fallback={<p>Loading</p>}>
{children}
<Route
path="*"
render={({ location }) => {
testLocation = location
return null
}}
/>
</Suspense>
</MemoryRouter>
</QueryClientProvider>
</QueryClientProviderV5>
)

const server = setupServer()
Expand Down Expand Up @@ -231,6 +246,9 @@ describe('App', () => {
}),
graphql.query('GetUploadTokenRequired', (info) => {
return HttpResponse.json({ data: { owner: null } })
}),
graphql.query('NavigatorData', () => {
return HttpResponse.json({ data: mockNavigatorData })
})
)
}
Expand Down
58 changes: 40 additions & 18 deletions src/layouts/BaseLayout/BaseLayout.test.tsx
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 { graphql, http, HttpResponse } from 'msw'
import { setupServer } from 'msw/node'
Expand Down Expand Up @@ -156,34 +160,48 @@ const internalUserHasSyncedProviders = {
termsAgreement: true,
}

const mockNavigatorData = {
owner: {
isCurrentUserPartOfOrg: true,
repository: {
__typename: 'Repository',
name: 'test-repo',
},
},
}

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

let testLocation: ReturnType<typeof useLocation>
const wrapper: (
initialEntries?: string[]
) => React.FC<React.PropsWithChildren> =
(initialEntries = ['/bb/batman/batcave']) =>
({ children }) => (
<QueryClientProvider client={queryClient}>
<MemoryRouter initialEntries={initialEntries}>
<Route path="/:provider/:owner/:repo">{children}</Route>
<Route
path="*"
render={({ location }) => {
testLocation = location
return null
}}
/>
</MemoryRouter>
</QueryClientProvider>
<QueryClientProviderV5 client={queryClientV5}>
<QueryClientProvider client={queryClient}>
<MemoryRouter initialEntries={initialEntries}>
<Route path="/:provider/:owner/:repo">{children}</Route>
<Route
path="*"
render={({ location }) => {
testLocation = location
return null
}}
/>
</MemoryRouter>
</QueryClientProvider>
</QueryClientProviderV5>
)

beforeAll(() => {
Expand All @@ -192,6 +210,7 @@ beforeAll(() => {

afterEach(() => {
queryClient.clear()
queryClientV5.clear()
server.resetHandlers()
vi.clearAllMocks()
})
Expand Down Expand Up @@ -236,7 +255,7 @@ describe('BaseLayout', () => {
}),
// Self hosted only
graphql.query('HasAdmins', (info) => {
return HttpResponse.json({ data: {} })
return HttpResponse.json({ data: { config: null } })
}),
graphql.query('Seats', (info) => {
return HttpResponse.json({ data: {} })
Expand All @@ -261,6 +280,9 @@ describe('BaseLayout', () => {
graphql.mutation('updateDefaultOrganization', (info) => {
return HttpResponse.json({ data: {} })
}),
graphql.query('NavigatorData', () => {
return HttpResponse.json({ data: mockNavigatorData })
}),
http.get('/internal/users/current', (info) => {
return HttpResponse.json({})
})
Expand Down
37 changes: 31 additions & 6 deletions src/layouts/BaseLayout/BaseLayout.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { useQuery as useQueryV5 } from '@tanstack/react-queryV5'
import { lazy, Suspense } from 'react'
import { Redirect } from 'react-router-dom'
import { Redirect, useParams } from 'react-router-dom'

import Footer from 'layouts/Footer'
import Header from 'layouts/Header'
Expand All @@ -14,6 +15,7 @@ import GlobalBanners from 'shared/GlobalBanners'
import GlobalTopBanners from 'shared/GlobalTopBanners'
import LoadingLogo from 'ui/LoadingLogo'

import { NavigatorDataQueryOpts } from './hooks/NavigatorDataQueryOpts'
import { useUserAccessGate } from './hooks/useUserAccessGate'

const DefaultOrgSelector = lazy(() => import('pages/DefaultOrgSelector'))
Expand Down Expand Up @@ -65,19 +67,42 @@ function OnboardingOrChildren({
return <>{children}</>
}

interface URLParams {
provider?: string
owner?: string
repo?: string
}

function BaseLayout({ children }: React.PropsWithChildren) {
const { provider, owner, repo } = useParams<URLParams>()
useTracking()
const { isImpersonating } = useImpersonate()
const {
isFullExperience,
showAgreeToTerms,
showDefaultOrgSelector,
redirectToSyncPage,
isLoading,
isLoading: isUserAccessGateLoading,
} = useUserAccessGate()
useTracking()
const { isImpersonating } = useImpersonate()

// we have to fetch the data for the navigator up here as we can't
// conditionally call a suspense query, as well we need a way to have the
// loader be shown while we're loading
const { data, isLoading: isNavigatorDataLoading } = useQueryV5({
enabled: !!provider && !!owner && !!repo,
...NavigatorDataQueryOpts({
// if these aren't provided, the query is disabled so we don't need to
// worry about the empty strings causing errors
provider: provider ?? '',
owner: owner ?? '',
repo: repo ?? '',
}),
})

// Pause rendering of a page till we know if the user is logged in or not
if (isLoading) return <FullPageLoader />
if (isUserAccessGateLoading || isNavigatorDataLoading) {
return <FullPageLoader />
}

return (
<>
Expand All @@ -89,7 +114,7 @@ function BaseLayout({ children }: React.PropsWithChildren) {
{isFullExperience || isImpersonating ? (
<>
<GlobalTopBanners />
<Header />
<Header hasRepoAccess={data?.hasRepoAccess} />
</>
) : (
<>
Expand Down
Loading
Loading