Skip to content

NWP-201: Issue virtual cards from the console - #177

Open
asemzahran wants to merge 7 commits into
JJFromTenex:mainfrom
asemzahran:NWP-201-issue-cards
Open

NWP-201: Issue virtual cards from the console#177
asemzahran wants to merge 7 commits into
JJFromTenex:mainfrom
asemzahran:NWP-201-issue-cards

Conversation

@asemzahran

Copy link
Copy Markdown

NWP-201: Issue virtual cards from the console

Spec: docs/specs/NWP-201-issue-cards.md, written before any code. One commit per step of the spec.

What was built

  • Server: src/lib/cardNumber.ts generates numbers on the 4242 BIN with a Luhn check digit. src/data/cardRules.ts holds the allowlists, validation, state machine, masking, and spend threshold. src/data/cards.ts holds the store operations. src/app/api/cards and src/app/api/cards/[id] are the routes.
  • UI: /cards list, the issue drawer, and /cards/[id] detail. Cards is also in the sidebar and breadcrumbs.

Core criteria

  • Issue a card: the drawer takes nickname, merchant, limit, currency, and category, and the new card appears in the list.
  • Card list: nickname, merchant, •••• 1234, limit, status, and created date.
  • Card detail: the full record, spend against the limit, and the status history.
  • Generated numbers: generated server-side on 4242 with a valid Luhn digit.
  • Reveal once: the full number exists only in the 201 response. Card has no field for it, and client state is cleared when the drawer closes.
  • Server-side validation: a missing merchant, a limit ≤ 0, a limit > 5,000,000, a non-integer limit (including "250.00"), and a currency outside USD/EUR/GBP all return 400 with per-field messages.

Stretch goals

  • Freeze and unfreeze from the list without a reload (PATCH, then router.refresh())
  • Spend progress bar on the detail page, amber past 80% (integer comparison, unit-tested)
  • Merchant category lock, chosen at issue and shown on the detail page
  • Tests: cardNumber.test.ts, cardRules.test.ts, and cards.test.ts, with Luhn, every transition, validation, idempotency, and no full number stored
  • Written empty and error states (list, form fields, form-level, and PATCH failures)
  • Idempotent issue: an Idempotency-Key header is required, and the server maps key → card. A replay returns 409 with the original id. Submit is also disabled while in flight.
  • Currency must match the merchant: the form sets the currency from the merchant, and the server rejects a mismatch (e.g. GBP on mch_05).
  • Honest spend: spent is 0 at issue and stays 0, because nothing links payments to cards. The detail page says so.
  • Cancel from the UI with a confirm: two steps, through the guarded PATCH, after which the terminal state shows no actions.
  • Audit trail: each transition is appended to card.history with a UTC timestamp and shown on the detail page.

Bugs fixed along the way

  • src/data/queries.ts sortPayments: amounts were compared with String(a.amount).localeCompare(...), so 900 sorted after 25000. It now compares the integer minor units numerically. Covered by src/data/queries.test.ts.
  • src/components/Drawer.tsx: the close button had no accessible name. It now has aria-label="Close".

How it was verified

  • npm test: 59/59 pass. tsc --noEmit and next lint are clean.
  • curl against the dev server:
    • a valid issue returns 201 with a 4242 number
    • a replayed key returns 409
    • no key returns 400
    • a missing merchant, 0, -100, 5000001, "250.00", XYZ, and GBP on an EUR merchant each return 400
    • list and detail responses contain no 13+ digit run
    • active→frozen→active returns 200, cancelled→active returns 409, and bogus returns 400
  • Browser:
    • empty state
    • server field errors render inline
    • the merchant sets the currency automatically
    • a card was issued, and the reveal screen showed the full number
    • after Done, the row shows •••• 2560 and the full number is gone from the DOM
    • freeze and unfreeze sent a PATCH and an _rsc fetch with no document reload
    • cancel with confirm, then the terminal state and a four-entry history
  • Not verified: Drawer and Select close animations did not unmount in the preview pane (animationend never fired there). Escape sets the Radix state to closed. This is the stock Tremor/Radix component, but I did not confirm focus return on close in a real browser.

Out of scope

  • Persistence (NWP-203)
  • Auth
  • Card network calls
  • Editing a limit (NWP-202)

🤖 Generated with Claude Code

asemzahran and others added 7 commits September 10, 2026 14:41
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…eshold

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Stores last four and a reference, never the full number. A reused
idempotency key returns the original card id instead of minting again.

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

POST /api/cards requires an Idempotency-Key and returns the full number
exactly once. GET list and detail carry last four only.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Freeze/unfreeze from the list refreshes in place. Cancel needs a
confirm and renders the terminal state. Detail shows spend against the
limit and the status history. Drawer close button gets an accessible name.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
sortPayments compared String(amount), so 900 sorted after 25000.

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

Copy link
Copy Markdown
Owner

Claude Code 101 — Repo Rescue

🏆 Build Battle Score: 88 / 100

One-line verdict: The most complete submission of the field — real idempotency, currency/merchant matching, audit trail, and an honest spend model, all backed by tests that would actually fail without the change; docked mainly because the diff is truncated and several load-bearing files (cardNumber.ts, types.ts, merchants.ts, the spec itself) aren't visible to verify directly.

Core criteria — 90 / 100 (35%)

  1. Issue a card: ✅ — drawer collects nickname/merchant/limit/currency/category, posts to /api/cards, list refreshes on close.
  2. Card list: ✅ — cards/page.tsx shows nickname, merchant, masked number, limit, status, created date.
  3. Card detail: ✅ — full record, spend vs. limit, plus a status-history timeline (beyond the ask).
  4. Generated numbers: ⚠️ — tests assert ^4242\d{12}$ and Luhn validity, but lib/cardNumber.ts itself is not in the diff, so the generator logic is unverified.
  5. Reveal once: ✅ — number only exists in the 201 payload; Card has no number field per the test asserting the stored JSON contains no 13+ digit run; client state cleared on drawer close.
  6. Server-side validation: ✅ — validateIssueCard runs in the route handler before any store write, covering all five listed cases plus non-integer limits.

Correctness rules — 90 / 100 (20%)

  • Minor units: ✅ — Number.isInteger guard, conversion happens once client-side (parseAmountToMinorUnits), server never parses money strings.
  • Luhn on 4242 BIN: ⚠️ — verified only indirectly through tests; generator source not in the visible diff.
  • Masking: ✅ — numberRef (a token) is stored, not the number; test explicitly checks no full number persists.
  • State machine: ✅ — TRANSITIONS table matches spec exactly, cancelled has no outgoing edges, exhaustively tested.
  • Server-side validation: ✅ — enforced in the route, not just the form.

Context and planning — 70 / 100 (10%)

PR claims a spec at docs/specs/NWP-201-issue-cards.md written before code, with one commit per spec step, and the delivered file layout (cardRules.ts, cards.ts, cardNumber.ts, routes) matches what's described. The spec file itself isn't in the diff, so its content and file-path accuracy can't be confirmed — scoring the plan as stated, not verified.

Code quality — 85 / 100 (15%)

Tests sit next to the code they cover (cardRules.test.ts, cards.test.ts, queries.test.ts) and read as genuine failing-without-the-change tests, not padding. Accessibility is handled deliberately: labelled fields, aria-invalid/aria-describedby wiring, focus moved to the first field on open and restored on confirm-cancel. No DB, no console.log/TODO visible. The claimed sortPayments fix is real, targeted, and tested. One minor concern: numberRef uses randomUUID() for a "token" with no stated purpose beyond decoration — harmless but unexplained.

PR description — 90 / 100 (5%)

Detailed, itemized against every core/stretch criterion, includes an honest "not verified" section (animation/focus-return caveat) rather than overclaiming. Strong.

Stretch goals — 95 / 100 (15%)

Tier 1: ✅ freeze/unfreeze without reload, ✅ amber progress bar (integer-compared, tested), ✅ category lock, ✅ Luhn/transition unit tests, ✅ written empty/error states — all five, visible in diff.
Tier 2: ✅ Idempotent issue (Idempotency-Key header + store.cardIssueKeys map, tested duplicate→409 in cards.test.ts) — src/app/api/cards/route.ts. ✅ Currency-matches-merchant (cardRules.ts merchant.currency !== currency check, form derives currency from merchant) — src/data/cardRules.ts. Additional Tier 2 items (honest spend-at-zero, confirm-cancel, audit trail) are also present in the diff but Tier 2 is capped at 0.50, already reached by the first two.


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

One thing to do differently next time: Include the spec file itself in the diff (or at minimum quote its key sections in the PR body) — the planning claim is entirely unverifiable as submitted, which is the single biggest gap between "stated" and "shown."

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


Powered by Anthropic and Tenex

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