test: payments round-2 — webhook gate/M2 refund, gift-card cancel re-issue, till lock contention, sweep VAT rescue coverage

- webhooks: booking-status gate rejects cancelled bookings, M2 stranded-charge refund row + alert, gift-card rows left pending, payable-booking side-effects, unknown-event 503, refund-before-row 503, webhook-after-sync no-double-complete
- giftcards: saved_card_id SCA wire, card_id+token rejected, resume re-issue never over-refunds entitlement, pending-Square-refund blocks, diff re-issue only what is owed
- sweep: VAT on split-rescued primary, all-tip rows VAT-free, till status/key-changed-while-locked skip, recordUntrackedTillSalePayment VAT
- till: suffixed-key slot scan lock held across Square round-trip

Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>

Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent)
This commit is contained in:
2026-08-22 00:34:51 +01:00
co-authored by Sisyphus
parent 77317e4a45
commit 985b114c8b
8 changed files with 725 additions and 37 deletions
+281 -1
View File
@@ -10,6 +10,7 @@ import (
"math" "math"
"net/http" "net/http"
"net/http/httptest" "net/http/httptest"
"strconv"
"strings" "strings"
"sync" "sync"
"testing" "testing"
@@ -17,6 +18,7 @@ import (
"crussell/clock" "crussell/clock"
"crussell/db" "crussell/db"
"crussell/internal/square"
"crussell/mw" "crussell/mw"
"crussell/testutils" "crussell/testutils"
"crussell/testutils/fixtures" "crussell/testutils/fixtures"
@@ -24,6 +26,8 @@ import (
"github.com/go-chi/chi/v5" "github.com/go-chi/chi/v5"
"github.com/jackc/pgx/v5" "github.com/jackc/pgx/v5"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
) )
func TestAdminCreateGiftCard(t *testing.T) { func TestAdminCreateGiftCard(t *testing.T) {
@@ -1323,7 +1327,7 @@ func TestAdminCreateGiftCard_SetsRollingExpiry(t *testing.T) {
// The test DB seeds gift_card_expiry_months = 24; the SQL computes // The test DB seeds gift_card_expiry_months = 24; the SQL computes
// NOW() + (24 * INTERVAL '1 month') = exactly +24 calendar months, so // NOW() + (24 * INTERVAL '1 month') = exactly +24 calendar months, so
// compare against AddDate(0, 24, 0) with slack for clock skew. // compare against AddDate(0, 24, 0) with slack for clock skew.
now := time.Now().UTC() now := clock.Now()
want := now.AddDate(0, 24, 0) want := now.AddDate(0, 24, 0)
if expiryDate.Before(want.Add(-24 * time.Hour)) { if expiryDate.Before(want.Add(-24 * time.Hour)) {
t.Errorf("expected expiry_date ~24 calendar months in the future, got %v (now %v)", expiryDate, now) t.Errorf("expected expiry_date ~24 calendar months in the future, got %v (now %v)", expiryDate, now)
@@ -2924,3 +2928,279 @@ func TestClaimExpiredBalance_NonAdminRefused(t *testing.T) {
t.Errorf("expected 'Admin access required' body, got: %s", w.Body.String()) t.Errorf("expected 'Admin access required' body, got: %s", w.Body.String())
} }
} }
// =============================================================================
// FIX 1 — saved_card_id on the gift-card buy surface
// =============================================================================
// TestBuyGiftCard_SCASavedCard_BindsCustomer pins the FIX 1 wire contract on
// the gift-card buy surface: a buy carrying saved_card_id + new_card_token
// (the SCA tokenize-result shape the account page sends) must resolve the
// charge as a SAVED-card SCA charge — the tokenize-result token is the one-time
// source_id, the Square customer derives from the saved-card row, and the
// payment row records the saved-card reference. Before the fix the unknown
// saved_card_id field was silently dropped and the token was charged as a
// new-card one-off with no customer binding (Square requires the customer for a
// card-on-file charge).
func TestBuyGiftCard_SCASavedCard_BindsCustomer(t *testing.T) {
ctx, tx := testutils.SetupTestTx(t)
userID, err := fixtures.CreateTestUser(tx)
require.NoError(t, err)
token := jwt.GenerateTestToken(userID, "verified_email")
t.Cleanup(func() { InvalidateSquareCustomerCache(userID) })
cardID, err := fixtures.CreateTestPaymentMethod(tx, userID, "ccof:mock_buy_sca", "VISA", "4242")
require.NoError(t, err)
rec := installRecordingClient(t)
scaToken := "cnon:sca-tokenize-buy"
reqBody, _ := json.Marshal(map[string]interface{}{
"amount": 2000,
"recipient_type": "friend",
"saved_card_id": cardID,
"new_card_token": scaToken,
"idempotency_key": "buy-gc-sca-savedcard",
})
r := httptest.NewRequest(http.MethodPost, "/api/user/giftcards/buy", bytes.NewBuffer(reqBody))
r.Header.Set("Authorization", "Bearer "+token)
r.Header.Set("Content-Type", "application/json")
r = r.WithContext(db.ContextWithTx(r.Context(), tx.(pgx.Tx)))
w := httptest.NewRecorder()
router := chi.NewRouter()
router.Use(mw.RequireAuth)
router.With(mw.RequireNonGuest).Post("/api/user/giftcards/buy", BuyGiftCard)
router.ServeHTTP(w, r)
require.Equal(t, http.StatusCreated, w.Code, "SCA saved-card buy must succeed, body: %s", w.Body.String())
rec.mu.Lock()
last := rec.lastReq
rec.mu.Unlock()
require.Equal(t, scaToken, last.SourceID, "the SCA tokenize-result token must be the charge source_id")
require.NotEqual(t, "ccof:mock_buy_sca", last.SourceID, "the stored ccof id must NOT be the source for the tokenize-result flow")
require.NotEmpty(t, last.CustomerID, "customer_id must derive from the saved-card row, not a new-card one-off")
require.NotNil(t, last.CustomerDetails, "a stored-credential charge must carry customer_details")
var payID string
require.NoError(t, tx.QueryRow(ctx, `
SELECT id FROM payments WHERE created_by = $1 AND payment_method = 'online_square' AND booking_id IS NULL
`, userID).Scan(&payID))
var squareSource, uscID sql.NullString
require.NoError(t, tx.QueryRow(ctx, `SELECT square_source_id, user_saved_card_id FROM payments WHERE id = $1`, payID).Scan(&squareSource, &uscID))
require.Equal(t, scaToken, squareSource.String, "the payment row must record the tokenize-result token as its square source")
require.Equal(t, cardID, uscID.String, "the payment row must reference the saved card")
}
// TestBuyGiftCard_CardIDPlusNewCardToken_Rejected pins the regression guard for
// the FIX 1 wire contract: the legacy card_id field must NEVER coexist with
// new_card_token on the gift-card buy surface — the SCA tokenize-result path
// requires saved_card_id (the frontend documents this and never sends the mixed
// shape). Before the guard the mixed shape was accepted and resolved without
// the saved-card customer binding.
func TestBuyGiftCard_CardIDPlusNewCardToken_Rejected(t *testing.T) {
ctx, tx := testutils.SetupTestTx(t)
userID, err := fixtures.CreateTestUser(tx)
require.NoError(t, err)
token := jwt.GenerateTestToken(userID, "verified_email")
cardID, err := fixtures.CreateTestPaymentMethod(tx, userID, "ccof:mock_buy_mixed", "VISA", "4242")
require.NoError(t, err)
rec := installRecordingClient(t)
reqBody, _ := json.Marshal(map[string]interface{}{
"amount": 2000,
"recipient_type": "friend",
"card_id": cardID,
"new_card_token": "cnon:sca-tokenize-mixed",
"idempotency_key": "buy-gc-cardid-plus-token",
})
r := httptest.NewRequest(http.MethodPost, "/api/user/giftcards/buy", bytes.NewBuffer(reqBody))
r.Header.Set("Authorization", "Bearer "+token)
r.Header.Set("Content-Type", "application/json")
r = r.WithContext(db.ContextWithTx(r.Context(), tx.(pgx.Tx)))
w := httptest.NewRecorder()
router := chi.NewRouter()
router.Use(mw.RequireAuth)
router.With(mw.RequireNonGuest).Post("/api/user/giftcards/buy", BuyGiftCard)
router.ServeHTTP(w, r)
require.Equal(t, http.StatusBadRequest, w.Code, "card_id + new_card_token must be rejected, body: %s", w.Body.String())
rec.mu.Lock()
last := rec.lastReq
rec.mu.Unlock()
require.Empty(t, last.SourceID, "no Square charge may be attempted for the rejected shape")
var payCount int
require.NoError(t, tx.QueryRow(ctx, `SELECT COUNT(*) FROM payments WHERE created_by = $1`, userID).Scan(&payCount))
assert.Equal(t, 0, payCount, "no payment row for the rejected shape")
}
// =============================================================================
// FIX 2 — CancelGiftCard resume must never over-refund the re-verified entitlement
// =============================================================================
// seedGiftCardCancelResume seeds an online gift-card purchase (as
// round9SeedGiftCardPurchase) plus a prior gift-card-cancel refund attempt in
// the given state (pending/failed) claiming the FULL purchase value under the
// deterministic cancel key. When spentPounds > 0 the card balance is reduced
// and a completed giftcard payment row records the till spend so
// giftCardSpendAtTill verifies the shortfall — the resume-over-refund scenario.
func seedGiftCardCancelResume(t *testing.T, ctx context.Context, tx pgx.Tx, userID string, amountPounds, spentPounds float64, refundStatus string) (cardID, paymentID, squarePaymentID string) {
t.Helper()
cardID, paymentID = round9SeedGiftCardPurchase(t, ctx, tx, userID, amountPounds, 0)
var sqID string
require.NoError(t, tx.QueryRow(ctx, `SELECT square_payment_id FROM payments WHERE id = $1`, paymentID).Scan(&sqID))
squarePaymentID = sqID
_, err := tx.Exec(ctx, `
INSERT INTO refunds (payment_id, booking_id, amount, status, reason, idempotency_key, created_by, created_at, origin)
VALUES ($1, NULL, $2, $3, $4, $5, $6, NOW(), 'giftcard_cancel')
`, paymentID, amountPounds, refundStatus, giftCardCancelRefundReason,
paymentID+"-gccancel-"+strconv.FormatInt(int64(math.Round(amountPounds*100)), 10), userID)
require.NoError(t, err)
if spentPounds > 0 {
_, err := tx.Exec(ctx, `UPDATE gift_cards SET amount_remaining = $1 WHERE id = $2`, amountPounds-spentPounds, cardID)
require.NoError(t, err)
_, err = tx.Exec(ctx, `
INSERT INTO payments (booking_id, payment_type, payment_method, status, amount, gift_card_id, created_by, idempotency_key, created_at, updated_at)
VALUES (NULL, 'full', 'giftcard', 'completed', $1, $2, $3, $4, NOW(), NOW())
`, spentPounds, cardID, userID, "till-spend-"+cardID)
require.NoError(t, err)
}
return cardID, paymentID, squarePaymentID
}
// TestCancelGiftCard_ResumeFailed_RefundsCurrentEntitlementNotPrior pins the
// FIX 2 over-refund exploit end-to-end: a cancel attempt that ended 'failed'
// at the full purchase value is retried AFTER a till spend. The resume must
// never refund more than the re-verified entitlement — min(priorAmount,
// currentEntitlement) — and must re-issue under a FRESH deterministic key (the
// old key encodes the old amount). Exploit before the fix: buy £50 → cancel
// fails → spend £5 at the till → retry refunded £50 against a £45 entitlement.
func TestCancelGiftCard_ResumeFailed_RefundsCurrentEntitlementNotPrior(t *testing.T) {
ctx, tx := testutils.SetupTestTx(t)
userID, err := fixtures.CreateTestUser(tx)
require.NoError(t, err)
token := jwt.GenerateTestToken(userID, "verified_email")
origClient := SquareClient
counting := &countingRefundClient{SquareClient: square.NewDevClient()}
SquareClient = counting
defer func() { SquareClient = origClient }()
cardID, paymentID, _ := seedGiftCardCancelResume(t, ctx, tx.(pgx.Tx), userID, 50.00, 5.00, "failed")
w := round9CancelGiftCardWithPaymentID(t, ctx, tx.(pgx.Tx), token, cardID, paymentID)
require.Equal(t, http.StatusOK, w.Code, "body: %s", w.Body.String())
calls := counting.refundCalls()
require.Len(t, calls, 1, "exactly one Square refund for the resumed cancellation")
assert.Equal(t, int64(4500), calls[0].Amount, "the resumed refund must be the £45 current entitlement, never the stale £50")
assert.NotEqual(t, paymentID+"-gccancel-5000", calls[0].IdempotencyKey, "the stale amount-encoding key must not be reused")
var amount float64
var status, key string
var sqRefundID sql.NullString
require.NoError(t, tx.QueryRow(ctx, `
SELECT amount, status, idempotency_key, square_refund_id FROM refunds WHERE payment_id = $1
`, paymentID).Scan(&amount, &status, &key, &sqRefundID))
assert.Equal(t, 45.00, amount, "the refund row records the re-issued £45")
assert.Equal(t, "completed", status, "the refund row resolves to completed")
assert.Equal(t, paymentID+"-gccancel-4500", key, "the refund row carries the fresh deterministic key")
require.True(t, sqRefundID.Valid && sqRefundID.String != "", "the Square refund id must be recorded")
var rem float64
var expiry sql.NullTime
require.NoError(t, tx.QueryRow(ctx, `SELECT amount_remaining, expiry_date FROM gift_cards WHERE id = $1`, cardID).Scan(&rem, &expiry))
assert.Equal(t, 0.00, rem, "card balance must be zeroed after the resumed cancellation")
require.True(t, expiry.Valid, "the cancelled card must carry an expiry date")
assert.False(t, expiry.Time.After(clock.Now()), "the cancelled card must be expired")
var cancelTxCount int
require.NoError(t, tx.QueryRow(ctx, `
SELECT COUNT(*) FROM gift_card_transactions WHERE gift_card_id = $1 AND transaction_type = 'cancelled'
`, cardID).Scan(&cancelTxCount))
assert.Equal(t, 1, cancelTxCount, "a 'cancelled' audit row must record the reversal")
}
// TestCancelGiftCard_ResumePending_SquareAlreadyRefunded_NoDoubleRefund pins
// the FIX 2 reconcile path: a 'pending' cancel row whose Square refund has in
// fact COMPLETED (a lost-response prior attempt that landed) must NOT issue a
// second refund — the handler reconciles via ListPaymentRefunds, resolves the
// row completed with the Square refund id, and neutralises the card.
func TestCancelGiftCard_ResumePending_SquareAlreadyRefunded_NoDoubleRefund(t *testing.T) {
ctx, tx := testutils.SetupTestTx(t)
userID, err := fixtures.CreateTestUser(tx)
require.NoError(t, err)
token := jwt.GenerateTestToken(userID, "verified_email")
origClient := SquareClient
mock := square.NewDevClient().(*square.MockClient)
counting := &countingRefundClient{SquareClient: mock}
SquareClient = counting
defer func() { SquareClient = origClient }()
cardID, paymentID, squarePaymentID := seedGiftCardCancelResume(t, ctx, tx.(pgx.Tx), userID, 50.00, 0, "pending")
// The prior attempt actually landed at Square: pre-seed a COMPLETED refund
// in the mock's ledger (the local row is still 'pending' because the
// outcome was never observed).
prelanded, err := counting.SquareClient.RefundPayment(ctx, square.RefundPaymentReq{
PaymentID: squarePaymentID,
Amount: 5000,
IdempotencyKey: "prelanded-" + paymentID,
Reason: giftCardCancelRefundReason,
})
require.NoError(t, err)
w := round9CancelGiftCardWithPaymentID(t, ctx, tx.(pgx.Tx), token, cardID, paymentID)
require.Equal(t, http.StatusOK, w.Code, "body: %s", w.Body.String())
assert.Contains(t, w.Body.String(), "already been refunded")
require.Empty(t, counting.refundCalls(), "no second Square refund when the prior refund already completed at Square")
var status string
var sqRefundID sql.NullString
require.NoError(t, tx.QueryRow(ctx, `SELECT status, square_refund_id FROM refunds WHERE payment_id = $1`, paymentID).Scan(&status, &sqRefundID))
assert.Equal(t, "completed", status, "the pending row must resolve to completed")
assert.Equal(t, prelanded.ID, sqRefundID.String, "the row records the pre-landed Square refund id")
var rem float64
var expiry sql.NullTime
require.NoError(t, tx.QueryRow(ctx, `SELECT amount_remaining, expiry_date FROM gift_cards WHERE id = $1`, cardID).Scan(&rem, &expiry))
assert.Equal(t, 0.00, rem, "the card must be neutralised")
require.True(t, expiry.Valid)
assert.False(t, expiry.Time.After(clock.Now()), "the card must be expired")
}
// TestCancelGiftCard_ResumePending_ReissuesWithFreshKey pins the FIX 2
// pending-resume re-issue branch when Square has NO completed refund: the
// entitlement-bounded amount is re-issued under the SAME deterministic key it
// was originally derived from (no spend in between → the fresh key equals the
// stored key, so Square dedups a prior landed refund) and the row is completed.
func TestCancelGiftCard_ResumePending_ReissuesWithFreshKey(t *testing.T) {
ctx, tx := testutils.SetupTestTx(t)
userID, err := fixtures.CreateTestUser(tx)
require.NoError(t, err)
token := jwt.GenerateTestToken(userID, "verified_email")
origClient := SquareClient
counting := &countingRefundClient{SquareClient: square.NewDevClient()}
SquareClient = counting
defer func() { SquareClient = origClient }()
cardID, paymentID, _ := seedGiftCardCancelResume(t, ctx, tx.(pgx.Tx), userID, 50.00, 0, "pending")
w := round9CancelGiftCardWithPaymentID(t, ctx, tx.(pgx.Tx), token, cardID, paymentID)
require.Equal(t, http.StatusOK, w.Code, "body: %s", w.Body.String())
calls := counting.refundCalls()
require.Len(t, calls, 1, "exactly one Square refund for the resumed pending cancellation")
assert.Equal(t, int64(5000), calls[0].Amount, "the full £50 entitlement is still due when no spend has occurred")
assert.Equal(t, paymentID+"-gccancel-5000", calls[0].IdempotencyKey, "the fresh key equals the original key when the amount is unchanged")
var status string
require.NoError(t, tx.QueryRow(ctx, `SELECT status FROM refunds WHERE payment_id = $1`, paymentID).Scan(&status))
assert.Equal(t, "completed", status)
}
@@ -13,6 +13,7 @@ import (
"testing" "testing"
"time" "time"
"crussell/clock"
"crussell/db" "crussell/db"
"crussell/internal/square" "crussell/internal/square"
"crussell/testutils" "crussell/testutils"
@@ -516,7 +517,7 @@ func TestDiscountPreview_ManyCampaignsExercisesSharedEligibility(t *testing.T) {
t.Fatalf("failed to create in-person payment: %v", err) t.Fatalf("failed to create in-person payment: %v", err)
} }
now := time.Now() now := clock.Now()
if _, err := tx.Exec(ctx, ` if _, err := tx.Exec(ctx, `
INSERT INTO discount_campaigns (name, campaign_type, discount_percent, status, start_date, end_date) INSERT INTO discount_campaigns (name, campaign_type, discount_percent, status, start_date, end_date)
VALUES ('Time Sale', 'time_based', 5, 'active', $1, $2) VALUES ('Time Sale', 'time_based', 5, 'active', $1, $2)
@@ -25,6 +25,7 @@ import (
"testing" "testing"
"time" "time"
"crussell/clock"
"crussell/db" "crussell/db"
"crussell/internal/square" "crussell/internal/square"
"crussell/testutils" "crussell/testutils"
@@ -57,12 +58,12 @@ func TestSweepStalePendingPayments_KeyedCutoffBoundary(t *testing.T) {
// INSIDE the 22h keyed cutoff: 21h55m old → created_at >= keyedCutoff → // INSIDE the 22h keyed cutoff: 21h55m old → created_at >= keyedCutoff →
// the keyed pass must NOT fetch it, and it is under the 24h stale cutoff // the keyed pass must NOT fetch it, and it is under the 24h stale cutoff
// too → stays pending. // too → stays pending. Seeded from clock.Now(), the sweep's cutoff source.
insideID, err := fixtures.CreateTestPayment(tx, bookingID, 2000.00, "online_square", "full", "pending") insideID, err := fixtures.CreateTestPayment(tx, bookingID, 2000.00, "online_square", "full", "pending")
if err != nil { if err != nil {
t.Fatalf("failed to create inside payment: %v", err) t.Fatalf("failed to create inside payment: %v", err)
} }
if _, err := tx.Exec(ctx, "UPDATE payments SET created_at = NOW() - INTERVAL '21 hours 55 minutes', idempotency_key = 'key-boundary-inside', square_source_id = 'cnon:test-card' WHERE id = $1", insideID); err != nil { if _, err := tx.Exec(ctx, "UPDATE payments SET created_at = $1, idempotency_key = 'key-boundary-inside', square_source_id = 'cnon:test-card' WHERE id = $2", clock.Now().Add(-21*time.Hour-55*time.Minute), insideID); err != nil {
t.Fatalf("failed to age the inside payment: %v", err) t.Fatalf("failed to age the inside payment: %v", err)
} }
@@ -73,7 +74,7 @@ func TestSweepStalePendingPayments_KeyedCutoffBoundary(t *testing.T) {
if err != nil { if err != nil {
t.Fatalf("failed to create outside payment: %v", err) t.Fatalf("failed to create outside payment: %v", err)
} }
if _, err := tx.Exec(ctx, "UPDATE payments SET created_at = NOW() - INTERVAL '22 hours 5 minutes', idempotency_key = 'key-boundary-outside', square_source_id = 'cnon:test-card' WHERE id = $1", outsideID); err != nil { if _, err := tx.Exec(ctx, "UPDATE payments SET created_at = $1, idempotency_key = 'key-boundary-outside', square_source_id = 'cnon:test-card' WHERE id = $2", clock.Now().Add(-22*time.Hour-5*time.Minute), outsideID); err != nil {
t.Fatalf("failed to age the outside payment: %v", err) t.Fatalf("failed to age the outside payment: %v", err)
} }
@@ -158,15 +159,15 @@ func TestSweepStaleTerminalCheckouts_AgeCutoffBoundary(t *testing.T) {
// INSIDE: 59m old → not past the 1h cutoff → must be left alone. // INSIDE: 59m old → not past the 1h cutoff → must be left alone.
if _, err := tx.Exec(ctx, ` if _, err := tx.Exec(ctx, `
INSERT INTO terminal_checkouts (checkout_id, booking_id, payment_type, status, amount, created_at) INSERT INTO terminal_checkouts (checkout_id, booking_id, payment_type, status, amount, created_at)
VALUES ($1, $2, 'full', 'PENDING', 50.00, NOW() - INTERVAL '59 minutes') VALUES ($1, $2, 'full', 'PENDING', 50.00, $3)
`, inside.ID, bookingID); err != nil { `, inside.ID, bookingID, clock.Now().Add(-59*time.Minute)); err != nil {
t.Fatalf("failed to seed inside terminal checkout: %v", err) t.Fatalf("failed to seed inside terminal checkout: %v", err)
} }
// OUTSIDE: 61m old → past the 1h cutoff → cancelled + marked failed. // OUTSIDE: 61m old → past the 1h cutoff → cancelled + marked failed.
if _, err := tx.Exec(ctx, ` if _, err := tx.Exec(ctx, `
INSERT INTO terminal_checkouts (checkout_id, booking_id, payment_type, status, amount, created_at) INSERT INTO terminal_checkouts (checkout_id, booking_id, payment_type, status, amount, created_at)
VALUES ($1, $2, 'full', 'PENDING', 50.00, NOW() - INTERVAL '61 minutes') VALUES ($1, $2, 'full', 'PENDING', 50.00, $3)
`, outside.ID, bookingID); err != nil { `, outside.ID, bookingID, clock.Now().Add(-61*time.Minute)); err != nil {
t.Fatalf("failed to seed outside terminal checkout: %v", err) t.Fatalf("failed to seed outside terminal checkout: %v", err)
} }
@@ -268,9 +269,9 @@ func TestSweepPendingSquareRefunds_AgeGuardBoundary(t *testing.T) {
var insideRefundID string var insideRefundID string
if err := tx.QueryRow(ctx, ` if err := tx.QueryRow(ctx, `
INSERT INTO refunds (payment_id, booking_id, amount, status, reason, idempotency_key, origin, created_at) INSERT INTO refunds (payment_id, booking_id, amount, status, reason, idempotency_key, origin, created_at)
VALUES ($1, $2, 25, 'pending', 'client_cancelled', $3, 'cancellation', NOW() - INTERVAL '22 hours 55 minutes') VALUES ($1, $2, 25, 'pending', 'client_cancelled', $3, 'cancellation', $4)
RETURNING id RETURNING id
`, insidePay, insideBooking, insidePay+"-square-2500").Scan(&insideRefundID); err != nil { `, insidePay, insideBooking, insidePay+"-square-2500", clock.Now().Add(-22*time.Hour-55*time.Minute)).Scan(&insideRefundID); err != nil {
t.Fatalf("failed to insert inside refund: %v", err) t.Fatalf("failed to insert inside refund: %v", err)
} }
@@ -279,9 +280,9 @@ func TestSweepPendingSquareRefunds_AgeGuardBoundary(t *testing.T) {
var outsideRefundID string var outsideRefundID string
if err := tx.QueryRow(ctx, ` if err := tx.QueryRow(ctx, `
INSERT INTO refunds (payment_id, booking_id, amount, status, reason, idempotency_key, origin, created_at) INSERT INTO refunds (payment_id, booking_id, amount, status, reason, idempotency_key, origin, created_at)
VALUES ($1, $2, 25, 'pending', 'client_cancelled', $3, 'cancellation', NOW() - INTERVAL '23 hours 5 minutes') VALUES ($1, $2, 25, 'pending', 'client_cancelled', $3, 'cancellation', $4)
RETURNING id RETURNING id
`, outsidePay, outsideBooking, outsidePay+"-square-2500").Scan(&outsideRefundID); err != nil { `, outsidePay, outsideBooking, outsidePay+"-square-2500", clock.Now().Add(-23*time.Hour-5*time.Minute)).Scan(&outsideRefundID); err != nil {
t.Fatalf("failed to insert outside refund: %v", err) t.Fatalf("failed to insert outside refund: %v", err)
} }
+187 -2
View File
@@ -387,7 +387,7 @@ func TestSweepStalePendingPayments_KeyedLostResponse_NoPayment_Failed(t *testing
if err != nil { if err != nil {
t.Fatalf("failed to create stale pending payment: %v", err) t.Fatalf("failed to create stale pending payment: %v", err)
} }
if _, err := tx.Exec(ctx, "UPDATE payments SET created_at = NOW() - INTERVAL '23 hours', idempotency_key = 'key-lost-response-never' WHERE id = $1", staleID); err != nil { if _, err := tx.Exec(ctx, "UPDATE payments SET created_at = NOW() - INTERVAL '23 hours', idempotency_key = 'key-lost-response-never', square_source_id = 'cnon:test-card' WHERE id = $1", staleID); err != nil {
t.Fatalf("failed to age the stale payment: %v", err) t.Fatalf("failed to age the stale payment: %v", err)
} }
@@ -2040,7 +2040,7 @@ func TestSweepStalePendingPayments_KeyedTillLostResponse_ProvenFailed_Clawbacks(
saleID, giftCardID := seedStaleTillSaleWithCard(t, ctx, tx, adminID, 50.00, "", true) saleID, giftCardID := seedStaleTillSaleWithCard(t, ctx, tx, adminID, 50.00, "", true)
// Move both the sale and its created gift card inside the key window (23h, // Move both the sale and its created gift card inside the key window (23h,
// created_at equality preserved → is_create stays true) and add the key. // created_at equality preserved → is_create stays true) and add the key.
if _, err := tx.Exec(ctx, "UPDATE till_sales SET created_at = NOW() - INTERVAL '23 hours', idempotency_key = 'key-lost-till-never' WHERE id = $1", saleID); err != nil { if _, err := tx.Exec(ctx, "UPDATE till_sales SET created_at = NOW() - INTERVAL '23 hours', idempotency_key = 'key-lost-till-never', square_source_id = 'cnon:test-card' WHERE id = $1", saleID); err != nil {
t.Fatalf("failed to age the till sale: %v", err) t.Fatalf("failed to age the till sale: %v", err)
} }
if _, err := tx.Exec(ctx, "UPDATE gift_cards SET created_at = NOW() - INTERVAL '23 hours' WHERE id = $1", giftCardID); err != nil { if _, err := tx.Exec(ctx, "UPDATE gift_cards SET created_at = NOW() - INTERVAL '23 hours' WHERE id = $1", giftCardID); err != nil {
@@ -4463,3 +4463,188 @@ func TestInsertCriticalPaymentNotification_FloodCap_ConcurrentSerialized(t *test
t.Errorf("expected the unacknowledged queue capped at exactly %d under concurrent inserts (serialized), got %d", adminnotify.MaxUnacknowledgedCriticalLogs, totalUnacked) t.Errorf("expected the unacknowledged queue capped at exactly %d under concurrent inserts (serialized), got %d", adminnotify.MaxUnacknowledgedCriticalLogs, totalUnacked)
} }
} }
// TestSweepStalePendingPayments_KeyedTillLockContended_DefersClawback locks the
// FIX 1 sweep side: the keyed stale-pending pass must acquire the SAME
// advisory lock a same-key retry holds while its Square charge is mid-flight
// ("crussell:till:<idempotency_key>") BEFORE failing a till_sale / clawing back
// its funded gift card. When the retry holds the lock (simulated here), the
// sweep's "no payment under the key" probe is premature — the charge may still
// land — so the sweep must DEFER the row (leave it pending, funding intact)
// instead of failing + clawing back. Without the lock it would mark the sale
// failed and delete the funded card, and the retry's charge would then land on
// a row that no longer accepts it (0 rows → CRITICAL): customer charged AND
// funding clawed back.
func TestSweepStalePendingPayments_KeyedTillLockContended_DefersClawback(t *testing.T) {
ctx, tx := testutils.SetupTestTx(t)
adminID, err := fixtures.CreateTestAdminUser(tx)
if err != nil {
t.Fatalf("failed to create admin user: %v", err)
}
pool := context.Background()
const key = "key-lock-contended-retry"
saleID, giftCardID := seedStaleTillSaleWithCard(t, ctx, tx, adminID, 50.00, "", true)
// Move both the sale and its created gift card inside the key window (23h,
// created_at equality preserved → is_create stays true) and add the key
// plus a chargeable source so the replay-by-key reconcile is valid under
// the mock's identical-body contract.
if _, err := tx.Exec(ctx, "UPDATE till_sales SET created_at = NOW() - INTERVAL '23 hours', idempotency_key = $1, square_source_id = 'cnon:test-card' WHERE id = $2", key, saleID); err != nil {
t.Fatalf("failed to age the till sale: %v", err)
}
if _, err := tx.Exec(ctx, "UPDATE gift_cards SET created_at = NOW() - INTERVAL '23 hours' WHERE id = $1", giftCardID); err != nil {
t.Fatalf("failed to age the gift card: %v", err)
}
// A fresh mock has no payment under the key → ReplayPaymentByKey returns
// ErrReplayKeyNotRetained → definitively failed → the sweep would normally
// claw back. The retry holds the till-sale lock, so the sweep must defer.
origClient := SquareClient
SquareClient = square.NewDevClient()
defer func() { SquareClient = origClient }()
// Simulate a same-key retry mid-flight at Square: hold the SAME advisory
// lock the retry path (CreateTillSale) pins across its Square round-trip.
retryConn, err := db.Conn.Acquire(pool)
if err != nil {
t.Fatalf("failed to acquire retry connection: %v", err)
}
defer retryConn.Release()
if _, err := retryConn.Exec(pool, `SELECT pg_advisory_lock(hashtext($1))`, "crussell:till:"+key); err != nil {
t.Fatalf("failed to hold the retry advisory lock: %v", err)
}
defer func() {
_, _ = retryConn.Exec(context.Background(), `SELECT pg_advisory_unlock(hashtext($1))`, "crussell:till:"+key)
}()
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 setup tx: %v", err)
}
start := time.Now()
if _, err := SweepStalePendingPayments(pool); err != nil {
t.Fatalf("sweep failed: %v", err)
}
// The sweep must have DEFERRED the row (bounded try-lock contended for ~3s)
// rather than failing/clawing-back while the retry may still land its charge.
var status string
if err := db.Conn.QueryRow(pool, `SELECT status FROM till_sales WHERE id = $1`, saleID).Scan(&status); err != nil {
t.Fatalf("failed to query till sale: %v", err)
}
if status != "pending" {
t.Errorf("expected the lock-contended till sale left 'pending' (deferred), got %q", status)
}
// The funded gift card must NOT have been clawed back (deleted).
var cardCount int
if err := db.Conn.QueryRow(pool, `SELECT COUNT(*) FROM gift_cards WHERE id = $1`, giftCardID).Scan(&cardCount); err != nil {
t.Fatalf("failed to count gift cards: %v", err)
}
if cardCount != 1 {
t.Errorf("expected the funded gift card untouched while the till-sale lock is contended, got %d cards", cardCount)
}
if elapsed := time.Since(start); elapsed < 2*time.Second {
t.Errorf("expected the sweep to wait out the ~3s bounded try-lock before deferring, returned after %v", elapsed)
}
}
// TestSweepStaleTerminalCheckouts_UntrackedTillSale_VATApplied locks the FIX 2
// gap: a stale card-machine till sale whose checkout COMPLETED at Square and
// was never polled is recorded by recordUntrackedTillSalePayment — and that
// rescue must apply VAT exactly like the synchronous till-completion path
// (GetTillCheckoutStatus) and the stale-pending rescue, or the rescued sale
// silently drops out of VAT reporting.
func TestSweepStaleTerminalCheckouts_UntrackedTillSale_VATApplied(t *testing.T) {
ctx, tx := testutils.SetupTestTx(t)
adminID, err := fixtures.CreateTestAdminUser(tx)
if err != nil {
t.Fatalf("failed to create admin user: %v", err)
}
// VAT-registered, SPV vouchers — the config the synchronous path uses.
if _, err := tx.Exec(ctx, `UPDATE business_settings SET is_vat_registered = TRUE, default_vat_rate = 20.00, voucher_type = 'SPV'`); err != nil {
t.Fatalf("failed to enable VAT in business_settings: %v", err)
}
const checkoutID = "chk_untracked_vat_terminal"
var saleID string
err = tx.QueryRow(ctx, `
INSERT INTO till_sales (item_type, description, quantity, unit_price, total_amount, payment_method, status, square_checkout_id, created_by, created_at, updated_at)
VALUES ('gift_card', 'Gift Card create', 1, 50.00, 50.00, 'in_person_card', 'pending', $1, $2, NOW() - INTERVAL '2 hours', NOW())
RETURNING id
`, checkoutID, adminID).Scan(&saleID)
if err != nil {
t.Fatalf("failed to seed stale terminal sale: %v", err)
}
origClient := SquareClient
SquareClient = &completedTerminalClient{SquareClient: square.NewDevClient(), checkoutID: checkoutID}
defer func() { SquareClient = origClient }()
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)
}
pool := context.Background()
t.Cleanup(func() {
_, _ = db.Conn.Exec(pool, `DELETE FROM till_sales WHERE id = $1`, saleID)
_, _ = db.Conn.Exec(pool, `DELETE FROM users WHERE id = $1`, adminID)
_, _ = db.Conn.Exec(pool, `UPDATE business_settings SET is_vat_registered = FALSE, voucher_type = 'SPV'`)
})
// Drop any other stale terminal rows left by parallel tests so the count is
// deterministic.
if _, err := db.Conn.Exec(pool, `DELETE FROM till_sales WHERE status = 'pending' AND square_checkout_id IS NOT NULL AND id <> $1`, saleID); err != nil {
t.Fatalf("failed to clean leftover stale terminal sales: %v", err)
}
if _, err := db.Conn.Exec(pool, `DELETE FROM terminal_checkouts WHERE status IN ('PENDING', 'IN_PROGRESS')`); err != nil {
t.Fatalf("failed to clean leftover stale booking terminal checkouts: %v", err)
}
n, err := SweepStaleTerminalCheckouts(pool)
if err != nil {
t.Fatalf("sweep failed: %v", err)
}
if n != 1 {
t.Errorf("expected the COMPLETED till-sale checkout recorded by the sweep, got %d resolutions", n)
}
var status string
if err := db.Conn.QueryRow(pool, "SELECT status FROM till_sales WHERE id = $1", saleID).Scan(&status); err != nil {
t.Fatalf("failed to query sale: %v", err)
}
if status != "completed" {
t.Errorf("expected a COMPLETED till-sale checkout's sale marked 'completed', got %q", status)
}
// FIX 2: the untracked-terminal rescue must carry the SAME VAT fields the
// synchronous path computes — £50.00 at 20% → £8.33 VAT, £41.67 net.
var isVATApplicable bool
var vatAmount, netAmount, vatRate sql.NullFloat64
if err := db.Conn.QueryRow(pool, `SELECT is_vat_applicable, vat_amount, net_amount, vat_rate FROM till_sales WHERE id = $1`, saleID).Scan(&isVATApplicable, &vatAmount, &netAmount, &vatRate); err != nil {
t.Fatalf("failed to query till sale VAT fields: %v", err)
}
if !isVATApplicable {
t.Errorf("expected is_vat_applicable=TRUE on the untracked-terminal-rescued till sale, got false")
}
if !vatAmount.Valid || vatAmount.Float64 != 8.33 {
t.Errorf("expected vat_amount 8.33 on the untracked-terminal-rescued till sale, got %v", vatAmount)
}
if !netAmount.Valid || netAmount.Float64 != 41.67 {
t.Errorf("expected net_amount 41.67 on the untracked-terminal-rescued till sale, got %v", netAmount)
}
if !vatRate.Valid || vatRate.Float64 != 20.00 {
t.Errorf("expected vat_rate 20.00 on the untracked-terminal-rescued till sale, got %v", vatRate)
}
}
+197 -2
View File
@@ -17,6 +17,7 @@ import (
"testing" "testing"
"time" "time"
"crussell/clock"
"crussell/db" "crussell/db"
"crussell/internal/square" "crussell/internal/square"
"crussell/mw" "crussell/mw"
@@ -2777,6 +2778,42 @@ func (c *tillSweepResolveClient) CreatePayment(ctx context.Context, req square.C
}, nil }, nil
} }
// tillMidFlightClawbackClient simulates the FULL sweep resolution racing a
// same-key retry: the sale is marked failed AND the funded gift card is
// CLAWED BACK (deleted for a create, exactly like RevertGiftCardFunding) while
// the retry's Square charge is in flight. GetPayment confirms the landed
// charge — the retry's 0-row branch must then re-credit the funding.
type tillMidFlightClawbackClient struct {
square.SquareClient
saleID string
giftCardID string
chargeID string
}
func (c *tillMidFlightClawbackClient) CreatePayment(ctx context.Context, req square.CreatePaymentReq) (*square.PaymentResult, error) {
if _, err := db.Conn.Exec(context.Background(), `UPDATE till_sales SET status = 'failed', updated_at = NOW() WHERE id = $1`, c.saleID); err != nil {
log.Printf("failed to simulate sweep resolution for till sale %s: %v", c.saleID, err)
}
if _, err := db.Conn.Exec(context.Background(), `DELETE FROM gift_card_transactions WHERE gift_card_id = $1`, c.giftCardID); err != nil {
log.Printf("failed to simulate clawback transaction delete: %v", err)
}
if _, err := db.Conn.Exec(context.Background(), `DELETE FROM gift_cards WHERE id = $1`, c.giftCardID); err != nil {
log.Printf("failed to simulate clawback card delete: %v", err)
}
return &square.PaymentResult{
Status: "COMPLETED",
SquarePayID: c.chargeID,
Amount: 5000,
Fees: 88,
CardBrand: "VISA",
CardLast4: "4242",
}, nil
}
func (c *tillMidFlightClawbackClient) GetPayment(ctx context.Context, paymentID string) (*square.PaymentResult, error) {
return &square.PaymentResult{Status: "COMPLETED", SquarePayID: c.chargeID}, nil
}
// TestCreateTillSale_SweepResolvedMidFlight_NoResurrect locks the claim-first // TestCreateTillSale_SweepResolvedMidFlight_NoResurrect locks the claim-first
// guard: a pending till sale retried while the stale-pending sweep / gift-card // guard: a pending till sale retried while the stale-pending sweep / gift-card
// clawback marks the sale 'failed' (the charge was proven never to complete, so // clawback marks the sale 'failed' (the charge was proven never to complete, so
@@ -3175,7 +3212,7 @@ func TestCreateTillSale_TopupExpiredCard_Rejected(t *testing.T) {
if totalAdded != 50.00 || remaining != 50.00 { if totalAdded != 50.00 || remaining != 50.00 {
t.Errorf("expected funds untouched (50.00/50.00), got %.2f/%.2f", totalAdded, remaining) t.Errorf("expected funds untouched (50.00/50.00), got %.2f/%.2f", totalAdded, remaining)
} }
if !expiry.Before(time.Now().UTC()) { if !expiry.Before(clock.Now()) {
t.Errorf("expected the expired card's expiry_date to NOT be reset (still in the past), got %v", expiry) t.Errorf("expected the expired card's expiry_date to NOT be reset (still in the past), got %v", expiry)
} }
var txCount int var txCount int
@@ -3248,7 +3285,7 @@ func TestCreateTillSale_TopupUnexpiredCard_Succeeds(t *testing.T) {
if totalAdded != 75.00 || remaining != 75.00 { if totalAdded != 75.00 || remaining != 75.00 {
t.Errorf("expected funds topped up to 75.00, got %.2f/%.2f", totalAdded, remaining) t.Errorf("expected funds topped up to 75.00, got %.2f/%.2f", totalAdded, remaining)
} }
if !expiry.After(time.Now().UTC()) { if !expiry.After(clock.Now()) {
t.Errorf("expected the live card's expiry to be rolled forward, got %v", expiry) t.Errorf("expected the live card's expiry to be rolled forward, got %v", expiry)
} }
} }
@@ -3367,3 +3404,161 @@ func TestCreateTillSale_DailyCap_Concurrent(t *testing.T) {
t.Errorf("expected issued gift-card value £%.2f, got £%.2f", perSale*float64(successes), totalCreated) t.Errorf("expected issued gift-card value £%.2f, got £%.2f", perSale*float64(successes), totalCreated)
} }
} }
// TestCreateTillSale_SweepClawedBackMidFlight_RecreditsFunding locks the FIX 1
// till side: when the stale-pending sweep (or webhook clawback) marked a
// pending till sale failed AND clawed back its funded gift card while a
// same-key retry's Square charge was mid-flight, the retry's post-charge
// completion UPDATE hits 0 rows — but the charge DID land, so the handler must
// re-credit the clawed-back funding before failing. The customer ends "charged
// AND funded" instead of "charged AND clawed back"; the sale row stays failed
// for manual reconciliation.
func TestCreateTillSale_SweepClawedBackMidFlight_RecreditsFunding(t *testing.T) {
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, "ccof:sq_test_card_id", "VISA", "1234")
if err != nil {
t.Fatalf("failed to create saved card: %v", err)
}
key := "till-midflight-clawback-key"
var giftCardID string
err = tx.QueryRow(ctx, `
INSERT INTO gift_cards (total_funds_added, amount_remaining, created_by, is_inventory, voucher_type_at_purchase)
VALUES (50.00, 50.00, $1, FALSE, 'SPV')
RETURNING id
`, adminID).Scan(&giftCardID)
if err != nil {
t.Fatalf("failed to create gift card: %v", err)
}
var saleID string
err = tx.QueryRow(ctx, `
INSERT INTO till_sales (item_type, item_id, description, quantity, unit_price, total_amount,
payment_method, status, user_id, user_saved_card_id, idempotency_key, created_by, created_at, updated_at)
VALUES ('gift_card', $1, 'Gift Card create', 1, 50.00, 50.00, 'online_square', 'pending',
$2, $3, $4, $5, NOW(), NOW())
RETURNING id
`, giftCardID, userID, cardID, key, adminID).Scan(&saleID)
if err != nil {
t.Fatalf("failed to seed pending till sale: %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 setup tx: %v", err)
}
pool := context.Background()
t.Cleanup(func() {
var recreditedIDs []string
rows, qErr := db.Conn.Query(pool, `SELECT id FROM gift_cards WHERE created_by = $1 AND id <> $2`, adminID, giftCardID)
if qErr == nil {
for rows.Next() {
var id string
if rows.Scan(&id) == nil {
recreditedIDs = append(recreditedIDs, id)
}
}
rows.Close()
}
for _, id := range recreditedIDs {
_, _ = db.Conn.Exec(pool, `DELETE FROM gift_card_transactions WHERE gift_card_id = $1`, id)
}
_, _ = db.Conn.Exec(pool, `DELETE FROM till_sales WHERE id = $1`, saleID)
_, _ = db.Conn.Exec(pool, `DELETE FROM gift_card_transactions WHERE gift_card_id = $1`, giftCardID)
_, _ = db.Conn.Exec(pool, `DELETE FROM gift_cards WHERE id = $1`, giftCardID)
for _, id := range recreditedIDs {
_, _ = db.Conn.Exec(pool, `DELETE FROM gift_cards WHERE id = $1`, id)
}
_, _ = db.Conn.Exec(pool, `DELETE FROM user_saved_cards WHERE id = $1`, cardID)
_, _ = db.Conn.Exec(pool, `DELETE FROM users WHERE id IN ($1, $2)`, userID, adminID)
})
const chargeID = "sqp_midflight_clawback_charge"
origClient := SquareClient
SquareClient = &tillMidFlightClawbackClient{SquareClient: square.NewDevClient(), saleID: saleID, giftCardID: giftCardID, chargeID: chargeID}
defer func() { SquareClient = origClient }()
logBuf, restore := captureStdLog(t)
defer restore()
reqBody := TillSaleRequest{
ItemType: "gift_card",
Action: "create",
Amount: 50.00,
PaymentMethod: "saved_card",
UserSavedCardID: &cardID,
UserID: &userID,
IdempotencyKey: key,
}
bodyBytes, _ := json.Marshal(reqBody)
req := httptest.NewRequest("POST", "/api/admin/till/sale", bytes.NewReader(bodyBytes))
req.Header.Set("Authorization", "Bearer "+adminToken)
req.Header.Set("Content-Type", "application/json")
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 the clawed-back mid-flight retry to fail the response (500), got %d. body: %s", w.Code, w.Body.String())
}
var status string
if err := db.Conn.QueryRow(pool, `SELECT status FROM till_sales WHERE id = $1`, saleID).Scan(&status); err != nil {
t.Fatalf("failed to query till sale: %v", err)
}
if status != "failed" {
t.Errorf("expected the sweep-resolved sale to stay 'failed' (not resurrected), got %q", status)
}
// FIX 1: the funded gift card the clawback deleted must have been RE-CREATED
// with the sale's value, and the sale re-pointed at it.
var recreditedID string
var recreditedTotal, recreditedRemaining float64
if err := db.Conn.QueryRow(pool, `
SELECT id, total_funds_added, amount_remaining FROM gift_cards
WHERE created_by = $1 AND id <> $2
`, adminID, giftCardID).Scan(&recreditedID, &recreditedTotal, &recreditedRemaining); err != nil {
t.Fatalf("expected a re-credited gift card for the mid-flight clawback, none found: %v", err)
}
if recreditedTotal != 50.00 || recreditedRemaining != 50.00 {
t.Errorf("expected the re-credited gift card funded at £50.00, got total £%.2f remaining £%.2f", recreditedTotal, recreditedRemaining)
}
var itemID string
if err := db.Conn.QueryRow(pool, `SELECT COALESCE(item_id, '') FROM till_sales WHERE id = $1`, saleID).Scan(&itemID); err != nil {
t.Fatalf("failed to query till sale item_id: %v", err)
}
if itemID != recreditedID {
t.Errorf("expected till sale %s re-pointed at the re-credited card %s, got item_id %s", saleID, recreditedID, itemID)
}
var txCount int
if err := db.Conn.QueryRow(pool, `SELECT COUNT(*) FROM gift_card_transactions WHERE gift_card_id = $1 AND reference_type = 'till_sale' AND reference_id = $2`, recreditedID, saleID).Scan(&txCount); err != nil {
t.Fatalf("failed to count re-credited purchase transaction: %v", err)
}
if txCount != 1 {
t.Errorf("expected exactly 1 purchase transaction on the re-credited card, got %d", txCount)
}
if got := logBuf.String(); !strings.Contains(got, "already resolved (0 rows updated)") {
t.Errorf("expected the CRITICAL 0-rows reconciliation log, got: %s", got)
}
if got := logBuf.String(); !strings.Contains(got, "gift-card funding re-credited") {
t.Errorf("expected the funding-re-credited log, got: %s", got)
}
}
@@ -119,22 +119,25 @@ func TestWebhook_AndSweep_DoNotDoubleComplete(t *testing.T) {
t.Errorf("expected the sweep to leave the webhook-completed payment alone, got %q", got) t.Errorf("expected the sweep to leave the webhook-completed payment alone, got %q", got)
} }
// No split records / completion side effects may have been applied by the // The webhook itself ran the payable-booking completion side-effects
// sweep (the rescue is gated on the row still pending). // (round-8 fix 2): the single £10 pending charge was re-split into a £5
// deposit primary + £5 balance record, and the fully-paid booking was
// completed. The sweep adds NOTHING on top — exactly one deposit + one
// balance row, no duplicates.
var recordCount int var recordCount int
if err := db.Conn.QueryRow(context.Background(), if err := db.Conn.QueryRow(context.Background(),
"SELECT COUNT(*) FROM payments WHERE booking_id = $1", bookingID).Scan(&recordCount); err != nil { "SELECT COUNT(*) FROM payments WHERE booking_id = $1", bookingID).Scan(&recordCount); err != nil {
t.Fatalf("failed to count payment records: %v", err) t.Fatalf("failed to count payment records: %v", err)
} }
if recordCount != 1 { if recordCount != 2 {
t.Errorf("expected exactly the one webhook-completed payment row (no sweep splits), got %d", recordCount) t.Errorf("expected the webhook-completed booking to have exactly 2 records (deposit + balance split), got %d", recordCount)
} }
var bookingStatus string var bookingStatus string
if err := db.Conn.QueryRow(context.Background(), if err := db.Conn.QueryRow(context.Background(),
"SELECT status FROM bookings WHERE id = $1", bookingID).Scan(&bookingStatus); err != nil { "SELECT status FROM bookings WHERE id = $1", bookingID).Scan(&bookingStatus); err != nil {
t.Fatalf("failed to query booking: %v", err) t.Fatalf("failed to query booking: %v", err)
} }
if bookingStatus != "pending" { if bookingStatus != "completed" {
t.Errorf("expected the sweep not to complete the booking after the webhook settled the row, got %q", bookingStatus) t.Errorf("expected the fully-paid webhook-completed booking to end 'completed', got %q", bookingStatus)
} }
} }
@@ -717,6 +717,10 @@ func TestWebhook_DisputeStateUpdated_Open_KeepsOpen(t *testing.T) {
func TestWebhook_PaymentUpdated_UpdatesPaymentStatus(t *testing.T) { func TestWebhook_PaymentUpdated_UpdatesPaymentStatus(t *testing.T) {
const squarePaymentID = "sqp_updated_completed" const squarePaymentID = "sqp_updated_completed"
payID := createWebhookTestPayment(t, squarePaymentID, "pending") payID := createWebhookTestPayment(t, squarePaymentID, "pending")
// The webhook's booking gate (round-8) only completes booking-attached
// rows — a booking-less row is a gift-card purchase and is left pending
// (C6). Attach a payable booking so the gate completes the payment.
attachWebhookTestBooking(t, payID, 10.00)
event := SquareWebhookEvent{ event := SquareWebhookEvent{
Type: "payment.updated", Type: "payment.updated",
@@ -872,6 +876,9 @@ func TestWebhook_PaymentUpdated_Completed_RescuesPendingTillSale(t *testing.T) {
func TestWebhook_PaymentUpdated_IdempotentReplay(t *testing.T) { func TestWebhook_PaymentUpdated_IdempotentReplay(t *testing.T) {
const squarePaymentID = "sqp_updated_idem" const squarePaymentID = "sqp_updated_idem"
payID := createWebhookTestPayment(t, squarePaymentID, "pending") payID := createWebhookTestPayment(t, squarePaymentID, "pending")
// The webhook's booking gate (round-8) only completes booking-attached
// rows — attach a payable booking so the gate completes the payment.
attachWebhookTestBooking(t, payID, 10.00)
event := SquareWebhookEvent{ event := SquareWebhookEvent{
Type: "payment.updated", Type: "payment.updated",
@@ -1016,10 +1023,15 @@ func TestWebhook_PaymentUpdated_OrphanedReplay_MarksOriginFailed(t *testing.T) {
} }
} }
// TestWebhook_PaymentUpdated_OrphanedReplay_NoOrigin_Noop verifies the orphan // TestWebhook_PaymentUpdated_OrphanedReplay_NoOrigin_Retries verifies the
// detection is a no-op when no pending origin row matches: the event is // round-8 fix 3 behavior: a COMPLETED payment whose square_payment_id matches
// acknowledged 200 without touching any row or raising a notification. // NO local row (payments or till_sales) AND NO pending origin row by
func TestWebhook_PaymentUpdated_OrphanedReplay_NoOrigin_Noop(t *testing.T) { // idempotency key/reference_id is a GENUINELY unknown charge. It is no longer
// acked 200 — the handler returns 503 so Square re-delivers (its retry budget
// bounds the retries) and writes NO dedup row, so the event can never be
// dropped permanently. No critical notification is raised (there is no issue to
// attribute, just an unresolved event).
func TestWebhook_PaymentUpdated_OrphanedReplay_NoOrigin_Retries(t *testing.T) {
const orphanSquareID = "sqp_orphan_noorigin" const orphanSquareID = "sqp_orphan_noorigin"
before := countCriticalNotifications(t) before := countCriticalNotifications(t)
@@ -1041,14 +1053,14 @@ func TestWebhook_PaymentUpdated_OrphanedReplay_NoOrigin_Noop(t *testing.T) {
}`), }`),
} }
w := deliverWebhook(t, event) w := deliverWebhook(t, event)
if w.Code != http.StatusOK { if w.Code != http.StatusServiceUnavailable {
t.Fatalf("expected 200, got %d: %s", w.Code, w.Body.String()) t.Fatalf("expected 503 for a genuinely unknown COMPLETED payment (unresolved money event must be retried, not acked), got %d. body: %s", w.Code, w.Body.String())
} }
if n := countCriticalNotifications(t) - before; n != 0 { if n := countCriticalNotifications(t) - before; n != 0 {
t.Errorf("expected no new critical notification with no origin match, got %d", n) t.Errorf("expected no new critical notification with no origin match, got %d", n)
} }
if got := countWebhookEvents(t, event.EventID); got != 1 { if got := countWebhookEvents(t, event.EventID); got != 0 {
t.Errorf("expected 1 dedup row, got %d", got) t.Errorf("expected NO dedup row for the unresolved unknown payment (Square must retry), got %d", got)
} }
} }
@@ -1602,6 +1614,9 @@ func TestWebhook_RefundUpdated_DoesNotDemoteCompleted(t *testing.T) {
func TestWebhook_EventTypeAliases_RouteToUpdatedHandlers(t *testing.T) { func TestWebhook_EventTypeAliases_RouteToUpdatedHandlers(t *testing.T) {
const sqPayID = "sqp_alias_pay" const sqPayID = "sqp_alias_pay"
payID := createWebhookTestPayment(t, sqPayID, "pending") payID := createWebhookTestPayment(t, sqPayID, "pending")
// The webhook's booking gate (round-8) only completes booking-attached
// rows — attach a payable booking so the gate completes the payment.
attachWebhookTestBooking(t, payID, 10.00)
payEvent := SquareWebhookEvent{ payEvent := SquareWebhookEvent{
Type: "payment.created", Type: "payment.created",
+14 -6
View File
@@ -423,10 +423,11 @@ func TestHandleSquareWebhook_BodyTooLarge(t *testing.T) {
func TestHandleSquareWebhook_ValidSignatureWithEnvKey(t *testing.T) { func TestHandleSquareWebhook_ValidSignatureWithEnvKey(t *testing.T) {
// Well-formed payment.updated event carrying the full nested payment object // Well-formed payment.updated event carrying the full nested payment object
// (data.object.payment with id/status) so handlePaymentUpdated can parse // (data.object.payment with id/status) so handlePaymentUpdated can parse
// and dispatch it — a known money event whose payload fails to parse // and dispatch it. The COMPLETED charge matches NO local row — round-8 fix
// returns 503 without a dedup row (Square retries), which is NOT this // 3: a genuinely unknown completed payment is retried (503, no dedup row)
// test's intent. The unique event_id and square_payment_id avoid colliding // rather than acked, so the signature passing is what this test proves (a
// with the other tests' dedup rows and payment fixtures. // bad signature would 403 before dispatch). The unique event_id avoids
// colliding with the other tests' dedup rows.
body := []byte(fmt.Sprintf(`{"type":"payment.updated","event_id":"evt_envkey_1","created_at":%q,"data":{"object":{"payment":{"id":"sqp_env_key_1","status":"COMPLETED","amount_money":{"amount":5000,"currency":"GBP"},"updated_at":%q}}}}`, nowInRFC3339(0), nowInRFC3339(0))) body := []byte(fmt.Sprintf(`{"type":"payment.updated","event_id":"evt_envkey_1","created_at":%q,"data":{"object":{"payment":{"id":"sqp_env_key_1","status":"COMPLETED","amount_money":{"amount":5000,"currency":"GBP"},"updated_at":%q}}}}`, nowInRFC3339(0), nowInRFC3339(0)))
key := "env-signing-key" key := "env-signing-key"
notificationURL := "http://localhost:8080/webhooks/square" notificationURL := "http://localhost:8080/webhooks/square"
@@ -439,8 +440,8 @@ func TestHandleSquareWebhook_ValidSignatureWithEnvKey(t *testing.T) {
t.Setenv("SQUARE_WEBHOOK_SIGNATURE_KEY", key) t.Setenv("SQUARE_WEBHOOK_SIGNATURE_KEY", key)
w := makeWebhookRequest(body, sig, context.Background()) w := makeWebhookRequest(body, sig, context.Background())
if w.Code != http.StatusOK { if w.Code != http.StatusServiceUnavailable {
t.Errorf("expected 200 with valid signature, got %d. body: %s", w.Code, w.Body.String()) t.Errorf("expected 503 for a signed COMPLETED payment.updated with no local row (unresolved unknown money event is retried, not acked), got %d. body: %s", w.Code, w.Body.String())
} }
} }
@@ -714,6 +715,10 @@ func TestHandleSquareWebhook_ConcurrentSameEvent_Serialized(t *testing.T) {
squarePaymentID := fmt.Sprintf("sqp_concurrent_same_%d", seq) squarePaymentID := fmt.Sprintf("sqp_concurrent_same_%d", seq)
eventID := fmt.Sprintf("evt_concurrent_same_%d", seq) eventID := fmt.Sprintf("evt_concurrent_same_%d", seq)
payID := createWebhookTestPayment(t, squarePaymentID, "pending") payID := createWebhookTestPayment(t, squarePaymentID, "pending")
// The webhook's booking gate (round-8) only completes booking-attached
// rows — a booking-less row is a gift-card purchase and is left pending
// (C6). Attach a payable booking so the gate completes the payment.
attachWebhookTestBooking(t, payID, 10.00)
event := SquareWebhookEvent{ event := SquareWebhookEvent{
Type: "payment.updated", Type: "payment.updated",
@@ -791,6 +796,9 @@ func TestHandleSquareWebhook_DedupCacheEviction_Redispatches(t *testing.T) {
const squarePaymentID = "sqp_dedup_eviction" const squarePaymentID = "sqp_dedup_eviction"
payID := createWebhookTestPayment(t, squarePaymentID, "pending") payID := createWebhookTestPayment(t, squarePaymentID, "pending")
// The webhook's booking gate (round-8) only completes booking-attached
// rows — attach a payable booking so the gate completes the payment.
attachWebhookTestBooking(t, payID, 10.00)
event := SquareWebhookEvent{ event := SquareWebhookEvent{
Type: "payment.updated", Type: "payment.updated",