popertots/Crussell
1
1
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix(backend): address gift card review findings

Browse Source 
Fix #1: remove expiry_date from gift card INSERTs (rolling 24-month via last_used_at only)

Fix #2: add FOR UPDATE to RedeemGiftCard SELECT (race condition)

Fix #6: add ?type=customer|inventory filter to GetGiftCards

Fix #7: add admin_audit_log INSERT to GetUserGiftCardBalanceAdmin

Fix #10: refactor normalizeCode to validators.NormalizeGiftCardCode

Schema: add admin_audit_log table (GDPR Article 30), update testdb.go

Tests: expiry-date-null, inventory-filter, audit-log, db table references

Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent)

Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
This commit is contained in:
popertots
2026-06-12 10:50:49 +01:00
co-authored by Sisyphus
parent e7fd9c89eb
commit 3c447a5c33
6 changed files with 399 additions and 154 deletions
Download Patch File Download Diff File Expand all files Collapse all files
backend/handlers/payments/giftcards.go
+97 -92
View File
@@ -4,9 +4,11 @@ import (
"database/sql"
"encoding/json"
"errors"
"fmt"
"log"
"net/http"
"strconv"
"strings"
"time"
"crussell/db"
@@ -123,7 +125,7 @@ func GetGiftCards(w http.ResponseWriter, r *http.Request) {
`).Scan(&resp.TotalUnclaimed)
if err != nil {
log.Printf("Failed to calculate total unclaimed gift cards: %v", err)
http.Error(w, "Internal server error", http.StatusInternalServerError)
http.Error(w, "internal server error", http.StatusInternalServerError)
return
}
@@ -133,60 +135,62 @@ func GetGiftCards(w http.ResponseWriter, r *http.Request) {
`).Scan(&resp.TotalUserBalances)
if err != nil {
log.Printf("Failed to calculate total user balances: %v", err)
http.Error(w, "Internal server error", http.StatusInternalServerError)
http.Error(w, "internal server error", http.StatusInternalServerError)
return
}
// --- Gift cards (paginated, optionally filtered) ---
filterType := query.Get("type")
whereClauses := []string{}
if searchTerm != "" {
whereClauses = append(whereClauses, "id ILIKE $1")
}
if filterType == "customer" {
whereClauses = append(whereClauses, "is_inventory = FALSE")
} else if filterType == "inventory" {
whereClauses = append(whereClauses, "is_inventory = TRUE")
}
whereSQL := ""
if len(whereClauses) > 0 {
whereSQL = "WHERE " + strings.Join(whereClauses, " AND ")
}
var gcTotal int
var gcCountQuery string
var gcListQuery string
var gcCountArgs []interface{}
var gcListArgs []interface{}
gcCountQuery := fmt.Sprintf(`SELECT COUNT(*) FROM gift_cards %s`, whereSQL)
gcListQuery := fmt.Sprintf(`
SELECT id, total_funds_added, amount_remaining, created_by, created_at, redeemed_at, redeemed_by, is_inventory, last_used_at
FROM gift_cards
%s
ORDER BY created_at DESC
LIMIT $%%d OFFSET $%%d
`, whereSQL)
if searchTerm != "" {
searchPattern := "%" + searchTerm + "%"
gcCountQuery = `
SELECT COUNT(*)
FROM gift_cards
WHERE id ILIKE $1
`
gcCountArgs = []interface{}{searchPattern}
gcListQuery = `
SELECT id, total_funds_added, amount_remaining, created_by, created_at, redeemed_at, redeemed_by, is_inventory, last_used_at
FROM gift_cards
WHERE id ILIKE $1
ORDER BY created_at DESC
LIMIT $2 OFFSET $3
`
gcListArgs = []interface{}{searchPattern, perPage, offset}
gcListQuery = fmt.Sprintf(gcListQuery, 2, 3)
} else {
gcCountQuery = `SELECT COUNT(*) FROM gift_cards`
gcCountArgs = []interface{}{}
gcListQuery = `
SELECT id, total_funds_added, amount_remaining, created_by, created_at, redeemed_at, redeemed_by, is_inventory, last_used_at
FROM gift_cards
ORDER BY created_at DESC
LIMIT $1 OFFSET $2
`
gcListArgs = []interface{}{perPage, offset}
gcListQuery = fmt.Sprintf(gcListQuery, 1, 2)
}
err = db.DB.QueryRow(ctx, gcCountQuery, gcCountArgs...).Scan(&gcTotal)
if err != nil {
log.Printf("Failed to count gift cards: %v", err)
http.Error(w, "Internal server error", http.StatusInternalServerError)
http.Error(w, "internal server error", http.StatusInternalServerError)
return
}
gcRows, err := db.DB.Query(ctx, gcListQuery, gcListArgs...)
if err != nil {
log.Printf("Failed to query gift cards: %v", err)
http.Error(w, "Internal server error", http.StatusInternalServerError)
http.Error(w, "internal server error", http.StatusInternalServerError)
return
}
defer gcRows.Close()
@@ -209,7 +213,7 @@ func GetGiftCards(w http.ResponseWriter, r *http.Request) {
)
if err != nil {
log.Printf("Failed to scan gift card: %v", err)
http.Error(w, "Internal server error", http.StatusInternalServerError)
http.Error(w, "internal server error", http.StatusInternalServerError)
return
}
@@ -263,7 +267,7 @@ func GetGiftCards(w http.ResponseWriter, r *http.Request) {
ubRows, err := db.DB.Query(ctx, ubListQuery, ubListArgs...)
if err != nil {
log.Printf("Failed to query user balances: %v", err)
http.Error(w, "Internal server error", http.StatusInternalServerError)
http.Error(w, "internal server error", http.StatusInternalServerError)
return
}
defer ubRows.Close()
@@ -281,7 +285,7 @@ func GetGiftCards(w http.ResponseWriter, r *http.Request) {
)
if err != nil {
log.Printf("Failed to scan user balance: %v", err)
http.Error(w, "Internal server error", http.StatusInternalServerError)
http.Error(w, "internal server error", http.StatusInternalServerError)
return
}
if ubTotal == 0 {
@@ -309,7 +313,7 @@ func CreateGiftCard(w http.ResponseWriter, r *http.Request) {
var req CreateGiftCardRequest
if err := json.NewDecoder(r.Body).Decode(&req); err != nil {
http.Error(w, "Invalid request", http.StatusBadRequest)
http.Error(w, "invalid request", http.StatusBadRequest)
return
}
@@ -325,7 +329,7 @@ func CreateGiftCard(w http.ResponseWriter, r *http.Request) {
tx, err := db.DB.Begin(ctx)
if err != nil {
log.Printf("Failed to begin transaction: %v", err)
http.Error(w, "Internal server error", http.StatusInternalServerError)
http.Error(w, "internal server error", http.StatusInternalServerError)
return
}
defer tx.Rollback(ctx)
@@ -347,7 +351,7 @@ func CreateGiftCard(w http.ResponseWriter, r *http.Request) {
)
if err != nil {
log.Printf("Failed to create gift card: %v", err)
http.Error(w, "Internal server error", http.StatusInternalServerError)
http.Error(w, "internal server error", http.StatusInternalServerError)
return
}
@@ -369,13 +373,13 @@ func CreateGiftCard(w http.ResponseWriter, r *http.Request) {
`, gc.ID, req.Amount, adminID, notes)
if err != nil {
log.Printf("Failed to record gift card transaction: %v", err)
http.Error(w, "Internal server error", http.StatusInternalServerError)
http.Error(w, "internal server error", http.StatusInternalServerError)
return
}
if err := tx.Commit(ctx); err != nil {
log.Printf("Failed to commit transaction: %v", err)
http.Error(w, "Internal server error", http.StatusInternalServerError)
http.Error(w, "internal server error", http.StatusInternalServerError)
return
}
@@ -395,7 +399,7 @@ func TopUpGiftCard(w http.ResponseWriter, r *http.Request) {
var req TopUpGiftCardRequest
if err := json.NewDecoder(r.Body).Decode(&req); err != nil {
http.Error(w, "Invalid request", http.StatusBadRequest)
http.Error(w, "invalid request", http.StatusBadRequest)
return
}
@@ -419,7 +423,7 @@ func TopUpGiftCard(w http.ResponseWriter, r *http.Request) {
tx, err := db.DB.Begin(ctx)
if err != nil {
log.Printf("Failed to begin transaction: %v", err)
http.Error(w, "Internal server error", http.StatusInternalServerError)
http.Error(w, "internal server error", http.StatusInternalServerError)
return
}
defer tx.Rollback(ctx)
@@ -434,7 +438,7 @@ func TopUpGiftCard(w http.ResponseWriter, r *http.Request) {
return
}
log.Printf("Failed to check gift card: %v", err)
http.Error(w, "Internal server error", http.StatusInternalServerError)
http.Error(w, "internal server error", http.StatusInternalServerError)
return
}
@@ -469,7 +473,7 @@ func TopUpGiftCard(w http.ResponseWriter, r *http.Request) {
)
if err != nil {
log.Printf("Failed to top up gift card: %v", err)
http.Error(w, "Internal server error", http.StatusInternalServerError)
http.Error(w, "internal server error", http.StatusInternalServerError)
return
}
@@ -483,13 +487,13 @@ func TopUpGiftCard(w http.ResponseWriter, r *http.Request) {
`, cardID, txType, req.Amount, adminID, notes)
if err != nil {
log.Printf("Failed to record gift card transaction: %v", err)
http.Error(w, "Internal server error", http.StatusInternalServerError)
http.Error(w, "internal server error", http.StatusInternalServerError)
return
}
if err := tx.Commit(ctx); err != nil {
log.Printf("Failed to commit transaction: %v", err)
http.Error(w, "Internal server error", http.StatusInternalServerError)
http.Error(w, "internal server error", http.StatusInternalServerError)
return
}
@@ -507,11 +511,11 @@ func TransferGiftCard(w http.ResponseWriter, r *http.Request) {
var req TransferGiftCardRequest
if err := json.NewDecoder(r.Body).Decode(&req); err != nil {
http.Error(w, "Invalid request", http.StatusBadRequest)
http.Error(w, "invalid request", http.StatusBadRequest)
return
}
req.ToCardID = normalizeCode(req.ToCardID)
req.ToCardID = validators.NormalizeGiftCardCode(req.ToCardID)
if !validators.IsValidID(req.ToCardID) {
http.Error(w, "Invalid destination gift card ID", http.StatusBadRequest)
return
@@ -530,7 +534,7 @@ func TransferGiftCard(w http.ResponseWriter, r *http.Request) {
tx, err := db.DB.Begin(ctx)
if err != nil {
log.Printf("Failed to begin transaction: %v", err)
http.Error(w, "Internal server error", http.StatusInternalServerError)
http.Error(w, "internal server error", http.StatusInternalServerError)
return
}
defer tx.Rollback(ctx)
@@ -545,7 +549,7 @@ func TransferGiftCard(w http.ResponseWriter, r *http.Request) {
return
}
log.Printf("Failed to check source gift card: %v", err)
http.Error(w, "Internal server error", http.StatusInternalServerError)
http.Error(w, "internal server error", http.StatusInternalServerError)
return
}
@@ -556,7 +560,7 @@ func TransferGiftCard(w http.ResponseWriter, r *http.Request) {
return
}
log.Printf("Failed to check destination gift card: %v", err)
http.Error(w, "Internal server error", http.StatusInternalServerError)
http.Error(w, "internal server error", http.StatusInternalServerError)
return
}
@@ -578,7 +582,7 @@ func TransferGiftCard(w http.ResponseWriter, r *http.Request) {
`, req.Amount, fromCardID)
if err != nil {
log.Printf("Failed to deduct from source: %v", err)
http.Error(w, "Internal server error", http.StatusInternalServerError)
http.Error(w, "internal server error", http.StatusInternalServerError)
return
}
@@ -591,13 +595,13 @@ func TransferGiftCard(w http.ResponseWriter, r *http.Request) {
`, req.Amount, req.ToCardID)
if err != nil {
log.Printf("Failed to add to destination: %v", err)
http.Error(w, "Internal server error", http.StatusInternalServerError)
http.Error(w, "internal server error", http.StatusInternalServerError)
return
}
if err := tx.Commit(ctx); err != nil {
log.Printf("Failed to commit transaction: %v", err)
http.Error(w, "Internal server error", http.StatusInternalServerError)
http.Error(w, "internal server error", http.StatusInternalServerError)
return
}
@@ -617,11 +621,11 @@ func RedeemGiftCard(w http.ResponseWriter, r *http.Request) {
var req RedeemGiftCardRequest
if err := json.NewDecoder(r.Body).Decode(&req); err != nil {
http.Error(w, "Invalid request", http.StatusBadRequest)
http.Error(w, "invalid request", http.StatusBadRequest)
return
}
code := normalizeCode(req.Code)
code := validators.NormalizeGiftCardCode(req.Code)
if !validators.IsValidID(code) {
http.Error(w, "Invalid gift card code format", http.StatusBadRequest)
return
@@ -630,7 +634,7 @@ func RedeemGiftCard(w http.ResponseWriter, r *http.Request) {
tx, err := db.DB.Begin(ctx)
if err != nil {
log.Printf("Failed to begin transaction: %v", err)
http.Error(w, "Internal server error", http.StatusInternalServerError)
http.Error(w, "internal server error", http.StatusInternalServerError)
return
}
defer tx.Rollback(ctx)
@@ -641,6 +645,7 @@ func RedeemGiftCard(w http.ResponseWriter, r *http.Request) {
SELECT amount_remaining, redeemed_by
FROM gift_cards
WHERE id = $1
FOR UPDATE
`, code).Scan(&amountRemaining, &redeemedBy)
if err != nil {
if errors.Is(err, pgx.ErrNoRows) {
@@ -648,7 +653,7 @@ func RedeemGiftCard(w http.ResponseWriter, r *http.Request) {
return
}
log.Printf("Failed to query gift card: %v", err)
http.Error(w, "Internal server error", http.StatusInternalServerError)
http.Error(w, "internal server error", http.StatusInternalServerError)
return
}
@@ -671,7 +676,7 @@ func RedeemGiftCard(w http.ResponseWriter, r *http.Request) {
`, userID, code)
if err != nil {
log.Printf("Failed to update gift card: %v", err)
http.Error(w, "Internal server error", http.StatusInternalServerError)
http.Error(w, "internal server error", http.StatusInternalServerError)
return
}
@@ -684,7 +689,7 @@ func RedeemGiftCard(w http.ResponseWriter, r *http.Request) {
`, userID, amountRemaining)
if err != nil {
log.Printf("Failed to update user gift card balance: %v", err)
http.Error(w, "Internal server error", http.StatusInternalServerError)
http.Error(w, "internal server error", http.StatusInternalServerError)
return
}
@@ -694,13 +699,13 @@ func RedeemGiftCard(w http.ResponseWriter, r *http.Request) {
`, code, amountRemaining, userID)
if err != nil {
log.Printf("Failed to record gift card transaction: %v", err)
http.Error(w, "Internal server error", http.StatusInternalServerError)
http.Error(w, "internal server error", http.StatusInternalServerError)
return
}
if err := tx.Commit(ctx); err != nil {
log.Printf("Failed to commit transaction: %v", err)
http.Error(w, "Internal server error", http.StatusInternalServerError)
http.Error(w, "internal server error", http.StatusInternalServerError)
return
}
@@ -728,7 +733,7 @@ func GetGiftCardBalance(w http.ResponseWriter, r *http.Request) {
return
}
log.Printf("Failed to query user balance: %v", err)
http.Error(w, "Internal server error", http.StatusInternalServerError)
http.Error(w, "internal server error", http.StatusInternalServerError)
return
}
@@ -745,6 +750,8 @@ func GetUserGiftCardBalanceAdmin(w http.ResponseWriter, r *http.Request) {
return
}
adminID, _ := ctx.Value(mw.UserIDKey).(string)
var balance float64
err := db.DB.QueryRow(ctx, "SELECT balance FROM user_giftcard_balances WHERE user_id = $1", userID).Scan(&balance)
if err != nil {
@@ -754,10 +761,18 @@ func GetUserGiftCardBalanceAdmin(w http.ResponseWriter, r *http.Request) {
return
}
log.Printf("Failed to query user balance: %v", err)
http.Error(w, "Internal server error", http.StatusInternalServerError)
http.Error(w, "internal server error", http.StatusInternalServerError)
return
}
detailsJSON := fmt.Sprintf(`{"balance": %.2f}`, balance)
if _, err := db.DB.Exec(ctx, `
INSERT INTO admin_audit_log (admin_id, action_type, target_user_id, details)
VALUES ($1, 'balance_check', $2, $3::jsonb)
`, adminID, userID, detailsJSON); err != nil {
log.Printf("Failed to record admin_audit_log (non-critical): %v", err)
}
w.Header().Set("Content-Type", "application/json")
json.NewEncoder(w).Encode(map[string]float64{"balance": balance})
}
@@ -772,7 +787,7 @@ func BuyGiftCard(w http.ResponseWriter, r *http.Request) {
var req BuyGiftCardRequest
if err := json.NewDecoder(r.Body).Decode(&req); err != nil {
http.Error(w, "Invalid request", http.StatusBadRequest)
http.Error(w, "invalid request", http.StatusBadRequest)
return
}
@@ -835,7 +850,7 @@ func BuyGiftCard(w http.ResponseWriter, r *http.Request) {
return
}
log.Printf("Failed to get card: %v", err)
http.Error(w, "Internal server error", http.StatusInternalServerError)
http.Error(w, "internal server error", http.StatusInternalServerError)
return
}
sourceID = card.SquareCardID
@@ -860,7 +875,7 @@ func BuyGiftCard(w http.ResponseWriter, r *http.Request) {
tx, err := db.DB.Begin(ctx)
if err != nil {
log.Printf("Failed to begin transaction: %v", err)
http.Error(w, "Internal server error", http.StatusInternalServerError)
http.Error(w, "internal server error", http.StatusInternalServerError)
return
}
defer tx.Rollback(ctx)
@@ -870,15 +885,14 @@ func BuyGiftCard(w http.ResponseWriter, r *http.Request) {
var cardID string
if req.RecipientType == "self" {
expiryDate := time.Now().AddDate(1, 0, 0)
err = tx.QueryRow(ctx, `
INSERT INTO gift_cards (total_funds_added, amount_remaining, created_by, redeemed_at, redeemed_by, is_inventory, expiry_date)
VALUES ($1, 0, $2, NOW(), $2, FALSE, $3)
INSERT INTO gift_cards (total_funds_added, amount_remaining, created_by, redeemed_at, redeemed_by, is_inventory)
VALUES ($1, 0, $2, NOW(), $2, FALSE)
RETURNING id
`, amountPounds, userID, expiryDate).Scan(&cardID)
`, amountPounds, userID).Scan(&cardID)
if err != nil {
log.Printf("Failed to insert gift card: %v", err)
http.Error(w, "Internal server error", http.StatusInternalServerError)
http.Error(w, "internal server error", http.StatusInternalServerError)
return
}
@@ -891,7 +905,7 @@ func BuyGiftCard(w http.ResponseWriter, r *http.Request) {
`, userID, amountPounds)
if err != nil {
log.Printf("Failed to update balance: %v", err)
http.Error(w, "Internal server error", http.StatusInternalServerError)
http.Error(w, "internal server error", http.StatusInternalServerError)
return
}
@@ -901,19 +915,18 @@ func BuyGiftCard(w http.ResponseWriter, r *http.Request) {
`, cardID, amountPounds, userID)
if err != nil {
log.Printf("Failed to record gift card transaction: %v", err)
http.Error(w, "Internal server error", http.StatusInternalServerError)
http.Error(w, "internal server error", http.StatusInternalServerError)
return
}
} else {
expiryDate := time.Now().AddDate(1, 0, 0)
err = tx.QueryRow(ctx, `
INSERT INTO gift_cards (total_funds_added, amount_remaining, created_by, is_inventory, expiry_date)
VALUES ($1, $1, $2, FALSE, $3)
INSERT INTO gift_cards (total_funds_added, amount_remaining, created_by, is_inventory)
VALUES ($1, $1, $2, FALSE)
RETURNING id
`, amountPounds, userID, expiryDate).Scan(&cardID)
`, amountPounds, userID).Scan(&cardID)
if err != nil {
log.Printf("Failed to insert gift card: %v", err)
http.Error(w, "Internal server error", http.StatusInternalServerError)
http.Error(w, "internal server error", http.StatusInternalServerError)
return
}
@@ -931,7 +944,7 @@ func BuyGiftCard(w http.ResponseWriter, r *http.Request) {
`, cardID, amountPounds, userID)
if err != nil {
log.Printf("Failed to record gift card transaction: %v", err)
http.Error(w, "Internal server error", http.StatusInternalServerError)
http.Error(w, "internal server error", http.StatusInternalServerError)
return
}
}
@@ -957,13 +970,13 @@ func BuyGiftCard(w http.ResponseWriter, r *http.Request) {
`, record.PaymentType, record.PaymentMethod, record.Status, record.Amount, record.SquarePaymentID, record.IdempotencyKey, record.Fees, record.UserSavedCardID, record.CreatedBy, record.CreatedAt, record.UpdatedAt)
if err != nil {
log.Printf("Failed to insert payment record: %v", err)
http.Error(w, "Internal server error", http.StatusInternalServerError)
http.Error(w, "internal server error", http.StatusInternalServerError)
return
}
if err := tx.Commit(ctx); err != nil {
log.Printf("Failed to commit buy transaction: %v", err)
http.Error(w, "Internal server error", http.StatusInternalServerError)
http.Error(w, "internal server error", http.StatusInternalServerError)
return
}
@@ -978,15 +991,7 @@ func BuyGiftCard(w http.ResponseWriter, r *http.Request) {
// --- Helpers ---
func normalizeCode(code string) string {
clean := ""
for _, char := range code {
if (char >= 'a' && char <= 'z') || (char >= 'A' && char <= 'Z') || (char >= '0' && char <= '9') {
clean += string(char)
}
}
return clean
}
type ExpiredBalance struct {
ID string `json:"id"`
@@ -1008,7 +1013,7 @@ func GetExpiredBalances(w http.ResponseWriter, r *http.Request) {
`)
if err != nil {
log.Printf("Failed to query expired balances: %v", err)
http.Error(w, "Internal server error", http.StatusInternalServerError)
http.Error(w, "internal server error", http.StatusInternalServerError)
return
}
defer rows.Close()
@@ -1022,7 +1027,7 @@ func GetExpiredBalances(w http.ResponseWriter, r *http.Request) {
err = rows.Scan(&b.ID, &accountID, &b.OriginalBalance, &b.ExpiredAt, &claimedAt, &claimedByAdmin, &notes)
if err != nil {
log.Printf("Failed to scan expired balance: %v", err)
http.Error(w, "Internal server error", http.StatusInternalServerError)
http.Error(w, "internal server error", http.StatusInternalServerError)
return
}
@@ -1087,7 +1092,7 @@ func ClaimExpiredBalance(w http.ResponseWriter, r *http.Request) {
return
}
log.Printf("Failed to check expired balance: %v", err)
http.Error(w, "Internal server error", http.StatusInternalServerError)
http.Error(w, "internal server error", http.StatusInternalServerError)
return
}
@@ -1103,7 +1108,7 @@ func ClaimExpiredBalance(w http.ResponseWriter, r *http.Request) {
`, adminID, req.Notes, req.BalanceID)
if err != nil {
log.Printf("Failed to claim expired balance: %v", err)
http.Error(w, "Internal server error", http.StatusInternalServerError)
http.Error(w, "internal server error", http.StatusInternalServerError)
return
}