Fix review findings: expiry bug (all 8 files), idempotency keys, card_expiry/card_cvc removal, URL encoding, BuyerEmail logging, ValidateCardInfo, saved-card test, future work doc
Backend: - Fix refund idempotency key: clock.Now() → deterministic (pr.ID + amount) - Fix ValidateCardInfo: enforce mutual exclusivity, handle empty strings symmetrically - Fix paymentFromSquare brand fallback (remove dead SourceType fallback) - Fix URL encoding: PathEscape → QueryEscape for customer_id query param - Fix BuyerEmail: log warning on DB error instead of silent discard - Fix idempotency key in createCardOnFileHTTP: time.Now() → deterministic hex hash - Add BuyerEmail to CreateTipPayment Square request - Move realBaseURL from shared file to square_dev.go (only used in dev) - Add TestTipPayment_WithSavedCard test (card_id path coverage) - Fix AMEX brand in mock (AMEX → AMERICAN_EXPRESS, fix test) Frontend: - Fix off-by-month expiry bug in ALL 8 files using year-month arithmetic (parseExpiryParts returns 1-indexed, SvelteDate expects 0-indexed) Files: tip/+page, pay-tip/[id], UserBookingModal, UserPaymentModal, BookingFlow, account/+page (add card + buy gift card sections) - Remove card_expiry/card_cvc from tip request bodies (backend has no fields) Docs: - Mark P9 (placeholder tokens) as completed, add P11 (Square Web Payments SDK) - Mark T13 (rune arithmetic) as completed
This commit is contained in:
@@ -1941,6 +1941,13 @@ func CreateTipPayment(w http.ResponseWriter, r *http.Request) {
|
||||
|
||||
// Step 2: DB transaction committed — safe to call Square now.
|
||||
// If Square fails, the record stays 'pending' for manual retry.
|
||||
|
||||
// Resolve the user's email for Square receipt delivery.
|
||||
var buyerEmail string
|
||||
if err := db.Conn.QueryRow(r.Context(), `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",
|
||||
@@ -1948,6 +1955,7 @@ func CreateTipPayment(w http.ResponseWriter, r *http.Request) {
|
||||
IdempotencyKey: idempotencyKey,
|
||||
ReferenceID: bookingID,
|
||||
Note: "tip",
|
||||
BuyerEmail: buyerEmail,
|
||||
}
|
||||
|
||||
paymentResult, err := SquareClient.CreatePayment(r.Context(), paymentReq)
|
||||
|
||||
@@ -15,6 +15,7 @@ import (
|
||||
|
||||
"crussell/clock"
|
||||
"crussell/db"
|
||||
"crussell/internal/square"
|
||||
"crussell/mw"
|
||||
"crussell/testutils"
|
||||
"crussell/testutils/fixtures"
|
||||
@@ -22,6 +23,8 @@ import (
|
||||
|
||||
"github.com/go-chi/chi/v5"
|
||||
"github.com/jackc/pgx/v5"
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
)
|
||||
|
||||
func makePaymentRequest(handler http.HandlerFunc, method, path string, body interface{}, token string, ctx context.Context) *httptest.ResponseRecorder {
|
||||
@@ -1858,6 +1861,59 @@ func TestTipPayment_MultipleTipsAllowed(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestTipPayment_WithSavedCard(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.
|
||||
payID, err := fixtures.CreateTestPayment(tx, bookingID, 5000.00, "online_square", "full", "completed")
|
||||
require.NoError(t, err)
|
||||
squarePayID := "sqp_test_saved_card"
|
||||
_, 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, `
|
||||
INSERT INTO user_saved_cards (user_id, square_card_id, brand, last_4, exp_month, exp_year, fingerprint)
|
||||
VALUES ($1, 'ccof_mock_saved', 'VISA', '1111', 12, 2030, 'sqfp_mock_saved')
|
||||
RETURNING id
|
||||
`, userID).Scan(&savedCardID)
|
||||
require.NoError(t, err)
|
||||
|
||||
req := CreateTipPaymentRequest{
|
||||
Amount: 1000,
|
||||
CardID: &savedCardID,
|
||||
}
|
||||
|
||||
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, "tip", resp.PaymentType)
|
||||
assert.Equal(t, int64(1000), resp.Amount)
|
||||
assert.Equal(t, "completed", resp.Status)
|
||||
assert.NotEmpty(t, resp.CardBrand)
|
||||
assert.NotEmpty(t, resp.CardLast4)
|
||||
}
|
||||
|
||||
func TestTipPayment_TransactionFailure_SkipsSquare(t *testing.T) {
|
||||
t.Parallel()
|
||||
ctx, tx := testutils.SetupTestTx(t)
|
||||
|
||||
@@ -6,6 +6,7 @@ import (
|
||||
"log"
|
||||
"log/slog"
|
||||
"math"
|
||||
"strconv"
|
||||
"time"
|
||||
|
||||
"crussell/clock"
|
||||
@@ -540,10 +541,12 @@ func ProcessPendingSquareRefunds(ctx context.Context, bookingID string, reason s
|
||||
}
|
||||
|
||||
refundCents := int64(math.Round(pr.Amount * 100))
|
||||
// Deterministic idempotency key derived from refund record ID and amount,
|
||||
// so retries (network timeout, proxy, etc.) don't create duplicate Square refunds.
|
||||
refundReq := square.RefundPaymentReq{
|
||||
PaymentID: *pr.SquarePaymentID,
|
||||
Amount: refundCents,
|
||||
IdempotencyKey: pr.ID + "-square-" + clock.Now().Format("20060102150405"),
|
||||
IdempotencyKey: pr.ID + "-square-" + strconv.FormatInt(refundCents, 10),
|
||||
Reason: reason,
|
||||
}
|
||||
sqResult, sqErr := SquareClient.RefundPayment(ctx, refundReq)
|
||||
|
||||
@@ -53,9 +53,15 @@ func ValidateRefundReason(reason string) error {
|
||||
return nil
|
||||
}
|
||||
|
||||
// ValidateCardInfo checks that at least one of cardID or newCardToken is provided
|
||||
// ValidateCardInfo checks that exactly one of cardID or newCardToken is provided,
|
||||
// non-nil, and non-empty.
|
||||
func ValidateCardInfo(cardID, newCardToken *string) error {
|
||||
if cardID == nil && (newCardToken == nil || *newCardToken == "") {
|
||||
hasCardID := cardID != nil && *cardID != ""
|
||||
hasToken := newCardToken != nil && *newCardToken != ""
|
||||
if hasCardID && hasToken {
|
||||
return errors.New("provide either card_id or new_card_token, not both")
|
||||
}
|
||||
if !hasCardID && !hasToken {
|
||||
return errors.New("either card_id or new_card_token is required")
|
||||
}
|
||||
return nil
|
||||
|
||||
Reference in New Issue
Block a user