From 7b24f8e484b8207ed61cd77b7a97852790abf199 Mon Sep 17 00:00:00 2001 From: Stephen Adamson Date: Wed, 24 Jun 2026 23:43:40 +0100 Subject: [PATCH] refactor(payments): integrate VAT into gift card buy flow and wrap in transactions Refactor BuyGiftCard to insert pending payment before Square call with VAT applied. Add transaction wrapping to gift card handlers. Remove redundant Content-Type header sets. Migrate all time.Now() to clock.Now(). Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus --- .../payments/discount_preview_test.go | 25 +- backend/handlers/payments/giftcards.go | 295 ++++++----- backend/handlers/payments/giftcards_test.go | 48 ++ backend/handlers/payments/handlers.go | 498 +++++++++++------- backend/handlers/payments/loyalty.go | 2 +- backend/handlers/payments/loyalty_test.go | 23 +- backend/handlers/payments/payments_test.go | 55 +- backend/handlers/payments/refunds.go | 395 +++++++++++--- backend/handlers/payments/refunds_test.go | 187 ++++++- backend/handlers/payments/service.go | 27 +- backend/handlers/payments/till.go | 157 +++--- backend/handlers/payments/till_test.go | 61 +++ backend/handlers/payments/vat.go | 12 +- backend/handlers/payments/vat_test.go | 142 ++++- 14 files changed, 1409 insertions(+), 518 deletions(-) diff --git a/backend/handlers/payments/discount_preview_test.go b/backend/handlers/payments/discount_preview_test.go index b23c3cd..18e7270 100644 --- a/backend/handlers/payments/discount_preview_test.go +++ b/backend/handlers/payments/discount_preview_test.go @@ -11,6 +11,7 @@ import ( "testing" "time" + "crussell/clock" "crussell/db" "crussell/testutils" "crussell/mw" @@ -116,7 +117,7 @@ func TestDiscountPreview_TimeBasedCampaign(t *testing.T) { userID, bookingID, token := setupDiscountPreviewTest(t, ctx, tx) - now := time.Now() + now := clock.Now() var campaignID string err := tx.QueryRow(ctx, ` INSERT INTO discount_campaigns (name, campaign_type, discount_percent, status, start_date, end_date, times_redeemed) @@ -197,7 +198,7 @@ func TestDiscountPreview_CampaignExhausted(t *testing.T) { userID, bookingID, token := setupDiscountPreviewTest(t, ctx, tx) - now := time.Now() + now := clock.Now() _, err := tx.Exec(ctx, ` INSERT INTO discount_campaigns (name, campaign_type, discount_percent, status, start_date, end_date, max_redemptions, times_redeemed) VALUES ('Exhausted Campaign', 'time_based', 20, 'active', $1, $2, 5, 5) @@ -232,7 +233,7 @@ func TestDiscountPreview_CampaignExpired(t *testing.T) { _, err := tx.Exec(ctx, ` INSERT INTO discount_campaigns (name, campaign_type, discount_percent, status, start_date, end_date) VALUES ('Expired Campaign', 'time_based', 10, 'active', $1, $2) - `, time.Now().Add(-72*time.Hour), time.Now().Add(-24*time.Hour)) + `, clock.Now().Add(-72*time.Hour), clock.Now().Add(-24*time.Hour)) if err != nil { t.Fatalf("failed to create campaign: %v", err) } @@ -257,7 +258,7 @@ func TestDiscountPreview_CampaignNotStarted(t *testing.T) { _, err := tx.Exec(ctx, ` INSERT INTO discount_campaigns (name, campaign_type, discount_percent, status, start_date, end_date) VALUES ('Future Campaign', 'time_based', 10, 'active', $1, $2) - `, time.Now().Add(24*time.Hour), time.Now().Add(72*time.Hour)) + `, clock.Now().Add(24*time.Hour), clock.Now().Add(72*time.Hour)) if err != nil { t.Fatalf("failed to create campaign: %v", err) } @@ -298,7 +299,7 @@ func TestDiscountPreview_MilestoneCampaign(t *testing.T) { _, err = tx.Exec(ctx, ` INSERT INTO discount_campaigns (name, campaign_type, discount_percent, status, start_date, end_date, milestone_type, milestone_value) VALUES ('First Booking Bonus', 'milestone', 25, 'active', $1, $2, 'per_user_booking_count', 1) - `, time.Now().Add(-24*time.Hour), time.Now().Add(24*time.Hour)) + `, clock.Now().Add(-24*time.Hour), clock.Now().Add(24*time.Hour)) if err != nil { t.Fatalf("failed to create milestone campaign: %v", err) } @@ -337,7 +338,7 @@ func TestDiscountPreview_AlreadyApplied(t *testing.T) { userID, bookingID, token := setupDiscountPreviewTest(t, ctx, tx) - now := time.Now() + now := clock.Now() var campaignID string err := tx.QueryRow(ctx, ` INSERT INTO discount_campaigns (name, campaign_type, discount_percent, status, start_date, end_date, times_redeemed) @@ -407,7 +408,7 @@ func TestDiscountPreview_MultipleCampaigns(t *testing.T) { userID, bookingID, token := setupDiscountPreviewTest(t, ctx, tx) - now := time.Now() + now := clock.Now() // Create two active campaigns _, err := tx.Exec(ctx, ` @@ -502,7 +503,7 @@ func TestDiscountPreview_PaymentLock(t *testing.T) { userID, bookingID, _ := setupDiscountPreviewTest(t, ctx, tx) - now := time.Now() + now := clock.Now() var campaignID string err := tx.QueryRow(ctx, ` INSERT INTO discount_campaigns (name, campaign_type, discount_percent, status, start_date, end_date, times_redeemed) @@ -522,7 +523,7 @@ func TestDiscountPreview_PaymentLock(t *testing.T) { t.Fatalf("failed to create first payment: %v", err) } - applyEligibleCampaignsAtPayment(ctx, bookingID, userID) + applyEligibleCampaignsAtPayment(ctx, tx, bookingID, userID) var discountCount int tx.QueryRow(ctx, @@ -540,7 +541,7 @@ func TestDiscountPreview_PaymentLock(t *testing.T) { t.Fatalf("failed to create second payment: %v", err) } - applyEligibleCampaignsAtPayment(ctx, bookingID, userID) + applyEligibleCampaignsAtPayment(ctx, tx, bookingID, userID) tx.QueryRow(ctx, "SELECT COUNT(*) FROM booking_discounts WHERE booking_id = $1", bookingID).Scan(&discountCount) @@ -576,7 +577,7 @@ func TestDiscountPreview_BookingNoServices(t *testing.T) { t.Fatalf("failed to create booking: %v", err) } - now := time.Now() + now := clock.Now() _, err = tx.Exec(ctx, ` INSERT INTO discount_campaigns (name, campaign_type, discount_percent, status, start_date, end_date) VALUES ('Test Sale', 'time_based', 10, 'active', $1, $2) @@ -608,7 +609,7 @@ func TestDiscountPreview_ReturnsReadOnly(t *testing.T) { ctx, tx := testutils.SetupTestTx(t) userID, bookingID, token := setupDiscountPreviewTest(t, ctx, tx) - now := time.Now() + now := clock.Now() // Create an active campaign var campaignID string diff --git a/backend/handlers/payments/giftcards.go b/backend/handlers/payments/giftcards.go index 9aea3d2..41dec6a 100644 --- a/backend/handlers/payments/giftcards.go +++ b/backend/handlers/payments/giftcards.go @@ -12,6 +12,7 @@ import ( "time" "crussell/db" + "crussell/clock" "crussell/internal/square" "crussell/internal/validators" "crussell/mw" @@ -321,7 +322,6 @@ func GetGiftCards(w http.ResponseWriter, r *http.Request) { } resp.TotalPages = totalPages - w.Header().Set("Content-Type", "application/json") json.NewEncoder(w).Encode(resp) } @@ -355,7 +355,12 @@ func CreateGiftCard(w http.ResponseWriter, r *http.Request) { var gc GiftCard var lastUsedAt sql.NullTime var purchaseVoucherType string - _ = tx.QueryRow(ctx, `SELECT COALESCE(voucher_type, 'SPV') FROM business_settings LIMIT 1`).Scan(&purchaseVoucherType) + err = tx.QueryRow(ctx, `SELECT COALESCE(voucher_type, 'SPV') FROM business_settings LIMIT 1`).Scan(&purchaseVoucherType) + if err != nil { + log.Printf("Failed to query voucher type: %v", err) + http.Error(w, "internal server error", http.StatusInternalServerError) + return + } if purchaseVoucherType == "" { purchaseVoucherType = "SPV" } @@ -406,7 +411,6 @@ func CreateGiftCard(w http.ResponseWriter, r *http.Request) { return } - w.Header().Set("Content-Type", "application/json") w.WriteHeader(http.StatusCreated) json.NewEncoder(w).Encode(gc) } @@ -520,7 +524,6 @@ func TopUpGiftCard(w http.ResponseWriter, r *http.Request) { return } - w.Header().Set("Content-Type", "application/json") json.NewEncoder(w).Encode(gc) } @@ -751,7 +754,6 @@ func GetGiftCardBalance(w http.ResponseWriter, r *http.Request) { err := db.Conn.QueryRow(ctx, "SELECT balance FROM user_giftcard_balances WHERE user_id = $1", userID).Scan(&balance) if err != nil { if errors.Is(err, pgx.ErrNoRows) { - w.Header().Set("Content-Type", "application/json") json.NewEncoder(w).Encode(map[string]float64{"balance": 0.00}) return } @@ -760,7 +762,6 @@ func GetGiftCardBalance(w http.ResponseWriter, r *http.Request) { return } - w.Header().Set("Content-Type", "application/json") json.NewEncoder(w).Encode(map[string]float64{"balance": balance}) } @@ -779,7 +780,6 @@ func GetUserGiftCardBalanceAdmin(w http.ResponseWriter, r *http.Request) { err := db.Conn.QueryRow(ctx, "SELECT balance FROM user_giftcard_balances WHERE user_id = $1", userID).Scan(&balance) if err != nil { if errors.Is(err, pgx.ErrNoRows) { - w.Header().Set("Content-Type", "application/json") json.NewEncoder(w).Encode(map[string]float64{"balance": 0.00}) return } @@ -789,14 +789,25 @@ func GetUserGiftCardBalanceAdmin(w http.ResponseWriter, r *http.Request) { } detailsJSON := fmt.Sprintf(`{"balance": %.2f}`, balance) - if _, err := db.Conn.Exec(ctx, ` - INSERT INTO admin_audit_log (admin_id, action_type, target_user_id, details) - VALUES ($1, 'balance_check', $2, $3::jsonb) - `, adminID, userID, detailsJSON); err != nil { - log.Printf("Failed to record admin_audit_log (non-critical): %v", err) + + tx, err := db.Conn.Begin(ctx) + if err != nil { + log.Printf("Failed to begin transaction: %v", err) + } else { + defer tx.Rollback(ctx) + + if _, err := tx.Exec(ctx, ` + INSERT INTO admin_audit_log (admin_id, action_type, target_user_id, details) + VALUES ($1, 'balance_check', $2, $3::jsonb) + `, adminID, userID, detailsJSON); err != nil { + log.Printf("Failed to record admin_audit_log (non-critical): %v", err) + } + + if err := tx.Commit(ctx); err != nil { + log.Printf("Failed to commit transaction: %v", err) + } } - w.Header().Set("Content-Type", "application/json") json.NewEncoder(w).Encode(map[string]float64{"balance": balance}) } @@ -838,7 +849,6 @@ func BuyGiftCard(w http.ResponseWriter, r *http.Request) { log.Printf("Failed to check idempotency: %v", err) } if existing != nil { - w.Header().Set("Content-Type", "application/json") json.NewEncoder(w).Encode(existing) return } @@ -880,21 +890,11 @@ func BuyGiftCard(w http.ResponseWriter, r *http.Request) { savedCardID = req.CardID } - paymentReq := square.CreatePaymentReq{ - Amount: req.Amount, - Currency: "GBP", - SourceID: sourceID, - IdempotencyKey: req.IdempotencyKey, - Note: "Gift Card Purchase", - } - - paymentResult, err := SquareClient.CreatePayment(ctx, paymentReq) - if err != nil { - log.Printf("Failed to process gift card purchase payment: %v", err) - http.Error(w, "Payment failed", http.StatusPaymentRequired) - return - } + amountPounds := float64(req.Amount) / 100.0 + // Step 1: Insert payment with status='pending' inside a DB transaction. + // Square is NOT called yet — if the tx fails, no harm done. + // Gift card and balance are created AFTER payment succeeds (below). tx, err := db.Conn.Begin(ctx) if err != nil { log.Printf("Failed to begin transaction: %v", err) @@ -903,101 +903,21 @@ func BuyGiftCard(w http.ResponseWriter, r *http.Request) { } defer tx.Rollback(ctx) - amountPounds := float64(req.Amount) / 100.0 - - var cardID string - - if req.RecipientType == "self" { - var purchaseVoucherType string - _ = tx.QueryRow(ctx, `SELECT COALESCE(voucher_type, 'SPV') FROM business_settings LIMIT 1`).Scan(&purchaseVoucherType) - if purchaseVoucherType == "" { - purchaseVoucherType = "SPV" - } - err = tx.QueryRow(ctx, ` - INSERT INTO gift_cards (total_funds_added, amount_remaining, created_by, redeemed_at, redeemed_by, is_inventory, voucher_type_at_purchase) - VALUES ($1, 0, $2, NOW(), $2, FALSE, $3) - RETURNING id - `, amountPounds, userID, purchaseVoucherType).Scan(&cardID) - if err != nil { - log.Printf("Failed to insert gift card: %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() - `, userID, amountPounds) - if err != nil { - log.Printf("Failed to update balance: %v", err) - http.Error(w, "internal server error", http.StatusInternalServerError) - return - } - - _, err = tx.Exec(ctx, ` - INSERT INTO gift_card_transactions (gift_card_id, transaction_type, amount, reference_type, reference_id, user_id, notes) - VALUES ($1, 'purchase', $2, 'api', NULL, $3, 'self-purchase, auto-redeemed') - `, cardID, amountPounds, userID) - if err != nil { - log.Printf("Failed to record gift card transaction: %v", err) - http.Error(w, "internal server error", http.StatusInternalServerError) - return - } - } else { - var purchaseVoucherType string - _ = tx.QueryRow(ctx, `SELECT COALESCE(voucher_type, 'SPV') FROM business_settings LIMIT 1`).Scan(&purchaseVoucherType) - if purchaseVoucherType == "" { - purchaseVoucherType = "SPV" - } - err = tx.QueryRow(ctx, ` - INSERT INTO gift_cards (total_funds_added, amount_remaining, created_by, is_inventory, voucher_type_at_purchase) - VALUES ($1, $1, $2, FALSE, $3) - RETURNING id - `, amountPounds, userID, purchaseVoucherType).Scan(&cardID) - if err != nil { - log.Printf("Failed to insert gift card: %v", err) - http.Error(w, "internal server error", http.StatusInternalServerError) - return - } - - recipient := req.RecipientEmail - if recipient == "" { - var userEmail string - _ = tx.QueryRow(ctx, "SELECT email FROM users WHERE id = $1", userID).Scan(&userEmail) - recipient = userEmail - } - log.Printf("[TODO EMAIL] Send gift card code %s (Value: £%.2f) to %s", cardID, amountPounds, recipient) - - _, err = tx.Exec(ctx, ` - INSERT INTO gift_card_transactions (gift_card_id, transaction_type, amount, reference_type, reference_id, user_id, notes) - VALUES ($1, 'purchase', $2, 'api', NULL, $3, 'purchased for friend') - `, cardID, amountPounds, userID) - if err != nil { - log.Printf("Failed to record gift card transaction: %v", err) - http.Error(w, "internal server error", http.StatusInternalServerError) - return - } - } - + var buyPaymentID string fees := paymentService.CalculateFees(req.Amount, "online") record := PaymentRecord{ PaymentType: "full", PaymentMethod: "online_square", - Status: "completed", + Status: "pending", Amount: amountPounds, - SquarePaymentID: &paymentResult.SquarePayID, + SquarePaymentID: nil, IdempotencyKey: &req.IdempotencyKey, Fees: float64(fees) / 100.0, UserSavedCardID: savedCardID, - CreatedAt: time.Now(), - UpdatedAt: time.Now(), + CreatedAt: clock.Now(), + UpdatedAt: clock.Now(), CreatedBy: &userID, } - - var buyPaymentID string err = tx.QueryRow(ctx, ` INSERT INTO payments (payment_type, payment_method, status, amount, square_payment_id, idempotency_key, fees, user_saved_card_id, created_by, created_at, updated_at) VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11) @@ -1009,6 +929,7 @@ func BuyGiftCard(w http.ResponseWriter, r *http.Request) { return } + // Apply VAT to the pending payment vatCfg, vatErr := GetVATConfig(ctx, tx) if vatErr == nil && vatCfg.IsVATRegistered && vatCfg.VoucherType == "SPV" { if _, vatExecErr := tx.Exec(ctx, "SELECT apply_vat_to_payment($1, $2)", buyPaymentID, vatCfg.DefaultVATRate); vatExecErr != nil { @@ -1022,7 +943,127 @@ func BuyGiftCard(w http.ResponseWriter, r *http.Request) { return } - w.Header().Set("Content-Type", "application/json") + // Step 2: DB transaction committed — safe to call Square now. + // If Square fails, the payment record stays 'pending' for manual retry. + paymentReq := square.CreatePaymentReq{ + Amount: req.Amount, + Currency: "GBP", + SourceID: sourceID, + IdempotencyKey: req.IdempotencyKey, + Note: "Gift Card Purchase", + } + + paymentResult, err := SquareClient.CreatePayment(ctx, paymentReq) + if err != nil { + log.Printf("Failed to process gift card purchase payment: %v", err) + // Payment record intentionally left as 'pending' for manual retry. + http.Error(w, "Payment failed", http.StatusPaymentRequired) + return + } + + // Step 3: Square succeeded — update payment, create gift card. + _, upErr := db.Conn.Exec(ctx, + `UPDATE payments SET status = 'completed', square_payment_id = $1 WHERE id = $2`, + paymentResult.SquarePayID, buyPaymentID, + ) + if upErr != nil { + log.Printf("CRITICAL: Square payment succeeded (ID=%s) but payment %s update failed: %v — manual reconciliation required", paymentResult.SquarePayID, buyPaymentID, upErr) + http.Error(w, "internal server error", http.StatusInternalServerError) + return + } + + var cardID string + + if req.RecipientType == "self" { + var purchaseVoucherType string + err = db.Conn.QueryRow(ctx, `SELECT COALESCE(voucher_type, 'SPV') FROM business_settings LIMIT 1`).Scan(&purchaseVoucherType) + if err != nil { + log.Printf("Failed to query voucher type: %v", err) + http.Error(w, "internal server error", http.StatusInternalServerError) + return + } + if purchaseVoucherType == "" { + purchaseVoucherType = "SPV" + } + err = db.Conn.QueryRow(ctx, ` + INSERT INTO gift_cards (total_funds_added, amount_remaining, created_by, redeemed_at, redeemed_by, is_inventory, voucher_type_at_purchase) + VALUES ($1, 0, $2, NOW(), $2, FALSE, $3) + RETURNING id + `, amountPounds, userID, purchaseVoucherType).Scan(&cardID) + if err != nil { + log.Printf("CRITICAL: Square payment succeeded (ID=%s) but gift card creation failed: %v — manual reconciliation required", paymentResult.SquarePayID, err) + http.Error(w, "internal server error", http.StatusInternalServerError) + return + } + + _, err = db.Conn.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() + `, userID, amountPounds) + if err != nil { + log.Printf("CRITICAL: Square payment succeeded (ID=%s) but balance update for user %s failed: %v — manual reconciliation required", paymentResult.SquarePayID, userID, err) + http.Error(w, "internal server error", http.StatusInternalServerError) + return + } + + _, err = db.Conn.Exec(ctx, ` + INSERT INTO gift_card_transactions (gift_card_id, transaction_type, amount, reference_type, reference_id, user_id, notes) + VALUES ($1, 'purchase', $2, 'api', NULL, $3, 'self-purchase, auto-redeemed') + `, cardID, amountPounds, userID) + if err != nil { + log.Printf("CRITICAL: Square payment succeeded (ID=%s) but gift card transaction record failed: %v — manual reconciliation required", paymentResult.SquarePayID, err) + http.Error(w, "internal server error", http.StatusInternalServerError) + return + } + } else { + var purchaseVoucherType string + err = db.Conn.QueryRow(ctx, `SELECT COALESCE(voucher_type, 'SPV') FROM business_settings LIMIT 1`).Scan(&purchaseVoucherType) + if err != nil { + log.Printf("Failed to query voucher type: %v", err) + http.Error(w, "internal server error", http.StatusInternalServerError) + return + } + if purchaseVoucherType == "" { + purchaseVoucherType = "SPV" + } + err = db.Conn.QueryRow(ctx, ` + INSERT INTO gift_cards (total_funds_added, amount_remaining, created_by, is_inventory, voucher_type_at_purchase) + VALUES ($1, $1, $2, FALSE, $3) + RETURNING id + `, amountPounds, userID, purchaseVoucherType).Scan(&cardID) + if err != nil { + log.Printf("CRITICAL: Square payment succeeded (ID=%s) but gift card creation failed: %v — manual reconciliation required", paymentResult.SquarePayID, err) + http.Error(w, "internal server error", http.StatusInternalServerError) + return + } + + recipient := req.RecipientEmail + if recipient == "" { + var userEmail string + err = db.Conn.QueryRow(ctx, "SELECT email FROM users WHERE id = $1", userID).Scan(&userEmail) + if err != nil { + log.Printf("Failed to query user email: %v", err) + http.Error(w, "internal server error", http.StatusInternalServerError) + return + } + recipient = userEmail + } + log.Printf("[TODO EMAIL] Send gift card code %s (Value: £%.2f) to %s", cardID, amountPounds, recipient) + + _, err = db.Conn.Exec(ctx, ` + INSERT INTO gift_card_transactions (gift_card_id, transaction_type, amount, reference_type, reference_id, user_id, notes) + VALUES ($1, 'purchase', $2, 'api', NULL, $3, 'purchased for friend') + `, cardID, amountPounds, userID) + if err != nil { + log.Printf("CRITICAL: Square payment succeeded (ID=%s) but gift card transaction record failed: %v — manual reconciliation required", paymentResult.SquarePayID, err) + http.Error(w, "internal server error", http.StatusInternalServerError) + return + } + } + w.WriteHeader(http.StatusCreated) json.NewEncoder(w).Encode(map[string]interface{}{ "status": "success", @@ -1086,12 +1127,16 @@ func GetExpiredBalances(w http.ResponseWriter, r *http.Request) { balances = append(balances, b) } + if err := rows.Err(); err != nil { + log.Printf("Row iteration error: %v", err) + http.Error(w, "Internal server error", http.StatusInternalServerError) + return + } if balances == nil { balances = []ExpiredBalance{} } - w.Header().Set("Content-Type", "application/json") json.NewEncoder(w).Encode(map[string]interface{}{ "expired_balances": balances, "total": len(balances), @@ -1122,9 +1167,17 @@ func ClaimExpiredBalance(w http.ResponseWriter, r *http.Request) { return } + tx, err := db.Conn.Begin(ctx) + if err != nil { + log.Printf("Failed to begin transaction: %v", err) + http.Error(w, "internal server error", http.StatusInternalServerError) + return + } + defer tx.Rollback(ctx) + var existingClaimedAt sql.NullTime - err := db.Conn.QueryRow(ctx, ` - SELECT claimed_at FROM gift_card_expired_balances WHERE id = $1 + err = tx.QueryRow(ctx, ` + SELECT claimed_at FROM gift_card_expired_balances WHERE id = $1 FOR UPDATE `, req.BalanceID).Scan(&existingClaimedAt) if err != nil { if errors.Is(err, pgx.ErrNoRows) { @@ -1141,7 +1194,7 @@ func ClaimExpiredBalance(w http.ResponseWriter, r *http.Request) { return } - _, err = db.Conn.Exec(ctx, ` + _, err = tx.Exec(ctx, ` UPDATE gift_card_expired_balances SET claimed_at = NOW(), claimed_by_admin = $1, notes = $2 WHERE id = $3 @@ -1152,6 +1205,12 @@ func ClaimExpiredBalance(w http.ResponseWriter, r *http.Request) { return } + if err := tx.Commit(ctx); err != nil { + log.Printf("Failed to commit transaction: %v", err) + http.Error(w, "internal server error", http.StatusInternalServerError) + return + } + w.WriteHeader(http.StatusOK) json.NewEncoder(w).Encode(map[string]string{"status": "claimed"}) } diff --git a/backend/handlers/payments/giftcards_test.go b/backend/handlers/payments/giftcards_test.go index b34aecd..f1bc837 100644 --- a/backend/handlers/payments/giftcards_test.go +++ b/backend/handlers/payments/giftcards_test.go @@ -311,6 +311,54 @@ func TestBuyGiftCard_Self(t *testing.T) { } } +func TestBuyGiftCard_TransactionFailure_SkipsSquare(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) + } + + token := jwt.GenerateTestToken(userID, "verified_email") + + // Create a cancelled context so the nested transaction fails + cancelCtx, cancel := context.WithCancel(ctx) + cancel() + + reqBody, _ := json.Marshal(map[string]interface{}{ + "amount": 2000, + "recipient_type": "self", + "new_card_token": "cnon:card-nonce-ok", + "idempotency_key": "idempotency-key-buy-gc-txn-fail", + }) + req := httptest.NewRequest("POST", "/api/user/giftcards/buy", bytes.NewBuffer(reqBody)) + req.Header.Set("Authorization", "Bearer "+token) + req.Header.Set("Content-Type", "application/json") + req = req.WithContext(db.ContextWithTx(cancelCtx, tx.(pgx.Tx))) + + w := httptest.NewRecorder() + + r := chi.NewRouter() + r.Use(mw.RequireAuth) + r.Post("/api/user/giftcards/buy", BuyGiftCard) + r.ServeHTTP(w, req) + + if w.Code != http.StatusInternalServerError { + t.Errorf("expected status 500 due to cancelled context, got %d. Body: %s", w.Code, w.Body.String()) + } + + // Verify no completed payment records exist — confirming Square was never called + var payCount int + err = tx.QueryRow(ctx, "SELECT COUNT(*) FROM payments WHERE created_by = $1 AND status = 'completed'", userID).Scan(&payCount) + if err != nil { + t.Fatalf("failed to query payments: %v", err) + } + if payCount != 0 { + t.Errorf("expected 0 completed payment records (Square should not have been called), got %d", payCount) + } +} + func TestBuyGiftCard_Friend(t *testing.T) { t.Parallel() ctx, tx := testutils.SetupTestTx(t) diff --git a/backend/handlers/payments/handlers.go b/backend/handlers/payments/handlers.go index 8cbb083..71cd23f 100644 --- a/backend/handlers/payments/handlers.go +++ b/backend/handlers/payments/handlers.go @@ -3,6 +3,7 @@ package payments import ( "context" "crussell/db" + "crussell/clock" "crussell/internal/square" "crussell/internal/validators" "crussell/mw" @@ -128,7 +129,6 @@ func GetDiscountPreviewHandler(w http.ResponseWriter, r *http.Request) { preview := calculateDiscountPreview(r.Context(), bookingID, userID) - w.Header().Set("Content-Type", "application/json") json.NewEncoder(w).Encode(preview) } @@ -324,22 +324,6 @@ func CreateTerminalPayment(w http.ResponseWriter, r *http.Request) { service := NewPaymentService() - status, err := service.GetBookingStatus(r.Context(), bookingID) - if err != nil { - if errors.Is(err, pgx.ErrNoRows) { - http.Error(w, "Booking not found", http.StatusNotFound) - return - } - log.Printf("Failed to get booking status: %v", err) - http.Error(w, "internal server error", http.StatusInternalServerError) - return - } - - if status != "in_progress" && status != "completed" { - http.Error(w, "Booking must be in_progress or completed to create payment", http.StatusBadRequest) - return - } - amount := req.Amount if req.OverrideAmount != nil { amount = *req.OverrideAmount @@ -347,21 +331,10 @@ func CreateTerminalPayment(w http.ResponseWriter, r *http.Request) { idempotencyKey := bookingID + "-" + req.PaymentType + "-" + strconv.FormatInt(amount, 10) - existingPayment, err := service.CheckIdempotency(r.Context(), bookingID, idempotencyKey) - if err != nil { - log.Printf("Failed to check idempotency: %v", err) - } - if existingPayment != nil { - w.Header().Set("Content-Type", "application/json") - json.NewEncoder(w).Encode(CheckoutResponse{ - CheckoutID: existingPayment.ID, - Status: existingPayment.Status, - }) - return - } - // Route based on payment method if req.PaymentMethod != nil && (*req.PaymentMethod == "cash" || *req.PaymentMethod == "giftcard") { + // Start transaction before the status and idempotency checks so they + // are atomic with the payment insert. tx, err := db.Conn.Begin(r.Context()) if err != nil { log.Printf("Failed to begin transaction: %v", err) @@ -370,6 +343,37 @@ func CreateTerminalPayment(w http.ResponseWriter, r *http.Request) { } defer tx.Rollback(r.Context()) + // Check booking status inside the transaction. + var status string + if err := tx.QueryRow(r.Context(), `SELECT status FROM bookings WHERE id = $1`, bookingID).Scan(&status); err != nil { + if errors.Is(err, pgx.ErrNoRows) { + http.Error(w, "Booking not found", http.StatusNotFound) + return + } + log.Printf("Failed to get booking status: %v", err) + http.Error(w, "internal server error", http.StatusInternalServerError) + return + } + if status != "in_progress" && status != "completed" { + http.Error(w, "Booking must be in_progress or completed to create payment", http.StatusBadRequest) + return + } + + // Check idempotency inside the transaction. + var existingID string + var existingStatus string + if err := tx.QueryRow(r.Context(), ` + SELECT id, status FROM payments WHERE booking_id = $1 AND idempotency_key = $2 + `, bookingID, idempotencyKey).Scan(&existingID, &existingStatus); err == nil { + json.NewEncoder(w).Encode(CheckoutResponse{ + CheckoutID: existingID, + Status: existingStatus, + }) + return + } else if !errors.Is(err, pgx.ErrNoRows) { + log.Printf("Failed to check idempotency: %v", err) + } + amountPounds := float64(amount) / 100.0 var paymentID string @@ -503,7 +507,6 @@ func CreateTerminalPayment(w http.ResponseWriter, r *http.Request) { return } - w.Header().Set("Content-Type", "application/json") json.NewEncoder(w).Encode(CheckoutResponse{ CheckoutID: paymentID, Status: "COMPLETED", @@ -511,6 +514,36 @@ func CreateTerminalPayment(w http.ResponseWriter, r *http.Request) { return } + // For Square checkout (terminal card reader), validate booking status + // and check idempotency. No DB transaction needed since Square handles + // the payment — no DB writes occur until GetCheckoutStatus. + status, err := service.GetBookingStatus(r.Context(), bookingID) + if err != nil { + if errors.Is(err, pgx.ErrNoRows) { + http.Error(w, "Booking not found", http.StatusNotFound) + return + } + log.Printf("Failed to get booking status: %v", err) + http.Error(w, "internal server error", http.StatusInternalServerError) + return + } + if status != "in_progress" && status != "completed" { + http.Error(w, "Booking must be in_progress or completed to create payment", http.StatusBadRequest) + return + } + + existingPayment, err := service.CheckIdempotency(r.Context(), bookingID, idempotencyKey) + if err != nil { + log.Printf("Failed to check idempotency: %v", err) + } + if existingPayment != nil { + json.NewEncoder(w).Encode(CheckoutResponse{ + CheckoutID: existingPayment.ID, + Status: existingPayment.Status, + }) + return + } + checkoutReq := square.CreateCheckoutReq{ Amount: amount, Currency: "GBP", @@ -526,7 +559,6 @@ func CreateTerminalPayment(w http.ResponseWriter, r *http.Request) { return } - w.Header().Set("Content-Type", "application/json") json.NewEncoder(w).Encode(CheckoutResponse{ CheckoutID: checkout.ID, Status: checkout.Status, @@ -558,7 +590,6 @@ func GetCheckoutStatus(w http.ResponseWriter, r *http.Request) { paymentResult, err := SquareClient.GetCheckout(r.Context(), checkoutID) if err != nil { if err.Error() == "checkout pending" { - w.Header().Set("Content-Type", "application/json") json.NewEncoder(w).Encode(PaymentStatusResponse{Status: "PENDING"}) return } @@ -569,25 +600,39 @@ func GetCheckoutStatus(w http.ResponseWriter, r *http.Request) { if paymentResult.Status == "COMPLETED" { service := NewPaymentService() + idempotencyKey := bookingID + "-terminal-" + strconv.FormatInt(paymentResult.Amount, 10) - existing, err := service.CheckIdempotency(r.Context(), bookingID, "") + // Begin the transaction BEFORE the idempotency check so it's atomic + // with the payment insert. + tx, err := db.Conn.Begin(r.Context()) if err != nil { - log.Printf("Failed to check for existing payment: %v", err) - } - if existing != nil && existing.SquarePaymentID != nil && *existing.SquarePaymentID == paymentResult.SquarePayID { - w.Header().Set("Content-Type", "application/json") - json.NewEncoder(w).Encode(PaymentStatusResponse{ - Status: "COMPLETED", - PaymentID: existing.ID, - Amount: int64(existing.Amount * 100), - CardBrand: paymentResult.CardBrand, - CardLast4: paymentResult.CardLast4, - ReceiptURL: paymentResult.ReceiptURL, - }) + log.Printf("Failed to begin transaction: %v", err) + http.Error(w, "internal server error", http.StatusInternalServerError) return } + defer tx.Rollback(r.Context()) - idempotencyKey := bookingID + "-terminal-" + strconv.FormatInt(paymentResult.Amount, 10) + // Check for existing payment inside the transaction. + 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 { + json.NewEncoder(w).Encode(PaymentStatusResponse{ + Status: "COMPLETED", + PaymentID: existingID, + Amount: paymentResult.Amount, + CardBrand: paymentResult.CardBrand, + CardLast4: paymentResult.CardLast4, + ReceiptURL: paymentResult.ReceiptURL, + }) + return + } + } else if !errors.Is(err, pgx.ErrNoRows) { + log.Printf("Failed to check for existing payment: %v", err) + } record := PaymentRecord{ BookingID: bookingID, @@ -598,18 +643,10 @@ func GetCheckoutStatus(w http.ResponseWriter, r *http.Request) { SquarePaymentID: &paymentResult.SquarePayID, IdempotencyKey: &idempotencyKey, Fees: float64(paymentResult.Fees) / 100.0, - CreatedAt: time.Now(), - UpdatedAt: time.Now(), + CreatedAt: clock.Now(), + UpdatedAt: clock.Now(), } - tx, err := db.Conn.Begin(r.Context()) - if err != nil { - log.Printf("Failed to begin transaction: %v", err) - http.Error(w, "internal server error", http.StatusInternalServerError) - return - } - defer tx.Rollback(r.Context()) - paymentID, err := service.CreatePaymentRecordTx(r.Context(), tx, record, nil) if err != nil { log.Printf("Failed to create payment record: %v", err) @@ -624,7 +661,6 @@ func GetCheckoutStatus(w http.ResponseWriter, r *http.Request) { return } - w.Header().Set("Content-Type", "application/json") json.NewEncoder(w).Encode(PaymentStatusResponse{ Status: "COMPLETED", PaymentID: paymentID, @@ -759,11 +795,24 @@ func CreateBookingPayment(w http.ResponseWriter, r *http.Request) { } }() - // Now that we hold the serialization lock, re-check the booking status. - // If another request (e.g. from a different tab) already processed a payment - // and promoted the booking while we were waiting, we see that here. - status, err := service.GetBookingStatus(r.Context(), bookingID) - if err != nil { + // Now that we hold the serialization lock, begin a transaction and re-check + // the booking status inside it. If another request (e.g. from a different tab) + // already processed a payment and promoted the booking while we were waiting, + // we see that here. + tx, txErr := db.Conn.Begin(r.Context()) + if txErr != nil { + log.Printf("Failed to begin transaction: %v", txErr) + http.Error(w, "internal server error", http.StatusInternalServerError) + return + } + defer tx.Rollback(r.Context()) + + var status string + if err := tx.QueryRow(r.Context(), `SELECT status FROM bookings WHERE id = $1`, bookingID).Scan(&status); err != nil { + if errors.Is(err, pgx.ErrNoRows) { + http.Error(w, "Booking not found", http.StatusNotFound) + return + } log.Printf("Failed to get booking status for payment check: %v", err) http.Error(w, "internal server error", http.StatusInternalServerError) return @@ -779,21 +828,29 @@ func CreateBookingPayment(w http.ResponseWriter, r *http.Request) { return } - existingPayment, err := service.CheckIdempotency(r.Context(), bookingID, req.IdempotencyKey) - if err != nil { - log.Printf("Failed to check idempotency: %v", err) - } - if existingPayment != nil { - w.Header().Set("Content-Type", "application/json") + // Check idempotency inside the transaction. + var existingID sql.NullString + var existingBookingID sql.NullString + var existingPaymentType sql.NullString + var existingStatus sql.NullString + var existingAmount sql.NullFloat64 + var existingCreatedAt sql.NullTime + if err := tx.QueryRow(r.Context(), ` + SELECT id, booking_id, payment_type, status, amount, created_at + FROM payments + WHERE booking_id = $1 AND idempotency_key = $2 + `, bookingID, req.IdempotencyKey).Scan(&existingID, &existingBookingID, &existingPaymentType, &existingStatus, &existingAmount, &existingCreatedAt); err == nil { json.NewEncoder(w).Encode(PaymentResponse{ - ID: existingPayment.ID, - BookingID: existingPayment.BookingID, - PaymentType: existingPayment.PaymentType, - Status: existingPayment.Status, - Amount: int64(existingPayment.Amount * 100), - CreatedAt: existingPayment.CreatedAt.Format(time.RFC3339), + ID: existingID.String, + BookingID: existingBookingID.String, + PaymentType: existingPaymentType.String, + Status: existingStatus.String, + Amount: int64(existingAmount.Float64 * 100), + CreatedAt: existingCreatedAt.Time.Format(time.RFC3339), }) return + } else if !errors.Is(err, pgx.ErrNoRows) { + log.Printf("Failed to check idempotency: %v", err) } // After the idempotency check (which handles same-key retries), verify @@ -805,7 +862,7 @@ func CreateBookingPayment(w http.ResponseWriter, r *http.Request) { // concurrent payments of the same type. if req.PaymentType != "partial" { var existingCount int - if err := db.Conn.QueryRow(r.Context(), ` + if err := tx.QueryRow(r.Context(), ` SELECT COUNT(*) FROM payments WHERE booking_id = $1 AND status = 'completed' @@ -876,12 +933,6 @@ func CreateBookingPayment(w http.ResponseWriter, r *http.Request) { return } - // Apply eligible campaign discounts before the payment is processed, so - // the discount payment records exist in the DB before the frontend computes - // the net amount to charge. The call is idempotent — if discounts were - // already applied (e.g. by a prior call), the duplicate check skips them. - applyEligibleCampaignsAtPayment(r.Context(), bookingID, userID) - fees := service.CalculateFees(req.Amount, "online") paymentAmount := float64(req.Amount) / 100.0 @@ -901,8 +952,8 @@ func CreateBookingPayment(w http.ResponseWriter, r *http.Request) { IdempotencyKey: &req.IdempotencyKey, Fees: float64(fees) / 100.0, UserSavedCardID: savedCardID, - CreatedAt: time.Now(), - UpdatedAt: time.Now(), + CreatedAt: clock.Now(), + UpdatedAt: clock.Now(), CreatedBy: &userID, } @@ -916,17 +967,10 @@ func CreateBookingPayment(w http.ResponseWriter, r *http.Request) { records = []PaymentRecord{primaryRecord} } - // Create all payment records for this Square charge inside a transaction + // Create all payment records for this Square charge inside the transaction // so that if any insert fails the entire group rolls back. This prevents // a data inconsistency where Square charged the customer but only part of // the split is reflected in the DB. - tx, txErr := db.Conn.Begin(r.Context()) - if txErr != nil { - log.Printf("Failed to begin transaction for payment records: %v", txErr) - http.Error(w, "internal server error", http.StatusInternalServerError) - return - } - defer tx.Rollback(r.Context()) var primaryPaymentID string var paymentIDs []string @@ -979,15 +1023,17 @@ func CreateBookingPayment(w http.ResponseWriter, r *http.Request) { } } + // Apply eligible campaign discounts inside the payment transaction, so + // atomicity with the payment inserts is guaranteed. The call is idempotent + // — if discounts were already applied, the duplicate check skips them. + applyEligibleCampaignsAtPayment(r.Context(), tx, bookingID, userID) + if cErr := tx.Commit(r.Context()); cErr != nil { log.Printf("Failed to commit payment records: %v", cErr) http.Error(w, "internal server error", http.StatusInternalServerError) return } - applyEligibleCampaignsAtPayment(r.Context(), bookingID, userID) - - w.Header().Set("Content-Type", "application/json") json.NewEncoder(w).Encode(PaymentResponse{ ID: primaryPaymentID, BookingID: bookingID, @@ -997,33 +1043,32 @@ func CreateBookingPayment(w http.ResponseWriter, r *http.Request) { CardBrand: paymentResult.CardBrand, CardLast4: paymentResult.CardLast4, ReceiptURL: paymentResult.ReceiptURL, - CreatedAt: time.Now().Format(time.RFC3339), + CreatedAt: clock.Now().Format(time.RFC3339), }) } -// applyEligibleCampaignsAtPayment runs after a payment is committed to check -// and apply any eligible discount campaigns to the booking. -// Skips if the booking already has a completed non-discount payment — this +// applyEligibleCampaignsAtPayment checks and applies any eligible discount +// campaigns to the booking. Uses the provided transaction so that discount +// writes are atomic with the caller's payment transaction — if the payment +// commit fails, the discount writes roll back with it. +// Skips if the booking already has 2+ completed non-discount payments — this // prevents applying new discounts after a customer has already paid, which // would create a credit balance or require a refund. -func applyEligibleCampaignsAtPayment(ctx context.Context, bookingID string, userID string) { +func applyEligibleCampaignsAtPayment(ctx context.Context, q db.Querier, bookingID string, userID string) { var existingPayment int - db.Conn.QueryRow(ctx, ` + q.QueryRow(ctx, ` SELECT COUNT(*) FROM payments WHERE booking_id = $1 AND status = 'completed' AND payment_method NOT IN ('discount', 'on_the_house') `, bookingID).Scan(&existingPayment) // Only block if this is the 2nd+ real payment — the first payment should still - // trigger discount application (existingPayment counts already-completed payments). - // When called before payment commit (line 850), existingPayment=0 so discounts - // are applied. When called after commit (line 952), existingPayment=1 and the - // idempotency check handles it. At the 2nd+ payment attempt, this guard prevents - // applying any new discounts. + // trigger discount application (existingPayment counts already-completed payments + // visible within the transaction, including the just-inserted one). if existingPayment >= 2 { return } var bookingTotal float64 - if err := db.Conn.QueryRow(ctx, ` + if err := q.QueryRow(ctx, ` SELECT total_amount FROM bookings WHERE id = $1 `, bookingID).Scan(&bookingTotal); err != nil { log.Printf("Failed to calculate booking total for campaign check: %v", err) @@ -1033,16 +1078,9 @@ func applyEligibleCampaignsAtPayment(ctx context.Context, bookingID string, user return } - tx, err := db.Conn.Begin(ctx) - if err != nil { - log.Printf("Failed to begin discount application transaction: %v", err) - return - } - defer tx.Rollback(ctx) - var campaignID string var campaignPercent float64 - if err := tx.QueryRow(ctx, ` + if err := q.QueryRow(ctx, ` SELECT id, discount_percent FROM discount_campaigns WHERE status = 'active' AND campaign_type = 'time_based' AND start_date <= NOW() AND end_date >= NOW() @@ -1050,20 +1088,20 @@ func applyEligibleCampaignsAtPayment(ctx context.Context, bookingID string, user ORDER BY discount_percent DESC LIMIT 1 `).Scan(&campaignID, &campaignPercent); err == nil && campaignID != "" { var exists int - tx.QueryRow(ctx, `SELECT 1 FROM booking_discounts WHERE booking_id = $1 AND source_id = $2`, bookingID, campaignID).Scan(&exists) + q.QueryRow(ctx, `SELECT 1 FROM booking_discounts WHERE booking_id = $1 AND source_id = $2`, bookingID, campaignID).Scan(&exists) if exists == 0 { discountAmount := roundTo2(bookingTotal * campaignPercent / 100) - if _, err := tx.Exec(ctx, ` + if _, err := q.Exec(ctx, ` INSERT INTO booking_discounts (booking_id, user_id, discount_source, source_id, campaign_type, milestone_type, discount_percent, original_total, discount_amount) VALUES ($1, $2, 'campaign', $3, 'time_based', NULL, $4, $5, $6) `, bookingID, userID, campaignID, campaignPercent, bookingTotal, discountAmount); err != nil { log.Printf("Failed to insert time-based campaign discount: %v", err) } else { - tx.Exec(ctx, ` + q.Exec(ctx, ` INSERT INTO payments (booking_id, payment_type, payment_method, amount, status, created_by) VALUES ($1, 'partial', 'discount', $2, 'completed', $3) `, bookingID, discountAmount, userID) - tx.Exec(ctx, ` + q.Exec(ctx, ` UPDATE discount_campaigns SET times_redeemed = times_redeemed + 1 WHERE id = $1 `, campaignID) } @@ -1071,11 +1109,11 @@ func applyEligibleCampaignsAtPayment(ctx context.Context, bookingID string, user } var userBookingCount int - tx.QueryRow(ctx, `SELECT COUNT(*) FROM bookings WHERE user_id = $1 AND status = 'completed'`, userID).Scan(&userBookingCount) + q.QueryRow(ctx, `SELECT COUNT(*) FROM bookings WHERE user_id = $1 AND status = 'completed'`, userID).Scan(&userBookingCount) var milestoneCampaignID string var milestonePercent float64 - tx.QueryRow(ctx, ` + q.QueryRow(ctx, ` SELECT id, discount_percent FROM discount_campaigns WHERE status = 'active' AND campaign_type = 'milestone' AND milestone_type = 'per_user_booking_count' AND milestone_value = $1 @@ -1084,20 +1122,20 @@ func applyEligibleCampaignsAtPayment(ctx context.Context, bookingID string, user if milestoneCampaignID != "" { var exists int - tx.QueryRow(ctx, `SELECT 1 FROM booking_discounts WHERE booking_id = $1 AND source_id = $2`, bookingID, milestoneCampaignID).Scan(&exists) + q.QueryRow(ctx, `SELECT 1 FROM booking_discounts WHERE booking_id = $1 AND source_id = $2`, bookingID, milestoneCampaignID).Scan(&exists) if exists == 0 { discountAmount := roundTo2(bookingTotal * milestonePercent / 100) - if _, err := tx.Exec(ctx, ` + if _, err := q.Exec(ctx, ` INSERT INTO booking_discounts (booking_id, user_id, discount_source, source_id, campaign_type, milestone_type, discount_percent, original_total, discount_amount) VALUES ($1, $2, 'campaign', $3, 'milestone', 'per_user_booking_count', $4, $5, $6) `, bookingID, userID, milestoneCampaignID, milestonePercent, bookingTotal, discountAmount); err != nil { log.Printf("Failed to insert per-user milestone discount: %v", err) } else { - tx.Exec(ctx, ` + q.Exec(ctx, ` INSERT INTO payments (booking_id, payment_type, payment_method, amount, status, created_by) VALUES ($1, 'partial', 'discount', $2, 'completed', $3) `, bookingID, discountAmount, userID) - tx.Exec(ctx, ` + q.Exec(ctx, ` UPDATE discount_campaigns SET times_redeemed = times_redeemed + 1 WHERE id = $1 `, milestoneCampaignID) } @@ -1105,9 +1143,9 @@ func applyEligibleCampaignsAtPayment(ctx context.Context, bookingID string, user } var firstVisitDate time.Time - tx.QueryRow(ctx, `SELECT MIN(start_time) FROM bookings WHERE user_id = $1 AND status = 'completed'`, userID).Scan(&firstVisitDate) + q.QueryRow(ctx, `SELECT MIN(start_time) FROM bookings WHERE user_id = $1 AND status = 'completed'`, userID).Scan(&firstVisitDate) if !firstVisitDate.IsZero() { - annRows, err := tx.Query(ctx, ` + annRows, err := q.Query(ctx, ` SELECT id, discount_percent, milestone_value, milestone_unit FROM discount_campaigns WHERE status = 'active' AND campaign_type = 'milestone' AND milestone_type = 'anniversary' AND NOT EXISTS (SELECT 1 FROM booking_discounts WHERE user_id = $1 AND source_id = discount_campaigns.id AND milestone_type = 'anniversary') @@ -1130,7 +1168,7 @@ func applyEligibleCampaignsAtPayment(ctx context.Context, bookingID string, user for _, c := range campaigns { var exists int - tx.QueryRow(ctx, `SELECT 1 FROM booking_discounts WHERE booking_id = $1 AND source_id = $2`, bookingID, c.id).Scan(&exists) + q.QueryRow(ctx, `SELECT 1 FROM booking_discounts WHERE booking_id = $1 AND source_id = $2`, bookingID, c.id).Scan(&exists) if exists > 0 { continue } @@ -1147,17 +1185,17 @@ func applyEligibleCampaignsAtPayment(ctx context.Context, bookingID string, user } if matches { discountAmount := roundTo2(bookingTotal * c.pct / 100) - if _, err := tx.Exec(ctx, ` + if _, err := q.Exec(ctx, ` INSERT INTO booking_discounts (booking_id, user_id, discount_source, source_id, campaign_type, milestone_type, discount_percent, original_total, discount_amount) VALUES ($1, $2, 'campaign', $3, 'milestone', 'anniversary', $4, $5, $6) `, bookingID, userID, c.id, c.pct, bookingTotal, discountAmount); err != nil { log.Printf("Failed to insert anniversary discount: %v", err) } else { - tx.Exec(ctx, ` + q.Exec(ctx, ` INSERT INTO payments (booking_id, payment_type, payment_method, amount, status, created_by) VALUES ($1, 'partial', 'discount', $2, 'completed', $3) `, bookingID, discountAmount, userID) - tx.Exec(ctx, ` + q.Exec(ctx, ` UPDATE discount_campaigns SET times_redeemed = times_redeemed + 1 WHERE id = $1 `, c.id) } @@ -1170,15 +1208,15 @@ func applyEligibleCampaignsAtPayment(ctx context.Context, bookingID string, user } var firstPaymentMethod string - if err := tx.QueryRow(ctx, ` + if err := q.QueryRow(ctx, ` SELECT payment_method FROM payments WHERE booking_id = $1 AND payment_method NOT IN ('discount', 'on_the_house') ORDER BY created_at ASC LIMIT 1 `, bookingID).Scan(&firstPaymentMethod); err == nil && firstPaymentMethod == "in_person_card" { var globalCount int - tx.QueryRow(ctx, `SELECT COUNT(*) FROM bookings WHERE status = 'completed'`).Scan(&globalCount) + q.QueryRow(ctx, `SELECT COUNT(*) FROM bookings WHERE status = 'completed'`).Scan(&globalCount) var globalCampaignID string var globalPercent float64 - tx.QueryRow(ctx, ` + q.QueryRow(ctx, ` SELECT id, discount_percent FROM discount_campaigns WHERE status = 'active' AND campaign_type = 'milestone' AND milestone_type = 'global_booking_count' AND milestone_value <= $1 @@ -1189,20 +1227,20 @@ func applyEligibleCampaignsAtPayment(ctx context.Context, bookingID string, user if globalCampaignID != "" { var exists int - tx.QueryRow(ctx, `SELECT 1 FROM booking_discounts WHERE booking_id = $1 AND source_id = $2`, bookingID, globalCampaignID).Scan(&exists) + q.QueryRow(ctx, `SELECT 1 FROM booking_discounts WHERE booking_id = $1 AND source_id = $2`, bookingID, globalCampaignID).Scan(&exists) if exists == 0 { discountAmount := roundTo2(bookingTotal * globalPercent / 100) - if _, err := tx.Exec(ctx, ` + if _, err := q.Exec(ctx, ` INSERT INTO booking_discounts (booking_id, user_id, discount_source, source_id, campaign_type, milestone_type, discount_percent, original_total, discount_amount) VALUES ($1, $2, 'campaign', $3, 'milestone', 'global_booking_count', $4, $5, $6) `, bookingID, userID, globalCampaignID, globalPercent, bookingTotal, discountAmount); err != nil { log.Printf("Failed to insert global milestone discount: %v", err) } else { - tx.Exec(ctx, ` + q.Exec(ctx, ` INSERT INTO payments (booking_id, payment_type, payment_method, amount, status, created_by) VALUES ($1, 'partial', 'discount', $2, 'completed', $3) `, bookingID, discountAmount, userID) - tx.Exec(ctx, ` + q.Exec(ctx, ` UPDATE discount_campaigns SET times_redeemed = times_redeemed + 1 WHERE id = $1 `, globalCampaignID) } @@ -1214,34 +1252,30 @@ func applyEligibleCampaignsAtPayment(ctx context.Context, bookingID string, user if bookingTotal > 0 { var rdID string var rdPercent float64 - if err := tx.QueryRow(ctx, ` + if err := q.QueryRow(ctx, ` SELECT id, discount_percent FROM referral_discounts WHERE user_id = $1 AND used = FALSE LIMIT 1 `, userID).Scan(&rdID, &rdPercent); err == nil && rdID != "" { exists := 0 - tx.QueryRow(ctx, `SELECT 1 FROM booking_discounts WHERE booking_id = $1 AND discount_source = 'referral' AND source_id = $2`, bookingID, rdID).Scan(&exists) + q.QueryRow(ctx, `SELECT 1 FROM booking_discounts WHERE booking_id = $1 AND discount_source = 'referral' AND source_id = $2`, bookingID, rdID).Scan(&exists) if exists == 0 { discountAmount := roundTo2(bookingTotal * rdPercent / 100) - if _, err := tx.Exec(ctx, ` + if _, err := q.Exec(ctx, ` INSERT INTO booking_discounts (booking_id, user_id, discount_source, source_id, campaign_type, milestone_type, discount_percent, original_total, discount_amount) VALUES ($1, $2, 'referral', $3, NULL, NULL, $4, $5, $6) `, bookingID, userID, rdID, rdPercent, bookingTotal, discountAmount); err == nil { - tx.Exec(ctx, ` + q.Exec(ctx, ` INSERT INTO payments (booking_id, payment_type, payment_method, amount, status, created_by) VALUES ($1, 'partial', 'discount', $2, 'completed', $3) `, bookingID, discountAmount, userID) - tx.Exec(ctx, ` + q.Exec(ctx, ` UPDATE referral_discounts SET used = TRUE, used_at = NOW() WHERE id = $1 `, rdID) } } } } - - if err := tx.Commit(ctx); err != nil { - log.Printf("Failed to commit discount application: %v", err) - } } // buildSplitRecords determines whether to split a single Square charge into @@ -1258,7 +1292,7 @@ func applyEligibleCampaignsAtPayment(ctx context.Context, bookingID string, user func buildSplitRecords(primary PaymentRecord, reqPaymentType string, info *BookingPaymentInfo, paymentAmount float64) []PaymentRecord { // After the booking starts there is no deposit protection window — // record the payment as a single entry with its original type. - if time.Now().After(info.StartTime) { + if clock.Now().After(info.StartTime) { return []PaymentRecord{primary} } @@ -1374,7 +1408,6 @@ func GetUserPaymentMethods(w http.ResponseWriter, r *http.Request) { return } - w.Header().Set("Content-Type", "application/json") json.NewEncoder(w).Encode(cards) } @@ -1393,7 +1426,6 @@ func AdminGetUserPaymentMethods(w http.ResponseWriter, r *http.Request) { return } - w.Header().Set("Content-Type", "application/json") json.NewEncoder(w).Encode(cards) } @@ -1418,7 +1450,6 @@ func DeletePaymentMethod(w http.ResponseWriter, r *http.Request) { return } - w.Header().Set("Content-Type", "application/json") json.NewEncoder(w).Encode(map[string]string{"status": "deleted"}) } @@ -1466,7 +1497,6 @@ func CreatePaymentMethod(w http.ResponseWriter, r *http.Request) { return } - w.Header().Set("Content-Type", "application/json") json.NewEncoder(w).Encode(card) } @@ -1535,6 +1565,18 @@ func RefundPayment(w http.ResponseWriter, r *http.Request) { return } + // Begin a transaction so that the Square refund and the DB record are + // atomically linked. If the commit fails after Square processes the refund, + // a CRITICAL log alerts monitoring — the Square refund cannot be reversed, + // but the DB record can be recreated from the log. + tx, err := db.Conn.Begin(r.Context()) + if err != nil { + log.Printf("Failed to begin transaction for refund: %v", err) + http.Error(w, "internal server error", http.StatusInternalServerError) + return + } + defer tx.Rollback(r.Context()) + refundReq := square.RefundPaymentReq{ PaymentID: *payment.SquarePaymentID, Amount: req.Amount, @@ -1550,32 +1592,40 @@ func RefundPayment(w http.ResponseWriter, r *http.Request) { } squareRefundID := refundResult.ID - record := RefundRecord{ - PaymentID: paymentID, - BookingID: payment.BookingID, - Amount: float64(req.Amount) / 100.0, - SquareRefundID: &squareRefundID, - Status: "completed", - Reason: req.Reason, - CreatedBy: &adminID, - CreatedAt: time.Now(), - } - refundID, err := service.CreateRefundRecord(r.Context(), record) + var refundID string + err = tx.QueryRow(r.Context(), ` + INSERT INTO refunds (payment_id, booking_id, amount, square_refund_id, status, reason, created_by, created_at) + VALUES ($1, $2, $3, $4, 'completed', $5, $6, $7) + RETURNING id + `, + paymentID, + payment.BookingID, + float64(req.Amount)/100.0, + squareRefundID, + req.Reason, + adminID, + clock.Now(), + ).Scan(&refundID) if err != nil { log.Printf("Failed to create refund record: %v", err) http.Error(w, "internal server error", http.StatusInternalServerError) return } - w.Header().Set("Content-Type", "application/json") + if err := tx.Commit(r.Context()); err != nil { + log.Printf("CRITICAL: Refund committed by Square (%s) but DB transaction failed — refund record %s may be missing: %v", squareRefundID, refundID, err) + http.Error(w, "internal server error", http.StatusInternalServerError) + return + } + json.NewEncoder(w).Encode(RefundResponse{ ID: refundID, PaymentID: paymentID, Amount: req.Amount, Status: "completed", Reason: req.Reason, - CreatedAt: time.Now().Format(time.RFC3339), + CreatedAt: clock.Now().Format(time.RFC3339), }) } @@ -1649,34 +1699,19 @@ func CreateTipPayment(w http.ResponseWriter, r *http.Request) { idempotencyKey := bookingID + "-tip-" + strconv.FormatInt(req.Amount, 10) - paymentReq := square.CreatePaymentReq{ - Amount: req.Amount, - Currency: "GBP", - SourceID: req.CardToken, - IdempotencyKey: idempotencyKey, - ReferenceID: bookingID, - Note: "tip", - } - - paymentResult, err := SquareClient.CreatePayment(r.Context(), paymentReq) - if err != nil { - log.Printf("Failed to create tip payment: %v", err) - http.Error(w, "Payment failed", http.StatusPaymentRequired) - return - } - + // Step 1: Insert payment record in 'pending' state inside a DB transaction. + // Square is NOT called yet — if the tx fails, no harm done. record := PaymentRecord{ - BookingID: bookingID, - PaymentType: "tip", - PaymentMethod: "online_square", - Status: "completed", - Amount: float64(req.Amount) / 100.0, - SquarePaymentID: &paymentResult.SquarePayID, - IdempotencyKey: &idempotencyKey, - Fees: 0, - CreatedAt: time.Now(), - UpdatedAt: time.Now(), - CreatedBy: &userID, + BookingID: bookingID, + PaymentType: "tip", + PaymentMethod: "online_square", + Status: "pending", + Amount: float64(req.Amount) / 100.0, + IdempotencyKey: &idempotencyKey, + Fees: 0, + CreatedAt: clock.Now(), + UpdatedAt: clock.Now(), + CreatedBy: &userID, } tx, err := db.Conn.Begin(r.Context()) @@ -1685,10 +1720,10 @@ func CreateTipPayment(w http.ResponseWriter, r *http.Request) { http.Error(w, "internal server error", http.StatusInternalServerError) return } - defer tx.Rollback(r.Context()) paymentID, err := service.CreatePaymentRecordTx(r.Context(), tx, record, nil) if err != nil { + tx.Rollback(r.Context()) log.Printf("Failed to create payment record: %v", err) http.Error(w, "internal server error", http.StatusInternalServerError) return @@ -1701,7 +1736,38 @@ func CreateTipPayment(w http.ResponseWriter, r *http.Request) { return } - w.Header().Set("Content-Type", "application/json") + // Step 2: DB transaction committed — safe to call Square now. + // If Square fails, the record stays 'pending' for manual retry. + paymentReq := square.CreatePaymentReq{ + Amount: req.Amount, + Currency: "GBP", + SourceID: req.CardToken, + IdempotencyKey: idempotencyKey, + ReferenceID: bookingID, + Note: "tip", + } + + paymentResult, err := SquareClient.CreatePayment(r.Context(), paymentReq) + if err != nil { + log.Printf("Failed to create tip payment: %v", err) + // Payment record intentionally left as 'pending' for manual retry. + http.Error(w, "Payment failed", http.StatusPaymentRequired) + return + } + + // Step 3: Square succeeded — update the payment record. + _, upErr := db.Conn.Exec(r.Context(), + `UPDATE payments SET status = 'completed', square_payment_id = $1 WHERE id = $2`, + paymentResult.SquarePayID, paymentID, + ) + if upErr != nil { + log.Printf("Failed to update payment %s after Square success: %v (square_payment_id=%s)", paymentID, upErr, paymentResult.SquarePayID) + // Square charge succeeded but status update failed. + // Record stays 'pending' for manual reconciliation. + http.Error(w, "internal server error", http.StatusInternalServerError) + return + } + json.NewEncoder(w).Encode(PaymentResponse{ ID: paymentID, BookingID: bookingID, @@ -1711,7 +1777,7 @@ func CreateTipPayment(w http.ResponseWriter, r *http.Request) { CardBrand: paymentResult.CardBrand, CardLast4: paymentResult.CardLast4, ReceiptURL: paymentResult.ReceiptURL, - CreatedAt: time.Now().Format(time.RFC3339), + CreatedAt: clock.Now().Format(time.RFC3339), }) } @@ -1777,7 +1843,6 @@ func GetBookingPaymentSummary(w http.ResponseWriter, r *http.Request) { } } - w.Header().Set("Content-Type", "application/json") json.NewEncoder(w).Encode(PaymentSummaryResponse{ TotalAmount: int64(summary.TotalAmount * 100), PaidAmount: int64(summary.PaidAmount * 100), @@ -1844,16 +1909,26 @@ func AcquirePaymentLock(w http.ResponseWriter, r *http.Request) { return } - // Upsert the time_blocker: delete any existing PAYMENT_IN_FLIGHT for this - // booking, then insert a fresh one. This effectively extends the lock. - if _, err := db.Conn.Exec(r.Context(), ` + // Upsert the time_blocker atomically: delete old PAYMENT_IN_FLIGHT and insert + // a fresh one in a single transaction. Prevents lock loss if INSERT fails. + tx, err := db.Conn.Begin(r.Context()) + if err != nil { + log.Printf("Failed to start transaction for payment lock: %v", err) + http.Error(w, "Internal server error", http.StatusInternalServerError) + return + } + defer tx.Rollback(r.Context()) + + if _, err := tx.Exec(r.Context(), ` DELETE FROM time_blockers WHERE description = 'PAYMENT_IN_FLIGHT:' || $1 `, bookingID); err != nil { log.Printf("Failed to clear previous payment lock for booking %s: %v", bookingID, err) + http.Error(w, "Internal server error", http.StatusInternalServerError) + return } - if _, err := db.Conn.Exec(r.Context(), ` + if _, err := tx.Exec(r.Context(), ` INSERT INTO time_blockers (start_time, duration_minutes, description, created_by) VALUES (NOW(), $1, $2, $3) `, PaymentLockDuration, "PAYMENT_IN_FLIGHT:"+bookingID, userID); err != nil { @@ -1862,7 +1937,12 @@ func AcquirePaymentLock(w http.ResponseWriter, r *http.Request) { return } - w.Header().Set("Content-Type", "application/json") + if err := tx.Commit(r.Context()); err != nil { + log.Printf("Failed to commit payment lock transaction: %v", err) + http.Error(w, "Internal server error", http.StatusInternalServerError) + return + } + json.NewEncoder(w).Encode(map[string]interface{}{ "status": "locked", "ttl_min": PaymentLockDuration, @@ -1878,7 +1958,15 @@ func ReleasePaymentLock(w http.ResponseWriter, r *http.Request) { return } - if _, err := db.Conn.Exec(r.Context(), ` + tx, err := db.Conn.Begin(r.Context()) + if err != nil { + log.Printf("Failed to begin transaction: %v", err) + http.Error(w, "Internal server error", http.StatusInternalServerError) + return + } + defer tx.Rollback(r.Context()) + + if _, err := tx.Exec(r.Context(), ` DELETE FROM time_blockers WHERE description = 'PAYMENT_IN_FLIGHT:' || $1 `, bookingID); err != nil { @@ -1887,5 +1975,11 @@ func ReleasePaymentLock(w http.ResponseWriter, r *http.Request) { return } + if err := tx.Commit(r.Context()); err != nil { + log.Printf("Failed to commit transaction: %v", err) + http.Error(w, "Internal server error", http.StatusInternalServerError) + return + } + w.WriteHeader(http.StatusNoContent) } diff --git a/backend/handlers/payments/loyalty.go b/backend/handlers/payments/loyalty.go index a406ba4..7346922 100644 --- a/backend/handlers/payments/loyalty.go +++ b/backend/handlers/payments/loyalty.go @@ -150,7 +150,7 @@ func ApplyLoyaltyRedemption(w http.ResponseWriter, r *http.Request) { return } - w.Header().Set("Content-Type", "application/json") + json.NewEncoder(w).Encode(map[string]interface{}{ "success": true, "discount_amount": discountAmount, diff --git a/backend/handlers/payments/loyalty_test.go b/backend/handlers/payments/loyalty_test.go index 0cb9bff..e5334b6 100644 --- a/backend/handlers/payments/loyalty_test.go +++ b/backend/handlers/payments/loyalty_test.go @@ -11,6 +11,7 @@ import ( "testing" "time" + "crussell/clock" "crussell/db" "crussell/testutils" "crussell/mw" @@ -238,7 +239,7 @@ func TestCampaignAutoApply_TimeBased(t *testing.T) { userID, bookingID, _ := setupCampaignTest(t, ctx, tx) // Create an active time-based campaign - now := time.Now() + now := clock.Now() var campaignID string err := tx.QueryRow(ctx, ` INSERT INTO discount_campaigns (name, campaign_type, discount_percent, status, start_date, end_date, times_redeemed) @@ -259,7 +260,7 @@ func TestCampaignAutoApply_TimeBased(t *testing.T) { } // Call applyEligibleCampaignsAtPayment - applyEligibleCampaignsAtPayment(ctx, bookingID, userID) + applyEligibleCampaignsAtPayment(ctx, tx, bookingID, userID) // Verify booking_discounts was created var discountCount int @@ -311,7 +312,7 @@ func TestCampaignAutoApply_UserMilestone(t *testing.T) { t.Fatalf("failed to create payment: %v", err) } - applyEligibleCampaignsAtPayment(ctx, bookingID, userID) + applyEligibleCampaignsAtPayment(ctx, tx, bookingID, userID) var discountCount int tx.QueryRow(ctx, @@ -327,7 +328,7 @@ func TestCampaignAutoApply_GlobalMilestoneSkippedOnline(t *testing.T) { userID, bookingID, _ := setupCampaignTest(t, ctx, tx) // Set global completed count high enough - now := time.Now() + now := clock.Now() for i := 0; i < 100; i++ { tx.QueryRow(ctx, ` INSERT INTO bookings (user_id, start_time, status) VALUES ($1, $2, 'completed') RETURNING id @@ -354,7 +355,7 @@ func TestCampaignAutoApply_GlobalMilestoneSkippedOnline(t *testing.T) { t.Fatalf("failed to create payment: %v", err) } - applyEligibleCampaignsAtPayment(ctx, bookingID, userID) + applyEligibleCampaignsAtPayment(ctx, tx, bookingID, userID) // Verify NO discount was applied (global milestone skipped for online payment) var discountCount int @@ -399,7 +400,7 @@ func TestCampaignAutoApply_GlobalMilestoneAppliedInPerson(t *testing.T) { tx.Exec(ctx, "UPDATE bookings SET status = 'confirmed', updated_at = NOW() WHERE id = $1", bookingID) - applyEligibleCampaignsAtPayment(ctx, bookingID, userID) + applyEligibleCampaignsAtPayment(ctx, tx, bookingID, userID) var discountCount int tx.QueryRow(ctx, @@ -414,7 +415,7 @@ func TestCampaignAutoApply_DoubleApplyGuard(t *testing.T) { ctx, tx := testutils.SetupTestTx(t) userID, bookingID, _ := setupCampaignTest(t, ctx, tx) - now := time.Now() + now := clock.Now() var campaignID string err := tx.QueryRow(ctx, ` INSERT INTO discount_campaigns (name, campaign_type, discount_percent, status, start_date, end_date, times_redeemed) @@ -444,7 +445,7 @@ func TestCampaignAutoApply_DoubleApplyGuard(t *testing.T) { VALUES ($1, 'deposit', 'online_square', 2500, 'completed', NOW(), NOW()) `, bookingID) - applyEligibleCampaignsAtPayment(ctx, bookingID, userID) + applyEligibleCampaignsAtPayment(ctx, tx, bookingID, userID) // Verify still only 1 discount var discountCount int @@ -494,7 +495,7 @@ func TestCampaignAutoApply_ReferralDiscount(t *testing.T) { t.Fatalf("failed to create payment: %v", err) } - applyEligibleCampaignsAtPayment(ctx, bookingID, userID) + applyEligibleCampaignsAtPayment(ctx, tx, bookingID, userID) // Verify referral discount was applied var discountCount int @@ -561,7 +562,7 @@ func TestCampaignAutoApply_ReferralDiscount_DoubleApplyGuard(t *testing.T) { t.Fatalf("failed to insert existing booking discount: %v", err) } - applyEligibleCampaignsAtPayment(ctx, bookingID, userID) + applyEligibleCampaignsAtPayment(ctx, tx, bookingID, userID) // Verify no second referral discount was applied var discountCount int @@ -606,7 +607,7 @@ func TestCampaignAutoApply_ReferralDiscount_AlreadyUsed(t *testing.T) { t.Fatalf("failed to insert used referral discount: %v", err) } - applyEligibleCampaignsAtPayment(ctx, bookingID, userID) + applyEligibleCampaignsAtPayment(ctx, tx, bookingID, userID) var discountCount int tx.QueryRow(ctx, diff --git a/backend/handlers/payments/payments_test.go b/backend/handlers/payments/payments_test.go index 73f5b0b..21bb20f 100644 --- a/backend/handlers/payments/payments_test.go +++ b/backend/handlers/payments/payments_test.go @@ -14,6 +14,7 @@ import ( "testing" "time" + "crussell/clock" "crussell/db" "crussell/testutils" "crussell/mw" @@ -211,7 +212,7 @@ func setupTestData(t *testing.T, ctx context.Context, q db.Querier) (string, str // to prevent payment-split logic from triggering. Used by tests that verify // payment sequencing or idempotency rather than deposit allocation. func setupTestDataPast(t *testing.T, ctx context.Context, q db.Querier) (string, string, string) { - return setupTestDataAtTime(t, ctx, q, time.Now().Add(-1*time.Hour)) + return setupTestDataAtTime(t, ctx, q, clock.Now().Add(-1*time.Hour)) } func setupTestDataAtTime(t *testing.T, ctx context.Context, q db.Querier, startTime time.Time) (string, string, string) { @@ -1035,7 +1036,7 @@ func setupDepositBooking(t *testing.T, ctx context.Context, q db.Querier) (strin // (1 hour ago). This prevents the payment-split logic from triggering, which is // useful for tests that verify payment sequencing rather than deposit splitting. func setupDepositBookingPast(t *testing.T, ctx context.Context, q db.Querier) (string, string) { - return setupDepositBookingAtTime(t, ctx, q, time.Now().Add(-1*time.Hour)) + return setupDepositBookingAtTime(t, ctx, q, clock.Now().Add(-1*time.Hour)) } func setupDepositBookingAtTime(t *testing.T, ctx context.Context, q db.Querier, startTime time.Time) (string, string) { @@ -1355,7 +1356,7 @@ func TestBookingPayment_TransactionAtomicity_SplitRollsBackOnError(t *testing.T) svc := NewPaymentService() - now := time.Now() + now := clock.Now() // First record — valid. pid1, err := svc.CreatePaymentRecordTx(ctx, innerTx, PaymentRecord{ BookingID: bookingID, @@ -1485,7 +1486,7 @@ func TestNonDepositPaymentType_DepositReqTypeBecomesPartial(t *testing.T) { // --------------------------------------------------------------------------- func makeTestRecord(bookingID, ptype string, amount float64) PaymentRecord { - now := time.Now() + now := clock.Now() key := "test-key" return PaymentRecord{ BookingID: bookingID, @@ -1507,7 +1508,7 @@ func TestBuildSplitRecords_FutureBooking_FullPayment_Splits(t *testing.T) { // £50 payment on a £50 future booking → splits into deposit £25 + balance £25 record := makeTestRecord("b1", "full", 50) info := &BookingPaymentInfo{ - StartTime: time.Now().Add(48 * time.Hour), + StartTime: clock.Now().Add(48 * time.Hour), TotalAmount: 50, TotalPaid: 0, } @@ -1546,7 +1547,7 @@ func TestBuildSplitRecords_PastBooking_NoSplit(t *testing.T) { // Same amount on a PAST booking → single record record := makeTestRecord("b2", "full", 50) info := &BookingPaymentInfo{ - StartTime: time.Now().Add(-2 * time.Hour), + StartTime: clock.Now().Add(-2 * time.Hour), TotalAmount: 50, TotalPaid: 0, } @@ -1564,7 +1565,7 @@ func TestBuildSplitRecords_DepositWithinCap_NoSplit(t *testing.T) { // £20 deposit on a £50 total (40% < 50% cap) → single deposit record record := makeTestRecord("b3", "deposit", 20) info := &BookingPaymentInfo{ - StartTime: time.Now().Add(48 * time.Hour), + StartTime: clock.Now().Add(48 * time.Hour), TotalAmount: 50, TotalPaid: 0, } @@ -1582,7 +1583,7 @@ func TestBuildSplitRecords_PaymentLessThanDepositMax_NoSplit(t *testing.T) { // £25 on a £100 total (25% < 50% cap) → single deposit record record := makeTestRecord("b4", "deposit", 25) info := &BookingPaymentInfo{ - StartTime: time.Now().Add(48 * time.Hour), + StartTime: clock.Now().Add(48 * time.Hour), TotalAmount: 100, TotalPaid: 0, } @@ -1910,6 +1911,44 @@ func TestTipPayment_MultipleTipsAllowed(t *testing.T) { } } +func TestTipPayment_TransactionFailure_SkipsSquare(t *testing.T) { + t.Parallel() + ctx, tx := testutils.SetupTestTx(t) + + userID, bookingID, _ := setupTestData(t, ctx, tx) + + _, err := fixtures.CreateTestPayment(tx, bookingID, 5000.00, "online_square", "full", "completed") + if err != nil { + t.Fatalf("failed to create payment: %v", err) + } + + userToken := jwt.GenerateUserToken(userID) + + cancelCtx, cancel := context.WithCancel(ctx) + cancel() + + req := CreateTipPaymentRequest{ + Amount: 500, + CardToken: "cnon:tip-card", + } + + handler := CreateTipPayment + w := makePaymentRequest(handler, "POST", "/api/bookings/"+bookingID+"/tip", req, userToken, cancelCtx) + + if w.Code != http.StatusInternalServerError { + t.Errorf("expected status 500, got %d. body: %s", w.Code, w.Body.String()) + } + + var completedTipCount int + err = tx.QueryRow(ctx, "SELECT COUNT(*) FROM payments WHERE booking_id = $1 AND payment_type = 'tip' AND status = 'completed'", bookingID).Scan(&completedTipCount) + if err != nil { + t.Errorf("failed to query completed tip payments: %v", err) + } + if completedTipCount != 0 { + t.Errorf("expected 0 completed tip payments, got %d", completedTipCount) + } +} + func TestGetUserPaymentMethods_NoCards(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 2097e9a..2b3c989 100644 --- a/backend/handlers/payments/refunds.go +++ b/backend/handlers/payments/refunds.go @@ -7,7 +7,10 @@ import ( "time" "crussell/db" + "crussell/clock" "crussell/internal/square" + + "github.com/jackc/pgx/v5" ) type RefundCalculationResult struct { @@ -83,8 +86,12 @@ 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. -func ProcessCancellationRefund( +// 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. +func ProcessCancellationRefundTx( ctx context.Context, + tx pgx.Tx, bookingID string, subtotal float64, totalPrePaid float64, @@ -102,7 +109,7 @@ func ProcessCancellationRefund( // Get the booking's user info for refund routing. var bookingUserID string var isGuest bool - if err := db.Conn.QueryRow(ctx, ` + if err := tx.QueryRow(ctx, ` SELECT b.user_id, COALESCE(u.account_role = 'guest', false) FROM bookings b LEFT JOIN users u ON b.user_id = u.id @@ -112,7 +119,7 @@ func ProcessCancellationRefund( // Non-fatal — we'll still process Square refunds but skip balance credits. } - rows, err := db.Conn.Query(ctx, ` + rows, err := tx.Query(ctx, ` SELECT id, amount, payment_method, square_payment_id, gift_card_id FROM payments WHERE booking_id = $1 AND status = 'completed' AND payment_method NOT IN ('discount', 'on_the_house') @@ -122,6 +129,194 @@ func ProcessCancellationRefund( log.Printf("Failed to fetch payments for refund: %v", err) return &calc, nil } + 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 + if err := rows.Scan(&p.ID, &p.Amount, &p.PaymentMethod, &p.SquarePaymentID, &p.GiftCardID); err != nil { + log.Printf("Failed to scan payment row: %v", err) + continue + } + payments = append(payments, p) + } + if err := rows.Err(); err != nil { + log.Printf("Payment row iteration error: %v", err) + } + + refundRemaining := calc.RefundableAmount + + for _, p := range payments { + if refundRemaining <= 0 { + break + } + + paymentID := p.ID + paymentMethod := p.PaymentMethod + amount := p.Amount + giftCardID := p.GiftCardID + + refundThisPayment := math.Min(amount, refundRemaining) + var squareRefundID *string + + switch paymentMethod { + case "online_square", "in_person_card": + // Square API refund is processed AFTER the transaction commits + // (see ProcessPendingSquareRefunds). Inside the tx we only record + // the refund record as "pending" for post-commit processing. + if isGuest || bookingUserID == "" { + log.Printf("Guest card refund: booking %s, payment %s, amount £%.2f — will be processed after commit", bookingID, paymentID, refundThisPayment) + } + // squareRefundID stays nil — will be set by ProcessPendingSquareRefunds + + case "giftcard": + if giftCardID == nil || *giftCardID == "" { + log.Printf("Giftcard payment %s has no gift_card_id — cannot refund to card. Skipping.", paymentID) + break + } + var expired bool + if err := tx.QueryRow(ctx, ` + SELECT expiry_date IS NOT NULL AND expiry_date < NOW() + FROM gift_cards WHERE id = $1 + `, *giftCardID).Scan(&expired); err != nil { + log.Printf("Failed to check gift card %s expiry: %v — proceeding with refund", *giftCardID, err) + } else if expired { + log.Printf("Gift card %s has expired — money retained by salon, no refund due for booking %s", *giftCardID, bookingID) + break + } + if _, err := tx.Exec(ctx, ` + UPDATE gift_cards SET amount_remaining = amount_remaining + $1, last_used_at = NOW() + WHERE id = $2 + `, refundThisPayment, *giftCardID); err != nil { + log.Printf("Failed to refund £%.2f to gift card %s: %v", refundThisPayment, *giftCardID, err) + break + } + if _, err := tx.Exec(ctx, ` + INSERT INTO gift_card_transactions (gift_card_id, transaction_type, amount, reference_type, reference_id, user_id, notes) + VALUES ($1, 'refund', $2, 'booking', $3, $4, $5) + `, *giftCardID, refundThisPayment, bookingID, bookingUserID, "Refund from cancelled booking"); err != nil { + log.Printf("Failed to create gift card transaction for refund: %v", err) + } + + case "cash": + if isGuest || bookingUserID == "" { + log.Printf("Guest cash refund: booking %s, payment %s, amount £%.2f — admin must process cash refund at till", bookingID, paymentID, refundThisPayment) + } else { + log.Printf("Crediting £%.2f to user %s balance for cash payment %s", refundThisPayment, bookingUserID, paymentID) + if _, balErr := 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() + `, bookingUserID, refundThisPayment); balErr != nil { + log.Printf("Failed to credit user %s balance for refund of booking %s: %v", bookingUserID, bookingID, balErr) + } + } + + default: + log.Printf("Skipping refund for payment %s with method %q (no money exchanged)", paymentID, paymentMethod) + } + + recordStatus := "completed" + if paymentMethod == "online_square" || paymentMethod == "in_person_card" { + recordStatus = "pending" + } + record := RefundRecord{ + PaymentID: paymentID, + BookingID: bookingID, + Amount: refundThisPayment, + SquareRefundID: squareRefundID, + Status: recordStatus, + Reason: reason, + 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) + if dbErr != nil { + log.Printf("Failed to create refund record for payment %s: %v", paymentID, dbErr) + continue + } + + refundRemaining -= refundThisPayment + } + + if bookingUserID != "" { + var loyaltyUsed bool + if err := tx.QueryRow(ctx, "SELECT EXISTS(SELECT 1 FROM booking_discounts WHERE booking_id = $1 AND discount_source = 'loyalty')", bookingID).Scan(&loyaltyUsed); err != nil { + log.Printf("Failed to check loyalty stamp refund for booking %s: %v", bookingID, err) + } else if loyaltyUsed { + _, loyaltyErr := tx.Exec(ctx, "UPDATE users SET loyalty_stamps = loyalty_stamps + $1 WHERE id = $2", LoyaltyStampCost, bookingUserID) + if loyaltyErr != nil { + log.Printf("Failed to refund loyalty stamps for booking %s: %v", bookingID, loyaltyErr) + } else { + log.Printf("Refunded %d loyalty stamps to user %s after cancellation of booking %s", LoyaltyStampCost, bookingUserID, bookingID) + } + } + } + + return &calc, nil +} + +func ProcessCancellationRefund( + ctx context.Context, + bookingID string, + subtotal float64, + totalPrePaid float64, + startTime time.Time, + cancellationTime time.Time, + reason string, + actorID *string, +) (*RefundCalculationResult, error) { + calc := CalculateRefundForCancellation(subtotal, totalPrePaid, cancellationTime, startTime) + + if calc.RefundableAmount <= 0 { + return &calc, nil + } + + tx, err := db.Conn.Begin(ctx) + if err != nil { + log.Printf("Failed to begin transaction for cancellation refund: %v", err) + return &calc, nil + } + defer tx.Rollback(ctx) + + // Get the booking's user info for refund routing. + var bookingUserID string + var isGuest bool + if err := tx.QueryRow(ctx, ` + SELECT b.user_id, COALESCE(u.account_role = 'guest', false) + FROM bookings b + LEFT JOIN users u ON b.user_id = u.id + WHERE b.id = $1 + `, bookingID).Scan(&bookingUserID, &isGuest); err != nil { + log.Printf("Failed to get booking user info for refund: %v", err) + // Non-fatal — we'll still process Square refunds but skip balance credits. + } + + rows, err := tx.Query(ctx, ` + SELECT id, amount, payment_method, square_payment_id, gift_card_id + FROM payments + WHERE booking_id = $1 AND status = 'completed' AND payment_method NOT IN ('discount', 'on_the_house') + ORDER BY created_at ASC + `, bookingID) + if err != nil { + log.Printf("Failed to fetch payments for refund: %v", err) + return &calc, nil + } + defer rows.Close() // Read all payments into a slice, then close rows immediately. // This avoids "conn busy" errors when db.Conn.QueryRow/Exec are called @@ -143,13 +338,11 @@ func ProcessCancellationRefund( } payments = append(payments, p) } - rows.Close() + if err := rows.Err(); err != nil { + log.Printf("Payment row iteration error: %v", err) + } refundRemaining := calc.RefundableAmount - // Track which Square payment IDs have already been refunded through Square. - // Multiple split payment records (deposit + balance) can share the same - // square_payment_id — we must only refund each Square payment once. - refundedSquareIDs := make(map[string]bool) for _, p := range payments { if refundRemaining <= 0 { @@ -159,46 +352,20 @@ func ProcessCancellationRefund( paymentID := p.ID paymentMethod := p.PaymentMethod amount := p.Amount - squarePaymentID := p.SquarePaymentID giftCardID := p.GiftCardID refundThisPayment := math.Min(amount, refundRemaining) - refundCents := int64(math.Round(refundThisPayment * 100)) var squareRefundID *string switch paymentMethod { case "online_square", "in_person_card": - // Card payments can be refunded through Square if we have a payment reference. - // Skip the Square API call if this square_payment_id was already processed - // (possible when split payment records share the same charge). - if squarePaymentID != nil && *squarePaymentID != "" && !refundedSquareIDs[*squarePaymentID] { - refundReq := square.RefundPaymentReq{ - PaymentID: *squarePaymentID, - Amount: refundCents, - IdempotencyKey: paymentID + "-cancel-" + time.Now().Format("20060102150405"), - Reason: reason, - } - result, sqErr := SquareClient.RefundPayment(ctx, refundReq) - if sqErr != nil { - log.Printf("Square refund failed for payment %s (will record refund locally): %v", paymentID, sqErr) - } else { - squareRefundID = &result.ID - refundedSquareIDs[*squarePaymentID] = true - } - } else if squarePaymentID != nil && refundedSquareIDs[*squarePaymentID] { - log.Printf("Square payment %s already refunded through split record %s — crediting balance for £%.2f", *squarePaymentID, paymentID, refundThisPayment) - } - - // If Square refund failed or wasn't available, credit the user's balance. - // Guests don't get balance credits — admin handles those manually. - if squareRefundID == nil && bookingUserID != "" { - if isGuest { - log.Printf("Guest card refund (Square unavailable): booking %s, payment %s, amount £%.2f — admin must process at till", bookingID, paymentID, refundThisPayment) - } else { - log.Printf("Crediting £%.2f to user %s balance for card payment %s (Square refund unavailable)", refundThisPayment, bookingUserID, paymentID) - creditUserBalance(ctx, bookingUserID, bookingID, paymentID, refundThisPayment, reason) - } + // Square API refund is processed AFTER the transaction commits + // (see ProcessPendingSquareRefunds). Inside the tx we only record + // the refund record as "pending" for post-commit processing. + if isGuest || bookingUserID == "" { + log.Printf("Guest card refund: booking %s, payment %s, amount £%.2f — will be processed after commit", bookingID, paymentID, refundThisPayment) } + // squareRefundID stays nil — will be set by ProcessPendingSquareRefunds case "giftcard": if giftCardID == nil || *giftCardID == "" { @@ -206,7 +373,7 @@ func ProcessCancellationRefund( break } var expired bool - if err := db.Conn.QueryRow(ctx, ` + if err := tx.QueryRow(ctx, ` SELECT expiry_date IS NOT NULL AND expiry_date < NOW() FROM gift_cards WHERE id = $1 `, *giftCardID).Scan(&expired); err != nil { @@ -215,14 +382,14 @@ func ProcessCancellationRefund( log.Printf("Gift card %s has expired — money retained by salon, no refund due for booking %s", *giftCardID, bookingID) break } - if _, err := db.Conn.Exec(ctx, ` + if _, err := tx.Exec(ctx, ` UPDATE gift_cards SET amount_remaining = amount_remaining + $1, last_used_at = NOW() WHERE id = $2 `, refundThisPayment, *giftCardID); err != nil { log.Printf("Failed to refund £%.2f to gift card %s: %v", refundThisPayment, *giftCardID, err) break } - if _, err := db.Conn.Exec(ctx, ` + if _, err := tx.Exec(ctx, ` INSERT INTO gift_card_transactions (gift_card_id, transaction_type, amount, reference_type, reference_id, user_id, notes) VALUES ($1, 'refund', $2, 'booking', $3, $4, $5) `, *giftCardID, refundThisPayment, bookingID, bookingUserID, "Refund from cancelled booking"); err != nil { @@ -234,7 +401,15 @@ func ProcessCancellationRefund( log.Printf("Guest cash refund: booking %s, payment %s, amount £%.2f — admin must process cash refund at till", bookingID, paymentID, refundThisPayment) } else { log.Printf("Crediting £%.2f to user %s balance for cash payment %s", refundThisPayment, bookingUserID, paymentID) - creditUserBalance(ctx, bookingUserID, bookingID, paymentID, refundThisPayment, reason) + if _, balErr := 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() + `, bookingUserID, refundThisPayment); balErr != nil { + log.Printf("Failed to credit user %s balance for refund of booking %s: %v", bookingUserID, bookingID, balErr) + } } default: @@ -242,21 +417,25 @@ func ProcessCancellationRefund( log.Printf("Skipping refund for payment %s with method %q (no money exchanged)", paymentID, paymentMethod) } + recordStatus := "completed" + if paymentMethod == "online_square" || paymentMethod == "in_person_card" { + recordStatus = "pending" + } record := RefundRecord{ PaymentID: paymentID, BookingID: bookingID, Amount: refundThisPayment, SquareRefundID: squareRefundID, - Status: "completed", + Status: recordStatus, Reason: reason, CreatedBy: actorID, - CreatedAt: time.Now(), + CreatedAt: clock.Now(), } - _, dbErr := db.Conn.Exec(ctx, ` + _, 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, 'completed', $5, $6, $7) - `, record.PaymentID, record.BookingID, record.Amount, record.SquareRefundID, record.Reason, record.CreatedBy, record.CreatedAt) + 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) if dbErr != nil { log.Printf("Failed to create refund record for payment %s: %v", paymentID, dbErr) continue @@ -267,32 +446,116 @@ func ProcessCancellationRefund( if bookingUserID != "" { var loyaltyUsed bool - db.Conn.QueryRow(ctx, "SELECT EXISTS(SELECT 1 FROM booking_discounts WHERE booking_id = $1 AND discount_source = 'loyalty')", bookingID).Scan(&loyaltyUsed) - if loyaltyUsed { - _, err := db.Conn.Exec(ctx, "UPDATE users SET loyalty_stamps = loyalty_stamps + $1 WHERE id = $2", LoyaltyStampCost, bookingUserID) - if err != nil { - log.Printf("Failed to refund loyalty stamps for booking %s: %v", bookingID, err) + if err := tx.QueryRow(ctx, "SELECT EXISTS(SELECT 1 FROM booking_discounts WHERE booking_id = $1 AND discount_source = 'loyalty')", bookingID).Scan(&loyaltyUsed); err != nil { + log.Printf("Failed to check loyalty stamp refund for booking %s: %v", bookingID, err) + } else if loyaltyUsed { + _, loyaltyErr := tx.Exec(ctx, "UPDATE users SET loyalty_stamps = loyalty_stamps + $1 WHERE id = $2", LoyaltyStampCost, bookingUserID) + if loyaltyErr != nil { + log.Printf("Failed to refund loyalty stamps for booking %s: %v", bookingID, loyaltyErr) } else { log.Printf("Refunded %d loyalty stamps to user %s after cancellation of booking %s", LoyaltyStampCost, bookingUserID, bookingID) } } } + if cErr := tx.Commit(ctx); cErr != nil { + log.Printf("CRITICAL: Failed to commit cancellation refund transaction for booking %s: %v", bookingID, cErr) + return &calc, nil + } + + // Process pending Square refunds after the transaction commits successfully. + // This ensures Square API calls only happen if the DB records persist. + ProcessPendingSquareRefunds(ctx, bookingID, reason) + return &calc, nil } -// creditUserBalance credits a refund amount to the user's gift card balance. -// The refunds table (payment_id, booking_id, amount, reason, created_by, created_at) -// provides the primary audit trail for FreeAgent reconciliation. -func creditUserBalance(ctx context.Context, userID, bookingID, paymentID string, amount float64, reason string) { - _, err := db.Conn.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() - `, userID, amount) +// 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. +// +// Call this AFTER the enclosing transaction (if any) has been committed. +func ProcessPendingSquareRefunds(ctx context.Context, bookingID string, reason string) { + 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' + AND p.payment_method IN ('online_square', 'in_person_card') + AND r.square_refund_id IS NULL + `, bookingID) if err != nil { - log.Printf("Failed to credit user %s balance for refund of booking %s: %v", userID, bookingID, err) + log.Printf("Failed to query pending Square refunds for booking %s: %v", bookingID, err) + return + } + defer rows.Close() + + type pendingRefund struct { + ID string + Amount float64 + SquarePaymentID *string + } + var pending []pendingRefund + 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) + continue + } + pending = append(pending, pr) + } + if err := rows.Err(); err != nil { + log.Printf("Pending refund row iteration error: %v", err) + } + + // Deduplicate: multiple split payment records can share the same + // square_payment_id — only refund each Square payment once. + refundedSquareIDs := make(map[string]bool) + + 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) + } + 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) + } + continue + } + + refundCents := int64(math.Round(pr.Amount * 100)) + refundReq := square.RefundPaymentReq{ + PaymentID: *pr.SquarePaymentID, + Amount: refundCents, + IdempotencyKey: pr.ID + "-square-" + clock.Now().Format("20060102150405"), + 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 + } + + 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) + } } } + + diff --git a/backend/handlers/payments/refunds_test.go b/backend/handlers/payments/refunds_test.go index a564c07..6721b3e 100644 --- a/backend/handlers/payments/refunds_test.go +++ b/backend/handlers/payments/refunds_test.go @@ -4,9 +4,12 @@ package payments import ( + "context" "testing" "time" + "crussell/clock" + "crussell/db" "crussell/testutils" "crussell/testutils/fixtures" ) @@ -329,7 +332,7 @@ func TestProcessCancellationRefund_GiftCardCreditsUserBalance(t *testing.T) { farFuture := time.Date(2099, 12, 31, 10, 0, 0, 0, time.UTC) result, err := ProcessCancellationRefund( ctx, bookingID, 100, 60, - farFuture, time.Now(), "client_cancelled", &userID, + farFuture, clock.Now(), "client_cancelled", &userID, ) if err != nil { t.Fatalf("ProcessCancellationRefund failed: %v", err) @@ -404,7 +407,7 @@ func TestProcessCancellationRefund_CashCreditsUserBalance(t *testing.T) { farFuture := time.Date(2099, 12, 31, 10, 0, 0, 0, time.UTC) result, err := ProcessCancellationRefund( ctx, bookingID, 100, 30, - farFuture, time.Now(), "client_cancelled", &userID, + farFuture, clock.Now(), "client_cancelled", &userID, ) if err != nil { t.Fatalf("ProcessCancellationRefund failed: %v", err) @@ -459,7 +462,7 @@ func TestProcessCancellationRefund_CardSquareRefundWithoutBalanceCredit(t *testi farFuture := time.Date(2099, 12, 31, 10, 0, 0, 0, time.UTC) result, err := ProcessCancellationRefund( ctx, bookingID, 100, 100, - farFuture, time.Now(), "client_cancelled", &userID, + farFuture, clock.Now(), "client_cancelled", &userID, ) if err != nil { t.Fatalf("ProcessCancellationRefund failed: %v", err) @@ -468,17 +471,31 @@ func TestProcessCancellationRefund_CardSquareRefundWithoutBalanceCredit(t *testi t.Errorf("expected refundable 100, got %.2f", result.RefundableAmount) } - // In dev/test the payment has no square_payment_id, so Square cannot process - // the refund and the amount falls through to a balance credit. In production - // with a real square_payment_id the Square API would handle the refund instead. + // 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. + 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) + } + + // No balance credit should have been created (Square payment method uses + // post-commit refund processing, not balance credits). var balance float64 err = tx.QueryRow(ctx, "SELECT COALESCE(balance, 0) FROM user_giftcard_balances WHERE user_id = $1", userID).Scan(&balance) if err != nil { - t.Fatalf("failed to query balance: %v", err) - } - if balance <= 0 { - t.Errorf("expected a balance credit (Square refund unavailable in mock), got %.2f", balance) + // No row = no balance credit — this is the expected outcome. + // The refund was processed as a direct record, not a balance credit. + t.Logf("no balance row (expected): %v", err) + } else if balance > 0 { + t.Errorf("expected no balance credit for Square payment, got %.2f", balance) } } @@ -520,7 +537,7 @@ func TestProcessCancellationRefund_DiscountPaymentSkipped(t *testing.T) { farFuture := time.Date(2099, 12, 31, 10, 0, 0, 0, time.UTC) result, err := ProcessCancellationRefund( ctx, bookingID, 100, 20, - farFuture, time.Now(), "client_cancelled", &userID, + farFuture, clock.Now(), "client_cancelled", &userID, ) if err != nil { t.Fatalf("ProcessCancellationRefund failed: %v", err) @@ -575,7 +592,7 @@ func TestProcessCancellationRefund_OnTheHousePaymentSkipped(t *testing.T) { farFuture := time.Date(2099, 12, 31, 10, 0, 0, 0, time.UTC) result, err := ProcessCancellationRefund( ctx, bookingID, 100, 100, - farFuture, time.Now(), "client_cancelled", &userID, + farFuture, clock.Now(), "client_cancelled", &userID, ) if err != nil { t.Fatalf("ProcessCancellationRefund failed: %v", err) @@ -640,7 +657,7 @@ func TestProcessCancellationRefund_MissingUserID_LogsWarning(t *testing.T) { farFuture := time.Date(2099, 12, 31, 10, 0, 0, 0, time.UTC) result, err := ProcessCancellationRefund( ctx, bookingID, 100, 50, - farFuture, time.Now(), "client_cancelled", nil, + farFuture, clock.Now(), "client_cancelled", nil, ) if err != nil { t.Fatalf("ProcessCancellationRefund failed: %v", err) @@ -704,7 +721,7 @@ func TestProcessCancellationRefund_GuestGiftcardDoesNotCreditBalance(t *testing. farFuture := time.Date(2099, 12, 31, 10, 0, 0, 0, time.UTC) result, err := ProcessCancellationRefund( ctx, bookingID, 100, 50, - farFuture, time.Now(), "client_cancelled", &userID, + farFuture, clock.Now(), "client_cancelled", &userID, ) if err != nil { t.Fatalf("ProcessCancellationRefund failed: %v", err) @@ -775,7 +792,7 @@ func TestProcessCancellationRefund_GuestCashDoesNotCreditBalance(t *testing.T) { farFuture := time.Date(2099, 12, 31, 10, 0, 0, 0, time.UTC) result, err := ProcessCancellationRefund( ctx, bookingID, 100, 30, - farFuture, time.Now(), "client_cancelled", &userID, + farFuture, clock.Now(), "client_cancelled", &userID, ) if err != nil { t.Fatalf("ProcessCancellationRefund failed: %v", err) @@ -829,7 +846,7 @@ func TestProcessCancellationRefund_SplitPayment_DeduplicatesSquareRefund(t *test } sameSquareID := "sqp_split_dedup_test" - now := time.Now() + now := clock.Now() // Create 2 payment records sharing the same square_payment_id — simulating a // split charge where one Square payment was recorded as deposit + balance. @@ -891,3 +908,141 @@ func TestProcessCancellationRefund_SplitPayment_DeduplicatesSquareRefund(t *test } } + +// ============================================================================= +// ProcessPendingSquareRefunds — post-commit Square refund processing +// ============================================================================= + +// 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). +func TestProcessPendingSquareRefunds_ProcessesPendingRecords(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) + } + + _, err = tx.Exec(ctx, "UPDATE bookings SET status = 'confirmed' WHERE id = $1", bookingID) + if err != nil { + t.Fatalf("failed to confirm booking: %v", err) + } + + // Create an online_square payment + _, err = fixtures.CreateTestPayment(tx, bookingID, 50, "online_square", "full", "completed") + if err != nil { + t.Fatalf("failed to create card payment: %v", err) + } + + // 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() + FROM payments WHERE booking_id = $1 AND payment_method = 'online_square' + `, bookingID) + if err != nil { + t.Fatalf("failed to insert pending refund: %v", err) + } + + // Commit the test transaction so the refund records are persisted. + 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) + } + + // Use a fresh context (no closed transaction) so db.Conn falls through to pool. + freshCtx := context.Background() + + // Now call the post-commit function + ProcessPendingSquareRefunds(freshCtx, bookingID, "client_cancelled") + + // Verify the refund record was marked completed + 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) + } +} + +// TestProcessPendingSquareRefunds_SkipsCompletedRecords verifies that +// ProcessPendingSquareRefunds does not modify already-completed refunds. +func TestProcessPendingSquareRefunds_SkipsCompletedRecords(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 a payment first + var paymentID string + err = tx.QueryRow(ctx, ` + INSERT INTO payments (booking_id, amount, payment_method, payment_type, status) + VALUES ($1, 50, 'cash', 'full', 'completed') RETURNING id + `, bookingID).Scan(&paymentID) + if err != nil { + t.Fatalf("failed to create payment: %v", err) + } + + // Insert a "completed" refund directly (simulating non-Square refund path) + _, err = tx.Exec(ctx, ` + INSERT INTO refunds (payment_id, booking_id, amount, status, reason, created_at) + VALUES ($1, $2, 50, 'completed', 'cash_refund', NOW()) + `, paymentID, bookingID) + if err != nil { + t.Fatalf("failed to insert completed 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) + } + + freshCtx := context.Background() + ProcessPendingSquareRefunds(freshCtx, bookingID, "client_cancelled") + + // Verify the completed refund was left untouched + 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 existing status 'completed', got %q", status) + } +} diff --git a/backend/handlers/payments/service.go b/backend/handlers/payments/service.go index 3fe7e63..04b50a3 100644 --- a/backend/handlers/payments/service.go +++ b/backend/handlers/payments/service.go @@ -3,9 +3,11 @@ package payments import ( "context" "crussell/db" + "crussell/clock" "crussell/internal/square" "errors" "fmt" + "log" "strconv" "strings" "time" @@ -461,14 +463,29 @@ func (s *PaymentService) GetUserPaymentMethods(ctx context.Context, userID strin } func (s *PaymentService) DeletePaymentMethod(ctx context.Context, cardID, userID string) error { - retainedUntil := time.Now().Add(7 * 365 * 24 * time.Hour) - _, err := db.Conn.Exec(ctx, ` + tx, err := db.Conn.Begin(ctx) + if err != nil { + log.Printf("Failed to begin transaction: %v", err) + return err + } + defer tx.Rollback(ctx) + + retainedUntil := clock.Now().Add(7 * 365 * 24 * time.Hour) + _, err = tx.Exec(ctx, ` UPDATE user_saved_cards SET deleted_at = NOW(), deleted_by = $1, retained_until = $2 WHERE id = $3 AND user_id = $1 `, userID, retainedUntil, cardID) + if err != nil { + return err + } - return err + if err := tx.Commit(ctx); err != nil { + log.Printf("Failed to commit transaction: %v", err) + return err + } + + return nil } func (s *PaymentService) CreatePaymentMethodFromDetails(ctx context.Context, userID, cardNumber, expiry, cvc string) (*SavedCard, error) { @@ -487,8 +504,8 @@ func (s *PaymentService) CreatePaymentMethodFromDetails(ctx context.Context, use expYear += 2000 // Check if card is expired - now := time.Now() - expiryDate := time.Date(expYear, time.Month(expMonth), 1, 0, 0, 0, 0, now.Location()) + now := clock.Now() + expiryDate := time.Date(expYear, time.Month(expMonth), 1, 0, 0, 0, 0, time.UTC) if expiryDate.Before(now) { return nil, errors.New("card has expired") } diff --git a/backend/handlers/payments/till.go b/backend/handlers/payments/till.go index 18e1c21..504d213 100644 --- a/backend/handlers/payments/till.go +++ b/backend/handlers/payments/till.go @@ -1,6 +1,7 @@ package payments import ( + "crussell/clock" "crussell/db" "crussell/internal/square" "crussell/internal/validators" @@ -11,7 +12,6 @@ import ( "fmt" "log" "net/http" - "time" "github.com/go-chi/chi/v5" "github.com/jackc/pgx/v5" @@ -96,7 +96,6 @@ func CreateTillSale(w http.ResponseWriter, r *http.Request) { err := db.Conn.QueryRow(ctx, `SELECT id FROM till_sales WHERE idempotency_key = $1`, req.IdempotencyKey).Scan(&existingID) if err == nil { // Existing sale found — return it (idempotent) - w.Header().Set("Content-Type", "application/json") json.NewEncoder(w).Encode(TillSaleResponse{ ID: existingID, ItemType: req.ItemType, @@ -121,7 +120,12 @@ func CreateTillSale(w http.ResponseWriter, r *http.Request) { var giftCardID string if req.Action == "create" { var purchaseVoucherType string - _ = tx.QueryRow(ctx, `SELECT COALESCE(voucher_type, 'SPV') FROM business_settings LIMIT 1`).Scan(&purchaseVoucherType) + err = tx.QueryRow(ctx, `SELECT COALESCE(voucher_type, 'SPV') FROM business_settings LIMIT 1`).Scan(&purchaseVoucherType) + if err != nil { + log.Printf("Failed to query voucher type: %v", err) + http.Error(w, "internal server error", http.StatusInternalServerError) + return + } if purchaseVoucherType == "" { purchaseVoucherType = "SPV" } @@ -243,34 +247,39 @@ func CreateTillSale(w http.ResponseWriter, r *http.Request) { var saleStatus string var dbPaymentMethod string + // Post-commit Square payment tracking: saved_card and online_square + // call Square AFTER the DB transaction commits, so a tx failure never + // leaves a Square charge with no DB record. + var needsSquarePayment bool + var savedCardSqCardID string + switch req.PaymentMethod { case "cash": saleStatus = "completed" dbPaymentMethod = "cash" if req.IdempotencyKey == "" { - req.IdempotencyKey = "till-cash-" + giftCardID + "-" + time.Now().Format("20060102150405.000000") + req.IdempotencyKey = "till-cash-" + giftCardID + "-" + clock.Now().Format("20060102150405.000000") } - case "saved_card": - dbPaymentMethod = "online_square" - if req.UserID != nil && *req.UserID != "" { - _, err = service.GetCardByIDQuerier(ctx, tx, *req.UserSavedCardID, *req.UserID) - if err != nil { - if errors.Is(err, pgx.ErrNoRows) { - http.Error(w, "Saved card not found", http.StatusNotFound) - return - } - log.Printf("Failed to verify saved card: %v", err) - http.Error(w, "internal server error", http.StatusInternalServerError) + case "saved_card": + dbPaymentMethod = "online_square" + if req.UserID != nil && *req.UserID != "" { + _, err = service.GetCardByIDQuerier(ctx, tx, *req.UserSavedCardID, *req.UserID) + if err != nil { + if errors.Is(err, pgx.ErrNoRows) { + http.Error(w, "Saved card not found", http.StatusNotFound) return } + log.Printf("Failed to verify saved card: %v", err) + http.Error(w, "internal server error", http.StatusInternalServerError) + return } + } - var sqCardID string err = tx.QueryRow(ctx, ` SELECT square_card_id FROM user_saved_cards WHERE id = $1 AND deleted_at IS NULL - `, *req.UserSavedCardID).Scan(&sqCardID) + `, *req.UserSavedCardID).Scan(&savedCardSqCardID) if err != nil { log.Printf("Failed to get saved card details: %v", err) http.Error(w, "Card not found", http.StatusNotFound) @@ -278,30 +287,15 @@ func CreateTillSale(w http.ResponseWriter, r *http.Request) { } if req.IdempotencyKey == "" { - req.IdempotencyKey = "till-sale-" + giftCardID + "-" + time.Now().Format("20060102150405.000000") + req.IdempotencyKey = "till-sale-" + giftCardID + "-" + clock.Now().Format("20060102150405.000000") } - paymentReq := square.CreatePaymentReq{ - Amount: penceAmount, - Currency: "GBP", - SourceID: sqCardID, - IdempotencyKey: req.IdempotencyKey, - Note: "Gift Card " + req.Action, - } - - paymentResult, err := SquareClient.CreatePayment(ctx, paymentReq) - if err != nil { - log.Printf("Failed to process saved card payment: %v", err) - http.Error(w, "Payment failed", http.StatusPaymentRequired) - return - } - - squarePaymentID = &paymentResult.SquarePayID - saleStatus = "completed" + saleStatus = "pending" + needsSquarePayment = true case "card_machine": dbPaymentMethod = "in_person_card" if req.IdempotencyKey == "" { - req.IdempotencyKey = "till-terminal-" + giftCardID + "-" + time.Now().Format("20060102150405.000000") + req.IdempotencyKey = "till-terminal-" + giftCardID + "-" + clock.Now().Format("20060102150405.000000") } checkoutReq := square.CreateCheckoutReq{ @@ -323,39 +317,17 @@ func CreateTillSale(w http.ResponseWriter, r *http.Request) { saleStatus = "pending" case "online_square": dbPaymentMethod = "online_square" - cardOnFile, err := SquareClient.CreateCardOnFileRaw(ctx, "till-"+giftCardID, req.CardNumber, req.CardExpMonth, req.CardExpYear, req.CardCVC) - if err != nil { - log.Printf("Failed to tokenize ephemeral card: %v", err) - http.Error(w, "Card tokenization failed", http.StatusInternalServerError) - return - } - if req.IdempotencyKey == "" { - req.IdempotencyKey = "till-online-" + giftCardID + "-" + time.Now().Format("20060102150405.000000") + req.IdempotencyKey = "till-online-" + giftCardID + "-" + clock.Now().Format("20060102150405.000000") } - paymentReq := square.CreatePaymentReq{ - Amount: penceAmount, - Currency: "GBP", - SourceID: cardOnFile.CardID, - IdempotencyKey: req.IdempotencyKey, - Note: "Gift Card " + req.Action, - } - - paymentResult, err := SquareClient.CreatePayment(ctx, paymentReq) - if err != nil { - log.Printf("Failed to process online card payment: %v", err) - http.Error(w, "Payment failed", http.StatusPaymentRequired) - return - } - - squarePaymentID = &paymentResult.SquarePayID - saleStatus = "completed" + saleStatus = "pending" + needsSquarePayment = true case "on_the_house": saleStatus = "completed" dbPaymentMethod = "on_the_house" if req.IdempotencyKey == "" { - req.IdempotencyKey = "till-on-the-house-" + giftCardID + "-" + time.Now().Format("20060102150405.000000") + req.IdempotencyKey = "till-on-the-house-" + giftCardID + "-" + clock.Now().Format("20060102150405.000000") } } @@ -400,7 +372,7 @@ func CreateTillSale(w http.ResponseWriter, r *http.Request) { return } - if req.PaymentMethod != "on_the_house" && saleStatus == "completed" { + if req.PaymentMethod != "on_the_house" && (saleStatus == "completed" || needsSquarePayment) { vatCfg, vatErr := GetVATConfig(ctx, tx) if vatErr == nil && vatCfg.IsVATRegistered && vatCfg.VoucherType == "SPV" { if _, vatExecErr := tx.Exec(ctx, "SELECT apply_vat_to_till_sale($1, $2)", tillSaleID, vatCfg.DefaultVATRate); vatExecErr != nil { @@ -415,7 +387,60 @@ func CreateTillSale(w http.ResponseWriter, r *http.Request) { return } - w.Header().Set("Content-Type", "application/json") + // Step 2: DB transaction committed — safe to call Square now. + // If Square fails, the till_sale record stays 'pending' for manual retry. + if needsSquarePayment { + var paymentResult *square.PaymentResult + var squareErr error + + if req.PaymentMethod == "saved_card" { + paymentReq := square.CreatePaymentReq{ + Amount: penceAmount, + Currency: "GBP", + SourceID: savedCardSqCardID, + IdempotencyKey: req.IdempotencyKey, + Note: "Gift Card " + req.Action, + } + paymentResult, squareErr = SquareClient.CreatePayment(ctx, paymentReq) + } else if req.PaymentMethod == "online_square" { + cardOnFile, cardErr := SquareClient.CreateCardOnFileRaw(ctx, "till-"+giftCardID, req.CardNumber, req.CardExpMonth, req.CardExpYear, req.CardCVC) + if cardErr != nil { + log.Printf("Failed to tokenize ephemeral card: %v", cardErr) + http.Error(w, "Card tokenization failed", http.StatusInternalServerError) + return + } + + paymentReq := square.CreatePaymentReq{ + Amount: penceAmount, + Currency: "GBP", + SourceID: cardOnFile.CardID, + IdempotencyKey: req.IdempotencyKey, + Note: "Gift Card " + req.Action, + } + paymentResult, squareErr = SquareClient.CreatePayment(ctx, paymentReq) + } + + if squareErr != nil { + log.Printf("Failed to process payment: %v", squareErr) + http.Error(w, "Payment failed", http.StatusPaymentRequired) + return + } + + // Square succeeded — update the till_sale record. + _, upErr := db.Conn.Exec(ctx, + `UPDATE till_sales SET status = 'completed', square_payment_id = $1 WHERE id = $2`, + paymentResult.SquarePayID, tillSaleID, + ) + if upErr != nil { + log.Printf("CRITICAL: Square payment succeeded (ID=%s) but till_sale %s update failed: %v — manual reconciliation required", paymentResult.SquarePayID, tillSaleID, upErr) + http.Error(w, "internal server error", http.StatusInternalServerError) + return + } + + saleStatus = "completed" + squarePaymentID = &paymentResult.SquarePayID + } + w.WriteHeader(http.StatusCreated) json.NewEncoder(w).Encode(TillSaleResponse{ ID: tillSaleID, @@ -452,7 +477,6 @@ func GetTillCheckoutStatus(w http.ResponseWriter, r *http.Request) { } if currentStatus == "completed" { - w.Header().Set("Content-Type", "application/json") json.NewEncoder(w).Encode(PaymentStatusResponse{ Status: "COMPLETED", }) @@ -462,7 +486,6 @@ func GetTillCheckoutStatus(w http.ResponseWriter, r *http.Request) { paymentResult, err := SquareClient.GetCheckout(r.Context(), checkoutID) if err != nil { if err.Error() == "checkout pending" { - w.Header().Set("Content-Type", "application/json") json.NewEncoder(w).Encode(PaymentStatusResponse{Status: "PENDING"}) return } @@ -500,7 +523,6 @@ func GetTillCheckoutStatus(w http.ResponseWriter, r *http.Request) { return } - w.Header().Set("Content-Type", "application/json") json.NewEncoder(w).Encode(PaymentStatusResponse{ Status: "COMPLETED", PaymentID: tillSaleID, @@ -512,6 +534,5 @@ func GetTillCheckoutStatus(w http.ResponseWriter, r *http.Request) { return } - w.Header().Set("Content-Type", "application/json") json.NewEncoder(w).Encode(PaymentStatusResponse{Status: "PENDING"}) } diff --git a/backend/handlers/payments/till_test.go b/backend/handlers/payments/till_test.go index 5b951f0..be20d05 100644 --- a/backend/handlers/payments/till_test.go +++ b/backend/handlers/payments/till_test.go @@ -5,6 +5,7 @@ package payments import ( "bytes" + "context" "encoding/json" "net/http" "net/http/httptest" @@ -344,3 +345,63 @@ func TestCreateTillSale_TopupOnRedeemedCard(t *testing.T) { t.Errorf("expected status 400, got %d. body: %s", w.Code, w.Body.String()) } } + +func TestCreateTillSale_SavedCard_TransactionFailure_SkipsSquare(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) + } + + cancelCtx, cancel := context.WithCancel(context.Background()) + cancel() + + reqBody := TillSaleRequest{ + ItemType: "gift_card", + Action: "create", + Amount: 50.00, + PaymentMethod: "saved_card", + UserSavedCardID: &cardID, + UserID: &userID, + } + + 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(cancelCtx, 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.StatusInternalServerError { + t.Errorf("expected status 500, got %d. body: %s", w.Code, w.Body.String()) + } + + var completedCount int + err = tx.QueryRow(ctx, "SELECT COUNT(*) FROM till_sales WHERE status = 'completed'").Scan(&completedCount) + if err != nil { + t.Errorf("failed to query till_sales: %v", err) + } + if completedCount != 0 { + t.Errorf("expected 0 completed till_sales, got %d", completedCount) + } +} diff --git a/backend/handlers/payments/vat.go b/backend/handlers/payments/vat.go index 8c71acc..0f4f9cb 100644 --- a/backend/handlers/payments/vat.go +++ b/backend/handlers/payments/vat.go @@ -45,12 +45,12 @@ func ApplyVATToBookingPayment(ctx context.Context, q db.Querier, paymentID strin if !vatCfg.IsVATRegistered { return } - // Discount and on_the_house payments must never have VAT applied. - // Read the payment method inside the same transactional context so that - // the just-inserted row is visible (defence against READ COMMITTED - // isolation when q is a pgx.Tx). - var method string - if qErr := q.QueryRow(ctx, "SELECT payment_method FROM payments WHERE id = $1", paymentID).Scan(&method); qErr == nil && (method == "discount" || method == "on_the_house") { + // Discount, on_the_house, and tip payments must never have VAT applied. + // Read the payment method and type inside the same transactional context + // so that the just-inserted row is visible (defence against READ + // COMMITTED isolation when q is a pgx.Tx). + var method, ptype string + if qErr := q.QueryRow(ctx, "SELECT payment_method, payment_type FROM payments WHERE id = $1", paymentID).Scan(&method, &ptype); qErr == nil && (method == "discount" || method == "on_the_house" || ptype == "tip") { return } if _, execErr := q.Exec(ctx, "SELECT apply_vat_to_payment($1, $2)", paymentID, vatCfg.DefaultVATRate); execErr != nil { diff --git a/backend/handlers/payments/vat_test.go b/backend/handlers/payments/vat_test.go index 6b0899f..eb42c01 100644 --- a/backend/handlers/payments/vat_test.go +++ b/backend/handlers/payments/vat_test.go @@ -12,6 +12,7 @@ import ( "testing" "time" + "crussell/clock" "crussell/db" "crussell/mw" "crussell/testutils" @@ -2544,8 +2545,8 @@ func TestVAT_DiscountPayment_NoVAT(t *testing.T) { PaymentMethod: "discount", Status: "completed", Amount: 10.00, - CreatedAt: time.Now(), - UpdatedAt: time.Now(), + CreatedAt: clock.Now(), + UpdatedAt: clock.Now(), CreatedBy: &adminID, } svc := NewPaymentService() @@ -3069,7 +3070,10 @@ func TestVAT_DisableVATRegistration_Lifecycle(t *testing.T) { // and back on, with payments correctly reflecting the current registration // state at the time each payment is made. func TestVAT_DisableAndReEnable_Lifecycle(t *testing.T) { - t.Parallel() + // Non-parallel: 47 VAT tests all UPDATE the shared business_settings row. + // With t.Parallel() + PostgreSQL row locks, tests block each other and + // this test's multiple state transitions (enable→disable→re-enable) are + // particularly susceptible to timeout. ctx, tx := testutils.SetupTestTx(t) _, err := tx.Exec(ctx, `UPDATE business_settings SET is_vat_registered = TRUE, default_vat_rate = 20.00`) @@ -3265,6 +3269,134 @@ func TestVAT_DisableAndReEnable_Lifecycle(t *testing.T) { // has a cash payment (with VAT) and a discount payment, the PaymentSummary // correctly reports TotalVATAmount (only the cash portion), TotalNetAmount, // PaidAmount, and RemainingAmount (total - paid). +// ─── T7: VAT Rounding Consistency Test ───────────────────────────────────── + +// TestVAT_RoundingConsistency verifies that for edge case amounts, +// vat_amount + net_amount = gross_amount exactly (no rounding drift). +func TestVAT_RoundingConsistency(t *testing.T) { + edgeCases := []struct { + name string + amount float64 + }{ + {"£10.005 rounding boundary", 10.005}, + {"£100.00 exact", 100.00}, + {"£33.33 repeating decimal", 33.33}, + {"£0.01 minimum", 0.01}, + {"£9.99 just under £10", 9.99}, + {"£19.99 just under £20", 19.99}, + {"£99.99 just under £100", 99.99}, + {"£1.00 single unit", 1.00}, + {"£7.50 half", 7.50}, + {"£66.66 repeating", 66.66}, + {"£199.99 large", 199.99}, + {"£0.50 half pound", 0.50}, + {"£0.49 just under half", 0.49}, + {"£0.51 just over half", 0.51}, + {"£999.99 near thousand", 999.99}, + } + + for _, tt := range edgeCases { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + ctx, tx := testutils.SetupTestTx(t) + + _, err := tx.Exec(ctx, `UPDATE business_settings SET is_vat_registered = TRUE, default_vat_rate = 20.00`) + if err != nil { + t.Fatalf("failed to update business_settings: %v", err) + } + + adminID, err := fixtures.CreateTestUser(tx) + if err != nil { + t.Fatalf("failed to create user: %v", err) + } + _, _ = tx.Exec(ctx, "UPDATE users SET account_role = 'admin' WHERE id = $1", adminID) + + svc := NewPaymentService() + serviceID, err := fixtures.CreateTestService(tx) + if err != nil { + t.Fatalf("failed to create service: %v", err) + } + defer fixtures.DeleteService(tx, serviceID) + + bookingID, err := fixtures.CreateTestBooking(tx, adminID, serviceID) + if err != nil { + t.Fatalf("failed to create booking: %v", err) + } + defer fixtures.DeleteBooking(tx, bookingID) + + _, _ = tx.Exec(ctx, "UPDATE bookings SET status = 'in_progress' WHERE id = $1", bookingID) + + // Create a payment directly + var paymentID string + err = tx.QueryRow(ctx, ` + INSERT INTO payments (booking_id, payment_type, payment_method, status, amount, created_by, created_at, updated_at) + VALUES ($1, 'full', 'cash', 'completed', $2, $3, NOW(), NOW()) + RETURNING id + `, bookingID, tt.amount, adminID).Scan(&paymentID) + if err != nil { + t.Fatalf("failed to insert payment: %v", err) + } + + // Read the actual stored gross amount (DB may round to 2dp) + var storedGross float64 + err = tx.QueryRow(ctx, "SELECT amount FROM payments WHERE id = $1", paymentID).Scan(&storedGross) + if err != nil { + t.Fatalf("failed to read stored amount: %v", err) + } + + // Apply VAT + ApplyVATToBookingPayment(ctx, tx, paymentID) + + // Verify vat + net = gross + var vatAmount sql.NullFloat64 + var netAmount sql.NullFloat64 + var isVATApplicable bool + err = tx.QueryRow(ctx, `SELECT is_vat_applicable, vat_amount, net_amount FROM payments WHERE id = $1`, paymentID).Scan(&isVATApplicable, &vatAmount, &netAmount) + + if !isVATApplicable { + t.Fatal("expected VAT to be applicable") + } + if !vatAmount.Valid { + t.Fatal("expected vat_amount to be set") + } + if !netAmount.Valid { + t.Fatal("expected net_amount to be set") + } + + sum := vatAmount.Float64 + netAmount.Float64 + diff := sum - storedGross + if diff < -0.02 || diff > 0.02 { + t.Errorf("vat(%.2f) + net(%.2f) = %.2f, expected stored gross %.2f (diff=%.4f)", + vatAmount.Float64, netAmount.Float64, sum, storedGross, diff) + } else if diff != 0 { + t.Logf("minor rounding drift: vat(%.2f) + net(%.2f) = %.2f, stored gross %.2f (diff=%.4f)", + vatAmount.Float64, netAmount.Float64, sum, storedGross, diff) + } + + // Verify the payment summary aggregates also hold (with tolerance) + summary, sErr := svc.GetBookingPaymentSummary(ctx, bookingID) + if sErr != nil { + t.Fatalf("GetBookingPaymentSummary failed: %v", sErr) + } + summarySum := summary.TotalVATAmount + summary.TotalNetAmount + summaryDiff := summarySum - summary.PaidAmount + if summaryDiff < -0.02 || summaryDiff > 0.02 { + t.Errorf("summary: TotalVATAmount(%.2f) + TotalNetAmount(%.2f) = %.2f, expected PaidAmount %.2f (diff=%.4f)", + summary.TotalVATAmount, summary.TotalNetAmount, summarySum, summary.PaidAmount, summaryDiff) + } + }) + } +} + +// ─── T8: SPV vs MPV Lifecycle Tests ───────────────────────────────────────── +// NOTE: SPV and MPV lifecycle tests already exist above: +// - TestSPV_FullLifecycle_BuyAndRedeem (line ~1334) +// - TestMPV_FullLifecycle_BuyAndRedeem (line ~1446) +// - TestVoucherToggle_SPVPurchase_MPVRedeem (line ~2059) +// - TestVoucherToggle_MPVPurchase_SPVRedeem (line ~2220) +// These comprehensively cover the full lifecycle of both voucher types +// including purchase, redemption, and toggle scenarios. + func TestVAT_DiscountAndCashPayment_RemainingBalance(t *testing.T) { t.Parallel() ctx, tx := testutils.SetupTestTx(t) @@ -3345,8 +3477,8 @@ func TestVAT_DiscountAndCashPayment_RemainingBalance(t *testing.T) { PaymentMethod: "discount", Status: "completed", Amount: 10.00, - CreatedAt: time.Now(), - UpdatedAt: time.Now(), + CreatedAt: clock.Now(), + UpdatedAt: clock.Now(), CreatedBy: &adminID, } svc := NewPaymentService()