fix(audit-wave-9): onboarding + first-run UX fixes (onboarding-auditor)
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) <noreply@anthropic.com>
This commit is contained in:
@@ -299,7 +299,8 @@ const GROUPS: AdminGroup[] = [
|
|||||||
{
|
{
|
||||||
href: 'onboarding',
|
href: 'onboarding',
|
||||||
label: 'Onboarding checklist',
|
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,
|
icon: LayoutDashboard,
|
||||||
},
|
},
|
||||||
],
|
],
|
||||||
|
|||||||
@@ -19,6 +19,11 @@ interface OnboardingStep {
|
|||||||
* checkmark auto-fills from the settings list. When undefined, the
|
* checkmark auto-fills from the settings list. When undefined, the
|
||||||
* step relies on the manual checkbox in `onboarding_status`. */
|
* step relies on the manual checkbox in `onboarding_status`. */
|
||||||
autoCheckSettingKey?: string;
|
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
|
/** Override: read this many users / tags / roles from a list endpoint
|
||||||
* and consider the step done when count > 0. */
|
* and consider the step done when count > 0. */
|
||||||
autoCheckListEndpoint?: string;
|
autoCheckListEndpoint?: string;
|
||||||
@@ -38,15 +43,24 @@ const STEPS: OnboardingStep[] = [
|
|||||||
label: 'Configure outgoing email',
|
label: 'Configure outgoing email',
|
||||||
description:
|
description:
|
||||||
'From-address, signature, footer, plus per-port SMTP overrides if you don’t use the global account.',
|
'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',
|
id: 'documenso',
|
||||||
href: 'documenso',
|
href: 'documenso',
|
||||||
label: 'Connect Documenso for EOIs',
|
label: 'Connect Documenso for EOIs',
|
||||||
description:
|
description:
|
||||||
'API credentials and the EOI template id, plus the in-app vs Documenso pathway choice.',
|
'API credentials, the EOI template id, plus the developer + approver identity that signs every EOI.',
|
||||||
autoCheckSettingKey: 'documenso_api_url',
|
// 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',
|
id: 'settings',
|
||||||
@@ -54,7 +68,10 @@ const STEPS: OnboardingStep[] = [
|
|||||||
label: 'Tune business rules + recommender weights',
|
label: 'Tune business rules + recommender weights',
|
||||||
description:
|
description:
|
||||||
'Pipeline weights, net-10 discount, berth recommender knobs (heat weights, fall-through policy).',
|
'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',
|
id: 'roles',
|
||||||
@@ -88,7 +105,7 @@ const STEPS: OnboardingStep[] = [
|
|||||||
},
|
},
|
||||||
{
|
{
|
||||||
id: 'forms',
|
id: 'forms',
|
||||||
href: '../',
|
href: 'forms',
|
||||||
label: 'Wire the website intake forms',
|
label: 'Wire the website intake forms',
|
||||||
description:
|
description:
|
||||||
'Inquiry forms on the marketing site dual-write into the CRM via /api/public/website-inquiries. Manually mark complete when verified.',
|
'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 all = [...settings.data.portSettings, ...settings.data.globalSettings];
|
||||||
const byKey = new Map(all.map((r) => [r.key, r.value]));
|
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<string, boolean> = {};
|
const checks: Record<string, boolean> = {};
|
||||||
const listChecks = await Promise.all(
|
const listChecks = await Promise.all(
|
||||||
STEPS.map(async (s) => {
|
STEPS.map(async (s) => {
|
||||||
if (s.autoCheckSettingKey) {
|
if (s.autoCheckSettingKey) {
|
||||||
const v = byKey.get(s.autoCheckSettingKey);
|
return [s.id, isPresent(byKey.get(s.autoCheckSettingKey))] as const;
|
||||||
return [s.id, v !== undefined && v !== null && v !== '' && v !== false] as const;
|
}
|
||||||
|
if (s.autoCheckSettingKeysAll) {
|
||||||
|
return [
|
||||||
|
s.id,
|
||||||
|
s.autoCheckSettingKeysAll.every((k) => isPresent(byKey.get(k))),
|
||||||
|
] as const;
|
||||||
}
|
}
|
||||||
if (s.autoCheckListEndpoint) {
|
if (s.autoCheckListEndpoint) {
|
||||||
try {
|
try {
|
||||||
@@ -226,7 +250,9 @@ export function OnboardingChecklist() {
|
|||||||
<p className="mt-1 text-[11px] text-emerald-700">
|
<p className="mt-1 text-[11px] text-emerald-700">
|
||||||
Auto-detected complete via{' '}
|
Auto-detected complete via{' '}
|
||||||
<code className="text-[10px]">
|
<code className="text-[10px]">
|
||||||
{step.autoCheckSettingKey ?? step.autoCheckListEndpoint}
|
{step.autoCheckSettingKey ??
|
||||||
|
step.autoCheckSettingKeysAll?.join(' + ') ??
|
||||||
|
step.autoCheckListEndpoint}
|
||||||
</code>
|
</code>
|
||||||
</p>
|
</p>
|
||||||
)}
|
)}
|
||||||
|
|||||||
@@ -124,11 +124,23 @@ export function BerthList() {
|
|||||||
// B2, …) so consecutive rows naturally share dock letters.
|
// B2, …) so consecutive rows naturally share dock letters.
|
||||||
mobileGroupBy={(row) => row.area ?? 'Unassigned'}
|
mobileGroupBy={(row) => row.area ?? 'Unassigned'}
|
||||||
emptyState={
|
emptyState={
|
||||||
<EmptyState
|
// Distinguish "no data at all" (fresh port, run the importer)
|
||||||
icon={Anchor}
|
// from "no rows after filtering" (adjust filters). The original
|
||||||
title="No berths found"
|
// copy implied data existed but was hidden, which misled fresh-
|
||||||
description="Berths are imported from external sources. Adjust your filters to find what you're looking for."
|
// port admins for whom there is literally nothing yet.
|
||||||
/>
|
Object.values(filters).every((v) => v === undefined || v === '') ? (
|
||||||
|
<EmptyState
|
||||||
|
icon={Anchor}
|
||||||
|
title="No berths yet"
|
||||||
|
description="Berths are imported from external sources. Run the importer once the source data is ready: pnpm tsx scripts/import-berths-from-nocodb.ts --apply --port-slug <slug>."
|
||||||
|
/>
|
||||||
|
) : (
|
||||||
|
<EmptyState
|
||||||
|
icon={Anchor}
|
||||||
|
title="No berths match these filters"
|
||||||
|
description="Adjust your filters or clear them to see every berth."
|
||||||
|
/>
|
||||||
|
)
|
||||||
}
|
}
|
||||||
/>
|
/>
|
||||||
</div>
|
</div>
|
||||||
|
|||||||
@@ -5,6 +5,7 @@ import { ports } from '@/lib/db/schema';
|
|||||||
import type { PortSettings } from '@/lib/db/schema/ports';
|
import type { PortSettings } from '@/lib/db/schema/ports';
|
||||||
import { createAuditLog, type AuditMeta } from '@/lib/audit';
|
import { createAuditLog, type AuditMeta } from '@/lib/audit';
|
||||||
import { ConflictError, NotFoundError } from '@/lib/errors';
|
import { ConflictError, NotFoundError } from '@/lib/errors';
|
||||||
|
import { logger } from '@/lib/logger';
|
||||||
import { emitToRoom } from '@/lib/socket/server';
|
import { emitToRoom } from '@/lib/socket/server';
|
||||||
import type { CreatePortInput, UpdatePortInput } from '@/lib/validators/ports';
|
import type { CreatePortInput, UpdatePortInput } from '@/lib/validators/ports';
|
||||||
import { ensureSystemRoots } from '@/lib/services/document-folders.service';
|
import { ensureSystemRoots } from '@/lib/services/document-folders.service';
|
||||||
@@ -41,9 +42,20 @@ export async function createPort(data: CreatePortInput, meta: AuditMeta) {
|
|||||||
})
|
})
|
||||||
.returning();
|
.returning();
|
||||||
|
|
||||||
// Non-fatal if this throws: ensureSystemRoots is re-runnable, and
|
// Non-fatal if this throws: ensureSystemRoots is re-runnable (any
|
||||||
// scripts/backfill-document-folders.ts heals orphaned ports.
|
// subsequent admin action self-heals via `ensureEntityFolder`'s
|
||||||
await ensureSystemRoots(port!.id, meta.userId);
|
// 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({
|
void createAuditLog({
|
||||||
userId: meta.userId,
|
userId: meta.userId,
|
||||||
|
|||||||
Reference in New Issue
Block a user