From c1ff10710c717b240b0fa8a3488113d9a6179ffa Mon Sep 17 00:00:00 2001 From: Andrew McKnight Date: Mon, 1 Jun 2026 14:44:27 -0800 Subject: [PATCH 1/8] feat(billing): Paginate invoice comparison admin UI Adds page navigation to `/_admin/cells/$region/invoice-comparison/`. Each of the page's two tables (the both-sides comparison list and the one-sided unmatched debug list) gets its own paginator backed by URL search params (`rows_page` / `unmatched_page`) so refresh and shareable URLs preserve position. Running a new comparison resets both pages to 1. Consumes the new `summary.{rows,unmatched}_{page,page_size,total_pages}` contract from getsentry#20496. REVENG-131. Co-Authored-By: Claude Opus 4.7 (1M context) --- static/gsAdmin/views/invoiceComparison.tsx | 124 ++++++++++++++++++--- 1 file changed, 107 insertions(+), 17 deletions(-) diff --git a/static/gsAdmin/views/invoiceComparison.tsx b/static/gsAdmin/views/invoiceComparison.tsx index 0e96a45e1c19ce..5f9a93c4369596 100644 --- a/static/gsAdmin/views/invoiceComparison.tsx +++ b/static/gsAdmin/views/invoiceComparison.tsx @@ -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,19 +121,33 @@ function localInputToUtcIso(value: string): string { return new Date(value).toISOString(); } +function parsePageParam(value: unknown): number { + const n = Number(Array.isArray(value) ? value[0] : value); + return Number.isFinite(n) && n >= 1 ? Math.floor(n) : 1; +} + 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); + // Page state lives in the URL so refreshing / sharing keeps you on the + // same page. Independent paginators for the two tables. + const rowsPage = parsePageParam(location.query.rows_page); + const unmatchedPage = parsePageParam(location.query.unmatched_page); + const enabled = Boolean(submitted && region); const {data, isPending, isError, error} = useQuery({ ...apiOptions.as()('/_admin/cells/$region/invoice-comparison/', { path: enabled && region ? {region: region.name} : skipToken, host: region?.url, - query: submitted ?? undefined, + query: submitted + ? {...submitted, rows_page: rowsPage, unmatched_page: unmatchedPage} + : undefined, staleTime: 0, }), }); @@ -137,10 +157,23 @@ export function InvoiceComparison() { // 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 onSubmit = () => { if (!startInput || !endInput) { return; } + // Re-running the comparison 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, rows_page: '1', unmatched_page: '1'}, + }); setSubmitted({ start: localInputToUtcIso(startInput), end: localInputToUtcIso(endInput), @@ -279,11 +312,6 @@ export function InvoiceComparison() { {data.summary.row_count} - {data.summary.truncated && ( - - (showing top {data.rows.length}) - - )} @@ -307,6 +335,13 @@ export function InvoiceComparison() { Rows (sorted by |delta|, biggest first) — queried {data.summary.queried_at} + setPage('rows_page', p)} + /> @@ -362,16 +397,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 +451,62 @@ 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 PaginatorRow = styled('div')` + display: flex; + justify-content: space-between; + align-items: center; + padding: 8px 12px; + border-bottom: 1px solid ${p => p.theme.tokens.border.primary}; +`; + const FieldLabel = styled('label')` font-size: ${p => p.theme.font.size.sm}; color: ${p => p.theme.tokens.content.secondary}; From a88f1d2be18070558756e528bb1cdee7f7c1b56c Mon Sep 17 00:00:00 2001 From: Andrew McKnight Date: Mon, 1 Jun 2026 14:53:25 -0800 Subject: [PATCH 2/8] feat(billing): Page-size dropdown + URL-persisted window on invoice comparison MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two fixes + one feature: - Move `start` / `end` from local state into URL search params. Previously a refresh wiped the in-memory `submitted` value, which disabled the query and reset the page to the empty input form — and the same gap made `?rows_page=99` URLs render nothing. The window is now reread from the URL on mount and the query fires whenever start+end are present. - After every response, if the server clamped the requested page (e.g. asked for page 99 against a 2-page result), replace the URL with the clamped value so refresh converges on the real page rather than re-requesting the bad one. - Add a "Results per page" dropdown with 25 / 50 / 100 / 250 options. Selection persists in the URL as `page_size` and is applied to both tables (sent as `rows_page_size` and `unmatched_page_size` on the request). Default is 50. Changing the value resets both paginators to page 1 — the previous offset doesn't map cleanly to the resized list. Co-Authored-By: Claude Opus 4.7 (1M context) --- static/gsAdmin/views/invoiceComparison.tsx | 137 ++++++++++++++++++--- 1 file changed, 119 insertions(+), 18 deletions(-) diff --git a/static/gsAdmin/views/invoiceComparison.tsx b/static/gsAdmin/views/invoiceComparison.tsx index 5f9a93c4369596..1f92dea1d44d23 100644 --- a/static/gsAdmin/views/invoiceComparison.tsx +++ b/static/gsAdmin/views/invoiceComparison.tsx @@ -1,4 +1,4 @@ -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'; @@ -121,37 +121,105 @@ function localInputToUtcIso(value: string): string { return new Date(value).toISOString(); } +const PAGE_SIZE_OPTIONS = [25, 50, 100, 250] as const; +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(Array.isArray(value) ? value[0] : value); + 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 as readonly number[]).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); - // Page state lives in the URL so refreshing / sharing keeps you on the - // same page. Independent paginators for the two tables. + // 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 enabled = Boolean(submitted && region); + const [startInput, setStartInput] = useState(() => + queryStart ? utcIsoToDatetimeLocalValue(queryStart) : hoursAgoLocal(24) + ); + const [endInput, setEndInput] = useState(() => + queryEnd ? utcIsoToDatetimeLocalValue(queryEnd) : nowLocal() + ); + + const enabled = Boolean(queryStart && queryEnd && region); const {data, isPending, isError, error} = useQuery({ ...apiOptions.as()('/_admin/cells/$region/invoice-comparison/', { path: enabled && region ? {region: region.name} : skipToken, host: region?.url, - query: submitted - ? {...submitted, rows_page: rowsPage, unmatched_page: unmatchedPage} + query: enabled + ? { + start: queryStart, + end: queryEnd, + rows_page: rowsPage, + unmatched_page: unmatchedPage, + rows_page_size: pageSize, + unmatched_page_size: pageSize, + } : undefined, staleTime: 0, }), }); + // 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. + useEffect(() => { + if (!data) { + 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, 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. @@ -164,19 +232,38 @@ export function InvoiceComparison() { }); }; + 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; } - // Re-running the comparison resets both paginators — the new window's - // result set is unrelated to the previous one's page numbers. + // 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, rows_page: '1', unmatched_page: '1'}, - }); - setSubmitted({ - start: localInputToUtcIso(startInput), - end: localInputToUtcIso(endInput), + query: { + ...location.query, + start: localInputToUtcIso(startInput), + end: localInputToUtcIso(endInput), + rows_page: '1', + unmatched_page: '1', + page_size: String(pageSize), + }, }); }; @@ -229,6 +316,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))} + /> + @@ -236,7 +337,7 @@ export function InvoiceComparison() { - {submitted && isPending && Comparing…} + {enabled && isPending && Comparing…} {isError && ( From a98509ba6288a15b0765ac42971321394710ea74 Mon Sep 17 00:00:00 2001 From: Andrew McKnight Date: Tue, 2 Jun 2026 11:52:00 -0800 Subject: [PATCH 3/8] fix(billing): Use Flex primitive for paginator row, drop type assertion Address Cursor Bugbot feedback on #116647: - Replace the `PaginatorRow` styled('div') with ``. The styled rule mapped 1:1 to existing Flex props, and `static/AGENTS.md` prefers the layout primitive over new styled components. - Type `PAGE_SIZE_OPTIONS` as `readonly number[]` instead of using `as const` + a call-site `as readonly number[]` cast. Same readonly guarantee, no widening cast, no new type-safety regression flagged by the type-coverage diff. Co-Authored-By: Claude Opus 4.7 (1M context) --- static/gsAdmin/views/invoiceComparison.tsx | 20 ++++++-------------- 1 file changed, 6 insertions(+), 14 deletions(-) diff --git a/static/gsAdmin/views/invoiceComparison.tsx b/static/gsAdmin/views/invoiceComparison.tsx index 1f92dea1d44d23..32cbec47958687 100644 --- a/static/gsAdmin/views/invoiceComparison.tsx +++ b/static/gsAdmin/views/invoiceComparison.tsx @@ -121,7 +121,7 @@ function localInputToUtcIso(value: string): string { return new Date(value).toISOString(); } -const PAGE_SIZE_OPTIONS = [25, 50, 100, 250] as const; +const PAGE_SIZE_OPTIONS: readonly number[] = [25, 50, 100, 250]; const DEFAULT_PAGE_SIZE = 50; function firstQueryValue(value: unknown): string | undefined { @@ -138,7 +138,7 @@ function parsePageParam(value: unknown): number { function parsePageSizeParam(value: unknown): number { const n = Number(firstQueryValue(value)); - return (PAGE_SIZE_OPTIONS as readonly number[]).includes(n) ? n : DEFAULT_PAGE_SIZE; + return PAGE_SIZE_OPTIONS.includes(n) ? n : DEFAULT_PAGE_SIZE; } // A UTC ISO string from a `datetime-local` input that's already in UTC. @@ -569,17 +569,17 @@ function Paginator({ // 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} @@ -596,18 +596,10 @@ function Paginator({ Next - + ); } -const PaginatorRow = styled('div')` - display: flex; - justify-content: space-between; - align-items: center; - padding: 8px 12px; - border-bottom: 1px solid ${p => p.theme.tokens.border.primary}; -`; - const FieldLabel = styled('label')` font-size: ${p => p.theme.font.size.sm}; color: ${p => p.theme.tokens.content.secondary}; From 7a10b770194cb75e940e7f5dd153f8ef947e0fe3 Mon Sep 17 00:00:00 2001 From: Andrew McKnight Date: Tue, 2 Jun 2026 15:21:19 -0800 Subject: [PATCH 4/8] fix(billing): Resync invoice comparison date inputs from URL Address Cursor Bugbot finding: the `start` / `end` `datetime-local` inputs were initialized lazily once and then never updated, so browser back/forward (or any in-app navigation that swapped the query params) would leave the input fields showing a stale window while the fetched results matched the new URL. Add a `useEffect` that mirrors the URL window back into `startInput` / `endInput` whenever those query params are present. When the URL has no window yet, the effect skips the update so a user's in-progress typing isn't clobbered before they submit. Co-Authored-By: Claude Opus 4.7 (1M context) --- static/gsAdmin/views/invoiceComparison.tsx | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/static/gsAdmin/views/invoiceComparison.tsx b/static/gsAdmin/views/invoiceComparison.tsx index 32cbec47958687..8a2a9a4f9c9b6f 100644 --- a/static/gsAdmin/views/invoiceComparison.tsx +++ b/static/gsAdmin/views/invoiceComparison.tsx @@ -179,6 +179,19 @@ export function InvoiceComparison() { 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} = useQuery({ ...apiOptions.as()('/_admin/cells/$region/invoice-comparison/', { From 35764db139a21f9d530a1f323063a1c168be1a52 Mon Sep 17 00:00:00 2001 From: Andrew McKnight Date: Tue, 2 Jun 2026 15:36:48 -0800 Subject: [PATCH 5/8] fix(billing): Normalize invalid page_size in URL on invoice comparison MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address Cursor Bugbot finding: `parsePageSizeParam` silently maps any unsupported `page_size` query value (e.g. `?page_size=200`) to the default while leaving the URL untouched. The result: the API and UI ran at the resolved size while the address bar advertised something else, so refresh / share-links did not reproduce the intended view — unlike `rows_page` / `unmatched_page`, which were already corrected. Add a small `useEffect` that, whenever the URL has a `page_size` value that doesn't match the resolved one, replaces the URL with the resolved value. Runs eagerly (no dependency on `data`) since the validity check is purely client-side, so the address bar matches the real page size before the first request even returns. Co-Authored-By: Claude Opus 4.7 (1M context) --- static/gsAdmin/views/invoiceComparison.tsx | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/static/gsAdmin/views/invoiceComparison.tsx b/static/gsAdmin/views/invoiceComparison.tsx index 8a2a9a4f9c9b6f..283317f1920419 100644 --- a/static/gsAdmin/views/invoiceComparison.tsx +++ b/static/gsAdmin/views/invoiceComparison.tsx @@ -211,6 +211,24 @@ export function InvoiceComparison() { }), }); + // 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. From 9796234b12508f8d0387220388bc84c707690f91 Mon Sep 17 00:00:00 2001 From: Andrew McKnight Date: Tue, 2 Jun 2026 15:42:25 -0800 Subject: [PATCH 6/8] fix(billing): Keep previous page visible while invoice comparison reloads MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address Cursor Bugbot finding: clicking Prev / Next (or changing the page-size dropdown) updated the React Query key, but the query didn't keep prior data while the next page fetched. The summary and both tables are gated on `{data && (…)}`, so the whole comparison view collapsed to the loading indicator on every page change — surprisingly janky for a paginator. Pass `placeholderData: keepPreviousData` so the previously-rendered page stays mounted until the new response arrives. Co-Authored-By: Claude Opus 4.7 (1M context) --- static/gsAdmin/views/invoiceComparison.tsx | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/static/gsAdmin/views/invoiceComparison.tsx b/static/gsAdmin/views/invoiceComparison.tsx index 283317f1920419..4cec28750c1499 100644 --- a/static/gsAdmin/views/invoiceComparison.tsx +++ b/static/gsAdmin/views/invoiceComparison.tsx @@ -1,7 +1,7 @@ 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'; @@ -209,6 +209,10 @@ export function InvoiceComparison() { : 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`), From 1dfd234604cc950135a1ca4fc111e68305d50e30 Mon Sep 17 00:00:00 2001 From: Andrew McKnight Date: Tue, 2 Jun 2026 15:48:58 -0800 Subject: [PATCH 7/8] fix(billing): Skip clamp effect against placeholder data on invoice comparison MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address Cursor Bugbot finding: with `placeholderData: keepPreviousData`, `data.summary` reflects the previously-rendered page while React Query loads the next one. The clamp effect that snaps the URL to the server-reported page therefore fired against stale summary data — every Prev / Next click would rewrite the URL back to the prior page, breaking pagination outright (and the same path broke clamped deep links once the first page loaded). Read `isPlaceholderData` from `useQuery` and bail out of the clamp effect while it's true, so the URL only converges on the server's authoritative page number once the new response actually lands. Co-Authored-By: Claude Opus 4.7 (1M context) --- static/gsAdmin/views/invoiceComparison.tsx | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/static/gsAdmin/views/invoiceComparison.tsx b/static/gsAdmin/views/invoiceComparison.tsx index 4cec28750c1499..a886e0e2252e67 100644 --- a/static/gsAdmin/views/invoiceComparison.tsx +++ b/static/gsAdmin/views/invoiceComparison.tsx @@ -193,7 +193,7 @@ export function InvoiceComparison() { }, [queryStart, queryEnd]); const enabled = Boolean(queryStart && queryEnd && region); - const {data, isPending, isError, error} = useQuery({ + const {data, isPending, isError, error, isPlaceholderData} = useQuery({ ...apiOptions.as()('/_admin/cells/$region/invoice-comparison/', { path: enabled && region ? {region: region.name} : skipToken, host: region?.url, @@ -236,8 +236,13 @@ export function InvoiceComparison() { // 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) { + if (!data || isPlaceholderData) { return; } const fixes: Record = {}; @@ -253,7 +258,15 @@ export function InvoiceComparison() { {replace: true} ); } - }, [data, rowsPage, unmatchedPage, navigate, location.pathname, location.query]); + }, [ + 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_* From bf577001db3b3666092894daaa97241c416d608f Mon Sep 17 00:00:00 2001 From: Andrew McKnight Date: Tue, 2 Jun 2026 16:21:03 -0800 Subject: [PATCH 8/8] ci(frontend): Filter and guard against empty jest --changedSince output Two related defensive fixes to the PR jest sharding path: - After `jest --listTests --json --changedSince= --passWithNoTests` returns, filter the parsed list down to actual test files (`*.spec.*`, `*.test.*`, or under `__tests__/`). On CI we've observed jest 30 returning non-test entries here (vendor JS, theme tokens, illustration components), which then get fed into `testMatch` and cause "Your test suite must contain at least one test" failures and dramatic slowdowns. - When the resolved `JEST_TESTS` (or assigned `testMatch`) is empty, return a no-match sentinel pattern so jest's `passWithNoTests` short-circuits cleanly. Previously `[] || defaultGlob` returned the empty array (arrays are truthy in JS), but the empty `testMatch` ended up triggering jest's own default discovery and running the whole suite anyway. Also adds a one-line log of the resolved test count when `MERGE_BASE` is set so future regressions in the changed-tests selection are visible at a glance. Co-Authored-By: Claude Opus 4.7 (1M context) --- jest.config.ts | 31 +++++++++++++++++++++++++++++-- 1 file changed, 29 insertions(+), 2 deletions(-) diff --git a/jest.config.ts b/jest.config.ts index a8fc9e61396882..f0e68a5aa0e935 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: [