From 81d4e64f692f24e055769d47c90516a9a33390fd Mon Sep 17 00:00:00 2001 From: Matt Date: Thu, 14 May 2026 15:04:13 +0200 Subject: [PATCH] refactor(interests): drop pipelineStage='completed' sentinel convention MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `outcome` is the canonical terminal-state signal. Pre-2026-05-14 `setInterestOutcome` also forced `pipelineStage='completed'` (a value outside the 7-stage canon) which: - broke `safeStage()` (silently coerced to 'enquiry' downstream) - prevented analytics from answering "what stage was the deal at when it closed?" because every closed deal looked identical - forced belt-and-suspenders filters everywhere ('outcome=won' AND 'pipeline_stage=completed') that became redundant after migration 0062 Changes: - `setInterestOutcome` no longer touches pipelineStage. Deal stays at whatever stage it was on when the outcome was recorded; outcome is the terminal signal. Audit log + websocket emit now carry `stageAtOutcome` instead of the stale `oldStage`. - `clearInterestOutcome` smarter reopen-stage logic: if current stage is the legacy 'completed' sentinel (pre-existing rows from before this commit), default to 'qualified'. Otherwise preserve the stage the deal was at, so reopening drops the rep back where they were. Explicit data.reopenStage still wins. - `/api/v1/admin/dashboard-stats` route reworked: per-stage breakdown now filters `outcome IS NULL` (only active rows count per stage); `closedTotal` derives from a new `outcome IS NOT NULL` count query; `completed30d` switches from `pipelineStage='completed' AND updatedAt` to `outcome IS NOT NULL AND outcomeAt` (avoids long-closed deals leaking into the window on unrelated edits). - `berth-interests-tab.tsx` "active" filter switches from `pipelineStage !== 'completed'` to `!outcome && !archivedAt` — the legacy check stopped matching post-refactor. - Socket event type `interest:outcomeSet` renames `oldStage` → `stageAtOutcome` with a doc-comment explaining the semantics shift. PIPELINE_STAGES canon is now the only valid pipeline_stage value range for newly-set outcomes. Legacy rows still carry 'completed' until they naturally churn through reopen + re-close, at which point they enter the new convention. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/app/api/v1/admin/dashboard-stats/route.ts | 39 ++++++++++++++--- src/components/berths/berth-interests-tab.tsx | 6 ++- src/lib/services/interests.service.ts | 43 +++++++++++-------- src/lib/socket/events.ts | 6 ++- 4 files changed, 68 insertions(+), 26 deletions(-) diff --git a/src/app/api/v1/admin/dashboard-stats/route.ts b/src/app/api/v1/admin/dashboard-stats/route.ts index 34dfb905..bec9e32f 100644 --- a/src/app/api/v1/admin/dashboard-stats/route.ts +++ b/src/app/api/v1/admin/dashboard-stats/route.ts @@ -1,5 +1,5 @@ import { NextResponse } from 'next/server'; -import { and, eq, isNull, gte, sql } from 'drizzle-orm'; +import { and, eq, isNotNull, isNull, gte, sql } from 'drizzle-orm'; import { withAuth, withPermission } from '@/lib/api/helpers'; import { db } from '@/lib/db'; @@ -18,13 +18,26 @@ export const GET = withAuth( const thirtyDaysAgo = new Date(Date.now() - 30 * 24 * 60 * 60 * 1000); const [pipelineRows, berthStatusRows, totals, recent] = await Promise.all([ + // Only active interests count toward per-stage breakdowns; + // terminal (outcome-set) interests are tracked separately via + // `closedTotal` below. Pre-2026-05-14 cleanup, terminal rows + // were grouped under the sentinel pipeline_stage='completed' + // bucket; the new convention leaves the stage where it was + // when the outcome was set, so we must filter by `outcome IS + // NULL` explicitly to keep the kanban breakdown honest. db .select({ stage: interests.pipelineStage, count: sql`count(*)::int`, }) .from(interests) - .where(and(eq(interests.portId, portId), isNull(interests.archivedAt))) + .where( + and( + eq(interests.portId, portId), + isNull(interests.archivedAt), + isNull(interests.outcome), + ), + ) .groupBy(interests.pipelineStage), db @@ -49,6 +62,18 @@ export const GET = withAuth( .select({ count: sql`count(*)::int` }) .from(berths) .where(eq(berths.portId, portId)), + // closedTotal: all-time count of terminal (outcome-set) + // non-archived interests. Drives the conversion-rate tile. + db + .select({ count: sql`count(*)::int` }) + .from(interests) + .where( + and( + eq(interests.portId, portId), + isNull(interests.archivedAt), + isNotNull(interests.outcome), + ), + ), ]), Promise.all([ @@ -61,14 +86,18 @@ export const GET = withAuth( gte(websiteSubmissions.receivedAt, sevenDaysAgo), ), ), + // "completed30d" = interests that hit a terminal outcome in + // the last 30 days (any outcome — won, lost, or cancelled). + // Use `outcome_at` not `updated_at` so unrelated edits to a + // long-closed deal don't drag it back into the window. db .select({ count: sql`count(*)::int` }) .from(interests) .where( and( eq(interests.portId, portId), - eq(interests.pipelineStage, 'completed'), - gte(interests.updatedAt, thirtyDaysAgo), + isNotNull(interests.outcome), + gte(interests.outcomeAt, thirtyDaysAgo), ), ), ]), @@ -90,10 +119,10 @@ export const GET = withAuth( const totalClients = totals[0][0]?.count ?? 0; const totalInterests = totals[1][0]?.count ?? 0; const totalBerths = totals[2][0]?.count ?? 0; + const closedTotal = totals[3][0]?.count ?? 0; const newInquiries7d = recent[0][0]?.count ?? 0; const completed30d = recent[1][0]?.count ?? 0; - const closedTotal = pipeline['completed'] ?? 0; const openTotal = totalInterests - closedTotal; const conversionPct = totalInterests > 0 ? Math.round((closedTotal / totalInterests) * 100) : 0; diff --git a/src/components/berths/berth-interests-tab.tsx b/src/components/berths/berth-interests-tab.tsx index 248eb93e..cf90ff53 100644 --- a/src/components/berths/berth-interests-tab.tsx +++ b/src/components/berths/berth-interests-tab.tsx @@ -75,7 +75,11 @@ export function BerthInterestsTab({ berthId }: BerthInterestsTabProps) { const rows = useMemo(() => { const all = data?.data ?? []; const filtered = all.filter((i) => { - if (stage === 'active') return i.pipelineStage !== 'completed' && !i.archivedAt; + // 2026-05-14 sentinel-stage cleanup: an interest is "active" when + // it has no terminal outcome and isn't archived. The legacy + // `pipelineStage !== 'completed'` check stopped working after the + // 7-stage refactor stopped writing 'completed' for terminal rows. + if (stage === 'active') return !i.outcome && !i.archivedAt; if (stage === 'lost') return Boolean(i.archivedAt); return true; }); diff --git a/src/lib/services/interests.service.ts b/src/lib/services/interests.service.ts index ad1cb4b3..c2fd5cf1 100644 --- a/src/lib/services/interests.service.ts +++ b/src/lib/services/interests.service.ts @@ -986,13 +986,13 @@ export async function advanceStageIfBehind( // ─── Set Outcome (Won / Lost) ──────────────────────────────────────────────── // -// Records a terminal outcome for the interest and moves the pipelineStage to -// `completed` so the funnel/kanban reflect the final state. The outcome -// distinguishes won deals (they made it through) from lost variants - funnel -// math and reports key off the `outcome` column to compute true conversion. -// -// Both the stage advance and the outcome write happen in one transaction so -// the timeline doesn't end up showing one without the other. +// Records a terminal outcome for the interest. The `outcome` column is the +// canonical terminal-state signal; `pipelineStage` stays where it was so +// reports can answer "what stage was this deal at when it closed?". Prior to +// 2026-05-14 this method forced pipelineStage='completed' — a sentinel +// outside the 7-stage canon that broke type narrowing + downstream stage +// label lookups. Active-interest queries filter by `outcome IS NULL` so +// the rep-facing kanban still hides closed deals. export async function setInterestOutcome( id: string, portId: string, @@ -1005,7 +1005,7 @@ export async function setInterestOutcome( if (!existing) throw new NotFoundError('Interest'); const oldOutcome = existing.outcome; - const oldStage = existing.pipelineStage; + const stageAtOutcome = existing.pipelineStage; const now = new Date(); await db @@ -1014,7 +1014,6 @@ export async function setInterestOutcome( outcome: data.outcome, outcomeReason: data.reason ?? null, outcomeAt: now, - pipelineStage: 'completed', updatedAt: now, }) .where(and(eq(interests.id, id), eq(interests.portId, portId))); @@ -1025,9 +1024,9 @@ export async function setInterestOutcome( action: 'update', entityType: 'interest', entityId: id, - oldValue: { outcome: oldOutcome, pipelineStage: oldStage }, - newValue: { outcome: data.outcome, pipelineStage: 'completed', reason: data.reason }, - metadata: { type: 'outcome_set' }, + oldValue: { outcome: oldOutcome, pipelineStage: stageAtOutcome }, + newValue: { outcome: data.outcome, pipelineStage: stageAtOutcome, reason: data.reason }, + metadata: { type: 'outcome_set', stageAtOutcome }, ipAddress: meta.ipAddress, userAgent: meta.userAgent, }); @@ -1035,7 +1034,7 @@ export async function setInterestOutcome( emitToRoom(`port:${portId}`, 'interest:outcomeSet', { interestId: id, outcome: data.outcome, - oldStage, + stageAtOutcome, }); // G-C4: fire interest_completed berth-rule for any non-null outcome @@ -1063,12 +1062,18 @@ export async function clearInterestOutcome( throw new ValidationError('Interest has no outcome to clear'); } - // Default reopen stage = qualified (closest analog of the legacy - // 'in_communication' under the 7-stage pipeline; rep can override - // via data.reopenStage). The legacy default was silently invalid - // post-migration 0062 — reopened interests landed in a non-canonical - // stage that fell through safeStage() to 'enquiry'. - const reopenStage = data.reopenStage ?? 'qualified'; + // Reopen-stage logic: + // - If the caller passed `data.reopenStage`, honor it (rep override). + // - Else if the current stage is the legacy 'completed' sentinel, + // default to 'qualified' (closest analog of the pre-refactor + // 'in_communication' which would have lived there). + // - Else preserve the current stage — post-refactor setOutcome stops + // touching pipelineStage, so the deal already knows where it was + // when the rep closed it. Reopening should drop the rep back into + // that same column on the kanban. + const reopenStage = + data.reopenStage ?? + (existing.pipelineStage === 'completed' ? 'qualified' : existing.pipelineStage); const now = new Date(); await db .update(interests) diff --git a/src/lib/socket/events.ts b/src/lib/socket/events.ts index 277524fa..6c6210a2 100644 --- a/src/lib/socket/events.ts +++ b/src/lib/socket/events.ts @@ -57,7 +57,11 @@ export interface ServerToClientEvents { 'interest:outcomeSet': (payload: { interestId: string; outcome: string; - oldStage: string; + /** Stage the deal was on when the outcome was recorded. Renamed + * from `oldStage` after the 2026-05-14 sentinel-stage cleanup — + * the value is now the actual stage where the deal closed, not + * a stale "what was it before we set 'completed'" marker. */ + stageAtOutcome: string; }) => void; 'interest:outcomeCleared': (payload: { interestId: string }) => void; 'interest:noteAdded': (payload: {