feat(active-interest): canonical predicate + fix stale getHotDeals rank
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) <noreply@anthropic.com>
This commit is contained in:
35
src/lib/services/active-interest.ts
Normal file
35
src/lib/services/active-interest.ts
Normal file
@@ -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;
|
||||||
|
}
|
||||||
@@ -11,14 +11,10 @@ import { documents } from '@/lib/db/schema/documents';
|
|||||||
import { reminders } from '@/lib/db/schema/operations';
|
import { reminders } from '@/lib/db/schema/operations';
|
||||||
import { systemSettings, auditLogs } from '@/lib/db/schema/system';
|
import { systemSettings, auditLogs } from '@/lib/db/schema/system';
|
||||||
import { PIPELINE_STAGES, STAGE_WEIGHTS } from '@/lib/constants';
|
import { PIPELINE_STAGES, STAGE_WEIGHTS } from '@/lib/constants';
|
||||||
|
import { activeInterestsWhere } from '@/lib/services/active-interest';
|
||||||
|
|
||||||
const DEFAULT_PIPELINE_WEIGHTS: Record<string, number> = STAGE_WEIGHTS;
|
const DEFAULT_PIPELINE_WEIGHTS: Record<string, number> = 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 ─────────────────────────────────────────────────────────────────────
|
// ─── KPIs ─────────────────────────────────────────────────────────────────────
|
||||||
|
|
||||||
export async function getKpis(portId: string) {
|
export async function getKpis(portId: string) {
|
||||||
@@ -30,7 +26,7 @@ export async function getKpis(portId: string) {
|
|||||||
const [activeInterestsRow] = await db
|
const [activeInterestsRow] = await db
|
||||||
.select({ value: count() })
|
.select({ value: count() })
|
||||||
.from(interests)
|
.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
|
// Pipeline value: SUM each berth's price ONCE regardless of how many active
|
||||||
// interests reference it. A berth with multiple interests would otherwise be
|
// 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)),
|
and(eq(interestBerths.interestId, interests.id), eq(interestBerths.isPrimary, true)),
|
||||||
)
|
)
|
||||||
.innerJoin(berths, eq(interestBerths.berthId, berths.id))
|
.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) => {
|
const pipelineValueUsd = pipelineRows.reduce((acc, row) => {
|
||||||
return acc + (row.price ? parseFloat(String(row.price)) : 0);
|
return acc + (row.price ? parseFloat(String(row.price)) : 0);
|
||||||
@@ -79,7 +75,7 @@ export async function getPipelineCounts(portId: string) {
|
|||||||
count: sql<number>`count(*)::int`,
|
count: sql<number>`count(*)::int`,
|
||||||
})
|
})
|
||||||
.from(interests)
|
.from(interests)
|
||||||
.where(and(eq(interests.portId, portId), isNull(interests.archivedAt), isActiveInterest))
|
.where(activeInterestsWhere(portId))
|
||||||
.groupBy(interests.pipelineStage);
|
.groupBy(interests.pipelineStage);
|
||||||
|
|
||||||
const countsByStage = Object.fromEntries(rows.map((r) => [r.stage, r.count]));
|
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)),
|
and(eq(interestBerths.interestId, interests.id), eq(interestBerths.isPrimary, true)),
|
||||||
)
|
)
|
||||||
.innerJoin(berths, eq(interestBerths.berthId, berths.id))
|
.innerJoin(berths, eq(interestBerths.berthId, berths.id))
|
||||||
.where(and(eq(interests.portId, portId), isNull(interests.archivedAt), isActiveInterest));
|
.where(activeInterestsWhere(portId));
|
||||||
|
|
||||||
// Build stageBreakdown
|
// Build stageBreakdown
|
||||||
const stageMap: Record<string, { count: number; weightedValue: number }> = {};
|
const stageMap: Record<string, { count: number; weightedValue: number }> = {};
|
||||||
@@ -194,20 +190,19 @@ export async function getBerthStatusDistribution(portId: string) {
|
|||||||
* dashboard without making them open the pipeline board.
|
* dashboard without making them open the pipeline board.
|
||||||
*/
|
*/
|
||||||
export async function getHotDeals(portId: string, limit = 5) {
|
export async function getHotDeals(portId: string, limit = 5) {
|
||||||
// Stage rank: bigger = closer to closing.
|
// Stage rank: bigger = closer to closing. Mirrors the 7-stage pipeline
|
||||||
// Reporting audit caught two stage-name typos: 'in_comms' and
|
// shipped 2026-05-14 (pipeline-refactor wave). Nurturing is a holding
|
||||||
// 'deposit_10' don't exist in the DB enum — canonical values are
|
// pen below qualified — supply-constrained ports flip deals there when
|
||||||
// 'in_communication' and 'deposit_10pct'. Those two stages were
|
// they can't progress. Won/lost/cancelled outcomes are filtered out via
|
||||||
// silently collapsing to the ELSE 0 branch.
|
// `outcome IS NULL` below, so they don't need a rank slot.
|
||||||
const rank = sql<number>`CASE ${interests.pipelineStage}
|
const rank = sql<number>`CASE ${interests.pipelineStage}
|
||||||
WHEN 'completed' THEN 8
|
WHEN 'contract' THEN 7
|
||||||
WHEN 'contract_signed' THEN 7
|
WHEN 'deposit_paid' THEN 6
|
||||||
WHEN 'contract_sent' THEN 6
|
WHEN 'reservation' THEN 5
|
||||||
WHEN 'deposit_10pct' THEN 5
|
WHEN 'eoi' THEN 4
|
||||||
WHEN 'eoi_signed' THEN 4
|
WHEN 'qualified' THEN 3
|
||||||
WHEN 'eoi_sent' THEN 3
|
WHEN 'nurturing' THEN 2
|
||||||
WHEN 'in_communication' THEN 2
|
WHEN 'enquiry' THEN 1
|
||||||
WHEN 'details_sent' THEN 1
|
|
||||||
ELSE 0
|
ELSE 0
|
||||||
END`;
|
END`;
|
||||||
|
|
||||||
@@ -228,13 +223,7 @@ export async function getHotDeals(portId: string, limit = 5) {
|
|||||||
and(eq(interestBerths.interestId, interests.id), eq(interestBerths.isPrimary, true)),
|
and(eq(interestBerths.interestId, interests.id), eq(interestBerths.isPrimary, true)),
|
||||||
)
|
)
|
||||||
.leftJoin(berths, eq(interestBerths.berthId, berths.id))
|
.leftJoin(berths, eq(interestBerths.berthId, berths.id))
|
||||||
.where(
|
.where(activeInterestsWhere(portId))
|
||||||
and(
|
|
||||||
eq(interests.portId, portId),
|
|
||||||
isNull(interests.archivedAt),
|
|
||||||
isNull(interests.outcome), // exclude won/lost — they're not "closing"
|
|
||||||
),
|
|
||||||
)
|
|
||||||
.orderBy(desc(rank), desc(interests.updatedAt))
|
.orderBy(desc(rank), desc(interests.updatedAt))
|
||||||
.limit(limit);
|
.limit(limit);
|
||||||
|
|
||||||
|
|||||||
37
tests/unit/active-interest.test.ts
Normal file
37
tests/unit/active-interest.test.ts
Normal file
@@ -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');
|
||||||
|
});
|
||||||
|
});
|
||||||
Reference in New Issue
Block a user