diff --git a/backend/handlers/payments/giftcards.go b/backend/handlers/payments/giftcards.go index d77324c..37e9924 100644 --- a/backend/handlers/payments/giftcards.go +++ b/backend/handlers/payments/giftcards.go @@ -999,12 +999,18 @@ func BuyGiftCard(w http.ResponseWriter, r *http.Request) { // Step 2: DB transaction committed — safe to call Square now. // If Square fails, the payment record stays 'pending' for manual retry. + var buyerEmail string + if err := db.Conn.QueryRow(ctx, `SELECT email FROM users WHERE id = $1`, userID).Scan(&buyerEmail); err != nil { + log.Printf("[SQUARE-PROD] Failed to resolve buyer email for user %s: %v (Square receipts will not be emailed)", userID, err) + } + paymentReq := square.CreatePaymentReq{ Amount: req.Amount, Currency: "GBP", SourceID: sourceID, IdempotencyKey: req.IdempotencyKey, Note: "Gift Card Purchase", + BuyerEmail: buyerEmail, } paymentResult, err := SquareClient.CreatePayment(ctx, paymentReq) diff --git a/backend/handlers/payments/handlers.go b/backend/handlers/payments/handlers.go index 8fda619..45b8d7b 100644 --- a/backend/handlers/payments/handlers.go +++ b/backend/handlers/payments/handlers.go @@ -2,6 +2,7 @@ package payments import ( "context" + "crypto/rand" "crussell/clock" "crussell/db" "crussell/internal/square" @@ -47,10 +48,11 @@ type RefundRequest struct { } type CreateTipPaymentRequest struct { - Amount int64 `json:"amount" validate:"required,gt=0"` - CardID *string `json:"card_id,omitempty"` - NewCardToken *string `json:"new_card_token,omitempty"` - SaveCard bool `json:"save_card"` + Amount int64 `json:"amount" validate:"required,gt=0"` + CardID *string `json:"card_id,omitempty"` + NewCardToken *string `json:"new_card_token,omitempty"` + SaveCard bool `json:"save_card"` + IdempotencyKey string `json:"idempotency_key,omitempty"` } type CheckoutResponse struct { @@ -759,6 +761,12 @@ func CreateBookingPayment(w http.ResponseWriter, r *http.Request) { return } + // Resolve buyer email for Square receipt delivery (failure is non-fatal). + var bookingBuyerEmail string + if err := db.Conn.QueryRow(r.Context(), `SELECT email FROM users WHERE id = $1`, userID).Scan(&bookingBuyerEmail); err != nil { + log.Printf("[SQUARE-PROD] Failed to resolve buyer email for user %s: %v (Square receipts will not be emailed)", userID, err) + } + // M8 // L5 @@ -980,6 +988,7 @@ func CreateBookingPayment(w http.ResponseWriter, r *http.Request) { IdempotencyKey: req.IdempotencyKey, ReferenceID: bookingID, Note: req.PaymentType, + BuyerEmail: bookingBuyerEmail, } paymentResult, err := SquareClient.CreatePayment(r.Context(), paymentReq) @@ -1801,7 +1810,14 @@ func CreateTipPayment(w http.ResponseWriter, r *http.Request) { return } - idempotencyKey := bookingID + "-tip-" + strconv.FormatInt(req.Amount, 10) + // Idempotency key: prefer the client-supplied UUID (one per attempt, so + // two legitimate identical tips on the same booking don't collapse into + // one). Fall back to a unique key when absent — must NOT be derived from + // request fields alone (bookingID + amount would dedupe distinct tips). + idempotencyKey := req.IdempotencyKey + if idempotencyKey == "" { + idempotencyKey = uniqueTipKey() + } // Resolve the card source ID — same pattern as CreateBookingPayment. var sourceID string @@ -2208,3 +2224,10 @@ func ReleasePaymentLock(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusNoContent) } + +// uniqueTipKey generates a unique idempotency key for tip payments where the +// client did not supply one. Client-supplied UUIDs handle retry dedup; this +// fallback only needs uniqueness so identical tips don't collapse. +func uniqueTipKey() string { + return "tip-" + rand.Text() +} diff --git a/backend/handlers/payments/payments_test.go b/backend/handlers/payments/payments_test.go index e919540..111a5e9 100644 --- a/backend/handlers/payments/payments_test.go +++ b/backend/handlers/payments/payments_test.go @@ -15,7 +15,6 @@ import ( "crussell/clock" "crussell/db" - "crussell/internal/square" "crussell/mw" "crussell/testutils" "crussell/testutils/fixtures" @@ -31,6 +30,38 @@ func makePaymentRequest(handler http.HandlerFunc, method, path string, body inte return makePaymentAuthRequest(handler, method, path, body, token, "", ctx) } +func TestValidateCardInfo(t *testing.T) { + empty := "" + cardID := "card_123" + token := "cnon:test" + + tests := []struct { + name string + cardID *string + newToken *string + wantError bool + }{ + {"both set rejected", &cardID, &token, true}, + {"card_id only ok", &cardID, nil, false}, + {"token only ok", nil, &token, false}, + {"neither set rejected", nil, nil, true}, + {"empty card_id rejected", &empty, nil, true}, + {"empty token rejected", nil, &empty, true}, + {"both empty rejected", &empty, &empty, true}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := ValidateCardInfo(tt.cardID, tt.newToken) + if tt.wantError { + assert.Error(t, err) + } else { + assert.NoError(t, err) + } + }) + } +} + func makePaymentAuthRequest(handler http.HandlerFunc, method, path string, body interface{}, token, userIDOverride string, ctx context.Context) *httptest.ResponseRecorder { var req *http.Request if body != nil { @@ -1875,18 +1906,6 @@ func TestTipPayment_WithSavedCard(t *testing.T) { _, err = tx.Exec(ctx, "UPDATE payments SET square_payment_id = $1 WHERE id = $2", squarePayID, payID) require.NoError(t, err) - // Register the payment in the mock so the tip flow works. - mockClient, ok := SquareClient.(*square.MockClient) - require.True(t, ok, "SquareClient must be a MockClient for this test") - _, err = mockClient.CreatePayment(ctx, square.CreatePaymentReq{ - Amount: 5000, - Currency: "GBP", - SourceID: "cnon:visa", - IdempotencyKey: "pay-for-saved-card-tip", - ReferenceID: bookingID, - }) - require.NoError(t, err) - // Create a saved card for this user. var savedCardID string err = tx.QueryRow(ctx, ` diff --git a/backend/handlers/payments/till.go b/backend/handlers/payments/till.go index 1e2946d..8550a82 100644 --- a/backend/handlers/payments/till.go +++ b/backend/handlers/payments/till.go @@ -105,16 +105,17 @@ func CreateTillSale(w http.ResponseWriter, r *http.Request) { // Idempotency check: if key provided, return existing sale if found if req.IdempotencyKey != "" { - var existingID string - err := db.Conn.QueryRow(ctx, `SELECT id FROM till_sales WHERE idempotency_key = $1`, req.IdempotencyKey).Scan(&existingID) + var existingID, existingStatus string + err := db.Conn.QueryRow(ctx, `SELECT id, status FROM till_sales WHERE idempotency_key = $1`, req.IdempotencyKey).Scan(&existingID, &existingStatus) if err == nil { - // Existing sale found — return it (idempotent) + // Existing sale found — return its ACTUAL status (may be 'pending' + // if a previous Square charge failed; must not report 'completed'). if err := json.NewEncoder(w).Encode(TillSaleResponse{ ID: existingID, ItemType: req.ItemType, TotalAmount: req.Amount, PaymentMethod: req.PaymentMethod, - Status: "completed", + Status: existingStatus, }); err != nil { log.Printf("Failed to encode JSON response: %v", err) } @@ -408,6 +409,18 @@ func CreateTillSale(w http.ResponseWriter, r *http.Request) { // Step 2: DB transaction committed — safe to call Square now. // If Square fails, the till_sale record stays 'pending' for manual retry. + // Resolve buyer email for Square receipt delivery (non-fatal if missing). + var buyerEmail string + if req.UserID != nil && *req.UserID != "" { + if err := db.Conn.QueryRow(ctx, `SELECT email FROM users WHERE id = $1`, *req.UserID).Scan(&buyerEmail); err != nil { + log.Printf("[SQUARE-PROD] Failed to resolve buyer email for user %s: %v (Square receipts will not be emailed)", *req.UserID, err) + } + } + if buyerEmail == "" { + if err := db.Conn.QueryRow(ctx, `SELECT email FROM users WHERE id = $1`, adminID).Scan(&buyerEmail); err != nil { + log.Printf("[SQUARE-PROD] Failed to resolve admin email for user %s: %v (Square receipts will not be emailed)", adminID, err) + } + } if needsSquarePayment { var paymentResult *square.PaymentResult var squareErr error @@ -419,6 +432,7 @@ func CreateTillSale(w http.ResponseWriter, r *http.Request) { SourceID: savedCardSqCardID, IdempotencyKey: req.IdempotencyKey, Note: "Gift Card " + req.Action, + BuyerEmail: buyerEmail, } paymentResult, squareErr = SquareClient.CreatePayment(ctx, paymentReq) } else if req.PaymentMethod == "online_square" { @@ -435,6 +449,7 @@ func CreateTillSale(w http.ResponseWriter, r *http.Request) { SourceID: cardOnFile.CardID, IdempotencyKey: req.IdempotencyKey, Note: "Gift Card " + req.Action, + BuyerEmail: buyerEmail, } paymentResult, squareErr = SquareClient.CreatePayment(ctx, paymentReq) } diff --git a/backend/handlers/payments/till_test.go b/backend/handlers/payments/till_test.go index 2f2d5f8..3fcd2da 100644 --- a/backend/handlers/payments/till_test.go +++ b/backend/handlers/payments/till_test.go @@ -881,11 +881,10 @@ func TestCreateTillSale_CreateCash(t *testing.T) { } } -// TestCreateTillSale_TwoIdenticalCreateSales_BothSucceed is a regression test: -// two identical keyless cash gift-card creations must BOTH return 201. The -// idempotency-key fallback must be unique per request (not derived from request -// fields, which are identical for the two sales) or the second sale would -// collide on the till_sales idempotency_key UNIQUE constraint and return 500. +// TestCreateTillSale_TwoIdenticalCreateSales_BothSucceed verifies that two +// identical keyless cash gift-card creations both return 201. The idempotency-key +// fallback must be unique per request so legitimate repeat sales don't collide +// on the till_sales idempotency_key UNIQUE constraint. func TestCreateTillSale_TwoIdenticalCreateSales_BothSucceed(t *testing.T) { t.Parallel() _, tx := testutils.SetupTestTx(t) diff --git a/backend/internal/square/square_http_client_test.go b/backend/internal/square/square_http_client_test.go new file mode 100644 index 0000000..7f0acbb --- /dev/null +++ b/backend/internal/square/square_http_client_test.go @@ -0,0 +1,45 @@ +//go:build test + +package square + +import "testing" + +func TestPaymentFromSquare_ElseBranch_SurfacesBrandWithoutCardID(t *testing.T) { + p := &sqPayment{ + ID: "pay_1", + Status: "COMPLETED", + TotalMoney: sqMoney{Amount: 5000, Currency: "GBP"}, + CardDetails: &sqCardDetails{ + Card: sqCard{ + ID: "", + CardBrand: "VISA", + Last4: "4242", + }, + }, + } + + result := paymentFromSquare(p) + if result.CardBrand != "VISA" { + t.Errorf("expected CardBrand VISA, got %s", result.CardBrand) + } + if result.CardLast4 != "4242" { + t.Errorf("expected CardLast4 4242, got %s", result.CardLast4) + } +} + +func TestPaymentFromSquare_NilCardDetails(t *testing.T) { + p := &sqPayment{ + ID: "pay_2", + Status: "COMPLETED", + TotalMoney: sqMoney{Amount: 2500, Currency: "GBP"}, + CardDetails: nil, + } + + result := paymentFromSquare(p) + if result.CardBrand != "" { + t.Errorf("expected empty CardBrand when no card details, got %s", result.CardBrand) + } + if result.Amount != 2500 { + t.Errorf("expected amount 2500, got %d", result.Amount) + } +} diff --git a/frontend/src/lib/components/account/UserBookingModal.svelte b/frontend/src/lib/components/account/UserBookingModal.svelte index a996fa0..054ede8 100644 --- a/frontend/src/lib/components/account/UserBookingModal.svelte +++ b/frontend/src/lib/components/account/UserBookingModal.svelte @@ -16,7 +16,7 @@ import { parseWallClockDate } from '$lib/utils/timeSlots'; import type { Booking, BookingDiscount, Payment } from '$lib/types/booking'; import CardInput from '$lib/components/payments/CardInput.svelte'; -import CardBrandIcon from '$lib/components/payments/CardBrandIcon.svelte'; + import CardBrandIcon from '$lib/components/payments/CardBrandIcon.svelte'; import { savedCardsStore, type SavedCard } from '$lib/stores/savedCards.svelte'; interface Props { open: boolean; @@ -138,6 +138,13 @@ import CardBrandIcon from '$lib/components/payments/CardBrandIcon.svelte'; let customTipInput = $state(''); let tipProcessing = $state(false); + // Cached idempotency key: generated once per tip attempt, reused on retry + // (so a network-timeout retry dedupes instead of double-charging), cleared on + // success. Reset when the tip amount changes so an amount change after a + // failed attempt gets a fresh key instead of a false dedup (under-charge). + let tipIdempotencyKey = $state(''); + let tipKeyedAmount = $state(0); + // Card selection state for tips let tipSavedCards = $state([]); let tipLoadingCards = $state(false); @@ -205,7 +212,9 @@ import CardBrandIcon from '$lib/components/payments/CardBrandIcon.svelte'; return expiryYearMonth < currentYearMonth; })() ); - const hasTipNewCardInvalidMonth = $derived(/^\d{2}\/\d{2}$/.test(tipNewCardExpiry) && tipNewCardExpiryParts === null); + const hasTipNewCardInvalidMonth = $derived( + /^\d{2}\/\d{2}$/.test(tipNewCardExpiry) && tipNewCardExpiryParts === null + ); const tipNewCardError = $derived( tipShowNewCard || tipSavedCards.length === 0 @@ -215,13 +224,19 @@ import CardBrandIcon from '$lib/components/payments/CardBrandIcon.svelte'; ? 'Invalid expiry month' : tipCardExpiryTouched && isTipNewCardExpiryPast ? 'This card has expired' - : tipCardExpiryTouched && tipNewCardExpiry.length > 0 && !/^\d{2}\/\d{2}$/.test(tipNewCardExpiry) + : tipCardExpiryTouched && + tipNewCardExpiry.length > 0 && + !/^\d{2}\/\d{2}$/.test(tipNewCardExpiry) ? 'Enter expiry as MM/YY' : tipCVCTouched && tipNewCardCVC.length < 3 && tipNewCardCVC.length > 0 ? 'Enter your CVC number' - : isValidLuhn(tipNewCardNumber) && /^\d{2}\/\d{2}$/.test(tipNewCardExpiry) && tipNewCardCVC.length >= 3 + : isValidLuhn(tipNewCardNumber) && + /^\d{2}\/\d{2}$/.test(tipNewCardExpiry) && + tipNewCardCVC.length >= 3 ? null - : tipNewCardNumber.length === 0 && tipNewCardExpiry.length === 0 && tipNewCardCVC.length === 0 + : tipNewCardNumber.length === 0 && + tipNewCardExpiry.length === 0 && + tipNewCardCVC.length === 0 ? null : 'Please complete all card fields' : null @@ -312,7 +327,12 @@ import CardBrandIcon from '$lib/components/payments/CardBrandIcon.svelte'; // Validate card details for new card payments if (tipShowNewCard || tipSavedCards.length === 0) { - if (!isValidLuhn(tipNewCardNumber) || !/^\d{2}\/\d{2}$/.test(tipNewCardExpiry) || isTipNewCardExpiryPast || tipNewCardCVC.length < 3) { + if ( + !isValidLuhn(tipNewCardNumber) || + !/^\d{2}\/\d{2}$/.test(tipNewCardExpiry) || + isTipNewCardExpiryPast || + tipNewCardCVC.length < 3 + ) { tipProcessing = false; toast.error(tipNewCardError || 'Please enter valid credit card details'); return; @@ -320,7 +340,14 @@ import CardBrandIcon from '$lib/components/payments/CardBrandIcon.svelte'; } try { - const body: Record = { amount: Math.round(tipAmount * 100) }; + if (!tipIdempotencyKey || tipKeyedAmount !== tipAmount) { + tipIdempotencyKey = crypto.randomUUID(); + tipKeyedAmount = tipAmount; + } + const body: Record = { + amount: Math.round(tipAmount * 100), + idempotency_key: tipIdempotencyKey + }; if (tipShowNewCard || tipSavedCards.length === 0) { body.new_card_token = tipNewCardNumber.replace(/\s/g, ''); @@ -339,6 +366,8 @@ import CardBrandIcon from '$lib/components/payments/CardBrandIcon.svelte'; throw new Error(errorText || 'Tip payment failed'); } toast.success('Thank you for your tip!'); + tipIdempotencyKey = ''; + tipKeyedAmount = 0; showTipModal = false; fetchBookingDetails(); } catch (err) { @@ -1173,6 +1202,8 @@ ${hasVAT ? `

VAT is included at ${biz?.default_vat_rate ?? 20} tipAmount = 0; selectedTipPreset = null; customTipInput = ''; + tipIdempotencyKey = ''; + tipKeyedAmount = 0; } }} > @@ -1221,7 +1252,9 @@ ${hasVAT ? `

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

- Payment Method + Payment Method {#if tipLoadingCards}
Loading payment methods...
@@ -1230,11 +1263,17 @@ ${hasVAT ? `

VAT is included at ${biz?.default_vat_rate ?? 20} {#each tipSavedCards as card (card.id)}

diff --git a/frontend/src/lib/components/admin/WeeklySchedule.svelte b/frontend/src/lib/components/admin/WeeklySchedule.svelte index 3ec19fa..9e10508 100644 --- a/frontend/src/lib/components/admin/WeeklySchedule.svelte +++ b/frontend/src/lib/components/admin/WeeklySchedule.svelte @@ -1,4 +1,5 @@ {#if showSvg} - + {:else} - diff --git a/frontend/src/routes/pay-tip/[id]/+page.svelte b/frontend/src/routes/pay-tip/[id]/+page.svelte index bff064d..46fe9d7 100644 --- a/frontend/src/routes/pay-tip/[id]/+page.svelte +++ b/frontend/src/routes/pay-tip/[id]/+page.svelte @@ -12,7 +12,7 @@ import { authStore } from '$lib/stores/auth.svelte'; import { apiFetch } from '$lib/utils/api'; import CardInput from '$lib/components/payments/CardInput.svelte'; -import CardBrandIcon from '$lib/components/payments/CardBrandIcon.svelte'; + import CardBrandIcon from '$lib/components/payments/CardBrandIcon.svelte'; import { savedCardsStore, type SavedCard } from '$lib/stores/savedCards.svelte'; // Types @@ -41,11 +41,17 @@ import CardBrandIcon from '$lib/components/payments/CardBrandIcon.svelte'; let loading = $state(true); let error = $state(null); let paymentState = $state<'idle' | 'processing' | 'success' | 'error'>('idle'); + + // Cached idempotency key: generated once per payment attempt, reused on retry + // (so a network-timeout retry dedupes instead of double-charging), cleared on + // success. Reset when the tip amount changes so an amount change after a + // failed attempt gets a fresh key instead of a false dedup (under-charge). + let tipIdempotencyKey = $state(''); + let tipKeyedAmount = $state(0); let pageState = $state<'loading' | 'authorized' | 'unauthorized' | 'admin'>('loading'); // Card selection state let savedCards = $state([]); - let loadingCards = $state(false); let selectedCardId = $state(null); let showNewCardForm = $state(false); @@ -118,7 +124,9 @@ import CardBrandIcon from '$lib/components/payments/CardBrandIcon.svelte'; return expiryYearMonth < currentYearMonth; })() ); - const hasNewCardInvalidMonth = $derived(/^\d{2}\/\d{2}$/.test(newCardExpiry) && newCardExpiryParts === null); + const hasNewCardInvalidMonth = $derived( + /^\d{2}\/\d{2}$/.test(newCardExpiry) && newCardExpiryParts === null + ); const newCardError = $derived( showNewCardForm || savedCards.length === 0 @@ -132,9 +140,13 @@ import CardBrandIcon from '$lib/components/payments/CardBrandIcon.svelte'; ? 'Enter expiry as MM/YY' : cardCVCTouched && newCardCVC.length < 3 && newCardCVC.length > 0 ? 'Enter your CVC number' - : isValidLuhn(newCardNumber) && /^\d{2}\/\d{2}$/.test(newCardExpiry) && newCardCVC.length >= 3 + : isValidLuhn(newCardNumber) && + /^\d{2}\/\d{2}$/.test(newCardExpiry) && + newCardCVC.length >= 3 ? null - : newCardNumber.length === 0 && newCardExpiry.length === 0 && newCardCVC.length === 0 + : newCardNumber.length === 0 && + newCardExpiry.length === 0 && + newCardCVC.length === 0 ? null : 'Please complete all card fields' : null @@ -232,21 +244,18 @@ import CardBrandIcon from '$lib/components/payments/CardBrandIcon.svelte'; if (savedCardsStore.loaded) { savedCards = savedCardsStore.cards; if (savedCards.length > 0 && !selectedCardId) { - selectedCardId = savedCards.find(c => c.is_default)?.id || savedCards[0].id; + selectedCardId = savedCards.find((c) => c.is_default)?.id || savedCards[0].id; } return; } - loadingCards = true; try { await savedCardsStore.fetch(); savedCards = savedCardsStore.cards; if (savedCards.length > 0 && !selectedCardId) { - selectedCardId = savedCards.find(c => c.is_default)?.id || savedCards[0].id; + selectedCardId = savedCards.find((c) => c.is_default)?.id || savedCards[0].id; } } catch { // ignore - } finally { - loadingCards = false; } } @@ -295,7 +304,12 @@ import CardBrandIcon from '$lib/components/payments/CardBrandIcon.svelte'; // Validate card details for new card payments if (showNewCardForm || savedCards.length === 0) { - if (!isValidLuhn(newCardNumber) || !/^\d{2}\/\d{2}$/.test(newCardExpiry) || isNewCardExpiryPast || newCardCVC.length < 3) { + if ( + !isValidLuhn(newCardNumber) || + !/^\d{2}\/\d{2}$/.test(newCardExpiry) || + isNewCardExpiryPast || + newCardCVC.length < 3 + ) { paymentState = 'idle'; toast.error(newCardError || 'Please enter valid credit card details'); return; @@ -303,8 +317,15 @@ import CardBrandIcon from '$lib/components/payments/CardBrandIcon.svelte'; } try { + if (!tipIdempotencyKey || tipKeyedAmount !== tipAmount) { + tipIdempotencyKey = crypto.randomUUID(); + tipKeyedAmount = tipAmount; + } const amountInPence = Math.round(tipAmount * 100); - const body: Record = { amount: amountInPence }; + const body: Record = { + amount: amountInPence, + idempotency_key: tipIdempotencyKey + }; if (showNewCardForm || savedCards.length === 0) { body.new_card_token = newCardNumber.replace(/\s/g, ''); @@ -325,6 +346,8 @@ import CardBrandIcon from '$lib/components/payments/CardBrandIcon.svelte'; } paymentState = 'success'; + tipIdempotencyKey = ''; + tipKeyedAmount = 0; toast.success('Thank you for your tip!'); } catch (err) { paymentState = 'error'; @@ -542,16 +565,15 @@ import CardBrandIcon from '$lib/components/payments/CardBrandIcon.svelte'; {#if savedCards.length > 0}
- + Payment Method
{#each savedCards as card (card.id)}