From 7c3f922725c98e593bff52a012ff0bd6f5093028 Mon Sep 17 00:00:00 2001 From: Stephen Adamson Date: Wed, 4 Mar 2026 20:11:51 +0000 Subject: [PATCH] Fix nbtb, add tests --- backend/handlers/admin/bookings_test.go | 196 +++++++++++++++++- backend/handlers/bookings/bookings_test.go | 139 +++++++++++++ backend/handlers/bookings/manage.go | 9 + backend/handlers/scheduling/default-hours.go | 15 +- .../handlers/scheduling/scheduling_test.go | 147 +++++++++++++ backend/testutils/fixtures/fixtures.go | 26 +++ backend/testutils/testdb/testdb.go | 2 + 7 files changed, 522 insertions(+), 12 deletions(-) diff --git a/backend/handlers/admin/bookings_test.go b/backend/handlers/admin/bookings_test.go index f693cc1..05db6ff 100644 --- a/backend/handlers/admin/bookings_test.go +++ b/backend/handlers/admin/bookings_test.go @@ -20,7 +20,9 @@ package admin // Authentication: All endpoints require admin role (403 for non-admins). import ( + "bytes" "context" + "encoding/json" "fmt" "net/http" "net/http/httptest" @@ -200,13 +202,23 @@ func TestAdminBookings_Create(t *testing.T) { t.Errorf("expected status 201, got %d. body: %s", w.Code, w.Body.String()) } - var booking bookings.Booking - if err := parseResponseBody(w, &booking); err != nil { + // Parse response with new format {"booking": {...}, "warnings": [...]} + var response map[string]interface{} + if err := parseResponseBody(w, &response); err != nil { t.Fatalf("failed to parse response: %v", err) } - if booking.Status != "confirmed" { - t.Errorf("expected status 'confirmed', got %s", booking.Status) + bookingData, ok := response["booking"].(map[string]interface{}) + if !ok { + t.Fatal("expected booking in response") + } + + status, ok := bookingData["status"].(string) + if !ok { + t.Fatal("expected status in booking") + } + if status != "confirmed" { + t.Errorf("expected status 'confirmed', got %s", status) } var count int @@ -1638,3 +1650,179 @@ func TestAdminBookings_List_DepositFields(t *testing.T) { t.Error("expected DepositAmount to be positive in list") } } + +// ============================================================================= +// Time Blocker Tests for Admin Bookings +// ============================================================================= + +// TestAdminBookings_Create_OverlappingBlocker_WithWarning verifies that admins can +// create bookings that overlap with time blockers, but receive a warning. +// The booking is still created (201 Created), unlike regular users who get 409. +func TestAdminBookings_Create_OverlappingBlocker_WithWarning(t *testing.T) { + cleanup := setupTestDB(t) + defer cleanup() + + adminID, err := fixtures.CreateTestAdminUser(db.DB) + if err != nil { + t.Fatalf("failed to create admin user: %v", err) + } + defer fixtures.DeleteUser(db.DB, adminID) + + userID, err := fixtures.CreateTestUser(db.DB) + if err != nil { + t.Fatalf("failed to create test user: %v", err) + } + defer fixtures.DeleteUser(db.DB, userID) + + serviceID, err := fixtures.CreateTestService(db.DB) + if err != nil { + t.Fatalf("failed to create test service: %v", err) + } + defer fixtures.DeleteService(db.DB, serviceID) + + // Create a time blocker for a specific time + ukLocation, _ := time.LoadLocation("Europe/London") + blockerTime := time.Date(2099, 12, 31, 10, 0, 0, 0, ukLocation) + _, err = db.DB.Exec(context.Background(), ` + INSERT INTO time_blockers (start_time, duration_minutes, description, created_by) + VALUES ($1, 60, 'Staff meeting', $2) + `, blockerTime, adminID) + if err != nil { + t.Fatalf("failed to create time blocker: %v", err) + } + + // Admin creates booking overlapping the blocker + req := bookings.AdminCreateBookingForUserRequest{ + UserID: userID, + StartTime: blockerTime, + ServiceIDs: []string{serviceID}, + } + + handler := http.HandlerFunc(bookings.AdminCreateBookingForUserHandler) + w := makeAdminRequest(handler, "POST", "/api/admin/bookings", req) + + // Admin should get 201 Created (not 409 Conflict) + if w.Code != http.StatusCreated { + t.Errorf("expected status 201, got %d. body: %s", w.Code, w.Body.String()) + } + + // Parse response to check for warnings + var response map[string]interface{} + if err := parseResponseBody(w, &response); err != nil { + t.Fatalf("failed to parse response: %v", err) + } + + // Check for warnings array + warnings, ok := response["warnings"].([]interface{}) + if !ok || len(warnings) == 0 { + t.Error("expected warnings array with at least one warning") + } else { + // Verify warning mentions the blocker + warningStr, ok := warnings[0].(string) + if !ok { + t.Errorf("expected warning to be a string, got: %v", warnings[0]) + } else if !bytes.Contains([]byte(warningStr), []byte("blocker")) { + t.Errorf("expected warning to mention 'blocker', got: %s", warningStr) + } + } + + // Verify booking was created + bookingData, ok := response["booking"].(map[string]interface{}) + if !ok { + t.Fatal("expected booking in response") + } + if bookingData["id"] == nil { + t.Error("expected booking ID to be set") + } +} + +// TestAdminBookings_Edit_OverlappingBlocker_WithWarning verifies that admins can +// edit bookings to overlap with time blockers, but receive a warning. +// The booking is still updated (200 OK with warnings), unlike regular users who get 409. +func TestAdminBookings_Edit_OverlappingBlocker_WithWarning(t *testing.T) { + cleanup := setupTestDB(t) + defer cleanup() + + adminID, err := fixtures.CreateTestAdminUser(db.DB) + if err != nil { + t.Fatalf("failed to create admin user: %v", err) + } + defer fixtures.DeleteUser(db.DB, adminID) + + userID, err := fixtures.CreateTestUser(db.DB) + if err != nil { + t.Fatalf("failed to create test user: %v", err) + } + defer fixtures.DeleteUser(db.DB, userID) + + serviceID, err := fixtures.CreateTestService(db.DB) + if err != nil { + t.Fatalf("failed to create test service: %v", err) + } + defer fixtures.DeleteService(db.DB, serviceID) + + // Create a booking first + bookingID, err := fixtures.CreateTestBooking(db.DB, userID, serviceID) + if err != nil { + t.Fatalf("failed to create test booking: %v", err) + } + defer fixtures.DeleteBooking(db.DB, bookingID) + + // Create a time blocker for a specific time + ukLocation, _ := time.LoadLocation("Europe/London") + blockerTime := time.Date(2099, 12, 31, 14, 0, 0, 0, ukLocation) + _, err = db.DB.Exec(context.Background(), ` + INSERT INTO time_blockers (start_time, duration_minutes, description, created_by) + VALUES ($1, 60, 'Staff meeting', NULL) + `, blockerTime) + if err != nil { + t.Fatalf("failed to create time blocker: %v", err) + } + + // Admin edits booking to overlap the blocker + req := bookings.EditBookingRequest{ + StartTime: blockerTime, + } + + // Set up chi router for URL param + r := chi.NewRouter() + r.Put("/api/admin/bookings/{id}", bookings.AdminEditBookingHandler) + + // Create request with chi context and admin auth + bodyBytes, _ := json.Marshal(req) + reqHTTP := httptest.NewRequest("PUT", "/api/admin/bookings/"+bookingID, bytes.NewReader(bodyBytes)) + reqHTTP.Header.Set("Content-Type", "application/json") + + // Add admin context + ctx := context.WithValue(reqHTTP.Context(), mw.UserIDKey, adminID) + ctx = context.WithValue(ctx, mw.UserRoleKey, "admin") + reqHTTP = reqHTTP.WithContext(ctx) + + w := httptest.NewRecorder() + r.ServeHTTP(w, reqHTTP) + + // Admin should get 200 OK (not 409 Conflict) + if w.Code != http.StatusOK { + t.Errorf("expected status 200, got %d. body: %s", w.Code, w.Body.String()) + } + + // Parse response + var response map[string]interface{} + if err := json.Unmarshal(w.Body.Bytes(), &response); err != nil { + t.Fatalf("failed to parse response: %v", err) + } + + // Check for warnings + warnings, ok := response["warnings"].([]interface{}) + if !ok || len(warnings) == 0 { + t.Error("expected warnings array with at least one warning") + } else { + // Verify warning mentions the blocker + warningStr, ok := warnings[0].(string) + if !ok { + t.Errorf("expected warning to be a string, got: %v", warnings[0]) + } else if !bytes.Contains([]byte(warningStr), []byte("blocker")) { + t.Errorf("expected warning to mention 'blocker', got: %s", warningStr) + } + } +} diff --git a/backend/handlers/bookings/bookings_test.go b/backend/handlers/bookings/bookings_test.go index a22267b..859d663 100644 --- a/backend/handlers/bookings/bookings_test.go +++ b/backend/handlers/bookings/bookings_test.go @@ -3012,4 +3012,143 @@ func TestBookings_Edit_OpenDay_UserAllowed(t *testing.T) { if w.Code != http.StatusOK { t.Errorf("expected status 200 for open day edit, got %d. body: %s", w.Code, w.Body.String()) } +} + +// ============================================================================= +// Time Blocker Tests for User Bookings +// ============================================================================= + +// TestBookings_Create_OverlappingBlocker_UserBlocked verifies that a regular user +// CANNOT create a booking that overlaps with a time blocker. They receive 409 Conflict. +func TestBookings_Create_OverlappingBlocker_UserBlocked(t *testing.T) { + cleanup := setupTestDB(t) + defer cleanup() + + seedDefaultWorkingHours(t) + + userID, err := fixtures.CreateTestUser(db.DB) + if err != nil { + t.Fatalf("failed to create test user: %v", err) + } + defer fixtures.DeleteUser(db.DB, userID) + + // Set deposits_required=0 to avoid 48h advance booking requirement + _, err = db.DB.Exec(context.Background(), "UPDATE users SET deposits_required = 0 WHERE id = $1", userID) + if err != nil { + t.Fatalf("failed to set deposits_required: %v", err) + } + + serviceID, err := fixtures.CreateTestService(db.DB) + if err != nil { + t.Fatalf("failed to create test service: %v", err) + } + defer fixtures.DeleteService(db.DB, serviceID) + + // Create a time blocker for a specific time + ukLocation, _ := time.LoadLocation("Europe/London") + blockerTime := time.Date(2099, 12, 31, 10, 0, 0, 0, ukLocation) + _, err = db.DB.Exec(context.Background(), ` + INSERT INTO time_blockers (start_time, duration_minutes, description, created_by) + VALUES ($1, 60, 'Staff meeting', NULL) + `, blockerTime) + if err != nil { + t.Fatalf("failed to create time blocker: %v", err) + } + + token := jwt.GenerateUserToken(userID) + + // User tries to create booking overlapping the blocker + req := CreateBookingRequest{ + StartTime: blockerTime, + ServiceIDs: []string{serviceID}, + } + + handler := http.HandlerFunc(CreateBookingHandler) + w := makeRequest(handler, "POST", "/api/bookings", req, token) + + // User should get 409 Conflict (not 201 Created) + if w.Code != http.StatusConflict { + t.Errorf("expected status 409, got %d. body: %s", w.Code, w.Body.String()) + } + + // Verify error message mentions the blocker + if !bytes.Contains(w.Body.Bytes(), []byte("blocked")) { + t.Errorf("expected error message to mention 'blocked', got: %s", w.Body.String()) + } + + // Verify NO booking was created + var count int + err = db.DB.QueryRow(context.Background(), + "SELECT COUNT(*) FROM bookings WHERE user_id = $1", userID).Scan(&count) + if err != nil { + t.Fatalf("failed to query bookings: %v", err) + } + if count != 0 { + t.Errorf("expected 0 bookings (user should be blocked), got %d", count) + } +} + +// TestBookings_Edit_OverlappingBlocker_UserBlocked verifies that a regular user +// CANNOT edit a booking to a time that overlaps with a time blocker. +func TestBookings_Edit_OverlappingBlocker_UserBlocked(t *testing.T) { + cleanup := setupTestDB(t) + defer cleanup() + + seedDefaultWorkingHours(t) + + userID, err := fixtures.CreateTestUser(db.DB) + if err != nil { + t.Fatalf("failed to create test user: %v", err) + } + defer fixtures.DeleteUser(db.DB, userID) + + // Set deposits_required=0 to avoid 48h advance booking requirement + _, err = db.DB.Exec(context.Background(), "UPDATE users SET deposits_required = 0 WHERE id = $1", userID) + if err != nil { + t.Fatalf("failed to set deposits_required: %v", err) + } + + serviceID, err := fixtures.CreateTestService(db.DB) + if err != nil { + t.Fatalf("failed to create test service: %v", err) + } + defer fixtures.DeleteService(db.DB, serviceID) + + // Create a booking first + bookingID, err := fixtures.CreateTestBooking(db.DB, userID, serviceID) + if err != nil { + t.Fatalf("failed to create test booking: %v", err) + } + defer fixtures.DeleteBooking(db.DB, bookingID) + + // Create a time blocker for a specific time + ukLocation, _ := time.LoadLocation("Europe/London") + blockerTime := time.Date(2099, 12, 31, 10, 0, 0, 0, ukLocation) + _, err = db.DB.Exec(context.Background(), ` + INSERT INTO time_blockers (start_time, duration_minutes, description, created_by) + VALUES ($1, 60, 'Staff meeting', NULL) + `, blockerTime) + if err != nil { + t.Fatalf("failed to create time blocker: %v", err) + } + + token := jwt.GenerateUserToken(userID) + + // User tries to edit booking to overlap the blocker + req := EditBookingRequest{ + StartTime: blockerTime, + } + + handler := http.HandlerFunc(EditBookingHandler) + w := makeRequest(handler, "PUT", "/api/bookings/"+bookingID, req, token) + + // User should get 409 Conflict + if w.Code != http.StatusConflict { + t.Errorf("expected status 409, got %d. body: %s", w.Code, w.Body.String()) + } + + // Verify error message mentions the blocker + if !bytes.Contains(w.Body.Bytes(), []byte("blocked")) { + t.Errorf("expected error message to mention 'blocked', got: %s", w.Body.String()) + } } \ No newline at end of file diff --git a/backend/handlers/bookings/manage.go b/backend/handlers/bookings/manage.go index 5e4d607..64535da 100644 --- a/backend/handlers/bookings/manage.go +++ b/backend/handlers/bookings/manage.go @@ -355,6 +355,15 @@ func AdminEditBookingHandler(w http.ResponseWriter, r *http.Request) { warnings = append(warnings, "Warning: This booking is outside standard working hours") } + // Check for time blocker overlap - admin can proceed with warning + blockerOverlap, blockerDesc, err := scheduling.CheckTimeBlockerOverlap(r.Context(), req.StartTime, newEndTime) + if err != nil { + log.Printf("Failed to check time blocker overlap: %v", err) + } + if blockerOverlap { + warnings = append(warnings, fmt.Sprintf("Warning: This booking overlaps with a time blocker: %s", blockerDesc)) + } + // Perform the update res, err := db.DB.Exec(r.Context(), ` UPDATE bookings diff --git a/backend/handlers/scheduling/default-hours.go b/backend/handlers/scheduling/default-hours.go index 235146a..21cc5bc 100644 --- a/backend/handlers/scheduling/default-hours.go +++ b/backend/handlers/scheduling/default-hours.go @@ -245,12 +245,12 @@ type TimeSlot struct { } type DayAvailableHours struct { - Date string `json:"date"` - Weekday int `json:"weekday"` - IsOpen bool `json:"isOpen"` - Slots []TimeSlot `json:"slots"` - Source string `json:"source"` -Blockers []TimeSlot `json:"blockers,omitempty"` + Date string `json:"date"` + Weekday int `json:"weekday"` + IsOpen bool `json:"isOpen"` + Slots []TimeSlot `json:"slots"` + Source string `json:"source"` + Blockers []TimeSlot `json:"blockers,omitempty"` } // --- GetAvailableHours (with bookings, UK-local) --- @@ -373,8 +373,7 @@ func GetAvailableHours(w http.ResponseWriter, r *http.Request) { StartTime: blocker.StartTime.Format("15:04"), - EndTime: endTime.Format("15:04"), - + EndTime: endTime.Format("15:04"), }) } diff --git a/backend/handlers/scheduling/scheduling_test.go b/backend/handlers/scheduling/scheduling_test.go index b55e72f..f68a692 100644 --- a/backend/handlers/scheduling/scheduling_test.go +++ b/backend/handlers/scheduling/scheduling_test.go @@ -24,6 +24,7 @@ import ( "net/http/httptest" "strconv" "testing" + "time" "crussell/db" "crussell/mw" @@ -557,3 +558,149 @@ func TestScheduling_UpdateExceptionalApplications_NonAdmin(t *testing.T) { t.Errorf("expected status 403, got %d. body: %s", w.Code, w.Body.String()) } } + +// ============================================================================= +// Time Blocker Tests for GetAvailableHours +// ============================================================================= + +// TestScheduling_GetAvailableHours_WithBlocker_NonAdmin verifies that non-admin +// users do NOT see blocked time slots in their available hours. +func TestScheduling_GetAvailableHours_WithBlocker_NonAdmin(t *testing.T) { + cleanup := setupTestDB(t) + defer cleanup() + + // Create a time blocker for 2026-03-16 10:00-11:00 (Monday - an open day) + ukLocation, _ := time.LoadLocation("Europe/London") + blockerTime := time.Date(2026, 3, 16, 10, 0, 0, 0, ukLocation) + _, err := db.DB.Exec(context.Background(), ` + INSERT INTO time_blockers (start_time, duration_minutes, description, created_by) + VALUES ($1, 60, 'Staff Meeting', NULL) + `, blockerTime) + if err != nil { + t.Fatalf("failed to create time blocker: %v", err) + } + + // Make request as non-admin user + handler := http.HandlerFunc(GetAvailableHours) + req := httptest.NewRequest("GET", "/api/scheduling/available-hours?start=2026-03-16&end=2026-03-16", nil) + + // Set non-admin context + ctx := context.WithValue(req.Context(), mw.UserIDKey, "user001") + ctx = context.WithValue(ctx, mw.UserRoleKey, "verified_email") + req = req.WithContext(ctx) + + w := httptest.NewRecorder() + handler.ServeHTTP(w, req) + + if w.Code != http.StatusOK { + t.Errorf("expected status 200, got %d. body: %s", w.Code, w.Body.String()) + } + + var response []DayAvailableHours + if err := json.Unmarshal(w.Body.Bytes(), &response); err != nil { + t.Fatalf("failed to unmarshal response: %v", err) + } + + if len(response) == 0 { + t.Fatal("expected at least one day in response") + } + + // Find the day with the blocker (2026-03-16) + var targetDay *DayAvailableHours + for i := range response { + if response[i].Date == "2026-03-16" { + targetDay = &response[i] + break + } + } + + if targetDay == nil { + t.Fatal("expected day 2026-03-16 in response") + } + + // Verify blocker is NOT visible in blockers field for non-admin + if len(targetDay.Blockers) > 0 { + t.Error("expected blockers field to be empty for non-admin users") + } + + // Verify 10:00-11:00 slot is NOT available (subtracted due to blocker) + for _, slot := range targetDay.Slots { + if slot.StartTime == "10:00" { + t.Error("expected 10:00 slot to be blocked and not available") + } + } +} + + +// TestScheduling_GetAvailableHours_WithBlocker_Admin verifies that admin users +// CAN see blocked time slots in the blockers field. +func TestScheduling_GetAvailableHours_WithBlocker_Admin(t *testing.T) { + cleanup := setupTestDB(t) + defer cleanup() + + // Create a time blocker for 2026-03-16 10:00-11:00 (Monday - open day) + ukLocation, _ := time.LoadLocation("Europe/London") + blockerTime := time.Date(2026, 3, 16, 10, 0, 0, 0, ukLocation) + _, err := db.DB.Exec(context.Background(), ` + INSERT INTO time_blockers (start_time, duration_minutes, description, created_by) + VALUES ($1, 60, 'Staff Meeting', NULL) + `, blockerTime) + if err != nil { + t.Fatalf("failed to create time blocker: %v", err) + } + + // Make request as admin user + handler := http.HandlerFunc(GetAvailableHours) + req := httptest.NewRequest("GET", "/api/scheduling/available-hours?start=2026-03-16&end=2026-03-16", nil) + + // Set admin context + ctx := context.WithValue(req.Context(), mw.UserIDKey, "admin001") + ctx = context.WithValue(ctx, mw.UserRoleKey, "admin") + req = req.WithContext(ctx) + + w := httptest.NewRecorder() + handler.ServeHTTP(w, req) + + if w.Code != http.StatusOK { + t.Errorf("expected status 200, got %d. body: %s", w.Code, w.Body.String()) + } + + var response []DayAvailableHours + if err := json.Unmarshal(w.Body.Bytes(), &response); err != nil { + t.Fatalf("failed to unmarshal response: %v", err) + } + + if len(response) == 0 { + t.Fatal("expected at least one day in response") + } + + // Find the day with the blocker (2026-03-16) + var targetDay *DayAvailableHours + for i := range response { + if response[i].Date == "2026-03-16" { + targetDay = &response[i] + break + } + } + + if targetDay == nil { + t.Fatal("expected day 2026-03-16 in response") + } + + // Verify blocker IS visible in blockers field for admin + if len(targetDay.Blockers) == 0 { + t.Error("expected blockers field to contain the blocker for admin users") + } else { + // Verify the blocker time range + found := false + for _, blocker := range targetDay.Blockers { + if blocker.StartTime == "10:00" && blocker.EndTime == "11:00" { + found = true + break + } + } + if !found { + t.Error("expected blocker 10:00-11:00 in blockers field") + } + } +} diff --git a/backend/testutils/fixtures/fixtures.go b/backend/testutils/fixtures/fixtures.go index 079910d..ae33ba2 100644 --- a/backend/testutils/fixtures/fixtures.go +++ b/backend/testutils/fixtures/fixtures.go @@ -6,6 +6,7 @@ package fixtures import ( "context" "fmt" + "time" "github.com/jackc/pgx/v5/pgxpool" "golang.org/x/crypto/bcrypt" @@ -202,3 +203,28 @@ func SafeDeleteService(db *pgxpool.Pool, serviceID string) error { func SafeDeleteBooking(db *pgxpool.Pool, bookingID string) error { return DeleteBooking(db, bookingID) } + +// CreateTestTimeBlocker creates a time blocker for testing +// Returns the blocker ID +func CreateTestTimeBlocker(pool *pgxpool.Pool, startTime time.Time, durationMinutes int, description string) (string, error) { + ctx := context.Background() + var blockerID string + err := pool.QueryRow(ctx, ` + INSERT INTO time_blockers (start_time, duration_minutes, description) + VALUES ($1, $2, $3) + RETURNING id + `, startTime, durationMinutes, description).Scan(&blockerID) + + if err != nil { + return "", fmt.Errorf("failed to create time blocker: %w", err) + } + + return blockerID, nil +} + +// DeleteTimeBlocker removes a time blocker from the database +func DeleteTimeBlocker(pool *pgxpool.Pool, blockerID string) error { + ctx := context.Background() + _, err := pool.Exec(ctx, "DELETE FROM time_blockers WHERE id = $1", blockerID) + return err +} diff --git a/backend/testutils/testdb/testdb.go b/backend/testutils/testdb/testdb.go index 9a8eb98..517a119 100644 --- a/backend/testutils/testdb/testdb.go +++ b/backend/testutils/testdb/testdb.go @@ -85,6 +85,7 @@ func Migrate(t *testing.T, pool *pgxpool.Pool) { "users", "images", "tags", + "time_blockers", "working_hours", "exceptional_group_applications", "exceptional_working_hours", @@ -204,6 +205,7 @@ func TruncateTables(t *testing.T, pool *pgxpool.Pool) { "admin_notifications", "user_referrals", "user_notification_preferences", + "time_blockers", "working_hours", "exceptional_working_hours", "exceptional_working_hours_groups",