From f560608dc1d2f3e764446bb86461ecd91175b2c1 Mon Sep 17 00:00:00 2001 From: JerrySentry Date: Thu, 25 Apr 2024 16:13:03 -0400 Subject: [PATCH] Assortment of UI changes for coverage over time for components --- .../ComponentsTab/ComponentsTable.spec.jsx | 3 ++ .../routes/ComponentsTab/ComponentsTable.tsx | 13 ++++- .../routes/ComponentsTab/hooks/query.ts | 3 ++ .../hooks/useComponentComparison.spec.tsx | 6 +++ .../hooks/useComponentComparison.tsx | 5 ++ .../SyncingBanner/SyncingBanner.spec.tsx | 2 +- .../SyncingBanner/SyncingBanner.tsx | 7 ++- .../TriggerSyncBanner/TriggerSyncBanner.tsx | 5 +- .../BranchSelector/BranchSelector.spec.tsx | 14 +++--- .../Header/BranchSelector/BranchSelector.tsx | 30 +++++++++-- .../RepoPage/ComponentsTab/Header/Header.tsx | 8 +-- .../ComponentsTable/ComponentsTable.spec.tsx | 8 ++- .../ComponentsTable/ComponentsTable.tsx | 50 ++++++++++++++++++- .../DeleteComponentModal.spec.tsx | 12 ++--- .../DeleteComponentModal.tsx | 34 +++++++++---- .../navigation/useNavLinks/useNavLinks.js | 9 +++- src/services/repo/useActivateMeasurements.tsx | 9 +++- .../ComponentsNotConfigured.tsx | 2 +- 18 files changed, 175 insertions(+), 45 deletions(-) diff --git a/src/pages/PullRequestPage/PullCoverage/routes/ComponentsTab/ComponentsTable.spec.jsx b/src/pages/PullRequestPage/PullCoverage/routes/ComponentsTab/ComponentsTable.spec.jsx index 26d4b4697e..1cfc06835b 100644 --- a/src/pages/PullRequestPage/PullCoverage/routes/ComponentsTab/ComponentsTable.spec.jsx +++ b/src/pages/PullRequestPage/PullCoverage/routes/ComponentsTab/ComponentsTable.spec.jsx @@ -57,6 +57,9 @@ const mockPull = { }, ], }, + head: { + branchName: 'abc', + }, }, }, }, diff --git a/src/pages/PullRequestPage/PullCoverage/routes/ComponentsTab/ComponentsTable.tsx b/src/pages/PullRequestPage/PullCoverage/routes/ComponentsTab/ComponentsTable.tsx index 43a9991152..2163a95615 100644 --- a/src/pages/PullRequestPage/PullCoverage/routes/ComponentsTab/ComponentsTable.tsx +++ b/src/pages/PullRequestPage/PullCoverage/routes/ComponentsTab/ComponentsTable.tsx @@ -10,6 +10,7 @@ import qs, { type ParsedQs } from 'qs' import { useMemo } from 'react' import { useLocation } from 'react-router-dom' +import A from 'ui/A' import Spinner from 'ui/Spinner' import TotalsNumber from 'ui/TotalsNumber' @@ -132,7 +133,17 @@ export default function ComponentsTable() { return ( <> -
+
+ + View components over time +
diff --git a/src/pages/PullRequestPage/PullCoverage/routes/ComponentsTab/hooks/query.ts b/src/pages/PullRequestPage/PullCoverage/routes/ComponentsTab/hooks/query.ts index 6c48f14ffb..bc87cc1930 100644 --- a/src/pages/PullRequestPage/PullCoverage/routes/ComponentsTab/hooks/query.ts +++ b/src/pages/PullRequestPage/PullCoverage/routes/ComponentsTab/hooks/query.ts @@ -40,6 +40,9 @@ export const query = ` message } } + head { + branchName + } } } ... on NotFoundError { diff --git a/src/pages/PullRequestPage/PullCoverage/routes/ComponentsTab/hooks/useComponentComparison.spec.tsx b/src/pages/PullRequestPage/PullCoverage/routes/ComponentsTab/hooks/useComponentComparison.spec.tsx index 5c03551b39..c8699d1a7a 100644 --- a/src/pages/PullRequestPage/PullCoverage/routes/ComponentsTab/hooks/useComponentComparison.spec.tsx +++ b/src/pages/PullRequestPage/PullCoverage/routes/ComponentsTab/hooks/useComponentComparison.spec.tsx @@ -28,6 +28,9 @@ const mockResponse = { }, ], }, + head: { + branchName: 'abc', + }, }, }, }, @@ -139,6 +142,9 @@ describe('useComponentComparison', () => { }, ], }, + head: { + branchName: 'abc', + }, }, }) ) diff --git a/src/pages/PullRequestPage/PullCoverage/routes/ComponentsTab/hooks/useComponentComparison.tsx b/src/pages/PullRequestPage/PullCoverage/routes/ComponentsTab/hooks/useComponentComparison.tsx index 80d5a77d5d..89788acac5 100644 --- a/src/pages/PullRequestPage/PullCoverage/routes/ComponentsTab/hooks/useComponentComparison.tsx +++ b/src/pages/PullRequestPage/PullCoverage/routes/ComponentsTab/hooks/useComponentComparison.tsx @@ -51,6 +51,11 @@ const RepositorySchema = z.object({ MissingHeadCommitSchema, MissingHeadReportSchema, ]), + head: z + .object({ + branchName: z.string().nullable(), + }) + .nullable(), }) .nullable(), }) diff --git a/src/pages/RepoPage/ComponentsTab/BackfillBanners/SyncingBanner/SyncingBanner.spec.tsx b/src/pages/RepoPage/ComponentsTab/BackfillBanners/SyncingBanner/SyncingBanner.spec.tsx index 28a3580e86..855f31f1ef 100644 --- a/src/pages/RepoPage/ComponentsTab/BackfillBanners/SyncingBanner/SyncingBanner.spec.tsx +++ b/src/pages/RepoPage/ComponentsTab/BackfillBanners/SyncingBanner/SyncingBanner.spec.tsx @@ -26,7 +26,7 @@ describe('SyncingBanner', () => { it('renders heading and content components', () => { render(, { wrapper }) const historicalDataTextLong = screen.getByText( - 'It might take up to 24 hours to view your data.' + 'It might take up to 1 hour to view your data.' ) expect(historicalDataTextLong).toBeInTheDocument() }) diff --git a/src/pages/RepoPage/ComponentsTab/BackfillBanners/SyncingBanner/SyncingBanner.tsx b/src/pages/RepoPage/ComponentsTab/BackfillBanners/SyncingBanner/SyncingBanner.tsx index 594ba57f19..63e3b75981 100644 --- a/src/pages/RepoPage/ComponentsTab/BackfillBanners/SyncingBanner/SyncingBanner.tsx +++ b/src/pages/RepoPage/ComponentsTab/BackfillBanners/SyncingBanner/SyncingBanner.tsx @@ -1,11 +1,14 @@ import Spinner from 'ui/Spinner' +import { LoadingTable } from '../../subroute/ComponentsTable/ComponentsTable' + function SyncingBanner() { return ( -
+
+
-

It might take up to 24 hours to view your data.

+

It might take up to 1 hour to view your data.

) diff --git a/src/pages/RepoPage/ComponentsTab/BackfillBanners/TriggerSyncBanner/TriggerSyncBanner.tsx b/src/pages/RepoPage/ComponentsTab/BackfillBanners/TriggerSyncBanner/TriggerSyncBanner.tsx index 3618b71faa..66dd606c6e 100644 --- a/src/pages/RepoPage/ComponentsTab/BackfillBanners/TriggerSyncBanner/TriggerSyncBanner.tsx +++ b/src/pages/RepoPage/ComponentsTab/BackfillBanners/TriggerSyncBanner/TriggerSyncBanner.tsx @@ -3,6 +3,8 @@ import { useParams } from 'react-router-dom' import { MEASUREMENT_TYPE, useActivateMeasurements } from 'services/repo' import Button from 'ui/Button' +import { LoadingTable } from '../../subroute/ComponentsTable/ComponentsTable' + type URLParams = { provider: string owner: string @@ -19,7 +21,8 @@ function TriggerSyncBanner() { }) return ( -
+
+

No data to display

You will need to enable components to see related coverage data.

diff --git a/src/pages/RepoPage/ComponentsTab/Header/BranchSelector/BranchSelector.spec.tsx b/src/pages/RepoPage/ComponentsTab/Header/BranchSelector/BranchSelector.spec.tsx index 493a293bce..04e3c981b9 100644 --- a/src/pages/RepoPage/ComponentsTab/Header/BranchSelector/BranchSelector.spec.tsx +++ b/src/pages/RepoPage/ComponentsTab/Header/BranchSelector/BranchSelector.spec.tsx @@ -203,7 +203,7 @@ describe('BranchSelector', () => { describe('with populated data', () => { it('renders the branch selector', async () => { const { queryClient } = setup() - render(, { + render(, { wrapper: wrapper(queryClient), }) @@ -213,7 +213,7 @@ describe('BranchSelector', () => { it('renders default branch as selected branch', async () => { const { queryClient } = setup() - render(, { + render(, { wrapper: wrapper(queryClient), }) @@ -226,7 +226,7 @@ describe('BranchSelector', () => { describe('user selects a branch', () => { it('updates params with selected branch', async () => { const { user, queryClient } = setup() - render(, { + render(, { wrapper: wrapper(queryClient), }) @@ -258,7 +258,7 @@ describe('BranchSelector', () => { isIntersecting: true, }) - render(, { + render(, { wrapper: wrapper(queryClient), }) @@ -281,7 +281,7 @@ describe('BranchSelector', () => { isIntersecting: true, }) - render(, { + render(, { wrapper: wrapper(queryClient), }) @@ -298,7 +298,7 @@ describe('BranchSelector', () => { describe('user searches for branch', () => { it('calls the api with the search value', async () => { const { mockSearching, user, queryClient } = setup() - render(, { + render(, { wrapper: wrapper(queryClient), }) @@ -319,7 +319,7 @@ describe('BranchSelector', () => { const { queryClient } = setup({ nullOverview: true, }) - render(, { + render(, { wrapper: wrapper(queryClient), }) diff --git a/src/pages/RepoPage/ComponentsTab/Header/BranchSelector/BranchSelector.tsx b/src/pages/RepoPage/ComponentsTab/Header/BranchSelector/BranchSelector.tsx index a16b25c1d2..f519014896 100644 --- a/src/pages/RepoPage/ComponentsTab/Header/BranchSelector/BranchSelector.tsx +++ b/src/pages/RepoPage/ComponentsTab/Header/BranchSelector/BranchSelector.tsx @@ -4,6 +4,7 @@ import { useParams } from 'react-router-dom' import { Branch, useBranch, useBranches } from 'services/branches' import { useLocationParams } from 'services/navigation' import { useRepoOverview } from 'services/repo' +import A from 'ui/A' import Icon from 'ui/Icon' import Select from 'ui/Select' @@ -21,7 +22,11 @@ const defaultQueryParams = { const getDecodedBranch = (branch?: string) => !!branch ? decodeURIComponent(branch) : undefined -const BranchSelector: React.FC = () => { +interface BranchSelectorProps { + isDisabled: boolean | undefined +} + +const BranchSelector: React.FC = ({ isDisabled }) => { const { provider, owner, repo, branch } = useParams() const { params, updateParams } = useLocationParams(defaultQueryParams) const [branchSearchTerm, setBranchSearchTerm] = useState('') @@ -48,7 +53,10 @@ const BranchSelector: React.FC = () => { }) const decodedBranch = getDecodedBranch(branch) - const selectedBranch = decodedBranch ?? overview?.defaultBranch ?? '' + + // @ts-expect-error + const selectedBranch = + decodedBranch || params.branch || overview?.defaultBranch || '' const { data: searchBranchValue } = useBranch({ provider, @@ -70,7 +78,7 @@ const BranchSelector: React.FC = () => { } return ( -
+

@@ -97,8 +105,24 @@ const BranchSelector: React.FC = () => { } }} onSearch={(term: string) => setBranchSearchTerm(term)} + disabled={isDisabled} /> + {selection?.head?.commitid && ( +

+ Source: latest commit{' '} + + {selection?.head?.commitid.slice(0, 7)} + +

+ )}

) } diff --git a/src/pages/RepoPage/ComponentsTab/Header/Header.tsx b/src/pages/RepoPage/ComponentsTab/Header/Header.tsx index f775f6d29f..fe5e2fcfc1 100644 --- a/src/pages/RepoPage/ComponentsTab/Header/Header.tsx +++ b/src/pages/RepoPage/ComponentsTab/Header/Header.tsx @@ -60,8 +60,8 @@ const Header = ({ return (
- -
+ +

Configured components

@@ -73,7 +73,7 @@ const Header = ({ Learn more

-
+

Historical trend

@@ -91,7 +91,7 @@ const Header = ({ renderSelected={({ label }: { label: string }) => label} />
-
+

Show by

diff --git a/src/pages/RepoPage/ComponentsTab/subroute/ComponentsTable/ComponentsTable.spec.tsx b/src/pages/RepoPage/ComponentsTab/subroute/ComponentsTable/ComponentsTable.spec.tsx index 6cd633554a..ad7fb9dc11 100644 --- a/src/pages/RepoPage/ComponentsTab/subroute/ComponentsTable/ComponentsTable.spec.tsx +++ b/src/pages/RepoPage/ComponentsTab/subroute/ComponentsTable/ComponentsTable.spec.tsx @@ -275,9 +275,7 @@ describe('ComponentsTable', () => { const { user } = setup({}) render(, { wrapper: wrapper() }) - const trashIconButtons = await screen.findAllByRole('button', { - name: /trash/, - }) + const trashIconButtons = await screen.findAllByTestId('delete-component') expect(trashIconButtons).toHaveLength(3) const [firstIcon] = trashIconButtons @@ -287,8 +285,8 @@ describe('ComponentsTable', () => { } }) - const deleteComponentModalText = await screen.findByText( - 'Delete Component' + const deleteComponentModalText = await screen.findByTestId( + 'delete-component-modal' ) expect(deleteComponentModalText).toBeInTheDocument() diff --git a/src/pages/RepoPage/ComponentsTab/subroute/ComponentsTable/ComponentsTable.tsx b/src/pages/RepoPage/ComponentsTab/subroute/ComponentsTable/ComponentsTable.tsx index 5938cc9dd8..a01d6efcc1 100644 --- a/src/pages/RepoPage/ComponentsTab/subroute/ComponentsTable/ComponentsTable.tsx +++ b/src/pages/RepoPage/ComponentsTab/subroute/ComponentsTable/ComponentsTable.tsx @@ -35,6 +35,50 @@ interface ComponentsTableHelper { delete: React.ReactElement | null } +export const LoadingTable = () => { + const data = useMemo(() => [], []) + const table = useReactTable({ + data, + columns, + getCoreRowModel: getCoreRowModel(), + }) + + return ( +
+ + + + + + + + + + {table.getHeaderGroups().map((headerGroup) => ( + + {headerGroup.headers.map((header) => ( + + ))} + + ))} + +
+
+ {flexRender( + header.column.columnDef.header, + header.getContext() + )} +
+
+
+ ) +} + const columnHelper = createColumnHelper() const columns = [ @@ -125,7 +169,7 @@ function createTableData({ /> ), lastUploaded: ( - + {lastUploaded ? formatTimeToNow(lastUploaded) : ''} ), @@ -135,6 +179,7 @@ function createTableData({ data-testid="delete-component" onClick={() => setModalInfo({ componentId: name, showModal: true })} className="text-ds-gray-tertiary hover:text-ds-gray-senary" + aria-label={'delete ' + name} > @@ -199,7 +244,8 @@ const ComponentTable = memo(function Table({ >
{flexRender( diff --git a/src/pages/RepoPage/ComponentsTab/subroute/ComponentsTable/DeleteComponentModal/DeleteComponentModal.spec.tsx b/src/pages/RepoPage/ComponentsTab/subroute/ComponentsTable/DeleteComponentModal/DeleteComponentModal.spec.tsx index 57a8732287..49802145b2 100644 --- a/src/pages/RepoPage/ComponentsTab/subroute/ComponentsTable/DeleteComponentModal/DeleteComponentModal.spec.tsx +++ b/src/pages/RepoPage/ComponentsTab/subroute/ComponentsTable/DeleteComponentModal/DeleteComponentModal.spec.tsx @@ -70,12 +70,12 @@ describe('DeleteComponentModal', () => { const messagePartOne = await screen.findByText(/This will remove the/) expect(messagePartOne).toBeInTheDocument() const messagePartTwo = await screen.findByText( - /component from the reports in app. You will also need to remove this component in your CI and codecov.yaml to stop uploads./ + /It will take some time to reflect this deletion./ ) expect(messagePartTwo).toBeInTheDocument() - const componentId = await screen.findByText(/component-123/) - expect(componentId).toBeInTheDocument() + const componentId = await screen.findAllByText(/component-123/) + expect(componentId).toHaveLength(3) }) it('renders delete and cancel buttons', async () => { @@ -90,7 +90,7 @@ describe('DeleteComponentModal', () => { } ) const deleteButton = await screen.findByRole('button', { - name: /Delete component/, + name: /Remove/, }) expect(deleteButton).toBeInTheDocument() const cancelButton = await screen.findByRole('button', { name: /Cancel/ }) @@ -108,7 +108,7 @@ describe('DeleteComponentModal', () => { wrapper, } ) - const title = await screen.findByText(/Delete Component/) + const title = await screen.findByTestId(/remove-component-123/) expect(title).toBeInTheDocument() }) }) @@ -129,7 +129,7 @@ describe('DeleteComponentModal', () => { ) const deleteButton = await screen.findByRole('button', { - name: /Delete component/, + name: /Remove/, }) await user.click(deleteButton) await waitFor(() => expect(closeModal).toHaveBeenCalled()) diff --git a/src/pages/RepoPage/ComponentsTab/subroute/ComponentsTable/DeleteComponentModal/DeleteComponentModal.tsx b/src/pages/RepoPage/ComponentsTab/subroute/ComponentsTable/DeleteComponentModal/DeleteComponentModal.tsx index f27de16b1b..5ec3862f65 100644 --- a/src/pages/RepoPage/ComponentsTab/subroute/ComponentsTable/DeleteComponentModal/DeleteComponentModal.tsx +++ b/src/pages/RepoPage/ComponentsTab/subroute/ComponentsTable/DeleteComponentModal/DeleteComponentModal.tsx @@ -17,14 +17,29 @@ const DeleteComponentModal = ({ isOpen, closeModal, componentId }: Props) => { onClose={closeModal} hasCloseButton={true} size="small" - title={Delete Component} + title={ + + Remove {componentId} + + } body={ -

- This will remove the{' '} - {componentId} component from - the reports in app. You will also need to remove this component in - your CI and codecov.yaml to stop uploads. -

+
+

+ This will remove the historical data of{' '} + {componentId}{' '} + component in the app and we can’t retrieve the data. +

+

+

+ Action required: You’ll need + to remove{' '} + {componentId}{' '} + component in your yaml file otherwise you’ll still see it in this + table. +

+

+

It will take some time to reflect this deletion.

+
} footer={
@@ -38,7 +53,7 @@ const DeleteComponentModal = ({ isOpen, closeModal, componentId }: Props) => {
diff --git a/src/services/navigation/useNavLinks/useNavLinks.js b/src/services/navigation/useNavLinks/useNavLinks.js index b40cd5b660..9e069a0553 100644 --- a/src/services/navigation/useNavLinks/useNavLinks.js +++ b/src/services/navigation/useNavLinks/useNavLinks.js @@ -382,12 +382,17 @@ export function useNavLinks() { }, componentsTab: { path: ( - { provider = p, owner = o, repo = r } = { + { provider = p, owner = o, repo = r, branch = undefined } = { provider: p, owner: o, repo: r, } - ) => `/${provider}/${owner}/${repo}/components`, + ) => { + if (branch) { + return `/${provider}/${owner}/${repo}/components?branch=${branch}` + } + return `/${provider}/${owner}/${repo}/components` + }, isExternalLink: false, text: 'Components', }, diff --git a/src/services/repo/useActivateMeasurements.tsx b/src/services/repo/useActivateMeasurements.tsx index 5befc707b9..f9755b1157 100644 --- a/src/services/repo/useActivateMeasurements.tsx +++ b/src/services/repo/useActivateMeasurements.tsx @@ -94,6 +94,11 @@ export function useActivateMeasurements({ queryClient.invalidateQueries([ 'BackfillFlagMemberships', + provider, + owner, + repo, + ]) + queryClient.invalidateQueries([ 'BackfillComponentMemberships', provider, owner, @@ -101,9 +106,11 @@ export function useActivateMeasurements({ ]) }, onError: () => { + const measurement = + measurementType === 'FLAG_COVERAGE' ? 'flag' : 'component' renderToast({ type: 'error', - title: 'Error activating flag measurements', + title: 'Error activating ' + measurement + ' measurements', content: 'Please try again. If the error persists please contact support', options: { diff --git a/src/shared/ComponentsNotConfigured/ComponentsNotConfigured.tsx b/src/shared/ComponentsNotConfigured/ComponentsNotConfigured.tsx index 83411e7c18..8c445afa5f 100644 --- a/src/shared/ComponentsNotConfigured/ComponentsNotConfigured.tsx +++ b/src/shared/ComponentsNotConfigured/ComponentsNotConfigured.tsx @@ -2,7 +2,7 @@ import Button from 'ui/Button' function ComponentsNotConfigured() { return ( -
+

No data to display