From b966d8106dc893a9852f282651949d59c4f8901f Mon Sep 17 00:00:00 2001 From: Matt Date: Thu, 14 May 2026 14:53:58 +0200 Subject: [PATCH] feat(active-interest): canonical predicate + fix stale getHotDeals rank MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Extract activeInterestsWhere(portId) as the single source of truth for "active interest" SQL filtering: scoped + archived_at IS NULL + outcome IS NULL. Won deals are now CLOSED, not active — pre-2026-05-14 the dashboard used a permissive `outcome IS NULL OR outcome = 'won'` that double-counted won revenue against the in-flight pipeline. Locked in PRE-DEPLOY-PLAN § 1.1.2. Bonus catch: getHotDeals rank-CASE referenced the OLD 9-stage pipeline names ('completed', 'contract_signed', 'contract_sent', 'deposit_10pct', 'eoi_signed', 'eoi_sent', 'in_communication', 'details_sent'). Every row hit the ELSE 0 branch under the new 7-stage model, collapsing ordering to updatedAt only — the widget silently stopped surfacing "closest to closing". Rebuilt the rank ladder against the current canonical stages (enquiry → ... → contract). Tests: 2 unit tests assert the predicate's compiled SQL contains "archived_at" IS NULL + "outcome" IS NULL, and never the legacy 'won' literal. Remaining sweep targets queued for the next commit: - client-archive-dossier.service.ts - client-restore.service.ts - client-archive.service.ts - reminders.service.ts - berths.service.ts (recommender feasibility) - interests.service.ts - report-generators.ts Co-Authored-By: Claude Opus 4.7 (1M context) --- src/lib/services/active-interest.ts | 35 ++++++++++++++++++++ src/lib/services/dashboard.service.ts | 47 ++++++++++----------------- tests/unit/active-interest.test.ts | 37 +++++++++++++++++++++ 3 files changed, 90 insertions(+), 29 deletions(-) create mode 100644 src/lib/services/active-interest.ts create mode 100644 tests/unit/active-interest.test.ts diff --git a/src/lib/services/active-interest.ts b/src/lib/services/active-interest.ts new file mode 100644 index 00000000..2ee6a04b --- /dev/null +++ b/src/lib/services/active-interest.ts @@ -0,0 +1,35 @@ +import { and, eq, isNull, type SQL } from 'drizzle-orm'; + +import { interests } from '@/lib/db/schema/interests'; + +/** + * Canonical "active interest" predicate for SQL WHERE clauses. + * + * An interest is **active** when it is: + * - scoped to the given `portId`, + * - not soft-archived (`archived_at IS NULL`), and + * - not yet terminal (`outcome IS NULL` — i.e. not won, lost, or + * cancelled). + * + * "Won" deals are explicitly **not active** under this definition: a won + * deal is closed business, distinct from in-flight pipeline. The 7-stage + * pipeline carries deals through `contract` as the final motion stage; + * setting an `outcome` (won/lost/cancelled) terminates the deal and + * removes it from the active pool everywhere KPIs, kanban, hot deals, + * pipeline value, and revenue forecast are computed. + * + * Pre-2026-05-14 the dashboard used a broader `outcome IS NULL OR + * outcome = 'won'` predicate; the new pipeline overhaul collapsed + * intermediate completion states into explicit stages, making the + * stricter definition the correct one across every surface. + * + * Always combine with the table's own filters; this helper is a leaf + * predicate, not a complete WHERE. + */ +export function activeInterestsWhere(portId: string): SQL { + return and( + eq(interests.portId, portId), + isNull(interests.archivedAt), + isNull(interests.outcome), + ) as SQL; +} diff --git a/src/lib/services/dashboard.service.ts b/src/lib/services/dashboard.service.ts index 12aae778..76ba913d 100644 --- a/src/lib/services/dashboard.service.ts +++ b/src/lib/services/dashboard.service.ts @@ -11,14 +11,10 @@ import { documents } from '@/lib/db/schema/documents'; import { reminders } from '@/lib/db/schema/operations'; import { systemSettings, auditLogs } from '@/lib/db/schema/system'; import { PIPELINE_STAGES, STAGE_WEIGHTS } from '@/lib/constants'; +import { activeInterestsWhere } from '@/lib/services/active-interest'; const DEFAULT_PIPELINE_WEIGHTS: Record = STAGE_WEIGHTS; -// "Active" = not archived AND not closed as lost/cancelled. Won interests are -// still counted because they represent revenue. Used everywhere KPIs say -// "active interests" or "pipeline value". -const isActiveInterest = sql`(${interests.outcome} IS NULL OR ${interests.outcome} = 'won')`; - // ─── KPIs ───────────────────────────────────────────────────────────────────── export async function getKpis(portId: string) { @@ -30,7 +26,7 @@ export async function getKpis(portId: string) { const [activeInterestsRow] = await db .select({ value: count() }) .from(interests) - .where(and(eq(interests.portId, portId), isNull(interests.archivedAt), isActiveInterest)); + .where(activeInterestsWhere(portId)); // Pipeline value: SUM each berth's price ONCE regardless of how many active // interests reference it. A berth with multiple interests would otherwise be @@ -44,7 +40,7 @@ export async function getKpis(portId: string) { and(eq(interestBerths.interestId, interests.id), eq(interestBerths.isPrimary, true)), ) .innerJoin(berths, eq(interestBerths.berthId, berths.id)) - .where(and(eq(interests.portId, portId), isNull(interests.archivedAt), isActiveInterest)); + .where(activeInterestsWhere(portId)); const pipelineValueUsd = pipelineRows.reduce((acc, row) => { return acc + (row.price ? parseFloat(String(row.price)) : 0); @@ -79,7 +75,7 @@ export async function getPipelineCounts(portId: string) { count: sql`count(*)::int`, }) .from(interests) - .where(and(eq(interests.portId, portId), isNull(interests.archivedAt), isActiveInterest)) + .where(activeInterestsWhere(portId)) .groupBy(interests.pipelineStage); const countsByStage = Object.fromEntries(rows.map((r) => [r.stage, r.count])); @@ -128,7 +124,7 @@ export async function getRevenueForecast(portId: string) { and(eq(interestBerths.interestId, interests.id), eq(interestBerths.isPrimary, true)), ) .innerJoin(berths, eq(interestBerths.berthId, berths.id)) - .where(and(eq(interests.portId, portId), isNull(interests.archivedAt), isActiveInterest)); + .where(activeInterestsWhere(portId)); // Build stageBreakdown const stageMap: Record = {}; @@ -194,20 +190,19 @@ export async function getBerthStatusDistribution(portId: string) { * dashboard without making them open the pipeline board. */ export async function getHotDeals(portId: string, limit = 5) { - // Stage rank: bigger = closer to closing. - // Reporting audit caught two stage-name typos: 'in_comms' and - // 'deposit_10' don't exist in the DB enum — canonical values are - // 'in_communication' and 'deposit_10pct'. Those two stages were - // silently collapsing to the ELSE 0 branch. + // Stage rank: bigger = closer to closing. Mirrors the 7-stage pipeline + // shipped 2026-05-14 (pipeline-refactor wave). Nurturing is a holding + // pen below qualified — supply-constrained ports flip deals there when + // they can't progress. Won/lost/cancelled outcomes are filtered out via + // `outcome IS NULL` below, so they don't need a rank slot. const rank = sql`CASE ${interests.pipelineStage} - WHEN 'completed' THEN 8 - WHEN 'contract_signed' THEN 7 - WHEN 'contract_sent' THEN 6 - WHEN 'deposit_10pct' THEN 5 - WHEN 'eoi_signed' THEN 4 - WHEN 'eoi_sent' THEN 3 - WHEN 'in_communication' THEN 2 - WHEN 'details_sent' THEN 1 + WHEN 'contract' THEN 7 + WHEN 'deposit_paid' THEN 6 + WHEN 'reservation' THEN 5 + WHEN 'eoi' THEN 4 + WHEN 'qualified' THEN 3 + WHEN 'nurturing' THEN 2 + WHEN 'enquiry' THEN 1 ELSE 0 END`; @@ -228,13 +223,7 @@ export async function getHotDeals(portId: string, limit = 5) { and(eq(interestBerths.interestId, interests.id), eq(interestBerths.isPrimary, true)), ) .leftJoin(berths, eq(interestBerths.berthId, berths.id)) - .where( - and( - eq(interests.portId, portId), - isNull(interests.archivedAt), - isNull(interests.outcome), // exclude won/lost — they're not "closing" - ), - ) + .where(activeInterestsWhere(portId)) .orderBy(desc(rank), desc(interests.updatedAt)) .limit(limit); diff --git a/tests/unit/active-interest.test.ts b/tests/unit/active-interest.test.ts new file mode 100644 index 00000000..570babb7 --- /dev/null +++ b/tests/unit/active-interest.test.ts @@ -0,0 +1,37 @@ +import { describe, expect, it } from 'vitest'; +import { PgDialect } from 'drizzle-orm/pg-core'; + +import { activeInterestsWhere } from '@/lib/services/active-interest'; + +/** + * Locks in the canonical active-interest predicate per + * `docs/PRE-DEPLOY-PLAN.md` § 1.1.2 / commit f86f511. The whole pipeline + * report + recommender + reminder + restore + dossier surface depends on + * this predicate matching the same set of rows on every read; if it + * drifts, "active interest" stops meaning the same thing across + * dashboard / kanban / hot deals / PDF reports. + */ +describe('activeInterestsWhere', () => { + const PORT_ID = '11111111-1111-1111-1111-111111111111'; + const dialect = new PgDialect(); + + it('compiles to: scoped to portId AND not archived AND no terminal outcome', () => { + const fragment = activeInterestsWhere(PORT_ID); + const compiled = dialect.sqlToQuery(fragment); + + expect(compiled.sql).toContain('"port_id"'); + expect(compiled.sql).toContain('"archived_at"'); + expect(compiled.sql).toContain('"outcome"'); + expect(compiled.sql).toMatch(/"archived_at"\s+is\s+null/i); + expect(compiled.sql).toMatch(/"outcome"\s+is\s+null/i); + expect(compiled.params).toContain(PORT_ID); + }); + + it('does NOT include the legacy `outcome = "won"` permissive branch', () => { + const fragment = activeInterestsWhere(PORT_ID); + const compiled = dialect.sqlToQuery(fragment); + + expect(compiled.sql.toLowerCase()).not.toContain("'won'"); + expect(compiled.params).not.toContain('won'); + }); +});