tyler/cyclone
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

feat(sp27): server-aggregate Remittances KPIs (count, paid, adjustments)

Browse Source 
The Remittances page's three KPI tiles — REMITS / TOTAL PAID /
ADJUSTMENTS — were computed page-locally via items.reduce(...) over
the merged tail of the current page + live delta. With a 100-row
default limit, a 1,739-row population showed count=100, paid=$16,934,
adjustments=$147 — silently understating reality because the page
hadn't loaded the remaining rows yet.

This change mirrors the silent-incompleteness fix that
/api/dashboard/kpis (commit 59c3275) and /api/remittances (commit
d81b6ed) made for their tiles:

  * CycloneStore.summarize_remittances() iterates the full filtered
    remittance population (no limit) and returns
    {count, total_paid, total_adjustments}. Mirrors iter_remittances
    with limit=_ITER_UNBOUNDED.
  * GET /api/remittances/summary — server endpoint with the same
    filter parameters as /api/remittances. Registered BEFORE the
    /api/remittances/stream handler so FastAPI doesn't treat
    'summary' as a stream sub-path.
  * api.listRemittanceSummary + useRemittanceSummary hook.
  * Remittances.tsx swaps off items.reduce, consumes the server
    summary. Tiles render the server totals so the values reflect
    the entire DB population, not the page-local sample.

Verified live: /api/remittances/summary returns
{count: 1739, total_paid: 227181.58, total_adjustments: 13792.65},
which matches DB ground truth exactly.

Tests: 8 new backend tests in test_api_remittances_summary.py; 1 new
frontend test in Remittances.test.tsx (kpi_tiles_use_server_summary_not
_page_local_reduce) plus the page-level test for the zero-valued mock
default.
This commit is contained in:
Nora
2026-06-29 14:16:24 -06:00
parent d8841834dc
commit 14fcbca5f1
7 changed files with 515 additions and 11 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/pages/Remittances.test.tsx
+76
View File
@@ -20,6 +20,7 @@ vi.mock("@/lib/api", () => ({
api: {
isConfigured: true,
listRemittances: vi.fn(),
listRemittanceSummary: vi.fn(),
getRemittance: vi.fn(),
},
ApiError: class ApiError extends Error {
@@ -224,6 +225,17 @@ describe("Remittances", () => {
returned: SAMPLE_REMITS.length,
has_more: false,
});
// Default summary mock so the page-level KPI tiles don't render
// `undefined`. Tests that care about specific summary values
// override this per-test (e.g. ``test_kpi_tiles_use_server_...
// _not_page_local_reduce``).
(
api.listRemittanceSummary as unknown as ReturnType<typeof vi.fn>
).mockResolvedValue({
count: 0,
total_paid: 0,
total_adjustments: 0,
});
// Default for the per-remit detail fetch — the drawer fetches
// this whenever `?remit=` is in the URL. Return a never-resolving
// promise so the drawer stays in the loading state; the smoke
@@ -487,4 +499,68 @@ describe("Remittances", () => {
unmount();
});
// -------------------------------------------------------------------
// KPI tiles must reflect the full DB population, NOT the page-local
// reduce over the visible rows. The bug repro is a 25-row page
// whose `items.reduce(...)` summed to $25.96 paid / $67.06
// adjustments while the real DB held $227,181.58 / $13,792.65
// across 1,739 rows. The "REMITS" tile already sources its count
// from the fixed `count_remittances` helper (commit d81b6ed).
// "TOTAL PAID" + "ADJUSTMENTS" must come from the new server-side
// summary endpoint (`/api/remittances/summary`) via
// `api.listRemittanceSummary` so they can't silently understate
// the true population. Mirrors the Dashboard silent-failure fix
// (commit 59c3275).
// -------------------------------------------------------------------
it("test_kpi_tiles_use_server_summary_not_page_local_reduce", async () => {
// Page sum over the visible 3 rows: paid=$525.96 (100+200+300-74.04),
// adjustments=$60 (60+0+0). Server ground truth (mocked): count=1739,
// paid=$227,181, adjustments=$13,792 — whole-dollar amounts chosen
// so `fmt.usd`'s maximumFractionDigits=0 doesn't round them and the
// assertion is unambiguous. The page must show the SERVER values,
// not the page sums.
(
api.listRemittanceSummary as unknown as ReturnType<typeof vi.fn>
).mockResolvedValue({
count: 1739,
total_paid: 227181,
total_adjustments: 13792,
});
const { unmount } = renderIntoContainer(React.createElement(Remittances));
await waitForText("PCN-1");
const body = document.body.textContent ?? "";
// The server totals must show on the tiles.
expect(body).toContain("$227,181");
expect(body).toContain("$13,792");
// Server count wins over the 3-row page total.
expect(body).toContain("1,739");
// Locate the KPI tile for "Total paid" specifically. The tiles
// share the `.display.mono.text-[28px]` class with several other
// page elements, so we walk by `.eyebrow` to find each one.
const totalPaidTile = Array.from(
document.querySelectorAll(".surface-2"),
).find((el) => el.querySelector(".eyebrow")?.textContent === "Total paid");
expect(totalPaidTile).toBeDefined();
const paidValue = totalPaidTile?.querySelector(
".display.mono.text-\\[28px\\]",
);
expect(paidValue?.textContent).toBe("$227,181");
const adjustmentsTile = Array.from(
document.querySelectorAll(".surface-2"),
).find(
(el) => el.querySelector(".eyebrow")?.textContent === "Adjustments",
);
expect(adjustmentsTile).toBeDefined();
const adjValue = adjustmentsTile?.querySelector(
".display.mono.text-\\[28px\\]",
);
expect(adjValue?.textContent).toBe("$13,792");
unmount();
});
});
src/pages/Remittances.tsx
+18 -10
View File
@@ -18,6 +18,7 @@ import { PageHeader } from "@/components/PageHeader";
import { TailStatusPill } from "@/components/TailStatusPill";
import { RemitDrawer } from "@/components/RemitDrawer";
import { useRemittances } from "@/hooks/useRemittances";
import { useRemittanceSummary } from "@/hooks/useRemittanceSummary";
import { useRemitDrawerUrlState } from "@/hooks/useRemitDrawerUrlState";
import { useRowKeyboard } from "@/hooks/useRowKeyboard";
import { useTailStream } from "@/hooks/useTailStream";
@@ -67,13 +68,20 @@ export function Remittances() {
useTailStream("remittances");
const items = useMergedTail("remittances", data?.items ?? [], tailFilterFn);
const total = items.reduce(
(acc, r) => ({
paid: acc.paid + r.paidAmount,
adjustments: acc.adjustments + r.adjustmentAmount,
}),
{ paid: 0, adjustments: 0 }
);
// KPI totals come from the server-aggregated summary endpoint over
// the FULL remittance population — NOT a page-local reduce over the
// current page + live-tail delta. The same silent-incompleteness
// pattern that `59c3275` (Dashboard) and `d81b6ed` (count tile)
// retired applies to the financial tiles; summing visible rows
// understates the true totals and the page looks complete. The
// server returns zero-filled values when the DB has no rows so the
// tiles render predictably while the initial fetch resolves.
//
// We don't pass the page's status chip here — the list endpoint
// (``useRemittances``) ignores status server-side and applies it
// client-side via the merged-tail filter, so passing it to the
// summary would inconsistently narrow only one of the two reads.
const { data: summary } = useRemittanceSummary();
const moveNext = useCallback(() => {
setSelectedIndex((i) => {
@@ -154,19 +162,19 @@ export function Remittances() {
<div className="surface-2 rounded-xl p-5">
<div className="eyebrow">Remits</div>
<div className="display mono text-[28px] leading-none mt-3 text-foreground">
{fmt.num(data?.total ?? 0)}
{fmt.num(summary?.count ?? data?.total ?? 0)}
</div>
</div>
<div className="surface-2 rounded-xl p-5">
<div className="eyebrow">Total paid</div>
<div className="display mono text-[28px] leading-none mt-3 text-[hsl(var(--success))]">
{fmt.usd(total.paid)}
{fmt.usd(summary?.total_paid ?? 0)}
</div>
</div>
<div className="surface-2 rounded-xl p-5">
<div className="eyebrow">Adjustments</div>
<div className="display mono text-[28px] leading-none mt-3 text-[hsl(var(--warning))]">
{fmt.usd(total.adjustments)}
{fmt.usd(summary?.total_adjustments ?? 0)}
</div>
</div>
</div>