From b97f6e945ce0c19e822a61d638093579fcb16c3e Mon Sep 17 00:00:00 2001 From: Matt Date: Tue, 2 Jun 2026 00:24:27 +0200 Subject: [PATCH] feat(reports): rep + source multi-select filters on Sales report MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes the two cross-cutting filter gaps in launch-readiness (rep multi-select + source multi-select). The Sales detail tables can now be narrowed by assigned rep and lead source alongside the existing stage / lead-category / outcome filters. - service: thread `assignedTo` + `sources` through the 5 filtered Sales queries (rep-performance, stalled, closing-this-month, recent-wins, lost-reason); add `getRepFilterOptions` for the rep dropdown's stable option list (distinct assigned reps port-wide, window-independent). - route: extract param parsing into a pure, unit-tested `parseSalesFilters` helper (source allowlisted against SOURCES; assignedTo passed through as free user-id list); return `repOptions` in the payload. - ui: static Source filter (SOURCES) + dynamic "Assigned to" filter (from payload repOptions, hidden until loaded); decouple the query builder from dynamic options via a stable FILTER_KEYS list. TDD: 8 new parseSalesFilters unit tests (allowlist drop, free-list passthrough, combine). tsc clean; 12/12 reports unit tests; browser- verified both filters fire `source=`/`assignedTo=` → 200. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/app/api/v1/reports/sales/route.ts | 56 +++----------- .../reports/sales/sales-report-client.tsx | 50 +++++++++++-- src/lib/services/reports/sales-filters.ts | 75 +++++++++++++++++++ src/lib/services/reports/sales.service.ts | 65 +++++++++++++++- .../services/reports/sales-filters.test.ts | 75 +++++++++++++++++++ 5 files changed, 264 insertions(+), 57 deletions(-) create mode 100644 src/lib/services/reports/sales-filters.ts create mode 100644 tests/unit/services/reports/sales-filters.test.ts diff --git a/src/app/api/v1/reports/sales/route.ts b/src/app/api/v1/reports/sales/route.ts index 086d0a32..4ef6b1bf 100644 --- a/src/app/api/v1/reports/sales/route.ts +++ b/src/app/api/v1/reports/sales/route.ts @@ -3,9 +3,9 @@ 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 { parseSalesFilters } from '@/lib/services/reports/sales-filters'; import { getSalesKpis, getPipelineFunnel, @@ -13,25 +13,15 @@ import { getWinRateOverTime, getSourceConversion, getRepLeaderboard, + getRepFilterOptions, getDealHeat, getRepPerformanceDetail, getStalledDeals, getClosingThisMonth, getRecentWins, getLostReasonBreakdown, - type SalesFilters, } from '@/lib/services/reports/sales.service'; -const LEAD_CATEGORIES = ['general_interest', 'specific_qualified', 'hot_lead'] as const; -const OUTCOMES = [ - 'won', - 'lost_other_marina', - 'lost_unqualified', - 'lost_no_response', - 'lost_other', - 'cancelled', -] as const; - /** * GET /api/v1/reports/sales?from=&to= * @@ -48,31 +38,10 @@ const OUTCOMES = [ const querySchema = z.object({ from: z.string().datetime().optional(), to: z.string().datetime().optional(), - // CSV-style list params. Empty string → undefined → no filter. - 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(), }); -/** - * Parse a CSV filter param into a typed allowlist. Unknown values are - * silently dropped — that way a stale bookmark with a removed enum - * value degrades to "no filter" instead of 400. - */ -function parseCsv( - raw: string | undefined, - allowed: ReadonlyArray, -): T[] | undefined { - if (!raw) return undefined; - const parts = raw - .split(',') - .map((s) => s.trim()) - .filter((s): s is T => (allowed as ReadonlyArray).includes(s)); - return parts.length > 0 ? parts : undefined; -} - function resolveRange(from?: string, to?: string): { from: Date; to: Date } { const now = new Date(); // Defaults: trailing 30 days. Matches the "Last 30 days" preset on @@ -90,28 +59,18 @@ export const GET = withAuth( withPermission('reports', 'view_dashboard', async (req: NextRequest, ctx) => { try { const params = req.nextUrl.searchParams; - const { from, to, stage, leadCategory, outcome, compare } = querySchema.parse({ + const { from, to, 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); - const leadCategories = parseCsv<(typeof LEAD_CATEGORIES)[number]>( - leadCategory, - LEAD_CATEGORIES, - ); - const outcomes = parseCsv<(typeof OUTCOMES)[number]>(outcome, OUTCOMES); - if (!stages && !leadCategories && !outcomes) return undefined; - return { stages, leadCategories, outcomes }; - })(); + // Detail-table filters: stage / leadCategory / outcome / source / + // assignedTo (rep). Parsed + allowlisted in one pure helper. + const filters = parseSalesFilters(params); const [ kpis, @@ -120,6 +79,7 @@ export const GET = withAuth( winRateOverTime, sourceConversion, repLeaderboard, + repOptions, dealHeat, repPerformanceDetail, stalledDeals, @@ -134,6 +94,7 @@ export const GET = withAuth( getWinRateOverTime(ctx.portId, range), getSourceConversion(ctx.portId), getRepLeaderboard(ctx.portId, range), + getRepFilterOptions(ctx.portId), getDealHeat(ctx.portId), getRepPerformanceDetail(ctx.portId, range, filters), getStalledDeals(ctx.portId, filters), @@ -166,6 +127,7 @@ export const GET = withAuth( winRateOverTime, sourceConversion, repLeaderboard, + repOptions, dealHeat, repPerformanceDetail, stalledDeals, diff --git a/src/components/reports/sales/sales-report-client.tsx b/src/components/reports/sales/sales-report-client.tsx index 49a7495f..de24e7ac 100644 --- a/src/components/reports/sales/sales-report-client.tsx +++ b/src/components/reports/sales/sales-report-client.tsx @@ -27,7 +27,13 @@ import { 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 { + PIPELINE_STAGES, + STAGE_LABELS, + OUTCOME_LABELS, + SOURCES, + type PipelineStage, +} from '@/lib/constants'; import { formatMoney } from '@/lib/reports/format-currency'; import type { ReportPayload } from '@/lib/reports/types'; @@ -197,6 +203,7 @@ interface SalesReportPayload { winRateOverTime: WinRateOverTime; sourceConversion: SourceConversionRow[]; repLeaderboard: RepLeaderboardRow[]; + repOptions: Array<{ userId: string; displayName: string }>; dealHeat: DealHeatSummary; repPerformanceDetail: RepPerformanceDetailRow[]; stalledDeals: StalledDealRow[]; @@ -222,7 +229,9 @@ const SOURCE_LABELS: Record = { unknown: 'unknown', }; -const FILTER_DEFS: FilterDefinition[] = [ +// Static filters — the rep ("Assigned to") filter is appended at render +// time from the report payload's `repOptions` (dynamic per port). +const STATIC_FILTER_DEFS: FilterDefinition[] = [ { key: 'stage', label: 'Stage', @@ -245,8 +254,19 @@ const FILTER_DEFS: FilterDefinition[] = [ type: 'multi-select', options: Object.entries(OUTCOME_LABELS).map(([value, label]) => ({ value, label })), }, + { + key: 'source', + label: 'Source', + type: 'multi-select', + options: SOURCES.map((s) => ({ value: s.value, label: s.label })), + }, ]; +// All filter keys we serialise to the query string. Kept stable here so +// the query builder is decoupled from the dynamic rep options (which +// arrive in the report payload). +const FILTER_KEYS = ['stage', 'leadCategory', 'outcome', 'source', 'assignedTo'] as const; + interface SalesTemplateConfig extends Record { kind: 'sales'; range: DateRange; @@ -299,10 +319,10 @@ export function SalesReportClient({ portSlug: _portSlug }: { portSlug: string }) const filterQs = useMemo(() => { const parts: string[] = []; - for (const def of FILTER_DEFS) { - const v = filterValues[def.key]; + for (const key of FILTER_KEYS) { + const v = filterValues[key]; if (Array.isArray(v) && v.length > 0) { - parts.push(`${def.key}=${encodeURIComponent(v.join(','))}`); + parts.push(`${key}=${encodeURIComponent(v.join(','))}`); } } return parts.length > 0 ? `&${parts.join('&')}` : ''; @@ -334,10 +354,28 @@ export function SalesReportClient({ portSlug: _portSlug }: { portSlug: string }) }; const sourceConversion = query.data?.data.sourceConversion ?? []; const repLeaderboard = query.data?.data.repLeaderboard ?? []; + const repOptions = query.data?.data.repOptions; // Locked decision: when only ONE rep has activity in window, the // leaderboard table is awkward (1-row scoreboard). Hide it; the Rep // performance detail (Task #32) will pick up the slack. const showLeaderboard = repLeaderboard.length > 1; + + // Append the dynamic rep ("Assigned to") filter once the payload's + // repOptions land. The FilterBar hides any multi-select with no + // options, so the rep filter simply doesn't render until then — and + // not at all for a port whose interests are all unassigned. + const filterDefs = useMemo(() => { + if (!repOptions || repOptions.length === 0) return STATIC_FILTER_DEFS; + return [ + ...STATIC_FILTER_DEFS, + { + key: 'assignedTo', + label: 'Assigned to', + type: 'multi-select', + options: repOptions.map((r) => ({ value: r.userId, label: r.displayName })), + }, + ]; + }, [repOptions]); const dealHeat = query.data?.data.dealHeat; const repPerformanceDetail = query.data?.data.repPerformanceDetail ?? []; const stalledDeals = query.data?.data.stalledDeals ?? []; @@ -798,7 +836,7 @@ export function SalesReportClient({ portSlug: _portSlug }: { portSlug: string })

Deal detail

s.value) as ReadonlyArray; + +/** + * Parse a CSV filter param into a typed allowlist. Unknown values are + * silently dropped — that way a stale bookmark with a removed enum value + * degrades to "no filter" instead of a 400. + */ +function parseCsvAllowed( + raw: string | null, + allowed: ReadonlyArray, +): T[] | undefined { + if (!raw) return undefined; + const parts = raw + .split(',') + .map((s) => s.trim()) + .filter((s): s is T => (allowed as ReadonlyArray).includes(s)); + return parts.length > 0 ? parts : undefined; +} + +/** + * Parse a CSV filter param into a free list of ids (no allowlist). Used + * for the rep filter — `assigned_to` holds user ids, which have no static + * enum. Empty / whitespace entries are dropped. Drizzle parameterises the + * downstream `inArray`, so unvalidated ids are injection-safe. + */ +function parseCsvFree(raw: string | null): string[] | undefined { + if (!raw) return undefined; + const parts = raw + .split(',') + .map((s) => s.trim()) + .filter((s) => s.length > 0); + return parts.length > 0 ? parts : undefined; +} + +/** + * Parse the Sales report filter query params into a `SalesFilters` + * object, or `undefined` when no filters are active. Pure — takes a + * `URLSearchParams` so it round-trips trivially in tests. + * + * Recognised params (all CSV): `stage`, `leadCategory`, `outcome`, + * `source`, `assignedTo`. Date-range (`from`/`to`) and `compare` are NOT + * filters and are ignored here. + */ +export function parseSalesFilters(params: URLSearchParams): SalesFilters | undefined { + const stages = parseCsvAllowed(params.get('stage'), PIPELINE_STAGES); + const leadCategories = parseCsvAllowed<(typeof LEAD_CATEGORIES)[number]>( + params.get('leadCategory'), + LEAD_CATEGORIES, + ); + const outcomes = parseCsvAllowed<(typeof OUTCOMES)[number]>(params.get('outcome'), OUTCOMES); + const sources = parseCsvAllowed(params.get('source'), SOURCE_VALUES); + const assignedTo = parseCsvFree(params.get('assignedTo')); + + if (!stages && !leadCategories && !outcomes && !sources && !assignedTo) return undefined; + return { stages, leadCategories, outcomes, sources, assignedTo }; +} diff --git a/src/lib/services/reports/sales.service.ts b/src/lib/services/reports/sales.service.ts index c67488df..20ccc6a8 100644 --- a/src/lib/services/reports/sales.service.ts +++ b/src/lib/services/reports/sales.service.ts @@ -25,6 +25,11 @@ export interface SalesFilters { stages?: PipelineStage[]; leadCategories?: string[]; outcomes?: string[]; + /** Filter to interests assigned to these user IDs (the "rep" filter). */ + assignedTo?: string[]; + /** Filter to interests with these `source` values (website / referral + * / broker / manual / other). */ + sources?: string[]; } /** @@ -56,6 +61,12 @@ function buildSalesFiltersWhere( if (applies.includes('outcomes') && filters.outcomes && filters.outcomes.length > 0) { conds.push(inArray(interests.outcome, filters.outcomes)); } + if (applies.includes('assignedTo') && filters.assignedTo && filters.assignedTo.length > 0) { + conds.push(inArray(interests.assignedTo, filters.assignedTo)); + } + if (applies.includes('sources') && filters.sources && filters.sources.length > 0) { + conds.push(inArray(interests.source, filters.sources)); + } if (conds.length === 0) return undefined; return and(...conds) as SQL; } @@ -746,6 +757,40 @@ export async function getRepLeaderboard( return result.sort((a, b) => b.pipelineValue - a.pipelineValue); } +// ─── Rep filter options ────────────────────────────────────────────────────── + +export interface RepFilterOption { + userId: string; + displayName: string; +} + +/** + * The distinct set of reps that have at least one interest assigned in + * the port — the option list for the Sales report's "Assigned to" (rep) + * filter. Deliberately independent of the active window and the other + * filters so the dropdown stays stable as the user narrows the report. + * Unassigned interests are excluded (the inner join drops null + * `assigned_to`); the filter targets named reps. + */ +export async function getRepFilterOptions(portId: string): Promise { + const rows = await db + .selectDistinct({ + userId: interests.assignedTo, + displayName: userProfiles.displayName, + }) + .from(interests) + .innerJoin(userProfiles, eq(userProfiles.userId, interests.assignedTo)) + .where(eq(interests.portId, portId)) + .orderBy(userProfiles.displayName); + + const options: RepFilterOption[] = []; + for (const r of rows) { + if (!r.userId) continue; // belt-and-braces: inner join already excludes nulls + options.push({ userId: r.userId, displayName: r.displayName ?? 'Unknown' }); + } + return options; +} + // ─── Deal heat (Section between leaderboard and detail tables) ─────────────── export type HeatBucket = 'hot' | 'warm' | 'cold'; @@ -937,7 +982,12 @@ export async function getRepPerformanceDetail( // One query for all open deals across all reps; bucket in JS. const targetCurrency = await resolvePortCurrency(portId); const now = Date.now(); - const filterWhere = buildSalesFiltersWhere(filters, ['stages', 'leadCategories']); + const filterWhere = buildSalesFiltersWhere(filters, [ + 'stages', + 'leadCategories', + 'assignedTo', + 'sources', + ]); const dealRows = await db .select({ id: interests.id, @@ -1038,7 +1088,12 @@ export async function getStalledDeals( const now = Date.now(); const targetCurrency = await resolvePortCurrency(portId); - const filterWhere = buildSalesFiltersWhere(filters, ['stages', 'leadCategories']); + const filterWhere = buildSalesFiltersWhere(filters, [ + 'stages', + 'leadCategories', + 'assignedTo', + 'sources', + ]); const rows = await db .select({ id: interests.id, @@ -1150,7 +1205,7 @@ export async function getClosingThisMonth( ); const stageList: PipelineStage[] = userStages && userStages.length > 0 ? userStages : ['reservation', 'deposit_paid', 'contract']; - const filterWhere = buildSalesFiltersWhere(filters, ['leadCategories']); + const filterWhere = buildSalesFiltersWhere(filters, ['leadCategories', 'assignedTo', 'sources']); const rows = await db .select({ @@ -1240,7 +1295,7 @@ export async function getRecentWins( filters?: SalesFilters, ): Promise { const targetCurrency = await resolvePortCurrency(portId); - const filterWhere = buildSalesFiltersWhere(filters, ['leadCategories']); + const filterWhere = buildSalesFiltersWhere(filters, ['leadCategories', 'assignedTo', 'sources']); const rows = await db .select({ id: interests.id, @@ -1338,6 +1393,8 @@ export async function getLostReasonBreakdown( const filterWhere = buildSalesFiltersWhere({ ...filters, outcomes: lossOutcomes }, [ 'leadCategories', 'outcomes', + 'assignedTo', + 'sources', ]); const rows = await db .select({ diff --git a/tests/unit/services/reports/sales-filters.test.ts b/tests/unit/services/reports/sales-filters.test.ts new file mode 100644 index 00000000..0878a977 --- /dev/null +++ b/tests/unit/services/reports/sales-filters.test.ts @@ -0,0 +1,75 @@ +import { describe, it, expect } from 'vitest'; + +import { parseSalesFilters } from '@/lib/services/reports/sales-filters'; + +function params(init: Record): URLSearchParams { + return new URLSearchParams(init); +} + +describe('parseSalesFilters', () => { + it('returns undefined when no filter params are present', () => { + expect(parseSalesFilters(params({}))).toBeUndefined(); + // from/to/compare are NOT filters — they must not produce a filter object. + expect( + parseSalesFilters(params({ from: '2026-01-01', to: '2026-02-01', compare: '1' })), + ).toBeUndefined(); + }); + + it('parses the pre-existing stage / leadCategory / outcome CSV params', () => { + const f = parseSalesFilters( + params({ + stage: 'eoi,reservation', + leadCategory: 'hot_lead', + outcome: 'won,cancelled', + }), + ); + expect(f?.stages).toEqual(['eoi', 'reservation']); + expect(f?.leadCategories).toEqual(['hot_lead']); + expect(f?.outcomes).toEqual(['won', 'cancelled']); + }); + + it('parses the new source filter against the SOURCES allowlist', () => { + const f = parseSalesFilters(params({ source: 'website,broker' })); + expect(f?.sources).toEqual(['website', 'broker']); + }); + + it('drops unknown source values (stale bookmark degrades to no filter)', () => { + // 'tiktok' is not a valid source — it gets stripped; only 'referral' survives. + const f = parseSalesFilters(params({ source: 'tiktok,referral' })); + expect(f?.sources).toEqual(['referral']); + }); + + it('treats an all-invalid source param as no source filter', () => { + const f = parseSalesFilters(params({ source: 'tiktok,carrierpigeon' })); + expect(f).toBeUndefined(); + }); + + it('parses the new assignedTo (rep) filter as a free list of user ids', () => { + // Rep ids are UUIDs — there is no static allowlist, so any non-empty + // value passes through. Drizzle parameterises inArray, so this is safe. + const f = parseSalesFilters(params({ assignedTo: 'usr_11111111,usr_22222222' })); + expect(f?.assignedTo).toEqual(['usr_11111111', 'usr_22222222']); + }); + + it('trims whitespace and drops empty entries in assignedTo', () => { + const f = parseSalesFilters(params({ assignedTo: ' usr_a , , usr_b ' })); + expect(f?.assignedTo).toEqual(['usr_a', 'usr_b']); + }); + + it('combines source + assignedTo with the existing filters', () => { + const f = parseSalesFilters( + params({ + stage: 'eoi', + source: 'website', + assignedTo: 'usr_a', + }), + ); + expect(f).toEqual({ + stages: ['eoi'], + leadCategories: undefined, + outcomes: undefined, + sources: ['website'], + assignedTo: ['usr_a'], + }); + }); +});