From 63226debb7715b28b27c3906ad4f2b959b6ff9eb Mon Sep 17 00:00:00 2001 From: Stephen Adamson Date: Tue, 4 Aug 2026 00:13:21 +0100 Subject: [PATCH] =?UTF-8?q?Revive=20soft-deleted=20saved=20cards=20in=20Sa?= =?UTF-8?q?veCardForUser=20upsert=20(DO=20NOTHING=20=E2=86=92=20DO=20UPDAT?= =?UTF-8?q?E)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A soft-deleted card row (DeletePaymentMethod sets deleted_at) still occupies the non-partial UNIQUE (user_id, square_card_id) slot. Re-saving the same physical card via a save_card=true charge hit ON CONFLICT DO NOTHING, got pgx.ErrNoRows, and the deleted_at IS NULL fallback SELECT also missed the row → 500. DO UPDATE now revives it (deleted_at/retained_until = NULL), matching CreatePaymentMethodFromToken's revival semantics; the ErrNoRows fallback is removed. Regression test: soft-delete then re-save same card id → existing row returned, deleted_at cleared. --- .../payments/payments_r6_review_test.go | 37 +++++++++++++++++++ backend/handlers/payments/service.go | 37 ++++++++++--------- 2 files changed, 57 insertions(+), 17 deletions(-) diff --git a/backend/handlers/payments/payments_r6_review_test.go b/backend/handlers/payments/payments_r6_review_test.go index 24a4e61..de26cbb 100644 --- a/backend/handlers/payments/payments_r6_review_test.go +++ b/backend/handlers/payments/payments_r6_review_test.go @@ -5,6 +5,7 @@ package payments import ( "bytes" "context" + "database/sql" "fmt" "log/slog" "net/http" @@ -411,3 +412,39 @@ func TestDeletePaymentMethod_LogsRedactCardToken(t *testing.T) { t.Errorf("expected redacted token prefix in logs, got %q", logs) } } + +// TestSaveCardForUser_RevivesSoftDeletedCard verifies the SaveCardForUser +// upsert: a user who soft-deleted a card (DeletePaymentMethod sets deleted_at, +// but the row still occupies the UNIQUE (user_id, square_card_id) slot) and +// then re-saves the SAME physical card via a save_card=true charge must get +// the existing row revived — NOT a pgx.ErrNoRows 500 from the old +// DO NOTHING + deleted_at IS NULL fallback. +func TestSaveCardForUser_RevivesSoftDeletedCard(t *testing.T) { + ctx, tx := testutils.SetupTestTx(t) + userID, err := fixtures.CreateTestUser(tx) + require.NoError(t, err) + + origClient := SquareClient + rec := &deletingCardClient{SquareClient: square.NewDevClient()} + SquareClient = rec + defer func() { SquareClient = origClient }() + + cardID, err := fixtures.CreateTestPaymentMethod(tx, userID, "ccof:sq_revive", "VISA", "4242") + require.NoError(t, err) + + svc := NewPaymentService() + require.NoError(t, svc.DeletePaymentMethod(ctx, cardID, userID), "soft-delete must succeed") + + var deletedAt string + require.NoError(t, tx.QueryRow(ctx, `SELECT COALESCE(deleted_at::text, '') FROM user_saved_cards WHERE id = $1`, cardID).Scan(&deletedAt)) + require.NotEqual(t, "", deletedAt, "precondition: card must be soft-deleted") + + // Re-save the same physical card (same square_card_id → same UNIQUE slot). + revivedID, err := svc.SaveCardForUser(ctx, userID, "cus_sq_revive", "ccof:sq_revive", "VISA", "4242", 12, 2030, "revive_fp") + require.NoError(t, err, "re-saving a soft-deleted card must not error") + require.Equal(t, cardID, revivedID, "the revived card must be the existing row, not a new insert") + + var revivedDeletedAt sql.NullString + require.NoError(t, tx.QueryRow(ctx, `SELECT deleted_at FROM user_saved_cards WHERE id = $1`, revivedID).Scan(&revivedDeletedAt)) + require.False(t, revivedDeletedAt.Valid, "the revived card must have deleted_at cleared") +} diff --git a/backend/handlers/payments/service.go b/backend/handlers/payments/service.go index 3ed0204..fe5b162 100644 --- a/backend/handlers/payments/service.go +++ b/backend/handlers/payments/service.go @@ -749,31 +749,34 @@ func (s *PaymentService) EnsureSquareCustomerForSavedCard(ctx context.Context, s // the DB and, on a first-save flow, re-run CreateCustomer). func (s *PaymentService) SaveCardForUser(ctx context.Context, userID, squareCustomerID, squareCardID, brand, last4 string, expMonth, expYear int, fingerprint string) (string, error) { var id string - // ON CONFLICT (user_id, square_card_id) DO NOTHING: resolveChargeSource - // runs CreateCardOnFile (deterministic sha256 key) + SaveCardForUser on a - // same-key retry of a save_card=true charge. Square returns the SAME ccof: - // id on the retry, so a plain INSERT would violate the per-user UNIQUE - // constraint (N-8). Upsert instead so the retry returns the existing row; - // a distinct card id is a brand-new row, never a mutation of another - // user's card. + // ON CONFLICT (user_id, square_card_id) DO UPDATE — two cases: + // 1. Same-key retry of a save_card=true charge (resolveChargeSource runs + // CreateCardOnFile with a deterministic sha256 key, so Square returns the + // SAME ccof: id): a plain INSERT would violate the per-user UNIQUE + // constraint (N-8) and DO NOTHING would 500 via the fallback select. + // 2. A soft-deleted card (DeletePaymentMethod set deleted_at but the row + // still occupies the UNIQUE slot): the DO UPDATE revives it + // (deleted_at/retained_until = NULL), matching CreatePaymentMethodFromToken. + // The conflict target is scoped per user — a card tokenized by user B that + // user A already saved is a brand-new row for B, never a mutation of A's row. err := db.Conn.QueryRow(ctx, ` INSERT INTO user_saved_cards ( user_id, square_card_id, square_customer_id, brand, last_4, exp_month, exp_year, fingerprint, is_default, created_at ) VALUES ($1, $2, $3, $4, $5, $6, $7, $8, false, NOW()) - ON CONFLICT (user_id, square_card_id) DO NOTHING + ON CONFLICT (user_id, square_card_id) DO UPDATE SET + square_customer_id = EXCLUDED.square_customer_id, + brand = EXCLUDED.brand, + last_4 = EXCLUDED.last_4, + exp_month = EXCLUDED.exp_month, + exp_year = EXCLUDED.exp_year, + fingerprint = EXCLUDED.fingerprint, + deleted_at = NULL, + retained_until = NULL + WHERE user_saved_cards.user_id = EXCLUDED.user_id RETURNING id `, userID, squareCardID, squareCustomerID, brand, last4, expMonth, expYear, fingerprint).Scan(&id) if err != nil { - if errors.Is(err, pgx.ErrNoRows) { - if err := db.Conn.QueryRow(ctx, ` - SELECT id FROM user_saved_cards - WHERE user_id = $1 AND square_card_id = $2 AND deleted_at IS NULL - `, userID, squareCardID).Scan(&id); err != nil { - return "", err - } - return id, nil - } return "", err } return id, nil