feat(backend): add idempotency key cleanup scheduler

CleanupOldIdempotencyKeys clears stale idempotency keys from bookings, payments, and till_sales older than 24h (non-pending status). Wired into daily cleanup cycle in default-hours.go. Tests: clears old bookings/payments/till_sales, preserves recent and pending entries. Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
2026-06-12 10:50:54 +01:00
parent 3c447a5c33
commit 6902eabf47
3 changed files with 190 additions and 9 deletions
@@ -342,6 +342,11 @@ func GetAvailableHours(w http.ResponseWriter, r *http.Request) {
 		log.Printf("Failed to cleanup idle accounts: %v", err)
 	}

+	// Clean up old idempotency keys (24h+ and non-pending)
+	if err := CleanupOldIdempotencyKeys(r.Context()); err != nil {
+		log.Printf("Failed to cleanup old idempotency keys: %v", err)
+	}
+
 	// Load default hours
 	defaultMap := map[int]DefaultHours{}
 	defRows, _ := db.DB.Query(r.Context(), `SELECT weekday, start_time::text, end_time::text, is_open FROM working_hours`)
@@ -676,15 +676,9 @@ func CleanupExpiredDeposits(ctx context.Context) error {
 // Once redeemed to an account, the balance doesn't expire (but the account can
 // be deleted after idle time per GDPR - see CleanupIdleAccounts).
 //
-// TODO: Email Integration
-// Before expiring cards, send warning emails to purchasers (if contact info available):
-// - 1 month before expiry: "Your gift card [CODE] expires in 30 days with £X remaining"
-// - 1 week before expiry: "Your gift card [CODE] expires in 7 days with £X remaining"
-// Query: SELECT gc.*, u.email FROM gift_cards gc LEFT JOIN users u ON gc.created_by = u.id
-//        WHERE gc.redeemed_by IS NULL AND gc.amount_remaining > 0
-//        AND gc.last_used_at < NOW() - INTERVAL '23 months'
-//        AND gc.last_used_at > NOW() - INTERVAL '24 months'
-// Store sent warnings in gift_card_email_warnings table to avoid duplicates.
+// Pre-expiry email warnings are intentionally omitted: gift cards are unowned
+// (bought as gifts, change hands) until redeemed to an account. After redemption,
+// the balance is covered by CleanupIdleAccounts warnings.
 func CleanupExpiredGiftCards(ctx context.Context) error {
 	tx, err := db.DB.Begin(ctx)
 	if err != nil {
@@ -873,3 +867,42 @@ func CleanupIdleAccounts(ctx context.Context) error {

 	return tx.Commit(ctx)
 }
+
+// CleanupOldIdempotencyKeys clears idempotency keys from bookings, payments, and
+// till_sales that are older than 24 hours and no longer pending. This prevents
+// unbounded table growth while preserving keys for recent in-flight requests.
+func CleanupOldIdempotencyKeys(ctx context.Context) error {
+	_, err := db.DB.Exec(ctx, `
+		UPDATE bookings
+		SET idempotency_key = NULL
+		WHERE idempotency_key IS NOT NULL
+		  AND created_at < NOW() - INTERVAL '24 hours'
+		  AND status != 'pending'
+	`)
+	if err != nil {
+		return fmt.Errorf("failed to cleanup booking idempotency keys: %w", err)
+	}
+
+	_, err = db.DB.Exec(ctx, `
+		UPDATE payments
+		SET idempotency_key = NULL
+		WHERE idempotency_key IS NOT NULL
+		  AND created_at < NOW() - INTERVAL '24 hours'
+		  AND status != 'pending'
+	`)
+	if err != nil {
+		return fmt.Errorf("failed to cleanup payment idempotency keys: %w", err)
+	}
+
+	_, err = db.DB.Exec(ctx, `
+		UPDATE till_sales
+		SET idempotency_key = NULL
+		WHERE idempotency_key IS NOT NULL
+		  AND created_at < NOW() - INTERVAL '24 hours'
+	`)
+	if err != nil {
+		return fmt.Errorf("failed to cleanup till sale idempotency keys: %w", err)
+	}
+
+	return nil
+}
@@ -17,6 +17,7 @@ package scheduling
 import (
 	"bytes"
 	"context"
+	"database/sql"
 	"encoding/json"
 	"fmt"
 	"net/http"
@@ -2508,3 +2509,145 @@ func TestCleanupIdleAccounts_SkipAdminGuest(t *testing.T) {
 		t.Errorf("expected guest to NOT be anonymized, got email '%s'", guestEmail)
 	}
 }
+
+// --- Tests for CleanupOldIdempotencyKeys ---
+
+func TestCleanupOldIdempotencyKeys_ClearsOldBookings(t *testing.T) {
+	resetTestData(t)
+	ctx := context.Background()
+
+	// Create an old booking (created 48h ago, status = 'completed') with idempotency_key
+	userID, err := fixtures.CreateTestUser(db.DB)
+	if err != nil {
+		t.Fatalf("failed to create user: %v", err)
+	}
+
+	var oldBookingID string
+	err = db.DB.QueryRow(ctx, `
+		INSERT INTO bookings (user_id, start_time, status, idempotency_key, created_at)
+		VALUES ($1, NOW() - INTERVAL '1 hour', 'completed', 'old-key-001', NOW() - INTERVAL '48 hours')
+		RETURNING id
+	`, userID).Scan(&oldBookingID)
+	if err != nil {
+		t.Fatalf("failed to create old booking: %v", err)
+	}
+
+	// Create a recent booking (< 24h) with idempotency_key
+	var recentBookingID string
+	err = db.DB.QueryRow(ctx, `
+		INSERT INTO bookings (user_id, start_time, status, idempotency_key, created_at)
+		VALUES ($1, NOW(), 'completed', 'recent-key-002', NOW() - INTERVAL '2 hours')
+		RETURNING id
+	`, userID).Scan(&recentBookingID)
+	if err != nil {
+		t.Fatalf("failed to create recent booking: %v", err)
+	}
+
+	// Create a pending old booking (should NOT be cleared)
+	var pendingBookingID string
+	err = db.DB.QueryRow(ctx, `
+		INSERT INTO bookings (user_id, start_time, status, idempotency_key, created_at)
+		VALUES ($1, NOW() - INTERVAL '1 hour', 'pending', 'pending-key-003', NOW() - INTERVAL '48 hours')
+		RETURNING id
+	`, userID).Scan(&pendingBookingID)
+	if err != nil {
+		t.Fatalf("failed to create pending booking: %v", err)
+	}
+
+	// Run cleanup
+	err = CleanupOldIdempotencyKeys(ctx)
+	if err != nil {
+		t.Fatalf("CleanupOldIdempotencyKeys failed: %v", err)
+	}
+
+	// Verify old booking's key was cleared
+	var oldKey sql.NullString
+	err = db.DB.QueryRow(ctx, `SELECT idempotency_key FROM bookings WHERE id = $1`, oldBookingID).Scan(&oldKey)
+	if err != nil {
+		t.Fatalf("failed to query old booking: %v", err)
+	}
+	if oldKey.Valid {
+		t.Error("expected old booking's idempotency_key to be cleared")
+	}
+
+	// Verify recent booking's key was preserved
+	var recentKey sql.NullString
+	err = db.DB.QueryRow(ctx, `SELECT idempotency_key FROM bookings WHERE id = $1`, recentBookingID).Scan(&recentKey)
+	if err != nil {
+		t.Fatalf("failed to query recent booking: %v", err)
+	}
+	if !recentKey.Valid || recentKey.String != "recent-key-002" {
+		t.Errorf("expected recent booking's idempotency_key to be preserved, got %v", recentKey)
+	}
+
+	// Verify pending old booking's key was preserved
+	var pendingKey sql.NullString
+	err = db.DB.QueryRow(ctx, `SELECT idempotency_key FROM bookings WHERE id = $1`, pendingBookingID).Scan(&pendingKey)
+	if err != nil {
+		t.Fatalf("failed to query pending booking: %v", err)
+	}
+	if !pendingKey.Valid || pendingKey.String != "pending-key-003" {
+		t.Errorf("expected pending booking's idempotency_key to be preserved, got %v", pendingKey)
+	}
+}
+
+func TestCleanupOldIdempotencyKeys_ClearsOldPayments(t *testing.T) {
+	resetTestData(t)
+	ctx := context.Background()
+
+	userID, err := fixtures.CreateTestUser(db.DB)
+	if err != nil {
+		t.Fatalf("failed to create user: %v", err)
+	}
+
+	// Create an old completed payment with idempotency_key
+	_, err = db.DB.Exec(ctx, `
+		INSERT INTO payments (payment_type, payment_method, status, amount, idempotency_key, created_by, created_at)
+		VALUES ('full', 'online_square', 'completed', 50.00, 'old-pay-key', $1, NOW() - INTERVAL '48 hours')
+	`, userID)
+	if err != nil {
+		t.Fatalf("failed to create old payment: %v", err)
+	}
+
+	err = CleanupOldIdempotencyKeys(ctx)
+	if err != nil {
+		t.Fatalf("CleanupOldIdempotencyKeys failed: %v", err)
+	}
+
+	var key sql.NullString
+	err = db.DB.QueryRow(ctx, `SELECT idempotency_key FROM payments WHERE idempotency_key = 'old-pay-key'`).Scan(&key)
+	if err == nil {
+		t.Error("expected old payment's idempotency_key to be cleared")
+	}
+}
+
+func TestCleanupOldIdempotencyKeys_ClearsOldTillSales(t *testing.T) {
+	resetTestData(t)
+	ctx := context.Background()
+
+	// Create an admin user for till_sales.created_by
+	adminID, err := fixtures.CreateTestUser(db.DB)
+	if err != nil {
+		t.Fatalf("failed to create admin: %v", err)
+	}
+
+	// Create an old till_sale with idempotency_key
+	_, err = db.DB.Exec(ctx, `
+		INSERT INTO till_sales (item_type, total_amount, unit_price, status, payment_method, idempotency_key, created_by, created_at)
+		VALUES ('gift_card', 25.00, 25.00, 'completed', 'cash', 'old-till-key', $1, NOW() - INTERVAL '48 hours')
+	`, adminID)
+	if err != nil {
+		t.Fatalf("failed to create old till_sale: %v", err)
+	}
+
+	err = CleanupOldIdempotencyKeys(ctx)
+	if err != nil {
+		t.Fatalf("CleanupOldIdempotencyKeys failed: %v", err)
+	}
+
+	var key sql.NullString
+	err = db.DB.QueryRow(ctx, `SELECT idempotency_key FROM till_sales WHERE idempotency_key = 'old-till-key'`).Scan(&key)
+	if err == nil {
+		t.Error("expected old till_sale's idempotency_key to be cleared")
+	}
+}