Skip to content

NWP-101: add export options - #164

Open
kumarjith wants to merge 2 commits into
JJFromTenex:mainfrom
kumarjith:NWP-101-export-options
Open

NWP-101: add export options#164
kumarjith wants to merge 2 commits into
JJFromTenex:mainfrom
kumarjith:NWP-101-export-options

Conversation

@kumarjith

Copy link
Copy Markdown

Ticket

Closes NWP-101

What changed

The payments export was fixed: every column, current filter only. The card last four shipped in every file, so anything going to a merchant had to be cleaned up by hand first. The Export button on /payments now opens an options dialog. Ops picks which columns are included, with the card last four off by default, and picks whether to export the current filter or all payments, with the row count for both shown before download. The filename names the scope, and column names are validated against an allowlist on the server rather than trusted from the query string. Both scopes run through the existing query builder behind GET /api/payments, so an export is never limited to the page the table happens to be showing.

How I verified it

npm test37 passed (37), 3 files. The baseline before this change was 28 passed; the 9 new tests are all in src/lib/csv.test.ts. npx next lint reported no warnings or errors, and npx tsc --noEmit was clean.

Against the dev server with curl:

Request What came back
?columns=nope 400 with {"message":"Select at least one column to export."}
no columns param header id,created_at,merchant,description,status,method,card_brand,amount,currency — no last4
?columns=amount,currency,id those three, in that order; first row $473.78,USD,pay_001644
?status=disputed content-disposition filename payments-disputed-2026-09-10.csv
?status=disputed&scope=all filename payments-all-2026-09-10.csv
?search=order filename payments-filtered-2026-09-10.csv
?status=disputed&columns=id vs same with &scope=all 32 data rows vs 1657 data rows

In the browser on /payments?status=disputed:

  • Clicked Export. The dialog opened with the heading "Export payments".

  • Scope showed "Current filter (33 rows)" selected and "All payments (1,658 rows)" unselected.

  • In the Columns list, "Card last four" was the only unchecked box; the other nine were checked.

  • Unchecked all nine remaining boxes. The message "Select at least one column to export." appeared under the list, the footer read "33 rows · 0 columns", and Download rendered as a disabled button rather than a link.

  • Rechecked Status then Payment ID. The Download link href read /api/payments/export?status=disputed&columns=id%2Cstatus&scope=filtered — the canonical column order, not the order clicked.

  • Selected "All payments". The href changed to /api/payments/export?columns=id%2Cstatus&scope=all, dropping the filter param.

  • Pressed Escape. The dialog closed.

  • npm test passes

  • New behavior is covered by a test

  • Checked it in the browser

Acceptance criteria

  • Ops can choose which columns are included. Card last-four is off by default.
  • Ops can choose scope: current filter or all payments. Current filter is the default, and the row count is visible before download.
  • The filename reflects the scope and the date, for example payments-disputed-2026-08-13.csv.
  • Amounts stay in minor units internally and are formatted once on the way out, with currency in its own column.
  • Deselecting every column disables Download rather than producing an empty file.

Bugs fixed along the way

None fixed. Three found and deliberately left, all pre-existing and all outside this ticket:

  • src/data/metrics.ts:25 — the bucket keys on line 18 come from lastUtcDays() and are UTC, but payments are bucketed with new Date(payment.createdAt).toLocaleDateString("en-CA"), which is server local time. On a host west of UTC, late-evening payments land in the wrong key or match no bucket and hit the continue on line 27, dropping them from the volume chart. utcDayKey in src/lib/dates.ts is the fix.
  • src/data/metrics.ts:31,34 — money is accumulated as a float in major units (bucket.captured += payment.amount / 100) and re-multiplied by Math.round(bucket.captured * 100) on report. The rounding hides drift rather than preventing it; summing the integer minor units needs no rounding at all.
  • src/data/queries.ts:81sortPayments compares amounts as strings via String(a.amount).localeCompare(String(b.amount)), so 9000 sorts before 25000. The export reuses sortPayments and inherits it.

I left the sort bug alone rather than changing the ordering of every existing export in the same pull request that changes its columns.

Notes for the reviewer

  • Drawer, not Dialog. .claude/rules/components.md states that src/components/ has a Dialog component. It does not. Drawer.tsx is the Radix Dialog wrapper that exists, and it had no consumers anywhere in the app before this change. Either the rule file or the component inventory needs correcting.
  • Native checkbox and radio inputs. There is no Checkbox component and @radix-ui/react-checkbox is not a dependency, so the dialog uses native inputs styled with Tailwind rather than adding a dependency. Each has an htmlFor label and both groups are wrapped in a fieldset with a legend.
  • The 400 is the enforcement, not the disabled button. parseExportColumns returning an empty array is what refuses the request; the disabled Download is a convenience. Unknown column names are dropped rather than erroring, so a stale bookmark degrades to fewer columns instead of failing outright.
  • Scope all agrees from both directions. The dialog drops the filter params when scope is all, and the route ignores filters when scope=all, so a hand-edited URL behaves the same as the dialog.
  • src/app/payments/page.tsx:36-45 re-implements the status allowlist inline instead of calling parseFilters, so there are two allowlists for one field. Pre-existing, not a security issue since the values only reach in-memory predicates, but it will drift. Left as-is.
  • Not verified: focus placement on open and focus return on close. Escape-to-close was confirmed by hand; the rest of the focus behaviour comes from Radix and I did not test it. There are no component tests in this repo and no React testing library installed, so the dialog itself has no automated coverage — the 9 new tests cover the column serializer and the allowlist only.
  • This branch also carries one commit unrelated to the ticket, docs: add Release Standards to the root CLAUDE.md. Happy to drop it if you would rather this PR were only NWP-101.

🤖 Generated with Claude Code

peris611 and others added 2 commits September 10, 2026 10:46
The payments export was fixed: every column, current filter only. The card
last four shipped in every file, so anything going to a merchant was edited
by hand first.

Adds an options dialog on the existing Export button:

- Columns are selectable, with the card last four off by default
  (DEFAULT_EXPORT_COLUMNS in src/lib/csv.ts).
- Scope is current filter or all payments, with both row counts shown before
  download. Both scopes go through the existing query builder, so an export
  is never limited to the current page.
- parseExportColumns validates column names against the allowlist server
  side; the route returns 400 rather than serving an empty file.
- The filename names the scope: payments-disputed-2026-08-13.csv.

Amounts stay in minor units and are still formatted once, in cell(), beside
their own currency column.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Records the rules we are working to: no direct commits to main, test
evidence attached before merging, and a one-line business impact summary on
every pull request.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants