diff --git a/backend/handlers/payments/concurrency_test.go b/backend/handlers/payments/concurrency_test.go new file mode 100644 index 0000000..101cfbc --- /dev/null +++ b/backend/handlers/payments/concurrency_test.go @@ -0,0 +1,310 @@ +//go:build test && dev + +package payments + +import ( + "context" + "net/http" + "net/http/httptest" + "sync" + "testing" + "time" + + "crussell/db" + "crussell/internal/square" + "crussell/testutils" + "crussell/testutils/fixtures" + "crussell/testutils/jwt" +) + +// slowCreatePaymentClient delays the Square charge so each handler holds its +// advisory lock long enough that a concurrent same-key request would race it +// without the lock (two goroutines both reading "no record", both charging). +type slowCreatePaymentClient struct { + square.SquareClient + delay time.Duration +} + +func (c *slowCreatePaymentClient) CreatePayment(ctx context.Context, req square.CreatePaymentReq) (*square.PaymentResult, error) { + time.Sleep(c.delay) + return c.SquareClient.CreatePayment(ctx, req) +} + +// cleanupConcurrentTestRows deletes the rows a concurrency test committed at +// pool level. These tests must COMMIT their setup so the advisory locks work +// across independent connections, which leaves committed rows in the shared +// test DB — without cleanup they leak into parallel tests (e.g. GetGiftCards +// counts gift_cards/user_giftcard_balances globally). FK-safe deletion order. +func cleanupConcurrentTestRows(t *testing.T, pool context.Context, userID, bookingID string) { + t.Helper() + t.Cleanup(func() { + var gcIDs []string + rows, err := db.Conn.Query(pool, `SELECT id FROM gift_cards WHERE created_by = $1`, userID) + if err == nil { + for rows.Next() { + var id string + if rows.Scan(&id) == nil { + gcIDs = append(gcIDs, id) + } + } + rows.Close() + } + for _, gcID := range gcIDs { + _, _ = db.Conn.Exec(pool, `DELETE FROM gift_card_transactions WHERE gift_card_id = $1`, gcID) + } + _, _ = db.Conn.Exec(pool, `DELETE FROM user_giftcard_balances WHERE user_id = $1`, userID) + if bookingID != "" { + _, _ = db.Conn.Exec(pool, `DELETE FROM payments WHERE booking_id = $1`, bookingID) + _, _ = db.Conn.Exec(pool, `DELETE FROM booking_services WHERE booking_id = $1`, bookingID) + _, _ = db.Conn.Exec(pool, `DELETE FROM bookings WHERE id = $1`, bookingID) + } + _, _ = db.Conn.Exec(pool, `DELETE FROM payments WHERE created_by = $1 OR idempotency_key LIKE 'concurrent-%'`, userID) + for _, gcID := range gcIDs { + _, _ = db.Conn.Exec(pool, `DELETE FROM gift_cards WHERE id = $1`, gcID) + } + _, _ = db.Conn.Exec(pool, `DELETE FROM user_saved_cards WHERE user_id = $1`, userID) + _, _ = db.Conn.Exec(pool, `DELETE FROM users WHERE id = $1`, userID) + }) +} + +// TestBuyGiftCard_ConcurrentSameKey_SingleRecord proves the BuyGiftCard +// advisory lock (giftcards.go): two goroutines POSTing the same idempotency key +// must produce exactly ONE payment record and ONE funded gift card — never +// 2× value for 1 charge. Without the lock, both goroutines pass the +// idempotency check, both reuse/insert pending records, and both fund the card. +func TestBuyGiftCard_ConcurrentSameKey_SingleRecord(t *testing.T) { + ctx, tx := testutils.SetupTestTx(t) + + userID, err := fixtures.CreateTestUser(tx) + if err != nil { + t.Fatalf("failed to create user: %v", err) + } + token := jwt.GenerateUserToken(userID) + cleanupConcurrentTestRows(t, context.Background(), userID, "") + + // 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 both sides 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 := &slowCreatePaymentClient{SquareClient: square.NewDevClient(), delay: 300 * time.Millisecond} + SquareClient = slow + defer func() { SquareClient = origClient }() + + pool := context.Background() + key := "buy-gc-concurrent-same-key" + reqBody := map[string]interface{}{ + "amount": 2000, + "recipient_type": "self", + "new_card_token": "cnon:concurrent-card", + "idempotency_key": key, + } + + var wg sync.WaitGroup + startBoth := make(chan struct{}) + recs := make([]*httptest.ResponseRecorder, 2) + + for i := 0; i < 2; i++ { + wg.Add(1) + go func(idx int) { + defer wg.Done() + <-startBoth + recs[idx] = makePaymentRequest(BuyGiftCard, "POST", "/api/user/giftcards/buy", reqBody, token, pool) + }(i) + } + close(startBoth) + wg.Wait() + + // Both requests must succeed — the lock serializes them and the second + // finds the completed record (idempotent dedup, HTTP 200), so neither + // double-charges nor errors. + for i, rec := range recs { + if rec.Code != http.StatusCreated && rec.Code != http.StatusOK { + t.Errorf("request %d expected 201 (create) or 200 (dedup), got %d. body: %s", i, rec.Code, rec.Body.String()) + } + } + + // Exactly one payment record for this key. + var payCount int + err = db.Conn.QueryRow(pool, + `SELECT COUNT(*) FROM payments WHERE idempotency_key = $1`, key).Scan(&payCount) + if err != nil { + t.Fatalf("failed to count payments: %v", err) + } + if payCount != 1 { + t.Errorf("expected exactly 1 payment record, got %d (double-charge!)", payCount) + } + + // Exactly one funded gift card for this user's self-purchase. + var gcCount int + err = db.Conn.QueryRow(pool, + `SELECT COUNT(*) FROM gift_cards WHERE total_funds_added = 20.00 AND created_by = $1`, userID).Scan(&gcCount) + if err != nil { + t.Fatalf("failed to count gift cards: %v", err) + } + if gcCount != 1 { + t.Errorf("expected exactly 1 funded gift card, got %d (2× value!)", gcCount) + } + + // User balance credited exactly once. + var balance float64 + err = db.Conn.QueryRow(pool, + `SELECT balance FROM user_giftcard_balances WHERE user_id = $1`, userID).Scan(&balance) + if err != nil { + t.Fatalf("failed to query balance: %v", err) + } + if balance != 20.00 { + t.Errorf("expected balance 20.00, got %.2f (double-credit!)", balance) + } +} + +// TestTipPayment_ConcurrentSameKey_SingleRecord proves the tip advisory lock +// (handlers.go): two goroutines POSTing the same booking with the same key must +// produce exactly ONE tip payment record — never two charges for one booking. +func TestTipPayment_ConcurrentSameKey_SingleRecord(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) + } + 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) + } + if _, err := fixtures.CreateTestPayment(tx, bookingID, 5000.00, "online_square", "full", "completed"); err != nil { + t.Fatalf("failed to create prior payment: %v", err) + } + token := jwt.GenerateUserToken(userID) + cleanupConcurrentTestRows(t, context.Background(), userID, bookingID) + + 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 := &slowCreatePaymentClient{SquareClient: square.NewDevClient(), delay: 300 * time.Millisecond} + SquareClient = slow + defer func() { SquareClient = origClient }() + + pool := context.Background() + key := "tip-concurrent-same-key" + cardToken := "cnon:concurrent-tip-card" + reqBody := CreateTipPaymentRequest{ + Amount: 500, + NewCardToken: &cardToken, + IdempotencyKey: key, + } + + var wg sync.WaitGroup + startBoth := make(chan struct{}) + recs := make([]*httptest.ResponseRecorder, 2) + + for i := 0; i < 2; i++ { + wg.Add(1) + go func(idx int) { + defer wg.Done() + <-startBoth + recs[idx] = makePaymentRequest(CreateTipPayment, "POST", "/api/bookings/"+bookingID+"/tip", reqBody, token, pool) + }(i) + } + close(startBoth) + wg.Wait() + + for i, rec := range recs { + if rec.Code != http.StatusOK { + t.Errorf("request %d expected 200, got %d. body: %s", i, rec.Code, rec.Body.String()) + } + } + + var tipCount int + err = db.Conn.QueryRow(pool, + `SELECT COUNT(*) FROM payments WHERE booking_id = $1 AND payment_type = 'tip' AND idempotency_key = $2`, bookingID, key).Scan(&tipCount) + if err != nil { + t.Fatalf("failed to count tip payments: %v", err) + } + if tipCount != 1 { + t.Errorf("expected exactly 1 tip payment record, got %d (double-charge!)", tipCount) + } +} + +// TestBookingPayment_ConcurrentSameKey_SingleRecord proves the booking-payment +// advisory lock (handlers.go): two goroutines paying the same booking with the +// same key must produce exactly ONE payment record. +func TestBookingPayment_ConcurrentSameKey_SingleRecord(t *testing.T) { + ctx, tx := testutils.SetupTestTx(t) + + userID, bookingID, _ := setupTestDataPast(t, ctx, tx) + token := jwt.GenerateUserToken(userID) + cleanupConcurrentTestRows(t, context.Background(), userID, bookingID) + + 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 := &slowCreatePaymentClient{SquareClient: square.NewDevClient(), delay: 300 * time.Millisecond} + SquareClient = slow + defer func() { SquareClient = origClient }() + + pool := context.Background() + key := "booking-pay-concurrent-same-key" + cardToken := "cnon:concurrent-booking-card" + reqBody := CreateBookingPaymentRequest{ + Amount: 5000, + PaymentType: "full", + NewCardToken: &cardToken, + IdempotencyKey: key, + } + + var wg sync.WaitGroup + startBoth := make(chan struct{}) + recs := make([]*httptest.ResponseRecorder, 2) + + for i := 0; i < 2; i++ { + wg.Add(1) + go func(idx int) { + defer wg.Done() + <-startBoth + recs[idx] = makePaymentRequest(CreateBookingPayment, "POST", "/api/bookings/"+bookingID+"/payment", reqBody, token, pool) + }(i) + } + close(startBoth) + wg.Wait() + + for i, rec := range recs { + if rec.Code != http.StatusOK { + t.Errorf("request %d expected 200, got %d. body: %s", i, rec.Code, rec.Body.String()) + } + } + + var payCount int + err := db.Conn.QueryRow(pool, + `SELECT COUNT(*) FROM payments WHERE booking_id = $1 AND idempotency_key = $2`, bookingID, key).Scan(&payCount) + if err != nil { + t.Fatalf("failed to count payments: %v", err) + } + if payCount != 1 { + t.Errorf("expected exactly 1 payment record, got %d (double-charge!)", payCount) + } +} diff --git a/backend/handlers/payments/till.go b/backend/handlers/payments/till.go index 05b9e84..4e42ae1 100644 --- a/backend/handlers/payments/till.go +++ b/backend/handlers/payments/till.go @@ -1,6 +1,7 @@ package payments import ( + "context" "crypto/rand" "database/sql" "encoding/json" @@ -109,7 +110,39 @@ func CreateTillSale(w http.ResponseWriter, r *http.Request) { return } - // Idempotency check: if key provided, return existing sale if found + // Serialize till-sale attempts on the idempotency key to prevent concurrent + // same-key requests from both passing the idempotency check, both funding + // the gift card, and one dying on the till_sales idempotency_key UNIQUE + // constraint after the funding already committed. Mirrors the gift-card + // advisory-lock pattern (giftcards.go). Lock is keyed on the idempotency + // key so distinct sales are unaffected; falls back to a per-request key + // when absent (client-supplied key is always used in practice). + lockKey := req.IdempotencyKey + if lockKey == "" { + lockKey = "till-" + rand.Text() + } + pinConn, err := db.Conn.Acquire(ctx) + if err != nil { + log.Printf("Failed to acquire connection for till-sale lock: %v", err) + http.Error(w, "internal server error", http.StatusInternalServerError) + return + } + defer pinConn.Release() + if _, err := pinConn.Exec(ctx, ` + SELECT pg_advisory_lock(hashtext('crussell:till:' || $1)) + `, lockKey); err != nil { + log.Printf("Failed to acquire till-sale serialization lock for %s: %v", lockKey, err) + http.Error(w, "internal server error", http.StatusInternalServerError) + return + } + defer func() { + if _, err := pinConn.Exec(context.Background(), ` + SELECT pg_advisory_unlock(hashtext('crussell:till:' || $1)) + `, lockKey); err != nil { + log.Printf("Failed to release till-sale serialization lock for %s: %v", lockKey, err) + } + }() + // Idempotency handling. A 'completed' sale is a dedup (return it). A // 'pending' sale means the previous Square charge failed — the gift card // was already funded in the committed transaction, so re-attempt the @@ -119,7 +152,8 @@ func CreateTillSale(w http.ResponseWriter, r *http.Request) { var existingPendingGiftCard string if req.IdempotencyKey != "" { var existingID, existingStatus, existingItemID string - err := db.Conn.QueryRow(ctx, `SELECT id, status, item_id FROM till_sales WHERE idempotency_key = $1`, req.IdempotencyKey).Scan(&existingID, &existingStatus, &existingItemID) + var existingTotal float64 + err := db.Conn.QueryRow(ctx, `SELECT id, status, item_id, total_amount FROM till_sales WHERE idempotency_key = $1`, req.IdempotencyKey).Scan(&existingID, &existingStatus, &existingItemID, &existingTotal) if err == nil { if existingStatus == "completed" { if err := json.NewEncoder(w).Encode(TillSaleResponse{ @@ -134,6 +168,15 @@ func CreateTillSale(w http.ResponseWriter, r *http.Request) { return } if existingStatus == "pending" { + // Guard the amount: a retry with a different amount must not + // reuse the pending sale — the gift card was already funded at + // the old amount, so charging the new amount to Square would + // leave the card funded at the wrong value. + if int64(math.Round(existingTotal*100)) != int64(math.Round(req.Amount*100)) { + log.Printf("Till-sale retry amount mismatch: pending record %s has %.2f, request has %.2f", existingID, existingTotal, req.Amount) + http.Error(w, "Amount does not match the pending till sale", http.StatusBadRequest) + return + } existingPendingID = existingID existingPendingGiftCard = existingItemID } @@ -314,6 +357,19 @@ func CreateTillSale(w http.ResponseWriter, r *http.Request) { } } + // Method-switch guard on pending-retry: if the original attempt was + // card_machine and created a live terminal checkout, the retry MUST stay + // card_machine and reuse that checkout. Switching to cash/on_the_house/ + // saved_card/online_square would report the sale completed while the + // original checkout is still live — the customer could be charged at the + // terminal AND by the new method (double charge). The terminal checkout + // cannot be cancelled via this API, so reject the switch outright. + if existingPendingID != "" && existingPendingCheckoutID != "" && req.PaymentMethod != "card_machine" { + log.Printf("Till-sale retry rejected: pending sale %s has a live card-machine checkout, cannot switch method from card_machine to %s", existingPendingID, req.PaymentMethod) + http.Error(w, "This pending sale is tied to a live card-machine checkout — retry with card machine payment", http.StatusConflict) + return + } + switch req.PaymentMethod { case "cash": saleStatus = "completed" @@ -540,6 +596,25 @@ func CreateTillSale(w http.ResponseWriter, r *http.Request) { saleStatus = "completed" } + // Pending-reuse resolved by a non-Square method (cash / on_the_house): the + // original sale row was inserted as 'pending' (prior Square attempt failed + // or the method was switched after a failed charge). The reused row skips + // the INSERT, and no Square payment runs, so the status must be flipped to + // 'completed' explicitly — otherwise the row stays pending forever while + // the response claims success. + if existingPendingID != "" && !needsSquarePayment && req.PaymentMethod != "card_machine" { + _, upErr := db.Conn.Exec(ctx, + `UPDATE till_sales SET status = 'completed', updated_at = NOW() WHERE id = $1 AND status = 'pending'`, + tillSaleID, + ) + if upErr != nil { + log.Printf("Failed to complete pending till sale %s after non-Square payment: %v", tillSaleID, upErr) + http.Error(w, "internal server error", http.StatusInternalServerError) + return + } + saleStatus = "completed" + } + w.WriteHeader(http.StatusCreated) if err := json.NewEncoder(w).Encode(TillSaleResponse{ ID: tillSaleID, diff --git a/backend/handlers/payments/till_test.go b/backend/handlers/payments/till_test.go index c05e38e..5edfbaf 100644 --- a/backend/handlers/payments/till_test.go +++ b/backend/handlers/payments/till_test.go @@ -1332,3 +1332,230 @@ func TestCreateTillSale_PendingRetry_CardMachine_ReusesCheckout(t *testing.T) { t.Errorf("expected till_sales.square_checkout_id to remain %q, got %q", storedCheckoutID, rowCheckoutID) } } + +// TestCreateTillSale_PendingRetry_AmountMismatch_Rejected verifies that a +// same-key retry with a different amount is rejected (400) instead of reusing +// the pending sale — the gift card was already funded at the old amount, so +// charging a new amount would leave the card funded at the wrong value. +// Mirrors the tip/gift-card retry amount guard. +func TestCreateTillSale_PendingRetry_AmountMismatch_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) + } + adminToken := jwt.GenerateTestToken(adminID, "admin") + + // Seed a PENDING till_sale funded at £50.00. + key := "till-pending-amount-mismatch-key" + 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, user_id, idempotency_key, created_by, created_at, updated_at) + VALUES ('gift_card', $1, 'Gift Card create', 1, 50.00, 50.00, 'online_square', 'pending', + NULL, $2, $3, NOW(), NOW()) + `, giftCardID, key, adminID) + if err != nil { + t.Fatalf("failed to seed pending till sale: %v", err) + } + + // Retry with the same key but a different amount (£60 instead of £50). + reqBody := TillSaleRequest{ + ItemType: "gift_card", + Action: "create", + Amount: 60.00, + PaymentMethod: "saved_card", + 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.StatusBadRequest { + t.Fatalf("expected 400 (amount mismatch), got %d. body: %s", w.Code, w.Body.String()) + } + + // The pending sale must be untouched. + var saleStatus string + var saleAmount float64 + var saleCount int + err = tx.QueryRow(ctx, `SELECT COUNT(*), MAX(status), COALESCE(MAX(total_amount), 0) FROM till_sales WHERE idempotency_key = $1`, key).Scan(&saleCount, &saleStatus, &saleAmount) + if err != nil { + t.Fatalf("failed to query till sale: %v", err) + } + if saleCount != 1 { + t.Errorf("expected 1 till sale, got %d", saleCount) + } + if saleStatus != "pending" { + t.Errorf("expected pending sale to remain pending after rejected retry, got %s", saleStatus) + } + if saleAmount != 50.00 { + t.Errorf("expected sale amount to remain 50.00, got %.2f", saleAmount) + } +} + +// TestCreateTillSale_PendingRetry_Cash_CompletesRow verifies that a same-key +// retry resolved by cash on a pending sale explicitly flips the row to +// 'completed' — previously the response claimed success while the DB row stayed +// pending forever (unreconciled). +func TestCreateTillSale_PendingRetry_Cash_CompletesRow(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 till_sale (prior online_square attempt failed post-commit). + key := "till-pending-cash-retry-key" + 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, user_id, idempotency_key, created_by, created_at, updated_at) + VALUES ('gift_card', $1, 'Gift Card create', 1, 50.00, 50.00, 'online_square', 'pending', + NULL, $2, $3, NOW(), NOW()) + `, giftCardID, key, adminID) + if err != nil { + t.Fatalf("failed to seed pending till sale: %v", err) + } + + // Retry with the same key, same amount, cash payment. + reqBody := TillSaleRequest{ + ItemType: "gift_card", + Action: "create", + Amount: 50.00, + PaymentMethod: "cash", + 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.StatusOK && w.Code != http.StatusCreated { + t.Fatalf("expected 200/201, got %d. body: %s", w.Code, w.Body.String()) + } + + // The reused row must now be 'completed' — the cash retry explicitly flips it. + var saleStatus string + var saleCount int + err = tx.QueryRow(ctx, `SELECT COUNT(*), MAX(status) FROM till_sales WHERE idempotency_key = $1`, key).Scan(&saleCount, &saleStatus) + 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 saleStatus != "completed" { + t.Errorf("expected pending sale to be completed by cash retry, got %s", saleStatus) + } +} + +// TestCreateTillSale_PendingRetry_CardMachineSwitch_Rejected verifies that a +// pending card_machine sale with a live checkout cannot be retried via a +// different method — the original terminal checkout is still live and could +// complete, causing a double charge. +func TestCreateTillSale_PendingRetry_CardMachineSwitch_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) + } + adminToken := jwt.GenerateTestToken(adminID, "admin") + + // Seed a PENDING card_machine sale with a stored live checkout. + key := "till-pending-method-switch-key" + 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, user_id, 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', + NULL, 'chk_live_switch', $2, $3, NOW(), NOW()) + `, giftCardID, key, adminID) + if err != nil { + t.Fatalf("failed to seed pending card-machine till sale: %v", err) + } + + // Retry with the same key, same amount, but cash — must be rejected (409). + reqBody := TillSaleRequest{ + ItemType: "gift_card", + Action: "create", + Amount: 50.00, + PaymentMethod: "cash", + 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.StatusConflict { + t.Fatalf("expected 409 (method switch on live checkout), got %d. body: %s", w.Code, w.Body.String()) + } + + // The pending sale must be untouched. + var saleStatus string + var saleCount int + err = tx.QueryRow(ctx, `SELECT COUNT(*), MAX(status) FROM till_sales WHERE idempotency_key = $1`, key).Scan(&saleCount, &saleStatus) + if err != nil { + t.Fatalf("failed to query till sale: %v", err) + } + if saleCount != 1 { + t.Errorf("expected 1 till sale, got %d", saleCount) + } + if saleStatus != "pending" { + t.Errorf("expected pending sale to remain pending after rejected switch, got %s", saleStatus) + } +} diff --git a/backend/internal/square/square_http_client.go b/backend/internal/square/square_http_client.go index b1b4462..0cc8725 100644 --- a/backend/internal/square/square_http_client.go +++ b/backend/internal/square/square_http_client.go @@ -279,7 +279,10 @@ type sqDisableCardResponse struct { // --------------------------------------------------------------------------- func createPaymentHTTP(ctx context.Context, req CreatePaymentReq) (*PaymentResult, error) { - hc := newHTTPClient() + return createPaymentHTTPWithClient(ctx, req, newHTTPClient()) +} + +func createPaymentHTTPWithClient(ctx context.Context, req CreatePaymentReq, hc *httpClient) (*PaymentResult, error) { body := sqCreatePaymentRequest{ SourceID: req.SourceID, IdempotencyKey: req.IdempotencyKey, @@ -327,7 +330,10 @@ func createCheckoutHTTPWithClient(ctx context.Context, req CreateCheckoutReq, hc } func getCheckoutHTTP(ctx context.Context, checkoutID string) (*PaymentResult, error) { - hc := newHTTPClient() + return getCheckoutHTTPWithClient(ctx, checkoutID, newHTTPClient()) +} + +func getCheckoutHTTPWithClient(ctx context.Context, checkoutID string, hc *httpClient) (*PaymentResult, error) { var tcResp sqTerminalCheckoutResponse if err := hc.doJSON(ctx, http.MethodGet, "/v2/terminals/checkouts/"+checkoutID, nil, &tcResp); err != nil { return nil, err @@ -374,7 +380,10 @@ var definitiveRefundCodes = map[string]bool{ } func refundPaymentHTTP(ctx context.Context, req RefundPaymentReq) (*RefundResult, error) { - hc := newHTTPClient() + return refundPaymentHTTPWithClient(ctx, req, newHTTPClient()) +} + +func refundPaymentHTTPWithClient(ctx context.Context, req RefundPaymentReq, hc *httpClient) (*RefundResult, error) { body := sqRefundPaymentRequest{ PaymentID: req.PaymentID, IdempotencyKey: req.IdempotencyKey, @@ -396,7 +405,10 @@ func refundPaymentHTTP(ctx context.Context, req RefundPaymentReq) (*RefundResult } func listRefundsHTTP(ctx context.Context, paymentID string, beginTime time.Time) ([]RefundResult, error) { - hc := newHTTPClient() + return listRefundsHTTPWithClient(ctx, paymentID, beginTime, newHTTPClient()) +} + +func listRefundsHTTPWithClient(ctx context.Context, paymentID string, beginTime time.Time, hc *httpClient) ([]RefundResult, error) { base := "/v2/refunds?begin_time=" + url.QueryEscape(beginTime.UTC().Format(time.RFC3339)) + "&limit=100" path := base results := []RefundResult{} @@ -420,7 +432,10 @@ func listRefundsHTTP(ctx context.Context, paymentID string, beginTime time.Time) } func createCardOnFileHTTP(ctx context.Context, userID, cardToken string) (*CardOnFile, error) { - hc := newHTTPClient() + return createCardOnFileHTTPWithClient(ctx, userID, cardToken, newHTTPClient()) +} + +func createCardOnFileHTTPWithClient(ctx context.Context, userID, cardToken string, hc *httpClient) (*CardOnFile, error) { // Deterministic idempotency key derived from user + card (not time-based) // so that retries with the same details don't create duplicate cards. diff --git a/backend/internal/square/square_http_client_test.go b/backend/internal/square/square_http_client_test.go index 8f0c608..86c81a0 100644 --- a/backend/internal/square/square_http_client_test.go +++ b/backend/internal/square/square_http_client_test.go @@ -4,10 +4,15 @@ package square import ( "context" + "crypto/sha256" "encoding/json" + "errors" + "fmt" "net/http" "net/http/httptest" + "strings" "testing" + "time" ) func TestPaymentFromSquare_ElseBranch_SurfacesBrandWithoutCardID(t *testing.T) { @@ -100,3 +105,470 @@ func TestCreateCheckoutHTTP_DeviceOptionsWireShape(t *testing.T) { t.Errorf("expected checkout.device_options.device_id = dvc_test, got %v", devOpts) } } + +// TestDoJSON_ErrorParsing covers the doJSON error branches: structured Square +// errors become *squareAPIError (with Code/Detail preserved), while non-JSON +// error bodies fall back to a plain error (so refund classification treats the +// failure as ambiguous). +func TestDoJSON_ErrorParsing(t *testing.T) { + t.Run("structured_square_error_becomes_squareAPIError", func(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusBadRequest) + _, _ = w.Write([]byte(`{"errors":[{"category":"PAYMENT_METHOD_ERROR","code":"REFUND_DECLINED","detail":"The refund was declined","field":"payment_id"}]}`)) + })) + defer srv.Close() + + hc := &httpClient{baseURL: srv.URL, token: "test-token", http: srv.Client()} + err := hc.doJSON(context.Background(), http.MethodPost, "/v2/refunds", nil, nil) + if err == nil { + t.Fatal("expected error") + } + var sqErr *squareAPIError + if !errors.As(err, &sqErr) { + t.Fatalf("expected *squareAPIError, got %T", err) + } + if sqErr.Code != "REFUND_DECLINED" { + t.Errorf("expected code REFUND_DECLINED, got %s", sqErr.Code) + } + if sqErr.Detail != "The refund was declined" { + t.Errorf("expected detail, got %q", sqErr.Detail) + } + }) + + t.Run("non_json_error_body_is_plain_error", func(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "text/plain") + w.WriteHeader(http.StatusInternalServerError) + _, _ = w.Write([]byte("upstream blew up")) + })) + defer srv.Close() + + hc := &httpClient{baseURL: srv.URL, token: "test-token", http: srv.Client()} + err := hc.doJSON(context.Background(), http.MethodPost, "/v2/refunds", nil, nil) + if err == nil { + t.Fatal("expected error") + } + var sqErr *squareAPIError + if errors.As(err, &sqErr) { + t.Fatalf("expected plain error, got *squareAPIError with code %s", sqErr.Code) + } + if !strings.Contains(err.Error(), "HTTP 500") { + t.Errorf("expected HTTP 500 in error, got %q", err.Error()) + } + }) + + t.Run("http_3xx_is_treated_as_error", func(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusPermanentRedirect) + })) + defer srv.Close() + + hc := &httpClient{baseURL: srv.URL, token: "test-token", http: srv.Client()} + if err := hc.doJSON(context.Background(), http.MethodPost, "/v2/payments", nil, nil); err == nil { + t.Fatal("expected error for 3xx status") + } + }) + + t.Run("empty_token_returns_error_before_http", func(t *testing.T) { + hc := &httpClient{baseURL: "http://unused", token: "", http: &http.Client{}} + err := hc.doJSON(context.Background(), http.MethodPost, "/v2/payments", nil, nil) + if err == nil || !strings.Contains(err.Error(), "SQUARE_ACCESS_TOKEN is not set") { + t.Fatalf("expected token error, got %v", err) + } + }) +} + +// TestCreatePaymentHTTP_WireShape verifies the exact request body and headers +// sent to POST /v2/payments: source_id, idempotency_key, amount_money, +// location_id fallback, tip_money, and the auth/Square-Version headers. +func TestCreatePaymentHTTP_WireShape(t *testing.T) { + var captured map[string]any + var gotAuth, gotVersion, gotContentType string + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method != http.MethodPost { + t.Errorf("expected POST, got %s", r.Method) + } + if r.URL.Path != "/v2/payments" { + t.Errorf("expected /v2/payments, got %s", r.URL.Path) + } + gotAuth = r.Header.Get("Authorization") + gotVersion = r.Header.Get("Square-Version") + gotContentType = r.Header.Get("Content-Type") + if err := json.NewDecoder(r.Body).Decode(&captured); err != nil { + t.Errorf("failed to decode request body: %v", err) + } + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"payment":{"id":"pay_1","status":"COMPLETED","total_money":{"amount":5000,"currency":"GBP"},"source_type":"CARD","card_details":{"card":{"id":"ccof_x","card_brand":"VISA","last_4":"4242"},"entry_method":"ON_FILE"},"location_id":"loc_env","created_at":"2026-07-31T00:00:00Z","updated_at":"2026-07-31T00:00:00Z"}}`)) + })) + defer srv.Close() + + hc := &httpClient{baseURL: srv.URL, token: "secret-token", locationID: "loc_env", http: srv.Client()} + tip := int64(500) + res, err := createPaymentHTTPWithClient(context.Background(), CreatePaymentReq{ + Amount: 5000, + Currency: "GBP", + SourceID: "cnon:test-card", + IdempotencyKey: "ik-payment-1", + ReferenceID: "bk_123", + Note: "deposit", + TipMoney: &tip, + BuyerEmail: "buyer@example.com", + }, hc) + if err != nil { + t.Fatalf("createPaymentHTTP failed: %v", err) + } + + if gotAuth != "Bearer secret-token" { + t.Errorf("expected Authorization 'Bearer secret-token', got %q", gotAuth) + } + if gotVersion != squareAPIVersion { + t.Errorf("expected Square-Version %q, got %q", squareAPIVersion, gotVersion) + } + if gotContentType != "application/json" { + t.Errorf("expected Content-Type application/json, got %q", gotContentType) + } + + if captured["source_id"] != "cnon:test-card" { + t.Errorf("expected source_id cnon:test-card, got %v", captured["source_id"]) + } + if captured["idempotency_key"] != "ik-payment-1" { + t.Errorf("expected idempotency_key ik-payment-1, got %v", captured["idempotency_key"]) + } + if captured["location_id"] != "loc_env" { + t.Errorf("expected location_id loc_env (env fallback), got %v", captured["location_id"]) + } + amt, ok := captured["amount_money"].(map[string]any) + if !ok { + t.Fatalf("expected amount_money object, got %v", captured["amount_money"]) + } + if amt["amount"] != float64(5000) || amt["currency"] != "GBP" { + t.Errorf("expected amount_money {5000 GBP}, got %v", amt) + } + tipMoney, ok := captured["tip_money"].(map[string]any) + if !ok { + t.Fatalf("expected tip_money object, got %v", captured["tip_money"]) + } + if tipMoney["amount"] != float64(500) { + t.Errorf("expected tip_money amount 500, got %v", tipMoney["amount"]) + } + + if res.ID != "pay_1" || res.Amount != 5000 || res.CardBrand != "VISA" || res.CardLast4 != "4242" { + t.Errorf("unexpected payment result: %+v", res) + } +} + +// TestCreatePaymentHTTP_TipMoneyAbsentWhenNil verifies tip_money is omitted +// when not set (it's omitempty in the wire struct). +func TestCreatePaymentHTTP_TipMoneyAbsentWhenNil(t *testing.T) { + var captured map[string]any + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if err := json.NewDecoder(r.Body).Decode(&captured); err != nil { + t.Errorf("failed to decode request body: %v", err) + } + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"payment":{"id":"pay_2","status":"COMPLETED","total_money":{"amount":1000,"currency":"GBP"},"source_type":"CARD","card_details":{"card":{"id":"ccof_x","card_brand":"MASTERCARD","last_4":"4444"},"entry_method":"ON_FILE"},"location_id":"loc","created_at":"2026-07-31T00:00:00Z","updated_at":"2026-07-31T00:00:00Z"}}`)) + })) + defer srv.Close() + + hc := &httpClient{baseURL: srv.URL, token: "t", http: srv.Client()} + _, err := createPaymentHTTPWithClient(context.Background(), CreatePaymentReq{ + Amount: 1000, Currency: "GBP", SourceID: "ccof:existing", IdempotencyKey: "ik-2", + }, hc) + if err != nil { + t.Fatalf("createPaymentHTTP failed: %v", err) + } + if _, present := captured["tip_money"]; present { + t.Errorf("expected tip_money to be absent when nil, got %v", captured["tip_money"]) + } +} + +// TestRefundPaymentHTTP_CodeClassification verifies definitive refund rejection +// codes map to ErrRefundDeclined, PAYMENT_ALREADY_REFUNDED maps to +// ErrRefundAlreadyProcessed, and ambiguous errors pass through unwrapped. +func TestRefundPaymentHTTP_CodeClassification(t *testing.T) { + cases := []struct { + name string + code string + wantErrIs error // nil = no sentinel expected + wantErrNil bool + }{ + {name: "refund_declined", code: "REFUND_DECLINED", wantErrIs: ErrRefundDeclined}, + {name: "amount_exceeded", code: "PAYMENT_REFUND_AMOUNT_EXCEEDED", wantErrIs: ErrRefundDeclined}, + {name: "invalid_payment_id", code: "INVALID_PAYMENT_ID", wantErrIs: ErrRefundDeclined}, + {name: "already_refunded", code: "PAYMENT_ALREADY_REFUNDED", wantErrIs: ErrRefundAlreadyProcessed}, + {name: "ambiguous_code", code: "INTERNAL_SERVER_ERROR", wantErrIs: nil}, + {name: "ambiguous_non_json", code: "", wantErrIs: nil}, // raw text body + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusBadRequest) + if tc.code == "" { + _, _ = w.Write([]byte("plain text failure")) + } else { + _, _ = w.Write([]byte(fmt.Sprintf(`{"errors":[{"category":"PAYMENT_METHOD_ERROR","code":%q,"detail":"boom"}]}`, tc.code))) + } + })) + defer srv.Close() + + hc := &httpClient{baseURL: srv.URL, token: "t", http: srv.Client()} + _, err := refundPaymentHTTPWithClient(context.Background(), RefundPaymentReq{ + PaymentID: "pay_1", Amount: 1000, IdempotencyKey: "ik-refund", + }, hc) + if tc.wantErrNil { + if err != nil { + t.Fatalf("expected nil error, got %v", err) + } + return + } + if err == nil { + t.Fatal("expected error") + } + if tc.wantErrIs == nil { + if errors.Is(err, ErrRefundDeclined) || errors.Is(err, ErrRefundAlreadyProcessed) { + t.Fatalf("expected NO sentinel for ambiguous error, got %v", err) + } + return + } + if !errors.Is(err, tc.wantErrIs) { + t.Errorf("expected errors.Is(%v), got %v", tc.wantErrIs, err) + } + }) + } +} + +// TestRefundPaymentHTTP_WireShape verifies the refund request body and success +// response parsing. +func TestRefundPaymentHTTP_WireShape(t *testing.T) { + var captured map[string]any + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method != http.MethodPost { + t.Errorf("expected POST, got %s", r.Method) + } + if r.URL.Path != "/v2/refunds" { + t.Errorf("expected /v2/refunds, got %s", r.URL.Path) + } + if err := json.NewDecoder(r.Body).Decode(&captured); err != nil { + t.Errorf("failed to decode request body: %v", err) + } + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"refund":{"id":"ref_1","status":"COMPLETED","amount_money":{"amount":1000,"currency":"GBP"},"payment_id":"pay_1","location_id":"loc","reason":"cancellation","created_at":"2026-07-31T00:00:00Z"}}`)) + })) + defer srv.Close() + + hc := &httpClient{baseURL: srv.URL, token: "t", http: srv.Client()} + res, err := refundPaymentHTTPWithClient(context.Background(), RefundPaymentReq{ + PaymentID: "pay_1", Amount: 1000, IdempotencyKey: "ik-refund", Reason: "cancellation", + }, hc) + if err != nil { + t.Fatalf("refundPaymentHTTP failed: %v", err) + } + if captured["payment_id"] != "pay_1" { + t.Errorf("expected payment_id pay_1, got %v", captured["payment_id"]) + } + if captured["idempotency_key"] != "ik-refund" { + t.Errorf("expected idempotency_key ik-refund, got %v", captured["idempotency_key"]) + } + if res.ID != "ref_1" || res.Status != "COMPLETED" || res.Amount != 1000 || res.PaymentID != "pay_1" { + t.Errorf("unexpected refund result: %+v", res) + } +} + +// TestGetCheckoutHTTP_StatusBranches covers the checkout polling state machine: +// PENDING and IN_PROGRESS → ErrCheckoutPending, CANCELED → generic error, +// COMPLETED without payment IDs → generic error, COMPLETED with payment IDs → +// fetches the payment. +func TestGetCheckoutHTTP_StatusBranches(t *testing.T) { + t.Run("pending_returns_ErrCheckoutPending", func(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"checkout":{"id":"chk_1","status":"PENDING","amount_money":{"amount":5000,"currency":"GBP"},"created_at":"2026-07-31T00:00:00Z","updated_at":"2026-07-31T00:00:00Z"}}`)) + })) + defer srv.Close() + hc := &httpClient{baseURL: srv.URL, token: "t", http: srv.Client()} + _, err := getCheckoutHTTPWithClient(context.Background(), "chk_1", hc) + if !errors.Is(err, ErrCheckoutPending) { + t.Fatalf("expected ErrCheckoutPending, got %v", err) + } + }) + + t.Run("in_progress_returns_ErrCheckoutPending", func(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"checkout":{"id":"chk_2","status":"IN_PROGRESS","amount_money":{"amount":5000,"currency":"GBP"},"created_at":"2026-07-31T00:00:00Z","updated_at":"2026-07-31T00:00:00Z"}}`)) + })) + defer srv.Close() + hc := &httpClient{baseURL: srv.URL, token: "t", http: srv.Client()} + _, err := getCheckoutHTTPWithClient(context.Background(), "chk_2", hc) + if !errors.Is(err, ErrCheckoutPending) { + t.Fatalf("expected ErrCheckoutPending, got %v", err) + } + }) + + t.Run("cancelled_is_generic_error", func(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"checkout":{"id":"chk_3","status":"CANCELED","amount_money":{"amount":5000,"currency":"GBP"},"created_at":"2026-07-31T00:00:00Z","updated_at":"2026-07-31T00:00:00Z"}}`)) + })) + defer srv.Close() + hc := &httpClient{baseURL: srv.URL, token: "t", http: srv.Client()} + _, err := getCheckoutHTTPWithClient(context.Background(), "chk_3", hc) + if err == nil || errors.Is(err, ErrCheckoutPending) { + t.Fatalf("expected generic non-pending error, got %v", err) + } + if !strings.Contains(err.Error(), "CANCELED") { + t.Errorf("expected status in error, got %q", err.Error()) + } + }) + + t.Run("completed_without_payment_ids_is_error", func(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"checkout":{"id":"chk_4","status":"COMPLETED","amount_money":{"amount":5000,"currency":"GBP"},"created_at":"2026-07-31T00:00:00Z","updated_at":"2026-07-31T00:00:00Z"}}`)) + })) + defer srv.Close() + hc := &httpClient{baseURL: srv.URL, token: "t", http: srv.Client()} + _, err := getCheckoutHTTPWithClient(context.Background(), "chk_4", hc) + if err == nil || !strings.Contains(err.Error(), "no payment IDs") { + t.Fatalf("expected 'no payment IDs' error, got %v", err) + } + }) + + t.Run("completed_fetches_payment", func(t *testing.T) { + var checkoutPath, paymentPath string + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + switch { + case r.URL.Path == "/v2/terminals/checkouts/chk_5": + checkoutPath = r.URL.Path + _, _ = w.Write([]byte(`{"checkout":{"id":"chk_5","status":"COMPLETED","amount_money":{"amount":5000,"currency":"GBP"},"payment_ids":["pay_5"],"created_at":"2026-07-31T00:00:00Z","updated_at":"2026-07-31T00:00:00Z"}}`)) + case r.URL.Path == "/v2/payments/pay_5": + paymentPath = r.URL.Path + _, _ = w.Write([]byte(`{"payment":{"id":"pay_5","status":"COMPLETED","total_money":{"amount":5000,"currency":"GBP"},"source_type":"CARD","card_details":{"card":{"id":"ccof_x","card_brand":"VISA","last_4":"4242"},"entry_method":"EMV"},"location_id":"loc","created_at":"2026-07-31T00:00:00Z","updated_at":"2026-07-31T00:00:00Z"}}`)) + default: + t.Errorf("unexpected path %s", r.URL.Path) + } + })) + defer srv.Close() + hc := &httpClient{baseURL: srv.URL, token: "t", http: srv.Client()} + res, err := getCheckoutHTTPWithClient(context.Background(), "chk_5", hc) + if err != nil { + t.Fatalf("getCheckoutHTTP failed: %v", err) + } + if checkoutPath == "" || paymentPath == "" { + t.Fatal("expected both checkout and payment fetches") + } + if res.ID != "pay_5" || res.Amount != 5000 || res.EntryMethod != "EMV" { + t.Errorf("unexpected payment result: %+v", res) + } + }) +} + +// TestListRefundsHTTP_Pagination verifies cursor-based pagination: multiple +// pages are fetched, filtered by paymentID, and the 20-page guard triggers. +func TestListRefundsHTTP_Pagination(t *testing.T) { + t.Run("two_pages_combined_and_filtered", func(t *testing.T) { + var paths []string + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + paths = append(paths, r.URL.String()) + w.Header().Set("Content-Type", "application/json") + if strings.Contains(r.URL.RawQuery, "cursor=page2") { + _, _ = w.Write([]byte(`{"refunds":[{"id":"ref_3","status":"COMPLETED","amount_money":{"amount":1000,"currency":"GBP"},"payment_id":"pay_target","created_at":"2026-07-31T00:00:00Z"},{"id":"ref_4","status":"COMPLETED","amount_money":{"amount":2000,"currency":"GBP"},"payment_id":"pay_other","created_at":"2026-07-31T00:00:00Z"}],"cursor":""}`)) + return + } + _, _ = w.Write([]byte(`{"refunds":[{"id":"ref_1","status":"COMPLETED","amount_money":{"amount":500,"currency":"GBP"},"payment_id":"pay_target","created_at":"2026-07-31T00:00:00Z"},{"id":"ref_2","status":"COMPLETED","amount_money":{"amount":900,"currency":"GBP"},"payment_id":"pay_other","created_at":"2026-07-31T00:00:00Z"}],"cursor":"page2"}`)) + })) + defer srv.Close() + + hc := &httpClient{baseURL: srv.URL, token: "t", http: srv.Client()} + begin := time.Date(2026, 7, 1, 0, 0, 0, 0, time.UTC) + refunds, err := listRefundsHTTPWithClient(context.Background(), "pay_target", begin, hc) + if err != nil { + t.Fatalf("listRefundsHTTP failed: %v", err) + } + if len(paths) != 2 { + t.Fatalf("expected 2 pages, got %d: %v", len(paths), paths) + } + if !strings.Contains(paths[0], "begin_time=") { + t.Errorf("expected begin_time in first request, got %q", paths[0]) + } + if !strings.Contains(paths[0], "limit=100") { + t.Errorf("expected limit=100 in first request, got %q", paths[0]) + } + // Only pay_target refunds survive the client-side filter. + if len(refunds) != 2 { + t.Fatalf("expected 2 filtered refunds (ref_1 + ref_3), got %d: %+v", len(refunds), refunds) + } + }) + + t.Run("page_guard_triggers_after_20_pages", func(t *testing.T) { + calls := 0 + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + calls++ + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"refunds":[],"cursor":"next"}`)) + })) + defer srv.Close() + + hc := &httpClient{baseURL: srv.URL, token: "t", http: srv.Client()} + _, err := listRefundsHTTPWithClient(context.Background(), "pay_x", time.Now(), hc) + if err == nil || !strings.Contains(err.Error(), "exceeded 20 pages") { + t.Fatalf("expected 20-page guard error, got %v", err) + } + if calls != 20 { + t.Errorf("expected exactly 20 HTTP calls before guard, got %d", calls) + } + }) +} + +// TestCreateCardOnFileHTTP_IdempotencyKey verifies the deterministic SHA-256 +// idempotency key derivation and the request wire shape (source_id + card). +func TestCreateCardOnFileHTTP_IdempotencyKey(t *testing.T) { + var captured map[string]any + var gotAuth string + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method != http.MethodPost { + t.Errorf("expected POST, got %s", r.Method) + } + if r.URL.Path != "/v2/cards" { + t.Errorf("expected /v2/cards, got %s", r.URL.Path) + } + gotAuth = r.Header.Get("Authorization") + if err := json.NewDecoder(r.Body).Decode(&captured); err != nil { + t.Errorf("failed to decode request body: %v", err) + } + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"card":{"id":"ccof_x","card_brand":"VISA","last_4":"4242","exp_month":12,"exp_year":2030,"fingerprint":"fp1","customer_id":"user_1","enabled":true,"version":1,"created_at":"2026-07-31T00:00:00Z"}}`)) + })) + defer srv.Close() + + hc := &httpClient{baseURL: srv.URL, token: "secret", http: srv.Client()} + res, err := createCardOnFileHTTPWithClient(context.Background(), "user_1", "cnon:test-card", hc) + if err != nil { + t.Fatalf("createCardOnFileHTTP failed: %v", err) + } + + sum := sha256.Sum256([]byte("user_1|cnon:test-card")) + wantIK := fmt.Sprintf("create-card-%x", sum) + if captured["idempotency_key"] != wantIK { + t.Errorf("expected idempotency_key %q, got %v", wantIK, captured["idempotency_key"]) + } + if captured["source_id"] != "cnon:test-card" { + t.Errorf("expected source_id cnon:test-card, got %v", captured["source_id"]) + } + card, ok := captured["card"].(map[string]any) + if !ok { + t.Fatalf("expected card object, got %v", captured["card"]) + } + if card["customer_id"] != "user_1" { + t.Errorf("expected card.customer_id user_1, got %v", card["customer_id"]) + } + if gotAuth != "Bearer secret" { + t.Errorf("expected Authorization 'Bearer secret', got %q", gotAuth) + } + if res.CardID != "ccof_x" || res.Brand != "VISA" || res.Last4 != "4242" { + t.Errorf("unexpected card result: %+v", res) + } +} diff --git a/backend/main.go b/backend/main.go index 5ce063b..0db3fd7 100644 --- a/backend/main.go +++ b/backend/main.go @@ -119,7 +119,11 @@ func initS3() { func initSquare() { payments.SquareClient = square.NewClient() - fmt.Println("Square client initialized (dev mock)") + if env := os.Getenv("SQUARE_ENVIRONMENT"); env == "sandbox" || env == "production" { + fmt.Printf("Square client initialized (%s, real API)\n", env) + } else { + fmt.Println("Square client initialized (dev mock)") + } } func healthCheckHandler(w http.ResponseWriter, r *http.Request) { @@ -128,7 +132,7 @@ func healthCheckHandler(w http.ResponseWriter, r *http.Request) { "backend": "ok", "database": "ok", "s3_storage": "ok", - "square_payments": "not_implemented", + "square_payments": "ok", "frontend": "unknown", } @@ -146,6 +150,10 @@ func healthCheckHandler(w http.ResponseWriter, r *http.Request) { services["s3_storage"] = "not_configured" } + if env := os.Getenv("SQUARE_ENVIRONMENT"); env == "" || env == "mock" { + services["square_payments"] = "mock" + } + if status == "degraded" { w.WriteHeader(http.StatusServiceUnavailable) } else { diff --git a/frontend/src/lib/components/account/UserBookingModal.svelte b/frontend/src/lib/components/account/UserBookingModal.svelte index 0452052..351feb4 100644 --- a/frontend/src/lib/components/account/UserBookingModal.svelte +++ b/frontend/src/lib/components/account/UserBookingModal.svelte @@ -15,10 +15,7 @@ import { computeBalanceDue } from '$lib/utils/booking'; import { parseWallClockDate } from '$lib/utils/timeSlots'; import type { Booking, BookingDiscount, Payment } from '$lib/types/booking'; - import CardBrandIcon from '$lib/components/payments/CardBrandIcon.svelte'; - import CardEntryUnavailable from '$lib/components/payments/CardEntryUnavailable.svelte'; - import SquareCardInput from '$lib/components/payments/SquareCardInput.svelte'; - import { isSquareConfigured } from '$lib/square/square'; + import CardSelection from '$lib/components/payments/CardSelection.svelte'; import { savedCardsStore, type SavedCard } from '$lib/stores/savedCards.svelte'; interface Props { open: boolean; @@ -147,13 +144,13 @@ let tipIdempotencyKey = $state(''); let tipKeyedAmount = $state(0); - // Card selection state for tips + // Card selection for tips — delegated to CardSelection.svelte. let tipSavedCards = $state([]); let tipLoadingCards = $state(false); - let tipSelectedCardId = $state(null); - let tipShowNewCardForm = $state(false); - let tipSquareCardReady = $state(false); - let tipSquareCardInput = $state(null); + let tipCardSelection = $state(null); + let tipSelectedCardId = $state(''); + let tipCardSelectionValid = $state(false); + let tipSaveCard = $state(false); // Cached nonce: tokenization is one-shot — a retry reuses this token instead // of re-tokenizing (the backend idempotency key dedups). let tipNonce = $state(''); @@ -162,9 +159,7 @@ authStore.currentUser?.role === 'verified_email' || authStore.currentUser?.role === 'affiliate' ); - const isTipCardValid = $derived( - tipSelectedCardId !== null || (tipShowNewCardForm && tipSquareCardReady) - ); + const isTipCardValid = $derived(tipCardSelectionValid); const tipPresets = $derived( selectedBooking @@ -233,11 +228,11 @@ let newCardToken: string | undefined; if (tipSelectedCardId) { // saved card — nothing to tokenize - } else if (tipShowNewCardForm && tipSquareCardInput) { + } else if (tipCardSelection) { // New-card mode: tokenize once per attempt, reuse the nonce on retry. if (!tipNonce) { try { - tipNonce = await tipSquareCardInput.tokenize(); + tipNonce = await tipCardSelection.tokenize(); } catch (err) { toast.error(err instanceof Error ? err.message : 'Card entry failed'); return; @@ -260,7 +255,7 @@ amount: Math.round(tipAmount * 100), idempotency_key: tipIdempotencyKey, ...(tipSelectedCardId ? { card_id: tipSelectedCardId } : {}), - ...(newCardToken ? { new_card_token: newCardToken, save_card: false } : {}) + ...(newCardToken ? { new_card_token: newCardToken, save_card: tipSaveCard } : {}) }; const response = await apiFetch(`/api/bookings/${selectedBooking.id}/tip`, { @@ -1112,8 +1107,8 @@ ${hasVAT ? `

VAT is included at ${biz?.default_vat_rate ?? 20} customTipInput = ''; tipIdempotencyKey = ''; tipKeyedAmount = 0; - tipSelectedCardId = null; - tipShowNewCardForm = false; + tipSelectedCardId = ''; + tipSaveCard = false; tipNonce = ''; } }} @@ -1169,79 +1164,15 @@ ${hasVAT ? `

VAT is included at ${biz?.default_vat_rate ?? 20} {#if tipLoadingCards}

Loading payment methods...
- {:else if tipSavedCards.length > 0} -
- {#each tipSavedCards as card (card.id)} - - {/each} - -
{:else} - {#if isSquareConfigured()} - (tipSquareCardReady = r)} - /> - {:else} - - {/if} - {/if} - - {#if tipSavedCards.length > 0 && tipShowNewCardForm} - {#if isSquareConfigured()} - (tipSquareCardReady = r)} - /> - {:else} - - {/if} + (tipCardSelectionValid = v)} + /> {/if} diff --git a/frontend/src/lib/components/booking/BookingFlow.svelte b/frontend/src/lib/components/booking/BookingFlow.svelte index 2594cc7..c90b7f0 100644 --- a/frontend/src/lib/components/booking/BookingFlow.svelte +++ b/frontend/src/lib/components/booking/BookingFlow.svelte @@ -36,9 +36,7 @@ import DatePicker from '$lib/components/booking/DatePicker.svelte'; import TimeSlotPicker from '$lib/components/booking/TimeSlotPicker.svelte'; import ServiceSelector from '$lib/components/booking/ServiceSelector.svelte'; - import CardEntryUnavailable from '$lib/components/payments/CardEntryUnavailable.svelte'; - import SquareCardInput from '$lib/components/payments/SquareCardInput.svelte'; - import { isSquareConfigured } from '$lib/square/square'; + import CardSelection from '$lib/components/payments/CardSelection.svelte'; import PolicyPopover from '$lib/components/ui/policyPopover.svelte'; import { POLICY } from '$lib/constants/policy'; import UserPaymentModal from '$lib/components/payments/UserPaymentModal.svelte'; @@ -84,16 +82,24 @@ // =============== Payment State =============== let userDepositsRequired = $state(0); let hasActiveBooking = $state(false); + // Saved cards, in the API shape (last_4/exp_month/exp_year) — consumed by + // CardSelection.svelte which renders the list, new-card toggle, and consent. let paymentMethods = $state< - Array<{ id: string; brand: string; last4: string; expiry_month: number; expiry_year: number }> + Array<{ + id: string; + brand: string; + last_4: string; + exp_month: number; + exp_year: number; + is_default?: boolean; + }> >([]); let paymentMethodsLoading = $state(false); - let selectedPaymentMethod = $state(null); + let selectedPaymentMethod = $state(''); + let paymentCardSelection = $state(null); + let paymentCardSelectionValid = $state(false); + let depositSaveCard = $state(false); let isProcessingPayment = $state(false); - // New-card mode (Square Web Payments tokenization) - let showNewCardForm = $state(false); - let squareCardReady = $state(false); - let squareCardInput = $state(null); // Cached nonce: tokenization is one-shot — a retry reuses this token instead // of re-tokenizing (the backend idempotency key dedups). let depositNonce = $state(''); @@ -106,15 +112,11 @@ // Payment flow state let depositPaid = $state(false); - // New-card form is active when toggled, or implicitly when there is no saved - // card to pick (guest flow / no saved cards yet). - const newCardMode = $derived( - showNewCardForm || !authStore.isAuthenticated || paymentMethods.length === 0 + const canSaveCards = $derived( + authStore.currentUser?.role === 'verified_email' || authStore.currentUser?.role === 'affiliate' ); - const depositCardFormValid = $derived( - selectedPaymentMethod !== null || (newCardMode && squareCardReady) - ); + const depositCardFormValid = $derived(paymentCardSelectionValid); // VAT registration status from public business info (via shared store) const vatRegistered = $derived(getBusinessInfo()?.is_vat_registered ?? false); @@ -275,7 +277,7 @@ } } - async function processPayment(amount: number) { + 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 // propagated to the button. @@ -287,11 +289,11 @@ let newCardToken: string | undefined; if (selectedPaymentMethod) { // saved card — nothing to tokenize - } else if (newCardMode && squareCardInput) { + } else if (paymentCardSelection) { // New-card mode: tokenize once per attempt, reuse the nonce on retry. if (!depositNonce) { try { - depositNonce = await squareCardInput.tokenize(); + depositNonce = await paymentCardSelection.tokenize(); } catch (err) { toast.error(err instanceof Error ? err.message : 'Card entry failed'); return; @@ -308,11 +310,19 @@ return; } const bookingId = confirmedBooking.id; - const amountCents = Math.round(amount * 100); + // Charge the SERVER-computed deposit: the booking response carries the + // authoritative deposit_amount (20% of the server-side total, which + // accounts for discounts / admin adjustments). The client-side + // getTotalPrice()*0.2 estimate can diverge and under-charge. + const depositAmount = + confirmedBooking.deposit_amount && confirmedBooking.deposit_amount > 0 + ? confirmedBooking.deposit_amount + : _amount; + const amountCents = Math.round(depositAmount * 100); // Cache the idempotency key per amount+card so a lost-response retry // reuses it (backend dedups) instead of double-charging. - const cardKey = selectedPaymentMethod ?? `new:${newCardToken ?? ''}`; + const cardKey = selectedPaymentMethod || `new:${newCardToken ?? ''}`; if ( !depositIdempotencyKey || depositKeyedAmount !== amountCents || @@ -328,7 +338,7 @@ amount: amountCents, idempotency_key: depositIdempotencyKey, ...(selectedPaymentMethod ? { card_id: selectedPaymentMethod } : {}), - ...(newCardToken ? { new_card_token: newCardToken, save_card: false } : {}) + ...(newCardToken ? { new_card_token: newCardToken, save_card: depositSaveCard } : {}) }; paymentAttempted = true; @@ -348,6 +358,7 @@ depositKeyedAmount = 0; depositKeyedCard = ''; depositNonce = ''; + depositSaveCard = false; // Immutable update — avoid mutating the existing object so // concurrent renders (e.g. a stale fetch) can't observe partial // state. (See audit: HIGH issue #3 — confirmedBooking mutated @@ -355,8 +366,8 @@ confirmedBooking = { ...confirmedBooking, deposit_paid: true, - amount_paid: (confirmedBooking.amount_paid || 0) + amount, - amount_due: Math.max(0, (confirmedBooking.amount_due || 0) - amount) + amount_paid: (confirmedBooking.amount_paid || 0) + depositAmount, + amount_due: Math.max(0, (confirmedBooking.amount_due || 0) - depositAmount) }; toast.success('Payment successful!'); } else { @@ -382,8 +393,21 @@ let depositKeyedAmount = $state(0); let depositKeyedCard = $state(''); - function formatCardExpiry(month: number, year: number): string { - return `${String(month).padStart(2, '0')}/${year.toString().slice(-2)}`; + async function fetchPaymentMethods() { + if (paymentMethodsLoading || paymentMethods.length > 0) return; + if (!authStore.isAuthenticated) return; + paymentMethodsLoading = true; + try { + const response = await apiFetch('/api/user/payment-methods'); + if (response.ok) { + // CardSelection auto-selects the default card once cards load. + paymentMethods = await response.json(); + } + } catch { + // non-fatal — the new-card form remains available + } finally { + paymentMethodsLoading = false; + } } // Fetch user deposit and active booking status when step 1 is reached @@ -394,6 +418,13 @@ } }); + // Fetch saved cards when the deposit payment step is shown. + $effect(() => { + if (currentStep === finalStep && depositRequired && authStore.isAuthenticated) { + fetchPaymentMethods(); + } + }); + // =============== Slot Reservation System =============== let _reservationId = $state(null); let reservationExpiresAt = $state(null); @@ -2296,88 +2327,28 @@ {#if authStore.isAuthenticated} {#if paymentMethodsLoading}
Loading payment methods...
- {:else if paymentMethods.length > 0} + {:else}
-

Saved Cards

-
- {#each paymentMethods as method (method.id)} -
-
-
- {method.brand} -
-
- **** {method.last4} - - {formatCardExpiry(method.expiry_month, method.expiry_year)} - -
-
- -
- {/each} - -
-
- {/if} - - {#if newCardMode} -
- {#if isSquareConfigured()} - (squareCardReady = r)} - /> - {:else} - - {/if} + (paymentCardSelectionValid = v)} + />
{/if} {:else}
- {#if isSquareConfigured()} - (squareCardReady = r)} - /> - {:else} - - {/if} + (paymentCardSelectionValid = v)} + />
{/if} diff --git a/frontend/src/lib/components/payments/CardSelection.svelte b/frontend/src/lib/components/payments/CardSelection.svelte index 009d47a..3818bb2 100644 --- a/frontend/src/lib/components/payments/CardSelection.svelte +++ b/frontend/src/lib/components/payments/CardSelection.svelte @@ -15,13 +15,15 @@ let { cards = [], - canSaveCards: _canSaveCards = false, + canSaveCards = false, selectedCardId = $bindable(''), + saveCard = $bindable(false), onValidityChange = (_valid: boolean) => {} }: { cards?: SelectableCard[]; canSaveCards?: boolean; selectedCardId?: string; + saveCard?: boolean; onValidityChange?: (valid: boolean) => void; } = $props(); @@ -30,6 +32,10 @@ let showNewCardForm = $state(false); let squareCardReady = $state(false); let squareCardInput = $state(null); + // Unique per instance: a plain counter would be instance-scoped in Svelte 5 + // (every instance restarting at 0), so two mounted CardSelection instances + // would collide on the same checkbox id. Pure SPA, so no SSR concern. + const consentId = `save-card-consent-${crypto.randomUUID()}`; // Auto-select the default saved card when cards first load. Guarded by // !showNewCardForm so the "Use a new card" click (selectedCardId = '') is @@ -125,5 +131,20 @@ {:else} {/if} + + {#if canSaveCards && squareCardReady} + + {/if} {/if} diff --git a/frontend/src/lib/components/payments/SquareCardInput.svelte b/frontend/src/lib/components/payments/SquareCardInput.svelte index 4321fd9..4fff33d 100644 --- a/frontend/src/lib/components/payments/SquareCardInput.svelte +++ b/frontend/src/lib/components/payments/SquareCardInput.svelte @@ -3,10 +3,6 @@ import CardEntryUnavailable from './CardEntryUnavailable.svelte'; import { getSquarePayments, isSquareConfigured } from '$lib/square/square'; - // Module-level counter keeps the element id stable across server/client render - // (a Math.random() id would mismatch during hydration). - let squareCardIdCounter = 0; - interface Props { /** Disable the form while a payment is processing. */ disabled?: boolean; @@ -21,7 +17,10 @@ let ready = $state(false); let initError = $state(null); - let uniqueId = $state(`square-card-${squareCardIdCounter++}`); + // Unique per instance so two mounted card forms never share an element id + // (e.g. the deposit step + the pay-early modal on the same page). This app + // is a pure SPA (no SSR/hydration), so a random id cannot mismatch. + let uniqueId = $state(`square-card-${crypto.randomUUID()}`); async function init() { if (!isSquareConfigured()) { diff --git a/frontend/src/lib/components/payments/UserPaymentModal.svelte b/frontend/src/lib/components/payments/UserPaymentModal.svelte index b94665d..3663aa5 100644 --- a/frontend/src/lib/components/payments/UserPaymentModal.svelte +++ b/frontend/src/lib/components/payments/UserPaymentModal.svelte @@ -23,7 +23,17 @@ defaultPaymentType?: 'full' | 'partial' | 'deposit'; } - const { booking, onClose, onComplete, canSaveCards = true, defaultPaymentType }: Props = $props(); + const { + booking, + onClose, + onComplete, + canSaveCards = false, + defaultPaymentType + }: Props = $props(); + + // Explicit consent: whether the new card is saved for next time. Toggled by + // the checkbox inside CardSelection; defaults to false (opt-in). + let saveCard = $state(false); type PaymentStatus = 'idle' | 'processing' | 'polling' | 'success' | 'error'; @@ -394,7 +404,7 @@ amount: amountCents, payment_type: paymentType, ...(cardId ? { card_id: cardId } : {}), - ...(newCardToken ? { new_card_token: newCardToken, save_card: canSaveCards } : {}), + ...(newCardToken ? { new_card_token: newCardToken, save_card: saveCard } : {}), idempotency_key: payIdempotencyKey }) }); @@ -687,8 +697,9 @@ - - {#if status === 'idle' && authStore.isAuthenticated} + + {#if (status === 'idle' || status === 'error') && authStore.isAuthenticated} {#if paymentMethodsLoading}
Loading payment methods...
{:else} @@ -697,6 +708,7 @@ cards={paymentMethods} {canSaveCards} bind:selectedCardId + bind:saveCard onValidityChange={(v) => (cardSelectionValid = v)} /> {/if} @@ -849,6 +861,17 @@ {#if status === 'error' && error}

{error}

+
{/if} diff --git a/frontend/src/routes/account/+page.svelte b/frontend/src/routes/account/+page.svelte index 660eb14..baf1b9f 100644 --- a/frontend/src/routes/account/+page.svelte +++ b/frontend/src/routes/account/+page.svelte @@ -6,6 +6,7 @@ import { toast } from 'svelte-sonner'; import CardBrandIcon from '$lib/components/payments/CardBrandIcon.svelte'; import CardEntryUnavailable from '$lib/components/payments/CardEntryUnavailable.svelte'; + import CardSelection from '$lib/components/payments/CardSelection.svelte'; import SquareCardInput from '$lib/components/payments/SquareCardInput.svelte'; import { isSquareConfigured } from '$lib/square/square'; import { extractErrorMessage, sanitizeText } from '$lib/utils/toast-safe'; @@ -205,9 +206,9 @@ let buySelectedCard = $state(''); let buyingGiftCard = $state(false); let purchaseResultCode = $state(null); - let buyShowNewCardForm = $state(false); - let buySquareCardReady = $state(false); - let buySquareCardInput = $state(null); + let buyCardSelection = $state(null); + let buyCardSelectionValid = $state(false); + let buySaveCard = $state(false); // Cached nonce: tokenization is one-shot — a retry reuses this token instead // of re-tokenizing (the backend idempotency key dedups). let buyNonce = $state(''); @@ -219,21 +220,8 @@ let buyKeyedAmount = $state(0); let buyKeyedCard = $state(''); - $effect(() => { - // Auto-select the default saved card when cards first load. When the - // "Use a new card" toggle is open, selectedCardId is cleared so the - // effect doesn't override the user's choice. - if (savedCardsStore.cards.length > 0 && !buySelectedCard && !buyShowNewCardForm) { - const defaultCard = - savedCardsStore.cards.find((c) => c.is_default) || savedCardsStore.cards[0]; - buySelectedCard = defaultCard.id; - } - }); - - // Derived validation for Buy Gift Card form - const isBuyCardValid = $derived( - buySelectedCard !== '' || (buyShowNewCardForm && buySquareCardReady) - ); + // Derived validation for Buy Gift Card form — delegated to CardSelection. + const isBuyCardValid = $derived(buyCardSelectionValid); async function fetchGiftCardBalance() { loadingBalance = true; @@ -283,11 +271,11 @@ let newCardToken: string | undefined; if (buySelectedCard) { // saved card — nothing to tokenize - } else if (buyShowNewCardForm && buySquareCardInput) { + } else if (buyCardSelection) { // New-card mode: tokenize once per attempt, reuse the nonce on retry. if (!buyNonce) { try { - buyNonce = await buySquareCardInput.tokenize(); + buyNonce = await buyCardSelection.tokenize(); } catch (err) { toast.error(err instanceof Error ? err.message : 'Card entry failed'); buyingGiftCard = false; @@ -308,7 +296,7 @@ // Cache the idempotency key per amount+card so a lost-response retry // reuses the same key (backend dedups) instead of double-charging. // Regenerate when the amount or card changes. - const cardKey = cardId ?? `new:${newCardToken ?? ''}`; + const cardKey = cardId || `new:${newCardToken ?? ''}`; if (!buyIdempotencyKey || buyKeyedAmount !== buyAmount || buyKeyedCard !== cardKey) { buyIdempotencyKey = generateIdempotencyKey(); buyKeyedAmount = buyAmount; @@ -323,7 +311,7 @@ recipient_type: buyRecipientType, recipient_email: buyRecipientEmail, ...(cardId ? { card_id: cardId } : {}), - ...(newCardToken ? { new_card_token: newCardToken, save_card: false } : {}), + ...(newCardToken ? { new_card_token: newCardToken, save_card: buySaveCard } : {}), idempotency_key: buyIdempotencyKey }) }); @@ -2179,73 +2167,14 @@ Payment Method - {#if savedCardsStore.cards.length > 0} -
- {#each savedCardsStore.cards as card (card.id)} - - {/each} - -
- {/if} - - {#if buyShowNewCardForm || savedCardsStore.cards.length === 0} - {#if isSquareConfigured()} - (buySquareCardReady = r)} - /> - {:else} - - {/if} - {/if} + (buyCardSelectionValid = v)} + /> - {/each} - - - - {:else} -
- - Payment Method - - {#if isSquareConfigured()} - (squareCardReady = r)} - /> - {:else} - - {/if} -
- {/if} - - {#if savedCards.length > 0 && showNewCardForm} - {#if isSquareConfigured()} - (squareCardReady = r)} /> - {:else} - - {/if} - {/if} + (cardSelectionValid = v)} + /> diff --git a/frontend/src/routes/tip/+page.svelte b/frontend/src/routes/tip/+page.svelte index 25dee11..0bcef46 100644 --- a/frontend/src/routes/tip/+page.svelte +++ b/frontend/src/routes/tip/+page.svelte @@ -6,10 +6,7 @@ import { SvelteDate } from 'svelte/reactivity'; import { toast } from 'svelte-sonner'; import { extractErrorMessage } from '$lib/utils/toast-safe'; - import CardBrandIcon from '$lib/components/payments/CardBrandIcon.svelte'; - import CardEntryUnavailable from '$lib/components/payments/CardEntryUnavailable.svelte'; - import SquareCardInput from '$lib/components/payments/SquareCardInput.svelte'; - import { isSquareConfigured } from '$lib/square/square'; + import CardSelection from '$lib/components/payments/CardSelection.svelte'; import { savedCardsStore, type SavedCard } from '$lib/stores/savedCards.svelte'; import { Button } from '$lib/components/ui/button'; import { Input } from '$lib/components/ui/input'; @@ -58,17 +55,22 @@ let tipIdempotencyKey = $state(''); let tipKeyedAmount = $state(0); - // Card selection state (same pattern as UserPaymentModal) + // Card selection — delegated to CardSelection.svelte (saved-card list, + // "Use a new card" toggle, SquareCardInput tokenization, consent checkbox). let savedCards = $state([]); - let selectedCardId = $state(null); - let showNewCardForm = $state(false); - let squareCardReady = $state(false); - let squareCardInput = $state(null); + let cardSelection = $state(null); + let selectedCardId = $state(''); + let cardSelectionValid = $state(false); + let saveCard = $state(false); // Cached nonce: tokenization is one-shot — a retry reuses this token instead // of re-tokenizing (the backend idempotency key dedups). let tipNonce = $state(''); - const isCardValid = $derived(selectedCardId !== null || (showNewCardForm && squareCardReady)); + const canSaveCards = $derived( + authStore.currentUser?.role === 'verified_email' || authStore.currentUser?.role === 'affiliate' + ); + + const isCardValid = $derived(cardSelectionValid); let selectedTip = $state(null); let customTip = $state(''); @@ -153,11 +155,11 @@ let newCardToken: string | undefined; if (selectedCardId) { // saved card — nothing to tokenize - } else if (showNewCardForm && squareCardInput) { + } else if (cardSelection) { // New-card mode: tokenize once per attempt, reuse the nonce on retry. if (!tipNonce) { try { - tipNonce = await squareCardInput.tokenize(); + tipNonce = await cardSelection.tokenize(); } catch (err) { toast.error(err instanceof Error ? err.message : 'Card entry failed'); return; @@ -181,7 +183,7 @@ amount: amountInPence, idempotency_key: tipIdempotencyKey, ...(selectedCardId ? { card_id: selectedCardId } : {}), - ...(newCardToken ? { new_card_token: newCardToken, save_card: false } : {}) + ...(newCardToken ? { new_card_token: newCardToken, save_card: saveCard } : {}) }; const response = await apiFetch(`/api/bookings/${booking.id}/tip`, { @@ -487,73 +489,14 @@ >Payment Method - {#if savedCards.length > 0} -
- {#each savedCards as card (card.id)} - - {/each} - -
- {/if} - - {#if showNewCardForm || savedCards.length === 0} - {#if isSquareConfigured()} - (squareCardReady = r)} - /> - {:else} - - {/if} - {/if} + (cardSelectionValid = v)} + /> diff --git a/obsidian/Crussell/Future Work - Gap Backlog.md b/obsidian/Crussell/Future Work - Gap Backlog.md index a1dea95..067d6f3 100644 --- a/obsidian/Crussell/Future Work - Gap Backlog.md +++ b/obsidian/Crussell/Future Work - Gap Backlog.md @@ -25,7 +25,7 @@ These are things that work fine in dev (with mocks) but need real implementation | # | Task | Effort | Area | Dev Status | Notes | |---|---|---|---|---|---| -| P1 | **Square payments: wire prod client alongside dev mock** | XL (5-7d) | Backend | Mock exists (`internal/square/square_dev.go:MockClient`). Prod client (`internal/square/square.go`) returns "not yet configured" for all 8 methods. Dev `devProdClient` (`square_dev.go:38-61`) also returns stubs when `SQUARE_ENVIRONMENT=sandbox` or `production`. Only `SQUARE_ENVIRONMENT=mock` processes payments (in-memory). The health endpoint reports `"not_implemented"`. | The in-memory Square mock was great for development — it let us build the full payment flow, refund logic, split records, VAT calculation, and saved cards without touching Square's API. Now we need the production SDK wired beside it. The mock already has the interface; implement `ProdClient` with real SDK calls. | +| P1 | **Square payments: wire prod client alongside dev mock** | XL (5-7d) | Backend | ✅ **COMPLETED Aug 2026** — `internal/square/square_http_client.go` implements the real REST client (payments, terminal checkouts, refunds, cards, list-refunds). Prod client (`internal/square/square.go`) and dev `devProdClient` (`square_dev.go`) both call real Square when `SQUARE_ENVIRONMENT=sandbox|production`; `mock` uses the in-memory client. The health endpoint reports `"mock"`/`"ok"` accordingly (was `"not_implemented"`). | | | P2 | **S3/R2 storage: implement prod side of the abstraction** | M (2-3d) | Backend | Dev works (`internal/s3/s3_dev.go` — RustFS + in-memory fallback). Prod side (`internal/s3/s3.go:52-62`) returns "not implemented" for Upload/Download/Delete. The prod `S3Client` struct lacks the `*s3.Client` field entirely — it was never populated. | The storage abstraction was defined early and the dev side got a full implementation. The prod side needs the AWS SDK v2 dependency and real S3/R2 calls. Portfolio images and profile pictures will start working in prod once this is done. | | P3 | **Square webhook event handling: from log-only to action** | S (1d) | Backend | Webhook signature verification works (HMAC-SHA256, references Square docs). Event parsing works. But `handlePaymentUpdated` and `handleRefundUpdated` (`square.go:88-94`) only log the event data — they never update booking/payment state. | The webhook receiver was built first (parse + verify). The handlers that act on events were deferred. Now they need to: update payment status on `payment.updated`, update refund status on `refund.updated`. | | P4 | **Payment reconciliation: add recovery for split-brain scenarios** | L (3-5d) | Backend | 11 `log.Printf("CRITICAL: ... manual reconciliation required")` calls exist across payment, refund, and till handlers. When Square succeeds but the DB transaction fails afterwards, state diverges with no automated recovery. | This happens when the application correctly processes a Square payment but then hits a DB error on commit. In dev, this was handled by just logging it. For prod, we need a reconciliation job or retry mechanism. | diff --git a/obsidian/Crussell/Overview.md b/obsidian/Crussell/Overview.md index 0c24d8a..569bd90 100644 --- a/obsidian/Crussell/Overview.md +++ b/obsidian/Crussell/Overview.md @@ -34,7 +34,7 @@ Square integration has two build-tagged implementations: - **Dev** (`//go:build dev`): Mock client simulates async checkout with polling. No real payments. - **Prod** (`//go:build !dev`): Connects to live Square API. Requires Square credentials in `.env`. -Saved cards stored in `user_saved_cards` with soft delete (`retained_until` for 7-year UK compliance). Refunds tracked in `refunds` table — partial or full. Square webhooks at `/api/webhooks/square` for payment status updates. +Saved cards stored in `user_saved_cards` with soft delete (`retained_until` for 7-year UK compliance). Refunds tracked in `refunds` table — partial or full. Square webhooks at `/api/webhooks/square` receive payment/refund events (HMAC-verified; currently log-only — status is tracked via the synchronous + sweep/reconcile paths, backlog P3). Fees column on `payments` stores actual Square deductions. `square_deposits` table for bank reconciliation (matching batch deposits to Mettle account). diff --git a/obsidian/Crussell/Technical Manual.md b/obsidian/Crussell/Technical Manual.md index bd0be4c..3ac0045 100644 --- a/obsidian/Crussell/Technical Manual.md +++ b/obsidian/Crussell/Technical Manual.md @@ -64,7 +64,7 @@ Backend (:8080) | `handlers/auth` | local.go, social.go | Registration (with referral code validation), login, refresh, email verification | | `handlers/bookings` | bookings.go, reserve.go, manage.go, admin_reserve.go, cancel_reservation.go, admin_cancel_reservation.go, closing_time.go | Booking CRUD, reservations with **self-blocking prevention** (`excludeUserID` parameter on `CheckTimeBlockerOverlap` + pre-overlap DELETE with IP hash anon cleanup), admin management, edit requests, discounts, closing hours validation (`checkClosingHours` + `getClosingTimeForDate` resolves staged default hours for bookings), active booking limits, GetBookingsByCreatedRange, created_by_name resolution, **explicit reservation cancellation** (`DELETE /api/bookings/reserve` for users, `DELETE /api/admin/bookings/reserve` for admin walk-in/call-in) | | `handlers/payments` | handlers.go, service.go, validators.go, giftcards.go, till.go, refunds.go, refund_policy.go | Square payments: terminal, online, refunds, tips, saved cards, gift cards (CRUD, topup, transfer, redeem, buy, expired balances, till sales). Refund calculation with notice-period tiers and deposit protection | -| `handlers/webhooks` | square.go | Square webhook handler for payment status updates. **Fail-closed signature check** — rejects requests with 403 when `SQUARE_WEBHOOK_SIGNATURE_KEY` is set but header is missing. Dev mode: skips verification when env var is empty. Still uses hex-encoding stub (`verifySquareSignature`) — production requires HMAC-SHA256 with base64 output, `x-square-hmacsha256-signature` header. See `TODO(PROD)` in source. | +| `handlers/webhooks` | square.go | Square webhook handler for payment status updates. **Fail-closed signature check** — rejects requests with 403 when `SQUARE_WEBHOOK_SIGNATURE_KEY` is set but header is missing. Dev mode: skips verification when env var is empty. HMAC-SHA256 signature verified per Square spec (base64 output, `x-square-hmacsha256-signature` header, notificationURL + body). `payment.updated`/`refund.updated` events are currently **log-only** (backlog P3 — status flows through the synchronous + sweep/reconcile paths instead). | | `handlers/admin` | users.go, analytics.go, custom_services.go, discount_campaigns.go, settings.go | Admin user management, custom services CRUD (list/create/get/update/promote/delete), discount campaigns, analytics (stub), business settings (GET/PUT with VAT, gift card config) | | `handlers/today` | today.go | Current/next appointment, today's grid, pending approvals, `DoneForDay` state with daily/weekly summary (`DailySummary` with `total_bookings`, `customers_served`, `summary_scope`), auto-status transitions, closed-day aggregation via `findWeekSummaryRange` + `computeAggregateSummary`. Exceptional hours lookup uses `exceptional_group_applications.week_start` (0=Monday). | | `handlers/user` | profile.go, account.go, guest.go, loyalty.go, customer_relationship.go, gdpr_export.go | User profile, guest creation (with CheckEmailHandler for registered-email detection), loyalty, contact info, GDPR export (async with 12h cache) | diff --git a/obsidian/Crussell/plans/p11-square-web-payments-sdk.md b/obsidian/Crussell/plans/p11-square-web-payments-sdk.md index 695f356..bf40e6e 100644 --- a/obsidian/Crussell/plans/p11-square-web-payments-sdk.md +++ b/obsidian/Crussell/plans/p11-square-web-payments-sdk.md @@ -47,7 +47,7 @@ The tokenization component. Loads the SDK, attaches the Square card iframe form, 1. **Square application credentials**: - `SQUARE_APPLICATION_ID` (client-side, public) - `SQUARE_LOCATION_ID` (already used server-side) - - Frontend needs the application ID in the browser context (e.g. `PUBLIC_SQUARE_APPLICATION_ID` Vite env var) + - Frontend needs the application ID in the browser context (`VITE_SQUARE_APPLICATION_ID` Vite env var — implemented; see `frontend/src/lib/square/square.ts`) 2. **Square account with Web Payments enabled** and a card processing merchant account. 3. Frontend must be HTTPS (or localhost) for the SDK to load.