diff --git a/docs/launch-readiness.md b/docs/launch-readiness.md index 2676703b..4bfaf41e 100644 --- a/docs/launch-readiness.md +++ b/docs/launch-readiness.md @@ -77,8 +77,20 @@ everything else is post-launch polish unless promoted. #### Cross-cutting capabilities (apply to every report) -- ❌ **Period comparison toggle** — "this period vs prior period" delta - arrows on KPI cards. Spec calls for it on every report. Not on any. +- ⚠️ **Period comparison toggle** — "this period vs prior period" delta + arrows on KPI cards. **Sales: SHIPPED locally (2026-05-31)** — a + "Compare to prior period" toggle in the header computes an + equal-length preceding window (`previousPeriodBounds`), the API + recomputes KPIs for that window behind `?compare=1`, and the five + window-derived tiles (Won, Lost, Win rate, Avg time-to-close, New + leads) render colour-correct "vs prior" deltas. Point-in-time tiles + (Active interests, Pipeline value) intentionally have no delta. + Persisted in the saved-template config. TDD'd: + `previousPeriodBounds` + `computeSalesKpiComparison` unit tests. + Operational already rendered period-start deltas. **Still open:** the + spec's "on every report" — Operational uses a different + "vs period start" baseline; reconcile the two semantics if a single + consistent comparison is wanted. - ❌ **Rep multi-select filter** — exists implicitly via the single-rep leaderboard collapse, but no explicit multi-select dropdown. - ❌ **Source multi-select filter** — Sales has lead-category + outcome diff --git a/src/app/api/v1/reports/sales/route.ts b/src/app/api/v1/reports/sales/route.ts index 1165510e..086d0a32 100644 --- a/src/app/api/v1/reports/sales/route.ts +++ b/src/app/api/v1/reports/sales/route.ts @@ -4,6 +4,8 @@ import { z } from 'zod'; import { withAuth, withPermission } from '@/lib/api/helpers'; import { errorResponse } from '@/lib/errors'; import { PIPELINE_STAGES, type PipelineStage } from '@/lib/constants'; +import { previousPeriodBounds } from '@/lib/analytics/range'; +import { computeSalesKpiComparison } from '@/lib/services/reports/sales-comparison'; import { getSalesKpis, getPipelineFunnel, @@ -50,6 +52,8 @@ const querySchema = z.object({ stage: z.string().optional(), leadCategory: z.string().optional(), outcome: z.string().optional(), + // "1" / "true" enables the prior-period comparison (adds per-KPI deltas). + compare: z.string().optional(), }); /** @@ -86,14 +90,17 @@ export const GET = withAuth( withPermission('reports', 'view_dashboard', async (req: NextRequest, ctx) => { try { const params = req.nextUrl.searchParams; - const { from, to, stage, leadCategory, outcome } = querySchema.parse({ + const { from, to, stage, leadCategory, outcome, compare } = querySchema.parse({ from: params.get('from') ?? undefined, to: params.get('to') ?? undefined, stage: params.get('stage') ?? undefined, leadCategory: params.get('leadCategory') ?? undefined, outcome: params.get('outcome') ?? undefined, + compare: params.get('compare') ?? undefined, }); const range = resolveRange(from, to); + const compareEnabled = compare === '1' || compare === 'true'; + const priorBounds = compareEnabled ? previousPeriodBounds(range) : null; const filters: SalesFilters | undefined = (() => { const stages = parseCsv(stage, PIPELINE_STAGES); @@ -119,6 +126,7 @@ export const GET = withAuth( closingThisMonth, recentWins, lostReasonBreakdown, + priorKpis, ] = await Promise.all([ getSalesKpis(ctx.portId, range), getPipelineFunnel(ctx.portId), @@ -132,11 +140,27 @@ export const GET = withAuth( getClosingThisMonth(ctx.portId, filters), getRecentWins(ctx.portId, filters), getLostReasonBreakdown(ctx.portId, range, filters), + // Prior-window KPIs for the comparison toggle. Runs in parallel + // with the main batch (depends only on the derived priorBounds); + // resolves to null when the toggle is off so we pay nothing. + priorBounds ? getSalesKpis(ctx.portId, priorBounds) : Promise.resolve(null), ]); + const comparison = + priorKpis && priorBounds + ? { + deltas: computeSalesKpiComparison(kpis, priorKpis), + priorRange: { + from: priorBounds.from.toISOString(), + to: priorBounds.to.toISOString(), + }, + } + : null; + return NextResponse.json({ data: { kpis, + comparison, funnel, stageVelocity, winRateOverTime, diff --git a/src/components/reports/sales/sales-report-client.tsx b/src/components/reports/sales/sales-report-client.tsx index 050ca743..49a7495f 100644 --- a/src/components/reports/sales/sales-report-client.tsx +++ b/src/components/reports/sales/sales-report-client.tsx @@ -3,9 +3,16 @@ import { useCallback, useMemo, useState } from 'react'; import { useSearchParams } from 'next/navigation'; import { useQuery } from '@tanstack/react-query'; -import { TrendingDown, TrendingUp } from 'lucide-react'; +import { + ArrowDownRight, + ArrowLeftRight, + ArrowUpRight, + TrendingDown, + TrendingUp, +} from 'lucide-react'; import { PageHeader } from '@/components/shared/page-header'; +import { Button } from '@/components/ui/button'; import { Card, CardContent, CardHeader, CardTitle } from '@/components/ui/card'; import { Skeleton } from '@/components/ui/skeleton'; import { Badge } from '@/components/ui/badge'; @@ -19,6 +26,7 @@ import { } from '@/components/shared/filter-bar'; import { rangeToBounds, type DateRange } from '@/lib/analytics/range'; import { apiFetch } from '@/lib/api/client'; +import { cn } from '@/lib/utils'; import { PIPELINE_STAGES, STAGE_LABELS, OUTCOME_LABELS, type PipelineStage } from '@/lib/constants'; import { formatMoney } from '@/lib/reports/format-currency'; import type { ReportPayload } from '@/lib/reports/types'; @@ -164,9 +172,26 @@ interface LostReasonRow { avgDaysFromFirstContactToLoss: number | null; } +// Per-KPI deltas for the prior-period comparison toggle. Only the +// window-derived tiles get a delta; the point-in-time tiles (Active +// interests, Pipeline value) have no prior-window analogue. +interface SalesKpiDeltas { + wonInWindow: number; + lostInWindow: number; + newLeadsInWindow: number; + winRate: number | null; + medianTimeToCloseDays: number | null; +} + +interface SalesComparison { + deltas: SalesKpiDeltas; + priorRange: { from: string; to: string }; +} + interface SalesReportPayload { data: { kpis: SalesKpis; + comparison: SalesComparison | null; funnel: FunnelRow[]; stageVelocity: StageVelocityRow[]; winRateOverTime: WinRateOverTime; @@ -226,6 +251,7 @@ interface SalesTemplateConfig extends Record { kind: 'sales'; range: DateRange; filters: FilterValues; + compare?: boolean; } export function SalesReportClient({ portSlug: _portSlug }: { portSlug: string }) { @@ -234,6 +260,7 @@ export function SalesReportClient({ portSlug: _portSlug }: { portSlug: string }) const [range, setRange] = useState('30d'); const [filterValues, setFilterValues] = useState({}); + const [compareEnabled, setCompareEnabled] = useState(false); const [activeTemplateId, setActiveTemplateId] = useState(initialTemplateId); // Wrap the user-driven setters so any view-state change clears the @@ -256,8 +283,8 @@ export function SalesReportClient({ portSlug: _portSlug }: { portSlug: string }) }, []); const currentConfig: SalesTemplateConfig = useMemo( - () => ({ kind: 'sales', range, filters: filterValues }), - [range, filterValues], + () => ({ kind: 'sales', range, filters: filterValues, compare: compareEnabled }), + [range, filterValues, compareEnabled], ); const handleApplyTemplate = useCallback((config: SalesTemplateConfig) => { @@ -265,6 +292,7 @@ export function SalesReportClient({ portSlug: _portSlug }: { portSlug: string }) // active-template badge, which the user-facing setters above do. if (config.range) setRange(config.range); setFilterValues(config.filters ?? {}); + setCompareEnabled(config.compare ?? false); }, []); const bounds = useMemo(() => rangeToBounds(range), [range]); @@ -281,15 +309,23 @@ export function SalesReportClient({ portSlug: _portSlug }: { portSlug: string }) }, [filterValues]); const query = useQuery({ - queryKey: ['reports', 'sales', bounds.from.toISOString(), bounds.to.toISOString(), filterQs], + queryKey: [ + 'reports', + 'sales', + bounds.from.toISOString(), + bounds.to.toISOString(), + filterQs, + compareEnabled, + ], queryFn: () => apiFetch( - `/api/v1/reports/sales?from=${encodeURIComponent(bounds.from.toISOString())}&to=${encodeURIComponent(bounds.to.toISOString())}${filterQs}`, + `/api/v1/reports/sales?from=${encodeURIComponent(bounds.from.toISOString())}&to=${encodeURIComponent(bounds.to.toISOString())}${filterQs}${compareEnabled ? '&compare=1' : ''}`, ), staleTime: 30_000, }); const kpis = query.data?.data.kpis; + const deltas = query.data?.data.comparison?.deltas ?? null; const funnel = query.data?.data.funnel ?? []; const stageVelocity = query.data?.data.stageVelocity ?? []; const winRateOverTime = query.data?.data.winRateOverTime ?? { @@ -529,6 +565,20 @@ export function SalesReportClient({ portSlug: _portSlug }: { portSlug: string }) actions={
+ kind="sales" currentConfig={currentConfig} @@ -561,11 +611,13 @@ export function SalesReportClient({ portSlug: _portSlug }: { portSlug: string }) label="Won in period" value={formatInt(kpis.wonInWindow)} valueTrend={kpis.wonInWindow > 0 ? 'positive' : 'neutral'} + delta={deltas ? buildKpiDelta(deltas.wonInWindow, 'up', formatInt) : null} /> 0 ? 'negative' : 'neutral'} + delta={deltas ? buildKpiDelta(deltas.lostInWindow, 'down', formatInt) : null} hint={ kpis.lossBreakdown.length > 0 ? kpis.lossBreakdown @@ -577,6 +629,11 @@ export function SalesReportClient({ portSlug: _portSlug }: { portSlug: string }) `${(abs * 100).toFixed(1)}pp`) + : null + } hint={kpis.winRate === null ? 'No closed deals in period' : 'Excludes cancellations'} /> `${Math.round(abs)}d`, + ) + : null + } hint={ kpis.medianTimeToCloseDays === null ? 'Need ≥3 won deals for a meaningful median' @@ -604,6 +670,7 @@ export function SalesReportClient({ portSlug: _portSlug }: { portSlug: string }) 0 ? kpis.newLeadsBySource @@ -759,14 +826,26 @@ export function SalesReportClient({ portSlug: _portSlug }: { portSlug: string }) // ─── KPI tile primitives ───────────────────────────────────────────────────── +interface KpiDeltaDisplay { + /** Pre-formatted magnitude with sign, e.g. "+3", "−2.0pp", "−1.5d". */ + formatted: string; + /** Colour semantics: positive = good (green), negative = bad (rose), + * neutral = no change (muted). Computed by the caller because "good" + * depends on the metric (more wins = good; longer time-to-close = bad). */ + tone: 'positive' | 'negative' | 'neutral'; +} + interface KpiCardProps { label: string; value: string; hint?: string; valueTrend?: 'positive' | 'negative' | 'neutral'; + /** Prior-period comparison delta. Rendered as a small "vs prior" line + * under the value when the comparison toggle is on. */ + delta?: KpiDeltaDisplay | null; } -function KpiCard({ label, value, hint, valueTrend = 'neutral' }: KpiCardProps) { +function KpiCard({ label, value, hint, valueTrend = 'neutral', delta }: KpiCardProps) { // Padding goes directly on the bare Card (skipping CardContent) // because CardContent ships with `p-4 pt-0 sm:p-6 sm:pt-0` for // use-with-CardHeader contexts. KPI tiles have no header, so any @@ -787,6 +866,26 @@ function KpiCard({ label, value, hint, valueTrend = 'neutral' }: KpiCardProps) { ) : null}
+ {delta ? ( +

+ {delta.tone === 'positive' ? ( + + ) : delta.tone === 'negative' ? ( + + ) : null} + {delta.formatted} + vs prior +

+ ) : null} {hint ? (

{hint}

) : null} @@ -825,6 +924,26 @@ function formatPercent(fraction: number): string { return `${Math.round(fraction * 1000) / 10}%`; } +/** + * Build the prior-period delta display for a KPI tile. `goodDirection` + * encodes which way is good for THIS metric (more wins = up-is-good; + * shorter time-to-close = down-is-good) so the colour matches intuition + * regardless of the raw sign. Returns null when the metric had no value + * in one of the two periods (e.g. win rate with no closed deals). + */ +function buildKpiDelta( + value: number | null, + goodDirection: 'up' | 'down', + format: (abs: number) => string, +): KpiDeltaDisplay | null { + if (value === null) return null; + if (value === 0) return { formatted: 'no change', tone: 'neutral' }; + const sign = value > 0 ? '+' : '−'; + const formatted = `${sign}${format(Math.abs(value))}`; + const isGood = goodDirection === 'up' ? value > 0 : value < 0; + return { formatted, tone: isGood ? 'positive' : 'negative' }; +} + // Money helpers come from the shared module — `formatMoney` for KPI // tile readability, `formatMoneyCompact` for tight dense tables. diff --git a/src/lib/analytics/range.ts b/src/lib/analytics/range.ts index d45e55f5..3b2fe575 100644 --- a/src/lib/analytics/range.ts +++ b/src/lib/analytics/range.ts @@ -100,3 +100,23 @@ export function rangeSpanDays(range: DateRange): number { } return rangeToDays(range); } + +/** + * Given a concrete {from, to} window, return the equal-length window + * immediately preceding it — used for "this period vs prior period" + * comparison on report KPIs. The prior window is contiguous (its `to` + * equals the current window's `from`) and exactly the same duration, so + * a 30-day current window compares against the prior 30 days. + * + * Pure: takes Dates in, returns fresh Dates, never mutates the input. + */ +export function previousPeriodBounds(bounds: { from: Date; to: Date }): { + from: Date; + to: Date; +} { + const lengthMs = bounds.to.getTime() - bounds.from.getTime(); + return { + from: new Date(bounds.from.getTime() - lengthMs), + to: new Date(bounds.from.getTime()), + }; +} diff --git a/src/lib/services/reports/sales-comparison.ts b/src/lib/services/reports/sales-comparison.ts new file mode 100644 index 00000000..66eb28ca --- /dev/null +++ b/src/lib/services/reports/sales-comparison.ts @@ -0,0 +1,47 @@ +/** + * Period-comparison diffing for the Sales report KPI strip. + * + * DB-free + pure so it unit-tests without a database. The route computes + * the current-window KPIs and the prior-window KPIs (via + * `getSalesKpis(portId, previousPeriodBounds(range))`) and hands both + * here to produce the per-tile deltas the KpiCard renders as + * "vs prior period" arrows. + * + * Only the window-derived KPIs get a delta — the point-in-time tiles + * (Active interests, Pipeline value) have no prior-window analogue, so + * they are intentionally absent here. + */ + +// Type-only import: erased at compile time, so this module does NOT pull +// in the DB driver that `sales.service.ts` imports at runtime. +import type { SalesKpis } from '@/lib/services/reports/sales.service'; + +export interface SalesKpiDeltas { + /** current - prior */ + wonInWindow: number; + /** current - prior */ + lostInWindow: number; + /** current - prior */ + newLeadsInWindow: number; + /** current - prior as a fraction [-1, 1]; null when either period has + * no rate (denominator was 0). */ + winRate: number | null; + /** current - prior in days; null when either period has no median + * (sample too small). */ + medianTimeToCloseDays: number | null; +} + +function diffNullable(current: number | null, prior: number | null): number | null { + if (current === null || prior === null) return null; + return current - prior; +} + +export function computeSalesKpiComparison(current: SalesKpis, prior: SalesKpis): SalesKpiDeltas { + return { + wonInWindow: current.wonInWindow - prior.wonInWindow, + lostInWindow: current.lostInWindow - prior.lostInWindow, + newLeadsInWindow: current.newLeadsInWindow - prior.newLeadsInWindow, + winRate: diffNullable(current.winRate, prior.winRate), + medianTimeToCloseDays: diffNullable(current.medianTimeToCloseDays, prior.medianTimeToCloseDays), + }; +} diff --git a/tests/unit/analytics/range.test.ts b/tests/unit/analytics/range.test.ts new file mode 100644 index 00000000..2fec1d07 --- /dev/null +++ b/tests/unit/analytics/range.test.ts @@ -0,0 +1,35 @@ +import { describe, it, expect } from 'vitest'; + +import { previousPeriodBounds } from '@/lib/analytics/range'; + +describe('previousPeriodBounds', () => { + it('returns the equal-length window immediately preceding the given bounds', () => { + const from = new Date('2026-05-01T00:00:00.000Z'); + const to = new Date('2026-05-31T00:00:00.000Z'); // 30-day span + const prior = previousPeriodBounds({ from, to }); + + // Prior window is contiguous (prior.to === current.from) and equal length. + expect(prior.to.toISOString()).toBe('2026-05-01T00:00:00.000Z'); + expect(prior.from.toISOString()).toBe('2026-04-01T00:00:00.000Z'); + }); + + it('preserves sub-day spans (e.g. a single-day window)', () => { + const from = new Date('2026-05-10T00:00:00.000Z'); + const to = new Date('2026-05-10T23:59:59.999Z'); + const prior = previousPeriodBounds({ from, to }); + + const lenMs = to.getTime() - from.getTime(); + expect(prior.to.getTime()).toBe(from.getTime()); + expect(prior.from.getTime()).toBe(from.getTime() - lenMs); + }); + + it('does not mutate the input bounds', () => { + const from = new Date('2026-05-01T00:00:00.000Z'); + const to = new Date('2026-05-31T00:00:00.000Z'); + const fromCopy = from.getTime(); + const toCopy = to.getTime(); + previousPeriodBounds({ from, to }); + expect(from.getTime()).toBe(fromCopy); + expect(to.getTime()).toBe(toCopy); + }); +}); diff --git a/tests/unit/services/reports/sales-comparison.test.ts b/tests/unit/services/reports/sales-comparison.test.ts new file mode 100644 index 00000000..ce8f90ee --- /dev/null +++ b/tests/unit/services/reports/sales-comparison.test.ts @@ -0,0 +1,67 @@ +import { describe, it, expect } from 'vitest'; + +import { computeSalesKpiComparison } from '@/lib/services/reports/sales-comparison'; +import type { SalesKpis } from '@/lib/services/reports/sales.service'; + +function makeKpis(overrides: Partial): SalesKpis { + return { + activeInterests: 0, + wonInWindow: 0, + lostInWindow: 0, + lossBreakdown: [], + winRate: null, + pipelineValue: 0, + pipelineValueCurrency: 'EUR', + pipelineValueExcludedCount: 0, + pipelineValueTotalActiveCount: 0, + medianTimeToCloseDays: null, + timeToCloseSampleSize: 0, + newLeadsInWindow: 0, + newLeadsBySource: [], + ...overrides, + }; +} + +describe('computeSalesKpiComparison', () => { + it('diffs the window-derived count fields (current - prior)', () => { + const current = makeKpis({ wonInWindow: 10, lostInWindow: 4, newLeadsInWindow: 20 }); + const prior = makeKpis({ wonInWindow: 6, lostInWindow: 5, newLeadsInWindow: 12 }); + + const deltas = computeSalesKpiComparison(current, prior); + + expect(deltas.wonInWindow).toBe(4); + expect(deltas.lostInWindow).toBe(-1); + expect(deltas.newLeadsInWindow).toBe(8); + }); + + it('diffs winRate as a fraction when both periods have a rate', () => { + const current = makeKpis({ winRate: 0.5 }); + const prior = makeKpis({ winRate: 0.4 }); + const deltas = computeSalesKpiComparison(current, prior); + expect(deltas.winRate).toBeCloseTo(0.1, 10); + }); + + it('returns null winRate delta when either period has no rate', () => { + expect( + computeSalesKpiComparison(makeKpis({ winRate: 0.5 }), makeKpis({ winRate: null })).winRate, + ).toBeNull(); + expect( + computeSalesKpiComparison(makeKpis({ winRate: null }), makeKpis({ winRate: 0.5 })).winRate, + ).toBeNull(); + }); + + it('diffs medianTimeToCloseDays only when both periods have a median', () => { + expect( + computeSalesKpiComparison( + makeKpis({ medianTimeToCloseDays: 30 }), + makeKpis({ medianTimeToCloseDays: 45 }), + ).medianTimeToCloseDays, + ).toBe(-15); + expect( + computeSalesKpiComparison( + makeKpis({ medianTimeToCloseDays: null }), + makeKpis({ medianTimeToCloseDays: 45 }), + ).medianTimeToCloseDays, + ).toBeNull(); + }); +});