Skip to content

NWP-201: issue virtual cards from the console - #170

Open
ShanKwanCho wants to merge 2 commits into
JJFromTenex:mainfrom
ShanKwanCho:NWP-201-issue-cards
Open

NWP-201: issue virtual cards from the console#170
ShanKwanCho wants to merge 2 commits into
JJFromTenex:mainfrom
ShanKwanCho:NWP-201-issue-cards

Conversation

@ShanKwanCho

@ShanKwanCho ShanKwanCho commented Sep 10, 2026

Copy link
Copy Markdown

Ticket

Closes NWP-201

What changed

Ops can issue a virtual card from the console instead of asking the platform team for one. /cards lists every card issued — nickname, merchant, masked number, spend limit, status, created date — and opening one shows its full record, its spend against the limit, and its history. Cards can be frozen, unfrozen, and cancelled from either the list or the detail page without a page reload.

Card numbers are generated on the server on the 4242 test BIN with a valid Luhn check digit. The full number exists in exactly one response, the creation one; the stored record carries only the last four and an opaque reference, so no later read can surface a PAN. Spend limits are integer minor units end to end, and every field is checked against an allowlist on the server before it reaches the store.

How I verified it

  • npm test passes
  • New behavior is covered by a test
  • Checked it in the browser

npm testTest Files 5 passed (5) / Tests 64 passed (64). 36 of those are new: 16 in src/lib/cards.test.ts, 20 in src/data/cards.test.ts. The generator case asserts the BIN, the sixteen-digit length, and Luhn validity across 500 generated numbers, and the transition case enumerates the whole state machine and asserts it permits exactly four edges.

npx tsc --noEmit — clean. npm run lint✔ No ESLint warnings or errors. npm run build — succeeds; /cards and /cards/[id] both render.

curl against npm run dev. Every case below was run and the status code is what came back:

Request Result
POST /api/cards with a valid body 201, body carries cardNumber: "4242043823655733" and card.last4: "5733"
GET /api/cards, GET /api/cards/card_000005 200, last4 only. Grepping both payloads for the issued number returns 0 matches, and for any run of 13+ digits, 0 matches
missing merchant / unknown merchant 400 "Choose a merchant." / "That merchant does not exist."
spendLimit: 0 / -1 400 "A spend limit must be greater than zero."
spendLimit: 5000001 400 "A spend limit cannot exceed 5000000 minor units."
spendLimit: 5000000 201 — the ceiling itself is allowed
spendLimit: 250.5 / "25000" 400 "A spend limit is a whole number of minor units — $250.00 is 25000."
currency: "JPY" 400 "Currency must be one of USD, EUR, GBP."
empty nickname / unknown category 400, each naming its field
PATCH active→frozen, frozen→active, active→cancelled 200
PATCH cancelled→active, cancelled→frozen 409 "A cancelled card is cancelled for good."
PATCH active→active 409
PATCH with status: "melted" 400. Unknown card id → 404

Browser. Issued a card through the drawer: merchant Halcyon Studio, limit typed as 1,250.75, category Software. Currency switched itself to GBP when the merchant was picked. The POST returned 201, the success screen showed 4242 1889 3176 2762 and Spend limit £1,250.75 GBP, and the list behind the drawer went from 4 cards to 5 with the new row reading •••• 2762 — no page reload.

Reveal-once, checked in the DOM rather than by eye. While the success screen was open, the page contained the full number. After clicking Done: not in the DOM, not in the URL, not in localStorage (which is empty), not in sessionStorage, not in a cookie. Reopening the issue drawer gives an empty form with no trace of it, and the card's own detail page shows •••• 2762.

Freeze without a reload. Set a Symbol on window, clicked Freeze on the detail page, and after the update the same Symbol was still there — the document was never replaced. The badge went Active → Frozen, the button became Unfreeze, and a "Frozen" entry appeared in the card's history.

Spend bar. /cards/card_000001 renders $2,164.00 of $2,500.00, 87% used · $336.00 left, with the bar amber. Its role="progressbar" carries aria-valuenow="87".

Empty state. Temporarily made the card seed return nothing, restarted the dev server, and loaded /cards: the table renders "No cards issued yet / Issue one and it appears here. The full number is shown once, on the screen right after you create it." and the count reads "0 cards". The seed was then restored — git diff on src/data/generate.ts against the commit is empty.

Error state. Submitted an 80-character nickname, which the form does not check but the server does. A role="alert" banner appeared reading "A nickname is at most 60 characters.", the form kept everything that was typed, and the list stayed at 4 cards — nothing was created.

Accessibility, read out of the accessibility tree. The issue drawer has aria-labelledby resolving to "Issue a virtual card" and focus moves into it on open. Escape closes it and focus returns to the "Issue card" trigger. Every input has a real <label for>; the three Radix comboboxes expose the names Merchant, Currency, and Category lock. After the fix below, the drawer reports zero buttons without an accessible name. Row actions are distinct per card — "Freeze Meta Ads", "Cancel Messe travel" — rather than eleven buttons called "Freeze". No orphaned labels, no skipped heading levels.

Acceptance criteria

Core

  • Issue a card. Drawer takes nickname, merchant, spend limit, and currency; submitting creates the card and it appears in the list.
  • Card list. /cards shows nickname, merchant, masked number, spend limit, status, and created date.
  • Card detail. Full record plus spend against the limit and the card's history.
  • Generated card numbers. Server-side, 4242 BIN, valid Luhn check digit. Asserted over 500 draws.
  • Reveal once, mask forever. Shown on the success screen and nowhere else; verified in the DOM after dismissal.
  • Server-side validation. Missing merchant, zero/negative limit, limit above 5,000,000, and any currency outside USD/EUR/GBP all rejected, plus non-integer limits and unknown categories.

Stretch

  • Freeze and unfreeze from the list and the detail page, without a full page reload.
  • Spend progress on the card detail, amber past 80%.
  • Merchant category lock, chosen at issue time and shown on the card — stored, allowlisted on the server, displayed. It is not enforced against anything, because there are no card transactions in this console to enforce it against. See the note below.
  • Tests. 36 new, beside the code they cover, npm test passing.
  • Empty and error states, written rather than default, and both exercised in the browser: the empty table says what will appear and warns that the number is shown once, the form surfaces the server's own message in a role="alert", and a refused transition renders next to the button that was clicked.

Beyond the ticket

  • A card's history. Each card keeps a timestamped list of what happened to it — issued, frozen, unfrozen, cancelled — rendered on the detail page in the same shape as the payment timeline. The ticket asks for an ops tool, and "what happened to this card last Tuesday" is the question ops asks second.
  • Currency follows the merchant. Picking a merchant sets the currency to that merchant's own, which is the wrong-currency mistake the ticket describes, removed rather than validated.
  • Double-submit guard. The submit button disables while a request is in flight, and both client components refuse a second request while one is pending.

Bugs fixed along the way

src/components/Drawer.tsx — the close button had no accessible name. The X in DrawerHeader is an icon-only Button whose RiCloseLine icon is aria-hidden, so it reached the accessibility tree with no name and a screen reader announced it as "button". Root cause is the missing aria-label on the Button, not the icon. Found while checking the issue drawer against .claude/rules/components.md. The Drawer is shared, so this also fixes the mobile sidebar in src/components/Sidebar.tsx. One line, in its own commit.

Two more things I found and deliberately did not touch, because both are outside this ticket:

  • src/data/queries.ts:sortPayments sorts by amount with String(a.amount).localeCompare(...), so 9000 sorts above 25000. It is a real defect in payment sorting and it has nothing to do with cards; fixing it here would have buried it in this diff.
  • merchant-console/CLAUDE.md says seed data is JSON loaded at boot. It is not — it is generated in src/data/generate.ts by a seeded PRNG. I followed the code rather than the document and seeded cards the same way.

Notes for the reviewer

Where the full number lives. issueCard in src/data/cards.ts is the only function that ever holds one, in a local, long enough to take its last four. The Card type has no field for it, so masking is a property of the type rather than a filter a future route could forget to apply. src/data/cards.test.ts asserts the stored record and the list payload do not contain it.

Why validation returns a result instead of throwing. validateIssueInput and setCardStatus return { ok: false, status, error, field }, which the route handlers map straight onto a response. It keeps the handlers as thin as api/payments/route.ts, and it means every boundary case is a Vitest assertion in the node environment rather than an HTTP test.

The one division in card money. src/app/cards/[id]/page.tsx divides spend by limit — both integers — to get a percentage, never an amount. remaining is integer subtraction, and formatMoney is the only thing that renders a decimal point.

A Tailwind constraint on the progress bar. .claude/rules/components.md forbids inline style, and Tailwind cannot build a class name from a runtime number, so the bar snaps to the nearest five percent from a fixed list of classes. The exact percentage is written beside it, so nothing is lost but a few pixels.

spent is seeded, never incremented. Authorising spend against a card is not in this ticket and payments carry no card reference, so seeded cards carry a spend figure to give the bar something real to draw and newly issued cards start at zero. The alternative — inventing a join onto store.payments — was a bigger diff pointed at the persistence work in NWP-203.

Deliberately not built: persistence, auth, real network calls, and editing a limit after issue. The spec is at docs/specs/NWP-201-issue-cards.md and the build follows it; the only departure is that the card history was not in the original plan.

One thing I could not check. There is no pre-push hook in this repository — build-battle/CLAUDE.md says so explicitly — so nothing was bypassed and nothing was skipped; there simply is not one. Nothing above was run with --no-verify.

🤖 Generated with Claude Code

Ops can issue a single-merchant virtual card from /cards, see every card
issued, and open one to check it — replacing the Slack thread that put the
wrong limit on two cards last month.

Server side owns everything that matters. Numbers are generated on the
4242 test BIN with a valid Luhn check digit and exist as a full number in
exactly one place: the creation response. The stored record carries the
last four and an opaque reference, so no later read can surface a PAN.
Spend limits are integer minor units end to end, validated against an
allowlist before they reach the store. Status is a guarded state machine —
active to frozen and back, either to cancelled, and cancelled is terminal.

Card reads and writes go through one module, src/data/cards.ts, which
returns a discriminated result the route handlers map straight onto a
status code. That keeps the handlers as thin as the payments one and makes
every validation boundary testable without HTTP.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@JJFromTenex

JJFromTenex commented Sep 10, 2026

Copy link
Copy Markdown
Owner

Claude Code 101 — Repo Rescue

🏆 Build Battle Score: 88 / 100

One-line verdict: A genuinely thorough, boundary-tested implementation that nails every core criterion and most of Tier 1 polish, but the two Tier 2 items it gestures at (currency-matches-merchant, idempotent issue) are UI conveniences, not server guarantees — and the diff is missing the files (src/lib/cards.ts, the spec) that would let a reviewer confirm the Luhn generator and the planning claims directly.

Core criteria — 95 / 100 (35%)

  1. Issue a card: ✅ — Drawer takes nickname/merchant/limit/currency/category, posts to /api/cards, list refreshes via router.refresh().
  2. Card list: ✅ — /cards/page.tsx shows all six required columns plus row actions.
  3. Card detail: ✅ — full record, spend bar, and history rendered in cards/[id]/page.tsx.
  4. Generated numbers: ⚠️issueCard calls generateCardNumber()/lastFour() and tests assert ^4242\d{12}$, but src/lib/cards.ts (the actual Luhn implementation) is not in this diff, so the check-digit logic itself is unverifiable from what's shown.
  5. Reveal once: ✅ — number lives only in local component state on the success screen, dropped on reset(); Card type has no field for it, and a test asserts the stored record and list payload never contain it.
  6. Server-side validation: ✅ — validateIssueInput in src/data/cards.ts is called from the route handler, not just the form.

Correctness rules — 90 / 100 (20%)

  • Minor units: ✅ — Number.isInteger enforced, formatMoney is the only place a decimal appears.
  • Luhn on 4242 BIN: ✅ (circumstantial) — pattern and prefix confirmed by tests shown, but the generator file itself isn't in the diff, so I can't verify the check digit is computed rather than templated.
  • Masking: ✅ — GET handlers serialize a Card type that has no PAN field; tests assert it directly.
  • State machine: ✅ — canTransition + setCardStatus enforce active⇄frozen, either→cancelled, cancelled terminal; tested exhaustively including the "already this status" 409 case.
  • Server-side validation: ✅ — confirmed in the route handler, not just the client.

Context and planning — 70 / 100 (10%)

The PR claims a spec at docs/specs/NWP-201-issue-cards.md and says the build follows it with one stated departure (card history). That file is not present in this diff (possibly truncated, possibly never included), so I can't confirm it cites real paths or matches the delivered code — I can only credit the PR narrative, which is specific and plausible. Scoring as "PR description lays out a considered plan and commits follow it" rather than the top tier, since the spec itself is unverifiable here.

Code quality — 90 / 100 (15%)

Tests sit at the data-layer boundary (src/data/cards.test.ts), which is the right place — validation, issuance, masking, and the full transition matrix are all exercised, and each would fail without the change. Accessibility is handled carefully (labelled inputs, aria-labels per row action, role="alert", focus into drawer). No DB, no seed-file editing, no stray console.log. One honesty gap: the "bugs found" section names a real defect (sortPayments string-sorting amounts) but explicitly declines to fix it — correct to keep scope tight, but it means no bug-fix credit here, and it's noted rather than buried.

PR description — 95 / 100 (5%)

Exceptionally detailed and honest — reports test counts, curl-verified status codes, DOM-level reveal-once verification, and an explicit "deliberately not built" list. Meets the bar for full credit.

Stretch goals — 75 / 100 (15%)

Tier 1 (all ✅, 0.50): freeze/unfreeze without reload (CardActions + router.refresh()); spend bar amber past 80% (AMBER_AT = 80); category lock stored, allowlisted, displayed; 36 tests including the full transition matrix; written empty/error states.

Tier 2:

  • Idempotent issue: ❌ — only a disabled-submit-while-pending guard (if (submitting) return), no idempotency key or server-side replay rejection. UI-only debounce is explicitly 0 per rubric.
  • Currency matches merchant: ❌ — the form auto-sets currency on merchant selection, but the currency <Select> remains editable and validateIssueInput only checks membership in CURRENCIES, never cross-checks against merchantById(merchantId).currency. A EUR merchant could still be issued a GBP card server-side.
  • Spend is honest: ❌ — new cards start at 0 (fine), but seeded cards carry hand-picked spent figures in generate.ts with no derivation from real store data, which the rubric treats as invented.
  • Cancel with confirm: ❌ — CardActions.move("cancelled") fires on a single click with no confirmation step.
  • Audit trail: ✅ — card.history records status + timestamp + note per transition, rendered on the detail page (src/app/cards/[id]/page.tsx).

Tier 1 (0.50) + Tier 2 (0.25, audit trail only) = 0.75.


Breakdown: Core (95 × 0.35) + Rules (90 × 0.20) + Context (70 × 0.10) + Quality (90 × 0.15) + PR (95 × 0.05) + Stretch (75 × 0.15) = 88 / 100

One thing to do differently next time: enforce merchant currency server-side (one line in validateIssueInput) — the form already knows the merchant's currency, so the gap between "the UI derives it" and "the server verifies it" is the cheapest possible fifteen minutes of extra credit that was left on the table.

The diff was too large to review in full, so only the first part was graded.


Powered by Anthropic and Tenex

The close control in DrawerHeader is an icon-only button whose icon is
aria-hidden, so it reached the accessibility tree with no name at all. A
screen reader announced it as "button".

Found while checking the issue-card drawer against the rule in
.claude/rules/components.md that a dialog be operable. The Drawer is
shared — the mobile sidebar in src/components/Sidebar.tsx uses it too — so
this fixes both.

Verified in the browser: the drawer now reports zero buttons without an
accessible name, Escape closes it, and focus returns to the trigger.

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