# Phase 10 — Post-Implementation Fixes and Loyalty Card Enhancements Follow-up to Phase 9: six issues found during real testing, root-caused and fixed in dependency order, with full regression coverage and a real Docker verification pass. This doc is the mandated 19-point summary. ## 1. Root cause of uploaded-image 404s Next.js 16's `next start` (production mode) resolves `public/` asset requests against something computed at build time, not a live filesystem check — files present when the image was built serve `200`; files written to `public/` at runtime (uploads) serve `404` even though they exist on disk with correct permissions. Confirmed empirically inside the running container (`docker exec` showed the file present and readable; the HTTP request still 404'd). Not a permissions/volume/Docker problem — a Next.js routing-resolution problem. ## 2. Media delivery architecture after the fix - `src/lib/media/media-storage.ts` now writes to a storage root **outside** `public/` (`storage/media` by default, overridable via `MEDIA_STORAGE_ROOT`), resolved with traversal-safe path joining. - A new Route Handler, `src/app/api/media/[...path]/route.ts`, reads and streams the file itself with the correct `content-type`/cache headers, 404s on anything missing or invalid, and rejects `..` traversal — bypassing Next's `public/` resolution entirely. - `storeMediaFile` verifies the file is readable back (`fs.access`) before reporting success, so a silent write failure is never reported as a success. - Storage stays provider-neutral: every caller only ever sees a `/api/media/...` URL, never a raw filesystem path, so swapping to S3 later only touches `media-storage.ts`. - A hand-written migration (`20260804202400_phase10_rewrite_legacy_media_urls`) rewrites every already-stored `/media/...` URL to `/api/media/...` across every image column in the schema, so images uploaded before this fix keep working without a manual data fix-up. ## 3. Docker media-volume changes - `Dockerfile`: creates and chowns `./storage/media` (was `./public/media`). - `docker-compose.local.yaml`: the `next-chair-media` volume now mounts at `/app/storage/media` — **same volume name**, deliberately, so the one real file already uploaded to the user's business reappears at its new path instead of being orphaned. Confirmed in this session: the pre-existing uploaded file (`gallery/eecbe189-....jpg`) is present under `storage/media/` and resolves `200` through `/api/media/...` after a full image rebuild and container recreation. - Verified end-to-end against the real persistent stack: rebuild → recreate → migrations auto-apply → existing upload still resolves → `/api/health` and every checked route return the expected status. ## 4. Loyalty schema changes Added to `LoyaltyProgramme` (migration `20260804202231_phase10_loyalty_card_and_permission_fixes`): - `rewardPosition RewardPositionMode @default(AFTER_ALL_STAMPS)` — new enum `AFTER_ALL_STAMPS | FINAL_SLOT_IS_REWARD`. - `cardTopRightText String?`, `cardFont LoyaltyCardFont @default(SANS)` (enum `SANS | SERIF | MONO`), `cardTopRightTextSize LoyaltyCardTextSize @default(MD)` (enum `SM | MD | LG`), `cardTopRightTextColor String?`, `cardBorderColor String?`, `cardMemberNumberColor String?`. - `CHECK (stampsRequired BETWEEN 1 AND 10)` on `stampsRequired`, confirmed genuinely enforced on MySQL 8 (a violating `UPDATE` against the real business's row fails with error 3819, verified in this session). - A separate idempotent migration (`20260804202300_phase10_backfill_owner_permissions`) backfills `MANAGE_MEDIA_LIBRARY`/`MANAGE_REVIEWS` onto every role already holding `MANAGE_STAFF` — see point 10. No destructive migration was needed: the real business's existing row had `stampsRequired = 10`, already within range. ## 5. Loyalty typography/colour controls `src/features/owner/components/loyalty-config-form.tsx` gained: - **Top-right card text**: plain text input + font (Sans/Serif/Mono, via `LoyaltyCardFont` — Tailwind's built-in generic stacks only, deliberately no new web fonts loaded) + size (Small/Medium/Large) + colour. - Font is **card-wide** (applies to all card text), size and colour are **specific to the top-right text only** — one clear, documented interpretation rather than per-line font controls. - **Border colour** and **member-number colour**, each via the existing `ColorPicker`. - The live preview renders through the exact same `LoyaltyCard` component the customer sees (`src/components/ui/loyalty-card.tsx`) — no preview-only rendering path that could drift from the real card. ## 6. Terms-line fix Two independent root causes, both fixed: 1. `getLoyaltySummary` never selected/returned `termsLine` at all — `LoyaltySummary` didn't have the field. 2. `(customer)/loyalty/page.tsx` didn't use it anyway — it always synthesized its own `"N more to your reward"` string and only showed it when no reward was available, silently discarding whatever the owner had actually configured. Fixed: `LoyaltySummary` now includes `termsLine`, and the page renders `loyalty.termsLine ?? undefined` — the real configured text, wrapped safely by the card's existing `text-center` styling, never overlapping the stamp grid (it sits below it in the flex layout). No configured value → nothing renders (the card's existing `{termsLine && ...}` guard), matching the brief's "no placeholder terms" requirement. Verified via a real E2E assertion against the seeded terms line (customer-journeys.spec.ts, journey 8). ## 7. Stamp-limit enforcement `MAX_STAMPS_REQUIRED = 10`, defined once in the new `src/services/loyalty/reward-threshold.ts` and re-exported from `loyalty.service.ts`, enforced at every layer: - `validateProgrammeConfig` (domain service). - The owner form's number input (`max={10}`, clamped on every keystroke). - The `LoyaltyProgramme_stampsRequired_range` MySQL CHECK constraint. - Existing integration test data already at or under 10 — no migration of bad data was needed (checked first, per the brief's instruction). ## 8. Reward-position mode behaviour New `resolveRewardThreshold()` (`src/services/loyalty/reward-threshold.ts`) is the single place this is decided: - `AFTER_ALL_STAMPS` (default, matches pre-Phase-10 behaviour exactly): threshold == `stampsRequired`. Every card position is a plain stamp. - `FINAL_SLOT_IS_REWARD`: threshold == `stampsRequired - 1` (clamped at 1). The last position is the reward itself, not a stamp to collect — rendered distinctly (a `Gift` icon outline while locked, filled reward styling once unlocked) regardless of whether `stampsRequired` was ever literally reached. Every consumer reads through this one function — never re-derives its own threshold: - `addStampTransaction` (shared by both the automatic completed-appointment path and the manual staff "Add Stamp" path — same threshold, proven by a dedicated integration test that awards one stamp each way and checks the reward fires at the crossing point). - `redeemReward` — deducts the threshold amount, not the raw `stampsRequired`, so redemption in `FINAL_SLOT_IS_REWARD` mode can't under- or over-deduct relative to what was actually collected. - `LoyaltyCard` (customer view and owner live preview, same component). - Every "X/Y stamps" progress display: customer Home, customer Loyalty page, staff Current/Clients/Client-detail/Appointment-detail pages, owner Appointment-detail page, and the batched staff customer-directory summary (`CustomerLoyaltyBrief.rewardThreshold`) — all previously showed `currentStamps`/`stampsRequired`, which would have under-represented progress in `FINAL_SLOT_IS_REWARD` mode. The owner's "Reward Timing" control is two clearly labeled option cards ("Reward after all stamps" / "Final position is the reward"), each with supporting copy computed from the live `stampsRequired` value, not an unlabeled boolean. ## 9. Google Place ID implementation The field already existed in the schema and the owner Reviews page (`reviews-manager.tsx`) — the actual root cause was elsewhere (see point 10). This session's changes: - Expanded the field's help text to explain how to find a Place ID via Google Maps' own Share → Embed flow, deliberately with **no hardcoded external URL** (per the standing "never fabricate a URL" constraint). - `updateReviewsSettingsAction` now trims the value, detects an actual change, and writes an audit log entry (`GOOGLE_PLACE_ID_CHANGED`, with `previousPlaceId`/`nextPlaceId` metadata) only when it changed. - Existing reviews are kept (not archived/deleted) when the Place ID changes — matching the recommended, documented behaviour; the "Sync Now" button already gave an immediate manual re-sync path. - Validation stays basic (non-empty, trimmed) — no restrictive regex that could reject a valid Place ID. ## 10. Reviews-navigation bug root cause Not a routing bug at all. A read-only query against the real, live database showed the installed business's Owner role was **missing** the `MANAGE_MEDIA_LIBRARY` and `MANAGE_REVIEWS` permissions — both added to the `Permission` enum in Phase 9, but `syncRolePermissions()` only runs at install/seed time, so an already-installed business's `RolePermission` rows never picked them up. The Reviews nav link's `href` and route config were always correct; the page (and the Place ID field on it) was simply inaccessible/hidden behind a permission check that silently failed. Fixed with an idempotent backfill migration (`20260804202300_phase10_backfill_owner_permissions`) rather than any client-side workaround — confirmed on the real database in this session (`SELECT` shows Owner now holds both permissions). ## 11. Home service preselection approach `preselectServiceAction` (`src/features/booking/actions/draft.ts`): receives only a `serviceId` from the client; the service's active/bookable status and business are always re-read from the database. No existing draft → creates one with the service selected. Existing draft → adds the service if not already present (no duplicates), clearing a previously chosen barber/date/time only if the selection actually changed — the same rule `saveServicesStepAction` already uses. Invalid/inactive/non-bookable service → a friendly inline error on the `BookServiceButton`, draft untouched. No price/duration/eligibility ever crosses the URL; everything is re-validated server-side against the signed booking-draft cookie. ## 12. Routes/components changed - New: `src/app/api/media/[...path]/route.ts`, `src/features/booking/components/book-service-button.tsx`, `src/services/loyalty/reward-threshold.ts`. - Rewritten: `src/lib/media/media-storage.ts`. - Extended: `src/services/loyalty/loyalty.service.ts` (new fields on `LoyaltySummary`/`ProgrammeConfigInput`/`CustomerLoyaltyBrief`, `resolveRewardThreshold` re-export), `src/components/ui/loyalty-card.tsx` (font/border/member-number/top-right-text props, reward-slot visuals), `src/features/owner/components/loyalty-config-form.tsx` (reward-timing selector, typography/colour controls), `src/features/booking/actions/draft.ts` and `schemas.ts` (`preselectServiceAction`), `src/features/home/components/home-tabs.tsx` (uses `BookServiceButton`), `src/features/owner/actions/reviews.ts` and `reviews-manager.tsx` (Place ID audit log + help text), `Dockerfile` and `docker-compose.local.yaml` (storage path). - Progress-display denominator fixed (now threshold-aware, not raw `stampsRequired`) in: `(customer)/home/page.tsx`, `(customer)/loyalty/page.tsx`, `(staff)/staff/current/page.tsx`, `(staff)/staff/clients/page.tsx`, `(staff)/staff/clients/[id]/page.tsx`, `(staff)/staff/appointments/[id]/page.tsx`, `(owner)/owner/appointments/[id]/page.tsx`, `(owner)/owner/settings/loyalty/page.tsx`. ## 13. Prisma migration Three migrations, all applied cleanly against the real, persistent database in this session's Docker verification: 1. `20260804202231_phase10_loyalty_card_and_permission_fixes` — the seven new `LoyaltyProgramme` columns + two new enums + the stamps-required CHECK constraint (generated via `prisma migrate dev`, CHECK hand-appended). 2. `20260804202300_phase10_backfill_owner_permissions` — hand-written, idempotent `INSERT IGNORE` permission backfill. 3. `20260804202400_phase10_rewrite_legacy_media_urls` — hand-written, idempotent legacy `/media/...` → `/api/media/...` URL rewrite. ## 14. Tests added/updated - Unit: `src/services/loyalty/__tests__/loyalty-config.test.ts` — 3 new `resolveRewardThreshold` cases (both modes + the 1-stamp clamp), plus the existing `validateProgrammeConfig` fixtures updated for the widened `ProgrammeConfigInput`. - Integration: `src/services/loyalty/__tests__/loyalty.service.integration.test.ts` — 3 new cases: `FINAL_SLOT_IS_REWARD` earns via a manual add one stamp early; the automatic and manual paths agree on the same threshold; redemption deducts the threshold, not the raw `stampsRequired`. `loyalty-owner.integration.test.ts` updated for the widened input type. - E2E: `customer-journeys.spec.ts` — 2 new journeys (9: Home "Book" preselects a service, preserves existing selections, doesn't duplicate on re-click; 10: the same for a guest) and one new assertion on journey 8 (the real configured terms line renders on the customer card). - Removed: an attempted integration test that called `preselectServiceAction` directly — it (and every other booking-draft Server Action in this codebase) depends on `next/headers`'s request-scoped `cookies()` and calls `redirect()` on success, neither of which works outside a real Next.js request; this matches this codebase's existing, documented convention (see `customer-booking.integration.test.ts`'s own header comment) of testing these thin Server Actions via Playwright only. The three rejection paths that don't touch `cookies()`/`redirect()` were still directly testable, but were dropped in favour of the simpler, already-covered E2E path once the client-bundle constraint below made clear how easy it is for this module boundary to silently break. ## 15. Full regression results - **Unit**: 253/253 passing (`npx vitest run`). - **Integration**: 185/185 passing, against a real MySQL 8 database (`npx vitest run --config vitest.integration.config.mts`). - **E2E — customer-journeys.spec.ts**: 10/10 passing. - **E2E — owner-journeys.spec.ts**: 13/13 passing (this run caught a real bug — see point 18). - **E2E — staff-journeys.spec.ts**: 6/10 passing. Journey 7 ("barber changes their working hours") fails on a clean database; journeys 8–10 didn't run as a result. Confirmed via `git status` that nothing in this phase touches working-hours code — this is a pre-existing issue, unrelated to any of the six fixes in this brief, and has been flagged as its own follow-up task rather than investigated here (see point 18). - `npx tsc --noEmit` and `npx eslint .` (whole repo): clean. - `npm run build` (production build): succeeds. The only console noise during the build is Prisma connection-pool timeouts while attempting to reach the database from this sandboxed build environment (not reachable here by design) — the build still completes and every route lists correctly. ## 16. Docker verification Performed against the user's real, persistent local stack (not a throwaway container), in this order: 1. `docker compose build next-chair` — image rebuilt with all Phase 10 code. 2. `docker compose up -d next-chair` — container recreated; entrypoint auto-applied the 3 pending migrations against the real database, reported "Syncing Super Administrator... skipping (none configured)" (expected — no `SUPER_ADMIN_EMAIL` set), then started successfully. 3. `curl` checks: `/api/health` → 200, `/home` → 200, `/loyalty` → 200, `/book` → 200, `/owner/settings/loyalty` → 307 (sign-in redirect, not a 500), `/owner/settings/reviews` → 307. No 500s anywhere. 4. The one real file already uploaded to this business's media library (`storage/media/gallery/....jpg`) still resolves `200` with `content-type: image/jpeg` through `/api/media/...` after the rebuild — confirms the Phase 10.1 media fix and the volume-path change survive a real rebuild+recreate cycle. 5. Direct `SELECT` against the real `LoyaltyProgramme` row: the new columns are present with correct defaults (`rewardPosition = AFTER_ALL_STAMPS`, `cardFont = SANS`, `cardTopRightTextSize = MD`) — the migration didn't corrupt or reset the business's existing configuration. 6. A live `UPDATE ... SET stampsRequired = 15` against the real row is rejected by the CHECK constraint (`Error 3819`) — confirms the constraint is genuinely enforced, not just declared. 7. Direct `SELECT` against `RolePermission`: the real Owner role now holds both `MANAGE_MEDIA_LIBRARY` and `MANAGE_REVIEWS`. Authenticated in-browser verification of the owner loyalty form and both reward modes was **not** re-run through this same Docker container (curl can't drive the sign-in form without the real owner's credentials, which this session doesn't have and won't attempt to bypass). That coverage comes from the Playwright E2E suite instead, run against a dev server using the identical application code and a real MySQL 8 database — see point 15. ## 17. Manual verification performed Everything in point 16, plus the E2E suite's own real-browser, real-form, real-database coverage of: the owner loyalty config form (typography, colours, reward-timing selector, live preview), a customer's card (terms line, reward-slot visuals), Home's "Book" button preselecting a service for both a guest and a signed-in customer, and the Reviews/Place ID settings page loading correctly for the Owner role. ## 18. Known limitations - **Staff working-hours E2E failure** — pre-existing, unrelated to this phase, flagged as its own follow-up task (spawned during this session) rather than investigated here, per the brief's scope. - **Duplicate `mug-saucer` icon-picker React key warning** — pre-existing, unrelated, also flagged as its own follow-up task. - **A real client-bundle bug was found and fixed during this phase's own verification**: `loyalty-config-form.tsx` (a Client Component) briefly imported `MAX_STAMPS_REQUIRED` from the full `loyalty.service.ts`, which transitively pulls in Prisma-touching notification code — this broke the owner loyalty settings page in a real browser (caught by the owner-journeys E2E suite, not by `tsc`/`eslint`/unit tests, none of which exercise the client bundle boundary). Fixed by moving both `resolveRewardThreshold` and `MAX_STAMPS_REQUIRED` into the small, side-effect-free `reward-threshold.ts` module client components can import directly. Documented here because it's a real, general risk pattern for this codebase — any future constant or pure helper that "happens to live" in a Prisma-calling service file is one import away from silently breaking a client bundle, and only an E2E run against a real browser will catch it. - Google Reviews sync itself (calling the real Google Places API) was not exercised end-to-end — this session has no real Google API credentials, and the brief's own "never call the Places API from every page load" constraint means this is only reachable via the 24h cron or the manual "Sync Now" button, neither of which this session can trigger against a real Google account. - The staff customer-directory's batched loyalty summary (`CustomerLoyaltyBrief`) now shows a threshold-aware denominator like every other progress display, but this one specific display wasn't separately covered by a new automated test (it's a one-line change, reusing the already-tested `resolveRewardThreshold`). ## 19. Anything requiring real external credentials - A Google Cloud API key + a real Google Business Profile Place ID, to exercise the actual Places API sync (see point 18). - The real owner account's password, to drive an authenticated Docker container walkthrough via `curl` rather than relying on the E2E suite's own (seeded, non-production) credentials.