NextChair/docs/phase-10-summary.md

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.