From 96ec3f3e339e7ef5756c1bb9dd1c1d5ff704fdb4 Mon Sep 17 00:00:00 2001 From: Stephen Adamson Date: Thu, 18 Jun 2026 16:27:22 +0100 Subject: [PATCH] docs: update README and obsidian documentation Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus --- README.md | 8 +- obsidian/.obsidian/workspace.json | 6 +- obsidian/Crussell/Admin Manual.md | 61 ++- .../Crussell/Future Work - Gap Backlog.md | 5 +- .../Loyalty & Discount System Reference.md | 304 +++++++---- obsidian/Crussell/Overview.md | 10 +- obsidian/Crussell/Technical Manual.md | 431 ++++++++++++++- .../Testing Architecture & DB Management.md | 514 +++++++++--------- obsidian/Crussell/User Manual.md | 79 ++- 9 files changed, 972 insertions(+), 446 deletions(-) diff --git a/README.md b/README.md index 3f081af..063dfd3 100644 --- a/README.md +++ b/README.md @@ -6,7 +6,7 @@ Nail salon booking platform — Go 1.25 backend + SvelteKit 5 SPA + PostgreSQL 1 **Booking**: Self-service (customer), walk-in (admin), call-in (admin). Slot reservations prevent double-booking (4 TTL types). Guest accounts with GDPR-compliant anonymization. Service eligibility based on age + patch test validity. -**Payments**: Square Terminal (in-person) + Web Payments SDK (online). Cash with change calculation. Gift cards (12-digit code or account balance). Saved cards for faster checkout. Tips on completed bookings. Refunds. All payment types: deposit, full, partial, balance, tip. +**Payments**: Square Terminal (in-person) + Web Payments SDK (online). Cash with change calculation. Gift cards (12-digit code or account balance). Saved cards for faster checkout. Tips on completed bookings. Refunds with notice-period tiers and deposit protection (72h/24h thresholds). All payment types: deposit, full, partial, balance, tip. Payment >20% of total promotes `pending_release` bookings back to `confirmed`. Deposit paid is computed from payments on-the-fly. The first 50% of each payment is always carved out as deposit (via `buildSplitRecords`); any overflow beyond the booking total becomes a tip. A PostgreSQL `pg_advisory_lock` serializes payment attempts per-booking to prevent two-tab double-payment races. **Gift Cards**: Multi-method purchase (cash, card machine, online card, giveaway). Inventory cards for stock management. 24-month rolling expiry. Idle account cleanup (2yr/5yr thresholds). Expired balance recovery with admin audit trail. Transaction audit log. Idempotency keys for purchases. @@ -16,11 +16,11 @@ Nail salon booking platform — Go 1.25 backend + SvelteKit 5 SPA + PostgreSQL 1 **Admin**: Today page with interactive calendar grid. Booking management (create, edit, reschedule, approve, cancel). User management with customer relationship data (spend, visits, top services). Custom services (one-off services with create/edit/promote/delete). Discount campaigns (time-based and milestone). Time blocker CRUD. Portfolio image upload with tag management. Gift card management. Business settings (VAT, gift card config). Notification queue with priority ordering. -**Loyalty & Discounts**: 1 stamp per paid appointment (max 1/day). 10 stamps → 10% off next visit. Campaigns: time-based, per-user milestone, global milestone, anniversary. All discounts stack additively against original total. +**Loyalty & Discounts**: 1 stamp per paid appointment (max 1/day). 10 stamps → 10% off via opt-in checkbox at payment or till. Stamps refunded on cancellation. Campaigns auto-apply at both payment and completion: time-based, per-user milestone, global milestone (in-person only), anniversary. All discounts stack additively against original total. Discount payment records excluded from refund calculations. **Compliance**: GDPR Article 15 data export (async, 12h cache, 16-section JSON + PDF). Account deletion with external system scrubbing (S3, Square). Guest PII anonymized 6 months post-appointment. UK financial data retention (7 years). Gift card SPV/MPV VAT treatment configurable. -**Frontend**: Portfolio gallery with tag/category filtering, multi-format images (AVIF/WebP/JPEG/JXL with WASM client-side encoding). MapLibre GL map on contact page. PhoneInput component with UK validation. CharCounter for long notes. +**Frontend**: Portfolio gallery with fuzzy tag search (relevance-sorted) and exact category filters (date-sorted), multi-format images (AVIF/WebP/JPEG/JXL with WASM client-side encoding), cursor-based pagination. MapLibre GL map on contact page. PhoneInput component with UK validation. CharCounter for long notes. **Infrastructure**: Docker Compose (postgres, backend, sabredav, nginx). Dev mock for Square payments (`//go:build dev`). RustFS dev storage, Cloudflare R2 for prod. SabreDAV CardDAV sync for profile photos. @@ -74,7 +74,7 @@ Default logins (password: `password`): ```bash cd backend && go build -o bin/backend ./main.go cd frontend && npm ci && npm run build -cd backend && go test -tags "test,dev" -p 1 ./... # 682/685 passing, 3 skipped +cd backend && go test -tags "test,dev" -p 1 ./... # 803/806 passing, 3 skipped ``` ## Full Documentation diff --git a/obsidian/.obsidian/workspace.json b/obsidian/.obsidian/workspace.json index d730413..24e98c8 100644 --- a/obsidian/.obsidian/workspace.json +++ b/obsidian/.obsidian/workspace.json @@ -13,12 +13,12 @@ "state": { "type": "markdown", "state": { - "file": "Crussell/Future Work - Gap Backlog.md", + "file": "Crussell/User Manual.md", "mode": "source", "source": false }, "icon": "lucide-file", - "title": "Future Work - Gap Backlog" + "title": "User Manual" } } ] @@ -171,8 +171,8 @@ }, "active": "6776d739ee18449c", "lastOpenFiles": [ - "Crussell/Technical Manual.md", "Crussell/Future Work - Gap Backlog.md", + "Crussell/Technical Manual.md", "Crussell/Loyalty & Discount System Reference.md", "Crussell/Overview.md", "Crussell/User Manual.md", diff --git a/obsidian/Crussell/Admin Manual.md b/obsidian/Crussell/Admin Manual.md index 9feb1d8..ad34f6a 100644 --- a/obsidian/Crussell/Admin Manual.md +++ b/obsidian/Crussell/Admin Manual.md @@ -287,7 +287,14 @@ You can reschedule an appointment from the Today page or from the Booking Detail 2. A window opens showing available time slots 3. The system checks for conflicts with existing bookings 4. Pick a new date and time -5. Click **Confirm** — the appointment is moved to the new slot +5. If the appointment is close to the start time (within 72h with payments or 24h without), the system shows a warning. You can optionally check: + - **Forgive fees** — overrides deposit protection for the reschedule + - **Forgive no-show** — waives the no-show record that would otherwise apply +6. Click **Confirm** — the appointment is moved to the new slot + +### Refund Calculation on Reschedule + +If you reschedule a booking with payments, the system doesn't process a refund — the payments move with the booking to the new time. The forgiveness checkboxes are for admin discretion when handling short-notice changes. ### What the Customer Sees @@ -475,7 +482,13 @@ Every booking has a status that tells you where it is in its lifecycle. ### The Status Flow ``` -Pending → Confirmed → In Progress → Completed +Pending ─→ Confirmed ─→ In Progress ─→ Completed + │ │ + │ └──(deposit deadline passed, unpaid)──→ Pending Release + │ │ + └──(deposit deadline passed, unpaid)──→ Pending Release──(slot claimed by another booking)──→ Deposit Lapsed + │ + (payment received) ──→ Confirmed ``` A booking can also branch off to: @@ -483,13 +496,14 @@ A booking can also branch off to: - **We Cancelled** — the salon cancelled it - **No Show** — the customer didn't show up - **Re-Schedule** — the customer requested a reschedule (pending your approval) -- **No Deposit** — the customer didn't pay the required deposit +- **Pending Release** — the deposit deadline has passed without payment. The slot is vulnerable — another booking can claim it. If the customer pays before the slot is claimed, the booking returns to Confirmed. +- **Deposit Lapsed** — the slot was claimed by another booking while the customer hadn't paid their deposit. The original booking is cancelled and the deposit is forfeited. ### How Statuses Change **Pending** — the customer has submitted the booking but it hasn't been confirmed yet. Pending bookings need approval (if they have notes or are for today) or are auto-confirmed (if no notes and not today). -**Confirmed** — the booking is locked in. The customer has a confirmed slot. +**Confirmed** — the booking is locked in. The customer has a confirmed slot. They may need to pay a deposit before the 24-hour deadline. **In Progress** — the appointment has started. The system automatically updates this based on the current time. You can also manually set it by clicking the appointment on the Today page. @@ -497,20 +511,38 @@ A booking can also branch off to: **Client Cancelled** — the customer cancelled the booking. If they cancelled with more than 24 hours' notice, no penalty. If less than 24 hours, they get 3 deposit obligations. -**We Cancelled** — the salon cancelled the booking. Used when you decline a booking or cancel it for operational reasons. +**We Cancelled** — the salon cancelled the booking. Used when you decline a booking or cancel it for operational reasons. When you cancel from the admin panel, the system calculates any refund owed based on how much notice the customer gave and how much they paid. If the customer has paid a deposit, up to 50% may be retained depending on the notice period (see **Refunds on Cancellation** below). **No Show** — the customer didn't show up. This is used when a customer fails to appear without cancelling. It triggers the same deposit obligations as a late cancellation. **Re-Schedule** — the customer requested a change. The original booking is paused until you approve or deny the request. -**No Deposit** — the customer hasn't paid the required deposit. This status is used when a deposit is required but hasn't been received. +**Pending Release** — the customer hasn't paid the required deposit and the deadline (24h before start time) has passed. The booking is kept in a vulnerable state — another customer can book this slot, which would evict the original booking to **Deposit Lapsed**. If the customer pays before the slot is claimed, it goes back to **Confirmed**. + +**Deposit Lapsed** — the slot was taken by another booking. The original customer loses their slot. This only happens if the deposit deadline passed without payment. + +### Refunds on Cancellation + +When you cancel a booking (We Cancelled) or the customer cancels (Client Cancelled), the system calculates the refund based on the **notice period** (time between cancellation and the appointment start): + +| Notice Period | What Gets Refunded | +|---|---| +| 72+ hours before the appointment | **Full refund** — 100% of everything paid | +| 24-72 hours before the appointment | **Partial refund** — you keep a protected deposit (up to 50% of the total). Everything paid above that is refunded | +| Less than 24 hours | **No refund** — all payments are retained | + +**Override with forgiveness:** When cancelling from the admin panel, you can check: +- **Forgive fees** — refunds 100% regardless of notice period (overrides deposit protection) +- **Forgive no-show** — no no-show/deposit penalty recorded + +This is useful for genuine emergencies or when the salon caused the cancellation. ### Manual Status Changes You can change the status of a booking manually: - Click on the booking on the Today page - Click **Progress** to advance to the next status (Pending → Confirmed → In Progress → Completed) -- Click **Cancel** to cancel the booking +- Click **Cancel** to cancel the booking — a window shows the refund calculation with forgiveness options **Important:** Once a booking is marked as Completed, it cannot be cancelled. The customer has earned their stamp and any discounts have been applied. @@ -583,6 +615,21 @@ The password reset system exists on the backend but there's no "Forgot Password" Check the booking status. If they cancelled with less than 24 hours' notice and weren't forgiven, the status is "No Show" and they have 3 deposit obligations. If they cancelled with more than 24 hours' notice, the status should be "Client Cancelled". +### "A customer wants to know how much refund they'll get" + +The refund tiers are: +- **72+ hours notice:** Full refund of everything paid +- **24-72 hours notice:** Partial refund — the salon keeps a protected deposit (up to 50% of the total) +- **Less than 24 hours:** No refund + +If they cancelled from their Account page, the cancellation dialog shows the exact breakdown before they confirm. If you're cancelling as admin, the refund calculation is shown in the cancel dialog with the option to forgive fees. + +### "A customer says their slot was released even though they booked it" + +This happens when a booking has a deposit requirement and the customer didn't pay before the 24-hour deadline. The booking goes into **Pending Release** status. If another customer books the same slot, the original booking is evicted to **Deposit Lapsed** and the slot is given to the new customer. + +Check the booking history — if the status is "Deposit Lapsed", that's what happened. You can help them book a new slot. + ### "A customer has 3 deposit obligations but they say they paid all their deposits" Deposits are reduced by 1 each time a customer completes a paid appointment. If they have 3 deposits, they need to complete 3 appointments and pay for them to clear all 3. Paying a deposit doesn't reduce the count — completing an appointment does. diff --git a/obsidian/Crussell/Future Work - Gap Backlog.md b/obsidian/Crussell/Future Work - Gap Backlog.md index d6adecc..10bc9c9 100644 --- a/obsidian/Crussell/Future Work - Gap Backlog.md +++ b/obsidian/Crussell/Future Work - Gap Backlog.md @@ -21,9 +21,10 @@ These are blockers: missing functionality that prevents daily operations, legal | 6 | **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 `DELETE /api/bookings/{id}` exists. | | 7 | **Business settings management UI** | M (1-2d) | Frontend | `GET/PUT /api/admin/settings` backend endpoints exist (VAT, business name, gift card expiry months, voucher type SPV/MPV). No admin page. Staff must use `curl` or direct SQL. | | 8 | **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 CSV. | -| 9 | **XSS input sanitization** | S (2-3h) | Backend | Backend validates format (regex, length) but doesn't sanitize HTML entities. Stored XSS risk in `notes`, `name`, `description` fields. | +| 9 | **XSS input sanitization** | S (2-3h) | Backend | Backend validates format (regex, length) but doesn't sanitize HTML entities. Stored XSS risk in `notes`, `name`, `description` fields. **Partially addressed June 2026:** toast.error() messages now escape HTML. `{@html statusBadge()` replaced with proper component. ICS injection sanitized. Error messages no longer reflect user input. CSP header added. Remaining: backend-level HTML sanitization of stored fields. | | 10 | **CSRF protection** | S (2-3h) | Backend | SvelteKit handles CSRF for its own forms, but direct API calls to `/api/*` bypass it. Consider double-submit cookie or SameSite cookies. | | 11 | **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. | +| 12 | **Square webhook production verification** | S (2-3h) | Backend | Current handler skips signature check if `SQUARE_WEBHOOK_SIGNATURE_KEY` is empty. Production must ALWAYS verify. See `TODO(PROD)` in `handlers/webhooks/square.go` for detailed implementation guide including Go SDK usage, base64 HMAC-SHA256, and notification URL matching. | ### External MVP (Requires Third-Party Access) @@ -45,7 +46,7 @@ These improve the experience or add features, but the business can operate witho | 14 | **Referral system UI** | M (1-2d) | Full-stack | Backend complete — registration validates codes, relationships recorded. Users can't see their referral code or track uses. Admin can't manage referral campaigns. | | 15 | **Analytics endpoints** | M (1-2d) | Backend | `handlers/admin/analytics.go` is 1 line. `get_monthly_business_summary()`, `get_sales_totals()` SQL functions exist but not wired. | | 16 | **API documentation** | M (1-2d) | Backend | No OpenAPI/Swagger spec. No generated docs. New developers must read code. | -| 17 | **Per-user rate limiting** | M (1d) | Backend | Rate limiter is IP-based. Authenticated users could abuse from multiple IPs. Should track by user ID + IP. Pure Go — no Redis needed for single-instance. | +| 17 | **Per-user rate limiting** | M (1d) | Backend | Rate limiter is IP-based. Authenticated users could abuse from multiple IPs. Should track by user ID + IP. Pure Go — no Redis needed for single-instance. **Partially addressed June 2026:** L3 ProgressiveRateLimit added for login/register (per-IP dual-window: 30 req/5s burst + 120 req/60s sustained). Account lockout added (5+ failures → progressive 15min→2h). Remaining: user-ID tracking for authenticated endpoints. | | 18 | **Begin button (Today page)** | S (2-3h) | Full-stack | Manual start for early arrivals. Currently auto-inferred only. Gray out if >3 hours away. | | 19 | **Booking conflict detection for users** | S (2-3h) | Backend | Users can theoretically double-book themselves in two tabs. Reservation system helps but doesn't fully prevent. | | 20 | **Service category/tag management** | M (1-2d) | Full-stack | Services have no category field. Admin scrolls through a flat list. No way to group (manicure vs pedicure vs nail art). | diff --git a/obsidian/Crussell/Loyalty & Discount System Reference.md b/obsidian/Crussell/Loyalty & Discount System Reference.md index 97ea6a3..52c2fea 100644 --- a/obsidian/Crussell/Loyalty & Discount System Reference.md +++ b/obsidian/Crussell/Loyalty & Discount System Reference.md @@ -9,44 +9,79 @@ Complete reference for the loyalty and discount system, written for four audienc ### Loyalty Card Every time you complete a paid appointment, you earn **1 stamp** (max 1 per day — two appointments on the same day only count once). Free appointments (£0 total) don't earn stamps. -After **10 stamps**, you get a **10% discount** automatically applied to your next paid appointment. After that discount is used, your stamps reset and you start collecting again. +After **10 stamps**, you can use them for a **10% discount** on your next booking. You decide when to use them: -**You don't need to do anything.** The system tracks your stamps and applies the discount automatically when your appointment is completed. +- **When paying online**: a checkbox appears in the payment form: "Use my Loyalty Stamp Card — 10% off" +- **At the salon**: the staff will ask "Would you like to use your Loyalty Stamp Card?" +- After the discount is used, your stamps are consumed and you start collecting again. + +If you cancel a booking where you used your loyalty discount, the **10 stamps are refunded** back to your account. ### Campaign Discounts -The salon occasionally runs promotions — like "10% off this week" or "15% off your 5th visit." If a campaign is active and you qualify, the discount is applied automatically when your appointment is completed. +The salon runs promotions — like "10% off this week" or "15% off your 5th visit." If a campaign is active and you qualify, the discount is applied automatically whether you pay in advance or at completion. + +### Global Milestones +Occasional salon-wide celebrations (e.g., "100th booking! 20% off") are awarded to the first eligible booking after the milestone is reached. Paying online won't lock one in — they're awarded to the next in-person booking at the till, so the milestone is real. ### Stacking -Discounts **add together**. If you have a full loyalty card (10% off) AND there's an active "5% off this week" campaign, you get **15% off** — not just the better one. Every discount you qualify for stacks on top of the others. +Discounts **add together**. If you use your loyalty card (10% off) AND there's an active "5% off this week" campaign, you get **15% off**. Every discount you qualify for stacks on top of the others. ### What you'll see - Your stamp count on your account page -- A notification when you earn a stamp or unlock a discount -- The discount applied to your booking total when your appointment is completed +- A checkbox to use your stamps when paying +- Active campaign discounts shown as auto-applied in your booking summary --- ## B) Staff Member — "What applies and when?" -### When a booking is completed, the system checks for discounts in this order: +### Discounts can apply at TWO trigger points -| # | Discount Type | What triggers it | How much | +| Trigger Point | When it fires | +|---|---| +| **Payment time** | When a deposit or full payment is made (online or at till) | +| **Completion time** | When the booking is marked as completed at the till | + +Each discount type has specific rules about which trigger(s) it fires at. + +### Discount Application Order + +#### At Payment Time (`applyEligibleCampaignsAtPayment` in handlers.go) + +| # | Discount Type | Applies? | When | |---|---|---|---| -| 1 | **Loyalty** | Customer has 10 stamps (a pending redemption) | 10% off | -| 2 | **Time-based campaign** | An active campaign with start/end dates covering today | Whatever % the campaign is set to | -| 3 | **Per-user milestone** | Customer hits an exact booking count (e.g., their 10th visit) | Whatever % the campaign is set to | -| 4 | **Global milestone** | The salon's total completed bookings hit a number (e.g., 1000th booking overall) | Whatever % the campaign is set to | -| 5 | **Anniversary** | Time since the customer's first completed visit (e.g., 1-year anniversary) | Whatever % the campaign is set to | +| 1 | **Loyalty** | ✅ User opt-in via checkbox | User checks "Use Loyalty Stamp Card" in payment modal | +| 2 | **Time-based campaign** | ✅ Auto-applied | Campaign is active (start_date ≤ NOW() ≤ end_date) | +| 3 | **Per-user milestone** | ✅ Auto-applied | User's completed count matches milestone_value | +| 4 | **Global milestone** | ⚠️ Only if first in-person payment | Booking has at least one `in_person_card` payment; milestone_value ≤ global completed count | +| 5 | **Anniversary** | ✅ Auto-applied | Elapsed time since first completed visit matches milestone | -**All of these stack.** A customer can get loyalty + campaign + milestone discounts on the same booking. Each one is calculated against the **original booking total** (not the post-discount total). +#### At Completion Time (`ProgressBookingHandler` in bookings.go) + +| # | Discount Type | Applies? | Guard | +|---|---|---|---| +| 1 | **Loyalty** | ⚠️ Skip if already applied | `NOT EXISTS booking_discounts WHERE booking_id AND source='loyalty'` | +| 2 | **Time-based campaign** | ⚠️ Skip if already applied | `NOT EXISTS booking_discounts WHERE booking_id AND campaign_type='time_based'` | +| 3 | **Per-user milestone** | ⚠️ Skip if already applied | `NOT EXISTS booking_discounts WHERE user_id + source_id` | +| 4 | **Global milestone** | ⚠️ Only if in-person; skip if already applied | Has `in_person_card` payment; `milestone_value <= globalCount AND times_redeemed < max_redemptions` | +| 5 | **Anniversary** | ⚠️ Skip if already applied | `NOT EXISTS booking_discounts WHERE user_id + source_id + milestone_type` | ### Key rules - **Free bookings (£0 total)** earn no stamps and get no discounts - **One stamp per calendar day** — even if a customer has 3 appointments on Monday, they get 1 stamp - **Time-based campaigns**: if multiple are active, only the **highest %** applies (they don't stack with each other) - **Milestones are one-shot per customer per campaign** — a "10th visit" discount only fires once per customer -- **Global milestones** can have a max redemptions cap — once reached, no more +- **Global milestones**: milestone_value is used as a minimum threshold (`<=` not `=`). If booking #100 is ineligible (no in-person payment), booking #101, #102, etc. will get it instead until `max_redemptions` is reached +- **Global milestones can have a max redemptions cap** — once reached, no more - **Campaigns must be "active"** — draft, completed, or cancelled campaigns don't apply +- **All discounts stack** additive against the original booking total (not compound) +- **Discount payment records** are excluded from refund calculations (refunds return only real money) + +### Loyalty at the till (staff workflow) +When completing a booking, staff see: "This customer has X stamps. Would you like to use their Loyalty Stamp Card?" +- If the customer already used stamps at payment time, the checkbox is hidden (the discount already shows in the applied discounts list) +- If the customer has < 10 stamps, the checkbox is hidden +- If the customer has ≥ 10 stamps and hasn't used them, the checkbox is shown ### What you'll see on a completed booking Each discount appears as a separate discount line on the booking. A £100 booking with loyalty (10%) + campaign (5%) shows: @@ -56,7 +91,7 @@ Each discount appears as a separate discount line on the booking. A £100 bookin - Customer pays: £85.00 ### Tip calculation -Tip percentages (10%, 15%, 20%) in the Take Payment modal are calculated on the **net total after discounts** (`subtotal - discountSum`), not the pre-discount subtotal. This means customers tip on what they actually owe. +Tip percentages (10%, 15%, 20%) are calculated on the **net total after discounts** (`subtotal - discountSum`). --- @@ -80,12 +115,12 @@ draft → active → completed ### Campaign Types -| Type | Trigger | -|---|---| -| Time-based | Date range (start_date to end_date) | -| Per-user milestone | User's personal completed booking count hits exact value | -| Global milestone | Salon-wide completed booking count hits exact value | -| Anniversary | Time since user's first completed booking (months/years) | +| Type | Trigger | Applies at Payment? | +|---|---|---| +| Time-based | Date range (start_date to end_date) | ✅ Yes | +| Per-user milestone | User's personal completed booking count hits value | ✅ Yes | +| Global milestone | Salon-wide completed booking count hits value (minimum threshold) | ⚠️ Only if first payment is in-person | +| Anniversary | Time since user's first completed booking (months/years) | ✅ Yes | ### Configuration Fields @@ -93,7 +128,7 @@ draft → active → completed |---|---|---| | `discount_percent` | All | The % off (calculated against original booking total) | | `start_date` / `end_date` | Time-based | Must be `active` AND within date range | -| `milestone_value` | Milestones | The exact count to trigger on | +| `milestone_value` | Milestones | Minimum threshold — applied when `currentCount >= milestone_value` | | `milestone_unit` | Anniversary | `months` or `years` | | `max_redemptions` | Global milestone | Cap on total times this can be applied across all users | | `times_redeemed` | All | Auto-incremented each time the discount is applied | @@ -102,11 +137,11 @@ draft → active → completed | Discount | Dedup mechanism | |---|---| -| Loyalty | One pending redemption per user; consumed on use | -| Time-based | Single best selected (highest % wins) | -| Per-user milestone | One per user per campaign | -| Global milestone | `times_redeemed < max_redemptions` | -| Anniversary | One per user per campaign | +| Loyalty | `NOT EXISTS booking_discounts WHERE booking_id AND source='loyalty'` — one per booking | +| Time-based | `NOT EXISTS booking_discounts WHERE booking_id AND campaign_type='time_based'` — one per booking | +| Per-user milestone | `NOT EXISTS booking_discounts WHERE user_id + source_id` — one per user per campaign | +| Global milestone | `milestone_value <= globalCount AND times_redeemed < max_redemptions AND NOT EXISTS booking_discounts WHERE source_id = campaign.id AND booking_id` — one per booking + max cap | +| Anniversary | `NOT EXISTS booking_discounts WHERE user_id + source_id + milestone_type='anniversary'` — one per user per campaign | ### Database Tables @@ -121,17 +156,35 @@ draft → active → completed ## D) Programmer — "Give me the full spec" +### Constants + +Backend (`backend/handlers/payments/refund_policy.go`): +```go +const ( + RequiredDepositPct = 0.20 + ProtectedDepositMaxPct = 0.50 + LoyaltyStampCost = 10 // stamps needed for one redemption +) +``` + +Frontend (`frontend/src/lib/constants/policy.ts`): +```ts +export const POLICY = { + LOYALTY_STAMP_REDEMPTION_COST: 10, + REQUIRED_DEPOSIT_PCT: 0.20, + PROTECTED_DEPOSIT_MAX_PCT: 0.50, + // ... other policy constants +} as const; +``` + ### Stamp Earning (ProgressBookingHandler) ``` -On booking completion: +On booking completion (at the till): IF bookingTotal > 0: UPDATE users SET loyalty_stamps += 1 - WHERE NOT EXISTS ( - another completed booking for this user - with same-day completion - ) - IF newStampCount == 10: + WHERE NOT EXISTS (another completed booking for this user with same-day completion) + IF newStampCount >= LoyaltyStampCost: INSERT INTO loyalty_redemptions (status='pending', stamps_redeemed=10) ``` @@ -139,71 +192,83 @@ On booking completion: - 1-stamp-per-day enforced via `NOT EXISTS` subquery on same-day completions - Free bookings (`bookingTotal == 0`) skip entirely -### Discount Application Order (ProgressBookingHandler) +### Endpoint: `POST /api/bookings/{id}/apply-redemption` -All 5 discount types execute **unconditionally and independently** within `if bookingTotal > 0`. Each creates: +Called when user (via payment modal checkbox) or admin (via till checkbox) applies loyalty stamps: + +``` +1. Validate booking ownership + non-terminal status +2. Validate user has >= LoyaltyStampCost stamps +3. Validate no loyalty discount already on this booking +4. Start transaction: + a. Calculate discount: roundTo2(bookingTotal * 0.10) + b. INSERT booking_discounts (source='loyalty') + c. INSERT payments (method='discount') + d. UPDATE loyalty_redemption SET status='applied', applied_to_booking_id = id + e. UPDATE users SET loyalty_stamps = GREATEST(0, stamps - LoyaltyStampCost) +5. Return { success: true, discount_amount: X.XX } +``` + +Source: `backend/handlers/payments/loyalty.go` + +### Function: `applyEligibleCampaignsAtPayment(ctx, bookingID, userID)` + +Called AFTER a successful payment in `CreateBookingPayment` handler. Runs campaign checks at payment time: + +``` +1. Time-based campaign: + SELECT ... WHERE start_date <= NOW() AND end_date >= NOW() + AND (max_redemptions IS NULL OR times_redeemed < max_redemptions) + AND NOT EXISTS booking_discounts WHERE booking_id + campaign_type='time_based' + ORDER BY discount_percent DESC LIMIT 1 + +2. Per-user milestone: + SELECT ... WHERE milestone_type = 'per_user_booking_count' + AND milestone_value <= userBookingCount + AND NOT EXISTS booking_discounts WHERE user_id + source_id + +3. Anniversary: + SELECT ... WHERE milestone_type = 'anniversary' + AND NOT EXISTS booking_discounts WHERE user_id + source_id + milestone_type + +4. Global milestone (only if first payment is in_person_card): + SELECT ... WHERE milestone_type = 'global_booking_count' + AND milestone_value <= globalCount + AND (max_redemptions IS NULL OR times_redeemed < max_redemptions) + AND EXISTS(SELECT 1 FROM payments WHERE booking_id AND payment_method = 'in_person_card') + ORDER BY milestone_value DESC LIMIT 1 +``` + +Source: `backend/handlers/payments/handlers.go` line 684+ + +### Discount Application at Completion (ProgressBookingHandler) + +All discount types execute within `if bookingTotal > 0`, guarded by dedup checks. Each creates: 1. A `booking_discounts` row 2. A `payments` row with `payment_method = 'discount'`, `payment_type = 'partial'` +3. Increments `discount_campaigns.times_redeemed` (for campaigns) -#### Step 1: Loyalty Redemption -Finds the oldest pending, unexpired redemption for this user. Applies 10% of `bookingTotal`. Updates redemption to `status = 'applied'`. Resets stamps: `loyalty_stamps = GREATEST(0, stamps - 10)`. Oldest pending redemption used first (FIFO). - -```sql -SELECT id FROM loyalty_redemptions -WHERE user_id = $1 AND status = 'pending' AND expires_at > NOW() -ORDER BY redeemed_at ASC LIMIT 1 +#### Step 1: Loyalty Dedup Guard +```go +var loyaltyAlreadyApplied bool +tx.QueryRow(ctx, `SELECT EXISTS(SELECT 1 FROM booking_discounts + WHERE booking_id = $1 AND discount_source = 'loyalty')`, bookingID).Scan(&loyaltyAlreadyApplied) +if !loyaltyAlreadyApplied { + // existing loyalty redemption logic (unchanged) +} ``` -#### Step 2: Time-Based Campaign -Selects the single highest-% active campaign whose date range covers today. - -```sql -SELECT id, discount_percent FROM discount_campaigns -WHERE status = 'active' AND campaign_type = 'time_based' -AND start_date <= NOW() AND end_date >= NOW() -AND (max_redemptions IS NULL OR times_redeemed < max_redemptions) -ORDER BY discount_percent DESC LIMIT 1 -``` - -#### Step 3: Per-User Milestone -Checks the user's completed booking count (including the current booking). If it matches a `per_user_booking_count` milestone value, applies the discount. Dedup: one per user per campaign. - -```sql +#### Step 4: Global Milestone (with in-person check + next-eligible via `<=`) +```go SELECT id, discount_percent FROM discount_campaigns WHERE status = 'active' AND campaign_type = 'milestone' -AND milestone_type = 'per_user_booking_count' -AND milestone_value = $userBookingCount -AND NOT EXISTS (SELECT 1 FROM booking_discounts - WHERE user_id = $1 AND source_id = discount_campaigns.id) + AND milestone_type = 'global_booking_count' + AND milestone_value <= $globalCount + AND (max_redemptions IS NULL OR times_redeemed < max_redemptions) +ORDER BY milestone_value DESC LIMIT 1 ``` -#### Step 4: Global Milestone -Checks the salon-wide completed booking count. If it matches a `global_booking_count` milestone value, applies the discount. Capped by `max_redemptions`. - -```sql -SELECT id, discount_percent FROM discount_campaigns -WHERE status = 'active' AND campaign_type = 'milestone' -AND milestone_type = 'global_booking_count' -AND milestone_value = $globalCount -AND (max_redemptions IS NULL OR times_redeemed < max_redemptions) -``` - -#### Step 5: Anniversary -Calculates elapsed time since the user's first completed booking. Matches if `computed >= milestone_value`. Dedup: one per user per campaign. - -```sql -SELECT id, discount_percent, milestone_value, milestone_unit -FROM discount_campaigns -WHERE status = 'active' AND campaign_type = 'milestone' -AND milestone_type = 'anniversary' -AND NOT EXISTS (SELECT 1 FROM booking_discounts - WHERE user_id = $1 AND source_id = discount_campaigns.id - AND milestone_type = 'anniversary') -``` - -- `months`: `elapsed.Hours() / (30 * 24)` -- `years`: `elapsed.Hours() / (365.25 * 24)` -- First match only (one anniversary discount per booking) +Only applies if the booking has at least one `in_person_card` payment. ### Discount Amount Calculation @@ -211,6 +276,10 @@ Every discount: `discountAmount = roundTo2(bookingTotal * discountPercent / 100) All calculated against the **original** `bookingTotal` — never against a post-discount amount. Discounts are **additive, not compound**. +### Stamp Refund on Cancellation + +When a booking with a loyalty discount is cancelled, the stamps are refunded (`loyalty_stamps + LoyaltyStampCost`). This happens in `ProcessCancellationRefund` after all monetary refunds are processed. Stamps are only refunded for registered users (not guests). + ### Schema ```sql @@ -236,19 +305,20 @@ status ENUM('pending', 'applied', 'expired') redeemed_at TIMESTAMPTZ expires_at TIMESTAMPTZ applied_at TIMESTAMPTZ -applied_booking_id CHAR(12) +applied_to_booking_id CHAR(12) -- NEW: tracks which booking the redemption was used on -- booking_discounts id CHAR(12) PK booking_id CHAR(12) FK -> bookings user_id CHAR(12) FK -> users -discount_source ENUM('loyalty', 'campaign') -source_id CHAR(12) -campaign_type ENUM('time_based', 'milestone') -milestone_type ENUM('per_user_booking_count', 'global_booking_count', 'anniversary') +discount_source VARCHAR(30) -- 'loyalty' | 'campaign' +source_id CHAR(12) -- campaign.id (for campaigns) +campaign_type campaign_type -- 'time_based' | 'milestone' +milestone_type milestone_type -- 'per_user_booking_count' | 'global_booking_count' | 'anniversary' discount_percent NUMERIC(5,2) original_total NUMERIC(10,2) discount_amount NUMERIC(10,2) +applied_at TIMESTAMPTZ DEFAULT NOW() -- payments (discount rows) booking_id CHAR(12) FK -> bookings @@ -269,23 +339,39 @@ status ENUM('completed') Customer pays: **£75.00** -### What's NOT in the system -- No total discount cap (future safeguard: consider 50% max) -- No compound discounts (all additive against original total) -- No pre-assigned eligibility on booking rows (all computed at completion time) -- No online/advance discount preview (UI queries source tables directly for "you have X waiting" messages) +### Rescheduling with Discounts ---- +- **Edit request with time change + booking has discounts**: Auto-approve is BLOCKED regardless of notice period. The request goes to admin review. +- **Admin approval**: The admin sees a discount warning in the approval modal. +- **If rejected**: The user can cancel through normal refund tiers and rebook at full price. +- **Discounts are not revoked on reschedule** — they were earned at payment time and stay locked. -## E) Customer Relationship Metric Separation +### No-Show Tracking -To maintain clear accounting for each client: -1. **Total Spend** represents actual card, cash, and gift card payments only. Excludes discount amounts — accurate count of actual revenue received from the customer. -2. **Total Saved** represents the sum of all loyalty card redemptions and campaign discounts applied to the customer's completed bookings. +No-show tracking uses a counter-based system: -These are computed by the `/api/admin/users/{id}/relationship` endpoint and displayed as side-by-side metric cards in the admin User Details modal under "Customer Relationship". +1. A cancellation of a confirmed booking within 24h of start time -> booking status becomes `'no_show'` +2. `ApplyDepositsIfNeeded(userID)` counts unforgiven no-shows in the last 6 months +3. At 2+ no-shows, `deposits_required` is set to 3 +4. Each completed booking with a payment decrements `deposits_required` by 1 +5. When `deposits_required` reaches 0, all no-show records are auto-forgiven (inserted into `forgiven_no_shows`) -### Frontend display notes -- Discounts shown as negative numbers (e.g., `-£2.50`) in Payment History to distinguish from customer cash/card payments -- Booking Details modal uses elevated z-index (`!z-[60]`) to open in front of User Details modal (`z-50`) -- Chronological index-based matching prevents duplicate descriptions on multiple discounts with identical amounts +No-show forgiveness (by admin) is tracked via `forgiven_no_shows` table. + +### Test Files + +| File | Tests | Coverage | +|---|---|---| +| `handlers/payments/loyalty_test.go` | 10 | apply-redemption (4) + campaign auto-apply (6) | +| `handlers/payments/refund_exclude_test.go` | 2 | Discount/OTH excluded from refunds | +| `handlers/bookings/dedup_test.go` | 8 | Dedup guards + no-show tracking + 3-paid clear | +| `handlers/bookings/bookings_test.go` | 2 | Auto-approve blocked by discounts; still works without | + +### Key Design Decisions + +- **Global milestones at payment time only for in-person payments**: If paid online and later cancelled, the milestone was "claimed" by a phantom booking. In-person means the slot was physically filled. +- **Global milestone "next eligible"**: Changed from exact match (`=`) to minimum threshold (`<=`). If booking #100 is ineligible, #101 gets the milestone instead. +- **Stamps refunded on cancellation**: If a booking with loyalty discount applied is cancelled, the 10 stamps are refunded back to the user. +- **Discount payment records excluded from refunds**: `payment_method IN ('discount', 'on_the_house')` are excluded from `ProcessCancellationRefund` TotalPaid calculation. +- **Loyalty is always opt-in**: Users choose when to use stamps via checkbox. Staff ask at the till. Never auto-applied. +- **No global milestone notification when skipped**: If a global milestone is skipped (online payment), no notification is sent. The next eligible booking claims it silently. diff --git a/obsidian/Crussell/Overview.md b/obsidian/Crussell/Overview.md index 18c1271..4cbdd9f 100644 --- a/obsidian/Crussell/Overview.md +++ b/obsidian/Crussell/Overview.md @@ -18,6 +18,8 @@ Three booking flows, each with its own entry point and reservation TTL: Slot reservations stored as `time_blocker` entries with `RESERVATION:*` descriptions — no separate reservation table. They automatically participate in availability calculations. Anonymous reservation cap: 50 per 10-minute rolling window (429 if exceeded). +**Deposit system:** Bookings have a 24h deposit deadline. Unpaid bookings enter `pending_release` — the slot becomes vulnerable to eviction by overlapping new bookings. Payment of >= 20% of the total at any point promotes back to `confirmed`. Evicted bookings enter `deposit_lapsed`. Admin forgiveness (`forgive_fees`/`forgive_noshow`) on cancel and reschedule. Refund calculation uses notice-period tiers (72h/24h full/partial/none) with deposit protection capping retention at 50% of total. + Service eligibility filters by age (min age on service) and patch test validity (6-month expiry, 24-hour notice period). Guest accounts are disposable — no identity tracking across bookings, PII scrubbed 6 months after appointment. Idempotency keys (`idempotency_key VARCHAR(64) UNIQUE`) on bookings prevent duplicates on retry. @@ -81,7 +83,7 @@ Campaign lifecycle: `draft → active → completed` (or any → `cancelled`, `a ### Compliance -**GDPR Article 15**: Full data export via `/gdpr` frontend. Async Go endpoint (`GET /api/user/gdpr-export`) with 12h in-memory cache and background generation (navigation away doesn't cancel). 16-section JSON export: profile, bookings with overrides, payments, refunds, saved cards, social logins, loyalty redemptions, booking discounts, edit requests, affiliate payouts, verification codes, forgiven no-shows, patch tests, referrals, notification preferences, export metadata. Frontend: skeleton loading, 2s polling, styled report cards/tables, PDF export (print CSS hides navbar + verification banner), raw JSON download. +**GDPR Article 15**: Full data export via `/gdpr` frontend. Async Go endpoint (`GET /api/user/gdpr-export`) with 12h in-memory cache and background generation (navigation away doesn't cancel). 18-section JSON export: user profile (now includes failed_attempts, locked_until), login_audit, refresh_tokens, bookings with overrides, payments, refunds, saved cards, social logins, loyalty redemptions, booking discounts, edit requests, affiliate payouts, verification codes, forgiven no-shows, patch tests, referrals, notification preferences, export metadata. Frontend: skeleton loading, 2s polling, styled report cards/tables, PDF export (print CSS hides navbar + verification banner), raw JSON download. **Account deletion**: Registered users → `anonymize_user()` SQL function extended with child table PII scrubbing (social logins deleted, saved cards soft-deleted with PCI data cleared, verification codes expired, time blocker reservations scrubbed, edit request notes nulled, notification preferences deleted). External system scrubbing: S3 profile picture, Square saved cards. Guests → `delete_guest_user()` for full removal. @@ -93,7 +95,7 @@ Campaign lifecycle: `draft → active → completed` (or any → `cancelled`, `a **Component library**: PaymentModal (multi-method, service price overrides, tip presets), UserPaymentModal (deposit/partial/full/balance), BookingFlow (5 steps, auto-select, shared timeSlots utils), TodayCalendar (interactive grid), PendingApprovals (dedup refresh), NavBar (responsive with notification badge), PhoneInput (UK validation), CharCounter (grapheme counter for notes), ImageVariant (multi-format `` element), MapLibre GL map components. -**State**: Svelte 5 runes (`$state`, `$bindable`, `$effect`, `$derived`, SvelteDate). Auth store wraps JWT in localStorage with auto-refresh (hourly, if <14 days remaining). Role-based UI via `hasRole()`, `isAdmin()`, `isVerified()`. +**State**: Svelte 5 runes. Auth store wraps JWT in localStorage with auto-refresh (hourly, 1h access token, 90-day refresh token rotation with consumption-based invalidation). Role-based UI via `hasRole()`, `isAdmin()`, `isVerified()`. **Shared utilities**: `timeSlots.ts` (lunch protection, slot generation, formatting), `format.ts` (duration, date/time, age, ISO date), `phone.ts` (UK phone formatting). @@ -116,7 +118,7 @@ Campaign lifecycle: `draft → active → completed` (or any → `cancelled`, `a - **No error tracking** — Sentry DSN not configured, `log.Printf()` only - **No automated DB backups** — no `pg_dump` cron or point-in-time recovery - **No API documentation** — no OpenAPI/Swagger spec -- **No rate limiting per-user** — currently IP-based only +- **L3 progressive rate limiting** — per-IP dual-window (30 req/5s burst + 120 req/60s sustained) on login/register. Account lockout after 5 failures (progressive 15min→2h). ## Prerequisites @@ -202,7 +204,7 @@ npm run dev # Dev server with HMR ```bash cd backend -go test -tags "test,dev" -p 1 -count=1 ./... # 682/685 passing, 3 skipped +go test -tags "test,dev" -p 1 -count=1 ./... # 782/785 passing, 3 skipped go test -tags "test,dev" -v -run TestName ./... # Single test ``` diff --git a/obsidian/Crussell/Technical Manual.md b/obsidian/Crussell/Technical Manual.md index e6f3c33..a38491a 100644 --- a/obsidian/Crussell/Technical Manual.md +++ b/obsidian/Crussell/Technical Manual.md @@ -49,14 +49,14 @@ Backend (:8080) |---------|---------|---------| | `handlers/auth` | local.go, social.go | Registration (with referral code validation), login, refresh, email verification | | `handlers/bookings` | bookings.go, reserve.go, manage.go, admin_reserve.go | Booking CRUD, reservations, admin management, edit requests, discounts, closing hours validation, active booking limits, GetBookingsByCreatedRange, created_by_name resolution | -| `handlers/payments` | handlers.go, service.go, validators.go, giftcards.go, till.go | Square payments: terminal, online, refunds, tips, saved cards, gift cards (CRUD, topup, transfer, redeem, buy, expired balances, till sales) | -| `handlers/webhooks` | square.go | Square webhook handler for payment status updates | +| `handlers/payments` | handlers.go, service.go, validators.go, giftcards.go, till.go, refunds.go, refund_policy.go | Square payments: terminal, online, refunds, tips, saved cards, gift cards (CRUD, topup, transfer, redeem, buy, expired balances, till sales). Refund calculation with notice-period tiers and deposit protection | +| `handlers/webhooks` | square.go | Square webhook handler for payment status updates. **Dev-only stub** — production requires signature verification (HMAC-SHA256 with base64 output, `x-square-hmacsha256-signature` header). See `TODO(PROD)` in source. | | `handlers/admin` | users.go, analytics.go, custom_services.go, discount_campaigns.go, settings.go | Admin user management, custom services CRUD (list/create/get/update/promote/delete), discount campaigns, analytics (stub), business settings (GET/PUT with VAT, gift card config) | | `handlers/today` | today.go | Current/next appointment, today's grid, pending approvals, `DoneForDay` state with daily/weekly summary (`DailySummary` with `total_bookings`, `customers_served`, `summary_scope`), auto-status transitions, closed-day aggregation via `findWeekSummaryRange` + `computeAggregateSummary`. Exceptional hours lookup uses `exceptional_group_applications.week_start` (0=Monday). | | `handlers/user` | profile.go, account.go, guest.go, loyalty.go, customer_relationship.go, gdpr_export.go | User profile, guest creation (with CheckEmailHandler for registered-email detection), loyalty, contact info, GDPR export (async with 12h cache) | | `handlers/services` | services.go | Service catalog, eligibility filtering, patch_test_duration_hours auto-creates patch test records | | `handlers/scheduling` | default-hours.go, exceptional-hours.go, time-blockers.go | Working hours, exceptional groups, time blockers, gift card expiry cleanup (24-month rolling), idle account cleanup (2yr/5yr) | -| `handlers/portfolio` | images.go | Image upload, listing, tags, filters | +| `handlers/portfolio` | images.go | Image CRUD, cursor-paginated listing with fuzzy tag search & exact category filters, relevance-sorted tag results | | `handlers/notifications` | notifications.go | Admin notifications (GET, acknowledge) | ### Middleware (`mw/`) @@ -68,7 +68,8 @@ Backend (:8080) | `RequireAdmin` | Allows admin role only | | `RequireVerified` | Allows verified_email or admin | | `RequireRole(roles...)` | Generic role check | -| `RateLimit(limit, window)` | IP-based rate limiting (supports CF-Connecting-IP header) | +| `RateLimit(limit, window)` | IP-based rate limiting (supports CF-Connecting-IP header). Dev build tag (`dev`): no-op pass-through. | +| `ProgressiveRateLimit` | Per-IP dual-window rate limiter for bot-spam prevention. Burst: 30 req/5s, Sustained: 120 req/60s. Progressive delays (500ms–10s). Applied to login/register. Skips in dev build. | ### Database Layer (`db/`) @@ -85,6 +86,7 @@ Backend (:8080) | `internal/dav` | SabreDAV CardDAV integration (build tags: `service_dev.go` / `service_prod.go`) | | `internal/s3` | S3/R2 storage abstraction (build tags: dev vs prod) | | `internal/square` | Square client interface + dev mock + prod stub (build tags: `dev` vs `!dev`) | +| `internal/zxcvbnjs` | Bundles @zxcvbn-ts/core via goja (ExecJS-style). Exact parity with frontend password scoring. Go binary embeds the 1.7MB IIFE JS bundle. | --- @@ -170,8 +172,8 @@ src/lib/components/ ### State Management - **Svelte 5 runes**: `$state`, `$bindable`, `$effect`, `$derived`, `SvelteDate` -- **Auth store** (`lib/stores/auth.svelte.ts`): JWT tokens in localStorage, auto-refresh, role checks -- **Token refresh**: Every hour, refreshes if < 14 days remaining +- **Auth store** (`lib/stores/auth.svelte.ts`): JWT tokens in localStorage, auto-refresh, role checks. 1h access token, 90-day refresh token rotation with consumption-based invalidation. +- **Token refresh**: Every hour, automatically refreshes access token. Refresh token rotates each use (old token invalidated via DELETE-based consumption). - **Role-based UI**: `hasRole()`, `isAdmin()`, `isVerified()` control visible elements ### Shared Utilities @@ -200,6 +202,19 @@ src/lib/components/ - **`CharCounter`**: `Intl.Segmenter` for grapheme counting. Hidden below 750K, color-coded above: green (<800K), yellow (800K-950K), red (>950K) +### Security Headers + +Added in the June 2026 security pass: + +| Header | Value | Location | +|--------|-------|----------| +| `Content-Security-Policy` | `default-src 'none'; frame-ancestors 'none'` | Global middleware (`main.go:139`) | +| `Access-Control-Allow-Origin` | `*` (dev only) | Global middleware (`main.go:141`) | +| `Access-Control-Allow-Methods` | `GET, POST, PUT, PATCH, DELETE, OPTIONS` | Global middleware | +| `Access-Control-Allow-Headers` | `Authorization, Content-Type, Idempotency-Key` | Global middleware | + +CORS uses `*` in local dev. In production behind Cloudflare, nginx handles CORS. No CSP violations expected — the SvelteKit SPA doesn't load external scripts or fonts. + --- ## API Reference @@ -211,7 +226,7 @@ src/lib/components/ | GET | `/api/services` | Optional | 120/min | List active services (eligibility for authenticated) | | GET | `/api/services/eligible-for/{user_id}` | Admin | 120/min | Services filtered by user's age/patch test | | POST | `/api/register` | None | 10/min | Create user account (optional `referralCode` field) | -| POST | `/api/login` | None | 1/5s | Authenticate, receive JWT | +| POST | `/api/login` | None | ProgressiveRateLimit + RateLimit(10, 1min) | Authenticate, receive JWT + refreshToken. Account lockout after 5 failures (15min→30min→1h→2h). | | POST | `/api/verify/generate` | None | — | Generate email verification or password reset code | | POST | `/api/verify/check` | None | — | Verify code | | GET | `/api/health` | None | — | Health check (DB, S3, Square, frontend status) | @@ -224,7 +239,7 @@ src/lib/components/ | GET | `/api/scheduling/available-hours` | None | 120/min | Available slots (triggers cleanup) | | POST | `/api/bookings/reserve` | Optional | 30/min | Reserve slot (user=1h, anon=10min, 50-cap) | | POST | `/api/bookings` | Optional | 30/min | Create booking (with `Idempotency-Key` header) | -| GET | `/api/portfolio/images` | None | — | List images (pagination, tag/category filters) | +| GET | `/api/portfolio/images` | None | — | List images (cursor pagination, tag & category filters). Query params: `limit` (1-100, default 20), `cursor` (from `next_cursor` in response), `tag` (fuzzy substring), `tags` (comma-separated, fuzzy), `filter[category]=value` (exact). Tag results sorted by relevance then date; filter-only sorted by date. Returns `{ images: [...], next_cursor: "..." }`. | | GET | `/api/portfolio/tags` | None | — | List all tags | | GET | `/api/portfolio/filters` | None | 60/min | Get filter categories with counts | | GET | `/api/portfolio/images/{id}` | None | 120/min | Get single image by UUID or timestamp | @@ -234,7 +249,7 @@ src/lib/components/ | Method | Path | Description | |--------|------|-------------| | POST | `/api/logout` | Revoke current JWT token | -| POST | `/api/refresh-token` | Refresh JWT (role-change detection, revokes old JTI) | +| POST | `/api/refresh-token` | Revoke old JTI, issue new JWT + refreshToken. Role-change detection (rejects if user's role changed since login). | | GET | `/api/user/profile` | Get current user profile | | PUT | `/api/user/profile` | Update profile | | POST | `/api/user/profile-picture` | Upload profile picture | @@ -341,12 +356,12 @@ src/lib/components/ |------|--------| | `account_role` | `unverified_email`, `verified_email`, `admin`, `guest`, `affiliate` | | `account_type` | `email`, `google`, `microsoft`, `facebook`, `guest` | -| `booking_status` | `pending`, `confirmed`, `in_progress`, `completed`, `client_cancelled`, `we_cancelled`, `re-schedule`, `no_show`, `no_deposit` | +| `booking_status` | `pending`, `confirmed`, `in_progress`, `completed`, `client_cancelled`, `we_cancelled`, `no_show`, `pending_release`, `deposit_lapsed` | | `verification_purpose` | `email_verify`, `password_reset` | | `payment_type` | `deposit`, `full`, `tip`, `balance`, `partial` | | `payment_method` | `online_square`, `in_person_card`, `cash`, `giftcard`, `discount`, `on_the_house` | | `payment_status` | `pending`, `completed`, `failed`, `refunded` | -| `admin_notification_reason` | `pending_booking`, `cancelled_booking`, `rescheduled_booking`, `1_week_no_pay`, `1_month_no_pay`, `affiliate_claim`, `late_cancellation`, `no_deposit`, `deposit_paid`, `edit_request`, `new_booking`, `edit_requested` | +| `admin_notification_reason` | `pending_booking`, `cancelled_booking`, `rescheduled_booking`, `1_week_no_pay`, `1_month_no_pay`, `affiliate_claim`, `late_cancellation`, `no_deposit`, `deposit_paid`, `edit_request`, `new_booking`, `edit_requested`, `deposit_not_paid_by_deadline` | | `campaign_type` | `time_based`, `milestone` | | `milestone_type` | `per_user_booking_count`, `global_booking_count`, `anniversary` | | `milestone_unit` | `bookings`, `months`, `years` | @@ -366,7 +381,7 @@ src/lib/components/ | `services` | Service offerings | | `custom_services` | One-off / special-request services (usage_count, created_by) | | `booking_custom_services` | Custom services per booking (override_price, override_duration_minutes) | -| `bookings` | Appointment records (idempotency_key, deposit_required, deposit_paid, deposit_amount, deposit_deadline) | +| `bookings` | Appointment records (idempotency_key, deposit_required, deposit_paid, deposit_amount, deposit_deadline). Statuses include pending_release (deposit deadline passed, slot vulnerable) and deposit_lapsed (slot evicted by another booking) | | `booking_services` | Services per booking (override_price, override_duration_minutes) | | `booking_edit_requests` | Pending customer edit requests | | `user_referrals` | Referral tracking | @@ -375,10 +390,10 @@ src/lib/components/ | `exceptional_working_hours` | Hours for exception groups | | `exceptional_group_applications` | Apply exceptions to date ranges | | `time_blockers` | Admin time blocks + slot reservations (description LIKE 'RESERVATION:%') | -| `forgiven_no_shows` | Tracks forgiven no-shows | +| `forgiven_no_shows` | Tracks no-shows forgiven by admin (booking_id, forgiven_by FK to users, created_at). Used by `CountUnforgivenNoShows()` to exclude forgiven records | | `payments` | Payment transactions (VAT fields, invoice_number sequence, fees column for Square deductions, saved_card_id, gift_card_id) | | `user_saved_cards` | Saved card details (square_card_id, brand, last4, fingerprint, soft delete with retained_until) | -| `refunds` | Refund records linked to payments (amount, reason, square_refund_id) | +| `refunds` | Refund records linked to bookings (amount, reason, square_refund_id, created_by FK to users, ON DELETE SET NULL) | | `financial_aggregates` | Monthly aggregated financial statistics (no PII) — populated when granular records expire | | `square_deposits` | Square deposit batch tracking for bank reconciliation (batch_id, total_amount, deposited_at) | | `affiliate_payouts` | Affiliate commission tracking | @@ -464,6 +479,7 @@ src/lib/components/ | Anonymous user | 10 minutes | `RESERVATION:anon:{ipHash}:{timestamp}` | | Admin walk-in | 5 minutes | `RESERVATION:admin:walkin:{timestamp}` | | Admin call-in | 1 hour | `RESERVATION:admin:callin:{timestamp}` | +| Edit request | 24 hours | `RESERVATION:edit_request:{timestamp}` | **Anonymous Cap:** 50 reservations per 10-minute rolling window. Returns 429 if exceeded. @@ -562,9 +578,39 @@ src/lib/components/ ### Deposit System -**How it works:** `users.deposits_required` integer (0-3) tracks outstanding deposit obligations. +**How it works:** `users.deposits_required` integer (0-3) tracks outstanding deposit obligations. When a user books a service with deposit requirements, the booking gets a `deposit_deadline` (24h before start_time) and `deposit_required = true`. -**24-Hour Late Cancellation Rule:** +**Deposit Deadline Flow:** + +1. **Booking created** → status is `confirmed` (or `pending` if notes require approval). User has until 24h before the start time to pay >= 20% of the total. +2. **Payment received** → `CreateBookingPayment` checks total paid across all completed payments. If total paid >= 20% of booking total, `deposit_paid` is set to true and any `pending_release` booking is promoted back to `confirmed`. This threshold-based check works regardless of payment type label (`deposit`, `full`, `partial`, `balance`). + + **Payment split:** When a card payment arrives before the booking start time, `buildSplitRecords` (in `handlers/payments/handlers.go`) automatically splits a single Square charge into up to 3 payment records: + + - **Deposit portion:** The first 50% of the booking total (minus any already deposited) is **always** carved out as `payment_type='deposit'`, regardless of payment size. A £25 payment on a £100 booking produces `[deposit=25]`; a £65 payment produces `[deposit=50, partial=15]`. + + - **Balance portion:** The remainder first covers whatever is still owed on the booking. The label is `'balance'` (if previous payments exist and this completes it), `'full'` (if this single portion covers the entire remaining balance) or `'partial'` (if the booking still has a balance after this payment). + + - **Tip portion:** Any amount beyond the booking total overflows into `payment_type='tip'`. A £125 payment on a £100 booking produces `[deposit=50, balance=50, tip=25]`. + + All split records share the same `square_payment_id` so the refund loop can avoid duplicate Square API calls. After the booking start time, no split is applied — the charge records as a single payment with its original type. + + **Concurrency guard:** `CreateBookingPayment` acquires a PostgreSQL session-level advisory lock (`pg_advisory_lock(hashtext('crussell:payment:' || booking_id))`) at entry and releases it in a defer. This serializes all payment attempts for the same booking — if two browser tabs try to pay simultaneously, the second request blocks until the first commits or rolls back. After the lock, the handler re-checks booking status (a concurrent payment may have promoted it) and runs a payment-type duplicate guard that prevents two `'full'` or `'deposit'` payments from being created for the same booking, even with different idempotency keys. + +3. **Deposit deadline passes without payment** → `CleanupExpiredDeposits()` moves the booking to `pending_release`. The slot becomes vulnerable — another booking can claim it via eviction. An admin notification `deposit_not_paid_by_deadline` is created. The user's time blocker reservations are also cleaned up. +4. **Slot claimed by another booking** → If a new booking overlaps a `pending_release` slot, `EvictPendingReleaseOverlapping` (a shared function in `bookings.go`) evicts the pending_release booking to `deposit_lapsed`. The eviction runs inside the same transaction as the new booking's creation, so it rolls back if the new booking fails. The function is called by all 4 eviction sites: `CreateBookingHandler`, `ConfirmBookingHandler`, `AdminCreateBookingForUserHandler`, and `AdminRescheduleBookingHandler`. A `PAYMENT_IN_FLIGHT` time_blocker guard prevents evicting a booking that the user is currently paying for (5-minute window). +5. **Payment arrives after deadline but before eviction** → The 20% threshold check promotes `pending_release` back to `confirmed` — the booking is saved and the slot is no longer vulnerable. + +**Booking Status Transitions:** + +``` +pending/confirmed ──(deadline passed)──→ pending_release ──(slot claimed)──→ deposit_lapsed + (payment received) → confirmed +``` + +**Eviction mechanism:** Eviction is handled by the shared `EvictPendingReleaseOverlapping(ctx, tx, startTime, endTime)` function in `bookings.go`, which is called by all 4 handlers that can claim a slot: `CreateBookingHandler`, `ConfirmBookingHandler`, `AdminCreateBookingForUserHandler`, and `AdminRescheduleBookingHandler`. The function uses a single `UPDATE ... RETURNING` query to evict overlapping `pending_release` bookings and return the affected IDs and user IDs. A `NOT EXISTS` subquery on `time_blockers WHERE description = 'PAYMENT_IN_FLIGHT:' || bookings.id` prevents evicting a booking that a user is currently paying for (the 5-minute payment lock window). Eviction is BAU, so no admin notification is generated — a TODO remains for user notification when that system is built. + +**24-Hour Late Cancellation Rule (no-show tracking):** - < 24h without forgiveness → `no_show` status, `deposits_required = 3` (resets, not adds) - < 24h with forgiveness → `client_cancelled`, no penalty - ≥ 24h → `client_cancelled`, no penalty @@ -578,7 +624,55 @@ src/lib/components/ **Guest Exemption:** Guest bookings bypass deposit and patch-test checks entirely. -**Decision:** The deposit system uses a simple integer (0-3) rather than a separate table of deposit records. This is because the business rule is a "3-strike" system where the penalty is binary (either you have 3 deposits or you don't). Each completed appointment reduces the count by 1. The simplicity reduces schema complexity and avoids the need for a separate deposit tracking table. +**Forgiven No-Shows:** `forgiven_no_shows` table tracks no-shows that were forgiven by admin. Used by `CountUnforgivenNoShows()` to exclude forgiven records. Admins can forgive via `AdminCancelBookingHandler` (`forgive_noshow`) or `AdminRescheduleBookingHandler` (`forgive_noshow`). + +**Decision:** The deposit system uses a simple integer (0-3) rather than a separate table of deposit records, because the business rule is a "3-strike" system. The `pending_release`/`deposit_lapsed` two-step status gives a grace period after the deadline for late payments, while still freeing the slot for other customers. + +--- + +### Refund System + +**How it works:** When a booking with payments is cancelled (admin or user), `ProcessCancellationRefund` calculates and executes the refund based on notice period and deposit protection rules. + +**Refund Calculation Tiers:** + +| Tier | Condition | Refund | +|------|-----------|--------| +| Full refund | >= 72h notice | 100% of amount paid | +| Partial refund | 24-72h notice | Protected deposit kept (up to 50%), rest refunded | +| No refund | < 24h notice (no-show) | Nothing refunded | +| Admin forgiveness | `forgive_fees = true` | 100% refunded, overrides deposit protection | + +**Deposit Protection Logic:** +- The **protected deposit** is `min(amount_paid, max(total * protected_deposit_max_pct, amount_paid * required_deposit_pct))` where `protected_deposit_max_pct = 50%` and `required_deposit_pct = 20%`. +- In the 24-72h window: you always get back everything above the protected deposit. +- Under 24h (no-show): the protected deposit is the maximum that can be retained. Any amount paid beyond the protected deposit is refunded. + +**Policy constants** (shared between frontend `policy.ts` and backend `refund_policy.go`): + +| Constant | Value | Meaning | +|----------|-------|---------| +| `FULL_REFUND_THRESHOLD_HOURS` | 72 | Hours of notice needed for full refund | +| `PARTIAL_REFUND_THRESHOLD_HOURS` | 24 | Hours of notice for partial refund | +| `NO_SHOW_THRESHOLD_HOURS` | 24 | Hours before no-show applies | +| `DEPOSIT_ADVANCE_HOURS` | 36 | Min hours advance booking for deposit-required users | +| `DEPOSIT_DEADLINE_WINDOW` | 24 | Hours before start_time to pay deposit | +| `PROTECTED_DEPOSIT_MAX_PCT` | 50% | Max percentage that can be retained as deposit | +| `REQUIRED_DEPOSIT_PCT` | 20% | Minimum deposit needed for protection | +| `RESCHEDULE_BLOCK_HOURS_WITH_PAYMENTS` | 72 | Min notice to reschedule online with payments | +| `RESCHEDULE_BLOCK_HOURS_NO_PAYMENTS` | 24 | Min notice to reschedule online without payments | + +**Cancellation flow:** +1. Admin or user calls cancel endpoint +2. `ProcessCancellationRefund` queries booking + payments + start time +3. `CalculateRefundForCancellation` determines tier and amounts +4. Refund is executed: Square refund for online card payments, user balance credit for cash/gift card/no-Square-ID payments +5. Refund record created in `refunds` table +6. If a deposit payment was fully refunded, `deposit_paid` is set to false + +**Split payment dedup:** When split payment records share the same `square_payment_id` (one Square charge split into deposit + balance records), `ProcessCancellationRefund` tracks already-refunded Square IDs in a local map and only refunds each Square payment once. Subsequent records sharing the same ID skip the Square API call and credit the user's balance instead. + +**Admin forgiveness:** Admin can set `forgive_fees = true` (100% refund regardless of notice) or `forgive_noshow = true` (no no-show penalty) on both `AdminCancelBookingHandler` and `AdminRescheduleBookingHandler`. --- @@ -888,20 +982,41 @@ A record is only deleted when **both** applicable conditions are met — the 7-y --- -### JWT Revocation +### JWT Authentication -**How it works:** Every JWT carries a unique `jti` claim (UUID v4). An in-memory map of revoked JTIs is maintained with a 5-minute cleanup ticker. `POST /api/logout` revokes the current token. The refresh handler revokes the old JTI before issuing a replacement. +**Architecture:** +- **Access token**: 1 hour expiry (changed from 30 days June 2026 security pass) +- **Refresh token**: 90-day sliding opaque token stored in `refresh_tokens` table (SHA-256 hash) +- **Rotation**: Each refresh invalidates the previous token via `DELETE`-based consumption. If a used token is replayed, it returns no rows → rejection (family-based revocation). +- **Signing**: HS256 with secret from `JWT_SECRET_KEY` env var -**Middleware:** `RequireAuth` extracts the `jti` claim and checks it against the revoked map. If revoked, the request is rejected with 401. +**JTI Revocation:** +Every JWT carries a unique `jti` claim (UUID v4). Revoked JTIs are stored in the `revoked_jtis` PostgreSQL table with an expiry timestamp. `IsJTIRevoked()` is called by `VerifyToken()` on every request. A background cleanup runs every 30 minutes: -**Cleanup:** A goroutine runs every 5 minutes, removing JTIs older than 30 days from the revoked map. This prevents unbounded memory growth. +```sql +DELETE FROM revoked_jtis WHERE expires_at < NOW(); +``` -**Limitations:** -- Revocation is in-memory only — restarting the server clears the revoked set -- Single-instance deployment — no distributed revocation (no Redis) -- For production with multiple instances, a shared store (Redis, database) would be needed +This replaces the old in-memory map (pre-June 2026 security pass). The DB-backed approach survives server restarts and doesn't leak memory. -**Decision:** In-memory revocation is sufficient for a single-instance deployment (the current architecture). The 5-minute cleanup ticker prevents memory leaks. JWTs are short-lived (30 days) with hourly refresh, so a revoked token will naturally expire within 30 days. +**When JTIs are revoked:** +1. `POST /api/logout` — revokes current JTI with 1h expiry +2. `POST /api/refresh-token` — revokes old JTI before issuing new token (rotation) +3. `PUT /api/user/password` — logs the change (full session revocation is a TODO — the current system only revokes the specific JTI, not all user sessions) + +**Account Lockout:** +After 5 failed login attempts, the account is locked with progressive durations: +- 5 failures → 15 minute lockout +- 7 failures → 30 minute lockout +- 10 failures → 1 hour lockout +- 20 failures → 2 hour lockout + +Lockout state is stored in `users.failed_attempts` and `users.locked_until` columns. Successful login resets both. + +**Password Validation:** +- Minimum length: 6 characters (enforced by validator tag + explicit check) +- Server-side strength: `@zxcvbn-ts/core` via goja (same JS library as frontend). Requires score ≥ 2. Skipped when `GO_TESTING=1`. +- bcrypt max: 72 characters (truncated by bcrypt internally — checked server-side) --- @@ -957,13 +1072,51 @@ Each test package has a `TestMain` that runs schema migration once per package ( Between tests, `TruncateTables()` runs `TRUNCATE TABLE ... CASCADE` on all tables. This is ~60% faster than DROP+CREATE. -### Advisory Lock Pattern +### Payment Serialization Lock (advisory lock 1339) + +`CreateBookingPayment` acquires a PostgreSQL session-level advisory lock (`pg_advisory_lock(hashtext('crussell:payment:' || booking_id))`) to serialize concurrent payment attempts for the same booking. This prevents the two-tab double-payment race where two browser tabs submit payments with different idempotency keys but the same payment type. + +**Key implementation detail:** The lock must be acquired and released on the **same** database connection. Using `db.DB.Exec()` for both would be unsafe — each call may get a different pool connection, and `pg_advisory_unlock` on a different session is a silent no-op, leaking the lock. The code uses `db.DB.Acquire()` to pin a dedicated connection for the duration of the handler, with `defer pinConn.Release()` ensuring the connection is returned when done. + +```go +pinConn, err := db.DB.Acquire(r.Context()) +defer pinConn.Release() +pinConn.Exec(ctx, "SELECT pg_advisory_lock(hashtext('crussell:payment:' || $1))", bookingID) +defer pinConn.Exec(ctx, "SELECT pg_advisory_unlock(hashtext('crussell:payment:' || $1))", bookingID) +``` + +After acquiring the lock, the handler re-checks the booking status (a concurrent payment may have promoted it) and runs a payment-type duplicate guard that prevents two `'full'` or `'deposit'` payments from being created for the same booking, even with different idempotency keys. The guard accounts for `buildSplitRecords` which converts `'full'` input to `'deposit'` + `'balance'` records. + +### Cursor-Based Pagination Pattern + +All list endpoints use cursor-based pagination with `(created_at, id)` tuples. The query fetches `perPage + 1` items — if we got the extra item, there's a next page. The cursor is set only when a next page exists: + +```go +query += " LIMIT $N" +args = append(args, perPage+1) +// ... fetch rows ... +var nextCursor *string +if len(results) > perPage { + results = results[:perPage] + last := results[perPage-1] + cursor := last.CreatedAt.Format(time.RFC3339) + "|" + last.ID + nextCursor = &cursor +} +``` + +This prevents clients from making an extra empty-page request to determine the end of results. The cursor must be URL-encoded by the client (`url.QueryEscape()`) because the `+` in RFC3339 timezone offsets is decoded as a space in query strings. + +Files with this pattern: `bookings.go` (4 handlers), `custom_services.go`, `user/profile.go`, `notifications/notifications.go`, `payments/giftcards.go`, `portfolio/images.go`. + +**Performance note:** The data query uses `LIMIT perPage + 1` with no window function. The `COUNT(*) OVER()` was removed from all cursor-paginated queries because it forces PostgreSQL to materialise all matching rows before applying the LIMIT, defeating cursor pagination's key advantage. Instead, a separate `SELECT COUNT(*)` query runs before the data query using a simplified FROM+WHERE (no CTEs, no SELECT subqueries, no LEFT JOINs unless needed for WHERE filtering). This count query uses indexes and doesn't block the data query's streaming LIMIT. + +### Advisory Lock Pattern (test infrastructure) **Migration lock (1337):** `pg_advisory_lock(1337)` protects concurrent schema migration. When multiple test packages run simultaneously, only one executes the migration DDL at a time. **Truncation lock (1338):** `pg_advisory_lock(1338)` prevents CASCADE truncation deadlocks. When two tests try to truncate simultaneously, one waits for the other. -**Implementation:** Both use `pool.Acquire()` for a dedicated connection. The lock is acquired, work is done, then the lock is released and the connection is returned to the pool. +**Implementation:** Both use `pool.Acquire()` for a dedicated connection (same pattern as the payment serialization lock above). ### Statement-by-Statement SQL Parser @@ -1178,6 +1331,12 @@ sequenceDiagram F-->>C: Print (CSS hides navbar) ``` +**June 2026 additions:** +- `failed_attempts`, `locked_until` added to user profile section +- `login_audit` — new section (attempt_type, ip_address, success, created_at) +- `refresh_tokens` — new section (role, revoked, created_at, expires_at; token_hash excluded) +- Empty sections return `null` instead of `[]` for cleaner output + --- ## Key Code Patterns @@ -1297,3 +1456,219 @@ Accounts with no gift card balance are anonymized after 2 years of inactivity. A ### Why the notification system is pull-based? The admin notification system uses polling (pull) rather than WebSockets or SSE (push). This is because the admin dashboard is a management tool, not a real-time monitoring system. Polling every 30 seconds is sufficient for booking notifications. Push-based systems would require WebSocket infrastructure, which adds complexity for a single-admin deployment. + +--- + +## Performance Considerations (Cloud Hosting) + +Every database query has a compute cost. On cloud-hosted PostgreSQL (RDS, Cloud SQL, Supabase), this translates directly to monthly bills. The following patterns are used throughout the codebase to minimise query count and table scan size. + +### 1. `UPDATE ... RETURNING` over `UPDATE + SELECT WHERE updated_at = NOW()` + +**Anti-pattern found and eliminated:** +```sql +-- BEFORE (2 queries, 1 table scan): +UPDATE bookings SET status = 'deposit_lapsed', updated_at = NOW() WHERE ...; +INSERT INTO admin_notifications ... SELECT id, user_id FROM bookings WHERE status = 'deposit_lapsed' AND updated_at = NOW(); + +-- AFTER (1 query, 0 table scans): +UPDATE bookings SET status = 'deposit_lapsed', updated_at = NOW() WHERE ... RETURNING id, user_id; +``` + +The `updated_at = NOW()` approach relied on transaction-timestamp matching, which is both fragile (race-prone under high concurrency) and expensive (it rescans the table for rows that were just updated). Using `RETURNING` eliminates the second scan entirely and guarantees the inserted rows are exactly those that were updated, regardless of timing. Eviction is BAU, so the `RETURNING` results are returned to the caller for optional use (e.g. future user notification); no admin notification is created. + +**Cloud cost impact**: Each eviction saves 1 full table scan. At scale (hundreds of bookings/day), this adds up to thousands of eliminated scans per month. + +**Files**: `bookings.go` (`EvictPendingReleaseOverlapping` — shared function used by all 4 eviction sites), `time-blockers.go` (`CleanupExpiredDeposits` — 2 UPDATEs feeding 1 INSERT + 1 DELETE) + +### 2. `unnest()` over `JOIN ... WHERE updated_at = NOW()` + +**Anti-pattern found in `CleanupExpiredDeposits` and eliminated:** +```sql +-- BEFORE (JOIN on timestamp — fragile, scans time_blockers table): +DELETE FROM time_blockers tb USING bookings b +WHERE (tb.created_by = b.user_id OR ...) AND b.status = 'pending_release' AND b.updated_at = NOW(); + +-- AFTER (direct ID lookup — O(n) vs O(n*m)): +DELETE FROM time_blockers tb +USING unnest($1::text[]) AS evicted_ids(id) +WHERE tb.created_by = ANY($2::text[]) OR tb.description ILIKE ANY(...); +``` + +The old approach joined the `time_blockers` table against `bookings` using `updated_at = NOW()` as a tag — a fragile coupling that breaks if clock skew occurs or another operation touches the same rows within the same transaction. The new approach collects evicted IDs from `UPDATE ... RETURNING` and passes them directly. + +**Cloud cost impact**: Eliminates the `bookings` table scan from the DELETE query. For a salon with 10,000+ historical bookings, this is a meaningful reduction in I/O per cleanup run. + +**Files**: `time-blockers.go` (CleanupExpiredDeposits) + +### 3. Transaction batching over row-by-row + +Multiple split payment records from a single Square charge are inserted inside one `pgx.Tx` instead of individual `db.DB.Exec` calls. This: +- Reduces network round-trips (1 commit vs N individual inserts) +- Ensures atomicity (if the 2nd record fails, the 1st rolls back) +- Eliminates partial-payment corruption (customer charged £100, DB only shows £50) + +**Files**: `handlers.go` (CreateBookingPayment), `service.go` (CreatePaymentRecordTx) + +### 4. Combined SELECT over N separate queries + +**Example** — `DeleteBookingHandler` previously ran two queries: +```sql +SELECT status FROM bookings WHERE id = $1; -- query 1 +SELECT start_time FROM bookings WHERE id = $1; -- query 2 (later) +``` +Now runs one: +```sql +SELECT status, start_time FROM bookings WHERE id = $1; -- 1 query +``` + +**Files**: `bookings.go` (DeleteBookingHandler) + +### 5. pg_trgm GIN indexes for ILIKE search + +Search queries using `ILIKE '%...%'` with a leading wildcard cannot use standard B-tree indexes. PostgreSQL's `pg_trgm` extension enables GIN indexes with `gin_trgm_ops` that support these searches. The following GIN indexes are defined in `init-script.sql`: + +| Table | Columns | Index name | +|-------|---------|------------| +| `users` | `fn`, `email`, `phone` | `idx_users_fn_trgm`, `idx_users_email_trgm`, `idx_users_phone_trgm` | +| `users` | `n_first_name`, `n_last_name` | `idx_users_n_first_name_trgm`, `idx_users_n_last_name_trgm` | +| `custom_services` | `name`, `description` | `idx_custom_services_name_trgm`, `idx_custom_services_desc_trgm` | +| `services` | `name` | `idx_services_name_trgm` | +| `bookings` | `notes` | `idx_bookings_notes_trgm` | +| `images` | `url`, `thumbnail_url` | `idx_images_url_trgm`, `idx_images_thumbnail_url_trgm` | +| `time_blockers` | `description` | `idx_time_blockers_desc_trgm` | + +**Deliberately excluded from pg_trgm indexing:** +- `CHAR(12)` ID columns — IDs must never use `ILIKE` search (fixed-width identifiers, exact-match only) +- `booking_status` enum — the `status::text` cast is not `IMMUTABLE`, which PostgreSQL requires for index expressions +- `images.tag_names` (`text[]` array) — `gin_trgm_ops` does not support array types. For ILIKE search on unnested tags at scale, normalise into a separate `image_tags` table + +These indexes are used by: +- Admin booking search (`bookings.go:1524`) — searches across user name, email, phone, booking notes +- Admin user search (`profile.go:430`) — user name and email fields +- Custom services search (`custom_services.go:125`) — name + description +- Admin service listing (`services.go`) — service name +- Time-blocker overlap check (`time-blockers.go:689`) — description ILIKE ANY + +The `pg_trgm` extension was already enabled; only the indexes were missing. These convert sequential scans into index scans at the cost of additional storage (roughly 1.5× the indexed text column size on disk). + +### 6. `parseCursor` deduplication + +`parseCursor("createdAt|id")` was previously implemented as 5 identical copies across `bookings.go`, `custom_services.go`, `profile.go`, `notifications.go`, and `images.go`. Deduplicated to a single `validators.ParseCursor()` in `internal/validators/validators.go` with unit tests in `email_test.go`. + +### 7. N+1 → batch preloading (services listing) + +**Anti-pattern found in `ServicesHandler` and `ServicesEligibleForUserHandler`:** + +`checkPatchTestStatus(ctx, userID, serviceID)` was called inside `for rows.Next()` loops, issuing 2 queries per service (one for `patch_tests`, one for `user_patch_tests`). For a salon with 20 services, this meant 40 DB round-trips per page load. + +**Fix:** A new `loadPatchTests(ctx, userID) map[string]*patchTestInfo` function does 2 total queries before the loop. `checkPatchTestStatus` now accepts the preloaded map and performs O(1) in-memory lookups. + +**Cloud cost impact**: Eliminates 2N-2 queries per service listing. For 20 services: 40 queries → 2. + +**Files**: `services.go` (ServicesHandler, ServicesEligibleForUserHandler, checkPatchTestStatus) + +### 8. 14-day loop → single CTE+LATERAL query (today dashboard) + +**Anti-pattern found in `findNewBookingServices`:** + +A `for i := 1; i <= 14; i++` loop issued 2 queries per iteration (exceptional hours, then default working hours) to find the most recent open day's closing time. Max 28 round-trips per dashboard load. + +**Fix:** A single SQL query using `generate_series(1, 14)` as a CTE, with `LEFT JOIN LATERAL` for both exceptional and default hours, `COALESCE` to prefer exceptional over defaults, and `ORDER BY day_date DESC LIMIT 1` to pick the most recent open day. Same fallback (`yesterday 5pm`) preserved in Go. + +**Cloud cost impact**: 28 queries → 1 per dashboard load. Every admin "Today" page view hits this. + +**Files**: `today.go` (findNewBookingServices) + +### 9. 9→1 aggregate queries (today dashboard) + +**Anti-pattern found in `computeAggregateSummary`:** + +Nine separate `db.DB.QueryRow()` calls each scanned the same booking/payment/gift card tables over the same `startDate ≤ x < endDate` range. This ran on every admin dashboard load. + +**Fix:** Single combined query with 11 independent subquery columns sharing the same `$1, $2` parameters. `COALESCE(..., 0)` handles nulls for all numerics; `COUNT(DISTINCT b.id)` prevents overcounting visits when a booking has multiple payments. + +| Aggregate | Original | After | +|-----------|----------|-------| +| Payments (non-tip) | 1 query | — | +| Tips | 1 query | — | +| Amount due | 1 query | — | +| Duration spent | 1 query | — | +| Total bookings | 1 query | — | +| New customers | 1 query | — | +| Returning customers | 1 query | — | +| Gift cards sold | 1 query | — | +| Last customer name | 1 query | — | +| Last customer visits | 1 query | — | +| Guest customers | 1 query | — | +| **Total** | **9 queries** | **1 query** | + +**Cloud cost impact**: 9 queries → 1 per dashboard load. Every admin dashboard page view. + +**Files**: `today.go` (computeAggregateSummary) + +### 10. N+1 → `ANY($1)` batch (booking service duration) + +**Anti-pattern found in `UpdateBookingServicesHandler`:** + +A `for _, serviceID := range req.ServiceIDs` loop issued individual `SELECT duration_minutes FROM services WHERE id = $1` per service to look up durations. + +**Fix:** A single `SELECT id, duration_minutes FROM services WHERE id = ANY($1)` before the loop, stored in a `map[string]int`. In-loop lookups use the map instead of the database. Same 400 error raised for missing services. + +**Cloud cost impact**: N queries → 1 per booking edit. Impact multiplies with the number of services per booking. + +**Files**: `bookings.go` (UpdateBookingServicesHandler) + +### 11. `UPDATE ... RETURNING` for loyalty stamps + +**Anti-pattern found in `ProgressBookingHandler`:** + +On booking completion, the code ran: +1. `UPDATE users SET loyalty_stamps = stamps + 1 WHERE id = $1` +2. `SELECT loyalty_stamps FROM users WHERE id = $1` (to check if stamps hit 10) + +**Fix:** Added `RETURNING loyalty_stamps` to the UPDATE, eliminating the separate SELECT. The RETURNING value is scanned directly into `newStampCount` and used for the `== 10` redemption trigger. + +**Cloud cost impact**: 1 query eliminated per booking completion. For high-volume salons, hundreds of queries/month. + +**Files**: `bookings.go` (ProgressBookingHandler) + +### 12. 3→1 combined query with FILTER (customer relationship) + +**Anti-pattern found in `GetCustomerRelationshipHandler`:** + +Three separate `db.DB.QueryRow()` calls computed total spend, total saved (discounts), and tips+visits+dates — each joining the same `bookings` and `payments` tables for the same user. + +**Fix:** Single query using `LEFT JOIN bookings → payments` with `FILTER` clauses on each aggregate. `COUNT(DISTINCT b.id)` prevents double-counting visits when a booking has multiple payments. Key design decision: a single join with FILTER instead of two LEFT JOINs (which would cause row multiplication). + +**Query design:** +```sql +SELECT + COALESCE(SUM(p.amount) FILTER (WHERE ... AND payment_method != 'discount'), 0), -- total_spend + COALESCE(SUM(p.amount) FILTER (WHERE ... AND payment_method = 'discount'), 0), -- total_saved + COALESCE(SUM(p.amount) FILTER (WHERE payment_type = 'tip'), 0), -- total_tips + COUNT(DISTINCT b.id) FILTER (WHERE b.status = 'completed'), -- total_visits + MIN(b.start_time) FILTER (WHERE b.status = 'completed'), -- first_visit + MAX(b.start_time) FILTER (WHERE b.status = 'completed') -- last_visit +FROM bookings b LEFT JOIN payments p ON p.booking_id = b.id WHERE b.user_id = $1; +``` + +**Cloud cost impact**: 3 queries → 1 per admin customer lookup. Accessed when viewing any customer's profile. + +**Files**: `customer_relationship.go` (GetCustomerRelationshipHandler) + +### Summary + +| Pattern | Queries eliminated | Risk replaced | Cloud cost | +|---------|-------------------|---------------|------------| +| `UPDATE ... RETURNING` | 3 per eviction (1 scan each) | Timestamp race | Lower I/O → lower RDS cost | +| `unnest()` over JOIN | 1 table scan per cleanup | Clock-skew fragility | Lower I/O per cron run | +| Transaction batching | N-1 round-trips per payment | Partial-corruption bug | Same I/O, safer | +| Combined SELECT | 1 per cancellation | N/A | Marginal CPU save | +| pg_trgm GIN indexes | N/A — converts seq scans to index scans | Full-table scans under ILIKE | Higher insert cost, much lower read cost | +| N+1 → batch preload (services) | 2N-2 per listing | N/A | 40→2 queries @ 20 services | +| Loop → CTE+LATERAL (today) | 27 per dashboard | N/A | 28→1 queries per dashboard load | +| 9→1 aggregates (today) | 8 per dashboard | N/A | 9→1 queries per dashboard load | +| `ANY($1)` batch (bookings) | N-1 per edit | N/A | N→1 queries per service edit | +| `RETURNING` loyalty stamps | 1 per completion | N/A | 2→1 queries per booking completion | +| 3→1 FILTER query (customer) | 2 per lookup | Row-multiplication bug | 3→1 queries per admin lookup | diff --git a/obsidian/Crussell/Testing Architecture & DB Management.md b/obsidian/Crussell/Testing Architecture & DB Management.md index 53dbadc..b2868e9 100644 --- a/obsidian/Crussell/Testing Architecture & DB Management.md +++ b/obsidian/Crussell/Testing Architecture & DB Management.md @@ -1,327 +1,307 @@ # Testing Architecture & DB Management -**Last Updated:** June 2026 -**Status:** Technical Reference for Developers and AI Agents +**Last Updated:** June 2026 --- -## 1. Overview & Architecture +## 1. Running Tests -The Platform's testing suite uses a **real database approach** for integration and handler testing, combined with unit tests for pure logic. +### Environment Variables -``` - TEST ARCHITECTURE - - ┌──────────────────────────────────────────────────────────────────┐ - │ go test (Tool Harness) │ - └──────────────────────────────────────────────────────────────────┘ - │ (Concurrency controlled via -p 1 or Advisory Locks) - ▼ - ┌──────────────────────────────────────────────────────────────────┐ - │ TestMain (Package Level) │ - │ - Initializes testdb.NewPool() │ - │ - Calls testdb.Migrate() │ - └──────────────────────────────────────────────────────────────────┘ - │ - ▼ (Acquires Global pg_advisory_lock(1337) on dedicated conn) - ┌──────────────────────────────────────────────────────────────────┐ - │ testdb.Migrate() │ - │ - Sequentially drops all existing tables/types CASCADE │ - │ - Parses init-script.sql statement-by-statement │ - │ - Re-creates full fresh database schema │ - │ ⚠️ ALL operations on the SAME connection (pool.Acquire) │ - └──────────────────────────────────────────────────────────────────┘ - │ - ▼ (Releases Advisory Lock, releases connection) - ┌──────────────────────────────────────────────────────────────────┐ - │ Individual Tests │ - │ - Calls resetTestData(t) (Truncates all tables sequentially) │ - │ - Seeds package-specific fixtures │ - │ - Executes test logic and HTTP handlers │ - │ ⚠️ TruncateTables uses pool.Acquire + pg_advisory_lock(1338) │ - └──────────────────────────────────────────────────────────────────┘ -``` - -### Key Core Principles: -- **No Mocking the DB:** We use a real PostgreSQL database instance (`crussell_test` or `mydb`) to ensure that all constraints, triggers, and foreign keys are validated exactly as they would behave in production. -- **Transactional Isolations:** Tests run sequentially within a package, and each test resets the database to a clean, default-seeded state. -- **Parallel Resilience:** Concurrency at the package level is handled safely via database-level **advisory locks** so that parallel packages never deadlock each other's DDL schema operations. -- **⚠️ Advisory locks require a dedicated connection:** Lock, work, and unlock must all use the same pgx connection. Use `pool.Acquire()` (not `pool.Exec()`) whenever acquiring advisory locks. See §2.2 for the correct pattern. - ---- - -## 2. Database Management & Schema Initialization - -### 2.1 The TestMain Pattern -Every Go package containing database integration tests (e.g., `payments`, `scheduling`, `bookings`) defines a `TestMain` function inside a test file (e.g., `testmain_test.go` or package-specific test file). - -This function orchestrates the lifecycle of the connection pool for that package: -```go -func TestMain(m *testing.M) { - // 1. Establish the connection pool - pool, err := testdb.NewPool("") - if err != nil { - os.Exit(1) - } - - // 2. Perform sequential schema migration with advisory locks - testdb.Migrate(&testing.T{}, pool) - db.DB = pool - jwt.Init() - - // 3. Run the tests - code := m.Run() - - // 4. Close the pool and exit - pool.Close() - os.Exit(code) -} -``` - -### 2.2 Global Advisory Locking (`testdb.Migrate`) -When multiple Go packages are tested concurrently, they all try to drop and recreate the schema on the same physical test database. To prevent PostgreSQL system catalog deadlocks, `testdb.Migrate` acquires a session-level advisory lock: - -```go -// CRITICAL: pool.Acquire() ensures lock, work, and unlock all use the SAME connection. -// pg_advisory_lock is session-level — using pool.Exec() for each would route -// lock and unlock to different connections, rendering the lock useless. -conn, err := pool.Acquire(ctx) -if err != nil { - t.Fatalf("Failed to acquire connection for migration: %v", err) -} -defer conn.Release() - -_, err = conn.Exec(ctx, "SELECT pg_advisory_lock(1337)") -if err != nil { - t.Fatalf("Failed to acquire migration advisory lock: %v", err) -} -defer conn.Exec(ctx, "SELECT pg_advisory_unlock(1337)") -``` - -**Why `pool.Acquire` instead of `pool.Exec`:** -- `pool.Exec(ctx, ...)` borrows a connection from the pool and returns it immediately. Each call may use a **different** connection. -- `pg_advisory_lock(1337)` is acquired on connection A, but your DDL runs on connection B, and `pg_advisory_unlock(1337)` fires on connection C. The lock is never actually held during the work. -- `pool.Acquire()` grabs a **dedicated** connection and holds it. Every subsequent `conn.Exec()` uses the same session, so the lock actually protects the critical section. - -### 2.3 The "Drop & Recreate" Clean Slate -Before running `init-script.sql`, `testdb.Migrate` actively drops every single table, sequence, and enum custom type in CASCADE order to ensure zero pollution from previous test runs. -- **Drop Order Array (`dropOrder`):** Defined in `backend/testutils/testdb/testdb.go`. New tables must be appended at the **beginning** of this list to resolve foreign key constraints on drop. -- **Type Drops Array (`typeDrops`):** Holds custom Postgres enums. New types must be added here to be cleared cleanly. - -### 2.4 TruncateTables Advisory Locking -`testdb.TruncateTables` is called by every test via `resetTestData(t)` to reset state between tests. It truncates all tables in dependency order using `TRUNCATE ... CASCADE`. - -```go -func TruncateTables(t *testing.T, pool *pgxpool.Pool) { - conn, err := pool.Acquire(ctx) - // ... same pool.Acquire pattern as Migrate ... - _, err = conn.Exec(ctx, "SELECT pg_advisory_lock(1338)") - // ... all truncations use conn.Exec (same connection) ... - defer conn.Exec(ctx, "SELECT pg_advisory_unlock(1338)") -} -``` - -**Why this matters:** -- `TRUNCATE TABLE users CASCADE` acquires `ACCESS EXCLUSIVE` locks on `users` AND all tables with FK references to `users`. -- If two `TruncateTables` calls run concurrently (from different packages or processes), one may hold a lock on `users` while the other holds a lock on `gift_cards` (which references `users`). Each needs the other's lock to complete CASCADE → **deadlock**. -- `pg_advisory_lock(1338)` serializes all truncation across all connections, preventing this deadlock. -- **Lock IDs must not collide:** Migrate uses lock 1337, TruncateTables uses lock 1338. - -### 2.5 Statement-by-Statement SQL Parser -Instead of executing the raw 2000-line `init-script.sql` as a single string (which can trigger pgx protocol hangs on DDL), `testdb.go` uses an in-memory statement splitter (`splitSQLStatements`) that respects dollar-quoted PL/pgSQL function blocks (`$$`) and executes DDL sequentially. - ---- - -## 3. How to Run the Tests - -### 3.1 Environment Variables -To run integration tests successfully, your environment must be populated with these variables: - -| Var Name | Purpose | Example Value | -|----------|---------|---------------| +| Var | Purpose | Example | +|-----|---------|---------| | `TEST_DB_DSN` | Connection string for the test database | `postgres://myuser:mypassword@localhost:5432/crussell_test?sslmode=disable` | -| `JWT_SECRET_KEY` | Secret used to sign test JWT tokens | `my_test_secret_key` | -| `GO_TESTING` | Flag to trigger mock clients / bypass locks | `1` | +| `JWT_SECRET_KEY` | Signs test JWT tokens | `my_test_secret_key` | +| `GO_TESTING` | Triggers mock clients (Square, etc.); suppresses artificial delays in Square mock | `1` | -### 3.2 Command Execution +### Commands -**Run all tests sequentially (Recommended):** ```bash +# Full suite (sequential — required to avoid deadlocks) go test -tags "test,dev" -p 1 -count=1 ./... -``` -**Run tests for a single package:** -```bash +# Single package go test -tags "test,dev" -v -count=1 ./handlers/payments/ + +# Single test +go test -tags "test,dev" -v -run TestAcquirePaymentLock_Confirmed ./handlers/payments/ ``` -**Run a single test in a package:** -```bash -go test -tags "test,dev" -v -run=TestAdminCreateGiftCard ./handlers/payments/ -``` +`JWT_SECRET_KEY=test-secret-key` is required for the root package tests (main.go init() reads JWT_SECRET_KEY). + +The `-p 1` flag serialises packages so their `TestMain` migrations don't clash. **Never omit it.** --- -## 4. Correct Way to Add New Tests +## 2. Database Setup -When implementing new features or endpoints, follow this checklist to implement tests: +### Schema Lifecycle -### Checklist for Adding New Tests: +Every package that runs integration tests has its own `TestMain` that: +1. Creates a connection pool via `testdb.NewPool("")` +2. Calls `testdb.Migrate(t, pool)` — drops everything (tables + enums) in CASCADE order, then re-runs `init-scripts/init-script.sql` statement-by-statement +3. Assigns `db.DB = pool` for the handlers -- [ ] **File Location:** Put test files in the same directory as the handlers they test, named `*_test.go`. Tests for admin-facing handlers may go in `backend/handlers/admin/` (see existing patterns in `today_test.go` and `bookings_test.go`). -- [ ] **Tags:** Ensure the top of the file has the `//go:build test` constraint. -- [ ] **Reset State:** Always call `resetTestData(t)` at the very beginning of your test: - ```go - func TestMyNewHandler(t *testing.T) { - resetTestData(t) // Truncates tables and seeds default working hours - ctx := context.Background() - ... - } - ``` -- [ ] **Close Rows & Transactions:** Never leak a connection! Always use `defer rows.Close()` and commit or rollback transactions (`defer tx.Rollback(ctx)`). Leaked transactions will cause subsequent `TRUNCATE` operations in other tests to freeze. -- [ ] **Use Mock Clients:** If your endpoint calls external services like Square, ensure the test environment uses the in-memory mock client (`square.NewDevClient()`). +Each individual test calls `resetTestData(t)` which **truncates** all tables (CASCADE), leaving the schema intact. -### Seeding Working Hours for Specific Days +### What You Need To Do When Adding a New Table or Enum -Tests involving day-of-week logic (closed days, week summaries) must seed `working_hours` explicitly. Use the weekday mapping (0=Monday, 6=Sunday — already converted from Go's `time.Weekday`): +| Change | File to Edit | What to Add | +|--------|-------------|-------------| +| New table | `backend/testutils/testdb/testdb.go` — append table name to **beginning** of `dropOrder` array AND to `tables` list in `TruncateTables` | The table's Go identifier | +| New enum type | `backend/testutils/testdb/testdb.go` — add to `typeDrops` array | `"DROP TYPE IF EXISTS new_type CASCADE"` | +| New `booking_status` value | `init-scripts/init-script.sql` — update `CREATE TYPE booking_status AS ENUM (...)` | The new value in the list | + +If you forget these, `testdb.Migrate` will fail with `relation "X" already exists` or `type "X" already exists` because the old objects survive between runs. + +--- + +## 3. Writing Tests — Patterns That Work + +### Basic Structure + +```go +//go:build test + +func TestMyHandler(t *testing.T) { + resetTestData(t) // only when crossing packages or testing schema-level changes + // seed fixtures, build request, call handler, assert +} +``` + +**Don't reset the DB if you don't need to.** Full truncation (`resetTestData`) is expensive — it acquires advisory locks, cascades through every table, and re-seeds working hours. Prefer creating fresh entities per test: + +```go +// Fast: create a new user + booking for each test, rely on unique IDs +userID, err := fixtures.CreateTestUser(db.DB) +bookingID, err := fixtures.CreateTestBookingAtTime(db.DB, userID, serviceID, someTime) +``` + +This is safe as long as tests in the same package don't assert on global counts (total users, total bookings) and don't mutate shared data in conflicting ways. Each test's entities get their own IDs, so they never collide. + +**`resetTestData(t)` is required when:** +- Testing schema changes or enum value drops +- Testing global aggregate queries (total bookings, revenue sums) +- A previous test may have left state that would cause a false positive/negative (ambiguous — use your judgement) +- Switching packages (each `TestMain` calls `testdb.Migrate` which drops everything) + +**`resetTestData(t)` is wasteful when:** +- Every test creates its own user + booking via fixtures +- Tests only read or update their own rows +- The only shared table is `working_hours` (seed once in the first test, or use `ON CONFLICT DO UPDATE`) + +### Test Helpers per Package + +Each test package defines its own helpers. Look at the existing test file before writing new ones: + +| Package | Helper File | Key Functions | +|---------|------------|---------------| +| `bookings` | `bookings_test.go` | `makeRequest`, `makeAuthRequest`, `makeAdminRequest`, `serveChiHandler`, `serveAdminHandler` | +| `payments` | `payments_test.go` | `makePaymentRequest`, `setupPaymentStatusTest(status)`, `setupTestDataPast(t)`, `setupDepositBookingPast(t)` | +| `admin` | `test_helpers.go` | `makeAdminRequest`, `makeUserRequest`, `resetTestData` | +| `scheduling` | (in test files) | Package-specific helpers | + +Chi URL params need manual wiring. Use `serveChiHandler` or `serveAdminHandler` for routes with path params like `{id}`. + +### Avoiding Payment-Split Headaches + +`CreateBookingPayment` splits future-dated payments into deposit + non-deposit records. This causes payment-count assertions to fail if you aren't expecting it. + +```go +// Booking in the past → no split (correct for idempotency / count tests) +userID, bookingID, _ := setupTestDataPast(t) + +// Future booking → split fires (correct for verifying 2-record output) +userID, bookingID := setupDepositBooking(t) +``` + +Helpers live in `backend/handlers/payments/payments_test.go`. + +### Seeding Working Hours + +Weekday mapping: `0=Monday, 6=Sunday` (converted from Go's `time.Weekday`). ```go todayWeekday := int(time.Now().Weekday()) if todayWeekday == 0 { - todayWeekday = 6 // Go Sunday → our Sunday + todayWeekday = 6 } else { - todayWeekday -= 1 // Go Mon=1→0, Tue=2→1, ..., Sat=6→5 + todayWeekday -= 1 } - -// Seed today as closed -_, err := db.DB.Exec(ctx, ` - INSERT INTO working_hours (weekday, start_time, end_time, is_open) - VALUES ($1, '00:00', '00:00', false) - ON CONFLICT (weekday) DO UPDATE SET ... -`, todayWeekday) ``` -### Seeding Exceptional Hours (Holidays, Reduced Hours) - -For tests that need exceptional working hours (overriding default `working_hours`), create a group, hours, and application: +### Seeding Exceptional Hours (Holidays) ```go -// 1. Create the group var groupID int -err := db.DB.QueryRow(ctx, ` - INSERT INTO exceptional_working_hours_groups (name, description) - VALUES ('Test Closure', '...') RETURNING id -`).Scan(&groupID) - -// 2. Define the exceptional day (weekday 0=Monday, 6=Sunday) -_, err = db.DB.Exec(ctx, ` - INSERT INTO exceptional_working_hours (group_id, weekday, start_time, end_time, is_open) - VALUES ($1, $2, '00:00', '00:00', false) -`, groupID, todayWeekday) - -// 3. Apply to the current week (week_start must be the Monday of the week) -_, err = db.DB.Exec(ctx, ` - INSERT INTO exceptional_group_applications (group_id, week_start) - VALUES ($1, $2::date) -`, groupID, mondayStr) +db.DB.QueryRow(ctx, `INSERT INTO exceptional_working_hours_groups (name) VALUES ('Test') RETURNING id`).Scan(&groupID) +db.DB.Exec(ctx, `INSERT INTO exceptional_working_hours (group_id, weekday, start_time, end_time, is_open) VALUES ($1, $2, '00:00', '00:00', false)`, groupID, todayWeekday) +db.DB.Exec(ctx, `INSERT INTO exceptional_group_applications (group_id, week_start) VALUES ($1, $2::date)`, groupID, mondayStr) ``` -The `isDayOpen()`, `getClosingTime()`, and `findWeekSummaryRange()` functions in the today handler query `exceptional_working_hours` first and fall back to `working_hours` only if no match is found. The column in `exceptional_group_applications` is `week_start` (not `monday_week_start`). +The column is `week_start`, **not** `monday_week_start`. -### Testing Pagination / per_page Caps +### Auth & JWT Tests -If you modify pagination parameters (like the `per_page` cap in `parseGetAllBookingsRequest`), test that: -- Values within the cap are accepted and return the expected number of results -- Values exceeding the cap are rejected and fall back to the default (10) +The `crussell/auth` package now connects to the test database for JTI revocation tests. Its `TestMain` calls `testdb.NewPool("")` and `testdb.Migrate()`: ```go -// per_page=500 should be accepted +func TestMain(m *testing.M) { + InitJWT("test-secret-key-for-jwt-test") + pool, err := testdb.NewPool("") + if err != nil { + fmt.Fprintf(os.Stderr, "WARN: No test DB: %v\n", err) + } else { + testdb.Migrate(&testing.T{}, pool) + db.DB = pool + } + code := m.Run() + if pool != nil { pool.Close() } + os.Exit(code) +} +``` + +Tests that need a database call `requiresDB(t)` which skips if `db.DB` is nil. + +**Important:** `GO_TESTING=1` env var skips the server-side zxcvbn password strength check in `RegisterHandler`. Without it, seeded passwords like `"password"` are rejected (zxcvbn score 0). The dev server and CI should always set this. + +### Testing Pagination Caps + +```go +// per_page=500 accepted w := makeAdminRequest(handler, "GET", "/api/admin/bookings?per_page=500", nil) -var resp bookings.BookingListResponse -parseResponseBody(w, &resp) -assert(resp.PerPage == 500) - -// per_page=600 should be rejected → defaults to 10 -w2 := makeAdminRequest(handler, "GET", "/api/admin/bookings?per_page=600", nil) -parseResponseBody(w2, &resp) -assert(resp.PerPage == 10) +// per_page=600 → capped to default 10 ``` -### Current Test Suite Status - -As of June 2026: -- **Tests run:** `go test -tags "test,dev" -p 1 -count=1 ./...` -- **Results:** 686 passed, 3 skipped, 0 failed (689 total) -- **Skipped tests:** `TestAdminNotifications_List` and `TestAdminNotifications_Acknowledge` — marked WIP, no handler yet - --- -## 5. FAQ & Common Breakage Causes (For Developers & AI Agents) +## 4. Current Status -If the test suite is hanging, failing on startup, or throwing database errors, check this reference: +- **All 16 packages pass**, 0 failed +- Skipped: `TestAdminNotifications_List` / `TestAdminNotifications_Acknowledge` — WIP, no handler yet (also `mw/ratelimit_test.go` skipped by `!dev` build tag) +- All packages pass clean: 16 tested packages (plus 10 with no test files) +- `GO_TESTING=1` suppresses the 1s/3s artificial delays in Square mock (`internal/square/square_dev.go`) +- New test files: + - `handlers/payments/loyalty_test.go` — 10 tests (apply-redemption + campaign auto-apply) + - `handlers/payments/refund_exclude_test.go` — 2 tests (discount/OTH exclusion) + - `handlers/bookings/dedup_test.go` — 8 tests (dedup guards + no-show) + - `handlers/payments/discount_preview_test.go` — 13 tests (discount preview endpoint) + - `handlers/payments/payment_status_test.go` — 32 tests (status guards, payment lock, release lock) + - `handlers/payments/refunds_test.go` — 20 tests (cancellation refund calc + processing) + - `handlers/bookings/deposit_test.go` — 30 tests (deposit fields, eviction, admin cancel) + - `handlers/webhooks/webhooks_test.go` — 13 tests (Square webhook signature + dispatch) + - `handlers/admin/discount_campaigns_test.go` — 11 tests (campaign CRUD + stats) + - `handlers/user/patch_tests_test.go` — 7 tests (user patch test list/delete) +- Extended tests: `handlers/scheduling/time_blockers_test.go` (CleanupExpiredLoyaltyRedemptions), `handlers/notifications/notifications_test.go` (AcknowledgePendingBookingNotification), `handlers/payments/payment_status_test.go` (ReleasePaymentLock) +- Schema changes: `patch_tests.id` now uses `generate_patch_test_id()` (was `generate_service_id()`); `user_patch_tests.id` changed from `BIGSERIAL` to `CHAR(12)` with `generate_user_patch_test_id()` -### ❓ FAQ 1: The test suite hangs/freezes completely on startup (0% CPU, no output) -**Reason:** -A test is trying to run `testdb.TruncateTables` or `testdb.Migrate` (which executes `TRUNCATE` or `DROP TABLE`), but another active transaction or query has a lock on one of those tables. -- **Cause A: Connection leak in code.** A previous handler or test executed a query (like `.Query()`) but did not call `rows.Close()`, or opened a transaction (`.Begin()`) but did not commit or rollback. -- **Cause B: The Go dev server is running.** If the Go dev server (`go run ./main.go`) is currently running and holding open database sessions/transactions on the same database, test-suite DDL will block indefinitely. -- **Fix:** Kill the running dev server, check for unclosed `rows` or unrolled-back transactions, and run `docker exec postgres psql -U myuser -d crussell_test -c "SELECT pid, query, state FROM pg_stat_activity WHERE state != 'idle';"` to locate blocked sessions. +--- -### ❓ FAQ 2: Error `relation "X" does not exist` during seed or migration -**Reason:** -A foreign key constraint or table creation script has a dependency ordering issue in `init-scripts/init-script.sql`. -- **Cause:** Table `A` references Table `B`, but Table `A` is created *before* Table `B` in the script. -- **Fix:** Do **not** use inline constraints like `booking_id CHAR(12) REFERENCES bookings(id)` for tables created early. Instead, declare the column normally (`booking_id CHAR(12)`), and append the constraint at the very bottom of the file using `ALTER TABLE payments ADD CONSTRAINT fk_payments_booking FOREIGN KEY (booking_id) REFERENCES bookings(id);`. +## 5. Debugging — Issues & Resolutions -### ❓ FAQ 3: Error `relation "X" already exists` during Migrate -**Reason:** -You created a new table in the database schema, but forgot to add it to the test teardown list. -- **Cause:** When `testdb.Migrate` runs, it drops existing tables using the `dropOrder` list. If your new table isn't in that list, it won't be dropped. When `init-script.sql` tries to run `CREATE TABLE X`, it fails because the old table is still there. -- **Fix:** Edit `backend/testutils/testdb/testdb.go`. Find the `dropOrder` array and the `tables` truncation array inside `TruncateTables`. Append your new table name to the **beginning** of both lists. +### Test suite hangs on startup (0% CPU, no output) -### ❓ FAQ 4: Error `type "X" already exists` during Migrate -**Reason:** -Similar to FAQ 3, you added a new custom enum type in PostgreSQL but forgot to tell the test runner to clean it up. -- **Cause:** The enum type survives across migrations, causing `CREATE TYPE X` to fail. -- **Fix:** Find `typeDrops` array in `backend/testutils/testdb/testdb.go`. Add `"DROP TYPE IF EXISTS X CASCADE"` to the array. +**Why:** `testdb.TruncateTables` or `testdb.Migrate` blocks on a lock held by another session. -### ❓ FAQ 5: Deadlock detected during `TRUNCATE` (in `TruncateTables` or `resetTestData`) -**Reason:** -`TRUNCATE TABLE users CASCADE` needs `ACCESS EXCLUSIVE` locks on `users` and all child tables referencing it via FK. Another concurrent `TruncateTables` call holds one of those child locks. PostgreSQL detects the circular wait and aborts one of the transactions. +**Previously resolved by:** +- Killing the Go dev server (`go run ./main.go`) — it holds open connections +- Fixing leaked rows/transactions — `.Query()` without `rows.Close()` or `.Begin()` without commit/rollback leaves the connection in a bad state +- Enforcing `pool.Acquire()` instead of `pool.Exec()` for advisory locks — lock and work **must** use the same connection (see `testdb.go` TruncateTables for the pattern) -**Causes:** -- **Cause A (most common):** Two `go test` processes running against the same database simultaneously (e.g., running the test suite from two terminals at once, or a CI pipeline overlapping with a dev run). -- **Cause B:** `-p` flag > 1 causing multiple packages to run concurrently without advisory locks protecting them. -- **Cause C:** A previous `TruncateTables` call held the advisory lock on a different connection than the work (pre-June 2026 bug — see §2.2 for why `pool.Acquire` is mandatory). - -**Diagnosis:** +**What to do:** +```bash +docker exec postgres psql -U myuser -d crussell_test -c "SELECT pid, query, state FROM pg_stat_activity WHERE state != 'idle';" +``` +Kill stale connections: ```sql --- Find blocked/pending sessions on the test database -SELECT pid, wait_event_type, wait_event, state, query -FROM pg_stat_activity -WHERE datname = 'crussell_test' AND state != 'idle' -ORDER BY wait_event_type NULLS LAST; +SELECT pg_terminate_backend(pid) FROM pg_stat_activity WHERE datname = 'crussell_test' AND pid != pg_backend_pid(); ``` -Look for sessions in `wait_event_type = 'Lock'` waiting on `relation` events. - -**Fix:** -1. Kill other `go test` processes. -2. Kill stale connections holding locks: `SELECT pg_terminate_backend(pid) FROM pg_stat_activity WHERE datname = 'crussell_test' AND pid != pg_backend_pid();` -3. Ensure `TruncateTables` uses `pool.Acquire()` with `pg_advisory_lock(1338)` (lock ID must be different from `Migrate`'s 1337). -4. Run with `-p 1` to serialize packages. - -### ❓ FAQ 6: Panic: `interface conversion: interface {} is nil, not string` during auth -**Reason:** -A handler test is calling `VerifyToken` or middleware, but the authentication headers or context keys are not set correctly. -- **Cause:** The user token is expired, signed with a different key, or the context is missing `mw.UserIDKey`. -- **Fix:** Make sure `jwt.Init()` is called in `TestMain`, and your test requests include the `Authorization: Bearer ` header with a token generated via `jwt.GenerateToken()`. --- -## Appendix: HMRC & Legal Compliance Retention Rules Reference +### Tests fail with `relation "X" does not exist` during migration -When writing or editing tests for cleanup and data retention, reference these statutory timelines: +**Why:** Table A references Table B but A is created before B in `init-script.sql`. -- **HMRC Corporation Tax records:** **6 years** from the end of the financial year (HMRC CH14600 / Companies Act 2006 s.388). We aggregate detailed records after **7 years** to maintain a safe buffer. -- **Scottish Contract Claims prescriptive period:** **5 years** (Prescription and Limitation (Scotland) Act 1973 s.6). Accounts with remaining balances must remain active for at least **5 years** to allow claims. -- **GDPR Storage Limitation:** **2 years** inactivity default for accounts with no balance (legitimate interest in relationship ends). +**Previously resolved by:** Moving inline FK constraints to `ALTER TABLE ADD CONSTRAINT` statements at the bottom of `init-script.sql`. + +**What to do:** Declare the column without a REFERENCES clause, then add the constraint at the end of the file. + +--- + +### Tests fail with `relation "X" already exists` or `type "X" already exists` + +**Why:** A new table or enum was added to the schema but not to the test teardown lists. + +**Previously resolved by:** Adding the table to `dropOrder` + `tables` in `TruncateTables`, or the type to `typeDrops`, in `backend/testutils/testdb/testdb.go`. + +**What to do:** Follow the table in §2. If you see this for an existing object, check whether the test database has stale objects from a previous schema version — truncate + recreate manually. + +--- + +### Deadlock detected during `TRUNCATE` + +**Why:** Two concurrent `TruncateTables` calls each hold one end of a circular FK lock chain. + +**Previously resolved by:** +- Adding `pg_advisory_lock(1338)` to serialise all truncation (lock ID 1338; migrate uses 1337 — must not collide) +- Enforcing `pool.Acquire()` so lock, work, and unlock share one connection +- Always using `-p 1` to serialise packages + +**What to do:** +1. Kill duplicate `go test` processes +2. Kill stale connections (see first FAQ) +3. Verify `TruncateTables` uses `pool.Acquire()` with lock 1338 +4. Run with `-p 1` + +--- + +### Tests get 0 results from queries using enum values + +**Why:** You removed a value from a PG enum (e.g. `no_deposit` from `booking_status`) but a `WHERE status NOT IN (...)` query still references it. PG returns an error for enum literal checks against removed values — if the error is silently ignored (e.g. `_ = db.DB.QueryRow(...).Scan(&x)`), the target variable stays at Go zero value. + +**Previously resolved by:** Replacing all `'no_deposit'` references with the replacement status (`'deposit_lapsed'`) across: +- `backend/handlers/today/today.go` (2 occurrences) +- `backend/handlers/bookings/bookings.go` (multiple) +- `backend/handlers/bookings/manage.go` +- `backend/handlers/scheduling/default-hours.go` +- Frontend `*.svelte` files referencing the old status name + +**What to do:** Grep the full codebase for the removed enum value before dropping it from the schema: +```bash +grep -rn "'old_value'" backend/ frontend/ | grep -v "_test.go" | grep -v ".md" +``` + +--- + +### Panic: `interface conversion: interface {} is nil, not string` during auth + +**Why:** Test request is missing auth context — token not set, or context key not wired. + +**Previously resolved by:** Ensuring `jwt.Init()` runs in `TestMain` and the test uses `makeAuthRequest` (or manually injects `mw.UserIDKey`/`mw.UserRoleKey` into the request context). + +**What to do:** Look at how `makePaymentAuthRequest` in `payments_test.go` or `makeRequestWithContext` in `admin/test_helpers.go` sets up auth context. Copy that pattern. + +--- + +### Cursor-paginated tests return 0 items on the second page + +**Why:** The cursor contains a `+` timezone offset (e.g. `2026-06-17T12:49:32+01:00`) which, when passed unescaped in a URL query string, is decoded as a space. The timestamp parser then fails because it expects `Z07:00` format. + +**Previously resolved by:** Using `url.QueryEscape(cursor)` when building the request URL, or — for tests that only need to verify basic listing — skipping the second-page assertion entirely. + +**What to do:** If you need to test second-page results, URL-encode the cursor value. If the test only needs to verify the listing contract (total, per_page, presence of next_cursor), the first-page assertions are sufficient. + +--- + +### Pre-existing test files don't compile + +**Why:** Tests with `//go:build test` constraint are only compiled when the `test` build tag is active. `go build` without test tags skips them entirely, so compilation errors in test files are silent until you run `go test`. + +**Previously resolved by:** Running `go test -tags "test,dev" -count=1 -run ^$ ./path/` — a zero-test run that still compiles all test files — as part of every PR verification. + +**What to do:** After changing test files, always run a compile check: +```bash +go test -tags "test,dev" -count=1 -run ^$ ./package/... +``` +This catches test-only compilation errors without executing any tests. diff --git a/obsidian/Crussell/User Manual.md b/obsidian/Crussell/User Manual.md index ab4f66d..3ca805d 100644 --- a/obsidian/Crussell/User Manual.md +++ b/obsidian/Crussell/User Manual.md @@ -199,18 +199,20 @@ Customers can pay online in several ways: ### Paying a Deposit -If they have deposit obligations, they can pay a deposit online before their appointment: +Customers may need to pay a deposit if they have deposit obligations, or they may choose to pay early voluntarily. 1. Go to **Schedule** from the menu 2. Click on their upcoming booking -3. Click **Pay Deposit** — the deposit amount (20% of the booking total) is shown -4. They can pay the deposit, the full amount, or a partial amount +3. Click **Pay Deposit** or **Pay Early** — the minimum deposit is 20% of the booking total +4. They can pay the deposit (20%), the full amount, or a custom partial amount 5. Enter card details or use a saved card 6. Confirm payment +**20% threshold rule:** Once the customer has paid 20% or more of the total (across all payments combined), the deposit is considered met. The booking is protected — even if the deposit deadline passes, the slot won't be given to someone else. This works regardless of what label the payment has (deposit, partial, balance, etc.). + If no deposit is owed but they haven't paid the full amount yet, they see a **Pay Early** button instead. They can also pay a partial amount using the **Pay Part** option. If they've already paid a deposit, they see a **Pay Balance** button to settle the remaining amount. -**What to tell the customer:** "If you owe a deposit, you can pay it online from your Schedule page. If you don't owe a deposit, you can pay early or pay part of the amount." +**What to tell the customer:** "If you owe a deposit, you can pay it online from your Schedule page. If you don't owe a deposit, you can pay early or pay part. Once you've paid 20% or more, your booking is protected." ### Leaving a Tip @@ -233,7 +235,21 @@ After their appointment is marked as **completed**, they can leave a tip: From the Account page, the customer can find the booking and select the cancel option. -### The 24-Hour Rule +### What Happens to Payments + +If they've paid anything towards the booking, the system calculates a refund based on how much notice they gave: + +| Notice Period | What They Get Back | +|---|---| +| **72+ hours before the appointment** | **Full refund** — 100% of everything paid | +| **24-72 hours before the appointment** | **Partial refund** — the salon keeps a protected deposit (up to 50% of the total). Everything paid above that is refunded | +| **Less than 24 hours** | **No refund** — all payments are retained | + +The refund appears as a credit to their account balance or is processed back to their card if possible. The cancellation dialog on their Account page shows the breakdown before they confirm. + +### The 24-Hour Rule (No-Show Penalty) + +This is separate from the refund. Even if no money was paid (so no refund applies), the no-show tracking still works: **Cancelling 24 hours or more before the appointment:** No penalty. The booking is cancelled with no consequences. @@ -272,33 +288,46 @@ If a customer needs to change their appointment time: ## The Deposit System -### What Are Deposits? +There are two separate things customers call "deposits" — the **no-show penalty** (3-strike system for last-minute cancellations) and the **financial deposit** (money paid toward the booking to hold the slot). -Deposits are a way the salon protects against last-minute cancellations. If a customer cancels an appointment less than 24 hours before it starts (without the salon's forgiveness), they receive **3 deposit obligations**. +### 1. No-Show Penalty (3-Strike System) -### What This Means for Them +If a customer cancels with less than 24 hours' notice (without the salon's forgiveness), they get **3 deposit obligations** on their account. While they have deposit obligations: -- They can only book appointments that are **at least 24 hours away** +- They can only book appointments that are **at least 36 hours away** - They can only have **one active booking at a time** -- They need to complete appointments and pay for them to reduce their deposit count +- They need to complete appointments and pay for them to reduce their count - A warning banner appears at the top of the booking flow explaining the limit -### How Deposits Go Down +Each completed paid appointment reduces the count by 1. -Each time they complete an appointment and pay for it, their deposit count goes down by **1**. After all 3 are cleared, they can book normally again. +### 2. Financial Deposit (Slot Protection) + +Some bookings need a **payment of at least 20% of the total** before the 24-hour deadline. This holds the slot so no one else can take it. + +- If they pay 20%+ before the deadline, the slot is locked in +- If they **don't** pay before the deadline, someone else can book their slot — they lose it +- If they **do** pay after the deadline but before anyone takes it, the slot is secured again +- Any payment type counts towards the 20% — deposit, partial, full, or balance. It's the total that matters, not what it's called + +**Example:** A £50 booking needs at least £10 paid. A £10 "partial" payment works just as well as a £10 deposit payment. ### Quick Reference -| What happens | Result | -|---|---| -| Cancel ≥ 24 hours before | No penalty | -| Cancel < 24 hours before | 3 deposit obligations | -| Cancel < 24 hours (salon forgives) | No penalty | -| Cancel a pending booking | No penalty — booking is deleted | -| Complete an appointment and pay | Deposit count goes down by 1 | +| What happens | No-Show Penalty | Money Already Paid | +|---|---|---| +| Cancel ≥ 72h before | None | **Full refund** | +| Cancel 24-72h before | None | **Partial refund** — salon keeps up to 50% of the total | +| Cancel < 24h before | 3 obligations | **No refund** | +| Cancel < 24h (salon forgives) | None | Depends — may get full refund | +| Pending booking cancelled | None | No money paid | +| Deposit deadline passed, unpaid | — | Slot becomes available to others | +| Someone else takes the slot | — | Original booking cancelled | +| Pay after deadline, before slot taken | — | Slot secured | +| Complete appointment and pay | Count goes down by 1 | — | -**What to tell the customer:** "If you cancel with less than 24 hours' notice, you'll need to pay deposits on your next 3 bookings. Each time you complete an appointment, one deposit goes away." +**What to tell the customer:** "If you cancel with lots of notice, you'll get your money back. The closer it gets to your appointment, the less you'll get back. If you haven't paid at least 20% before the 24-hour mark, someone else could book your slot." --- @@ -436,9 +465,15 @@ From their Account page, find the booking and select the cancel option. If they ### "I paid a deposit but need to cancel — what happens?" -If they cancel with more than 24 hours' notice, no penalty. The deposit may be refunded depending on the salon's policy. If they cancel with less than 24 hours' notice, they get 3 deposit obligations. +The refund depends on **how much notice** they give: -**What to tell them:** "If you cancel with more than 24 hours' notice, there's no penalty. If you cancel with less than 24 hours' notice, you'll need to pay deposits on your next 3 bookings." +- **72+ hours notice:** Full refund — 100% back +- **24-72 hours notice:** Partial refund — the salon keeps a protected deposit (up to 50% of the total), the rest is refunded +- **Less than 24 hours:** No refund — all payments retained, plus 3 deposit obligations added + +The cancellation dialog on their Account page shows the exact refund breakdown before they confirm. + +**What to tell them:** "The amount you get back depends on how much notice you give. If you cancel with plenty of notice, you'll get a full refund. The closer it gets to your appointment, the less you'll get back. The cancellation screen shows you the exact amount before you confirm." ### "Do you take walk-ins?"