Fix tip retry amount guard false 400 on non-exact pence values

The pending-retry amount guard compared pence via int64(pounds*100), which truncates instead of rounding. For non-exact pound values (e.g. £1.14 stored as the float64 1.1399999999999999) the truncation yields 113 != 114, falsely rejecting a legitimate same-amount retry with 400. Since the frontend reuses the idempotency key on same-amount retries, every retry was rejected, permanently stranding the pending record (and any orphaned Square charge) with no recovery path. Fix: compare in pence via math.Round — the existing pattern already used in refunds.go — so non-exact values round to the true pence. Also applied the same correction to the sibling lossy conversions: - CreateTipPayment / CreateBookingPayment completed-dedup responses (would have reported 113p for a 114p payment) - BuyGiftCard retry amount guard (latent: £10/£20/£50 are float-exact so it never bit, but the identical trap is now closed) Tests: - TestTipPayment_RetryPending_NonExactAmountSucceeds: 114p pending record + same-amount retry completes and charges 114p (failed on the old truncation with 400) - TestTipPayment_RetryPending_AmountMismatchRejected: a same-key retry at a different amount is still rejected with 400 and the pending record is left untouched Verified: full backend suite green (25/25 packages, 0 failures), -race clean on handlers/payments, go build + go vet clean.
2026-08-22 00:34:49 +01:00
parent 4f5dd5c426
commit d54f526b56
3 changed files with 112 additions and 6 deletions
@@ -8,6 +8,7 @@ import (
 	"fmt"
 	"log"
 	"log/slog"
+	"math"
 	"net/http"
 	"strconv"
 	"strings"
@@ -944,7 +945,7 @@ func BuyGiftCard(w http.ResponseWriter, r *http.Request) {
 				// Guard the amount: a retry with a different amount must not
 				// reuse the pending record (gift card would be issued at the
 				// new amount against the old charge record).
-				if int64(existing.Amount*100) != req.Amount {
+				if int64(math.Round(existing.Amount*100)) != req.Amount {
 					log.Printf("Gift card retry amount mismatch: pending record %s has %.2f, request has %d pence", existing.ID, existing.Amount, req.Amount)
 					http.Error(w, "Amount does not match the pending gift card payment", http.StatusBadRequest)
 					return
@@ -907,7 +907,7 @@ func CreateBookingPayment(w http.ResponseWriter, r *http.Request) {
 			BookingID:   existingBookingID.String,
 			PaymentType: existingPaymentType.String,
 			Status:      existingStatus.String,
-			Amount:      int64(existingAmount.Float64 * 100),
+			Amount:      int64(math.Round(existingAmount.Float64 * 100)),
 			CreatedAt:   existingCreatedAt.Time.Format(time.RFC3339),
 		}); err != nil {
 			log.Printf("Failed to encode JSON response: %v", err)
@@ -1929,7 +1929,7 @@ func CreateTipPayment(w http.ResponseWriter, r *http.Request) {
 			BookingID:   existingBookingID.String,
 			PaymentType: existingPaymentType.String,
 			Status:      existingStatus.String,
-			Amount:      int64(existingAmount.Float64 * 100),
+			Amount:      int64(math.Round(existingAmount.Float64 * 100)),
 			CreatedAt:   existingCreatedAt.Time.Format(time.RFC3339),
 		}); err != nil {
 			log.Printf("Failed to encode JSON response: %v", err)
@@ -1939,9 +1939,12 @@ func CreateTipPayment(w http.ResponseWriter, r *http.Request) {
 		// Previous Square call failed — reuse the pending record and re-attempt.
 		// Guard the amount: a retry with a different amount must not mutate the
 		// original record (books, VAT, refund caps) or silently charge the new
-		// amount against the old record.
-		if int64(existingAmount.Float64*100) != req.Amount {
-			log.Printf("Tip retry amount mismatch: pending record %s has %d pence, request has %d pence", existingID.String, int64(existingAmount.Float64*100), req.Amount)
+		// amount against the old record. Compare in pence via math.Round — the
+		// stored pounds value is float64, so int64(pounds*100) truncation would
+		// reject legitimate same-amount retries for non-exact values (e.g. £1.14
+		// stored as 1.1399999999999999 → int64 gives 113 ≠ 114).
+		if int64(math.Round(existingAmount.Float64*100)) != req.Amount {
+			log.Printf("Tip retry amount mismatch: pending record %s has %d pence, request has %d pence", existingID.String, int64(math.Round(existingAmount.Float64*100)), req.Amount)
 			http.Error(w, "Amount does not match the pending tip payment", http.StatusBadRequest)
 			return
 		}
@@ -1989,6 +1989,108 @@ func TestTipPayment_RetryPending_ReattemptsCharge(t *testing.T) {
 	assert.Equal(t, "completed", status)
 }

+// TestTipPayment_RetryPending_NonExactAmountSucceeds verifies the pending-retry
+// amount guard compares pence exactly. £1.14 is stored as the float64
+// 1.1399999999999999, so a naive int64(pounds*100) truncation yields 113 and
+// falsely rejects a legitimate same-amount retry of 114 pence.
+func TestTipPayment_RetryPending_NonExactAmountSucceeds(t *testing.T) {
+	t.Parallel()
+	ctx, tx := testutils.SetupTestTx(t)
+
+	userID, bookingID, _ := setupTestData(t, ctx, tx)
+	userToken := jwt.GenerateUserToken(userID)
+
+	// Create a completed payment so the tip is allowed.
+	_, err := fixtures.CreateTestPayment(tx, bookingID, 5000.00, "online_square", "full", "completed")
+	require.NoError(t, err)
+
+	// Create a saved card for this user.
+	var savedCardID string
+	err = tx.QueryRow(ctx, `
+		INSERT INTO user_saved_cards (user_id, square_card_id, brand, last_4, exp_month, exp_year, fingerprint)
+		VALUES ($1, 'ccof_mock_retry', 'VISA', '1111', 12, 2030, 'sqfp_mock_retry')
+		RETURNING id
+	`, userID).Scan(&savedCardID)
+	require.NoError(t, err)
+
+	// Failed prior attempt: a PENDING tip of £1.14 (114 pence) with the same key.
+	key := "tip-retry-key-114"
+	_, err = tx.Exec(ctx, `
+		INSERT INTO payments (booking_id, payment_type, payment_method, status, amount, idempotency_key, created_at, updated_at, created_by)
+		VALUES ($1, 'tip', 'online_square', 'pending', 1.14, $2, NOW(), NOW(), $3)
+	`, bookingID, key, userID)
+	require.NoError(t, err)
+
+	req := CreateTipPaymentRequest{
+		Amount:         114,
+		CardID:         &savedCardID,
+		IdempotencyKey: key,
+	}
+
+	handler := CreateTipPayment
+	w := makePaymentRequest(handler, "POST", "/api/bookings/"+bookingID+"/tip", req, userToken, ctx)
+	require.Equal(t, http.StatusOK, w.Code, "body: %s", w.Body.String())
+
+	var resp PaymentResponse
+	require.NoError(t, parsePaymentResponseBody(w, &resp))
+	assert.Equal(t, "completed", resp.Status, "a same-amount retry of a non-exact pence value must re-attempt and complete")
+	assert.Equal(t, int64(114), resp.Amount)
+
+	// Exactly one payment record for this key, now completed.
+	var count int
+	var status string
+	err = tx.QueryRow(ctx, `SELECT COUNT(*), MAX(status) FROM payments WHERE idempotency_key = $1`, key).Scan(&count, &status)
+	require.NoError(t, err)
+	assert.Equal(t, 1, count, "must reuse the pending record, not insert a duplicate")
+	assert.Equal(t, "completed", status)
+}
+
+// TestTipPayment_RetryPending_AmountMismatchRejected verifies the amount guard
+// still rejects a retry that changes the amount on the same idempotency key.
+func TestTipPayment_RetryPending_AmountMismatchRejected(t *testing.T) {
+	t.Parallel()
+	ctx, tx := testutils.SetupTestTx(t)
+
+	userID, bookingID, _ := setupTestData(t, ctx, tx)
+	userToken := jwt.GenerateUserToken(userID)
+
+	_, err := fixtures.CreateTestPayment(tx, bookingID, 5000.00, "online_square", "full", "completed")
+	require.NoError(t, err)
+
+	var savedCardID string
+	err = tx.QueryRow(ctx, `
+		INSERT INTO user_saved_cards (user_id, square_card_id, brand, last_4, exp_month, exp_year, fingerprint)
+		VALUES ($1, 'ccof_mock_retry', 'VISA', '1111', 12, 2030, 'sqfp_mock_retry')
+		RETURNING id
+	`, userID).Scan(&savedCardID)
+	require.NoError(t, err)
+
+	// Failed prior attempt: PENDING tip of £10.00 (1000 pence).
+	key := "tip-retry-key-mismatch"
+	_, err = tx.Exec(ctx, `
+		INSERT INTO payments (booking_id, payment_type, payment_method, status, amount, idempotency_key, created_at, updated_at, created_by)
+		VALUES ($1, 'tip', 'online_square', 'pending', 10.00, $2, NOW(), NOW(), $3)
+	`, bookingID, key, userID)
+	require.NoError(t, err)
+
+	// Retry with a DIFFERENT amount but the same key → must 400, not charge.
+	req := CreateTipPaymentRequest{
+		Amount:         2000,
+		CardID:         &savedCardID,
+		IdempotencyKey: key,
+	}
+
+	handler := CreateTipPayment
+	w := makePaymentRequest(handler, "POST", "/api/bookings/"+bookingID+"/tip", req, userToken, ctx)
+	require.Equal(t, http.StatusBadRequest, w.Code, "a same-key retry with a different amount must be rejected")
+
+	// The pending record must be untouched.
+	var status string
+	err = tx.QueryRow(ctx, `SELECT status FROM payments WHERE idempotency_key = $1`, key).Scan(&status)
+	require.NoError(t, err)
+	assert.Equal(t, "pending", status)
+}
+
 func TestTipPayment_TransactionFailure_SkipsSquare(t *testing.T) {
 	t.Parallel()
 	ctx, tx := testutils.SetupTestTx(t)