From 3f9d8b7e43e1fccdd4afc67f5744a5f3209a87f8 Mon Sep 17 00:00:00 2001 From: nicholas-codecov Date: Thu, 19 Oct 2023 11:36:06 -0300 Subject: [PATCH 1/4] remove flag from Title and move higher up so it won't have any conflicts --- src/ui/FileViewer/ToggleHeader/Title/Title.jsx | 11 ++--------- src/ui/FileViewer/ToggleHeader/Title/Title.spec.jsx | 7 ------- 2 files changed, 2 insertions(+), 16 deletions(-) diff --git a/src/ui/FileViewer/ToggleHeader/Title/Title.jsx b/src/ui/FileViewer/ToggleHeader/Title/Title.jsx index 918e3b159e..9326c87c55 100644 --- a/src/ui/FileViewer/ToggleHeader/Title/Title.jsx +++ b/src/ui/FileViewer/ToggleHeader/Title/Title.jsx @@ -6,7 +6,6 @@ import { useState } from 'react' import { useLocationParams } from 'services/navigation' import { useRepoBackfilled, useRepoFlagsSelect } from 'services/repo' -import { useFlags } from 'shared/featureFlags' import Icon from 'ui/Icon' import MultiSelect from 'ui/MultiSelect' @@ -56,10 +55,6 @@ export const TitleFlags = () => { const flagsMeasurementsActive = !!repoBackfilledData?.flagsMeasurementsActive const noFlagsPresent = eq(repoBackfilledData?.flagsCount, 0) - const { coverageTabFlagMutliSelect } = useFlags({ - coverageTabFlagMutliSelect: false, - }) - const { data: flagsData, isLoading: flagsIsLoading, @@ -69,9 +64,7 @@ export const TitleFlags = () => { filters: { term: flagSearch }, options: { suspense: false, - enabled: - !!coverageTabFlagMutliSelect || - (flagsMeasurementsActive && !noFlagsPresent && isTimescaleEnabled), + enabled: flagsMeasurementsActive && !noFlagsPresent && isTimescaleEnabled, }, }) @@ -84,7 +77,7 @@ export const TitleFlags = () => { } } - if (!coverageTabFlagMutliSelect || noFlagsPresent) { + if (noFlagsPresent) { return null } diff --git a/src/ui/FileViewer/ToggleHeader/Title/Title.spec.jsx b/src/ui/FileViewer/ToggleHeader/Title/Title.spec.jsx index a775015cc3..60cd160dab 100644 --- a/src/ui/FileViewer/ToggleHeader/Title/Title.spec.jsx +++ b/src/ui/FileViewer/ToggleHeader/Title/Title.spec.jsx @@ -6,8 +6,6 @@ import { setupServer } from 'msw/node' import { MemoryRouter, Route } from 'react-router-dom' import useIntersection from 'react-use/lib/useIntersection' -import { useFlags } from 'shared/featureFlags' - import Title, { TitleFlags, TitleHitCount } from './Title' jest.mock('shared/featureFlags') @@ -176,7 +174,6 @@ describe('Title', () => { describe('TitleFlags', () => { function setup( { - flagValue = true, isIntersecting = false, noNextPage = false, backfillData = mockBackfillHasFlagsAndActive, @@ -190,10 +187,6 @@ describe('TitleFlags', () => { const user = userEvent.setup() const mockApiVars = jest.fn() - useFlags.mockReturnValue({ - coverageTabFlagMutliSelect: flagValue, - }) - useIntersection.mockReturnValue({ isIntersecting: isIntersecting }) server.use( From 085d8ace4df201b8b173346df36760312ec12d53 Mon Sep 17 00:00:00 2001 From: nicholas-codecov Date: Thu, 19 Oct 2023 11:43:36 -0300 Subject: [PATCH 2/4] add msw mock for tier name in CoverageTab.spec --- src/pages/RepoPage/CoverageTab/CoverageTab.spec.jsx | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/pages/RepoPage/CoverageTab/CoverageTab.spec.jsx b/src/pages/RepoPage/CoverageTab/CoverageTab.spec.jsx index cc075981e6..fe79d42ead 100644 --- a/src/pages/RepoPage/CoverageTab/CoverageTab.spec.jsx +++ b/src/pages/RepoPage/CoverageTab/CoverageTab.spec.jsx @@ -4,6 +4,8 @@ import { graphql, rest } from 'msw' import { setupServer } from 'msw/node' import { MemoryRouter, Route } from 'react-router-dom' +import { TierNames } from 'services/tier' + import CoverageTab from './CoverageTab' const queryClient = new QueryClient({ @@ -207,6 +209,12 @@ describe('Coverage Tab', () => { graphql.query('BackfillFlagMemberships', (req, res, ctx) => res(ctx.status(200), ctx.data({})) ), + graphql.query('OwnerTier', (req, res, ctx) => { + return res( + ctx.status(200), + ctx.data({ owner: { plan: { tierName: TierNames.PRO } } }) + ) + }), rest.get( '/internal/:provider/:owner/:repo/coverage/tree', (req, res, ctx) => { From c7e957fb8fe3293b6e05242f6feeefd4627da283 Mon Sep 17 00:00:00 2001 From: nicholas-codecov Date: Thu, 19 Oct 2023 11:43:57 -0300 Subject: [PATCH 3/4] update FlagMultiSelect to return null if the user/owner is on a team plan --- .../subroute/FileExplorer/FlagMultiSelect.jsx | 10 ++++++- .../FileExplorer/FlagMultiSelect.spec.jsx | 28 +++++++++++++++++++ 2 files changed, 37 insertions(+), 1 deletion(-) diff --git a/src/pages/RepoPage/CoverageTab/subroute/FileExplorer/FlagMultiSelect.jsx b/src/pages/RepoPage/CoverageTab/subroute/FileExplorer/FlagMultiSelect.jsx index 1586ec62e3..d1c8d424b3 100644 --- a/src/pages/RepoPage/CoverageTab/subroute/FileExplorer/FlagMultiSelect.jsx +++ b/src/pages/RepoPage/CoverageTab/subroute/FileExplorer/FlagMultiSelect.jsx @@ -2,9 +2,11 @@ import eq from 'lodash/eq' import isUndefined from 'lodash/isUndefined' import PropTypes from 'prop-types' import { useState } from 'react' +import { useParams } from 'react-router-dom' import { useLocationParams } from 'services/navigation' import { useRepoBackfilled, useRepoFlagsSelect } from 'services/repo' +import { TierNames, useTier } from 'services/tier' import { useFlags } from 'shared/featureFlags' import Icon from 'ui/Icon' import MultiSelect from 'ui/MultiSelect' @@ -16,10 +18,12 @@ const defaultQueryParams = { } function FlagMultiSelect() { + const { provider, owner } = useParams() const { params, updateParams } = useLocationParams(defaultQueryParams) const [selectedFlags, setSelectedFlags] = useState(params?.flags) const [flagSearch, setFlagSearch] = useState(null) + const { data: tierName } = useTier({ provider, owner }) const { data: repoBackfilledData } = useRepoBackfilled() const isTimescaleEnabled = !!repoBackfilledData?.isTimescaleEnabled @@ -45,7 +49,11 @@ function FlagMultiSelect() { }, }) - if (!coverageTabFlagMutliSelect || noFlagsPresent) { + if ( + !coverageTabFlagMutliSelect || + noFlagsPresent || + tierName === TierNames.TEAM + ) { return null } diff --git a/src/pages/RepoPage/CoverageTab/subroute/FileExplorer/FlagMultiSelect.spec.jsx b/src/pages/RepoPage/CoverageTab/subroute/FileExplorer/FlagMultiSelect.spec.jsx index 6fd016ce84..fa101f04da 100644 --- a/src/pages/RepoPage/CoverageTab/subroute/FileExplorer/FlagMultiSelect.spec.jsx +++ b/src/pages/RepoPage/CoverageTab/subroute/FileExplorer/FlagMultiSelect.spec.jsx @@ -6,6 +6,7 @@ import { setupServer } from 'msw/node' import { MemoryRouter, Route } from 'react-router-dom' import useIntersection from 'react-use/lib/useIntersection' +import { TierNames } from 'services/tier' import { useFlags } from 'shared/featureFlags' import FlagMultiSelect from './FlagMultiSelect' @@ -144,11 +145,13 @@ describe('FlagMultiSelect', () => { isIntersecting = false, noNextPage = false, backfillData = mockBackfillHasFlagsAndActive, + tierValue = TierNames.PRO, } = { flagValue: false, isIntersecting: false, noNextPage: false, mockBackfillHasFlagsAndActive: mockBackfillHasFlagsAndActive, + tierValue: TierNames.PRO, } ) { const user = userEvent.setup() @@ -172,6 +175,18 @@ describe('FlagMultiSelect', () => { }), graphql.query('BackfillFlagMemberships', (req, res, ctx) => { return res(ctx.status(200), ctx.data(backfillData)) + }), + graphql.query('OwnerTier', (req, res, ctx) => { + if (tierValue === TierNames.TEAM) { + return res( + ctx.status(200), + ctx.data({ owner: { plan: { tierName: TierNames.TEAM } } }) + ) + } + return res( + ctx.status(200), + ctx.data({ owner: { plan: { tierName: TierNames.PRO } } }) + ) }) ) @@ -294,6 +309,19 @@ describe('FlagMultiSelect', () => { }) }) + describe('when on team plan', () => { + it('does not show multi select', async () => { + setup({ flagValue: true, tierValue: TierNames.TEAM }) + + const { container } = render(, { wrapper }) + + await waitFor(() => queryClient.isFetching) + await waitFor(() => !queryClient.isFetching) + + await waitFor(() => expect(container).toBeEmptyDOMElement()) + }) + }) + describe('when timescale is disabled', () => { it('renders disabled multi select', async () => { setup({ flagValue: true, backfillData: mockBackfillTimeScaleDisabled }) From 4ee99cd822efe9b79ea093de2dda7e8cda3be7db Mon Sep 17 00:00:00 2001 From: nicholas-codecov Date: Thu, 19 Oct 2023 11:44:53 -0300 Subject: [PATCH 4/4] move flag control up to fileviewer and also hide flag selector if on team plan --- .../subroute/Fileviewer/Fileviewer.jsx | 15 ++++- .../subroute/Fileviewer/Fileviewer.spec.jsx | 55 +++++++++++++++++-- 2 files changed, 63 insertions(+), 7 deletions(-) diff --git a/src/pages/RepoPage/CoverageTab/subroute/Fileviewer/Fileviewer.jsx b/src/pages/RepoPage/CoverageTab/subroute/Fileviewer/Fileviewer.jsx index d8a2e864d2..eaad3c0947 100644 --- a/src/pages/RepoPage/CoverageTab/subroute/Fileviewer/Fileviewer.jsx +++ b/src/pages/RepoPage/CoverageTab/subroute/Fileviewer/Fileviewer.jsx @@ -1,12 +1,23 @@ import { useParams } from 'react-router-dom' +import { TierNames, useTier } from 'services/tier' +import { useFlags } from 'shared/featureFlags' import RawFileviewer from 'shared/RawFileviewer' import { useTreePaths } from 'shared/treePaths' import Breadcrumb from 'ui/Breadcrumb' function FileView() { const { treePaths } = useTreePaths() - const { ref: commit } = useParams() + const { provider, owner, ref: commit } = useParams() + + const { coverageTabFlagMutliSelect } = useFlags({ + coverageTabFlagMutliSelect: false, + }) + + const { data: tierName } = useTier({ provider, owner }) + + const showFlagSelector = + coverageTabFlagMutliSelect && tierName !== TierNames.TEAM return ( ) } diff --git a/src/pages/RepoPage/CoverageTab/subroute/Fileviewer/Fileviewer.spec.jsx b/src/pages/RepoPage/CoverageTab/subroute/Fileviewer/Fileviewer.spec.jsx index 2cf91124ed..26bd46eca6 100644 --- a/src/pages/RepoPage/CoverageTab/subroute/Fileviewer/Fileviewer.spec.jsx +++ b/src/pages/RepoPage/CoverageTab/subroute/Fileviewer/Fileviewer.spec.jsx @@ -1,13 +1,16 @@ import { QueryClient, QueryClientProvider } from '@tanstack/react-query' -import { render, screen } from '@testing-library/react' +import { render, screen, waitFor } from '@testing-library/react' import { graphql } from 'msw' import { setupServer } from 'msw/node' import { MemoryRouter, Route } from 'react-router-dom' +import { TierNames } from 'services/tier' +import { useFlags } from 'shared/featureFlags' import { useScrollToLine } from 'ui/CodeRenderer/hooks/useScrollToLine' import FileView from './Fileviewer' +jest.mock('shared/featureFlags') jest.mock('ui/CodeRenderer/hooks/useScrollToLine') const mockOwner = { @@ -93,10 +96,10 @@ const mockBackfillResponse = { }, } +const server = setupServer() const queryClient = new QueryClient({ defaultOptions: { queries: { retry: false } }, }) -const server = setupServer() const wrapper = ( @@ -127,13 +130,17 @@ afterAll(() => { }) describe('FileView', () => { - function setup() { + function setup({ tierName = TierNames.PRO } = { tierName: TierNames.PRO }) { useScrollToLine.mockImplementation(() => ({ lineRef: () => {}, handleClick: jest.fn(), targeted: false, })) + useFlags.mockReturnValue({ + coverageTabFlagMutliSelect: true, + }) + server.use( graphql.query('DetailOwner', (req, res, ctx) => res(ctx.status(200), ctx.data({ owner: mockOwner })) @@ -149,15 +156,26 @@ describe('FileView', () => { }), graphql.query('FlagsSelect', (req, res, ctx) => { return res(ctx.status(200), ctx.data(mockFlagResponse)) + }), + graphql.query('OwnerTier', (req, res, ctx) => { + if (tierName === TierNames.TEAM) { + return res( + ctx.status(200), + ctx.data({ owner: { plan: { tierName: TierNames.TEAM } } }) + ) + } + return res( + ctx.status(200), + ctx.data({ owner: { plan: { tierName: TierNames.PRO } } }) + ) }) ) } describe('rendering component', () => { - beforeEach(() => setup()) - describe('displaying the tree path', () => { it('displays repo link', async () => { + setup() render(, { wrapper: wrapper() }) const repoName = await screen.findByRole('link', { name: 'mightynein' }) @@ -169,6 +187,7 @@ describe('FileView', () => { }) it('displays directory link', async () => { + setup() render(, { wrapper: wrapper() }) const repoName = await screen.findByRole('link', { name: 'folder' }) @@ -180,6 +199,7 @@ describe('FileView', () => { }) it('displays file name', async () => { + setup() render(, { wrapper: wrapper() }) const fileName = await screen.findByText('file.js') @@ -189,6 +209,7 @@ describe('FileView', () => { describe('displaying the file viewer', () => { it('sets the correct url link', async () => { + setup() render(, { wrapper: wrapper() }) const copyLink = await screen.findByRole('link', { @@ -198,5 +219,29 @@ describe('FileView', () => { expect(copyLink).toHaveAttribute('href', '#folder/file.js') }) }) + + describe('displaying the flag selector', () => { + it('renders the flag multi select', async () => { + setup() + render(, { wrapper: wrapper() }) + + const select = await screen.findByText('All flags') + expect(select).toBeInTheDocument() + }) + + describe('on a team plan', () => { + it('does not render the flag multi select', async () => { + setup({ tierName: TierNames.TEAM }) + render(, { wrapper: wrapper() }) + + await waitFor(() => queryClient.isFetching) + await waitFor(() => !queryClient.isFetching) + + await waitFor(() => + expect(screen.queryByText('All flags')).not.toBeInTheDocument() + ) + }) + }) + }) }) })