Skip to content

feat: Use new default-org on internal user for default org decision #3905

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 2 commits into from
Jul 2, 2025
Merged
Show file tree
Hide file tree
Changes from all 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
76 changes: 12 additions & 64 deletions src/App.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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'

Expand Down Expand Up @@ -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: '[email protected]',
privateAccess: true,
onboardingCompleted: true,
businessEmail: '[email protected]',
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: {
Expand Down Expand Up @@ -198,7 +155,6 @@ afterAll(() => {

describe('App', () => {
function setup({
hasLoggedInUser,
hasSession,
}: {
hasLoggedInUser?: boolean
Expand All @@ -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: {} })
}),
Expand Down Expand Up @@ -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 () => {
Expand Down Expand Up @@ -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 () => {
Expand All @@ -637,32 +587,30 @@ describe('App', () => {

describe('user not logged in', () => {
it('redirects to login', async () => {
setup({ hasLoggedInUser: true, hasSession: false })
setup({ hasSession: false })
render(<App />, { wrapper: wrapper(['*']) })
await waitFor(() => expect(testLocation.pathname).toBe('/login'))
})
})

describe('user has session, not logged in', () => {
it('redirects to session default', async () => {
setup({ hasLoggedInUser: false, hasSession: true })
setup({ hasSession: true })

render(<App />, { 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(<App />, { wrapper: wrapper(['/blah']) })

await waitFor(() =>
expect(testLocation.pathname).toBe('/plan/cool-service/cool-guy')
expect(testLocation.pathname).toBe('/plan/gh/codecov')
)
})
})
Expand All @@ -673,7 +621,7 @@ describe('App', () => {
mockedUseLocationParams.mockReturnValue({
params: { setup_action: 'install' },
})
setup({ hasLoggedInUser: true, hasSession: true })
setup({ hasSession: true })
render(<App />, { wrapper: wrapper(['/gh?setup_action=install']) })

await waitFor(() => expect(testLocation.pathname).toBe('/gh/codecov'))
Expand All @@ -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(<App />, { wrapper: wrapper(['/gh']) })

Expand All @@ -706,7 +654,7 @@ describe('App', () => {
})
// after redirecting the param should be removed
.mockReturnValue({ params: {} })
setup({ hasLoggedInUser: true, hasSession: true })
setup({ hasSession: true })

render(<App />, {
wrapper: wrapper(['/gh?to=/gh/path/does/not/exist']),
Expand Down
37 changes: 10 additions & 27 deletions src/App.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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'

Expand All @@ -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'
Expand All @@ -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<URLParams>()
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<string, string> = {}

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<string, string> = {}

// 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
}
}
Expand Down
3 changes: 3 additions & 0 deletions src/layouts/BaseLayout/BaseLayout.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ const mockUser = {
},
],
termsAgreement: true,
defaultOrg: 'cool-username',
}

const mockUserNoTermsAgreement = {
Expand Down Expand Up @@ -133,6 +134,7 @@ const internalUserNoSyncedProviders = {
externalId: '123',
termsAgreement: true,
owners: [],
defaultOrg: null,
}

const internalUserHasSyncedProviders = {
Expand All @@ -150,6 +152,7 @@ const internalUserHasSyncedProviders = {
service: 'github',
},
],
defaultOrg: 'cool-username',
termsAgreement: true,
}

Expand Down
4 changes: 4 additions & 0 deletions src/layouts/BaseLayout/hooks/useUserAccessGate.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,7 @@ const internalUserNoSyncedProviders = {
externalId: '123',
termsAgreement: null,
owners: [],
defaultOrg: null,
}

const internalUserHasSyncedProviders = {
Expand All @@ -173,6 +174,7 @@ const internalUserHasSyncedProviders = {
stats: null,
},
],
defaultOrg: 'cool-owner',
}

const internalUserWithSignedTOS = {
Expand All @@ -190,6 +192,7 @@ const internalUserWithSignedTOS = {
stats: null,
},
],
defaultOrg: 'cool-owner',
termsAgreement: true,
}

Expand All @@ -208,6 +211,7 @@ const internalUserWithUnsignedTOS = {
stats: null,
},
],
defaultOrg: 'cool-owner',
termsAgreement: false,
}

Expand Down
15 changes: 13 additions & 2 deletions src/pages/SyncProviderPage/SyncProviderPage.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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(<SyncProviderPage />, { wrapper })

Expand Down
Loading