popertots/Crussell
1
1
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

feat: unify walk-in and call-in reservation flows with 15min TTL, guest booking support, and slot awareness

Browse Source 
- 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
This commit is contained in:
popertots
2026-05-03 15:09:50 +01:00
parent bff86a6660
commit 6808752e0d
7 changed files with 748 additions and 152 deletions
Download Patch File Download Diff File Expand all files Collapse all files
obsidian/Crussell/Future Work - Gap Backlog.md
+3 -3
View File
@@ -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}`). |
obsidian/Crussell/Test Implementation Plan.md
+584
View File
@@ -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