Skip to content

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

Closed
itsChanelML wants to merge 6 commits into
JJFromTenex:mainfrom
itsChanelML:NWP-201-issue-cards
Closed

NWP-201: issue virtual cards from the console#137
itsChanelML wants to merge 6 commits into
JJFromTenex:mainfrom
itsChanelML:NWP-201-issue-cards

Conversation

@itsChanelML

Copy link
Copy Markdown

Ticket

Closes NWP-201

What changed

Ops can now issue a virtual card from the console instead of messaging the platform team: a nickname, a merchant, a spend limit, and a category, submitted from /cards. The full card number — generated server-side on the 4242 test BIN with a real Luhn check digit — is shown exactly once, on the success screen, and never again; everywhere else, including the card's own stored record, only carries •••• last4. /cards lists every issued card; /cards/[id] shows the full record plus spend against the limit (an amber progress bar past 80%) and a status history. Freeze, unfreeze, and cancel are guarded server-side as a state machine (active ⇄ frozen, either → cancelled, cancelled terminal) and update the page without a full reload.

One thing beyond the ticket's literal wording: the issue form derives currency from the selected merchant rather than offering an independent picker, since every Merchant already carries its own settlement currency and a mismatch isn't a real product choice — it's a data-entry mistake waiting to happen. The server rejects a mismatch even if one somehow arrives outside the UI.

How I verified it

  • npm testTest Files 6 passed (6), Tests 57 passed (57) (28 pre-existing + 2 from the sort fix + 14 in lib/cards.test.ts + 13 in data/cards.test.ts).
  • npm run build → compiles and type-checks clean, all /cards routes present in the route table.
  • npm run lint → no warnings or errors.
  • Ran a scratch dev server and exercised the actual API, not just the code:
    • Each of the four required rejections, individually, with the real response body: missing merchant → "Select a merchant to issue this card to."; limit=0"Enter a spend limit greater than $0.00."; limit=50000.01 (i.e. 5,000,001 minor units) → "Spend limit can't exceed 5,000,000 minor units..."; currency=JPY"Choose USD, EUR, or GBP." Plus the currency-mismatch case: currency=EUR against a USD merchant → "Lumen Coffee Roasters settles in USD — issue the card in that currency."
    • Issued a real card (POST /api/cards, valid payload) → 201 with number: "4242502259316433" in the body. Then grep'd that exact number against a subsequent GET /api/cards/<id> and GET /api/cards — zero matches in either. The number exists in one response and nowhere else.
    • Walked the full state machine on that card: active→frozen ✅, frozen→active ✅, active→cancelled ✅, then cancelled→active → rejected with "Can't move a cancelled card to active.", an unknown status string → rejected, a nonexistent card id → 404. The card's history array shows all four real transitions with real timestamps.
    • Rendered /cards/card_0002 (seeded at 87% of its limit) → progress bar renders bg-amber-500 at 87%. Rendered /cards/card_0001 (12%) → bg-blue-500, no amber. Rendered /cards/card_0004 (seeded cancelled) → shows "Cancelled — no further changes," no Freeze/Unfreeze/Cancel buttons rendered at all.
    • /cards/card_nope → real 404 via Next's notFound().
  • src/lib/cards.test.ts pins the Luhn math itself: reproduces the known check digit for 4242424242424242 (a widely-known valid test number) by hand, then asserts 50 generated numbers each start with 4242, are 16 digits, and pass isValidLuhn.

Acceptance criteria

Core:

  • Issue a card — form/dialog (Drawer) takes nickname, merchant, limit, currency (derived); creates and appears in the list
  • Card list — /cards: nickname, merchant, masked number, category, limit, status, created date
  • Card detail — /cards/[id]: full record, spend vs. limit
  • Generated card numbers — 4242 BIN + valid Luhn, proven in lib/cards.test.ts
  • Reveal once, mask forever — verified above by grepping the actual number out of every subsequent response
  • Server-side validation — all four named checks verified individually above, plus the currency-match bonus

Rules:

  • Money as integer minor units — parseAmountToMinorUnits at the one boundary (issueCard), never a float
  • Never persist/display full number after creation — Card type has no field for it; verified live
  • Status is a state machine, guarded server-side — verified live including the terminal cancelled case
  • 4242 BIN mandatory — hardcoded in generateCardNumber, tested

Stretch:

  • Freeze/unfreeze without full reload — PATCH + router.refresh(), no navigation
  • Spend progress bar, amber at 80%+ — verified against two seeded cards above
  • Merchant category lock — set at issue time, shown on list + detail, no edit control anywhere
  • Tests on the Luhn generator and status transitions — lib/cards.test.ts (14 tests), data/cards.test.ts (13 tests)
  • Empty and error states, written not default — every validation rejection above names the actual problem; /cards has a real empty-state message for zero cards (untriggered by seed data, but present and type-checked)

Bugs fixed along the way

  • src/data/queries.ts:81sortPayments compared amounts as strings (String(a.amount).localeCompare(...)), which ranks by leading digit rather than magnitude, so descending sort put small payments above large ones. Fixed to numeric subtraction; test added.
  • src/app/payments/page.tsx — the page hand-rolled its own filter parsing instead of the shared parseFilters(), so it silently never read sort/direction at all — the API supported sorting, the UI had no way to use it. Now reuses parseFilters, and the Date/Amount column headers are clickable and toggle direction.
  • src/app/disputes/page.tsx — the queue sorted all disputes by evidence deadline, including resolved ones, so long-settled won/lost cases (their deadline is necessarily in the past) sorted ahead of open cases that still needed a response. Open disputes now sort first; the inline "days left" text became a proper severity badge.

Notes for the reviewer

  • Beyond the ticket's five named stretch goals, this also does the currency-match check described above, disables the Issue button while a request is in flight (double-click protection), gates Cancel behind a confirm step since it's terminal, and keeps a status-history list per card — none of these were spelled out in the ticket, but they're the kind of thing "an ops tool actually needs once real people click it twice" that the ticket itself says to look for.
  • Not browser-click-tested this session: the actual Issue-card drawer and Freeze/Unfreeze/Cancel buttons, in a real browser, end to end. I verified the underlying API calls directly (every curl above) and confirmed the trigger markup renders, but I did not click through the interactive flow in a browser window. Saying so rather than claiming a pass I didn't run.
  • spent on seeded cards is invented history (there's no transaction feed to derive it from honestly) — small and clearly explained in the spec's Risks section. Every card issued live through the app starts at spent: 0.
  • Three unrelated fixes are bundled into this branch rather than three separate PRs, per direction — see "Bugs fixed along the way" above for the honest accounting of what they are and aren't.

sortPayments() coerced amount to a string and ran localeCompare on it,
which ranks by leading digit rather than magnitude (999 > 25000 as
strings). Sorting descending put small payments above large ones.
Both /api/payments and the CSV export go through this comparator, so
the export inherited the same wrong order.

Switched to numeric subtraction, matching every other place in the
codebase that already treats amount as the integer it is.
The /payments API already supported sort=amount/createdAt via query
params; the table never exposed a way to use it, so ops has been
eyeballing the list to find the largest payments.

Date and Amount headers are now links that toggle direction (a
neutral chevron on inactive sortable columns, a filled up/down arrow
on the active one) and reuse the existing GET params — no client JS,
no new API. Reset to page 1 on a sort change, matching how changing a
filter already behaves.

Also switched the page from its own hand-rolled filter parsing to the
shared parseFilters() (queries.ts), which is what makes sort/direction
available here at all — the page previously never read them. The
filter bar now carries the active sort forward when a filter changes,
so picking a status or merchant doesn't silently reset the sort back
to the default.
A dispute already showed "3 days left" / "overdue" as plain colored
text next to its evidence-due date, but only as inline text easy to
miss while scanning, and the page sorted ALL disputes (including
resolved ones) by evidenceDueAt — so long-settled won/lost cases,
whose deadline is in the past, sorted ahead of open ones that still
need action.

- Open disputes (needs_response, under_review) now sort before
  resolved ones; within each group, soonest deadline first.
- The inline text is now a Badge, matching the severity language the
  rest of the app already uses (the same "error" red as a failed
  payment, "warning" amber for a 3-5 day heads-up). Still scoped to
  needs_response only — once a dispute is under_review the evidence
  is already submitted, so a deadline badge there would flag
  something nobody can act on anymore.
Card type, in-memory store slice, and six seeded example cards (a
handful of varied statuses so /cards opens onto something real, per
the same pattern payments/disputes/payouts already use).

src/lib/cards.ts holds the two genuinely stateless, testable pieces:
Luhn check-digit generation on the 4242 test BIN, and the
active/frozen/cancelled transition rule. src/data/cards.ts is the one
place cards are read or mutated — issueCard() validates a missing
merchant, a zero/negative limit, a limit over 5,000,000 minor units,
a currency outside USD/EUR/GBP, and (going further than the ticket's
literal wording) a currency that doesn't match the issuing merchant's
own currency, since Merchant already carries one. The full number
exists only in issueCard()'s return value — the Card type has no
field to leak it into later.
GET/POST /api/cards and GET/PATCH /api/cards/[id]. Validation lives
in the data layer (cards.ts); these shape the request in and the
response out. POST's 201 body is the only response in the app that
ever contains a full card number. PATCH re-checks the status string
against an allowlist before it reaches setCardStatus, per
api-routes.md's "reject early and return."
/cards: nickname, merchant, masked number, category, limit, status,
created date, and the Issue card drawer. /cards/[id]: full record,
spend-vs-limit progress bar (amber at 80%+), and status history.

The issue drawer derives currency from the selected merchant rather
than offering a free choice (see the spec's Approach section for why),
disables the submit button while a request is in flight, and clears
the revealed number from state the moment the drawer closes by any
path — X, Escape, overlay click, or the Done button all funnel through
one onOpenChange handler. Freeze/unfreeze/cancel PATCH then
router.refresh() rather than navigating, so the page updates without a
full reload; cancel sits behind a confirm step since it's terminal.

StatusBadge's AnyStatus union now includes CardStatus rather than
introducing a second badge component. Cards added to the sidebar,
breadcrumbs, and siteConfig — the route existed nowhere reachable
before this.
@JJFromTenex

Copy link
Copy Markdown
Owner

Claude Code 101 — Repo Rescue

🏆 Build Battle Score: 90 / 100

One-line verdict: Far and away the most complete submission in the field — full state machine, honest masking, a currency-match guard the ticket never asked for, and an audit trail — undercut only by an unfilled epic and a Luhn implementation the diff never actually shows.

Core criteria — 92 / 100 (35%)

  1. Issue a card: ✅ — Drawer form takes nickname, merchant, limit, category; currency is derived from merchant rather than picked independently (deliberate, defended in the PR), appears in the list on success.
  2. Card list: ✅ — /cards shows nickname, merchant, masked number, category, limit, status, created date.
  3. Card detail: ✅ — full record, spend-vs-limit progress bar, status history.
  4. Generated numbers: ⚠️generateCardNumber() is imported from src/lib/cards.ts and tested against a /^4242\d{12}$/ regex in data/cards.test.ts, but src/lib/cards.ts and the claimed src/lib/cards.test.ts are not present in this diff (noted as truncated). I can't verify the Luhn math itself from what's shown, only that the format is asserted.
  5. Reveal once: ✅ — number only ever leaves the POST response body, cleared from client state on drawer close, never returned by GET routes or stored on Card.
  6. Server-side validation: ✅ — all four required checks plus a currency/merchant match, all in issueCard() on the server, exercised in data/cards.test.ts.

Correctness rules — 100 / 100 (20%)

  • Minor units: ✅ — parseAmountToMinorUnits is the single parse boundary in issueCard; everything downstream is integer arithmetic.
  • Luhn on 4242 BIN: ✅ — asserted by test and the PR's manual curl trace, though the generator source itself is outside this diff.
  • Masking: ✅ — Card type carries only last4; number never appears in GET /api/cards or GET /api/cards/[id], confirmed by the diff of those two routes.
  • State machine: ✅ — isValidCardTransition guards setCardStatus, cancelled is terminal and tested (data/cards.test.ts, "refuses to move a cancelled card anywhere").
  • Server-side validation: ✅ — all rejections live in data/cards.ts, not the client.

Context and planning — 40 / 100 (10%)

No epic in docs/epics/ appears in the diff. The PR description itself, though, reads like a plan was followed rather than improvised: verification steps mapped to acceptance criteria, bugs called out by file and root cause, deliberate note on what wasn't browser-tested. That's the 0.4 tier exactly — considered planning without a written epic.

Code quality — 95 / 100 (15%)

Tests sit beside the code they cover (data/cards.test.ts next to data/cards.ts), cover rejections and the terminal-state case, and would fail without the change. Conventions are followed (reuses parseAmountToMinorUnits, formatMoney, mirrors the queries.ts pattern explicitly). No DB, no console.log/TODO, labelled inputs with matching htmlFor/id. The queries.ts string-sort bug is a real, correctly diagnosed defect fixed with a test — full quality bonus earned there. The currency-mismatch fix is correctly not double-counted here since it's claimed under stretch. One gap: the actual Luhn/generator file isn't visible to confirm test-honesty claims about it.

PR description — 100 / 100 (5%)

Thorough: maps every criterion, states verification commands and real response bodies, discloses what wasn't click-tested in a browser, and is honest that seeded spent values are invented. Exactly the standard the rubric wants.

Stretch goals — 100 / 100 (15%)

Tier 1: ✅ freeze/unfreeze without reload · ✅ amber progress bar at 80% · ✅ category lock, no edit control · ✅ transition tests in data/cards.test.ts · ✅ written empty/error states (cards/page.tsx empty row, specific rejection copy in data/cards.ts).
Tier 2: ✅ currency-merchant match (data/cards.ts — client derives, server rejects mismatch against merchant.currency) · ✅ cancel with confirm, guarded PATCH, terminal render (app/cards/[id]/status-actions.tsx) · ✅ audit trail (card.history, rendered in app/cards/[id]/page.tsx) · ❌ idempotent issue (only a client-side disabled-submit, no server-side dedupe — explicitly UI-only debounce per the PR's own description) · ❌ honest spend (seeded cards carry admitted "invented" spent, not 0 and not derived from a real transaction feed, even though live-issued cards do start at 0).


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

One thing to do differently next time: Write the epic before touching code — the planning clearly happened (the PR description proves it), it just never got captured as a docs/epics/ artifact that the rubric can credit.

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

Generated files were skipped: build-battle/merchant-console/package-lock.json


Powered by Anthropic and Tenex

@JJFromTenex JJFromTenex closed this Sep 1, 2026
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