b8e521f4e7909524114df1500fbb7ae926aa4672
9
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 popertots
|
faceb9809c |
fix: review-loop A — discount credit on admin payments, campaign over-credit cap, sweep replay window, dedup refund revalidation, duplication/modularisation, GBP pence naming
Round-A fresh review (6 agents) + fix + secondary cross-cutting + verification rounds: - F1: campaign discounts reduce the charged amount (deposit credit + admin PaymentModal discounted total); capDiscountToRemainingObligation prevents over-credit at completion in all four campaign blocks - F2: sweep replay rescue distinguishes legitimate same-key retries (21h window) from expired-key new charges; ccof blind-fails leave pending + CRITICAL instead of clawing back - F3: post-start online overflow carved as a tip record (mirrors terminal split builder) - A1: single-source Square decline-code classification (till delegates to square.IsDefinitivePaymentError) - A2/A5: refund attempt-cap literals consolidated; refund-failure counter capped + reset on terminal resolutions + admin notifications - A3/A9: idempotency helpers adopted across derivations; IsExplicitDevOrMockEnv relocated + all gates unified (incl. health-check) - A7: 2FA user+IP limiter + TRUST_PROXY_HEADERS startup warning; SNAPSHOT_ENC_KEY startup validation; TWO_FACTOR_PEPPER docs corrected - A8: snapshot encryption on all 6 write sites + marker-aware reuse paths; MPV->SPV effective voucher type (single VAT point) - A10/A11/A12/A16: gift-card slot scan advances past failed; amount-aware refund reconciliation; completed-booking refund re-check; PaymentWasRefunded on SquareClient interface - Dedup refund revalidation on tip/terminal/gift-card paths; sweep acknowledged_at IS NULL parity; refund-notification single source (exported payments.InsertRefundFailedNotifications) - Duplication/modularisation round: shared frontend helpers (sanitizeDecimalInput, campaignDiscountCents, twoFactorBlocksSavedCards getter, generateUUID), single-source MaxIdempotencyKeyLength, notification-helper consolidation, snapshot-guard comments - Cross-cutting GBP rename: Cents->Pence across backend + frontend + tests (26 identifiers, 16 files) - Tests: 11 behavior-change tests updated to new invariants; coverage for fixed functions; frontend vitest 55 tests; docs corrected (test counts, 2FA delivery, pre-launch checklist, resolution status) - gitleaks: allowlist backend/internal/square test fixtures (mock idempotency keys) All 25 backend packages pass; frontend 55/55 + build clean; env-docs 41/41. |
||
|
popertots
|
4b28e93710 |
fix: tip double-count, fully-paid auto-completion, discount-refund hardening
Tip double-count (root cause of £33.75 vs £28.75 display):
- Remove mock's fixed +500p auto-tip when AllowTipping is true (square_dev.go) —
real Square only enables a terminal prompt, it never adds a tip to the amount
- Set AllowTipping=false in CreateTerminalPayment: the frontend already embeds
the tip in the amount, so the terminal must not prompt for a second tip
- M4 tip split now derives the tip as charged amount minus remaining booking
value ('after 100% is tips'), not from Square's TipAmount field
- Success screens divide paymentResult.amount by 100 (pence -> pounds) in both
PaymentModal and UserPaymentModal
Fully-paid bookings auto-complete:
- Extract ApplyBookingCompletionSideEffects into payments package (shared by
admin progress endpoint and payment paths; avoids circular import)
- Add bookingIsFullyPaid + completeFullyPaidBooking: when completed non-tip
payments reach 100% of the booking total, an active booking transitions to
'completed' so it leaves the admin Current Appointment view
- Wired into CreateBookingPayment (inside tx) and GetCheckoutStatus (terminal,
after commit); completion side-effects (loyalty, campaigns, deposits_required)
fire identically to the manual progress endpoint
- Add /admin/bookings/{id}/refund route (AdminRefundBooking)
Discount-refund hardening:
- RefundPayment explicitly rejects discount/on_the_house payments (was relying
on the incidental NULL-square_payment_id guard)
- Hide the Refund button for discount/on_the_house payments in EditBookingModal
- Cancel-refund estimate in BookingModal also excludes on_the_house
- Cancellation refund loop + GetBookingPaymentInfo + GetBookingRefundableAmountCents
exclude payment_type='tip' from refundable totals
Tip flow (start-time guard) fixes tests:
- Tip tests updated to use past-dated bookings (tips now require booking started)
Tests:
- m4_tip_refund_redesign_test.go (tip split, refund exclusion, admin refund cap)
- m5_fully_paid_completion_test.go (online + terminal full-payment completion,
partial stays active, tip excluded, cancelled stays cancelled)
- Full suite passes with -race (25 packages)
|
||
|
popertots
|
a8d54f1e2a |
Fix review findings: aggregated-refund/saved-card/legacy-refund idempotency keys, structured Square error classification, CSP for Square SDK
Money-safety idempotency fixes (external review bugs 1-3): - processChargeGroup: aggregated refund key now hashes the sorted pending-row set (chargeID-square-agg-<sha256 suffix>) so a changed group can never mark a new row completed against an old smaller refund; >45-char chargeIDs use a hashed prefix instead of verbatim truncation (which would collide charges on Square's global key dedup). Same-set crash-retry keeps Square's dedup. - CreateTerminalPayment saved_card: two-tier idempotency key — client-supplied per-attempt UUID preferred (distinct identical charges no longer collapse), deterministic booking+type+amount+card fallback for no-key retry safety. PaymentModal sends a per-charge UUID cleared after success. - ensureRefundKey: legacy NULL-key manual refunds persist a generated key to the row BEFORE the Square call (race-safe AND idempotency_key IS NULL guard), so a lost-response retry reuses the key and never double-refunds. Wired into resumeManualPendingRefund and the sweep's manual-retry loop. Classification + money-safety hardening: - till.go/sweep.go: structured square.ErrorCode/IsNotFound are authoritative when present; message-substring matching only for non-structured errors (dev mock, client-side status errors). Fixes fragile string-matching driving sweep retries and gift-card clawbacks. - SaveCardForUser: ON CONFLICT (user_id, square_card_id) DO NOTHING + re-select (was a latent UNIQUE-violation 500 on save-card retry). - CreateBookingPayment: partial payments re-validated against remaining balance inside the advisory lock (closes concurrent-overpayment race). - InvalidateSquareCustomerCache on GDPR erasure paths (account.go, time-blockers.go stale-guest anonymization). - GetUserGiftCardBalanceAdmin: in-handler admin check (defense-in-depth). - getCheckoutHTTP: warn on multi-payment checkouts instead of dropping payments[1:]. - Cash/giftcard terminal branch: removed dead idempotency SELECT, "tip-" -> "till-" prefix. - UserPaymentModal: removed vestigial polling state; proper interval cleanup. - account/+page.svelte: gift-card redeem dialog links /terms. - nginx CSP: allow *.squarecdn.com and js.squareup.com so the Square Web Payments SDK + card iframe can tokenize behind the proxy. Tests: +8 regression tests covering changed-set refund keys, legacy NULL-key single-refund, saved-card client-key dedup/no-dedup, concurrent partials, and cache invalidation. Full suite + race detector clean via run-tests.sh lockfile. |
||
|
popertots
|
352f9e50d4 |
Fix till-sale money safety: sweep gift-card clawback, orphaned-checkout cancel, cash-retry reconciliation
HIGH-1: a till sale whose card-machine checkout is provably dead, or whose Square reconcile proves the charge never landed, now claws back the funded gift card atomically with the failed mark (claim-first gating UPDATE serializes against the admin retry; blind-fail and ambiguous/lost-response rows never claw back, and a bare CANCEL_REQUESTED is not treated as proof of non-completion). HIGH-2: a card-machine checkout created at Square but not committed is cancelled on any pre-commit failure. HIGH-3: a cash/on_the_house retry of a pending card sale is reconciled at Square first (COMPLETED rescues + refuses cash; NOT_FOUND/FAILED/CANCELED allows cash; lost-response forces the card-method retry; ambiguous rejects). GetTillCheckoutStatus no longer resurrects a swept-failed sale, and the cash-completion UPDATE checks RowsAffected so the admin is never told to take cash against an already-resolved sale. 23 new tests covering the clawback matrix, the reconcile-or-reject matrix, cancel-on-error, and a till concurrency test. |
||
|
popertots
|
8416f033d6 |
Fix gift-card transfer deadlock: lock card rows in deterministic ID order
TransferGiftCard locked source-then-destination in caller-chosen order, so concurrent cross-transfers (A→B and B→A) acquired row locks in opposite orders and deadlocked (SQLSTATE 40P01), aborting one transfer with a generic 500. Both rows are now locked in lexicographically sorted ID order (case-insensitive, matching the 12-hex mixed-case card codes), with the scanned values mapped back to source/destination roles afterward. Adds a deterministic concurrency regression test that holds both row locks on dedicated connections to force the AB-BA cycle, proving the old code deadlocks and the new code serializes cleanly. |
||
|
popertots
|
738f6b6a51 |
Expand payment test coverage: lock contention, nonce-direct, provisional rows, GDPR scrub, validators
Close the coverage-gap round: terminal CreateCheckout-failure marks the provisional row failed, GetCheckoutStatus reference_id mismatch 400, deadline wire shape, concurrent loyalty redemption 409, delete_guest_user + stale-guest saved-card scrubbing, ValidateAmount and isTokenLike direct units, bounded try-lock timeout, buildSplitRecords tip-overflow, and concurrent same-key dedup for gift card / booking / tip / checkout. |
||
|
popertots
|
7439fa86c1 |
Fix payment review round 3: saved-card idempotency, stale-pending sweep, webhook fail-closed
R1/R4: saved_card branch in CreateTerminalPayment now mirrors CreateTipPayment - advisory lock (crussell:payment:<bookingID>) serializes concurrent double-clicks - deterministic key bookingID-sc-type-amount-cardID (<=45 chars) so a lost-response retry derives the same key and dedups instead of double-charging - idempotency switch inside the lock: completed -> dedup, pending -> reuse with pence amount-guard, failed -> clean 409 - success response includes card_brand/card_last4 (frontend already reads them) R2: add 'failed' case to all four retry switches (tip, booking, gift card, till) - a swept/definitively-rejected record returns 409 instead of 500-ing on the idempotency_key UNIQUE constraint R3: extend SweepStalePendingPayments to till_sales card rows - sweeps pending till_sales (online_square/in_person_card) past Square's ~24h key retention, closing the double-charge window for till sales - swept rows logged with the same CRITICAL manual-reconciliation marker as the refund sweep Webhook fail-closed: reject 503 when SQUARE_WEBHOOK_SIGNATURE_KEY unset, 403 on bad signature (was: skip verification in dev) Refund status resolution: refunds now resolve by Square status (COMPLETED/PENDING/FAILED/REJECTED) instead of assuming completed; real error codes (REFUND_AMOUNT_INVALID, PAYMENT_NOT_REFUNDABLE, REFUND_ALREADY_PENDING) added to the definitive/processed classification HTTP client: CreateCard key truncated to <=45 chars, device_options always sent (env SQUARE_TERMINAL_DEVICE_ID fallback), processing_fee reads amount_money, ListCards cursor loop, refund keys hashed to <=45 chars Other fixes: payment/till/gift-card advisory-lock + FOR UPDATE asymmetries, GetPaymentByID NULL scans, loyalty redemption lock, card upsert on conflict, mock ccof: prefix parity, IsValidSquareCheckoutID for real Square IDs, isAdminRequest defense-in-depth on all 6 admin payment handlers, webhook signature docs, M8/L5 debug markers removed Docs: README/FC/TM/Overview updated (22 jobs, 20 CRITICAL sites, 23-section GDPR export, sweep jobs, webhook fail-closed); P11 plan marks remaining items (sandbox smoke test, M-8 customer_id, saved-card key dedup trade-off) as deferred with rationale; gap backlog pruned of completed items |
||
|
popertots
|
9ff591fa4e |
Fix payment review round 2: refund idempotency, pending-resume safety, terminal-completion lock
Refund idempotency (P2):
- RefundRequest gains an optional client idempotency_key: two DISTINCT equal
partial refunds of one payment no longer collide on the amount-derived key
(the second was silently swallowed as a dedup)
- Extract resumeManualPendingRefund: resumes a pending refund with the row's
OWN stored key, so Square's key dedup returns the original refund if the
prior attempt completed — never issues a second
- (payment, amount) pending fallback: when the exact-key lookup misses (admin
reopened the modal, new UUID), resume the matching pending row instead of
creating a second pending row the sweep would double-process
- 409 in-flight guard: if a pending refund exists for the payment but no
same-amount row matches, reject a different-amount refund (money state at
Square is unknown — no new refund is safe until it resolves)
- Frontend (EditBookingModal): UUID per refund attempt, reused on retry,
mirroring the tip flow
Terminal completion (P3):
- GetCheckoutStatus serializes on pg_advisory_lock('crussell:terminal:' ||
SquarePayID) on a pinned connection — concurrent polls of the same checkout
can no longer both pass the dedup SELECT and race the UNIQUE constraint
Card-on-file / doc-only:
- Document why CreateCardOnFile is NOT rolled back on payment failure
(deterministic sha256 retry returns the same card; deletion breaks it)
- Document HasCompletedPayment's deliberate 'tip' exclusion
Regression tests:
- TestRefund_TwoEqualPartialRefunds_ClientKeyDisambiguates
- TestRefund_PendingResume_NewKeyAfterModalReopen (proves stored-key resume)
- TestRefund_PendingResume_DifferentAmountRejected (409 + no second row)
- TestRefund_GuardCountsPendingRefunds updated: 400 -> 409 (in-flight guard
fires first — strictly safer, blocks before any Square attempt)
- TestGetCheckoutStatus_ConcurrentPolls_SingleRecord (real two-goroutine race)
|
||
|
popertots
|
53ca89603d |
Fix payment review round: till integrity, HTTP client tests, concurrency tests, card-selection consolidation
Addresses the payment review (all 10 blocking + 2 minor findings): Till money-integrity (CreateTillSale): - Add pg_advisory_lock on the idempotency key (concurrent same-key double-funding race) - Guard amount on pending-reuse retry (mirrors tip/gift-card guards) - Explicitly complete the row for cash/on_the_house pending-reuse - Reject method-switch on a live card-machine checkout (double-charge guard) - 3 regression tests (amount-mismatch, cash-completes-row, method-switch) BookingFlow: - Fetch saved cards at the deposit step (was dead code) - Charge the server-computed deposit_amount, not the client estimate HTTP client tests (was untested): doJSON error parsing, refund sentinel classification, payment/refund/card wire shapes, checkout polling states, list-refunds pagination + 20-page guard, sha256 card idempotency key Concurrency regression tests: real two-goroutine races for BuyGiftCard, tip, and booking-payment locks asserting exactly-one record each Frontend: - Fix CRIT-1: zero-saved-card users blocked (all flows now handle it) - Consolidate tip/deposit/Buy-Gift-Card card UI onto CardSelection - Explicit save-card consent checkbox (was silent/inconsistent) - Fix stale saved-card field names in BookingFlow (last4 -> last_4) - Unique instance ids (crypto.randomUUID) in CardSelection/SquareCardInput - UserPaymentModal: keep card form mounted on error + Try Again button Health/docs: /api/health reports square state (mock/ok, was not_implemented), close P1 backlog, correct stale webhook and env-var claims |