Files
NextChair/docs/phase-4-summary.md
T
Stefan Lazic 6ae145d187 Phase 4
Seed data extended with a second barber (different hours/services), a beard-trim service + category, breaks, time off, a guest customer, and appointments in all three key states — verified against a live throwaway Postgres.
14 new integration tests (38 total with Phase 3's) covering guest/authenticated booking, any-barber assignment, slot-conflict handling, ownership enforcement, guest-data isolation, and book-again repricing.
Playwright E2E suite for the 6 required customer journeys, with its own global-setup.ts that migrates and seeds a database automatically.
Full verification pass, which caught and fixed three real bugs along the way (not just test-workarounds):
A date off-by-one bug in the calendar picker that made real weekdays show as closed for anyone in a UTC+ timezone (including the business's own).
A dialog that could vanish mid-confirmation on cancellation, before the customer could read the late-cancellation notice.
A silent test-cleanup leak in the Phase 3 fixtures that had been leaking stray businesses since Phase 3.
Plus a baseline desktop-responsive fix (centered max-w-2xl columns) across Home/Appointments/Settings/booking flow, since desktop had been effectively unstyled.
Final state: 90 unit tests, 38 integration tests, and all 6 E2E journeys passing together on a fresh database; clean tsc, eslint, and next build. Full writeup with routes, security model, guest-matching rules, known limitations, and what to check before Phase 5 is in docs/phase-4-summary.md.
2026-08-02 21:39:24 +01:00

128 lines
16 KiB
Markdown

# Phase 4 — Customer Booking Experience: Summary
Full guest/registered customer journey — browse the business, pick services, pick a barber/date/time, enter details, review, confirm, and manage the resulting appointment (view, reschedule, cancel, book again). Every business rule is enforced by the Phase 3 domain services; nothing in this phase re-implements availability, validation, or booking logic.
---
## 1. What was built
- **Customer Home** (`/home`) — real business data: hero, opening status, description, services preview, barber list, reviews (demo-labelled), opening hours, contact links. For a signed-in customer with a profile: greeting, upcoming-appointment card, loyalty progress (if the programme is enabled), preferred barber, and a "Book Again" shortcut. Guests see the same public page with no personalised sections.
- **Full booking flow** (`/book → /book/barber-time → /book/details → /book/review → /book/confirmation`) — multi-service selection, specific-barber-or-"anyone available" selection, a shared calendar/time-slot picker, guest-or-authenticated details capture, a review step with edit-any-step links, and a confirmation screen with add-to-calendar, guest account-creation prompt (never forced), and next-step actions.
- **Customer Appointments** (`/appointments`) — Upcoming/Past/Cancelled tabs backed by real data, per-card actions gated by status (View/Reschedule/Cancel/Book Again).
- **Appointment Details** (`/appointments/[id]`) — full appointment record, cancellation details when applicable, calendar export, status-appropriate actions.
- **Reschedule** (`/appointments/[id]/reschedule`) — reuses the same barber/date/time picker as new bookings, single-step, redirects to the appointment's details page on success.
- **Cancel** — a Modal-based confirmation dialog (optional reason, late-cancellation notice), never deletes the appointment.
- **Book Again** — starts a new booking with the previous services (and barber, if still bookable) preselected; always requires a fresh date/time and always reprices at current service prices.
- **Add to Calendar** — an RFC 5545 `.ics` file generated server-side, served from a Route Handler, ownership-checked like every other appointment access.
- **Reviews** — a provider-abstracted demo dataset, clearly labelled "Demo data" in the UI, so a real Google Reviews integration can replace it later without touching the Home page.
- **Guest identity** — a documented, conservative matching strategy (see §7) resolving which `CustomerProfile` a guest booking belongs to, without ever attaching a booking to an existing account without verification.
## 2. Routes created / updated
New:
- `/book`, `/book/barber-time`, `/book/details`, `/book/review`, `/book/confirmation` (own layout, outside `(customer)`, no bottom nav — guests must reach it)
- `/appointments/[id]`, `/appointments/[id]/reschedule` (reschedule is deliberately **outside** the `(customer)` route group too — see §12)
- `/api/appointments/[id]/calendar.ics` (the one legitimate Route Handler this phase; everything else is a Server Action)
Updated:
- `/home` — rewritten from a placeholder to real data
- `/appointments` — rewritten from a placeholder to real data
- `/settings` — added a guest sign-in prompt (previously always required auth via `proxy.ts`)
- `(customer)/layout.tsx` and `src/proxy.ts` — `/home`, `/appointments`, `/settings` are no longer hard-gated by the proxy; each page now shows its own in-context sign-in prompt for a guest instead of a blanket redirect, because Home must render a public, unauthenticated experience
## 3. Components created
Feature components (`src/features/booking/components/`): `ServiceSelector`, `BarberDateTimePicker` (shared by booking **and** rescheduling), `DetailsForm`, `ConfirmBookingButton`, `CancelAppointmentDialog`, `BookAgainButton`.
Supporting: `src/lib/business/get-primary-business.ts`, `src/lib/business/opening-status.ts`, `src/lib/booking-draft/*` (cookie, schema, HMAC signing), `src/lib/calendar/ics.ts`.
No new design-system primitives were needed — Phase 2's Button/Input/Select/Checkbox/Textarea/Modal/Card/Tabs/EmptyState/StatusPill/AppointmentCard covered everything.
## 4. Domain-service changes
Minimal, on the "genuine missing capability" bar the brief set:
- **`BookingService.CancelBookingInput.cancelledByUserId`** widened from required to optional — a guest `CustomerProfile` has no linked `User`, so a guest cancelling their own booking has nothing to attribute it to. Every existing caller that already passes a `userId` is unaffected.
- **New `CustomerService`** (`src/services/customer/customer.service.ts`) — anticipated by Phase 3's own "future services" list. Owns exactly one thing: resolving which `CustomerProfile` a booking belongs to (see §7). Calls no other domain service and is called by, not merged into, `BookingService`.
- **`BookingValidationErrorCode`** gained one entry, `INVALID_INPUT`, for malformed Server Action input that never reaches a real validation check.
- **`src/features/booking/authorization.ts`** (new, not a domain service) — `assertCanAccessAppointment` / `canAccessAppointment`. Deliberately kept in the feature layer, not `AppointmentService`, because ownership rules differ for future staff/owner tooling ("any staff member in this business") — see the file's own header comment.
No changes to `AvailabilityService`, `SchedulingService`, `StaffAvailabilityService`, or `BusinessRulesService`.
## 5. Prisma schema changes
None beyond what Phase 3 already had. `cancelledByUserId` was already a nullable column; only the TypeScript input type changed (§4).
## 6. Booking-state approach
An HMAC-**signed but not encrypted** cookie (`nc_booking_draft`, 1 hour, httpOnly, secure in prod, `sameSite=lax`), keyed off `AUTH_SECRET`, holding the draft (services, staff, date/time, guest details, notes, policy acknowledgement). A separate short-lived cookie (`nc_last_booking`, 30 min) carries only the newly created appointment's id to the confirmation page, so that page never needs a URL query parameter.
**Why signed-only, not encrypted:** the threat model here is *tampering*, not *disclosure* — the cookie holds the customer's own freshly-typed data, on the app's own domain, not anyone else's secrets. Signing detects tampering; the customer already has the data. Every value is still re-validated server-side at confirm time regardless of what the cookie says (see §8).
Every `/book/*` step is a Server Action that reads, mutates, and rewrites this cookie, then `redirect()`s to the next step — so a refresh, browser back, or an in-flow sign-in never loses progress, and no client-only state is load-bearing.
## 7. Guest-customer matching approach
Documented in full in `src/services/customer/customer.service.ts`; summary:
1. An authenticated caller always books under their own profile (found via `User.customerProfile`, created on first booking if missing). No matching logic runs at all — there's no ambiguity to resolve.
2. A guest caller is matched against an existing profile only if **both** email (case-insensitive, normalised in application code — not Prisma's `mode: "insensitive"`, which is Postgres/Mongo-only and would break on a MySQL swap) **and** phone match exactly, against `isGuest = true AND userId IS NULL` profiles only.
3. A guest is never matched against a registered customer's profile, even on an exact email match — that would attach a guest's booking to a real account without verification.
4. No match → a new guest profile is created. Existing-profile fields are never overwritten on a match.
5. **Guest appointment access is "order tracking" by unguessable id**, not by verified identity — no magic-link/OTP system was built this phase. This is a deliberate, documented scope limitation (see §12), not an oversight.
## 8. Security / ownership rules
- `assertCanAccessAppointment` re-checked on **every** appointment read/mutation a customer can trigger (details, reschedule, cancel, book-again, calendar export) — an appointment id is never treated as permission on its own for a registered customer's booking.
- Every mutating Server Action re-derives the session server-side; none trust a client-supplied user or customer id.
- `confirmBookingAction` resolves the customer via `CustomerService` from the session/cookie, never from client input — a customer cannot submit someone else's `customerId`.
- Price, duration, and total are never taken from the client. `BookingService.createBooking` always re-reads the current `Service` rows and re-validates the slot inside its own transaction at commit time (this is Phase 3 behaviour, unchanged — Phase 4 never bypasses it).
- No sensitive customer data is placed in a URL — see §6.
- Raw Prisma access is confined to Server Components/Actions; no `src/generated/prisma` import exists in a Client Component.
## 9. Tests written
- **Unit** (Vitest, no DB, part of the existing 90-test suite): `sign.test.ts` (HMAC sign/verify round-trip and tamper rejection), plus targeted tests for the booking-draft schema, error-code→message mapping, and the `.ics` generator's escaping/folding/timezone handling.
- **Integration** (Vitest, real throwaway Postgres, `src/features/booking/__tests__/customer-booking.integration.test.ts`, 14 new tests on top of Phase 3's 24): guest booking (new profile, case-insensitive reuse, one-weak-identifier-never-merges), authenticated booking (idempotent profile resolution), multi-service + specific-barber booking, any-barber booking (least-busy assignment), slot-becomes-unavailable-before-confirmation, customer appointment retrieval (own-only, status-filtered), guest cancellation without `cancelledByUserId`, reschedule to a different barber, book-again repricing at the *current* price rather than the historical snapshot, ownership enforcement (own/stranger/undefined session), and guest-to-guest data isolation. All 38 integration tests pass, run repeatedly against a fresh database.
- **End-to-end** (Playwright, `e2e/`, real dev server + real seeded Postgres): the 6 required journeys — guest books, registered customer books, customer views upcoming/past/cancelled, customer reschedules, customer cancels, customer books again from history. Every journey drives real forms, real Server Actions, and real redirects; none rely on mocks or snapshots. `e2e/global-setup.ts` applies migrations and the seed automatically given a `DATABASE_URL`.
Running them:
```bash
npm run test # unit, no DB
npm run test:integration # needs DATABASE_URL → a real Postgres
npm run test:e2e # needs DATABASE_URL → a real (throwaway) Postgres; starts its own dev server
```
## 10. What was manually verified
- Full guest booking flow and full authenticated booking flow, driven by hand in a real browser against seeded data, on both a 375px mobile viewport and a 1440px desktop viewport.
- The `/dev/scheduling-test` page's calendar against the same seeded data, to isolate a date-handling bug (see §12) from the Phase 4 UI before fixing it.
- `npm run build`, `npx tsc --noEmit`, and `npx eslint .` all clean on the final state.
- All three automated suites (90 unit / 38 integration / 6 E2E) passed together on one freshly created, migrated, and seeded database, more than once, including immediately after every fix described in §12.
## 11. Assumptions made
- **Single business.** `getPrimaryBusiness()` still assumes exactly one `Business` row, per Phase 2/3's existing assumption — unchanged this phase, just consumed by more pages now.
- **"Update booking" from the customer's side means notes, reschedule, or cancel — never a direct date/time edit outside `rescheduleBooking`.** Matches Phase 3's own stated assumption.
- **A returning guest's identity is trusted only as far as email+phone go.** No stronger verification (OTP, magic link) was in scope.
- **Reviews are demo data** until a real provider is wired up behind the existing `ReviewsProvider` abstraction.
- **"Book Again" preselects only currently-active services and a currently-bookable barber** — anything since discontinued or deactivated is silently dropped from the preselection rather than blocking the rebooking.
## 12. Known limitations (and what was fixed this phase after being found by testing)
Found and fixed while verifying, not left in place:
- **A date off-by-one bug** in the shared barber/date/time picker (`toYMD` in `barber-date-time-picker.tsx` and the same pattern in the Phase 3 dev-test page) used `date.toISOString().slice(0, 10)` to key calendar availability, which silently shifts a date back by one day for anyone in a timezone *ahead* of UTC — including the business's own Europe/London during BST. Real Mondays showed as closed. Fixed by reading the `Date` object's local components instead of round-tripping through UTC; verified by comparing the dev-test page's calendar before and after against the same seeded staff hours.
- **A dialog-unmounts-mid-confirmation bug** in `CancelAppointmentDialog`: `cancelCustomerAppointmentAction` called `revalidatePath("/appointments")` immediately on success, which refetches the Upcoming list before the user can read the late-cancellation notice or click "Done" — the cancelled appointment disappears from that list, unmounting the dialog showing the confirmation out from under them. Removed the `revalidatePath` calls (every affected page is already `dynamic = "force-dynamic"`, so they bought no real cache freshness) and now rely on the dialog's own `router.refresh()` on close, which was already there.
- **A silent test-cleanup leak** in the Phase 3 integration fixtures: `cleanupTestBusiness` tried to delete the test `Business` first, but `Appointment.staff` is `onDelete: Restrict` (a deliberate production safeguard), which blocked the cascade to `StaffProfile` for any test that had actually booked something — the failure was swallowed by a bare `.catch(() => {})`. This had been silently leaking a "Test Barbershop" row per affected test run since Phase 3; it only surfaced now because a manual verification pass queried `getPrimaryBusiness()` against a database that had just run the integration suite and picked up a leaked row instead of the real seed. Fixed by deleting the business's appointments before the business itself.
- **Desktop layout was effectively unstyled for the "browsing" screens** (Home, Appointments, Settings, and the bottom nav) — content simply didn't reflow or cap its width above mobile size, leaving a large empty right-hand column rather than anything intentional. Applied a **baseline** fix (a centered `max-w-2xl` content column, matching the pattern the booking flow's individual steps already used) across Home, Appointments, Settings, the shared `BottomNav`, `BarberDateTimePicker`, and `DetailsForm`. **This is not a full desktop redesign** — no side-by-side / grid layouts were built for browsing screens, only a consistent centered treatment so nothing looks broken. A genuine desktop-specific layout (e.g. a wider services grid, a split-pane appointment list) is a reasonable Phase 5+ item if desktop usage turns out to matter.
- **Reschedule's route was moved out of the `(customer)` route group** (`(customer)/appointments/[id]/reschedule` → top-level `appointments/[id]/reschedule`) after discovering the shared picker's own fixed bottom action bar was being covered by the persistent bottom nav on that route, blocking the "Confirm Reschedule" button. Matches the same reasoning `/book`'s layout already documents.
- **Guest appointment access remains order-tracking-by-id only** (§7, point 5) — not a bug, but worth restating as the clearest remaining security-adjacent scope boundary: a guest with the appointment id can view/reschedule/cancel it without further verification. Fine for this phase's threat model (ids are unguessable `cuid`s, never enumerable), but a real magic-link/OTP flow would close this gap properly.
## 13. Before Phase 5
- Decide whether guest-booking access needs stronger verification (magic link / OTP) before this becomes a compliance concern, or whether "unguessable id" remains acceptable for the foreseeable future.
- If desktop usage is expected to matter, budget real design time for Home/Appointments rather than the baseline centering applied here.
- The `e2e/` suite and `test:integration` script both assume a disposable Postgres the caller provides — there's no CI wiring yet to spin one up automatically; that's worth doing before this becomes the main safety net.
- Loyalty UI, staff/owner-facing screens, notifications, and payments are all still untouched, exactly as scoped.