From ae8735ba2fe370b7d9ce9cd6ea4a764500e6d968 Mon Sep 17 00:00:00 2001 From: Stephen Adamson Date: Sat, 1 Aug 2026 18:18:39 +0100 Subject: [PATCH] Close refund system and gate raw-PAN card entry Refund system (Round 3 fixes + follow-up + alignment): - Serialize cancellation refunds against the manual handler via per-payment advisory locks taken before the prior-refunds read (pg_advisory_xact_lock, ascending, same crussell:refund: key space) - Aggregate pending cancellation refunds into ONE Square refund per charge (stable charge-level -square-agg key); atomic group UPDATE keeps crash-retry amounts identical for Square key-dedup - Persist paymentID-square-amount idempotency keys on cancellation refunds; scheduler reads the stored key (legacy fallback for old rows) - Add sweep-pending-square-refunds cron (*/5, concurrency 1) with refund_attempts cap; sweep retries stale manual pending refunds with each row's own stored idempotency key - Reconcile at Square (GET /v2/refunds ListPaymentRefunds) before every terminal failed transition: tri-state result leaves rows pending on reconcile error instead of false-failing; PAYMENT_ALREADY_REFUNDED resolves to completed - Move over-refund guard inside the lock, counting completed + pending (excluding failed); ErrRefundDeclined distinguishes definitive vs ambiguous outcomes - forgiveFees now executes a real full refund (forceFullRefund override) with admin_forgiven_fees reason threaded to Square - Surface failed card refunds in the admin notification centre (refund_failed enum, RETURNING-id pre-pass inserts, NOT EXISTS dedup) - Dedup double-cancel refund inserts via ON CONFLICT (idempotency_key) DO NOTHING without consuming refundRemaining Frontend: - Remove all raw-PAN card entry: zero card_number/card_cvc/new_card_token in request bodies; gate new-card entry behind CardEntryUnavailable notice + newCardDisabled prop across all 8 flows - Delete hand-rolled CardInput.svelte; keep CardSelection saved-card UI and CardEntryUnavailable fallback - Update cancellation-policy page to in-person cash pickup wording Tests: - Rewrite the two amount-blind dedup tests to assert real money movement (single call, aggregated amount, shared refund ID) - Add coverage: manual refund vs cancellation serialization (concurrent goroutines), reconcile error vs no-match branches, stale manual retry, forgive-fees real refund row + reason, double-cancel dedup, mock refund key dedup, ListPaymentRefunds filtering - Fix time-dependent booking flakes with fixtures.NextWorkingDayAt - 25/25 packages pass; -race clean on payments/square/db/jobs/bookings --- backend/handlers/bookings/deposit_test.go | 121 +- backend/handlers/bookings/manage.go | 30 +- backend/handlers/bookings/overlap_test.go | 10 +- backend/handlers/payments/handlers.go | 313 +++- .../handlers/payments/payment_status_test.go | 131 ++ backend/handlers/payments/payments_test.go | 175 ++ backend/handlers/payments/refunds.go | 1041 ++++++++++- backend/handlers/payments/refunds_test.go | 1580 ++++++++++++++++- backend/handlers/payments/service.go | 33 +- backend/handlers/payments/till.go | 127 +- backend/handlers/payments/till_test.go | 294 +++ backend/internal/jobs/cleanup.go | 9 + backend/internal/jobs/scheduler_test.go | 11 +- backend/internal/square/square.go | 9 +- backend/internal/square/square_dev.go | 53 +- backend/internal/square/square_dev_test.go | 158 ++ backend/internal/square/square_http_client.go | 63 +- backend/internal/square/types.go | 28 +- backend/testutils/fixtures/fixtures.go | 10 + .../account/UserBookingModal.svelte | 195 +- .../admin/GiftCardsManagement.svelte | 272 +-- .../lib/components/booking/BookingFlow.svelte | 147 +- .../payments/CardEntryUnavailable.svelte | 21 + .../lib/components/payments/CardInput.svelte | 101 -- .../components/payments/CardSelection.svelte | 168 +- .../payments/UserPaymentModal.svelte | 24 +- frontend/src/lib/constants/payments.ts | 3 + frontend/src/routes/account/+page.svelte | 450 +---- .../routes/admin/notifications/+page.svelte | 4 +- .../routes/cancellation-policy/+page.svelte | 5 +- frontend/src/routes/pay-tip/[id]/+page.svelte | 204 +-- frontend/src/routes/tip/+page.svelte | 203 +-- init-scripts/init-script.sql | 4 +- 33 files changed, 4129 insertions(+), 1868 deletions(-) create mode 100644 frontend/src/lib/components/payments/CardEntryUnavailable.svelte delete mode 100644 frontend/src/lib/components/payments/CardInput.svelte create mode 100644 frontend/src/lib/constants/payments.ts diff --git a/backend/handlers/bookings/deposit_test.go b/backend/handlers/bookings/deposit_test.go index 485b376..957ce30 100644 --- a/backend/handlers/bookings/deposit_test.go +++ b/backend/handlers/bookings/deposit_test.go @@ -6,14 +6,17 @@ import ( "bytes" "context" "encoding/json" + "fmt" "net/http" "strings" + "sync" "testing" "time" "crussell/clock" "crussell/db" "crussell/handlers/payments" + "crussell/internal/square" "crussell/mw" "crussell/testutils" "crussell/testutils/fixtures" @@ -360,9 +363,11 @@ func TestRequestEditHandler_NoticePeriod_SetsNoShowWarningHeader(t *testing.T) { t.Fatalf("failed to create service: %v", err) } - // Booking starting in 48 hours (24-72h window, no payments). - midRange := clock.Now().Add(48 * time.Hour) - bookingID, err := fixtures.CreateTestBookingAtTime(tx, userID, serviceID, midRange) + // Booking 24-72h from now (no-show warning tier) at a fixed working-hour slot: + // 10:00 UTC two days out is always 35-58h away, so the warning fires but the + // "too close" 403 never does, at any wall-clock hour. + bookingTime := fixtures.NextWorkingDayAt(2, 10) + bookingID, err := fixtures.CreateTestBookingAtTime(tx, userID, serviceID, bookingTime) if err != nil { t.Fatalf("failed to create booking: %v", err) } @@ -373,7 +378,8 @@ func TestRequestEditHandler_NoticePeriod_SetsNoShowWarningHeader(t *testing.T) { } token := jwt.GenerateUserToken(userID) - newTime := midRange.Add(48 * time.Hour) + // 10:00 UTC four days out — same slot as bookingTime, inside working hours. + newTime := bookingTime.Add(48 * time.Hour) handler := http.HandlerFunc(RequestEditHandler) w := makeRequest(handler, "POST", "/api/bookings/"+bookingID+"/edit-request", map[string]interface{}{ @@ -574,6 +580,12 @@ func TestAdminCancelBookingHandler_ForgiveFeesFullRefund(t *testing.T) { t.Fatalf("failed to create user: %v", err) } + // A real admin user so the refund row's created_by FK is satisfied. + adminID, err := fixtures.CreateTestAdminUser(tx) + if err != nil { + t.Fatalf("failed to create admin user: %v", err) + } + serviceID, err := fixtures.CreateTestService(tx) if err != nil { t.Fatalf("failed to create service: %v", err) @@ -591,20 +603,45 @@ func TestAdminCancelBookingHandler_ForgiveFeesFullRefund(t *testing.T) { t.Fatalf("failed to confirm booking: %v", err) } - _, err = fixtures.CreateTestPayment(tx, bookingID, 100, "online_square", "full", "completed") + paymentID, err := fixtures.CreateTestPayment(tx, bookingID, 100, "online_square", "full", "completed") if err != nil { t.Fatalf("failed to create payment: %v", err) } + // Give the payment a Square reference so the post-commit sweep does NOT + // terminal-pre-pass it to 'failed' (NULL square refs are unresolvable). + _, err = tx.Exec(ctx, "UPDATE payments SET square_payment_id = 'sqp_admin_forgive_fees' WHERE id = $1", paymentID) + if err != nil { + t.Fatalf("failed to set square_payment_id: %v", err) + } + + // Commit the setup so the handler runs at pool level: AdminCancelBookingHandler + // acquires pg_advisory_xact_lock on the card payments, and inside the test-env + // outer per-test transaction (savepoints don't release xact locks) the locks + // would deadlock the post-commit sweep's session locks. Pool-level mirrors + // production, where the handler's tx commits and releases the locks. + pgxTx := db.TxFromContext(ctx) + if pgxTx == nil { + t.Fatal("no transaction in context") + } + if err := pgxTx.Commit(ctx); err != nil { + t.Fatalf("failed to commit test tx: %v", err) + } + freshCtx := context.Background() + + // Make the post-commit refund processing leave the row 'pending' (ambiguous + // Square transport failure → refund_attempts=1) so the test can assert the + // row exists as pending with the full amount. + origSquare := payments.SquareClient + forgiveFeesClient := &forgiveFeesAmbiguousClient{SquareClient: square.NewDevClient()} + payments.SquareClient = forgiveFeesClient + defer func() { payments.SquareClient = origSquare }() w := serveChiHandler(AdminCancelBookingHandler, "POST", "/api/admin/bookings/"+bookingID+"/cancel", "/api/admin/bookings/{id}/cancel", map[string]interface{}{ "forgive_fees": true, }, func(baseCtx context.Context) context.Context { baseCtx = context.WithValue(baseCtx, mw.UserRoleKey, "admin") - adminToken := jwt.GenerateAdminToken() - if info := extractUserFromTestJWT(adminToken); info != nil { - baseCtx = context.WithValue(baseCtx, mw.UserIDKey, info.userID) - } - return db.ContextWithTx(baseCtx, db.TxFromContext(ctx)) + baseCtx = context.WithValue(baseCtx, mw.UserIDKey, adminID) + return baseCtx }) if w.Code != http.StatusOK { @@ -625,6 +662,70 @@ func TestAdminCancelBookingHandler_ForgiveFeesFullRefund(t *testing.T) { if refundCalc["refundable_amount"] != 100.0 { t.Errorf("expected refundable_amount 100, got %v", refundCalc["refundable_amount"]) } + + // The forgiven-fees FULL refund must actually execute — a refund row for + // the whole £100 must exist as 'pending' (created by ProcessCancellationRefundTx, + // resolved post-commit), not just a synthetic response claim. + var refundCount int + err = db.Conn.QueryRow(freshCtx, + "SELECT COUNT(*) FROM refunds WHERE booking_id = $1", bookingID).Scan(&refundCount) + if err != nil { + t.Fatalf("failed to query refunds: %v", err) + } + if refundCount < 1 { + t.Errorf("expected at least 1 refund row for forgiven fees full refund, got %d", refundCount) + } + var refundStatus string + var refundAmount float64 + err = db.Conn.QueryRow(freshCtx, + "SELECT status, amount FROM refunds WHERE booking_id = $1 LIMIT 1", bookingID).Scan(&refundStatus, &refundAmount) + if err != nil { + t.Fatalf("failed to query refund row: %v", err) + } + if refundStatus != "pending" { + t.Errorf("expected refund status 'pending', got %q", refundStatus) + } + if refundAmount != 100 { + t.Errorf("expected refund amount 100, got %.2f", refundAmount) + } + + // The Square sweep must be called with the forgiven-fees reason — the + // reason is threaded from the refund rows through to the post-commit + // ProcessPendingSquareRefunds call (P3b), not the generic admin_cancelled. + calls := forgiveFeesClient.refundCalls() + if len(calls) == 0 { + t.Fatal("expected the post-commit sweep to call Square RefundPayment") + } + if calls[0].Reason != "admin_forgiven_fees" { + t.Errorf("expected Square refund reason %q, got %q", "admin_forgiven_fees", calls[0].Reason) + } +} + +// forgiveFeesAmbiguousClient simulates a transport-level Square failure so the +// ForgiveFees test's post-commit refund processing leaves the row 'pending'. +// It also records every RefundPayment request so the test can assert the +// forgiven-fees reason reaches Square. +type forgiveFeesAmbiguousClient struct { + square.SquareClient + mu sync.Mutex + calls []square.RefundPaymentReq +} + +func (c *forgiveFeesAmbiguousClient) RefundPayment(ctx context.Context, req square.RefundPaymentReq) (*square.RefundResult, error) { + c.mu.Lock() + c.calls = append(c.calls, req) + c.mu.Unlock() + return nil, fmt.Errorf("network error: connection reset by peer") +} + +func (c *forgiveFeesAmbiguousClient) refundCalls() []square.RefundPaymentReq { + c.mu.Lock() + defer c.mu.Unlock() + return append([]square.RefundPaymentReq(nil), c.calls...) +} + +func (c *forgiveFeesAmbiguousClient) ListPaymentRefunds(ctx context.Context, paymentID string, beginTime time.Time) ([]square.RefundResult, error) { + return nil, fmt.Errorf("network error: connection reset by peer") } func TestAdminCancelBookingHandler_NormalRefundOver72h(t *testing.T) { diff --git a/backend/handlers/bookings/manage.go b/backend/handlers/bookings/manage.go index 38ba1ce..4e3a95f 100644 --- a/backend/handlers/bookings/manage.go +++ b/backend/handlers/bookings/manage.go @@ -198,20 +198,20 @@ func AdminCancelBookingHandler(w http.ResponseWriter, r *http.Request) { // Status update succeeded — now process the refund in the SAME transaction // so that a commit failure rolls back both the status change and the refund. - if forgiveFees && totalPaid > 0 { - refundResult = &payments.RefundCalculationResult{ - TotalPrePaid: totalPaid, - RefundableAmount: totalPaid, - KeptAmount: 0, - Tier: "admin_full_refund", + // refundReason threads through to the refund rows AND the post-commit Square + // sweep so forgiven-fee bookings are labelled "admin_forgiven_fees" (audit). + refundReason := "admin_cancelled" + if totalPaid > 0 { + if forgiveFees { + refundReason = "admin_forgiven_fees" + // Full refund of the net pre-paid amount regardless of notice tier — + // the override makes the refund actually execute (rows created and + // swept), instead of the old synthetic response-only claim. + refundResult, err = payments.ProcessCancellationRefundTx(r.Context(), tx, bookingID, totalAmount, totalPaid, payInfo.StartTime, clock.Now(), refundReason, &adminID, true) + } else if calculatedRefund { + refundResult, err = payments.ProcessCancellationRefundTx(r.Context(), tx, bookingID, totalAmount, totalPaid, payInfo.StartTime, clock.Now(), refundReason, &adminID, false) } - } - if calculatedRefund { - var calc *payments.RefundCalculationResult - calc, err = payments.ProcessCancellationRefundTx(r.Context(), tx, bookingID, totalAmount, totalPaid, payInfo.StartTime, clock.Now(), "admin_cancelled", &adminID) - if err == nil { - refundResult = calc - } else { + if err != nil { refundFailed = true log.Printf("ALERT: AdminCancelBookingHandler — ProcessCancellationRefundTx failed for booking %s after status was updated to we_cancelled. Refund was NOT processed. The transaction WILL be committed (cancellation stands, no refund). Error: %v", bookingID, err) } @@ -275,8 +275,8 @@ func AdminCancelBookingHandler(w http.ResponseWriter, r *http.Request) { // Process pending Square refunds after the transaction commits successfully. // This ensures Square API calls only happen if the DB records persist. - if calculatedRefund { - payments.ProcessPendingSquareRefunds(r.Context(), bookingID, "admin_cancelled") + if totalPaid > 0 { + payments.ProcessPendingSquareRefunds(r.Context(), bookingID, refundReason) } if refundFailed || (refundResult != nil && refundResult.RefundableAmount > 0) { diff --git a/backend/handlers/bookings/overlap_test.go b/backend/handlers/bookings/overlap_test.go index 7508567..2dd6e23 100644 --- a/backend/handlers/bookings/overlap_test.go +++ b/backend/handlers/bookings/overlap_test.go @@ -1852,8 +1852,14 @@ func TestAdminApproveEditRequest_EvictsPendingRelease(t *testing.T) { } dur := durationMinutes(t, ctx, tx, serviceID) - // Use a booking <48h from now so RequestEdit does NOT auto-approve - nearTime := clock.Now().Add(40 * time.Hour).Truncate(time.Second) + // Use a booking 24-48h from now at a fixed working-hour slot so RequestEdit + // neither 403s (<24h away = too close) nor auto-approves (>48h away would + // consume the edit request before the admin can approve it). A raw + // clock.Now().Add(Kh) can land after 19:00 London and fail closing hours. + nearTime := fixtures.NextWorkingDayAt(1, 10) + if nearTime.Sub(clock.Now()) < 24*time.Hour { + nearTime = fixtures.NextWorkingDayAt(2, 10) + } bookingA, err := fixtures.CreateTestBookingAtTime(tx, userID, serviceID, nearTime) if err != nil { diff --git a/backend/handlers/payments/handlers.go b/backend/handlers/payments/handlers.go index c022ecd..97cc2b7 100644 --- a/backend/handlers/payments/handlers.go +++ b/backend/handlers/payments/handlers.go @@ -646,10 +646,14 @@ func GetCheckoutStatus(w http.ResponseWriter, r *http.Request) { if paymentResult.Status == "COMPLETED" { service := NewPaymentService() - idempotencyKey := bookingID + "-terminal-" + strconv.FormatInt(paymentResult.Amount, 10) + // Deterministic idempotency key derived from booking + amount + Square + // payment ID. The Square payment ID disambiguates two distinct + // equal-amount charges on the same booking, so equal amounts never + // collide on the UNIQUE constraint. + idempotencyKey := bookingID + "-terminal-" + strconv.FormatInt(paymentResult.Amount, 10) + "-" + paymentResult.SquarePayID - // Begin the transaction BEFORE the idempotency check so it's atomic - // with the payment insert. + // Begin the transaction BEFORE the dedup lookup so it's atomic with the + // payment insert. tx, err := db.Conn.Begin(r.Context()) if err != nil { log.Printf("Failed to begin transaction: %v", err) @@ -662,26 +666,26 @@ func GetCheckoutStatus(w http.ResponseWriter, r *http.Request) { } }() - // Check for existing payment inside the transaction. + // Dedup by Square payment ID: a double poll of the same terminal + // checkout must return the existing payment row instead of inserting a + // duplicate (which previously 500'd on the idempotency-key UNIQUE + // violation after the customer had already paid). var existingID string - var existingSquarePayID sql.NullString if err := tx.QueryRow(r.Context(), ` - SELECT id, COALESCE(square_payment_id, '') FROM payments - WHERE booking_id = $1 AND idempotency_key = $2 - `, bookingID, idempotencyKey).Scan(&existingID, &existingSquarePayID); err == nil { - if existingSquarePayID.Valid && existingSquarePayID.String == paymentResult.SquarePayID { - if err := json.NewEncoder(w).Encode(PaymentStatusResponse{ - Status: "COMPLETED", - PaymentID: existingID, - Amount: paymentResult.Amount, - CardBrand: paymentResult.CardBrand, - CardLast4: paymentResult.CardLast4, - ReceiptURL: paymentResult.ReceiptURL, - }); err != nil { - log.Printf("Failed to encode JSON response: %v", err) - } - return + SELECT id FROM payments + WHERE booking_id = $1 AND square_payment_id = $2 + `, bookingID, paymentResult.SquarePayID).Scan(&existingID); err == nil { + if err := json.NewEncoder(w).Encode(PaymentStatusResponse{ + Status: "COMPLETED", + PaymentID: existingID, + Amount: paymentResult.Amount, + CardBrand: paymentResult.CardBrand, + CardLast4: paymentResult.CardLast4, + ReceiptURL: paymentResult.ReceiptURL, + }); err != nil { + log.Printf("Failed to encode JSON response: %v", err) } + return } else if !errors.Is(err, pgx.ErrNoRows) { log.Printf("Failed to check for existing payment: %v", err) } @@ -1662,17 +1666,9 @@ func RefundPayment(w http.ResponseWriter, r *http.Request) { return } - alreadyRefunded, err := service.GetAlreadyRefundedAmount(r.Context(), paymentID) - if err != nil { - log.Printf("Failed to get already refunded amount: %v", err) - http.Error(w, "internal server error", http.StatusInternalServerError) - return - } - - if req.Amount+alreadyRefunded > int64(math.Round(payment.Amount*100)) { - http.Error(w, "Refund amount exceeds payment amount", http.StatusBadRequest) - return - } + // Deterministic idempotency key so a same-key retry (network timeout) + // does not create a second Square refund. + idempotencyKey := paymentID + "-refund-" + strconv.FormatInt(req.Amount, 10) // Serialize refund attempts per payment to prevent two concurrent refunds // both passing the over-refund guard and both charging Square. Mirrors the @@ -1700,15 +1696,24 @@ func RefundPayment(w http.ResponseWriter, r *http.Request) { } }() - // Deterministic idempotency key so a same-key retry (network timeout) - // does not create a second Square refund. - idempotencyKey := paymentID + "-refund-" + strconv.FormatInt(req.Amount, 10) - - // Check for an existing refund with this key — dedup completed refunds. + // Dedup/resume (inside the lock): a same-key retry of a completed or + // in-flight (pending) refund must not create a second Square refund. Runs + // BEFORE the over-refund guard so a resuming refund never evaluates its own + // pending row against the guard. + var existingRefundID sql.NullString var existingRefundStatus sql.NullString - err = db.Conn.QueryRow(r.Context(), `SELECT status FROM refunds WHERE idempotency_key = $1`, idempotencyKey).Scan(&existingRefundStatus) - if err == nil && existingRefundStatus.String == "completed" { + var existingRefundAmount sql.NullFloat64 + var existingRefundOrigin sql.NullString + var existingRefundReason sql.NullString + var existingRefundCreatedAt sql.NullTime + var existingRefundKey sql.NullString + err = db.Conn.QueryRow(r.Context(), ` + SELECT id, status, amount, origin, reason, created_at, idempotency_key FROM refunds WHERE idempotency_key = $1 + `, idempotencyKey).Scan(&existingRefundID, &existingRefundStatus, &existingRefundAmount, &existingRefundOrigin, &existingRefundReason, &existingRefundCreatedAt, &existingRefundKey) + switch { + case err == nil && existingRefundStatus.String == "completed": if err := json.NewEncoder(w).Encode(RefundResponse{ + ID: existingRefundID.String, PaymentID: paymentID, Amount: req.Amount, Status: "completed", @@ -1718,9 +1723,198 @@ func RefundPayment(w http.ResponseWriter, r *http.Request) { log.Printf("Failed to encode JSON response: %v", err) } return - } - if err != nil && !errors.Is(err, pgx.ErrNoRows) { + case err == nil && existingRefundStatus.String == "pending": + // Resume the in-flight refund: the DB row was committed but the Square + // call never completed (network timeout, crash, etc.). Retry Square with + // the same idempotency key so Square returns the original refund if one + // exists, never a second one. + resumeAmount := int64(math.Round(existingRefundAmount.Float64 * 100)) + resumeReq := square.RefundPaymentReq{ + PaymentID: *payment.SquarePaymentID, + Amount: resumeAmount, + IdempotencyKey: idempotencyKey, + Reason: req.Reason, + } + resumeResult, resumeErr := SquareClient.RefundPayment(r.Context(), resumeReq) + if resumeErr != nil { + if errors.Is(resumeErr, square.ErrRefundAlreadyProcessed) { + // PAYMENT_ALREADY_REFUNDED — money already moved at Square. + // Resolve the pending row to completed (square_refund_id stays + // NULL) so the guard can never over-refund on top of it. + if _, upErr := db.Conn.Exec(r.Context(), `UPDATE refunds SET status = 'completed' WHERE id = $1`, existingRefundID.String); upErr != nil { + log.Printf("Failed to resolve refund %s completed after PAYMENT_ALREADY_REFUNDED: %v", existingRefundID.String, upErr) + } + log.Printf("Refund %s already processed at Square — marked completed", existingRefundID.String) + if err := json.NewEncoder(w).Encode(RefundResponse{ + ID: existingRefundID.String, + PaymentID: paymentID, + Amount: req.Amount, + Status: "completed", + Reason: req.Reason, + CreatedAt: clock.Now().Format(time.RFC3339), + }); err != nil { + log.Printf("Failed to encode JSON response: %v", err) + } + return + } + if errors.Is(resumeErr, square.ErrRefundDeclined) { + // Definitive rejection — mark failed so it never retries and + // never blocks future refunds. + if _, upErr := db.Conn.Exec(r.Context(), `UPDATE refunds SET status = 'failed' WHERE id = $1`, existingRefundID.String); upErr != nil { + log.Printf("Failed to mark refund %s failed after definitive rejection: %v", existingRefundID.String, upErr) + } + log.Printf("Refund %s definitively declined by Square: %v", existingRefundID.String, resumeErr) + http.Error(w, "Refund failed", http.StatusInternalServerError) + return + } + // Ambiguous error — leave pending for the scheduler to retry. + log.Printf("Failed to resume refund %s (left pending): %v", existingRefundID.String, resumeErr) + http.Error(w, "Refund failed", http.StatusInternalServerError) + return + } + if _, upErr := db.Conn.Exec(r.Context(), + `UPDATE refunds SET status = 'completed', square_refund_id = $1 WHERE id = $2`, + resumeResult.ID, existingRefundID.String, + ); upErr != nil { + log.Printf("CRITICAL: Square refund committed (%s) but DB update for refund %s failed — manual reconciliation required: %v", resumeResult.ID, existingRefundID.String, upErr) + http.Error(w, "internal server error", http.StatusInternalServerError) + return + } + if err := json.NewEncoder(w).Encode(RefundResponse{ + ID: existingRefundID.String, + PaymentID: paymentID, + Amount: req.Amount, + Status: "completed", + Reason: req.Reason, + CreatedAt: clock.Now().Format(time.RFC3339), + }); err != nil { + log.Printf("Failed to encode JSON response: %v", err) + } + return + case err == nil && existingRefundStatus.String == "failed": + // A failed row with origin='manual' may actually have moved money at + // Square (response loss after a definitive decline). Reconcile FIRST — + // an exact-amount COMPLETED refund resolves the row to completed. + // Otherwise the reconcile proves money did NOT move, so re-issuing with + // the stored key/amount is safe (Square dedups same-key retries). + if existingRefundOrigin.String == "manual" { + refundSqPaymentID := *payment.SquarePaymentID + resumeAmount := int64(math.Round(existingRefundAmount.Float64 * 100)) + var reconcileTime time.Time + if existingRefundCreatedAt.Valid { + reconcileTime = existingRefundCreatedAt.Time + } + sqRefundID, rcErr := reconcileRefundAtSquare(r.Context(), refundSqPaymentID, resumeAmount, reconcileTime) + switch { + case rcErr != nil: + // Reconcile failed — unknown whether Square refunded. Do NOT + // re-issue on an unknown state: re-issuing would be safe + // against Square's key dedup, but if money already moved the + // over-refund guard would lose sight of it. Surface a retry. + log.Printf("Failed to reconcile refund %s against Square before re-issue (%v) — not re-issuing, ask the admin to retry", existingRefundID.String, rcErr) + http.Error(w, "Unable to verify refund status with Square, please retry", http.StatusServiceUnavailable) + return + case sqRefundID != nil: + if _, upErr := db.Conn.Exec(r.Context(), `UPDATE refunds SET status = 'completed', square_refund_id = $1 WHERE id = $2`, *sqRefundID, existingRefundID.String); upErr != nil { + log.Printf("Failed to mark refund %s completed after Square reconcile: %v", existingRefundID.String, upErr) + } + if err := json.NewEncoder(w).Encode(RefundResponse{ + ID: existingRefundID.String, + PaymentID: paymentID, + Amount: req.Amount, + Status: "completed", + Reason: req.Reason, + CreatedAt: clock.Now().Format(time.RFC3339), + }); err != nil { + log.Printf("Failed to encode JSON response: %v", err) + } + return + } + reissueReq := square.RefundPaymentReq{ + PaymentID: refundSqPaymentID, + Amount: resumeAmount, + IdempotencyKey: existingRefundKey.String, + Reason: existingRefundReason.String, + } + reissueResult, reissueErr := SquareClient.RefundPayment(r.Context(), reissueReq) + switch { + case reissueErr == nil: + if _, upErr := db.Conn.Exec(r.Context(), + `UPDATE refunds SET status = 'completed', square_refund_id = $1 WHERE id = $2`, + reissueResult.ID, existingRefundID.String, + ); upErr != nil { + log.Printf("CRITICAL: Square refund committed (%s) but DB update for refund %s failed — manual reconciliation required: %v", reissueResult.ID, existingRefundID.String, upErr) + http.Error(w, "internal server error", http.StatusInternalServerError) + return + } + if err := json.NewEncoder(w).Encode(RefundResponse{ + ID: existingRefundID.String, + PaymentID: paymentID, + Amount: req.Amount, + Status: "completed", + Reason: req.Reason, + CreatedAt: clock.Now().Format(time.RFC3339), + }); err != nil { + log.Printf("Failed to encode JSON response: %v", err) + } + return + case errors.Is(reissueErr, square.ErrRefundAlreadyProcessed): + if _, upErr := db.Conn.Exec(r.Context(), `UPDATE refunds SET status = 'completed' WHERE id = $1`, existingRefundID.String); upErr != nil { + log.Printf("Failed to resolve refund %s completed after PAYMENT_ALREADY_REFUNDED: %v", existingRefundID.String, upErr) + } + if err := json.NewEncoder(w).Encode(RefundResponse{ + ID: existingRefundID.String, + PaymentID: paymentID, + Amount: req.Amount, + Status: "completed", + Reason: req.Reason, + CreatedAt: clock.Now().Format(time.RFC3339), + }); err != nil { + log.Printf("Failed to encode JSON response: %v", err) + } + return + case errors.Is(reissueErr, square.ErrRefundDeclined): + if _, upErr := db.Conn.Exec(r.Context(), `UPDATE refunds SET status = 'failed' WHERE id = $1`, existingRefundID.String); upErr != nil { + log.Printf("Failed to mark refund %s failed after re-issue rejection: %v", existingRefundID.String, upErr) + } + log.Printf("Refund %s re-issued with stored key definitively declined by Square: %v", existingRefundID.String, reissueErr) + http.Error(w, "Refund failed", http.StatusInternalServerError) + return + default: + // Ambiguous re-issue — put the row back to 'pending' so the + // sweep's manual retry pass can re-attempt it. + if _, upErr := db.Conn.Exec(r.Context(), `UPDATE refunds SET status = 'pending' WHERE id = $1`, existingRefundID.String); upErr != nil { + log.Printf("Failed to mark refund %s pending after ambiguous re-issue: %v", existingRefundID.String, upErr) + } + log.Printf("Refund %s re-issue left pending (ambiguous): %v", existingRefundID.String, reissueErr) + http.Error(w, "Refund failed", http.StatusInternalServerError) + return + } + } + // Previously definitively rejected (non-manual) — a same-key retry cannot + // succeed and the UNIQUE key would block re-insertion. Surface the + // failure instead of 500-ing on a duplicate. + log.Printf("Refund %s was previously marked failed — same-key retry rejected", existingRefundID.String) + http.Error(w, "Refund failed", http.StatusInternalServerError) + return + case err != nil && !errors.Is(err, pgx.ErrNoRows): log.Printf("Failed to check refund idempotency: %v", err) + http.Error(w, "internal server error", http.StatusInternalServerError) + return + } + + // Over-refund guard (inside the lock so concurrent refunds can't both pass). + // GetAlreadyRefundedAmount counts completed AND pending refunds. + alreadyRefunded, err := service.GetAlreadyRefundedAmount(r.Context(), paymentID) + if err != nil { + log.Printf("Failed to get already refunded amount: %v", err) + http.Error(w, "internal server error", http.StatusInternalServerError) + return + } + + if req.Amount+alreadyRefunded > int64(math.Round(payment.Amount*100)) { + http.Error(w, "Refund amount exceeds payment amount", http.StatusBadRequest) + return } // Begin a transaction. Insert the refund record as 'pending' first, commit, @@ -1740,8 +1934,8 @@ func RefundPayment(w http.ResponseWriter, r *http.Request) { var refundID string err = tx.QueryRow(r.Context(), ` - INSERT INTO refunds (payment_id, booking_id, amount, status, reason, idempotency_key, created_by, created_at) - VALUES ($1, $2, $3, 'pending', $4, $5, $6, $7) + INSERT INTO refunds (payment_id, booking_id, amount, status, reason, idempotency_key, created_by, created_at, origin) + VALUES ($1, $2, $3, 'pending', $4, $5, $6, $7, 'manual') RETURNING id `, paymentID, @@ -1773,8 +1967,39 @@ func RefundPayment(w http.ResponseWriter, r *http.Request) { refundResult, err := SquareClient.RefundPayment(r.Context(), refundReq) if err != nil { - // Refund record intentionally left as 'pending' for the scheduler to - // re-attempt (refunds.go ProcessPendingSquareRefunds). + if errors.Is(err, square.ErrRefundAlreadyProcessed) { + // PAYMENT_ALREADY_REFUNDED — money already moved at Square. Resolve + // to completed (square_refund_id stays NULL) rather than failed so + // the over-refund guard can never issue money on top of it. + if _, upErr := db.Conn.Exec(r.Context(), `UPDATE refunds SET status = 'completed' WHERE id = $1`, refundID); upErr != nil { + log.Printf("Failed to resolve refund %s completed after PAYMENT_ALREADY_REFUNDED: %v", refundID, upErr) + } + log.Printf("Refund %s already processed at Square — marked completed", refundID) + if err := json.NewEncoder(w).Encode(RefundResponse{ + ID: refundID, + PaymentID: paymentID, + Amount: req.Amount, + Status: "completed", + Reason: req.Reason, + CreatedAt: clock.Now().Format(time.RFC3339), + }); err != nil { + log.Printf("Failed to encode JSON response: %v", err) + } + return + } + if errors.Is(err, square.ErrRefundDeclined) { + // Definitive rejection (declined / already refunded / invalid + // payment) — mark the refund failed so it never retries and never + // blocks future refunds. + if _, upErr := db.Conn.Exec(r.Context(), `UPDATE refunds SET status = 'failed' WHERE id = $1`, refundID); upErr != nil { + log.Printf("Failed to mark refund %s failed after definitive rejection: %v", refundID, upErr) + } + log.Printf("Refund %s definitively declined by Square: %v", refundID, err) + http.Error(w, "Refund failed", http.StatusInternalServerError) + return + } + // Ambiguous error — refund record intentionally left as 'pending' for + // the scheduler to re-attempt (refunds.go ProcessPendingSquareRefunds). log.Printf("Failed to refund payment (refund %s left pending): %v", refundID, err) http.Error(w, "Refund failed", http.StatusInternalServerError) return diff --git a/backend/handlers/payments/payment_status_test.go b/backend/handlers/payments/payment_status_test.go index e66c04e..03014d0 100644 --- a/backend/handlers/payments/payment_status_test.go +++ b/backend/handlers/payments/payment_status_test.go @@ -678,3 +678,134 @@ func TestGetCheckoutStatus_Completed(t *testing.T) { t.Error("expected card_last4 to be set") } } + +// pollCheckoutStatus polls GetCheckoutStatus until the checkout reports +// COMPLETED, returning the decoded response. +func pollCheckoutStatus(t *testing.T, ctx context.Context, checkoutID, bookingID, adminToken string) PaymentStatusResponse { + t.Helper() + var resp PaymentStatusResponse + assert.Eventually(t, func() bool { + statusReq := httptest.NewRequest("GET", "/api/admin/payments/"+checkoutID+"/status?booking_id="+bookingID, nil) + statusRCtx := chi.NewRouteContext() + statusRCtx.URLParams.Add("checkout_id", checkoutID) + statusCtx := context.WithValue(ctx, chi.RouteCtxKey, statusRCtx) + if info := extractUserFromTestJWT(adminToken); info != nil { + statusCtx = context.WithValue(statusCtx, mw.UserIDKey, info.userID) + statusCtx = context.WithValue(statusCtx, mw.UserRoleKey, info.role) + } + statusReq = statusReq.WithContext(statusCtx) + + w2 := httptest.NewRecorder() + GetCheckoutStatus(w2, statusReq) + if w2.Code != http.StatusOK { + return false + } + if err := json.NewDecoder(w2.Body).Decode(&resp); err != nil { + return false + } + return resp.Status == "COMPLETED" + }, 10*time.Second, 200*time.Millisecond, "expected checkout to complete") + return resp +} + +// createTerminalCheckout creates a terminal checkout via CreateTerminalPayment +// and returns the checkout ID from the response. +func createTerminalCheckout(t *testing.T, ctx context.Context, bookingID, adminToken string, amount int64) string { + t.Helper() + handler := CreateTerminalPayment + req := CreateTerminalPaymentRequest{ + Amount: amount, + PaymentType: "full", + } + w := makePaymentRequest(handler, "POST", "/api/admin/bookings/"+bookingID+"/payment", req, adminToken, ctx) + if w.Code != http.StatusOK { + t.Fatalf("expected status 200, got %d. body: %s", w.Code, w.Body.String()) + } + var createResp CheckoutResponse + if err := json.NewDecoder(w.Body).Decode(&createResp); err != nil { + t.Fatalf("failed to decode create response: %v", err) + } + if createResp.CheckoutID == "" { + t.Fatal("expected checkout_id to be set") + } + return createResp.CheckoutID +} + +func TestGetCheckoutStatus_DoublePoll_SinglePaymentRow(t *testing.T) { + // A double poll of the same terminal checkout must return the existing + // payment row instead of inserting a duplicate (which previously 500'd on + // the idempotency-key UNIQUE violation after the customer had paid). + origClient := SquareClient + SquareClient = &testCheckoutClient{ + SquareClient: origClient, + hexIDs: make(map[string]string), + } + defer func() { SquareClient = origClient }() + + ctx, tx := testutils.SetupTestTx(t) + _, bookingID, _ := setupTestData(t, ctx, tx) + adminToken := jwt.GenerateAdminToken() + + checkoutID := createTerminalCheckout(t, ctx, bookingID, adminToken, 5000) + + first := pollCheckoutStatus(t, ctx, checkoutID, bookingID, adminToken) + if first.PaymentID == "" { + t.Fatal("expected payment_id from first poll") + } + + // Second poll of the same checkout — deduped against the existing row. + second := pollCheckoutStatus(t, ctx, checkoutID, bookingID, adminToken) + if second.PaymentID == "" { + t.Fatal("expected payment_id from second poll") + } + if second.PaymentID != first.PaymentID { + t.Errorf("expected same payment_id on re-poll, got %q then %q", first.PaymentID, second.PaymentID) + } + + var rowCount int + err := tx.QueryRow(ctx, "SELECT COUNT(*) FROM payments WHERE booking_id = $1", bookingID).Scan(&rowCount) + if err != nil { + t.Fatalf("failed to count payment rows: %v", err) + } + if rowCount != 1 { + t.Errorf("expected exactly 1 payment row after double poll, got %d", rowCount) + } +} + +func TestGetCheckoutStatus_TwoEqualAmountCharges_NoCollision(t *testing.T) { + // Two distinct terminal charges on the same booking with the same final + // amount must each create their own payment row (the deposit + equal-amount + // balance case) — no 500 on the idempotency-key UNIQUE collision. + origClient := SquareClient + SquareClient = &testCheckoutClient{ + SquareClient: origClient, + hexIDs: make(map[string]string), + } + defer func() { SquareClient = origClient }() + + ctx, tx := testutils.SetupTestTx(t) + _, bookingID, _ := setupTestData(t, ctx, tx) + adminToken := jwt.GenerateAdminToken() + + checkoutA := createTerminalCheckout(t, ctx, bookingID, adminToken, 5000) + checkoutB := createTerminalCheckout(t, ctx, bookingID, adminToken, 5000) + + respA := pollCheckoutStatus(t, ctx, checkoutA, bookingID, adminToken) + respB := pollCheckoutStatus(t, ctx, checkoutB, bookingID, adminToken) + + if respA.PaymentID == "" || respB.PaymentID == "" { + t.Fatal("expected payment_ids for both checkouts") + } + if respA.PaymentID == respB.PaymentID { + t.Error("expected two distinct payment rows for two distinct Square charges") + } + + var rowCount int + err := tx.QueryRow(ctx, "SELECT COUNT(*) FROM payments WHERE booking_id = $1", bookingID).Scan(&rowCount) + if err != nil { + t.Fatalf("failed to count payment rows: %v", err) + } + if rowCount != 2 { + t.Errorf("expected exactly 2 payment rows for two equal-amount charges, got %d", rowCount) + } +} diff --git a/backend/handlers/payments/payments_test.go b/backend/handlers/payments/payments_test.go index e8f4da1..f48e2a0 100644 --- a/backend/handlers/payments/payments_test.go +++ b/backend/handlers/payments/payments_test.go @@ -15,6 +15,7 @@ import ( "crussell/clock" "crussell/db" + "crussell/internal/square" "crussell/mw" "crussell/testutils" "crussell/testutils/fixtures" @@ -772,6 +773,180 @@ func TestRefund_PendingPaymentRejected(t *testing.T) { } } +func TestRefund_PendingSameKeyRetry_Resumes(t *testing.T) { + // A retry of an in-flight refund (DB row pending, Square call never + // completed) must resume rather than insert a second row or issue a second + // Square refund. The handler retries Square with the same idempotency key + // and completes the pending row. + t.Parallel() + ctx, tx := testutils.SetupTestTx(t) + + _, bookingID, _ := setupTestData(t, ctx, tx) + + adminToken := jwt.GenerateAdminToken() + + paymentID, err := fixtures.CreateTestPayment(tx, bookingID, 5000.00, "in_person_card", "full", "completed") + if err != nil { + t.Fatalf("failed to create payment: %v", err) + } + _, err = tx.Exec(ctx, "UPDATE payments SET square_payment_id = 'sqp_pending_resume' WHERE id = $1", paymentID) + if err != nil { + t.Fatalf("failed to update payment: %v", err) + } + + // Seed a pending refund with the same key the handler will compute. + key := paymentID + "-refund-2500" + _, err = tx.Exec(ctx, ` + INSERT INTO refunds (payment_id, booking_id, amount, status, reason, idempotency_key, created_at) + VALUES ($1, $2, 25, 'pending', 'customer request', $3, NOW()) + `, paymentID, bookingID, key) + if err != nil { + t.Fatalf("failed to seed pending refund: %v", err) + } + + req := RefundRequest{ + Amount: 2500, + Reason: "customer request", + } + + handler := RefundPayment + w := makePaymentRequest(handler, "POST", "/api/admin/payments/"+paymentID+"/refund", req, adminToken, ctx) + if w.Code != http.StatusOK { + t.Fatalf("expected status 200, got %d. body: %s", w.Code, w.Body.String()) + } + + // The pending row must now be completed with a Square refund ID — and no + // second refund row inserted. + var refundCount int + err = tx.QueryRow(ctx, `SELECT COUNT(*) FROM refunds WHERE payment_id = $1`, paymentID).Scan(&refundCount) + if err != nil { + t.Fatalf("failed to query refunds: %v", err) + } + if refundCount != 1 { + t.Errorf("expected 1 refund row (resumed, not duplicated), got %d", refundCount) + } + + var status string + var squareRefundID *string + err = tx.QueryRow(ctx, `SELECT status, square_refund_id FROM refunds WHERE idempotency_key = $1`, key).Scan(&status, &squareRefundID) + if err != nil { + t.Fatalf("failed to query refund: %v", err) + } + if status != "completed" { + t.Errorf("expected status completed, got %q", status) + } + if squareRefundID == nil || *squareRefundID == "" { + t.Error("expected square_refund_id to be set after resume") + } +} + +func TestRefund_GuardCountsPendingRefunds(t *testing.T) { + // The over-refund guard must count pending refunds (an in-flight Square + // refund) as well as completed ones, so a second refund cannot push the + // total past the payment amount. + t.Parallel() + ctx, tx := testutils.SetupTestTx(t) + + _, bookingID, _ := setupTestData(t, ctx, tx) + + adminToken := jwt.GenerateAdminToken() + + paymentID, err := fixtures.CreateTestPayment(tx, bookingID, 100.00, "in_person_card", "full", "completed") + if err != nil { + t.Fatalf("failed to create payment: %v", err) + } + _, err = tx.Exec(ctx, "UPDATE payments SET square_payment_id = 'sqp_guard_pending' WHERE id = $1", paymentID) + if err != nil { + t.Fatalf("failed to update payment: %v", err) + } + + // Seed a pending refund of £70 (as if a previous attempt's Square call is in flight). + _, err = tx.Exec(ctx, ` + INSERT INTO refunds (payment_id, booking_id, amount, status, reason, created_at) + VALUES ($1, $2, 70, 'pending', 'in flight', NOW()) + `, paymentID, bookingID) + if err != nil { + t.Fatalf("failed to seed pending refund: %v", err) + } + + // A further £40 refund would total £110 > £100 — must be rejected. + req := RefundRequest{ + Amount: 4000, + Reason: "over refund attempt", + } + + handler := RefundPayment + w := makePaymentRequest(handler, "POST", "/api/admin/payments/"+paymentID+"/refund", req, adminToken, ctx) + if w.Code != http.StatusBadRequest { + t.Errorf("expected status 400, got %d. body: %s", w.Code, w.Body.String()) + } +} + +func TestRefund_PaymentAlreadyRefunded_MarksCompleted(t *testing.T) { + // Square reports PAYMENT_ALREADY_REFUNDED — the money has already moved. + // The refund row must resolve to 'completed' (NOT 'failed', which would let + // the over-refund guard re-issue money on top of it), square_refund_id + // stays NULL, and the API returns 200. + ctx, tx := testutils.SetupTestTx(t) + + _, bookingID, _ := setupTestData(t, ctx, tx) + + adminID, err := fixtures.CreateTestAdminUser(tx) + if err != nil { + t.Fatalf("failed to create admin user: %v", err) + } + + adminToken := jwt.GenerateTestToken(adminID, "admin") + + paymentID, err := fixtures.CreateTestPayment(tx, bookingID, 5000.00, "in_person_card", "full", "completed") + if err != nil { + t.Fatalf("failed to create payment: %v", err) + } + _, err = tx.Exec(ctx, "UPDATE payments SET square_payment_id = 'sqp_already_refunded' WHERE id = $1", paymentID) + if err != nil { + t.Fatalf("failed to set square_payment_id: %v", err) + } + + origClient := SquareClient + mock := square.NewDevClient().(*square.MockClient) + mock.FailRefundCode = "PAYMENT_ALREADY_REFUNDED" + SquareClient = mock + defer func() { SquareClient = origClient }() + + req := RefundRequest{ + Amount: 5000, + Reason: "customer request", + } + + handler := RefundPayment + w := makePaymentRequest(handler, "POST", "/api/admin/payments/"+paymentID+"/refund", req, adminToken, ctx) + if w.Code != http.StatusOK { + t.Fatalf("expected 200, got %d. body: %s", w.Code, w.Body.String()) + } + + var resp RefundResponse + if err := parsePaymentResponseBody(w, &resp); err != nil { + t.Fatalf("failed to parse response: %v", err) + } + if resp.Status != "completed" { + t.Errorf("expected response status 'completed', got %q", resp.Status) + } + + var status string + var squareRefundID *string + err = tx.QueryRow(ctx, + "SELECT status, square_refund_id FROM refunds WHERE payment_id = $1", paymentID).Scan(&status, &squareRefundID) + if err != nil { + t.Fatalf("failed to query refund: %v", err) + } + if status != "completed" { + t.Errorf("expected refund status 'completed', got %q", status) + } + if squareRefundID != nil && *squareRefundID != "" { + t.Errorf("expected square_refund_id NULL (no new refund issued), got %q", *squareRefundID) + } +} + func TestTipPayment_HappyPath(t *testing.T) { t.Parallel() ctx, tx := testutils.SetupTestTx(t) diff --git a/backend/handlers/payments/refunds.go b/backend/handlers/payments/refunds.go index f3781fa..9b61b6a 100644 --- a/backend/handlers/payments/refunds.go +++ b/backend/handlers/payments/refunds.go @@ -3,9 +3,11 @@ package payments import ( "context" "errors" + "fmt" "log" "log/slog" "math" + "sort" "strconv" "time" @@ -25,6 +27,17 @@ type RefundCalculationResult struct { Tier string `json:"tier"` } +// paymentRow is one completed payment on a booking, read into a slice before +// the refund loop runs (pgx.Tx does not support concurrent queries on the same +// connection, so rows must be fully drained before Exec/QueryRow in the loop). +type paymentRow struct { + ID string + Amount float64 + PaymentMethod string + SquarePaymentID *string + GiftCardID *string +} + // CalculateRefundForCancellation computes the refund amounts for a cancelled booking. // // Track A (universal — same rules for all bookings): @@ -89,9 +102,39 @@ func CalculateRefundForCancellation( // It processes refunds against completed payments on the booking up to the // calculated refundable amount, creating refund records in the database. // Returns the refund calculation and whether any refunds were processed. +// lockCancellationPayments serializes a cancellation refund against the manual +// RefundPayment handler and the sweep. Both hold +// `pg_advisory_lock(hashtext('crussell:refund:' || payment_id))` (session-level) +// on the card payment ids they touch; a cancellation that computes residuals +// without the same locks can over-refund against a manual refund in flight (the +// manual guard read precedes the cancellation's commit). Locks are acquired in +// ascending payment_id order (matching processChargeGroup) to avoid deadlocks, +// and only for card methods the manual handler can touch. +func lockCancellationPayments(ctx context.Context, q db.Querier, payments []paymentRow) error { + var ids []string + for _, p := range payments { + if p.PaymentMethod == "online_square" || p.PaymentMethod == "in_person_card" { + ids = append(ids, p.ID) + } + } + if len(ids) == 0 { + return nil + } + sort.Strings(ids) + for _, pid := range ids { + if _, err := q.Exec(ctx, `SELECT pg_advisory_xact_lock(hashtext('crussell:refund:' || $1))`, pid); err != nil { + return fmt.Errorf("failed to acquire cancellation refund lock for payment %s: %w", pid, err) + } + } + return nil +} + // ProcessCancellationRefundTx is like ProcessCancellationRefund but uses an // externally-provided transaction. The caller owns the transaction lifecycle // (commit/rollback). Pass a non-nil pgx.Tx to share an existing transaction. +// forceFullRefund overrides the notice-tier calculation so the ENTIRE net +// pre-paid amount is refunded (admin "forgive fees" path) regardless of how +// close to the appointment the cancellation happens. func ProcessCancellationRefundTx( ctx context.Context, tx pgx.Tx, @@ -102,9 +145,16 @@ func ProcessCancellationRefundTx( cancellationTime time.Time, reason string, actorID *string, + forceFullRefund bool, ) (*RefundCalculationResult, error) { calc := CalculateRefundForCancellation(subtotal, totalPrePaid, cancellationTime, startTime) + if forceFullRefund { + calc.RefundableAmount = calc.TotalPrePaid + calc.KeptAmount = 0 + calc.Tier = "admin_full_refund" + } + if calc.RefundableAmount <= 0 { return &calc, nil } @@ -135,13 +185,6 @@ func ProcessCancellationRefundTx( defer rows.Close() // Read all payments into a slice, then close rows immediately. - type paymentRow struct { - ID string - Amount float64 - PaymentMethod string - SquarePaymentID *string - GiftCardID *string - } var payments []paymentRow for rows.Next() { var p paymentRow @@ -155,8 +198,37 @@ func ProcessCancellationRefundTx( log.Printf("Payment row iteration error: %v", err) } + // Serialize against the manual RefundPayment handler and the sweep — the + // residual calculation below must not race a manual refund in flight. If + // any lock fails, abort: continuing without the lock reopens the over-refund + // race. pg_advisory_xact_lock auto-releases at the caller's commit. + if err := lockCancellationPayments(ctx, tx, payments); err != nil { + return nil, err + } + refundRemaining := calc.RefundableAmount + // Prior refunds per payment record (completed + pending) — the loop must + // not re-refund money already returned. Sums by payment_id; pending counts + // because a Square call may already be in flight. + priorRefunds := make(map[string]float64) + prRows, prErr := tx.Query(ctx, ` + SELECT payment_id, COALESCE(SUM(amount), 0) FROM refunds + WHERE booking_id = $1 AND status IN ('completed', 'pending') + GROUP BY payment_id`, bookingID) + if prErr != nil { + log.Printf("Failed to query prior refunds for booking %s: %v", bookingID, prErr) + } else { + for prRows.Next() { + var pid string + var amt float64 + if err := prRows.Scan(&pid, &amt); err == nil { + priorRefunds[pid] = amt + } + } + prRows.Close() + } + for _, p := range payments { if refundRemaining <= 0 { break @@ -167,7 +239,13 @@ func ProcessCancellationRefundTx( amount := p.Amount giftCardID := p.GiftCardID - refundThisPayment := math.Min(amount, refundRemaining) + already := priorRefunds[paymentID] + residual := math.Round((amount-already)*100) / 100 + if residual <= 0 { + // already fully refunded — don't consume refundRemaining + continue + } + refundThisPayment := math.Min(residual, refundRemaining) var squareRefundID *string switch paymentMethod { @@ -240,18 +318,35 @@ func ProcessCancellationRefundTx( SquareRefundID: squareRefundID, Status: recordStatus, Reason: reason, + Origin: "cancellation", CreatedBy: actorID, CreatedAt: clock.Now(), } - _, dbErr := tx.Exec(ctx, ` - INSERT INTO refunds (payment_id, booking_id, amount, square_refund_id, status, reason, created_by, created_at) - VALUES ($1, $2, $3, $4, $5, $6, $7, $8) - `, record.PaymentID, record.BookingID, record.Amount, record.SquareRefundID, record.Status, record.Reason, record.CreatedBy, record.CreatedAt) + // Deterministic idempotency key so a scheduler retry can never issue a + // second Square refund. Format never collides with the handler's + // "-refund-" keys. + refundKey := paymentID + "-square-" + strconv.FormatInt(int64(math.Round(refundThisPayment*100)), 10) + record.IdempotencyKey = &refundKey + + tag, dbErr := tx.Exec(ctx, ` + INSERT INTO refunds (payment_id, booking_id, amount, square_refund_id, status, reason, idempotency_key, created_by, created_at, origin) + VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10) + ON CONFLICT (idempotency_key) DO NOTHING + `, record.PaymentID, record.BookingID, record.Amount, record.SquareRefundID, record.Status, record.Reason, record.IdempotencyKey, record.CreatedBy, record.CreatedAt, record.Origin) if dbErr != nil { log.Printf("Failed to create refund record for payment %s: %v", paymentID, dbErr) continue } + // tag.RowsAffected() == 0 means the same idempotency_key already exists + // (a prior refund row for this payment+amount in 'failed' state — money + // never moved, but a row exists). Dedup — skip WITHOUT consuming + // refundRemaining so the loop can allocate to the next payment, exactly + // as the pre-ON-CONFLICT UNIQUE-violation path behaved. + if tag.RowsAffected() == 0 { + log.Printf("Refund for payment %s amount £%.2f already exists (idempotency dedup) — skipping without consuming refundRemaining", paymentID, refundThisPayment) + continue + } refundRemaining -= refundThisPayment } @@ -329,13 +424,6 @@ func ProcessCancellationRefund( // This avoids "conn busy" errors when db.Conn.QueryRow/Exec are called // inside the processing loop with a per-test transaction (pgx.Tx does not // support concurrent queries on the same connection). - type paymentRow struct { - ID string - Amount float64 - PaymentMethod string - SquarePaymentID *string - GiftCardID *string - } var payments []paymentRow for rows.Next() { var p paymentRow @@ -349,8 +437,38 @@ func ProcessCancellationRefund( log.Printf("Payment row iteration error: %v", err) } + // Serialize against the manual RefundPayment handler and the sweep. If any + // lock fails, log + return the error — the caller (bookings.go + // DeleteBookingHandler) aborts the cancellation so the user can retry; + // continuing without the lock reopens the over-refund race. + if err := lockCancellationPayments(ctx, tx, payments); err != nil { + log.Printf("Failed to acquire cancellation refund locks for booking %s: %v", bookingID, err) + return &calc, err + } + refundRemaining := calc.RefundableAmount + // Prior refunds per payment record (completed + pending) — the loop must + // not re-refund money already returned. Sums by payment_id; pending counts + // because a Square call may already be in flight. + priorRefunds := make(map[string]float64) + prRows, prErr := tx.Query(ctx, ` + SELECT payment_id, COALESCE(SUM(amount), 0) FROM refunds + WHERE booking_id = $1 AND status IN ('completed', 'pending') + GROUP BY payment_id`, bookingID) + if prErr != nil { + log.Printf("Failed to query prior refunds for booking %s: %v", bookingID, prErr) + } else { + for prRows.Next() { + var pid string + var amt float64 + if err := prRows.Scan(&pid, &amt); err == nil { + priorRefunds[pid] = amt + } + } + prRows.Close() + } + for _, p := range payments { if refundRemaining <= 0 { break @@ -361,7 +479,13 @@ func ProcessCancellationRefund( amount := p.Amount giftCardID := p.GiftCardID - refundThisPayment := math.Min(amount, refundRemaining) + already := priorRefunds[paymentID] + residual := math.Round((amount-already)*100) / 100 + if residual <= 0 { + // already fully refunded — don't consume refundRemaining + continue + } + refundThisPayment := math.Min(residual, refundRemaining) var squareRefundID *string switch paymentMethod { @@ -435,18 +559,35 @@ func ProcessCancellationRefund( SquareRefundID: squareRefundID, Status: recordStatus, Reason: reason, + Origin: "cancellation", CreatedBy: actorID, CreatedAt: clock.Now(), } - _, dbErr := tx.Exec(ctx, ` - INSERT INTO refunds (payment_id, booking_id, amount, square_refund_id, status, reason, created_by, created_at) - VALUES ($1, $2, $3, $4, $5, $6, $7, $8) - `, record.PaymentID, record.BookingID, record.Amount, record.SquareRefundID, record.Status, record.Reason, record.CreatedBy, record.CreatedAt) + // Deterministic idempotency key so a scheduler retry can never issue a + // second Square refund. Format never collides with the handler's + // "-refund-" keys. + refundKey := paymentID + "-square-" + strconv.FormatInt(int64(math.Round(refundThisPayment*100)), 10) + record.IdempotencyKey = &refundKey + + tag, dbErr := tx.Exec(ctx, ` + INSERT INTO refunds (payment_id, booking_id, amount, square_refund_id, status, reason, idempotency_key, created_by, created_at, origin) + VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10) + ON CONFLICT (idempotency_key) DO NOTHING + `, record.PaymentID, record.BookingID, record.Amount, record.SquareRefundID, record.Status, record.Reason, record.IdempotencyKey, record.CreatedBy, record.CreatedAt, record.Origin) if dbErr != nil { log.Printf("Failed to create refund record for payment %s: %v", paymentID, dbErr) continue } + // tag.RowsAffected() == 0 means the same idempotency_key already exists + // (a prior refund row for this payment+amount in 'failed' state — money + // never moved, but a row exists). Dedup — skip WITHOUT consuming + // refundRemaining so the loop can allocate to the next payment, exactly + // as the pre-ON-CONFLICT UNIQUE-violation path behaved. + if tag.RowsAffected() == 0 { + log.Printf("Refund for payment %s amount £%.2f already exists (idempotency dedup) — skipping without consuming refundRemaining", paymentID, refundThisPayment) + continue + } refundRemaining -= refundThisPayment } @@ -477,92 +618,816 @@ func ProcessCancellationRefund( return &calc, nil } -// ProcessPendingSquareRefunds queries for refund records where the refund was -// inserted as "pending" (Square refund not yet processed) and calls the Square -// API to process them. This ensures Square API calls happen AFTER the DB -// transaction commits — if the commit fails, no Square money is lost. +// ProcessPendingSquareRefunds resolves a booking's pending cancellation card +// refunds AFTER the enclosing transaction has committed — Square API calls +// only happen if the DB records persist. // -// Call this AFTER the enclosing transaction (if any) has been committed. +// Pending refunds are aggregated into ONE Square refund per charge +// (square_payment_id): split payment records sharing a single Square charge +// are refunded together, never per-row. See processChargeGroup. func ProcessPendingSquareRefunds(ctx context.Context, bookingID string, reason string) { + // (a) Terminal pre-pass scoped to this booking: card refunds with no + // Square reference can never be refunded via Square. Mark them failed + // so they stop retrying, and surface the affected booking in the admin + // notification centre for in-person arrangement. Must run OUTSIDE any + // GROUP BY — Postgres lumps NULLs together, so these rows can't be + // handled in the charge grouping below. rows, err := db.Conn.Query(ctx, ` - SELECT r.id, r.amount, p.square_payment_id - FROM refunds r - JOIN payments p ON r.payment_id = p.id - WHERE r.booking_id = $1 - AND r.status = 'pending' + UPDATE refunds r SET status = 'failed' + FROM payments p + WHERE p.id = r.payment_id + AND r.booking_id = $1 + AND r.status = 'pending' AND r.refund_attempts < 3 AND p.payment_method IN ('online_square', 'in_person_card') - AND r.square_refund_id IS NULL + AND p.square_payment_id IS NULL + AND r.origin = 'cancellation' + RETURNING r.id `, bookingID) if err != nil { - log.Printf("Failed to query pending Square refunds for booking %s: %v", bookingID, err) - return + log.Printf("Failed to mark Square-less card refunds failed for booking %s: %v", bookingID, err) + } else { + var failedIDs []string + for rows.Next() { + var id string + if err := rows.Scan(&id); err == nil { + failedIDs = append(failedIDs, id) + } + } + if err := rows.Err(); err != nil { + log.Printf("Failed to iterate Square-less card refunds for booking %s: %v", bookingID, err) + } + rows.Close() + if len(failedIDs) > 0 { + log.Printf("Marked %d Square-less card refund(s) failed for booking %s (in-person arrangement needed)", len(failedIDs), bookingID) + } + insertRefundFailedNotifications(ctx, failedIDs) + } + + // (b) This booking's card charges with pending refunds — one aggregated + // Square refund per charge. + for _, chargeID := range queryChargesWithPendingRefunds(ctx, "r.booking_id = $1", bookingID) { + if _, err := processChargeGroup(ctx, chargeID, fetchPendingChargeRows(ctx, chargeID), reason); err != nil { + log.Printf("Failed to process charge %s for booking %s: %v", chargeID, bookingID, err) + } + } +} + +// SweepPendingSquareRefunds is the scheduler job that retries every booking's +// pending cancellation card refunds. Registered in internal/jobs/cleanup.go as +// "sweep-pending-square-refunds". +func SweepPendingSquareRefunds(ctx context.Context) (int, error) { + // (a) Terminal pre-pass across all bookings: card refunds with no Square + // reference can never be refunded via Square → mark failed and surface + // for in-person arrangement. Must run OUTSIDE any GROUP BY — Postgres + // lumps NULLs together, so these rows can't be handled in the charge + // grouping below. + rows, err := db.Conn.Query(ctx, ` + UPDATE refunds r SET status = 'failed' + FROM payments p + WHERE p.id = r.payment_id + AND r.status = 'pending' AND r.refund_attempts < 3 + AND p.payment_method IN ('online_square', 'in_person_card') + AND p.square_payment_id IS NULL + AND r.origin = 'cancellation' + RETURNING r.id + `) + if err != nil { + log.Printf("Failed to mark Square-less card refunds failed: %v", err) + } else { + var failedIDs []string + for rows.Next() { + var id string + if err := rows.Scan(&id); err == nil { + failedIDs = append(failedIDs, id) + } + } + if err := rows.Err(); err != nil { + log.Printf("Failed to iterate Square-less card refunds during sweep: %v", err) + } + rows.Close() + if len(failedIDs) > 0 { + log.Printf("Marked %d Square-less card refund(s) failed (in-person arrangement needed)", len(failedIDs)) + } + insertRefundFailedNotifications(ctx, failedIDs) + } + + // (b) Charges with pending refunds — one aggregated Square refund each. + processed := 0 + for _, chargeID := range queryChargesWithPendingRefunds(ctx, "") { + n, err := processChargeGroup(ctx, chargeID, fetchPendingChargeRows(ctx, chargeID), "scheduled_retry") + if err != nil { + log.Printf("Sweep: failed to process charge %s: %v", chargeID, err) + continue + } + processed += n + } + + // (c) Stale MANUAL pending refunds (the handler's ambiguous-error path) are + // invisible to the cancellation passes above (they filter origin='manual' + // out) — without this pass they were never retried, permanently blocking + // the over-refund guard and depressing booking TotalPaid. + n, err := sweepManualPendingSquareRefunds(ctx) + if err != nil { + log.Printf("Sweep: failed to process manual pending refunds: %v", err) + } else { + processed += n + } + return processed, nil +} + +// pendingChargeRow is one eligible pending cancellation refund row tied to a +// single Square charge. +type pendingChargeRow struct { + ID string // refunds.id + PaymentID string // payments.id — advisory-lock ordering + Amount float64 + CreatedAt time.Time +} + +// queryChargesWithPendingRefunds returns the distinct square_payment_ids that +// have at least one eligible pending cancellation refund row. extraWhere is an +// optional extra SQL predicate bound by args (e.g. "r.booking_id = $1"). +func queryChargesWithPendingRefunds(ctx context.Context, extraWhere string, args ...any) []string { + q := ` + SELECT DISTINCT p.square_payment_id + FROM refunds r + JOIN payments p ON p.id = r.payment_id + WHERE r.status = 'pending' AND r.refund_attempts < 3 + AND p.square_payment_id IS NOT NULL + AND p.payment_method IN ('online_square', 'in_person_card') + AND r.origin = 'cancellation'` + if extraWhere != "" { + q += " AND " + extraWhere + } + rows, err := db.Conn.Query(ctx, q, args...) + if err != nil { + log.Printf("Failed to query charges with pending refunds: %v", err) + return nil } defer rows.Close() - - type pendingRefund struct { - ID string - Amount float64 - SquarePaymentID *string - } - var pending []pendingRefund + var out []string for rows.Next() { - var pr pendingRefund - if err := rows.Scan(&pr.ID, &pr.Amount, &pr.SquarePaymentID); err != nil { - log.Printf("Failed to scan pending refund row: %v", err) + var s string + if err := rows.Scan(&s); err != nil { + log.Printf("Failed to scan charge id: %v", err) + continue + } + out = append(out, s) + } + if err := rows.Err(); err != nil { + log.Printf("Charge id iteration error: %v", err) + } + return out +} + +// fetchPendingChargeRows returns all eligible pending cancellation refund rows +// for a single Square charge, ordered by refund id. +func fetchPendingChargeRows(ctx context.Context, chargeID string) []pendingChargeRow { + rows, err := db.Conn.Query(ctx, ` + SELECT r.id, p.id, r.amount, r.created_at + FROM refunds r + JOIN payments p ON p.id = r.payment_id + WHERE p.square_payment_id = $1 + AND r.status = 'pending' AND r.refund_attempts < 3 + AND r.origin = 'cancellation' + ORDER BY r.id + `, chargeID) + if err != nil { + log.Printf("Failed to query pending refunds for charge %s: %v", chargeID, err) + return nil + } + defer rows.Close() + var out []pendingChargeRow + for rows.Next() { + var pr pendingChargeRow + if err := rows.Scan(&pr.ID, &pr.PaymentID, &pr.Amount, &pr.CreatedAt); err != nil { + log.Printf("Failed to scan pending refund row for charge %s: %v", chargeID, err) + continue + } + out = append(out, pr) + } + if err := rows.Err(); err != nil { + log.Printf("Pending refund row iteration error for charge %s: %v", chargeID, err) + } + return out +} + +// reconcileRefundAtSquare checks Square for a COMPLETED refund matching the +// exact charge-level amount before the age guard / attempt cap marks rows +// 'failed'. Tri-state return: +// +// (id, nil) — exact COMPLETED refund found → caller marks rows completed +// (nil, nil) — genuinely no match → caller may mark rows failed +// (nil, err) — reconcile FAILED (network/API error) → caller MUST leave +// rows pending and skip the terminal transition. Marking +// failed on an unknown state would let the over-refund guard +// exclude money that actually left the business. +// +// Exact equality on amount AND status COMPLETED AND payment_id: a larger +// COMPLETED refund on the same charge is a manual per-record refund, +// attributing it would mark our rows completed when the aggregate money never +// moved. +func reconcileRefundAtSquare(ctx context.Context, chargeID string, totalCents int64, oldestCreatedAt time.Time) (*string, error) { + refunds, err := SquareClient.ListPaymentRefunds(ctx, chargeID, oldestCreatedAt) + if err != nil { + log.Printf("Failed to reconcile charge %s against Square: %v", chargeID, err) + return nil, err + } + for i := range refunds { + r := &refunds[i] + if r.PaymentID == chargeID && r.Status == "COMPLETED" && r.Amount == totalCents { + return &r.ID, nil + } + } + return nil, nil +} + +// insertRefundFailedNotifications surfaces failed refunds in the admin +// notification centre — one row per affected booking. The sweep only processes +// 'pending' rows, so this fires once per row transition (no spam); the +// NOT EXISTS guard prevents duplicates on re-runs. +func insertRefundFailedNotifications(ctx context.Context, refundIDs []string) { + if len(refundIDs) == 0 { + return + } + tag, err := db.Conn.Exec(ctx, ` + INSERT INTO admin_notifications (reason, booking_id, created_at) + SELECT DISTINCT 'refund_failed'::admin_notification_reason, booking_id, NOW() + FROM refunds + WHERE id = ANY($1) AND status = 'failed' + AND NOT EXISTS ( + SELECT 1 FROM admin_notifications an + WHERE an.reason = 'refund_failed' AND an.booking_id = refunds.booking_id + ) + `, refundIDs) + if err != nil { + log.Printf("Failed to insert admin_notifications for failed refunds: %v", err) + return + } + if n := int(tag.RowsAffected()); n > 0 { + log.Printf("Inserted %d admin_notification(s) for failed refunds", n) + } +} + +// pendingRowsAtAttemptCap returns the refund ids still pending at the 3-attempt +// cap — the candidates for terminal 'failed' resolution. +func pendingRowsAtAttemptCap(ctx context.Context, ids []string) []string { + if len(ids) == 0 { + return nil + } + rows, err := db.Conn.Query(ctx, ` + SELECT id FROM refunds + WHERE id = ANY($1) AND status = 'pending' AND refund_attempts >= 3 + `, ids) + if err != nil { + log.Printf("Failed to query refunds at attempt cap: %v", err) + return nil + } + defer rows.Close() + var out []string + for rows.Next() { + var id string + if err := rows.Scan(&id); err != nil { + log.Printf("Failed to scan refund id at attempt cap: %v", err) + continue + } + out = append(out, id) + } + return out +} + +// processChargeGroup issues ONE Square refund for a charge (square_payment_id) +// covering the sum of its pending cancellation refund rows — the owner +// directive that split records sharing a charge produce a single Square +// refund, never one per row. +// +// Callers pass the charge's current pending rows (fetchPendingChargeRows); the +// rows are re-read under the per-payment advisory lock so a concurrent manual +// refund or another sweep can't double-process. +func processChargeGroup(ctx context.Context, chargeID string, rows []pendingChargeRow, reason string) (int, error) { + if len(rows) == 0 { + return 0, nil + } + + // Serialize against the manual RefundPayment handler: acquire the SAME + // per-payment advisory lock (`hashtext('crussell:refund:' || payment_id)`) + // the handler uses, in ascending payment_id order to avoid deadlocks. + pinConn, err := db.Conn.Acquire(ctx) + if err != nil { + log.Printf("Failed to acquire connection for refund lock (charge %s): %v", chargeID, err) + return 0, nil + } + defer pinConn.Release() + + paymentIDs := make([]string, 0, len(rows)) + seen := make(map[string]bool, len(rows)) + for _, r := range rows { + if !seen[r.PaymentID] { + seen[r.PaymentID] = true + paymentIDs = append(paymentIDs, r.PaymentID) + } + } + sort.Strings(paymentIDs) + + locked := 0 + for _, pid := range paymentIDs { + if _, err := pinConn.Exec(ctx, ` + SELECT pg_advisory_lock(hashtext('crussell:refund:' || $1)) + `, pid); err != nil { + log.Printf("Failed to acquire refund lock for payment %s (charge %s): %v", pid, chargeID, err) + break + } + locked++ + } + if locked < len(paymentIDs) { + // Give up on this charge (the manual handler may hold the lock). The + // sweep continues to the next charge rather than aborting fatally. + for _, pid := range paymentIDs[:locked] { + if _, err := pinConn.Exec(context.Background(), ` + SELECT pg_advisory_unlock(hashtext('crussell:refund:' || $1)) + `, pid); err != nil { + log.Printf("Failed to release refund lock for payment %s: %v", pid, err) + } + } + return 0, nil + } + defer func() { + for _, pid := range paymentIDs { + if _, err := pinConn.Exec(context.Background(), ` + SELECT pg_advisory_unlock(hashtext('crussell:refund:' || $1)) + `, pid); err != nil { + log.Printf("Failed to release refund lock for payment %s: %v", pid, err) + } + } + }() + + // Re-read under the lock — only rows still pending and under the attempt + // cap are eligible (a concurrent manual refund may have resolved some). + ids := make([]string, 0, len(rows)) + for _, r := range rows { + ids = append(ids, r.ID) + } + pendingRows, err := db.Conn.Query(ctx, ` + SELECT id, amount, created_at FROM refunds + WHERE id = ANY($1) AND status = 'pending' AND refund_attempts < 3 + `, ids) + if err != nil { + log.Printf("Failed to re-read pending refunds under lock (charge %s): %v", chargeID, err) + return 0, nil + } + var pending []pendingChargeRow + for pendingRows.Next() { + var pr pendingChargeRow + if err := pendingRows.Scan(&pr.ID, &pr.Amount, &pr.CreatedAt); err != nil { + log.Printf("Failed to scan pending refund under lock (charge %s): %v", chargeID, err) continue } pending = append(pending, pr) } - if err := rows.Err(); err != nil { - log.Printf("Pending refund row iteration error: %v", err) + pendingRows.Close() + if len(pending) == 0 { + return 0, nil } - // Deduplicate: multiple split payment records can share the same - // square_payment_id — only refund each Square payment once. - refundedSquareIDs := make(map[string]bool) - + // Age guard: Square's idempotency-key retention is finite (~24h). If the + // oldest pending row predates 23 hours, re-issuing with the same charge key + // risks Square treating it as a NEW refund → double refund. Reconcile FIRST: + // money may already have moved at Square (response loss), and failing the + // rows without checking would let the over-refund guard exclude money that + // actually left the business. Only when Square shows no exact COMPLETED + // refund do we mark failed and surface for manual review. + oldest := pending[0].CreatedAt + for _, pr := range pending[1:] { + if pr.CreatedAt.Before(oldest) { + oldest = pr.CreatedAt + } + } + var totalCents int64 for _, pr := range pending { - if pr.SquarePaymentID == nil || *pr.SquarePaymentID == "" { - // No Square payment ID — mark as completed (no API call needed) - _, upErr := db.Conn.Exec(ctx, `UPDATE refunds SET status = 'completed' WHERE id = $1`, pr.ID) - if upErr != nil { - log.Printf("Failed to update refund %s to completed: %v", pr.ID, upErr) + totalCents += int64(math.Round(pr.Amount * 100)) + } + if clock.Now().Sub(oldest) > 23*time.Hour { + sqRefundID, rcErr := reconcileRefundAtSquare(ctx, chargeID, totalCents, oldest) + switch { + case rcErr != nil: + // Reconcile failed — unknown whether Square refunded. Leave rows + // pending for the next sweep; NEVER mark failed on an unknown state + // (that would let the over-refund guard exclude moved money). + log.Printf("Reconcile failed for aged charge %s (%v) — leaving %d refund row(s) pending for the next sweep", chargeID, rcErr, len(pending)) + return 0, nil + case sqRefundID != nil: + if _, upErr := db.Conn.Exec(ctx, ` + UPDATE refunds SET status = 'completed', square_refund_id = $1 + WHERE id = ANY($2) AND status = 'pending' + `, *sqRefundID, idsOf(pending)); upErr != nil { + log.Printf("Failed to mark aged pending refunds completed after Square reconcile (charge %s): %v", chargeID, upErr) } - continue - } - - if refundedSquareIDs[*pr.SquarePaymentID] { - // Already refunded this Square payment via a previous split record. - // Mark this refund as completed since the money is already returned. - if _, upErr := db.Conn.Exec(ctx, `UPDATE refunds SET status = 'completed' WHERE id = $1`, pr.ID); upErr != nil { - log.Printf("Failed to update refund %s to completed (deduped): %v", pr.ID, upErr) + log.Printf("Aged card refunds for charge %s reconciled at Square — COMPLETED refund %s found, marked completed", chargeID, *sqRefundID) + return len(pending), nil + default: + if _, upErr := db.Conn.Exec(ctx, ` + UPDATE refunds SET status = 'failed' + WHERE id = ANY($1) AND status = 'pending' + `, idsOf(pending)); upErr != nil { + log.Printf("Failed to mark aged pending refunds failed (charge %s): %v", chargeID, upErr) } - continue + insertRefundFailedNotifications(ctx, idsOf(pending)) + // TODO (P6 email/SMS delivery): notify user AND admin when the email/SMS + // system lands; until then the admin_notifications row above is the only + // channel. Verify the Square dashboard first. + log.Printf("Card refunds for charge %s are older than 23h and Square shows no COMPLETED refund — marked 'failed' and admin notified; TODO email user+admin to arrange in-person cash pickup at the salon (give at least a day's notice for cash on hand)", chargeID) + return len(pending), nil } + } - refundCents := int64(math.Round(pr.Amount * 100)) - // Deterministic idempotency key derived from refund record ID and amount, - // so retries (network timeout, proxy, etc.) don't create duplicate Square refunds. - refundReq := square.RefundPaymentReq{ - PaymentID: *pr.SquarePaymentID, - Amount: refundCents, - IdempotencyKey: pr.ID + "-square-" + strconv.FormatInt(refundCents, 10), - Reason: reason, - } - sqResult, sqErr := SquareClient.RefundPayment(ctx, refundReq) - if sqErr != nil { - log.Printf("Square refund failed for pending refund %s (payment %s): %v — record left as 'pending' for manual retry", pr.ID, *pr.SquarePaymentID, sqErr) - // Leave status as 'pending' — can be retried manually or via admin tool. - continue + // ONE Square refund per charge with a STABLE charge-level idempotency key. + // The key is used ONLY for the Square call — never stored in + // refunds.idempotency_key (the per-row keys remain the audit trail). Square + // dedups same-key retries, so crash-retry amounts stay identical. + sqResult, sqErr := SquareClient.RefundPayment(ctx, square.RefundPaymentReq{ + PaymentID: chargeID, + Amount: totalCents, + IdempotencyKey: chargeID + "-square-agg", + Reason: reason, + }) + switch { + case sqErr == nil: + // ATOMIC — one statement for the whole group, never per-row. Keeps + // crash-retry amounts identical so Square's key-dedup returns the + // original refund. + if _, upErr := db.Conn.Exec(ctx, ` + UPDATE refunds SET status = 'completed', square_refund_id = $1 + WHERE id = ANY($2) AND status = 'pending' + `, sqResult.ID, idsOf(pending)); upErr != nil { + log.Printf("CRITICAL: Square refund committed (%s) but DB update for charge %s failed — manual reconciliation required: %v", sqResult.ID, chargeID, upErr) } + return len(pending), nil - refundedSquareIDs[*pr.SquarePaymentID] = true - - _, upErr := db.Conn.Exec(ctx, ` - UPDATE refunds SET square_refund_id = $1, status = 'completed' WHERE id = $2 - `, sqResult.ID, pr.ID) - if upErr != nil { - log.Printf("CRITICAL: Square refund succeeded (ID=%s) but DB record %s update failed: %v — manual reconciliation required", sqResult.ID, pr.ID, upErr) + case errors.Is(sqErr, square.ErrRefundAlreadyProcessed): + // PAYMENT_ALREADY_REFUNDED — money already moved at Square. + if _, upErr := db.Conn.Exec(ctx, ` + UPDATE refunds SET status = 'completed' + WHERE id = ANY($1) AND status = 'pending' + `, idsOf(pending)); upErr != nil { + log.Printf("Failed to resolve refunds after PAYMENT_ALREADY_REFUNDED (charge %s): %v", chargeID, upErr) } + return len(pending), nil + + case errors.Is(sqErr, square.ErrRefundDeclined): + // Definitive decline — money will never move. Bump attempts; at >=3 + // mark failed and surface for manual arrangement. + if _, upErr := db.Conn.Exec(ctx, ` + UPDATE refunds SET refund_attempts = refund_attempts + 1 + WHERE id = ANY($1) AND status = 'pending' + `, idsOf(pending)); upErr != nil { + log.Printf("Failed to increment refund attempts (charge %s): %v", chargeID, upErr) + } + if capIDs := pendingRowsAtAttemptCap(ctx, idsOf(pending)); len(capIDs) > 0 { + if _, upErr := db.Conn.Exec(ctx, ` + UPDATE refunds SET status = 'failed' + WHERE id = ANY($1) AND status = 'pending' AND refund_attempts >= 3 + `, capIDs); upErr != nil { + log.Printf("Failed to mark refunds failed after 3 attempts (charge %s): %v", chargeID, upErr) + } + insertRefundFailedNotifications(ctx, capIDs) + // TODO (P6 email/SMS delivery): notify user AND admin when the email/SMS + // system lands; until then the admin_notifications row above is the only + // channel. Arrange in-person cash pickup at the salon (a day's notice for + // cash on hand). + log.Printf("Card refund for charge %s definitively declined by Square — marked 'failed' and admin notified; TODO email user+admin to arrange in-person cash pickup at the salon (give at least a day's notice for cash on hand)", chargeID) + } + return 0, nil + + default: + // Ambiguous — Square may or may not have processed. Retried by the + // sweep, capped at 3 attempts. + if _, upErr := db.Conn.Exec(ctx, ` + UPDATE refunds SET refund_attempts = refund_attempts + 1 + WHERE id = ANY($1) AND status = 'pending' + `, idsOf(pending)); upErr != nil { + log.Printf("Failed to increment refund attempts (charge %s): %v", chargeID, upErr) + } + // At the cap, money may have moved at Square despite the ambiguous + // responses — reconcile BEFORE marking failed (same bug class as the + // age guard). An exact COMPLETED refund resolves to completed. + if capIDs := pendingRowsAtAttemptCap(ctx, idsOf(pending)); len(capIDs) > 0 { + sqRefundID, rcErr := reconcileRefundAtSquare(ctx, chargeID, totalCents, oldest) + switch { + case rcErr != nil: + // Reconcile failed — unknown whether Square refunded. Leave + // rows pending for the next sweep; NEVER mark failed on an + // unknown state (that would let the over-refund guard exclude + // moved money). + log.Printf("Reconcile failed for ambiguous charge %s (%v) — leaving %d refund row(s) pending for the next sweep", chargeID, rcErr, len(capIDs)) + case sqRefundID != nil: + if _, upErr := db.Conn.Exec(ctx, ` + UPDATE refunds SET status = 'completed', square_refund_id = $1 + WHERE id = ANY($2) AND status = 'pending' + `, *sqRefundID, capIDs); upErr != nil { + log.Printf("Failed to mark ambiguous refunds completed after Square reconcile (charge %s): %v", chargeID, upErr) + } + log.Printf("Ambiguous card refunds for charge %s reconciled at Square — COMPLETED refund %s found, marked completed", chargeID, *sqRefundID) + default: + if _, upErr := db.Conn.Exec(ctx, ` + UPDATE refunds SET status = 'failed' + WHERE id = ANY($1) AND status = 'pending' AND refund_attempts >= 3 + `, capIDs); upErr != nil { + log.Printf("Failed to mark refunds failed after 3 attempts (charge %s): %v", chargeID, upErr) + } + insertRefundFailedNotifications(ctx, capIDs) + // TODO (P6 email/SMS delivery): notify user AND admin when the email/SMS + // system lands; until then the admin_notifications row above is the only + // channel. + log.Printf("Card refund for charge %s is AMBIGUOUS (Square may have processed it) and no COMPLETED refund found at Square — marked 'failed' and admin notified; TODO email user+admin, VERIFY Square dashboard before arranging in-person cash pickup at the salon (give at least a day's notice for cash on hand)", chargeID) + } + } + return 0, nil } } + +func idsOf(rows []pendingChargeRow) []string { + ids := make([]string, 0, len(rows)) + for _, r := range rows { + ids = append(ids, r.ID) + } + return ids +} + +// manualPendingRow is one stale manual refund row (the handler's ambiguous-error +// path) eligible for retry by the sweep. +type manualPendingRow struct { + ID string + PaymentID string + Amount float64 + IdempotencyKey string + Reason string + SquarePaymentID string + CreatedAt time.Time +} + +// sweepManualPendingSquareRefunds retries stale MANUAL refunds left 'pending' +// by the RefundPayment handler's ambiguous-error path. The cancellation passes +// filter origin='cancellation', so manual rows were never re-attempted: they +// permanently blocked the over-refund guard and depressed booking TotalPaid. +// Each row is retried with its OWN stored idempotency key (Square dedups +// same-key retries, so the retry is idempotent). +func sweepManualPendingSquareRefunds(ctx context.Context) (int, error) { + rows, err := db.Conn.Query(ctx, ` + SELECT r.id, r.payment_id, r.amount, r.idempotency_key, r.reason, + p.square_payment_id, r.created_at + FROM refunds r + JOIN payments p ON p.id = r.payment_id + WHERE r.status = 'pending' AND r.origin = 'manual' + AND r.refund_attempts < 3 AND r.square_refund_id IS NULL + AND p.square_payment_id IS NOT NULL + ORDER BY r.payment_id, r.id + `) + if err != nil { + log.Printf("Failed to query manual pending refunds for retry: %v", err) + return 0, nil + } + var pending []manualPendingRow + for rows.Next() { + var pr manualPendingRow + var key *string + if err := rows.Scan(&pr.ID, &pr.PaymentID, &pr.Amount, &key, &pr.Reason, &pr.SquarePaymentID, &pr.CreatedAt); err != nil { + log.Printf("Failed to scan manual pending refund: %v", err) + continue + } + if key != nil { + pr.IdempotencyKey = *key + } + pending = append(pending, pr) + } + rows.Close() + if len(pending) == 0 { + return 0, nil + } + + // Group by payment_id, ascending — matching the lock ordering the manual + // handler and processChargeGroup use so the sweep never deadlocks them. + groups := make(map[string][]manualPendingRow) + var paymentIDs []string + for _, pr := range pending { + if _, ok := groups[pr.PaymentID]; !ok { + paymentIDs = append(paymentIDs, pr.PaymentID) + } + groups[pr.PaymentID] = append(groups[pr.PaymentID], pr) + } + sort.Strings(paymentIDs) + + processed := 0 + for _, pid := range paymentIDs { + n, err := processManualPaymentGroup(ctx, pid, groups[pid]) + if err != nil { + log.Printf("Sweep: failed to process manual pending refunds for payment %s: %v", pid, err) + continue + } + processed += n + } + return processed, nil +} + +// processManualPaymentGroup retries one payment's stale manual pending refunds +// under the SAME per-payment advisory lock the manual RefundPayment handler +// holds across its guard read — so a re-issued refund can never double-spend +// against a concurrent manual refund. Rows are re-read under the lock; each is +// issued to Square with its OWN stored idempotency key and stored reason. +func processManualPaymentGroup(ctx context.Context, paymentID string, rows []manualPendingRow) (int, error) { + pinConn, err := db.Conn.Acquire(ctx) + if err != nil { + return 0, err + } + defer pinConn.Release() + if _, err := pinConn.Exec(ctx, ` + SELECT pg_advisory_lock(hashtext('crussell:refund:' || $1)) + `, paymentID); err != nil { + return 0, err + } + defer func() { + if _, err := pinConn.Exec(context.Background(), ` + SELECT pg_advisory_unlock(hashtext('crussell:refund:' || $1)) + `, paymentID); err != nil { + log.Printf("Failed to release refund lock for payment %s: %v", paymentID, err) + } + }() + + ids := make([]string, 0, len(rows)) + for _, r := range rows { + ids = append(ids, r.ID) + } + // Re-read under the lock — only rows still pending and under the attempt + // cap are eligible (a concurrent manual refund may have resolved some). + prRows, err := db.Conn.Query(ctx, ` + SELECT r.id, r.amount, r.idempotency_key, r.reason, r.created_at, + r.payment_id, p.square_payment_id + FROM refunds r + JOIN payments p ON p.id = r.payment_id + WHERE r.id = ANY($1) AND r.status = 'pending' AND r.refund_attempts < 3 + ORDER BY r.id + `, ids) + if err != nil { + return 0, err + } + var pending []manualPendingRow + for prRows.Next() { + var pr manualPendingRow + var key *string + if err := prRows.Scan(&pr.ID, &pr.Amount, &key, &pr.Reason, &pr.CreatedAt, &pr.PaymentID, &pr.SquarePaymentID); err != nil { + log.Printf("Failed to scan manual pending refund under lock: %v", err) + continue + } + if key != nil { + pr.IdempotencyKey = *key + } + pending = append(pending, pr) + } + prRows.Close() + if len(pending) == 0 { + return 0, nil + } + + // Age guard (mirrors processChargeGroup): Square's idempotency-key + // retention is finite (~24h). Reconcile FIRST — an exact COMPLETED refund + // at Square resolves to completed even though our rows are stale. + oldest := pending[0].CreatedAt + for _, pr := range pending[1:] { + if pr.CreatedAt.Before(oldest) { + oldest = pr.CreatedAt + } + } + if clock.Now().Sub(oldest) > 23*time.Hour { + processedAged := 0 + for i := range pending { + pr := &pending[i] + amountCents := int64(math.Round(pr.Amount * 100)) + sqRefundID, rcErr := reconcileRefundAtSquare(ctx, pr.SquarePaymentID, amountCents, pr.CreatedAt) + switch { + case rcErr != nil: + // Reconcile failed — unknown whether Square refunded. Leave + // this row pending for the next sweep; NEVER mark failed on an + // unknown state. + log.Printf("Reconcile failed for aged manual refund %s (%v) — leaving pending for the next sweep", pr.ID, rcErr) + case sqRefundID != nil: + if _, upErr := db.Conn.Exec(ctx, ` + UPDATE refunds SET status = 'completed', square_refund_id = $1 + WHERE id = $2 AND status = 'pending' + `, *sqRefundID, pr.ID); upErr != nil { + log.Printf("Failed to mark aged manual refund %s completed after Square reconcile: %v", pr.ID, upErr) + } + processedAged++ + default: + if _, upErr := db.Conn.Exec(ctx, ` + UPDATE refunds SET status = 'failed' + WHERE id = $1 AND status = 'pending' + `, pr.ID); upErr != nil { + log.Printf("Failed to mark aged manual refund %s failed: %v", pr.ID, upErr) + } + insertRefundFailedNotifications(ctx, []string{pr.ID}) + // TODO (P6 email/SMS delivery): notify user AND admin when the email/SMS + // system lands; until then the admin_notifications row above is the only + // channel. + log.Printf("Manual refund %s is older than 23h and Square shows no COMPLETED refund — marked 'failed' and admin notified; TODO email user+admin to arrange in-person cash pickup at the salon (give at least a day's notice for cash on hand)", pr.ID) + } + } + return processedAged, nil + } + + processed := 0 + for i := range pending { + pr := &pending[i] + amountCents := int64(math.Round(pr.Amount * 100)) + sqResult, sqErr := SquareClient.RefundPayment(ctx, square.RefundPaymentReq{ + PaymentID: pr.SquarePaymentID, + Amount: amountCents, + IdempotencyKey: pr.IdempotencyKey, + Reason: pr.Reason, + }) + switch { + case sqErr == nil: + if _, upErr := db.Conn.Exec(ctx, ` + UPDATE refunds SET status = 'completed', square_refund_id = $1 + WHERE id = $2 + `, sqResult.ID, pr.ID); upErr != nil { + log.Printf("CRITICAL: Square refund committed (%s) but DB update for manual refund %s failed — manual reconciliation required: %v", sqResult.ID, pr.ID, upErr) + } + processed++ + + case errors.Is(sqErr, square.ErrRefundAlreadyProcessed): + if _, upErr := db.Conn.Exec(ctx, ` + UPDATE refunds SET status = 'completed' + WHERE id = $1 + `, pr.ID); upErr != nil { + log.Printf("Failed to resolve manual refund %s completed after PAYMENT_ALREADY_REFUNDED: %v", pr.ID, upErr) + } + processed++ + + case errors.Is(sqErr, square.ErrRefundDeclined): + if _, upErr := db.Conn.Exec(ctx, ` + UPDATE refunds SET refund_attempts = refund_attempts + 1 + WHERE id = $1 + `, pr.ID); upErr != nil { + log.Printf("Failed to increment attempts for manual refund %s: %v", pr.ID, upErr) + } + if attempts := currentRefundAttempts(ctx, pr.ID); attempts >= 3 { + resolveManualRefundAtCap(ctx, pr, amountCents) + } + + default: + // Ambiguous — Square may or may not have processed. Retried by the + // sweep, capped at 3 attempts. + if _, upErr := db.Conn.Exec(ctx, ` + UPDATE refunds SET refund_attempts = refund_attempts + 1 + WHERE id = $1 + `, pr.ID); upErr != nil { + log.Printf("Failed to increment attempts for manual refund %s: %v", pr.ID, upErr) + } + if attempts := currentRefundAttempts(ctx, pr.ID); attempts >= 3 { + resolveManualRefundAtCap(ctx, pr, amountCents) + } + } + } + return processed, nil +} + +// resolveManualRefundAtCap reconciles a manual refund row that just hit the +// 3-attempt cap: money may have moved at Square despite decline/ambiguous +// responses, so reconcile FIRST — an exact COMPLETED refund resolves the row to +// completed; otherwise mark failed and notify the admin. +func resolveManualRefundAtCap(ctx context.Context, pr *manualPendingRow, amountCents int64) { + sqRefundID, rcErr := reconcileRefundAtSquare(ctx, pr.SquarePaymentID, amountCents, pr.CreatedAt) + switch { + case rcErr != nil: + // Reconcile failed — unknown whether Square refunded. Leave the row + // pending for the next sweep; NEVER mark failed on an unknown state + // (that would let the over-refund guard exclude moved money). + log.Printf("Reconcile failed for manual refund %s at attempt cap (%v) — leaving pending for the next sweep", pr.ID, rcErr) + case sqRefundID != nil: + if _, upErr := db.Conn.Exec(ctx, ` + UPDATE refunds SET status = 'completed', square_refund_id = $1 + WHERE id = $2 + `, *sqRefundID, pr.ID); upErr != nil { + log.Printf("Failed to mark manual refund %s completed after Square reconcile: %v", pr.ID, upErr) + } + default: + if _, upErr := db.Conn.Exec(ctx, ` + UPDATE refunds SET status = 'failed' + WHERE id = $1 + `, pr.ID); upErr != nil { + log.Printf("Failed to mark manual refund %s failed after 3 attempts: %v", pr.ID, upErr) + } + insertRefundFailedNotifications(ctx, []string{pr.ID}) + // TODO (P6 email/SMS delivery): notify user AND admin when the email/SMS + // system lands; until then the admin_notifications row above is the only + // channel. + log.Printf("Manual refund %s reached 3 attempts with no COMPLETED refund found at Square — marked 'failed' and admin notified; TODO email user+admin, VERIFY Square dashboard before arranging in-person cash pickup at the salon (give at least a day's notice for cash on hand)", pr.ID) + } +} + +func currentRefundAttempts(ctx context.Context, refundID string) int { + var n int + if err := db.Conn.QueryRow(ctx, `SELECT refund_attempts FROM refunds WHERE id = $1`, refundID).Scan(&n); err != nil { + log.Printf("Failed to read refund_attempts for %s: %v", refundID, err) + } + return n +} diff --git a/backend/handlers/payments/refunds_test.go b/backend/handlers/payments/refunds_test.go index 521f551..a1d86da 100644 --- a/backend/handlers/payments/refunds_test.go +++ b/backend/handlers/payments/refunds_test.go @@ -4,15 +4,82 @@ package payments import ( "context" + "fmt" + "net/http" + "sync" "testing" "time" "crussell/clock" "crussell/db" + "crussell/internal/square" "crussell/testutils" "crussell/testutils/fixtures" + "crussell/testutils/jwt" ) +// countingRefundClient wraps a square.SquareClient and records every +// RefundPayment call so tests can assert exactly-once charge-level refunds. +type countingRefundClient struct { + square.SquareClient + mu sync.Mutex + calls []square.RefundPaymentReq +} + +func (c *countingRefundClient) RefundPayment(ctx context.Context, req square.RefundPaymentReq) (*square.RefundResult, error) { + c.mu.Lock() + c.calls = append(c.calls, req) + c.mu.Unlock() + return c.SquareClient.RefundPayment(ctx, req) +} + +func (c *countingRefundClient) refundCalls() []square.RefundPaymentReq { + c.mu.Lock() + defer c.mu.Unlock() + return append([]square.RefundPaymentReq(nil), c.calls...) +} + +// ambiguousRefundClient simulates a transport-level failure (no sentinel +// error) — Square may or may not have processed the refund. +type ambiguousRefundClient struct { + square.SquareClient +} + +func (c *ambiguousRefundClient) RefundPayment(ctx context.Context, req square.RefundPaymentReq) (*square.RefundResult, error) { + return nil, fmt.Errorf("network error: connection reset by peer") +} + +// ListPaymentRefunds simulates the same transport failure so reconcile paths +// fail gracefully (log + nil) instead of panicking on the nil embedded client. +func (c *ambiguousRefundClient) ListPaymentRefunds(ctx context.Context, paymentID string, beginTime time.Time) ([]square.RefundResult, error) { + return nil, fmt.Errorf("network error: connection reset by peer") +} + +// reconcileErrorClient wraps a working SquareClient (so RefundPayment +// succeeds) but simulates a transport failure on ListPaymentRefunds — the +// reconcile, not the refund call, fails. Used to lock the tri-state error +// branch: an unknown reconcile state must leave rows pending. +type reconcileErrorClient struct { + square.SquareClient +} + +func (c *reconcileErrorClient) ListPaymentRefunds(ctx context.Context, paymentID string, beginTime time.Time) ([]square.RefundResult, error) { + return nil, fmt.Errorf("network error: connection reset by peer") +} + +// slowRefundClient delays the Square call so the manual RefundPayment handler +// holds its advisory lock long enough that a concurrent cancellation refund +// would race it without the D1 serialization locks. +type slowRefundClient struct { + square.SquareClient + delay time.Duration +} + +func (c *slowRefundClient) RefundPayment(ctx context.Context, req square.RefundPaymentReq) (*square.RefundResult, error) { + time.Sleep(c.delay) + return c.SquareClient.RefundPayment(ctx, req) +} + // ============================================================================= // CalculateRefundForCancellation - Pure function tests // ============================================================================= @@ -472,16 +539,28 @@ func TestProcessCancellationRefund_CardSquareRefundWithoutBalanceCredit(t *testi // Square API refund is processed AFTER the transaction commits (see // ProcessPendingSquareRefunds). In dev/test the payment has no - // square_payment_id, so the pending refund is marked "completed" without - // a Square API call. No balance credit is generated — the refund record - // existence is the authoritative record of the refund. + // square_payment_id, so the pending cancellation refund can never be + // issued via Square — it is marked "failed" (no API call possible) and + // surfaced for in-person arrangement. No balance credit is generated. var status string err = tx.QueryRow(ctx, "SELECT status FROM refunds WHERE booking_id = $1", bookingID).Scan(&status) if err != nil { t.Fatalf("failed to query refund status: %v", err) } - if status != "completed" { - t.Errorf("expected refund status 'completed', got %q", status) + if status != "failed" { + t.Errorf("expected refund status 'failed' (card refund with no Square reference), got %q", status) + } + + // P1-B: the pre-pass must surface an admin_notifications row for the + // affected booking (in-person arrangement needed). + var notifCount int + err = tx.QueryRow(ctx, + `SELECT COUNT(*) FROM admin_notifications WHERE booking_id = $1 AND reason = 'refund_failed'`, bookingID).Scan(¬ifCount) + if err != nil { + t.Fatalf("failed to query admin_notifications: %v", err) + } + if notifCount < 1 { + t.Errorf("expected at least 1 admin_notification with reason 'refund_failed', got %d", notifCount) } // No balance credit should have been created (Square payment method uses @@ -813,14 +892,14 @@ func TestProcessCancellationRefund_GuestCashDoesNotCreditBalance(t *testing.T) { } // ============================================================================= -// Refund with split payments — verify dedup when 2 records share square_payment_id +// Refund with split payments — verify ONE Square refund per charge // ============================================================================= -func TestProcessCancellationRefund_SplitPayment_DeduplicatesSquareRefund(t *testing.T) { +func TestProcessCancellationRefund_SplitPayment_SingleSquareRefund(t *testing.T) { // When a single Square charge is split into 2 DB payment records (deposit + balance) - // sharing the same square_payment_id, the refund loop must only call Square once. - // The second record should be credited to the user balance instead. - t.Parallel() + // sharing the same square_payment_id, the refund loop must produce exactly ONE + // Square refund covering the whole charge — never one per record, and never + // one call plus a silent "completed" with no money moved. ctx, tx := testutils.SetupTestTx(t) userID, err := fixtures.CreateTestUser(tx) @@ -879,12 +958,31 @@ func TestProcessCancellationRefund_SplitPayment_DeduplicatesSquareRefund(t *test t.Fatalf("failed to create balance record: %v", err) } + // Commit the setup: ProcessCancellationRefund acquires pg_advisory_xact_lock + // on the card payments, and in the test env those locks would be held by the + // outer per-test transaction (savepoints don't release xact locks) — which + // would deadlock the post-commit sweep's session locks. Running at pool level + // mirrors production, where the cancellation tx commits and releases the locks. + pgxTx := db.TxFromContext(ctx) + if pgxTx == nil { + t.Fatal("no transaction in context") + } + if err := pgxTx.Commit(ctx); err != nil { + t.Fatalf("failed to commit test tx: %v", err) + } + freshCtx := context.Background() + + origClient := SquareClient + counting := &countingRefundClient{SquareClient: square.NewDevClient()} + SquareClient = counting + defer func() { SquareClient = origClient }() + // Cancel 72+ hours before → full refund of £50. farFuture := time.Date(2099, 12, 28, 8, 0, 0, 0, time.UTC) start := time.Date(2099, 12, 31, 10, 0, 0, 0, time.UTC) result, err := ProcessCancellationRefund( - ctx, bookingID, 100, 50, + freshCtx, bookingID, 100, 50, start, farFuture, "client_cancelled", &userID, ) if err != nil { @@ -894,18 +992,68 @@ func TestProcessCancellationRefund_SplitPayment_DeduplicatesSquareRefund(t *test t.Errorf("expected refundable 50, got %.2f", result.RefundableAmount) } - // Should have created 1 Square refund (for the deposit record) and credited - // the balance portion via user balance. + // Exactly ONE Square call for the charge — the aggregated amount, keyed + // by the stable charge-level idempotency key. + calls := counting.refundCalls() + if len(calls) != 1 { + t.Fatalf("expected exactly 1 Square refund call for the charge, got %d", len(calls)) + } + if calls[0].Amount != 5000 { + t.Errorf("expected aggregated Square refund of 5000 pence, got %d", calls[0].Amount) + } + if calls[0].IdempotencyKey != sameSquareID+"-square-agg" { + t.Errorf("expected charge-level idempotency key %q, got %q", sameSquareID+"-square-agg", calls[0].IdempotencyKey) + } + + // Both refund records must be completed and share ONE square_refund_id — + // the old bug left one record completed with square_refund_id NULL (no + // money moved for it). var refundCount int - err = tx.QueryRow(ctx, + err = db.Conn.QueryRow(freshCtx, "SELECT COUNT(*) FROM refunds WHERE booking_id = $1", bookingID).Scan(&refundCount) if err != nil { t.Fatalf("failed to query refunds: %v", err) } if refundCount != 2 { - t.Errorf("expected 2 refund records (1 Square + 1 balance credit), got %d", refundCount) + t.Errorf("expected 2 refund records, got %d", refundCount) } + rows, err := db.Conn.Query(freshCtx, ` + SELECT amount, status, square_refund_id FROM refunds WHERE booking_id = $1 ORDER BY amount + `, bookingID) + if err != nil { + t.Fatalf("failed to query refund rows: %v", err) + } + defer rows.Close() + + var refundIDs []string + refundCount = 0 + for rows.Next() { + var amount float64 + var status string + var sqRefundID *string + if err := rows.Scan(&amount, &status, &sqRefundID); err != nil { + t.Fatalf("failed to scan refund row: %v", err) + } + refundCount++ + if amount != 25.00 { + t.Errorf("expected refund amount 25.00, got %.2f", amount) + } + if status != "completed" { + t.Errorf("expected refund status 'completed', got %q", status) + } + if sqRefundID == nil || *sqRefundID == "" { + t.Error("expected square_refund_id to be set — the old bug left it NULL with money not moved") + } else { + refundIDs = append(refundIDs, *sqRefundID) + } + } + if refundCount != 2 { + t.Errorf("expected 2 refund rows, got %d", refundCount) + } + if len(refundIDs) == 2 && refundIDs[0] != refundIDs[1] { + t.Errorf("expected both records to share the SAME square_refund_id, got %q and %q", refundIDs[0], refundIDs[1]) + } } // ============================================================================= @@ -913,9 +1061,9 @@ func TestProcessCancellationRefund_SplitPayment_DeduplicatesSquareRefund(t *test // ============================================================================= // TestProcessPendingSquareRefunds_ProcessesPendingRecords verifies that -// ProcessPendingSquareRefunds queries for "pending" Square refund records -// without square_refund_id and marks them "completed" (no actual Square -// API call when square_payment_id is NULL in mock/dev). +// ProcessPendingSquareRefunds resolves a pending cancellation card refund whose +// payment has NO square_payment_id: it can never be refunded via Square, so it +// is marked "failed" (terminal pre-pass) rather than silently completed. func TestProcessPendingSquareRefunds_ProcessesPendingRecords(t *testing.T) { t.Parallel() ctx, tx := testutils.SetupTestTx(t) @@ -949,8 +1097,8 @@ func TestProcessPendingSquareRefunds_ProcessesPendingRecords(t *testing.T) { // Manually insert a "pending" refund record (simulating what ProcessCancellationRefundTx creates) _, err = tx.Exec(ctx, ` - INSERT INTO refunds (payment_id, booking_id, amount, status, reason, created_at) - SELECT id, $1, amount, 'pending', 'client_cancelled', NOW() + INSERT INTO refunds (payment_id, booking_id, amount, status, reason, origin, created_at) + SELECT id, $1, amount, 'pending', 'client_cancelled', 'cancellation', NOW() FROM payments WHERE booking_id = $1 AND payment_method = 'online_square' `, bookingID) if err != nil { @@ -972,14 +1120,14 @@ func TestProcessPendingSquareRefunds_ProcessesPendingRecords(t *testing.T) { // Now call the post-commit function ProcessPendingSquareRefunds(freshCtx, bookingID, "client_cancelled") - // Verify the refund record was marked completed + // The card refund has no Square reference — it must be marked failed. var status string err = db.Conn.QueryRow(freshCtx, `SELECT status FROM refunds WHERE booking_id = $1`, bookingID).Scan(&status) if err != nil { t.Fatalf("failed to query refund status: %v", err) } - if status != "completed" { - t.Errorf("expected refund status 'completed', got %q", status) + if status != "failed" { + t.Errorf("expected refund status 'failed' (no square_payment_id), got %q", status) } } @@ -1046,6 +1194,194 @@ func TestProcessPendingSquareRefunds_SkipsCompletedRecords(t *testing.T) { } } +// TestProcessPendingSquareRefunds_IssuesRefundWithSquareCall verifies that a +// pending cancellation refund on a card charge with a square_payment_id is +// actually issued to Square (real API call) and completed with the returned +// square_refund_id. The charge-level idempotency key is used for the call. +func TestProcessPendingSquareRefunds_IssuesRefundWithSquareCall(t *testing.T) { + t.Parallel() + ctx, tx := testutils.SetupTestTx(t) + + userID, err := fixtures.CreateTestUser(tx) + if err != nil { + t.Fatalf("failed to create user: %v", err) + } + + serviceID, err := fixtures.CreateTestService(tx) + if err != nil { + t.Fatalf("failed to create service: %v", err) + } + + bookingID, err := fixtures.CreateTestBookingAtTime(tx, userID, serviceID, + time.Date(2099, 12, 31, 10, 0, 0, 0, time.UTC)) + if err != nil { + t.Fatalf("failed to create booking: %v", err) + } + + // Create an online_square payment with a square_payment_id so the scheduler + // calls the mock. + paymentID, err := fixtures.CreateTestPayment(tx, bookingID, 25.00, "online_square", "deposit", "completed") + if err != nil { + t.Fatalf("failed to create card payment: %v", err) + } + _, err = tx.Exec(ctx, "UPDATE payments SET square_payment_id = 'sqp_stored_key' WHERE id = $1", paymentID) + if err != nil { + t.Fatalf("failed to set square_payment_id: %v", err) + } + + // Seed a pending CANCELLATION refund with a stored idempotency key. + storedKey := paymentID + "-square-2500" + var refundID string + err = tx.QueryRow(ctx, ` + INSERT INTO refunds (payment_id, booking_id, amount, status, reason, idempotency_key, origin, created_at) + VALUES ($1, $2, 25, 'pending', 'client_cancelled', $3, 'cancellation', NOW()) + RETURNING id + `, paymentID, bookingID, storedKey).Scan(&refundID) + if err != nil { + t.Fatalf("failed to insert pending refund: %v", err) + } + + // Commit the test tx so the refund rows persist (scheduler reads via pool). + pgxTx := db.TxFromContext(ctx) + if pgxTx == nil { + t.Fatal("no transaction in context") + } + if err := pgxTx.Commit(ctx); err != nil { + t.Fatalf("failed to commit test tx: %v", err) + } + + freshCtx := context.Background() + ProcessPendingSquareRefunds(freshCtx, bookingID, "client_cancelled") + + // The refund must be completed with a square_refund_id — the Square call + // happened (with the stable charge-level idempotency key). + var status string + var squareRefundID *string + err = db.Conn.QueryRow(freshCtx, `SELECT status, square_refund_id FROM refunds WHERE id = $1`, refundID).Scan(&status, &squareRefundID) + if err != nil { + t.Fatalf("failed to query refund: %v", err) + } + if status != "completed" { + t.Errorf("expected refund status 'completed', got %q", status) + } + if squareRefundID == nil || *squareRefundID == "" { + t.Error("expected square_refund_id to be set (Square was called)") + } +} + +// TestProcessPendingSquareRefunds_SameChargePending_IssuesRefund verifies that +// a pending cancellation refund on a split record sharing a square_payment_id +// with an already-completed manual refund is ISSUED (money moved) for the +// residual — the amount-blind suppression (the P0 bug) would have marked it +// completed with no money moving. +func TestProcessPendingSquareRefunds_SameChargePending_IssuesRefund(t *testing.T) { + ctx, tx := testutils.SetupTestTx(t) + + userID, err := fixtures.CreateTestUser(tx) + if err != nil { + t.Fatalf("failed to create user: %v", err) + } + + serviceID, err := fixtures.CreateTestService(tx) + if err != nil { + t.Fatalf("failed to create service: %v", err) + } + + bookingID, err := fixtures.CreateTestBookingAtTime(tx, userID, serviceID, + time.Date(2099, 12, 31, 10, 0, 0, 0, time.UTC)) + if err != nil { + t.Fatalf("failed to create booking: %v", err) + } + + // Split payment records sharing one square_payment_id (deposit + balance). + depositID, err := fixtures.CreateTestPayment(tx, bookingID, 25.00, "online_square", "deposit", "completed") + if err != nil { + t.Fatalf("failed to create deposit payment: %v", err) + } + balanceID, err := fixtures.CreateTestPayment(tx, bookingID, 25.00, "online_square", "balance", "completed") + if err != nil { + t.Fatalf("failed to create balance payment: %v", err) + } + sameSquareID := "sqp_same_charge_pending" + _, err = tx.Exec(ctx, "UPDATE payments SET square_payment_id = $1 WHERE id IN ($2, $3)", sameSquareID, depositID, balanceID) + if err != nil { + t.Fatalf("failed to set square_payment_id: %v", err) + } + + // A completed MANUAL refund on the deposit record (money already moved back). + _, err = tx.Exec(ctx, ` + INSERT INTO refunds (payment_id, booking_id, amount, status, reason, square_refund_id, idempotency_key, origin, created_at) + VALUES ($1, $2, 25, 'completed', 'manual refund', 'ref_seeded', $3, 'manual', NOW()) + `, depositID, bookingID, depositID+"-refund-2500") + if err != nil { + t.Fatalf("failed to insert completed refund: %v", err) + } + + // A pending CANCELLATION refund on the balance record sharing the charge. + _, err = tx.Exec(ctx, ` + INSERT INTO refunds (payment_id, booking_id, amount, status, reason, idempotency_key, origin, created_at) + VALUES ($1, $2, 25, 'pending', 'client_cancelled', $3, 'cancellation', NOW()) + `, balanceID, bookingID, balanceID+"-square-2500") + if err != nil { + t.Fatalf("failed to insert pending refund: %v", err) + } + + // Commit the test tx so the refund rows persist (scheduler reads via pool). + pgxTx := db.TxFromContext(ctx) + if pgxTx == nil { + t.Fatal("no transaction in context") + } + if err := pgxTx.Commit(ctx); err != nil { + t.Fatalf("failed to commit test tx: %v", err) + } + + origClient := SquareClient + counting := &countingRefundClient{SquareClient: square.NewDevClient()} + SquareClient = counting + defer func() { SquareClient = origClient }() + + freshCtx := context.Background() + ProcessPendingSquareRefunds(freshCtx, bookingID, "client_cancelled") + + // The residual £25 must be ISSUED — a real Square call happens (the old + // amount-blind suppression marked it completed with NO money moved). + calls := counting.refundCalls() + if len(calls) != 1 { + t.Fatalf("expected exactly 1 Square refund call for the residual, got %d", len(calls)) + } + if calls[0].Amount != 2500 { + t.Errorf("expected Square refund of 2500 pence (the residual), got %d", calls[0].Amount) + } + if calls[0].IdempotencyKey != sameSquareID+"-square-agg" { + t.Errorf("expected charge-level idempotency key %q, got %q", sameSquareID+"-square-agg", calls[0].IdempotencyKey) + } + + // The balance pending refund must be completed WITH a square_refund_id — + // money moved, NOT left NULL. + var status string + var squareRefundID *string + err = db.Conn.QueryRow(freshCtx, `SELECT status, square_refund_id FROM refunds WHERE payment_id = $1`, balanceID).Scan(&status, &squareRefundID) + if err != nil { + t.Fatalf("failed to query refund: %v", err) + } + if status != "completed" { + t.Errorf("expected balance refund status 'completed', got %q", status) + } + if squareRefundID == nil || *squareRefundID == "" { + t.Error("expected square_refund_id to be set (money moved) — the P0 bug left it NULL") + } + + // The completed manual refund is untouched. + var manualStatus string + err = db.Conn.QueryRow(freshCtx, `SELECT status FROM refunds WHERE payment_id = $1`, depositID).Scan(&manualStatus) + if err != nil { + t.Fatalf("failed to query manual refund: %v", err) + } + if manualStatus != "completed" { + t.Errorf("expected manual refund status 'completed', got %q", manualStatus) + } +} + // ============================================================================= // ProcessCancellationRefundTx — transactional variant // ============================================================================= @@ -1088,7 +1424,7 @@ func TestProcessCancellationRefundTx_Success(t *testing.T) { now := time.Date(2099, 12, 28, 8, 0, 0, 0, time.UTC) start := time.Date(2099, 12, 31, 10, 0, 0, 0, time.UTC) - result, err := ProcessCancellationRefundTx(ctx, innerTx, bookingID, 50, 50, start, now, "client_cancelled", &userID) + result, err := ProcessCancellationRefundTx(ctx, innerTx, bookingID, 50, 50, start, now, "client_cancelled", &userID, false) if err != nil { t.Fatalf("ProcessCancellationRefundTx failed: %v", err) } @@ -1150,7 +1486,7 @@ func TestProcessCancellationRefundTx_NoRefundNeeded(t *testing.T) { now := time.Date(2099, 12, 31, 12, 0, 0, 0, time.UTC) start := time.Date(2099, 12, 31, 10, 0, 0, 0, time.UTC) - result, err := ProcessCancellationRefundTx(ctx, innerTx, bookingID, 100, 0, start, now, "no_show", &userID) + result, err := ProcessCancellationRefundTx(ctx, innerTx, bookingID, 100, 0, start, now, "no_show", &userID, false) if err != nil { t.Fatalf("ProcessCancellationRefundTx failed: %v", err) } @@ -1173,6 +1509,108 @@ func TestProcessCancellationRefundTx_NoRefundNeeded(t *testing.T) { } } +// TestProcessCancellationRefund_DoubleCancel_FailedRow_Dedups locks the P3a fix: +// a second cancel whose prior refund row is 'failed' (money never moved at +// Square, so the residual is recomputed in full) must NOT crash on the UNIQUE +// idempotency_key. The ON CONFLICT DO NOTHING dedup skips the INSERT without +// creating a duplicate row and without touching the original 'failed' row. +func TestProcessCancellationRefund_DoubleCancel_FailedRow_Dedups(t *testing.T) { + ctx, tx := testutils.SetupTestTx(t) + innerTx := db.TxFromContext(ctx) + if innerTx == nil { + t.Fatal("no transaction in context") + } + + userID, err := fixtures.CreateTestUser(tx) + if err != nil { + t.Fatalf("failed to create user: %v", err) + } + + serviceID, err := fixtures.CreateTestService(tx) + if err != nil { + t.Fatalf("failed to create service: %v", err) + } + + // >72h before the appointment → full refund. + bookingID, err := fixtures.CreateTestBookingAtTime(tx, userID, serviceID, + time.Date(2099, 12, 31, 10, 0, 0, 0, time.UTC)) + if err != nil { + t.Fatalf("failed to create booking: %v", err) + } + + paymentID, err := fixtures.CreateTestPayment(tx, bookingID, 50, "online_square", "full", "completed") + if err != nil { + t.Fatalf("failed to create payment: %v", err) + } + // Square-backed payment (not Square-less) so the row is recorded 'pending' + // with the deterministic idempotency key paymentID-square-5000. + _, err = tx.Exec(ctx, "UPDATE payments SET square_payment_id = $1 WHERE id = $2", "sqp_double_cancel", paymentID) + if err != nil { + t.Fatalf("failed to set square_payment_id: %v", err) + } + + now := time.Date(2099, 12, 28, 8, 0, 0, 0, time.UTC) + start := time.Date(2099, 12, 31, 10, 0, 0, 0, time.UTC) + + // First cancel creates the pending refund row with the deterministic key. + result, err := ProcessCancellationRefundTx(ctx, innerTx, bookingID, 50, 50, start, now, "client_cancelled", &userID, false) + if err != nil { + t.Fatalf("first ProcessCancellationRefundTx failed: %v", err) + } + if result == nil || result.RefundableAmount != 50 { + t.Fatalf("expected full refund of 50, got %+v", result) + } + + var rowCount int + err = tx.QueryRow(ctx, "SELECT COUNT(*) FROM refunds WHERE payment_id = $1", paymentID).Scan(&rowCount) + if err != nil { + t.Fatalf("failed to count refund rows after first cancel: %v", err) + } + if rowCount != 1 { + t.Fatalf("expected 1 refund row after first cancel, got %d", rowCount) + } + var rowKey string + err = tx.QueryRow(ctx, "SELECT idempotency_key FROM refunds WHERE payment_id = $1", paymentID).Scan(&rowKey) + if err != nil { + t.Fatalf("failed to query refund idempotency_key: %v", err) + } + if rowKey != paymentID+"-square-5000" { + t.Fatalf("expected idempotency_key %q, got %q", paymentID+"-square-5000", rowKey) + } + + // Simulate the sweep failing all 3 attempts: the row flips to 'failed'. + // 'failed' is deliberately excluded from the residual sum (money never + // moved at Square), so a second cancel recomputes the FULL residual and + // attempts to INSERT the same idempotency key again. + _, err = tx.Exec(ctx, "UPDATE refunds SET status = 'failed' WHERE payment_id = $1", paymentID) + if err != nil { + t.Fatalf("failed to flip refund row to failed: %v", err) + } + + // Second cancel: same payment, same amount, same key. The ON CONFLICT + // (idempotency_key) DO NOTHING must dedup — no error, no duplicate row, + // and the original 'failed' row untouched. + if _, err := ProcessCancellationRefundTx(ctx, innerTx, bookingID, 50, 50, start, now, "client_cancelled", &userID, false); err != nil { + t.Fatalf("second ProcessCancellationRefundTx failed: %v", err) + } + + err = tx.QueryRow(ctx, "SELECT COUNT(*) FROM refunds WHERE payment_id = $1", paymentID).Scan(&rowCount) + if err != nil { + t.Fatalf("failed to count refund rows after second cancel: %v", err) + } + if rowCount != 1 { + t.Errorf("expected exactly 1 refund row after dedup, got %d", rowCount) + } + var rowStatus string + err = tx.QueryRow(ctx, "SELECT status FROM refunds WHERE payment_id = $1", paymentID).Scan(&rowStatus) + if err != nil { + t.Fatalf("failed to query refund status: %v", err) + } + if rowStatus != "failed" { + t.Errorf("expected original row still 'failed' (untouched), got %q", rowStatus) + } +} + // ============================================================================= // CreateRefundRecord — PaymentService method // ============================================================================= @@ -1342,3 +1780,1095 @@ func TestCreateRefundRecord_NilCreatedBy(t *testing.T) { t.Errorf("expected created_by NULL, got %q", *storedCreatedBy) } } + +// ============================================================================= +// Charge-level aggregation & sweep tests +// ============================================================================= + +// TestProcessPendingSquareRefunds_SplitCharge_OneAggregateRefund verifies the +// P0 fix: two pending refund rows sharing one square_payment_id produce ONE +// Square refund of the aggregate amount, and both rows complete with the SAME +// square_refund_id (never one row completed with money unmoved). +func TestProcessPendingSquareRefunds_SplitCharge_OneAggregateRefund(t *testing.T) { + ctx, tx := testutils.SetupTestTx(t) + + userID, err := fixtures.CreateTestUser(tx) + if err != nil { + t.Fatalf("failed to create user: %v", err) + } + + serviceID, err := fixtures.CreateTestService(tx) + if err != nil { + t.Fatalf("failed to create service: %v", err) + } + + bookingID, err := fixtures.CreateTestBookingAtTime(tx, userID, serviceID, + time.Date(2099, 12, 31, 10, 0, 0, 0, time.UTC)) + if err != nil { + t.Fatalf("failed to create booking: %v", err) + } + + // Split payment records sharing one square_payment_id. + depositID, err := fixtures.CreateTestPayment(tx, bookingID, 25.00, "online_square", "deposit", "completed") + if err != nil { + t.Fatalf("failed to create deposit payment: %v", err) + } + balanceID, err := fixtures.CreateTestPayment(tx, bookingID, 25.00, "online_square", "balance", "completed") + if err != nil { + t.Fatalf("failed to create balance payment: %v", err) + } + sameSquareID := "sqp_aggregate_split" + _, err = tx.Exec(ctx, "UPDATE payments SET square_payment_id = $1 WHERE id IN ($2, $3)", sameSquareID, depositID, balanceID) + if err != nil { + t.Fatalf("failed to set square_payment_id: %v", err) + } + + // Two pending cancellation refunds (deposit £25 + balance £25). + for _, pid := range []string{depositID, balanceID} { + _, err = tx.Exec(ctx, ` + INSERT INTO refunds (payment_id, booking_id, amount, status, reason, idempotency_key, origin, created_at) + VALUES ($1, $2, 25, 'pending', 'client_cancelled', $3, 'cancellation', NOW()) + `, pid, bookingID, pid+"-square-2500") + if err != nil { + t.Fatalf("failed to insert pending refund: %v", err) + } + } + + pgxTx := db.TxFromContext(ctx) + if pgxTx == nil { + t.Fatal("no transaction in context") + } + if err := pgxTx.Commit(ctx); err != nil { + t.Fatalf("failed to commit test tx: %v", err) + } + + origClient := SquareClient + counting := &countingRefundClient{SquareClient: square.NewDevClient()} + SquareClient = counting + defer func() { SquareClient = origClient }() + + freshCtx := context.Background() + ProcessPendingSquareRefunds(freshCtx, bookingID, "client_cancelled") + + calls := counting.refundCalls() + if len(calls) != 1 { + t.Fatalf("expected exactly 1 Square refund call for the charge, got %d", len(calls)) + } + if calls[0].Amount != 5000 { + t.Errorf("expected aggregated amount 5000 pence (25+25), got %d", calls[0].Amount) + } + if calls[0].IdempotencyKey != sameSquareID+"-square-agg" { + t.Errorf("expected idempotency key %q, got %q", sameSquareID+"-square-agg", calls[0].IdempotencyKey) + } + + // Both rows completed with the same square_refund_id, amounts preserved. + rows, err := db.Conn.Query(freshCtx, ` + SELECT amount, status, square_refund_id FROM refunds WHERE booking_id = $1 ORDER BY amount + `, bookingID) + if err != nil { + t.Fatalf("failed to query refunds: %v", err) + } + defer rows.Close() + var sqIDs []string + count := 0 + for rows.Next() { + var amount float64 + var status string + var sqID *string + if err := rows.Scan(&amount, &status, &sqID); err != nil { + t.Fatalf("failed to scan refund row: %v", err) + } + count++ + if amount != 25.00 { + t.Errorf("expected amount 25.00 preserved, got %.2f", amount) + } + if status != "completed" { + t.Errorf("expected status 'completed', got %q", status) + } + if sqID == nil || *sqID == "" { + t.Error("expected square_refund_id set (money moved)") + } else { + sqIDs = append(sqIDs, *sqID) + } + } + if count != 2 { + t.Errorf("expected 2 refund rows, got %d", count) + } + if len(sqIDs) == 2 && sqIDs[0] != sqIDs[1] { + t.Errorf("expected both rows to share the SAME square_refund_id, got %q and %q", sqIDs[0], sqIDs[1]) + } +} + +// TestSweepPendingSquareRefunds_NullSquareRef_NoReference_MarksFailed verifies +// the sweep's terminal pre-pass: a pending cancellation card refund with NO +// square_payment_id can never be refunded via Square — it is marked failed. +func TestSweepPendingSquareRefunds_NullSquareRef_NoReference_MarksFailed(t *testing.T) { + ctx, tx := testutils.SetupTestTx(t) + + userID, err := fixtures.CreateTestUser(tx) + if err != nil { + t.Fatalf("failed to create user: %v", err) + } + + serviceID, err := fixtures.CreateTestService(tx) + if err != nil { + t.Fatalf("failed to create service: %v", err) + } + + bookingID, err := fixtures.CreateTestBookingAtTime(tx, userID, serviceID, + time.Date(2099, 12, 31, 10, 0, 0, 0, time.UTC)) + if err != nil { + t.Fatalf("failed to create booking: %v", err) + } + + // online_square payment with square_payment_id NULL (fixture default). + paymentID, err := fixtures.CreateTestPayment(tx, bookingID, 50.00, "online_square", "full", "completed") + if err != nil { + t.Fatalf("failed to create card payment: %v", err) + } + + _, err = tx.Exec(ctx, ` + INSERT INTO refunds (payment_id, booking_id, amount, status, reason, origin, created_at) + VALUES ($1, $2, 50, 'pending', 'client_cancelled', 'cancellation', NOW()) + `, paymentID, bookingID) + if err != nil { + t.Fatalf("failed to insert pending refund: %v", err) + } + + pgxTx := db.TxFromContext(ctx) + if pgxTx == nil { + t.Fatal("no transaction in context") + } + if err := pgxTx.Commit(ctx); err != nil { + t.Fatalf("failed to commit test tx: %v", err) + } + + origClient := SquareClient + counting := &countingRefundClient{SquareClient: square.NewDevClient()} + SquareClient = counting + defer func() { SquareClient = origClient }() + + freshCtx := context.Background() + if _, err := SweepPendingSquareRefunds(freshCtx); err != nil { + t.Fatalf("SweepPendingSquareRefunds failed: %v", err) + } + + var status string + err = db.Conn.QueryRow(freshCtx, `SELECT status FROM refunds WHERE payment_id = $1`, paymentID).Scan(&status) + if err != nil { + t.Fatalf("failed to query refund status: %v", err) + } + if status != "failed" { + t.Errorf("expected refund status 'failed', got %q", status) + } + + if calls := counting.refundCalls(); len(calls) != 0 { + t.Errorf("expected NO Square calls (no square_payment_id), got %d", len(calls)) + } + + // P1-B: the terminal pre-pass must surface an admin_notifications row for + // the affected booking (in-person arrangement needed). + var notifCount int + err = db.Conn.QueryRow(freshCtx, + `SELECT COUNT(*) FROM admin_notifications WHERE booking_id = $1 AND reason = 'refund_failed'`, bookingID).Scan(¬ifCount) + if err != nil { + t.Fatalf("failed to query admin_notifications: %v", err) + } + if notifCount < 1 { + t.Errorf("expected at least 1 admin_notification with reason 'refund_failed', got %d", notifCount) + } +} + +// TestSweepPendingSquareRefunds_AttemptsExhausted_NotProcessed verifies that a +// pending refund row already at the 3-attempt cap is never processed by the +// sweep: it is filtered out (refund_attempts < 3), no Square call happens, and +// its status is left untouched. +func TestSweepPendingSquareRefunds_AttemptsExhausted_NotProcessed(t *testing.T) { + ctx, tx := testutils.SetupTestTx(t) + + userID, err := fixtures.CreateTestUser(tx) + if err != nil { + t.Fatalf("failed to create user: %v", err) + } + + serviceID, err := fixtures.CreateTestService(tx) + if err != nil { + t.Fatalf("failed to create service: %v", err) + } + + bookingID, err := fixtures.CreateTestBookingAtTime(tx, userID, serviceID, + time.Date(2099, 12, 31, 10, 0, 0, 0, time.UTC)) + if err != nil { + t.Fatalf("failed to create booking: %v", err) + } + + paymentID, err := fixtures.CreateTestPayment(tx, bookingID, 50.00, "online_square", "full", "completed") + if err != nil { + t.Fatalf("failed to create card payment: %v", err) + } + _, err = tx.Exec(ctx, "UPDATE payments SET square_payment_id = 'sqp_attempts_exhausted' WHERE id = $1", paymentID) + if err != nil { + t.Fatalf("failed to set square_payment_id: %v", err) + } + + // Pending cancellation refund already at the 3-attempt cap. + _, err = tx.Exec(ctx, ` + INSERT INTO refunds (payment_id, booking_id, amount, status, reason, origin, refund_attempts, created_at) + VALUES ($1, $2, 50, 'pending', 'client_cancelled', 'cancellation', 3, NOW()) + `, paymentID, bookingID) + if err != nil { + t.Fatalf("failed to insert pending refund: %v", err) + } + + pgxTx := db.TxFromContext(ctx) + if pgxTx == nil { + t.Fatal("no transaction in context") + } + if err := pgxTx.Commit(ctx); err != nil { + t.Fatalf("failed to commit test tx: %v", err) + } + + origClient := SquareClient + counting := &countingRefundClient{SquareClient: square.NewDevClient()} + SquareClient = counting + defer func() { SquareClient = origClient }() + + freshCtx := context.Background() + if _, err := SweepPendingSquareRefunds(freshCtx); err != nil { + t.Fatalf("SweepPendingSquareRefunds failed: %v", err) + } + + var status string + var attempts int + err = db.Conn.QueryRow(freshCtx, `SELECT status, refund_attempts FROM refunds WHERE payment_id = $1`, paymentID).Scan(&status, &attempts) + if err != nil { + t.Fatalf("failed to query refund: %v", err) + } + if status != "pending" { + t.Errorf("expected status untouched ('pending'), got %q", status) + } + if attempts != 3 { + t.Errorf("expected refund_attempts untouched (3), got %d", attempts) + } + if calls := counting.refundCalls(); len(calls) != 0 { + t.Errorf("expected NO Square calls for an attempts=3 row, got %d", len(calls)) + } +} + +// TestProcessPendingSquareRefunds_Declined_ThreeAttempts_Failed verifies the +// definitive-decline path: each run increments refund_attempts, and the third +// run marks the rows failed. +func TestProcessPendingSquareRefunds_Declined_ThreeAttempts_Failed(t *testing.T) { + ctx, tx := testutils.SetupTestTx(t) + + userID, err := fixtures.CreateTestUser(tx) + if err != nil { + t.Fatalf("failed to create user: %v", err) + } + + serviceID, err := fixtures.CreateTestService(tx) + if err != nil { + t.Fatalf("failed to create service: %v", err) + } + + bookingID, err := fixtures.CreateTestBookingAtTime(tx, userID, serviceID, + time.Date(2099, 12, 31, 10, 0, 0, 0, time.UTC)) + if err != nil { + t.Fatalf("failed to create booking: %v", err) + } + + paymentID, err := fixtures.CreateTestPayment(tx, bookingID, 25.00, "online_square", "deposit", "completed") + if err != nil { + t.Fatalf("failed to create card payment: %v", err) + } + _, err = tx.Exec(ctx, "UPDATE payments SET square_payment_id = 'sqp_declined' WHERE id = $1", paymentID) + if err != nil { + t.Fatalf("failed to set square_payment_id: %v", err) + } + + _, err = tx.Exec(ctx, ` + INSERT INTO refunds (payment_id, booking_id, amount, status, reason, origin, created_at) + VALUES ($1, $2, 25, 'pending', 'client_cancelled', 'cancellation', NOW()) + `, paymentID, bookingID) + if err != nil { + t.Fatalf("failed to insert pending refund: %v", err) + } + + pgxTx := db.TxFromContext(ctx) + if pgxTx == nil { + t.Fatal("no transaction in context") + } + if err := pgxTx.Commit(ctx); err != nil { + t.Fatalf("failed to commit test tx: %v", err) + } + + origClient := SquareClient + mock := square.NewDevClient().(*square.MockClient) + mock.FailRefundCode = "REFUND_DECLINED" + SquareClient = mock + defer func() { SquareClient = origClient }() + + freshCtx := context.Background() + + // Run 1 and 2: attempts increment, row stays pending. + ProcessPendingSquareRefunds(freshCtx, bookingID, "client_cancelled") + ProcessPendingSquareRefunds(freshCtx, bookingID, "client_cancelled") + + var status string + var attempts int + err = db.Conn.QueryRow(freshCtx, `SELECT status, refund_attempts FROM refunds WHERE payment_id = $1`, paymentID).Scan(&status, &attempts) + if err != nil { + t.Fatalf("failed to query refund: %v", err) + } + if attempts != 2 { + t.Errorf("expected refund_attempts 2 after two declined runs, got %d", attempts) + } + if status != "pending" { + t.Errorf("expected status 'pending' before the cap is hit, got %q", status) + } + + // Run 3: cap hit → failed. + ProcessPendingSquareRefunds(freshCtx, bookingID, "client_cancelled") + err = db.Conn.QueryRow(freshCtx, `SELECT status, refund_attempts FROM refunds WHERE payment_id = $1`, paymentID).Scan(&status, &attempts) + if err != nil { + t.Fatalf("failed to query refund: %v", err) + } + if status != "failed" { + t.Errorf("expected status 'failed' after 3 declined runs, got %q", status) + } + if attempts != 3 { + t.Errorf("expected refund_attempts 3 after three declined runs, got %d", attempts) + } +} + +// TestProcessPendingSquareRefunds_Ambiguous_ThreeAttempts_StaysPendingOnReconcileError +// verifies the tri-state reconcile under the ambiguous path (plain transport +// error): the row is retried up to the 3-attempt cap, and on the run that hits +// the cap the reconcile against Square ALSO fails (same transport failure) — an +// unknown state. The row MUST stay 'pending' (NOT 'failed', which would let the +// over-refund guard exclude money that may have moved) and NO admin_notification +// is inserted; the next sweep retries the reconcile. +func TestProcessPendingSquareRefunds_Ambiguous_ThreeAttempts_StaysPendingOnReconcileError(t *testing.T) { + ctx, tx := testutils.SetupTestTx(t) + + userID, err := fixtures.CreateTestUser(tx) + if err != nil { + t.Fatalf("failed to create user: %v", err) + } + + serviceID, err := fixtures.CreateTestService(tx) + if err != nil { + t.Fatalf("failed to create service: %v", err) + } + + bookingID, err := fixtures.CreateTestBookingAtTime(tx, userID, serviceID, + time.Date(2099, 12, 31, 10, 0, 0, 0, time.UTC)) + if err != nil { + t.Fatalf("failed to create booking: %v", err) + } + + paymentID, err := fixtures.CreateTestPayment(tx, bookingID, 25.00, "online_square", "deposit", "completed") + if err != nil { + t.Fatalf("failed to create card payment: %v", err) + } + _, err = tx.Exec(ctx, "UPDATE payments SET square_payment_id = 'sqp_ambiguous' WHERE id = $1", paymentID) + if err != nil { + t.Fatalf("failed to set square_payment_id: %v", err) + } + + _, err = tx.Exec(ctx, ` + INSERT INTO refunds (payment_id, booking_id, amount, status, reason, origin, created_at) + VALUES ($1, $2, 25, 'pending', 'client_cancelled', 'cancellation', NOW()) + `, paymentID, bookingID) + if err != nil { + t.Fatalf("failed to insert pending refund: %v", err) + } + + pgxTx := db.TxFromContext(ctx) + if pgxTx == nil { + t.Fatal("no transaction in context") + } + if err := pgxTx.Commit(ctx); err != nil { + t.Fatalf("failed to commit test tx: %v", err) + } + + origClient := SquareClient + SquareClient = &ambiguousRefundClient{} + defer func() { SquareClient = origClient }() + + freshCtx := context.Background() + for i := 0; i < 3; i++ { + ProcessPendingSquareRefunds(freshCtx, bookingID, "client_cancelled") + } + + var status string + var attempts int + err = db.Conn.QueryRow(freshCtx, `SELECT status, refund_attempts FROM refunds WHERE payment_id = $1`, paymentID).Scan(&status, &attempts) + if err != nil { + t.Fatalf("failed to query refund: %v", err) + } + if status != "pending" { + t.Errorf("expected status 'pending' after 3 ambiguous runs with a failing reconcile, got %q", status) + } + if attempts != 3 { + t.Errorf("expected refund_attempts 3 after three ambiguous runs, got %d", attempts) + } + + // The terminal failure must NOT fire: the reconcile returned a network + // error (unknown state), so the row stays pending and no admin + // notification is inserted. + var notifCount int + err = db.Conn.QueryRow(freshCtx, + `SELECT COUNT(*) FROM admin_notifications WHERE booking_id = $1 AND reason = 'refund_failed'`, bookingID).Scan(¬ifCount) + if err != nil { + t.Fatalf("failed to query admin_notifications: %v", err) + } + if notifCount != 0 { + t.Errorf("expected NO admin_notification with reason 'refund_failed' (reconcile error leaves rows pending), got %d", notifCount) + } +} + +// TestProcessPendingSquareRefunds_PartialManualThenCancel_IssuesResidual +// verifies the per-record prior-refund subtraction: after a £30 manual refund +// on the deposit record, a £70 cancellation refund issues the residual (£20 +// deposit + £50 balance) as ONE aggregate £70 Square refund. +func TestProcessPendingSquareRefunds_PartialManualThenCancel_IssuesResidual(t *testing.T) { + ctx, tx := testutils.SetupTestTx(t) + + userID, err := fixtures.CreateTestUser(tx) + if err != nil { + t.Fatalf("failed to create user: %v", err) + } + + serviceID, err := fixtures.CreateTestService(tx) + if err != nil { + t.Fatalf("failed to create service: %v", err) + } + + bookingID, err := fixtures.CreateTestBookingAtTime(tx, userID, serviceID, + time.Date(2099, 12, 31, 10, 0, 0, 0, time.UTC)) + if err != nil { + t.Fatalf("failed to create booking: %v", err) + } + + _, err = tx.Exec(ctx, "UPDATE bookings SET status = 'confirmed' WHERE id = $1", bookingID) + if err != nil { + t.Fatalf("failed to confirm booking: %v", err) + } + + sameSquareID := "sqp_residual" + svc := NewPaymentService() + now := clock.Now() + + depositID, err := svc.CreatePaymentRecord(ctx, PaymentRecord{ + BookingID: bookingID, + PaymentType: "deposit", + PaymentMethod: "online_square", + Status: "completed", + Amount: 50.00, + SquarePaymentID: &sameSquareID, + CreatedAt: now, + UpdatedAt: now, + }, nil) + if err != nil { + t.Fatalf("failed to create deposit record: %v", err) + } + + _, err = svc.CreatePaymentRecord(ctx, PaymentRecord{ + BookingID: bookingID, + PaymentType: "balance", + PaymentMethod: "online_square", + Status: "completed", + Amount: 50.00, + SquarePaymentID: &sameSquareID, + CreatedAt: now, + UpdatedAt: now, + }, nil) + if err != nil { + t.Fatalf("failed to create balance record: %v", err) + } + + // £30 manually refunded on the deposit record (completed, origin='manual'). + _, err = tx.Exec(ctx, ` + INSERT INTO refunds (payment_id, booking_id, amount, status, reason, square_refund_id, idempotency_key, origin, created_at) + VALUES ($1, $2, 30, 'completed', 'manual refund', 'ref_manual_30', $3, 'manual', NOW()) + `, depositID, bookingID, depositID+"-refund-3000") + if err != nil { + t.Fatalf("failed to insert manual refund: %v", err) + } + + // Commit the setup: ProcessCancellationRefund acquires pg_advisory_xact_lock + // on the card payments, and in the test env those locks would be held by the + // outer per-test transaction (savepoints don't release xact locks) — which + // would deadlock the post-commit sweep's session locks. Running at pool level + // mirrors production, where the cancellation tx commits and releases the locks. + pgxTx := db.TxFromContext(ctx) + if pgxTx == nil { + t.Fatal("no transaction in context") + } + if err := pgxTx.Commit(ctx); err != nil { + t.Fatalf("failed to commit test tx: %v", err) + } + freshCtx := context.Background() + + origClient := SquareClient + counting := &countingRefundClient{SquareClient: square.NewDevClient()} + SquareClient = counting + defer func() { SquareClient = origClient }() + + // Cancel >72h before with net paid £70 (after the £30 manual refund). + // Refundable = £70 → the loop must only issue the residual. + farFuture := time.Date(2099, 12, 28, 8, 0, 0, 0, time.UTC) + start := time.Date(2099, 12, 31, 10, 0, 0, 0, time.UTC) + result, err := ProcessCancellationRefund( + freshCtx, bookingID, 100, 70, + start, farFuture, "client_cancelled", &userID, + ) + if err != nil { + t.Fatalf("ProcessCancellationRefund failed: %v", err) + } + if result.RefundableAmount != 70 { + t.Errorf("expected refundable 70, got %.2f", result.RefundableAmount) + } + + calls := counting.refundCalls() + if len(calls) != 1 { + t.Fatalf("expected exactly 1 Square refund call for the residual, got %d", len(calls)) + } + if calls[0].Amount != 7000 { + t.Errorf("expected aggregated residual of 7000 pence (£70), got %d", calls[0].Amount) + } + if calls[0].IdempotencyKey != sameSquareID+"-square-agg" { + t.Errorf("expected idempotency key %q, got %q", sameSquareID+"-square-agg", calls[0].IdempotencyKey) + } + + // Two pending rows created for the residual: deposit £20 + balance £50. + rows, err := db.Conn.Query(freshCtx, ` + SELECT amount, status, origin FROM refunds + WHERE booking_id = $1 AND origin = 'cancellation' ORDER BY amount + `, bookingID) + if err != nil { + t.Fatalf("failed to query cancellation refunds: %v", err) + } + defer rows.Close() + var amounts []float64 + count := 0 + for rows.Next() { + var amount float64 + var status, origin string + if err := rows.Scan(&amount, &status, &origin); err != nil { + t.Fatalf("failed to scan refund row: %v", err) + } + count++ + amounts = append(amounts, amount) + if status != "completed" { + t.Errorf("expected status 'completed', got %q", status) + } + } + if count != 2 { + t.Fatalf("expected 2 cancellation refund rows, got %d", count) + } + if amounts[0] != 20.00 || amounts[1] != 50.00 { + t.Errorf("expected residual amounts £20 + £50, got %.2f + %.2f", amounts[0], amounts[1]) + } + + // The manual refund row is untouched. + var manualStatus string + var manualAmount float64 + err = db.Conn.QueryRow(freshCtx, "SELECT status, amount FROM refunds WHERE idempotency_key = $1", depositID+"-refund-3000").Scan(&manualStatus, &manualAmount) + if err != nil { + t.Fatalf("failed to query manual refund: %v", err) + } + if manualStatus != "completed" { + t.Errorf("expected manual refund status 'completed', got %q", manualStatus) + } + if manualAmount != 30.00 { + t.Errorf("expected manual refund amount 30.00, got %.2f", manualAmount) + } +} + +// ============================================================================= +// D1 — cancellation loop serializes against a concurrent manual refund +// ============================================================================= + +// TestCancellationRefund_SerializesAgainstManualRefund proves the D1 fix: the +// cancellation refund loop acquires the same per-payment advisory locks as the +// manual RefundPayment handler, so the two can never BOTH read zero prior +// refunds and both refund the same payment. Without the locks, a manual refund +// whose guard read happens before the cancellation tx commits over-refunds the +// payment by the full amount. The manual handler's Square call is slowed so the +// window is wide enough that the racy interleaving would occur without the fix. +func TestCancellationRefund_SerializesAgainstManualRefund(t *testing.T) { + ctx, tx := testutils.SetupTestTx(t) + userID, err := fixtures.CreateTestUser(tx) + if err != nil { + t.Fatalf("failed to create user: %v", err) + } + adminID, err := fixtures.CreateTestAdminUser(tx) + if err != nil { + t.Fatalf("failed to create admin user: %v", err) + } + serviceID, err := fixtures.CreateTestService(tx) + if err != nil { + t.Fatalf("failed to create service: %v", err) + } + start := time.Date(2099, 12, 31, 10, 0, 0, 0, time.UTC) + bookingID, err := fixtures.CreateTestBookingAtTime(tx, userID, serviceID, start) + if err != nil { + t.Fatalf("failed to create booking: %v", err) + } + paymentID, err := fixtures.CreateTestPayment(tx, bookingID, 100, "online_square", "full", "completed") + if err != nil { + t.Fatalf("failed to create payment: %v", err) + } + _, err = tx.Exec(ctx, "UPDATE payments SET square_payment_id = 'sqp_serialize' WHERE id = $1", paymentID) + if err != nil { + t.Fatalf("failed to set square_payment_id: %v", err) + } + // Commit the setup so both goroutines operate at pool level — the advisory + // locks only serialize across independent connections, and a per-test tx + // would route one side's reads through a single shared connection. + innerTx := db.TxFromContext(ctx) + if innerTx == nil { + t.Fatal("no transaction in context") + } + if err := innerTx.Commit(ctx); err != nil { + t.Fatalf("failed to commit setup tx: %v", err) + } + + origClient := SquareClient + slow := &slowRefundClient{SquareClient: square.NewDevClient(), delay: 300 * time.Millisecond} + SquareClient = slow + defer func() { SquareClient = origClient }() + + pool := context.Background() + adminToken := jwt.GenerateTestToken(adminID, "admin") + req := RefundRequest{Amount: 10000, Reason: "customer request"} + + var wg sync.WaitGroup + startBoth := make(chan struct{}) + var manualCode int + var manualErr, cancelErr error + + wg.Add(1) + go func() { + defer wg.Done() + <-startBoth + rec := makePaymentRequest(RefundPayment, "POST", "/api/admin/payments/"+paymentID+"/refund", req, adminToken, pool) + manualCode = rec.Code + }() + + wg.Add(1) + go func() { + defer wg.Done() + <-startBoth + cancelTx, err := db.Conn.Pool().Begin(pool) + if err != nil { + cancelErr = err + return + } + defer func() { + // Rollback is a no-op after a successful commit. + _ = cancelTx.Rollback(pool) + }() + if _, err := ProcessCancellationRefundTx(pool, cancelTx, bookingID, 100, 100, start, clock.Now(), "client_cancelled", &userID, false); err != nil { + cancelErr = err + return + } + cancelErr = cancelTx.Commit(pool) + }() + close(startBoth) + wg.Wait() + + if manualErr != nil { + t.Fatalf("manual refund goroutine failed: %v", manualErr) + } + if cancelErr != nil { + t.Fatalf("cancellation goroutine failed: %v", cancelErr) + } + + // Manual outcome: 200 (manual won, cancellation created no row) or 400 + // (cancellation's committed pending row blocked the manual over-refund). + if manualCode != http.StatusOK && manualCode != http.StatusBadRequest { + t.Errorf("expected manual refund 200 (won) or 400 (blocked by cancellation), got %d", manualCode) + } + + // Money-correctness invariant: at most ONE refund row resolves the payment + // and the total never exceeds the £100 payment. + var refundCount int + err = db.Conn.QueryRow(pool, + `SELECT COUNT(*) FROM refunds WHERE payment_id = $1 AND status IN ('completed','pending')`, paymentID).Scan(&refundCount) + if err != nil { + t.Fatalf("failed to count refunds: %v", err) + } + if refundCount > 1 { + t.Errorf("over-refund: %d refund rows for a single £100 payment", refundCount) + } + var refundedPence int64 + err = db.Conn.QueryRow(pool, + `SELECT COALESCE(SUM(ROUND(amount * 100)), 0)::bigint FROM refunds WHERE payment_id = $1 AND status IN ('completed','pending')`, paymentID).Scan(&refundedPence) + if err != nil { + t.Fatalf("failed to sum refunds: %v", err) + } + if refundedPence > 10000 { + t.Errorf("over-refund: %d pence refunded on a 10000-pence payment", refundedPence) + } +} + +// ============================================================================= +// D2 — the sweep retries stale MANUAL pending refunds with their own key +// ============================================================================= + +// TestSweepPendingSquareRefunds_RetriesStaleManualRefund verifies the D2 fix: +// a manual refund left 'pending' by the handler's ambiguous-error path is +// retried by the sweep with the row's OWN stored idempotency key (never the +// -square-agg key), and resolves to completed. +func TestSweepPendingSquareRefunds_RetriesStaleManualRefund(t *testing.T) { + ctx, tx := testutils.SetupTestTx(t) + + userID, err := fixtures.CreateTestUser(tx) + if err != nil { + t.Fatalf("failed to create user: %v", err) + } + serviceID, err := fixtures.CreateTestService(tx) + if err != nil { + t.Fatalf("failed to create service: %v", err) + } + bookingID, err := fixtures.CreateTestBookingAtTime(tx, userID, serviceID, + time.Date(2099, 12, 31, 10, 0, 0, 0, time.UTC)) + if err != nil { + t.Fatalf("failed to create booking: %v", err) + } + + paymentID, err := fixtures.CreateTestPayment(tx, bookingID, 50.00, "online_square", "full", "completed") + if err != nil { + t.Fatalf("failed to create card payment: %v", err) + } + _, err = tx.Exec(ctx, "UPDATE payments SET square_payment_id = 'sqp_manual_retry' WHERE id = $1", paymentID) + if err != nil { + t.Fatalf("failed to set square_payment_id: %v", err) + } + + storedKey := paymentID + "-refund-5000" + var refundID string + err = tx.QueryRow(ctx, ` + INSERT INTO refunds (payment_id, booking_id, amount, status, reason, idempotency_key, origin, created_at) + VALUES ($1, $2, 50, 'pending', 'customer request', $3, 'manual', NOW()) + RETURNING id + `, paymentID, bookingID, storedKey).Scan(&refundID) + if err != nil { + t.Fatalf("failed to insert stale manual pending refund: %v", err) + } + + pgxTx := db.TxFromContext(ctx) + if pgxTx == nil { + t.Fatal("no transaction in context") + } + if err := pgxTx.Commit(ctx); err != nil { + t.Fatalf("failed to commit test tx: %v", err) + } + + origClient := SquareClient + counting := &countingRefundClient{SquareClient: square.NewDevClient()} + SquareClient = counting + defer func() { SquareClient = origClient }() + + freshCtx := context.Background() + // The sweep processes the whole shared test database — clear any pending + // rows left by earlier sequential tests (e.g. the D1 concurrency test when + // the cancellation won) so the call count below is deterministic. + if _, err := db.Conn.Exec(freshCtx, `DELETE FROM refunds WHERE status = 'pending' AND id <> $1`, refundID); err != nil { + t.Fatalf("failed to clean leftover pending refunds: %v", err) + } + if _, err := SweepPendingSquareRefunds(freshCtx); err != nil { + t.Fatalf("SweepPendingSquareRefunds failed: %v", err) + } + + calls := counting.refundCalls() + if len(calls) != 1 { + t.Fatalf("expected exactly 1 Square refund call for the stale manual refund, got %d", len(calls)) + } + if calls[0].IdempotencyKey != storedKey { + t.Errorf("expected the row's OWN stored idempotency key %q, got %q", storedKey, calls[0].IdempotencyKey) + } + if calls[0].Amount != 5000 { + t.Errorf("expected refund of 5000 pence, got %d", calls[0].Amount) + } + if calls[0].PaymentID != "sqp_manual_retry" { + t.Errorf("expected Square payment id sqp_manual_retry, got %q", calls[0].PaymentID) + } + + var status string + var squareRefundID *string + err = db.Conn.QueryRow(freshCtx, `SELECT status, square_refund_id FROM refunds WHERE id = $1`, refundID).Scan(&status, &squareRefundID) + if err != nil { + t.Fatalf("failed to query refund: %v", err) + } + if status != "completed" { + t.Errorf("expected refund status 'completed', got %q", status) + } + if squareRefundID == nil || *squareRefundID == "" { + t.Error("expected square_refund_id to be set") + } +} + +// ============================================================================= +// D3 — the 23h age guard reconciles against Square before marking failed +// ============================================================================= + +// TestProcessPendingSquareRefunds_AgeGuard_ReconcilesCompletedRefund verifies +// the D3 fix: an aged pending refund (>23h) whose money actually moved at +// Square (pre-seeded COMPLETED refund via MockClient) is resolved to completed +// instead of failed — no new Square call is issued, and the square_refund_id +// matches the refund Square already recorded. +func TestProcessPendingSquareRefunds_AgeGuard_ReconcilesCompletedRefund(t *testing.T) { + ctx, tx := testutils.SetupTestTx(t) + + userID, err := fixtures.CreateTestUser(tx) + if err != nil { + t.Fatalf("failed to create user: %v", err) + } + serviceID, err := fixtures.CreateTestService(tx) + if err != nil { + t.Fatalf("failed to create service: %v", err) + } + bookingID, err := fixtures.CreateTestBookingAtTime(tx, userID, serviceID, + time.Date(2099, 12, 31, 10, 0, 0, 0, time.UTC)) + if err != nil { + t.Fatalf("failed to create booking: %v", err) + } + + paymentID, err := fixtures.CreateTestPayment(tx, bookingID, 25.00, "online_square", "deposit", "completed") + if err != nil { + t.Fatalf("failed to create card payment: %v", err) + } + _, err = tx.Exec(ctx, "UPDATE payments SET square_payment_id = 'sqp_age_reconcile' WHERE id = $1", paymentID) + if err != nil { + t.Fatalf("failed to set square_payment_id: %v", err) + } + + var refundID string + err = tx.QueryRow(ctx, ` + INSERT INTO refunds (payment_id, booking_id, amount, status, reason, origin, created_at) + VALUES ($1, $2, 25, 'pending', 'client_cancelled', 'cancellation', NOW() - INTERVAL '25 hours') + RETURNING id + `, paymentID, bookingID).Scan(&refundID) + if err != nil { + t.Fatalf("failed to insert aged pending refund: %v", err) + } + + pgxTx := db.TxFromContext(ctx) + if pgxTx == nil { + t.Fatal("no transaction in context") + } + if err := pgxTx.Commit(ctx); err != nil { + t.Fatalf("failed to commit test tx: %v", err) + } + + origClient := SquareClient + mock := square.NewDevClient().(*square.MockClient) + // Pre-seed the COMPLETED refund Square recorded for this charge — the exact + // amount, same payment. This simulates the money already having moved. + seeded, err := mock.RefundPayment(context.Background(), square.RefundPaymentReq{ + PaymentID: "sqp_age_reconcile", + Amount: 2500, + IdempotencyKey: "seed-age-guard", + Reason: "client_cancelled", + }) + if err != nil { + t.Fatalf("failed to seed completed Square refund: %v", err) + } + counting := &countingRefundClient{SquareClient: mock} + SquareClient = counting + defer func() { SquareClient = origClient }() + + freshCtx := context.Background() + ProcessPendingSquareRefunds(freshCtx, bookingID, "client_cancelled") + + var status string + var squareRefundID *string + err = db.Conn.QueryRow(freshCtx, `SELECT status, square_refund_id FROM refunds WHERE id = $1`, refundID).Scan(&status, &squareRefundID) + if err != nil { + t.Fatalf("failed to query refund: %v", err) + } + if status != "completed" { + t.Errorf("expected aged refund resolved to 'completed' (money moved at Square), got %q", status) + } + if squareRefundID == nil || *squareRefundID != seeded.ID { + t.Errorf("expected square_refund_id %q (the refund Square recorded), got %v", seeded.ID, squareRefundID) + } + + // The reconcile must NOT have re-issued a new Square refund. + if calls := counting.refundCalls(); len(calls) != 0 { + t.Errorf("expected NO new Square refund call during reconcile, got %d", len(calls)) + } +} + +// TestProcessPendingSquareRefunds_AgeGuard_ReconcileError_LeavesPending locks +// the tri-state reconcile error branch: an aged pending refund (>23h) whose +// reconcile against Square FAILS (network error) must stay 'pending' — NOT +// 'failed', which would let the over-refund guard exclude money that may have +// moved at Square. The charge-level refund must NOT be re-issued either. +func TestProcessPendingSquareRefunds_AgeGuard_ReconcileError_LeavesPending(t *testing.T) { + ctx, tx := testutils.SetupTestTx(t) + + userID, err := fixtures.CreateTestUser(tx) + if err != nil { + t.Fatalf("failed to create user: %v", err) + } + serviceID, err := fixtures.CreateTestService(tx) + if err != nil { + t.Fatalf("failed to create service: %v", err) + } + bookingID, err := fixtures.CreateTestBookingAtTime(tx, userID, serviceID, + time.Date(2099, 12, 31, 10, 0, 0, 0, time.UTC)) + if err != nil { + t.Fatalf("failed to create booking: %v", err) + } + + paymentID, err := fixtures.CreateTestPayment(tx, bookingID, 25.00, "online_square", "deposit", "completed") + if err != nil { + t.Fatalf("failed to create card payment: %v", err) + } + _, err = tx.Exec(ctx, "UPDATE payments SET square_payment_id = 'sqp_age_reconcile_error' WHERE id = $1", paymentID) + if err != nil { + t.Fatalf("failed to set square_payment_id: %v", err) + } + + _, err = tx.Exec(ctx, ` + INSERT INTO refunds (payment_id, booking_id, amount, status, reason, origin, created_at) + VALUES ($1, $2, 25, 'pending', 'client_cancelled', 'cancellation', NOW() - INTERVAL '25 hours') + `, paymentID, bookingID) + if err != nil { + t.Fatalf("failed to insert aged pending refund: %v", err) + } + + pgxTx := db.TxFromContext(ctx) + if pgxTx == nil { + t.Fatal("no transaction in context") + } + if err := pgxTx.Commit(ctx); err != nil { + t.Fatalf("failed to commit test tx: %v", err) + } + + origClient := SquareClient + recErr := &reconcileErrorClient{SquareClient: square.NewDevClient()} + counting := &countingRefundClient{SquareClient: recErr} + SquareClient = counting + defer func() { SquareClient = origClient }() + + freshCtx := context.Background() + ProcessPendingSquareRefunds(freshCtx, bookingID, "client_cancelled") + + var status string + var attempts int + err = db.Conn.QueryRow(freshCtx, `SELECT status, refund_attempts FROM refunds WHERE payment_id = $1`, paymentID).Scan(&status, &attempts) + if err != nil { + t.Fatalf("failed to query refund: %v", err) + } + if status != "pending" { + t.Errorf("expected aged refund left 'pending' (reconcile error = unknown state), got %q", status) + } + if attempts != 0 { + t.Errorf("expected refund_attempts untouched (0) when the reconcile fails before any refund call, got %d", attempts) + } + + // The reconcile error must NOT have fired a charge-level Square refund. + if calls := counting.refundCalls(); len(calls) != 0 { + t.Errorf("expected NO Square refund call when the reconcile fails, got %d", len(calls)) + } + + // No admin notification — the row was not marked failed. + var notifCount int + err = db.Conn.QueryRow(freshCtx, + `SELECT COUNT(*) FROM admin_notifications WHERE booking_id = $1 AND reason = 'refund_failed'`, bookingID).Scan(¬ifCount) + if err != nil { + t.Fatalf("failed to query admin_notifications: %v", err) + } + if notifCount != 0 { + t.Errorf("expected NO admin_notification with reason 'refund_failed' (reconcile error leaves rows pending), got %d", notifCount) + } +} + +// TestProcessPendingSquareRefunds_AgeGuard_NoMatch_MarksFailed locks the +// tri-state reconcile nil+nil branch: an aged pending refund (>23h) with NO +// exact COMPLETED refund at Square (genuine no-match — money provably did not +// move) is marked 'failed' and surfaced via admin_notification. +func TestProcessPendingSquareRefunds_AgeGuard_NoMatch_MarksFailed(t *testing.T) { + ctx, tx := testutils.SetupTestTx(t) + + userID, err := fixtures.CreateTestUser(tx) + if err != nil { + t.Fatalf("failed to create user: %v", err) + } + serviceID, err := fixtures.CreateTestService(tx) + if err != nil { + t.Fatalf("failed to create service: %v", err) + } + bookingID, err := fixtures.CreateTestBookingAtTime(tx, userID, serviceID, + time.Date(2099, 12, 31, 10, 0, 0, 0, time.UTC)) + if err != nil { + t.Fatalf("failed to create booking: %v", err) + } + + paymentID, err := fixtures.CreateTestPayment(tx, bookingID, 25.00, "online_square", "deposit", "completed") + if err != nil { + t.Fatalf("failed to create card payment: %v", err) + } + _, err = tx.Exec(ctx, "UPDATE payments SET square_payment_id = 'sqp_age_no_match' WHERE id = $1", paymentID) + if err != nil { + t.Fatalf("failed to set square_payment_id: %v", err) + } + + _, err = tx.Exec(ctx, ` + INSERT INTO refunds (payment_id, booking_id, amount, status, reason, origin, created_at) + VALUES ($1, $2, 25, 'pending', 'client_cancelled', 'cancellation', NOW() - INTERVAL '25 hours') + `, paymentID, bookingID) + if err != nil { + t.Fatalf("failed to insert aged pending refund: %v", err) + } + + pgxTx := db.TxFromContext(ctx) + if pgxTx == nil { + t.Fatal("no transaction in context") + } + if err := pgxTx.Commit(ctx); err != nil { + t.Fatalf("failed to commit test tx: %v", err) + } + + origClient := SquareClient + // The MockClient records no refunds for this charge → reconcile returns a + // genuine no-match (nil, nil). + mock := square.NewDevClient().(*square.MockClient) + counting := &countingRefundClient{SquareClient: mock} + SquareClient = counting + defer func() { SquareClient = origClient }() + + freshCtx := context.Background() + ProcessPendingSquareRefunds(freshCtx, bookingID, "client_cancelled") + + var status string + err = db.Conn.QueryRow(freshCtx, `SELECT status FROM refunds WHERE payment_id = $1`, paymentID).Scan(&status) + if err != nil { + t.Fatalf("failed to query refund: %v", err) + } + if status != "failed" { + t.Errorf("expected aged refund marked 'failed' (Square shows no COMPLETED refund), got %q", status) + } + + // The reconcile showed no match — the charge-level refund must NOT fire. + if calls := counting.refundCalls(); len(calls) != 0 { + t.Errorf("expected NO Square refund call on a genuine no-match reconcile, got %d", len(calls)) + } + + // The terminal failure must surface an admin_notifications row. + var notifCount int + err = db.Conn.QueryRow(freshCtx, + `SELECT COUNT(*) FROM admin_notifications WHERE booking_id = $1 AND reason = 'refund_failed'`, bookingID).Scan(¬ifCount) + if err != nil { + t.Fatalf("failed to query admin_notifications: %v", err) + } + if notifCount < 1 { + t.Errorf("expected at least 1 admin_notification with reason 'refund_failed', got %d", notifCount) + } +} diff --git a/backend/handlers/payments/service.go b/backend/handlers/payments/service.go index 9cded59..ec16139 100644 --- a/backend/handlers/payments/service.go +++ b/backend/handlers/payments/service.go @@ -65,6 +65,8 @@ type RefundRecord struct { SquareRefundID *string Status string Reason string + Origin string // 'manual' (admin handler) or 'cancellation' (cancellation loop) + IdempotencyKey *string CreatedBy *string CreatedAt time.Time } @@ -154,8 +156,8 @@ func (s *PaymentService) CreateRefundRecord(ctx context.Context, record RefundRe var id string err := db.Conn.QueryRow(ctx, ` INSERT INTO refunds ( - payment_id, booking_id, amount, square_refund_id, status, reason, created_by, created_at - ) VALUES ($1, $2, $3, $4, $5, $6, $7, $8) + payment_id, booking_id, amount, square_refund_id, status, reason, idempotency_key, created_by, created_at, origin + ) VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10) RETURNING id `, record.PaymentID, @@ -164,8 +166,10 @@ func (s *PaymentService) CreateRefundRecord(ctx context.Context, record RefundRe record.SquareRefundID, record.Status, record.Reason, + record.IdempotencyKey, record.CreatedBy, record.CreatedAt, + record.Origin, ).Scan(&id) if err != nil { @@ -239,7 +243,7 @@ func (s *PaymentService) GetBookingPaymentSummary(ctx context.Context, bookingID summary.TotalNetAmount = totalNetAmount refundRows, err := db.Conn.Query(ctx, ` - SELECT id, payment_id, booking_id, amount, square_refund_id, status, reason, created_by, created_at + SELECT id, payment_id, booking_id, amount, square_refund_id, status, reason, idempotency_key, created_by, created_at, origin FROM refunds WHERE booking_id = $1 AND status = 'completed' ORDER BY created_at ASC @@ -255,7 +259,7 @@ func (s *PaymentService) GetBookingPaymentSummary(ctx context.Context, bookingID var r RefundRecord err := refundRows.Scan( &r.ID, &r.PaymentID, &r.BookingID, &r.Amount, &r.SquareRefundID, - &r.Status, &r.Reason, &r.CreatedBy, &r.CreatedAt, + &r.Status, &r.Reason, &r.IdempotencyKey, &r.CreatedBy, &r.CreatedAt, &r.Origin, ) if err != nil { return nil, err @@ -348,11 +352,17 @@ func (s *PaymentService) GetPaymentByID(ctx context.Context, paymentID string) ( return &p, nil } +// GetAlreadyRefundedAmount returns the total refunded amount (in pence) for a +// payment, counting both 'completed' and 'pending' refunds. Pending refunds are +// counted because a Square call may already be in flight for them — excluding +// them would let a concurrent refund over-refund the payment. 'failed' refunds +// are excluded: they were definitively rejected by Square and must not block +// future refund attempts. func (s *PaymentService) GetAlreadyRefundedAmount(ctx context.Context, paymentID string) (int64, error) { var amount float64 err := db.Conn.QueryRow(ctx, ` SELECT COALESCE(SUM(amount), 0) FROM refunds - WHERE payment_id = $1 AND status = 'completed' + WHERE payment_id = $1 AND status IN ('completed', 'pending') `, paymentID).Scan(&amount) if err != nil { @@ -391,18 +401,27 @@ type BookingPaymentInfo struct { } // GetBookingPaymentInfo fetches the booking start time, total service amount, and -// total completed payments for a booking. +// total net paid (completed payments minus completed/pending refunds) for a +// booking. Refunds are subtracted so cancellation refunds never double-refund +// money that has already been returned (e.g. via a manual admin refund). func (s *PaymentService) GetBookingPaymentInfo(ctx context.Context, bookingID string) (*BookingPaymentInfo, error) { var info BookingPaymentInfo err := db.Conn.QueryRow(ctx, ` SELECT b.start_time, b.status, COALESCE(b.total_amount, 0), - COALESCE(pt.total_paid, 0) + COALESCE(pt.total_paid, 0) - COALESCE(rr.total_refunded, 0) FROM bookings b LEFT JOIN ( SELECT booking_id, SUM(amount) AS total_paid FROM payments WHERE booking_id = $1 AND status = 'completed' AND payment_method NOT IN ('discount', 'on_the_house') GROUP BY booking_id ) pt ON b.id = pt.booking_id + LEFT JOIN ( + SELECT p.booking_id, SUM(r.amount) AS total_refunded + FROM refunds r + JOIN payments p ON r.payment_id = p.id + WHERE p.booking_id = $1 AND r.status IN ('completed', 'pending') + GROUP BY p.booking_id + ) rr ON b.id = rr.booking_id WHERE b.id = $1 `, bookingID).Scan(&info.StartTime, &info.Status, &info.TotalAmount, &info.TotalPaid) if err != nil { diff --git a/backend/handlers/payments/till.go b/backend/handlers/payments/till.go index 674e99a..05b9e84 100644 --- a/backend/handlers/payments/till.go +++ b/backend/handlers/payments/till.go @@ -100,6 +100,14 @@ func CreateTillSale(w http.ResponseWriter, r *http.Request) { http.Error(w, "gift_card_id is required for topup", http.StatusBadRequest) return } + if req.Action == "topup" && req.RedeemToUserID != nil && *req.RedeemToUserID != "" { + // Redeem is create-only. Topping up an unredeemed card (which can still + // carry residual balance) and then redeeming would zero amount_remaining + // while crediting the user only the top-up amount — destroying money. + // Fail loudly rather than silently ignoring the redeem. + http.Error(w, "Cannot redeem a top-up to a user account", http.StatusBadRequest) + return + } // Idempotency check: if key provided, return existing sale if found // Idempotency handling. A 'completed' sale is a dedup (return it). A @@ -183,6 +191,40 @@ func CreateTillSale(w http.ResponseWriter, r *http.Request) { http.Error(w, "internal server error", http.StatusInternalServerError) return } + + // If the gift card should be immediately redeemed to a user's account balance + // (e.g. admin selected "add to account" rather than "generate gift code"). + // Runs only on the FIRST attempt of a create — a pending retry reuses the + // already-funded gift card and must never re-run this, or the user's balance + // would be credited a second time (money loss to the business). + if req.RedeemToUserID != nil && *req.RedeemToUserID != "" { + _, err = tx.Exec(ctx, ` + UPDATE gift_cards + SET amount_remaining = 0, + redeemed_at = NOW(), + redeemed_by = $1, + last_used_at = NOW() + WHERE id = $2 + `, *req.RedeemToUserID, giftCardID) + if err != nil { + log.Printf("Failed to redeem gift card to user account: %v", err) + http.Error(w, "internal server error", http.StatusInternalServerError) + return + } + + _, err = tx.Exec(ctx, ` + INSERT INTO user_giftcard_balances (user_id, balance, updated_at) + VALUES ($1, $2, NOW()) + ON CONFLICT (user_id) DO UPDATE SET + balance = user_giftcard_balances.balance + EXCLUDED.balance, + updated_at = NOW() + `, *req.RedeemToUserID, req.Amount) + if err != nil { + log.Printf("Failed to update user gift card balance: %v", err) + http.Error(w, "internal server error", http.StatusInternalServerError) + return + } + } } else { cardID := validators.NormalizeGiftCardCode(*req.GiftCardID) var redeemedBy sql.NullString @@ -244,37 +286,6 @@ func CreateTillSale(w http.ResponseWriter, r *http.Request) { } } - // If the gift card should be immediately redeemed to a user's account balance - // (e.g. admin selected "add to account" rather than "generate gift code") - if req.RedeemToUserID != nil && *req.RedeemToUserID != "" { - _, err = tx.Exec(ctx, ` - UPDATE gift_cards - SET amount_remaining = 0, - redeemed_at = NOW(), - redeemed_by = $1, - last_used_at = NOW() - WHERE id = $2 - `, *req.RedeemToUserID, giftCardID) - if err != nil { - log.Printf("Failed to redeem gift card to user account: %v", err) - http.Error(w, "internal server error", http.StatusInternalServerError) - return - } - - _, err = tx.Exec(ctx, ` - INSERT INTO user_giftcard_balances (user_id, balance, updated_at) - VALUES ($1, $2, NOW()) - ON CONFLICT (user_id) DO UPDATE SET - balance = user_giftcard_balances.balance + EXCLUDED.balance, - updated_at = NOW() - `, *req.RedeemToUserID, req.Amount) - if err != nil { - log.Printf("Failed to update user gift card balance: %v", err) - http.Error(w, "internal server error", http.StatusInternalServerError) - return - } - } - penceAmount := int64(math.Round(req.Amount * 100)) var squarePaymentID *string @@ -288,6 +299,21 @@ func CreateTillSale(w http.ResponseWriter, r *http.Request) { var needsSquarePayment bool var savedCardSqCardID string + // Pending-retry for card_machine: the original Square checkout may still be + // live at the terminal. If the pending till_sales row already recorded a + // square_checkout_id, reuse it instead of creating a second checkout — a + // fresh checkout would orphan the original, which can still complete and + // become an untracked charge. + var existingPendingCheckoutID string + if existingPendingID != "" { + err = tx.QueryRow(ctx, `SELECT COALESCE(square_checkout_id, '') FROM till_sales WHERE id = $1`, existingPendingID).Scan(&existingPendingCheckoutID) + if err != nil { + log.Printf("Failed to query existing pending sale checkout: %v", err) + http.Error(w, "internal server error", http.StatusInternalServerError) + return + } + } + switch req.PaymentMethod { case "cash": saleStatus = "completed" @@ -333,23 +359,32 @@ func CreateTillSale(w http.ResponseWriter, r *http.Request) { req.IdempotencyKey = uniqueTillKey() } - checkoutReq := square.CreateCheckoutReq{ - Amount: penceAmount, - Currency: "GBP", - IdempotencyKey: req.IdempotencyKey, - ReferenceID: giftCardID, - TipEnabled: false, - } + if existingPendingCheckoutID != "" { + // Pending retry — reuse the checkout already created for this sale + // instead of creating a second one. The original checkout may still + // be live at the terminal; a fresh checkout would orphan it into an + // untracked charge. + squareCheckoutID = &existingPendingCheckoutID + saleStatus = "pending" + } else { + checkoutReq := square.CreateCheckoutReq{ + Amount: penceAmount, + Currency: "GBP", + IdempotencyKey: req.IdempotencyKey, + ReferenceID: giftCardID, + TipEnabled: false, + } - checkout, err := SquareClient.CreateCheckout(ctx, checkoutReq) - if err != nil { - log.Printf("Failed to create Square checkout: %v", err) - http.Error(w, "Failed to create card machine payment", http.StatusInternalServerError) - return - } + checkout, err := SquareClient.CreateCheckout(ctx, checkoutReq) + if err != nil { + log.Printf("Failed to create Square checkout: %v", err) + http.Error(w, "Failed to create card machine payment", http.StatusInternalServerError) + return + } - squareCheckoutID = &checkout.ID - saleStatus = "pending" + squareCheckoutID = &checkout.ID + saleStatus = "pending" + } case "online_square": dbPaymentMethod = "online_square" if req.IdempotencyKey == "" { diff --git a/backend/handlers/payments/till_test.go b/backend/handlers/payments/till_test.go index 232209d..c05e38e 100644 --- a/backend/handlers/payments/till_test.go +++ b/backend/handlers/payments/till_test.go @@ -1038,3 +1038,297 @@ func TestCreateTillSale_TwoIdenticalCreateSales_BothSucceed(t *testing.T) { require.Equal(t, http.StatusCreated, w.Code, "sale %d: expected 201, got %d. body: %s", i+1, w.Code, w.Body.String()) } } + +// TestCreateTillSale_PendingRetry_RedeemToUser_BalanceCreditedOnce verifies that +// a same-key retry of a pending create-with-redeem sale does NOT re-credit the +// user's balance. The redeem (zeroing the gift card + crediting +// user_giftcard_balances) must only run on the FIRST attempt of a create; a +// pending retry reuses the already-redeemed gift card and would otherwise credit +// the user a second time (money loss to the business). +func TestCreateTillSale_PendingRetry_RedeemToUser_BalanceCreditedOnce(t *testing.T) { + t.Parallel() + ctx, tx := testutils.SetupTestTx(t) + + adminID, err := fixtures.CreateTestAdminUser(tx) + if err != nil { + t.Fatalf("failed to create admin user: %v", err) + } + userID, err := fixtures.CreateTestUser(tx) + if err != nil { + t.Fatalf("failed to create user: %v", err) + } + adminToken := jwt.GenerateTestToken(adminID, "admin") + + cardID, err := fixtures.CreateTestPaymentMethod(tx, userID, "sq_test_card_id", "VISA", "1234") + if err != nil { + t.Fatalf("failed to create saved card: %v", err) + } + + // Seed a PENDING till_sale (prior attempt where Square failed after the DB + // transaction committed) whose gift card was already redeemed and the user + // already credited the first-attempt amount (£50). + key := "till-pending-retry-redeem-key" + var giftCardID string + err = tx.QueryRow(ctx, ` + INSERT INTO gift_cards (total_funds_added, amount_remaining, created_by, redeemed_by, redeemed_at, is_inventory, voucher_type_at_purchase) + VALUES (50.00, 0.00, $1, $2, NOW(), FALSE, 'SPV') + RETURNING id + `, adminID, userID).Scan(&giftCardID) + if err != nil { + t.Fatalf("failed to seed redeemed gift card: %v", err) + } + _, err = tx.Exec(ctx, ` + INSERT INTO user_giftcard_balances (user_id, balance, updated_at) + VALUES ($1, 50.00, NOW()) + `, userID) + if err != nil { + t.Fatalf("failed to seed user gift card balance: %v", err) + } + _, err = tx.Exec(ctx, ` + INSERT INTO till_sales (item_type, item_id, description, quantity, unit_price, total_amount, + payment_method, status, user_id, user_saved_card_id, idempotency_key, created_by, created_at, updated_at) + VALUES ('gift_card', $1, 'Gift Card create', 1, 50.00, 50.00, 'online_square', 'pending', + $2, $3, $4, $5, NOW(), NOW()) + `, giftCardID, userID, cardID, key, adminID) + if err != nil { + t.Fatalf("failed to seed pending till sale: %v", err) + } + + // Retry with the same key, requesting the redeem again. + redeemUserID := userID + reqBody := TillSaleRequest{ + ItemType: "gift_card", + Action: "create", + Amount: 50.00, + PaymentMethod: "saved_card", + UserSavedCardID: &cardID, + UserID: &userID, + IdempotencyKey: key, + RedeemToUserID: &redeemUserID, + } + + bodyBytes, _ := json.Marshal(reqBody) + req := httptest.NewRequest("POST", "/api/admin/till/sale", bytes.NewReader(bodyBytes)) + req.Header.Set("Authorization", "Bearer "+adminToken) + req.Header.Set("Content-Type", "application/json") + req = req.WithContext(db.ContextWithTx(req.Context(), tx.(pgx.Tx))) + + w := httptest.NewRecorder() + r := chi.NewRouter() + r.Use(mw.RequireAuth) + r.Post("/api/admin/till/sale", CreateTillSale) + r.ServeHTTP(w, req) + + if w.Code != http.StatusOK && w.Code != http.StatusCreated { + t.Fatalf("expected 200/201, got %d. body: %s", w.Code, w.Body.String()) + } + + // The sale must now be 'completed' (Square re-attempted and succeeded). + var saleStatus string + err = tx.QueryRow(ctx, `SELECT status FROM till_sales WHERE idempotency_key = $1`, key).Scan(&saleStatus) + if err != nil { + t.Fatalf("failed to query till sale: %v", err) + } + if saleStatus != "completed" { + t.Errorf("expected sale status 'completed' after retry, got %s", saleStatus) + } + + // The user balance must still be £50 — credited EXACTLY ONCE, not £100. + var balance float64 + err = tx.QueryRow(ctx, `SELECT balance FROM user_giftcard_balances WHERE user_id = $1`, userID).Scan(&balance) + if err != nil { + t.Fatalf("failed to query user gift card balance: %v", err) + } + if balance != 50.00 { + t.Errorf("expected balance 50.00 (credited once), got %.2f", balance) + } + + // The gift card must remain fully redeemed (amount_remaining still 0). + var amountRemaining float64 + err = tx.QueryRow(ctx, `SELECT amount_remaining FROM gift_cards WHERE id = $1`, giftCardID).Scan(&amountRemaining) + if err != nil { + t.Fatalf("failed to query gift card: %v", err) + } + if amountRemaining != 0.00 { + t.Errorf("expected amount_remaining 0.00, got %.2f", amountRemaining) + } +} + +// TestCreateTillSale_TopupWithRedeem_Rejected verifies that a top-up request +// carrying a redeem_to_user_id is rejected outright. Allowing redeem on a first +// attempt topup destroys money: the topup branch accepts unredeemed cards with +// residual balance, and topping up £20 onto a card with £40 residual then +// redeeming would zero amount_remaining (£40 lost) while crediting the user only +// the top-up amount. +func TestCreateTillSale_TopupWithRedeem_Rejected(t *testing.T) { + t.Parallel() + ctx, tx := testutils.SetupTestTx(t) + + adminID, err := fixtures.CreateTestAdminUser(tx) + if err != nil { + t.Fatalf("failed to create admin user: %v", err) + } + redeemerID, err := fixtures.CreateTestUser(tx) + if err != nil { + t.Fatalf("failed to create redeemer user: %v", err) + } + adminToken := jwt.GenerateTestToken(adminID, "admin") + + // An unredeemed card carrying residual balance — the dangerous topup+redeem case. + var cardID string + err = tx.QueryRow(ctx, ` + INSERT INTO gift_cards (total_funds_added, amount_remaining, created_by, is_inventory, voucher_type_at_purchase) + VALUES (40.00, 40.00, $1, FALSE, 'SPV') + RETURNING id + `, adminID).Scan(&cardID) + if err != nil { + t.Fatalf("failed to insert gift card: %v", err) + } + + reqBody := TillSaleRequest{ + ItemType: "gift_card", + Action: "topup", + Amount: 20.00, + PaymentMethod: "on_the_house", + GiftCardID: &cardID, + RedeemToUserID: &redeemerID, + } + + bodyBytes, _ := json.Marshal(reqBody) + req := httptest.NewRequest("POST", "/api/admin/till/sale", bytes.NewReader(bodyBytes)) + req.Header.Set("Authorization", "Bearer "+adminToken) + req.Header.Set("Content-Type", "application/json") + req = req.WithContext(db.ContextWithTx(req.Context(), tx.(pgx.Tx))) + + w := httptest.NewRecorder() + r := chi.NewRouter() + r.Use(mw.RequireAuth) + r.Post("/api/admin/till/sale", CreateTillSale) + r.ServeHTTP(w, req) + + if w.Code != http.StatusBadRequest { + t.Fatalf("expected 400, got %d. body: %s", w.Code, w.Body.String()) + } + + // No rows created: no till_sale, card untouched, no user balance credited. + var saleCount int + err = tx.QueryRow(ctx, `SELECT COUNT(*) FROM till_sales`).Scan(&saleCount) + if err != nil { + t.Fatalf("failed to query till_sales: %v", err) + } + if saleCount != 0 { + t.Errorf("expected 0 till_sales, got %d", saleCount) + } + + var totalFunds, amountRemaining float64 + err = tx.QueryRow(ctx, `SELECT total_funds_added, amount_remaining FROM gift_cards WHERE id = $1`, cardID).Scan(&totalFunds, &amountRemaining) + if err != nil { + t.Fatalf("failed to query gift card: %v", err) + } + if totalFunds != 40.00 { + t.Errorf("expected total_funds_added 40.00, got %.2f", totalFunds) + } + if amountRemaining != 40.00 { + t.Errorf("expected amount_remaining 40.00, got %.2f", amountRemaining) + } + + var balanceCount int + err = tx.QueryRow(ctx, `SELECT COUNT(*) FROM user_giftcard_balances WHERE user_id = $1`, redeemerID).Scan(&balanceCount) + if err != nil { + t.Fatalf("failed to query user_giftcard_balances: %v", err) + } + if balanceCount != 0 { + t.Errorf("expected no user gift card balance row, got %d", balanceCount) + } +} + +// TestCreateTillSale_PendingRetry_CardMachine_ReusesCheckout verifies that a +// same-key retry of a pending card_machine sale reuses the checkout already +// stored on the till_sales row instead of calling Square CreateCheckout a second +// time. The original checkout may still be live at the terminal; a fresh +// checkout would orphan it into an untracked charge. +func TestCreateTillSale_PendingRetry_CardMachine_ReusesCheckout(t *testing.T) { + t.Parallel() + ctx, tx := testutils.SetupTestTx(t) + + adminID, err := fixtures.CreateTestAdminUser(tx) + if err != nil { + t.Fatalf("failed to create admin user: %v", err) + } + adminToken := jwt.GenerateTestToken(adminID, "admin") + + // Seed a PENDING card_machine till_sale that already has a Square checkout. + key := "till-card-machine-reuse-key" + storedCheckoutID := "chk_pending_retry_stored" + var giftCardID string + err = tx.QueryRow(ctx, ` + INSERT INTO gift_cards (total_funds_added, amount_remaining, created_by, is_inventory, voucher_type_at_purchase) + VALUES (50.00, 50.00, $1, FALSE, 'SPV') + RETURNING id + `, adminID).Scan(&giftCardID) + if err != nil { + t.Fatalf("failed to create gift card: %v", err) + } + _, err = tx.Exec(ctx, ` + INSERT INTO till_sales (item_type, item_id, description, quantity, unit_price, total_amount, + payment_method, status, square_checkout_id, idempotency_key, created_by, created_at, updated_at) + VALUES ('gift_card', $1, 'Gift Card create', 1, 50.00, 50.00, 'in_person_card', 'pending', + $2, $3, $4, NOW(), NOW()) + `, giftCardID, storedCheckoutID, key, adminID) + if err != nil { + t.Fatalf("failed to seed pending card_machine till sale: %v", err) + } + + reqBody := TillSaleRequest{ + ItemType: "gift_card", + Action: "create", + Amount: 50.00, + PaymentMethod: "card_machine", + IdempotencyKey: key, + } + + bodyBytes, _ := json.Marshal(reqBody) + req := httptest.NewRequest("POST", "/api/admin/till/sale", bytes.NewReader(bodyBytes)) + req.Header.Set("Authorization", "Bearer "+adminToken) + req.Header.Set("Content-Type", "application/json") + req = req.WithContext(db.ContextWithTx(req.Context(), tx.(pgx.Tx))) + + w := httptest.NewRecorder() + r := chi.NewRouter() + r.Use(mw.RequireAuth) + r.Post("/api/admin/till/sale", CreateTillSale) + r.ServeHTTP(w, req) + + if w.Code != http.StatusCreated { + t.Fatalf("expected 201, got %d. body: %s", w.Code, w.Body.String()) + } + + var resp TillSaleResponse + if err := json.NewDecoder(w.Body).Decode(&resp); err != nil { + t.Fatalf("failed to decode response: %v", err) + } + + // The response must reference the STORED checkout, not a freshly created one. + // CreateCheckout always generates a new chk_mock_* id, so any second call + // would surface a different id here. + if resp.CheckoutID == nil || *resp.CheckoutID != storedCheckoutID { + t.Errorf("expected checkout_id to be the stored %q, got %v", storedCheckoutID, resp.CheckoutID) + } + if resp.Status != "pending" { + t.Errorf("expected status 'pending', got %s", resp.Status) + } + + // The till_sales row must still reference the same checkout id, unchanged. + var rowCheckoutID string + var saleCount int + err = tx.QueryRow(ctx, `SELECT COUNT(*), COALESCE(MAX(square_checkout_id), '') FROM till_sales WHERE idempotency_key = $1`, key).Scan(&saleCount, &rowCheckoutID) + if err != nil { + t.Fatalf("failed to query till sale: %v", err) + } + if saleCount != 1 { + t.Errorf("expected 1 till sale (reuse, not duplicate), got %d", saleCount) + } + if rowCheckoutID != storedCheckoutID { + t.Errorf("expected till_sales.square_checkout_id to remain %q, got %q", storedCheckoutID, rowCheckoutID) + } +} diff --git a/backend/internal/jobs/cleanup.go b/backend/internal/jobs/cleanup.go index ca706bc..b8115ea 100644 --- a/backend/internal/jobs/cleanup.go +++ b/backend/internal/jobs/cleanup.go @@ -5,6 +5,7 @@ import ( "crussell/auth" authHandlers "crussell/handlers/auth" + "crussell/handlers/payments" "crussell/handlers/scheduling" "crussell/handlers/user" "crussell/mw" @@ -47,6 +48,14 @@ func RegisterAll(s *Scheduler) { Handler: user.CleanupGDPRExportCache, }) + s.Register(Job{ + Name: "sweep-pending-square-refunds", + Schedule: "*/5 * * * *", + Timeout: 60 * time.Second, + Concurrency: 1, + Handler: payments.SweepPendingSquareRefunds, + }) + // === MID FREQUENCY — every minute (progressive rate limiter was on 30s) === s.Register(Job{ diff --git a/backend/internal/jobs/scheduler_test.go b/backend/internal/jobs/scheduler_test.go index 7eabaea..41ed830 100644 --- a/backend/internal/jobs/scheduler_test.go +++ b/backend/internal/jobs/scheduler_test.go @@ -413,8 +413,8 @@ func TestRegisterAll_RegistersExpectedJobs(t *testing.T) { s := New() RegisterAll(s) - if got := len(s.registry); got != 20 { - t.Fatalf("RegisterAll() registered %d jobs, want 20", got) + if got := len(s.registry); got != 21 { + t.Fatalf("RegisterAll() registered %d jobs, want 21", got) } registered := make(map[string]Job, len(s.registry)) @@ -442,7 +442,7 @@ func TestRegisterAll_RegistersExpectedJobs(t *testing.T) { // TestRegisterAll_ValidSchedules verifies all cron expressions in registered jobs // parse without panic. RegisterAll internally calls Register, which parses every -// schedule; if this test completes without panic, all 19 schedules are valid. +// schedule; if this test completes without panic, all 21 schedules are valid. func TestRegisterAll_ValidSchedules(t *testing.T) { t.Parallel() @@ -472,6 +472,7 @@ func expectedJobNames() map[string]bool { "cleanup-expired-deposits": true, "cleanup-rate-limiters": true, "cleanup-gdpr-export-cache": true, + "sweep-pending-square-refunds": true, "cleanup-progressive-rate-limiter": true, "cleanup-expired-loyalty-redemptions": true, "cleanup-old-idempotency-keys": true, @@ -508,8 +509,8 @@ func TestRegisterAll_NoDuplicateCronExpressions(t *testing.T) { // Known-intentional groupings: jobs at the same frequency that touch // disjoint tables (no contention risk). knownGroupings := map[int]bool{ - 4: true, // */5 * * * * — 4 cleanup jobs, different domains - 5: true, // 0 * * * * — 5 hourly cleanup jobs, different tables + 5: true, // */5 * * * * — 5 cleanup jobs (incl. sweep-pending-square-refunds), different domains + // 0 * * * * — 5 hourly cleanup jobs, different tables 2: true, // 0 2 * * * — 2 daily cleanup jobs, different tables } diff --git a/backend/internal/square/square.go b/backend/internal/square/square.go index 95db980..828fca5 100644 --- a/backend/internal/square/square.go +++ b/backend/internal/square/square.go @@ -2,7 +2,10 @@ package square -import "context" +import ( + "context" + "time" +) var Client SquareClient @@ -43,3 +46,7 @@ func (p *ProdClient) GetCardsOnFile(ctx context.Context, userID string) ([]CardO func (p *ProdClient) DeleteCardOnFile(ctx context.Context, cardID string) error { return deleteCardOnFileHTTP(ctx, cardID) } + +func (p *ProdClient) ListPaymentRefunds(ctx context.Context, paymentID string, beginTime time.Time) ([]RefundResult, error) { + return listRefundsHTTP(ctx, paymentID, beginTime) +} diff --git a/backend/internal/square/square_dev.go b/backend/internal/square/square_dev.go index 8f78688..4455e1e 100644 --- a/backend/internal/square/square_dev.go +++ b/backend/internal/square/square_dev.go @@ -30,9 +30,14 @@ type MockClient struct { payments map[string]*PaymentResult paymentByKey map[string]*PaymentResult refunds map[string]*RefundResult + refundByKey map[string]*RefundResult completed map[string]*PaymentResult HoldCheckouts bool ShouldFail bool // if true, CreatePayment/RefundPayment return errors for testing error paths + // FailRefundCode simulates a specific Square refund rejection code. Empty + // = normal success; when set (e.g. "PAYMENT_ALREADY_REFUNDED"), + // RefundPayment returns the sentinel-wrapped error for that code. + FailRefundCode string } type devProdClient struct{} @@ -58,6 +63,9 @@ func (d *devProdClient) GetCardsOnFile(ctx context.Context, userID string) ([]Ca func (d *devProdClient) DeleteCardOnFile(ctx context.Context, cardID string) error { return deleteCardOnFileHTTP(ctx, cardID) } +func (d *devProdClient) ListPaymentRefunds(ctx context.Context, paymentID string, beginTime time.Time) ([]RefundResult, error) { + return listRefundsHTTP(ctx, paymentID, beginTime) +} func NewClient() SquareClient { return NewDevClient() @@ -76,6 +84,7 @@ func NewDevClient() SquareClient { payments: make(map[string]*PaymentResult), paymentByKey: make(map[string]*PaymentResult), refunds: make(map[string]*RefundResult), + refundByKey: make(map[string]*RefundResult), completed: make(map[string]*PaymentResult), } } @@ -287,7 +296,15 @@ func (m *MockClient) GetCheckout(ctx context.Context, checkoutID string) (*Payme func (m *MockClient) RefundPayment(ctx context.Context, req RefundPaymentReq) (*RefundResult, error) { if m.ShouldFail { - return nil, fmt.Errorf("mock: refund declined (simulated failure)") + return nil, fmt.Errorf("%w: refund declined (simulated failure)", ErrRefundDeclined) + } + if m.FailRefundCode != "" { + switch m.FailRefundCode { + case "PAYMENT_ALREADY_REFUNDED": + return nil, fmt.Errorf("%w: payment already fully refunded (simulated)", ErrRefundAlreadyProcessed) + default: + return nil, fmt.Errorf("%w: %s (simulated failure)", ErrRefundDeclined, m.FailRefundCode) + } } log.Printf("[SQUARE-MOCK] RefundPayment: payment=%s, amount=%d", req.PaymentID, req.Amount) mockSleep(1 * time.Second) @@ -295,6 +312,17 @@ func (m *MockClient) RefundPayment(ctx context.Context, req RefundPaymentReq) (* m.mu.Lock() defer m.mu.Unlock() + // Real Square dedups on idempotency key: a retry with the same key returns + // the original refund rather than issuing a second refund. The mock mirrors + // this so dev/testing behaves like production (and the pending-refund + // resume path can rely on it). + if req.IdempotencyKey != "" { + if existing, ok := m.refundByKey[req.IdempotencyKey]; ok { + log.Printf("[SQUARE-MOCK] RefundPayment dedup hit: key=%s → id=%s", req.IdempotencyKey, existing.ID) + return existing, nil + } + } + now := clock.Now().UTC() refundID := fmt.Sprintf("ref_mock_%d", now.UnixNano()) @@ -326,6 +354,9 @@ func (m *MockClient) RefundPayment(ctx context.Context, req RefundPaymentReq) (* CreatedAt: now.Format(time.RFC3339), } m.refunds[refundID] = result + if req.IdempotencyKey != "" { + m.refundByKey[req.IdempotencyKey] = result + } log.Printf("[SQUARE-MOCK] Refund completed: id=%s, payment=%s, amount=%d", refundID, req.PaymentID, amount) return result, nil } @@ -404,6 +435,26 @@ func (m *MockClient) DeleteCardOnFile(ctx context.Context, cardID string) error return fmt.Errorf("card not found: %s", cardID) } +func (m *MockClient) ListPaymentRefunds(ctx context.Context, paymentID string, beginTime time.Time) ([]RefundResult, error) { + log.Printf("[SQUARE-MOCK] ListPaymentRefunds: payment=%s, begin=%s", paymentID, beginTime.UTC().Format(time.RFC3339)) + + m.mu.Lock() + defer m.mu.Unlock() + + out := []RefundResult{} + for _, r := range m.refunds { + if r.PaymentID != paymentID { + continue + } + createdAt, err := time.Parse(time.RFC3339, r.CreatedAt) + if err == nil && createdAt.Before(beginTime) { + continue + } + out = append(out, *r) + } + return out, nil +} + // isTokenLike returns true for Square source_id tokens: cnon:xxx nonces and // ccof:xxx card IDs. Raw PANs (all digits) are NOT token-like and are rejected. func isTokenLike(s string) bool { diff --git a/backend/internal/square/square_dev_test.go b/backend/internal/square/square_dev_test.go index 0671cc2..cdae39e 100644 --- a/backend/internal/square/square_dev_test.go +++ b/backend/internal/square/square_dev_test.go @@ -4,6 +4,7 @@ package square import ( "context" + "errors" "fmt" "sync" "testing" @@ -299,6 +300,54 @@ func TestRefundPayment_ShouldFail(t *testing.T) { assert.Nil(t, result) } +func TestDevClient_RefundPayment_PaymentAlreadyRefunded(t *testing.T) { + // PAYMENT_ALREADY_REFUNDED means the money already moved at Square, so the + // mock must return ErrRefundAlreadyProcessed (never ErrRefundDeclined) and + // must not store a refund — the caller resolves the record to 'completed'. + client := NewDevClient().(*MockClient) + client.FailRefundCode = "PAYMENT_ALREADY_REFUNDED" + + ctx := context.Background() + req := RefundPaymentReq{ + PaymentID: "pay_mock_already_refunded", + Amount: 5000, + IdempotencyKey: "refund-key-already", + Reason: "already refunded", + } + + result, err := client.RefundPayment(ctx, req) + require.Error(t, err) + assert.Nil(t, result) + assert.True(t, errors.Is(err, ErrRefundAlreadyProcessed), "expected ErrRefundAlreadyProcessed, got: %v", err) + assert.False(t, errors.Is(err, ErrRefundDeclined), "already-processed refund must not be classified as declined: %v", err) + + client.mu.RLock() + defer client.mu.RUnlock() + assert.Len(t, client.refunds, 0, "no refund must be stored when the payment is already refunded") + assert.Len(t, client.refundByKey, 0, "no refund-by-key entry must be stored when the payment is already refunded") +} + +func TestDevClient_RefundPayment_FailRefundCode_OtherCode(t *testing.T) { + // Any other code configured via FailRefundCode preserves the prior + // ErrRefundDeclined classification (e.g. REFUND_DECLINED in prod). + client := NewDevClient().(*MockClient) + client.FailRefundCode = "REFUND_DECLINED" + + ctx := context.Background() + req := RefundPaymentReq{ + PaymentID: "pay_mock_refund_declined", + Amount: 5000, + IdempotencyKey: "refund-key-declined", + Reason: "declined", + } + + result, err := client.RefundPayment(ctx, req) + require.Error(t, err) + assert.Nil(t, result) + assert.True(t, errors.Is(err, ErrRefundDeclined), "expected ErrRefundDeclined, got: %v", err) + assert.False(t, errors.Is(err, ErrRefundAlreadyProcessed), "declined refund must not be classified as already processed: %v", err) +} + func TestDevClient_ConcurrentPayments(t *testing.T) { client := NewDevClient().(*MockClient) @@ -402,6 +451,46 @@ func TestDevClient_CreatePayment_DedupsOnIdempotencyKey(t *testing.T) { assert.Equal(t, first.ID, byKey.ID) } +func TestDevClient_RefundPayment_DedupsOnIdempotencyKey(t *testing.T) { + // Real Square dedups on idempotency key: a same-key retry returns the + // original refund. The mock must mirror this or the pending-refund resume + // path can't be exercised (and a retry could double-refund the customer). + client := NewDevClient().(*MockClient) + ctx := context.Background() + + paymentResult, err := client.CreatePayment(ctx, CreatePaymentReq{ + Amount: 10000, + Currency: "GBP", + SourceID: "cnon:test-card", + IdempotencyKey: "payment-for-refund-dedup", + ReferenceID: "booking-refund-dedup", + }) + require.NoError(t, err) + + req := RefundPaymentReq{ + PaymentID: paymentResult.ID, + Amount: 5000, + IdempotencyKey: "refund-dedup-key-1", + Reason: "customer request", + } + + first, err := client.RefundPayment(ctx, req) + require.NoError(t, err) + require.NotEmpty(t, first.ID) + + second, err := client.RefundPayment(ctx, req) + require.NoError(t, err) + assert.Equal(t, first.ID, second.ID, "same-key retry must return the original refund, not a new one") + + // Only one refund stored in the mock's refunds map (deduped). + client.mu.RLock() + defer client.mu.RUnlock() + assert.Len(t, client.refunds, 1, "same-key retry must not store a second refund") + byKey := client.refundByKey["refund-dedup-key-1"] + assert.NotNil(t, byKey) + assert.Equal(t, first.ID, byKey.ID) +} + func TestDevClient_CreatePayment_AutocompleteFalse(t *testing.T) { client := NewDevClient().(*MockClient) ctx := context.Background() @@ -519,6 +608,75 @@ func TestDevClient_CreateCheckout_HoldCheckouts(t *testing.T) { require.Error(t, err) } +func TestDevClient_ListPaymentRefunds_FiltersByPaymentAndTime(t *testing.T) { + client := NewDevClient().(*MockClient) + ctx := context.Background() + + begin := time.Date(2026, 7, 1, 0, 0, 0, 0, time.UTC) + client.mu.Lock() + client.refunds["ref_1"] = &RefundResult{ + ID: "ref_1", Status: "COMPLETED", Amount: 5000, PaymentID: "pay_a", + LocationID: "L_MOCK", Reason: "customer request", CreatedAt: begin.Add(2 * 24 * time.Hour).Format(time.RFC3339), + } + client.refunds["ref_2"] = &RefundResult{ + ID: "ref_2", Status: "COMPLETED", Amount: 2500, PaymentID: "pay_b", + LocationID: "L_MOCK", Reason: "customer request", CreatedAt: begin.Add(3 * 24 * time.Hour).Format(time.RFC3339), + } + client.refunds["ref_3"] = &RefundResult{ + ID: "ref_3", Status: "COMPLETED", Amount: 1000, PaymentID: "pay_a", + LocationID: "L_MOCK", Reason: "customer request", CreatedAt: begin.Add(-1 * 24 * time.Hour).Format(time.RFC3339), + } + client.mu.Unlock() + + results, err := client.ListPaymentRefunds(ctx, "pay_a", begin) + require.NoError(t, err) + require.Len(t, results, 1, "only the pay_a refund created after beginTime must be returned") + assert.Equal(t, "ref_1", results[0].ID) + assert.Equal(t, int64(5000), results[0].Amount) + assert.Equal(t, "COMPLETED", results[0].Status) + assert.Equal(t, "pay_a", results[0].PaymentID) +} + +func TestDevClient_ListPaymentRefunds_AfterRefundPayment(t *testing.T) { + client := NewDevClient().(*MockClient) + ctx := context.Background() + + paymentResult, err := client.CreatePayment(ctx, CreatePaymentReq{ + Amount: 10000, + Currency: "GBP", + SourceID: "cnon:test-card", + IdempotencyKey: "payment-for-list-refunds", + ReferenceID: "booking-list-refunds", + }) + require.NoError(t, err) + + refundResult, err := client.RefundPayment(ctx, RefundPaymentReq{ + PaymentID: paymentResult.ID, + Amount: 5000, + IdempotencyKey: "refund-for-list", + Reason: "customer request", + }) + require.NoError(t, err) + + results, err := client.ListPaymentRefunds(ctx, paymentResult.ID, time.Now().Add(-24*time.Hour)) + require.NoError(t, err) + require.Len(t, results, 1, "the refund stored by RefundPayment must be listed") + assert.Equal(t, refundResult.ID, results[0].ID) + assert.Equal(t, int64(5000), results[0].Amount) + assert.Equal(t, "COMPLETED", results[0].Status) + assert.Equal(t, paymentResult.ID, results[0].PaymentID) +} + +func TestDevClient_ListPaymentRefunds_Empty(t *testing.T) { + client := NewDevClient().(*MockClient) + ctx := context.Background() + + results, err := client.ListPaymentRefunds(ctx, "pay_unknown", time.Now().Add(-24*time.Hour)) + require.NoError(t, err) + assert.NotNil(t, results, "must return an empty slice, not nil") + assert.Empty(t, results) +} + func TestDetectCardInfo_Variants(t *testing.T) { tests := []struct { sourceID string diff --git a/backend/internal/square/square_http_client.go b/backend/internal/square/square_http_client.go index ac4d3bc..b1b4462 100644 --- a/backend/internal/square/square_http_client.go +++ b/backend/internal/square/square_http_client.go @@ -91,7 +91,8 @@ func (c *httpClient) doJSON(ctx context.Context, method, path string, body, targ var errResp struct{ Errors []SquareError `json:"errors"` } if json.Unmarshal(respBody, &errResp) == nil && len(errResp.Errors) > 0 { se := errResp.Errors[0] - return fmt.Errorf("square: %s %s: [%s/%s] %s (field: %s)", method, path, se.Category, se.Code, se.Detail, se.Field) + msg := fmt.Sprintf("square: %s %s: [%s/%s] %s (field: %s)", method, path, se.Category, se.Code, se.Detail, se.Field) + return &squareAPIError{Code: se.Code, Detail: se.Detail, err: errors.New(msg)} } return fmt.Errorf("square: %s %s: HTTP %d: %s", method, path, resp.StatusCode, string(respBody)) } @@ -230,6 +231,11 @@ type sqRefundPaymentResponse struct { Refund sqRefund `json:"refund"` } +type sqListRefundsResponse struct { + Refunds []sqRefund `json:"refunds"` + Cursor string `json:"cursor"` +} + type sqRefund struct { ID string `json:"id"` Status string `json:"status"` @@ -343,6 +349,30 @@ func getCheckoutHTTP(ctx context.Context, checkoutID string) (*PaymentResult, er return paymentFromSquare(&payResp.Payment), nil } +// squareAPIError wraps a formatted Square API error while exposing the +// structured Square error code so callers can classify definitive business +// rejections (e.g. ErrRefundDeclined) vs ambiguous transport/server errors. +type squareAPIError struct { + Code string + Detail string + err error +} + +func (e *squareAPIError) Error() string { return e.err.Error() } +func (e *squareAPIError) Unwrap() error { return e.err } + +// Definitive Square refund rejection codes — the refund was declined and can +// never succeed, so retrying is pointless and the refund record should be +// marked 'failed'. Anything else (transport errors, 5xx) is left ambiguous so +// callers leave the refund 'pending' for a scheduler retry. Note that +// PAYMENT_ALREADY_REFUNDED is intentionally absent — the money has already +// moved, so it maps to ErrRefundAlreadyProcessed instead of ErrRefundDeclined. +var definitiveRefundCodes = map[string]bool{ + "REFUND_DECLINED": true, + "PAYMENT_REFUND_AMOUNT_EXCEEDED": true, + "INVALID_PAYMENT_ID": true, +} + func refundPaymentHTTP(ctx context.Context, req RefundPaymentReq) (*RefundResult, error) { hc := newHTTPClient() body := sqRefundPaymentRequest{ @@ -353,11 +383,42 @@ func refundPaymentHTTP(ctx context.Context, req RefundPaymentReq) (*RefundResult } var resp sqRefundPaymentResponse if err := hc.doJSON(ctx, http.MethodPost, "/v2/refunds", body, &resp); err != nil { + var sqErr *squareAPIError + if errors.As(err, &sqErr) && definitiveRefundCodes[sqErr.Code] { + return nil, fmt.Errorf("%w: %v", ErrRefundDeclined, err) + } + if errors.As(err, &sqErr) && sqErr.Code == "PAYMENT_ALREADY_REFUNDED" { + return nil, fmt.Errorf("%w: %v", ErrRefundAlreadyProcessed, err) + } return nil, err } return refundFromSquare(&resp.Refund), nil } +func listRefundsHTTP(ctx context.Context, paymentID string, beginTime time.Time) ([]RefundResult, error) { + hc := newHTTPClient() + base := "/v2/refunds?begin_time=" + url.QueryEscape(beginTime.UTC().Format(time.RFC3339)) + "&limit=100" + path := base + results := []RefundResult{} + for page := 0; page < 20; page++ { + var resp sqListRefundsResponse + if err := hc.doJSON(ctx, http.MethodGet, path, nil, &resp); err != nil { + return nil, err + } + for i := range resp.Refunds { + r := &resp.Refunds[i] + if r.PaymentID == paymentID { + results = append(results, *refundFromSquare(r)) + } + } + if resp.Cursor == "" { + return results, nil + } + path = base + "&cursor=" + url.QueryEscape(resp.Cursor) + } + return nil, fmt.Errorf("square: list refunds exceeded 20 pages (infinite loop guard)") +} + func createCardOnFileHTTP(ctx context.Context, userID, cardToken string) (*CardOnFile, error) { hc := newHTTPClient() diff --git a/backend/internal/square/types.go b/backend/internal/square/types.go index 572fe3f..bcd6030 100644 --- a/backend/internal/square/types.go +++ b/backend/internal/square/types.go @@ -1,6 +1,25 @@ package square -import "context" +import ( + "context" + "errors" + "time" +) + +// ErrRefundDeclined is returned by RefundPayment when Square definitively +// rejects a refund (refund declined, refund amount exceeds the original +// charge, invalid payment ID, etc.). Callers use errors.Is to distinguish a +// definitive business rejection — where the refund record should be marked +// 'failed' and never retried — from an ambiguous transport/5xx error that is +// safe to retry later. Note: PAYMENT_ALREADY_REFUNDED is NOT a decline — the +// money has already moved, so it maps to ErrRefundAlreadyProcessed instead. +var ErrRefundDeclined = errors.New("square: refund declined") + +// ErrRefundAlreadyProcessed is returned by RefundPayment when Square reports +// PAYMENT_ALREADY_REFUNDED — the payment is already fully refunded at Square, +// so the money has already moved. Callers resolve the refund record to +// 'completed' rather than 'failed' (which would let the guard over-refund). +var ErrRefundAlreadyProcessed = errors.New("square: refund already processed") // CreatePaymentReq maps to Square's CreatePayment endpoint (POST /v2/payments). // Square API reference: https://developer.squareup.com/reference/square/payments-api/create-payment @@ -139,4 +158,11 @@ type SquareClient interface { CreateCardOnFile(ctx context.Context, userID, cardToken string) (*CardOnFile, error) GetCardsOnFile(ctx context.Context, userID string) ([]CardOnFile, error) DeleteCardOnFile(ctx context.Context, cardID string) error + + // ListPaymentRefunds returns the refunds Square has recorded for a payment + // (charge), created at or after beginTime. Used to reconcile pending refund + // rows against Square before marking them failed (money may already have + // moved). Square's endpoint lists account-wide; the caller filters by + // PaymentID client-side. + ListPaymentRefunds(ctx context.Context, paymentID string, beginTime time.Time) ([]RefundResult, error) } diff --git a/backend/testutils/fixtures/fixtures.go b/backend/testutils/fixtures/fixtures.go index 9386aa2..880c3c2 100644 --- a/backend/testutils/fixtures/fixtures.go +++ b/backend/testutils/fixtures/fixtures.go @@ -9,6 +9,7 @@ import ( "sync/atomic" "time" + "crussell/clock" "crussell/db" "golang.org/x/crypto/bcrypt" ) @@ -178,6 +179,15 @@ func CreateTestBookingAtTime(q db.Querier, userID, serviceID string, startTime t return bookingID, nil } +// NextWorkingDayAt returns the time at `hour` UTC on a day `daysAhead` days from +// now. Hours in [8, 18] are guaranteed inside the fixture's Mon-Sun 08:00-20:00 +// London working hours at any wall-clock time (London is at most UTC+1), unlike +// clock.Now().Add(N * time.Hour), which can land after closing and flake tests. +func NextWorkingDayAt(daysAhead, hour int) time.Time { + day := clock.Now().AddDate(0, 0, daysAhead) + return time.Date(day.Year(), day.Month(), day.Day(), hour, 0, 0, 0, time.UTC) +} + func CreateTestVerifiedUser(q db.Querier) (string, error) { return createTestUser(q, "Verified", "User", "verified@test.com", "verified_email") } diff --git a/frontend/src/lib/components/account/UserBookingModal.svelte b/frontend/src/lib/components/account/UserBookingModal.svelte index 054ede8..8c2de45 100644 --- a/frontend/src/lib/components/account/UserBookingModal.svelte +++ b/frontend/src/lib/components/account/UserBookingModal.svelte @@ -15,8 +15,8 @@ import { computeBalanceDue } from '$lib/utils/booking'; import { parseWallClockDate } from '$lib/utils/timeSlots'; import type { Booking, BookingDiscount, Payment } from '$lib/types/booking'; - import CardInput from '$lib/components/payments/CardInput.svelte'; import CardBrandIcon from '$lib/components/payments/CardBrandIcon.svelte'; + import CardEntryUnavailable from '$lib/components/payments/CardEntryUnavailable.svelte'; import { savedCardsStore, type SavedCard } from '$lib/stores/savedCards.svelte'; interface Props { open: boolean; @@ -149,106 +149,12 @@ let tipSavedCards = $state([]); let tipLoadingCards = $state(false); let tipSelectedCardId = $state(null); - let tipShowNewCard = $state(false); - - // New card form state for tips - let tipNewCardNumber = $state(''); - let tipNewCardExpiry = $state(''); - let tipNewCardCVC = $state(''); - let tipSaveCardFuture = $state(false); - let tipCardNumberTouched = $state(false); - let tipCardExpiryTouched = $state(false); - let tipCVCTouched = $state(false); const canSaveCards = $derived( authStore.currentUser?.role === 'verified_email' || authStore.currentUser?.role === 'affiliate' ); - // Card validation (matching UserPaymentModal pattern) - function isValidLuhn(cardNumber: string): boolean { - const s = cardNumber.replace(/\D/g, ''); - let sum = 0; - let alternate = false; - for (let i = s.length - 1; i >= 0; i--) { - let n = parseInt(s[i], 10); - if (alternate) { - n *= 2; - if (n > 9) n -= 9; - } - sum += n; - alternate = !alternate; - } - return sum % 10 === 0 && s.length >= 13 && s.length <= 19; - } - - function handleTipFieldBlur(field: string) { - if (field === 'cardNumber') tipCardNumberTouched = true; - else if (field === 'cardExpiry') tipCardExpiryTouched = true; - else if (field === 'cardCVC') tipCVCTouched = true; - } - - function handleTipFieldInput(field: string) { - if (field === 'cardNumber') tipCardNumberTouched = false; - else if (field === 'cardExpiry') tipCardExpiryTouched = false; - else if (field === 'cardCVC') tipCVCTouched = false; - } - - function parseExpiryParts(value: string): { month: number; year: number } | null { - if (!/^\d{2}\/\d{2}$/.test(value)) return null; - const [monthStr, yearStr] = value.split('/'); - const month = parseInt(monthStr, 10); - const year = 2000 + parseInt(yearStr, 10); - if (month < 1 || month > 12) return null; - return { month, year }; - } - - const tipNewCardExpiryParts = $derived(parseExpiryParts(tipNewCardExpiry)); - const isTipNewCardExpiryPast = $derived( - tipNewCardExpiryParts !== null && - (() => { - const expiryYearMonth = tipNewCardExpiryParts.year * 12 + tipNewCardExpiryParts.month; - const now = new SvelteDate(); - const currentYearMonth = now.getFullYear() * 12 + now.getMonth() + 1; - return expiryYearMonth < currentYearMonth; - })() - ); - const hasTipNewCardInvalidMonth = $derived( - /^\d{2}\/\d{2}$/.test(tipNewCardExpiry) && tipNewCardExpiryParts === null - ); - - const tipNewCardError = $derived( - tipShowNewCard || tipSavedCards.length === 0 - ? tipCardNumberTouched && !isValidLuhn(tipNewCardNumber) && tipNewCardNumber.length > 0 - ? 'Invalid card number' - : tipCardExpiryTouched && hasTipNewCardInvalidMonth - ? 'Invalid expiry month' - : tipCardExpiryTouched && isTipNewCardExpiryPast - ? 'This card has expired' - : tipCardExpiryTouched && - tipNewCardExpiry.length > 0 && - !/^\d{2}\/\d{2}$/.test(tipNewCardExpiry) - ? 'Enter expiry as MM/YY' - : tipCVCTouched && tipNewCardCVC.length < 3 && tipNewCardCVC.length > 0 - ? 'Enter your CVC number' - : isValidLuhn(tipNewCardNumber) && - /^\d{2}\/\d{2}$/.test(tipNewCardExpiry) && - tipNewCardCVC.length >= 3 - ? null - : tipNewCardNumber.length === 0 && - tipNewCardExpiry.length === 0 && - tipNewCardCVC.length === 0 - ? null - : 'Please complete all card fields' - : null - ); - - const isTipCardValid = $derived( - tipSelectedCardId !== null || - (isValidLuhn(tipNewCardNumber) && - tipNewCardExpiryParts !== null && - !isTipNewCardExpiryPast && - tipNewCardCVC.length >= 3) - ); + const isTipCardValid = $derived(tipSelectedCardId !== null); const tipPresets = $derived( selectedBooking @@ -314,31 +220,17 @@ return; } - if (tipSavedCards.length > 0 && !tipSelectedCardId && !tipShowNewCard) { - toast.error('Please select a payment method'); + if (tipSavedCards.length === 0) { + toast.error('Please add a saved card or contact the salon to pay by another method'); return; } - if ((tipShowNewCard || tipSavedCards.length === 0) && !tipNewCardNumber.replace(/\s/g, '')) { - toast.error('Please enter your card number'); + if (!tipSelectedCardId) { + toast.error('Please select a payment method'); return; } tipProcessing = true; - // Validate card details for new card payments - if (tipShowNewCard || tipSavedCards.length === 0) { - if ( - !isValidLuhn(tipNewCardNumber) || - !/^\d{2}\/\d{2}$/.test(tipNewCardExpiry) || - isTipNewCardExpiryPast || - tipNewCardCVC.length < 3 - ) { - tipProcessing = false; - toast.error(tipNewCardError || 'Please enter valid credit card details'); - return; - } - } - try { if (!tipIdempotencyKey || tipKeyedAmount !== tipAmount) { tipIdempotencyKey = crypto.randomUUID(); @@ -346,16 +238,10 @@ } const body: Record = { amount: Math.round(tipAmount * 100), - idempotency_key: tipIdempotencyKey + idempotency_key: tipIdempotencyKey, + card_id: tipSelectedCardId }; - if (tipShowNewCard || tipSavedCards.length === 0) { - body.new_card_token = tipNewCardNumber.replace(/\s/g, ''); - body.save_card = tipSaveCardFuture; - } else { - body.card_id = tipSelectedCardId; - } - const response = await apiFetch(`/api/bookings/${selectedBooking.id}/tip`, { method: 'POST', headers: { 'Content-Type': 'application/json' }, @@ -1264,13 +1150,10 @@ ${hasVAT ? `

VAT is included at ${biz?.default_vat_rate ?? 20} {/each} - - - {#if tipShowNewCard} -

- - {#if tipNewCardError} -
{tipNewCardError}
- {/if} -
- {/if} {:else} -
- - {#if tipNewCardError} -
{tipNewCardError}
- {/if} -
+ {/if} diff --git a/frontend/src/lib/components/admin/GiftCardsManagement.svelte b/frontend/src/lib/components/admin/GiftCardsManagement.svelte index 3e07182..af268ca 100644 --- a/frontend/src/lib/components/admin/GiftCardsManagement.svelte +++ b/frontend/src/lib/components/admin/GiftCardsManagement.svelte @@ -9,6 +9,7 @@ import { EmailInput } from '$lib/components/ui/email-input'; import * as Modal from '$lib/components/ui/dialog'; import { Skeleton } from '$lib/components/ui/skeleton'; + import CardEntryUnavailable from '$lib/components/payments/CardEntryUnavailable.svelte'; import { range } from '$lib/utils/format'; import { formatUserName } from '$lib/utils/nameDisplay'; import { SvelteDate, SvelteURLSearchParams } from 'svelte/reactivity'; @@ -107,7 +108,6 @@ | 'amount_email' | 'payment' | 'cash_entry' - | 'card_details' | 'processing' | 'success' | 'error' @@ -142,9 +142,6 @@ // Payment Processing States let cashAmount = $state(''); - let ephemeralCardNumber = $state(''); - let ephemeralCardExpiry = $state(''); - let ephemeralCardCVC = $state(''); let paymentError = $state(''); let paymentResult = $state<{ id: string; @@ -180,14 +177,7 @@ } let topUpStep = $state< - | 'choice' - | 'amount' - | 'payment' - | 'cash_entry' - | 'card_details' - | 'processing' - | 'success' - | 'error' + 'choice' | 'amount' | 'payment' | 'cash_entry' | 'processing' | 'success' | 'error' >('choice'); let topUpMode = $state<'giveaway' | 'purchase'>('giveaway'); @@ -469,9 +459,6 @@ // Reset payment cashAmount = ''; - ephemeralCardNumber = ''; - ephemeralCardExpiry = ''; - ephemeralCardCVC = ''; paymentError = ''; paymentResult = null; cardMachineItemID = null; @@ -480,38 +467,9 @@ // =============== Embedded Payment Handlers =============== - const isEphemeralCardValid = $derived( - ephemeralCardNumber.replace(/\s/g, '').length >= 13 && - ephemeralCardExpiry.includes('/') && - ephemeralCardExpiry.length === 5 && - ephemeralCardCVC.length >= 3 - ); - - function handleEphemeralCardNumberInput(e: Event) { - const target = e.currentTarget as HTMLInputElement; - const clean = target.value.replace(/\D/g, ''); - const formatted = clean.match(/.{1,4}/g)?.join(' ') || clean; - ephemeralCardNumber = formatted.slice(0, 19); - } - - function handleEphemeralExpiryInput(e: Event) { - const target = e.currentTarget as HTMLInputElement; - const clean = target.value.replace(/\D/g, ''); - if (clean.length > 2) { - ephemeralCardExpiry = clean.slice(0, 2) + '/' + clean.slice(2, 4); - } else { - ephemeralCardExpiry = clean; - } - } - - function handleEphemeralCvcInput(e: Event) { - const target = e.currentTarget as HTMLInputElement; - ephemeralCardCVC = target.value.replace(/\D/g, '').slice(0, 4); - } - function setModalStep( actionType: 'create' | 'topup', - step: 'processing' | 'success' | 'error' | 'payment' | 'cash_entry' | 'card_details' + step: 'processing' | 'success' | 'error' | 'payment' | 'cash_entry' ) { if (actionType === 'create') { generateStep = step; @@ -636,54 +594,6 @@ setModalStep(actionType, 'error'); } - async function handleEmbeddedEphemeralCardPayment(actionType: 'create' | 'topup', gcId?: string) { - const cardNum = ephemeralCardNumber.replace(/\s/g, ''); - const [monthStr, yearStr] = ephemeralCardExpiry.split('/'); - const expMonth = parseInt(monthStr, 10); - const expYear = 2000 + parseInt(yearStr, 10); - - setModalStep(actionType, 'processing'); - processingMessage = 'Processing card payment...'; - try { - const amt = actionType === 'create' ? Number(generateAmount) : Number(topUpAmount); - const body: Record = { - item_type: 'gift_card', - action: actionType, - amount: amt, - payment_method: 'online_square', - idempotency_key: getIdempotencyKey(), - card_number: cardNum, - card_exp_month: expMonth, - card_exp_year: expYear, - card_cvc: ephemeralCardCVC - }; - if (gcId) body.gift_card_id = gcId; - if (selectedCustomer) body.user_id = selectedCustomer.id; - if (actionType === 'create' && generateType === 'account' && selectedCustomer) - body.redeem_to_user_id = selectedCustomer.id; - - const res = await apiFetch('/api/admin/till/sale', { - method: 'POST', - headers: { - 'Content-Type': 'application/json' - }, - body: JSON.stringify(body) - }); - if (res.ok) { - const data = await res.json(); - paymentResult = { ...data }; - setModalStep(actionType, 'success'); - await fetchGiftCards(); - } else { - paymentError = await res.text(); - setModalStep(actionType, 'error'); - } - } catch { - paymentError = 'Network error processing card payment'; - setModalStep(actionType, 'error'); - } - } - async function handleEmbeddedGiveawayTopUp(gcId: string) { topUpStep = 'processing'; processingMessage = 'Processing on-the-house top-up...'; @@ -1495,8 +1405,6 @@ Select the payment method. {:else if generateStep === 'cash_entry'} Enter cash amount received. - {:else if generateStep === 'card_details'} - Enter card payment details. {/if} @@ -1902,25 +1810,11 @@ Cash - +
+ +
@@ -1964,68 +1858,6 @@ Confirm Cash - {:else if generateStep === 'card_details'} -
-
-

Online Card Processing

-
-
- - -
-
-
- - -
-
- - -
-
-
-
-
- - - - {:else if generateStep === 'processing'}
@@ -2219,25 +2049,11 @@ Cash - +
+ +
@@ -2279,68 +2095,6 @@ Confirm Cash - {:else if topUpStep === 'card_details'} -
-
-

Online Card Processing

-
-
- - -
-
-
- - -
-
- - -
-
-
-
-
- - - - {:else if topUpStep === 'processing'}
([]); let paymentMethodsLoading = $state(false); let selectedPaymentMethod = $state(null); - let showNewCardForm = $state(false); let isProcessingPayment = $state(false); // Synchronous double-click guard. Svelte 5 reactivity is async (effects run // on the next microtask), so `isProcessingPayment` may not propagate to the @@ -95,76 +94,10 @@ // reactive flag is checked synchronously at the start of processPayment. let isProcessingPaymentSync = false; - // New card form fields - let newCardNumber = $state(''); - let newCardExpiry = $state(''); - let newCardCVC = $state(''); - let cardNumberTouched = $state(false); - let cardExpiryTouched = $state(false); - let cardCVCTouched = $state(false); - - function parseExpiryParts(value: string): { month: number; year: number } | null { - if (!/^\d{2}\/\d{2}$/.test(value)) return null; - const [monthStr, yearStr] = value.split('/'); - const month = parseInt(monthStr, 10); - const year = 2000 + parseInt(yearStr, 10); - if (month < 1 || month > 12) return null; - return { month, year }; - } - - function isValidLuhn(cardNumber: string): boolean { - const s = cardNumber.replace(/\D/g, ''); - let sum = 0; - let alternate = false; - for (let i = s.length - 1; i >= 0; i--) { - let n = parseInt(s[i], 10); - if (alternate) { - n *= 2; - if (n > 9) n -= 9; - } - sum += n; - alternate = !alternate; - } - return sum % 10 === 0 && s.length >= 13 && s.length <= 19; - } - - const expiryParts = $derived(parseExpiryParts(newCardExpiry)); - // Payment flow state let depositPaid = $state(false); - const cardError = $derived( - cardNumberTouched && !isValidLuhn(newCardNumber) && newCardNumber.length > 0 - ? 'Invalid card number' - : cardExpiryTouched && - expiryParts !== null && - (() => { - const em = expiryParts.year * 12 + expiryParts.month; - const now2 = new SvelteDate(); - const cm = now2.getFullYear() * 12 + now2.getMonth() + 1; - return em < cm; - })() - ? 'This card has expired' - : cardExpiryTouched && !/^\d{2}\/\d{2}$/.test(newCardExpiry) && newCardExpiry.length > 0 - ? 'Enter expiry as MM/YY' - : cardCVCTouched && newCardCVC.length < 3 && newCardCVC.length > 0 - ? 'Enter your CVC number' - : isValidLuhn(newCardNumber) && - /^\d{2}\/\d{2}$/.test(newCardExpiry) && - newCardCVC.length >= 3 - ? null - : newCardNumber.length === 0 && newCardExpiry.length === 0 && newCardCVC.length === 0 - ? null - : 'Please complete all card fields' - ); - - const depositCardFormValid = $derived( - selectedPaymentMethod !== null || - (showNewCardForm && - newCardNumber.replace(/\s/g, '').length >= 13 && - /^\d{2}\/\d{2}$/.test(newCardExpiry) && - newCardCVC.length >= 3) - ); + const depositCardFormValid = $derived(selectedPaymentMethod !== null); // VAT registration status from public business info (via shared store) const vatRegistered = $derived(getBusinessInfo()?.is_vat_registered ?? false); @@ -325,18 +258,6 @@ } } - function handleFieldBlur(field: string) { - if (field === 'cardNumber') cardNumberTouched = true; - else if (field === 'cardExpiry') cardExpiryTouched = true; - else if (field === 'cardCVC') cardCVCTouched = true; - } - - function handleFieldInput(field: string) { - if (field === 'cardNumber') cardNumberTouched = false; - else if (field === 'cardExpiry') cardExpiryTouched = false; - else if (field === 'cardCVC') cardCVCTouched = false; - } - async function processPayment(amount: number) { // Synchronous double-click guard — set BEFORE any await so a rapid second // click is rejected immediately, even before the reactive `disabled` has @@ -346,6 +267,10 @@ isProcessingPayment = true; paymentAttempted = false; try { + if (!selectedPaymentMethod) { + toast.error('Please select a saved card'); + return; + } await submitAndProceed(); if (!confirmedBooking) { toast.error('Booking was not created. Please try again.'); @@ -356,7 +281,7 @@ // Cache the idempotency key per amount+card so a lost-response retry // reuses it (backend dedups) instead of double-charging. - const cardKey = selectedPaymentMethod ?? newCardNumber.replace(/\s/g, '') ?? ''; + const cardKey = selectedPaymentMethod; if ( !depositIdempotencyKey || depositKeyedAmount !== amountCents || @@ -370,18 +295,10 @@ const body: Record = { payment_type: 'deposit', amount: amountCents, - idempotency_key: depositIdempotencyKey + idempotency_key: depositIdempotencyKey, + card_id: selectedPaymentMethod }; - if (selectedPaymentMethod) { - body.card_id = selectedPaymentMethod; - } else { - const rawNumber = newCardNumber.replace(/\s/g, ''); - if (rawNumber.length >= 13) { - body.new_card_token = rawNumber; - } - } - paymentAttempted = true; const response = await apiFetch(`/api/bookings/${bookingId}/payment`, { @@ -2345,7 +2262,7 @@ {#if authStore.isAuthenticated} {#if paymentMethodsLoading} -
Loading payment methods...
+
Loading payment methods...
{:else if paymentMethods.length > 0}

Saved Cards

@@ -2373,10 +2290,7 @@ @@ -2384,34 +2298,19 @@ {/each}
+ {:else} +
+ +
{/if} - - {#if !showNewCardForm} - - {/if} - {/if} - - {#if showNewCardForm || !authStore.isAuthenticated} - - {#if cardError} -

{cardError}

- {/if} + {:else} +
+ +
{/if}
diff --git a/frontend/src/lib/components/payments/CardEntryUnavailable.svelte b/frontend/src/lib/components/payments/CardEntryUnavailable.svelte new file mode 100644 index 0000000..d835c58 --- /dev/null +++ b/frontend/src/lib/components/payments/CardEntryUnavailable.svelte @@ -0,0 +1,21 @@ + + +
+
+ + + + +

{message}

+
+
diff --git a/frontend/src/lib/components/payments/CardInput.svelte b/frontend/src/lib/components/payments/CardInput.svelte deleted file mode 100644 index 1ec8e88..0000000 --- a/frontend/src/lib/components/payments/CardInput.svelte +++ /dev/null @@ -1,101 +0,0 @@ - - -
-

Card Details

-
-
- - { - cardNumber = formatNumber((e.target as HTMLInputElement).value); - onfieldinput('cardNumber'); - }} - onblur={() => onfieldblur('cardNumber')} - placeholder="1234 5678 9012 3456" - maxlength={19} - {disabled} - /> -
-
-
- - { - cardExpiry = formatExpiry((e.target as HTMLInputElement).value); - onfieldinput('cardExpiry'); - }} - onblur={() => onfieldblur('cardExpiry')} - placeholder="MM/YY" - maxlength={5} - {disabled} - /> -
-
- - onfieldinput('cardCVC')} - onblur={() => onfieldblur('cardCVC')} - placeholder="123" - maxlength={4} - {disabled} - /> -
-
- {#if showSaveCard} -
- - -
- {/if} -
-
diff --git a/frontend/src/lib/components/payments/CardSelection.svelte b/frontend/src/lib/components/payments/CardSelection.svelte index 3b44591..4a7cb7d 100644 --- a/frontend/src/lib/components/payments/CardSelection.svelte +++ b/frontend/src/lib/components/payments/CardSelection.svelte @@ -1,7 +1,6 @@