From d101fef8ea4d285464d2a672d09d1a37d77307c3 Mon Sep 17 00:00:00 2001 From: Guillermo Marin <52298929+guillermoscript@users.noreply.github.com> Date: Sat, 25 Jul 2026 20:12:35 +0200 Subject: [PATCH] fix(payouts): stop the school payout history summing across currencies (#531) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The school-facing "Total paid out" card added by #499 reduced every paid payout into one number and rendered it with a hardcoded 'USD', so a school selling in more than one currency read an invented figure — the arithmetic sum of its dollar and euro payouts, denominated in dollars — sitting above per-row amounts that were each correct in their own currency. The super-admin page had already been fixed for exactly this in #497, but its `money()` and `formatByCurrency()` helpers were private to that page module. The invariant lived in one file's local scope, so the next payout screen had nothing to inherit and re-derived the bug. Move both helpers into lib/payments/format-money.ts alongside a `sumByCurrency()` that groups rows by their own currency (normalizing the code, so 'usd' and 'USD' share a bucket) and point both pages at it. The school card now renders one figure per currency and no longer names a currency the data didn't supply; the platform page keeps its existing output, now from shared code. Covered by tests/unit/format-money.test.ts, including the two-currency case from the issue. Display only — no query, schema, or RLS change. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Kgmz9Co2tW8KNfCFKLGEfN --- app/[locale]/dashboard/admin/payouts/page.tsx | 24 ++-- app/[locale]/platform/payouts/page.tsx | 27 ++--- lib/payments/format-money.ts | 66 +++++++++++ tests/unit/format-money.test.ts | 107 ++++++++++++++++++ 4 files changed, 193 insertions(+), 31 deletions(-) create mode 100644 lib/payments/format-money.ts create mode 100644 tests/unit/format-money.test.ts diff --git a/app/[locale]/dashboard/admin/payouts/page.tsx b/app/[locale]/dashboard/admin/payouts/page.tsx index e5f1474f..413c9a2d 100644 --- a/app/[locale]/dashboard/admin/payouts/page.tsx +++ b/app/[locale]/dashboard/admin/payouts/page.tsx @@ -4,6 +4,7 @@ import { getTranslations } from 'next-intl/server' import { format } from 'date-fns' import { es, enUS } from 'date-fns/locale' import { getCurrentTenantId, getCurrentUserId } from '@/lib/supabase/tenant' +import { formatByCurrency, formatMoney, sumByCurrency } from '@/lib/payments/format-money' import { AdminBreadcrumb } from '@/components/admin/admin-breadcrumb' import { Card, CardContent, CardHeader, CardTitle } from '@/components/ui/card' import { Badge } from '@/components/ui/badge' @@ -72,19 +73,13 @@ export default async function AdminPayoutsPage({ const rows = payouts || [] - // Summary calculations - const totalPaid = rows - .filter((p) => p.status === 'paid') - .reduce((sum, p) => sum + (p.amount || 0), 0) + // Summary calculations. Paid payouts are totalled per currency and rendered as + // one figure each — a school selling in USD and EUR reads two figures, not + // their sum in dollars (#531, same invariant #497 fixed on the platform page). + const totalPaidByCurrency = sumByCurrency(rows.filter((p) => p.status === 'paid')) const pendingCount = rows.filter((p) => p.status === 'pending' || p.status === 'processing').length - const fmt = (amount: number, currency: string) => - new Intl.NumberFormat(locale, { - style: 'currency', - currency: (currency || 'USD').toUpperCase(), - }).format(amount) - const statusBadge = (status: string) => { switch (status) { case 'paid': @@ -149,8 +144,11 @@ export default async function AdminPayoutsPage({

{t('stats.totalPaid')}

-

- {fmt(totalPaid, 'USD')} +

+ {formatByCurrency(totalPaidByCurrency, locale)}

{t('stats.totalPaidDesc')} @@ -232,7 +230,7 @@ export default async function AdminPayoutsPage({ : '—'} - {fmt(payout.amount || 0, payout.currency || 'USD')} + {formatMoney(payout.amount, payout.currency, locale)} {statusBadge(payout.status || '')} diff --git a/app/[locale]/platform/payouts/page.tsx b/app/[locale]/platform/payouts/page.tsx index 3fad3ede..bf9a45f5 100644 --- a/app/[locale]/platform/payouts/page.tsx +++ b/app/[locale]/platform/payouts/page.tsx @@ -3,6 +3,7 @@ import { getTranslations } from 'next-intl/server' import { getPayoutsOwed } from '@/app/actions/platform/payouts' import { Card, CardContent, CardHeader, CardTitle } from '@/components/ui/card' import { MarkPayoutPaidDialog } from '@/components/platform/mark-payout-paid-dialog' +import { formatByCurrency, formatMoney } from '@/lib/payments/format-money' import { IconArrowBackUp, IconCoin, @@ -11,19 +12,9 @@ import { IconWalletOff, } from '@tabler/icons-react' -/** - * Formatted in the reader's locale; the currency code always comes from the row - * itself — balances are grouped per currency and never converted (#497). - */ -const money = (n: number, currency: string, locale: string) => - new Intl.NumberFormat(locale, { style: 'currency', currency: currency.toUpperCase() }).format(n ?? 0) - -/** Renders one line per currency — amounts in different currencies are never summed into one number (#497). */ -function formatByCurrency(byCurrency: Record, locale: string) { - const entries = Object.entries(byCurrency).filter(([, amount]) => amount !== 0) - if (entries.length === 0) return money(0, 'usd', locale) - return entries.map(([currency, amount]) => money(amount, currency, locale)).join(' · ') -} +// `formatMoney` / `formatByCurrency` live in lib/payments/format-money.ts — the +// no-cross-currency-sums rule this page established in #497 is shared with the +// school-facing payout history rather than re-derived there (#531). const PROVIDER_LABEL: Record = { paypal: 'PayPal', @@ -291,24 +282,24 @@ export default async function PlatformPayoutsPage({ ? '—' : Object.keys(r.byProvider).map((p) => PROVIDER_LABEL[p] ?? p).join(', ')} - {money(r.grossCollected, r.currency, locale)} + {formatMoney(r.grossCollected, r.currency, locale)} {r.schoolPercentage}% - {money(r.alreadyPaid, r.currency, locale)} + {formatMoney(r.alreadyPaid, r.currency, locale)} - {money(r.netOwed, r.currency, locale)} + {formatMoney(r.netOwed, r.currency, locale)} diff --git a/lib/payments/format-money.ts b/lib/payments/format-money.ts new file mode 100644 index 00000000..9b983071 --- /dev/null +++ b/lib/payments/format-money.ts @@ -0,0 +1,66 @@ +/** + * Money formatting for the payout screens. + * + * The invariant these helpers exist to hold: **amounts in different currencies + * are never added together, and a currency code is never hardcoded at the + * render site.** #497 fixed that on the super-admin payouts page, but the fix + * lived in that page's local scope, so the school-facing payout history added + * later re-introduced the same bug (#531). Keeping the grouping and the + * formatting here — tested, in one place — is what stops the next payout screen + * from re-deriving it a third time. + * + * No conversion happens anywhere in this module. A USD figure and a EUR figure + * stay two figures. + */ + +/** The minimum shape a row needs to be groupable: an amount and the currency it is denominated in. */ +export interface MoneyRow { + amount: number | null + currency: string | null +} + +/** Totals keyed by upper-cased ISO currency code, e.g. `{ USD: 1240.5, EUR: 860 }`. */ +export type AmountsByCurrency = Record + +const DEFAULT_CURRENCY = 'USD' + +/** Upper-cases the code and falls back to USD, so `'usd'`, `'USD'` and a null column share one bucket. */ +const normalizeCurrency = (currency: string | null | undefined) => + (currency || DEFAULT_CURRENCY).toUpperCase() + +/** + * Formatted in the reader's locale; the currency code always comes from the + * data, never from the call site. + */ +export function formatMoney(amount: number | null | undefined, currency: string | null | undefined, locale: string) { + return new Intl.NumberFormat(locale, { + style: 'currency', + currency: normalizeCurrency(currency), + }).format(amount ?? 0) +} + +/** + * Groups rows into one total per currency. Callers filter first (e.g. to `paid` + * payouts) and pass what survives — this only ever adds an amount to the bucket + * for its own currency, so there is no arrangement of inputs that produces a + * cross-currency sum. + */ +export function sumByCurrency(rows: readonly MoneyRow[]): AmountsByCurrency { + const totals: AmountsByCurrency = {} + for (const row of rows) { + const currency = normalizeCurrency(row.currency) + totals[currency] = (totals[currency] ?? 0) + (row.amount ?? 0) + } + return totals +} + +/** + * Renders one figure per currency, joined — `"$1,240.50 · €860.00"`. Currencies + * that net to zero are dropped so the common single-currency school still reads + * as a single number; an empty set renders one zero rather than an empty card. + */ +export function formatByCurrency(byCurrency: AmountsByCurrency, locale: string) { + const entries = Object.entries(byCurrency).filter(([, amount]) => amount !== 0) + if (entries.length === 0) return formatMoney(0, DEFAULT_CURRENCY, locale) + return entries.map(([currency, amount]) => formatMoney(amount, currency, locale)).join(' · ') +} diff --git a/tests/unit/format-money.test.ts b/tests/unit/format-money.test.ts new file mode 100644 index 00000000..acba90c5 --- /dev/null +++ b/tests/unit/format-money.test.ts @@ -0,0 +1,107 @@ +import { describe, it, expect } from 'vitest' +import { formatByCurrency, formatMoney, sumByCurrency } from '@/lib/payments/format-money' + +/** + * The rule under test is the one #497 established and #531 found broken on the + * school-facing payout history: payout amounts are grouped per currency and + * never added across currencies, and the currency code always comes from the + * data. Formatting assertions use `toContain` on the digits plus a separate + * symbol check, so a change in ICU's spacing or grouping doesn't produce a false + * failure about arithmetic. + */ + +const rows = (...pairs: Array<[number | null, string | null]>) => + pairs.map(([amount, currency]) => ({ amount, currency })) + +describe('sumByCurrency', () => { + it('keeps each currency in its own bucket', () => { + expect(sumByCurrency(rows([1240.5, 'usd'], [860, 'eur']))).toEqual({ USD: 1240.5, EUR: 860 }) + }) + + it('sums multiple rows within one currency', () => { + expect(sumByCurrency(rows([100, 'usd'], [25.5, 'usd'], [10, 'eur']))).toEqual({ + USD: 125.5, + EUR: 10, + }) + }) + + it('treats currency codes case-insensitively so one currency never splits into two lines', () => { + expect(sumByCurrency(rows([100, 'usd'], [50, 'USD'], [1, 'Usd']))).toEqual({ USD: 151 }) + }) + + it('buckets a null currency as USD (the payouts.currency column default) rather than dropping the row', () => { + expect(sumByCurrency(rows([40, null], [60, 'usd']))).toEqual({ USD: 100 }) + }) + + it('treats a null amount as zero', () => { + expect(sumByCurrency(rows([null, 'usd'], [10, 'usd']))).toEqual({ USD: 10 }) + }) + + it('returns an empty object for no rows', () => { + expect(sumByCurrency([])).toEqual({}) + }) + + it('never produces a total larger than any single currency bucket (the #531 regression)', () => { + const totals = sumByCurrency(rows([1240.5, 'usd'], [860, 'eur'])) + // The bug summed these to 2100.5 and labelled it USD. No bucket may hold that. + expect(Object.values(totals)).not.toContain(2100.5) + expect(totals.USD).toBe(1240.5) + }) +}) + +describe('formatMoney', () => { + it('formats with the currency it is given, not a hardcoded one', () => { + expect(formatMoney(860, 'eur', 'en')).toContain('860') + expect(formatMoney(860, 'eur', 'en')).toContain('€') + expect(formatMoney(1240.5, 'usd', 'en')).toContain('$') + }) + + it('accepts lower-case codes', () => { + expect(formatMoney(10, 'gbp', 'en')).toContain('£') + }) + + it('falls back to USD for a missing currency and to zero for a missing amount', () => { + expect(formatMoney(10, null, 'en')).toContain('$') + expect(formatMoney(null, 'usd', 'en')).toContain('0') + }) + + it('respects the reader locale', () => { + // es-ES puts the symbol after the number; the point is that locale is honoured, + // not which side it lands on. + expect(formatMoney(1240.5, 'eur', 'es')).toContain('€') + }) +}) + +describe('formatByCurrency', () => { + it('renders one figure per currency instead of a single mixed number', () => { + const rendered = formatByCurrency({ USD: 1240.5, EUR: 860 }, 'en') + expect(rendered).toContain('$') + expect(rendered).toContain('€') + expect(rendered).toContain('1,240.50') + expect(rendered).toContain('860') + expect(rendered).not.toContain('2,100.50') + }) + + it('renders a single-currency school as one plain figure', () => { + const rendered = formatByCurrency({ USD: 1240.5 }, 'en') + expect(rendered).toContain('$1,240.50') + expect(rendered).not.toContain('·') + }) + + it('drops currencies that net to zero', () => { + const rendered = formatByCurrency({ USD: 100, EUR: 0 }, 'en') + expect(rendered).toContain('$100') + expect(rendered).not.toContain('€') + }) + + it('renders one zero rather than an empty card when there is nothing to show', () => { + expect(formatByCurrency({}, 'en')).toContain('0') + expect(formatByCurrency({ USD: 0, EUR: 0 }, 'en')).toContain('0') + }) + + it('composes with sumByCurrency end to end for the #531 case', () => { + const paid = rows([1240.5, 'usd'], [860, 'eur']) + const rendered = formatByCurrency(sumByCurrency(paid), 'en') + expect(rendered).toBe('$1,240.50 · €860.00') + }) +})