fix: Shift GH config banner location and hide tokenless banner for ne…

…wly onboarded

src/pages/OwnerPage/HeaderBanners/GithubConfigBanner/GithubConfigBanner.jsx

@@ -14,7 +14,7 @@ const GithubConfigBanner = () => {
   if (!isGh) return null
 
   return (
-    <div className="mb-2">
+    <div className="mb-2 mt-4">
       <Banner>
         <BannerHeading>
           <h2 className="flex justify-center gap-2 font-semibold">

src/pages/OwnerPage/HeaderBanners/HeaderBanners.jsx

@@ -4,10 +4,9 @@ import { useParams } from 'react-router-dom'
 import config from 'config'
 
 import { useOwnerPageData } from 'pages/OwnerPage/hooks'
-import { useAccountDetails, usePlanData } from 'services/account'
+import { usePlanData } from 'services/account'
 
 import ExceededUploadsAlert from './ExceededUploadsAlert'
-import GithubConfigBanner from './GithubConfigBanner'
 import ReachingUploadLimitAlert from './ReachingUploadLimitAlert'
 
 const useUploadsInfo = () => {
@@ -31,14 +30,9 @@ const useUploadsInfo = () => {
   return { isUploadLimitExceeded, isApproachingUploadLimit }
 }
 
-const AlertBanners = ({
-  isUploadLimitExceeded,
-  isApproachingUploadLimit,
-  hasGhApp,
-}) => {
+const AlertBanners = ({ isUploadLimitExceeded, isApproachingUploadLimit }) => {
   return (
     <>
-      {!hasGhApp && <GithubConfigBanner />}
       {isUploadLimitExceeded ? (
         <ExceededUploadsAlert />
       ) : isApproachingUploadLimit ? (
@@ -55,17 +49,8 @@ AlertBanners.propTypes = {
 }
 
 export default function HeaderBanners() {
-  const { owner, provider } = useParams()
-  // TODO: refactor this to add a gql field for the integration id used to determine if the org has a GH app
-  const { data: accountDetails } = useAccountDetails({
-    provider,
-    owner,
-  })
-
   const { isUploadLimitExceeded, isApproachingUploadLimit } = useUploadsInfo()
 
-  const hasGhApp = !!accountDetails?.integrationId
-
   if (config.IS_SELF_HOSTED) {
     return null
   }
@@ -75,7 +60,6 @@ export default function HeaderBanners() {
       <AlertBanners
         isUploadLimitExceeded={isUploadLimitExceeded}
         isApproachingUploadLimit={isApproachingUploadLimit}
-        hasGhApp={hasGhApp}
       />
     </>
   )

src/pages/OwnerPage/HeaderBanners/HeaderBanners.test.jsx

@@ -1,5 +1,5 @@
 import { QueryClient, QueryClientProvider } from '@tanstack/react-query'
-import { render, screen, waitFor } from '@testing-library/react'
+import { render, screen } from '@testing-library/react'
 import { graphql, http, HttpResponse } from 'msw'
 import { setupServer } from 'msw/node'
 import { MemoryRouter, Route } from 'react-router-dom'
@@ -225,29 +225,6 @@ describe('HeaderBanners', () => {
     })
   })
 
-  describe('user does not have gh app installed', () => {
-    beforeEach(() => {
-      setup({
-        integrationId: null,
-      })
-    })
-
-    it('displays github app config banner', async () => {
-      render(
-        <HeaderBanners
-          provider="gh"
-          owner={{ username: 'codecov', isCurrentUserPartOfOrg: true }}
-        />,
-        { wrapper }
-      )
-
-      await waitFor(() => {
-        const banner = screen.getByText("Codecov's GitHub app")
-        return expect(banner).toBeInTheDocument()
-      })
-    })
-  })
-
   describe('user is running in self hosted mode', () => {
     beforeEach(() => {
       setup({

src/pages/OwnerPage/OwnerPage.jsx

@@ -4,7 +4,7 @@ import { useHistory, useParams } from 'react-router-dom'
 import SilentNetworkErrorWrapper from 'layouts/shared/SilentNetworkErrorWrapper'
 import NotFound from 'pages/NotFound'
 import { useOwnerPageData } from 'pages/OwnerPage/hooks'
-import { useSentryToken } from 'services/account'
+import { useAccountDetails, useSentryToken } from 'services/account'
 import { useLocationParams } from 'services/navigation'
 import { renderToast } from 'services/toast'
 import { ActiveContext } from 'shared/context'
@@ -38,7 +38,7 @@ const useSentryTokenRedirect = ({ ownerData }) => {
 }
 
 function OwnerPage() {
-  const { provider } = useParams()
+  const { owner, provider } = useParams()
   const { data: ownerData } = useOwnerPageData()
   const { params } = useLocationParams({
     repoDisplay: 'All',
@@ -66,6 +66,14 @@ function OwnerPage() {
     }
   }, [userStartedTrial])
 
+  // TODO: refactor this to add a gql field for the integration id used to determine if the org has a GH app
+  const { data: accountDetails } = useAccountDetails({
+    provider,
+    owner,
+  })
+
+  const hasGhApp = !!accountDetails?.integrationId
+
   if (!ownerData) {
     return <NotFound />
   }
@@ -83,7 +91,10 @@ function OwnerPage() {
           <Tabs owner={ownerData} provider={provider} />
         )}
         <ActiveContext.Provider value={params?.repoDisplay}>
-          <ListRepo canRefetch={ownerData?.isCurrentUserPartOfOrg} />
+          <ListRepo
+            canRefetch={ownerData?.isCurrentUserPartOfOrg}
+            hasGhApp={hasGhApp}
+          />
         </ActiveContext.Provider>
       </div>
     </div>

src/pages/OwnerPage/OwnerPage.test.jsx

@@ -1,6 +1,6 @@
 import { QueryClient, QueryClientProvider } from '@tanstack/react-query'
 import { render, screen, waitFor } from '@testing-library/react'
-import { graphql, HttpResponse } from 'msw'
+import { graphql, http, HttpResponse } from 'msw'
 import { setupServer } from 'msw/node'
 import { MemoryRouter, Route } from 'react-router-dom'
 
@@ -59,9 +59,10 @@ afterAll(() => {
 
 describe('OwnerPage', () => {
   function setup(
-    { owner, successfulMutation = true } = {
+    { owner, successfulMutation = true, integrationId = 9 } = {
       owner: null,
       successfulMutation: true,
+      integrationId: 9,
     }
   ) {
     server.use(
@@ -85,6 +86,9 @@ describe('OwnerPage', () => {
         return HttpResponse.json({
           data: { saveSentryState: null },
         })
+      }),
+      http.get('/internal/gh/codecov/account-details/', () => {
+        return HttpResponse.json({ integrationId })
       })
     )
   }
@@ -242,4 +246,18 @@ describe('OwnerPage', () => {
       await waitFor(() => expect(mockRemoveItem).toHaveBeenCalled())
     })
   })
+
+  describe('when user does not have gh app installed', () => {
+    beforeEach(() => {
+      setup({
+        integrationId: null,
+      })
+    })
+
+    it('displays github app config banner', async () => {
+      render(<OwnerPage />, { wrapper })
+      const banner = await screen.findByText("Codecov's GitHub app")
+      expect(banner).toBeInTheDocument()
+    })
+  })
 })

src/shared/GlobalTopBanners/TokenlessBanner/TokenlessBanner.test.tsx

@@ -80,21 +80,23 @@ describe('TokenlessBanner', () => {
 
   it('renders nothing when tokenlessSection flag is false', () => {
     setup({ tokenlessSection: false })
-    const { container } = render(<TokenlessBanner />, { wrapper: wrapper() })
+    const { container } = render(<TokenlessBanner />, {
+      wrapper: wrapper(['/gh/codecov']),
+    })
     expect(container).toBeEmptyDOMElement()
   })
 
   it('renders nothing when owner is not provided', () => {
     setup()
     const { container } = render(<TokenlessBanner />, {
-      wrapper: wrapper(['/gh/']),
+      wrapper: wrapper(['/gh/codecov']),
     })
     expect(container).toBeEmptyDOMElement()
   })
 
   it('renders TokenRequiredBanner when uploadTokenRequired is true', async () => {
     setup({ uploadTokenRequired: true })
-    render(<TokenlessBanner />, { wrapper: wrapper() })
+    render(<TokenlessBanner />, { wrapper: wrapper(['/gh/codecov']) })
 
     await waitFor(() => {
       const banner = screen.getByText('TokenRequiredBanner')
@@ -104,11 +106,26 @@ describe('TokenlessBanner', () => {
 
   it('renders TokenNotRequiredBanner when uploadTokenRequired is false', async () => {
     setup({ uploadTokenRequired: false })
-    render(<TokenlessBanner />, { wrapper: wrapper() })
+    render(<TokenlessBanner />, { wrapper: wrapper(['/gh/codecov']) })
 
     await waitFor(() => {
       const banner = screen.getByText('TokenNotRequiredBanner')
       expect(banner).toBeInTheDocument()
     })
   })
+
+  it('renders nothing if coming from onboarding', async () => {
+    setup({ uploadTokenRequired: true })
+    render(<TokenlessBanner />, {
+      wrapper: wrapper(['/gh/codecov?source=onboarding']),
+    })
+    await waitFor(() => {
+      expect(screen.queryByText('TokenRequiredBanner')).not.toBeInTheDocument()
+    })
+    await waitFor(() => {
+      expect(
+        screen.queryByText('TokenNotRequiredBanner')
+      ).not.toBeInTheDocument()
+    })
+  })
 })

src/shared/GlobalTopBanners/TokenlessBanner/TokenlessBanner.tsx

@@ -1,6 +1,8 @@
 import React, { lazy } from 'react'
 import { useParams } from 'react-router-dom'
 
+import { ONBOARDING_SOURCE } from 'pages/TermsOfService/constants'
+import { useLocationParams } from 'services/navigation'
 import { useUploadTokenRequired } from 'services/uploadTokenRequired'
 import { useFlags } from 'shared/featureFlags'
 
@@ -18,8 +20,11 @@ const TokenlessBanner: React.FC = () => {
   })
   const { provider, owner } = useParams<UseParams>()
   const { data } = useUploadTokenRequired({ provider, owner, enabled: !!owner })
+  const { params } = useLocationParams()
+  // @ts-expect-error useLocationParams needs to be typed
+  const cameFromOnboarding = params['source'] === ONBOARDING_SOURCE
 
-  if (!tokenlessSection || !owner || !data) return null
+  if (!tokenlessSection || !owner || !data || cameFromOnboarding) return null
 
   return data?.uploadTokenRequired ? (
     <TokenRequiredBanner />

src/shared/ListRepo/ListRepo.jsx

@@ -3,6 +3,7 @@ import PropTypes from 'prop-types'
 import { Suspense, useContext } from 'react'
 import { useParams } from 'react-router-dom'
 
+import GithubConfigBanner from 'pages/OwnerPage/HeaderBanners/GithubConfigBanner'
 import { ONBOARDING_SOURCE } from 'pages/TermsOfService/constants'
 import { useLocationParams } from 'services/navigation'
 import { orderingOptions } from 'services/repos/orderingOptions'
@@ -29,7 +30,7 @@ export const repoDisplayOptions = Object.freeze({
   ALL: { text: 'All', status: undefined },
 })
 
-function ListRepo({ canRefetch }) {
+function ListRepo({ canRefetch, hasGhApp }) {
   const { provider, owner } = useParams()
   const { params, updateParams } = useLocationParams(defaultQueryParams)
   const { data: isTeamPlan } = useIsTeamPlan({ provider, owner })
@@ -59,6 +60,7 @@ function ListRepo({ canRefetch }) {
 
   return (
     <>
+      {!hasGhApp && <GithubConfigBanner />}
       <OrgControlTable
         searchValue={params.search}
         repoDisplay={repoDisplay}
@@ -103,6 +105,7 @@ function ListRepo({ canRefetch }) {
 
 ListRepo.propTypes = {
   canRefetch: PropTypes.bool.isRequired,
+  hasGhApp: PropTypes.bool,
 }
 
 export default ListRepo