Skip to content

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

Closed
mmudda wants to merge 1 commit into
JJFromTenex:mainfrom
mmudda:NWP-201-issue-cards
Closed

mmudda wants to merge 1 commit into
JJFromTenex:mainfrom
mmudda:NWP-201-issue-cards

Conversation

@mmudda

@mmudda mmudda commented Sep 10, 2026

Copy link
Copy Markdown

Ticket

Closes NWP-201

What changed

Ops can issue a virtual card without messaging the platform team. /cards lists every issued card — nickname, merchant, masked number, spend limit, status, created date — with an Issue card panel that takes a nickname, merchant, spend limit and currency. Submitting creates the card, shows the full number once, and the card appears in the list masked. Opening a card shows its full record and how much of the limit it has spent.

Numbers are generated on the server on the 4242 test BIN with a valid Luhn check digit. The full number leaves the server exactly once, in the creation response; only the last four is ever stored, so it cannot be re-read from the list, the detail route, or the record itself.

How I verified it

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

npm test56 passing across 5 files, including 22 new cases in src/lib/cards.test.ts covering the Luhn generator, the BIN, masking and the status machine. npx tsc --noEmit and npm run lint both clean.

Against the dev server:

  • Issued a card and got 4242226956502653 — 16 digits, 4242 BIN, Luhn valid; stored last4 was 2653 and the card record had no number field.
  • GET /api/cards and GET /api/cards/:id both return records with no number key.
  • Every rejection returns 400 with a readable message: missing merchant, unknown merchant (mch_99), 0, -500, 5000001, 250.5, JPY, blank nickname. Exactly 5000000 returns 201.
  • Status machine: active→frozen 200, frozen→active 200, active→cancelled 200, cancelled→active and cancelled→frozen both 409. Unknown status 400, unknown card 404.
  • Generator holds across 200 different digit sources in the test suite, not one lucky seed.

In the browser: /cards renders the list with masked numbers and per-merchant currency (£ for Halcyon Studio, € for Brandt & Sohn). Opened the Issue card panel, filled it, submitted — the card was created and appeared in the list masked after the refresh. /cards/card_0005 shows 84% of limit used with the bar in amber and the "Past 80% of the limit" note; /cards/card_9999 returns 404.

Acceptance criteria

Core

  • Issue a card. Form takes nickname, merchant, spend limit and currency; submitting creates it and it appears in the list. — src/app/cards/issue-dialog.tsxPOST /api/cards.
  • Card list. /cards shows nickname, merchant, masked number, spend limit, status and created date.
  • Card detail. Full record plus spend against the limit, with a progress bar.
  • Generated card numbers. Server-side, 4242 BIN, valid Luhn — generateCardNumber() in src/lib/cards.ts.
  • Reveal once, mask forever. Full number only in the creation response; last4 only on the record; cleared from client state when the panel closes.
  • Server-side validation. All six rules enforced in parseCardDraft(), independent of the client.

Stretch

  • Freeze and unfreeze from the list without a full page reload — PATCH /api/cards/:id then router.refresh().
  • Spend progress on the detail, amber past 80%.
  • Merchant category lock — not built.
  • Tests on the Luhn generator and status transitions, beside the code they cover, npm test passing.
  • Empty and error states written rather than default — empty list, per-field limit guidance, an inline role="alert" on a failed submit, and a plain-language message on a refused transition.

Bugs fixed along the way

Two defects in the metrics path, found by /ship-ready, both pre-existing and unrelated to cards.

src/data/metrics.ts — daily volume bucketed in server local time. The bucket keys came from lastUtcDays() (UTC) but the lookup key came from new Date(p.createdAt).toLocaleDateString("en-CA"), which uses the server's zone. Root cause: a UTC key space read with a local-time key. On this machine (UTC−7) a payment at 2026-08-13T02:00:00Z was filed under 2026-08-12; anything falling before the oldest key hit if (!bucket) continue and disappeared from the chart with no error. Now uses the utcDayKey() helper that already existed for this.

src/data/metrics.ts — amounts accumulated as floats. bucket.captured += payment.amount / 100 built a float total in major units across ~1,600 payments, then Math.round(x * 100) converted it back. Root cause: converting for readability inside the accumulator instead of at the formatter. Now accumulates integer minor units and returns them directly. src/data/analytics.ts had the same division in the data layer; volumeByWeek and countAndVolumeByWeek now return minor units like merchantRollup already did, and the three chart formatters convert at the edge.

New src/data/metrics.test.ts pins both. The two bucketing tests are deliberately mirrored — an early-UTC and a late-UTC payment — so one fails on a machine west of UTC and the other east of it, wherever CI runs.

Notes for the reviewer

  • The mask shows the real last four, not the literal •••• 4242. The ticket writes the mask as •••• 4242, but 4242 is also the BIN, so that example is ambiguous. I stored and display the actual last four (•••• 2653), which matches the existing payments table and rule 2's "store the last four". If the literal string was intended, it is one line in maskCardNumber().
  • Spend is a seeded field, not derived. Cards have no transactions in this codebase, so Card.spent is generated with the seed data across a spread of utilisations (including one at 96%) to give the detail view something real. Newly issued cards start at 0.
  • Seed data added, none edited. Ten cards in generate.ts, using the same deterministic PRNG, so everyone gets identical records.
  • StatusBadge was extended rather than duplicated — card statuses joined the existing union instead of a second badge component.
  • The limit field converts at the boundary. Ops types 2500.00; parseAmountToMinorUnits() (already in src/lib/money.ts) converts once before the request, and the server re-validates the integer independently.
  • Known limitation, documented not fixed: headlineMetrics() sums USD, EUR and GBP into one figure and the overview renders it with a dollar sign. It is a mixed-currency total, not a USD one. Fixing it needs an FX source or per-currency reporting, neither of which is in scope here, so I replaced the file's incorrect "reported in USD minor units" comment with an explicit KNOWN LIMITATION block. Worth its own ticket.
  • Not covered by a test: the route handlers. The 400/409/404 paths were verified by curl only; automated coverage is on the generator, the status machine and the validator.
  • Not included: a CLAUDE.md edit and a package-lock.json name correction are in my working tree but left out of this PR — neither is ticket work.

🤖 Generated with Claude Code

Ops issued virtual cards by messaging the platform team, who created them
by hand. Twelve to twenty times a week, hours per card, and two cards last
month with the wrong limit because the request lived in a Slack thread.

Adds /cards: a list of every issued card, an issue form taking a nickname,
merchant, spend limit and currency, and a detail view showing the record
and its spend against the limit.

Card numbers are generated server-side on the 4242 test BIN with a valid
Luhn check digit. The full number is returned exactly once, in the creation
response, and only the last four is stored -- it is masked everywhere else.
Status is a state machine guarded on the server: active reversibly to
frozen, either to cancelled, and cancelled is terminal.

Every field of an issue request is validated server-side: the merchant must
exist, the limit must be whole minor units above zero and no more than
5,000,000, and the currency must be USD, EUR or GBP.

Also fixes two defects /ship-ready surfaced in the metrics path, unrelated
to this ticket: dailyVolume bucketed by server-local day against UTC keys,
which misfiled payments and silently dropped some off the oldest edge; and
amounts were accumulated as floats in major units before being converted
back to minor units.

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

Copy link
Copy Markdown
Owner

Claude Code 101 — Repo Rescue

🏆 Build Battle Score: 85 / 100

One-line verdict: A clean, well-tested core with honest documentation of its gaps — but it stops short of nearly every Tier 2 stretch, and the ticket's own "merchant category lock" hint was skipped.

Core criteria — 95 / 100 (35%)

  1. Issue a card: ✅ — issue-dialog.tsx collects nickname/merchant/limit/currency, posts to /api/cards, list refreshes via router.refresh().
  2. Card list: ✅ — cards/page.tsx shows all six required fields plus a freeze/unfreeze action.
  3. Card detail: ✅ — [id]/page.tsx shows full record, remaining balance, and an amber-past-80% progress bar.
  4. Generated numbers: ✅ — generateCardNumber() called server-side in the POST route on the 4242 BIN; the implementation file itself (src/lib/cards.ts) isn't in this diff (noted as truncated), but 200-seed Luhn tests and route usage make this credible.
  5. Reveal once: ✅ — number only ever leaves in the POST response; Card type has no number field, only last4; client state is discarded on drawer close.
  6. Server-side validation: ✅ — parseCardDraft() runs in the route handler, independently tested against all six rejection cases plus the exact-5,000,000 boundary.

Correctness rules — 100 / 100 (20%)

  • Minor units: ✅ — spendLimit/spent stored as integers; also retrofitted analytics.ts/metrics.ts to stop the major-unit float round-trip.
  • Luhn on 4242 BIN: ✅ — generator takes a digit source (not hardcoded), verified against 200 seeds and the textbook Luhn example.
  • Masking: ✅ — no number field on the Card type, list/detail endpoints confirmed number-free, reveal state cleared on close.
  • State machine: ✅ — canTransition guard in the PATCH handler; cancelled→active/frozen both return 409 and are tested.
  • Server-side validation: ✅ — enforced in the route, independent of the client form's own (also present) checks.

Context and planning — 70 / 100 (10%)

No spec file surfaces in the diff and none is referenced in the PR body. What the description does provide is a considered, file-accurate plan mapped against the acceptance criteria (issue-dialog.tsxPOST /api/cards, generateCardNumber() in src/lib/cards.ts, etc.), and the delivered code matches it closely — that's the 0.7 tier, not the 1.0 tier that requires an actual spec artifact.

Code quality — 90 / 100 (15%)

Tests sit beside the code they cover (cards.test.ts, metrics.test.ts), are specific enough to fail without the fix, and the PR is honest that route-handler status codes were only curl-verified, not automated — that candor is worth crediting rather than penalizing. StatusBadge was extended, not duplicated; no DB/ORM added; no console.log/TODO visible. The two metrics.ts bug fixes (local-time bucketing, float accumulation) are real, root-caused, and pinned by mirrored UTC-edge tests — the full +0.10 credit applies here.

PR description — 100 / 100 (5%)

Exemplary: states what was built, walks the acceptance criteria file-by-file, reports verified npm test/tsc/lint output, discloses an unmet stretch goal and an untested surface plainly, and documents a known limitation it chose not to fix.

Stretch goals — 40 / 100 (15%)

Tier 1: Freeze/unfreeze without reload ✅ · Spend progress bar (amber >80%) ✅ · Merchant category lock ❌ (explicitly not built) · Luhn/status tests ✅ · Empty/error states ✅ — 4 of 5.
Tier 2: Idempotent issue ❌ (submit is only client-disabled via isLoading/canSubmit in issue-dialog.tsx, no server-side dedup) · Currency matches merchant ❌ (form defaults currency from the merchant but the separate currency Select lets ops override it, and parseCardDraft in the route only checks USD/EUR/GBP membership, never equality with merchant.currency) · Spend is honest ❌ (seed cards get invented utilisation values in generate.ts, not 0 and not derived from real transactions — honestly disclosed in the PR, but the rubric still marks this un-credited) · Cancel-with-confirm ❌ (no cancel action anywhere in card-actions.tsx) · Audit trail ❌ (no transition log on the card or detail page).


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

One thing to do differently next time: Spend the last stretch minutes wiring the server-side currency check against merchants.ts rather than polishing the empty-state copy — it was the one Tier 2 item the diff came closest to already having (the UI already derives currency from the merchant) and would have been the cheapest point on the board.

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