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

feat: Hide BA trend chart if Timescale is disabled #3708

Merged
Merged
Show file tree
Hide file tree
Changes from 3 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
66 changes: 47 additions & 19 deletions src/pages/RepoPage/BundlesTab/BundleContent/BundleContent.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,8 @@ const mockRepoOverview = {
},
}

const mockBranchBundles = {
const mockBranchBundles = (isTimescaleEnabled: boolean) => ({
config: { isTimescaleEnabled },
owner: {
repository: {
__typename: 'Repository',
Expand Down Expand Up @@ -63,9 +64,10 @@ const mockBranchBundles = {
},
},
},
}
})

const mockBranchBundlesError = {
config: { isTimescaleEnabled: false },
owner: {
repository: {
__typename: 'Repository',
Expand All @@ -85,6 +87,7 @@ const mockBranchBundlesError = {
}

const mockEmptyBundleSelection = {
config: { isTimescaleEnabled: false },
owner: {
repository: {
__typename: 'Repository',
Expand Down Expand Up @@ -113,14 +116,8 @@ const mockAssets = {
routes: ['/'],
extension: 'js',
bundleData: {
loadTime: {
threeG: 2000,
highSpeed: 2000,
},
size: {
uncompress: 3000,
gzip: 4000,
},
loadTime: { threeG: 2000, highSpeed: 2000 },
size: { uncompress: 3000, gzip: 4000 },
},
measurements: {
change: { size: { uncompress: 5 } },
Expand Down Expand Up @@ -217,14 +214,8 @@ const mockBundleSummary = {
name: 'bundle1',
moduleCount: 10,
bundleData: {
loadTime: {
threeG: 1000,
highSpeed: 500,
},
size: {
gzip: 1000,
uncompress: 2000,
},
loadTime: { threeG: 1000, highSpeed: 500 },
size: { gzip: 1000, uncompress: 2000 },
},
},
},
Expand Down Expand Up @@ -282,12 +273,14 @@ afterAll(() => {
interface SetupArgs {
isBundleError?: boolean
isEmptyBundleSelection?: boolean
isTimescaleEnabled?: boolean
}

describe('BundleContent', () => {
function setup({
isBundleError = false,
isEmptyBundleSelection = false,
isTimescaleEnabled = true,
}: SetupArgs) {
server.use(
graphql.query('BranchBundleSummaryData', () => {
Expand All @@ -296,7 +289,9 @@ describe('BundleContent', () => {
} else if (isEmptyBundleSelection) {
return HttpResponse.json({ data: mockEmptyBundleSelection })
}
return HttpResponse.json({ data: mockBranchBundles })
return HttpResponse.json({
data: mockBranchBundles(isTimescaleEnabled),
})
}),
graphql.query('GetRepoOverview', () => {
return HttpResponse.json({ data: mockRepoOverview })
Expand Down Expand Up @@ -398,6 +393,39 @@ describe('BundleContent', () => {
expect(moduleCount).toBeInTheDocument()
})
})

describe('rendering the trend chart', () => {
describe('when timescale is enabled', () => {
it('renders the trend chart', async () => {
setup({ isTimescaleEnabled: true })
render(<BundleContent />, {
wrapper: wrapper(
'/gh/codecov/test-repo/bundles/main/test-bundle'
),
})

const chart = await screen.findByText('Hide chart')
expect(chart).toBeInTheDocument()
})
})

describe('when timescale is disabled', () => {
it('renders the trend chart', async () => {
setup({ isTimescaleEnabled: false })
render(<BundleContent />, {
wrapper: wrapper(
'/gh/codecov/test-repo/bundles/main/test-bundle'
),
})

const bundleName = await screen.findByText('asset-1')
expect(bundleName).toBeInTheDocument()

const chart = screen.queryByText('Hide chart')
expect(chart).not.toBeInTheDocument()
})
})
})
})

describe('when only the branch is set', () => {
Expand Down
53 changes: 39 additions & 14 deletions src/pages/RepoPage/BundlesTab/BundleContent/BundleContent.tsx
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
import { useSuspenseQuery as useSuspenseQueryV5 } from '@tanstack/react-queryV5'
import { lazy, Suspense } from 'react'
import { Switch, useParams } from 'react-router-dom'

import { SentryRoute } from 'sentry'

import { useBranchBundleSummary } from 'services/bundleAnalysis/useBranchBundleSummary'
import { BranchBundleSummaryQueryOpts } from 'services/bundleAnalysis/BranchBundleSummaryQueryOpts'
import { useRepoOverview } from 'services/repo'
import Spinner from 'ui/Spinner'
import { ToggleElement } from 'ui/ToggleElement'

Expand Down Expand Up @@ -32,9 +34,30 @@ const Loader = () => (
)

const BundleContent: React.FC = () => {
const { provider, owner, repo, branch, bundle } = useParams<URLParams>()
const {
provider,
owner,
repo,
branch: branchParam,
bundle,
} = useParams<URLParams>()

const { data } = useBranchBundleSummary({ provider, owner, repo, branch })
const { data: repoOverview } = useRepoOverview({
provider,
repo,
owner,
})

const branch = branchParam ?? repoOverview?.defaultBranch

const { data } = useSuspenseQueryV5(
BranchBundleSummaryQueryOpts({
provider,
owner,
repo,
branch,
})
)

const bundleType =
data?.branch?.head?.bundleAnalysis?.bundleAnalysisReport?.__typename
Expand All @@ -49,14 +72,16 @@ const BundleContent: React.FC = () => {
{bundleType === 'BundleAnalysisReport' ? (
<Switch>
<SentryRoute path="/:provider/:owner/:repo/bundles/:branch/:bundle">
<ToggleElement
showButtonContent="Show chart"
hideButtonContent="Hide chart"
localStorageKey="is-bundle-chart-hidden"
toggleRowElement={<TrendDropdown />}
>
<BundleChart />
</ToggleElement>
{data.config.isTimescaleEnabled ? (
<ToggleElement
showButtonContent="Show chart"
hideButtonContent="Hide chart"
localStorageKey="is-bundle-chart-hidden"
toggleRowElement={<TrendDropdown />}
>
<BundleChart />
</ToggleElement>
) : null}
<Suspense fallback={<Loader />}>
<AssetsTable />
</Suspense>
Expand All @@ -67,13 +92,13 @@ const BundleContent: React.FC = () => {
'/:provider/:owner/:repo/bundles/',
]}
>
<InfoBanner branch={branch} bundle={bundle} />
<InfoBanner branch={branchParam} bundle={bundle} />
Copy link
Contributor

@ajay-sentry ajay-sentry Feb 4, 2025

Choose a reason for hiding this comment

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

jw, why are these branchParam vs. branch?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was done to maintain the current implementation, but I've updated it to work across the board now.

<AssetEmptyTable />
</SentryRoute>
</Switch>
) : bundleType === undefined && !branch ? (
) : bundleType === undefined && !branchParam ? (
<>
<InfoBanner branch={branch} bundle={bundle} />
<InfoBanner branch={branchParam} bundle={bundle} />
<AssetEmptyTable />
</>
) : (
Expand Down
Loading