diff --git a/jest.config.ts b/jest.config.ts index a8fc9e613968..f0e68a5aa0e9 100644 --- a/jest.config.ts +++ b/jest.config.ts @@ -73,7 +73,22 @@ if (CI && !process.env.JEST_LIST_TESTS_INNER) { encoding: 'utf-8', env: {...process.env, JEST_LIST_TESTS_INNER: '1'}, }); - JEST_TESTS = JSON.parse(stdout); + const parsed: string[] = JSON.parse(stdout); + // `jest --listTests` is supposed to honor `testMatch`, but on CI we've + // seen it return non-test files (vendor JS, theme tokens, etc.) when + // combined with `--changedSince`. Filter to actual test files so the + // shard runner doesn't try to `it()`-execute random source files and + // burn the 30-min budget on "Your test suite must contain at least + // one test." failures. + JEST_TESTS = parsed.filter( + file => /\.(spec|test)\.[jt]sx?$/.test(file) || file.includes('/__tests__/') + ); + if (MERGE_BASE) { + console.log( + `JEST_TESTS: ${JEST_TESTS.length} test file(s) selected ` + + `(raw listTests output: ${parsed.length})` + ); + } } catch (err: any) { if (err.code) { throw new Error(`err code ${err.code} when spawning process`); @@ -215,6 +230,7 @@ function getTestsForGroup( if ( JEST_TESTS && + JEST_TESTS.length > 0 && typeof CI_NODE_TOTAL !== 'undefined' && typeof CI_NODE_INDEX !== 'undefined' ) { @@ -315,7 +331,18 @@ const config: Config.InitialOptions = { '/tests/js/setup.ts', '/tests/js/setupFramework.ts', ], - testMatch: testMatch || ['/(static|tests/js)/**/?(*.)+(spec|test).[jt]s?(x)'], + // When `testMatch` was assigned by the sharding block above and ended up + // empty (PR diff selected zero tests via `--changedSince`), `[] || X` + // would yield `[]` (empty arrays are truthy), and jest then falls back to + // its own default discovery and runs the whole suite. Treat an empty + // assigned array the same as "no shard work to do" so `passWithNoTests` + // can short-circuit. + testMatch: + testMatch && testMatch.length > 0 + ? testMatch + : MERGE_BASE + ? ['/__no_tests_for_this_shard__'] + : ['/(static|tests/js)/**/?(*.)+(spec|test).[jt]s?(x)'], testPathIgnorePatterns: ['/tests/sentry/lang/javascript/'], unmockedModulePathPatterns: [ diff --git a/static/gsAdmin/views/invoiceComparison.tsx b/static/gsAdmin/views/invoiceComparison.tsx index 0e96a45e1c19..a886e0e2252e 100644 --- a/static/gsAdmin/views/invoiceComparison.tsx +++ b/static/gsAdmin/views/invoiceComparison.tsx @@ -1,7 +1,7 @@ -import {Fragment, useState} from 'react'; +import {Fragment, useEffect, useState} from 'react'; import {css} from '@emotion/react'; import styled from '@emotion/styled'; -import {skipToken, useQuery} from '@tanstack/react-query'; +import {keepPreviousData, skipToken, useQuery} from '@tanstack/react-query'; import {Alert} from '@sentry/scraps/alert'; import {Tag, type TagProps} from '@sentry/scraps/badge'; @@ -19,6 +19,8 @@ import {PanelBody} from 'sentry/components/panels/panelBody'; import {PanelHeader} from 'sentry/components/panels/panelHeader'; import {apiOptions} from 'sentry/utils/api/apiOptions'; import {getRegions} from 'sentry/utils/regions'; +import {useLocation} from 'sentry/utils/useLocation'; +import {useNavigate} from 'sentry/utils/useNavigate'; type RowStatus = 'match' | 'mismatch' | 'legacy_only' | 'platform_only'; @@ -42,12 +44,16 @@ type Summary = { platform_total_cents: number; queried_at: string; row_count: number; + rows_page: number; + rows_page_size: number; + rows_total_pages: number; start: string; - truncated: boolean; unmatched_invoice_count: number; unmatched_invoice_pct: number; unmatched_org_count: number; - unmatched_truncated: boolean; + unmatched_page: number; + unmatched_page_size: number; + unmatched_total_pages: number; }; type UnmatchedSide = 'legacy_only' | 'platform_only'; @@ -115,35 +121,197 @@ function localInputToUtcIso(value: string): string { return new Date(value).toISOString(); } +const PAGE_SIZE_OPTIONS: readonly number[] = [25, 50, 100, 250]; +const DEFAULT_PAGE_SIZE = 50; + +function firstQueryValue(value: unknown): string | undefined { + if (Array.isArray(value)) { + return value[0]; + } + return typeof value === 'string' ? value : undefined; +} + +function parsePageParam(value: unknown): number { + const n = Number(firstQueryValue(value)); + return Number.isFinite(n) && n >= 1 ? Math.floor(n) : 1; +} + +function parsePageSizeParam(value: unknown): number { + const n = Number(firstQueryValue(value)); + return PAGE_SIZE_OPTIONS.includes(n) ? n : DEFAULT_PAGE_SIZE; +} + +// A UTC ISO string from a `datetime-local` input that's already in UTC. +// Used to convert the URL-persisted absolute time back into a value the +// input control will accept (which is local-tz, no offset). +function utcIsoToDatetimeLocalValue(iso: string): string { + const d = new Date(iso); + if (Number.isNaN(d.getTime())) { + return ''; + } + const pad = (n: number) => String(n).padStart(2, '0'); + return ( + `${d.getFullYear()}-${pad(d.getMonth() + 1)}-${pad(d.getDate())}` + + `T${pad(d.getHours())}:${pad(d.getMinutes())}` + ); +} + export function InvoiceComparison() { const regions = getRegions(); + const location = useLocation(); + const navigate = useNavigate(); const [region, setRegion] = useState(regions[0] ?? null); - const [startInput, setStartInput] = useState(hoursAgoLocal(24)); - const [endInput, setEndInput] = useState(nowLocal()); - const [submitted, setSubmitted] = useState<{end: string; start: string} | null>(null); - const enabled = Boolean(submitted && region); - const {data, isPending, isError, error} = useQuery({ + // URL is the source of truth for everything that affects the query and + // the displayed paginator state, so refresh / share-link reproduces the + // exact view. Local state only tracks what the user is typing in the + // date inputs before they click Run. + const queryStart = firstQueryValue(location.query.start); + const queryEnd = firstQueryValue(location.query.end); + const rowsPage = parsePageParam(location.query.rows_page); + const unmatchedPage = parsePageParam(location.query.unmatched_page); + const pageSize = parsePageSizeParam(location.query.page_size); + + const [startInput, setStartInput] = useState(() => + queryStart ? utcIsoToDatetimeLocalValue(queryStart) : hoursAgoLocal(24) + ); + const [endInput, setEndInput] = useState(() => + queryEnd ? utcIsoToDatetimeLocalValue(queryEnd) : nowLocal() + ); + + // Keep the date inputs in sync with the URL on back/forward and in-app + // navigation that changes start/end (e.g. opening a shared link). When the + // URL has no window yet we leave the inputs alone so the user's in-progress + // typing isn't clobbered. + useEffect(() => { + if (queryStart) { + setStartInput(utcIsoToDatetimeLocalValue(queryStart)); + } + if (queryEnd) { + setEndInput(utcIsoToDatetimeLocalValue(queryEnd)); + } + }, [queryStart, queryEnd]); + + const enabled = Boolean(queryStart && queryEnd && region); + const {data, isPending, isError, error, isPlaceholderData} = useQuery({ ...apiOptions.as()('/_admin/cells/$region/invoice-comparison/', { path: enabled && region ? {region: region.name} : skipToken, host: region?.url, - query: submitted ?? undefined, + query: enabled + ? { + start: queryStart, + end: queryEnd, + rows_page: rowsPage, + unmatched_page: unmatchedPage, + rows_page_size: pageSize, + unmatched_page_size: pageSize, + } + : undefined, staleTime: 0, }), + // Keep the previously-rendered page visible while paginating or + // changing page size, so the tables and summary don't unmount and + // collapse the layout to a spinner between requests. + placeholderData: keepPreviousData, }); + // If the URL has an unsupported `page_size` (e.g. `?page_size=200`), + // rewrite it to the resolved value so the address bar matches what the + // API and UI actually use — and refresh / share-links reproduce the + // intended view. Done eagerly (not gated on `data`) since the validity + // check is purely client-side. + const rawPageSize = firstQueryValue(location.query.page_size); + useEffect(() => { + if (rawPageSize !== undefined && rawPageSize !== String(pageSize)) { + navigate( + { + pathname: location.pathname, + query: {...location.query, page_size: String(pageSize)}, + }, + {replace: true} + ); + } + }, [rawPageSize, pageSize, navigate, location.pathname, location.query]); + + // If the backend clamped our requested page (e.g. user landed on + // ?rows_page=99 against a 2-page result), rewrite the URL to the + // clamped value so refresh + share-links converge on the real page. + // + // Skip while `data` is the placeholder from the previous page — its + // summary describes the prior request, not the one we just kicked + // off, so honoring it would rewrite the URL back to where we just + // were and break Prev/Next. + useEffect(() => { + if (!data || isPlaceholderData) { + return; + } + const fixes: Record = {}; + if (data.summary.rows_page !== rowsPage) { + fixes.rows_page = String(data.summary.rows_page); + } + if (data.summary.unmatched_page !== unmatchedPage) { + fixes.unmatched_page = String(data.summary.unmatched_page); + } + if (Object.keys(fixes).length > 0) { + navigate( + {pathname: location.pathname, query: {...location.query, ...fixes}}, + {replace: true} + ); + } + }, [ + data, + isPlaceholderData, + rowsPage, + unmatchedPage, + navigate, + location.pathname, + location.query, + ]); + // The endpoint returns rows pre-sorted by |delta_pct| desc; this component // does not re-sort. See AdminInvoiceComparisonEndpoint and its test_sort_* // tests for the contract. const rows = data?.rows ?? []; + const setPage = (key: 'rows_page' | 'unmatched_page', page: number) => { + navigate({ + pathname: location.pathname, + query: {...location.query, [key]: String(page)}, + }); + }; + + const setPageSize = (size: number) => { + // Changing page size with a non-1 page is almost never what the user + // wants — the offset they were viewing maps to a different position in + // the resized list. Reset both paginators. + navigate({ + pathname: location.pathname, + query: { + ...location.query, + page_size: String(size), + rows_page: '1', + unmatched_page: '1', + }, + }); + }; + const onSubmit = () => { if (!startInput || !endInput) { return; } - setSubmitted({ - start: localInputToUtcIso(startInput), - end: localInputToUtcIso(endInput), + // Re-running the comparison persists the chosen window to the URL and + // resets both paginators — the new window's result set is unrelated + // to the previous one's page numbers. + navigate({ + pathname: location.pathname, + query: { + ...location.query, + start: localInputToUtcIso(startInput), + end: localInputToUtcIso(endInput), + rows_page: '1', + unmatched_page: '1', + page_size: String(pageSize), + }, }); }; @@ -196,6 +364,20 @@ export function InvoiceComparison() { onChange={e => setEndInput(e.target.value)} /> + + Results per page + ( + + )} + value={String(pageSize)} + options={PAGE_SIZE_OPTIONS.map(n => ({ + label: String(n), + value: String(n), + }))} + onChange={opt => setPageSize(Number(opt.value))} + /> + @@ -203,7 +385,7 @@ export function InvoiceComparison() { - {submitted && isPending && Comparing…} + {enabled && isPending && Comparing…} {isError && ( @@ -279,11 +461,6 @@ export function InvoiceComparison() { {data.summary.row_count} - {data.summary.truncated && ( - - (showing top {data.rows.length}) - - )} @@ -307,6 +484,13 @@ export function InvoiceComparison() { Rows (sorted by |delta|, biggest first) — queried {data.summary.queried_at} + setPage('rows_page', p)} + /> @@ -362,16 +546,15 @@ export function InvoiceComparison() { - - Unmatched orgs (one side only, sorted by |amount|) - {data.summary.unmatched_truncated && ( - - showing top {data.unmatched.length} of{' '} - {data.summary.unmatched_org_count} orgs - - )} - + Unmatched orgs (one side only, sorted by |amount|) + setPage('unmatched_page', p)} + />
@@ -417,6 +600,54 @@ export function InvoiceComparison() { ); } +function Paginator({ + page, + totalPages, + total, + pageSize, + onChange, +}: { + onChange: (page: number) => void; + page: number; + pageSize: number; + total: number; + totalPages: number; +}) { + // Empty result set: nothing to page through. Render the row count anyway + // so the table doesn't look like it's missing its header. + if (totalPages <= 1) { + return ( + + + {total === 0 ? 'No rows' : `${total} row${total === 1 ? '' : 's'}`} + + + ); + } + const start = (page - 1) * pageSize + 1; + const end = Math.min(page * pageSize, total); + return ( + + + {start.toLocaleString()}–{end.toLocaleString()} of {total.toLocaleString()} · page{' '} + {page} of {totalPages} + + + + + + + ); +} + const FieldLabel = styled('label')` font-size: ${p => p.theme.font.size.sm}; color: ${p => p.theme.tokens.content.secondary};