Revive soft-deleted saved cards in SaveCardForUser upsert (DO NOTHING → DO UPDATE)

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.
This commit is contained in:
2026-08-22 00:34:49 +01:00
parent a8d54f1e2a
commit 63226debb7
2 changed files with 57 additions and 17 deletions
@@ -5,6 +5,7 @@ package payments
import ( import (
"bytes" "bytes"
"context" "context"
"database/sql"
"fmt" "fmt"
"log/slog" "log/slog"
"net/http" "net/http"
@@ -411,3 +412,39 @@ func TestDeletePaymentMethod_LogsRedactCardToken(t *testing.T) {
t.Errorf("expected redacted token prefix in logs, got %q", logs) 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")
}
+20 -17
View File
@@ -749,31 +749,34 @@ func (s *PaymentService) EnsureSquareCustomerForSavedCard(ctx context.Context, s
// the DB and, on a first-save flow, re-run CreateCustomer). // 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) { func (s *PaymentService) SaveCardForUser(ctx context.Context, userID, squareCustomerID, squareCardID, brand, last4 string, expMonth, expYear int, fingerprint string) (string, error) {
var id string var id string
// ON CONFLICT (user_id, square_card_id) DO NOTHING: resolveChargeSource // ON CONFLICT (user_id, square_card_id) DO UPDATE — two cases:
// runs CreateCardOnFile (deterministic sha256 key) + SaveCardForUser on a // 1. Same-key retry of a save_card=true charge (resolveChargeSource runs
// same-key retry of a save_card=true charge. Square returns the SAME ccof: // CreateCardOnFile with a deterministic sha256 key, so Square returns the
// id on the retry, so a plain INSERT would violate the per-user UNIQUE // SAME ccof: id): a plain INSERT would violate the per-user UNIQUE
// constraint (N-8). Upsert instead so the retry returns the existing row; // constraint (N-8) and DO NOTHING would 500 via the fallback select.
// a distinct card id is a brand-new row, never a mutation of another // 2. A soft-deleted card (DeletePaymentMethod set deleted_at but the row
// user's card. // 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, ` err := db.Conn.QueryRow(ctx, `
INSERT INTO user_saved_cards ( 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 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()) ) 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 RETURNING id
`, userID, squareCardID, squareCustomerID, brand, last4, expMonth, expYear, fingerprint).Scan(&id) `, userID, squareCardID, squareCustomerID, brand, last4, expMonth, expYear, fingerprint).Scan(&id)
if err != nil { 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 "", err
} }
return id, nil return id, nil