From dddffeee1f73340450cacb6f54ea74f04d684c6d Mon Sep 17 00:00:00 2001 From: Andrew McKnight Date: Wed, 17 Jun 2026 16:38:13 -0800 Subject: [PATCH 1/6] feat(gsAdmin): rework invoice-comparison summary + link receipt cells Reshapes the admin invoice-comparison summary to lead with the parity metric operators actually scan for: the share of compared (both-sided) orgs whose per-org delta exceeds 1%. The Grid loses Legacy total, Platform total, Total delta, and Rows (each either redundant with the per-row data or duplicated by the paginator) so the headline ">1% diff" stat has room to breathe alongside the existing Unmatched and invoice-count stats; 7 cards collapses to 4. Per-row, the Legacy and Platform `$` amount cells become `Link`s to `/settings/{slug}/billing/receipts/{guid}/` whenever the row resolves to exactly one invoice on that side (the typical case), using the new `legacy_invoice_guids` / `platform_invoice_guids` fields the backend companion change adds. Multi-invoice rows fall back to plain text -- a single link can't represent N receipts and the count badge already signals the cardinality. Backend companion: getsentry feat(billing) commit, same Refs. Refs REVENG-221 Co-Authored-By: Claude Opus 4.7 (1M context) --- static/gsAdmin/views/invoiceComparison.tsx | 71 +++++++++++----------- 1 file changed, 37 insertions(+), 34 deletions(-) diff --git a/static/gsAdmin/views/invoiceComparison.tsx b/static/gsAdmin/views/invoiceComparison.tsx index 47c1fa5013fc22..eb839388425d78 100644 --- a/static/gsAdmin/views/invoiceComparison.tsx +++ b/static/gsAdmin/views/invoiceComparison.tsx @@ -29,19 +29,21 @@ type Row = { delta_pct: number | null; legacy_amount: number | null; legacy_invoice_count: number; + legacy_invoice_guids: string[]; organization_id: number; organization_slug: string | null; platform_amount: number | null; platform_invoice_count: number; + platform_invoice_guids: string[]; status: RowStatus; }; type Summary = { end: string; legacy_count: number; - legacy_total_cents: number; + over_threshold_count: number; + over_threshold_pct: number; platform_count: number; - platform_total_cents: number; queried_at: string; row_count: number; rows_page: number; @@ -87,6 +89,22 @@ function formatDollars(cents: number | null) { return `$${dollars.toLocaleString(undefined, {minimumFractionDigits: 2, maximumFractionDigits: 2})}`; } +function receiptUrl(orgSlug: string, guid: string) { + return `/settings/${orgSlug}/billing/receipts/${guid}/`; +} + +// Render the $ amount as a link to the receipt details page when we can +// pick a single invoice to point at. Multiple-invoice rows fall back to +// plain text — the count badge next to the cell already signals there's +// more than one, and per-invoice drill-down isn't a common workflow. +function renderAmountCell(cents: number | null, guids: string[], orgSlug: string | null) { + const dollars = formatDollars(cents); + if (!orgSlug || guids.length !== 1) { + return dollars; + } + return {dollars}; +} + function formatPercent(pct: number | null) { if (pct === null) { // No legacy baseline — sorts to top of the list. @@ -405,11 +423,11 @@ export function InvoiceComparison() { Summary @@ -431,36 +449,13 @@ export function InvoiceComparison() { - Legacy total + {'>1% diff'} - {formatDollars(data.summary.legacy_total_cents)} - - - - - Platform total - - - {formatDollars(data.summary.platform_total_cents)} - - - - - Total delta - - - {formatDollars( - data.summary.legacy_total_cents - data.summary.platform_total_cents - )} - - - - - Rows - - - {data.summary.row_count} + {formatPercent(data.summary.over_threshold_pct)} + + ({data.summary.over_threshold_count} of {data.summary.row_count}) + @@ -522,13 +517,21 @@ export function InvoiceComparison() { )} - {formatDollars(row.legacy_amount)}{' '} + {renderAmountCell( + row.legacy_amount, + row.legacy_invoice_guids, + row.organization_slug + )}{' '} ({row.legacy_invoice_count}) - {formatDollars(row.platform_amount)}{' '} + {renderAmountCell( + row.platform_amount, + row.platform_invoice_guids, + row.organization_slug + )}{' '} ({row.platform_invoice_count}) From cf2f4d8b229967f8f6e7afab1a083008827c4c63 Mon Sep 17 00:00:00 2001 From: Andrew McKnight Date: Thu, 18 Jun 2026 11:58:36 -0800 Subject: [PATCH 2/6] fix(gsAdmin): guard renderAmountCell against missing *_invoice_guids MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Row / UnmatchedRow types declare ``*_invoice_guids`` as ``string[]`` based on the post-deploy backend shape, but a stale-cached response from before the backend companion shipped (or any future shape drift that drops the field) leaves ``guids`` runtime-``undefined``. The old ``guids.length !== 1`` check then threw ``TypeError`` on ``undefined.length``, crashing the page. Widen the parameter to ``string[] | undefined`` and replace the bare ``.length`` access with optional chaining so the cell renders as plain text in those cases — same fallback the multi-invoice branch already uses. Caught by Sentry's PR review bot at #117970 (comment). Refs REVENG-221 Co-Authored-By: Claude Opus 4.7 (1M context) --- static/gsAdmin/views/invoiceComparison.tsx | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/static/gsAdmin/views/invoiceComparison.tsx b/static/gsAdmin/views/invoiceComparison.tsx index f7acf2153087de..fb06142dd8a142 100644 --- a/static/gsAdmin/views/invoiceComparison.tsx +++ b/static/gsAdmin/views/invoiceComparison.tsx @@ -102,9 +102,18 @@ function receiptUrl(orgSlug: string, guid: string) { // Uses a plain `` (not router ``) because gsAdmin is a separate // app bundle from the org-facing settings UI; the receipts page lives in // the latter, so navigating there has to be a full page load. -function renderAmountCell(cents: number | null, guids: string[], orgSlug: string | null) { +// +// `guids` is widened to allow `undefined` so that a stale-cached response +// from before the backend companion shipped (or any future shape drift +// that drops the field) renders as plain text rather than crashing the +// page on `undefined.length`. +function renderAmountCell( + cents: number | null, + guids: string[] | undefined, + orgSlug: string | null +) { const dollars = formatDollars(cents); - if (!orgSlug || guids.length !== 1) { + if (!orgSlug || guids?.length !== 1) { return dollars; } return {dollars}; From ec7bac46e3e6667e23b07c4e307c0b696156079a Mon Sep 17 00:00:00 2001 From: Andrew McKnight Date: Thu, 18 Jun 2026 12:41:01 -0800 Subject: [PATCH 3/6] review(gsAdmin): narrow PR to just the >1% diff stat card MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per the getsentry backend revert (companion PR): drop the ``renderAmountCell`` helper and the ``*_invoice_guids`` plural type fields; the singular ``*_invoice_guid`` fields and master's ``InvoiceAmount`` component are sufficient since rows have at most one invoice per side in practice. Restoring the original 7-card summary Grid (Legacy invoices, Platform invoices, Legacy total, Platform total, Total delta, Rows, Unmatched) and adding just the new ">1% diff" card → 8 cards. The totals + delta + rows cards aren't being dropped after all; that reshape was out of scope for this PR. What's left in this PR: the ">1% diff" stat card. Nothing else. Refs REVENG-221 Co-Authored-By: Claude Opus 4.7 (1M context) --- static/gsAdmin/views/invoiceComparison.tsx | 124 +++++++++++++-------- 1 file changed, 78 insertions(+), 46 deletions(-) diff --git a/static/gsAdmin/views/invoiceComparison.tsx b/static/gsAdmin/views/invoiceComparison.tsx index fb06142dd8a142..ab6a6ae85ad7f0 100644 --- a/static/gsAdmin/views/invoiceComparison.tsx +++ b/static/gsAdmin/views/invoiceComparison.tsx @@ -27,23 +27,27 @@ type RowStatus = 'match' | 'mismatch' | 'legacy_only' | 'platform_only'; type Row = { delta_cents: number; delta_pct: number | null; + // guid is present only when the side has exactly one invoice in the window + // (otherwise there's no single invoice to deep-link to). legacy_amount: number | null; legacy_invoice_count: number; - legacy_invoice_guids: string[]; + legacy_invoice_guid: string | null; organization_id: number; organization_slug: string | null; platform_amount: number | null; platform_invoice_count: number; - platform_invoice_guids: string[]; + platform_invoice_guid: string | null; status: RowStatus; }; type Summary = { end: string; legacy_count: number; + legacy_total_cents: number; over_threshold_count: number; over_threshold_pct: number; platform_count: number; + platform_total_cents: number; queried_at: string; row_count: number; rows_page: number; @@ -63,7 +67,8 @@ type UnmatchedSide = 'legacy_only' | 'platform_only'; type UnmatchedRow = { amount: number; invoice_count: number; - invoice_guids: string[]; + // Present only when the org has exactly one invoice on its one side. + invoice_guid: string | null; organization_id: number; organization_slug: string | null; side: UnmatchedSide; @@ -90,33 +95,26 @@ function formatDollars(cents: number | null) { return `$${dollars.toLocaleString(undefined, {minimumFractionDigits: 2, maximumFractionDigits: 2})}`; } -function receiptUrl(orgSlug: string, guid: string) { - return `/settings/${orgSlug}/billing/receipts/${guid}/`; -} - -// Render the $ amount as a link to the receipt details page when we can -// pick a single invoice to point at. Multiple-invoice rows fall back to -// plain text — the count badge next to the cell already signals there's -// more than one, and per-invoice drill-down isn't a common workflow. -// -// Uses a plain `` (not router ``) because gsAdmin is a separate -// app bundle from the org-facing settings UI; the receipts page lives in -// the latter, so navigating there has to be a full page load. -// -// `guids` is widened to allow `undefined` so that a stale-cached response -// from before the backend companion shipped (or any future shape drift -// that drops the field) renders as plain text rather than crashing the -// page on `undefined.length`. -function renderAmountCell( - cents: number | null, - guids: string[] | undefined, - orgSlug: string | null -) { - const dollars = formatDollars(cents); - if (!orgSlug || guids?.length !== 1) { - return dollars; +// Render a dollar amount, deep-linking to the org-scoped invoice detail page +// when we have both the org slug and a single invoice's guid. The receipts +// page (CustomerInvoiceDetailsEndpoint) resolves legacy and platform invoices +// alike by guid. This is a plain anchor — not a router Link — because gsAdmin +// is a separate app bundle from the org-facing settings UI, so navigating +// there is a full page load (see the cross-app links in dataRequests.tsx). +function InvoiceAmount({ + cents, + guid, + orgSlug, +}: { + cents: number | null; + guid: string | null; + orgSlug: string | null; +}) { + const amount = formatDollars(cents); + if (!guid || !orgSlug) { + return amount; } - return {dollars}; + return {amount}; } function formatPercent(pct: number | null) { @@ -437,11 +435,11 @@ export function InvoiceComparison() { Summary @@ -461,6 +459,40 @@ export function InvoiceComparison() { {data.summary.platform_count} + + + Legacy total + + + {formatDollars(data.summary.legacy_total_cents)} + + + + + Platform total + + + {formatDollars(data.summary.platform_total_cents)} + + + + + Total delta + + + {formatDollars( + data.summary.legacy_total_cents - data.summary.platform_total_cents + )} + + + + + Rows + + + {data.summary.row_count} + + {'>1% diff'} @@ -531,21 +563,21 @@ export function InvoiceComparison() { )} - {renderAmountCell( - row.legacy_amount, - row.legacy_invoice_guids, - row.organization_slug - )}{' '} + {' '} ({row.legacy_invoice_count}) - {renderAmountCell( - row.platform_amount, - row.platform_invoice_guids, - row.organization_slug - )}{' '} + {' '} ({row.platform_invoice_count}) @@ -604,11 +636,11 @@ export function InvoiceComparison() { {row.side} - {renderAmountCell( - row.amount, - row.invoice_guids, - row.organization_slug - )} + {row.invoice_count} From 434738bad2036a7c2d18a365722551a6b4c49cd6 Mon Sep 17 00:00:00 2001 From: Andrew McKnight Date: Thu, 18 Jun 2026 12:47:34 -0800 Subject: [PATCH 4/6] feat(gsAdmin): drop Legacy total / Platform total / Total delta / Rows cards MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per the original REVENG-221 ticket scope: "get rid of legacy total and platform total numbers" + "remove rows count because it's == platform invoices number". The previous review-feedback commit over-reverted these on the UI side — Noah's "don't refactor other stuff" was about the backend guid plumbing, not the summary card removal, which was always part of REVENG-221's display intent. Backend still emits ``legacy_total_cents`` / ``platform_total_cents`` for compatibility with the deployed master frontend; this PR's frontend just stops displaying them. Grid shrinks from 8 cards to 4: Legacy invoices, Platform invoices, >1% diff, Unmatched. Refs REVENG-221 Co-Authored-By: Claude Opus 4.7 (1M context) --- static/gsAdmin/views/invoiceComparison.tsx | 40 ++-------------------- 1 file changed, 2 insertions(+), 38 deletions(-) diff --git a/static/gsAdmin/views/invoiceComparison.tsx b/static/gsAdmin/views/invoiceComparison.tsx index ab6a6ae85ad7f0..36551cc8adf935 100644 --- a/static/gsAdmin/views/invoiceComparison.tsx +++ b/static/gsAdmin/views/invoiceComparison.tsx @@ -43,11 +43,9 @@ type Row = { type Summary = { end: string; legacy_count: number; - legacy_total_cents: number; over_threshold_count: number; over_threshold_pct: number; platform_count: number; - platform_total_cents: number; queried_at: string; row_count: number; rows_page: number; @@ -435,11 +433,11 @@ export function InvoiceComparison() { Summary @@ -459,40 +457,6 @@ export function InvoiceComparison() { {data.summary.platform_count} - - - Legacy total - - - {formatDollars(data.summary.legacy_total_cents)} - - - - - Platform total - - - {formatDollars(data.summary.platform_total_cents)} - - - - - Total delta - - - {formatDollars( - data.summary.legacy_total_cents - data.summary.platform_total_cents - )} - - - - - Rows - - - {data.summary.row_count} - - {'>1% diff'} From 457ed46d1d6374438f39ff42bce891b0df0d40e2 Mon Sep 17 00:00:00 2001 From: Andrew McKnight Date: Thu, 18 Jun 2026 12:54:26 -0800 Subject: [PATCH 5/6] =?UTF-8?q?fix(gsAdmin):=20render=20summary-stat=20nul?= =?UTF-8?q?l=20as=20N/A,=20not=20``=E2=88=9E``?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ``formatPercent`` returns the infinity symbol for a ``null`` input because that's meaningful for ``delta_pct`` rows — legacy=\$0 with non-zero platform is "undefined drift" and sorts to the top of the list. That semantic doesn't carry to summary ratios like ``over_threshold_pct`` / ``unmatched_invoice_pct``: a runtime ``null``/``undefined`` there only means the backend didn't populate the field (deploy-window race, response-shape drift), not "drift went to infinity". Rendering ``∞`` in those cells is misleading. Adds ``formatPercentOrNA`` for the summary case (N/A on missing, otherwise same percentage formatter as before). The existing ``formatPercent`` keeps the ``∞`` behavior for ``delta_pct`` rows. Caught by Sentry's PR review bot at #117970 (comment). Refs REVENG-221 Co-Authored-By: Claude Opus 4.7 (1M context) --- static/gsAdmin/views/invoiceComparison.tsx | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/static/gsAdmin/views/invoiceComparison.tsx b/static/gsAdmin/views/invoiceComparison.tsx index 36551cc8adf935..8508a7eef0aaad 100644 --- a/static/gsAdmin/views/invoiceComparison.tsx +++ b/static/gsAdmin/views/invoiceComparison.tsx @@ -123,6 +123,20 @@ function formatPercent(pct: number | null) { return `${(pct * 100).toLocaleString(undefined, {minimumFractionDigits: 1, maximumFractionDigits: 1})}%`; } +// `formatPercent` renders ``null`` as ``∞`` because for ``delta_pct`` rows +// a missing percentage means "undefined drift" (legacy=$0 with non-zero +// platform — sorts to the top of the list). That semantic doesn't apply +// to summary ratios like ``over_threshold_pct`` / ``unmatched_invoice_pct``, +// where a runtime ``null``/``undefined`` would just mean the backend +// didn't populate the field (deploy-window race, response-shape drift). +// Render those as ``N/A`` instead of pretending the metric blew up. +function formatPercentOrNA(pct: number | null | undefined) { + if (pct === null || pct === undefined) { + return N/A; + } + return `${(pct * 100).toLocaleString(undefined, {minimumFractionDigits: 1, maximumFractionDigits: 1})}%`; +} + // `datetime-local` inputs use the user's local timezone with no offset // in the string (e.g. "2026-05-26T22:30"). Format a Date for that field. function toDatetimeLocalValue(d: Date): string { @@ -462,7 +476,7 @@ export function InvoiceComparison() { {'>1% diff'} - {formatPercent(data.summary.over_threshold_pct)} + {formatPercentOrNA(data.summary.over_threshold_pct)} ({data.summary.over_threshold_count} of {data.summary.row_count}) @@ -473,7 +487,7 @@ export function InvoiceComparison() { Unmatched - {formatPercent(data.summary.unmatched_invoice_pct)} + {formatPercentOrNA(data.summary.unmatched_invoice_pct)} ({data.summary.unmatched_invoice_count} of{' '} {data.summary.legacy_count + data.summary.platform_count}) From 6d26b0f74f93f7f86ea3dda5014ab3644c4c71c1 Mon Sep 17 00:00:00 2001 From: Andrew McKnight Date: Thu, 18 Jun 2026 13:05:42 -0800 Subject: [PATCH 6/6] fix(gsAdmin): guard summary counts against ``undefined`` at render time MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Summary type declares counts (``legacy_count``, ``platform_count``, ``over_threshold_count``, ``row_count``, ``unmatched_invoice_count``) as ``number``, but a deploy-window race or response-shape drift could leave them runtime-``undefined`` and render the literal string ``undefined`` in the UI (or ``NaN`` for the Unmatched denominator, which sums two counts). Adds ``formatCountOrNA`` — same defensive philosophy as the ``formatPercentOrNA`` helper added in the previous commit — and routes every count interpolation in the summary Grid through it. Falls back to an em dash on null / undefined / NaN, matching the missing-value affordance ``formatDollars`` already uses. Caught by Sentry's PR review bot at #117970 (comment). Refs REVENG-221 Co-Authored-By: Claude Opus 4.7 (1M context) --- static/gsAdmin/views/invoiceComparison.tsx | 29 ++++++++++++++++++---- 1 file changed, 24 insertions(+), 5 deletions(-) diff --git a/static/gsAdmin/views/invoiceComparison.tsx b/static/gsAdmin/views/invoiceComparison.tsx index 8508a7eef0aaad..eb724538c4f11c 100644 --- a/static/gsAdmin/views/invoiceComparison.tsx +++ b/static/gsAdmin/views/invoiceComparison.tsx @@ -137,6 +137,21 @@ function formatPercentOrNA(pct: number | null | undefined) { return `${(pct * 100).toLocaleString(undefined, {minimumFractionDigits: 1, maximumFractionDigits: 1})}%`; } +// Same deploy-window-race rationale as `formatPercentOrNA`: the type +// declares these as `number` but a stale-cached response or any future +// shape drift could leave them runtime-`undefined`, which would render +// the literal string "undefined" in the UI. NaN is also caught — it +// shows up when arithmetic propagates an undefined operand (e.g. +// `legacy_count + platform_count` for the Unmatched denominator). +// Falling back to an em dash matches the missing-value affordance +// `formatDollars` already uses. +function formatCountOrNA(n: number | null | undefined) { + if (n === null || n === undefined || Number.isNaN(n)) { + return —; + } + return n.toLocaleString(); +} + // `datetime-local` inputs use the user's local timezone with no offset // in the string (e.g. "2026-05-26T22:30"). Format a Date for that field. function toDatetimeLocalValue(d: Date): string { @@ -460,7 +475,7 @@ export function InvoiceComparison() { Legacy invoices - {data.summary.legacy_count} + {formatCountOrNA(data.summary.legacy_count)} @@ -468,7 +483,7 @@ export function InvoiceComparison() { Platform invoices - {data.summary.platform_count} + {formatCountOrNA(data.summary.platform_count)} @@ -478,7 +493,8 @@ export function InvoiceComparison() { {formatPercentOrNA(data.summary.over_threshold_pct)} - ({data.summary.over_threshold_count} of {data.summary.row_count}) + ({formatCountOrNA(data.summary.over_threshold_count)} of{' '} + {formatCountOrNA(data.summary.row_count)}) @@ -489,8 +505,11 @@ export function InvoiceComparison() { {formatPercentOrNA(data.summary.unmatched_invoice_pct)} - ({data.summary.unmatched_invoice_count} of{' '} - {data.summary.legacy_count + data.summary.platform_count}) + ({formatCountOrNA(data.summary.unmatched_invoice_count)} of{' '} + {formatCountOrNA( + data.summary.legacy_count + data.summary.platform_count + )} + )