diff --git a/docs/audit-2026-05-15.md b/docs/audit-2026-05-15.md new file mode 100644 index 00000000..c51c80c7 --- /dev/null +++ b/docs/audit-2026-05-15.md @@ -0,0 +1,117 @@ +# Comprehensive Playwright Audit — 2026-05-15 + +Scope: full coverage of admin, sales-rep, viewer, portal, catch-up wizard, single-tree responsive shell, plus spot-checks on yacht / interest / berth detail surfaces. + +## Setup + +- Dev server: localhost:3000 (running) +- Users: + - super_admin: `admin@portnimara.test` / `SuperAdmin12345!` + - sales_agent: `agent@portnimara.test` / `SalesAgent12345!` + - viewer: `viewer@portnimara.test` / `ViewerUser12345!` +- Port slug: `port-nimara` + +## Verified working (positive findings) + +- ✅ super-admin login + dashboard renders, all 34 admin pages return 200 +- ✅ Recent commits' workflow features: + - F22 AlertTriangle icon on override-required stages + - F23 inline yacht-prereq picker fires when leaving Enquiry without a yacht (confirmed end-to-end: "A yacht must be linked before leaving Enquiry. Pick one below to move to Qualified.") + - F25 documents-hub folder selection persists in `?folder=root` querystring + - F44 OwnerPicker has Client/Company tabs visible in popover (just hidden by Select trigger summary) +- ✅ **#67 catch-up workflow end-to-end**: manually flipped berth A2 → reconciliation queue picked it up → wizard quick-created client + interest + cleared override + reason stamped "Reconciled via interest " + redirected to interest detail +- ✅ **#26 single-tree shell**: at viewport 390px only mobile shell mounts (1 nav, no desktop sidebar); at 1440px only desktop shell mounts; clean swap on resize +- ✅ Permission gating: viewer + sales-agent get no "New Client"/admin nav; viewer POST to /clients returns 403 +- ✅ Audit log captures all writes (tag create, berth update, interest create, client create) including the reconcile event with `reconciledInterestId` metadata + +## Findings + +### A1 — Dashboard Recent Activity surfaces raw `permission_denied` rows with no label + +- `/api/v1/dashboard/activity` returns entries with `action: "permission_denied"` and `label: null`. The activity feed renders just the action badge with nothing beside it. From earlier audits, 6 of these are stacked at the top of the dashboard for the super-admin. +- Fix options: filter `permission_denied` out of the feed, OR map them to readable copy ("Permission denied: tried to view audit log (denied)") using `metadata.attemptedAction`. +- Effort: XS. + +### A2 — Activity feed renders legacy 9-stage enum values + +- `pipelineStage: "deposit_10pct"` and `"contract_sent"` still appear in `oldValue` / `newValue` for historical rows. These should map to the 7-stage labels at render time so the feed reads as `Eoi → Deposit Paid` not `eoi_signed → deposit_10pct`. +- The mapping table lives in seed-synthetic-data.ts (`details_sent→enquiry` etc.) — pull it into a shared `LEGACY_STAGE_REMAP` helper for activity-feed read paths. +- Effort: S. + +### A16 — File upload to documents hub root fails with validation error + +- Repro: open `/documents`, click "Upload file", drop any file in. POST to `/api/v1/files/upload` returns 400 with field errors on `clientId`, `yachtId`, `companyId`, `category`, `entityType`, `entityId` — all "expected string, received null". +- Root cause: the client sends `null` for unset optional fields; the validator expects them either absent or strings. Mismatch. +- Fix: either make the zod schema accept `.nullable()` on those fields OR strip nulls in `FileUploadZone` / `FolderDropZone` before POST. +- Effort: XS. + +### A17 — `/api/v1/admin/ports` requires X-Port-Id but is the bootstrap port-resolver + +- Symptom: as sales-agent, every page load fires a 400 to `/api/v1/admin/ports` ("Port context required"). Repeats on every apiFetch call because `apiFetch` calls this endpoint to resolve port-slug→port-id. +- Bigger problem: the endpoint is gated to super-admin (`requireSuperAdmin`). Sales-reps and viewers will NEVER get a ports list from this endpoint, so the bootstrap path always falls through to the Zustand store. The 400 noise is wasted work + log spam. +- Fix: add a `/api/v1/me/ports` endpoint that returns the caller's accessible ports without the super-admin gate, and have `client.ts` use it. OR seed the PortProvider context into a `__INITIAL_PORTS__` window global on first paint and skip the fetch entirely. +- Effort: S. + +### A18 — `/api/v1/users` returns 404 vs `/api/v1/admin/audit` returns 403 (inconsistent perm denials) + +- Both endpoints reject sales-agent access but use different status codes. Pick one — either always 404 (hide existence) or always 403 (acknowledge but deny). The 403/404 split is the kind of inconsistency a pentester probes to map permissions. +- Effort: XS sweep. + +### A4 — F19 empty-contact filter never runs because zod-validation rejects first + +- Repro: open New Client dialog, fill Full Name + one valid email, click "Add Contact" to insert an empty row, click Create Client. Nothing happens (no toast, no submit, no POST in network). +- Root cause: my F19 fix put the empty-row prune in the **mutationFn**, but `handleSubmit(zodResolver)` validates the form FIRST. The empty contact's `value: z.string().min(1)` fails silently — handleSubmit short-circuits without surfacing an error on the empty row (the field has no `errors.contacts[1].value` rendered because the schema-level message attaches to the array path). +- Fix: prune empty contact rows in a custom onSubmit wrapper BEFORE handleSubmit/zod sees them, OR change the field-array schema to allow empty rows and let the mutationFn prune. +- Effort: XS. + +### A19_b — Portal `/portal/login` shows "Client portal unavailable" + +- The portal is gated by a per-port `client_portal_enabled` system setting. The route layout renders a friendly message but no admin path is obvious to a fresh-eyes operator. +- Two distinct problems: + - **Discoverability**: the admin landing card for "System Settings" doesn't surface a "Enable client portal" toggle prominently. A new operator would have to know the setting key. + - **Portal scope**: the portal currently only has activation + reset password + sign-in surfaces. Once the rep logs the client in, they land on... what? Worth a separate scoping session to flesh out: their interests, their documents, their signing queue, payment history, message thread. +- Recommendation: spec a "Phase 0 portal MVP" (read-only views of own interests + documents + signed-PDF download) before promoting it to clients. Treat the rest as v1.3 backlog. +- Effort: portal MVP S-M depending on scope. + +### A3 — Dev-only CSP error spam from react-grab + +- `react-grab` dev script tries to load `fonts.googleapis.com/css2?family=Geist` and triggers a CSP block on every page load (2 console errors). Cosmetic since react-grab isn't loaded in prod, but the dev console gets noisy. +- Fix: either drop the react-grab include or extend dev CSP `style-src` to allow `https://fonts.googleapis.com`. +- Effort: XS. + +### A5 — Socket.IO WebSocket repeatedly fails to connect in dev + +- Console floods with "WebSocket is closed before the connection is established" — at least 6 occurrences per page in this session. Socket-io server endpoint at /socket.io/ isn't reachable from the Next dev server. +- Likely root cause: Socket.IO server runs as a sidecar in compose but `pnpm dev` only starts Next, so the realtime channel is permanently broken in dev. Realtime invalidation features (interest/folder updates) silently never fire. +- Fix: either start the socket server alongside `pnpm dev` (concurrently script), gate the SocketProvider behind a feature flag in dev, or stub the client to no-op when the endpoint 404s the first handshake. +- Effort: S. + +### A6 — Some DialogContent missing aria-describedby + +- React warnings: `Missing 'Description' or 'aria-describedby={undefined}' for {DialogContent}`. At least one Dialog opens without a DialogDescription. +- Fix: audit Dialog usages and either add a DialogDescription or pass `aria-describedby={undefined}` explicitly where genuinely no description is needed. +- Effort: S. + +### A8 — Legacy `statusOverrideMode = "auto"` values still in seed data + +- Berth A1 (and likely others) has `statusOverrideMode: "auto"` from the NocoDB legacy import. The new code writes 'manual' | 'automated' | null; 'auto' is unrecognized. +- Treated as "not manual" by the reconcile-queue filter so it's benign today, but the column should be normalized — either migrate legacy 'auto' → null in a migration, or treat 'auto' explicitly in the read paths. +- Effort: XS. + +### A9 — Catch-up wizard pipeline stage default doesn't match berth status + +- Open the wizard on a berth where status=under_offer; the stage picker defaults to "New Enquiry" instead of "EOI" (the most common manual-flip case). +- Root cause in `catch-up-wizard.tsx`: the default-stage logic only fires when the initial state isn't in the allowed set; 'enquiry' IS in the allowed set for under_offer, so it stays. Should default to EOI on first open via a `useEffect` keyed on `berth?.data.status`. +- Effort: XS. + +### A19 — F27 same-stage write still returns 200 + body instead of 204 + +- Spec said "same-stage write → 204 No Content (no-op)". The service early-returns `existing` correctly (no audit log emitted), but the route handler wraps it in `{ data: existing }` and returns 200. +- Fix: have the service return a discriminated result like `{ kind: 'no-op' } | { kind: 'updated', interest }`, and the route handler returns 204 for the no-op branch. +- Effort: XS (route handler tweak). + +### A20 — F44 OwnerPicker — toggle hidden until popover opens (minor UX) + +- The yacht-create form shows just "Select owner..." with no visible indication that it supports both clients AND companies. The Client/Company toggle pills only appear once the popover is open. +- Fix option: surface "Owned by: Client | Company" as a segmented control above the picker, OR add a hint chip "Client/Company" next to the label. +- Effort: XS.