Skip to content

Returns money: refund to the original payment or to store credit, restocking fees, and a return that books as its own order - #33

Merged
goetchstone merged 1 commit into
mainfrom
feat/returns-money
Oct 16, 2026
Merged

goetchstone merged 1 commit into
mainfrom
feat/returns-money

Conversation

@goetchstone

Copy link
Copy Markdown
Owner

Summary

PR 2b of the stock-ledger plan: returns money, as you directed on 2026-10-16. A refund goes back the way it was paid, or onto the customer's account as store credit toward a new sale or a rewrite, and a return can keep a restocking fee. The customer never waits on the piece being put back or written off (#31's returned-stock queue).

Behind that, it fixes the money bugs listed in docs/domains/returns.md → "Returns money", each closed there:

  • a card refunded "to store credit" refunded the card and issued credit;
  • the processor was called inside the database transaction;
  • the refund route had no cap and no checks, and a second refund overwrote the first;
  • store credit could be spent on someone else's order;
  • the return detail screen crashed on load;
  • a refunded order showed as owing the refund again.

What changed

  • A return books as its own order (DECISIONS The README shows the project's health at a glance: CI, Security, CodeQL and license badges #20, lib/returns/bookReturn.ts). The first time money moves, the return becomes a RETURNED order:

    • one negative line for what came back (its share of the original line's net, tax and cost);
    • one positive line for the restocking fee;
    • an invoice dated that day, only when the original was invoiced.

    The original order is never rewritten. Once booked, the return can't be cancelled, and its order can't be edited or deleted through any order or line route (returnOrderEditProblem, one shared check).

  • Paying it out (lib/returns/refundReturn.ts):

    • First, what the original order still owes is paid from the return, as store credit spent on that order.
    • Then up to the cap: the value, less the fee, less refunds that are COMPLETED or PENDING.
    • Original payment: processRefund, refunded the way it was taken.
    • Store credit: no processor and no till.
  • processRefund in three steps:

    1. Reserve the refund row under the payment's lock.
    2. Call the processor outside any transaction, with the key holt-refund-<id> (Stripe and Square both take it now).
    3. Settle.

    A processor error leaves the row PENDING: still counted by the cap, out of balances and the journal. Retry reuses the same key. The method override is gone. The stale-payment sweeper and the open-checkout check skip PENDING refunds.

  • Store credit (lib/customerCredit.ts):

    • one writer per movement, in the caller's transaction, under a per-customer lock;
    • spent only on the same customer's order, never past zero (new CHECK Customer_creditBalance_nonnegative, mirrored in db-guards.sql).

    It's offered as a register tender, and as "Apply store credit" on an order's screen for rewrites and exchanges. The register's tender route is now payment.take: the MANAGER/ADMIN role list it had refused the Register role (a static read of resolveRoleListAccess). Its line is removed from role-array-api-routes.txt.

  • Restocking fee: a percent or an amount, typed per return, 0 by default, fixed by the first refund. A nonzero fee needs POS_TRANSACTIONS/"Restocking Fee" mapped. The migration adds that row unmapped; the demo seed maps it to a new 4-4950.

  • Journal:

    • A native return's order is recognised with its own negative lines.
    • Its fee line is booked to the fee account.
    • A refund on it settles its receivable (the reversesPaymentId skip now applies only off a return order).
    • The journal and the till's expected count leave out PENDING, FAILED and VOIDED payments; they used to include every row.
    • The daily reconciliation counts the fee account as revenue.
    • Unclassified Returns uses the journal's own return select.
  • Return detail:

    • One contract, lib/returns/returnDetailContract.ts, for the API and the screen.
    • A new refund panel.
    • The disposition card links to the returned-stock queue instead of three hardcoded store names (rule 61).
  • Migration 20261019120000_returns_money:

    • adds Return.returnOrderId, Return.restockingFeeLineItemId and Payment.returnId;
    • carries each refundPaymentId onto its refund row, then drops refundPaymentId and refundAmount;
    • adds the fee mapping row and the credit CHECK.
  • Docs:

Trace / Verification

  • Preflight (read-only, local databases): the demo database has 21 returns, all with a refund to carry over. No local database holds a negative credit balance, so the CHECK is fully validated.
  • The migration: a fresh scratch database built by npm run setup -- --scale=ci (all migrations, then the seed). prisma migrate diff --exit-code printed No difference detected. All 21 seeded returns name their line, and all 21 seeded refunds point at their payment.
  • The return day's journal, from a real database (returnMoney.integration.test.ts): $1,000 net, $63.50 tax, $400 cost, a 15% fee, refunded in cash. It came out Dr Sales 1,000, Dr Tax 63.50, Dr Inventory 400 / Cr COGS 400, Cr Restocking Fee 150, Cr Cash 913.50, with AR netting to zero and no Over/Short line. A written-off piece books to shrinkage instead.
  • Mutation checks, each restored afterwards; each fails its suite:
    • no return cap: returnMoney 3/12 failed;
    • the old journal guard: 2/12 (the day plugs);
    • the fee line not mapped in the journal: 1/12;
    • credit spent on anyone's order: 1/12;
    • no credit lock: 1/12 (two spends of the same $200 both succeed);
    • a processor refund never PENDING: 1/12;
    • the original's balance not paid first: 1/12;
    • no return-day invoice: 2/12;
    • a refunded return booked again: 1/12;
    • the return's order editable: 1/12;
    • the fee changeable after booking: 1/12;
    • the journal taking every payment: generateSalesJournal 1/20;
    • the sweeper failing PENDING refunds: pendingPaymentLifecycle 1/16;
    • proration ignoring quantity: returnMoney (unit) 1/9.
  • In the browser, on the seeded scratch database:
    • A new return created from New Return lands on its detail screen, which used to crash. It shows $3,903.30 back ($3,682.36 + $220.94 tax).
    • A 10% fee shows −$368.24; refunding the default to store credit records $3,535.06, and the change history shows the booking and the refund.
    • The database shows a RETURNED order with an invoice, and the credit issued against it.
    • On another order for the same customer, "Apply $500.00" brings the balance due to $0.
    • At the register, the same customer's order offers a Store Credit tender; paying $627.80 with it leaves $2,407.26 of credit.
    • This run found that seeded refunds were labelled "Store credit". Fixed: the label now comes from the method, and the seed links each refund to its payment.
  • Callers (grep -rn):
    • processRefund: the two refund routes.
    • recordPayment: the order payments route; recordPaymentTx is new and used by refundReturn.
    • customerCredit's old issueCredit/applyCredit had no callers. getBalance and adjustCredit keep their signatures.
    • The provider refund() is called only from paymentService.
  • Not verifiable here: a real Stripe or Square refund. The processor was stubbed, as stripeProvider.refund spied on: errors once, then succeeds. Idempotency keys are honoured by the processors, not by Holt; the retry window is set to Stripe's 24 hours, and Square's key lifetime is [NEEDS VERIFICATION].

Scan / gate state

npm run check:local on the branch:

Step Result
1 validate ✓ (0 errors; warnings 935 → 934)
2 test:coverage (all unit + all integration, merged) ✓: statements 70.03%, branches 58.46%, functions 75.43%, lines 71.35% (#32: 69.84 / 58.28 / 75.09 / 71.09)
3 Sonar not run: no SONAR_TOKEN locally
4 markdownlint ✓
5 Semgrep ✓ 0 findings
6 OSV skipped (no lockfile change)

After that run, one test was added: the edit-guard case in returnMoney.integration.test.ts, run on its own (12/12). The rebase onto main changed nothing in the tree: main is #32, byte-identical to the branch this was built on.

Your decisions (defaults built in, easy to change)

  1. Restocking fee: typed per return (built), or a store default percent?
  2. Is the fee taxed? Built: no, so the full tax goes back.
  3. Commission on the fee: built: the return claws back commission on what came back, as an imported return does; the fee isn't treated separately.

Not in this PR, and why

Tracked in docs/domains/returns.md → "Not done, tracked here":

  • A manual store-credit adjustment posts no journal entry.
  • A return whose original is invoiced only later is never reversed in the journal.
  • A write-off decided after the return day's journal is posted still books as restock on that day.
  • A PENDING refund older than 24 hours can only be voided.
  • Returns-desk refunds aren't tied to a till.
  • The register's "return mode" carts have no payout path.

Test plan

  • CI: Lint/Typecheck/Format/Test, integration, "Migrations match the schema", setup/seed/smoke, Semgrep, osv-scanner
  • After merge and the local rebuild: open a return, refund part of it to store credit, then spend the credit at the register
  • Map "Restocking Fee" (Admin › Setup › Accounting) on any deployment that will keep fees

🤖 Generated with Claude Code

…redit, a return can keep a restocking fee, and a native return books as its own order so the day's journal reverses the sale with no plug

Ledger plan PR 2b (docs/domains/returns.md "Returns money"; DECISIONS #20).

- lib/returns/: bookReturn (the return's own RETURNED order: a negative
  line for what came back, the fee line, an invoice when the original was
  invoiced), refundReturn (pays what the original owes first, then to the
  original payment or to store credit, under the return's cap),
  returnMoney (pure rules), returnDetail + returnDetailContract (one shape
  for the API and the screen, which crashed on load).
- processRefund in three steps: reserve under the payment's lock, call the
  processor outside any transaction with the key holt-refund-<id>, settle.
  A processor error leaves the refund PENDING and counted; retry reuses the
  key. No method override: a card is never refunded "to store credit".
- lib/customerCredit: one writer per credit movement, in the caller's
  transaction, under a per-customer lock, only on the customer's own
  order; CHECK Customer_creditBalance_nonnegative. A register tender and
  "Apply store credit" on an order spend it; the tender route is
  payment.take (it refused the Register role).
- Journal: return orders recognised with their own lines, the fee to
  POS_TRANSACTIONS/"Restocking Fee", refunds on them settle AR; PENDING,
  FAILED and VOIDED payments left out of the journal and the till.
- A return's order is locked against order and line edits; a booked
  return cannot be cancelled.
- Migration 20261019120000_returns_money: returnOrderId,
  restockingFeeLineItemId, Payment.returnId (refundPaymentId carried over
  and dropped), the fee mapping row, the credit CHECK.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@goetchstone
goetchstone merged commit de41316 into main Oct 16, 2026
12 checks passed
@goetchstone
goetchstone deleted the feat/returns-money branch October 17, 2026 16:42
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.

1 participant