From 920eb761fd1b09e67b192faf5ff78e9fb3900407 Mon Sep 17 00:00:00 2001 From: Stephen Adamson Date: Sun, 5 Jul 2026 11:44:52 +0100 Subject: [PATCH] feat: add excludeUserID param to scheduling time blockers Add optional excludeUserID parameter to GetTimeBlockersInRange and CheckTimeBlockerOverlap so a user's own RESERVATION entries are excluded from overlap checks. This prevents users from self-blocking on their existing reservation when checking availability or confirming a booking. Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus --- backend/handlers/scheduling/default-hours.go | 9 +- backend/handlers/scheduling/time-blockers.go | 23 ++- .../handlers/scheduling/time_blockers_test.go | 180 +++++++++++++++++- 3 files changed, 192 insertions(+), 20 deletions(-) diff --git a/backend/handlers/scheduling/default-hours.go b/backend/handlers/scheduling/default-hours.go index 878e274..120e814 100644 --- a/backend/handlers/scheduling/default-hours.go +++ b/backend/handlers/scheduling/default-hours.go @@ -494,8 +494,13 @@ func GetAvailableHours(w http.ResponseWriter, r *http.Request) { } bookingRows.Close() - // Load time blockers - blockers, err := GetTimeBlockersInRange(r.Context(), start, end) + // Load time blockers, excluding the current user's own RESERVATION entries + // so their existing reservation doesn't hide the slot from them + var excludeUserID *string + if uid, ok := r.Context().Value(mw.UserIDKey).(string); ok && uid != "" { + excludeUserID = &uid + } + blockers, err := GetTimeBlockersInRange(r.Context(), start, end, excludeUserID) if err != nil { log.Printf("Failed to load time blockers: %v", err) } diff --git a/backend/handlers/scheduling/time-blockers.go b/backend/handlers/scheduling/time-blockers.go index d6d811e..58024ba 100644 --- a/backend/handlers/scheduling/time-blockers.go +++ b/backend/handlers/scheduling/time-blockers.go @@ -218,16 +218,18 @@ func DeleteTimeBlocker(w http.ResponseWriter, r *http.Request) { // --- Helper: Get Time Blockers in Range --- // --- Helper: Get Time Blockers in Range --- -// Returns blockers for the given date range, expanded for recurring blockers +// Returns blockers for the given date range, expanded for recurring blockers. +// If excludeUserID is non-nil, RESERVATION entries owned by that user are excluded. // Used by GetAvailableHours to subtract blocked time from available slots -func GetTimeBlockersInRange(ctx context.Context, start, end time.Time) ([]TimeBlocker, error) { +func GetTimeBlockersInRange(ctx context.Context, start, end time.Time, excludeUserID *string) ([]TimeBlocker, error) { rows, err := db.Conn.Query(ctx, ` SELECT id, start_time, duration_minutes, description, cron_expression, created_at, created_by FROM time_blockers - WHERE (cron_expression IS NULL AND start_time >= $1 AND start_time <= $2) - OR (cron_expression IS NOT NULL) + WHERE ((cron_expression IS NULL AND start_time >= $1 AND start_time <= $2) + OR (cron_expression IS NOT NULL)) + AND ($3::text IS NULL OR NOT (description LIKE 'RESERVATION:%' AND created_by = $3)) ORDER BY start_time - `, start, end) + `, start, end, excludeUserID) if err != nil { return nil, err } @@ -314,14 +316,15 @@ func expandCronOccurrences(blocker TimeBlocker, rangeStart, rangeEnd time.Time) // --- Helper: Check Time Blocker Overlap --- // Returns (hasOverlap, blockerDescription, error) -// Used by booking handlers to check for blocker conflicts -func CheckTimeBlockerOverlap(ctx context.Context, startTime, endTime time.Time) (bool, string, error) { +// Used by booking handlers to check for blocker conflicts. +// If excludeUserID is non-nil, RESERVATION entries owned by that user are skipped. +func CheckTimeBlockerOverlap(ctx context.Context, startTime, endTime time.Time, excludeUserID *string) (bool, string, error) { // Get all blockers in an expanded range that could overlap // We need to look further back because recurring blockers could span multiple periods searchStart := startTime.AddDate(0, -1, 0) // Look back 1 month for recurring patterns searchEnd := endTime - blockers, err := GetTimeBlockersInRange(ctx, searchStart, searchEnd) + blockers, err := GetTimeBlockersInRange(ctx, searchStart, searchEnd, excludeUserID) if err != nil { return false, "", err } @@ -352,6 +355,10 @@ func CheckTimeBlockerOverlap(ctx context.Context, startTime, endTime time.Time) // - Anonymous (RESERVATION:anon): older than 10 minutes // - Admin walk-in (RESERVATION:admin:walkin:%): older than 15 minutes // - Admin call-in (RESERVATION:admin:callin:%): older than 15 minutes +// - Edit request (RESERVATION:edit_request:%): older than 24 hours +// - Payment in-flight (PAYMENT_IN_FLIGHT:%): TTL via duration_minutes column +// (AcquirePaymentLock sets duration_minutes = PaymentLockDuration = 5min and +// start_time = NOW(), so the condition evaluates to "cleanup after 5 minutes". func CleanupOldReservations(ctx context.Context) error { oneHourAgo := clock.Now().Add(-1 * time.Hour) tenMinutesAgo := clock.Now().Add(-10 * time.Minute) diff --git a/backend/handlers/scheduling/time_blockers_test.go b/backend/handlers/scheduling/time_blockers_test.go index 39a963a..0d5e24e 100644 --- a/backend/handlers/scheduling/time_blockers_test.go +++ b/backend/handlers/scheduling/time_blockers_test.go @@ -340,7 +340,7 @@ func TestCheckTimeBlockerOverlap(t *testing.T) { // Test case 1: Exact overlap (10:00-11:00) hasOverlap, desc, err := CheckTimeBlockerOverlap(ctx, time.Date(2026, 3, 15, 10, 0, 0, 0, time.UTC), - time.Date(2026, 3, 15, 11, 0, 0, 0, time.UTC)) + time.Date(2026, 3, 15, 11, 0, 0, 0, time.UTC), nil) if err != nil { t.Fatalf("CheckTimeBlockerOverlap failed: %v", err) } @@ -354,7 +354,7 @@ func TestCheckTimeBlockerOverlap(t *testing.T) { // Test case 2: No overlap (09:00-10:00 - ends exactly when blocker starts) hasOverlap, _, err = CheckTimeBlockerOverlap(ctx, time.Date(2026, 3, 15, 9, 0, 0, 0, time.UTC), - time.Date(2026, 3, 15, 10, 0, 0, 0, time.UTC)) + time.Date(2026, 3, 15, 10, 0, 0, 0, time.UTC), nil) if err != nil { t.Fatalf("CheckTimeBlockerOverlap failed: %v", err) } @@ -365,7 +365,7 @@ func TestCheckTimeBlockerOverlap(t *testing.T) { // Test case 3: Partial overlap (10:30-11:30 - starts during blocker) hasOverlap, _, err = CheckTimeBlockerOverlap(ctx, time.Date(2026, 3, 15, 10, 30, 0, 0, time.UTC), - time.Date(2026, 3, 15, 11, 30, 0, 0, time.UTC)) + time.Date(2026, 3, 15, 11, 30, 0, 0, time.UTC), nil) if err != nil { t.Fatalf("CheckTimeBlockerOverlap failed: %v", err) } @@ -376,7 +376,7 @@ func TestCheckTimeBlockerOverlap(t *testing.T) { // Test case 4: Partial overlap (09:30-10:30 - ends during blocker) hasOverlap, _, err = CheckTimeBlockerOverlap(ctx, time.Date(2026, 3, 15, 9, 30, 0, 0, time.UTC), - time.Date(2026, 3, 15, 10, 30, 0, 0, time.UTC)) + time.Date(2026, 3, 15, 10, 30, 0, 0, time.UTC), nil) if err != nil { t.Fatalf("CheckTimeBlockerOverlap failed: %v", err) } @@ -387,7 +387,7 @@ func TestCheckTimeBlockerOverlap(t *testing.T) { // Test case 5: No overlap (completely before blocker) hasOverlap, _, err = CheckTimeBlockerOverlap(ctx, time.Date(2026, 3, 15, 8, 0, 0, 0, time.UTC), - time.Date(2026, 3, 15, 9, 0, 0, 0, time.UTC)) + time.Date(2026, 3, 15, 9, 0, 0, 0, time.UTC), nil) if err != nil { t.Fatalf("CheckTimeBlockerOverlap failed: %v", err) } @@ -398,7 +398,7 @@ func TestCheckTimeBlockerOverlap(t *testing.T) { // Test case 6: No overlap (completely after blocker) hasOverlap, _, err = CheckTimeBlockerOverlap(ctx, time.Date(2026, 3, 15, 14, 0, 0, 0, time.UTC), - time.Date(2026, 3, 15, 15, 0, 0, 0, time.UTC)) + time.Date(2026, 3, 15, 15, 0, 0, 0, time.UTC), nil) if err != nil { t.Fatalf("CheckTimeBlockerOverlap failed: %v", err) } @@ -435,7 +435,7 @@ func TestGetTimeBlockersInRange(t *testing.T) { start := time.Date(2026, 3, 1, 0, 0, 0, 0, time.UTC) end := time.Date(2026, 3, 16, 23, 59, 59, 0, time.UTC) - blockers, err := GetTimeBlockersInRange(ctx, start, end) + blockers, err := GetTimeBlockersInRange(ctx, start, end, nil) if err != nil { t.Fatalf("GetTimeBlockersInRange failed: %v", err) } @@ -484,7 +484,7 @@ func TestGetTimeBlockersInRange_Empty(t *testing.T) { start := time.Date(2026, 4, 1, 0, 0, 0, 0, time.UTC) end := time.Date(2026, 4, 30, 23, 59, 59, 0, time.UTC) - blockers, err := GetTimeBlockersInRange(ctx, start, end) + blockers, err := GetTimeBlockersInRange(ctx, start, end, nil) if err != nil { t.Fatalf("GetTimeBlockersInRange failed: %v", err) } @@ -519,7 +519,7 @@ func TestGetTimeBlockersInRange_IncludesRecurring(t *testing.T) { start := time.Date(2026, 3, 1, 0, 0, 0, 0, time.UTC) end := time.Date(2026, 3, 31, 23, 59, 59, 0, time.UTC) - blockers, err := GetTimeBlockersInRange(ctx, start, end) + blockers, err := GetTimeBlockersInRange(ctx, start, end, nil) if err != nil { t.Fatalf("GetTimeBlockersInRange failed: %v", err) } @@ -951,7 +951,7 @@ func TestGetTimeBlockersInRange_IncludesReservations(t *testing.T) { start := time.Date(tomorrow.Year(), tomorrow.Month(), tomorrow.Day(), 0, 0, 0, 0, time.UTC) end := time.Date(tomorrow.Year(), tomorrow.Month(), tomorrow.Day(), 23, 59, 59, 0, time.UTC) - blockers, err := GetTimeBlockersInRange(ctx, start, end) + blockers, err := GetTimeBlockersInRange(ctx, start, end, nil) if err != nil { t.Fatalf("GetTimeBlockersInRange failed: %v", err) } @@ -962,6 +962,166 @@ func TestGetTimeBlockersInRange_IncludesReservations(t *testing.T) { } } +// --- Tests for GetTimeBlockersInRange (Exclude User Reservations) --- + +// TestGetTimeBlockersInRange_ExcludeOwnReservation verifies that when +// excludeUserID is set, RESERVATION entries owned by that user are excluded +// from results, while other blockers (including other users' reservations) remain. +func TestGetTimeBlockersInRange_ExcludeOwnReservation(t *testing.T) { + t.Parallel() + ctx, tx := resetTestData(t) + + userAID, err := fixtures.CreateTestUser(tx) + if err != nil { + t.Fatalf("failed to create user A: %v", err) + } + userBID, err := fixtures.CreateTestUser(tx) + if err != nil { + t.Fatalf("failed to create user B: %v", err) + } + + tomorrow := clock.Now().Add(24 * time.Hour) + dayStart := time.Date(tomorrow.Year(), tomorrow.Month(), tomorrow.Day(), 0, 0, 0, 0, time.UTC) + dayEnd := time.Date(tomorrow.Year(), tomorrow.Month(), tomorrow.Day(), 23, 59, 59, 0, time.UTC) + + // Create a regular blocker (always returned) + _, err = tx.Exec(ctx, ` + INSERT INTO time_blockers (start_time, duration_minutes, description, created_by) + VALUES ($1, 60, 'Staff meeting', NULL) + `, dayStart.Add(9*time.Hour)) + if err != nil { + t.Fatalf("failed to create regular blocker: %v", err) + } + + // Create a RESERVATION for userA + _, err = tx.Exec(ctx, ` + INSERT INTO time_blockers (start_time, duration_minutes, description, created_by) + VALUES ($1, 60, 'RESERVATION:user:' || $2 || ':123', $2) + `, dayStart.Add(10*time.Hour), userAID) + if err != nil { + t.Fatalf("failed to create userA reservation: %v", err) + } + + // Create a RESERVATION for userB + _, err = tx.Exec(ctx, ` + INSERT INTO time_blockers (start_time, duration_minutes, description, created_by) + VALUES ($1, 60, 'RESERVATION:user:' || $2 || ':456', $2) + `, dayStart.Add(11*time.Hour), userBID) + if err != nil { + t.Fatalf("failed to create userB reservation: %v", err) + } + + // Test 1: excludeUserID = userA — should see regular + userB, but NOT userA's reservation + blockers, err := GetTimeBlockersInRange(ctx, dayStart, dayEnd, &userAID) + if err != nil { + t.Fatalf("GetTimeBlockersInRange failed: %v", err) + } + if len(blockers) != 2 { + t.Errorf("expected 2 blockers (regular + userB) when excluding userA, got %d", len(blockers)) + for _, b := range blockers { + t.Logf(" blocker: %s created_by=%v", b.Description, b.CreatedBy) + } + } + + // Test 2: excludeUserID = nil — should see all 3 (regular + userA + userB) + blockers, err = GetTimeBlockersInRange(ctx, dayStart, dayEnd, nil) + if err != nil { + t.Fatalf("GetTimeBlockersInRange failed: %v", err) + } + if len(blockers) != 3 { + t.Errorf("expected 3 blockers (all) when excludeUserID=nil, got %d", len(blockers)) + } + + // Test 3: excludeUserID = userB — should see regular + userA, but NOT userB's reservation + blockers, err = GetTimeBlockersInRange(ctx, dayStart, dayEnd, &userBID) + if err != nil { + t.Fatalf("GetTimeBlockersInRange failed: %v", err) + } + if len(blockers) != 2 { + t.Errorf("expected 2 blockers (regular + userA) when excluding userB, got %d", len(blockers)) + } +} + +// --- Tests for CheckTimeBlockerOverlap (Exclude User Reservations) --- + +// TestCheckTimeBlockerOverlap_ExcludeOwnReservation verifies that RESERVATION +// entries are not considered overlapping when excludeUserID matches the owner. +func TestCheckTimeBlockerOverlap_ExcludeOwnReservation(t *testing.T) { + t.Parallel() + ctx, tx := resetTestData(t) + + userID, err := fixtures.CreateTestUser(tx) + if err != nil { + t.Fatalf("failed to create user: %v", err) + } + + tomorrow := clock.Now().Add(24 * time.Hour) + slotStart := time.Date(tomorrow.Year(), tomorrow.Month(), tomorrow.Day(), 10, 0, 0, 0, time.UTC) + slotEnd := slotStart.Add(1 * time.Hour) + + // Create a regular blocker (always detected) + _, err = tx.Exec(ctx, ` + INSERT INTO time_blockers (start_time, duration_minutes, description, created_by) + VALUES ($1, 60, 'Existing blocker', NULL) + `, slotStart) + if err != nil { + t.Fatalf("failed to create regular blocker: %v", err) + } + + // Create a RESERVATION for this user at a different time + reservationStart := slotStart.Add(2 * time.Hour) + _, err = tx.Exec(ctx, ` + INSERT INTO time_blockers (start_time, duration_minutes, description, created_by) + VALUES ($1, 60, 'RESERVATION:user:' || $2 || ':123', $2) + `, reservationStart, userID) + if err != nil { + t.Fatalf("failed to create user reservation: %v", err) + } + + // Test 1: excludeUserID = userID — the regular blocker at slotStart should + // still be detected, but the user's own RESERVATION at reservationStart + // should be ignored. + hasOverlap, desc, err := CheckTimeBlockerOverlap(ctx, slotStart, slotEnd, &userID) + if err != nil { + t.Fatalf("CheckTimeBlockerOverlap failed: %v", err) + } + if !hasOverlap { + t.Error("expected overlap with regular blocker even when excluding own reservation") + } + if desc != "Existing blocker" { + t.Errorf("expected description 'Existing blocker', got %s", desc) + } + + // Test 2: excludeUserID = nil — the regular blocker should still be detected + hasOverlap, _, err = CheckTimeBlockerOverlap(ctx, slotStart, slotEnd, nil) + if err != nil { + t.Fatalf("CheckTimeBlockerOverlap failed: %v", err) + } + if !hasOverlap { + t.Error("expected overlap with regular blocker when excludeUserID=nil") + } + + // Test 3: Overlap with own RESERVATION at reservationStart — with excludeUserID + // set, this should NOT be detected as overlap + resEnd := reservationStart.Add(1 * time.Hour) + hasOverlap, _, err = CheckTimeBlockerOverlap(ctx, reservationStart, resEnd, &userID) + if err != nil { + t.Fatalf("CheckTimeBlockerOverlap failed: %v", err) + } + if hasOverlap { + t.Error("expected NO overlap with own RESERVATION when excludeUserID matches") + } + + // Test 4: Without excludeUserID, the own RESERVATION SHOULD be detected + hasOverlap, _, err = CheckTimeBlockerOverlap(ctx, reservationStart, resEnd, nil) + if err != nil { + t.Fatalf("CheckTimeBlockerOverlap failed: %v", err) + } + if !hasOverlap { + t.Error("expected overlap with own RESERVATION when excludeUserID=nil") + } +} + // --- Tests for AnonymizeStaleGuestAccounts --- // TestAnonymizeStaleGuestAccounts_Exactly6Months verifies that a guest with