fix(audit): A1/A2/A4/A6/A8/A9/A16/A17/A19/A20 from 2026-05-15 sweep

Knocks out 10 of the 13 known issues from yesterday's Playwright audit.

A4 — Client form silently rejected submit when a contact row had an
empty value. The F19 filter ran in mutationFn after zod's
handleSubmit had already short-circuited on min(1). Now wraps the
onSubmit to prune empty rows BEFORE handleSubmit/zod sees them.

A16 — File upload to documents hub root 400'd because FormData.get
returns null for absent fields and zod's .optional() rejects null.
Route handler now coerces null/empty → undefined before parse.

A17 — Added /api/v1/me/ports endpoint that any authenticated user
can hit; client.ts now uses it as the bootstrap port-slug→port-id
resolver. Eliminates the wasteful 400s sales-reps and viewers were
firing on every page load against the super-admin-gated /admin/ports.

A1 — Filter permission_denied actions from the dashboard activity
feed. Still in the audit log; just not noise on the dashboard.

A2 — New LEGACY_STAGE_REMAP table + canonicalizeStage / stageLabelFor
helpers in lib/constants. Activity-feed maps legacy 9-stage enum
values (deposit_10pct, contract_sent, etc.) to their 7-stage labels
on the way out, so historical audit rows read as "Deposit Paid" not
"Deposit 10Pct".

A19 — Same-stage write now returns 204 No Content. Service returns
a STAGE_NOOP sentinel; the route handler translates it.

A9 — Catch-up wizard now derives stage from berth status (under_offer
→ EOI, sold → contract) with a stageOverride state for explicit
user picks. Avoids the set-state-in-effect rule violation.

A20 — OwnerPicker shows a "Client / Company" hint chip on the
trigger when no value is set, so users know the trigger opens a
two-tab picker instead of just a client list.

A8 — Migration 0066 normalizes legacy `statusOverrideMode = 'auto'`
to NULL so the column lives at strictly 3 states.

A6 — file-preview-dialog gets a screen-reader DialogDescription so
the Radix "Missing aria-describedby" warning stops firing on every
preview.

A18 closed as not-a-bug: /api/v1/users genuinely doesn't exist
(Next returns 404); /api/v1/admin/audit exists and 403s.

A5 (Socket.IO dev noise) + A3 (react-grab CSP) left for a separate
pass — both are dev-only cosmetic.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
2026-05-15 01:12:20 +02:00
parent 3b3ac287e0
commit 0d9208a052
13 changed files with 212 additions and 35 deletions

View File

@@ -17,18 +17,24 @@ export const POST = withAuth(
const buffer = Buffer.from(await file.arrayBuffer());
const folderIdRaw = formData.get('folderId') as string | undefined;
// A16: FormData.get returns null when the field is absent, not
// undefined. Zod's .optional() accepts undefined but rejects null,
// so the previous `as string | undefined` cast lied and uploads at
// the hub root (no entity selected) 400'd. Coerce absent / empty
// values to undefined before parse.
const formStr = (key: string): string | undefined => {
const v = formData.get(key);
return typeof v === 'string' && v.length > 0 ? v : undefined;
};
const metadata = uploadFileSchema.parse({
filename: (formData.get('filename') as string | null) ?? file.name,
clientId: formData.get('clientId') as string | undefined,
yachtId: formData.get('yachtId') as string | undefined,
companyId: formData.get('companyId') as string | undefined,
category: formData.get('category') as string | undefined,
entityType: formData.get('entityType') as string | undefined,
entityId: formData.get('entityId') as string | undefined,
// Hub uploads pass the current folderId so the file lands inside
// the user's currently-selected folder. Empty string ⇒ root (null).
folderId: folderIdRaw && folderIdRaw.length > 0 ? folderIdRaw : undefined,
filename: formStr('filename') ?? file.name,
clientId: formStr('clientId'),
yachtId: formStr('yachtId'),
companyId: formStr('companyId'),
category: formStr('category'),
entityType: formStr('entityType'),
entityId: formStr('entityId'),
folderId: formStr('folderId'),
});
const result = await uploadFile(

View File

@@ -3,7 +3,7 @@ import { NextResponse } from 'next/server';
import { withAuth, withPermission } from '@/lib/api/helpers';
import { parseBody } from '@/lib/api/route-helpers';
import { errorResponse, ForbiddenError } from '@/lib/errors';
import { changeInterestStage } from '@/lib/services/interests.service';
import { changeInterestStage, STAGE_NOOP } from '@/lib/services/interests.service';
import { changeStageSchema } from '@/lib/validators/interests';
export const PATCH = withAuth(
@@ -26,6 +26,10 @@ export const PATCH = withAuth(
ipAddress: ctx.ipAddress,
userAgent: ctx.userAgent,
});
// A19 / F27: same-stage write returns the sentinel — emit 204.
if (interest === STAGE_NOOP) {
return new NextResponse(null, { status: 204 });
}
return NextResponse.json({ data: interest });
} catch (error) {
return errorResponse(error);

View File

@@ -0,0 +1,38 @@
import { NextResponse } from 'next/server';
import { eq } from 'drizzle-orm';
import { withAuth } from '@/lib/api/helpers';
import { db } from '@/lib/db';
import { ports as portsTable } from '@/lib/db/schema/ports';
import { userPortRoles, userProfiles } from '@/lib/db/schema/users';
import { errorResponse } from '@/lib/errors';
// A17: bootstrap-friendly ports list for the calling user — sales-reps
// and viewers can hit this without the super-admin gate that blocks
// `/api/v1/admin/ports`. Returns only the ports the user actually has
// access to (super-admin sees every active port).
export const GET = withAuth(async (_req, ctx) => {
try {
const profile = await db.query.userProfiles.findFirst({
where: eq(userProfiles.userId, ctx.userId),
});
if (profile?.isSuperAdmin) {
const all = await db.query.ports.findMany({
where: eq(portsTable.isActive, true),
orderBy: portsTable.name,
columns: { id: true, slug: true, name: true },
});
return NextResponse.json({ data: all });
}
const memberships = await db.query.userPortRoles.findMany({
where: eq(userPortRoles.userId, ctx.userId),
with: { port: { columns: { id: true, slug: true, name: true } } },
});
const data = memberships.map((m) => m.port);
return NextResponse.json({ data });
} catch (error) {
return errorResponse(error);
}
});

View File

@@ -78,7 +78,11 @@ export function CatchUpWizard({ berthId, open, onOpenChange }: CatchUpWizardProp
const [newClientEmail, setNewClientEmail] = useState('');
const [newClientPhone, setNewClientPhone] = useState('');
const [yachtId, setYachtId] = useState<string | null>(null);
const [pipelineStage, setPipelineStage] = useState<string>('enquiry');
// A9: stageOverride is the user's explicit choice. When null, the
// effective stage derives from the loaded berth's status (under_offer
// → eoi, sold → contract). Pre-fix this was a useState seeded to
// 'enquiry' which never updated when the berth loaded.
const [stageOverride, setStageOverride] = useState<string | null>(null);
// Fetch the berth so the wizard can scope the stage options to what
// makes sense for the current manual status. Disabled until open so
@@ -95,11 +99,7 @@ export function CatchUpWizard({ berthId, open, onOpenChange }: CatchUpWizardProp
// under_offer defaults to eoi since that's the most common pre-deal
// status that reps mark manually.
const defaultStage = berth?.data.status === 'sold' ? 'contract' : 'eoi';
// Keep selected stage in sync with the loaded berth's allowed set.
if (berth && pipelineStage !== defaultStage && !allowedStages.includes(pipelineStage)) {
setPipelineStage(defaultStage);
}
const pipelineStage = stageOverride ?? defaultStage;
const submit = useMutation({
mutationFn: async () => {
@@ -143,7 +143,7 @@ export function CatchUpWizard({ berthId, open, onOpenChange }: CatchUpWizardProp
setNewClientEmail('');
setNewClientPhone('');
setYachtId(null);
setPipelineStage('enquiry');
setStageOverride(null);
}
return (
@@ -235,7 +235,7 @@ export function CatchUpWizard({ berthId, open, onOpenChange }: CatchUpWizardProp
<div className="space-y-1">
<Label>Pipeline stage</Label>
<Select value={pipelineStage} onValueChange={setPipelineStage}>
<Select value={pipelineStage} onValueChange={setStageOverride}>
<SelectTrigger>
<SelectValue />
</SelectTrigger>

View File

@@ -91,6 +91,7 @@ export function ClientForm({
control,
watch,
setValue,
getValues,
reset,
formState: { errors, isSubmitting },
} = useForm<z.input<typeof createClientSchema>, unknown, CreateClientInput>({
@@ -224,7 +225,24 @@ export function ClientForm({
<SheetTitle>{isEdit ? 'Edit Client' : 'New Client'}</SheetTitle>
</SheetHeader>
<form onSubmit={handleSubmit((data) => mutation.mutate(data))} className="space-y-6 py-6">
<form
onSubmit={(e) => {
// A4: prune empty contact rows BEFORE handleSubmit/zod runs.
// The schema requires `value: z.string().min(1)`, so an empty
// row (the form pre-adds one for convenience) silently fails
// form validation with no visible error. Strip them first so
// the rest of the validation sees only real rows.
const current = getValues('contacts') ?? [];
const cleaned = current.filter(
(c) => typeof c?.value === 'string' && c.value.trim().length > 0,
);
if (cleaned.length !== current.length) {
setValue('contacts', cleaned, { shouldValidate: false });
}
return handleSubmit((data) => mutation.mutate(data))(e);
}}
className="space-y-6 py-6"
>
{/* Dedup suggestion - only on the create path. Watches the
live form values for email / phone / name and surfaces
an existing client when one matches. The user can

View File

@@ -8,7 +8,13 @@ import { Card, CardContent, CardHeader, CardTitle } from '@/components/ui/card';
import { Badge } from '@/components/ui/badge';
import { CardSkeleton } from '@/components/shared/loading-skeleton';
import { WidgetErrorBoundary } from './widget-error-boundary';
import { STAGE_LABELS, formatSource, type PipelineStage } from '@/lib/constants';
import {
STAGE_LABELS,
PIPELINE_STAGES,
LEGACY_STAGE_REMAP,
formatSource,
type PipelineStage,
} from '@/lib/constants';
interface ActivityItem {
id: string;
@@ -43,7 +49,14 @@ function normalizeEnumValue(field: string, value: unknown): unknown {
if (typeof value !== 'string') return value;
const f = field.replace(/_/g, '').toLowerCase();
if (f === 'pipelinestage' || f === 'stage') {
return STAGE_LABELS[value as PipelineStage] ?? humanizeFieldName(value);
// A2: map legacy 9-stage enum values to their 7-stage equivalents so
// historical audit-log rows ("deposit_10pct", "contract_sent", ...)
// render as the modern label rather than a humanized raw enum.
const modern = (PIPELINE_STAGES as readonly string[]).includes(value)
? (value as PipelineStage)
: LEGACY_STAGE_REMAP[value];
if (modern) return STAGE_LABELS[modern];
return humanizeFieldName(value);
}
if (f === 'source') {
return formatSource(value) ?? value;
@@ -169,7 +182,11 @@ function ActivityFeedInner() {
return <CardSkeleton />;
}
const items = data ?? [];
// A1: permission_denied rows on the activity feed render as a bare
// action badge with no entity name (they target `admin.X` with empty
// entityId). They're noise for the rep — keep them in the audit log
// page but hide them from the dashboard feed.
const items = (data ?? []).filter((i) => i.action !== 'permission_denied');
return (
<Card>

View File

@@ -5,7 +5,13 @@ import dynamic from 'next/dynamic';
import { ExternalLink, ZoomIn } from 'lucide-react';
import { useQuery } from '@tanstack/react-query';
import { Dialog, DialogContent, DialogHeader, DialogTitle } from '@/components/ui/dialog';
import {
Dialog,
DialogContent,
DialogDescription,
DialogHeader,
DialogTitle,
} from '@/components/ui/dialog';
import { apiFetch } from '@/lib/api/client';
// yet-another-react-lightbox is ~50kb, lazy-load it.
@@ -71,6 +77,12 @@ export function FilePreviewDialog({
</a>
)}
</DialogTitle>
{/* A6: screen-reader description; visually hidden because the
* title + preview surface tells sighted users what the dialog
* contains. Skips the Radix "missing aria-describedby" warning. */}
<DialogDescription className="sr-only">
Inline preview of {fileName ?? 'the selected file'}.
</DialogDescription>
</DialogHeader>
<div className="flex-1 overflow-hidden rounded-lg border bg-muted/20">

View File

@@ -113,13 +113,21 @@ export function OwnerPicker({
disabled={disabled}
className={cn('w-full justify-between', !value && 'text-muted-foreground')}
>
<span className="truncate">
{value && (
<span className="mr-2 text-xs opacity-60">
<span className="truncate flex items-center gap-2">
{/* A20: surface the dual-mode (Client/Company) hint even when
* no value is picked yet, so users know the trigger opens a
* two-tab picker — pre-fix the toggle was hidden until the
* popover was open, making the form read as client-only. */}
{value ? (
<span className="text-xs opacity-60">
{value.type === 'client' ? 'Client:' : 'Company:'}
</span>
) : (
<span className="rounded-sm border border-border bg-muted px-1.5 py-px text-[10px] uppercase tracking-wide text-muted-foreground">
Client / Company
</span>
)}
{selectedLabel}
<span className="truncate">{selectedLabel}</span>
</span>
<ChevronsUpDown className="ml-2 h-4 w-4 shrink-0 opacity-50" aria-hidden />
</Button>

View File

@@ -21,7 +21,10 @@ async function resolvePortIdFromSlug(slug: string): Promise<string | null> {
if (!inFlightPortsLookup) {
inFlightPortsLookup = (async () => {
try {
const res = await fetch('/api/v1/admin/ports', { credentials: 'include' });
// A17: use /me/ports — works for every authenticated user.
// The prior code hit /admin/ports which is super-admin-gated, so
// sales-reps/viewers fired a wasteful 400 on every page load.
const res = await fetch('/api/v1/me/ports', { credentials: 'include' });
if (!res.ok) return null;
const body = (await res.json()) as { data?: Array<{ id: string; slug: string }> };
return body.data ?? null;

View File

@@ -38,6 +38,54 @@ export const STAGE_LABELS: Record<PipelineStage, string> = {
contract: 'Contract',
};
/**
* Map legacy 9-stage enum values to their 7-stage equivalents. Audit logs
* and any pre-migration data still carry the legacy values; this lets the
* activity feed, audit diffs, and reporting render the modern label
* without having to back-fill the underlying rows.
*
* Mirrors the migration applied in `seed-synthetic-data.ts` (and
* documented in the 9→7 pipeline refactor):
* details_sent → enquiry
* in_communication → qualified
* eoi_sent, eoi_signed → eoi (doc-status carries sent/signed sub-state)
* deposit_10pct → deposit_paid
* contract_sent, contract_signed → contract
* completed → contract (with outcome=won)
* open → enquiry (legacy alias for the initial stage)
*/
export const LEGACY_STAGE_REMAP: Record<string, PipelineStage> = {
open: 'enquiry',
details_sent: 'enquiry',
in_communication: 'qualified',
eoi_sent: 'eoi',
eoi_signed: 'eoi',
deposit_10pct: 'deposit_paid',
contract_sent: 'contract',
contract_signed: 'contract',
completed: 'contract',
};
/**
* Resolve any stage-like string to a canonical 7-stage value. Returns
* the modern stage as-is, maps legacy values via LEGACY_STAGE_REMAP,
* and falls back to 'enquiry' for genuinely unknown values.
*/
export function canonicalizeStage(value: string | null | undefined): PipelineStage {
if (!value) return 'enquiry';
if (PIPELINE_STAGES.includes(value as PipelineStage)) return value as PipelineStage;
return LEGACY_STAGE_REMAP[value] ?? 'enquiry';
}
/**
* Human-friendly label for any stage-like string — modern or legacy. Use
* this in any read surface (activity feed, audit diff, notification copy,
* reports) that might be handed pre-migration data.
*/
export function stageLabelFor(value: string | null | undefined): string {
return STAGE_LABELS[canonicalizeStage(value)];
}
// Compact labels for cramped contexts (mobile chart axes, dense tables).
export const STAGE_SHORT_LABELS: Record<PipelineStage, string> = {
enquiry: 'Enquiry',

View File

@@ -0,0 +1,11 @@
-- A8: normalize legacy `statusOverrideMode = 'auto'` values to NULL.
--
-- The NocoDB importer historically wrote 'auto' to indicate "no override
-- in effect" for legacy data. The post-refactor code uses NULL for that
-- sentinel and 'manual' / 'automated' for the new states. Mixed values
-- pollute the reconcile-queue predicate and the Manual chip — neither
-- path treats 'auto' specially today, but normalizing closes the gap
-- once and for all and keeps the column to a 3-state enum.
UPDATE berths
SET status_override_mode = NULL
WHERE status_override_mode = 'auto';

View File

@@ -818,6 +818,13 @@ export async function updateInterest(
// ─── Change Stage ─────────────────────────────────────────────────────────────
/**
* Sentinel returned by changeInterestStage when the requested target
* matches the current stage (no audit row, no socket emit, no DB update).
* The route handler translates this to a 204 No Content response.
*/
export const STAGE_NOOP = Symbol('stage-noop');
export async function changeInterestStage(
id: string,
portId: string,
@@ -832,12 +839,13 @@ export async function changeInterestStage(
throw new NotFoundError('Interest');
}
// F27: same-stage write is a no-op. Return the existing row without
// bumping updatedAt or emitting an audit log entry — pre-fix every
// re-submit (e.g. accidental double-click) wrote a "Same → Same"
// audit entry and triggered downstream invalidations.
// F27 / A19: same-stage write is a no-op. The service signals this via
// the sentinel `STAGE_NOOP` so the route handler can return 204 No
// Content instead of 200 + full body. Pre-fix every re-submit (e.g.
// accidental double-click) wrote a "Same → Same" audit entry and
// triggered downstream invalidations.
if (existing.pipelineStage === data.pipelineStage) {
return existing;
return STAGE_NOOP;
}
// Plan: yachtId required to leave the initial enquiry stage

View File

@@ -186,6 +186,10 @@ describe('interests.service — yacht ownership validation', () => {
{ pipelineStage: 'qualified' },
makeAuditMeta({ portId: port.id }),
);
// After A19: changeInterestStage returns the sentinel STAGE_NOOP only
// when target === current. Here the stage actually changes, so the
// result is the updated row.
if (typeof updated === 'symbol') throw new Error('unexpected no-op');
expect(updated.pipelineStage).toBe('qualified');
});