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()