From 77829485a7d10ed0df810355c86c3914c101f6c1 Mon Sep 17 00:00:00 2001 From: Matt Date: Tue, 2 Jun 2026 12:24:46 +0200 Subject: [PATCH] =?UTF-8?q?fix(audit):=20H5=20=E2=80=94=20keep=20yacht=20o?= =?UTF-8?q?wnership-history=20ledger=20consistent=20on=20archive/restore?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Extracts transferOwnershipTx (close open yacht_ownership_history row + open a new one + update denormalized owner) from transferOwnership, and uses it in client-archive + client-restore instead of writing only the denormalized columns — which left the ledger showing the old owner as current and let the next real transfer close the wrong row. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/lib/services/client-archive.service.ts | 16 +++- src/lib/services/client-restore.service.ts | 29 ++++-- src/lib/services/yachts.service.ts | 100 +++++++++++++++------ 3 files changed, 105 insertions(+), 40 deletions(-) diff --git a/src/lib/services/client-archive.service.ts b/src/lib/services/client-archive.service.ts index bbfc3900..2ce108ea 100644 --- a/src/lib/services/client-archive.service.ts +++ b/src/lib/services/client-archive.service.ts @@ -30,6 +30,7 @@ import { type ClientArchiveDossier, } from '@/lib/services/client-archive-dossier.service'; import { activeInterestsWhere } from '@/lib/services/active-interest'; +import { transferOwnershipTx } from '@/lib/services/yachts.service'; // ─── Decision payload (what the UI sends to the server) ───────────────────── @@ -246,10 +247,17 @@ export async function archiveClientWithDecisions(args: { `Yacht ${d.yachtId}: transfer requires newOwnerType + newOwnerId`, ); } - await tx - .update(yachts) - .set({ currentOwnerType: d.newOwnerType, currentOwnerId: d.newOwnerId }) - .where(eq(yachts.id, d.yachtId)); + // Move ownership through the shared ledger-aware helper so the + // yacht_ownership_history row is closed + reopened in lock-step + // with the denormalized columns. Writing only the denorm columns + // (the prior bug) left the history's open row pointing at the + // archived client, corrupting the ledger on the next real transfer. + await transferOwnershipTx(tx as unknown as typeof db, { + yachtId: d.yachtId, + newOwner: { type: d.newOwnerType, id: d.newOwnerId }, + transferReason: `Smart-archive: ${decisions.reason || 'client archived'}`, + createdBy: meta.userId, + }); persistedDecisions.push({ kind: 'yacht_transferred', refId: d.yachtId, diff --git a/src/lib/services/client-restore.service.ts b/src/lib/services/client-restore.service.ts index af42f8f6..4a11035f 100644 --- a/src/lib/services/client-restore.service.ts +++ b/src/lib/services/client-restore.service.ts @@ -28,6 +28,7 @@ import { portalUsers } from '@/lib/db/schema/portal'; import { documents } from '@/lib/db/schema/documents'; import { createAuditLog, type AuditMeta } from '@/lib/audit'; import { activeInterestsWhere } from '@/lib/services/active-interest'; +import { transferOwnershipTx } from '@/lib/services/yachts.service'; import { ConflictError, NotFoundError } from '@/lib/errors'; import type { ArchiveMetadata } from '@/lib/services/client-archive.service'; @@ -310,14 +311,14 @@ export async function restoreClientWithSelections(args: { // Apply auto-reversals. for (const r of dossier.autoReversible) { - await applyReversal(tx, r, args.clientId); + await applyReversal(tx, r, args.clientId, args.meta.userId); autoReversed += 1; } // Apply opted-in prompts. for (const r of dossier.reversibleWithPrompt) { if (!opted.has(r.id)) continue; - await applyReversal(tx, r, args.clientId); + await applyReversal(tx, r, args.clientId, args.meta.userId); promptedReversed += 1; } @@ -357,7 +358,12 @@ export async function restoreClientWithSelections(args: { }; } -async function applyReversal(tx: Tx, r: RestoreReversal, clientId: string): Promise { +async function applyReversal( + tx: Tx, + r: RestoreReversal, + clientId: string, + actorUserId: string, +): Promise { switch (r.kind) { case 'berth_released': { // Re-link the berth to whichever interest originally owned it @@ -397,11 +403,18 @@ async function applyReversal(tx: Tx, r: RestoreReversal, clientId: string): Prom break; } case 'yacht_transferred': { - // Transfer back to the restored client. - await tx - .update(yachts) - .set({ currentOwnerType: 'client', currentOwnerId: clientId }) - .where(eq(yachts.id, r.refId)); + // Transfer back to the restored client through the shared + // ledger-aware helper: closes the open yacht_ownership_history row + // (whoever the archive transfer pointed it at) and opens a fresh one + // for the restored client, keeping the ledger and denorm columns in + // sync. Writing only the denorm columns (the prior bug) re-corrupted + // the history on every restore. + await transferOwnershipTx(tx as unknown as typeof db, { + yachtId: r.refId, + newOwner: { type: 'client', id: clientId }, + transferReason: 'Smart-restore: ownership returned to restored client', + createdBy: actorUserId, + }); break; } case 'yacht_marked_sold_away': diff --git a/src/lib/services/yachts.service.ts b/src/lib/services/yachts.service.ts index 6c227703..e8fc5fd4 100644 --- a/src/lib/services/yachts.service.ts +++ b/src/lib/services/yachts.service.ts @@ -219,6 +219,72 @@ export async function archiveYacht(id: string, portId: string, meta: AuditMeta) emitToRoom(`port:${portId}`, 'yacht:archived', { yachtId: id }); } +/** + * Transaction-aware ownership transfer. Performs the FULL ledger move — + * closes the open `yacht_ownership_history` row, opens a new one for the + * new owner, and updates the yacht's denormalized current-owner columns — + * so the history ledger and the denormalized columns never drift apart. + * + * Use this from any flow that moves a yacht to a new owner inside a + * transaction (smart-archive / restore included). The public + * `transferOwnership` wraps this in its own tx + audit/socket emissions. + * + * NOTE: callers are responsible for any same-owner / owner-exists + * validation they need; this helper intentionally does only the ledger + * write so it stays composable. `effectiveDate` defaults to now() when + * omitted (archive/restore have no operator-chosen date). + */ +export async function transferOwnershipTx( + tx: typeof db, + args: { + yachtId: string; + newOwner: { type: 'client' | 'company'; id: string }; + effectiveDate?: Date; + transferReason?: string | null; + transferNotes?: string | null; + createdBy: string; + }, +): Promise { + const effectiveDate = args.effectiveDate ?? new Date(); + + // Close the currently-active history row (endDate IS NULL → guarded by + // the idx_yoh_active partial unique index, so there's at most one). + await tx + .update(yachtOwnershipHistory) + .set({ endDate: effectiveDate }) + .where( + and( + eq(yachtOwnershipHistory.yachtId, args.yachtId), + sql`${yachtOwnershipHistory.endDate} IS NULL`, + ), + ); + + // Open the new active row for the incoming owner. + await tx.insert(yachtOwnershipHistory).values({ + yachtId: args.yachtId, + ownerType: args.newOwner.type, + ownerId: args.newOwner.id, + startDate: effectiveDate, + endDate: null, + transferReason: args.transferReason ?? null, + transferNotes: args.transferNotes ?? null, + createdBy: args.createdBy, + }); + + // Update denormalized current-owner columns to match the ledger head. + const [updated] = await tx + .update(yachts) + .set({ + currentOwnerType: args.newOwner.type, + currentOwnerId: args.newOwner.id, + updatedAt: new Date(), + }) + .where(eq(yachts.id, args.yachtId)) + .returning(); + + return updated!; +} + export async function transferOwnership( yachtId: string, portId: string, @@ -271,40 +337,18 @@ export async function transferOwnership( resolveOwnerName(data.newOwner.type, data.newOwner.id), ]); - // Close the currently-active history row - await tx - .update(yachtOwnershipHistory) - .set({ endDate: data.effectiveDate }) - .where( - and( - eq(yachtOwnershipHistory.yachtId, yachtId), - sql`${yachtOwnershipHistory.endDate} IS NULL`, - ), - ); - - // Open new row - await tx.insert(yachtOwnershipHistory).values({ + // Close the open history row, open the new one, and sync the + // denormalized columns — all via the shared tx-aware helper so the + // ledger invariant holds for every transfer pathway. + const updated = await transferOwnershipTx(tx, { yachtId, - ownerType: data.newOwner.type, - ownerId: data.newOwner.id, - startDate: data.effectiveDate, - endDate: null, + newOwner: data.newOwner, + effectiveDate: data.effectiveDate, transferReason: data.transferReason ?? null, transferNotes: data.transferNotes ?? null, createdBy: meta.userId, }); - // Update denormalized current-owner columns - const [updated] = await tx - .update(yachts) - .set({ - currentOwnerType: data.newOwner.type, - currentOwnerId: data.newOwner.id, - updatedAt: new Date(), - }) - .where(eq(yachts.id, yachtId)) - .returning(); - // Audit log shape designed for the EntityActivityFeed sentence // formatter: a discrete `transfer` action + human-readable owner // names render as "Matt transferred owner from X to Y" instead of