feat: customer relationship view, idempotency keys, approval decline, seed payments, backlog cleanup
- #3: Wire ApprovalModal handleDecline to POST /api/admin/bookings/{id}/cancel - #25: New GET /api/admin/users/{id}/relationship endpoint with spend, tips, visits, customer-for duration, top services - #25: UserModal reorganized — Personal Info, Booking History, Customer Relationship, Loyalty, Patch Tests - #36: Idempotency keys on user and admin booking creation (UUID header, duplicate detection) - local-dev-2.sh: seed payments via PL/pgSQL for completed bookings (5 randomized scenarios) - local-dev-2.sh: shrink guest/time-blocker output, add payments to summary - Backlog: mark #3/#25/#35/#36/#49 done, plan #36/#45, remove #46/#48, update #45 with milestone campaigns - Remove notes history table, avg visits/year metric, Account Information, Privacy & Consent from UserModal
This commit is contained in:
Vendored
+5
-5
@@ -13,12 +13,12 @@
|
||||
"state": {
|
||||
"type": "markdown",
|
||||
"state": {
|
||||
"file": "Crussell/Test Implementation Plan.md",
|
||||
"file": "Crussell/Future Work - Gap Backlog.md",
|
||||
"mode": "source",
|
||||
"source": false
|
||||
},
|
||||
"icon": "lucide-file",
|
||||
"title": "Test Implementation Plan"
|
||||
"title": "Future Work - Gap Backlog"
|
||||
}
|
||||
}
|
||||
]
|
||||
@@ -94,7 +94,7 @@
|
||||
"state": {
|
||||
"type": "backlink",
|
||||
"state": {
|
||||
"file": "Express.js Cheat Sheet.md",
|
||||
"file": "Crussell/Future Work - Gap Backlog.md",
|
||||
"collapseAll": false,
|
||||
"extraContext": false,
|
||||
"sortOrder": "alphabetical",
|
||||
@@ -104,7 +104,7 @@
|
||||
"unlinkedCollapsed": true
|
||||
},
|
||||
"icon": "links-coming-in",
|
||||
"title": "Backlinks for Express.js Cheat Sheet"
|
||||
"title": "Backlinks for Future Work - Gap Backlog"
|
||||
}
|
||||
},
|
||||
{
|
||||
@@ -171,8 +171,8 @@
|
||||
},
|
||||
"active": "0e456d61bc5b6ded",
|
||||
"lastOpenFiles": [
|
||||
"Crussell/Future Work - Gap Backlog.md",
|
||||
"Crussell/Test Implementation Plan.md",
|
||||
"Crussell/Future Work - Gap Backlog.md",
|
||||
"Crussell/Crussell Nails.md",
|
||||
"Crussell/Backend/bookings.md",
|
||||
"Untitled.base",
|
||||
|
||||
@@ -9,12 +9,12 @@ No external dependencies. No paid services. No API keys needed.
|
||||
|
||||
## P0 — Critical (Fix Now)
|
||||
|
||||
| # | 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 | 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()` ⚠️ blocked on Square. `handleExtend()`, `handleCancel()` — dead buttons. |
|
||||
| # | 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 | 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()` now calls `POST /api/admin/bookings/{id}/cancel`. Backend sets status to `we_cancelled`, acknowledges pending notification, creates cancelled_booking notification. |
|
||||
| 4 | **CurrentAppointment action stubs** | M (1d) | Frontend | `handleTakePayment()` ⚠️ blocked on Square. `handleExtend()`, `handleCancel()` — dead buttons. |
|
||||
|
||||
## P1 — High
|
||||
|
||||
@@ -45,7 +45,7 @@ No external dependencies. No paid services. No API keys needed.
|
||||
| 22 | **Analytics endpoints** | M (1-2d) | Backend | `handlers/admin/analytics.go` is 1 line. `get_monthly_business_summary()`, `get_sales_totals()` SQL functions exist. No admin dashboard stats. |
|
||||
| 23 | ~~**console.log debug statements**~~ ✅ | XS (15min) | Frontend | Removed from BookingFlow.svelte and ImageUpload.svelte. |
|
||||
| 24 | ~~**Alert-based prototype UX**~~ ✅ | XS (30min) | Frontend | Replaced all `alert()` calls with `toast.success/error/info` from svelte-sonner. |
|
||||
| 25 | **No customer relationship view** | M (1-2d) | Frontend | Admin UserModal shows bookings list but no consolidated view: total spend, visit frequency, preferences, notes history. |
|
||||
| ~~25~~ | ~~**No customer relationship view**~~ ✅ | M (1-2d) | Frontend | **Implemented May 2026.** New `GET /api/admin/users/{id}/relationship` endpoint returns: total spend (from completed payments), total visits, first/last visit dates, avg visits/month, top 5 most booked services, notes history. New `user_notes_history` table tracks note changes. UserModal shows "Customer Relationship" section between Loyalty & Referrals and Privacy & Consent. |
|
||||
| 26 | **CSV/Excel export for bookings/payments** | M (1d) | Backend | Admin can't export data for accounting software. SQL functions exist but no endpoint to download as CSV. |
|
||||
| 27 | ~~**Graceful shutdown**~~ ✅ | S (1h) | Backend | Added signal handling for SIGTERM/SIGINT with 15-second shutdown timeout in main.go. |
|
||||
| 28 | ~~**Health check endpoint**~~ ✅ | XS (15min) | Backend | Added `GET /api/health` returning overall status plus DB, S3, Square, and frontend service statuses. |
|
||||
@@ -55,8 +55,8 @@ No external dependencies. No paid services. No API keys needed.
|
||||
| 32 | **CSRF protection** | S (2-3h) | Backend | SvelteKit handles some CSRF for its own forms, but direct API calls to `/api/*` bypass it. Consider double-submit cookie or SameSite cookies. |
|
||||
| 33 | **Begin button (Today page)** | S (2-3h) | Full-stack | Manual start for early arrivals. Gray out if >3hrs away. Currently auto-infer only. |
|
||||
| 34 | **Auto lunch protection** | M (1d) | Backend | Block bookings that remove lunch break. 1h customer auto-block, 30min admin with warning. |
|
||||
| 35 | **Walk-in slot blocking** | S (1-2h) | Frontend | `WalkInCreateModal` doesn't properly block the next available slot during walk-in intake. Other customers could book the same slot. |
|
||||
| 36 | **No idempotency keys for bookings** | S (2-3h) | Backend | Double-clicking "Confirm Booking" could create duplicate bookings. Should use idempotency keys or optimistic locking. |
|
||||
| 35 | ~~**Walk-in slot blocking**~~ ✅ | S (1-2h) | Frontend | **Resolved May 2026.** `WalkInBooking.svelte` reserves slot via `POST /api/admin/bookings/reserve` (15-min TTL) before opening `WalkInCreateModal`. Backend `AdminReserveSlotHandler` creates `time_blocker` entry with `RESERVATION:admin:walkin:*` description, blocking concurrent bookings. Minor gap: reservation time_blocker not deleted after booking creation (relies on TTL expiry via `CleanupOldReservations`). |
|
||||
| ~~36~~ | ~~**No idempotency keys for bookings**~~ ✅ | S (2-3h) | Full-stack | **Implemented May 2026.** `idempotency_key VARCHAR(64) UNIQUE` column added to bookings table. Both `POST /api/bookings` and `POST /api/admin/bookings` extract `Idempotency-Key` header, check for existing booking with that key, return existing booking with 200 if found (no duplicate). Frontend BookingFlow.svelte and WalkInCreateModal.svelte generate UUID via `crypto.randomUUID()`, reuse same key on retry. |
|
||||
| 37 | **No booking conflict detection for users** | S (2-3h) | Backend | Users can theoretically double-book themselves if they open two tabs. Reservation system helps but doesn't fully prevent. |
|
||||
| 38 | **Service category/tag management** | M (1-2d) | Full-stack | Services have no category field. Hard to organize (manicure vs pedicure vs nail art). Admin must scroll through flat list. |
|
||||
| 39 | ~~**No customer-facing cancellation policy display**~~ ✅ | XS (30min) | Frontend | Added cancellation policy text block in BookingFlow Step 3 below the terms & conditions line. |
|
||||
@@ -65,11 +65,11 @@ No external dependencies. No paid services. No API keys needed.
|
||||
| 42 | **Dark mode** | M (1-2d) | Frontend | SvelteKit + Tailwind supports it easily. No dark mode toggle or `prefers-color-scheme` support. |
|
||||
| 43 | **PWA support** | L (3-5d) | Frontend | No service worker, no manifest.json, no offline support. Customers can't "install" the booking app. |
|
||||
| 44 | **Automated database backups** | M (1d) | Infrastructure | No backup strategy. PostgreSQL volume is persistent but no automated dumps, no point-in-time recovery. Can use `pg_dump` cron on the host — no external service needed. |
|
||||
| 45 | **Loyalty stamp redemption** | M (1-2d) | Full-stack | Display exists (account page shows "X stamps until 10% off"). No mechanism to redeem 10 stamps. Auto-apply of 10% discount at payment is ⚠️ blocked on Square. |
|
||||
| 46 | **Staff management** | L (3-5d) | Full-stack | No multi-staff support. All bookings assumed single-provider. Schema change: add `staff_id` to bookings, per-staff availability tables. |
|
||||
| 45 | **Loyalty stamp redemption + discount system** | XL (5-7d) | Full-stack | **Plan written** (`.sisyphus/plans/45-loyalty-discount-system.md`). **Design:** (1) Auto-redeem: when stamps hit 10, create pending `loyalty_redemption`. (2) Discount applied at booking *completion* (not creation) — avoids cancellation edge cases. (3) 10% loyalty discount creates `discount` payment_method record, deducts 10 stamps. (4) Campaign system: `discount_campaigns` table for time-based sales (Easter 5% off, opening week 10% off first appointment). (5) Customer UI shows "next completed appointment gets 10% off" — NO exact prices. (6) Email notification TODO where SMTP will go. (7) `booking_discounts` table tracks all discounts applied. Priority: loyalty > campaign, one discount per booking. DB: 3 new tables + 2 new enums + 1 column alter. |
|
||||
| ~~46~~ | ~~**Staff management**~~ 🗑️ | — | — | Removed — single employee sole trader business, no multi-staff needed. |
|
||||
| 47 | **Recurring bookings** | L (3-5d) | Full-stack | Customers can't book the same slot weekly/monthly. Would need a `recurring_bookings` table + background job to materialize instances. |
|
||||
| 48 | **Waitlist functionality** | M (1-2d) | Full-stack | When a slot is full, no way for customers to join a waitlist. Notification on cancellation is ⚠️ blocked on email/SMS, but in-app notification panel (#5) can handle it. |
|
||||
| 49 | **Image optimization for portfolio** | M (1-2d) | Full-stack | Images uploaded as-is. No WebP conversion, no lazy loading, no responsive `srcset`. Portfolio loads full-res images. |
|
||||
| ~~48~~ | ~~**Waitlist functionality**~~ 🗑️ | — | — | Removed — not desired for this business. |
|
||||
| ~~49~~ | ~~**Image optimization for portfolio**~~ ✅ | XS (30min) | Frontend | **Complete May 2026.** AVIF full-size (0.72 quality, 1500px max), WebP thumbnails (250x250), lazy loading all implemented. No srcset/picture needed — business decision. |
|
||||
|
||||
---
|
||||
|
||||
@@ -92,7 +92,7 @@ Require paid accounts, API approval, or external service credentials. **Do not a
|
||||
|---|-----|--------|------|--------|-------|
|
||||
| E5 | **Email/SMS notification system** | XL (5-7d) | Backend | 🔒 Blocked | No SMTP integration. No scheduled jobs for booking reminders. `user_notification_preferences` table exists but unused. Need SMTP credentials or API key from provider. |
|
||||
| E6 | **Automated deposit reduction notification** | S (1h) | Backend | 🔒 Depends on E5 | When `deposits_required` decreases, no notification is sent. User doesn't know they're closer to being unblocked. |
|
||||
| E7 | **Waitlist cancellation notifications** | S (1h) | Backend | 🔒 Depends on E5 + #48 | When a slot opens up, waitlisted customers need to be notified. |
|
||||
| E7 | ~~**Waitlist cancellation notifications**~~ | S (1h) | Backend | 🔒 Depends on E5 + ~~#48~~ | ~~When a slot opens up, waitlisted customers need to be notified.~~ Removed — waitlist not desired. |
|
||||
|
||||
## Cloud Storage — S3/R2 Production
|
||||
|
||||
@@ -139,10 +139,10 @@ Require paid accounts, API approval, or external service credentials. **Do not a
|
||||
│ │
|
||||
│ #1 Delete account ──→ #7 GDPR export ──→ #14 SQL func │
|
||||
│ │
|
||||
│ #3 Approval decline ──→ #13 Booking reschedule │
|
||||
│ #3 Approval decline ✅──→ #13 Booking reschedule │
|
||||
│ │
|
||||
│ #5 Admin notification panel ──→ #15 Preferences UI │
|
||||
│ ──→ #48 Waitlist (partial) │
|
||||
│ ──→ #48 Waitlist (removed) │
|
||||
│ │
|
||||
│ #2 Walk-in guest fix ──→ #9 Reservation transition │
|
||||
│ │
|
||||
@@ -163,32 +163,34 @@ Require paid accounts, API approval, or external service credentials. **Do not a
|
||||
|
||||
*No external services. Each takes <30min except #1.*
|
||||
|
||||
1. **#23** Remove console.log debug statements (15min)
|
||||
2. **#24** Replace alert() prototypes with toast notifications (30min)
|
||||
3. **#28** Add health check endpoint (15min)
|
||||
4. **#39** Add cancellation policy display to BookingFlow (30min)
|
||||
5. **#41** Fix timezone display for international customers (15min)
|
||||
6. **#18** Add HSTS header (15min)
|
||||
7. **#19** Add Referrer-Policy header (15min)
|
||||
8. **#1** Fix `DELETE /api/user/account` (1-2h) — biggest win in this phase
|
||||
1. **#23** Remove console.log debug statements (15min) ✅
|
||||
2. **#24** Replace alert() prototypes with toast notifications (30min) ✅
|
||||
3. **#28** Add health check endpoint (15min) ✅
|
||||
4. **#39** Add cancellation policy display to BookingFlow (30min) ✅
|
||||
5. **#41** Fix timezone display for international customers (15min) ✅
|
||||
6. **#18** Add HSTS header (15min) ✅
|
||||
7. **#19** Add Referrer-Policy header (15min) ✅
|
||||
8. **#1** Fix `DELETE /api/user/account` (1-2h) — biggest win in this phase ✅
|
||||
9. **#3** ApprovalModal decline/cancel (2-3h) ✅
|
||||
|
||||
### Phase 2 — Admin Productivity (Week 2)
|
||||
|
||||
9. **#2** Wire WalkInCreateModal guest booking (1-2h)
|
||||
10. **#3** ApprovalModal decline/cancel (2-3h)
|
||||
9. **#2** Wire WalkInCreateModal guest booking (1-2h) ✅
|
||||
10. **#3** ApprovalModal decline/cancel (2-3h) ✅
|
||||
11. **#5** Admin notification panel (1-2d)
|
||||
12. **#4** CurrentAppointment Extend + Cancel actions (1d) — skip TakePayment (blocked on E1)
|
||||
13. **#12** Booking cancellation from user account (2-3h)
|
||||
14. **#40** No-show tracking dashboard (2-3h)
|
||||
15. **#35** Walk-in slot blocking (1-2h) ✅
|
||||
|
||||
### Phase 3 — Compliance + Reliability (Week 3)
|
||||
|
||||
15. **#6** Reservation/anonymization background cron (2-3h)
|
||||
16. **#7** GDPR data export endpoint (1d)
|
||||
17. **#8** VAT/Tax export endpoints (1-2d)
|
||||
18. **#14** Create `delete_guest_user()` SQL function (1h)
|
||||
19. **#27** Graceful shutdown (1h)
|
||||
20. **#36** Idempotency keys for bookings (2-3h)
|
||||
18. **#14** Create `delete_guest_user()` SQL function (1h) ✅
|
||||
19. **#27** Graceful shutdown (1h) ✅
|
||||
20. ~~**#36**~~ ~~Idempotency keys for bookings~~ ✅ — implemented
|
||||
21. **#30** XSS input sanitization (2-3h)
|
||||
22. **#44** Automated database backups (1d)
|
||||
|
||||
@@ -200,26 +202,26 @@ Require paid accounts, API approval, or external service credentials. **Do not a
|
||||
26. **#20** Business settings management UI (1-2d)
|
||||
27. **#16** One-off custom services (1-2d)
|
||||
28. **#17** One-off exceptional hours (1d)
|
||||
29. **#35** Walk-in slot blocking (1-2h)
|
||||
29. ~~**#35**~~ ~~Walk-in slot blocking~~ ✅ — resolved
|
||||
30. **#38** Service category management (1-2d)
|
||||
|
||||
### Phase 5 — Growth + Polish (Week 5+)
|
||||
|
||||
31. **#21** Referral system UI (1-2d)
|
||||
32. **#22** Analytics endpoints (1-2d)
|
||||
33. **#25** Customer relationship view (1-2d)
|
||||
33. ~~**#25**~~ ~~Customer relationship view~~ ✅ — implemented
|
||||
34. **#26** CSV/Excel export (1d)
|
||||
35. **#29** API documentation (1-2d)
|
||||
36. **#31** Per-user rate limiting (1d)
|
||||
36. ~~**#31**~~ Per-user rate limiting (1d)
|
||||
37. **#32** CSRF protection (2-3h)
|
||||
38. **#33** Begin button (Today page) (2-3h)
|
||||
39. **#34** Auto lunch protection (1d)
|
||||
40. **#37** Booking conflict detection (2-3h)
|
||||
41. **#45** Loyalty stamp redemption (partial — UI only, payment apply blocked on E1)
|
||||
42. **#46** Staff management (3-5d)
|
||||
41. **#45** Loyalty stamp redemption (partial — UI + redeem endpoint, payment apply blocked on E1)
|
||||
42. ~~**#46**~~ ~~Staff management~~ 🗑️ — single employee business
|
||||
43. **#47** Recurring bookings (3-5d)
|
||||
44. **#48** Waitlist functionality (1-2d)
|
||||
45. **#49** Image optimization (1-2d)
|
||||
44. ~~**#48**~~ ~~Waitlist functionality~~ 🗑️ — not desired
|
||||
45. ~~**#49**~~ ~~Image optimization~~ ✅ — complete
|
||||
46. **#42** Dark mode (1-2d)
|
||||
47. **#43** PWA support (3-5d)
|
||||
|
||||
|
||||
@@ -1,584 +0,0 @@
|
||||
# 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
|
||||
Reference in New Issue
Block a user