diff --git a/src/app/api/v1/files/upload/route.ts b/src/app/api/v1/files/upload/route.ts index cfc1f53b..9c8cd010 100644 --- a/src/app/api/v1/files/upload/route.ts +++ b/src/app/api/v1/files/upload/route.ts @@ -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( diff --git a/src/app/api/v1/interests/[id]/stage/route.ts b/src/app/api/v1/interests/[id]/stage/route.ts index bd30215d..91b728e3 100644 --- a/src/app/api/v1/interests/[id]/stage/route.ts +++ b/src/app/api/v1/interests/[id]/stage/route.ts @@ -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); diff --git a/src/app/api/v1/me/ports/route.ts b/src/app/api/v1/me/ports/route.ts new file mode 100644 index 00000000..1456cdbc --- /dev/null +++ b/src/app/api/v1/me/ports/route.ts @@ -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); + } +}); diff --git a/src/components/berths/catch-up-wizard.tsx b/src/components/berths/catch-up-wizard.tsx index 0495f94a..120ccc33 100644 --- a/src/components/berths/catch-up-wizard.tsx +++ b/src/components/berths/catch-up-wizard.tsx @@ -78,7 +78,11 @@ export function CatchUpWizard({ berthId, open, onOpenChange }: CatchUpWizardProp const [newClientEmail, setNewClientEmail] = useState(''); const [newClientPhone, setNewClientPhone] = useState(''); const [yachtId, setYachtId] = useState(null); - const [pipelineStage, setPipelineStage] = useState('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(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
- diff --git a/src/components/clients/client-form.tsx b/src/components/clients/client-form.tsx index 0fed2fb6..1d640b0d 100644 --- a/src/components/clients/client-form.tsx +++ b/src/components/clients/client-form.tsx @@ -91,6 +91,7 @@ export function ClientForm({ control, watch, setValue, + getValues, reset, formState: { errors, isSubmitting }, } = useForm, unknown, CreateClientInput>({ @@ -224,7 +225,24 @@ export function ClientForm({ {isEdit ? 'Edit Client' : 'New Client'} -
mutation.mutate(data))} className="space-y-6 py-6"> + { + // 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 diff --git a/src/components/dashboard/activity-feed.tsx b/src/components/dashboard/activity-feed.tsx index 05301117..107d492a 100644 --- a/src/components/dashboard/activity-feed.tsx +++ b/src/components/dashboard/activity-feed.tsx @@ -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 ; } - 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 ( diff --git a/src/components/files/file-preview-dialog.tsx b/src/components/files/file-preview-dialog.tsx index 005f4de6..f45ea719 100644 --- a/src/components/files/file-preview-dialog.tsx +++ b/src/components/files/file-preview-dialog.tsx @@ -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({ )} + {/* 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. */} + + Inline preview of {fileName ?? 'the selected file'}. +
diff --git a/src/components/shared/owner-picker.tsx b/src/components/shared/owner-picker.tsx index 2a697ffb..eb70ccce 100644 --- a/src/components/shared/owner-picker.tsx +++ b/src/components/shared/owner-picker.tsx @@ -113,13 +113,21 @@ export function OwnerPicker({ disabled={disabled} className={cn('w-full justify-between', !value && 'text-muted-foreground')} > - - {value && ( - + + {/* 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 ? ( + {value.type === 'client' ? 'Client:' : 'Company:'} + ) : ( + + Client / Company + )} - {selectedLabel} + {selectedLabel} diff --git a/src/lib/api/client.ts b/src/lib/api/client.ts index 15c4b553..b67b8ae6 100644 --- a/src/lib/api/client.ts +++ b/src/lib/api/client.ts @@ -21,7 +21,10 @@ async function resolvePortIdFromSlug(slug: string): Promise { 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; diff --git a/src/lib/constants.ts b/src/lib/constants.ts index 9e582b83..c6b7bf7f 100644 --- a/src/lib/constants.ts +++ b/src/lib/constants.ts @@ -38,6 +38,54 @@ export const STAGE_LABELS: Record = { 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 = { + 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 = { enquiry: 'Enquiry', diff --git a/src/lib/db/migrations/0066_normalize_legacy_status_override.sql b/src/lib/db/migrations/0066_normalize_legacy_status_override.sql new file mode 100644 index 00000000..72f7217d --- /dev/null +++ b/src/lib/db/migrations/0066_normalize_legacy_status_override.sql @@ -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'; diff --git a/src/lib/services/interests.service.ts b/src/lib/services/interests.service.ts index 79761d06..9dfefa3c 100644 --- a/src/lib/services/interests.service.ts +++ b/src/lib/services/interests.service.ts @@ -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 diff --git a/tests/integration/interests-yacht.test.ts b/tests/integration/interests-yacht.test.ts index 7f044c40..2aad5888 100644 --- a/tests/integration/interests-yacht.test.ts +++ b/tests/integration/interests-yacht.test.ts @@ -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'); });