From a8dec0badabc649b4e18b887db4499c1b030c5be Mon Sep 17 00:00:00 2001 From: Matt Date: Wed, 13 May 2026 12:15:46 +0200 Subject: [PATCH] fix(audit-wave-9): onboarding + first-run UX fixes (onboarding-auditor) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address the CRITICAL and high-leverage HIGH items from the onboarding-auditor report: **C1 — checklist auto-checks were reading the wrong setting keys** A port that had actually been configured still showed three steps as incomplete, permanently capping the checklist at < 70 %. - email step: `sales_email_smtp_host` → `smtp_host_override` (the key the email admin page actually persists). - documenso step: `documenso_api_url` → compound gate `documenso_api_url_override` + `documenso_developer_email` + `documenso_approver_email` + `documenso_eoi_template_id`. All four are required for `buildDocumensoPayload` not to error out; checking only the URL falsely greenlit the step until a rep tried to send an EOI and Documenso 404'd. - settings step: `recommender_top_n_default` → `heat_weight_recency`. The defaults are layered (port > global > built-in), so a port using the built-ins never writes the `top_n_default` row — old key was an unreachable green. heat_weight_recency genuinely means "admin tuned the recommender". **C2 — forms step href was broken** `STEPS[8].href = '../'` resolved through the Link template to the dashboard, not `/admin/forms`. Fixed to `'forms'`. **C3 — EOI signer-identity gate** Folded into the new compound-gate logic on the documenso step (see C1). Now matches what the EOI pipeline actually requires before it can send. **C4 — ensureSystemRoots failure mode poisoned port creation** `ports.service.createPort` awaited `ensureSystemRoots` after the port row had committed, so a throw bubbled out as a 500 even though the inline comment said "non-fatal if this throws". Wrap in try/catch + logger.warn — the row stays live, the next admin action self-heals via `ensureEntityFolder`, and the operator doesn't retry into a 409. **H5 — berth-list empty-state copy misleads fresh ports** "Berths are imported from external sources. Adjust your filters..." implied data existed but was hidden. Branch on whether any filter is active: with none, suggest running `import-berths-from-nocodb.ts`; with filters, the original "adjust filters" message. **M4 — admin-sections-browser description was wrong** "Setup checklist for fresh ports (read-only references)" implied the page was read-only when it has working manual-completion checkboxes and discouraged clicking in. Reworded. Additionally, the OnboardingStep type gains an optional `autoCheckSettingKeysAll` field for compound gates (used by the documenso step), and the auto-detected hint shows all keys when the gate is compound. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../admin/admin-sections-browser.tsx | 3 +- src/components/admin/onboarding-checklist.tsx | 42 +++++++++++++++---- src/components/berths/berth-list.tsx | 22 +++++++--- src/lib/services/ports.service.ts | 18 ++++++-- 4 files changed, 68 insertions(+), 17 deletions(-) diff --git a/src/components/admin/admin-sections-browser.tsx b/src/components/admin/admin-sections-browser.tsx index 8eceb633..821c9ae1 100644 --- a/src/components/admin/admin-sections-browser.tsx +++ b/src/components/admin/admin-sections-browser.tsx @@ -299,7 +299,8 @@ const GROUPS: AdminGroup[] = [ { href: 'onboarding', label: 'Onboarding checklist', - description: 'Setup checklist for fresh ports (read-only references).', + description: + 'Step-by-step setup checklist for fresh ports — auto-detects what you’ve configured and lets you mark manual steps complete.', icon: LayoutDashboard, }, ], diff --git a/src/components/admin/onboarding-checklist.tsx b/src/components/admin/onboarding-checklist.tsx index 7041826b..b54672c7 100644 --- a/src/components/admin/onboarding-checklist.tsx +++ b/src/components/admin/onboarding-checklist.tsx @@ -19,6 +19,11 @@ interface OnboardingStep { * checkmark auto-fills from the settings list. When undefined, the * step relies on the manual checkbox in `onboarding_status`. */ autoCheckSettingKey?: string; + /** Multi-key gate: all listed setting keys must be present (non-empty) + * for the step to auto-complete. Useful for compound checks where a + * single key would falsely mark "done" — e.g. Documenso needs a URL + * plus signer identity plus a template id, not just the URL. */ + autoCheckSettingKeysAll?: readonly string[]; /** Override: read this many users / tags / roles from a list endpoint * and consider the step done when count > 0. */ autoCheckListEndpoint?: string; @@ -38,15 +43,24 @@ const STEPS: OnboardingStep[] = [ label: 'Configure outgoing email', description: 'From-address, signature, footer, plus per-port SMTP overrides if you don’t use the global account.', - autoCheckSettingKey: 'sales_email_smtp_host', + autoCheckSettingKey: 'smtp_host_override', }, { id: 'documenso', href: 'documenso', label: 'Connect Documenso for EOIs', description: - 'API credentials and the EOI template id, plus the in-app vs Documenso pathway choice.', - autoCheckSettingKey: 'documenso_api_url', + 'API credentials, the EOI template id, plus the developer + approver identity that signs every EOI.', + // Compound gate: an EOI cannot be sent without ALL of these. A + // port-admin who saves only the URL would otherwise see the step go + // green and discover the gap on first EOI attempt (Documenso 404s + // on a missing template, or sends recipients with empty names). + autoCheckSettingKeysAll: [ + 'documenso_api_url_override', + 'documenso_developer_email', + 'documenso_approver_email', + 'documenso_eoi_template_id', + ], }, { id: 'settings', @@ -54,7 +68,10 @@ const STEPS: OnboardingStep[] = [ label: 'Tune business rules + recommender weights', description: 'Pipeline weights, net-10 discount, berth recommender knobs (heat weights, fall-through policy).', - autoCheckSettingKey: 'recommender_top_n_default', + // Recommender defaults are layered (port > global > built-in), so a + // port that uses the built-ins never writes a row. Use a tuned + // heat-weight as the "admin actually saw + chose" sentinel instead. + autoCheckSettingKey: 'heat_weight_recency', }, { id: 'roles', @@ -88,7 +105,7 @@ const STEPS: OnboardingStep[] = [ }, { id: 'forms', - href: '../', + href: 'forms', label: 'Wire the website intake forms', description: 'Inquiry forms on the marketing site dual-write into the CRM via /api/public/website-inquiries. Manually mark complete when verified.', @@ -120,12 +137,19 @@ export function OnboardingChecklist() { const all = [...settings.data.portSettings, ...settings.data.globalSettings]; const byKey = new Map(all.map((r) => [r.key, r.value])); + const isPresent = (v: unknown) => v !== undefined && v !== null && v !== '' && v !== false; + const checks: Record = {}; const listChecks = await Promise.all( STEPS.map(async (s) => { if (s.autoCheckSettingKey) { - const v = byKey.get(s.autoCheckSettingKey); - return [s.id, v !== undefined && v !== null && v !== '' && v !== false] as const; + return [s.id, isPresent(byKey.get(s.autoCheckSettingKey))] as const; + } + if (s.autoCheckSettingKeysAll) { + return [ + s.id, + s.autoCheckSettingKeysAll.every((k) => isPresent(byKey.get(k))), + ] as const; } if (s.autoCheckListEndpoint) { try { @@ -226,7 +250,9 @@ export function OnboardingChecklist() {

Auto-detected complete via{' '} - {step.autoCheckSettingKey ?? step.autoCheckListEndpoint} + {step.autoCheckSettingKey ?? + step.autoCheckSettingKeysAll?.join(' + ') ?? + step.autoCheckListEndpoint}

)} diff --git a/src/components/berths/berth-list.tsx b/src/components/berths/berth-list.tsx index 019d314c..3f294864 100644 --- a/src/components/berths/berth-list.tsx +++ b/src/components/berths/berth-list.tsx @@ -124,11 +124,23 @@ export function BerthList() { // B2, …) so consecutive rows naturally share dock letters. mobileGroupBy={(row) => row.area ?? 'Unassigned'} emptyState={ - + // Distinguish "no data at all" (fresh port, run the importer) + // from "no rows after filtering" (adjust filters). The original + // copy implied data existed but was hidden, which misled fresh- + // port admins for whom there is literally nothing yet. + Object.values(filters).every((v) => v === undefined || v === '') ? ( + + ) : ( + + ) } /> diff --git a/src/lib/services/ports.service.ts b/src/lib/services/ports.service.ts index 0bab8542..fb66929e 100644 --- a/src/lib/services/ports.service.ts +++ b/src/lib/services/ports.service.ts @@ -5,6 +5,7 @@ import { ports } from '@/lib/db/schema'; import type { PortSettings } from '@/lib/db/schema/ports'; import { createAuditLog, type AuditMeta } from '@/lib/audit'; import { ConflictError, NotFoundError } from '@/lib/errors'; +import { logger } from '@/lib/logger'; import { emitToRoom } from '@/lib/socket/server'; import type { CreatePortInput, UpdatePortInput } from '@/lib/validators/ports'; import { ensureSystemRoots } from '@/lib/services/document-folders.service'; @@ -41,9 +42,20 @@ export async function createPort(data: CreatePortInput, meta: AuditMeta) { }) .returning(); - // Non-fatal if this throws: ensureSystemRoots is re-runnable, and - // scripts/backfill-document-folders.ts heals orphaned ports. - await ensureSystemRoots(port!.id, meta.userId); + // Non-fatal if this throws: ensureSystemRoots is re-runnable (any + // subsequent admin action self-heals via `ensureEntityFolder`'s + // fallback, and `scripts/backfill-document-folders.ts` covers + // orphaned ports). Swallow + log instead of propagating, so the + // operator doesn't see a 500 from `createPort` against an already- + // committed port row — they'd retry and hit a 409 slug-exists. + try { + await ensureSystemRoots(port!.id, meta.userId); + } catch (err) { + logger.warn( + { portId: port!.id, err }, + 'ensureSystemRoots failed after port create — port row is live; system folders will be created on first admin action.', + ); + } void createAuditLog({ userId: meta.userId,