docs(uat): SHIPPED annotations for PR1 batch + accumulated UAT findings
PR1 batch (2d57417) covered 7 Wave-1 blockers; each finding entry now
carries an inline `**SHIPPED in 2d57417:**` line summarizing what
landed and (where applicable) what remains parked for later waves
(backfill scripts, nested-folder migration, platform-wide form-error
audit).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -29,6 +29,83 @@ _Copy tweaks, alignment, single-prop edits, obvious typos._
|
||||
> - **InterestDocumentsTab label clarity** — _src/components/interests/interest-documents-tab.tsx_ — the tab has two sections: "Legal documents" (Documenso envelopes — EOI / Reservation / Contract, signature-driven) and "Attachments" (general file uploads). "Legal documents" is misleading — the section is scoped to _signature envelopes_, not any legal doc. A rep uploading externally-signed PDFs (lawyer-prepared addenda, etc.) currently goes into Attachments — fine, but the label gap suggests reps expect "Legal documents" to accept external uploads too. Two paths: (a) rename "Legal documents" → "Signature documents" (or "Contracts & EOI") to scope it correctly, OR (b) allow external uploads into that section (more disruptive — needs file-classification metadata). ~15 min for rename + tooltip; ~2 h for upload route.
|
||||
> - **Berth recommender: drop the "Tier X" prefix, keep plain-English label + add tooltip** — _src/components/interests/berth-recommender-panel.tsx:181_ (the pill render) and _:94-99_ (`TIER_LABELS` map) — the pill currently renders `Tier A · Open` / `Tier B · Fall-through` / `Tier C · Active interest` / `Tier D · Late stage`. The four tier letters are internal taxonomy from `berth-recommender.service.ts` (A = never had interest, B = past fall-through, C = active interest, D = active in late stage); reps don't speak in tier letters and the suffix label already carries the meaning. Fix: (1) drop the `Tier {rec.tier} · ` prefix in the rendered pill — show just `tier.label` (e.g. "Open" / "Fall-through" / "Active interest" / "Late stage") so the chip is self-explanatory. (2) Wrap the pill in a `Popover` (click) or `Tooltip` (hover) that explains the four-state ladder in plain English: "Recommender state — **Open**: never had interest. **Fall-through**: prior interest didn't close (warm). **Active interest**: another deal is in play. **Late stage**: another deal is near-sold." (3) Optional: a small `?` icon next to the chip so the tooltip is discoverable without hovering. The internal `Tier` type stays as-is in the service (it has semantic value in the SQL ladder + admin settings); only the UI label changes. ~15 min. Captured 2026-05-18 from UAT.
|
||||
> - **ChartCard: center the chart vertically when grid row is taller than the chart** — _src/components/dashboard/chart-card.tsx_ — every chart widget (`pipeline-funnel`, `occupancy-timeline`, `lead-source`, `berth-status`, `source-conversion`, …) wraps a fixed-height `ResponsiveContainer` (240-280px) inside `ChartCard`. The Card is `h-full` (stretches to its grid-row height) but the inner content keeps its 240-280px and pins to the top — when a neighbour card in the same row is taller (e.g. Pipeline Value with its full per-stage breakdown), the chart card has visible empty space below the chart. Fix: convert `ChartCard` to a flex-column (`<Card className="h-full flex flex-col">`); `CardHeader` keeps natural height; `CardContent` gets `flex-1 flex items-center` so the chart's wrapping div sits vertically centered in the remaining space. ResponsiveContainer stays at its declared fixed height. Affects all chart widgets via one wrapper change — no per-chart edits. ~10 min. Captured 2026-05-18 from UAT.
|
||||
> - **UploadForSigningDialog feels cramped — fix inner content distribution + right-size the dialog** — _src/components/documents/upload-for-signing-dialog.tsx:166_ (currently `max-w-5xl` = 1024px) + the recipient-row + form fields inside DialogBody. Visual symptom: dialog renders at full 5xl width but inner content clusters on the left ~60% with truncated email field (`email@examp...` clipped), narrow Document title input, tiny 4-row Optional message textarea, and massive whitespace to the right. Combination makes the dialog feel narrow AND empty.
|
||||
> - **Fix:**
|
||||
> - **(a) Right-size the dialog:** drop to `max-w-3xl` (768px) — content fills naturally instead of swimming in 5xl.
|
||||
> - **(b) Recipient row flex distribution:** `Name` input → `flex-1`, `email` input → `flex-[2]` (~2x name's width — emails are longer), role select → `w-32 shrink-0`, delete icon → `shrink-0`. Today every field is at its intrinsic width with no flex hint, so the row doesn't fill horizontal space.
|
||||
> - **(c) Document title + Optional message inputs:** make sure they have `w-full` on the wrapper so they span the dialog's content width.
|
||||
> - **(d) Optional message textarea:** bump rows from 4 → 6 minimum (`rows={6}` or `min-h-[8rem]`) so reps writing real messages have room.
|
||||
> - **(e) Audit the other steps of the wizard** (select-file, place-fields) for the same content-distribution issues since they share DialogBody.
|
||||
> - **Effort:** ~20-30 min. Captured 2026-05-21 from UAT. **Pairs nicely with:** the platform-wide form-error UX work (Bucket 2) — both touch how form content is presented in dialogs.
|
||||
> - **ColumnPicker: add "Hide all columns" symmetric to "Show all columns"** — _src/components/shared/column-picker.tsx:58-60 (`showAll()`) + 116-123 (button render)_ — current picker has a "Show all columns" footer item that clears the hidden set. Add a parallel `hideAll()` that sets `hidden = columns.filter(c => !c.alwaysVisible).map(c => c.id)` — hides every toggleable column while preserving `alwaysVisible` ones. Render a "Hide all columns" footer item next to "Show all columns" with the same visibility gate (only shown when ≥1 toggleable column is currently visible, mirroring the `canShowAll` logic). Since column-picker is shared across every DataTable surface (berths, clients, interests, yachts, companies, reservations, invoices, audit-log, expenses), the fix lands platform-wide automatically. ~5 min. Captured 2026-05-21 from UAT.
|
||||
> - **OnboardingChecklist: auto-check uses raw setting-row presence, not resolver chain → ports using env fallback or global config never auto-tick + super_admin discoverability gap** — _src/components/admin/onboarding-checklist.tsx:32-105 (STEPS def)_ + _src/lib/services/port-config.ts_ (the resolver chain like `getPortDocumensoConfig`) + new dashboard tile + new topbar banner for the discoverability half. Two linked bugs surfaced UAT 2026-05-21.
|
||||
> - **(a) [bug] Auto-check sentinels are too strict.** Examples:
|
||||
> - Email step (line 46) checks `smtp_host_override` — only fires when port has its own override row. Ports using global SMTP (the common case) never auto-tick even though email works.
|
||||
> - Documenso step (lines 58-63) requires ALL of 4 port-level overrides. Per CLAUDE.md, Documenso supports env fallback (`getPortDocumensoConfig` does `adminValue ?? env.DOCUMENSO_API_KEY`), so a working port using env config registers as not-onboarded forever.
|
||||
> - Same pattern likely for storage, settings, etc. — any setting with a resolver chain falls into this trap.
|
||||
> - **Fix:** replace each `autoCheckSettingKey` with an `autoCheckResolver` function (named import from `src/lib/services/port-config.ts` etc.) that runs the full resolver chain and returns `true` when the functional config is complete. New OnboardingStep shape: `{ id, label, description, href, autoCheckResolver?: (portId) => Promise<boolean> }`. Sentinels stay for steps where direct setting-row presence IS the truth (e.g. branding logo URL).
|
||||
> - Belt-and-braces: surface what's resolving from where directly in the step row (e.g. "Email: ✓ Using global SMTP" vs "Email: ✓ Per-port override"). Closes the "why is this checked?" gap for admins later.
|
||||
> - **(b) [feature] Super_admin discoverability — nudge until onboarding hits 100%.** Today the checklist only appears on the one admin onboarding page; a super_admin who skips that page never sees it. Multi-surface nudges:
|
||||
> - **Topbar banner** when onboarding < 100% — slim chip showing "Setup X% complete · Continue →" (links back to /admin/onboarding). Dismissible per-session (returns next login). Only visible to super_admin.
|
||||
> - **Dashboard rail tile** "Continue setup" — small card on the dashboard widget rail showing the next incomplete step + a button. Disappears entirely at 100%.
|
||||
> - **In-app notification (existing notification infra)** — fires once per week per super_admin until 100%, with a deep-link back to the checklist. "Your setup is X% complete — N items remaining."
|
||||
> - **Onboarding-complete celebration** — small toast + a one-time 🎉 highlight when the 100th item ticks. Acknowledges the finish-line so the nudges going silent feels intentional, not just a bug.
|
||||
> - **Permission gating:** all surfaces gate on `super_admin` (or whatever role the onboarding page itself is gated on) so non-super-admins don't see noise about settings they can't change.
|
||||
> - **Effort:** ~3-4h for (a) (resolver-chain audit + 6-8 step migrations + tests) + ~3-4h for (b) (topbar banner + dashboard tile + notification job + celebration). Total ~6-8h. Captured 2026-05-21 from UAT.
|
||||
> - **Agent audit (a11y + i18n) — 2026-05-21 — 27 findings bundled** — read-only Opus-agent pass over login/dashboard/interest-detail/client-detail/berth-detail/public-form/portal/admin surfaces. Ship as themed sub-PRs, not one mega-PR.
|
||||
> - **a11y — discrete fixes (~3-4h total):**
|
||||
> - Add `aria-label="Row actions for {name}"` on icon-only kebab triggers — _interest-columns.tsx:296_, _client-columns.tsx:301_, _berth-columns.tsx:175_. ~10min.
|
||||
> - Add `aria-label` + `aria-pressed` on Table/Board view toggle — _interest-list.tsx:187-202_. ~5min.
|
||||
> - Add `aria-expanded` + `aria-controls` on the "Show/Hide upcoming milestones" disclosure — _interest-tabs.tsx:484-494_. ~5min.
|
||||
> - Same for recommender "Hide/Add filters" — _berth-recommender-panel.tsx:466-471_. ~3min.
|
||||
> - Fix BrandedAuthShell logo `alt` default (`'Sign in'` shows on every page) — use `alt=""` when no port name OR pass per-page override — _branded-auth-shell.tsx:32,58_. ~10min.
|
||||
> - Mark PDF logo crop image decorative (`alt=""`) — _pdf-logo-uploader.tsx:312-318_. ~3min.
|
||||
> - Add `scope="col"` on raw `<th>` cells (or migrate to shadcn `<TableHead>`) — _berth-interests-tab.tsx:149-154_, _bulk-hard-delete-dialog.tsx:185-186_, _bulk-add-berths-wizard.tsx:226-231_. ~10min.
|
||||
> - Wrap "Loading…" auth fallbacks in `role="status" aria-live="polite"` — _set-password/page.tsx:107_, _portal/activate/page.tsx:9-11_, _supplemental-info/[token]/page.tsx:140-147_. ~10min.
|
||||
> - Add `aria-live` region on supplemental-info async state swaps — _supplemental-info/[token]/page.tsx:150-186_. ~10min.
|
||||
> - Add `<Label>` (or `aria-label`) on recommender filter selects — _berth-recommender-panel.tsx:306, 325, 343_. ~10min.
|
||||
> - Make `<legend>` styling visually distinct in supplemental-info — _supplemental-info/[token]/page.tsx:200, 249_. ~5min.
|
||||
> - Link set-password hint via `aria-describedby` — _set-password/page.tsx:147_. ~3min.
|
||||
> - **a11y — contrast/visual issues (Bucket 4 candidates):**
|
||||
> - `text-[#007bff]` 12px link below AA contrast on auth pages — darken to `#0058b3` or always-underline — _login/set-password/reset-password pages_. ~5min. **Severity: medium** (WCAG 1.4.1 violation).
|
||||
> - `text-muted-foreground/{40-70}` opacity stacking puts text below AA on muted bg — _interest-detail-header.tsx:493_, _client-detail-header.tsx:173,184_, _contacts-editor.tsx:280,292_, _client-interests-tab.tsx:160_, _berth-interest-pulse.tsx:165_, _invoice-card.tsx:149_. Audit + replace with semantic tokens. ~1h. **Platform pattern.**
|
||||
> - `text-[10px]` / `text-[11px]` micro-type on stage chips, pipeline counts, badges across 20+ surfaces — bump to 12px min — _client-pipeline-summary, client-card, dedup-suggestion-panel, contacts-editor, bulk-hard-delete-dialog, berth-interest-pulse, kpi-tile_. ~1h. **Platform pattern.**
|
||||
> - **i18n — discrete fixes (~1.5h total):**
|
||||
> - Fix invalid locale tag `'en-EU'` → use `undefined` (honour user) or proper BCP-47 — _payments-section.tsx:66_. ~3min.
|
||||
> - Calendar month dropdown passes `'default'` instead of resolved locale — _ui/calendar.tsx:35_. ~5min.
|
||||
> - Date formatting hardcoded `en-GB`/`en-US` across 10+ document/template surfaces — centralize via `formatDate()` helper honouring `useLocale()` — _documents-hub.tsx:373_, _document-list.tsx:83_, _document-detail.tsx:271_, _signing-details-dialog.tsx:81,103_, _entity-folder-view.tsx:81_, _template-list.tsx:132,224_, _reservation-detail.tsx:285_. ~1h.
|
||||
> - Currency formatter hardcoded `'en-US'` on all invoice/expense totals — same fix pattern — _invoice-columns.tsx:81_, _invoice-detail.tsx:232_, _expense-columns.tsx:87,103_, _expense-detail.tsx:191,200_. ~30min.
|
||||
> - `currency.ts` hardcodes English currency labels — delete, let Intl resolve — _src/lib/utils/currency.ts:11-29_. ~30min.
|
||||
> - **i18n — platform decisions (Bucket 3 candidates):**
|
||||
> - `next-intl` is wired but NEVER used — zero `useTranslations()` calls in src/. Decision: commit to i18n migration OR rip out the dead infrastructure. Holding both is tech-debt. ~scope depends on commitment.
|
||||
> - Naive ternary pluralization (`count === 1 ? 'X' : 'Xs'`) across 15+ surfaces — won't translate to Polish/Arabic/Russian. Route through `Intl.PluralRules` / next-intl's `t.rich`. ~1h after i18n decision lands.
|
||||
> - **Zero use of CSS logical properties — 1,173 instances of `ml-/mr-/pl-/pr-/text-left/text-right` and zero `ms-/me-/ps-/pe-/text-start/text-end`.** RTL support would require global refactor. If RTL is roadmap-bound: adopt logical properties going forward + add lint rule. ~30min for the lint guard; multi-day if RTL is real. **Note only for now.**
|
||||
> - **Platform patterns (Bucket 3):**
|
||||
> - **Form validation never sets `aria-invalid` / `role="alert"` / `aria-live`** across every react-hook-form caller. SR users get zero feedback on validation failure. Build a shared `<FieldError>` component emitting both visible text + ARIA. Sweep all forms. ~2h. **Bundles with the Bucket 2 form-error UX finding** — same surfaces, same primitive.
|
||||
> - **Icon-only buttons inconsistent — ~50% have `aria-label`, rest have nothing or only `sr-only` text.** Add `jsx-a11y/control-has-associated-label` lint rule + sweep. ~1h.
|
||||
> - **Sweep: remove em-dashes from all user-facing copy (toast messages, button labels, helper text, banners, dialog descriptions, empty states)** — em-dashes (`—`) feel AI-generated and add visual noise; user reads them as "Claude wrote this." Replace with periods, commas, colons, or simple hyphens depending on context. **Scope:** _src/components_ (every UI string), _src/lib/email/templates_ (email body copy), _src/lib/templates_ (merge-field labels + EOI body), _src/app_ (page-level copy), public form copy, error messages from `src/lib/errors`. **Out of scope (keep em-dashes):** code comments, JSDoc, audit-log entries, structured logging, this UAT findings doc itself (internal docs are fine). **Method:** grep `—` across `src/`, manually triage each match (some are inside JSX, some inside string literals); replace per context. Heuristic: if a user could see the character, replace it. **Effort:** ~2-3h depending on hit count (rough estimate 200-400 instances). Captured 2026-05-21 from UAT. **Going forward:** add an ESLint rule banning `—` in JSX text + string literals inside `src/components` so new code doesn't reintroduce them.
|
||||
> - **Custom-field form: "Sort Order" needs an explainer tooltip — example of a broader gap** — _src/components/admin/custom-fields/custom-field-form.tsx:298-308_ — surfaces a specific instance of a platform-wide gap: see the next finding for the full sweep.
|
||||
> - **DocumentList DocRow kebab: add "Download" action** — _src/components/documents/document-list.tsx:86-109_ — current kebab has Send-for-Signing (draft only), Move-to-folder, Delete. No Download. Reps reviewing a signed doc from the interest's documents tab have to navigate into the document detail to download. Add a `<DropdownMenuItem>` at the top of the menu when `doc.signedFileId` is set (or `doc.fileId` for non-Documenso docs like manual uploads), wired to the same `apiFetch('/api/v1/files/[id]/download')` + anchor-click pattern used elsewhere. Permission-gate by `files.download` if that perm exists. ~10 min. Captured 2026-05-21 from UAT.
|
||||
> - **InterestEoiTab "Open" link too ambiguous — relabel to "Open in Documents"** — _src/components/interests/interest-eoi-tab.tsx:163_ — the link in the EOI history list goes to `/${portSlug}/documents/${d.id}` (Documents Hub doc detail) but the label just says "Open" + an external-link icon. Rep can't tell where it goes until they hover. Change to `Open in Documents` (or `View in Documents`). Apply the same idiom anywhere else a cross-section navigation link uses bare "Open" — quick grep + sweep. ~5 min. Captured 2026-05-21 from UAT.
|
||||
> - **PaymentsSection: deprioritize layout — move below milestones + collapse-by-default at Reservation** — _src/components/interests/interest-tabs.tsx:633 + 846-852_ (current `showPaymentsSection = reservationStageReached` + renders at line 847, ABOVE the milestone strip at line 859+) + _src/components/interests/payments-section.tsx_ (the section component itself). Today: hidden pre-Reservation (correct ✓), shows as a full expanded card at Reservation+ sitting above the milestone strip. Section is reference/history once expected — milestone work (active step) should be the rep's primary visual focus, not deposits-tracking.
|
||||
> - **Fix (three states):**
|
||||
> - **Pre-Reservation:** keep hidden (no change).
|
||||
> - **Reservation+ stage, no deposits recorded yet:** render as a slim collapsed bar at the bottom of OverviewTab (below the milestone strip + below the Berth requirements / Tags / Latest note grid). Bar shows `Deposits · Not received yet` + a `Track deposit →` CTA that expands the section in place. Sits last on the page so it doesn't pull eye away from the active milestone.
|
||||
> - **Reservation+ stage, deposits exist:** same below-the-milestones placement, but the collapsed bar carries a summary chip: `Deposits · $10,000 received · 2 payments · Expand`. Click expands the full PaymentsSection inline. The summary chip uses the existing currency-format helper.
|
||||
> - **Render order change in interest-tabs.tsx:** lift the PaymentsSection mount from its current position (line 846-852, above milestones) to AFTER the milestone strip + AFTER the OverviewTab grid (below "Latest note", Tags, Berth requirements). It becomes the last visual element on the OverviewTab.
|
||||
> - **Collapse state:** persist per-interest via Zustand or react-query cache (so re-opening the same deal remembers the rep's last expand/collapse). Default collapsed unless a deposit was added in this session.
|
||||
> - **Effort:** ~1-1.5h (layout reorder + collapsed-bar state + summary chip + render-order verification). Captured 2026-05-21 from UAT.
|
||||
> - **WatchersCard empty state missing bottom padding** — _src/components/documents/document-detail.tsx:546_ — `<p className="text-xs text-muted-foreground">No one is watching this document yet.</p>` has no margin while the sibling populated `<ul>` at line 548 has `mb-3 space-y-1`. Empty state text sits flush against the add-watcher form below. Add `mb-3` to the empty-state `<p>` to match. ~30s. Captured 2026-05-21 from UAT.
|
||||
> - **DocumentDetail Interest link should show berth(s), not duplicate the client name** — _src/components/documents/document-detail.tsx:96 (type) + 237-241 (linked-entity row builder)_ + the document-detail API service that hydrates `linked.interest`. Today renders `Client: Matthew Ciaccio · Interest: Matthew Ciaccio` — visually redundant, and the Interest link carries no distinct information. Should be `Client: Matthew Ciaccio · Interest: A1-A3, B5-B7` (berth range via the existing `formatBerthRange()` helper from `src/lib/templates/berth-range.ts`, same idiom as the locked folder-naming convention and the external-EOI default title).
|
||||
> - **Backend:** swap the response payload's `interest: { id, clientName }` → `interest: { id, berthLabel }` where `berthLabel` is derived in the service layer from the interest's primary or in-bundle berths. Falls back to "No berths linked" when no berths are attached.
|
||||
> - **Frontend:** change line 241 from `sub: linked.interest.clientName` → `sub: linked.interest.berthLabel ?? 'No berths linked'`.
|
||||
> - **Effort:** ~15-20 min including type updates + a vitest covering the multi-berth + no-berths paths. Captured 2026-05-21 from UAT. Cross-ref: pairs with the shared title-derivation helper note in the external-EOI bundle (Bucket 2) — single `deriveBerthLabel(interest)` helper used everywhere.
|
||||
> - **Platform-wide `<FileInputButton>` primitive — replace 7 raw `<Input type="file">` instances with native browser-default styling** — _new_ `src/components/ui/file-input-button.tsx` + sweep — `<input type="file">` rendered without a wrapper shows the browser-default "Choose File / No file chosen" UI, which looks raw and inconsistent across Chromium / Safari / Firefox / Comet. We already use the correct idiom in `expense-form-dialog.tsx:389` (Button + hidden input + filename row) and `file-upload-zone.tsx`, but 7 other call sites still use the raw pattern.
|
||||
> - **Affected files:** `external-eoi-upload-dialog.tsx:92`, `template-editor.tsx:486 + 526`, `brochures-admin-panel.tsx:213`, `berth-documents-tab.tsx:176`, `won-status-panel.tsx:200`, `pdf-logo-uploader.tsx:278`, `settings-form-card.tsx:486`.
|
||||
> - **Component shape:** `<FileInputButton accept={...} multiple={...} onFilesPicked={(files) => ...} label="Upload PDF" icon={<Upload />} variant="outline" size="sm" />`. Renders a styled Button (Upload icon + label) + hidden `<input type="file">` underneath. Optional: after-pick filename row with X to clear, mirroring the expense form's pattern.
|
||||
> - **Sweep:** drop-in replacement at each of the 7 sites. Pair with the platform-wide file-preview work (Bucket 3) so picker-then-preview becomes consistent everywhere.
|
||||
> - **Effort:** ~10 min for the primitive; ~30-45 min for the 7-site sweep. Total ~1h. Captured 2026-05-21 from UAT.
|
||||
> - **EOI empty state: add "Mark as signed without file" button (parity with Reservation + Contract tabs)** — _src/components/interests/interest-eoi-tab.tsx:553-562_ (`EmptyEoiState` only renders Generate + Upload paper-signed) — `MarkExternallySignedDialog` already supports `docType: 'eoi'` (mark-externally-signed-dialog.tsx:37-41) with full copy ("Flips the EOI sub-status to 'signed' without uploading a file…"); the reservation tab uses the same dialog via a third ghost-button row (interest-reservation-tab.tsx:378-380). EOI tab's empty state just never grew the button. Add it as a third ghost-variant Button, wired to a `setMarkExternalOpen(true)` state hook + the existing dialog. ~5-10 min. Captured 2026-05-21 from UAT.
|
||||
> - **Activity feed: "See all" link to the full audit log** — _src/components/dashboard/activity-feed.tsx_ (ActivityFeedInner, around line 175) — the card lists the most recent audit events but has no jump-off to the full audit-log page. Add a "See all" link in the card header (or as a trailing row underneath the list). Confirm the target route (likely `/{portSlug}/admin/audit-log`) and permission-gate the link by the same `audit_log.view` perm the admin sidebar uses, so non-admin reps see the card but not the link. ~10 min.
|
||||
|
||||
1. **Dev-mode banner dismissible** — _src/components/shared/dev-mode-banner.tsx:23_ — added X close button + localStorage persistence keyed by redirect address. Fixed in this session.
|
||||
@@ -72,12 +149,73 @@ _Component refactors, multi-file edits, single-service tweaks, new validators._
|
||||
|
||||
> **Outstanding (gaps on shipped work + rapid UAT capture):**
|
||||
>
|
||||
> - **Platform-wide admin-settings tooltip audit — add explainers wherever a setting isn't self-explanatory to a basic admin user** — _src/components/admin/_ (every settings/form component) + _src/components/admin/shared/registry-driven-form.tsx_ (the unified driver many admin pages use) + _src/components/ui/_ (new `<FieldLabel>` primitive bundling label + optional info-icon tooltip). The custom-field form's "Sort Order" label is one example of a recurring problem across admin pages: many fields carry ambiguous labels (Weight, Priority, Display order, Threshold, Confidence, TTL, Cap, etc.) with no inline explainer. Basic admins are forced to guess, ask another team member, or read source code.
|
||||
> - **Approach (single audit pass, lots of surface area):**
|
||||
> - **(a) Convention via shared primitive** — new `<FieldLabel htmlFor={...} tooltip={...}>{label}</FieldLabel>` component that renders the `<Label>` + (when `tooltip` is set) a small `<Info>` icon button that opens a `<Tooltip>` (hover on desktop, tap on mobile via Radix). Drop-in replacement for every `<Label>` in admin forms. Eliminates inconsistent tooltip styling and makes future additions trivial.
|
||||
> - **(b) Audit pass surface-by-surface** — sweep every admin page + dialog:
|
||||
> - `src/components/admin/custom-fields/` (Sort Order — confirmed)
|
||||
> - `src/components/admin/settings/settings-manager.tsx` (any setting with non-obvious unit/scale)
|
||||
> - `src/components/admin/shared/registry-driven-form.tsx` — when a registry entry has a `description` already defined, it should auto-flow into the tooltip; sweep the registry definitions for missing descriptions
|
||||
> - `src/components/admin/email/` (email send-from / IMAP setup — bounce-poller, attachment threshold, ...)
|
||||
> - `src/components/admin/branding/` (PDF logo scale, brand naming convention, ...)
|
||||
> - `src/components/admin/users/` (role-permission matrix, override hierarchies, ...)
|
||||
> - `src/components/admin/roles/` (permission scope semantics)
|
||||
> - `src/components/admin/vocabularies/` (per-port vocabulary overrides — how cascades work)
|
||||
> - `src/components/admin/ai/` (model selection, confidence thresholds, budget caps)
|
||||
> - `src/components/admin/storage/` (S3 vs filesystem, when each makes sense, migration warnings)
|
||||
> - `src/components/admin/templates/` (template merge fields, allowed-tokens semantics)
|
||||
> - `src/components/admin/forms/` (form-template field types, public form behavior)
|
||||
> - `src/components/admin/documenso/` (per-port API key vs env fallback, v1 vs v2, sendMode)
|
||||
> - `src/components/admin/audit/` (retention, severity filters)
|
||||
> - Anywhere using `<Switch>` + `<Label>` together (often pure toggle with no context)
|
||||
> - **(c) Tooltip-writing guidelines** (put in a brief CLAUDE-style note inline near `<FieldLabel>`):
|
||||
> - 1-2 sentences max, plain English, end with a usage tip when applicable
|
||||
> - State the unit explicitly when applicable ("...in days", "...in MB", "...in feet")
|
||||
> - Mention default behavior when relevant ("Leave 0 to use the system default")
|
||||
> - For dangerous settings, lead with the risk ("Changing this triggers a re-index of every berth — schedule for low-traffic hours")
|
||||
> - Don't restate the label; explain the **why** and **how to choose a value**
|
||||
> - **(d) i18n-ready** — tooltip text routes through the existing i18n catalog so future localization passes don't need a re-audit. Where i18n keys don't exist yet, create them on the fly.
|
||||
> - **Acceptance criteria:** every admin form field without an obvious meaning has a tooltip. Definition of "obvious": a label like "Name" or "Email" is self-explanatory; "Sort Order" / "Weight" / "Threshold" / "Cap" / "TTL" are not.
|
||||
> - **Effort:** ~6-10h end-to-end (FieldLabel primitive + audit ~15-20 admin pages × ~10-15 fields each, write 1-2 sentence tooltips per ambiguous field, sweep registry-driven-form description gaps). Worth a focused half-day. Captured 2026-05-21 from UAT.
|
||||
> - **Platform-wide form-error UX: scroll-to-first-error + focus + summary banner (29 form surfaces)** — _new_ `src/hooks/use-form-scroll-to-error.ts` + _src/components/forms/_ (form-error-summary component) + audit pass over every `useForm` + `zodResolver` caller in `src/components` (29 files including expense-form-dialog, client-form, interest-form, yacht-form, company-form, reservation forms, admin forms, …). Today's pattern: a form with validation errors renders per-field messages via `{errors.X && <p className="text-xs text-destructive">{errors.X.message}</p>}` (good), but on submit-with-errors there's no scroll-to-first-error, no focus-the-failed-field, and no summary banner — so the user just gets dropped at the top of the form with no indication of what failed. Especially bad on tall drawers/dialogs where the failing field is below the fold. Surfaced via expense-form-dialog UAT 2026-05-21.
|
||||
> - **Fix shape:**
|
||||
> - **(a) Shared hook** `useFormScrollToError(formMethods)` — wraps `handleSubmit` to add an `onError` callback that: (i) reads `errors` from react-hook-form, (ii) finds the first errored field's DOM node by `name` attribute (or `id`), (iii) `scrollIntoView({ block: 'center', behavior: 'smooth' })`, (iv) focuses the input (`.focus()`). For drawer/dialog content, scroll inside the scrolling container rather than the page.
|
||||
> - **(b) FormErrorSummary component** — renders at the top of the form when there are ≥ 2 validation errors: a small red banner listing each failed field as an anchor link ("Amount is required · Currency is required") that on click scrolls + focuses that field. For a single error, hook-only (no banner needed — scroll handles it).
|
||||
> - **(c) Audit pass:** verify every zod schema has explicit error messages on required fields (`.min(1, 'Amount is required')` not bare `.string()`); fix the bare cases. The default zod "Required" message is generic and unhelpful.
|
||||
> - **(d) Consistent inline error rendering:** standardize the per-field error block into a small `<FormFieldError errors={errors} name="amount" />` helper so we don't keep open-coding the `{errors.X && <p ...>{errors.X.message}</p>}` block in every form. Migrate the existing 29 surfaces opportunistically.
|
||||
> - **Behavior on success:** unchanged — submit proceeds, drawer/dialog closes, toast fires.
|
||||
> - **Mobile consideration:** on tall mobile-bottom-sheet forms, scroll-to-first-error needs to scroll the sheet content, not the page (otherwise nothing visible changes). The hook detects the scrolling ancestor at runtime.
|
||||
> - **Effort:** ~3-4h end-to-end (hook + summary component + 29-form audit + zod-message fixes). Captured 2026-05-21 from UAT.
|
||||
> - **Berths list "Active interests" column: static count → click/hover popover with interest details + stage-colored count chip** — _src/components/berths/berth-columns.tsx:288-297_ (current static number cell) + _src/lib/services/berths.service.ts (list endpoint extension)_ + new component `<BerthInterestsPopover berthId={...} count={...} highestStage={...} />`. Today renders just `1` / `3` / `—` — unscannable when a rep wants to know WHO has interest in a berth.
|
||||
> - **Design (locked recommendation, can revisit at remediation):**
|
||||
> - **Cell:** count chip (`1`, `3`) with subtle outline + hover/focus indicator. Color-coded by the **highest-active-stage** interest on the berth (e.g., border-red-500 if any at Contract, border-amber-500 at Reservation, border-emerald-500 at EOI+, neutral when only at earlier stages). Encodes stage urgency without expanding.
|
||||
> - **Click/hover (desktop and mobile via Radix Popover):** opens a popover listing each active interest. Each row: client name (link to client detail) · stage badge · berth label (this berth's mooring + role: primary / in EOI bundle / specific interest) · created date · "Open interest →" link to the interest detail. Sort by stage desc so the most-progressed deal sits at top.
|
||||
> - **Empty state (count = 0):** column shows `—` (no popover trigger). Today's behavior, unchanged.
|
||||
> - **Mobile:** tap-to-open via Radix Popover's built-in mobile UX. Width capped at `min(360px, calc(100vw - 32px))` so the popover stays usable on small screens.
|
||||
> - **Service-side:** extend the berths-list response to include `topActiveInterests: Array<{interestId, clientId, clientName, pipelineStage, isPrimary, isInEoiBundle, isSpecificInterest, createdAt}>` (cap at top 5, "View all" link in the popover footer when > 5). Single query that returns this alongside the count via `array_agg` in the existing correlated subquery — no N+1.
|
||||
> - **Permission gating:** the popover row's "Open interest →" link respects `interests.view`. Client name link respects `clients.view`. Hide entire popover when neither perm is held (count chip becomes static for view-only roles).
|
||||
> - **Effort:** ~2-3h end-to-end (service extension + popover component + stage-color logic + tests). Captured 2026-05-21 from UAT.
|
||||
> - **Interest Overview Email + Phone rows: combobox picker across client's contacts + quick-add new contact** — _src/components/interests/interest-tabs.tsx:958-1000 (Email + Phone EditableRow blocks)_ + _src/components/interests/interest-tabs.tsx:122-129 (`clientPrimaryEmail/Phone[ContactId]` types)_ + _src/lib/services/interests.service.ts (getInterestById)_ + _src/lib/services/client-contacts.service.ts_ + _new_ component `<ClientContactPicker channel="email|phone" clientId={...} selectedContactId={...} onSelect={...} />`. Today's surface shows ONLY the client's primary email + primary phone via inline editor. Two real gaps surfaced UAT 2026-05-21:
|
||||
> - **Gap 1 — empty state has no quick-add:** when client has no primary contact for the channel, line 976-978 renders `<span>—</span>` with no affordance. Rep has to navigate to the Client Detail's Contacts tab to add one. Should expose `+ Add email` / `+ Add phone` inline that POSTs a new `client_contacts` row + marks isPrimary=true.
|
||||
> - **Gap 2 — no multi-contact picker:** clients with multiple contacts per channel (e.g. 3 emails — personal, work, assistant) get only the primary shown. Rep can't pick which one applies to THIS deal. Picker needs a dropdown listing every contact for the channel, pre-selecting the current primary, with each row showing the value + label (Personal / Work / etc.) + a `Set as primary` action + a `+ Add new email` / `+ Add new phone` row at the bottom that POSTs a new client_contacts row.
|
||||
> - **Inheritance clarification — current model already does this:** there's no separate `interests.contactEmail/Phone` column today. The displayed Email/Phone ARE the client's primary contacts (resolved server-side, edited in place via PATCH to `client_contacts`). So edits at the interest level auto-update the client. The user's "vice versa" framing assumes per-interest contact overrides exist — they don't.
|
||||
> - **Two design options for the picker semantics:**
|
||||
> - **Design A (recommended, single source of truth):** picker just chooses which contact to set as `isPrimary=true` for this client. Affects every other surface that reads `clientPrimaryEmail`. No schema change. Simpler.
|
||||
> - **Design B (per-interest contact override):** add `interests.preferred_email_contact_id` + `preferred_phone_contact_id` nullable FK to a specific `client_contacts` row. Each interest can pin a non-primary contact for itself; falls back to client's primary when null. Schema change + service-layer fallback logic + UI to mark "use this for this deal only". Useful only if a single client routinely buys multiple deals with different contact preferences per deal — uncommon for marina sales.
|
||||
> - **Decision-pending:** lean Design A unless leadership confirms the multi-deal-per-client divergence case is real.
|
||||
> - **Effort:** ~3-4h for Design A end-to-end (picker component + empty-state quick-add + service-side `setPrimary` action + tests + accessibility). ~5-7h for Design B with the schema + fallback logic. Captured 2026-05-21 from UAT.
|
||||
> - **Inline phone editor on the Contact row** — _src/components/interests/interest-tabs.tsx:973_ — current implementation uses a plain `InlineEditableField` text variant on Phone, so reps can't pick a country code from a dropdown or get AsYouType formatting (both available via `<PhoneInput>` in `src/components/shared/phone-input.tsx`). Wrap `PhoneInput` in a display-vs-edit toggle and PATCH both `value` (national string) + `valueE164` + `valueCountry` to `/api/v1/clients/{id}/contacts/{contactId}`. ~30-60 min.
|
||||
> - **ft ↔ m unit switching on Berth Requirements** — _src/components/interests/interest-tabs.tsx_ — the three inline-editable dim rows hard-code `(ft)` in the label. The interest already carries `desiredLengthUnit` ('ft' | 'm'); other surfaces (BerthRecommenderPanel) honour it. Add a small unit toggle that flips the rendered display (and converts on save so the canonical `desired*Ft` column stays in feet). Same pattern as elsewhere in the app (per CLAUDE.md mooring/berth dims model). ~30-45 min.
|
||||
> - **Client Overview should summarize current interest's requirements** — _src/components/clients/_ — one-line "current interest needs X × Y × Z" summary on the client detail Overview tab; reps currently have to drill into Interests tab to see what a client wants. ~30 min.
|
||||
> - **Duplicate Reminder surfaces on Interest Overview** — _src/components/interests/interest-tabs.tsx_ — the legacy "Reminder" panel (driven by `interest.reminderEnabled / reminderDays / reminderLastFired`) and the new "REMINDERS" section (driven by the `reminders` table via the bell-in-header) both render on the same tab and tell different stories. The legacy field still drives a real backend worker (`processFollowUpReminders` in `reminders.service.ts:428` — creates auto-follow-up reminders when no activity in N days), so we can't just delete the field. Approach: hide the legacy "Reminder" panel from the OverviewTab grid; surface the recurring-follow-up config either as a slim row inside the REMINDERS section or as a setting on the interest detail header. Keep the worker untouched. ~1 h.
|
||||
> - **LinkedBerthsList: no "add another berth" affordance from the card** — _src/components/interests/linked-berths-list.tsx_ — multi-berth interests are first-class (`interest_berths` is the source of truth per CLAUDE.md) but the LinkedBerthsList card doesn't expose an inline "Add a berth" button. Reps have to use the BerthRecommenderPanel below — discoverability gap. Add a CTA button to the card header (gated by `berths.edit`) that opens a picker / sheet to add another `interest_berths` row. ~45 min.
|
||||
> - **Supplemental-info-request: link should be reusable, not single-use** — _src/lib/services/supplemental-info_ (token model) — current email says "can only be used once"; user wants it valid until expiry so a partial submission can be revisited. Drop the single-use guard, keep TTL gate. Audit the public endpoint to ensure no token-fingerprint reuse risk before lifting the limit. ~30 min.
|
||||
> - **Supplemental-info-request: distinct Regenerate vs Resend actions + issue history** — _src/components/interests/supplemental-info-request-button.tsx:83_ (the current "Resend" label) + _src/lib/services/_ (the issue endpoint that today mints a new token on every POST) — once the link becomes reusable-until-expiry (per the "should be reusable, not single-use" finding above), the single "Resend" button conflates two semantically different actions: (a) mint a NEW token (invalidates the previous one — needed when the old one expired, was leaked, or the client deleted the email), and (b) re-email the EXISTING still-valid token (needed when the client just lost the email — same token, same form-state, just push through SMTP again so they can pick up where they left off). The current implementation always does (a) — the "Resend" copy is misleading. Plus once we have reusable tokens, the rep loses visibility into "what token did we send when?" — the inline `link` state only holds the last-minted one.
|
||||
> - **Fix:**
|
||||
> - **(a) Service split:** `regenerateSupplementalLink(interestId)` mints a new token + invalidates outstanding ones for the same interest (or keeps them parallel — design call; recommendation: invalidate, so one client only has one valid link at a time and the rep doesn't have to reason about which one is which). `resendSupplementalLinkEmail(tokenId)` emails the named existing token via SMTP without mutating the token table. Two API routes: `POST /api/v1/interests/{id}/supplemental-info-request` for regenerate, `POST /api/v1/interests/{id}/supplemental-info-request/{tokenId}/resend` for resend.
|
||||
> - **(b) UI:** swap the single button for a small action group that surfaces the most recent valid token's metadata (`Issued <relative time> · expires in <N days>`) with two buttons next to it — `Resend email` (primary, fires resend on the existing token) + `Regenerate link` (ghost, mints new). If no valid token exists, show only `Generate link`. Pair this with the "separate generate + send" finding below so the rep can also generate-without-sending (e.g. share through WhatsApp).
|
||||
> - **(c) History:** small expandable section "View past requests" listing the last 3-5 issued tokens with timestamp + status (active / expired / submitted / revoked). Each row gets a "Revoke" action for the active ones (defensive — covers the "we sent it to the wrong email" case). Schema-wise this is just rendering existing rows in the supplemental-info-tokens table.
|
||||
> - **Effort:** ~2-3h end-to-end including the service split, two API routes, UI rework, audit-log entries on each action, and a vitest covering the resend-doesn't-mutate-token guarantee. Captured 2026-05-21 from UAT. Cross-ref: ties into the "link should be reusable, not single-use" + "separate generate link and send email" findings — best done as one coherent rework.
|
||||
> - **Supplemental-info-request: separate "generate link" and "send email"** — _src/components/interests/supplemental-info-request-button.tsx_ — currently one button auto-generates + sends. User wants two steps: button 1 generates + shows the link (rep can copy / share manually); button 2 sends the templated email through SMTP. Backend change: split the existing service into `generateSupplementalLink()` and `sendSupplementalLinkEmail(linkId)`. UI change: replace single-click action with two-step UI showing link state. ~1 h.
|
||||
> - **Past-milestones strip → expandable history with inline doc preview** — _src/components/interests/interest-tabs.tsx_ (the past-milestones strip at ~line 863) — currently a one-line collapsed summary per past milestone (just title + summary). Reps want to drill into the history of a specific milestone (e.g. see which EOI round was signed, the doc contents, who signed, when). Convert the strip into an accordion: each past milestone expands to show its associated docs + sub-status timeline + inline PDF preview using the existing pdf viewer primitive. Useful for deals with multiple EOI rounds (rework after rejection, re-sent reservation agreements, etc.) where audit trail matters. ~3-4 h.
|
||||
> - **InterestBerthStatusBanner: name + link the competing deal** — _src/components/interests/interest-berth-status-banner.tsx_ — the banner that surfaces when a linked berth is under offer to a different active deal currently just says "this berth is under offer elsewhere" without identifying which interest. Reps want a small inline detail: client name + deal stage + a link button to the competing interest, so they can size up the situation (e.g. "this lead won't make it, treat ours as backup"). Service-side: extend the `getInterestBerthStatus()` (or equivalent) response with a `competingInterest: { id, clientName, pipelineStage, ... } | null` field, then surface in the banner. Permission-gate the link by `interests.view`. ~1 h.
|
||||
@@ -91,6 +229,20 @@ _Component refactors, multi-file edits, single-service tweaks, new validators._
|
||||
> - (d) UI: replace the literal `"Dimensions"` header string in each column definition with a small `<DimensionUnitToggle />` component (label + segmented toggle `ft | m`). Column body cells render via the formatter. Apply to all 5 surfaces in one pass for visual consistency.
|
||||
> - (e) Document-generation path: leave EOI / contract / template merge-field rendering untouched — it already pulls entry-unit values per `effectiveDimensionUnit` in the EOI dialog (per CLAUDE.md merge-field architecture).
|
||||
> - **Effort:** ~1.5-2h end-to-end (helper + pref + hook + toggle component + 5 column-definition swaps + a vitest for the formatter). The toggle persists across page reloads + tabs by virtue of going through `/me/preferences`. Captured 2026-05-18 from UAT.
|
||||
> - **Berth list: "Rates (USD)" + "Pricing valid" columns hidden by default (or removed) — short-term rental fields irrelevant to purchase/long-term ports** — _src/components/berths/berth-columns.tsx:391-417_ + _src/lib/db/schema/berths.ts:62-69_ + the NocoDB import that populates them. These columns surface daily/weekly slip rental rates + a `pricing_valid_until` date — relevant for marinas that lease berths by the day/week (transient marinas), irrelevant for Port Nimara's sales-only model. Visible by default in `DEFAULT_VISIBLE_COLUMNS` (line 123-124), so every Port Nimara user sees two columns of `—` cluttering the table.
|
||||
> - **Path (recommended): hide by default, keep available in column picker.** Drop `'rates'` + `'pricingValidUntil'` from the default-visible array; reps at a transient-rental port can enable via the existing Columns picker. Preserves the schema + import paths for future ports without removing functionality. ~5 min.
|
||||
> - **Smarter alternative (Path 3 in the chat thinking):** conditional default-visibility — only include `'rates'` + `'pricingValidUntil'` in the default-visible set if the port has at least one non-null rate value. Auto-shows for ports that use them, auto-hides for ports that don't. ~30 min including the port-level data check + cache invalidation when rates land. More polished but heavier.
|
||||
> - **Aggressive alternative (Path 1):** delete the columns + the four `*_usd` schema columns + the import paths if no port ever plans to use them. Decision: defer until we know whether ANY port in the roadmap does transient rentals. For now, hide-by-default is the right call.
|
||||
> - **Bundle with:** the "trim default-visible columns" recommendation in the platform-wide table-density finding below — same audit pass, same author.
|
||||
> - **Effort:** ~5 min (Path: hide-by-default). Captured 2026-05-21 from UAT.
|
||||
> - **Platform-wide table density: cells shrink-wrap content instead of triggering horizontal scroll — columns need min-widths + nowrap defaults** — _src/components/ui/table.tsx:7_ (wrapper is already `overflow-auto`, good ✓) + _src/components/ui/table.tsx (TableCell base — missing `whitespace-nowrap`)_ + _src/components/berths/berth-columns.tsx_ (no `size`/`minSize` on any column except line 447's `size: 48` outlier) + every other DataTable column definition in the app. Surfaced on the berths list (UAT 2026-05-21): with ~14 columns visible, every cell wraps into 3-6 lines because the table tries to fit everything in viewport. Example pain: "Bull bollard type B · 40 ton break load" wraps into 6 lines; "63m × 14.19m (draft 4.42m)" wraps into 3 lines; "Car (3t) to Vessel" wraps into 3 lines. Result: row height bloats to 200px+, the table becomes nearly unusable.
|
||||
> - **Fix (platform-wide, single PR):**
|
||||
> - **(a) TableCell base default:** add `whitespace-nowrap` to the base TableCell className in `src/components/ui/table.tsx`. Single-line content stays single-line. Cells that genuinely need wrapping (long note teasers, etc.) opt-out via `className="whitespace-normal"` per-cell.
|
||||
> - **(b) Per-column `min-w-[X]` token system:** define a small set of width tokens in a shared helper based on content type — `colW.short` (status badges, count chips), `colW.medium` (mooring numbers, short labels), `colW.long` (dimensions, addresses), `colW.money` (price columns). Apply via TanStack `size: ...` or via cell className `min-w-[X]`. Reuse across every DataTable.
|
||||
> - **(c) Truncate-with-tooltip for verbose cells:** the Cleat / Bollard / Access columns carry strings like "Bull bollard type B · 40 ton break load" — too long for any reasonable column width. Apply `truncate max-w-[200px]` + `title={value}` so the cell shows ellipsis + full text on hover. Optionally wrap in a `<Tooltip>` for touch parity on mobile.
|
||||
> - **(d) Audit visible-by-default columns:** with 14 columns showing on the berth list, even with correct widths the table is overwhelming. Trim the default-visible set to 7-8 essentials (Mooring, Area, Latest deal stage, Active interests, Dimensions, Boat size, Price, Status) and move the rest behind the existing Columns picker (already wired per CLAUDE.md). Reps who need bollard/cleat/access details can enable those columns explicitly.
|
||||
> - **Apply to all DataTable surfaces:** berths list, interests list, clients list, yachts list, companies list, reservations list, invoices list, audit-log list, expenses list. Each has its own column file; single audit pass tags the min-w token per column.
|
||||
> - **Effort:** ~3-4h end-to-end (TableCell base + width token helper + column-def sweep + truncate-tooltip on verbose cells + default-visible audit). Captured 2026-05-21 from UAT.
|
||||
> - **Berth list "Latest deal stage" column: make sortable by pipeline-stage rank** — _src/components/berths/berth-columns.tsx:273-287_ + _src/lib/services/berths.service.ts:80-120_ — the column currently has `enableSorting: false`; sorts by status / area / active interests / etc. already work via the existing `sortColumn` switch + `customOrderBy` correlated-subquery pattern (see `activeInterestCount` at lines 107-120). latestInterestStage isn't a column on `berths` — it's the highest-ranked active interest's stage, populated in a two-pass post-fetch.
|
||||
> - **Fix:** (a) drop `enableSorting: false` on the column. (b) Add a `'latestInterestStage'` case to the sortColumn switch returning `null` (handled in customOrderBy, like `activeInterestCount`). (c) Add a `stageSort` correlated subquery mirroring `demandSort`: select the rank of the highest-active-stage interest per berth via a `CASE i.pipeline_stage WHEN 'enquiry' THEN 1 WHEN 'qualified' THEN 2 ... WHEN 'contract' THEN 7 END` ladder, then `ORDER BY ... ASC/DESC` per `query.order`. Filter same as demandSort (`port_id`, `archived_at IS NULL`, `outcome IS NULL`). Berths with no active interest → NULL; use `NULLS LAST` (ascending) / flip per direction so they land at the bottom regardless.
|
||||
> - **Effort:** ~45 min. Pure additive — no schema work, no API contract change. Captured 2026-05-18 from UAT.
|
||||
@@ -112,14 +264,19 @@ _Component refactors, multi-file edits, single-service tweaks, new validators._
|
||||
> - **(d) Roles & Permissions stays separate** — different concept (template vs individual), low edit frequency, would bury both if merged. Cross-link: each user row's role chip → opens role edit page; role detail page → "N users with this role" with a link back.
|
||||
> - **Permission gating:** confirm the unified page enforces the OR of permissions for both surfaces (`users.view` for the user rows, `invitations.manage` for sending/revoking). The "Invite" button gates on `invitations.manage`; the kebab actions per-row gate appropriately.
|
||||
> - **Effort:** ~3-4h end-to-end — table extension + state filter + invitation rows + the API stitch + redirect + sidebar/catalog cleanup + tests. Captured 2026-05-18 from UAT.
|
||||
> - **Consolidate every AI-feature admin control onto `/admin/ai`** — _src/app/(dashboard)/[portSlug]/admin/ai/page.tsx_ + _src/components/admin/_ (new per-feature embedded forms) + _src/lib/db/schema/ai-usage.ts_ (existing ai*usage table for spend rollup) + \_src/components/admin/ocr-settings-form.tsx* (pattern to mirror) — the AI admin page already has master controls (`ai.master`), provider credentials (`ai.providers`), and the Receipt OCR settings embedded inline. The "Per-feature settings" card I just removed pointed at two dead routes (`../berth-pdf-parser`, `../recommender`) — surfacing the gap that AI feature-tuning isn't consistently centralized. User wants every AI-using feature's admin knobs reachable from one page.
|
||||
> - **Consolidate every AI-feature admin control onto `/admin/ai` — REMOVE from current scattered locations (reinforced UAT 2026-05-21)** — _src/app/(dashboard)/[portSlug]/admin/ai/page.tsx_ + _src/components/admin/_ (new per-feature embedded forms) + _src/lib/db/schema/ai-usage.ts_ (existing ai*usage table for spend rollup) + \_src/components/admin/ocr-settings-form.tsx* (pattern to mirror) — the AI admin page already has master controls (`ai.master`), provider credentials (`ai.providers`), and the Receipt OCR settings embedded inline. The "Per-feature settings" card I just removed pointed at two dead routes (`../berth-pdf-parser`, `../recommender`) — surfacing the gap that AI feature-tuning isn't consistently centralized. User wants every AI-using feature's admin knobs reachable from one page.
|
||||
> - **Scope (only include features that actually call an LLM today; don't include aspirational ones):**
|
||||
> - **Berth PDF parser AI fallback** — 3-tier parse per CLAUDE.md (AcroForm → OCR → optional AI on low confidence). Knobs to expose: provider override (per-feature override of the global `ai.providers` choice), confidence threshold below which the AI tier fires, per-call budget cap, prompt template (advanced/optional). New embedded form `<BerthPdfParserAiSettingsForm embedded />` reading registry section `ai.berth_pdf_parser`.
|
||||
> - **Receipt OCR** — already there ✓
|
||||
> - **Future-feature placeholders explicitly NOT included until they ship:** berth recommender (currently "Pure SQL (no AI)" per CLAUDE.md — surfacing it as an AI setting today would mislead admins into thinking they're tuning an LLM); AI-assisted contact-log action extraction (Bucket 3 #7 future feature); AI inquiry intake parsing if/when it ships. Add each to `/admin/ai` only when the underlying feature lands.
|
||||
> - **AI spend dashboard at the bottom of the page** — new card showing: current month spend total (across all AI features), top 3 features by spend, recent expensive calls (model, feature, cost, timestamp). Reads from `ai_usage` table. Helps admins debug cost spikes without leaving the AI page. Optional but high-leverage for an admin who just saw a budget alert.
|
||||
> - **Cross-linking principle:** each per-feature AI section on `/admin/ai` shows a small "Non-AI settings for this feature live at →" link to the corresponding admin page (e.g. for berth PDF parser, link to wherever the OCR confidence + AcroForm overrides live). Vice-versa: each feature page gets a "AI fallback settings live at /admin/ai →" link in the relevant section. Keeps the split-brain risk in check — admins always have a one-click path between the two.
|
||||
> - **Effort:** ~30 min for the berth PDF parser embedded section + registry definition, ~1.5h for the AI spend dashboard, ~30 min for the cross-link sweep. Total ~2.5h. Captured 2026-05-18 from UAT.
|
||||
> - **Effort:** ~30 min for the berth PDF parser embedded section + registry definition, ~1.5h for the AI spend dashboard, ~30 min for the cross-link sweep, ~30-45 min for the explicit removal-from-other-surfaces audit (grep every admin page for AI toggle / API-key field / model-selector / temperature-slider and migrate to /admin/ai). Total ~3h. Captured 2026-05-18 from UAT, reinforced 2026-05-21 (user spotted yet another scattered AI setting in `src/components/admin/settings/settings-manager.tsx:241` — confirms the consolidation work needs explicit "delete from old location" alongside "add to new location" to avoid drift). Captured 2026-05-21 reinforcement.
|
||||
> - **Explicit removal scope** — audit and remove (not just add to /admin/ai):
|
||||
> - Any AI-related setting inside `settings-manager.tsx` SettingsManager cards
|
||||
> - Any model/temperature/provider fields inside per-feature admin pages (OCR settings, berth-PDF settings, template-editor settings)
|
||||
> - Any AI-related env-resolver fields exposed via RegistryDrivenForm on non-AI admin pages
|
||||
> - Cross-link replaced original location with a small banner: "AI settings for this feature live at `/admin/ai` →" (per the cross-linking principle already in the entry).
|
||||
> - **Password-reveal eye toggle silently no-ops when value resolves from env (or anywhere outside port/global)** — _src/components/admin/shared/registry-driven-form.tsx:440-463_ (eye-toggle click handler) + _src/app/api/v1/admin/settings/[key]/reveal/route.ts_ (server endpoint that intentionally refuses to leak env-resolved secrets per its docstring) — user clicks the eye on a sensitive field and the dots stay, no toast, no error. Root cause: the click handler only fires `reveal.mutate()` when `resolved?.isSet && resolved.source ∈ {'port', 'global'}`. When the value is resolved from `env` (legacy `.env` fallback) or `default`, the handler skips the reveal call and just sets `setShowSecret(true)`. The Input then flips `type` from `password` to `text` — but the draft is still empty, so the placeholder `'••••••••'` (set unconditionally for `sensitive` fields at line 555) keeps rendering. Net effect: indistinguishable from "the toggle is broken."
|
||||
> - **Fix options:**
|
||||
> - **(a) Best UX:** show a clear inline message + tooltip on the eye button when `resolved.source === 'env'` (or `'default'`): "Value comes from the environment — cannot reveal in-app. Configure in admin to view." Disable the button or change its tooltip so the user knows why nothing happens. ~15 min.
|
||||
@@ -148,6 +305,80 @@ _Component refactors, multi-file edits, single-service tweaks, new validators._
|
||||
> - **(iii) The `max-h-[300px]` hard cap** clips longer lists. While the cap exists, scrolling SHOULD still reach the end — but combined with (i)/(ii) it caps the effective scroll distance. **Fix attempt:** use a height-aware token: `max-h-[min(400px,var(--radix-popper-available-height,400px))]` so the list grows when the popover has room and caps at 400px otherwise.
|
||||
> - **Investigation order:** (1) check cmdk version + upgrade if old → may auto-fix the focus-scope issue and let us remove the manual wheel handler. (2) Test with manual handler removed. (3) If still buggy, add the controlled-highlight reset on scroll. (4) Bump the max-h as the easy win.
|
||||
> - **Effort:** ~30-60 min including upgrade + testing across the YachtPicker, ClientPicker, CompanyPicker, command-search topbar, and any other Command consumers. Captured 2026-05-18 from UAT — affects every Command-based dropdown app-wide; high-leverage single-component fix.
|
||||
> - **DECIDED 2026-05-21 (do not adopt Documenso embed editor):** evaluated `@documenso/embed-react`'s `EmbedCreateEnvelope` / `EmbedUpdateEnvelope` as a replacement for our custom field-placement UI. Per Documenso V2 editor docs (callout block): _"Embedded editor is included with Enterprise plans. It is also available as a paid add-on for the Platform Plan. Contact sales for access."_ Enterprise licensing is a hard no for us. Custom rebuild is the path. We're already ~70% there with `upload-for-signing-dialog.tsx`; remaining scope is the 4-item bundle below (~12-16h total). Full V2-editor parity (multi-file envelopes, Assistant/Viewer roles, dictate-next-signer, all envelope settings) would be ~30-40h but is not justified by our actual marina-CRM flows. Skip multi-file/assistant role; defer per-document envelope settings (expiration / redirect / custom reply-to) until a rep actually asks for them.
|
||||
> - **Smart search: fuzzy-match pipeline stage names, surface inline mini-list of interests at that stage** — _src/components/search/command-search.tsx_ + _src/lib/services/search.service.ts_ + _src/lib/constants.ts:31 (`STAGE_LABELS`)_ + _src/hooks/use-search.ts_. Today's command-K search includes each interest's stage in result rows (rendered via `STAGE_LABELS`) but doesn't search BY stage — typing "Reservation" only matches interests with that text in their fields. User wants: type "reservation" → see a dedicated "STAGE: Reservation (N deals)" section at the top of the dropdown listing the top 5-10 interests at that stage, with each row showing client + berth label so the rep can click directly. Bottom of section: "View all N in Reservation →" link to the filtered interests list.
|
||||
> - **Design (locked 2026-05-21):** inline mini-list in the search dropdown (Option A from the design clarification). Top 5-10 interests per matched stage; "View all" link jumps to `/interests?stage=<canonical>`.
|
||||
> - **Backend:** new search section `'stage-matches'` returning `{ stage: PipelineStage, label: string, totalCount: number, sampleInterests: Array<{id, clientName, berthLabel, ...}> }[]`. Fuzzy match the query against `STAGE_LABELS` values + common aliases ("res" → reservation, "eoi" → eoi_signed/eoi_sent, "dep" → deposit_paid, "qual" → qualified, "won"/"contract" → contract_signed, etc.). Use `fuse.js` or a tiny custom ranker on the labels — there are only ~9 stages so even O(n) scan is fine.
|
||||
> - **Frontend:** new section in the command-search dropdown rendered above the text-match "Interests" section. Borrow the existing `SectionHeading` + result-row idioms. Use the existing `berthLabel` helper (the same one used in the DocumentDetail Interest link fix and the external-EOI title default) so naming is consistent across surfaces.
|
||||
> - **Alias catalog (lives next to `STAGE_LABELS`):** add a small `STAGE_SEARCH_ALIASES: Record<PipelineStage, string[]>` map for non-obvious matches ("hot lead" → qualified? probably not, leave it conservative). Aliases stay short and unambiguous — prefer false negatives over false positives.
|
||||
> - **Effort:** ~2-3h end-to-end (backend section + fuzzy ranker + frontend render + alias catalog + a vitest covering "res" matching reservation but not "reservation_signed" if that's a thing). Captured 2026-05-21 from UAT.
|
||||
> - **Watchers configurable at document creation time (currently post-creation only)** — _src/components/documents/eoi-generate-dialog.tsx_, _src/components/documents/upload-for-signing-dialog.tsx_, _src/components/interests/external-eoi-upload-dialog.tsx_, _src/components/documents/create-document-wizard.tsx:157_ (hardcoded `watchers: []`), _src/lib/services/documents.service.ts (create paths)_, _src/lib/services/document-watchers.service.ts_ (or wherever the watcher CRUD lives). Today watchers can only be added AFTER creation via WatchersCard on the document detail. Reps usually know upfront who needs visibility (manager, developer, legal) and shouldn't have to navigate to the doc after creating it.
|
||||
> - **Server-side defaults (fires on every create path):**
|
||||
> - **Creating user** — always auto-added. The person who just created the doc almost certainly wants notifications about events on it.
|
||||
> - **Interest's `assignedTo`** — if different from creator, auto-add. The deal owner gets visibility on doc events even if a different rep generated/uploaded.
|
||||
> - **Per-port admin setting `default_document_watcher_user_ids: string[]`** — admin configures org-wide watchers (sales manager, legal, etc.). Apply on every create. Configurable in `/admin/settings` under a new "Document defaults" section.
|
||||
> - **UI in each creation dialog:** small "Watchers" section (collapsed by default — "X watchers · Edit"), opens to show:
|
||||
> - Each auto-added user with an `(auto)` badge so the rep can see who's already included without redundant clicks.
|
||||
> - A user-picker to add additional watchers from the port's user roster.
|
||||
> - An "X" to remove an auto-added watcher for this specific doc (e.g. rep wants this confidential, removes the default sales-manager). Doesn't affect the global default.
|
||||
> - **Apply to ALL creation dialogs uniformly:** Documenso EOI generate, external EOI upload, Documenso upload-for-signing (reservation/contract), generic create-document wizard. Build the section as a shared `<DocumentWatchersField>` primitive so each dialog mounts it the same way.
|
||||
> - **Service:** extend the document-create endpoints to accept `watcherUserIds: string[]` (replacing the current `[]` hardcode). On create, server: (i) inserts the explicit user IDs from the request, (ii) inserts the per-port default IDs, (iii) inserts creator + assignedTo if not already in the union. Dedupe by user_id + document_id (existing unique index, presumably).
|
||||
> - **Effort:** ~3-4h end-to-end (admin setting + UI section + 4 dialog wirings + service-side defaults + tests). Captured 2026-05-21 from UAT. **Cross-ref:** ties into the external-EOI bundle below — the watchers section sits naturally next to the signatories editor in the same dialog. Build them in one pass.
|
||||
> - **External-EOI upload: per-signatory role tagging + email auto-fill + "Email copy" distribution** — _src/components/interests/external-eoi-upload-dialog.tsx_ (current free-text `signerNames` field; needs structured rows) + _src/components/documents/document-detail.tsx:208-214 + 297-299_ (current "Email signatories" placeholder stub) + _src/lib/services/system-settings/_ (new `default_developer_email` + `default_developer_name` per port) + _new_ `src/components/documents/email-copy-dialog.tsx` + _src/lib/services/document-sends.service.ts_ (already exists per CLAUDE.md, extend for the new dispatch path). Three linked feature gaps surfaced during UAT 2026-05-21 on the external-EOI flow:
|
||||
> - **(a) Per-signatory role tagging at upload time** — today's dialog has a free-text "Signer names" CSV input only. No structured concept of WHO each person is (Client vs. Developer vs. Rep vs. Witness vs. CC), so the system can't auto-fill emails downstream or build a proper recipient list for the email-copy flow. **Fix:** replace the freetext field with a recipient list editor (same idiom as the Documenso upload-for-signing-dialog's recipients step — name + email + role per row + add/remove buttons). Add a `signatories: Array<{ name, email, role }>` field to the service's input shape; persist on the document row (existing `documents.metadata` JSONB or a dedicated `document_signatories` table — TBD by scope, JSONB is cheaper for v1). Role enum: `'client' | 'developer' | 'rep' | 'witness' | 'cc'`.
|
||||
> - **(b) Smart email + name auto-fill based on role** — when a rep adds a row and selects a role, the dialog pre-populates name + email from the right source. Rep can still edit. Sources:
|
||||
> - **Client** → `interests.clientId` → `clients.contacts` where `channel='email' AND isPrimary=true`, fallback to first email. Name from `clients.fullName`.
|
||||
> - **Developer** → new per-port system settings `default_developer_name` + `default_developer_email` (admin-editable in `/admin/email` or a new "Default signatories" section). Surfaces consistently across EOI / Reservation / Contract upload flows.
|
||||
> - **Rep** → `interests.assignedTo` → `users.email` + `users.fullName`.
|
||||
> - **Witness / CC** → no auto-fill, manual entry. Rep optionally types or picks from a contacts autocomplete.
|
||||
> - **UI:** when role is selected and a row's email/name is empty, fire `setValue` with the resolved default. If the resolved data is missing (e.g. no clientId on the interest, no developer configured), show a small "No default available — enter manually" hint inline.
|
||||
> - **(c) "Email signatories" → "Email copy" with multi-select + actual send** — _document-detail.tsx:208-214_ — current button is a placeholder. Build the real flow:
|
||||
> - **Rename** "Email signatories" → "Email copy" (clearer intent: "send a copy of the signed document").
|
||||
> - **Dialog UX:** click opens a dialog listing every signatory on the document (from (a)'s structured list) as a checklist. **All checked by default.** Optional "Add other recipient" row at the bottom for emailing someone not on the original signing list (lawyer, accountant, etc.). Optional message field (plain text, like the existing send-out compose UI).
|
||||
> - **Send pipeline:** uses the existing sales send-out infrastructure (per CLAUDE.md "Send-from accounts" section): nodemailer transport with per-port `default_developer_email` → no wait, that's the sales send-from. Send-from is the configured `sales_send_from` mailbox. Body is rendered via `renderEmailBody()` (per CLAUDE.md "Audit → document_sends" section). Each send creates a `document_sends` row keyed to the document + recipient, supporting bounce tracking + reply monitoring.
|
||||
> - **Attachment:** PDF threshold check (per the existing `email_attach_threshold_mb` setting) — under threshold → attached inline; over → 24h signed-URL link (escapes filename per the existing XSS protection).
|
||||
> - **Audit trail:** each recipient gets a `document_sends` row. Existing "Recent sends" / activity surfaces light up automatically.
|
||||
> - **Rate limit:** existing 50-sends-per-user-per-hour cap applies.
|
||||
> - **(d-prereq) Create `document_signers` rows on external upload so "X / Y signed" badge works** — _src/components/documents/document-detail.tsx:278_ reads `signers.filter(s => s.status === 'signed').length / signers.length` from the `DetailSigner[]` array. For manually-uploaded external EOIs the array is empty (the upload writes only freetext `signerNames` metadata) → badge renders `0 / 0 signed` even with 3 signers entered in the dialog. Fix is downstream of (a): when migrating from freetext to the structured `signatories: Array<{name, email, role}>` shape, the service should also insert `document_signers` rows (one per signatory), all pre-stamped `status='signed'`, `signedAt=input.signedAt`, `signingOrder=index+1`, `invitedAt=null` (no invitation was sent — this is a backfill of an external signing event). Counter then renders `3/3 signed` correctly. ~15 min on top of (a)'s service work. Captured 2026-05-21 from UAT.
|
||||
> - **(d) Default document title should reference client + berth(s), not just date** — _external-eoi-upload-dialog.tsx:103_ (current placeholder `'External EOI - <date>'`) — when the rep accepts the default, the document lands as `External EOI — 2026-05-21`, which is unscannable in any document list when a port has multiple deals closing on the same day. **Fix:** derive the default at dialog open time using the same `formatBerthRange()` helper that powers the locked folder-naming convention (Bucket 4 #5). Format: `External EOI — <Client name> — <berth range> — <YYYY-MM-DD>` (e.g. `External EOI — Matthew Ciaccio — A1-A3, B5-B7 — 2026-05-21`). When no client or berths are linked, gracefully fall back to the current minimal form. Apply the same idiom to the Reservation + Contract external-upload dialogs for consistency. ~15 min.
|
||||
> - **Effort:** ~5-7h end-to-end. ~1.5h for (a) — structured recipient editor + service shape change + migration if a dedicated table is preferred. ~1h for (b) — auto-fill resolver + admin setting for developer defaults + UI wiring. ~3-4h for (c) — dialog + send service + branded email template + audit + attachment-vs-link logic. ~15min for (d). Captured 2026-05-21 from UAT. **Cross-ref:** the broader UploadForSigningDialog rework (item below) needs the same role-tagging UI — build the recipient-list editor once and reuse on both dialogs. The default-title derivation in (d) also belongs as a shared helper since Reservation/Contract uploads should match.
|
||||
> - **UploadForSigningDialog comprehensive rework — 4 linked issues** — _src/components/documents/upload-for-signing-dialog.tsx_ — surfaced together during UAT 2026-05-21 of the Reservation Agreement send flow. All four touch the same dialog and should ship as one coherent pass.
|
||||
> - **(a) [bug] "Failed to load PDF file" on the place-fields step** — the place-fields step uses `URL.createObjectURL(file)` (line 265) as `fileUrl` and passes it to react-pdf inside `FieldPlacementStep`. `pdf-viewer.tsx:149` `onLoadError` fires when react-pdf can't parse the blob. Likely causes to check: (i) the uploaded file isn't a PDF (PNG, DOCX, etc. — select-file step likely doesn't enforce `application/pdf` mime check); (ii) PDF.js worker URL misconfigured (every PDF fails the same way); (iii) blob revoked too early (`useEffect` cleanup at line 266-270 — though the deps look right); (iv) react-pdf version-incompatible with the worker bundle. **First debug step:** check browser devtools console for the actual error message — currently it's collapsed into a generic "Failed to load PDF file." string. Surface the underlying error to the UI ("Couldn't parse PDF — check that you uploaded a `.pdf` file, not an image or Word doc.") so the rep can self-diagnose.
|
||||
> - **(b) [ux] Dialog way too small for the place-fields step** — dialog is `max-w-5xl` (1024px, line 166) which is fine for the recipients step, but the place-fields step has a 176px-wide field palette + 200px-wide recipients list on the left and only ~650px for the PDF preview on the right. A US Letter page at fit-width in 650px is barely legible, and field placement requires precision. **Fix:** make the dialog adaptive per-step: `max-w-3xl` for select-file + configure-recipients steps (768px is plenty for forms), but expand to `max-w-[1400px]` or `max-w-[95vw]` on the place-fields step where horizontal PDF space matters most. Alternative: full-screen modal pattern for the place-fields step only (escape exits, top bar shows step indicator + Back/Send). Also shrink the field palette from `w-44` (176px) to `w-32` (128px) by using icon-only buttons with tooltips — recovers ~50px of PDF width.
|
||||
> - **(c) [feature gap] PlacedField shape missing `defaultValue` + `fieldMeta` (no UI to configure dropdown options, pre-fills, field labels, validation)** — _line 85-96, PlacedField interface_ — the current shape carries position + type + recipientIndex only. Documenso v2 `field/create-many` accepts per-field metadata that today's UI can't set:
|
||||
> - **Dropdown:** options array. Today: rep places a Dropdown field → recipient sees an empty dropdown at signing time → blocked.
|
||||
> - **Radio:** group label + option array. Same issue.
|
||||
> - **Pre-filled defaults:** e.g., place a Name field assigned to "Matt Ciaccio" recipient + auto-populate with `interest.client.fullName` so the rep doesn't have to retype. Maps to Documenso's `defaultValue` per field.
|
||||
> - **Text validation:** regex, minLength, maxLength — for fields like "passport number" or "phone".
|
||||
> - **Field label:** custom label shown above the field at signing time (today defaults to the type name).
|
||||
> - **Fix:** extend `PlacedField` with `defaultValue?: string`, `fieldMeta?: { options?: string[]; label?: string; required?: boolean; validation?: { regex?: string; minLength?: number; maxLength?: number } }`. Right-side properties panel on field selection (the selected-field UI already exists per the FieldPlacementStep code) gets new inputs per type:
|
||||
> - Dropdown / Radio: textarea for "Options (one per line)".
|
||||
> - Text / Name / Email / Number: input for "Default value" + optional "Pre-fill from" picker (Client name / Client email / Berth mooring / Interest date / …).
|
||||
> - All types: "Required" checkbox + custom Label override.
|
||||
> - The "Pre-fill from" picker is essentially a **per-field merge token** — borrowed from the EOI template merge-field catalog (`src/lib/templates/merge-fields.ts`). Reuse that token list + resolver so the same {{tokens}} that work in EOI templates work as field defaults here. Stitches the two flows conceptually: signer fields can be pre-filled from the same data sources EOI merge fields use.
|
||||
> - **Backend wiring:** extend the v2 `field/create-many` payload in `documenso-client.ts` to pass `defaultValue` + `fieldMeta` (Documenso v2 supports these per their field API).
|
||||
> - **(d) [behavior] Reservation flow should save as draft, not auto-distribute — match EOI pattern** — _line 361 + 477_ (the dialog reads `defaults?.data?.sendMode === 'auto'` system setting + changes the Send button label). User wants reservation agreement to ALWAYS save as Documenso draft so the rep can review in Documenso (preview email copy, double-check field placement, etc.) before manually triggering send. Per CLAUDE.md doc audit, EOI already uses this pattern (v2 `/template/use without distribute` → DRAFT envelope → rep distributes separately). Reservation should mirror.
|
||||
> - **Fix:** option A (per-document choice, recommended) — add a small radio above the Send button in the dialog footer: `⦿ Save as draft (review in Documenso, send later) · ◯ Send immediately`. Default to "Save as draft" for reservation agreements (and contracts, by parity), since these high-stakes documents merit a review step. EOIs already follow the draft pattern; this just brings reservation/contract in line.
|
||||
> - Option B (force manual) — hardcode `sendMode='manual'` for reservation / contract document types regardless of system setting. Less flexible but simpler.
|
||||
> - **Lift the system `sendMode` setting to a per-document-type setting** so admins can independently configure auto-send for EOI / reservation / contract.
|
||||
> - **Effort:** ~6-9h end-to-end for the full bundle (a + b + c + d + sweep of EoiGenerateDialog for parity on items b/c if applicable + tests). The dialog-width fix alone is ~30min; the rest of the work is the field-metadata schema + UI extension which is the heaviest piece. Captured 2026-05-21 from UAT.
|
||||
> - **Skip-ahead backfill flow: surface real backfill controls below the banner (date pickers + signed-doc upload per gap)** — _src/components/interests/skip-ahead-banner.tsx:71_ (banner copy says "Backfill ... below" but nothing renders below), _src/components/interests/interest-tabs.tsx_ (the MilestoneSection past-phase render), _src/lib/services/interests.service.ts_ (PATCH path for date*eoi_sent / date_eoi_signed / date_reservation_signed / date_deposit_received), \_src/components/interests/interest-documents-tab.tsx* (existing upload flow we can lift from) — when a rep manually jumps a deal forward (e.g. Qualified → Reservation via the stage dropdown), the SkipAheadBanner fires and tells them to backfill, but the milestone card immediately below shows checkmarks with no controls to actually (a) set the historical date or (b) upload the signed PDF as evidence. The current `MilestoneAdvanceButton` has the date popover affordance, but it's only rendered for the NEXT unchecked step — past-but-undated steps render as a static checkmark + "—" with no edit affordance.
|
||||
> - **Fix:**
|
||||
> - (a) When a milestone is in the past phase AND its date column is null, render an inline "Set date" button next to the checkmark that opens the same Popover used by `MilestoneAdvanceButton` (date input defaulting to today, accepts any past date). On confirm, PATCH the relevant `date_*` column. No stage transition fires — just a date stamp.
|
||||
> - (b) When a milestone is in the past phase AND its doc-status is not `'signed'` (or there's no associated `files.id` for the signed PDF), render an "Upload signed PDF" button next to "Set date" that opens a file picker, posts to the existing storage path, and flips the matching `*DocStatus` column to `'signed'` (mirrors what the Documenso webhook does on completion). For EOI specifically, the upload should link to the `documents` row representing the EOI so the file lands in the Documents hub via the same auto-deposit flow.
|
||||
> - (c) Banner copy: convert the gap names from passive text into clickable jump-targets that scroll-into-view the corresponding past milestone card (e.g. "EOI sent date · EOI signed date" become anchor links). Reduces the "where is 'below'?" friction.
|
||||
> - **Effort:** ~3-4h. Captured 2026-05-21 from UAT. (Bundles findings #1, #2, #3 below into one coherent backfill UX.)
|
||||
> - **Current-stage milestone hidden under "Upcoming milestones" when its sub-steps are already checked off (active phase mislabelled)** — _src/components/interests/interest-tabs.tsx:611-624_ (`milestoneCompletion` map + `firstIncompleteKey` derivation) — the phase classifier marks a milestone as `'past'` whenever ALL its sub-steps are complete, so when the interest is at Reservation stage with reservation-agreement-signed already ticked (via the manual stage-jump), the Reservation milestone is `past` and EOI (which still has gaps because the rep hasn't backfilled) becomes the `firstIncompleteKey` → flagged as "NEXT STEP". Net effect (image 23): EOI shows as "NEXT STEP" + Reservation gets buried in the "Upcoming milestones" accordion even though it's the actual current stage.
|
||||
> - **Fix:** introduce a third concept besides `past | current | future` — the milestone that owns the CURRENT pipeline stage (regardless of completion) should always be `current` and never be collapsed into the past-strip nor the upcoming-accordion. Compute the rep's "true current" milestone by mapping `interest.pipelineStage` → milestone key (eoi/eoi_sent/eoi_signed → 'eoi'; reservation → 'reservation'; deposit_paid → 'deposit'; contract_sent/contract_signed → 'contract'). The `firstIncompleteKey` rule still works for nurturing / qualified stages where no milestone naturally owns the stage. Past-but-fully-done milestones BEFORE the current stage go in the past-strip; future milestones go in the upcoming-accordion. Pair with the backfill-controls fix above so a "current" milestone with missing dates still has the affordances to fill them.
|
||||
> - **Effort:** ~30-45 min. Captured 2026-05-21 from UAT.
|
||||
> - **Qualification auto-confirm "intent confirmed" once stage ≥ EOI (extend `computeAutoSatisfied`)** — _src/lib/services/qualification.service.ts:342-360_ (`computeAutoSatisfied` only branches on `'dimensions'` — `'intent_confirmed'` falls through to `false`) + the call-site context build at lines 296-316 (needs `pipelineStage` added) — when a rep manually advances the deal past Qualified to EOI/Reservation/Deposit/Contract, "Intent confirmed" still requires an explicit tick. The act of signing an EOI is itself the strongest signal of intent — leaving the row unchecked makes the checklist feel like noise. Extend the auto-satisfaction context with `pipelineStage`, add an `if (key === 'intent_confirmed') return stageIdx > qualifiedIdx;` branch, and `computeEvidence` returns "Stage advanced past Qualified" when triggered. Rep can still untick to overrule.
|
||||
> - **Effort:** ~30 min including the evidence string + an integration test. Captured 2026-05-21 from UAT.
|
||||
> - **Qualification: stale explicit-tick survives removal of underlying auto-evidence (esp. dimensions)** — _src/lib/services/qualification.service.ts:296-334_ (`confirmed: explicit || autoSatisfied`) — `autoSatisfied` is recomputed at fetch time, but `explicit` persists in `interestQualifications.confirmed` once a rep has manually ticked the row. Result: if dims were present at one point, the rep clicked the box (or the auto-tick happened alongside an explicit click somewhere in the flow), then dims are later removed, the row STAYS ticked because `explicit=true` covers for `autoSatisfied=false`. The `AUTO` badge disappears so it now looks like a manual confirmation — but the rep may have no memory of making it. Footgun: checklist claims "Dimensions confirmed" with no underlying data.
|
||||
> - **Fix (recommended — strict for derived-only criteria):** for keys where there's no rep judgement involved (`dimensions` today; future similar "does X data exist" checks), make the row purely derived — ignore `explicit`, return `confirmed: autoSatisfied`. Removing dims always unticks. Keep `explicit || autoSatisfied` for judgement-based keys like `intent_confirmed`. Implement by marking each criterion with a `derivedOnly: boolean` flag (lives next to the auto-rule) and branching in the merge.
|
||||
> - **Alt (lenient with warning):** keep the OR but surface an `inconsistent` flag (`explicit && !autoSatisfied`) — UI renders the row with an amber "Evidence missing — re-verify" annotation, lets the rep re-confirm or untick.
|
||||
> - **Effort:** ~45 min for strict (incl. integration test covering the remove-dims-after-tick flow); ~1h for lenient (annotation + amber styling). Captured 2026-05-21 from UAT.
|
||||
> - **Qualification checklist: collapse to one-line summary once "All confirmed"** — _src/components/interests/qualification-checklist.tsx_ — once every row is confirmed (explicit + auto combined), the full card stops being a gate and just occupies prime Overview real estate. Replace the expanded card with a single-row summary: `✓ Qualification — all confirmed (dimensions · intent)` + a chevron to expand on demand. Audit trail stays one click away. While expanded the rep can still untick or add notes; on next page load the card re-collapses if fully confirmed. Pairs naturally with the auto-confirm-on-stage-advance change above — deals at Reservation+ stage land with a collapsed Qualification block instead of a full card. Don't redesign the checklist content per stage (cognitive load); just change the visual weight once it's no longer informationally hot.
|
||||
> - **Effort:** ~30 min. Captured 2026-05-21 from UAT.
|
||||
> - **Yacht Ownership History tab: flesh out the controls; don't remove (carries real semantic load)** — _src/components/yachts/yacht-ownership-history.tsx_ + _src/components/yachts/yacht-tabs.tsx:333_ + _src/components/yachts/yacht-form.tsx:337-345_ (existing Transfer affordance) + _src/lib/services/yachts.service.ts:215_ (`transferOwnership` service) + _src/lib/db/schema/yachts.ts:72-96_ (`yachtOwnershipHistory` table with partial unique index `(yacht_id) WHERE end_date IS NULL`).
|
||||
> - **Why keep:** the table isn't decorative — (i) partial unique index enforces one active owner at a time; (ii) berth reservation logic (`berth-reservations.service.ts`) gates "active company_membership on the owning company", so the yacht's ownership chain materially affects berth standing; (iii) the data is **already auto-populated** by `createYacht`, `transferOwnership`, and `public-interest.service.ts` — no rep effort required to maintain; (iv) audit trail value for disputed deals, EOIs generated under prior ownership, etc. Removing the tab AND/OR the table would lose audit fidelity and force reservation logic to derive ownership some other way. The "no way to enter/change" perception is a UI gap, not a missing concept.
|
||||
> - **Flesh-out scope (recommended):**
|
||||
@@ -270,14 +501,84 @@ _New UI surfaces, new endpoints, schema migrations, multi-step flows._
|
||||
> - **[Umami] Click-to-filter the page from the world map** — _src/components/website-analytics/visitor-world-map.tsx_ + new `country` filter store + thread through every `useUmamiTop*` hook — `VisitorWorldMap` already accepts an `onCountryClick(iso2)` prop that's unused. Wire it to a page-wide country filter (Zustand store or URL search param `country=US`) that scopes every card on the page to that country's data. Mirrors Umami's own click-through behaviour. Cap: ~2-3 h. Captured 2026-05-19.
|
||||
> - **[Umami] Per-rep `identify()` calls for attribution** — _src/components/auth/use-session.tsx (or wherever the session is hydrated)_ + _src/lib/services/umami.service.ts (new `identifyRep` wrapper)_ — call `umami.identify({sessionId, role: 'rep', repId: user.id})` on every authenticated CRM session so Umami's Sessions list can show "this lead came in while Matt was working hours". Privacy-gated: only fires for super-admin / sales-manager / sales-agent roles, never for residential-partner, never for portal-side users. Captured 2026-05-19; deferred as the privacy/value trade-off needs a product call before building.
|
||||
|
||||
0. **Platform-wide chart library migration: recharts → ECharts** — _src/components/dashboard/_ + _src/components/website-analytics/_ + _src/components/berths/_ — we now run two chart libraries side-by-side: ECharts (just adopted for the world choropleth + tree-shaken, canvas renderer, d3-geo projection) and recharts (everything else: berth-status donut, occupancy-timeline line, pipeline-funnel bar, lead-source pie, source-conversion bar, berth-heat-widget bars, pageviews-vs-sessions area, pipeline-value-tile mini-bars — ~8+ components). **Trade-off analysis (done 2026-05-19 during analytics build):** ECharts wins on visual polish (better default styling, smoother animations, native legend/tooltip behaviour), comprehensive chart types (sunbursts, sankeys, parallel coords, heatmaps, geo all out of the box), and canvas-renderer performance on dense series; recharts wins on React-idiom (declarative `<Area>` / `<Bar>` children vs imperative option objects) and bundle size for the very simplest charts. **Migration cost:** ~6–10 h to port the existing 8 components; each is a 50–150 LOC swap from `<ResponsiveContainer><AreaChart>…` to an `<ReactEChartsCore option={…} />` with tree-shaken module imports. **Pre-reqs already in place:** `transpilePackages: ['echarts', 'zrender', 'echarts-for-react']` added to `next.config.ts`, `d3-geo` installed, dynamic-import + canvas-renderer pattern proven on the world map. **Recommendation:** do as a single coordinated pass (consistency wins over piecemeal), gated on a free afternoon — none of the existing recharts components are buggy, this is purely about platform-wide visual + capability parity with the new analytics surfaces. Captured 2026-05-19 during the Umami flesh-out work.
|
||||
0. **Form-template fields bind to Interest/Client data — autofill, override-preservation history, dual-surface audit trail** — _src/lib/db/schema/documents.ts:290-309 (`formTemplates.fields` JSONB)_ + the New-form-template dialog UI (admin/forms) + _src/lib/services/supplemental-forms.service.ts_ (resolve + submit paths) + new `interest_field_history` table (or extend `audit_logs` with a dedicated `source='supplemental_form'` tag) + Interest detail + Client detail views (surface the override trail). Substantial feature touching the template builder, the public-facing supplemental form, and two record views.
|
||||
> - **(a) Template-builder: bind each field to an Interest/Client data point via dropdown.** Today's Field row asks for a freetext `key` + `label` + `type`. Replace `key` with a dropdown listing every bindable data point keyed by a stable token, e.g.:
|
||||
> - Interest-scoped: `interest.desiredLengthFt`, `interest.desiredWidthFt`, `interest.desiredDraftFt`, `interest.notes`, `interest.source`, `interest.tags`, ...
|
||||
> - Client-scoped: `client.fullName`, `client.dateOfBirth`, `client.nationality`, `client.passportNumber`, `client.residentialAddress`, ...
|
||||
> - Client-contact-scoped (per channel): `client.contacts.primaryEmail`, `client.contacts.primaryPhone` (resolved server-side to the `client_contacts` row with `isPrimary=true`).
|
||||
> - Yacht-scoped (when interest has a linked yacht): `yacht.name`, `yacht.lengthFt`, `yacht.makeAndModel`, ...
|
||||
> - Custom (no binding): freetext `key` for fields that don't map to any record column. Submission stored as-is in `form_submissions.data` JSONB, surfaced for rep review but not written back to any record.
|
||||
> - **Field shape extension:** `{ key, label, type, required, bindingPath?: string }` where `bindingPath` is the dotted-token from the bindable catalog. `key` stays as the JSONB submission key (so existing templates keep working — `bindingPath` is purely additive).
|
||||
> - **Catalog source:** define once in `src/lib/services/form-bindings-catalog.ts` exporting `BINDABLE_FIELDS: Array<{ path, label, entity, resolveCurrentValue, writeBack }>` — each entry knows where the value lives, how to read it, and how to write it back. Reuses the existing merge-fields infra (per CLAUDE.md `src/lib/templates/merge-fields.ts`) so the same vocabulary powers EOI templates AND supplemental forms.
|
||||
> - **(b) Public form autofill.** When the client opens the supplemental URL, server-side resolver:
|
||||
> - Loads the interest + client + linked yacht for the token.
|
||||
> - For each field with a `bindingPath`, calls `resolveCurrentValue()` to get the current stored value.
|
||||
> - Returns each field with a `currentValue` so the public form mounts pre-filled. Client reviews → edits if needed → submits.
|
||||
> - Fields without a binding stay empty (client-provided input).
|
||||
> - **(c) Submit handler: diff + override-preservation history.** On submit, for each bound field:
|
||||
> - Compare submitted value against current value (case-sensitive for free-text; deep-equal for arrays/objects).
|
||||
> - **Unchanged** → no-op. Don't write back, don't audit (saves noise).
|
||||
> - **Changed** → (i) call `writeBack(submittedValue)` to update the underlying interest/client/contact column. (ii) Append a history row: `{ portId, interestId, clientId, fieldPath, oldValue, newValue, source: 'supplemental_form', submissionId, providedAt, providedBy: 'client' }`. (iii) Audit log entry for the same change (existing audit infra) so org-wide audit reports see it.
|
||||
> - **New schema:** `interest_field_history` table — `id, port_id, interest_id, client_id (nullable, denormalized for client-detail queries), field_path text, old_value jsonb, new_value jsonb, source text ('supplemental_form' | 'rep_edit' | 'system_inferred'), submission_id (FK to form_submissions, nullable), created_at, created_by` + indexes on `(port_id, interest_id, created_at desc)` and `(port_id, client_id, created_at desc)` for the dual-surface lookups. Alternative: stuff in `audit_logs` with `source='supplemental_form'` and reuse the existing diff schema — cheaper but harder to query for the "show me the override history for this field" UX.
|
||||
> - **(d) UI surfacing on both record views.**
|
||||
> - **Interest detail:** small "i" icon next to each field that has history. Hover/click opens a popover: `Previous value: <X> · Updated by client via supplemental form on <date>`. Stacks multiple history rows in chronological order.
|
||||
> - **Client detail:** same UX, with an additional context line: `Updated via supplemental form for interest <berth label> on <date> → [Open interest]`. Cross-link goes to the source interest. Reuses the same `berthLabel` helper from the document-detail Interest link fix.
|
||||
> - Bonus: a dedicated "Field override history" section on the interest detail's Activity tab listing every override sourced from supplemental forms (or rep edits) for that interest — gives compliance + dispute resolution a single audit surface.
|
||||
> - **(e) Edge cases to think through:**
|
||||
> - **Required fields that resolve to existing values** — should they bypass `required` validation since they're pre-filled? Yes; required = "must have a value at submit time", not "must be re-entered by client".
|
||||
> - **Multi-value paths** (e.g. `client.contacts.primaryEmail` — what if client has none?) — `resolveCurrentValue` returns null, field renders empty, client provides one, submit writes a new client_contacts row marked isPrimary=true.
|
||||
> - **Type coercion mismatches** — bind path returns a number (`desiredLengthFt`), form field type is `text`. Catalog defines the canonical type per path; template builder validates compatibility at save time.
|
||||
> - **Sensitive fields** (passport, DOB) — `BINDABLE_FIELDS` entries flag `sensitivity: 'pii' | 'public' | 'internal'`; the supplemental form template builder warns / blocks selecting PII fields without explicit admin override (avoids accidental public-form data leak).
|
||||
> - **Effort:** ~12-16h end-to-end. ~2-3h for the catalog + resolver/writer infra. ~2h for the template-builder dropdown UI. ~2-3h for the autofill resolver in the public form service. ~3-4h for the submit diff + history table + audit + writeback. ~2h for the dual-surface UI (interest + client detail history popover). ~1h for sensitive-field gating + edge cases. Captured 2026-05-21 from UAT. **Cross-ref:** ties into the existing supplemental-info-request findings in Bucket 2 (reusable-not-single-use, generate+send split, regenerate+resend) — ship the binding/autofill/history work AFTER those land so the supplemental form is mature enough to carry the additional complexity.
|
||||
1. **Universal in-system preview for every file type (extend FilePreviewDialog beyond PDF + images)** — _src/components/files/file-preview-dialog.tsx:60-120_ — today only `mimeType?.startsWith('image/')` and `mimeType === 'application/pdf'` render; everything else falls through to a blank preview surface (no message, no fallback). User wants every document previewable in-system without forcing a download. Today's gaps: Office documents (.docx / .xlsx / .pptx), plain text (.txt / .csv / .md), email exports (.eml / .msg), video / audio, archives (.zip — see-into).
|
||||
|
||||
1. **Bulk-price editing UI** — _src/components/berths/_, _src/components/berths/berth-columns.tsx_ — backend shipped this session (new `berths.update_prices` permission across schema + 6 role maps + admin UI + factories; validators `updateBerthPriceSchema` + `bulkUpdateBerthPricesSchema`; services `updateBerthPrice` + `bulkUpdateBerthPrices` — both per-row audited with `fieldChanged='price'`; routes `PATCH /api/v1/berths/[id]/price` + `POST /api/v1/berths/bulk-update-prices`, ≤500 berths per batch). UI work pending: (a) wire `InlineEditableField` into the price cell of `berth-columns.tsx` (click → input → PATCH) gated by `can('berths', 'update_prices')`; (b) add `bulk-price-edit-sheet.tsx` (right-side Sheet, per-row inputs, "Set all to" + "Apply % adjust" shortcuts) wired to `bulkActions` on the `<DataTable />` in `berth-list.tsx`. ~2–3 h to ship the UI.
|
||||
2. **Pipeline Value tile should respect dashboard timeframe** — _src/components/dashboard/pipeline-value-tile.tsx_, _src/lib/services/dashboard.service.ts_ — the dashboard has a Today / 7d / 30d / 90d / Custom filter at the top (`Last 30 days` shown beside the greeting) but the Pipeline Value tile shows an absolute snapshot regardless. Should be constrained to the active timeframe: e.g. "Pipeline as of end of range" + "Revenue actually realized in range" (closed-won × berth price for interests whose `outcome_at` falls in the window). Needs: dashboard-wide timeframe context (Zustand store or React Query keyed by range), forecast/KPI service variants that accept a `range`, and a "realized vs forecast" line in the tile. ~3–4 h.
|
||||
3. **Stylized branded PDF report exporter from the dashboard** — _new feature_ — let users pick which analytics widgets to include and export a branded (port logo + colours, header/footer) PDF report. Builds on existing `pdfme` (templates) and `pdf-lib` (filling) infra plus the per-port branding already in `system_settings`. Trigger from a "Export report" button on the dashboard header; modal lets the user toggle widgets to include (KPIs, pipeline funnel, occupancy timeline, revenue breakdown, source attribution, berth demand, hot deals, recent activity, …) and optional sections (web analytics if a per-port integration is configured — GA4/Plausible/Umami — not yet wired). Server-side rendering of the report PDF via headless Chromium or pdfme. ~6–10 h (new product surface).
|
||||
4. **Web analytics integration (companion to #3)** — _new feature_ — per-port web analytics provider config in admin (GA4 / Plausible / Umami / Cloudflare), surfaced as widgets on the dashboard and ingestable into the branded PDF report. Needs: settings UI, provider adapter layer (`src/lib/integrations/analytics/`), dashboard widgets, and inclusion in the report exporter. ~8–12 h.
|
||||
5. **Supplemental-info-request email: branded HTML styling** — _src/lib/email/templates/_ — the email is plain HTML (logo missing, no header card, no blurred background), inconsistent with the other branded transactional emails (portal activation / reset / login wrap content in a `BrandedAuthShell`-equivalent HTML layout per CLAUDE.md). Rebuild the template to match the table-based, max-width 600, logo + blurred overhead background look, pulling port branding from `system_settings`. ~1-2 h.
|
||||
6. **Residential interests list: visual + functional parity with the main InterestList** — _src/components/residential/residential-interests-list.tsx_ vs _src/components/interests/interest-list.tsx_ + _interest-card.tsx_ + _interest-columns.tsx_ + _interest-filters.tsx_ — the residential interests page today is a slim search + stage-filter list (~200 lines). The main InterestList (~700 lines + supporting files) carries the bulk of the product idiom: card / table / kanban view modes (kanban is desktop-only), `usePaginatedQuery` with sort + saved views, full `FilterBar` (search, stage, tags, owner, source, date ranges), `ColumnPicker` for table mode, bulk actions wired to `/interests/bulk` (archive, change stage, add/remove tag), realtime invalidation across multiple event names, per-row archive flow, kebab actions, `InterestCard` rich row component. Reps switching between berth interests and residential interests today get two visually-divergent experiences for what is effectively the same conceptual surface.
|
||||
- **Coverage tiers:**
|
||||
- **Tier 1 (cheap, native-browser):** plain text (`text/plain`), CSV, Markdown → fetch + render in a styled `<pre>` or via a small markdown renderer (`react-markdown` already a likely dep — verify); video (`video/*`) → `<video controls src=…>`; audio (`audio/*`) → `<audio controls src=…>`. ~1-2h for all four.
|
||||
- **Tier 2 (lib-based, no server work):** DOCX → `mammoth.js` (~25KB gzipped) renders to HTML in-browser, good fidelity for text/headings/tables, loses complex formatting; XLSX → `sheetjs` (`xlsx` package) renders to an HTML table; PPTX → tricky, browser-side support is poor (recommend skip → fall back to "Download to view"). ~3-4h.
|
||||
- **Tier 3 (server-side conversion):** for fidelity on complex Office docs, route through a headless LibreOffice or `gotenberg` service to convert to PDF, then preview with the existing PdfViewer. Adds infra cost (Docker container for the converter). ~6-10h including ops setup. **Recommendation:** defer Tier 3 to a follow-up; ship Tier 1 + 2 first and accept the fidelity loss for Office docs.
|
||||
- **Fallback UX:** when the mime type isn't in any tier, render an empty-state card: file icon + filename + size + "Preview not supported for this file type. [Download to view]" button. Today's silent-blank surface is the bug.
|
||||
- **Recent Files preview-click fix** (Bucket 4 #7) folds into this: as we audit every preview surface, wire click handlers consistently on FileGrid / RecentFilesList / DocumentList rows. Don't ship preview support without making sure every list surface is actually clickable.
|
||||
- **Effort:** ~5-7h for Tier 1 + Tier 2 + fallback + clickability audit. Tier 3 deferred. Captured 2026-05-21 from UAT.
|
||||
|
||||
0. **Platform-wide date picker primitive (desktop popover + mobile native) — replace 22 `<input type="date|datetime-local">` sites** — _new_ `src/components/ui/date-picker.tsx` + `src/components/ui/date-time-picker.tsx`, then sweep 22 call sites (see list below). Native browser date/datetime inputs render with inconsistent, ugly UI on desktop (varies by Chromium/Safari/Firefox; Comet shows the worst variant). Mobile system pickers are the opposite — touch-friendly wheel/spinner UX that we want to keep. Build a wrapper that switches based on viewport.
|
||||
> - **Design (no new deps needed):** we already have `react-day-picker@10`, `date-fns@4`, and `src/components/ui/calendar.tsx`. Follow the canonical shadcn pattern (verified via Context7 against current shadcn docs):
|
||||
> - `<DatePicker>` — desktop: trigger Button shows formatted date + chevron, opens Popover containing `<Calendar mode="single" captionLayout="dropdown" />` (the dropdown caption gives month/year nav for fast jumping to historical dates — critical for the backfill UX). Mobile: native `<input type="date">` for the system picker.
|
||||
> - `<DateTimePicker>` — desktop: same Popover with Calendar plus a native `<input type="time" step="60">` in the popover footer (shadcn-canonical approach — hides webkit-picker-indicator via `[&::-webkit-calendar-picker-indicator]:hidden` and surfaces a `Clock` icon). Mobile: native `<input type="datetime-local">`.
|
||||
> - **Mobile detection:** use existing `useIsMobile` hook (if absent, add one via `window.matchMedia('(max-width: 640px)')` + `useSyncExternalStore` so SSR works). CSS-only show/hide is an alternative but DOM duplication wastes a tiny amount; hook-based is cleaner.
|
||||
> - **Same prop shape as today's `<Input type="date">`** so call-site migration is `<Input type="date" value=… onChange=… />` → `<DatePicker value=… onChange=… />` — minimal surface area change.
|
||||
> - **Optional polish (defer to v2):** add a `naturalLanguage` flag using `chrono-node` (~2KB) so users can type "next Tuesday" / "in 3 days" — particularly nice on the reminder form's due-date field. Skip for v1 to keep scope tight.
|
||||
> - **Call sites to migrate (22 files found via `grep "datetime-local|type=\"date\""`):** `src/app/(dashboard)/[portSlug]/invoices/new/page.tsx`, `src/app/(dashboard)/[portSlug]/expenses/scan/page.tsx`, `src/components/berths/berth-form.tsx`, `src/components/invoices/invoice-detail.tsx`, `src/components/yachts/yacht-transfer-dialog.tsx`, `src/components/reservations/reservation-detail.tsx`, `src/components/reservations/berth-reserve-dialog.tsx`, `src/components/expenses/expense-form-dialog.tsx`, `src/components/admin/audit/audit-log-list.tsx`, `src/components/shared/inline-editable-field.tsx`, `src/components/shared/filter-bar.tsx`, `src/components/scan/scan-shell.tsx`, `src/components/dashboard/date-range-picker.tsx`, `src/components/interests/payments-section.tsx`, `src/components/interests/interest-tabs.tsx` (incl. the `MilestoneAdvanceButton` popover at line 318), `src/components/interests/interest-contact-log-tab.tsx`, `src/components/interests/external-eoi-upload-dialog.tsx`, `src/components/reminders/snooze-dialog.tsx`, `src/components/companies/add-membership-dialog.tsx`, `src/components/reminders/reminder-form.tsx`, `src/components/companies/company-form.tsx`, `src/components/reports/generate-report-form.tsx`. Several callers (e.g. `filter-bar.tsx`, `inline-editable-field.tsx`, `date-range-picker.tsx`) wrap the input and need slightly more care — small refactor of the wrapper, not a 1-line swap.
|
||||
> - **Effort:** ~45 min to build the two wrappers + `useIsMobile` (if needed); ~2-3h to sweep all 22 call sites + visual verification in browser. Total ~3-4h. Captured 2026-05-21 from UAT.
|
||||
1. **Platform-wide chart library migration: recharts → ECharts** — _src/components/dashboard/_ + _src/components/website-analytics/_ + _src/components/berths/_ — we now run two chart libraries side-by-side: ECharts (just adopted for the world choropleth + tree-shaken, canvas renderer, d3-geo projection) and recharts (everything else: berth-status donut, occupancy-timeline line, pipeline-funnel bar, lead-source pie, source-conversion bar, berth-heat-widget bars, pageviews-vs-sessions area, pipeline-value-tile mini-bars — ~8+ components). **Trade-off analysis (done 2026-05-19 during analytics build):** ECharts wins on visual polish (better default styling, smoother animations, native legend/tooltip behaviour), comprehensive chart types (sunbursts, sankeys, parallel coords, heatmaps, geo all out of the box), and canvas-renderer performance on dense series; recharts wins on React-idiom (declarative `<Area>` / `<Bar>` children vs imperative option objects) and bundle size for the very simplest charts. **Migration cost:** ~6–10 h to port the existing 8 components; each is a 50–150 LOC swap from `<ResponsiveContainer><AreaChart>…` to an `<ReactEChartsCore option={…} />` with tree-shaken module imports. **Pre-reqs already in place:** `transpilePackages: ['echarts', 'zrender', 'echarts-for-react']` added to `next.config.ts`, `d3-geo` installed, dynamic-import + canvas-renderer pattern proven on the world map. **Recommendation:** do as a single coordinated pass (consistency wins over piecemeal), gated on a free afternoon — none of the existing recharts components are buggy, this is purely about platform-wide visual + capability parity with the new analytics surfaces. Captured 2026-05-19 during the Umami flesh-out work.
|
||||
|
||||
2. **Bulk-price editing UI** — _src/components/berths/_, _src/components/berths/berth-columns.tsx_ — backend shipped this session (new `berths.update_prices` permission across schema + 6 role maps + admin UI + factories; validators `updateBerthPriceSchema` + `bulkUpdateBerthPricesSchema`; services `updateBerthPrice` + `bulkUpdateBerthPrices` — both per-row audited with `fieldChanged='price'`; routes `PATCH /api/v1/berths/[id]/price` + `POST /api/v1/berths/bulk-update-prices`, ≤500 berths per batch). UI work pending: (a) wire `InlineEditableField` into the price cell of `berth-columns.tsx` (click → input → PATCH) gated by `can('berths', 'update_prices')`; (b) add `bulk-price-edit-sheet.tsx` (right-side Sheet, per-row inputs, "Set all to" + "Apply % adjust" shortcuts) wired to `bulkActions` on the `<DataTable />` in `berth-list.tsx`. ~2–3 h to ship the UI.
|
||||
3. **Pipeline Value tile should respect dashboard timeframe** — _src/components/dashboard/pipeline-value-tile.tsx_, _src/lib/services/dashboard.service.ts_ — the dashboard has a Today / 7d / 30d / 90d / Custom filter at the top (`Last 30 days` shown beside the greeting) but the Pipeline Value tile shows an absolute snapshot regardless. Should be constrained to the active timeframe: e.g. "Pipeline as of end of range" + "Revenue actually realized in range" (closed-won × berth price for interests whose `outcome_at` falls in the window). Needs: dashboard-wide timeframe context (Zustand store or React Query keyed by range), forecast/KPI service variants that accept a `range`, and a "realized vs forecast" line in the tile. ~3–4 h.
|
||||
3a. **Remove `/admin/reports` entirely (redundant with configurable Dashboard) + integrate PDF-report exporter into the Dashboard header** — _src/app/(dashboard)/[portSlug]/admin/reports/page.tsx + src/components/admin/reports-dashboard.tsx_ (DELETE) + _src/components/dashboard/dashboard-shell.tsx (or wherever the dashboard header lives)_ (ADD "Export to PDF" button) + the PDF exporter dialog (next entry). Today's `/admin/reports` page renders Pipeline funnel + Berth occupancy + activity feed — every card is also a Dashboard widget, and the Dashboard is configurable while this page is fixed. Surfaced UAT 2026-05-21 as "feels useless since we have the dashboard" + user follow-up 2026-05-21: "the pdf report exporter we will need to integrate into the dashboard — or make a dedicated reports page with even more charts/stats (though i think this may be redundant)." > - **Decision (locked 2026-05-21):** integrate PDF exporter into the Dashboard, remove `/admin/reports`. Path: (a) delete `src/app/(dashboard)/[portSlug]/admin/reports/page.tsx` + `src/components/admin/reports-dashboard.tsx`; (b) drop the "Reports" link from admin nav + search-nav-catalog (cross-ref the duplicate-key dedupe finding in Bucket 4 — same catalog file); (c) add a redirect from `/admin/reports` → `/dashboard` so any bookmark/external link lands sensibly; (d) add "Export to PDF" button in the Dashboard header (right-hand controls cluster, next to the date-range picker). > - **Why not a dedicated more-charts reports page:** a separate Reports page with "even more charts" inevitably duplicates Dashboard data. Either the Dashboard lags behind, or the Reports page becomes a copy. Better to invest that effort in adding more widgets directly to the Dashboard (which is configurable, so reps who don't want the extra cards can hide them). > - **What if leadership later wants a fixed read-only exec view?** revisit then — by that point we'll know whether reps actually use it or just print the Dashboard. YAGNI for now. > - **Effort:** ~30 min for the route removal + redirect + nav cleanup. PDF exporter itself is feature #3 below — that's where the substantive work is. Captured 2026-05-21 from UAT.
|
||||
4. **Stylized branded PDF report exporter — Dashboard-integrated (locked 2026-05-21)** — _src/components/dashboard/_ (new `<ExportReportDialog>` + Dashboard header trigger) + _src/lib/services/dashboard-report.service.ts (new)_ + the existing `pdfme` (templates) and `pdf-lib` (filling) infra plus per-port branding from `system_settings`. **Location decision locked:** lives on the Dashboard, NOT on a separate `/admin/reports` page (which is being removed — see 3a above).
|
||||
> - **UX flow:**
|
||||
> - Trigger: "Export to PDF" button in the Dashboard header (right-hand cluster, next to the date-range picker).
|
||||
> - Modal: widget toggle list pre-populated with every widget the user has currently visible on their Dashboard + the option to add hidden ones for this export. Each toggle row shows a thumbnail/preview of the widget for visual confirmation.
|
||||
> - Range: defaults to the Dashboard's current date range; can be overridden in the modal.
|
||||
> - Optional fields: report title, subtitle, custom subheader (e.g. "Q1 2026 board review"), optional commentary text block at the top.
|
||||
> - Branding: auto-pulls port logo + primary colour + header/footer from `system_settings` (per CLAUDE.md branding section). No per-export branding override (matches the locked "don't duplicate branding everywhere" principle).
|
||||
> - **Available widgets at export time** (any widget visible to the user on their Dashboard, gated by their permissions):
|
||||
> - KPI tiles (pipeline value, active deals, website analytics tile)
|
||||
> - Pipeline funnel
|
||||
> - Occupancy timeline
|
||||
> - Revenue breakdown REMOVED — already deleted in Bucket 1 #16 cleanup, exclude from export catalog too
|
||||
> - Source attribution / Lead source
|
||||
> - Berth demand / Hot deals
|
||||
> - Recent activity (capped at top N)
|
||||
> - Website analytics widgets (pageviews, sessions, visitors, top pages/countries) when Umami is configured
|
||||
> - Clients by country (when Bucket 3 #7 lands)
|
||||
> - World-map visitor heatmap (when Bucket 3 lands)
|
||||
> - **Server-side rendering approach:** lean toward **`pdfme` templated rendering** (already used per CLAUDE.md, no headless-Chromium ops cost). Each widget gets a `WidgetExportTemplate` definition mapping its data to a pdfme schema fragment. Composed at export time based on which widgets the user toggled on. Falls back to a simple text-table rendering for widgets without a dedicated template (gives partial coverage on day 1, fancy charts shipped iteratively).
|
||||
> - **Charts as PNG fallback** — pdfme can't render Recharts/ECharts components natively. Server-side: render each chosen widget to a PNG via a headless renderer (puppeteer or playwright running against the same chart components), then embed the PNG in the pdfme template. Pre-cache PNGs per widget per range to avoid regenerating on every export.
|
||||
> - **Export-history table** (`exported_reports`): id, port_id, user_id, file_id, widgets_included, date_range_from, date_range_to, title, created_at. Reps can re-download past exports without regenerating.
|
||||
> - **Effort:** ~10-14h end-to-end. ~3h for the dialog + widget toggles + modal. ~3-4h for the server-side composition + pdfme template fragments per widget. ~2-3h for chart-to-PNG rendering pipeline. ~1-2h for the export-history table + list UI. ~1-2h for the per-widget template definitions. Captured 2026-05-21 from UAT. **Cross-ref:** 3a (location decision); existing branding infra (per CLAUDE.md); chart-library migration to ECharts (Bucket 3 #00) — if that lands first, the PNG-rendering pipeline gets simpler (ECharts has a native server-side PNG export via canvas).
|
||||
5. **Web analytics integration (companion to #3)** — _new feature_ — per-port web analytics provider config in admin (GA4 / Plausible / Umami / Cloudflare), surfaced as widgets on the dashboard and ingestable into the branded PDF report. Needs: settings UI, provider adapter layer (`src/lib/integrations/analytics/`), dashboard widgets, and inclusion in the report exporter. ~8–12 h.
|
||||
6. **Supplemental-info-request email: branded HTML styling** — _src/lib/email/templates/_ — the email is plain HTML (logo missing, no header card, no blurred background), inconsistent with the other branded transactional emails (portal activation / reset / login wrap content in a `BrandedAuthShell`-equivalent HTML layout per CLAUDE.md). Rebuild the template to match the table-based, max-width 600, logo + blurred overhead background look, pulling port branding from `system_settings`. ~1-2 h.
|
||||
7. **Residential interests list: visual + functional parity with the main InterestList** — _src/components/residential/residential-interests-list.tsx_ vs _src/components/interests/interest-list.tsx_ + _interest-card.tsx_ + _interest-columns.tsx_ + _interest-filters.tsx_ — the residential interests page today is a slim search + stage-filter list (~200 lines). The main InterestList (~700 lines + supporting files) carries the bulk of the product idiom: card / table / kanban view modes (kanban is desktop-only), `usePaginatedQuery` with sort + saved views, full `FilterBar` (search, stage, tags, owner, source, date ranges), `ColumnPicker` for table mode, bulk actions wired to `/interests/bulk` (archive, change stage, add/remove tag), realtime invalidation across multiple event names, per-row archive flow, kebab actions, `InterestCard` rich row component. Reps switching between berth interests and residential interests today get two visually-divergent experiences for what is effectively the same conceptual surface.
|
||||
> - **Scope breakdown:**
|
||||
> - **(a) Card view + visual parity (highest leverage)** — replace the table-style `<li>`-per-row layout with a `ResidentialInterestCard` mirroring `InterestCard` (header with client name + stage chip + last-activity, body with preferences/notes preview, footer with quick actions). Reuse the existing `<DataTable />` primitive for the table mode so column picker + sort + bulk-select come for free. ~3-4h.
|
||||
> - **(b) Export to PDF + CSV** — match the export affordance the main page has (or, if the main page lacks it, add it to both surfaces in the same pass — captured here so it lands on both). PDF: render rows + summary header via `pdfme` / `pdf-lib` (existing infra per CLAUDE.md), branded with port logo. CSV: server-side endpoint `/api/v1/residential/interests/export?format=csv|pdf` (or client-side generation if the dataset is bounded — residential volumes are typically small). Trigger from a kebab menu on the page header. ~2h.
|
||||
@@ -287,7 +588,7 @@ _New UI surfaces, new endpoints, schema migrations, multi-step flows._
|
||||
> - **Recommendation:** ship (a) + (b) in one ~5-6h pass for the high-visibility wins (cards + export). Defer (c) until the residential team complains about filter gaps. Skip (d) unless verified-needed.
|
||||
> - **Companion fix:** see Bucket 1 finding "Residential namespace breadcrumb link is 404" — if the parity work lands a `/residential` landing page, that breadcrumb finding folds into this.
|
||||
> - Captured 2026-05-18 from UAT.
|
||||
7. **Residential inquiry → auto-forward to external partner email(s)** — _src/lib/services/residential.service.ts_ (`createResidentialInterest`), _src/app/api/public/residential-inquiries/route.ts:97_ (public intake), _src/lib/services/settings.service.ts_ + admin settings UI, _src/lib/email/templates/_ (new template), BullMQ enqueue — residential clients are managed by an external partner; every new residential inquiry needs to be forwarded automatically to one or more configured email addresses so the partner can act on it.
|
||||
8. **Residential inquiry → auto-forward to external partner email(s)** — _src/lib/services/residential.service.ts_ (`createResidentialInterest`), _src/app/api/public/residential-inquiries/route.ts:97_ (public intake), _src/lib/services/settings.service.ts_ + admin settings UI, _src/lib/email/templates/_ (new template), BullMQ enqueue — residential clients are managed by an external partner; every new residential inquiry needs to be forwarded automatically to one or more configured email addresses so the partner can act on it.
|
||||
> - **Settings model:** new per-port `system_settings` keys: `residential_forward_enabled` (bool, default false), `residential_forward_recipients` (JSON array of email addresses — `to`), `residential_forward_cc` (JSON array, optional), `residential_forward_filter` (optional discriminator — e.g. only forward inquiries with certain `source` values or above a price/size threshold; v1 ships without this and forwards everything).
|
||||
> - **Admin UI:** new section in `src/app/(dashboard)/[portSlug]/admin/settings/` ("Residential routing") with: enable toggle, recipient list editor (add/remove emails, drag-reorder, per-row "primary" flag for the To field vs CC), template preview ("Send sample to me"), and a small "Last forwarded N inquiries in the past 7 days" stat for confidence. Permission-gated by `admin.manage_settings`.
|
||||
> - **Email template:** new branded HTML template `residential-inquiry-forwarded.tsx` in `src/lib/email/templates/` matching the existing branded-shell idiom (port logo + table-based layout per CLAUDE.md) — body includes inquiry fields (client name, contact channel, preferences, notes, source, submission timestamp, link to the residential interest in the CRM if the partner has portal access; otherwise a "view in CRM" stub).
|
||||
@@ -296,7 +597,7 @@ _New UI surfaces, new endpoints, schema migrations, multi-step flows._
|
||||
> - **Effort:** ~3-4h for settings + template + service hook + BullMQ wiring; +1h for admin UI + sample-send button. Captured 2026-05-18 from UAT.
|
||||
>
|
||||
> - **Related:** see Feature 6 below — auto-link residential to existing main-client records, which fires at the same moment in the create pipeline; build (5) and (6) in one pass so the forwarded email can carry the "matched to existing CRM client X" context if a link was found.
|
||||
8. **Auto-link residential interests to existing main-client records (same person)** — _src/lib/services/residential.service.ts_ (`createResidentialClient` + `createResidentialInterest`), _src/app/api/public/residential-inquiries/route.ts_, new schema migration adding _src/lib/db/schema/residential.ts_ join table, _src/components/residential/residential-client-detail-header.tsx_ + _src/components/clients/client-detail-header.tsx_ (surface the link on both sides), new admin/dev script for backfill — when the same person who exists in the main berth client list registers a residential interest (or vice-versa), the two records should auto-link so reps can see the full relationship at a glance.
|
||||
9. **Auto-link residential interests to existing main-client records (same person)** — _src/lib/services/residential.service.ts_ (`createResidentialClient` + `createResidentialInterest`), _src/app/api/public/residential-inquiries/route.ts_, new schema migration adding _src/lib/db/schema/residential.ts_ join table, _src/components/residential/residential-client-detail-header.tsx_ + _src/components/clients/client-detail-header.tsx_ (surface the link on both sides), new admin/dev script for backfill — when the same person who exists in the main berth client list registers a residential interest (or vice-versa), the two records should auto-link so reps can see the full relationship at a glance.
|
||||
> - **Why a link, not a merge:** the two pipelines are operationally distinct (different team handles residential, different lifecycle stages, different downstream services). A hard merge would conflate records that should remain queryable separately. A symbolic link preserves both records while making the relationship discoverable.
|
||||
> - **Schema:** new join table `residential_client_links (id, port_id, residential_client_id, client_id, linked_at, linked_by_user_id, link_method enum('auto_email_match' | 'auto_phone_match' | 'manual'), confidence numeric(3,2), notes text)` — composite unique on `(port_id, residential_client_id, client_id)` so the same pair can't be linked twice. Both FKs ON DELETE CASCADE so dropping either side cleans the link automatically.
|
||||
> - **Match logic** (at residential client/interest create time): normalize the residential `email` to lowercase and check against `client_contacts.value` WHERE `channel='email'`; normalize `phoneE164` and check against `client_contacts.valueE164` WHERE `channel='phone'`. Email match → confidence 0.95 (auto-link, log audit); phone match → confidence 0.80 (auto-link with a "candidate match" badge so the rep can confirm); both match → confidence 0.99. If multiple candidate main-clients match (shared email — family/spouse case), DO NOT auto-link; instead surface all candidates in a UI banner for the rep to pick. Same logic runs in reverse when a new main-client is created (look for matching residential client).
|
||||
@@ -342,12 +643,132 @@ _Functional defects. Tag each with `[critical|high|medium|low]` prefix._
|
||||
> - **Additional problem:** the codebase has a full per-port vocabularies system (_src/lib/vocabularies.ts_) where `berth_side_pontoon_options` is registered as admin-editable, with defaults sourced from `BERTH_SIDE_PONTOON_OPTIONS`. The wizard not only uses the wrong list — it bypasses the admin-editability entirely. Even after fixing the values, admins won't be able to tune the list per-port unless the wizard reads through `getVocabulary('berth_side_pontoon_options')` like other surfaces should.
|
||||
> - **Fix:** (a) delete `SIDE_PONTOON_OPTIONS` at line 42. (b) Replace the two `SIDE_PONTOON_OPTIONS.filter(Boolean).map(...)` blocks (lines 264 + 334) with a call to the vocabulary hook — confirm the pattern used by `BerthForm` / single-berth edit (likely `useVocabulary('berth_side_pontoon_options')` or a server-component read). (c) Audit every other dropdown in the wizard for the same pattern: `BERTH_MOORING_TYPES`, `BERTH_CLEAT_TYPES`, `BERTH_BOLLARD_TYPES`, `BERTH_ACCESS_OPTIONS` are all registered as admin-editable vocabularies — verify the wizard reads through `getVocabulary` for all of them, not a local constant. (d) **Data backfill:** the four wrong values (`Port` / `Starboard` / `Bow` / `Stern`) may already be in production rows added via this wizard — write a one-off script to either remap them (`Port → Quay PT` or similar based on the port team's intent) or null them out + flag for manual review. Coordinate with the port team before running.
|
||||
> - **Effort:** ~30min for the wizard fix + dropdown audit, ~30min for the backfill script + dry-run. Total ~1h plus a stakeholder check on the remap mapping. **Severity high** because (i) silently writing out-of-vocabulary data is a long-tail data-integrity problem and (ii) it shadows the existing admin-editability infra (operators may not realize the vocab is overridable for this field because the wizard ignores it). Captured 2026-05-18 from UAT.
|
||||
> - **SHIPPED in 2d57417:** wizard now reads `useVocabulary('berth_side_pontoon_options')` instead of the wrong hard-coded enum; admin-editable per-port overrides honoured automatically. Data-backfill script + cross-vocab audit (mooring/cleat/bollard/access — none currently surfaced in the wizard but registered as editable) parked as follow-up.
|
||||
>
|
||||
> 0. **[high] All file downloads land with a blob-UUID filename + no extension** — _src/components/dashboard/chart-card.tsx:34_ (PNG/CSV exports), _src/app/(dashboard)/[portSlug]/expenses/page.tsx:95_ (CSV/XLSX export), _src/components/clients/client-files-tab.tsx:42_, _src/components/companies/company-files-tab.tsx:42_, _src/components/interests/interest-documents-tab.tsx:72_, _src/components/interests/interest-eoi-tab.tsx:597_, _src/components/admin/backup-admin-panel.tsx:90_ — 7 separate download sites share a near-identical anchor-click pattern that creates `<a download="<name>">`, calls `.click()`, and revokes the URL — but **the anchor is never appended to the document**, so Chromium-based browsers (Comet/Arc/Chrome) silently ignore the `download` attribute and fall back to using the blob URL's UUID for the filename (no extension). Captured UAT screenshot: dashboard chart "Download PNG" lands as `939c78df-48cc-466c-a22e-53e9dea69294` 35.5 KB instead of `<chart-name>.png`. Fix: extract a single `triggerBlobDownload(blob, filename)` helper into `src/lib/utils/download.ts` that (1) `document.body.appendChild(a)`, (2) `a.click()`, (3) `a.remove()`, (4) `URL.revokeObjectURL(url)` on a microtask/next-tick so Chrome has time to read the URL. Refactor all 7 call sites to import it; delete the local copies (and the chart-card-local `triggerBlobDownload` declared at chart-card.tsx:34). ~20-30 min including manual verification of each download surface. **Affects every file-export flow** — bumping severity to high. Captured 2026-05-18 from UAT.
|
||||
> 0. **[high] All file downloads land with a blob-UUID filename + no extension** — _src/components/dashboard/chart-card.tsx:34_ (PNG/CSV exports), _src/app/(dashboard)/[portSlug]/expenses/page.tsx:95_ (CSV/XLSX export), _src/components/clients/client-files-tab.tsx:42_, _src/components/companies/company-files-tab.tsx:42_, _src/components/interests/interest-documents-tab.tsx:72_, _src/components/interests/interest-eoi-tab.tsx:597_, _src/components/admin/backup-admin-panel.tsx:90_ — 7 separate download sites share a near-identical anchor-click pattern that creates `<a download="<name>">`, calls `.click()`, and revokes the URL — but **the anchor is never appended to the document**, so Chromium-based browsers (Comet/Arc/Chrome) silently ignore the `download` attribute and fall back to using the blob URL's UUID for the filename (no extension). Captured UAT screenshot: dashboard chart "Download PNG" lands as `939c78df-48cc-466c-a22e-53e9dea69294` 35.5 KB instead of `<chart-name>.png`. Fix: extract a single `triggerBlobDownload(blob, filename)` helper into `src/lib/utils/download.ts` that (1) `document.body.appendChild(a)`, (2) `a.click()`, (3) `a.remove()`, (4) `URL.revokeObjectURL(url)` on a microtask/next-tick so Chrome has time to read the URL. Refactor all 7 call sites to import it; delete the local copies (and the chart-card-local `triggerBlobDownload` declared at chart-card.tsx:34). ~20-30 min including manual verification of each download surface. **Affects every file-export flow** — bumping severity to high. Captured 2026-05-18 from UAT. **SHIPPED in 2d57417:** added `src/lib/utils/download.ts` with `triggerBlobDownload(blob, filename)` (DOM-attached anchor + deferred URL revoke) + sibling `triggerUrlDownload(url, filename)` for presigned-URL paths; refactored all 7 call sites, dropped the chart-card-local copy.
|
||||
|
||||
1. **[high] Duplicate row for berth E17 in port-nimara** — DB: two `berths` rows with `mooring_number='E17'`, both with `price=NULL`. The canonical mooring format is meant to be unique per port (see CLAUDE.md "Mooring number canonical format"). Surfaced by the dashboard tile via the new "berth price missing" chip but the root cause is missing/leaked unique constraint. Recommend: dedupe + add partial unique index on `(port_id, mooring_number) WHERE archived_at IS NULL`. Deferred per session call (warning-only UI ships now).
|
||||
2. **[medium] Stage advance allowed without berth price** — Service-level: `changeInterestStage` lets an interest reach EOI/Reservation/Deposit Paid/Contract on a primary berth whose `price` is NULL. EOI doc generation downstream presumably renders blank/$0 for the quote field. Cross-port impact unknown. Recommend: add a `ValidationError("Berth price must be set before advancing past Qualified")` gate in `changeInterestStage` for stages eoi+. Deferred per session call.
|
||||
3. **[high] Command-search quick-create buttons routed to dead `/new` pages** — _src/components/search/command-search.tsx_ — ZeroState "New client/yacht/company" buttons pushed `/<entity>/new?name=…` which matched the `[id]` dynamic segment and rendered the entity-not-found page. Fixed by switching to `/<entity>?create=1&prefill_name=…` (the existing `useCreateFromUrl` convention) + adding `prefill` prop support to `YachtForm` + `CompanyForm` and wiring `prefill_name` reads in their list components. Now correctly pops the create sheet pre-filled. Fixed in this session.
|
||||
3. **[medium] Smart search renders duplicate React keys for `/admin/templates` — console warning + potential render glitch** — _src/lib/services/search-nav-catalog.ts:89-94 + 275-280_ + _src/components/search/command-search.tsx:587_. Two entries in the nav catalog both point at `/:portSlug/admin/templates` ("settings" category at line 89 with EOI/Documenso keywords, "admin" category at line 275 with PDF/email-template keywords). The search renderer keys rows by `navigation:${href}` → React fires the "Encountered two children with the same key" warning. Visible in console as `navigation:/port-nimara/admin/templates`. Behavior is unsupported — could cause omitted/duplicated rows.
|
||||
- **Fix (layered):**
|
||||
- **(a) Catalog dedupe:** merge the two entries — keep the "settings" one (line 89, matches surrounding /admin/branding + /admin/storage cluster), absorb the admin-version's keywords (`'pdf templates'`, `'email templates'`, `'merge fields'`, `'eoi template'`), delete the duplicate at line 275.
|
||||
- **(b) Defensive render-side key:** even after dedupe, change command-search.tsx:587 to compose keys as `navigation:${href}:${category}` (or filter duplicates by href at catalog-load time). Protects against the same bug recurring when new nav entries land.
|
||||
- **Audit:** grep the catalog for any other href that appears twice — likely candidates around /admin/email, /admin/users, /admin/settings if similar consolidations happened. Single dedupe sweep at the top of the catalog file.
|
||||
- **Effort:** ~15 min. Captured 2026-05-21 from UAT console.
|
||||
- **SHIPPED in 2d57417:** dedupe lives at the catalog-search layer (`searchNavCatalog` keeps the highest-scoring entry per href via a Map) so any future intentional cross-category re-entries are safe; the two `/admin/templates` rows were also merged into a single richer-keyword entry.
|
||||
4. **[medium] Overview "Latest note" teaser is stale after creating a note in the Notes tab (no cross-query invalidation)** — _src/components/shared/notes-list.tsx:164-184_ (create/update/delete mutations) + _src/components/interests/interest-tabs.tsx:1083-1104_ (teaser reads `interest.recentNote` + `interest.notesCount` from the parent interest detail object). The notes-list mutations invalidate `[entityType, entityId, 'notes', 'own' | 'aggregated']` but not the parent `['interests', interestId]` query that hydrates `recentNote` / `notesCount`. Net effect: rep adds a note in the Notes tab → switches to Overview → teaser still shows the previous note + the old count until a hard refresh. Same gap presumably affects Client / Company / Yacht detail Overviews if they have similar embedded latest-note teasers.
|
||||
- **Fix:** add an optional `parentInvalidateKey?: QueryKey` prop to `NotesList`; on each mutation's `onSuccess`, invalidate it alongside the notes query key. The interest tab passes `['interests', interestId]`; the client/company/yacht tabs pass their equivalent. Belt-and-braces: also invalidate inside the parent entity's note-related mutations if any exist directly.
|
||||
- **Effort:** ~20-30 min (prop + 4 call sites + a vitest covering the invalidation chain). Captured 2026-05-21 from UAT.
|
||||
- **SHIPPED in 2d57417:** `NotesList` now takes `parentInvalidateKey?: QueryKey`; wired through 5 callers (interests, clients, yachts, companies, residential_clients, residential_interests). Create / update / delete mutations invalidate the parent detail query alongside the notes query key.
|
||||
5. **[high] InterestDocumentsTab uploads land with `client_id=NULL` — invisible in Attachments + no client subfolder auto-created** — _src/components/interests/interest-documents-tab.tsx:141-147_ (caller passes `entityType="client"` + `entityId={clientId}` but NOT `clientId` separately) + _src/components/files/file-upload-zone.tsx:63_ (only appends `clientId` to the form body when given as a prop) + _src/lib/services/files.ts:85-101_ (`uploadFile` reads `data.clientId ?? null` literally — does not derive it from `entityType==='client' + entityId`). Net effect: upload POST hits `/api/v1/files/upload` with `entityType=client&entityId=<UUID>` but no `clientId` form field, so the `files` row lands with `client_id = NULL`. Cascading bugs: (a) the Documents tab's "Attachments" list (`GET /api/v1/files?clientId=<UUID>`, filters on `eq(files.clientId, clientId)`) returns empty — file vanishes from the interest's Documents tab; (b) Documents Hub auto-deposit can't `ensureEntityFolder` for the client (it walks `files.clientId`), so the `Clients/<client-name>/` subfolder under the system root is never created — file lives at root in "All documents" but isn't filed by entity. The file IS reachable via the port-wide "All documents" view because that query has no clientId filter.
|
||||
- **Fix (recommended at service layer — durable):** in `src/lib/services/files.ts:uploadFile`, when `data.entityType==='client'` AND `data.clientId` is not set, default `data.clientId = data.entityId`. Same for `entityType==='company'` → `companyId`, `entityType==='yacht'` → `yachtId`. Catches any other caller making the same mistake. Plus `ensureEntityFolder` should fire on every upload that lands with an entity FK, not only when explicit clientId was provided.
|
||||
- **Caller fix (belt + braces):** pass `clientId={interest.clientId}` alongside `entityType` + `entityId` in interest-documents-tab.tsx:141-147. Audit other FileUploadZone call-sites for the same pattern (client-files-tab, yacht-files-tab, company-files-tab).
|
||||
- **Backfill needed:** existing rows uploaded via this path have `client_id=NULL` despite having `entity_type='client'` + `entity_id=<UUID>`. One-off script to backfill `client_id` from `entity_id` where entity_type='client' AND client_id IS NULL; same for company/yacht. Then re-run `ensureEntityFolder` for affected rows so the Documents Hub tree catches up.
|
||||
- **Effort:** ~30 min for service-layer fix + caller audit + backfill script. **High severity** — affects every interest-tab upload on the platform, breaks the Documents Hub filing model for those files. Captured 2026-05-21 from UAT.
|
||||
- **SHIPPED (service layer + caller) in 2d57417:** `uploadFile` in `src/lib/services/files.ts` now derives `clientId/companyId/yachtId` from `(entityType, entityId)` when the explicit FK isn't passed. Interest-documents-tab also passes `clientId={interest.clientId}` belt-and-braces. **Backfill script + nested-folder migration remain outstanding** — those bundle with the larger Bucket 4 #6 "nested document subfolders" feature in PR Batch 4.
|
||||
6. **[medium] External EOI upload — 3 linked bugs: lying toast + broken View button + UUID-named download** — _src/components/interests/external-eoi-upload-dialog.tsx:60_ + _src/components/interests/interest-eoi-tab.tsx:589-605 (`SignedPdfActions`)_ + _src/app/api/v1/files/[id]/download/route.ts_ + _src/lib/services/files.ts (`getDownloadUrl`)_. Surfaced together during UAT 2026-05-21 of the backfill flow.
|
||||
- **(a) Toast lies about stage advance** — server (`external-eoi.service.ts:142-160`) only advances stage when current is `open|details_sent|in_communication|eoi_sent`; at Reservation+ it correctly leaves the stage alone. But the client toast hardcodes `"External EOI uploaded — interest advanced to EOI Signed"` regardless of what the server did. **Fix:** have `uploadExternallySignedEoi` return `{ stageChanged: boolean, newStage?: PipelineStage }`; client toasts conditionally: stageChanged → "External EOI uploaded — stage advanced to EOI Signed"; else → "External EOI uploaded — filed against this deal (stage unchanged)". ~20 min.
|
||||
- **(b) "View" button downloads instead of previewing in-app** — `SignedPdfActions.open('view')` opens the presigned URL via `window.open`. Browser behavior depends on `Content-Disposition` header from MinIO/S3 — defaulting to `attachment` triggers download every time. **Fix:** swap `window.open` for the existing `FilePreviewDialog` component (already supports PDFs + images per `file-preview-dialog.tsx:60-61`). Lift a `[previewFile, setPreviewFile]` state to the parent EOI tab and render `<FilePreviewDialog open={!!previewFile} fileId={previewFile?.id} ... />` once. SignedPdfActions's View button just sets the preview state. Pairs with the platform-wide "preview-everything" Bucket 3 feature so the same inline-preview surface gets full file-type coverage. ~30 min.
|
||||
- **(c) Download filename is the storage-key UUID** — same root cause: `Content-Disposition` doesn't include a filename, so the browser uses the URL's last path segment (the UUID per `generateStorageKey`). **Fix:** generate the presign in `getDownloadUrl` with `response-content-disposition: attachment; filename="<files.filename>"` (S3/MinIO presign param). Honors the original filename stored in `files.filename`. ~15 min including a sweep of other download call sites — `client-files-tab.tsx`, `company-files-tab.tsx`, `interest-documents-tab.tsx`, `interest-eoi-tab.tsx` all hit the same endpoint. Consider also adding a sibling `response-content-disposition: inline` mode (e.g. `GET /api/v1/files/[id]/download?disposition=inline`) for the cases where we DO want native browser preview as a fallback to FilePreviewDialog.
|
||||
- **(d) [high] Server discards `dateEoiSigned` + `eoiStatus` when stage is past EOI — skip-ahead banner falsely persists** — _src/lib/services/external-eoi.service.ts:142-160_ — when current stage is past `eoi_sent` (e.g. `reservation`, `deposit_paid`, `contract_*`), the `else` branch (lines 157-160) only updates `updatedAt`, ignoring the `signedAt` from the form. So even though the user uploaded an externally-signed EOI with a valid date, `interests.dateEoiSigned` stays NULL → the SkipAheadBanner keeps demanding the rep backfill the EOI signed date with no way to satisfy it.
|
||||
- **Fix:** split the two concerns. Document metadata (dateEoiSigned + eoiStatus='signed') should ALWAYS be written from the upload — only the pipelineStage advance is gated:
|
||||
```ts
|
||||
const shouldAdvanceStage = ['open', 'details_sent', 'in_communication', 'eoi_sent'].includes(
|
||||
interest.pipelineStage,
|
||||
);
|
||||
await tx
|
||||
.update(interests)
|
||||
.set({
|
||||
dateEoiSigned: interest.dateEoiSigned ?? input.signedAt ?? new Date(),
|
||||
eoiStatus: 'signed',
|
||||
pipelineStage: shouldAdvanceStage ? 'eoi_signed' : interest.pipelineStage,
|
||||
updatedAt: new Date(),
|
||||
})
|
||||
.where(eq(interests.id, interestId));
|
||||
```
|
||||
- **Also audit:** `interest-rules-engine` / `evaluateRule('eoi_signed', ...)` should fire on this path too (a manually-uploaded external EOI is still an EOI-signed event for the rules engine — berth-rules like "auto-mark berth Under Offer" depend on it).
|
||||
- ~30-45 min including the audit + integration test.
|
||||
- **(e) [medium] No edit affordance for uploaded-EOI metadata post-upload (signedAt / signerNames / notes / title locked)** — the EOI tab's history list at `interest-eoi-tab.tsx` shows uploaded documents but exposes no edit button. Once a rep uploads with a typo in signerNames or the wrong signedAt date, they can't correct it — they'd have to delete and re-upload (losing the audit log link).
|
||||
- **Fix:** add an "Edit metadata" icon-button next to View / Download on each external-EOI row in the EOI tab. Opens a small dialog with the same fields the upload dialog has (signedAt, signerNames, notes, title), pre-filled. Submit PATCHes the document's metadata JSONB + the interest's `dateEoiSigned` (when changed) in one transaction. Audit-log the change with old→new diff.
|
||||
- Permission gate: `documents.edit_metadata` or reuse `documents.upload_signed` (the same permission that allowed the upload).
|
||||
- Side concern: the same edit affordance probably belongs on signed Reservations and signed Contracts too — but those are typically Documenso-bound (signedAt is webhook-attested), so editing should be more restricted there. For external EOIs the rep is the source of truth for signedAt anyway, so editing is safe.
|
||||
- ~1-1.5h including dialog component + service PATCH + audit log + permission gate.
|
||||
- **Effort:** ~3-4h total for all five sub-issues (was 1-1.5h before (d) + (e) landed). Captured 2026-05-21 from UAT.
|
||||
7. **[high] Expense form: zod refine on `receiptFileIds` fires invisibly — Create button does nothing because the error renders nowhere** — _src/components/expenses/expense-form-dialog.tsx:64-77_ (form registers `useForm` + `zodResolver(createExpenseSchema)`) + _src/lib/validators/expenses.ts:40-47_ (schema-level `.refine()` requiring `receiptFileIds.length > 0 || noReceiptAcknowledged === true`, attached to `path: ['receiptFileIds']`). The form keeps `uploadedReceipt` + `noReceipt` in local React state, never injecting them into the form values via `setValue`. They're spliced into the payload INSIDE `onSubmit` (lines 188-189) — but `onSubmit` is never reached because validation fails first: zodResolver sees `receiptFileIds: undefined` in form values, the refine fails, `errors.receiptFileIds` is set. The form has NO `{errors.receiptFileIds && <p>...}` block, so the error is invisible. Browser scrolls to top of failed form. User reports "I filled everything in and uploaded a receipt — clicking Create does nothing."
|
||||
- **Fix (recommended — single source of truth in react-hook-form):**
|
||||
- When `handleFileChange` succeeds: `setValue('receiptFileIds', [uploadedReceipt.id], { shouldValidate: true })`.
|
||||
- When the "no receipt" checkbox toggles: `setValue('noReceiptAcknowledged', noReceipt, { shouldValidate: true })`. Optionally also `setValue('receiptFileIds', undefined)` when noReceipt is checked.
|
||||
- When `clearReceipt` runs: `setValue('receiptFileIds', undefined, { shouldValidate: true })`.
|
||||
- Then drop the local `uploadedReceipt` / `noReceipt` state and read `watch('receiptFileIds')` / `watch('noReceiptAcknowledged')` instead for the UI (or keep them as a UI-only mirror for filename display, but make form state authoritative).
|
||||
- **Alt (lighter touch):** keep the local state but drop the schema-level refine; move that validation into `onSubmit` manually after merging local state. Loses the form-error idiom — discouraged.
|
||||
- **Belt + braces (sweep):** audit every form that has `.refine()` rules on fields NOT registered with the form. Same pattern likely exists elsewhere (any form with file uploads or sub-components managing their own state). Add a defensive check: on submit, log/toast a developer warning if a zod error fires on a field that has no error-rendering surface — would have surfaced this bug.
|
||||
- **Effort:** ~30 min for the expense form fix; ~2-3h for the broader audit of similar refines + state-sync gaps. Captured 2026-05-21 from UAT. **Cross-ref:** the platform-wide form-error UX work in Bucket 2 (scroll-to-first-error + summary banner) would have surfaced this bug visibly — bundle the two as a single rollout so each form audited gets both the missing setValue + the missing error-render surface fixed in one pass.
|
||||
- **SHIPPED (expense form only) in 2d57417:** `handleFileChange` / `clearReceipt` / `noReceipt` checkbox now mirror to form state via `setValue`; edit-mode `reset()` pre-fills `noReceiptAcknowledged` from the existing expense row. The platform-wide refine-vs-error-surface audit + the broader form-error UX work remain in Wave 3.
|
||||
8. **[high] Documents filing model needs nested entity subfolders (Interests under Clients; Yachts/Companies parity) — design decisions locked 2026-05-21** — _src/lib/db/schema/files.ts_ (add nullable `interest_id` FK + indexes) + _src/lib/db/schema/document_folders.ts_ (extend entity-folder model to support nested entity folders) + _src/lib/services/files.ts_ (`uploadFile`, `ensureEntityFolder`) + _src/lib/services/document-folders.ts_ + _src/components/files/file-upload-zone.tsx_ (accept + forward `interestId`) + _src/components/interests/interest-documents-tab.tsx_ (caller wires interestId) — companion to bug #4 above. Today's schema can't represent per-interest filing: `files` has no `interest_id` and `document_folders.ensureEntityFolder` only knows top-level client/company/yacht roots. Reps want `Clients/<Client name>/<Interest folder>/<file>` so they can find "everything for this specific deal" in one place — including across multiple historical deals for the same client.
|
||||
- **Locked design decisions (from UAT 2026-05-21):**
|
||||
- **D1. Folder naming pattern (single-berth):** `<mooring> · <created month>` (e.g. `A1 · 2026-04`). Stable for the deal's lifetime — does NOT update on stage transitions. Only renames once, on close: appends ` (Lost)` / ` (Won)`. Bookmark / email references stay valid.
|
||||
- **D2. Folder naming pattern (multi-berth):** `<berth range> · <created month>` using the existing `formatBerthRange()` helper from `src/lib/templates/berth-range.ts` — same idiom as the EOI Berth Number field (per CLAUDE.md). Example: `A1-A3, B5-B7 · 2026-04`.
|
||||
- **D3. Default upload scope from an Interest page:** radio with two options, **default selected = "This deal (Interest <folder name>)"**, alternate = "Client-level (all deals)". Rep flips to client-level when uploading general docs like passport scans from the interest page.
|
||||
- **D4. Scope of nesting:** apply to **Interests + Yachts + Companies** (full hierarchy). Yacht folders nest under their owner (Client or Company) per `yachts.current_owner_type/id`. Company-owned yachts nest under their company folder.
|
||||
- **D5. Rename triggers:** ONLY on close (Won/Lost) or archive. Active deals keep stable names. Primary-berth changes during active life do NOT re-derive (avoids churn).
|
||||
- **D6. Storage backend (S3 / MinIO / filesystem):** zero implications. Documents Hub folder tree is metadata-only (`document_folders` in Postgres); object keys stay UUID-based (`<portSlug>/<entity>/<entityId>/<uuid>.<ext>` per `generateStorageKey`) and never move on folder rename. Soft-rescue delete is also metadata-only.
|
||||
- **Schema changes:**
|
||||
- Add `files.interest_id uuid` nullable FK + index on `(port_id, interest_id) WHERE archived_at IS NULL`. Existing rows stay NULL (= client-level, no interest scope).
|
||||
- Extend `document_folders.entity_type` to accept `'interest'` (and confirm `'yacht'`, `'company'` are already supported per CLAUDE.md). Existing partial unique index `uniq_document_folders_entity` on `(port_id, entity_type, entity_id) WHERE entity_id IS NOT NULL` still applies. Nested rows: `parent_id` points to the parent client/company folder (not the system root) so the tree carries the hierarchy.
|
||||
- **Folder-name derivation helper:** new `src/lib/services/document-folder-naming.ts` exporting `deriveInterestFolderName(interest, interestBerths)`:
|
||||
- Read `interest.dateCreated` (or `createdAt`) → format as `YYYY-MM`.
|
||||
- Resolve berths via `interestBerths.filter(b => b.isInEoiBundle ?? b.isPrimary)` (fall back to all linked berths if none flagged).
|
||||
- Single berth → `<mooring> · <month>`. Multiple berths → `${formatBerthRange(moorings)} · <month>`. No berths linked → `Deal <short-id> · <month>` fallback.
|
||||
- Append ` (Won)` / ` (Lost)` when `interest.outcome` is set; ` (Archived)` when `interest.archivedAt` is set without outcome.
|
||||
- Pure function, unit-tested.
|
||||
- **Service-layer wiring (combines with the #4 service-layer fix):**
|
||||
- `uploadFile`: when `entityType==='interest'` OR `interestId` is set → resolve parent client via `interests.clientId`, call `ensureEntityFolder('client', clientId)`, then `ensureEntityFolder('interest', interestId, parentFolderId: clientFolderId, name: deriveInterestFolderName(...))`, file the row at the interest folder. Three-tier: PORT root → client subfolder → interest sub-subfolder.
|
||||
- `uploadFile`: when `entityType==='yacht'` OR `yachtId` set → resolve owner (`yachts.currentOwnerType` + `currentOwnerId`), ensure owner folder, ensure yacht subfolder under it.
|
||||
- `uploadFile`: when only `clientId` set (no interestId, no yachtId) → file at client folder (today's behavior).
|
||||
- **The #4 derive-clientId-from-entityType fix collapses into this:** `uploadFile` now always derives the FK from `entityType + entityId` if not explicitly passed. The bug-#4 hot-fix is the trivial 1-line version; this larger work is the durable version.
|
||||
- **Upload-time UI affordance (D3):**
|
||||
- `FileUploadZone` accepts a new `scopeOptions?: Array<{ id, label, entityType, entityId }>` prop + a `defaultScopeId?: string`. Renders a small radio above the dropzone when ≥ 2 options.
|
||||
- InterestDocumentsTab passes `scopeOptions = [{ id: 'interest', label: 'This deal (Interest <name>)', entityType: 'interest', entityId: interestId }, { id: 'client', label: 'Client-level (all deals)', entityType: 'client', entityId: clientId }]` with `defaultScopeId='interest'`.
|
||||
- YachtDocumentsTab (when it lands) passes 2 options: `'yacht'` (default) + `'owner'` (client/company-level).
|
||||
- Client / Company / Yacht detail pages with no parent context render the dropzone without the radio (single-scope upload).
|
||||
- **Lifecycle hooks (D5):**
|
||||
- Interest outcome lands (Won / Lost): rename folder via a service helper that re-runs `deriveInterestFolderName` and `UPDATE document_folders SET name=...`.
|
||||
- Interest archived: append ` (Archived)` if no outcome set.
|
||||
- Soft-rescue per CLAUDE.md — never hard-delete folders even on `archive`.
|
||||
- Primary-berth changes mid-deal: NO rename (per D5 — stable during active life). The folder name reflects creation-time berths; current berths are visible elsewhere in UI.
|
||||
- **List query updates:**
|
||||
- InterestDocumentsTab "Attachments" section: surface BOTH (i) files with `files.interest_id === interestId` under a "This deal" subheading + (ii) files with `files.client_id === clientId AND interest_id IS NULL` under a "From client" subheading. Mirrors the aggregated-projection idiom (per CLAUDE.md).
|
||||
- Documents Hub tree: render interest subfolders inside parent client folder. Add a small outcome chip per interest folder (Won / Lost / Active).
|
||||
- **Backfill (combines with #4 backfill):**
|
||||
- Files with `entity_type='interest' + entity_id=<UUID>` but missing `interest_id` column → backfill `interest_id = entity_id`; derive parent `client_id` from `interests.client_id`; run `ensureEntityFolder` for both levels.
|
||||
- Files with `entity_type='yacht'` + `entity_id` but missing `yacht_id` → mirror.
|
||||
- Files with only `client_id` set pre-feature stay at client-folder level — no interest scope retroactively (can't infer which interest they belonged to).
|
||||
- One-off script `pnpm tsx scripts/backfill-nested-document-folders.ts --apply` — idempotent, per-port advisory-locked.
|
||||
- **Effort:** ~6-8h end-to-end (migration + service rewrites + folder-name derivation + upload-zone affordance + tree rendering + lifecycle hooks + backfill + tests). Bundles bug #4 — both touch the same code paths. Captured 2026-05-21 from UAT.
|
||||
9. **[medium] SelectTrigger height (`h-9`) doesn't match Input height (`h-11`) — platform-wide visual inconsistency** — _src/components/ui/select.tsx:22_ (SelectTrigger default `h-9` = 36px) + _src/components/ui/input.tsx:18_ (Input default `h-11` = 44px). Every form where an Input sits next to a Select has an 8px height mismatch. Surfaced specifically on _src/components/expenses/expense-form-dialog.tsx:222-247_ (the Amount + Currency two-column row) but affects ALL such combinations across the platform. Fixing locally with `className="h-11"` on each call site is a sweep over dozens of spots and creates drift the next time someone copies the pattern.
|
||||
- **Fix (platform-wide):** introduce a `size` variant on SelectTrigger mirroring Button's idiom — `<SelectTrigger size="default" | "sm">`. Default to `"default"` = `h-11` so it pairs with the Input default out of the box. Migrate explicitly-compact uses (filter bars, dense table headers) to pass `size="sm"` = `h-9` to preserve their current density.
|
||||
- **Audit step:** grep every `<SelectTrigger>` and `<Select>` call site; flag the ones in compact contexts (FilterBar, DataTable header dropdowns, dense admin lists) for the `size="sm"` override; everything else inherits the new h-11 default.
|
||||
- **Effort:** ~1h for the component change + audit + sweeping the explicit `size="sm"` overrides. Higher upside: enforces visual parity for every future form. Captured 2026-05-21 from UAT.
|
||||
10. **[medium] Platform-wide: every file-row surface should be click-to-preview by default (currently action is hidden behind kebab on FileGrid; Recent Files rows don't respond at all)** — confirmed on _src/components/files/file-grid.tsx:103-150_ (card body is a static `<div>` with no `onClick`; Preview action lives inside `MoreHorizontal` kebab → opacity-0 unless hovered) + _src/components/documents/_ "Recent Files" rendering surface (rows entirely non-clickable per earlier UAT — preview AND download both dead). Same UX gap repeats across every file-row surface; ship one fix pattern everywhere instead of per-component patches.
|
||||
- **Fix shape (apply uniformly):**
|
||||
- **Click target = preview** — the card/row body becomes a `<button onClick={() => onPreview(file)}>` (or accessible `<div role="button" tabIndex={0}>` with keyboard support). Click opens `FilePreviewDialog` directly. Hover state already implies clickability via `hover:border-primary/50 hover:shadow-sm` — wiring the click matches the visual affordance.
|
||||
- **Kebab stays as "More actions"** — Download, Rename, Delete remain in the dropdown. Drop the redundant "Preview" entry from the kebab once the body click does it.
|
||||
- **Non-previewable mime types** — still click-to-preview, but `FilePreviewDialog` renders its fallback empty state ("Preview not supported for this file type. [Download to view]"). Pairs with the universal-preview feature already queued in Bucket 3.
|
||||
- **Affected surfaces** (audit during the sweep):
|
||||
- `src/components/files/file-grid.tsx` — interest/client/company documents grid (confirmed UAT)
|
||||
- `src/components/documents/document-list.tsx` `DocRow` — table-row name cell should be click-to-preview (confirmed UAT 2026-05-21: clicking on the "External EOI — 2026-05-21" filename does nothing)
|
||||
- `src/components/documents/aggregated-section.tsx` — the "Recent Files / Inflight Workflows" panels
|
||||
- `src/components/documents/entity-folder-view.tsx`
|
||||
- Any list surface that takes a `files` array + an `onPreview` callback
|
||||
- **Title cell specifically:** wrap the filename cell in a button-styled span with `onClick={() => onPreview(row)}` so the rep's natural click target works. Keep the row's other action cells (View, Download, kebab) untouched — they're secondary affordances.
|
||||
- **Bundle with Bucket 3 #000 (universal preview)** — pointless to make every row click-to-preview if half the file types render a blank dialog. Ship the two together: file-row surfaces all click-to-preview AND `FilePreviewDialog` handles every mime type (or shows a graceful fallback).
|
||||
- **Effort:** ~1-1.5h for the click-target sweep across 4-5 surfaces; ~5-7h with the universal-preview piece bundled. Captured 2026-05-21 from UAT (FileGrid surfaced specifically; Recent Files captured earlier).
|
||||
11. **[high] Supplemental-info form blocked by portal kill-switch (route nested under `(portal)` group)** — _src/app/(portal)/public/supplemental-info/[token]/page.tsx_ (current location) + _src/app/(portal)/layout.tsx:25-37_ (`isPortalDisabledGlobally()` short-circuit returns "Client portal unavailable" screen for ALL children). The supplemental-info form is token-protected and conceptually independent of the portal login concept — it's a one-shot URL emailed to a client to fill in extra info for an EOI, and should always work as long as the token is valid. But because the route lives inside the `(portal)` route group, it inherits the layout's "portal disabled?" gate. Net effect: any port that hasn't opted into the client portal (the default state for most ports right now) cannot use the supplemental-info flow at all — clients see the "Client portal unavailable" screen when they click the emailed link, even though the rep just sent it successfully.
|
||||
- **Fix:** move the file from `src/app/(portal)/public/supplemental-info/[token]/page.tsx` → `src/app/public/supplemental-info/[token]/page.tsx` (out of the route group). URL stays identical (`/public/supplemental-info/<token>`) because Next route groups don't affect URLs — the route group's only effect was layout inheritance, and moving it drops the portal gate. Verify the API route at `/api/public/supplemental-info/[token]` doesn't have a similar nesting issue (likely fine — `/api/` paths don't share the `(portal)` layout).
|
||||
- **Sweep:** audit `src/app/(portal)/` for any other anonymous token routes that should be outside the group. Currently `find` only returns the one file, but worth verifying as new public flows are added (password-reset tokens, magic-link tokens for non-portal flows, etc.).
|
||||
- **Effort:** ~10 min for the move + verify (no code change, just file relocation + manual click-through). Captured 2026-05-21 from UAT.
|
||||
- **SHIPPED in 2d57417:** route relocated via `git mv` to `src/app/public/supplemental-info/[token]/page.tsx`. URL `/public/supplemental-info/<token>` unchanged (route groups don't affect URLs). Sweep of `src/app/(portal)/` confirmed no other public token routes were similarly nested.
|
||||
12. **[high] Command-search quick-create buttons routed to dead `/new` pages** — _src/components/search/command-search.tsx_ — ZeroState "New client/yacht/company" buttons pushed `/<entity>/new?name=…` which matched the `[id]` dynamic segment and rendered the entity-not-found page. Fixed by switching to `/<entity>?create=1&prefill_name=…` (the existing `useCreateFromUrl` convention) + adding `prefill` prop support to `YachtForm` + `CompanyForm` and wiring `prefill_name` reads in their list components. Now correctly pops the create sheet pre-filled. Fixed in this session.
|
||||
|
||||
---
|
||||
|
||||
|
||||
Reference in New Issue
Block a user