From 0ea1bb64b443ed7f7013492ebd9f01732b12f3c2 Mon Sep 17 00:00:00 2001 From: Stephen Adamson Date: Wed, 24 Jun 2026 23:43:23 +0100 Subject: [PATCH] feat(bookings): add closing_time validation and repo layer Extract closing hours check into reusable checkClosingHours helper. Add repo.go for shared DB query helpers. Update admin_reserve to use closing_time and move overlap check inside transaction with FOR UPDATE. Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus --- backend/handlers/bookings/admin_reserve.go | 66 ++-- .../handlers/bookings/admin_reserve_test.go | 315 +++++++++++++++++- backend/handlers/bookings/closing_time.go | 35 ++ .../handlers/bookings/closing_time_test.go | 94 ++++++ backend/handlers/bookings/repo.go | 46 +++ 5 files changed, 520 insertions(+), 36 deletions(-) create mode 100644 backend/handlers/bookings/closing_time.go create mode 100644 backend/handlers/bookings/closing_time_test.go create mode 100644 backend/handlers/bookings/repo.go diff --git a/backend/handlers/bookings/admin_reserve.go b/backend/handlers/bookings/admin_reserve.go index 2caeda5..3d6a8d0 100644 --- a/backend/handlers/bookings/admin_reserve.go +++ b/backend/handlers/bookings/admin_reserve.go @@ -3,6 +3,7 @@ package bookings import ( "context" "crussell/db" + "crussell/clock" "github.com/jackc/pgx/v5" "crussell/handlers/scheduling" "crussell/mw" @@ -78,7 +79,7 @@ func AdminReserveSlotHandler(w http.ResponseWriter, r *http.Request) { http.Error(w, "At least one service or custom service is required for call-in bookings", http.StatusBadRequest) return } - if req.StartTime.Before(time.Now()) { + if req.StartTime.Before(clock.Now()) { http.Error(w, "Start time cannot be in the past", http.StatusBadRequest) return } @@ -100,7 +101,7 @@ func AdminReserveSlotHandler(w http.ResponseWriter, r *http.Request) { return } svcDuration = req.DurationMinutes - allowablePast := time.Now().Add(-1 * time.Minute) + allowablePast := clock.Now().Add(-1 * time.Minute) if req.StartTime.Before(allowablePast) { http.Error(w, "Start time cannot be more than 1 minute in the past", http.StatusBadRequest) return @@ -124,29 +125,17 @@ func AdminReserveSlotHandler(w http.ResponseWriter, r *http.Request) { return } - localEnd := localStart.Add(time.Duration(svcDuration) * time.Minute) - closeTime, _ := time.Parse("15:04:05", closeStr) - if localEnd.Hour() > closeTime.Hour() || (localEnd.Hour() == closeTime.Hour() && localEnd.Minute() > closeTime.Minute()) { - http.Error(w, "Cannot book this time - services would extend beyond closing hours", http.StatusBadRequest) + localEndLondon := localStart.Add(time.Duration(svcDuration) * time.Minute).In(londonLocation) + if err := checkClosingHours(localEndLondon, closeStr); err != nil { + if errors.Is(err, ErrPastClosing) { + http.Error(w, "Cannot book this time - services would extend beyond closing hours", http.StatusBadRequest) + } else { + http.Error(w, "Invalid closing time in schedule", http.StatusInternalServerError) + } return } } - var cnt int - if err := db.Conn.QueryRow(r.Context(), ` - SELECT COUNT(*) FROM bookings WHERE status IN ('pending','confirmed','in_progress','completed') - AND start_time < $2 - AND end_time > $1 - `, req.StartTime, endTime).Scan(&cnt); err != nil { - log.Printf("Failed to check overlap: %v", err) - http.Error(w, "Internal server error", http.StatusInternalServerError) - return - } - if cnt > 0 { - http.Error(w, "Cannot book this time - slot overlaps with an existing booking", http.StatusConflict) - return - } - blockerOverlap, _, err := scheduling.CheckTimeBlockerOverlap(r.Context(), req.StartTime, endTime) if err != nil { log.Printf("Failed to check time blocker overlap: %v", err) @@ -163,6 +152,35 @@ func AdminReserveSlotHandler(w http.ResponseWriter, r *http.Request) { } defer tx.Rollback(r.Context()) + // Check booking overlap inside transaction + // pending_release is excluded — those bookings are evicted at creation time + // by AdminCreateBookingForUserHandler / CreateBookingHandler. + overlapRows, err := tx.Query(r.Context(), ` + SELECT 1 FROM bookings WHERE status IN ('pending','confirmed','in_progress','completed') + AND start_time < $2 + AND end_time > $1 + FOR UPDATE + `, req.StartTime, endTime) + if err != nil { + log.Printf("Failed to check overlap: %v", err) + http.Error(w, "Internal server error", http.StatusInternalServerError) + return + } + var cnt int + for overlapRows.Next() { + cnt++ + } + overlapRows.Close() + if err := overlapRows.Err(); err != nil { + log.Printf("Overlap row iteration error: %v", err) + http.Error(w, "Internal server error", http.StatusInternalServerError) + return + } + if cnt > 0 { + http.Error(w, "Cannot book this time - slot overlaps with an existing booking", http.StatusConflict) + return + } + _, err = tx.Exec(r.Context(), ` DELETE FROM time_blockers WHERE description LIKE 'RESERVATION:admin:%' @@ -179,7 +197,7 @@ func AdminReserveSlotHandler(w http.ResponseWriter, r *http.Request) { customerID = *req.UserID } - description := fmt.Sprintf("RESERVATION:admin:%s:%s:%d", req.ReservationType, customerID, time.Now().UnixNano()) + description := fmt.Sprintf("RESERVATION:admin:%s:%s:%d", req.ReservationType, customerID, clock.Now().UnixNano()) var reservationID string var createdAt time.Time err = tx.QueryRow(r.Context(), ` @@ -209,7 +227,7 @@ func AdminReserveSlotHandler(w http.ResponseWriter, r *http.Request) { TTLMinutes: req.TTLMinutes, } - w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusCreated) if err := json.NewEncoder(w).Encode(response); err != nil { log.Printf("Failed to encode response: %v", err) @@ -246,7 +264,7 @@ func calculateServiceDurationWithOverrides(ctx context.Context, serviceIDs []str SELECT id, duration_minutes FROM services WHERE id = ANY($1) UNION ALL SELECT id, duration_minutes FROM custom_services WHERE id = ANY($1) - `, serviceIDs, serviceIDs) + `, serviceIDs) if err != nil { return 0, err } diff --git a/backend/handlers/bookings/admin_reserve_test.go b/backend/handlers/bookings/admin_reserve_test.go index 12a8d3c..e24b774 100644 --- a/backend/handlers/bookings/admin_reserve_test.go +++ b/backend/handlers/bookings/admin_reserve_test.go @@ -19,6 +19,7 @@ import ( "testing" "time" + "crussell/clock" "crussell/testutils" "crussell/mw" "crussell/testutils/fixtures" @@ -74,7 +75,7 @@ func TestAdminReserveSlot_WalkIn_Success(t *testing.T) { } defer fixtures.DeleteUser(tx, adminID) - tomorrow := time.Now().Add(24 * time.Hour) + tomorrow := clock.Now().Add(24 * time.Hour) now := time.Date(tomorrow.Year(), tomorrow.Month(), tomorrow.Day(), 12, 0, 0, 0, tomorrow.Location()) req := AdminReserveSlotRequest{ ReservationType: "walkin", @@ -149,7 +150,7 @@ func TestAdminReserveSlot_CallIn_Success(t *testing.T) { t.Fatalf("failed to update service duration: %v", err) } - tomorrow := time.Now().Add(24 * time.Hour).Truncate(time.Second) + tomorrow := clock.Now().Add(24 * time.Hour).Truncate(time.Second) tomorrow = time.Date(tomorrow.Year(), tomorrow.Month(), tomorrow.Day(), 10, 0, 0, 0, tomorrow.Location()) req := AdminReserveSlotRequest{ @@ -209,7 +210,7 @@ func TestAdminReserveSlot_WalkIn_MissingDuration(t *testing.T) { } defer fixtures.DeleteUser(tx, adminID) - now := time.Now() + now := clock.Now() req := AdminReserveSlotRequest{ ReservationType: "walkin", StartTime: now, @@ -248,7 +249,7 @@ func TestAdminReserveSlot_CallIn_MissingServices(t *testing.T) { } defer fixtures.DeleteUser(tx, adminID) - tomorrow := time.Now().Add(24 * time.Hour).Truncate(time.Second) + tomorrow := clock.Now().Add(24 * time.Hour).Truncate(time.Second) tomorrow = time.Date(tomorrow.Year(), tomorrow.Month(), tomorrow.Day(), 10, 0, 0, 0, tomorrow.Location()) req := AdminReserveSlotRequest{ @@ -292,7 +293,7 @@ func TestAdminReserveSlot_InvalidReservationType(t *testing.T) { req := AdminReserveSlotRequest{ ReservationType: "invalid", - StartTime: time.Now(), + StartTime: clock.Now(), DurationMinutes: 30, } @@ -348,7 +349,7 @@ func TestAdminReserveSlot_SlotOverlap(t *testing.T) { t.Fatalf("failed to set deposits_required: %v", err) } - tomorrow := time.Now().Add(24 * time.Hour).Truncate(time.Second) + tomorrow := clock.Now().Add(24 * time.Hour).Truncate(time.Second) tomorrow = time.Date(tomorrow.Year(), tomorrow.Month(), tomorrow.Day(), 10, 0, 0, 0, tomorrow.Location()) bookingID, err := fixtures.CreateTestBooking(tx, userID, serviceID) @@ -401,7 +402,7 @@ func TestAdminReserveSlot_ReplacesExisting(t *testing.T) { } defer fixtures.DeleteUser(tx, adminID) - tomorrow := time.Now().Add(24 * time.Hour) + tomorrow := clock.Now().Add(24 * time.Hour) now := time.Date(tomorrow.Year(), tomorrow.Month(), tomorrow.Day(), 12, 0, 0, 0, tomorrow.Location()) req := AdminReserveSlotRequest{ @@ -490,7 +491,7 @@ func TestAdminReserveSlot_WalkIn_PastStart(t *testing.T) { } defer fixtures.DeleteUser(tx, adminID) - pastTime := time.Now().Add(-5 * time.Minute) + pastTime := clock.Now().Add(-5 * time.Minute) req := AdminReserveSlotRequest{ ReservationType: "walkin", StartTime: pastTime, @@ -533,7 +534,7 @@ func TestAdminReserveSlot_OutOfHours_CallIn_Success(t *testing.T) { // Baseline test DB has 08:00-20:00 hours. Book 19:30 + 60min = 20:30 (> 20:00 closing) // Without out_of_hours this would fail; with out_of_hours=true it should succeed. - tomorrow := time.Now().Add(24 * time.Hour) + tomorrow := clock.Now().Add(24 * time.Hour) lateBooking := time.Date(tomorrow.Year(), tomorrow.Month(), tomorrow.Day(), 19, 30, 0, 0, tomorrow.Location()) req := AdminReserveSlotRequest{ @@ -582,7 +583,7 @@ func TestAdminReserveSlot_OutOfHours_WithoutFlag_Fails(t *testing.T) { // Baseline test DB has 08:00-20:00 hours. Book 19:30 + 60min = 20:30 (> 20:00 closing) // Without out_of_hours this should be rejected. - tomorrow := time.Now().Add(24 * time.Hour) + tomorrow := clock.Now().Add(24 * time.Hour) lateBooking := time.Date(tomorrow.Year(), tomorrow.Month(), tomorrow.Day(), 19, 30, 0, 0, tomorrow.Location()) req := AdminReserveSlotRequest{ @@ -617,7 +618,7 @@ func TestAdminReserveSlot_OutOfHours_WalkIn_Success(t *testing.T) { // Baseline test DB has 08:00-20:00 hours. Book walk-in 19:30 + 60min = 20:30 (> 20:00 closing) // Without out_of_hours this would fail; with out_of_hours=true it should succeed. - tomorrow := time.Now().Add(24 * time.Hour) + tomorrow := clock.Now().Add(24 * time.Hour) lateBooking := time.Date(tomorrow.Year(), tomorrow.Month(), tomorrow.Day(), 19, 30, 0, 0, tomorrow.Location()) req := AdminReserveSlotRequest{ @@ -672,7 +673,7 @@ func TestAdminReserveSlot_OutOfHours_TimeBlockerBlocks(t *testing.T) { defer fixtures.DeleteUser(tx, adminID) // Create a time blocker at a specific future time - tomorrow := time.Now().Add(24 * time.Hour) + tomorrow := clock.Now().Add(24 * time.Hour) blockerStart := time.Date(tomorrow.Year(), tomorrow.Month(), tomorrow.Day(), 14, 0, 0, 0, tomorrow.Location()) _, err = tx.Exec(ctx, ` INSERT INTO time_blockers (start_time, duration_minutes, description, created_by) @@ -700,5 +701,295 @@ func TestAdminReserveSlot_OutOfHours_TimeBlockerBlocks(t *testing.T) { } } +// TestAdminReserveSlot_BST_ClosingBoundary verifies that during BST (UTC+1), +// the closing-time check uses London local time, not UTC. A booking ending at +// 20:01 BST (19:01 UTC) should be rejected when closing is 20:00 BST, even +// though the UTC hour (19) is before the closing hour (20). Seed data has +// working hours 08:00-20:00 for all days. +func TestAdminReserveSlot_BST_ClosingBoundary(t *testing.T) { + t.Parallel() + ctx, tx := testutils.SetupTestTx(t) + + adminID, err := fixtures.CreateTestAdminUser(tx) + if err != nil { + t.Fatalf("failed to create admin: %v", err) + } + defer fixtures.DeleteUser(tx, adminID) + + // Use a Monday in BST (2099-06-15 is a Monday in BST). Seed working hours + // for Monday are 08:00-20:00. + // Start at 18:31 UTC (= 19:31 BST), 30 min duration → ends at 19:01 UTC (= 20:01 BST). + // This should be rejected because 20:01 BST > 20:00 BST closing. + // Without .In(londonLocation), UTC hour 19 < closing 20, so this would + // incorrectly pass — the fix catches it. + bstDay := time.Date(2099, 6, 15, 18, 31, 0, 0, time.UTC) + req := AdminReserveSlotRequest{ + ReservationType: "walkin", + StartTime: bstDay, + DurationMinutes: 30, + TTLMinutes: 15, + } + + handler := http.HandlerFunc(AdminReserveSlotHandler) + w := makeAdminReserveRequest(handler, req, adminID, ctx) + + // Must be rejected (exceeds closing hours in London time). + // Without .In(londonLocation), UTC hour 19 would be < closing 20 and pass. + if w.Code != http.StatusBadRequest { + t.Errorf("expected 400 BadRequest (exceeds closing hours in BST), got %d. body: %s", w.Code, w.Body.String()) + } + + // Verify the error message mentions closing hours. + if !strings.Contains(w.Body.String(), "closing hours") && !strings.Contains(w.Body.String(), "closing") { + t.Errorf("expected error about closing hours, got: %s", w.Body.String()) + } + + // Now test a booking that ends within closing hours (19:30 BST < 20:00 BST). + // Start at 18:00 UTC (= 19:00 BST), 30 min duration → ends at 18:30 UTC (= 19:30 BST). + bstDayOK := time.Date(2099, 6, 15, 18, 0, 0, 0, time.UTC) + req2 := AdminReserveSlotRequest{ + ReservationType: "walkin", + StartTime: bstDayOK, + DurationMinutes: 30, + TTLMinutes: 15, + } + + w2 := makeAdminReserveRequest(handler, req2, adminID, ctx) + if w2.Code != http.StatusCreated { + t.Errorf("expected 201 Created (within closing hours in BST), got %d. body: %s", w2.Code, w2.Body.String()) + } +} + +// ============================================================================= +// Autumn DST (BST→GMT Transition) Tests +// ============================================================================= + +// TestAdminReserveSlot_AutumnDST_ClosingBoundary verifies that during autumn DST +// (BST→GMT transition on Oct 25, 2026), the closing-time check works correctly +// when London is in GMT (UTC+0). After the transition at 02:00 BST (→ 01:00 GMT), +// local time equals UTC. This test sets Sunday's closing to 17:00 and verifies: +// 1. A slot ending exactly at 17:00 GMT (= 17:00 UTC) is allowed (end == closing) +// 2. A slot ending 1 minute after 17:00 GMT (= 17:01 UTC) is rejected +// +// Use of londonLocation for time.Date construction ensures the test time is +// interpreted in the local timezone context of the autumn DST transition day. +func TestAdminReserveSlot_AutumnDST_ClosingBoundary(t *testing.T) { + t.Parallel() + ctx, tx := testutils.SetupTestTx(t) + + adminID, err := fixtures.CreateTestAdminUser(tx) + if err != nil { + t.Fatalf("failed to create admin: %v", err) + } + defer fixtures.DeleteUser(tx, adminID) + + // Oct 25, 2026 is a Sunday (DB weekday 6). BST ends at 02:00 BST (→ 01:00 GMT), + // so the entire working day is in GMT. Override Sunday's closing to 17:00 to test + // the closing boundary on the autumn DST transition day. + _, err = tx.Exec(ctx, ` + INSERT INTO working_hours (weekday, start_time, end_time, is_open) + VALUES (6, '08:00', '17:00', true) + ON CONFLICT (weekday) DO UPDATE SET start_time = '08:00', end_time = '17:00', is_open = true + `) + if err != nil { + t.Fatalf("failed to set Sunday working hours: %v", err) + } + + handler := http.HandlerFunc(AdminReserveSlotHandler) + + // Test 1: End exactly at 17:00 GMT (= 17:00 UTC, since GMT = UTC+0). + // Start 16:30 GMT + 30 min → ends 17:00 GMT → end == closing, allowed. + closingSlot := time.Date(2026, 10, 25, 16, 30, 0, 0, londonLocation) + req1 := AdminReserveSlotRequest{ + ReservationType: "walkin", + StartTime: closingSlot, + DurationMinutes: 30, + TTLMinutes: 15, + } + w1 := makeAdminReserveRequest(handler, req1, adminID, ctx) + if w1.Code != http.StatusCreated { + t.Errorf("ending exactly at 17:00 GMT: expected 201 (end == closing allowed), got %d. body: %s", + w1.Code, w1.Body.String()) + } + + // Parse response and verify duration + var resp1 AdminReserveSlotResponse + if err := json.Unmarshal(w1.Body.Bytes(), &resp1); err != nil { + t.Fatalf("failed to parse response: %v", err) + } + if resp1.DurationMinutes != 30 { + t.Errorf("expected duration 30, got %d", resp1.DurationMinutes) + } + + // Test 2: End 1 min after 17:00 GMT (= 17:01 UTC). + // Start 16:31 GMT + 30 min → ends 17:01 GMT → rejected (past closing). + pastClose := time.Date(2026, 10, 25, 16, 31, 0, 0, londonLocation) + req2 := AdminReserveSlotRequest{ + ReservationType: "walkin", + StartTime: pastClose, + DurationMinutes: 30, + TTLMinutes: 15, + } + w2 := makeAdminReserveRequest(handler, req2, adminID, ctx) + if w2.Code != http.StatusBadRequest { + t.Errorf("ending 1 min after 17:00 GMT: expected 400, got %d. body: %s", + w2.Code, w2.Body.String()) + } + + // Verify error message mentions closing hours + if !strings.Contains(w2.Body.String(), "closing hours") && !strings.Contains(w2.Body.String(), "closing") { + t.Errorf("expected error about closing hours, got: %s", w2.Body.String()) + } +} + +// TestAdminReserveSlot_BST_WeekdayLookup verifies that the weekday used for +// working-hours lookup uses London time, not UTC. At 23:30 UTC on a Sunday +// in BST (= 00:30 BST Monday), UTC says Sunday (DB weekday 6) but London says +// Monday (DB weekday 0). Deleting Sunday's row should cause failure WITHOUT +// the fix, but succeed WITH the fix (London-time weekday=Monday, row exists). +func TestAdminReserveSlot_BST_WeekdayLookup(t *testing.T) { + t.Parallel() + ctx, tx := testutils.SetupTestTx(t) + + adminID, err := fixtures.CreateTestAdminUser(tx) + if err != nil { + t.Fatalf("failed to create admin: %v", err) + } + defer fixtures.DeleteUser(tx, adminID) + + // Delete Sunday's (DB weekday 6) working hours row. + _, err = tx.Exec(ctx, "DELETE FROM working_hours WHERE weekday = 6") + if err != nil { + t.Fatalf("failed to delete Sunday hours: %v", err) + } + + // Also seed Monday (weekday 0) explicitly so the test doesn't depend on fixture defaults. + _, err = tx.Exec(ctx, ` + INSERT INTO working_hours (weekday, start_time, end_time, is_open) + VALUES (0, '09:00', '17:00', true) + ON CONFLICT (weekday) DO UPDATE SET start_time = '09:00', end_time = '17:00', is_open = true + `) + if err != nil { + t.Fatalf("failed to seed Monday hours: %v", err) + } + + // Book at 23:30 UTC on a Sunday in BST (2099-06-14 is Sunday, 2099-06-15 is Monday). + // 23:30 UTC Sunday = 00:30 BST Monday. London time = Monday (DB weekday 0, exists). + // UTC time = Sunday (DB weekday 6, deleted). + sunday2330UTC := time.Date(2099, 6, 14, 23, 30, 0, 0, time.UTC) + req := AdminReserveSlotRequest{ + ReservationType: "walkin", + StartTime: sunday2330UTC, + DurationMinutes: 30, + TTLMinutes: 15, + } + + handler := http.HandlerFunc(AdminReserveSlotHandler) + w := makeAdminReserveRequest(handler, req, adminID, ctx) + + if w.Code != http.StatusCreated { + t.Errorf("expected 201 Created (London weekday=Monday, row exists), got %d. body: %s — UTC weekday=Sunday (deleted), London weekday=Monday (exists)", w.Code, w.Body.String()) + } +} + +// TestAdminReserveSlot_ClosingComparison_EdgeCases verifies that the closing-time +// string comparison correctly handles edge cases at the boundary (BUG 5 fix). +// Tests: ending exactly at closing, 1 min before, and 1 min after. +func TestAdminReserveSlot_ClosingComparison_EdgeCases(t *testing.T) { + t.Parallel() + ctx, tx := testutils.SetupTestTx(t) + + adminID, err := fixtures.CreateTestAdminUser(tx) + if err != nil { + t.Fatalf("failed to create admin: %v", err) + } + defer fixtures.DeleteUser(tx, adminID) + + // Seed data has all days open 08:00-20:00. Test various closing edges. + // Use a BST Monday (2099-06-15, BST period so UTC != London). + // 20:00 BST = 19:00 UTC. A service ending at 19:59 UTC = 20:59 BST (after closing). + // Actually seed data is 08:00-20:00 BST, so 20:00 BST closing = 19:00 UTC. + + // Test 1: End exactly at closing (20:00 BST = 19:00 UTC) — should be allowed + // (end == closing is not "beyond" closing — the check is strict greater-than). + endAtClose := time.Date(2099, 6, 15, 18, 30, 0, 0, time.UTC) // start 18:30 UTC + req1 := AdminReserveSlotRequest{ + ReservationType: "walkin", + StartTime: endAtClose, + DurationMinutes: 30, + TTLMinutes: 15, + } + handler := http.HandlerFunc(AdminReserveSlotHandler) + w1 := makeAdminReserveRequest(handler, req1, adminID, ctx) + if w1.Code != http.StatusCreated { + t.Errorf("closing exactly at 20:00 BST: expected 201 (end == closing is allowed), got %d", w1.Code) + } + + // Test 2: End 1 minute after closing (20:01 BST = 19:01 UTC) — should reject + oneMinAfter := time.Date(2099, 6, 15, 18, 31, 0, 0, time.UTC) + req2 := AdminReserveSlotRequest{ + ReservationType: "walkin", + StartTime: oneMinAfter, + DurationMinutes: 30, + TTLMinutes: 15, + } + w2 := makeAdminReserveRequest(handler, req2, adminID, ctx) + if w2.Code != http.StatusBadRequest { + t.Errorf("closing 1 min after 20:00 BST: expected 400, got %d", w2.Code) + } +} + +// TestAdminReserveSlot_PendingRelease_DoesNotBlock verifies that a +// pending_release booking does NOT block the admin reserve endpoint. +// Like the user-facing reserve, admin reserves are pre-checks — eviction +// happens when the actual booking is created. +func TestAdminReserveSlot_PendingRelease_DoesNotBlock(t *testing.T) { + t.Parallel() + ctx, tx := testutils.SetupTestTx(t) + + adminID, err := fixtures.CreateTestAdminUser(tx) + if err != nil { + t.Fatalf("failed to create admin: %v", err) + } + + userID, err := fixtures.CreateTestUser(tx) + if err != nil { + t.Fatalf("failed to create test user: %v", err) + } + + serviceID, err := fixtures.CreateTestService(tx) + if err != nil { + t.Fatalf("failed to create test service: %v", err) + } + + future := clock.Now().Add(7 * 24 * time.Hour).Truncate(24 * time.Hour).Add(10 * time.Hour) + + // Create a pending_release booking at this time slot + _, err = tx.Exec(ctx, ` + INSERT INTO bookings (user_id, start_time, status, deposit_required) + VALUES ($1, $2, 'pending_release', false) + `, userID, future) + if err != nil { + t.Fatalf("failed to create pending_release booking: %v", err) + } + + // Admin reserves the same slot — should succeed + req := AdminReserveSlotRequest{ + ReservationType: "callin", + StartTime: future, + ServiceIDs: []string{serviceID}, + TTLMinutes: 15, + } + handler := http.HandlerFunc(AdminReserveSlotHandler) + w := makeAdminReserveRequest(handler, req, adminID, ctx) + + if w.Code == http.StatusConflict { + t.Errorf("pending_release should NOT block admin reserve – it is evictable, got 409") + } + if w.Code != http.StatusCreated { + t.Errorf("expected 201 for admin reserve with only pending_release overlap, got %d. body: %s", w.Code, w.Body.String()) + } +} + // TestAdminReserveSlot_SlotOverlap tests that a reservation fails when the // requested time slot overlaps with an existing booking. diff --git a/backend/handlers/bookings/closing_time.go b/backend/handlers/bookings/closing_time.go new file mode 100644 index 0000000..193fa70 --- /dev/null +++ b/backend/handlers/bookings/closing_time.go @@ -0,0 +1,35 @@ +package bookings + +import ( + "errors" + "strconv" + "strings" + "time" +) + +// ErrPastClosing is returned by checkClosingHours when the booking's end time +// exceeds the working hours closing time for that day. +var ErrPastClosing = errors.New("booking extends beyond closing hours") + +// checkClosingHours verifies that a booking end time (in Europe/London wall-clock +// time) does not exceed the closing time stored as "HH:MM" in the working_hours +// table. Returns ErrPastClosing if the booking runs past close, or a generic +// error if closeStr cannot be parsed. +func checkClosingHours(localEnd time.Time, closeStr string) error { + parts := strings.Split(closeStr, ":") + if len(parts) < 2 { + return errors.New("invalid closing time format") + } + closeHour, err := strconv.Atoi(parts[0]) + if err != nil { + return errors.New("invalid closing time format") + } + closeMin, err := strconv.Atoi(parts[1]) + if err != nil { + return errors.New("invalid closing time format") + } + if localEnd.Hour() > closeHour || (localEnd.Hour() == closeHour && localEnd.Minute() > closeMin) { + return ErrPastClosing + } + return nil +} diff --git a/backend/handlers/bookings/closing_time_test.go b/backend/handlers/bookings/closing_time_test.go new file mode 100644 index 0000000..6421f5b --- /dev/null +++ b/backend/handlers/bookings/closing_time_test.go @@ -0,0 +1,94 @@ +//go:build test && dev +// +build test,dev + +package bookings + +import ( + "testing" + "time" +) + +func TestCheckClosingHours_WithinHours(t *testing.T) { + // Closing at 17:00, booking ends at 16:30 — should pass. + london, err := time.LoadLocation("Europe/London") + if err != nil { + t.Fatalf("failed to load location: %v", err) + } + localEnd := time.Date(2026, 6, 24, 16, 30, 0, 0, london) + if err := checkClosingHours(localEnd, "17:00"); err != nil { + t.Errorf("expected no error for 16:30 end vs 17:00 close, got: %v", err) + } +} + +func TestCheckClosingHours_AtClosing(t *testing.T) { + // Closing at 17:00, booking ends exactly at 17:00 — should pass. + london, err := time.LoadLocation("Europe/London") + if err != nil { + t.Fatalf("failed to load location: %v", err) + } + localEnd := time.Date(2026, 6, 24, 17, 0, 0, 0, london) + if err := checkClosingHours(localEnd, "17:00"); err != nil { + t.Errorf("expected no error for 17:00 end vs 17:00 close, got: %v", err) + } +} + +func TestCheckClosingHours_PastClosing(t *testing.T) { + // Closing at 17:00, booking ends at 17:01 — should return ErrPastClosing. + london, err := time.LoadLocation("Europe/London") + if err != nil { + t.Fatalf("failed to load location: %v", err) + } + localEnd := time.Date(2026, 6, 24, 17, 1, 0, 0, london) + if err := checkClosingHours(localEnd, "17:00"); !IsPastClosing(err) { + t.Errorf("expected ErrPastClosing for 17:01 end vs 17:00 close, got: %v", err) + } +} + +func TestCheckClosingHours_WellPastClosing(t *testing.T) { + // Closing at 17:00, booking ends at 18:00 — should return ErrPastClosing. + london, err := time.LoadLocation("Europe/London") + if err != nil { + t.Fatalf("failed to load location: %v", err) + } + localEnd := time.Date(2026, 6, 24, 18, 0, 0, 0, london) + if err := checkClosingHours(localEnd, "17:00"); !IsPastClosing(err) { + t.Errorf("expected ErrPastClosing for 18:00 end vs 17:00 close, got: %v", err) + } +} + +func TestCheckClosingHours_InvalidFormat(t *testing.T) { + // Malformed closing time string — should return a generic error, not ErrPastClosing. + london, err := time.LoadLocation("Europe/London") + if err != nil { + t.Fatalf("failed to load location: %v", err) + } + localEnd := time.Date(2026, 6, 24, 14, 0, 0, 0, london) + + tests := []struct { + name string + closeStr string + }{ + {"empty string", ""}, + {"no colon", "1700"}, + {"non-numeric hour", "ab:00"}, + {"non-numeric minute", "17:ab"}, + {"only hour", "17"}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + err := checkClosingHours(localEnd, tc.closeStr) + if err == nil { + t.Error("expected error for invalid format, got nil") + } + if IsPastClosing(err) { + t.Errorf("expected non-past-closing error for invalid format %q, got ErrPastClosing", tc.closeStr) + } + }) + } +} + +// IsPastClosing reports whether err indicates the booking extends beyond closing hours. +// Exported helper for tests. +func IsPastClosing(err error) bool { + return err == ErrPastClosing +} diff --git a/backend/handlers/bookings/repo.go b/backend/handlers/bookings/repo.go new file mode 100644 index 0000000..ff95183 --- /dev/null +++ b/backend/handlers/bookings/repo.go @@ -0,0 +1,46 @@ +package bookings + +import ( + "context" + "time" + + "crussell/db" +) + +// GetBookingStatus returns the status of a booking by ID. +// Uses db.Conn directly — not for use inside transactions. +func GetBookingStatus(ctx context.Context, bookingID string) (string, error) { + var status string + err := db.Conn.QueryRow(ctx, `SELECT status FROM bookings WHERE id = $1`, bookingID).Scan(&status) + if err != nil { + return "", err + } + return status, nil +} + +// GetBookingStartTime returns the start_time of a booking by ID. +// Uses db.Conn directly — not for use inside transactions. +func GetBookingStartTime(ctx context.Context, bookingID string) (time.Time, error) { + var startTime time.Time + err := db.Conn.QueryRow(ctx, `SELECT start_time FROM bookings WHERE id = $1`, bookingID).Scan(&startTime) + if err != nil { + return time.Time{}, err + } + return startTime, nil +} + +// BookingExists checks if a booking with the given ID exists. +// Uses db.Conn directly — not for use inside transactions. +func BookingExists(ctx context.Context, bookingID string) (bool, error) { + var exists bool + err := db.Conn.QueryRow(ctx, `SELECT EXISTS(SELECT 1 FROM bookings WHERE id = $1)`, bookingID).Scan(&exists) + return exists, err +} + +// CountUserBookingsInStatus counts the number of bookings for a user with a specific status. +// Uses db.Conn directly — not for use inside transactions. +func CountUserBookingsInStatus(ctx context.Context, userID, status string) (int, error) { + var count int + err := db.Conn.QueryRow(ctx, `SELECT COUNT(*) FROM bookings WHERE user_id = $1 AND status = $2`, userID, status).Scan(&count) + return count, err +}