diff --git a/src/App.test.tsx b/src/App.test.tsx index ece1683220..b60190e9a6 100644 --- a/src/App.test.tsx +++ b/src/App.test.tsx @@ -13,7 +13,6 @@ import { type Mock, vi } from 'vitest' import config from 'config' import { useLocationParams } from 'services/navigation/useLocationParams' -import { Plans } from 'shared/utils/billing' import App from './App' @@ -60,57 +59,15 @@ const internalUser = { integrationId: null, name: null, ownerid: 123, - service: 'cool-service', + service: 'gh', stats: null, username: 'cool-guy', }, ], + defaultOrg: 'codecov', termsAgreement: true, } -const user = { - me: { - owner: { - defaultOrgUsername: 'codecov', - }, - email: 'jane.doe@codecov.io', - privateAccess: true, - onboardingCompleted: true, - businessEmail: 'jane.doe@codecov.io', - termsAgreement: true, - user: { - name: 'Jane Doe', - username: 'janedoe', - avatarUrl: 'http://127.0.0.1/avatar-url', - avatar: 'http://127.0.0.1/avatar-url', - student: false, - studentCreatedAt: null, - studentUpdatedAt: null, - }, - trackingMetadata: { - service: 'github', - ownerid: 123, - serviceId: '123', - plan: Plans.USERS_DEVELOPER, - staff: false, - hasYaml: false, - bot: null, - delinquent: null, - didTrial: null, - planProvider: null, - planUserCount: 1, - createdAt: 'timestamp', - updatedAt: 'timestamp', - profile: { - createdAt: 'timestamp', - otherGoal: null, - typeProjects: [], - goals: [], - }, - }, - }, -} - const mockRepoOverview = { owner: { repository: { @@ -198,7 +155,6 @@ afterAll(() => { describe('App', () => { function setup({ - hasLoggedInUser, hasSession, }: { hasLoggedInUser?: boolean @@ -218,12 +174,6 @@ describe('App', () => { graphql.query('DetailOwner', () => HttpResponse.json({ data: { owner: 'codecov' } }) ), - graphql.query('CurrentUser', () => { - if (hasLoggedInUser) { - return HttpResponse.json({ data: user }) - } - HttpResponse.json({ data: {} }) - }), graphql.query('GetPlanData', () => { return HttpResponse.json({ data: {} }) }), @@ -436,7 +386,7 @@ describe('App', () => { 'cloud routing', ({ testLabel, pathname, expected }) => { beforeEach(() => { - setup({ hasLoggedInUser: true, hasSession: true }) + setup({ hasSession: true }) }) it(`renders the ${testLabel} page`, async () => { @@ -620,7 +570,7 @@ describe('App', () => { ({ testLabel, pathname, expected }) => { beforeEach(() => { config.IS_SELF_HOSTED = true - setup({ hasLoggedInUser: true, hasSession: true }) + setup({ hasSession: true }) }) it(`renders the ${testLabel} page`, async () => { @@ -637,7 +587,7 @@ describe('App', () => { describe('user not logged in', () => { it('redirects to login', async () => { - setup({ hasLoggedInUser: true, hasSession: false }) + setup({ hasSession: false }) render(, { wrapper: wrapper(['*']) }) await waitFor(() => expect(testLocation.pathname).toBe('/login')) }) @@ -645,24 +595,22 @@ describe('App', () => { describe('user has session, not logged in', () => { it('redirects to session default', async () => { - setup({ hasLoggedInUser: false, hasSession: true }) + setup({ hasSession: true }) render(, { wrapper: wrapper(['/blah']) }) - await waitFor(() => - expect(testLocation.pathname).toBe('/cool-service/cool-guy') - ) + await waitFor(() => expect(testLocation.pathname).toBe('/gh/codecov')) }) it('redirects to plan page if to param === plan', async () => { mockedUseLocationParams.mockReturnValue({ params: { to: 'plan' }, }) - setup({ hasLoggedInUser: false, hasSession: true }) + setup({ hasSession: true }) render(, { wrapper: wrapper(['/blah']) }) await waitFor(() => - expect(testLocation.pathname).toBe('/plan/cool-service/cool-guy') + expect(testLocation.pathname).toBe('/plan/gh/codecov') ) }) }) @@ -673,7 +621,7 @@ describe('App', () => { mockedUseLocationParams.mockReturnValue({ params: { setup_action: 'install' }, }) - setup({ hasLoggedInUser: true, hasSession: true }) + setup({ hasSession: true }) render(, { wrapper: wrapper(['/gh?setup_action=install']) }) await waitFor(() => expect(testLocation.pathname).toBe('/gh/codecov')) @@ -687,7 +635,7 @@ describe('App', () => { mockedUseLocationParams.mockReturnValue({ params: { to: '/gh/codecov/test-app/pull/123' }, }) - setup({ hasLoggedInUser: true, hasSession: true }) + setup({ hasSession: true }) render(, { wrapper: wrapper(['/gh']) }) @@ -706,7 +654,7 @@ describe('App', () => { }) // after redirecting the param should be removed .mockReturnValue({ params: {} }) - setup({ hasLoggedInUser: true, hasSession: true }) + setup({ hasSession: true }) render(, { wrapper: wrapper(['/gh?to=/gh/path/does/not/exist']), diff --git a/src/App.tsx b/src/App.tsx index a984df73fc..8a8a19b511 100644 --- a/src/App.tsx +++ b/src/App.tsx @@ -2,7 +2,7 @@ import { ReactQueryDevtools } from '@tanstack/react-query-devtools' import qs from 'qs' import { lazy } from 'react' import { Toaster } from 'react-hot-toast' -import { Redirect, Switch, useParams } from 'react-router-dom' +import { Redirect, Switch } from 'react-router-dom' import config from 'config' @@ -13,8 +13,7 @@ import EnterpriseLoginLayout from 'layouts/EnterpriseLoginLayout' import LoginLayout from 'layouts/LoginLayout' import { useLocationParams } from 'services/navigation/useLocationParams' import { ToastNotificationProvider } from 'services/toastNotification/context' -import { useInternalUser, useUser } from 'services/user' -import { isProvider } from 'shared/api/helpers' +import { useInternalUser } from 'services/user' import 'ui/Table/Table.css' import 'ui/FileList/FileList.css' import { ThemeContextProvider } from 'shared/ThemeContext' @@ -33,45 +32,29 @@ const PullRequestPage = lazy(() => import('./pages/PullRequestPage')) const RepoPage = lazy(() => import('./pages/RepoPage')) const SyncProviderPage = lazy(() => import('./pages/SyncProviderPage')) -interface URLParams { - provider: string -} - const HomePageRedirect = () => { - const { provider } = useParams() - const { data: currentUser } = useUser() const { data: internalUser } = useInternalUser({}) const { params } = useLocationParams() // @ts-expect-error useLocationParams needs to be typed const { setup_action: setupAction, to } = params + // create a query params object to be added to the redirect URL + const queryParams: Record = {} let redirectURL = '/login' if (internalUser && internalUser.owners) { const service = internalUser.owners[0]?.service - const username = internalUser.owners[0]?.username - redirectURL = `/${service}/${username}` - if (to === 'plan') { - redirectURL = '/plan' + redirectURL - } - } - - // create a query params object to be added to the redirect URL - const queryParams: Record = {} - - // only redirect if we have a provider and it's a valid provider and the user is logged in - if (provider && isProvider(provider) && currentUser) { - // set the redirect URL to the owner's default org or the user's username - const defaultOrg = - currentUser.owner?.defaultOrgUsername ?? currentUser.user?.username - redirectURL = `/${provider}/${defaultOrg}` + const defaultOrg = internalUser.defaultOrg + redirectURL = `/${service}/${defaultOrg ? defaultOrg : ''}` if (setupAction) { // eslint-disable-next-line camelcase queryParams.setup_action = setupAction } - // ensure that we only redirect if the user is not setting up the action and we don't want to redirect if we're already redirecting to the plan page - else if (to && to !== 'plan') { + + if (to === 'plan') { + redirectURL = '/plan' + redirectURL + } else if (to) { redirectURL = to } } diff --git a/src/layouts/BaseLayout/BaseLayout.test.tsx b/src/layouts/BaseLayout/BaseLayout.test.tsx index dee2e7a1d4..eda9f6d66a 100644 --- a/src/layouts/BaseLayout/BaseLayout.test.tsx +++ b/src/layouts/BaseLayout/BaseLayout.test.tsx @@ -60,6 +60,7 @@ const mockUser = { }, ], termsAgreement: true, + defaultOrg: 'cool-username', } const mockUserNoTermsAgreement = { @@ -133,6 +134,7 @@ const internalUserNoSyncedProviders = { externalId: '123', termsAgreement: true, owners: [], + defaultOrg: null, } const internalUserHasSyncedProviders = { @@ -150,6 +152,7 @@ const internalUserHasSyncedProviders = { service: 'github', }, ], + defaultOrg: 'cool-username', termsAgreement: true, } diff --git a/src/layouts/BaseLayout/hooks/useUserAccessGate.test.tsx b/src/layouts/BaseLayout/hooks/useUserAccessGate.test.tsx index 2c40fd0a9a..defa503a68 100644 --- a/src/layouts/BaseLayout/hooks/useUserAccessGate.test.tsx +++ b/src/layouts/BaseLayout/hooks/useUserAccessGate.test.tsx @@ -155,6 +155,7 @@ const internalUserNoSyncedProviders = { externalId: '123', termsAgreement: null, owners: [], + defaultOrg: null, } const internalUserHasSyncedProviders = { @@ -173,6 +174,7 @@ const internalUserHasSyncedProviders = { stats: null, }, ], + defaultOrg: 'cool-owner', } const internalUserWithSignedTOS = { @@ -190,6 +192,7 @@ const internalUserWithSignedTOS = { stats: null, }, ], + defaultOrg: 'cool-owner', termsAgreement: true, } @@ -208,6 +211,7 @@ const internalUserWithUnsignedTOS = { stats: null, }, ], + defaultOrg: 'cool-owner', termsAgreement: false, } diff --git a/src/pages/SyncProviderPage/SyncProviderPage.test.tsx b/src/pages/SyncProviderPage/SyncProviderPage.test.tsx index 17468e8da4..5f5ef75bd8 100644 --- a/src/pages/SyncProviderPage/SyncProviderPage.test.tsx +++ b/src/pages/SyncProviderPage/SyncProviderPage.test.tsx @@ -23,7 +23,8 @@ const createMockedInternalUser = ( name: null, externalId: null, termsAgreement: false, - owners: owner !== undefined ? [owner] : [], + owners: owner != null ? [owner] : [], + defaultOrg: owner != null ? owner.username : null, }) const server = setupServer() @@ -202,7 +203,17 @@ describe('SyncProviderPage', () => { describe('user does not have a service', () => { it('redirects user to /', async () => { - setup({ user: createMockedInternalUser(null) }) + setup({ + user: createMockedInternalUser({ + name: 'noservice', + service: '', + username: 'noservice', + avatarUrl: 'http://127.0.0.1/cool-user-avatar', + integrationId: null, + ownerid: 123, + stats: null, + }), + }) render(, { wrapper }) diff --git a/src/pages/TermsOfService/TermsOfService.test.tsx b/src/pages/TermsOfService/TermsOfService.test.tsx index c4406a96d9..b18ad0af5f 100644 --- a/src/pages/TermsOfService/TermsOfService.test.tsx +++ b/src/pages/TermsOfService/TermsOfService.test.tsx @@ -68,6 +68,7 @@ const mockedUserData = { externalId: null, owners: null, termsAgreement: false, + defaultOrg: null, } const mocks = vi.hoisted(() => ({ @@ -163,6 +164,7 @@ describe('TermsOfService', () => { externalId: '', owners: null, termsAgreement: false, + defaultOrg: null, }, }) ) @@ -238,6 +240,7 @@ describe('TermsOfService', () => { externalId: '1234', owners: null, termsAgreement: false, + defaultOrg: null, }, }) @@ -312,6 +315,7 @@ describe('TermsOfService', () => { externalId: '1234', owners: null, name: 'Chetney', + defaultOrg: null, }, }, [expectPageIsReady], @@ -331,6 +335,7 @@ describe('TermsOfService', () => { name: 'Chetney', externalId: '1234', owners: null, + defaultOrg: null, }, }, [expectPageIsReady], @@ -352,6 +357,7 @@ describe('TermsOfService', () => { name: 'Chetney', externalId: '1234', owners: null, + defaultOrg: null, }, }, [expectPageIsReady], @@ -374,6 +380,7 @@ describe('TermsOfService', () => { externalId: '1234', owners: null, email: '', + defaultOrg: null, }, }, [expectPageIsReady], @@ -401,6 +408,7 @@ describe('TermsOfService', () => { name: 'Chetney', externalId: '1234', owners: null, + defaultOrg: null, }, }, [expectPageIsReady], @@ -419,6 +427,7 @@ describe('TermsOfService', () => { name: '', externalId: '1234', owners: null, + defaultOrg: null, }, }, [expectPageIsReady], @@ -451,6 +460,7 @@ describe('TermsOfService', () => { name: '', externalId: '1234', owners: null, + defaultOrg: null, }, }, [expectPageIsReady], @@ -478,6 +488,7 @@ describe('TermsOfService', () => { name: '', externalId: '1234', owners: null, + defaultOrg: null, }, }, [expectPageIsReady], @@ -509,6 +520,7 @@ describe('TermsOfService', () => { username: 'chetney', }, ], + defaultOrg: 'chetney', }, }, [expectRedirectTo, '/gh/codecov/cool-repo'], @@ -569,6 +581,7 @@ describe('TermsOfService', () => { externalId: '', owners: null, termsAgreement: false, + defaultOrg: null, }, }) const removeFromDom = vi.fn() @@ -595,6 +608,7 @@ describe('TermsOfService', () => { externalId: '', owners: null, termsAgreement: false, + defaultOrg: null, }, }) config.SENTRY_DSN = 'dsn' @@ -621,6 +635,7 @@ describe('TermsOfService', () => { externalId: '', owners: null, termsAgreement: false, + defaultOrg: null, }, }) config.SENTRY_DSN = 'dsn' diff --git a/src/services/user/useInternalUser.test.tsx b/src/services/user/useInternalUser.test.tsx index 0709106593..231ec8be31 100644 --- a/src/services/user/useInternalUser.test.tsx +++ b/src/services/user/useInternalUser.test.tsx @@ -47,6 +47,7 @@ describe('useInternalUser', () => { externalId: '1234', termsAgreement: false, owners: [], + defaultOrg: null, }) }) ) @@ -64,6 +65,7 @@ describe('useInternalUser', () => { externalId: '1234', owners: [], termsAgreement: false, + defaultOrg: null, }) ) }) diff --git a/src/services/user/useInternalUser.ts b/src/services/user/useInternalUser.ts index d7caf158b7..553d07ba5b 100644 --- a/src/services/user/useInternalUser.ts +++ b/src/services/user/useInternalUser.ts @@ -29,6 +29,7 @@ const InternalUserSchema = z externalId: z.string().nullable(), owners: z.array(OwnerSchema).nullable(), termsAgreement: z.boolean().nullable(), + defaultOrg: z.string().nullable(), }) .nullable()