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 <clio-agent@sisyphuslabs.ai>
This commit is contained in:
2026-06-24 23:43:23 +01:00
co-authored by Sisyphus
parent 58996c553a
commit 0ea1bb64b4
5 changed files with 520 additions and 36 deletions
+303 -12
View File
@@ -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.