From 6808752e0de8e191a96645578f29716318682ad7 Mon Sep 17 00:00:00 2001 From: Stephen Adamson Date: Sun, 3 May 2026 15:09:50 +0100 Subject: [PATCH] feat: unify walk-in and call-in reservation flows with 15min TTL, guest booking support, and slot awareness - backend/handlers/bookings/admin_reserve.go: - Add explicit reservation_type field ("walkin" | "callin") to request struct - Remove TTL-based heuristic for type detection - Walk-in: uses duration_minutes, allows null user_id, 1min past grace - Call-in: requires service_ids, validates future time, calculates duration from services - Both types now use 15-minute TTL - backend/handlers/scheduling/time-blockers.go: - Update CleanupOldReservations: both walkin and callin use 15min TTL (was 10min/60min) - frontend/WalkInBooking.svelte: - Full rewrite of reservation logic - If available now and >15min remaining: reserve from now to slot end - If <=15min or not available: reserve next full slot - Always reserves before opening modal (never open without hold) - Passes reservedDuration to modal - TTL changed from 5 to 15 minutes - frontend/WalkInCreateModal.svelte: - Replace dead commented-out guest code with working guest creation - Guest account created at submit time (not earlier) - Phone defaults to +447700900000 if blank - Phone field marked optional with helper text - Name split into firstName/lastName for backend - Validation relaxed: only name required for guests - frontend/BookingCreateModal.svelte: - TTL changed from 60 to 15 minutes - Add reservation_type: "callin" to reserve payload - Guest creation uses correct firstName/lastName fields - Default guest phone to +447700900000 - Reservation no longer requires selectedUserId (works for guests) - docs: Update Future Work backlog to mark completed items --- backend/handlers/bookings/admin_reserve.go | 81 ++- backend/handlers/scheduling/time-blockers.go | 9 +- .../admin/BookingCreateModal.svelte | 61 +- .../lib/components/admin/WalkInBooking.svelte | 82 +-- .../components/admin/WalkInCreateModal.svelte | 77 ++- .../Crussell/Future Work - Gap Backlog.md | 6 +- obsidian/Crussell/Test Implementation Plan.md | 584 ++++++++++++++++++ 7 files changed, 748 insertions(+), 152 deletions(-) create mode 100644 obsidian/Crussell/Test Implementation Plan.md diff --git a/backend/handlers/bookings/admin_reserve.go b/backend/handlers/bookings/admin_reserve.go index 0862c94..0c99108 100644 --- a/backend/handlers/bookings/admin_reserve.go +++ b/backend/handlers/bookings/admin_reserve.go @@ -26,7 +26,9 @@ type AdminReserveSlotRequest struct { StartTime time.Time `json:"start_time"` ServiceIDs []string `json:"service_ids"` ServiceOverrides []ServiceOverrideRequest `json:"service_overrides"` - TTLMinutes int `json:"ttl_minutes"` // 5 for walk-in, 60 for call-in + TTLMinutes int `json:"ttl_minutes"` // 15 for both walk-in and call-in + ReservationType string `json:"reservation_type"` // "walkin" or "callin" + DurationMinutes int `json:"duration_minutes"` // explicit duration for walk-in (ignored for call-in) } // AdminReserveSlotResponse represents the response for admin slot reservation @@ -40,14 +42,12 @@ type AdminReserveSlotResponse struct { // AdminReserveSlotHandler creates a temporary admin slot reservation (walk-in or call-in) func AdminReserveSlotHandler(w http.ResponseWriter, r *http.Request) { - // a. Extract admin user ID from context adminID, ok := r.Context().Value(mw.UserIDKey).(string) if !ok || adminID == "" { http.Error(w, "Authentication required", http.StatusUnauthorized) return } - // b. Parse JSON body var req AdminReserveSlotRequest if err := json.NewDecoder(r.Body).Decode(&req); err != nil { log.Printf("Failed to decode request: %v", err) @@ -55,43 +55,55 @@ func AdminReserveSlotHandler(w http.ResponseWriter, r *http.Request) { return } - // Validate start_time is present if req.StartTime.IsZero() { http.Error(w, "start_time is required", http.StatusBadRequest) return } - // Validate service_ids are present - if len(req.ServiceIDs) == 0 { - http.Error(w, "At least one service is required", http.StatusBadRequest) + if req.ReservationType != "walkin" && req.ReservationType != "callin" { + http.Error(w, "reservation_type must be 'walkin' or 'callin'", http.StatusBadRequest) return } - // Default ttl_minutes to 60 if 0 if req.TTLMinutes == 0 { - req.TTLMinutes = 60 + req.TTLMinutes = 15 } - // c. Calculate total duration from services, respecting overrides - svcDuration, err := calculateServiceDurationWithOverrides(r.Context(), req.ServiceIDs, req.ServiceOverrides) - if err != nil { - log.Printf("Failed to calculate duration: %v", err) - http.Error(w, "Internal server error", http.StatusInternalServerError) - return + var svcDuration int + + if req.ReservationType == "callin" { + if len(req.ServiceIDs) == 0 { + http.Error(w, "At least one service is required for call-in bookings", http.StatusBadRequest) + return + } + if req.StartTime.Before(time.Now()) { + http.Error(w, "Start time cannot be in the past", http.StatusBadRequest) + return + } + var err error + svcDuration, err = calculateServiceDurationWithOverrides(r.Context(), req.ServiceIDs, req.ServiceOverrides) + if err != nil { + log.Printf("Failed to calculate duration: %v", err) + http.Error(w, "Internal server error", http.StatusInternalServerError) + return + } + if svcDuration == 0 { + http.Error(w, "Invalid service IDs", http.StatusBadRequest) + return + } + } else { + if req.DurationMinutes <= 0 { + http.Error(w, "duration_minutes is required for walk-in reservations", http.StatusBadRequest) + return + } + svcDuration = req.DurationMinutes + allowablePast := time.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 + } } - if svcDuration == 0 { - http.Error(w, "Invalid service IDs", http.StatusBadRequest) - return - } - - // d. Validate start_time not in the past - if req.StartTime.Before(time.Now()) { - http.Error(w, "Start time cannot be in the past", http.StatusBadRequest) - return - } - - // e. Validate working hours exist for that weekday and slot fits within open->close weekday := int(req.StartTime.Weekday()) var closeStr string if err := db.DB.QueryRow(r.Context(), `SELECT end_time::text FROM working_hours WHERE weekday = $1`, weekday).Scan(&closeStr); err != nil { @@ -111,7 +123,6 @@ func AdminReserveSlotHandler(w http.ResponseWriter, r *http.Request) { return } - // f. Check existing booking overlap (same query as CreateBookingHandler line ~1197) var cnt int db.DB.QueryRow(r.Context(), ` SELECT COUNT(*) FROM bookings WHERE status IN ('confirmed','in_progress','completed') @@ -126,7 +137,6 @@ func AdminReserveSlotHandler(w http.ResponseWriter, r *http.Request) { return } - // g. Check time blocker overlap using scheduling.CheckTimeBlockerOverlap blockerOverlap, blockerDesc, err := scheduling.CheckTimeBlockerOverlap(r.Context(), req.StartTime, endTime) if err != nil { log.Printf("Failed to check time blocker overlap: %v", err) @@ -135,7 +145,6 @@ func AdminReserveSlotHandler(w http.ResponseWriter, r *http.Request) { return } - // h. Delete any existing admin reservation for this admin _, err = db.DB.Exec(r.Context(), ` DELETE FROM time_blockers WHERE description LIKE 'RESERVATION:admin:%' @@ -147,20 +156,12 @@ func AdminReserveSlotHandler(w http.ResponseWriter, r *http.Request) { return } - // i. Determine reservation type: if ttl_minutes <= 10 → "walkin", else → "callin" - reservationType := "callin" - if req.TTLMinutes <= 10 { - reservationType = "walkin" - } - - // j. Determine customer ID from request or use "guest" customerID := "guest" if req.UserID != nil && *req.UserID != "" { customerID = *req.UserID } - // Insert new reservation - description := fmt.Sprintf("RESERVATION:admin:%s:%s:%d", reservationType, customerID, time.Now().UnixNano()) + description := fmt.Sprintf("RESERVATION:admin:%s:%s:%d", req.ReservationType, customerID, time.Now().UnixNano()) var reservationID string var createdAt time.Time err = db.DB.QueryRow(r.Context(), ` @@ -174,10 +175,8 @@ func AdminReserveSlotHandler(w http.ResponseWriter, r *http.Request) { return } - // k. Calculate expires_at based on TTL expiresAt := createdAt.Add(time.Duration(req.TTLMinutes) * time.Minute) - // Return 201 with response response := AdminReserveSlotResponse{ ID: reservationID, StartTime: req.StartTime, diff --git a/backend/handlers/scheduling/time-blockers.go b/backend/handlers/scheduling/time-blockers.go index 8f03089..ac40e57 100644 --- a/backend/handlers/scheduling/time-blockers.go +++ b/backend/handlers/scheduling/time-blockers.go @@ -337,19 +337,20 @@ func CheckTimeBlockerOverlap(ctx context.Context, startTime, endTime time.Time) // CleanupOldReservations deletes expired reservations: // - Logged-in (RESERVATION:user): older than 1 hour // - Anonymous (RESERVATION:anon): older than 10 minutes -// - Admin walk-in (RESERVATION:admin:walkin:%): older than 10 minutes -// - Admin call-in (RESERVATION:admin:callin:%): older than 1 hour +// - Admin walk-in (RESERVATION:admin:walkin:%): older than 15 minutes +// - Admin call-in (RESERVATION:admin:callin:%): older than 15 minutes func CleanupOldReservations(ctx context.Context) error { oneHourAgo := time.Now().Add(-1 * time.Hour) tenMinutesAgo := time.Now().Add(-10 * time.Minute) + fifteenMinutesAgo := time.Now().Add(-15 * time.Minute) _, err := db.DB.Exec(ctx, ` DELETE FROM time_blockers WHERE (description LIKE 'RESERVATION:user:%' AND created_at < $1) OR (description LIKE 'RESERVATION:anon:%' AND created_at < $2) - OR (description LIKE 'RESERVATION:admin:walkin:%' AND created_at < $2) + OR (description LIKE 'RESERVATION:admin:walkin:%' AND created_at < $3) OR (description LIKE 'RESERVATION:admin:callin:%' AND created_at < $3) - `, oneHourAgo, tenMinutesAgo, oneHourAgo) + `, oneHourAgo, tenMinutesAgo, fifteenMinutesAgo) return err } diff --git a/frontend/src/lib/components/admin/BookingCreateModal.svelte b/frontend/src/lib/components/admin/BookingCreateModal.svelte index dd29089..7350d61 100644 --- a/frontend/src/lib/components/admin/BookingCreateModal.svelte +++ b/frontend/src/lib/components/admin/BookingCreateModal.svelte @@ -361,7 +361,7 @@ // =============== Reservation =============== async function reserveSlot(): Promise { - if (!selectedUserId || !selectedDate || !selectedTime) { + if (!selectedDate || !selectedTime) { return false; } @@ -387,19 +387,22 @@ } } + const payload = { + user_id: selectedUserId || null, + start_time: startTimeISO, + service_ids: serviceIds, + service_overrides: overrides.length > 0 ? overrides : [], + ttl_minutes: 15, + reservation_type: 'callin' + }; + const response = await fetch('/api/admin/bookings/reserve', { method: 'POST', headers: { 'Content-Type': 'application/json', Authorization: `Bearer ${authStore.currentToken}` }, - body: JSON.stringify({ - user_id: selectedUserId, - start_time: startTimeISO, - service_ids: serviceIds, - service_overrides: overrides.length > 0 ? overrides : [], - ttl_minutes: 60 - }) + body: JSON.stringify(payload) }); if (response.ok) { @@ -732,28 +735,32 @@ try { let finalUserId = selectedUserId; - if (userType === 'guest') { - const createRes = await fetch('/api/users/guest', { - method: 'POST', - headers: { - 'Content-Type': 'application/json', - Authorization: `Bearer ${authStore.currentToken}` - }, - body: JSON.stringify({ - name: guestName, - phone: guestPhone - }) - }); + if (userType === 'guest') { + const phone = guestPhone.trim() || '+447700900000'; + const createRes = await fetch('/api/users/guest', { + method: 'POST', + headers: { + 'Content-Type': 'application/json', + Authorization: `Bearer ${authStore.currentToken}` + }, + body: JSON.stringify({ + firstName: guestName.trim().split(' ')[0] || 'Guest', + lastName: guestName.trim().split(' ').slice(1).join(' ') || 'Customer', + phone: phone, + email: `callin-${Date.now()}@guest.invalid` + }) + }); - if (!createRes.ok) { - toast.error('Failed to create guest user'); - return; - } - - const guestUser = await createRes.json(); - finalUserId = guestUser.id; + if (!createRes.ok) { + toast.error('Failed to create guest user'); + submitting = false; + return; } + const guestUser = await createRes.json(); + finalUserId = guestUser.id; + } + if (!finalUserId) throw new Error('User ID required'); if (!selectedDate || !selectedTime) throw new Error('Date and time required'); diff --git a/frontend/src/lib/components/admin/WalkInBooking.svelte b/frontend/src/lib/components/admin/WalkInBooking.svelte index 96c35ab..b0c38c8 100644 --- a/frontend/src/lib/components/admin/WalkInBooking.svelte +++ b/frontend/src/lib/components/admin/WalkInBooking.svelte @@ -9,6 +9,8 @@ import type { AvailableHoursDay } from '$lib/types/booking'; + const RESERVATION_TTL = 15; + let showCreateModal = $state(false); let slotInfo = $state<{ isAvailableNow: boolean; @@ -21,38 +23,30 @@ let noSlotsToday = $state(false); let currentTime = $state(new Date()); - // Reservation state for walk-in let reservationId = $state(null); let reservationExpiresAt = $state(null); let reservationCountdown = $state(''); let isReserving = $state(false); + let reservedDuration = $state(0); onMount(() => { calculateSlotAvailability(); - // Update current time every minute for live countdown const interval = setInterval(() => { currentTime = new Date(); - }, 60000); // Update every minute + }, 60000); return () => clearInterval(interval); }); - /** - * Converts "HH:MM" or "HH:MM:SS" time string to minutes since midnight - */ function timeToMinutes(time: string): number { const parts = time.split(':').map(Number); return parts[0] * 60 + parts[1]; } - /** - * Converts minutes to hours and minutes for display - */ function formatDuration(minutes: number): string { const hours = Math.floor(minutes / 60); const mins = minutes % 60; - if (hours > 0 && mins > 0) { return `${hours} hour${hours !== 1 ? 's' : ''}, ${mins} minute${mins !== 1 ? 's' : ''}`; } else if (hours > 0) { @@ -62,23 +56,15 @@ } } - /** - * Calculate live remaining time based on current time - */ function getLiveRemainingMinutes(): number | null { if (!slotInfo?.isAvailableNow || !slotInfo.slotEndMinutes) return null; - const now = currentTime.getHours() * 60 + currentTime.getMinutes(); const remaining = slotInfo.slotEndMinutes - now; return Math.max(0, remaining); } - /** - * Calculate live wait time based on current time - */ function getLiveWaitMinutes(): number | null { if (slotInfo?.isAvailableNow || !slotInfo?.startTime) return null; - const now = currentTime.getHours() * 60 + currentTime.getMinutes(); const slotStartMinutes = timeToMinutes(slotInfo.startTime); const wait = slotStartMinutes - now; @@ -93,7 +79,6 @@ const now = new SvelteDate(); const today = new CalendarDate(now.getFullYear(), now.getMonth() + 1, now.getDate()); - // Fetch today's available hours const response = await fetch(`/api/scheduling/available-hours?start=${today}&end=${today}`, { headers: { Authorization: `Bearer ${authStore.currentToken}` } }); @@ -111,26 +96,22 @@ return; } - // Current time in minutes since midnight const currentMinutes = now.getHours() * 60 + now.getMinutes(); - // Check if we're currently in an available slot for (const slot of todayData.slots) { const slotStartMinutes = timeToMinutes(slot.startTime); const slotEndMinutes = timeToMinutes(slot.endTime); - // Are we currently within this slot? if (currentMinutes >= slotStartMinutes && currentMinutes < slotEndMinutes) { const remainingMinutes = slotEndMinutes - currentMinutes; slotInfo = { isAvailableNow: true, durationMinutes: remainingMinutes, - slotEndMinutes: slotEndMinutes // Store for live countdown + slotEndMinutes: slotEndMinutes }; return; } - // Is this a future slot? if (slotStartMinutes > currentMinutes) { const waitMinutes = slotStartMinutes - currentMinutes; const durationMinutes = slotEndMinutes - slotStartMinutes; @@ -144,7 +125,6 @@ } } - // No current or future slots available noSlotsToday = true; } catch (err) { console.error('Failed to calculate slot availability', err); @@ -161,7 +141,7 @@ return `${displayHours}:${minutes.toString().padStart(2, '0')} ${period}`; } - async function reserveWalkInSlot(startTime: string): Promise { + async function reserveWalkInSlot(startTime: string, durationMinutes: number): Promise { isReserving = true; try { @@ -189,13 +169,16 @@ start_time: startTimeISO, service_ids: [], service_overrides: [], - ttl_minutes: 5 + ttl_minutes: RESERVATION_TTL, + reservation_type: 'walkin', + duration_minutes: durationMinutes }) }); if (response.ok) { const data = await response.json(); reservationId = data.id; + reservedDuration = data.duration_minutes; reservationExpiresAt = new Date(data.expires_at); startWalkInCountdown(); return true; @@ -252,15 +235,39 @@ (window as any).__walkInCountdownInterval = setInterval(updateCountdown, 1000); } - function handleStartWalkIn() { - if (slotInfo?.isAvailableNow) { - showCreateModal = true; - } else if (slotInfo?.startTime) { - reserveWalkInSlot(slotInfo.startTime).then((reserved) => { - if (reserved) { - showCreateModal = true; + async function handleStartWalkIn() { + if (!slotInfo) return; + + let reserveTime: string; + let reserveDuration: number; + + if (slotInfo.isAvailableNow) { + const liveRemaining = getLiveRemainingMinutes() ?? 0; + if (liveRemaining > RESERVATION_TTL) { + // Available now with >15min remaining — reserve from now to slot end + const now = new SvelteDate(); + const currentMin = now.getHours() * 60 + now.getMinutes(); + reserveTime = `${String(now.getHours()).padStart(2, '0')}:${String(now.getMinutes()).padStart(2, '0')}`; + reserveDuration = slotInfo.slotEndMinutes! - currentMin; + } else { + // Available now but ≤15min — reserve next slot instead + await calculateSlotAvailability(); + if (!slotInfo || slotInfo.isAvailableNow || !slotInfo.startTime) { + toast.error('No suitable slot available'); + return; } - }); + reserveTime = slotInfo.startTime; + reserveDuration = slotInfo.durationMinutes; + } + } else { + // Not available now — reserve the next slot + reserveTime = slotInfo.startTime!; + reserveDuration = slotInfo.durationMinutes; + } + + const reserved = await reserveWalkInSlot(reserveTime, reserveDuration); + if (reserved) { + showCreateModal = true; } } @@ -269,6 +276,7 @@ reservationId = null; reservationExpiresAt = null; reservationCountdown = ''; + reservedDuration = 0; if ((window as any).__walkInCountdownInterval) { clearInterval((window as any).__walkInCountdownInterval); } @@ -325,8 +333,8 @@ {#if showCreateModal} diff --git a/frontend/src/lib/components/admin/WalkInCreateModal.svelte b/frontend/src/lib/components/admin/WalkInCreateModal.svelte index 0e9f212..5e6969a 100644 --- a/frontend/src/lib/components/admin/WalkInCreateModal.svelte +++ b/frontend/src/lib/components/admin/WalkInCreateModal.svelte @@ -107,7 +107,7 @@ const isOverDuration = $derived(getTotalDuration() > maxSlotDuration); const canProceedStep1 = $derived( - userType === 'member' ? !!selectedUserId : !!(guestName.trim() && guestPhone.trim()) + userType === 'member' ? !!selectedUserId : !!guestName.trim() ); const canProceedStep2 = $derived(selectedServices.length > 0 && !isOverDuration); @@ -271,36 +271,35 @@ return; } - let finalUserId = selectedUserId; + let finalUserId = selectedUserId; - if (userType === 'guest') { - // TODO: Implement /api/users/guest endpoint - // For now, show error - toast.error('Guest booking not yet implemented'); + if (userType === 'guest') { + const phone = guestPhone.trim() || '+447700900000'; + const createRes = await fetch('/api/users/guest', { + method: 'POST', + headers: { + 'Content-Type': 'application/json', + Authorization: `Bearer ${authStore.currentToken}` + }, + body: JSON.stringify({ + firstName: guestName.trim().split(' ')[0] || 'Walk-in', + lastName: guestName.trim().split(' ').slice(1).join(' ') || 'Guest', + phone: phone, + email: `walkin-${Date.now()}@guest.invalid` + }) + }); + + if (!createRes.ok) { + toast.error('Failed to create guest user'); + submitting = false; return; - - // const createRes = await fetch('/api/users/guest', { - // method: 'POST', - // headers: { - // 'Content-Type': 'application/json', - // Authorization: `Bearer ${authStore.currentToken}` - // }, - // body: JSON.stringify({ - // name: guestName, - // phone: guestPhone - // }) - // }); - - // if (!createRes.ok) { - // toast.error('Failed to create guest user'); - // return; - // } - - // const guestUser = await createRes.json(); - // finalUserId = guestUser.id; } - if (!finalUserId) throw new Error('User ID required'); + const guestUser = await createRes.json(); + finalUserId = guestUser.id; + } + + if (!finalUserId) throw new Error('User ID required'); // Use the available slot start time from the widget let start: Date; @@ -355,8 +354,6 @@ notes: notes.trim() || null }; - // TODO: When guest booking is fully implemented, ensure walk-in guest reservations properly transition to real bookings. - const res = await fetch('/api/admin/bookings', { method: 'POST', headers: { @@ -595,17 +592,17 @@ oninput={(e) => (guestName = e.currentTarget.value)} /> -
- - (guestPhone = e.currentTarget.value)} - /> -
+
+ + (guestPhone = e.currentTarget.value)} + /> +

Booking as a guest creates a temporary record. Encourage them to sign up for loyalty benefits. diff --git a/obsidian/Crussell/Future Work - Gap Backlog.md b/obsidian/Crussell/Future Work - Gap Backlog.md index 0a1f59d..0c7d2d6 100644 --- a/obsidian/Crussell/Future Work - Gap Backlog.md +++ b/obsidian/Crussell/Future Work - Gap Backlog.md @@ -12,9 +12,9 @@ No external dependencies. No paid services. No API keys needed. | # | Gap | Effort | Area | Notes | |---|-----|--------|------|-------| | 1 | ~~`DELETE /api/user/account` is a no-op~~ ✅ | S (1-2h) | Backend | Wired to `anonymize_user()` for registered users and `delete_guest_user()` for guests. CardDAV contact deleted best-effort. | -| 2 | **WalkInCreateModal guest booking errors out** | S (1-2h) | Frontend | Line 277: commented-out guest creation code. Shows "Guest booking not yet implemented" toast despite backend fully working. | +| 2 | ~~**WalkInCreateModal guest booking errors out**~~ ✅ | S (1-2h) | Frontend | Guest creation now fires at submit time in both walk-in and call-in flows. Phone defaults to +447700900000 if left blank. | | 3 | **ApprovalModal decline/cancel stub** | S (2-3h) | Frontend | `handleDecline()` shows "Coming soon" toast. Admin cannot reject pending bookings. Backend confirm/cancel endpoints exist — decline just needs a cancel call. | -| 4 | **CurrentAppointment action stubs** | M (1d) | Frontend | `handleTakePayment()`, `handleExtend()`, `handleCancel()` all show "Coming soon". Today page has 3 dead buttons. Take payment is ⚠️ blocked on Square, but Extend and Cancel are local. | +| 4 | **CurrentAppointment action stubs** | M (1d) | Frontend | `handleTakePayment()` ⚠️ blocked on Square. `handleExtend()`, `handleCancel()` — dead buttons. | ## P1 — High @@ -24,7 +24,7 @@ No external dependencies. No paid services. No API keys needed. | 6 | **Reservation/anonymization cron** | S (2-3h) | Backend | `CleanupOldReservations()` and `AnonymizeStaleGuestAccounts()` only fire on availability fetch. If no one fetches availability, expired reservations persist and stale guests aren't anonymized. Should be a background ticker in `main.go`. | | 7 | **GDPR data export endpoint** | M (1d) | Backend | `export_all_user_data()` SQL function exists (JSON export). No Go handler wired. Required for GDPR Article 15 SAR requests. | | 8 | **VAT/Tax export endpoints** | M (1-2d) | Backend | `get_vat_return_data()`, `export_sales_transactions()` SQL functions exist. No admin API to trigger them. Needed for HMRC compliance. | -| 9 | **Walk-in guest reservation → booking transition** | S (1h) | Frontend | WalkInCreateModal line 358: TODO notes guest reservations may not properly transition to real bookings. Needs verification + fix. | +| ~~9~~ | ~~**Walk-in guest reservation → booking transition**~~ ✅ | S (1h) | Frontend | Done — guest accounts created at submit time, reservation system uses explicit reservation_type field, both walk-in and call-in use 15min TTL. | | 10 | **Password reset flow not wired to frontend** | S (2-3h) | Frontend | Backend has `/api/verify/generate` and `/api/verify/check` endpoints. Login page has no "forgot password" link or form. | | 11 | **Email verification flow not wired to frontend** | S (2-3h) | Frontend | Users register with `unverified_email` role. No UI to enter verification code or resend code. `+layout.svelte` has alert-based prototype. | | 12 | **Booking cancellation from user account** | S (2-3h) | Frontend | UserBookingModal shows booking details but no cancel button. Users must call/email to cancel. Backend endpoint exists (`DELETE /api/bookings/{id}`). | diff --git a/obsidian/Crussell/Test Implementation Plan.md b/obsidian/Crussell/Test Implementation Plan.md new file mode 100644 index 0000000..c66d5f5 --- /dev/null +++ b/obsidian/Crussell/Test Implementation Plan.md @@ -0,0 +1,584 @@ +# Test Implementation Plan + +**Target:** Fill all testable gaps in the Crussell backend test suite. +**Scope:** Go unit tests only (no integration tests, no frontend tests unless trivial). +**Files to modify/create:** See tasks below. +**Total estimated effort:** 4-6 hours. + +--- + +## Context + +Crussell is a Go 1.25 + chi router + PostgreSQL nail salon booking app. Tests use `pgxpool` with a dedicated test database. Build tag: `//go:build test`. Fixtures in `testutils/fixtures/`. JWT helpers in `testutils/jwt/`. + +Key patterns to follow: +- `setupTest(t)` creates a fresh DB pool + migrations +- `defer cleanup()` to drop +- `fixtures.CreateTestUser(pool)` creates a registered user +- `fixtures.CreateTestGuestUser(pool)` creates a guest user +- `fixtures.CreateTestService(pool)` creates a service +- `fixtures.CreateTestAdminUser(pool)` creates an admin +- `jwt.GenerateUserToken(userID)` / `jwt.GenerateAdminToken()` for auth +- `httptest.NewRecorder()` + handler direct calls for API tests + +--- + +## Task 1: Admin Reserve Slot Handler Tests + +**File:** `backend/handlers/bookings/admin_reserve_test.go` (new file) +**What to test:** `POST /api/admin/bookings/reserve` (`AdminReserveSlotHandler`) +**Why:** Zero tests exist. We just rewrote this handler extensively. + +### Test Cases + +```go +TestAdminReserveSlot_WalkIn_Success +``` +- Create admin user + get admin token +- POST with `reservation_type: "walkin"`, `start_time: now`, `duration_minutes: 30`, `service_ids: []`, `ttl_minutes: 15`, `user_id: null` +- Assert 201 Created +- Assert response has `id`, `expires_at` ≈ now+15min, `duration_minutes: 30` +- Query DB: verify `time_blockers` row exists with description `RESERVATION:admin:walkin:%` + +```go +TestAdminReserveSlot_CallIn_Success +``` +- Create admin + regular user + service (30min duration) +- POST with `reservation_type: "callin"`, `start_time: tomorrow 10:00`, `service_ids: [svcID]`, `ttl_minutes: 15`, `user_id: userID` +- Assert 201 +- Assert response `duration_minutes` = service duration +- Verify description: `RESERVATION:admin:callin:%` + +```go +TestAdminReserveSlot_WalkIn_MissingDuration +``` +- POST walk-in without `duration_minutes` +- Assert 400, body contains "duration_minutes is required" + +```go +TestAdminReserveSlot_CallIn_MissingServices +``` +- POST call-in with empty `service_ids` +- Assert 400, body contains "At least one service is required" + +```go +TestAdminReserveSlot_InvalidReservationType +``` +- POST with `reservation_type: "invalid"` +- Assert 400, body contains "reservation_type must be 'walkin' or 'callin'" + +```go +TestAdminReserveSlot_SlotOverlap +``` +- Create admin + existing booking at 10:00 tomorrow (30min) +- POST call-in for 10:15 tomorrow (overlaps) +- Assert 409 Conflict + +```go +TestAdminReserveSlot_ReplacesExisting +``` +- Create admin, reserve once, get reservation ID +- Reserve again (same admin) +- Assert 201 +- Query DB: old reservation should be deleted, new one exists + +```go +TestAdminReserveSlot_WalkIn_PastStart +``` +- POST walk-in with `start_time: now - 5 minutes` +- Assert 400 (or 201 with 1-minute grace — check handler logic) + +### Notes +- The handler is in `backend/handlers/bookings/admin_reserve.go` +- The struct is `AdminReserveSlotRequest` +- Handler extracts admin ID from `r.Context().Value(mw.UserIDKey)` +- Walk-in allows `start_time` up to 1 minute in the past (line 99 in handler) +- Call-in requires `start_time` in the future (line 95 in handler) + +--- + +## Task 2: CleanupOldReservations Admin TTL Tests + +**File:** `backend/handlers/scheduling/time_blockers_test.go` (add to existing) +**What to test:** `CleanupOldReservations` now cleans admin walkin/callin at 15 minutes +**Why:** Existing test only covers `RESERVATION:user:%` (1 hour). Admin paths were just changed from 10min/60min to 15min/15min. + +### Test Cases + +```go +TestCleanupOldReservations_AdminWalkIn +``` +- Insert `RESERVATION:admin:walkin:guest:123` with `created_at: now - 16 minutes` +- Insert `RESERVATION:admin:walkin:guest:456` with `created_at: now - 14 minutes` +- Call `CleanupOldReservations(ctx)` +- Assert 16-min old deleted, 14-min old preserved + +```go +TestCleanupOldReservations_AdminCallIn +``` +- Same as above but with `RESERVATION:admin:callin:guest:123` +- Same assertions + +```go +TestCleanupOldReservations_MixedTypes +``` +- Insert 6 reservations: user (old + recent), anon (old + recent), walkin (old + recent), callin (old + recent) +- Call cleanup +- Assert only "old" ones from each type are deleted (user >1h, anon >10min, admin >15min) + +### Notes +- Use `time.Now().Add(-16 * time.Minute)` for old, `time.Now().Add(-14 * time.Minute)` for recent +- The function is in `backend/handlers/scheduling/time_blockers.go` line 337 +- SQL pattern: `description LIKE 'RESERVATION:admin:walkin:%'` and `created_at < $3` (15min ago) + +--- + +## Task 3: Health Check Endpoint Tests + +**File:** `backend/handlers/handlers_test.go` or new `backend/handlers/health_test.go` +**What to test:** `GET /api/health` (`healthCheckHandler` in `main.go`) +**Why:** Brand new endpoint, zero tests. + +### Test Cases + +```go +TestHealthCheck_OK +``` +- Call `healthCheckHandler` directly with `httptest.NewRecorder()` +- Assert 200 OK +- Assert JSON has `status: "ok"`, `services.backend: "ok"`, `services.database: "ok"` + +```go +TestHealthCheck_Degraded +``` +- Temporarily set `db.DB = nil` (or use a bad connection) +- Call handler +- Assert 503 Service Unavailable +- Assert `status: "degraded"`, `services.database: "error"` +- Restore db.DB after test + +### Notes +- Handler is `healthCheckHandler` in `backend/main.go` (lines 63-97) +- Uses `db.DB.Ping()` and checks `s3.Client == nil` +- Returns 503 when degraded (we fixed this in a previous commit) + +--- + +## Task 4: Deposit Reduction on Payment Completion + +**File:** `backend/handlers/admin/bookings_test.go` (add to existing) +**What to test:** When a booking transitions to `completed` with ≥1 payment, `deposits_required` decreases by 1. +**Why:** Business rule exists in SQL (`get_vat_return_data` area) but no explicit test. + +### Test Cases + +```go +TestAdminBookings_Confirm_CompletesWithPayment_ReducesDeposits +``` +- Create user with `deposits_required = 2` +- Create booking, confirm it, progress to `in_progress`, add a payment +- Transition to `completed` +- Assert user's `deposits_required` = 1 + +```go +TestAdminBookings_Confirm_CompletesWithoutPayment_NoReduction +``` +- Create user with `deposits_required = 2` +- Create booking, complete it without payment +- Assert `deposits_required` still = 2 + +### Notes +- This may require SQL-level verification since the reduction logic might be in a trigger or cron +- Check `init-scripts/init-script.sql` for `update_booking_status` or similar triggers +- The user's `deposits_required` field is in the `users` table + +--- + +## Task 5: No-Show Accumulation (2+ in 6 months) + +**File:** `backend/handlers/bookings/bookings_test.go` (add to existing) +**What to test:** 2+ unforgiven no-shows in 6 months → `deposits_required = 3` +**Why:** Critical business rule with no test coverage. + +### Test Cases + +```go +TestBookings_Delete_SecondNoShowIn6Months_ResetsDepositsTo3 +``` +- Create user with `deposits_required = 0` +- Create booking 1, cancel <24h without forgiveness (no_show) +- Create booking 2, cancel <24h without forgiveness (no_show) +- Assert user `deposits_required = 3` + +```go +TestBookings_Delete_SingleNoShow_NoDepositReset +``` +- Create user with `deposits_required = 0` +- Create booking, cancel <24h without forgiveness +- Assert user `deposits_required = 3` (or 1? check actual behavior) + +```go +TestBookings_Delete_NoShowOlderThan6Months_NotCounted +``` +- Create user, create booking 7 months ago, mark as no_show +- Create new booking, cancel <24h without forgiveness +- Assert `deposits_required` only counts the recent one + +### Notes +- Check the actual SQL/function logic for this rule +- May need to manipulate `created_at` or booking dates directly in DB +- The `forgiven_no_shows` table tracks forgiven instances + +--- + +## Task 6: Admin Booking with enforce_deposits=false + +**File:** `backend/handlers/admin/bookings_test.go` (add to existing) +**What to test:** `enforce_deposits: false` actually bypasses deposit checks. +**Why:** Tests exist for `enforce_deposits=true` but not the bypass path. + +### Test Cases + +```go +TestAdminBookings_Create_EnforceDepositsFalse_BypassesLimit +``` +- Create user with `deposits_required = 3` (blocked) +- Create one active booking for this user +- Try to create second booking with `enforce_deposits: false` +- Assert 201 Created (should succeed despite deposits) + +```go +TestAdminBookings_Create_EnforceDepositsFalse_Within24h +``` +- Create user with `deposits_required = 3` +- Try to create booking <24h in advance with `enforce_deposits: false` +- Assert 201 Created + +### Notes +- `enforce_deposits` is a field in the admin booking creation request +- Default is `true` (enforce) + +--- + +## Task 7: Guest User Creation Edge Cases + +**File:** `backend/handlers/bookings/bookings_test.go` (add to existing) OR new file +**What to test:** Validation edge cases for `POST /api/users/guest` +**Why:** Only success, duplicate email, and registered collision are tested. + +### Test Cases + +```go +TestGuestUser_Create_InvalidPhone +``` +- POST with `phone: "not-a-phone"` +- Assert 400 + +```go +TestGuestUser_Create_EmptyFirstName +``` +- POST with `firstName: ""` +- Assert 400 + +```go +TestGuestUser_Create_NameTooLong +``` +- POST with `firstName: strings.Repeat("a", 51)` +- Assert 400 + +```go +TestGuestUser_Create_InvalidEmail +``` +- POST with `email: "not-an-email"` +- Assert 400 + +### Notes +- Handler is in `backend/handlers/user/guest.go` +- Validation: first/last name 1-50 chars, email format, UK phone format +- Phone normalization strips non-digit/+ chars + +--- + +## Task 8: GetTimeBlockersInRange Excludes Reservations + +**File:** `backend/handlers/scheduling/time_blockers_test.go` (add to existing) +**What to test:** `GetTimeBlockersInRange` does NOT return `RESERVATION:%` entries. +**Why:** We added `AND description NOT LIKE 'RESERVATION:%'` to prevent self-blocking. This needs explicit coverage. + +### Test Cases + +```go +TestGetTimeBlockersInRange_ExcludesReservations +``` +- Insert a regular blocker ("Staff meeting") at 10:00 +- Insert a reservation ("RESERVATION:user:abc:123") at 11:00 +- Call `GetTimeBlockersInRange(ctx, start, end)` covering both +- Assert result contains only "Staff meeting", not the reservation + +### Notes +- Function is in `backend/handlers/scheduling/time-blockers.go` line 198 +- Query has `AND description NOT LIKE 'RESERVATION:%'` + +--- + +## Task 9: AnonymizeStaleGuestAccounts Edge Cases + +**File:** `backend/handlers/scheduling/time_blockers_test.go` (add to existing) +**What to test:** Boundary conditions for guest anonymization. +**Why:** Only basic "7 months old gets anonymized" is tested. + +### Test Cases + +```go +TestAnonymizeStaleGuestAccounts_Exactly6Months +``` +- Create guest with booking start_time = exactly 6 months ago +- Run `AnonymizeStaleGuestAccounts()` +- Assert guest IS anonymized (start_time + 6 months = now) + +```go +TestAnonymizeStaleGuestAccounts_ActiveBooking_Skipped +``` +- Create guest with past booking (7 months ago) AND active booking (tomorrow) +- Run cleanup +- Assert guest NOT anonymized (has active booking) + +```go +TestAnonymizeStaleGuestAccounts_NoBookings +``` +- Create guest with NO bookings +- Run cleanup +- Assert guest NOT anonymized (no booking to measure from) + +### Notes +- Function is in `backend/handlers/scheduling/time-blockers.go` +- Anonymizes 6 months after booking's `start_time`, not `created_at` +- Skips guests with active or pending bookings + +--- + +## Task 10: Admin Walk-In with Guest User + +**File:** `backend/handlers/admin/bookings_test.go` (add to existing) +**What to test:** `POST /api/admin/bookings` with a guest user ID (walk-in flow) +**Why:** Walk-in can create bookings for guest accounts. + +### Test Cases + +```go +TestAdminBookings_Create_WalkInGuestUser +``` +- Create admin + create guest user via fixtures +- POST admin booking with `user_id: guestID` +- Assert 201 +- Verify booking created with correct user + +### Notes +- Use `fixtures.CreateTestGuestUser(pool)` to get a guest user ID +- The admin booking endpoint is `POST /api/admin/bookings` + +--- + +## Task 11: Patch Test Recording Endpoint + +**File:** `backend/handlers/admin/users_test.go` (add to existing) +**What to test:** `POST /api/admin/users/{id}/patch-tests` +**Why:** Admin can record patch test completion for walk-in customers. No tests found. + +### Test Cases + +```go +TestAdminUsers_RecordPatchTest +``` +- Create admin + regular user +- POST patch test record for user with service requiring patch test +- Assert 201 or 200 +- Query `user_patch_tests` table, verify row exists + +```go +TestAdminUsers_RecordPatchTest_AlreadyExists +``` +- Record patch test once +- Record again for same user/service +- Assert appropriate behavior (update or reject duplicate) + +### Notes +- Check actual handler behavior for duplicate handling +- Endpoint: `POST /api/admin/users/{id}/patch-tests` + +--- + +## Task 12: Notification Acknowledgment Edge Cases + +**File:** `backend/handlers/notifications/notifications_test.go` (add to existing) +**What to test:** Acknowledging already-acknowledged or non-existent notifications. +**Why:** Partial coverage exists, edge cases may not be covered. + +### Test Cases + +```go +TestNotifications_Acknowledge_AlreadyAcknowledged +``` +- Create notification, acknowledge it +- Acknowledge again +- Assert appropriate response (200 or 409) + +```go +TestNotifications_Acknowledge_NonExistent +``` +- Acknowledge notification ID that doesn't exist +- Assert 404 + +### Notes +- The existing tests already cover some of this — verify before writing + +--- + +## Task 13: Email Verification Code Flow + +**File:** `backend/handlers/auth/auth_test.go` (add to existing) +**What to test:** `POST /api/verify/generate` and `POST /api/verify/check` +**Why:** Endpoints exist but no tests for code expiry, reuse, or invalid code. + +### Test Cases + +```go +TestVerifyGenerate_CodeExpires +``` +- Generate code +- Wait (or manipulate DB `created_at` to be 25 hours ago) +- Try to verify with expired code +- Assert failure + +```go +TestVerifyCheck_InvalidCode +``` +- POST verify with wrong code +- Assert 400 or 401 + +```go +TestVerifyCheck_ReuseCode +``` +- Generate code, verify successfully +- Try to verify same code again +- Assert failure (code should be consumed) + +### Notes +- Check actual expiry time in SQL (likely 24 hours) +- Codes may be single-use or multi-use — verify behavior + +--- + +## Task 14: Password Reset Flow + +**File:** `backend/handlers/auth/auth_test.go` (add to existing) +**What to test:** Password reset token generation and validation. +**Why:** Backend endpoints exist but no tests found. + +### Test Cases + +```go +TestPasswordReset_GenerateCode +``` +- POST generate for existing user +- Assert 200 +- Verify code exists in `verification_codes` table + +```go +TestPasswordReset_InvalidCode +``` +- POST check with wrong code +- Assert failure + +```go +TestPasswordReset_ExpiredCode +``` +- Generate code, expire it (manipulate DB) +- Try to verify +- Assert failure + +--- + +## Task 15: Contact Info Endpoint + +**File:** `backend/handlers/services/services_test.go` or new `contact_test.go` +**What to test:** `GET /api/contact` +**Why:** Simple endpoint, zero tests. + +### Test Cases + +```go +TestContact_ReturnsInfo +``` +- Create admin user with profile data +- Call `GET /api/contact` +- Assert 200 with admin's business info + +```go +TestContact_NoAdmin +``` +- Delete all admin users +- Call endpoint +- Assert 404 or empty response + +### Notes +- Returns info from the FIRST admin user in the system +- Endpoint is `GET /api/contact` (public, no auth) + +--- + +## Task 16: Portfolio Image EXIF Stripping + +**File:** `backend/handlers/portfolio/images_test.go` (add to existing) +**What to test:** Uploaded images have EXIF/GPS data stripped. +**Why:** Security feature exists but untested. + +### Test Cases + +```go +TestPortfolio_Upload_EXIFStripped +``` +- Create a test image WITH EXIF GPS data embedded +- Upload via `POST /api/portfolio/images` +- Download the image +- Parse EXIF, assert no GPS coordinates present + +### Notes +- This may require creating a test image with EXIF data +- The `imaging` library is used for processing +- This is a more complex test — may need helper to generate test image + +--- + +## Execution Order + +1. **Task 1** (Admin Reserve) — highest priority, most complex, recently changed +2. **Task 2** (Cleanup TTL) — small, recently changed +3. **Task 3** (Health Check) — small, new endpoint +4. **Task 8** (Reservation exclusion) — small, recently changed +5. **Tasks 4-6** (Deposit logic) — medium, business critical +6. **Tasks 7, 9-11** (Guest + Patch Test + Walk-in) — medium +7. **Tasks 12-15** (Edge cases) — low priority, smaller +8. **Task 16** (EXIF) — lowest, complex + +--- + +## Success Criteria + +- All new tests pass (`go test -tags test ./...`) +- No regressions in existing tests +- Code coverage report shows improvement in handlers/bookings and handlers/scheduling +- Tests follow existing patterns (fixtures, jwt, setupTest, cleanup) + +--- + +## References + +- `backend/handlers/bookings/admin_reserve.go` — handler to test +- `backend/handlers/bookings/reserve_test.go` — pattern for reservation tests +- `backend/handlers/scheduling/time_blockers_test.go` — pattern for cleanup tests +- `backend/handlers/admin/bookings_test.go` — pattern for admin booking tests +- `backend/handlers/bookings/bookings_test.go` — pattern for deposit/no-show tests +- `backend/testutils/fixtures/fixtures.go` — available fixture functions +- `backend/testutils/jwt/jwt.go` — token generation +- `init-scripts/init-script.sql` — SQL functions/triggers