diff --git a/docs/ADVANCED-FILTERS-BRIEF.md b/docs/ADVANCED-FILTERS-BRIEF.md new file mode 100644 index 00000000..98adffe2 --- /dev/null +++ b/docs/ADVANCED-FILTERS-BRIEF.md @@ -0,0 +1,502 @@ +# ADVANCED FILTERS — Sonnet execution brief + +> Full design + all 10 phases + risk register: `~/.claude/plans/today-lets-work-on-humble-avalanche.md` +> This brief covers **Phase 0 and Phase 1 only**. Do not start Phase 2 — stop at the review gate. + +## Why this exists (read before touching anything) + +EdgeX filtering is enum-only and operator-less. We are building a Notion / Twenty-CRM style +filter engine: field → operator → value, stacked conditions, AND/OR, saved views. + +**The point is not the popover.** The predicate logic for leads is currently hand-maintained in +**four** places, and they have already drifted: + +| # | Mirror | Location | +|---|---|---| +| 1 | `getLeads()` / `getLeadsPage()` | `src/lib/supabase/queries.ts:99-130`, `:304-332` — docstring at L259 says the semantics are *"copied to match getLeads()'s applyFilters exactly, not reimplemented"* | +| 2 | Inline chain in the API route | `src/app/(main)/api/v1/leads/route.ts:307-451` (~145 lines) | +| 3 | `lead_aggregates()` SQL | `supabase/migrations/194_*.sql:97-155` — **already drifted**: no `p_stage_eq`, documented at `route.ts:462-466` | +| 4 | AI tool | `src/lib/ai/tools/universal/search-leads.ts` | + +Phase 1 builds the single compiler all four will eventually route through. It is pure TypeScript +with **zero imports from the rest of the app and zero consumers** — the lowest-risk, highest-value +PR in the whole plan. + +--- + +# PHASE 0 — Perf pre-work + +**Branch:** `feature/leads-tags-other-seqscan` +**Why first:** advanced filters multiply query cost. Fix the known seq scan now so every later +measurement is against a sane baseline. + +## The problem + +`src/app/(main)/api/v1/leads/route.ts:314`: + +```ts +// Exclude "other" tagged contacts — they live on the /contacts page, not in lead lists +query = query.not("tags", "cs", '{"other"}'); +``` + +This is unconditional on **every** leads request. It is a **negated GIN predicate** — `NOT (tags @> '{"other"}')` +cannot use `idx_leads_tags`, so it forces a seq scan. The route already documents the cost at L152-156: +*"Count is exact but costly (measured 432ms on prod's 16,898-row Admizz tenant — a seq scan forced by the tags filter)."* + +The same predicate appears in `src/lib/supabase/queries.ts:115` and `search-leads.ts` — fix all sites consistently. + +## Steps + +1. **Measure first. Do not guess.** Against **stage** (`dymeudcddasqpomfpjvt`), capture `EXPLAIN (ANALYZE, BUFFERS)` for the Admizz tenant on: + - default list page (page 1, 25 rows, no filters) + - list page + 3 filters + - the exact-count query + Paste the raw plans into the PR body. This is the baseline for the whole project. + +2. **Pick the fix based on what the plan actually shows.** Two candidates, in preference order: + + **A — partial index (purely additive, preferred):** + ```sql + CREATE INDEX idx_leads_tenant_created_active_nonother + ON leads (tenant_id, created_at DESC, id DESC) + WHERE deleted_at IS NULL AND converted_at IS NULL AND NOT (tags @> ARRAY['other']::text[]); + ``` + Verify the planner *actually picks it up* — a partial index only helps if Postgres can prove the + query predicate implies the index predicate. If `EXPLAIN` still shows a seq scan, A has failed; go to B. + + **B — generated column (guaranteed, but rewrites the table):** + ```sql + ALTER TABLE leads ADD COLUMN is_contact BOOLEAN + GENERATED ALWAYS AS (tags @> ARRAY['other']::text[]) STORED; + CREATE INDEX idx_leads_tenant_created_active_lead + ON leads (tenant_id, created_at DESC, id DESC) + WHERE deleted_at IS NULL AND converted_at IS NULL AND is_contact = false; + ``` + Then all four call sites become `.is("is_contact", false)`. + ⚠️ `ADD COLUMN ... GENERATED ... STORED` takes an ACCESS EXCLUSIVE lock and rewrites the table. + Sub-second at ~17k rows, but **state the measured lock duration from stage in the PR**. + +3. **Migration number:** `ls supabase/migrations/ | sort | tail` → `200` is free today. **Re-check after + rebasing onto latest `origin/stage` right before merge** — the repo already has duplicate `110/197/198` + pairs; do not add another. Transactional, additive, rollback line in the header, before/after counts. + ⚠️ If you need `CREATE INDEX CONCURRENTLY` it cannot run inside a transaction — see `085_unique_display_id.sql` + for the precedent, and say so explicitly in the PR. + +4. **Verify row counts are byte-identical before/after** on stage for the Admizz tenant, several filter + combinations. This change must be a pure perf change with zero semantic difference. + +**Definition of done:** `npm run build` + `npm run test` green; `npx eslint --max-warnings 50` clean; +before/after `EXPLAIN ANALYZE` in the PR body; identical row counts proven. + +**Rollback:** revert the PR. The index is additive and can be left in place. + +--- + +# PHASE 1 — The core filter library + +**Branch:** `feature/filter-engine-core` +**Migration:** none. **Consumers: none.** Nothing in the app imports any of this yet — that is deliberate. + +## Files to create — `src/lib/filters/` + +``` +types.ts the AST + FieldDef + FieldSource + CompileCtx +schema.ts zod validation (discriminated union on `op`) +operators.ts OPERATORS_BY_TYPE + isOperatorAllowed() +serialize.ts base64url encode/decode + size caps +pgrst.ts ★ security-critical: escaping/quoting for PostgREST filter strings +compile.ts compileFilter() — the one predicate implementation +legacy-leads-params.ts legacyLeadsParamsToTree() — the ~22 existing GET params → a tree +``` + +plus tests: `compile.test.ts`, `pgrst.test.ts`, `serialize.test.ts`, `legacy-leads-params.test.ts`. + +`vitest.config.ts` runs `environment: "node"` over `src/**/*.test.ts`. Everything here must be a +**pure function** — no React, no DOM, no Supabase import. Model the shape on the existing +`src/components/pipeline/kanban-column-params.test.ts`, which is the only unit-tested filter code today. + +## 1. `types.ts` + +```ts +export type FilterFieldType = + | "text" | "number" | "date" | "boolean" + | "select" | "multiselect" | "uuid" | "tags" | "relation"; + +export type FilterOperator = + | "is" | "is_not" | "is_empty" | "is_not_empty" + | "contains" | "not_contains" | "starts_with" | "ends_with" + | "is_any_of" | "is_none_of" | "has_all" + | "gt" | "gte" | "lt" | "lte" | "between" + | "before" | "after" | "on" | "date_between" + | "within_last" | "within_next" // "7d" | "30d" | "3m" | "1y" + | "is_true" | "is_false"; + +export interface FilterCondition { + id: string; // stable client key, round-trips through the URL + field: string; // REGISTRY key — NEVER a DB column. Resolution happens only in compile.ts + op: FilterOperator; + value?: string | number | boolean | string[] | [number, number] | [string, string]; +} + +export interface FilterLeafGroup { conjunction: "and" | "or"; conditions: FilterCondition[] } + +export interface FilterGroup { + conjunction: "and" | "or"; + conditions: FilterCondition[]; + groups?: FilterLeafGroup[]; // depth STOPS here — enforced by the type, not a runtime guard +} + +export type FilterTree = FilterGroup; +export const EMPTY_TREE: FilterTree = { conjunction: "and", conditions: [] }; +``` + +Depth-2 is deliberate: two distinct interfaces means `compileGroup` is two non-recursive functions — +no cycle guard, no stack-depth DoS surface on a user-supplied query string. + +`FieldSource` is the discriminant that keeps every known trap out of generic operator code: + +```ts +export type FieldSource = + | { kind: "column"; column: string } + | { kind: "columns"; columns: string[]; fullNamePairs?: boolean } // the search field + | { kind: "array_column"; column: string } // tags, destinations + | { kind: "jsonb"; column: "custom_fields"; path: string } + | { kind: "promoted"; column: string; jsonb: { column: "custom_fields"; path: string } } + | { kind: "embed"; relation: string; column: string; embedSelect: string } + | { kind: "virtual"; compile: (c: FilterCondition, ctx: CompileCtx) => string }; +``` + +`FieldDef` carries: `key, label, type, source, operators?, options?, emptyIsBlankString?, industries?, +group, icon? (lucide name as a STRING), filterable, sortable?, sortColumns?, columnKey?, accessor?, +visibleTo?`. Full annotated declaration is in §2.2 of the plan file — copy it verbatim. + +`CompileCtx` must carry `{ tz: string; now: Date; industryId: string | null; permissions: ResolvedPermissions }`. +**`now` is injected, never `Date.now()` inside the compiler** — otherwise the date tests are non-deterministic. + +## 2. `schema.ts` — zod + +`zod@^4.4.3` is already a prod dependency (`package.json:54`). This is its first non-AI use, and that is +intentional: `src/lib/api/validation.ts` is body-only and **every one of its validators returns `null` +(pass) for an absent or wrong-typed value** — it structurally cannot gate a recursive tree. + +Use a **discriminated union on `op`** so each operator's value shape is validated precisely +(no-value ops carry no `value`; list ops carry `string[]`; `between` carries a tuple; etc.). + +Hard caps — these are the URL-size defence, not cosmetics: +- `z.array(z.string().min(1).max(200)).min(1).max(200)` per list operator +- ≤12 conditions per leaf group, ≤20 root conditions, ≤5 groups, **≤25 conditions total** via `.refine()` +- `.min(1)` on list values so `is_any_of []` is a **422, never a silent no-op** (see the empty-pipeline-allow-list risk) + +Full schema in §1.2 of the plan file. + +## 3. `serialize.ts` + +```ts +export const FILTER_PARAM = "f"; +export const VIEW_PARAM = "view"; +export const MAX_ENCODED_LEN = 4096; // budget under undici's ~16KB header block, after cookies + +export function encodeFilterTree(tree: FilterTree): string; // base64url(JSON), unpadded +export function decodeFilterTree(raw: string): + | { ok: true; tree: FilterTree } + | { ok: false; errors: Record }; // → apiValidationError() +export function isEmptyTree(tree: FilterTree): boolean; +export function countActiveConditions(tree: FilterTree): number; +``` + +base64url over `encodeURIComponent`: percent-encoding inflates `{ " ,` ~3×; base64 is a flat 1.33×. +`MAX_ENCODED_LEN` exists because oversized `.in()` lists have **already caused a production bug** +(the 300-id counselor visibility cap). Exceeding it must produce a 422 with an actionable message +("too many values — save this as a view"), never an opaque transport failure. + +## 4. `pgrst.ts` — ★ THE SECURITY-CRITICAL FILE + +Three rules, in this order. Get these wrong and we ship a filter-injection hole. + +1. **Column names are NEVER derived from input.** `registry[cond.field]` must exist and be + `filterable: true`, else 422. Every column string comes from a `FieldSource` literal in the + registry. This is allow-listing *by construction* — there must be no code path that can splice + an attacker string into the column position. Same discipline as the existing `SORT_COLUMNS` + allow-list (`route.ts:102-108`) and its 422 regression test (`route.test.ts:510-516`). +2. **Operators are allow-listed** per field type via `isOperatorAllowed(field, op)` *before* compilation. +3. **Values are always escaped — never "sanitized by deletion."** + +```ts +const NEEDS_QUOTE = /[,.:()"'\\{}\[\]\s]/; + +export function pgVal(raw: string): string { + if (raw === "") return '""'; + if (!NEEDS_QUOTE.test(raw)) return raw; + return `"${raw.replace(/\\/g, "\\\\").replace(/"/g, '\\"')}"`; +} + +export function pgLike(raw: string, mode: "contains" | "prefix" | "suffix" | "exact"): string { + const lit = raw.replace(/([\\%_])/g, "\\$1"); // escape USER wildcards first… + const pat = mode === "contains" ? `%${lit}%` + : mode === "prefix" ? `${lit}%` + : mode === "suffix" ? `%${lit}` + : lit; // …then add OURS outside the escape + return pgVal(pat); +} + +export function pgCol(column: string, jsonPath?: string): string { + if (!jsonPath) return column; + if (!/^[a-z0-9_]{1,64}$/i.test(jsonPath)) throw new FilterCompileError("bad jsonb key"); + return `${column}->>${jsonPath}`; +} + +export const and = (...p: string[]) => (p.length === 1 ? p[0] : `and(${p.join(",")})`); +export const or = (...p: string[]) => (p.length === 1 ? p[0] : `or(${p.join(",")})`); +export const not = (p: string) => `not.${p}`; +``` + +This also **fixes a live bug**: the current `search.replace(/[,().]/g, "")` (`route.ts:379`) silently +mangles legitimate input like `o'brien@x.co.uk`. Proper quoting replaces deletion. + +Array literals (`tags.eq.{}` for "is empty") emit `{}` as a bare constant, never built from input. + +## 5. `compile.ts` + +```ts +compileFilter(builder: B, tree: FilterTree, registry: FieldRegistry, ctx: CompileCtx): B +``` + +**The invariant that keeps tenant isolation intact — do not violate it:** +`compileFilter` **receives a builder and returns a builder. It must NEVER call `.from()`, `.select()` +or `.rpc()`.** Tenant scoping, the `leads_visible_to_user` RPC (`src/lib/leads/visibility-query.ts`, +`supabase/migrations/179_*.sql`), pipeline/list allow-lists and shared-pool logic all stay exactly +where they are. A compiler that constructs its own base query will either leak the whole tenant to a +counselor or return zero rows — both have precedent in this repo. + +Consequence: the compiler is **client-agnostic** and works identically on the RPC branch +(`userClient.rpc(...)`, RLS-enforced) and the service branch. It also emits **no RPC args**, so the +"never pass explicit `null` in RPC args" rule (PostgREST serializes JS `null` as the string `"null"` → 22P02) +stays entirely inside `visibility-query.ts`. + +**Two output modes:** +- **Pure-AND trees → native builder calls** (`.eq/.in/.gte/.contains/.ilike`). This is the fast path and + it is what keeps `idx_leads_tenant_created_active` usable. Prefer it whenever the condition is not + inside an OR. +- **OR groups → constructed filter strings** via `pgrst.ts`, applied with a single `.or(...)`. + +**The negation rule — the #1 correctness trap, and it is deliberate:** +In SQL, `col <> 'x'` evaluates to NULL (i.e. *excludes* the row) when `col IS NULL`. So a naive +"status is not Contacted" silently hides every lead with no status — users read that as data loss. +**Every negative operator compiles to `or(C.is.null, )`.** Notion, Airtable and Twenty all +behave this way. Put this in a comment block at the top of `compile.ts` and give it a dedicated +`describe("negation includes empty rows")`. + +**Trap handling, one code path each:** +- `kind: "promoted"` — positive ops **OR** the two legs (real column, legacy `custom_fields` path); + negative ops **AND** the two negated legs (De Morgan). Without this, legacy Admizz education rows + silently vanish from a "Field of study is X" filter. Note `destinations` ↔ legacy `custom_fields.countries` + (path ≠ column name). +- `kind: "virtual"` — `status`→`stage_id ?? status`, `source`→`form_config_id ?? intake_source`, + `location`→`city + country`, `created`→`created_at`. The registry key is NOT the column name. +- `is_none_of` on `kind: "embed"` is **not supported** — `!inner + not.in` means "has *a* collaborator + who isn't X", which is semantically wrong and duplicates parent rows. Omit it from `relation` in + `OPERATORS_BY_TYPE`; `planFilter` must 422 it. +- Dates: compute day boundaries from `ctx.tz`, never from server-local time. Kathmandu is UTC+5:45 — + a naive "created today" misses the first 5h45m of leads. + +The complete **operator × field-type mapping table** (every cell specified, incl. jsonb containment +via `custom_fields.cs.{"k":"V"}` which is GIN-backed) is §3.4 of the plan file. Implement it exactly. + +## 6. `legacy-leads-params.ts` + +```ts +export function legacyLeadsParamsToTree(sp: URLSearchParams): FilterTree +``` + +Converts the existing toolbar params — `status, search, form, tag, created, industry, source (csv), +assignees (csv), collaborators (csv)` — into a `FilterTree`. + +**Do NOT include scope params**: `list`, `funnel`, `stage`, `branch_id`, `assigned_to`, `include_converted`, +`page`, `pageSize`, `count`, `sort`, `order`, `facets`. Those are *scope*, applied by the route, and must +never enter the tree. In particular the pipeline allow-list stays a route-applied predicate so +`pipelineAccess.ids === []` keeps failing closed (`.in("pipeline_id", [])` → 0 rows). + +**This function is the whole de-duplication strategy.** In Phase 2 the route will build a tree from +`?f=` *or* from these legacy params and run both through the same compiler — which makes the existing +`route.test.ts` suite a full-fidelity regression harness against real production semantics. + +## Tests — ~120, and they are the deliverable + +- **`pgrst.test.ts` — fuzz `pgVal`/`pgLike`/`pgCol`** with `, . ( ) " \ ' % _ { } \n`, unicode, 200-char + values, and specifically the injection probe **`"a,tenant_id.neq.x"`**. Assert the output cannot break + out of the value position. +- **`compile.test.ts`** — every operator × every field type against the §3.4 table. Plus: + - `describe("negation includes empty rows")` — every negative op × every type emits `or(C.is.null, …)` + - promoted dual-read, **both polarities**, on `field_of_study` and `destinations`/`countries` + - `is_any_of []` throws/422s rather than becoming a no-op + - `is_none_of` on a `relation` field is rejected + - dates with a **frozen `ctx.now`** across `UTC`, `Asia/Kathmandu`, `America/New_York`, including a DST-transition day +- **`serialize.test.ts`** — round-trip; a 250-UUID `is_any_of` is rejected with a usable message, not a transport error +- **`legacy-leads-params.test.ts`** — each legacy param produces the expected condition; scope params are ignored + +## Definition of done (Phase 1) + +- `npm run test` green, `npm run build` clean, `npx eslint --max-warnings 50` clean + (build-clean has red-deployed before — lint separately, per SOP) +- Nothing outside `src/lib/filters/` is modified +- PR to **`stage`** (never `main`), rebased onto latest `origin/stage` + +## STOP HERE + +Do not start Phase 2. Do not merge without the review gate — post the PR link and the test output, +and wait. Historically this step has been skipped; it is not optional. + +--- + +--- + +# PHASE 0-FIXUP — renumber migration 200 → 201 + +**Branch:** existing `feature/leads-tags-other-seqscan` (amend PR #367, do not open a new one) +**Visible surface: NONE.** + +## Why + +**PR #366** (`feature/classes-managers-fees-lockdown`, Anish's work — open, unmerged) already owns +`200_class_managers.sql`, and it is **already applied to the stage ledger** (2026-08-06 17:36). Our +`200_leads_tags_other_partial_index.sql` is a second file with the same number. + +Root cause of the miss: the brief said to take the next number from `ls supabase/migrations/ | sort | tail`, +which returns `199` — because #366's file is not merged to `stage` yet. **Checking the repo alone is not +enough.** The number must be free across: repo files **+ every open PR's migration files + the stage ledger.** + +Verified 2026-08-07: `201` is free on all three. Only #366 and #367 claim 200; the other open PRs with +migrations are stale (100/101/138-142). + +## This does NOT depend on Anish's PR + +Zero file overlap between #366 and #367/#368 — no merge conflict, no ordering dependency. **Do not wait +for #366 to merge, and do not touch his branch.** We renumber ours; his work is untouched either way. + +## Steps + +1. `git mv supabase/migrations/200_leads_tags_other_partial_index.sql supabase/migrations/201_leads_tags_other_partial_index.sql` +2. Update the `INSERT INTO public.schema_migrations (version) VALUES (...)` line inside the file to the new filename. +3. Fill in the `-- Applied: stage ` placeholder in the header — it is still literally ``. +4. On the **stage DB only** (`dymeudcddasqpomfpjvt`), delete the now-stale ledger row so the pipeline + re-applies cleanly under the new name: + `DELETE FROM public.schema_migrations WHERE version = '200_leads_tags_other_partial_index.sql';` + Re-application is a **no-op** — `CREATE INDEX CONCURRENTLY IF NOT EXISTS` and the index already exists. + Do not drop the index. Do not touch the `200_class_managers.sql` row. +5. Rebase onto latest `origin/stage`, force-push, re-check the number is still free before merge. + +**Proof required in the PR body:** `psql` output showing the ledger no longer has the old row, and that +`idx_leads_tenant_created_active_nonother` still exists. + +--- + +# PHASE 2 — Lead field registry + server-side `?f=` + +**Branch:** `feature/filter-engine-leads-route` (branch from `stage` **after** Phase 1 merges) +**Migration:** none. **Visible surface: NONE** — `?f=` has no UI until Phase 3; this phase is +behaviour-preserving by construction. + +## Goal + +Kill **mirror 2**: replace the ~145-line inline `.eq/.in/.or/.contains/.gte` chain at +`src/app/(main)/api/v1/leads/route.ts:307-451` with a single `compileFilter()` call, and route the +existing legacy params through the same tree. + +**The gate that makes this safe:** because legacy params compile through the new tree, the existing +`route.test.ts` suite becomes a full-fidelity regression harness against real production semantics. +**It must pass completely unmodified.** If you find yourself editing an existing test to make it green, +stop — the compiler is wrong, not the test. + +## Work + +### 1. Add `planFilter` to `src/lib/filters/` — Phase 1 omitted it + +Phase 1 exports only `compileFilter`. That is not sufficient for this route, because of a real ordering +constraint at `route.ts:295-302`: + +```ts +const selectColumns: string = collaboratorIds.length > 0 + ? `${LEADS_LIST_COLUMNS},lead_collaborators!inner(user_id)` + : LEADS_LIST_COLUMNS; +``` + +`.select()` is called **before** `compileFilter` would run, so the route must know *in advance* whether +the tree contains an `embed`-kind condition. Add: + +```ts +export function planFilter(tree: FilterTree, registry: FieldRegistry, ctx: CompileCtx): + | { ok: true; embeds: string[] } // e.g. ["lead_collaborators!inner(user_id)"] + | { ok: false; errors: Record }; // → apiValidationError() 422 +``` + +`planFilter` validates every condition (unknown field / not filterable / operator not allowed / +`visibleTo` denies) **up front and returns all errors**, rather than `compileFilter` throwing mid-chain +on the first bad one. The route calls `planFilter` → 422 on error → uses `embeds` to build `selectColumns` +→ then `compileFilter`. Add unit tests for it alongside the Phase 1 suite. + +### 2. `src/lib/filters/registry/{index,leads}.ts` + +`leadFields(ctx)` returns `FieldDef[]`, industry- and permission-filtered. Cover at minimum the 9 axes the +toolbar has today (status, search, form, tag, created, industry, source, assignees, collaborators) plus the +obvious first-class columns. Traps to encode as `FieldSource` kinds, not as special cases: + +- `status` → `virtual` (`stage_id ?? status`), `source` → `virtual` (`form_config_id ?? intake_source`), + `location` → `virtual` (`city + country`), `created` → column `created_at`. **The registry key is not the column name.** +- `field_of_study` / `destinations` → `promoted` (legacy rows carry `custom_fields.field_of_study` / + `custom_fields.countries` — note path ≠ column name). +- `collaborators` → `embed` with `embedSelect: "lead_collaborators!inner(user_id)"`. +- `data_completeness`, `next_task`, `assigned_role` → `filterable: false`. +- **No `cf:*` custom fields in this phase** — that is Phase 6. + +Fold `SORT_COLUMNS` (`route.ts:102-108`) into `FieldDef.sortColumns`. The unknown-sort → **422** behaviour +and its regression test (`route.test.ts:510-516`, `?sort=custom_fields->x`) must survive byte-identically. + +### 3. Rewrite the route's predicate section + +Build the tree from `?f=` (via `decodeFilterTree`, enforcing `MAX_ENCODED_LEN` → 422) **else** from +`legacyLeadsParamsToTree(searchParams)`. Then `planFilter` → `compileFilter`. + +**Leave these completely alone** — they are *scope*, not filters, and must never enter the tree: +`list`, `funnel`, `stage`, `branch_id`, `assigned_to`, `include_converted`, `page`, `pageSize`, `count`, +`sort`, `order`, `facets`, the `visibleLeadsBase()` call and its two-client split, the pipeline allow-list +(`.in("pipeline_id", [...])` — **must keep failing closed on `[]`**), the shared-pool `.in("assigned_to", …)`, +the list/funnel/recycle-bin resolution at L207-264, and the `.not("tags","cs",'{"other"}')` exclusion. + +**Facets:** if `?f=` is present, skip `getSourceFacet()` entirely and return `counts: null` — do **not** +pass partial params to `lead_aggregates()`, which would produce subtly *wrong* counts (worse than none). +The `treeToAggregateParams()` downgrade that restores exact counts lands in Phase 5. `?facets=source` +without `?f=` must behave exactly as today. + +### 4. Do NOT touch in this phase + +`src/lib/supabase/queries.ts` (`getLeads`/`getLeadsPage`) and `src/lib/ai/tools/universal/search-leads.ts`. +Those are Phase 2b. One mirror at a time. + +## Proof required in the PR body + +- Full `route.test.ts` output showing it passes **unmodified** (`git diff` on that file must be empty, + except for genuinely new tests you *add*). +- New equivalence tests: for each legacy param, `?f=` and the legacy form return + byte-identical result sets on stage. +- **Live stage run as four roles** — owner, admin, counselor (`restrictToSelf`), branch-scoped — with the + same tree. Assert counselor's rows ⊆ their own leads and branch rows ⊆ branch leads. This is the + documented tenant-isolation / counselor-scoping hard gate; a compiler that replaced the visibility base + would show up here and nowhere else. +- `EXPLAIN ANALYZE` on a 3-condition pure-AND tree confirming `idx_leads_tenant_created_active` is still used + (the native fast path must not have degraded into `.or()` strings). + +## Definition of done + +`npm run test` green, `npm run build` clean, `npx eslint --max-warnings 50` clean, PR to `stage`, +**stop at the review gate.** Do not start Phase 3. + +--- + +## Non-negotiables for all phases + +- Branch from **latest `origin/stage`**; rebase again right before merge. Squash-merge to `stage`. + `stage` is branch-protected and requires **1 approval** — you cannot self-merge. +- Never merge to `main`. Never apply anything to the prod DB (`pirhnklvtjjpuvbvibxf`). +- Stage DB is `dymeudcddasqpomfpjvt`. **Stage lead data is real customer PII** — 16,436 of Admizz's + 16,684 leads carry a real phone number. Do not paste it anywhere or point third-party services at it. +- The Vercel PR check always fails and is non-blocking — judge CI on GitHub Actions Lint / Type Check / Build / Test only. diff --git a/src/app/(main)/(dashboard)/classes/page.tsx b/src/app/(main)/(dashboard)/classes/page.tsx index fa997dd7..3117f379 100644 --- a/src/app/(main)/(dashboard)/classes/page.tsx +++ b/src/app/(main)/(dashboard)/classes/page.tsx @@ -3,7 +3,8 @@ import { getCurrentUserTenant } from "@/lib/supabase/queries"; import { getFeatureAccess } from "@/industries/_loader"; import { FEATURES } from "@/industries/_registry"; import { createClient, createServiceClient } from "@/lib/supabase/server"; -import { leadQueryScope, canEnrollStudents } from "@/lib/api/permissions"; +import { leadQueryScope } from "@/lib/api/permissions"; +import { canEnrollStudents, canMarkClassAttendance, canViewFullRoster } from "@/lib/api/class-attendance"; import { branchMemberIds } from "@/lib/leads/branch-membership"; import { visibleLeadsBase } from "@/lib/leads/visibility-query"; import { POSITION_ROUTE_MAP } from "@/industries/education-consultancy/features/new-leads-triage/position-routing"; @@ -57,26 +58,24 @@ export default async function ClassesRoute() { : null; const scope = leadQueryScope(tenantData.permissions, tenantData.userId, tenantData.branchId ?? null, poolSlug); - // Attendance markers need the full class roster to mark attendance — own-scope - // lead filtering (built for the leads list) would otherwise hide classmates - // they aren't personally assigned to. Compute this before the roster query so - // it can bypass the own-scope restriction below. - const canMarkAttendance = - tenantData.role === "owner" || - tenantData.role === "admin" || - !!( - await supabase - .from("class_attendance_markers") - .select("user_id") - .eq("tenant_id", tenantData.tenant.id) - .eq("user_id", tenantData.userId) - .maybeSingle() - ).data; + const authSubject = { role: tenantData.role, userId: tenantData.userId, tenantId: tenantData.tenant.id }; + + // Roster-view bypass: class_managers.view_roster grants full-roster visibility + // independent of attendance-marking capability — own-scope lead filtering (built + // for the leads list) would otherwise hide classmates the viewer isn't personally + // assigned to. Compute this before the roster query so it can bypass the + // own-scope restriction below. canMarkAttendance below is the separate + // capability that gates the "Take attendance" button. + const [canViewRoster, canMarkAttendance, canEnroll] = await Promise.all([ + canViewFullRoster(authSubject), + canMarkClassAttendance(authSubject), + canEnrollStudents(authSubject), + ]); let leadIds: string[] | null = null; let teamMemberIds: string[] | null = null; - if (scope.restrictToSelf && scope.userId && !canMarkAttendance) { + if (scope.restrictToSelf && scope.userId && !canViewRoster) { // Visibility-scoped (uncapped; migration 179) — includes collaborator-visible leads, // not just direct assignments. const { data, error } = await visibleLeadsBase({ user: userClient, service: supabase }, tenantData.tenant.id, scope).is("deleted_at", null); @@ -121,15 +120,54 @@ export default async function ClassesRoute() { end_date: string | null; }>; + // Fees totals (aggregate amount + per-class collection %) are owner-only — + // computed here, not in the client, so a non-owner is never handed a + // precomputed total to read off props/devtools. Per-student fee_amount still + // ships in `enrollments` regardless of role — that's the explicit, separate + // "individual fee stays visible to roster viewers" requirement — so this + // narrows the specific gap (a ready-made aggregate on a platter), it does not + // make the aggregate unreconstructable by someone who can already see every + // student's fee (summing what they're allowed to see was never in scope to + // prevent). + const canSeeFeesTotals = tenantData.role === "owner"; + let feesCollected: number | null = null; + let classFeePct: Record | null = null; + if (canSeeFeesTotals) { + let total = 0; + const byClass: Record = {}; + for (const e of enrollments) { + const feePaid = e.fee_paid as boolean; + const feeAmount = e.fee_amount as number | null; + const status = e.status as string; + const classId = e.class_id as string; + if (feePaid && feeAmount != null) total += feeAmount; + if (status !== "inactive") { + const entry = byClass[classId] ?? { paid: 0, payable: 0 }; + entry.payable += 1; + if (feePaid) entry.paid += 1; + byClass[classId] = entry; + } + } + feesCollected = total; + classFeePct = {}; + for (const cls of classes) { + const entry = byClass[cls.id]; + classFeePct[cls.id] = entry && entry.payable > 0 ? Math.round((entry.paid / entry.payable) * 100) : null; + } + } + return (
); diff --git a/src/app/(main)/(dashboard)/leads/[id]/page.tsx b/src/app/(main)/(dashboard)/leads/[id]/page.tsx index 67477cc7..8a3ae6f6 100644 --- a/src/app/(main)/(dashboard)/leads/[id]/page.tsx +++ b/src/app/(main)/(dashboard)/leads/[id]/page.tsx @@ -13,7 +13,8 @@ import { } from "@/lib/supabase/queries"; import { createServiceClient } from "@/lib/supabase/server"; import { LeadDetailV2 } from "@/components/dashboard/lead/lead-detail-v2"; -import { canSeeNav, canAccessList, leadQueryScope, canEnrollStudents } from "@/lib/api/permissions"; +import { canSeeNav, canAccessList, leadQueryScope } from "@/lib/api/permissions"; +import { canEnrollStudents } from "@/lib/api/class-attendance"; import { canBypassProspectQualification } from "@/lib/leads/prospect-qualification"; import { canCreateOrReorderApplications } from "@/lib/api/applications"; import { isOffFunnelLeadList } from "@/lib/leads/list-funnel"; @@ -311,7 +312,7 @@ export default async function LeadDetailPage({ stageAssigneeMap={stageAssigneeMap} canManageApplications={tenantData.permissions.canManageApplications} canManageApplicationPanel={canCreateOrReorderApplications(tenantData, lead)} - canEnroll={canEnrollStudents(tenantData.permissions, tenantData.positionSlug)} + canEnroll={await canEnrollStudents({ role: tenantData.role, userId: tenantData.userId, tenantId: tenantData.tenant.id })} leadLists={accessibleLists} activeLeadLists={activeLeadLists} classesActive={classesActive} diff --git a/src/app/(main)/api/v1/class-enrollments/[id]/route.ts b/src/app/(main)/api/v1/class-enrollments/[id]/route.ts index 54454998..b27da429 100644 --- a/src/app/(main)/api/v1/class-enrollments/[id]/route.ts +++ b/src/app/(main)/api/v1/class-enrollments/[id]/route.ts @@ -14,7 +14,8 @@ import { createServiceClient } from "@/lib/supabase/server"; import { getFeatureAccess } from "@/industries/_loader"; import { FEATURES } from "@/industries/_registry"; import { createAuditLog, emitEvent } from "@/lib/api/audit"; -import { shouldRestrictToSelf, canEnrollStudents } from "@/lib/api/permissions"; +import { shouldRestrictToSelf } from "@/lib/api/permissions"; +import { canEnrollStudents } from "@/lib/api/class-attendance"; import { getLeadMembership } from "@/lib/leads/branch-membership"; interface Props { @@ -74,7 +75,7 @@ export async function PATCH(request: NextRequest, { params }: Props) { const auth = await authenticateRequest(); if (!auth) return apiUnauthorized(); if (!getFeatureAccess(auth.industryId, FEATURES.CLASSES)) return apiForbidden(); - if (!canEnrollStudents(auth.permissions, auth.positionSlug)) return apiForbidden(); + if (!(await canEnrollStudents(auth))) return apiForbidden(); let body: Record; try { @@ -182,7 +183,7 @@ export async function DELETE(_request: NextRequest, { params }: Props) { const auth = await authenticateRequest(); if (!auth) return apiUnauthorized(); if (!getFeatureAccess(auth.industryId, FEATURES.CLASSES)) return apiForbidden(); - if (!canEnrollStudents(auth.permissions, auth.positionSlug)) return apiForbidden(); + if (!(await canEnrollStudents(auth))) return apiForbidden(); const supabase = await createServiceClient(); const db = await scopedClient(auth); diff --git a/src/app/(main)/api/v1/class-enrollments/route.ts b/src/app/(main)/api/v1/class-enrollments/route.ts index 2c9ae3d7..a37c563c 100644 --- a/src/app/(main)/api/v1/class-enrollments/route.ts +++ b/src/app/(main)/api/v1/class-enrollments/route.ts @@ -17,7 +17,8 @@ import { createServiceClient } from "@/lib/supabase/server"; import { getFeatureAccess } from "@/industries/_loader"; import { FEATURES } from "@/industries/_registry"; import { createAuditLog, emitEvent } from "@/lib/api/audit"; -import { shouldRestrictToSelf, canEnrollStudents } from "@/lib/api/permissions"; +import { shouldRestrictToSelf } from "@/lib/api/permissions"; +import { canEnrollStudents } from "@/lib/api/class-attendance"; import { getLeadMembership } from "@/lib/leads/branch-membership"; export async function GET(request: NextRequest) { @@ -82,7 +83,7 @@ export async function POST(request: NextRequest) { const auth = await authenticateRequest(); if (!auth) return apiUnauthorized(); if (!getFeatureAccess(auth.industryId, FEATURES.CLASSES)) return apiForbidden(); - if (!canEnrollStudents(auth.permissions, auth.positionSlug)) return apiForbidden(); + if (!(await canEnrollStudents(auth))) return apiForbidden(); let body: Record; try { diff --git a/src/app/(main)/api/v1/class-managers/route.ts b/src/app/(main)/api/v1/class-managers/route.ts new file mode 100644 index 00000000..55aafa52 --- /dev/null +++ b/src/app/(main)/api/v1/class-managers/route.ts @@ -0,0 +1,169 @@ +import { NextRequest } from "next/server"; +import { authenticateRequest } from "@/lib/api/auth"; +import { + apiSuccess, + apiUnauthorized, + apiForbidden, + apiError, + apiValidationError, +} from "@/lib/api/response"; +import { validate, required } from "@/lib/api/validation"; +import { createRequestLogger } from "@/lib/logger"; +import { scopedClient } from "@/lib/supabase/scoped"; +import { getFeatureAccess } from "@/industries/_loader"; +import { FEATURES } from "@/industries/_registry"; +import { createAuditLog, emitEvent } from "@/lib/api/audit"; + +interface ClassManagerRow { + tenant_id: string; + user_id: string; + enroll_students: boolean; + mark_attendance: boolean; + view_roster: boolean; + granted_by: string | null; + created_at: string; + updated_at: string; +} + +// GET /api/v1/class-managers — list all class_managers grants for the tenant, +// enriched with user email/display name for the settings table. Owner/admin only. +export async function GET() { + const auth = await authenticateRequest(); + if (!auth) return apiUnauthorized(); + if (!getFeatureAccess(auth.industryId, FEATURES.CLASSES)) return apiForbidden(); + if (auth.role !== "owner" && auth.role !== "admin") return apiForbidden(); + + const db = await scopedClient(auth); + + const { data: grants, error } = await db + .from("class_managers") + .select("*") + .order("created_at", { ascending: true }); + + if (error) return apiError("DB_ERROR", "Failed to fetch class managers", 500); + + const rows = (grants ?? []) as unknown as ClassManagerRow[]; + + // Enrich with user email/name via auth.admin — same pattern as /api/v1/team. + const { data: authData } = await db.raw().auth.admin.listUsers({ perPage: 1000 }); + const userMap = new Map(); + const nameMap = new Map(); + for (const u of authData?.users || []) { + userMap.set(u.id, u.email || ""); + const meta = u.user_metadata as Record | undefined; + nameMap.set(u.id, (meta?.name ?? meta?.full_name ?? null) as string | null); + } + + const enriched = rows.map((r) => ({ + userId: r.user_id, + email: userMap.get(r.user_id) || "Unknown", + name: nameMap.get(r.user_id) ?? null, + enrollStudents: r.enroll_students, + markAttendance: r.mark_attendance, + viewRoster: r.view_roster, + grantedBy: r.granted_by, + createdAt: r.created_at, + updatedAt: r.updated_at, + })); + + return apiSuccess(enriched); +} + +// PATCH /api/v1/class-managers — upsert a single user's grant. +// Body: { userId, enrollStudents, markAttendance, viewRoster }. Owner/admin only. +export async function PATCH(request: NextRequest) { + const requestId = crypto.randomUUID(); + const log = createRequestLogger({ requestId, method: "PATCH", path: "/api/v1/class-managers" }); + + const auth = await authenticateRequest(); + if (!auth) return apiUnauthorized(); + if (!getFeatureAccess(auth.industryId, FEATURES.CLASSES)) return apiForbidden(); + if (auth.role !== "owner" && auth.role !== "admin") return apiForbidden(); + + let body: Record; + try { + body = await request.json(); + } catch { + return apiError("INVALID_JSON", "Request body must be valid JSON", 400); + } + + const { valid, errors } = validate(body, { + userId: [required("userId")], + }); + if (!valid) return apiValidationError(errors); + + const userId = String(body.userId); + const enrollStudents = Boolean(body.enrollStudents); + const markAttendance = Boolean(body.markAttendance); + const viewRoster = Boolean(body.viewRoster); + + const db = await scopedClient(auth); + + // Confirm the target user belongs to this tenant before granting. + const { data: member } = await db + .from("tenant_users") + .select("user_id") + .eq("user_id", userId) + .maybeSingle(); + if (!member) return apiError("NOT_FOUND", "User is not a member of this tenant", 404); + + const { data: existing } = await db + .from("class_managers") + .select("enroll_students, mark_attendance, view_roster") + .eq("user_id", userId) + .maybeSingle() as { data: Pick | null }; + + const { data: upserted, error } = await db + .from("class_managers") + .upsert( + { + user_id: userId, + enroll_students: enrollStudents, + mark_attendance: markAttendance, + view_roster: viewRoster, + granted_by: auth.userId, + }, + { onConflict: "tenant_id,user_id" } + ) + .select("*") + .single(); + + if (error) { + log.error({ error }, "Failed to upsert class manager grant"); + return apiError("DB_ERROR", "Failed to update class manager grant", 500); + } + + await Promise.all([ + createAuditLog({ + tenantId: auth.tenantId, + userId: auth.userId, + action: "class_manager.updated", + entityType: "class_manager", + entityId: userId, + changes: { + grant: { + old: existing + ? { + enrollStudents: existing.enroll_students, + markAttendance: existing.mark_attendance, + viewRoster: existing.view_roster, + } + : null, + new: { enrollStudents, markAttendance, viewRoster }, + }, + }, + requestId, + }), + emitEvent({ + tenantId: auth.tenantId, + type: "class_manager.updated", + entityType: "class_manager", + entityId: userId, + requestId, + payload: { enrollStudents, markAttendance, viewRoster }, + }), + ]); + + log.info({ userId }, "Class manager grant updated"); + return apiSuccess(upserted); +} diff --git a/src/app/(main)/api/v1/leads/[id]/classes/route.ts b/src/app/(main)/api/v1/leads/[id]/classes/route.ts index 154ee3a6..a489c0aa 100644 --- a/src/app/(main)/api/v1/leads/[id]/classes/route.ts +++ b/src/app/(main)/api/v1/leads/[id]/classes/route.ts @@ -2,7 +2,8 @@ import { NextRequest } from "next/server"; import { createServiceClient } from "@/lib/supabase/server"; import { authenticateRequest, requireLeadBranchAccess } from "@/lib/api/auth"; import { getLeadMembership } from "@/lib/leads/branch-membership"; -import { shouldRestrictToSelf, canEnrollStudents } from "@/lib/api/permissions"; +import { shouldRestrictToSelf } from "@/lib/api/permissions"; +import { canEnrollStudents } from "@/lib/api/class-attendance"; import { apiSuccess, apiUnauthorized, @@ -77,7 +78,7 @@ export async function POST(request: NextRequest, context: RouteContext) { const auth = await authenticateRequest(); if (!auth) return apiUnauthorized(); if (!getFeatureAccess(auth.industryId, FEATURES.CLASSES)) return apiForbidden(); - if (!canEnrollStudents(auth.permissions, auth.positionSlug)) return apiForbidden(); + if (!(await canEnrollStudents(auth))) return apiForbidden(); const supabase = await createServiceClient(); diff --git a/src/app/(main)/api/v1/leads/route.test.ts b/src/app/(main)/api/v1/leads/route.test.ts index 15867706..a5e16349 100644 --- a/src/app/(main)/api/v1/leads/route.test.ts +++ b/src/app/(main)/api/v1/leads/route.test.ts @@ -2,6 +2,8 @@ import { describe, it, expect, vi, beforeEach } from "vitest"; import type { NextRequest } from "next/server"; import type { AuthContext } from "@/lib/api/auth"; import type { ResolvedPermissions } from "@/lib/api/permissions"; +import { legacyLeadsParamsToTree } from "@/lib/filters/legacy-leads-params"; +import { encodeFilterTree, FILTER_PARAM } from "@/lib/filters/serialize"; // --- mocks ----------------------------------------------------------- // @@ -106,10 +108,23 @@ function makeLeadsChain(calls: Call[]) { const chain: Record = { select: record("select"), eq: record("eq"), + neq: record("neq"), is: record("is"), or: record("or"), in: record("in"), not: record("not"), + // gt/gte/lt/lte/ilike/contains/overlaps: unexercised before the Phase 2 + // ?f=/legacy-toolbar equivalence tests below — those are the first tests + // in this file to reach compileFilter's native fast path for these ops + // (e.g. .contains() for a tag filter, which route.ts has called directly + // since before this phase, just never under a test). + gt: record("gt"), + gte: record("gte"), + lt: record("lt"), + lte: record("lte"), + ilike: record("ilike"), + contains: record("contains"), + overlaps: record("overlaps"), order: record("order"), range: () => Promise.resolve({ data: [], error: null, count: 0 }), }; @@ -690,3 +705,131 @@ describe("GET /api/v1/leads — ?stage= filter (pipeline-column-pagination Phase expect(calls.some(([m, a]) => m === "or" && String(a[0]).includes("acme"))).toBe(true); }); }); + +// --- ADVANCED-FILTERS-BRIEF Phase 2: ?f= vs legacy toolbar-param equivalence --- + +function encodedTreeFor(legacyParams: Record): string { + return encodeFilterTree(legacyLeadsParamsToTree(new URLSearchParams(legacyParams))); +} + +describe("GET /api/v1/leads — ?f= compiles through the SAME compileFilter() as legacy params (ADVANCED-FILTERS-BRIEF Phase 2)", () => { + beforeEach(() => { + authenticateRequestMock.mockReset(); + createServiceClientMock.mockReset(); + createClientMock.mockReset(); + getFeatureAccessMock.mockReset(); + branchMemberIdsMock.mockReset(); + getFeatureAccessMock.mockReturnValue(false); + branchMemberIdsMock.mockResolvedValue([]); + createClientMock.mockResolvedValue(fakeUserClient([])); + authenticateRequestMock.mockResolvedValue( + authFixture({ userId: "admin-1", role: "owner", permissions: permissions({ leadScope: "all" }) }), + ); + }); + + it("?status=contacted and its equivalent ?f= tree produce byte-identical query calls", async () => { + const legacyCalls: Call[] = []; + createServiceClientMock.mockResolvedValue(fakeDb({ leadsCalls: legacyCalls })); + const { GET } = await import("./route"); + const legacyRes = await GET(fakeReq({ status: "contacted" })); + expect(legacyRes.status).toBe(200); + + const fCalls: Call[] = []; + createServiceClientMock.mockResolvedValue(fakeDb({ leadsCalls: fCalls })); + const fRes = await GET(fakeReq({ [FILTER_PARAM]: encodedTreeFor({ status: "contacted" }) })); + expect(fRes.status).toBe(200); + + expect(fCalls).toEqual(legacyCalls); + }); + + it("?tag=vip and its equivalent ?f= tree produce byte-identical query calls (native .contains() path)", async () => { + const legacyCalls: Call[] = []; + createServiceClientMock.mockResolvedValue(fakeDb({ leadsCalls: legacyCalls })); + const { GET } = await import("./route"); + await GET(fakeReq({ tag: "vip" })); + + const fCalls: Call[] = []; + createServiceClientMock.mockResolvedValue(fakeDb({ leadsCalls: fCalls })); + await GET(fakeReq({ [FILTER_PARAM]: encodedTreeFor({ tag: "vip" }) })); + + expect(fCalls).toEqual(legacyCalls); + expect(legacyCalls).toContainEqual(["contains", ["tags", ["vip"]]]); + }); + + it("?industry=__none__ and its equivalent ?f= tree produce byte-identical query calls (native .is(null) path)", async () => { + const legacyCalls: Call[] = []; + createServiceClientMock.mockResolvedValue(fakeDb({ leadsCalls: legacyCalls })); + const { GET } = await import("./route"); + await GET(fakeReq({ industry: "__none__" })); + + const fCalls: Call[] = []; + createServiceClientMock.mockResolvedValue(fakeDb({ leadsCalls: fCalls })); + await GET(fakeReq({ [FILTER_PARAM]: encodedTreeFor({ industry: "__none__" }) })); + + expect(fCalls).toEqual(legacyCalls); + expect(legacyCalls).toContainEqual(["is", ["prospect_industry", null]]); + }); + + it("?assignees=unassigned, and its equivalent ?f= tree produce byte-identical query calls", async () => { + const uuid = "11111111-2222-4333-8444-555555555555"; + const legacyCalls: Call[] = []; + createServiceClientMock.mockResolvedValue(fakeDb({ leadsCalls: legacyCalls })); + const { GET } = await import("./route"); + await GET(fakeReq({ assignees: `unassigned,${uuid}` })); + + const fCalls: Call[] = []; + createServiceClientMock.mockResolvedValue(fakeDb({ leadsCalls: fCalls })); + await GET(fakeReq({ [FILTER_PARAM]: encodedTreeFor({ assignees: `unassigned,${uuid}` }) })); + + expect(fCalls).toEqual(legacyCalls); + }); + + it("?collaborators= and its equivalent ?f= tree both add the lead_collaborators!inner(user_id) embed and strip it from the response", async () => { + const legacyCalls: Call[] = []; + createServiceClientMock.mockResolvedValue(fakeDb({ leadsCalls: legacyCalls })); + const { GET } = await import("./route"); + await GET(fakeReq({ collaborators: "u1" })); + + const fCalls: Call[] = []; + createServiceClientMock.mockResolvedValue(fakeDb({ leadsCalls: fCalls })); + const fRes = await GET(fakeReq({ [FILTER_PARAM]: encodedTreeFor({ collaborators: "u1" }) })); + + expect(fRes.status).toBe(200); + expect(fCalls).toEqual(legacyCalls); + expect(legacyCalls.some(([m, a]) => m === "select" && String(a[0]).includes("lead_collaborators!inner(user_id)"))).toBe(true); + }); + + it("a malformed ?f= (not valid base64url JSON) is a 422, never an unhandled throw", async () => { + createServiceClientMock.mockResolvedValue(fakeDb({ leadsCalls: [] })); + const { GET } = await import("./route"); + const res = await GET(fakeReq({ [FILTER_PARAM]: "not-valid-base64url-json!!" })); + expect(res.status).toBe(422); + }); + + it("an ?f= tree referencing an unknown field is a single 422 with all errors, not a throw mid-compile", async () => { + createServiceClientMock.mockResolvedValue(fakeDb({ leadsCalls: [] })); + const { GET } = await import("./route"); + const tree = { conjunction: "and" as const, conditions: [{ id: "c1", field: "not_a_real_field", op: "is" as const, value: "x" }] }; + const res = await GET(fakeReq({ [FILTER_PARAM]: encodeFilterTree(tree) })); + expect(res.status).toBe(422); + }); + + it("?facets=source with ?f= present skips getSourceFacet entirely and returns counts:null, never partial/wrong counts", async () => { + createServiceClientMock.mockResolvedValue(fakeDb({ leadsCalls: [] })); + const { GET } = await import("./route"); + const res = await GET(fakeReq({ facets: "source", [FILTER_PARAM]: encodedTreeFor({ status: "contacted" }) })); + const body = await res.json(); + expect(res.status).toBe(200); + expect(body.data).toEqual({ facet: "source", options: [], counts: null }); + }); + + it("?facets=source WITHOUT ?f= is completely unaffected — still returns the legacy {facet,options} shape", async () => { + createClientMock.mockResolvedValue({ rpc: () => Promise.resolve({ data: [], error: null }) }); + createServiceClientMock.mockResolvedValue(fakeDb({ leadsCalls: [] })); + const { GET } = await import("./route"); + const res = await GET(fakeReq({ facets: "source" })); + const body = await res.json(); + expect(res.status).toBe(200); + expect(body.data).toEqual({ facet: "source", options: [] }); + }); +}); diff --git a/src/app/(main)/api/v1/leads/route.ts b/src/app/(main)/api/v1/leads/route.ts index 2d775051..f3f8075c 100644 --- a/src/app/(main)/api/v1/leads/route.ts +++ b/src/app/(main)/api/v1/leads/route.ts @@ -35,6 +35,11 @@ import { POSITION_ROUTE_MAP } from "@/industries/education-consultancy/features/ import { addLeadCollaborator } from "@/lib/leads/collaborators"; import { visibleLeadsBase } from "@/lib/leads/visibility-query"; import { getSourceFacet } from "@/lib/leads/aggregates"; +import { compileFilter, planFilter } from "@/lib/filters/compile"; +import { decodeFilterTree, FILTER_PARAM } from "@/lib/filters/serialize"; +import { legacyLeadsParamsToTree } from "@/lib/filters/legacy-leads-params"; +import { leadFields } from "@/lib/filters/registry"; +import type { CompileCtx, FilterTree, ResolvedPermissions as FilterResolvedPermissions } from "@/lib/filters/types"; import { normalizeEmail, normalizePhone, @@ -95,17 +100,12 @@ archive_reason,archived_by,archived_at,archived_from_list_id,archived_from_statu last_activity_at,stage_changed_at,created_at,updated_at"; // Sort allow-list — never interpolate a client-supplied column name into the query. -// Only "created_at" is covered by an index (idx_leads_tenant_created_active); the -// others page-table already offered client-side and are kept for parity, at the -// cost of an in-memory sort node over the tenant's active row set (bounded by -// tenant size, not full-table — see PR report for measured cost). -const SORT_COLUMNS: Record = { - created_at: ["created_at"], - last_activity_at: ["last_activity_at"], - updated_at: ["updated_at"], - first_name: ["first_name", "last_name"], - email: ["email"], -}; +// Folded into the lead field registry's FieldDef.sortColumns (src/lib/filters/registry/leads.ts) +// as of the advanced-filters Phase 2 rewrite — see the sort-key resolution below. Only +// "created_at" is covered by an index (idx_leads_tenant_created_active); the others +// page-table already offered client-side and are kept for parity, at the cost of an +// in-memory sort node over the tenant's active row set (bounded by tenant size, not +// full-table — see PR report for measured cost). // Only UUID-shaped tokens are ever interpolated into a raw `.or()` filter string // (the `assignees` filter below) — anything else is dropped rather than trusted. @@ -156,25 +156,28 @@ export async function GET(request: NextRequest) { const wantCount = searchParams.get("count") !== "0"; // Toolbar secondary filters (form/counselor/collaborators/source/tag/created/ - // prospect industry) — applied server-side against the FULL matching set, not just - // the loaded page. All values below are passed through supabase-js's parameterized - // filter methods (.eq/.in/.contains/.gte), never string-interpolated into a raw - // filter, except `assignees` — its UUID-validated ids are interpolated into an - // `.or()` string below because that's the only way to express "unassigned OR in - // this list"; validation happens right before that interpolation. + // prospect industry) — raw values are still read here because the facets=source + // branch further below (legacy path only — see the ?f= check there) recomputes + // counts from these exact params, mirroring getSourceFacet's existing contract. + // The actual PAGE query no longer applies these directly: they (or an equivalent + // ?f= tree) are compiled once via compileFilter() below (ADVANCED-FILTERS-BRIEF + // Phase 2) — see filterTree/filterPlan. const formFilter = searchParams.get("form"); // Pipeline-board column identity (KANBAN-PAGINATION-BRIEF Phase 1 / stage filter) — a // pipeline column is a stage_id, unlike the list-Kanban's (list,status) columns. Only // a UUID-shaped value is ever applied; anything else is dropped silently (same // never-interpolate-untrusted-input posture as `assignees`/`collaborators` above, - // though this one goes through .eq(), never a raw string). + // though this one goes through .eq(), never a raw string). This is SCOPE, not a + // toolbar filter — it never enters the filter tree. const stageFilterRaw = searchParams.get("stage"); const stageFilter = stageFilterRaw && UUID_RE.test(stageFilterRaw) ? stageFilterRaw : null; const tagFilter = searchParams.get("tag"); const createdFilter = searchParams.get("created"); // today | week | month const industryFilter = searchParams.get("industry"); // prospect_industry value, or "__none__" - const sourceFilter = (searchParams.get("source") || "") - .split(",").map((s) => s.trim()).filter(Boolean); + // NOTE: no `sourceFilter` variable here — the facets=source branch below computes + // source's OWN facet, so it (like the pre-Phase-2 code) never needs source's value + // for anything other than building the filter tree, which legacyLeadsParamsToTree + // reads directly off searchParams itself. const assigneesTokens = (searchParams.get("assignees") || "") .split(",").map((s) => s.trim()).filter(Boolean); // Collaborator filter is user ids only (never leaked into a raw string — see below), @@ -182,10 +185,46 @@ export async function GET(request: NextRequest) { const collaboratorIds = (searchParams.get("collaborators") || "") .split(",").map((s) => s.trim()).filter(Boolean); - // Sort — allow-listed against SORT_COLUMNS, never interpolated. id is always the + // ADVANCED-FILTERS-BRIEF Phase 2: build the filter tree from ?f= if present, + // else from the legacy toolbar params above — both compile through the SAME + // compileFilter() call further below, which is the whole point (the existing + // route.test.ts suite is a full-fidelity regression harness for the compiler + // against real production semantics precisely because the legacy path routes + // through it too). `tz` is fixed at "UTC" for now — no legacy toolbar param + // needs tz-aware day boundaries (they all use rolling within_last windows or + // plain column comparisons), and per-tenant timezone wiring for the real + // date-picker UI is Phase 3+ work. + const compileCtx: CompileCtx = { + tz: "UTC", + now: new Date(), + industryId: auth.industryId, + // src/lib/filters/types.ts deliberately does not import the real ResolvedPermissions + // (zero-imports-from-the-rest-of-the-app invariant — see its own doc comment); this + // structural cast is the seam. No field on it is currently read by any FieldDef's + // visibleTo in this registry, so this is a type-level bridge only, not a behavior gap. + permissions: auth.permissions as unknown as FilterResolvedPermissions, + }; + const filterRegistry = leadFields(compileCtx); + + const rawFilterParam = searchParams.get(FILTER_PARAM); + let filterTree: FilterTree; + if (rawFilterParam !== null) { + const decoded = decodeFilterTree(rawFilterParam); + if (!decoded.ok) return apiValidationError(decoded.errors); + filterTree = decoded.tree; + } else { + filterTree = legacyLeadsParamsToTree(searchParams); + } + + const filterPlan = planFilter(filterTree, filterRegistry, compileCtx); + if (!filterPlan.ok) return apiValidationError(filterPlan.errors); + + // Sort — allow-listed against the field registry's sortColumns (folded in from + // the former standalone SORT_COLUMNS map), never interpolated. id is always the // final tiebreaker so a paginated sort never reshuffles rows between pages. const sortKey = searchParams.get("sort") || "created_at"; - const sortColumns = SORT_COLUMNS[sortKey]; + const sortField = filterRegistry[sortKey]; + const sortColumns = sortField?.sortable ? sortField.sortColumns : undefined; if (!sortColumns) { return apiValidationError({ sort: [`Unknown sort key "${sortKey}"`] }); } @@ -297,9 +336,15 @@ export async function GET(request: NextRequest) { // collapses inference to `ParserError` (see the LEADS_LIST_COLUMNS comment above). // The `data as Lead[]` / `Record` casts below already carry the // real typing, same as every other dynamically-shaped query in this route. - const selectColumns: string = collaboratorIds.length > 0 - ? `${LEADS_LIST_COLUMNS},lead_collaborators!inner(user_id)` - : LEADS_LIST_COLUMNS; + // + // Which embeds are needed is now planFilter's job (filterPlan.embeds), not a + // direct collaboratorIds.length check — planFilter already validated the tree + // (422'd above if bad), so by the time we're here every embed it names is safe + // to add to the select. `.select()` must be called before compileFilter (which + // itself never calls .select() — see compile.ts's module doc comment), hence + // filterPlan is computed above, before the query is even built. + const hasCollaboratorsEmbed = filterPlan.embeds.includes("lead_collaborators!inner(user_id)"); + const selectColumns: string = filterPlan.embeds.length > 0 ? `${LEADS_LIST_COLUMNS},${filterPlan.embeds.join(",")}` : LEADS_LIST_COLUMNS; let query = useVisibilityRpc ? visibleLeadsBase({ user: userClient, service: supabase }, auth.tenantId, scope, countOpts).select(selectColumns) : supabase.from("leads").select(selectColumns, countOpts).eq("tenant_id", auth.tenantId); @@ -366,79 +411,21 @@ export async function GET(request: NextRequest) { query = query.eq("assigned_to", assignedTo); } - if (status) { - query = query.eq("status", status); - } - if (stageFilter) { query = query.eq("stage_id", stageFilter); } - if (search) { - // Sanitize search input to prevent PostgREST filter injection - const sanitized = search.replace(/[,().]/g, ""); - if (sanitized) { - const orClauses = [ - `first_name.ilike.%${sanitized}%`, - `last_name.ilike.%${sanitized}%`, - `email.ilike.%${sanitized}%`, - `phone.ilike.%${sanitized}%`, - ]; - - // Full-name search: "John Smith" won't match first_name/last_name - // individually since PostgREST can't filter on a concatenated - // expression — match token pairs against first/last in either order. - const tokens = sanitized.trim().split(/\s+/).filter(Boolean); - if (tokens.length >= 2) { - const [t1, t2] = tokens; - orClauses.push( - `and(first_name.ilike.%${t1}%,last_name.ilike.%${t2}%)`, - `and(first_name.ilike.%${t2}%,last_name.ilike.%${t1}%)` - ); - } - - query = query.or(orClauses.join(",")); - } - } - - // ── Toolbar secondary filters (form/counselor/collaborators/source/tag/created/ - // prospect industry) — composed with every filter above via AND, same as status/ - // search. These used to be applied client-side over whichever page happened to be - // loaded, which silently narrowed a "300 matching leads" filter down to "2, because - // that's all that fit on this page" (LEADS-SERVER-PAGINATION-BRIEF review). ── - if (formFilter && formFilter !== "all") { - query = query.eq("form_config_id", formFilter); - } - - if (assigneesTokens.length > 0) { - const wantsUnassigned = assigneesTokens.includes("unassigned"); - const ids = assigneesTokens.filter((t) => t !== "unassigned" && UUID_RE.test(t)); - if (wantsUnassigned && ids.length > 0) { - query = query.or(`assigned_to.is.null,assigned_to.in.(${ids.join(",")})`); - } else if (wantsUnassigned) { - query = query.is("assigned_to", null); - } else if (ids.length > 0) { - query = query.in("assigned_to", ids); - } - } - - if (collaboratorIds.length > 0) { - query = query.in("lead_collaborators.user_id", collaboratorIds); - } - - if (sourceFilter.length > 0) { - query = query.in("intake_source", sourceFilter); - } - - if (tagFilter && tagFilter !== "all") { - query = query.contains("tags", [tagFilter]); - } - - if (industryFilter && industryFilter !== "all") { - query = industryFilter === "__none__" - ? query.is("prospect_industry", null) - : query.eq("prospect_industry", industryFilter); - } + // ADVANCED-FILTERS-BRIEF Phase 2: every toolbar filter that used to be a + // hand-written .eq/.in/.or/.contains/.gte chain here (status, search, form, + // assignees, collaborators, source, tag, industry, created) now compiles + // through the SAME compileFilter() call whether it came from ?f= or from + // the legacy params via legacyLeadsParamsToTree — see filterTree/filterPlan + // above. compileFilter never touches .select()/.from()/.rpc() (see its + // module doc comment), so it's safe to call on `query` at this point + // regardless of which branch (visibleLeadsBase RPC vs plain service query) + // built it. `stage`/`list`/`funnel`/branch/pipeline/shared-pool SCOPE + // filters above and below this call are deliberately untouched. + query = compileFilter(query, filterTree, filterRegistry, compileCtx); const DAY_MS = 24 * 60 * 60 * 1000; const CREATED_WINDOW_MS: Record = { today: DAY_MS, week: 7 * DAY_MS, month: 30 * DAY_MS }; @@ -446,10 +433,6 @@ export async function GET(request: NextRequest) { ? new Date(Date.now() - CREATED_WINDOW_MS[createdFilter]) : null; - if (createdAfter) { - query = query.gte("created_at", createdAfter.toISOString()); - } - // Opt-in Source facet (?facets=source) — same "opt-in, separate round-trip" shape // as ?counts=1 (lead-lists route). Computed via lead_aggregates() (migration 194's // ADDENDUM) over every filter above EXCEPT source itself, per the brief: the option @@ -465,6 +448,17 @@ export async function GET(request: NextRequest) { // gets a facet computed WITHOUT the stage restriction — flagged, not silently fixed; // closing it needs a migration (out of this PR's additive-only-locally scope). if (searchParams.get("facets") === "source" && !onlyDeleted) { + // ADVANCED-FILTERS-BRIEF Phase 2: a ?f= tree has no lead_aggregates() mirror yet + // (that's the treeToAggregateParams() downgrade landing in Phase 5) — passing a + // PARTIAL translation of the tree into lead_aggregates would produce a facet with + // subtly WRONG counts (missing whatever the tree expresses that the RPC's fixed + // param list can't), which is worse than no counts at all. Skip getSourceFacet() + // entirely and say so explicitly via counts: null. Legacy callers (?facets=source + // without ?f=) are completely unaffected — this branch only short-circuits for ?f=. + if (rawFilterParam !== null) { + return apiSuccess({ facet: "source", options: [], counts: null }); + } + // Match route.ts:370's `.in("pipeline_id", [])` semantics exactly: an empty allowlist // means the page returns zero leads, so the facet must be empty too. aggregates.ts // omits an empty p_pipeline_ids (→ NULL → no restriction), which would otherwise @@ -563,7 +557,7 @@ export async function GET(request: NextRequest) { // Strip the lead_collaborators embed — it only existed to filter (see selectColumns // above), it is not part of the Lead shape the client expects. `data`'s inferred type // is the widened-string fallback (see selectColumns above), hence the `unknown` hop. - const strippedData: Array> = collaboratorIds.length > 0 + const strippedData: Array> = hasCollaboratorsEmbed ? (data as unknown as Array>).map((row) => { // eslint-disable-next-line @typescript-eslint/no-unused-vars const { lead_collaborators, ...rest } = row; diff --git a/src/components/dashboard/settings/class-managers.tsx b/src/components/dashboard/settings/class-managers.tsx new file mode 100644 index 00000000..72cb4cee --- /dev/null +++ b/src/components/dashboard/settings/class-managers.tsx @@ -0,0 +1,233 @@ +"use client"; + +import { useState, useEffect, useCallback } from "react"; +import { toast } from "sonner"; +import { UserCog } from "lucide-react"; +import { Checkbox } from "@/components/ui/checkbox"; +import { Badge } from "@/components/ui/badge"; +import { + Card, + CardContent, + CardDescription, + CardHeader, + CardTitle, +} from "@/components/ui/card"; +import { + Table, + TableBody, + TableCell, + TableHead, + TableHeader, + TableRow, +} from "@/components/ui/table"; + +interface TeamMember { + user_id: string; + role: string; + name: string | null; + email: string; +} + +interface ClassManagerGrant { + userId: string; + email: string; + name: string | null; + enrollStudents: boolean; + markAttendance: boolean; + viewRoster: boolean; +} + +interface ManagerRow { + userId: string; + role: string; + name: string | null; + email: string; + enrollStudents: boolean; + markAttendance: boolean; + viewRoster: boolean; +} + +type GrantField = "enrollStudents" | "markAttendance" | "viewRoster"; + +export function ClassManagers() { + const [rows, setRows] = useState([]); + const [loading, setLoading] = useState(true); + const [savingKey, setSavingKey] = useState(null); + + const fetchData = useCallback(async () => { + setLoading(true); + try { + const [teamRes, grantsRes] = await Promise.all([ + fetch("/api/v1/team"), + fetch("/api/v1/class-managers"), + ]); + + const members: TeamMember[] = teamRes.ok + ? ((await teamRes.json()).data ?? []) + : []; + const grants: ClassManagerGrant[] = grantsRes.ok + ? ((await grantsRes.json()).data ?? []) + : []; + + const grantMap = new Map(grants.map((g) => [g.userId, g])); + + const merged: ManagerRow[] = members + .filter((m) => m.role !== "owner" && m.role !== "admin") + .map((m) => { + const grant = grantMap.get(m.user_id); + return { + userId: m.user_id, + role: m.role, + name: m.name, + email: m.email, + enrollStudents: grant?.enrollStudents ?? false, + markAttendance: grant?.markAttendance ?? false, + viewRoster: grant?.viewRoster ?? false, + }; + }); + + setRows(merged); + } catch { + toast.error("Failed to load class managers"); + } finally { + setLoading(false); + } + }, []); + + useEffect(() => { + fetchData(); + }, [fetchData]); + + async function handleToggle(row: ManagerRow, field: GrantField, checked: boolean) { + const key = `${row.userId}:${field}`; + const previousValue = row[field]; + let next: ManagerRow = { ...row, [field]: checked }; + + // Read the current row (not the closure's stale `row`) so two rapid toggles + // on the same user don't clobber each other's optimistic update on revert. + setRows((prev) => + prev.map((r) => { + if (r.userId !== row.userId) return r; + next = { ...r, [field]: checked }; + return next; + }) + ); + setSavingKey(key); + + try { + const res = await fetch("/api/v1/class-managers", { + method: "PATCH", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ + userId: row.userId, + enrollStudents: next.enrollStudents, + markAttendance: next.markAttendance, + viewRoster: next.viewRoster, + }), + }); + if (!res.ok) throw new Error("Failed to update grant"); + toast.success("Class manager access updated"); + } catch { + // Revert only this field to its pre-toggle value, not the whole row — + // avoids clobbering a concurrent toggle on a different field. + setRows((prev) => + prev.map((r) => (r.userId === row.userId ? { ...r, [field]: previousValue } : r)) + ); + toast.error("Failed to update class manager access"); + } finally { + setSavingKey(null); + } + } + + if (loading) { + return ( + + + + + Class Managers + + + +

Loading…

+
+
+ ); + } + + return ( + + + + + Class Managers + + + Grant non-admin team members permission to enroll students, mark attendance, + or view the full class roster. Owners and admins always have full access. + + + + {rows.length === 0 ? ( +

+ No other team members yet. +

+ ) : ( + + + + User + Enroll Students + Mark Attendance + View Roster + + + + {rows.map((row) => ( + + +

{row.name || row.email}

+

{row.email}

+
+ + + handleToggle(row, "enrollStudents", checked === true) + } + /> + + + + handleToggle(row, "markAttendance", checked === true) + } + /> + + + + handleToggle(row, "viewRoster", checked === true) + } + /> + +
+ ))} +
+
+ )} +
+ + Owners & Admins + + always have enroll, attendance, and roster access — no grant needed. +
+
+
+ ); +} diff --git a/src/components/dashboard/settings/modal/panels/academic-operations-panel.tsx b/src/components/dashboard/settings/modal/panels/academic-operations-panel.tsx index f5295360..1e2d4928 100644 --- a/src/components/dashboard/settings/modal/panels/academic-operations-panel.tsx +++ b/src/components/dashboard/settings/modal/panels/academic-operations-panel.tsx @@ -2,15 +2,17 @@ import { PanelContent, PanelSection } from "../panel-shell"; import { ClassesManager } from "@/components/dashboard/settings/classes-manager"; +import { ClassManagers } from "@/components/dashboard/settings/class-managers"; import { AgentsManager } from "@/components/dashboard/settings/agents-manager"; import { useSettingsModal } from "@/contexts/settings-modal-context"; import { getFeatureAccess } from "@/industries/_loader"; import { FEATURES } from "@/industries/_registry"; export function AcademicOperationsPanel() { - const { industryId } = useSettingsModal(); + const { industryId, role } = useSettingsModal(); const hasClasses = getFeatureAccess(industryId, FEATURES.CLASSES); const hasApplicationTracking = getFeatureAccess(industryId, FEATURES.APPLICATION_TRACKING); + const isAdminTier = role === "owner" || role === "admin"; return ( @@ -19,6 +21,11 @@ export function AcademicOperationsPanel() { )} + {hasClasses && isAdminTier && ( + + + + )} {hasApplicationTracking && ( diff --git a/src/industries/education-consultancy/features/classes/pages/classes-workspace.tsx b/src/industries/education-consultancy/features/classes/pages/classes-workspace.tsx index 0f51aa9a..ec849e0d 100644 --- a/src/industries/education-consultancy/features/classes/pages/classes-workspace.tsx +++ b/src/industries/education-consultancy/features/classes/pages/classes-workspace.tsx @@ -72,9 +72,13 @@ interface ClassesWorkspaceProps { canEnroll: boolean; canMarkAttendance: boolean; tenantId: string; + /** Owner-only, computed server-side in page.tsx — see comment there for why. */ + canSeeFeesTotals: boolean; + feesCollected: number | null; + classFeePct: Record | null; } -export function ClassesWorkspace({ classes, enrollments: initialEnrollments, canManage, canEnroll, canMarkAttendance }: ClassesWorkspaceProps) { +export function ClassesWorkspace({ classes, enrollments: initialEnrollments, canManage, canEnroll, canMarkAttendance, canSeeFeesTotals, feesCollected, classFeePct }: ClassesWorkspaceProps) { const router = useRouter(); const { openSettings } = useSettingsModal(); const [selectedClassId, setSelectedClassId] = useState(classes[0]?.id ?? null); @@ -148,13 +152,11 @@ export function ClassesWorkspace({ classes, enrollments: initialEnrollments, can const activeLeadIds = new Set(); let demoCount = 0; let demoConvertedCount = 0; - let feesCollected = 0; let unpaidActiveCount = 0; const byLeadClass = new Map(); for (const e of enrollments) { if (e.status === "active") activeLeadIds.add(e.lead_id); - if (e.fee_paid && e.fee_amount != null) feesCollected += e.fee_amount; if (e.status === "active" && !e.fee_paid) unpaidActiveCount++; const key = `${e.lead_id}:${e.class_id}`; @@ -173,21 +175,15 @@ export function ClassesWorkspace({ classes, enrollments: initialEnrollments, can return { activeStudents: activeLeadIds.size, conversionRate: demoCount > 0 ? Math.round((demoConvertedCount / demoCount) * 100) : null, - feesCollected, unpaidActiveCount, }; }, [enrollments]); const classStats = useMemo(() => { - const map: Record = {}; + const map: Record = {}; for (const cls of classes) { const list = enrollmentsByClass[cls.id] ?? []; - const active = list.filter((e) => e.status !== "inactive"); - map[cls.id] = { - count: new Set(list.map((e) => e.lead_id)).size, - paidCount: active.filter((e) => e.fee_paid).length, - payableCount: active.length, - }; + map[cls.id] = { count: new Set(list.map((e) => e.lead_id)).size }; } return map; }, [classes, enrollmentsByClass]); @@ -296,7 +292,12 @@ export function ClassesWorkspace({ classes, enrollments: initialEnrollments, can {/* Stat strip */} -
+
Active students @@ -311,14 +312,16 @@ export function ClassesWorkspace({ classes, enrollments: initialEnrollments, can {workspaceStats.conversionRate == null ? "—" : `${workspaceStats.conversionRate}%`}
-
-
- Fees collected -
-
- {workspaceStats.feesCollected.toLocaleString(undefined, { maximumFractionDigits: 0 })} + {canSeeFeesTotals && ( +
+
+ Fees collected +
+
+ {(feesCollected ?? 0).toLocaleString(undefined, { maximumFractionDigits: 0 })} +
-
+ )}
Unpaid (active) @@ -349,8 +352,7 @@ export function ClassesWorkspace({ classes, enrollments: initialEnrollments, can ) : ( classes.map((cls) => { const count = classStats[cls.id]?.count ?? 0; - const { paidCount, payableCount } = classStats[cls.id] ?? { paidCount: 0, payableCount: 0 }; - const feePct = payableCount > 0 ? Math.round((paidCount / payableCount) * 100) : null; + const feePct = classFeePct?.[cls.id] ?? null; const isActive = cls.id === selectedClassId; const badge = endDateBadge(cls.end_date); return ( @@ -382,16 +384,20 @@ export function ClassesWorkspace({ classes, enrollments: initialEnrollments, can <> · {cls.default_fee.toLocaleString(undefined, { maximumFractionDigits: 0 })} fee )}
-
-
-
-
- Fees collected - {feePct != null ? `${feePct}%` : "—"} -
+ {canSeeFeesTotals && ( + <> +
+
+
+
+ Fees collected + {feePct != null ? `${feePct}%` : "—"} +
+ + )} ); }) diff --git a/src/lib/api/class-attendance.ts b/src/lib/api/class-attendance.ts index 77f78feb..0b5acf0a 100644 --- a/src/lib/api/class-attendance.ts +++ b/src/lib/api/class-attendance.ts @@ -1,20 +1,66 @@ -import type { AuthContext } from "@/lib/api/auth"; -import { scopedClient } from "@/lib/supabase/scoped"; +import { scopedClientForTenant } from "@/lib/supabase/scoped"; /** - * Attendance marking is gated by a dedicated allowlist table - * (class_attendance_markers) rather than a position permission, because it must - * name specific users (Purnima, Kamana, Pratima) without granting the rest of - * their position (counselor) any new access. Owners/admins always pass. + * Minimal shape these helpers need — satisfied by the API-route `AuthContext` + * (src/lib/api/auth.ts) as well as the `getCurrentUserTenant()` Server Component + * shape (src/lib/supabase/queries.ts), which has `role`/`userId`/`tenant.id` + * instead of a flat `tenantId`. Callers pass either directly. */ -export async function canMarkClassAttendance(auth: AuthContext): Promise { +interface ClassAccessSubject { + role: string; + userId: string; + tenantId: string; +} + +/** + * Classes-access capabilities (enroll students, mark attendance, view full + * roster) are gated by the `class_managers` grant table — a per-user, + * per-capability allowlist admins manage from Settings — rather than a + * position permission, because they must name specific users without + * granting the rest of their position (e.g. counselor) any new access. + * Owners/admins always pass. + * + * `class_managers` replaces two older mechanisms: the `class_attendance_markers` + * allowlist table (still read-only/historical — do not write to it) and the + * hardcoded `CLASS_ENROLL_POSITIONS` position-slug list in permissions.ts. + */ +export async function canMarkClassAttendance(auth: ClassAccessSubject): Promise { + if (auth.role === "owner" || auth.role === "admin") return true; + + const db = await scopedClientForTenant(auth.tenantId); + const { data } = await db + .from("class_managers") + .select("user_id") + .eq("user_id", auth.userId) + .eq("mark_attendance", true) + .maybeSingle(); + + return !!data; +} + +export async function canEnrollStudents(auth: ClassAccessSubject): Promise { + if (auth.role === "owner" || auth.role === "admin") return true; + + const db = await scopedClientForTenant(auth.tenantId); + const { data } = await db + .from("class_managers") + .select("user_id") + .eq("user_id", auth.userId) + .eq("enroll_students", true) + .maybeSingle(); + + return !!data; +} + +export async function canViewFullRoster(auth: ClassAccessSubject): Promise { if (auth.role === "owner" || auth.role === "admin") return true; - const db = await scopedClient(auth); + const db = await scopedClientForTenant(auth.tenantId); const { data } = await db - .from("class_attendance_markers") + .from("class_managers") .select("user_id") .eq("user_id", auth.userId) + .eq("view_roster", true) .maybeSingle(); return !!data; diff --git a/src/lib/api/permissions.ts b/src/lib/api/permissions.ts index 63779a34..499e7828 100644 --- a/src/lib/api/permissions.ts +++ b/src/lib/api/permissions.ts @@ -136,10 +136,9 @@ export function canManageClasses(p: ResolvedPermissions): boolean { export function canManageHR(p: ResolvedPermissions): boolean { return p.canManageHR; } -const CLASS_ENROLL_POSITIONS = new Set(["branch-manager", "lead-executive", "counselor", "application-executive"]); -export function canEnrollStudents(p: ResolvedPermissions, positionSlug: string | null | undefined): boolean { - return p.baseTier === "owner" || p.baseTier === "admin" || CLASS_ENROLL_POSITIONS.has(positionSlug ?? ""); -} +// canEnrollStudents moved to src/lib/api/class-attendance.ts — it now reads the +// class_managers grant table (admin-managed, per-user) instead of this hardcoded +// position-slug allowlist. See that file's docstring. export function canAccessPipeline(p: ResolvedPermissions, pipelineId: string): boolean { return p.pipelineAccess === "all" || p.pipelineAccess.ids.has(pipelineId); } diff --git a/src/lib/filters/compile.test.ts b/src/lib/filters/compile.test.ts new file mode 100644 index 00000000..3ad7e009 --- /dev/null +++ b/src/lib/filters/compile.test.ts @@ -0,0 +1,622 @@ +import { describe, it, expect } from "vitest"; +import { compileFilter, planFilter, type QueryBuilder } from "./compile"; +import { FilterCompileError, type CompileCtx, type FieldRegistry, type FilterCondition, type FilterTree } from "./types"; + +// ── Fake builder ───────────────────────────────────────────────────────── +// Deliberately implements ONLY the QueryBuilder interface — no from()/ +// select()/rpc() exist on it at all, so any accidental call to one of those +// inside compileFilter would be a TypeScript compile error, not just a test +// failure. Every call is recorded verbatim so tests can assert exactly which +// path (native builder call vs. constructed .or() string) fired. +class FakeBuilder implements QueryBuilder { + calls: string[] = []; + private record(entry: string): this { + this.calls.push(entry); + return this; + } + eq(c: string, v: unknown): this { + return this.record(`eq(${c},${JSON.stringify(v)})`); + } + neq(c: string, v: unknown): this { + return this.record(`neq(${c},${JSON.stringify(v)})`); + } + is(c: string, v: null | boolean): this { + return this.record(`is(${c},${JSON.stringify(v)})`); + } + in(c: string, vs: readonly unknown[]): this { + return this.record(`in(${c},${JSON.stringify(vs)})`); + } + gt(c: string, v: unknown): this { + return this.record(`gt(${c},${JSON.stringify(v)})`); + } + gte(c: string, v: unknown): this { + return this.record(`gte(${c},${JSON.stringify(v)})`); + } + lt(c: string, v: unknown): this { + return this.record(`lt(${c},${JSON.stringify(v)})`); + } + lte(c: string, v: unknown): this { + return this.record(`lte(${c},${JSON.stringify(v)})`); + } + ilike(c: string, p: string): this { + return this.record(`ilike(${c},${p})`); + } + contains(c: string, v: readonly unknown[] | Record): this { + return this.record(`contains(${c},${JSON.stringify(v)})`); + } + overlaps(c: string, v: readonly unknown[]): this { + return this.record(`overlaps(${c},${JSON.stringify(v)})`); + } + not(c: string, op: string, v: unknown): this { + return this.record(`not(${c},${op},${JSON.stringify(v)})`); + } + or(f: string): this { + return this.record(`or(${f})`); + } + + orPayloads(): string[] { + return this.calls.filter((c) => c.startsWith("or(")).map((c) => c.slice(3, -1)); + } +} + +// ── Fixture registry — one FieldDef per FieldSource kind ──────────────── +const registry: FieldRegistry = { + first_name: { key: "first_name", label: "First name", type: "text", source: { kind: "column", column: "first_name" }, group: "Basic", filterable: true }, + age: { key: "age", label: "Age", type: "number", source: { kind: "column", column: "age" }, group: "Basic", filterable: true }, + created_at: { key: "created_at", label: "Created", type: "date", source: { kind: "column", column: "created_at" }, group: "Dates", filterable: true }, + is_active: { key: "is_active", label: "Active", type: "boolean", source: { kind: "column", column: "is_active" }, group: "Basic", filterable: true }, + industry: { key: "industry", label: "Industry", type: "select", source: { kind: "column", column: "prospect_industry" }, group: "Basic", filterable: true }, + assigned_to: { key: "assigned_to", label: "Assigned to", type: "uuid", source: { kind: "column", column: "assigned_to" }, group: "Basic", filterable: true }, + tags: { key: "tags", label: "Tags", type: "tags", source: { kind: "array_column", column: "tags" }, group: "Basic", filterable: true }, + note: { key: "note", label: "Note", type: "text", source: { kind: "jsonb", column: "custom_fields", path: "note" }, group: "Custom", filterable: true }, + field_of_study: { + key: "field_of_study", + label: "Field of study", + type: "text", + source: { kind: "promoted", column: "field_of_study", jsonb: { column: "custom_fields", path: "field_of_study" } }, + group: "Education", + filterable: true, + }, + destinations: { + key: "destinations", + label: "Destinations", + type: "multiselect", + source: { kind: "promoted", column: "destinations", jsonb: { column: "custom_fields", path: "countries" } }, + group: "Education", + filterable: true, + }, + search: { + key: "search", + label: "Search", + type: "text", + source: { kind: "columns", columns: ["first_name", "last_name"], fullNamePairs: true }, + group: "Basic", + filterable: true, + }, + collaborators: { + key: "collaborators", + label: "Collaborators", + type: "relation", + source: { kind: "embed", relation: "lead_collaborators", column: "user_id", embedSelect: "lead_collaborators!inner(user_id)" }, + group: "Basic", + filterable: true, + }, + status: { + key: "status", + label: "Status", + type: "select", + // key != column trap: stage_id if present, else the legacy `status` column. + source: { + kind: "virtual", + compile: (c: FilterCondition) => { + const val = String(c.value); + if (c.op === "is") return `or(stage_id.eq.${val},and(stage_id.is.null,status.eq.${val}))`; + if (c.op === "is_not") + return `or(stage_id.is.null,and(stage_id.neq.${val},status.is.null),and(stage_id.is.null,status.is.null),and(stage_id.is.null,status.neq.${val}))`; + throw new FilterCompileError(`status virtual field: unsupported op ${c.op}`, "unsupported"); + }, + }, + group: "Basic", + filterable: true, + }, + hidden: { key: "hidden", label: "Hidden", type: "text", source: { kind: "column", column: "secret" }, group: "Basic", filterable: false }, +}; + +const ctx: CompileCtx = { tz: "UTC", now: new Date("2026-01-15T12:00:00.000Z"), industryId: null, permissions: {} }; + +function compile(tree: FilterTree): FakeBuilder { + return compileFilter(new FakeBuilder(), tree, registry, ctx); +} + +function cond(id: string, field: string, op: FilterCondition["op"], value?: FilterCondition["value"]): FilterCondition { + return value === undefined ? { id, field, op } : { id, field, op, value }; +} + +function andTree(...conditions: FilterCondition[]): FilterTree { + return { conjunction: "and", conditions }; +} + +// ── Builder invariant ───────────────────────────────────────────────────── + +describe("compileFilter builder invariant", () => { + it("FakeBuilder has no from/select/rpc — a call to any of them would be a TS error, not a runtime one", () => { + expect((new FakeBuilder() as unknown as Record).from).toBeUndefined(); + expect((new FakeBuilder() as unknown as Record).select).toBeUndefined(); + expect((new FakeBuilder() as unknown as Record).rpc).toBeUndefined(); + }); + + it("returns the SAME builder instance it was given (mutated, not replaced)", () => { + const builder = new FakeBuilder(); + const result = compileFilter(builder, andTree(cond("c1", "first_name", "is", "Jane")), registry, ctx); + expect(result).toBe(builder); + }); + + it("an empty tree makes zero calls", () => { + const builder = compile(andTree()); + expect(builder.calls).toEqual([]); + }); +}); + +// ── Native fast path (pure AND, positive, single-column) ───────────────── + +describe("native fast path — pure AND trees use builder calls, not .or() strings", () => { + it('"is" -> eq()', () => { + const b = compile(andTree(cond("c1", "first_name", "is", "Jane"))); + expect(b.calls).toEqual(['eq(first_name,"Jane")']); + }); + + it('"contains"/"starts_with"/"ends_with" -> ilike()', () => { + expect(compile(andTree(cond("c1", "first_name", "contains", "an"))).calls).toEqual(["ilike(first_name,%an%)"]); + expect(compile(andTree(cond("c1", "first_name", "starts_with", "Ja"))).calls).toEqual(["ilike(first_name,Ja%)"]); + expect(compile(andTree(cond("c1", "first_name", "ends_with", "ne"))).calls).toEqual(["ilike(first_name,%ne)"]); + }); + + it('"gt"/"gte"/"lt"/"lte" -> respective builder methods', () => { + expect(compile(andTree(cond("c1", "age", "gt", 18))).calls).toEqual(["gt(age,18)"]); + expect(compile(andTree(cond("c1", "age", "gte", 18))).calls).toEqual(["gte(age,18)"]); + expect(compile(andTree(cond("c1", "age", "lt", 65))).calls).toEqual(["lt(age,65)"]); + expect(compile(andTree(cond("c1", "age", "lte", 65))).calls).toEqual(["lte(age,65)"]); + }); + + it('"between" -> chained gte().lte()', () => { + const b = compile(andTree(cond("c1", "age", "between", [18, 65]))); + expect(b.calls).toEqual(["gte(age,18)", "lte(age,65)"]); + }); + + it('"is_any_of" on a scalar (uuid) field -> in()', () => { + const b = compile(andTree(cond("c1", "assigned_to", "is_any_of", ["u1", "u2"]))); + expect(b.calls).toEqual(['in(assigned_to,["u1","u2"])']); + }); + + it('"is_any_of" on an array_column (tags) -> overlaps()', () => { + const b = compile(andTree(cond("c1", "tags", "is_any_of", ["vip", "urgent"]))); + expect(b.calls).toEqual(['overlaps(tags,["vip","urgent"])']); + }); + + it('"has_all" on tags -> contains()', () => { + const b = compile(andTree(cond("c1", "tags", "has_all", ["vip", "urgent"]))); + expect(b.calls).toEqual(['contains(tags,["vip","urgent"])']); + }); + + it('"is_true"/"is_false" -> eq(col, bool)', () => { + expect(compile(andTree(cond("c1", "is_active", "is_true"))).calls).toEqual(["eq(is_active,true)"]); + expect(compile(andTree(cond("c1", "is_active", "is_false"))).calls).toEqual(["eq(is_active,false)"]); + }); + + it('"is_empty"/"is_not_empty" on a NON-text, non-array field -> is()/not() single call', () => { + expect(compile(andTree(cond("c1", "assigned_to", "is_empty"))).calls).toEqual(["is(assigned_to,null)"]); + expect(compile(andTree(cond("c1", "assigned_to", "is_not_empty"))).calls).toEqual(['not(assigned_to,is,null)']); + }); + + it("multiple AND conditions all use native calls, chained", () => { + const b = compile(andTree(cond("c1", "first_name", "is", "Jane"), cond("c2", "age", "gte", 18))); + expect(b.calls).toEqual(['eq(first_name,"Jane")', "gte(age,18)"]); + expect(b.orPayloads()).toEqual([]); + }); +}); + +// ── String path — conditions that need compound predicates ─────────────── + +describe("string path — conditions requiring compound predicates use .or()", () => { + it('"is_empty" on TEXT includes the blank-string leg (NULL-or-empty-string), via .or()', () => { + const b = compile(andTree(cond("c1", "first_name", "is_empty"))); + expect(b.calls).toHaveLength(1); + expect(b.calls[0]).toBe('or(or(first_name.is.null,first_name.eq.""))'); + }); + + it('"is_not_empty" on TEXT is an and() of not-null and not-blank, via .or()', () => { + const b = compile(andTree(cond("c1", "first_name", "is_not_empty"))); + expect(b.calls[0]).toBe('or(and(first_name.not.is.null,first_name.neq.""))'); + }); + + it('"is_empty" on an array_column emits the bare {} literal, via .or()', () => { + const b = compile(andTree(cond("c1", "tags", "is_empty"))); + expect(b.calls[0]).toBe("or(or(tags.is.null,tags.eq.{}))"); + }); + + it('"is_not_empty" on an array_column, via .or()', () => { + const b = compile(andTree(cond("c1", "tags", "is_not_empty"))); + expect(b.calls[0]).toBe("or(and(tags.not.is.null,tags.neq.{}))"); + }); + + it("a jsonb-kind field renders a ->> accessor", () => { + const b = compile(andTree(cond("c1", "note", "is", "hello"))); + expect(b.calls[0]).toBe('or(custom_fields->>note.eq.hello)'); + }); + + it("date operators always go through the string path (never native)", () => { + const b = compile(andTree(cond("c1", "created_at", "before", "2026-01-01T00:00:00.000Z"))); + expect(b.calls[0]).toMatch(/^or\(created_at\.lt\./); + }); +}); + +// ── Negation includes empty rows — the #1 correctness trap ─────────────── + +describe("negation includes empty rows", () => { + const negativeCases: { op: FilterCondition["op"]; field: string; value: FilterCondition["value"] }[] = [ + { op: "is_not", field: "first_name", value: "Jane" }, + { op: "is_not", field: "age", value: 5 as unknown as string }, // number type via "is"/"is_not" scalarValue + { op: "is_not", field: "assigned_to", value: "u1" }, + { op: "is_not", field: "industry", value: "engineering" }, + { op: "not_contains", field: "first_name", value: "an" }, + { op: "is_none_of", field: "assigned_to", value: ["u1", "u2"] }, + { op: "is_none_of", field: "tags", value: ["vip"] }, + ]; + + it.each(negativeCases)("$op on $field compiles to or(.is.null, )", ({ op, field, value }) => { + const b = compile(andTree(cond("c1", field, op, value))); + expect(b.calls).toHaveLength(1); + const col = registry[field].source.kind === "array_column" || registry[field].source.kind === "column" ? (registry[field].source as { column: string }).column : ""; + // The rendered predicate is `or(.is.null,)`, itself wrapped + // once more by applyConditionToBuilder's own .or(predicate) call — so the + // NULL leg appears immediately after the INNER or(, not the outer one. + expect(b.calls[0]).toContain(`or(${col}.is.null,`); + }); + + it('"is_not" on a NULL row must be INCLUDED, not excluded — .is.null is the first OR leg', () => { + const b = compile(andTree(cond("c1", "industry", "is_not", "engineering"))); + expect(b.calls[0]).toBe("or(or(prospect_industry.is.null,prospect_industry.neq.engineering))"); + }); + + it('"not_contains" NULL-inclusive form', () => { + const b = compile(andTree(cond("c1", "first_name", "not_contains", "an"))); + expect(b.calls[0]).toBe("or(or(first_name.is.null,first_name.not.ilike.%an%))"); + }); + + it('"is_none_of" on a scalar column NULL-inclusive form', () => { + const b = compile(andTree(cond("c1", "assigned_to", "is_none_of", ["u1", "u2"]))); + expect(b.calls[0]).toBe("or(or(assigned_to.is.null,assigned_to.not.in.(u1,u2)))"); + }); + + it('"is_none_of" on an array_column (tags) NULL-inclusive form uses .not.ov.', () => { + const b = compile(andTree(cond("c1", "tags", "is_none_of", ["vip"]))); + expect(b.calls[0]).toBe("or(or(tags.is.null,tags.not.ov.{vip}))"); + }); +}); + +// ── Promoted dual-read (legacy custom_fields trap) — both polarities ───── + +describe("promoted field dual-read (legacy custom_fields trap)", () => { + it("positive op ORs the real column and the legacy jsonb leg (field_of_study, text)", () => { + const b = compile(andTree(cond("c1", "field_of_study", "is", "Computer Science"))); + expect(b.calls[0]).toBe('or(or(field_of_study.eq."Computer Science",custom_fields->>field_of_study.eq."Computer Science"))'); + }); + + it("negative op ANDs the two NEGATED, NULL-inclusive legs — De Morgan (field_of_study, text)", () => { + const b = compile(andTree(cond("c1", "field_of_study", "is_not", "Computer Science"))); + expect(b.calls[0]).toBe( + 'or(and(or(field_of_study.is.null,field_of_study.neq."Computer Science"),or(custom_fields->>field_of_study.is.null,custom_fields->>field_of_study.neq."Computer Science")))' + ); + }); + + it("a legacy-only row (no real column value) still matches a positive promoted filter", () => { + // This test documents the CONTRACT, not live data: the compiled predicate + // ORs both legs, so a caller running it against a row where only + // custom_fields.field_of_study is set will still match — verified by the + // shape of the OR above (real leg does not gate the json leg). + const b = compile(andTree(cond("c1", "field_of_study", "is", "Nursing"))); + expect(b.calls[0]).toContain("custom_fields->>field_of_study.eq.Nursing"); + }); + + it("positive op ORs both legs for a promoted ARRAY field (destinations/countries — path != column name)", () => { + const b = compile(andTree(cond("c1", "destinations", "is_any_of", ["Australia", "Canada"]))); + expect(b.calls[0]).toBe("or(or(destinations.ov.{Australia,Canada},custom_fields->>countries.ov.{Australia,Canada}))"); + }); + + it("negative op ANDs the two negated legs for a promoted ARRAY field (is_none_of)", () => { + const b = compile(andTree(cond("c1", "destinations", "is_none_of", ["Australia"]))); + expect(b.calls[0]).toBe( + "or(and(or(destinations.is.null,destinations.not.ov.{Australia}),or(custom_fields->>countries.is.null,custom_fields->>countries.not.ov.{Australia})))" + ); + }); +}); + +// ── is_any_of [] must throw, never silently no-op ───────────────────────── + +describe("is_any_of with an empty value array", () => { + it("throws FilterCompileError rather than compiling to a silent no-op filter", () => { + expect(() => compile(andTree(cond("c1", "assigned_to", "is_any_of", [])))).toThrow(FilterCompileError); + }); + + it("also throws for is_none_of [] and has_all [] hand-built trees", () => { + expect(() => compile(andTree(cond("c1", "assigned_to", "is_none_of", [])))).toThrow(FilterCompileError); + expect(() => compile(andTree(cond("c1", "tags", "has_all", [])))).toThrow(FilterCompileError); + }); +}); + +// ── is_none_of rejected on a relation (embed) field ─────────────────────── + +describe("is_none_of on a relation field", () => { + it("is rejected — !inner + not.in means 'has a collaborator who isn't X', not 'has none of X'", () => { + expect(() => compile(andTree(cond("c1", "collaborators", "is_none_of", ["u1"])))).toThrow(FilterCompileError); + }); + + it("is_any_of on the same relation field IS supported", () => { + const b = compile(andTree(cond("c1", "collaborators", "is_any_of", ["u1"]))); + expect(b.calls[0]).toBe('or(lead_collaborators.user_id.in.(u1))'); + }); +}); + +// ── Unknown / not-filterable / disallowed-operator rejection ───────────── + +describe("registry and operator gating", () => { + it("throws for a field key not present in the registry", () => { + expect(() => compile(andTree(cond("c1", "does_not_exist", "is", "x")))).toThrow(FilterCompileError); + }); + + it("throws for a field marked filterable: false", () => { + expect(() => compile(andTree(cond("c1", "hidden", "is", "x")))).toThrow(FilterCompileError); + }); + + it("throws when the operator isn't allowed for the field's type (has_all on a uuid field)", () => { + expect(() => compile(andTree(cond("c1", "assigned_to", "has_all", ["u1"])))).toThrow(FilterCompileError); + }); + + it("throws when the operator isn't allowed for the field's type (gt on a text field)", () => { + expect(() => compile(andTree(cond("c1", "first_name", "gt" as FilterCondition["op"], "x")))).toThrow(FilterCompileError); + }); +}); + +// ── Virtual field (key != column trap) ──────────────────────────────────── + +describe("virtual field source", () => { + it("delegates entirely to the field's own compile() function", () => { + const b = compile(andTree(cond("c1", "status", "is", "new"))); + expect(b.calls[0]).toBe("or(or(stage_id.eq.new,and(stage_id.is.null,status.eq.new)))"); + }); +}); + +// ── Search field (columns kind) + full-name-pair matching ──────────────── + +describe("columns-kind field (multi-column search)", () => { + it("ORs a single-token search across every column", () => { + const b = compile(andTree(cond("c1", "search", "contains", "jane"))); + expect(b.calls[0]).toBe("or(or(first_name.ilike.%jane%,last_name.ilike.%jane%))"); + }); + + it("adds full-name token-pair legs for a two-token search, in either order", () => { + const b = compile(andTree(cond("c1", "search", "contains", "Jane Smith"))); + const payload = b.orPayloads()[0]; + expect(payload).toContain("and(first_name.ilike.%Jane%,last_name.ilike.%Smith%)"); + expect(payload).toContain("and(first_name.ilike.%Smith%,last_name.ilike.%Jane%)"); + }); + + it("does not add pair legs for a single-token search", () => { + const b = compile(andTree(cond("c1", "search", "contains", "jane"))); + expect(b.orPayloads()[0]).not.toContain("and("); + }); +}); + +// ── OR groups — group semantics ─────────────────────────────────────────── + +describe("group semantics", () => { + it("root conjunction 'or' combines root conditions into ONE .or() call", () => { + const tree: FilterTree = { + conjunction: "or", + conditions: [cond("c1", "industry", "is", "engineering"), cond("c2", "industry", "is", "design")], + }; + const b = compile(tree); + expect(b.calls).toEqual(["or(or(prospect_industry.eq.engineering,prospect_industry.eq.design))"]); + }); + + it("root 'and' + a groups[] entry with conjunction 'or' -> native root call + one separate .or() for the group", () => { + const tree: FilterTree = { + conjunction: "and", + conditions: [cond("c1", "first_name", "is", "Jane")], + groups: [{ conjunction: "or", conditions: [cond("g1", "industry", "is", "engineering"), cond("g2", "industry", "is", "design")] }], + }; + const b = compile(tree); + expect(b.calls).toEqual(['eq(first_name,"Jane")', "or(or(prospect_industry.eq.engineering,prospect_industry.eq.design))"]); + }); + + it("a groups[] entry with conjunction 'and' applies its conditions natively too, ANDed with everything else", () => { + const tree: FilterTree = { + conjunction: "and", + conditions: [cond("c1", "first_name", "is", "Jane")], + groups: [{ conjunction: "and", conditions: [cond("g1", "age", "gte", 18)] }], + }; + const b = compile(tree); + expect(b.calls).toEqual(['eq(first_name,"Jane")', "gte(age,18)"]); + }); + + it("multiple OR groups each produce their own separate .or() call", () => { + const tree: FilterTree = { + conjunction: "and", + conditions: [], + groups: [ + { conjunction: "or", conditions: [cond("g1", "industry", "is", "a"), cond("g2", "industry", "is", "b")] }, + { conjunction: "or", conditions: [cond("g3", "tags", "is_any_of", ["x"]), cond("g4", "tags", "is_any_of", ["y"])] }, + ], + }; + const b = compile(tree); + expect(b.calls).toHaveLength(2); + expect(b.calls[0]).toContain("prospect_industry"); + expect(b.calls[1]).toContain("tags"); + }); + + it("an empty root + empty groups makes no calls at all", () => { + const b = compile({ conjunction: "and", conditions: [], groups: [{ conjunction: "or", conditions: [] }] }); + expect(b.calls).toEqual([]); + }); +}); + +// ── Dates: frozen ctx.now, tz-aware day boundaries, DST ─────────────────── + +describe("dates — tz-aware boundaries with a frozen ctx.now", () => { + function onCtx(tz: string): CompileCtx { + return { ...ctx, tz }; + } + + it('"on" in UTC — a plain 24h day', () => { + const b = compileFilter(new FakeBuilder(), andTree(cond("c1", "created_at", "on", "2026-06-15")), registry, onCtx("UTC")); + expect(b.calls[0]).toBe('or(and(created_at.gte."2026-06-15T00:00:00.000Z",created_at.lt."2026-06-16T00:00:00.000Z"))'); + }); + + it('"on" in Asia/Kathmandu (UTC+5:45, no DST) — boundaries shifted by the fixed offset', () => { + const b = compileFilter(new FakeBuilder(), andTree(cond("c1", "created_at", "on", "2026-06-15")), registry, onCtx("Asia/Kathmandu")); + expect(b.calls[0]).toBe('or(and(created_at.gte."2026-06-14T18:15:00.000Z",created_at.lt."2026-06-15T18:15:00.000Z"))'); + }); + + it('"on" in America/New_York on an ordinary (non-transition) day — EDT, UTC-4', () => { + const b = compileFilter(new FakeBuilder(), andTree(cond("c1", "created_at", "on", "2026-06-15")), registry, onCtx("America/New_York")); + expect(b.calls[0]).toBe('or(and(created_at.gte."2026-06-15T04:00:00.000Z",created_at.lt."2026-06-16T04:00:00.000Z"))'); + }); + + it('"on" in America/New_York on the SPRING-FORWARD DST transition day (2026-03-08) — a 23h wall-clock day', () => { + const b = compileFilter(new FakeBuilder(), andTree(cond("c1", "created_at", "on", "2026-03-08")), registry, onCtx("America/New_York")); + // A naive `end = start + 24h` would compute 2026-03-09T05:00:00.000Z, + // leaking one hour of the FOLLOWING day into this filter. The correct end + // is the next day's OWN local midnight under the new EDT offset: + // 2026-03-09T04:00:00.000Z — a 23-hour wall-clock day. + expect(b.calls[0]).toBe('or(and(created_at.gte."2026-03-08T05:00:00.000Z",created_at.lt."2026-03-09T04:00:00.000Z"))'); + }); + + it('"on" in America/New_York on the FALL-BACK DST transition day (2026-11-01) — a 25h wall-clock day', () => { + const b = compileFilter(new FakeBuilder(), andTree(cond("c1", "created_at", "on", "2026-11-01")), registry, onCtx("America/New_York")); + expect(b.calls[0]).toBe('or(and(created_at.gte."2026-11-01T04:00:00.000Z",created_at.lt."2026-11-02T05:00:00.000Z"))'); + }); + + it('"date_between" spans from the first date\'s start to the second date\'s end', () => { + const b = compileFilter( + new FakeBuilder(), + andTree(cond("c1", "created_at", "date_between", ["2026-06-15", "2026-06-17"])), + registry, + onCtx("UTC") + ); + expect(b.calls[0]).toBe('or(and(created_at.gte."2026-06-15T00:00:00.000Z",created_at.lt."2026-06-18T00:00:00.000Z"))'); + }); + + it('"within_last" uses ctx.now, never Date.now() — deterministic across runs', () => { + const b = compileFilter(new FakeBuilder(), andTree(cond("c1", "created_at", "within_last", "7d")), registry, onCtx("UTC")); + expect(b.calls[0]).toBe('or(and(created_at.gte."2026-01-08T12:00:00.000Z",created_at.lte."2026-01-15T12:00:00.000Z"))'); + }); + + it('"within_next" uses ctx.now for both bounds', () => { + const b = compileFilter(new FakeBuilder(), andTree(cond("c1", "created_at", "within_next", "3m")), registry, onCtx("UTC")); + expect(b.calls[0]).toBe('or(and(created_at.gte."2026-01-15T12:00:00.000Z",created_at.lte."2026-04-15T12:00:00.000Z"))'); + }); + + it('"within_last" with a year unit', () => { + const b = compileFilter(new FakeBuilder(), andTree(cond("c1", "created_at", "within_last", "1y")), registry, onCtx("UTC")); + expect(b.calls[0]).toBe('or(and(created_at.gte."2025-01-15T12:00:00.000Z",created_at.lte."2026-01-15T12:00:00.000Z"))'); + }); + + it('"before" and "after" compile to a direct instant comparison', () => { + const b1 = compileFilter(new FakeBuilder(), andTree(cond("c1", "created_at", "before", "2026-01-01T00:00:00.000Z")), registry, onCtx("UTC")); + expect(b1.calls[0]).toBe('or(created_at.lt."2026-01-01T00:00:00.000Z")'); + const b2 = compileFilter(new FakeBuilder(), andTree(cond("c1", "created_at", "after", "2026-01-01T00:00:00.000Z")), registry, onCtx("UTC")); + expect(b2.calls[0]).toBe('or(created_at.gt."2026-01-01T00:00:00.000Z")'); + }); + + it('"is_empty"/"is_not_empty" on a date field', () => { + expect(compileFilter(new FakeBuilder(), andTree(cond("c1", "created_at", "is_empty")), registry, onCtx("UTC")).calls[0]).toBe("or(created_at.is.null)"); + expect(compileFilter(new FakeBuilder(), andTree(cond("c1", "created_at", "is_not_empty")), registry, onCtx("UTC")).calls[0]).toBe( + "or(created_at.not.is.null)" + ); + }); + + it("rejects a malformed relative-date value", () => { + expect(() => + compileFilter(new FakeBuilder(), andTree(cond("c1", "created_at", "within_last", "bogus")), registry, onCtx("UTC")) + ).toThrow(FilterCompileError); + }); +}); + +// ── planFilter: validate-everything-up-front + embed collection ────────── + +describe("planFilter", () => { + it("returns ok:true with an empty embeds list for a tree with no embed-kind conditions", () => { + const result = planFilter(andTree(cond("c1", "first_name", "is", "Jane")), registry, ctx); + expect(result).toEqual({ ok: true, embeds: [] }); + }); + + it("collects the embedSelect string for an embed-kind condition", () => { + const result = planFilter(andTree(cond("c1", "collaborators", "is_any_of", ["u1"])), registry, ctx); + expect(result).toEqual({ ok: true, embeds: ["lead_collaborators!inner(user_id)"] }); + }); + + it("dedupes the same embed across multiple conditions on the same relation", () => { + const result = planFilter( + { conjunction: "and", conditions: [cond("c1", "collaborators", "is_any_of", ["u1"])], groups: [{ conjunction: "and", conditions: [cond("c2", "collaborators", "is_any_of", ["u2"])] }] }, + registry, + ctx + ); + expect(result).toEqual({ ok: true, embeds: ["lead_collaborators!inner(user_id)"] }); + }); + + it("reports an unknown field as an error keyed by the condition's field, not a throw", () => { + const result = planFilter(andTree(cond("c1", "nope", "is", "x")), registry, ctx); + expect(result.ok).toBe(false); + if (!result.ok) expect(result.errors.nope?.[0]).toMatch(/unknown filter field/); + }); + + it("reports a not-filterable field as an error, not a throw", () => { + const result = planFilter(andTree(cond("c1", "hidden", "is", "x")), registry, ctx); + expect(result.ok).toBe(false); + if (!result.ok) expect(result.errors.hidden?.[0]).toMatch(/not filterable/); + }); + + it("reports a disallowed operator as an error", () => { + const result = planFilter(andTree(cond("c1", "age", "contains", "5")), registry, ctx); + expect(result.ok).toBe(false); + if (!result.ok) expect(result.errors.age?.[0]).toMatch(/operator contains is not allowed/); + }); + + it("reports is_none_of on a relation field as an error, matching compileFilter's rejection", () => { + const result = planFilter(andTree(cond("c1", "collaborators", "is_none_of", ["u1"])), registry, ctx); + expect(result.ok).toBe(false); + if (!result.ok) expect(result.errors.collaborators?.[0]).toMatch(/is_none_of is not allowed|is_none_of is not supported/); + }); + + it("reports an empty list value for is_any_of as an error rather than a silent no-op", () => { + const result = planFilter(andTree(cond("c1", "tags", "is_any_of", [])), registry, ctx); + expect(result.ok).toBe(false); + if (!result.ok) expect(result.errors.tags?.[0]).toMatch(/requires at least one value/); + }); + + it("collects EVERY error across multiple bad conditions in one pass, not just the first", () => { + const result = planFilter(andTree(cond("c1", "nope", "is", "x"), cond("c2", "hidden", "is", "y")), registry, ctx); + expect(result.ok).toBe(false); + if (!result.ok) { + expect(Object.keys(result.errors).sort()).toEqual(["hidden", "nope"]); + } + }); + + it("denies a field whose visibleTo predicate rejects the caller's permissions", () => { + const gatedRegistry: FieldRegistry = { + ...registry, + gated: { + key: "gated", + label: "Gated", + type: "text", + source: { kind: "column", column: "secret2" }, + group: "Basic", + filterable: true, + visibleTo: () => false, + }, + }; + const result = planFilter(andTree(cond("c1", "gated", "is", "x")), gatedRegistry, ctx); + expect(result.ok).toBe(false); + if (!result.ok) expect(result.errors.gated?.[0]).toMatch(/not accessible/); + }); +}); diff --git a/src/lib/filters/compile.ts b/src/lib/filters/compile.ts new file mode 100644 index 00000000..b5333bda --- /dev/null +++ b/src/lib/filters/compile.ts @@ -0,0 +1,580 @@ +import { and, arrayLiteral, EMPTY_ARRAY_LITERAL, or, pgCol, pgLike, pgVal } from "./pgrst"; +import { isOperatorAllowed } from "./operators"; +import { + FilterCompileError, + NEGATIVE_OPERATORS, + type CompileCtx, + type FieldDef, + type FieldRegistry, + type FilterCondition, + type FilterTree, +} from "./types"; + +// compileFilter() — the one predicate implementation all four hand-maintained +// mirrors (getLeads/getLeadsPage, the route.ts inline chain, lead_aggregates(), +// search-leads.ts) are meant to eventually route through. +// +// THE INVARIANT THAT KEEPS TENANT ISOLATION INTACT — DO NOT VIOLATE IT: +// compileFilter receives a builder and returns a builder. It must NEVER call +// .from(), .select() or .rpc(). Tenant scoping, the leads_visible_to_user RPC, +// pipeline/list allow-lists and shared-pool logic all stay exactly where they +// are, applied by the CALLER before/after this function runs. A compiler that +// constructs its own base query will either leak the whole tenant to a +// counselor or return zero rows — both have precedent in this repo. +// +// THE NEGATION RULE — the #1 correctness trap, and it is deliberate: +// In SQL, `col <> 'x'` evaluates to NULL (i.e. EXCLUDES the row) when +// `col IS NULL`. So a naive "status is not Contacted" silently hides every +// lead with no status — users read that as data loss. Every operator in +// NEGATIVE_OPERATORS (is_not, not_contains, is_none_of) compiles to +// `or(.is.null, )`. Notion, Airtable and Twenty all behave +// this way. See compile.test.ts's "negation includes empty rows" suite. +// +// GROUP SEMANTICS (this module's own contract — there are zero consumers yet, +// so this is the first place these semantics are defined): +// - `tree.conditions` combine with each other via `tree.conjunction`. +// - Each entry in `tree.groups` is an independent bracketed clause (its own +// conjunction governs the conditions inside it). +// - The root-conditions clause and every group clause are ANDed together — +// i.e. "(root conditions joined by tree.conjunction) AND (group 1) AND +// (group 2) AND …". This matches the Notion/Twenty UX: a top-level +// AND/OR toggle plus independent "+ Add filter group" blocks. + +export interface QueryBuilder { + eq(column: string, value: unknown): this; + neq(column: string, value: unknown): this; + is(column: string, value: null | boolean): this; + in(column: string, values: readonly unknown[]): this; + gt(column: string, value: unknown): this; + gte(column: string, value: unknown): this; + lt(column: string, value: unknown): this; + lte(column: string, value: unknown): this; + ilike(column: string, pattern: string): this; + contains(column: string, value: readonly unknown[] | Record): this; + overlaps(column: string, value: readonly unknown[]): this; + not(column: string, operator: string, value: unknown): this; + or(filters: string): this; +} + +function requireField(registry: FieldRegistry, key: string): FieldDef { + const field = registry[key]; + if (!field) throw new FilterCompileError(`unknown filter field: ${JSON.stringify(key)}`, "unknown_field"); + if (!field.filterable) throw new FilterCompileError(`field is not filterable: ${JSON.stringify(key)}`, "not_filterable"); + return field; +} + +function requireOperator(field: FieldDef, cond: FilterCondition): void { + if (!isOperatorAllowed(field, cond.op)) { + throw new FilterCompileError(`operator ${cond.op} is not allowed on field ${field.key}`, "operator_not_allowed"); + } + if (field.source.kind === "embed" && cond.op === "is_none_of") { + // !inner + not.in means "has *a* collaborator who isn't X" — semantically + // wrong (and duplicates parent rows), not "has none of X". Reject even if + // a caller's registry override mistakenly allow-lists it. + throw new FilterCompileError("is_none_of is not supported on relation fields", "unsupported"); + } +} + +function isNegative(op: FilterCondition["op"]): boolean { + return NEGATIVE_OPERATORS.includes(op); +} + +// ── Date-window helpers (tz-aware, ctx.now-injected — never Date.now()) ───── + +// `dateStr`'s first 10 chars (YYYY-MM-DD) are the target LOCAL calendar date +// in `tz` — taken as-is, never round-tripped through `new Date(dateStr)` +// first. Doing that round-trip would anchor the string to UTC midnight and +// then re-derive the date by formatting in `tz`, which silently shifts the +// calendar day for any negative-offset zone (e.g. UTC midnight formatted in +// America/New_York is the previous evening) — exactly the trap this function +// exists to avoid. +function localMidnightUtc(dateStr: string, tz: string): Date { + const datePart = dateStr.slice(0, 10); + // naiveUtc: the target date's midnight, misinterpreted as a UTC instant. + const naiveUtc = new Date(`${datePart}T00:00:00.000Z`); + // Format that (wrong) instant in `tz`, then re-parse those wall-clock digits + // as if THEY were UTC — the difference from naiveUtc is exactly tz's offset + // at this date (correct across DST since it's evaluated at this specific date). + const asTzWallClock = naiveUtc.toLocaleString("sv-SE", { timeZone: tz }).replace(" ", "T") + "Z"; + const offsetMs = naiveUtc.getTime() - new Date(asTzWallClock).getTime(); + return new Date(naiveUtc.getTime() + offsetMs); +} + +function nextCalendarDate(dateStr: string): string { + const d = new Date(`${dateStr.slice(0, 10)}T00:00:00.000Z`); + d.setUTCDate(d.getUTCDate() + 1); + return d.toISOString().slice(0, 10); +} + +// `end` is the NEXT calendar date's own local midnight — NOT `start + 24h`. +// On a DST transition day (e.g. America/New_York, second Sunday of March) +// the wall-clock day is only 23 hours; a fixed +24h offset would silently +// leak one hour of the FOLLOWING day into an "on " filter. See +// compile.test.ts's DST-transition-day coverage. +function dayBoundsInTz(dateStr: string, tz: string): { start: string; end: string } { + return { + start: localMidnightUtc(dateStr, tz).toISOString(), + end: localMidnightUtc(nextCalendarDate(dateStr), tz).toISOString(), + }; +} + +function parseRelativeWindow(value: string): { amount: number; unit: "d" | "m" | "y" } { + const match = /^(\d+)([dmy])$/.exec(value); + if (!match) throw new FilterCompileError(`invalid relative date value: ${JSON.stringify(value)}`, "invalid_value"); + return { amount: Number(match[1]), unit: match[2] as "d" | "m" | "y" }; +} + +function addRelativeWindow(base: Date, value: string): Date { + const { amount, unit } = parseRelativeWindow(value); + const result = new Date(base.getTime()); + if (unit === "d") result.setUTCDate(result.getUTCDate() + amount); + else if (unit === "m") result.setUTCMonth(result.getUTCMonth() + amount); + else result.setUTCFullYear(result.getUTCFullYear() + amount); + return result; +} + +// ── Predicate rendering: FieldDef + FilterCondition -> a single pgrst string ─ +// (or a list of legs, for the "promoted" dual-read trap, which the caller +// combines via De Morgan.) + +function renderScalarOpAgainstColumn( + col: string, + op: FilterCondition["op"], + value: FilterCondition["value"], + field: FieldDef, + isArrayColumn: boolean +): string { + switch (op) { + case "is": + return `${col}.eq.${pgVal(String(value))}`; + case "is_not": + return `${col}.neq.${pgVal(String(value))}`; + case "is_empty": + if (isArrayColumn) return or(`${col}.is.null`, `${col}.eq.${EMPTY_ARRAY_LITERAL}`); + return field.type === "text" || field.emptyIsBlankString === true + ? or(`${col}.is.null`, `${col}.eq.${pgVal("")}`) + : `${col}.is.null`; + case "is_not_empty": + if (isArrayColumn) return and(`${col}.not.is.null`, `${col}.neq.${EMPTY_ARRAY_LITERAL}`); + return field.type === "text" || field.emptyIsBlankString === true + ? and(`${col}.not.is.null`, `${col}.neq.${pgVal("")}`) + : `${col}.not.is.null`; + case "contains": + return `${col}.ilike.${pgLike(String(value), "contains")}`; + case "not_contains": + return `${col}.not.ilike.${pgLike(String(value), "contains")}`; + case "starts_with": + return `${col}.ilike.${pgLike(String(value), "prefix")}`; + case "ends_with": + return `${col}.ilike.${pgLike(String(value), "suffix")}`; + case "gt": + return `${col}.gt.${pgVal(String(value))}`; + case "gte": + return `${col}.gte.${pgVal(String(value))}`; + case "lt": + return `${col}.lt.${pgVal(String(value))}`; + case "lte": + return `${col}.lte.${pgVal(String(value))}`; + case "between": { + const [min, max] = value as [number, number]; + return and(`${col}.gte.${pgVal(String(min))}`, `${col}.lte.${pgVal(String(max))}`); + } + case "is_true": + return `${col}.eq.true`; + case "is_false": + return `${col}.eq.false`; + default: + throw new FilterCompileError(`operator ${op} has no scalar rendering`, "unsupported"); + } +} + +function renderListOpAgainstColumn(col: string, op: FilterCondition["op"], values: string[], isArrayColumn: boolean): string { + if (isArrayColumn) { + switch (op) { + case "is_any_of": + return `${col}.ov.${arrayLiteral(values)}`; + case "is_none_of": + return `${col}.not.ov.${arrayLiteral(values)}`; + case "has_all": + return `${col}.cs.${arrayLiteral(values)}`; + default: + throw new FilterCompileError(`operator ${op} has no array rendering`, "unsupported"); + } + } + switch (op) { + case "is_any_of": + return `${col}.in.(${values.map(pgVal).join(",")})`; + case "is_none_of": + return `${col}.not.in.(${values.map(pgVal).join(",")})`; + case "has_all": + // Scalar columns can't "contain all" of a list of discrete values — + // registries must not offer has_all on a non-array field. + throw new FilterCompileError("has_all requires an array_column/tags field", "unsupported"); + default: + throw new FilterCompileError(`operator ${op} has no list rendering`, "unsupported"); + } +} + +function renderDateOpAgainstColumn(col: string, op: FilterCondition["op"], value: FilterCondition["value"], ctx: CompileCtx): string { + switch (op) { + case "is_empty": + return `${col}.is.null`; + case "is_not_empty": + return `${col}.not.is.null`; + case "before": + return `${col}.lt.${pgVal(new Date(String(value)).toISOString())}`; + case "after": + return `${col}.gt.${pgVal(new Date(String(value)).toISOString())}`; + case "on": { + const { start, end } = dayBoundsInTz(String(value), ctx.tz); + return and(`${col}.gte.${pgVal(start)}`, `${col}.lt.${pgVal(end)}`); + } + case "date_between": { + const [from, to] = value as [string, string]; + const startBound = dayBoundsInTz(from, ctx.tz); + const endBound = dayBoundsInTz(to, ctx.tz); + return and(`${col}.gte.${pgVal(startBound.start)}`, `${col}.lt.${pgVal(endBound.end)}`); + } + case "within_last": { + const since = subtractRelativeWindow(ctx.now, String(value)); + return and(`${col}.gte.${pgVal(since.toISOString())}`, `${col}.lte.${pgVal(ctx.now.toISOString())}`); + } + case "within_next": { + const until = addRelativeWindow(ctx.now, String(value)); + return and(`${col}.gte.${pgVal(ctx.now.toISOString())}`, `${col}.lte.${pgVal(until.toISOString())}`); + } + default: + throw new FilterCompileError(`operator ${op} has no date rendering`, "unsupported"); + } +} + +function subtractRelativeWindow(base: Date, value: string): Date { + const { amount, unit } = parseRelativeWindow(value); + const result = new Date(base.getTime()); + if (unit === "d") result.setUTCDate(result.getUTCDate() - amount); + else if (unit === "m") result.setUTCMonth(result.getUTCMonth() - amount); + else result.setUTCFullYear(result.getUTCFullYear() - amount); + return result; +} + +function renderAgainstColumn(col: string, field: FieldDef, cond: FilterCondition): string { + const isArrayColumn = field.source.kind === "array_column" || field.type === "tags" || field.type === "multiselect"; + if (field.type === "date") return renderDateOpAgainstColumn(col, cond.op, cond.value, currentCtxRef); + if (Array.isArray(cond.value) && (cond.op === "is_any_of" || cond.op === "is_none_of" || cond.op === "has_all")) { + return renderListOpAgainstColumn(col, cond.op, cond.value as string[], isArrayColumn); + } + return renderScalarOpAgainstColumn(col, cond.op, cond.value, field, isArrayColumn); +} + +// `renderAgainstColumn` needs ctx for date math but stays a narrow function for +// the non-date branches; threading ctx through every call site would obscure +// the branch structure above, so it's captured via a module-scoped ref that +// `compileCondition` sets immediately before rendering. Never read outside a +// synchronous render call. +let currentCtxRef: CompileCtx; + +// Positive-vs-negative wrapping: negative operators always include the NULL +// leg. This wrapping happens ONCE, at the top, so every column-kind branch +// below stays free of the trap. +function renderConditionPredicate(col: string, field: FieldDef, cond: FilterCondition): string { + const rendered = renderAgainstColumn(col, field, cond); + if (!isNegative(cond.op)) return rendered; + return or(`${col}.is.null`, rendered); +} + +function renderPromotedPredicate(field: FieldDef & { source: Extract }, cond: FilterCondition): string { + const realCol = field.source.column; + const jsonCol = pgCol(field.source.jsonb.column, field.source.jsonb.path); + const realLeg = renderAgainstColumn(realCol, field, cond); + const jsonLeg = renderAgainstColumn(jsonCol, field, cond); + + if (!isNegative(cond.op)) { + // Positive ops OR the two legs — either the real column or the legacy + // custom_fields path satisfying the condition is enough. Without this, + // legacy Admizz education rows silently vanish from e.g. "Field of study + // is X" because their data lives only in custom_fields. + return or(realLeg, jsonLeg); + } + + // Negative ops AND the two NEGATED legs (De Morgan): NOT(a OR b) = NOT a AND + // NOT b. Each leg itself still gets the NULL-inclusive wrapping so a row + // missing BOTH the real column and the legacy path still matches "is not X". + const negRealLeg = or(`${realCol}.is.null`, realLeg); + const negJsonLeg = or(`${jsonCol}.is.null`, jsonLeg); + return and(negRealLeg, negJsonLeg); +} + +function renderColumnsPredicate(field: FieldDef & { source: Extract }, cond: FilterCondition): string { + const { columns, fullNamePairs } = field.source; + const isNeg = isNegative(cond.op); + // Render each column's leg using the RAW operator (never wrapped here) — the + // null-inclusive wrapping is applied once, below, at the combination step, + // not per-leg, because for a multi-column field the trap fix isn't "wrap + // each leg" (that would compute an OR of ORs) but "AND the null-inclusive + // negation of every column" (De Morgan over the whole set) — see the isNeg + // branch below. + const legs = columns.map((c) => renderAgainstColumn(c, field, cond)); + + if (!isNeg && fullNamePairs && columns.length >= 2 && typeof cond.value === "string") { + // "John Smith" won't match any single column — match token pairs against + // the first two columns in either order (mirrors route.ts's full-name + // search special-case). Only meaningful for a positive "contains"-style + // match; there is no sound negation of a token-pair match, so it's + // skipped for negative operators. + const tokens = cond.value.trim().split(/\s+/).filter(Boolean); + if (tokens.length >= 2) { + const [a, b] = columns; + const [t1, t2] = tokens; + legs.push( + and(renderAgainstColumn(a, field, { ...cond, value: t1 }), renderAgainstColumn(b, field, { ...cond, value: t2 })), + and(renderAgainstColumn(a, field, { ...cond, value: t2 }), renderAgainstColumn(b, field, { ...cond, value: t1 })) + ); + } + } + + if (!isNeg) return or(...legs); + + // Negative: "does not match in ANY column" = AND over columns of + // (column IS NULL OR column does-not-match) — a row with a NULL column is + // never, by itself, a reason to exclude the row from a "not contains" match. + const negatedLegs = columns.map((c, i) => or(`${c}.is.null`, legs[i])); + return and(...negatedLegs); +} + +function renderCondition(field: FieldDef, cond: FilterCondition, ctx: CompileCtx): string { + currentCtxRef = ctx; + + switch (field.source.kind) { + case "column": + return renderConditionPredicate(field.source.column, field, cond); + case "array_column": + return renderConditionPredicate(field.source.column, field, cond); + case "jsonb": + return renderConditionPredicate(pgCol(field.source.column, field.source.path), field, cond); + case "promoted": + return renderPromotedPredicate(field as FieldDef & { source: Extract }, cond); + case "columns": + return renderColumnsPredicate(field as FieldDef & { source: Extract }, cond); + case "embed": { + // Dotted relation.column filtering an embedded resource. The caller is + // responsible for including that relation in the select with `!inner` + // (e.g. `lead_collaborators!inner(user_id)`) — the compiler never calls + // .select(), so it cannot arrange that itself. is_none_of is rejected in + // requireOperator() before this is reached. + const dotted = `${field.source.relation}.${field.source.column}`; + return renderConditionPredicate(dotted, field, cond); + } + case "virtual": + // The field owns its own translation (e.g. status -> stage_id ?? status). + // Negation-NULL wrapping is the field's responsibility here since only it + // knows the real column(s) involved. + return field.source.compile(cond, ctx); + default: { + const exhaustive: never = field.source; + throw new FilterCompileError(`unhandled field source kind: ${JSON.stringify(exhaustive)}`, "unsupported"); + } + } +} + +function resolveAndValidate(registry: FieldRegistry, cond: FilterCondition): FieldDef { + const field = requireField(registry, cond.field); + requireOperator(field, cond); + + if ((cond.op === "is_any_of" || cond.op === "is_none_of" || cond.op === "has_all") && Array.isArray(cond.value) && cond.value.length === 0) { + // Belt-and-suspenders: schema.ts already enforces .min(1), but a caller + // that hand-builds a tree (bypassing decodeFilterTree) must not get a + // silent no-op — the empty-pipeline-allow-list incident is exactly this + // shape of bug, just on a different filter axis. + throw new FilterCompileError(`${cond.op} requires at least one value`, "invalid_value"); + } + + return field; +} + +type SimpleColumnField = FieldDef & { source: Extract }; + +// A condition is eligible for the NATIVE fast path — a plain .eq/.in/.gte/… +// builder call instead of a constructed filter string — only when it is a +// single-column, single-call, positive predicate. Everything else (negative +// operators needing the NULL-inclusive OR, multi-leg promoted/columns fields, +// embedded-relation filters, and date operators which all need tz-aware +// string construction) falls back to the string path via a per-condition +// `.or(predicate)` call, which still ANDs correctly with every other filter +// param PostgREST sees — see the module doc comment's "GROUP SEMANTICS". +function isNativeEligible(field: FieldDef, cond: FilterCondition): boolean { + if (isNegative(cond.op)) return false; + if (field.source.kind !== "column" && field.source.kind !== "array_column") return false; + if (field.type === "date") return false; + if ( + (cond.op === "is_empty" || cond.op === "is_not_empty") && + (field.type === "text" || field.source.kind === "array_column" || field.emptyIsBlankString === true) + ) { + return false; // needs a compound OR/AND (NULL-or-blank), not a single call + } + return true; +} + +function applyNative(builder: B, field: SimpleColumnField, cond: FilterCondition): B { + const col = field.source.column; + const isArrayColumn = field.source.kind === "array_column"; + switch (cond.op) { + case "is": + return builder.eq(col, cond.value); + case "is_empty": + return builder.is(col, null); + case "is_not_empty": + return builder.not(col, "is", null); + case "contains": + return builder.ilike(col, pgLike(String(cond.value), "contains")); + case "starts_with": + return builder.ilike(col, pgLike(String(cond.value), "prefix")); + case "ends_with": + return builder.ilike(col, pgLike(String(cond.value), "suffix")); + case "gt": + return builder.gt(col, cond.value); + case "gte": + return builder.gte(col, cond.value); + case "lt": + return builder.lt(col, cond.value); + case "lte": + return builder.lte(col, cond.value); + case "between": { + const [min, max] = cond.value as [number, number]; + return builder.gte(col, min).lte(col, max); + } + case "is_any_of": { + const values = cond.value as string[]; + return isArrayColumn ? builder.overlaps(col, values) : builder.in(col, values); + } + case "has_all": { + const values = cond.value as string[]; + return builder.contains(col, values); + } + case "is_true": + return builder.eq(col, true); + case "is_false": + return builder.eq(col, false); + default: + throw new FilterCompileError(`operator ${cond.op} has no native rendering`, "unsupported"); + } +} + +function applyConditionToBuilder(builder: B, registry: FieldRegistry, cond: FilterCondition, ctx: CompileCtx): B { + const field = resolveAndValidate(registry, cond); + + if (isNativeEligible(field, cond)) { + return applyNative(builder, field as SimpleColumnField, cond); + } + + const predicate = renderCondition(field, cond, ctx); + return builder.or(predicate); +} + +function applyAndConditions(builder: B, registry: FieldRegistry, conditions: FilterCondition[], ctx: CompileCtx): B { + let out = builder; + for (const cond of conditions) out = applyConditionToBuilder(out, registry, cond, ctx); + return out; +} + +// A true OR clause cannot be expressed as separate builder calls (those AND +// together) — every condition in the clause is rendered to a predicate string +// and combined into ONE `.or(...)` call. +function applyOrConditions(builder: B, registry: FieldRegistry, conditions: FilterCondition[], ctx: CompileCtx): B { + if (conditions.length === 0) return builder; + const parts = conditions.map((cond) => { + const field = resolveAndValidate(registry, cond); + return renderCondition(field, cond, ctx); + }); + return builder.or(or(...parts)); +} + +/** + * compileFilter(builder, tree, registry, ctx) -> builder + * + * Receives a builder, returns a builder. Never calls .from()/.select()/.rpc() — + * see the module doc comment above for why that invariant is load-bearing. + * + * Pure-AND trees use native builder calls wherever a condition allows it (the + * fast path — see isNativeEligible). Root-level OR and each `groups[]` entry + * with conjunction "or" fall back to one constructed `.or(...)` filter string + * per clause, per the module doc comment's GROUP SEMANTICS. + */ +export function compileFilter(builder: B, tree: FilterTree, registry: FieldRegistry, ctx: CompileCtx): B { + let out = + tree.conjunction === "and" + ? applyAndConditions(builder, registry, tree.conditions, ctx) + : applyOrConditions(builder, registry, tree.conditions, ctx); + + for (const group of tree.groups ?? []) { + out = + group.conjunction === "and" + ? applyAndConditions(out, registry, group.conditions, ctx) + : applyOrConditions(out, registry, group.conditions, ctx); + } + + return out; +} + +export type PlanFilterResult = { ok: true; embeds: string[] } | { ok: false; errors: Record }; + +// planFilter() validates every condition in a tree UP FRONT and collects every +// error, instead of compileFilter's throw-on-first-bad-condition — a caller +// building a route response needs every validation problem in a single 422, +// not just the first one. It also answers the ordering question a caller like +// the leads route has: `.select()` must know about any `embed`-kind condition +// (to add the `!inner(...)` join) BEFORE compileFilter ever runs, since +// compileFilter itself never calls .select() (see the module doc comment). +function checkCondition( + cond: FilterCondition, + registry: FieldRegistry, + ctx: CompileCtx, + errors: Record, + embeds: Set +): void { + const push = (msg: string) => (errors[cond.field] ??= []).push(msg); + + const field = registry[cond.field]; + if (!field) { + push(`unknown filter field: ${JSON.stringify(cond.field)}`); + return; + } + if (!field.filterable) { + push(`field is not filterable: ${JSON.stringify(cond.field)}`); + return; + } + if (field.visibleTo && !field.visibleTo(ctx.permissions)) { + push(`field is not accessible: ${JSON.stringify(cond.field)}`); + return; + } + if (!isOperatorAllowed(field, cond.op)) { + push(`operator ${cond.op} is not allowed on field ${field.key}`); + return; + } + if (field.source.kind === "embed" && cond.op === "is_none_of") { + push("is_none_of is not supported on relation fields"); + return; + } + if ( + (cond.op === "is_any_of" || cond.op === "is_none_of" || cond.op === "has_all") && + Array.isArray(cond.value) && + cond.value.length === 0 + ) { + push(`${cond.op} requires at least one value`); + return; + } + + if (field.source.kind === "embed") embeds.add(field.source.embedSelect); +} + +export function planFilter(tree: FilterTree, registry: FieldRegistry, ctx: CompileCtx): PlanFilterResult { + const errors: Record = {}; + const embeds = new Set(); + + for (const cond of tree.conditions) checkCondition(cond, registry, ctx, errors, embeds); + for (const group of tree.groups ?? []) { + for (const cond of group.conditions) checkCondition(cond, registry, ctx, errors, embeds); + } + + if (Object.keys(errors).length > 0) return { ok: false, errors }; + return { ok: true, embeds: Array.from(embeds) }; +} diff --git a/src/lib/filters/legacy-leads-params.test.ts b/src/lib/filters/legacy-leads-params.test.ts new file mode 100644 index 00000000..15e942dc --- /dev/null +++ b/src/lib/filters/legacy-leads-params.test.ts @@ -0,0 +1,161 @@ +import { describe, it, expect } from "vitest"; +import { legacyLeadsParamsToTree } from "./legacy-leads-params"; + +function sp(entries: Record): URLSearchParams { + return new URLSearchParams(entries); +} + +describe("legacyLeadsParamsToTree", () => { + it("returns an empty and-tree when no params are present", () => { + const tree = legacyLeadsParamsToTree(sp({})); + expect(tree).toEqual({ conjunction: "and", conditions: [] }); + }); + + it("maps status -> field:status op:is", () => { + const tree = legacyLeadsParamsToTree(sp({ status: "new" })); + expect(tree.conditions).toContainEqual({ id: "legacy:status", field: "status", op: "is", value: "new" }); + }); + + it("ignores status=all", () => { + const tree = legacyLeadsParamsToTree(sp({ status: "all" })); + expect(tree.conditions.find((c) => c.field === "status")).toBeUndefined(); + }); + + it("maps search -> field:search op:contains, trimmed", () => { + const tree = legacyLeadsParamsToTree(sp({ search: " jane doe " })); + expect(tree.conditions).toContainEqual({ id: "legacy:search", field: "search", op: "contains", value: "jane doe" }); + }); + + it("ignores a blank/whitespace-only search", () => { + const tree = legacyLeadsParamsToTree(sp({ search: " " })); + expect(tree.conditions.find((c) => c.field === "search")).toBeUndefined(); + }); + + it("maps form -> field:form op:is, ignoring 'all'", () => { + expect(legacyLeadsParamsToTree(sp({ form: "abc-123" })).conditions).toContainEqual({ + id: "legacy:form", + field: "form", + op: "is", + value: "abc-123", + }); + expect(legacyLeadsParamsToTree(sp({ form: "all" })).conditions).toEqual([]); + }); + + it("maps tag -> field:tags op:has_all with a single-item array, ignoring 'all'", () => { + expect(legacyLeadsParamsToTree(sp({ tag: "vip" })).conditions).toContainEqual({ + id: "legacy:tag", + field: "tags", + op: "has_all", + value: ["vip"], + }); + expect(legacyLeadsParamsToTree(sp({ tag: "all" })).conditions).toEqual([]); + }); + + it.each([ + ["today", "1d"], + ["week", "7d"], + ["month", "30d"], + ])("maps created=%s -> field:created op:within_last value:%s", (window, relative) => { + expect(legacyLeadsParamsToTree(sp({ created: window })).conditions).toContainEqual({ + id: "legacy:created", + field: "created", + op: "within_last", + value: relative, + }); + }); + + it("ignores created=all and an unrecognized window", () => { + expect(legacyLeadsParamsToTree(sp({ created: "all" })).conditions).toEqual([]); + expect(legacyLeadsParamsToTree(sp({ created: "decade" })).conditions).toEqual([]); + }); + + it("maps industry -> field:industry op:is, ignoring 'all'", () => { + expect(legacyLeadsParamsToTree(sp({ industry: "engineering" })).conditions).toContainEqual({ + id: "legacy:industry", + field: "industry", + op: "is", + value: "engineering", + }); + }); + + it("maps industry=__none__ -> field:industry op:is_empty, no value", () => { + const tree = legacyLeadsParamsToTree(sp({ industry: "__none__" })); + expect(tree.conditions).toContainEqual({ id: "legacy:industry", field: "industry", op: "is_empty" }); + }); + + it("maps source (csv) -> field:source op:is_any_of", () => { + expect(legacyLeadsParamsToTree(sp({ source: "web, referral ,ads" })).conditions).toContainEqual({ + id: "legacy:source", + field: "source", + op: "is_any_of", + value: ["web", "referral", "ads"], + }); + }); + + it("omits source condition entirely when the csv is empty", () => { + expect(legacyLeadsParamsToTree(sp({ source: "" })).conditions.find((c) => c.field === "source")).toBeUndefined(); + }); + + it("maps assignees (csv, incl. 'unassigned' token) -> field:assignees op:is_any_of", () => { + expect(legacyLeadsParamsToTree(sp({ assignees: "unassigned,11111111-1111-1111-1111-111111111111" })).conditions).toContainEqual({ + id: "legacy:assignees", + field: "assignees", + op: "is_any_of", + value: ["unassigned", "11111111-1111-1111-1111-111111111111"], + }); + }); + + it("maps collaborators (csv) -> field:collaborators op:is_any_of", () => { + expect(legacyLeadsParamsToTree(sp({ collaborators: "22222222-2222-2222-2222-222222222222" })).conditions).toContainEqual({ + id: "legacy:collaborators", + field: "collaborators", + op: "is_any_of", + value: ["22222222-2222-2222-2222-222222222222"], + }); + }); + + it("combines every param into one AND'd root tree", () => { + const tree = legacyLeadsParamsToTree( + sp({ status: "new", search: "jane", form: "f1", tag: "vip", created: "week", industry: "eng", source: "web", assignees: "a1", collaborators: "c1" }) + ); + expect(tree.conjunction).toBe("and"); + expect(tree.groups).toBeUndefined(); + expect(tree.conditions).toHaveLength(9); + }); + + describe("scope params are never included", () => { + const scopeParams = { + list: "prospects", + funnel: "sales", + stage: "stage-id", + branch_id: "branch-1", + assigned_to: "user-1", // singular scope — NOT the plural `assignees` toolbar filter + include_converted: "1", + page: "2", + pageSize: "50", + count: "0", + sort: "created_at", + order: "desc", + facets: "source", + }; + + it("produces zero conditions from a params object containing ONLY scope params", () => { + const tree = legacyLeadsParamsToTree(sp(scopeParams)); + expect(tree.conditions).toEqual([]); + }); + + it("scope params don't leak in even when toolbar filters are also present", () => { + const tree = legacyLeadsParamsToTree(sp({ ...scopeParams, status: "new" })); + expect(tree.conditions).toHaveLength(1); + expect(tree.conditions[0].field).toBe("status"); + for (const key of Object.keys(scopeParams)) { + expect(tree.conditions.some((c) => c.field === key)).toBe(false); + } + }); + + it("assigned_to (scope, singular) never produces an 'assignees' condition", () => { + const tree = legacyLeadsParamsToTree(sp({ assigned_to: "user-1" })); + expect(tree.conditions.find((c) => c.field === "assignees")).toBeUndefined(); + }); + }); +}); diff --git a/src/lib/filters/legacy-leads-params.ts b/src/lib/filters/legacy-leads-params.ts new file mode 100644 index 00000000..7049a826 --- /dev/null +++ b/src/lib/filters/legacy-leads-params.ts @@ -0,0 +1,95 @@ +import type { FilterCondition, FilterTree } from "./types"; + +// Converts the ~9 existing /api/v1/leads toolbar params into a FilterTree — +// the whole de-duplication strategy (see docs/ADVANCED-FILTERS-BRIEF.md). Once +// a route builds a tree from either `?f=` or these legacy params and runs both +// through the same compileFilter(), the existing route.test.ts suite becomes a +// full-fidelity regression harness for the compiler against real production +// semantics. +// +// Field keys here ("status", "search", "tags", …) are REGISTRY keys, resolved +// against whatever FieldRegistry a later phase supplies — this module does not +// own or import a registry itself (there isn't one yet in Phase 1). +// +// SCOPE PARAMS ARE NEVER INCLUDED. `list`, `funnel`, `stage`, `branch_id`, +// `assigned_to` (singular — scope), `include_converted`, `page`, `pageSize`, +// `count`, `sort`, `order`, `facets` are applied by the route as query SCOPE, +// not as filter conditions. In particular the pipeline allow-list must stay a +// route-applied predicate so `pipelineAccess.ids === []` keeps failing closed +// (`.in("pipeline_id", [])` -> 0 rows) — folding it into the tree would risk an +// "optimize away empty .in()" refactor silently leaking the tenant. Note that +// `assignees` (plural, CSV, may include the literal token "unassigned") is a +// DIFFERENT param from the scope-only `assigned_to` and IS a toolbar filter. + +const CREATED_WINDOW_TO_RELATIVE: Record = { + today: "1d", + week: "7d", + month: "30d", +}; + +function csv(raw: string | null): string[] { + if (!raw) return []; + return raw + .split(",") + .map((s) => s.trim()) + .filter(Boolean); +} + +function condition(id: string, field: string, op: FilterCondition["op"], value?: FilterCondition["value"]): FilterCondition { + return value === undefined ? { id, field, op } : { id, field, op, value }; +} + +export function legacyLeadsParamsToTree(sp: URLSearchParams): FilterTree { + const conditions: FilterCondition[] = []; + + const status = sp.get("status"); + if (status && status !== "all") { + conditions.push(condition("legacy:status", "status", "is", status)); + } + + const search = sp.get("search"); + if (search && search.trim()) { + conditions.push(condition("legacy:search", "search", "contains", search.trim())); + } + + const form = sp.get("form"); + if (form && form !== "all") { + conditions.push(condition("legacy:form", "form", "is", form)); + } + + const tag = sp.get("tag"); + if (tag && tag !== "all") { + conditions.push(condition("legacy:tag", "tags", "has_all", [tag])); + } + + const created = sp.get("created"); + if (created && created !== "all" && CREATED_WINDOW_TO_RELATIVE[created]) { + conditions.push(condition("legacy:created", "created", "within_last", CREATED_WINDOW_TO_RELATIVE[created])); + } + + const industry = sp.get("industry"); + if (industry && industry !== "all") { + if (industry === "__none__") { + conditions.push(condition("legacy:industry", "industry", "is_empty")); + } else { + conditions.push(condition("legacy:industry", "industry", "is", industry)); + } + } + + const source = csv(sp.get("source")); + if (source.length > 0) { + conditions.push(condition("legacy:source", "source", "is_any_of", source)); + } + + const assignees = csv(sp.get("assignees")); + if (assignees.length > 0) { + conditions.push(condition("legacy:assignees", "assignees", "is_any_of", assignees)); + } + + const collaborators = csv(sp.get("collaborators")); + if (collaborators.length > 0) { + conditions.push(condition("legacy:collaborators", "collaborators", "is_any_of", collaborators)); + } + + return { conjunction: "and", conditions }; +} diff --git a/src/lib/filters/operators.ts b/src/lib/filters/operators.ts new file mode 100644 index 00000000..53bd956e --- /dev/null +++ b/src/lib/filters/operators.ts @@ -0,0 +1,54 @@ +import type { FieldDef, FilterFieldType, FilterOperator } from "./types"; + +// The operator x field-type mapping table. `is_none_of` is deliberately absent +// from `relation` — see compile.ts: `!inner` + `not.in` on an embedded relation +// means "has *a* row that isn't X", which is semantically wrong (and duplicates +// parent rows), not "has none of X". The UI must never offer it for a relation +// field, and the compiler 422s it if a caller tries anyway. +export const OPERATORS_BY_TYPE: Record = { + text: ["is", "is_not", "is_empty", "is_not_empty", "contains", "not_contains", "starts_with", "ends_with"], + number: ["is", "is_not", "is_empty", "is_not_empty", "gt", "gte", "lt", "lte", "between"], + date: ["is_empty", "is_not_empty", "before", "after", "on", "date_between", "within_last", "within_next"], + boolean: ["is_true", "is_false"], + select: ["is", "is_not", "is_empty", "is_not_empty", "is_any_of", "is_none_of"], + multiselect: ["is_any_of", "is_none_of", "has_all", "is_empty", "is_not_empty"], + uuid: ["is", "is_not", "is_empty", "is_not_empty", "is_any_of", "is_none_of"], + tags: ["has_all", "is_any_of", "is_none_of", "is_empty", "is_not_empty"], + relation: ["is_any_of", "is_empty", "is_not_empty"], +}; + +export function operatorsForField(field: FieldDef): FilterOperator[] { + return field.operators ?? OPERATORS_BY_TYPE[field.type]; +} + +export function isOperatorAllowed(field: FieldDef, op: FilterOperator): boolean { + return operatorsForField(field).includes(op); +} + +// Operators whose schema shape carries no `value` at all. +export const NO_VALUE_OPERATORS: readonly FilterOperator[] = ["is_empty", "is_not_empty", "is_true", "is_false"] as const; + +// Operators whose value is a non-empty string[] (bounded — see schema.ts). +export const LIST_VALUE_OPERATORS: readonly FilterOperator[] = ["is_any_of", "is_none_of", "has_all"] as const; + +// Operators whose value is a single scalar (string | number). +export const SCALAR_VALUE_OPERATORS: readonly FilterOperator[] = [ + "is", + "is_not", + "contains", + "not_contains", + "starts_with", + "ends_with", + "gt", + "gte", + "lt", + "lte", + "before", + "after", + "on", + "within_last", + "within_next", +] as const; + +// Operators whose value is a 2-tuple. +export const TUPLE_VALUE_OPERATORS: readonly FilterOperator[] = ["between", "date_between"] as const; diff --git a/src/lib/filters/pgrst.test.ts b/src/lib/filters/pgrst.test.ts new file mode 100644 index 00000000..b1a9ff2b --- /dev/null +++ b/src/lib/filters/pgrst.test.ts @@ -0,0 +1,202 @@ +import { describe, it, expect } from "vitest"; +import { and, arrayLiteral, EMPTY_ARRAY_LITERAL, not, or, pgCol, pgLike, pgVal } from "./pgrst"; +import { FilterCompileError } from "./types"; + +describe("pgVal", () => { + it("passes through a plain value unquoted", () => { + expect(pgVal("hello")).toBe("hello"); + expect(pgVal("abc123")).toBe("abc123"); + }); + + it("quotes an empty string as a bare pair of quotes", () => { + expect(pgVal("")).toBe('""'); + }); + + it.each([ + ["a,b", '"a,b"'], + ["a.b", '"a.b"'], + ["a:b", '"a:b"'], + ["a(b)", '"a(b)"'], + ['a"b', '"a\\"b"'], + ["a'b", "\"a'b\""], + ["a\\b", '"a\\\\b"'], + ["a{b}", '"a{b}"'], + ["a[b]", '"a[b]"'], + ["a b", '"a b"'], + ["a\nb", '"a\nb"'], + ])("quotes and escapes %j -> %j", (input, expected) => { + expect(pgVal(input)).toBe(expected); + }); + + it("escapes backslashes before quotes so the quote escape can't be forged", () => { + // A naively-ordered escape (quotes first, then backslashes) would let an + // attacker-controlled backslash absorb the escaping backslash meant for a + // quote. Verify backslash-then-quote escaping survives round-trip logic: + // every literal `"` in the input must appear as `\"` in the output, and + // every literal `\` must appear as `\\`. + const input = 'a\\"b'; + const out = pgVal(input); + expect(out).toBe('"a\\\\\\"b"'); + }); + + it("handles unicode without corrupting it", () => { + expect(pgVal("héllo")).toBe("héllo"); + expect(pgVal("日本語, test")).toBe('"日本語, test"'); + }); + + it("handles a 200-char value", () => { + const long = "a".repeat(200); + expect(pgVal(long)).toBe(long); + const longWithComma = "a".repeat(199) + ","; + expect(pgVal(longWithComma)).toBe(`"${longWithComma}"`); + }); + + describe("injection probes", () => { + it("cannot break out of the value position with a filter-string payload", () => { + const payload = "a,tenant_id.neq.x"; + const out = pgVal(payload); + // The whole thing must be one quoted unit — no bare, unescaped `,` that + // a naive concatenation into `col.eq.` could parse as a second + // filter clause. + expect(out).toBe('"a,tenant_id.neq.x"'); + expect(out.startsWith('"')).toBe(true); + expect(out.endsWith('"')).toBe(true); + // No unescaped quote in the middle that could terminate the value early. + const inner = out.slice(1, -1); + expect(inner.match(/(? { + const payload = '",tenant_id.neq.x,"'; + const out = pgVal(payload); + const inner = out.slice(1, -1); + expect(inner.match(/(? { + const payload = "x),or(tenant_id.neq.y"; + const out = pgVal(payload); + expect(out).toBe(`"${payload}"`); + }); + }); +}); + +describe("pgLike", () => { + it("wraps a contains pattern in %...%", () => { + expect(pgLike("abc", "contains")).toBe("%abc%"); + }); + + it("wraps a prefix pattern as val%", () => { + expect(pgLike("abc", "prefix")).toBe("abc%"); + }); + + it("wraps a suffix pattern as %val", () => { + expect(pgLike("abc", "suffix")).toBe("%abc"); + }); + + it("leaves an exact pattern unwrapped", () => { + expect(pgLike("abc", "exact")).toBe("abc"); + }); + + it("escapes the user's OWN % before adding ours, so a literal % isn't a wildcard", () => { + // Escaped pattern contains a backslash, so the final pgVal() pass also + // quotes it and doubles that backslash — see the "pgLike always quotes + // through pgVal" note below. + expect(pgLike("50%", "contains")).toBe('"%50\\\\%%"'); + }); + + it("escapes the user's OWN _ before adding ours", () => { + expect(pgLike("a_b", "contains")).toBe('"%a\\\\_b%"'); + }); + + it("escapes a literal backslash in the user's input", () => { + expect(pgLike("a\\b", "contains")).toBe('"%a\\\\\\\\b%"'); + }); + + it("quotes the final pattern when it needs quoting (e.g. contains a comma)", () => { + expect(pgLike("a,b", "contains")).toBe('"%a,b%"'); + }); + + it("does not mangle o'brien@x.co.uk — the live bug this file fixes", () => { + // route.ts's `search.replace(/[,().]/g, "")` used to silently delete + // characters from legitimate input. pgLike must preserve every character, + // only escaping/quoting as needed. + const out = pgLike("o'brien@x.co.uk", "contains"); + expect(out).toContain("o'brien@x.co.uk"); + expect(out).toBe("\"%o'brien@x.co.uk%\""); + }); + + it("handles a 200-char value", () => { + const long = "b".repeat(200); + expect(pgLike(long, "contains")).toBe(`%${long}%`); + }); +}); + +describe("pgCol", () => { + it("returns the bare column when no jsonPath is given", () => { + expect(pgCol("status")).toBe("status"); + }); + + it("builds a ->> accessor for a valid jsonb key", () => { + expect(pgCol("custom_fields", "field_of_study")).toBe("custom_fields->>field_of_study"); + }); + + it("accepts alphanumeric + underscore keys up to 64 chars", () => { + const key = "a".repeat(64); + expect(pgCol("custom_fields", key)).toBe(`custom_fields->>${key}`); + }); + + it("rejects a 65-char key", () => { + expect(() => pgCol("custom_fields", "a".repeat(65))).toThrow(FilterCompileError); + }); + + it("rejects a jsonPath attempting injection via special characters", () => { + expect(() => pgCol("custom_fields", "x->>tenant_id.neq.y")).toThrow(FilterCompileError); + expect(() => pgCol("custom_fields", "x'; drop table leads;--")).toThrow(FilterCompileError); + expect(() => pgCol("custom_fields", "")).toThrow(FilterCompileError); + }); +}); + +describe("and / or / not combinators", () => { + it("returns the bare predicate unwrapped when there is exactly one", () => { + expect(and("a.eq.1")).toBe("a.eq.1"); + expect(or("a.eq.1")).toBe("a.eq.1"); + }); + + it("wraps multiple predicates in and(...)", () => { + expect(and("a.eq.1", "b.eq.2")).toBe("and(a.eq.1,b.eq.2)"); + }); + + it("wraps multiple predicates in or(...)", () => { + expect(or("a.eq.1", "b.eq.2")).toBe("or(a.eq.1,b.eq.2)"); + }); + + it("supports nesting and() inside or()", () => { + expect(or("a.eq.1", and("b.eq.2", "c.eq.3"))).toBe("or(a.eq.1,and(b.eq.2,c.eq.3))"); + }); + + it("prefixes a predicate with not.", () => { + expect(not("a.eq.1")).toBe("not.a.eq.1"); + }); +}); + +describe("array literals", () => { + it("EMPTY_ARRAY_LITERAL is a bare constant", () => { + expect(EMPTY_ARRAY_LITERAL).toBe("{}"); + }); + + it("builds a brace-delimited literal from plain values", () => { + expect(arrayLiteral(["a", "b", "c"])).toBe("{a,b,c}"); + }); + + it("escapes an element that itself needs quoting", () => { + expect(arrayLiteral(["a,b", "c"])).toBe('{"a,b",c}'); + }); + + it("never builds a literal from an untrusted raw string directly", () => { + // arrayLiteral always goes element-by-element through pgVal — there is no + // path that concatenates a raw joined string. + const out = arrayLiteral(["}, tenant_id.neq.x, {"]); + expect(out).toBe('{"}, tenant_id.neq.x, {"}'); + }); +}); diff --git a/src/lib/filters/pgrst.ts b/src/lib/filters/pgrst.ts new file mode 100644 index 00000000..d2e7e861 --- /dev/null +++ b/src/lib/filters/pgrst.ts @@ -0,0 +1,63 @@ +import { FilterCompileError } from "./types"; + +// ★ SECURITY-CRITICAL FILE ★ +// +// Three rules, in this order. Get these wrong and we ship a filter-injection hole: +// +// 1. Column names are NEVER derived from input. Every column string that reaches +// these functions comes from a FieldSource literal in a field registry — +// resolved and validated in compile.ts before any of these are called. There +// must be no code path that splices an attacker string into the column +// position. Same discipline as the existing SORT_COLUMNS allow-list +// (route.ts:102-108) and its 422 regression test. +// 2. Operators are allow-listed per field type via isOperatorAllowed() (see +// operators.ts) BEFORE compilation — not enforced here, enforced by the caller. +// 3. Values are always escaped — never "sanitized by deletion." This fixes a +// live bug: `search.replace(/[,().]/g, "")` (route.ts:379) silently mangles +// legitimate input like `o'brien@x.co.uk`. Proper quoting replaces deletion. + +const NEEDS_QUOTE = /[,.:()"'\\{}[\]\s]/; + +export function pgVal(raw: string): string { + if (raw === "") return '""'; + if (!NEEDS_QUOTE.test(raw)) return raw; + return `"${raw.replace(/\\/g, "\\\\").replace(/"/g, '\\"')}"`; +} + +export type LikeMode = "contains" | "prefix" | "suffix" | "exact"; + +export function pgLike(raw: string, mode: LikeMode): string { + // Escape the USER's own wildcard characters first, so a search for a literal + // "50%" or "a_b" doesn't turn into a wildcard match… + const lit = raw.replace(/([\\%_])/g, "\\$1"); + const pat = mode === "contains" ? `%${lit}%` : mode === "prefix" ? `${lit}%` : mode === "suffix" ? `%${lit}` : lit; + // …THEN add ours outside the escape, and quote the whole thing through pgVal + // so a value containing `,` `(` `)` etc. still can't break out of the value + // position. + return pgVal(pat); +} + +const JSONB_KEY_RE = /^[a-z0-9_]{1,64}$/i; + +export function pgCol(column: string, jsonPath?: string): string { + if (jsonPath === undefined) return column; + if (!JSONB_KEY_RE.test(jsonPath)) { + throw new FilterCompileError(`invalid jsonb key: ${JSON.stringify(jsonPath)}`, "invalid_value"); + } + return `${column}->>${jsonPath}`; +} + +export const and = (...predicates: string[]): string => (predicates.length === 1 ? predicates[0] : `and(${predicates.join(",")})`); + +export const or = (...predicates: string[]): string => (predicates.length === 1 ? predicates[0] : `or(${predicates.join(",")})`); + +export const not = (predicate: string): string => `not.${predicate}`; + +// Array literals — `tags.eq.{}` for "array is empty" — are always a bare +// constant built from nothing but the array's own element count, never from +// input values. +export const EMPTY_ARRAY_LITERAL = "{}"; + +export function arrayLiteral(values: string[]): string { + return `{${values.map((v) => pgVal(v)).join(",")}}`; +} diff --git a/src/lib/filters/registry/index.ts b/src/lib/filters/registry/index.ts new file mode 100644 index 00000000..7a5e92d1 --- /dev/null +++ b/src/lib/filters/registry/index.ts @@ -0,0 +1 @@ +export { leadFields } from "./leads"; diff --git a/src/lib/filters/registry/leads.test.ts b/src/lib/filters/registry/leads.test.ts new file mode 100644 index 00000000..b1f950a0 --- /dev/null +++ b/src/lib/filters/registry/leads.test.ts @@ -0,0 +1,151 @@ +import { describe, it, expect } from "vitest"; +import { compileFilter, planFilter, type QueryBuilder } from "../compile"; +import { FilterCompileError, type CompileCtx, type FilterCondition, type FilterTree } from "../types"; +import { leadFields } from "./leads"; + +// Spot-checks that the leads registry's legacy-facing fields (status, source, +// assignees) compile to the SAME predicate route.ts's hand-rolled chain emits +// today. status/source are strict single-column (status / intake_source) — +// see the doc comment at the top of leads.ts for why value-shape dispatch was +// tried and reverted. + +class FakeBuilder implements QueryBuilder { + calls: string[] = []; + private record(entry: string): this { + this.calls.push(entry); + return this; + } + eq(c: string, v: unknown): this { + return this.record(`eq(${c},${JSON.stringify(v)})`); + } + neq(c: string, v: unknown): this { + return this.record(`neq(${c},${JSON.stringify(v)})`); + } + is(c: string, v: null | boolean): this { + return this.record(`is(${c},${JSON.stringify(v)})`); + } + in(c: string, vs: readonly unknown[]): this { + return this.record(`in(${c},${JSON.stringify(vs)})`); + } + gt(c: string, v: unknown): this { + return this.record(`gt(${c},${JSON.stringify(v)})`); + } + gte(c: string, v: unknown): this { + return this.record(`gte(${c},${JSON.stringify(v)})`); + } + lt(c: string, v: unknown): this { + return this.record(`lt(${c},${JSON.stringify(v)})`); + } + lte(c: string, v: unknown): this { + return this.record(`lte(${c},${JSON.stringify(v)})`); + } + ilike(c: string, p: string): this { + return this.record(`ilike(${c},${p})`); + } + contains(c: string, v: readonly unknown[] | Record): this { + return this.record(`contains(${c},${JSON.stringify(v)})`); + } + overlaps(c: string, v: readonly unknown[]): this { + return this.record(`overlaps(${c},${JSON.stringify(v)})`); + } + not(c: string, op: string, v: unknown): this { + return this.record(`not(${c},${op},${JSON.stringify(v)})`); + } + or(f: string): this { + return this.record(`or(${f})`); + } +} + +const ctx: CompileCtx = { tz: "UTC", now: new Date("2026-01-15T12:00:00.000Z"), industryId: null, permissions: {} }; +const registry = leadFields(ctx); + +function cond(id: string, field: string, op: FilterCondition["op"], value?: FilterCondition["value"]): FilterCondition { + return value === undefined ? { id, field, op } : { id, field, op, value }; +} + +function andTree(...conditions: FilterCondition[]): FilterTree { + return { conjunction: "and", conditions }; +} + +function compile(tree: FilterTree): FakeBuilder { + return compileFilter(new FakeBuilder(), tree, registry, ctx); +} + +describe("leads registry — legacy value-shape equivalence", () => { + it("status: a text slug (never a UUID from the legacy toolbar) targets the status column, matching route.ts's .eq('status', value)", () => { + const b = compile(andTree(cond("c1", "status", "is", "contacted"))); + expect(b.calls).toEqual(["or(status.eq.contacted)"]); + }); + + it("source: a text slug list targets intake_source, matching route.ts's .in('intake_source', sourceFilter)", () => { + const b = compile(andTree(cond("c1", "source", "is_any_of", ["google_ads", "referral"]))); + expect(b.calls).toEqual(["or(intake_source.in.(google_ads,referral))"]); + }); + + it("status: a UUID-shaped value still targets status (no stage_id dispatch), matching route.ts's .eq('status', value)", () => { + const b = compile(andTree(cond("c1", "status", "is", "11111111-2222-4333-8444-555555555555"))); + expect(b.calls).toEqual(["or(status.eq.11111111-2222-4333-8444-555555555555)"]); + }); + + it("source: a UUID-shaped value still targets intake_source (no form_config_id dispatch)", () => { + const b = compile(andTree(cond("c1", "source", "is_any_of", ["11111111-2222-4333-8444-555555555555"]))); + expect(b.calls).toEqual(["or(intake_source.eq.11111111-2222-4333-8444-555555555555)"]); + }); + + it("source: an empty value list throws instead of compiling to a silent match-everything predicate (R12 fail-closed)", () => { + const source = registry.source.source; + if (source.kind !== "virtual") throw new Error("expected virtual source"); + expect(() => source.compile({ id: "c1", field: "source", op: "is_any_of", value: [] }, ctx)).toThrow(FilterCompileError); + expect(() => source.compile({ id: "c1", field: "source", op: "is_none_of", value: [] }, ctx)).toThrow(FilterCompileError); + }); + + it("form: a UUID targets form_config_id directly, matching route.ts's .eq('form_config_id', formFilter)", () => { + const b = compile(andTree(cond("c1", "form", "is", "11111111-2222-4333-8444-555555555555"))); + expect(b.calls).toEqual(["eq(form_config_id,\"11111111-2222-4333-8444-555555555555\")"]); + }); + + it("assignees: unassigned + ids matches route.ts's or(assigned_to.is.null,assigned_to.in.(...))", () => { + const b = compile(andTree(cond("c1", "assignees", "is_any_of", ["unassigned", "11111111-2222-4333-8444-555555555555"]))); + expect(b.calls).toEqual(["or(or(assigned_to.is.null,assigned_to.in.(11111111-2222-4333-8444-555555555555)))"]); + }); + + it("assignees: only unassigned matches route.ts's .is('assigned_to', null)", () => { + const b = compile(andTree(cond("c1", "assignees", "is_any_of", ["unassigned"]))); + expect(b.calls).toEqual(["or(assigned_to.is.null)"]); + }); + + it("assignees: every token invalid and no 'unassigned' falls through to no filter, matching route.ts's silent no-op", () => { + const b = compile(andTree(cond("c1", "assignees", "is_any_of", ["garbage"]))); + expect(b.calls).toEqual(["or(id.not.is.null)"]); + }); + + it("collaborators is embed-kind and is planned as the exact !inner select route.ts's selectColumns ternary builds today", () => { + const plan = planFilter(andTree(cond("c1", "collaborators", "is_any_of", ["u1"])), registry, ctx); + expect(plan).toEqual({ ok: true, embeds: ["lead_collaborators!inner(user_id)"] }); + }); + + it("tags has_all matches route.ts's .contains('tags', [tagFilter]) via the native path", () => { + const b = compile(andTree(cond("c1", "tags", "has_all", ["vip"]))); + expect(b.calls).toEqual(['contains(tags,["vip"])']); + }); + + it("industry __none__ (is_empty) matches route.ts's .is('prospect_industry', null) via the native path", () => { + const b = compile(andTree(cond("c1", "industry", "is_empty"))); + expect(b.calls).toEqual(["is(prospect_industry,null)"]); + }); + + it("data_completeness / next_task / assigned_role are not filterable in Phase 2", () => { + for (const key of ["data_completeness", "next_task", "assigned_role"]) { + const plan = planFilter(andTree(cond("c1", key, "is", "x")), registry, ctx); + expect(plan.ok).toBe(false); + } + }); + + it("SORT_COLUMNS is folded in: created_at/last_activity_at/updated_at/first_name/email all carry sortColumns", () => { + expect(registry.created_at.sortColumns).toEqual(["created_at"]); + expect(registry.last_activity_at.sortColumns).toEqual(["last_activity_at"]); + expect(registry.updated_at.sortColumns).toEqual(["updated_at"]); + expect(registry.first_name.sortColumns).toEqual(["first_name", "last_name"]); + expect(registry.email.sortColumns).toEqual(["email"]); + }); +}); diff --git a/src/lib/filters/registry/leads.ts b/src/lib/filters/registry/leads.ts new file mode 100644 index 00000000..2ce6620c --- /dev/null +++ b/src/lib/filters/registry/leads.ts @@ -0,0 +1,319 @@ +import { and, or, pgVal } from "../pgrst"; +import { FilterCompileError, type CompileCtx, type FieldDef, type FieldRegistry, type FilterCondition } from "../types"; + +// Lead field registry — Phase 2 of docs/ADVANCED-FILTERS-BRIEF.md. Covers the 9 +// existing /api/v1/leads toolbar axes (status, search, form, tag, created, +// industry, source, assignees, collaborators) plus the "obvious first-class +// columns" the brief calls for. No `cf:*` custom fields yet — that's Phase 6. +// +// Every field here is deliberately scoped to match TODAY's legacy toolbar +// semantics byte-for-byte (see legacy-leads-params.ts + route.test.ts), not +// the more ambitious coalescing a few of the brief's registry notes describe. +// +// - "status" -> the brief says virtual `stage_id ?? status`. This registry +// is intentionally NOT that: it's strict single-column, targeting `status` +// only — exactly what route.ts's `.eq('status', value)` does today. A +// value-shape dispatch (UUID -> stage_id) was tried and reverted: it isn't +// a no-op for every caller, because /api/v1/leads is reachable by any +// authenticated session, not just the toolbar. `?status=` today +// resolves to `status.eq.` -> zero rows (status is a VARCHAR(20) +// CHECK-constrained column, so a UUID can never match); dispatching by +// shape would silently turn that into a `stage_id.eq.` match. Stage +// gets its own FieldDef (-> stage_id) in Phase 3, coexisting with this one +// exactly like `assigned_to` (scope) coexists with `assignees` (filter). +// - "source" -> same reasoning, strict single-column targeting +// `intake_source` only, matching route.ts's `.in('intake_source', ...)`. +// The SEPARATE `form` field below still targets form_config_id directly +// and unconditionally, exactly like route.ts's existing `?form=` handling. + +const UUID_RE = /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i; + +function asList(value: FilterCondition["value"]): string[] { + if (Array.isArray(value)) return value as string[]; + return value === undefined ? [] : [String(value)]; +} + +// ── status: virtual, strict single-column (status only) ──────────────────── + +function compileStatus(cond: FilterCondition): string { + if (cond.op === "is_empty") return "status.is.null"; + if (cond.op === "is_not_empty") return "status.not.is.null"; + + const value = String(cond.value); + if (cond.op === "is") return `status.eq.${pgVal(value)}`; + // is_not — NULL-inclusive negation rule. + return or("status.is.null", `status.neq.${pgVal(value)}`); +} + +// ── source: virtual, strict single-column (intake_source only) ───────────── + +function compileSource(cond: FilterCondition): string { + if (cond.op === "is_empty") return "intake_source.is.null"; + if (cond.op === "is_not_empty") return "intake_source.not.is.null"; + + const values = asList(cond.value); + if (values.length === 0) throw new FilterCompileError(`${cond.op} requires at least one value`, "invalid_value"); + + const isNeg = cond.op === "is_not" || cond.op === "is_none_of"; + if (!isNeg) { + return values.length === 1 ? `intake_source.eq.${pgVal(values[0])}` : `intake_source.in.(${values.map(pgVal).join(",")})`; + } + // is_not / is_none_of — NULL-inclusive negation rule. + return or("intake_source.is.null", values.length === 1 ? `intake_source.neq.${pgVal(values[0])}` : `intake_source.not.in.(${values.map(pgVal).join(",")})`); +} + +// ── assignees: virtual — the "unassigned" sentinel + UUID-validated ids, ─── +// matching route.ts's existing ?assignees= handling exactly, including its +// (surprising but current) fall-through-to-no-filter when every token is +// invalid and "unassigned" wasn't requested. + +function compileAssignees(cond: FilterCondition): string { + const values = asList(cond.value); + const wantsUnassigned = values.includes("unassigned"); + const ids = values.filter((v) => v !== "unassigned" && UUID_RE.test(v)); + + if (wantsUnassigned && ids.length > 0) return or("assigned_to.is.null", `assigned_to.in.(${ids.map(pgVal).join(",")})`); + if (wantsUnassigned) return "assigned_to.is.null"; + if (ids.length > 0) return ids.length === 1 ? `assigned_to.eq.${pgVal(ids[0])}` : `assigned_to.in.(${ids.map(pgVal).join(",")})`; + return "id.not.is.null"; // no valid tokens — legacy applies no filter in this case +} + +// ── location: virtual, city + country combined — no legacy equivalent ────── + +function compileLocation(cond: FilterCondition): string { + if (cond.op === "is_empty") return and("city.is.null", "country.is.null"); + if (cond.op === "is_not_empty") return or("city.not.is.null", "country.not.is.null"); + const value = String(cond.value); + const pattern = pgVal(`%${value.replace(/([\\%_])/g, "\\$1")}%`); + if (cond.op === "contains") return or(`city.ilike.${pattern}`, `country.ilike.${pattern}`); + // not_contains — De Morgan over the OR'd positive match, each leg NULL-inclusive: + // NOT(city ilike P OR country ilike P) = (city IS NULL OR city NOT ilike P) AND (country IS NULL OR country NOT ilike P) + return and(or("city.is.null", `city.not.ilike.${pattern}`), or("country.is.null", `country.not.ilike.${pattern}`)); +} + +// `ctx` is accepted (not yet read) so a future industry/permission-filtered +// registry — e.g. hiding field_of_study for a non-education tenant — is a +// change inside this function, not a signature change at every call site. +export function leadFields(ctx: CompileCtx): FieldRegistry { + void ctx; + const fields: FieldDef[] = [ + // ── toolbar axes ───────────────────────────────────────────────────── + { + key: "status", + label: "Status", + type: "select", + source: { kind: "virtual", compile: compileStatus }, + operators: ["is", "is_not", "is_empty", "is_not_empty"], + group: "Basic", + filterable: true, + sortable: false, + }, + { + key: "search", + label: "Search (name, email, phone)", + type: "text", + source: { kind: "columns", columns: ["first_name", "last_name", "email", "phone"], fullNamePairs: true }, + group: "Basic", + filterable: true, + }, + { + key: "form", + label: "Form", + type: "uuid", + source: { kind: "column", column: "form_config_id" }, + group: "Basic", + filterable: true, + }, + { + key: "tags", + label: "Tags", + type: "tags", + source: { kind: "array_column", column: "tags" }, + group: "Basic", + filterable: true, + }, + { + key: "created", + label: "Created", + type: "date", + source: { kind: "column", column: "created_at" }, + group: "Dates", + filterable: true, + }, + { + key: "industry", + label: "Prospect industry", + type: "select", + source: { kind: "column", column: "prospect_industry" }, + emptyIsBlankString: false, + group: "Basic", + filterable: true, + }, + { + key: "source", + label: "Source", + type: "select", + source: { kind: "virtual", compile: compileSource }, + operators: ["is", "is_not", "is_any_of", "is_none_of", "is_empty", "is_not_empty"], + group: "Basic", + filterable: true, + }, + { + key: "assignees", + label: "Assigned to", + type: "uuid", + source: { kind: "virtual", compile: compileAssignees }, + operators: ["is_any_of"], + group: "Basic", + filterable: true, + }, + { + key: "collaborators", + label: "Collaborators", + type: "relation", + source: { kind: "embed", relation: "lead_collaborators", column: "user_id", embedSelect: "lead_collaborators!inner(user_id)" }, + operators: ["is_any_of"], + group: "Basic", + filterable: true, + }, + + // ── promoted legacy custom_fields dual-read ────────────────────────── + { + key: "field_of_study", + label: "Field of study", + type: "text", + source: { kind: "promoted", column: "field_of_study", jsonb: { column: "custom_fields", path: "field_of_study" } }, + emptyIsBlankString: true, + group: "Education", + filterable: true, + }, + { + key: "destinations", + label: "Destinations", + type: "multiselect", + source: { kind: "promoted", column: "destinations", jsonb: { column: "custom_fields", path: "countries" } }, + group: "Education", + filterable: true, + }, + + // ── obvious first-class columns (also folds SORT_COLUMNS in) ───────── + { + key: "created_at", + label: "Created at", + type: "date", + source: { kind: "column", column: "created_at" }, + group: "Dates", + filterable: true, + sortable: true, + sortColumns: ["created_at"], + }, + { + key: "last_activity_at", + label: "Last activity", + type: "date", + source: { kind: "column", column: "last_activity_at" }, + group: "Dates", + filterable: true, + sortable: true, + sortColumns: ["last_activity_at"], + }, + { + key: "updated_at", + label: "Updated at", + type: "date", + source: { kind: "column", column: "updated_at" }, + group: "Dates", + filterable: true, + sortable: true, + sortColumns: ["updated_at"], + }, + { + key: "first_name", + label: "Name", + type: "text", + source: { kind: "columns", columns: ["first_name", "last_name"], fullNamePairs: true }, + emptyIsBlankString: true, + group: "Basic", + filterable: true, + sortable: true, + sortColumns: ["first_name", "last_name"], + }, + { + key: "email", + label: "Email", + type: "text", + source: { kind: "column", column: "email" }, + emptyIsBlankString: true, + group: "Basic", + filterable: true, + sortable: true, + sortColumns: ["email"], + }, + { + key: "phone", + label: "Phone", + type: "text", + source: { kind: "column", column: "phone" }, + emptyIsBlankString: true, + group: "Basic", + filterable: true, + }, + { + key: "city", + label: "City", + type: "text", + source: { kind: "column", column: "city" }, + emptyIsBlankString: true, + group: "Basic", + filterable: true, + }, + { + key: "country", + label: "Country", + type: "text", + source: { kind: "column", column: "country" }, + emptyIsBlankString: true, + group: "Basic", + filterable: true, + }, + { + key: "location", + label: "Location", + type: "text", + source: { kind: "virtual", compile: compileLocation }, + operators: ["contains", "not_contains", "is_empty", "is_not_empty"], + group: "Basic", + filterable: true, + }, + + // ── explicitly not filterable in Phase 2 ───────────────────────────── + { + key: "data_completeness", + label: "Data completeness", + type: "text", + source: { kind: "column", column: "custom_fields" }, + group: "Basic", + filterable: false, + }, + { + key: "next_task", + label: "Next task", + type: "text", + source: { kind: "column", column: "custom_fields" }, + group: "Basic", + filterable: false, + }, + { + key: "assigned_role", + label: "Assigned role", + type: "text", + source: { kind: "column", column: "custom_fields" }, + group: "Basic", + filterable: false, + }, + ]; + + const registry: FieldRegistry = {}; + for (const field of fields) registry[field.key] = field; + return registry; +} diff --git a/src/lib/filters/schema.ts b/src/lib/filters/schema.ts new file mode 100644 index 00000000..3d10ae86 --- /dev/null +++ b/src/lib/filters/schema.ts @@ -0,0 +1,95 @@ +import { z } from "zod"; + +// Zod validation for the filter AST — a discriminated union on `op` so every +// operator's value shape is checked precisely (no-value ops carry no `value`, +// list ops carry a bounded string[], `between`/`date_between` carry a 2-tuple, +// etc). `src/lib/api/validation.ts` is body-only and every one of its +// validators PASSES on an absent or wrong-typed value — it structurally cannot +// gate a recursive, attacker-supplied tree. This is zod's first non-AI use in +// the codebase; `zod@^4.4.3` is already a prod dependency. +// +// The caps below are the URL-size defence, not cosmetics — see serialize.ts's +// MAX_ENCODED_LEN and the "300-id counselor visibility cap" prod incident this +// is designed to never repeat. `is_any_of []` (an empty allow-list) throws a +// 422 here rather than silently becoming a no-op filter that leaks rows — see +// compile.test.ts and the empty-pipeline-allow-list risk in the plan doc. + +const idSchema = z.string().min(1).max(128); +const fieldSchema = z.string().min(1).max(128); + +const scalarValue = z.union([z.string().min(1).max(4096), z.number()]); +const stringValue = z.string().min(1).max(4096); +const numberValue = z.number(); +const listValue = z.array(z.string().min(1).max(200)).min(1).max(200); +const numberTuple = z.tuple([z.number(), z.number()]); +const dateTuple = z.tuple([z.string().min(1).max(64), z.string().min(1).max(64)]); +const relativeDateValue = z.string().regex(/^\d+[dmy]$/, "expected a relative date like \"7d\", \"3m\", \"1y\""); + +function condition(op: Op, value: ValueShape) { + return z.object({ id: idSchema, field: fieldSchema, op: z.literal(op), value }); +} + +function conditionNoValue(op: Op) { + return z.object({ id: idSchema, field: fieldSchema, op: z.literal(op), value: z.undefined().optional() }); +} + +export const conditionSchema = z.discriminatedUnion("op", [ + // universal + condition("is", scalarValue), + condition("is_not", scalarValue), + conditionNoValue("is_empty"), + conditionNoValue("is_not_empty"), + // text + condition("contains", stringValue), + condition("not_contains", stringValue), + condition("starts_with", stringValue), + condition("ends_with", stringValue), + // sets / arrays — `.min(1)` so an empty allow-list is a 422, never a silent no-op + condition("is_any_of", listValue), + condition("is_none_of", listValue), + condition("has_all", listValue), + // number + condition("gt", numberValue), + condition("gte", numberValue), + condition("lt", numberValue), + condition("lte", numberValue), + condition("between", numberTuple), + // date + condition("before", stringValue), + condition("after", stringValue), + condition("on", stringValue), + condition("date_between", dateTuple), + condition("within_last", relativeDateValue), + condition("within_next", relativeDateValue), + // boolean + conditionNoValue("is_true"), + conditionNoValue("is_false"), +]); + +export type ConditionInput = z.infer; + +const MAX_CONDITIONS_PER_LEAF_GROUP = 12; +const MAX_ROOT_CONDITIONS = 20; +const MAX_GROUPS = 5; +const MAX_TOTAL_CONDITIONS = 25; + +export const leafGroupSchema = z.object({ + conjunction: z.enum(["and", "or"]), + conditions: z.array(conditionSchema).max(MAX_CONDITIONS_PER_LEAF_GROUP), +}); + +export const filterTreeSchema = z + .object({ + conjunction: z.enum(["and", "or"]), + conditions: z.array(conditionSchema).max(MAX_ROOT_CONDITIONS), + groups: z.array(leafGroupSchema).max(MAX_GROUPS).optional(), + }) + .refine( + (tree) => { + const groupConditions = (tree.groups ?? []).reduce((sum, g) => sum + g.conditions.length, 0); + return tree.conditions.length + groupConditions <= MAX_TOTAL_CONDITIONS; + }, + { message: `total conditions across root + groups must not exceed ${MAX_TOTAL_CONDITIONS}` } + ); + +export type FilterTreeInput = z.infer; diff --git a/src/lib/filters/serialize.test.ts b/src/lib/filters/serialize.test.ts new file mode 100644 index 00000000..75a9c02d --- /dev/null +++ b/src/lib/filters/serialize.test.ts @@ -0,0 +1,182 @@ +import { describe, it, expect } from "vitest"; +import { countActiveConditions, decodeFilterTree, encodeFilterTree, isEmptyTree, MAX_ENCODED_LEN, FILTER_PARAM, VIEW_PARAM } from "./serialize"; +import { EMPTY_TREE, type FilterTree } from "./types"; + +describe("constants", () => { + it("FILTER_PARAM is 'f' and VIEW_PARAM is 'view'", () => { + expect(FILTER_PARAM).toBe("f"); + expect(VIEW_PARAM).toBe("view"); + }); + + it("MAX_ENCODED_LEN is 4096", () => { + expect(MAX_ENCODED_LEN).toBe(4096); + }); +}); + +describe("encodeFilterTree / decodeFilterTree round-trip", () => { + it("round-trips the empty tree", () => { + const encoded = encodeFilterTree(EMPTY_TREE); + const decoded = decodeFilterTree(encoded); + expect(decoded).toEqual({ ok: true, tree: EMPTY_TREE }); + }); + + it("round-trips a tree with one condition", () => { + const tree: FilterTree = { + conjunction: "and", + conditions: [{ id: "c1", field: "status", op: "is", value: "new" }], + }; + const decoded = decodeFilterTree(encodeFilterTree(tree)); + expect(decoded).toEqual({ ok: true, tree }); + }); + + it("round-trips a tree with groups", () => { + const tree: FilterTree = { + conjunction: "and", + conditions: [{ id: "c1", field: "status", op: "is_any_of", value: ["new", "contacted"] }], + groups: [ + { + conjunction: "or", + conditions: [ + { id: "g1", field: "source", op: "is", value: "web" }, + { id: "g2", field: "source", op: "is", value: "referral" }, + ], + }, + ], + }; + const decoded = decodeFilterTree(encodeFilterTree(tree)); + expect(decoded).toEqual({ ok: true, tree }); + }); + + it("produces a base64url string (no + / = characters)", () => { + const encoded = encodeFilterTree({ + conjunction: "and", + conditions: [{ id: "c1", field: "search", op: "contains", value: "a+b/c=d" }], + }); + expect(encoded).not.toMatch(/[+/=]/); + }); + + it("is unpadded", () => { + // Pick a payload whose base64 (padded) would definitely have '=' padding. + const encoded = encodeFilterTree({ conjunction: "and", conditions: [] }); + expect(encoded.endsWith("=")).toBe(false); + }); +}); + +describe("decodeFilterTree error handling", () => { + it("rejects an empty string", () => { + const result = decodeFilterTree(""); + expect(result.ok).toBe(false); + }); + + it("rejects invalid base64url", () => { + const result = decodeFilterTree("!!!not-base64!!!"); + expect(result.ok).toBe(false); + }); + + it("rejects base64url that decodes to invalid JSON", () => { + const encoded = Buffer.from("not json{{{", "utf8").toString("base64url"); + const result = decodeFilterTree(encoded); + expect(result.ok).toBe(false); + }); + + it("rejects a tree failing zod validation, with per-field errors", () => { + const encoded = Buffer.from(JSON.stringify({ conjunction: "xor", conditions: [] }), "utf8").toString("base64url"); + const result = decodeFilterTree(encoded); + expect(result.ok).toBe(false); + if (!result.ok) { + expect(Object.keys(result.errors).length).toBeGreaterThan(0); + } + }); + + it("rejects an encoded string over MAX_ENCODED_LEN with an actionable message, not a transport error", () => { + const hugeTree: FilterTree = { + conjunction: "and", + conditions: [{ id: "c1", field: "assignees", op: "is_any_of", value: Array.from({ length: 250 }, (_, i) => `00000000-0000-0000-0000-${String(i).padStart(12, "0")}`) }], + }; + const encoded = encodeFilterTree(hugeTree); + expect(encoded.length).toBeGreaterThan(MAX_ENCODED_LEN); + const result = decodeFilterTree(encoded); + expect(result.ok).toBe(false); + if (!result.ok) { + expect(result.errors.f?.[0]).toMatch(/too large/i); + expect(result.errors.f?.[0]).toMatch(/view/i); + } + }); + + it("rejects a tree exceeding the 25-total-conditions cap", () => { + const conditions = Array.from({ length: 26 }, (_, i) => ({ id: `c${i}`, field: "status", op: "is" as const, value: "new" })); + const encoded = Buffer.from(JSON.stringify({ conjunction: "and", conditions }), "utf8").toString("base64url"); + const result = decodeFilterTree(encoded); + expect(result.ok).toBe(false); + }); + + it("rejects is_any_of with an empty value array (422, not a silent no-op)", () => { + const encoded = Buffer.from( + JSON.stringify({ conjunction: "and", conditions: [{ id: "c1", field: "assignees", op: "is_any_of", value: [] }] }), + "utf8" + ).toString("base64url"); + const result = decodeFilterTree(encoded); + expect(result.ok).toBe(false); + }); +}); + +describe("isEmptyTree", () => { + it("is true for EMPTY_TREE", () => { + expect(isEmptyTree(EMPTY_TREE)).toBe(true); + }); + + it("is true for a tree with an empty groups array", () => { + expect(isEmptyTree({ conjunction: "and", conditions: [], groups: [] })).toBe(true); + }); + + it("is false when root has a condition", () => { + expect(isEmptyTree({ conjunction: "and", conditions: [{ id: "c1", field: "status", op: "is", value: "new" }] })).toBe(false); + }); + + it("is false when only a group has a condition", () => { + expect( + isEmptyTree({ + conjunction: "and", + conditions: [], + groups: [{ conjunction: "or", conditions: [{ id: "g1", field: "status", op: "is", value: "new" }] }], + }) + ).toBe(false); + }); +}); + +describe("countActiveConditions", () => { + it("is 0 for the empty tree", () => { + expect(countActiveConditions(EMPTY_TREE)).toBe(0); + }); + + it("counts root conditions", () => { + expect( + countActiveConditions({ + conjunction: "and", + conditions: [ + { id: "c1", field: "status", op: "is", value: "new" }, + { id: "c2", field: "source", op: "is", value: "web" }, + ], + }) + ).toBe(2); + }); + + it("counts root + all group conditions", () => { + expect( + countActiveConditions({ + conjunction: "and", + conditions: [{ id: "c1", field: "status", op: "is", value: "new" }], + groups: [ + { conjunction: "or", conditions: [{ id: "g1", field: "source", op: "is", value: "web" }] }, + { + conjunction: "or", + conditions: [ + { id: "g2", field: "tag", op: "is", value: "vip" }, + { id: "g3", field: "tag", op: "is", value: "urgent" }, + ], + }, + ], + }) + ).toBe(4); + }); +}); diff --git a/src/lib/filters/serialize.ts b/src/lib/filters/serialize.ts new file mode 100644 index 00000000..8327c588 --- /dev/null +++ b/src/lib/filters/serialize.ts @@ -0,0 +1,67 @@ +import { filterTreeSchema } from "./schema"; +import type { FilterTree } from "./types"; + +// URL transport for the filter AST. base64url(JSON) instead of encodeURIComponent: +// percent-encoding inflates `{ " ,` roughly 3x, base64 is a flat 1.33x — and this +// budget is real: MAX_ENCODED_LEN exists because an oversized `.in()` list has +// ALREADY caused a production bug (the 300-id counselor visibility cap). Going +// over it must produce a 422 with an actionable message ("save this as a +// view"), never an opaque transport failure at the undici ~16KB header ceiling. + +export const FILTER_PARAM = "f"; +export const VIEW_PARAM = "view"; +export const MAX_ENCODED_LEN = 4096; + +export type DecodeResult = { ok: true; tree: FilterTree } | { ok: false; errors: Record }; + +export function encodeFilterTree(tree: FilterTree): string { + return Buffer.from(JSON.stringify(tree), "utf8").toString("base64url"); +} + +export function decodeFilterTree(raw: string): DecodeResult { + if (typeof raw !== "string" || raw.length === 0) { + return { ok: false, errors: { f: ["empty filter parameter"] } }; + } + + if (raw.length > MAX_ENCODED_LEN) { + return { + ok: false, + errors: { f: [`filter is too large (${raw.length} > ${MAX_ENCODED_LEN} chars) — save this as a view instead`] }, + }; + } + + let json: string; + try { + json = Buffer.from(raw, "base64url").toString("utf8"); + } catch { + return { ok: false, errors: { f: ["not valid base64url"] } }; + } + + let parsed: unknown; + try { + parsed = JSON.parse(json); + } catch { + return { ok: false, errors: { f: ["not valid JSON after decoding"] } }; + } + + const result = filterTreeSchema.safeParse(parsed); + if (!result.success) { + const errors: Record = {}; + for (const issue of result.error.issues) { + const key = issue.path.length > 0 ? issue.path.join(".") : "f"; + (errors[key] ??= []).push(issue.message); + } + return { ok: false, errors }; + } + + return { ok: true, tree: result.data as FilterTree }; +} + +export function isEmptyTree(tree: FilterTree): boolean { + return tree.conditions.length === 0 && (tree.groups ?? []).every((g) => g.conditions.length === 0); +} + +export function countActiveConditions(tree: FilterTree): number { + const groupConditions = (tree.groups ?? []).reduce((sum, g) => sum + g.conditions.length, 0); + return tree.conditions.length + groupConditions; +} diff --git a/src/lib/filters/types.ts b/src/lib/filters/types.ts new file mode 100644 index 00000000..d0b82a9d --- /dev/null +++ b/src/lib/filters/types.ts @@ -0,0 +1,147 @@ +// Advanced Filters — the AST + field-registry contract. +// +// Pure types + literal constants. No React, no DOM, no Supabase import — this +// file (and everything else in src/lib/filters/) has zero imports from the rest +// of the app, by design (see docs/ADVANCED-FILTERS-BRIEF.md). Phase 1 has zero +// consumers; a real FieldRegistry for leads lands in a later phase. + +export type FilterFieldType = + | "text" + | "number" + | "date" + | "boolean" + | "select" + | "multiselect" + | "uuid" + | "tags" + | "relation"; + +export type FilterOperator = + | "is" + | "is_not" + | "is_empty" + | "is_not_empty" + | "contains" + | "not_contains" + | "starts_with" + | "ends_with" + | "is_any_of" + | "is_none_of" + | "has_all" + | "gt" + | "gte" + | "lt" + | "lte" + | "between" + | "before" + | "after" + | "on" + | "date_between" + | "within_last" // "7d" | "30d" | "3m" | "1y" + | "within_next" + | "is_true" + | "is_false"; + +// Operators whose naive SQL translation (<>, NOT, NOT IN) evaluates to NULL — +// and therefore EXCLUDES the row — when the underlying column is NULL. Every +// operator in this set must compile to `or(.is.null, )`, never +// a bare negation. See compile.ts's top-of-file comment for the full rationale. +export const NEGATIVE_OPERATORS: readonly FilterOperator[] = [ + "is_not", + "not_contains", + "is_none_of", +] as const; + +export type FilterValue = + | string + | number + | boolean + | string[] + | [number, number] + | [string, string]; + +export interface FilterCondition { + id: string; // stable client key, round-trips through the URL + field: string; // REGISTRY key — NEVER a DB column. Resolution happens only in compile.ts + op: FilterOperator; + value?: FilterValue; +} + +export interface FilterLeafGroup { + conjunction: "and" | "or"; + conditions: FilterCondition[]; +} + +export interface FilterGroup { + conjunction: "and" | "or"; + conditions: FilterCondition[]; + groups?: FilterLeafGroup[]; // depth STOPS here — enforced by the type, not a runtime guard +} + +export type FilterTree = FilterGroup; + +export const EMPTY_TREE: FilterTree = { conjunction: "and", conditions: [] }; + +// ── Field registry ────────────────────────────────────────────────────────── + +// The discriminant that keeps every documented trap (dual-read legacy jsonb, +// key != column, embed pluralization) out of generic operator code. Every +// column string a compiled query can ever emit comes from one of these +// literals in a registry entry — never from client input. That is the entire +// injection defense for column position (see pgrst.ts for value escaping). +export type FieldSource = + | { kind: "column"; column: string } + | { kind: "columns"; columns: string[]; fullNamePairs?: boolean } // the search field + | { kind: "array_column"; column: string } // tags, destinations + | { kind: "jsonb"; column: "custom_fields"; path: string } + | { kind: "promoted"; column: string; jsonb: { column: "custom_fields"; path: string } } + | { kind: "embed"; relation: string; column: string; embedSelect: string } + | { kind: "virtual"; compile: (c: FilterCondition, ctx: CompileCtx) => string }; + +// Minimal local mirror of the permission shape a `visibleTo` predicate needs. +// Deliberately NOT imported from src/lib/api/permissions.ts — that would break +// the zero-imports-from-the-rest-of-the-app invariant this phase is built on. +// A real consumer (Phase 2+) can widen this or replace it with the real type +// at the call site without changing anything in this directory. +export interface ResolvedPermissions { + leadScope?: string; + [key: string]: unknown; +} + +export interface CompileCtx { + tz: string; // IANA zone, e.g. "Asia/Kathmandu" — day boundaries are computed from this, never server-local time + now: Date; // injected, never Date.now() inside the compiler — keeps date tests deterministic + industryId: string | null; + permissions: ResolvedPermissions; +} + +export interface FieldDef { + key: string; // registry key referenced by FilterCondition.field + label: string; + type: FilterFieldType; + source: FieldSource; + operators?: FilterOperator[]; // overrides OPERATORS_BY_TYPE[type] when present + options?: { value: string; label: string }[]; + emptyIsBlankString?: boolean; // text field where "" and NULL are both "empty" + industries?: string[]; // undefined = all industries + group: string; // UI grouping label ("Basic", "Dates", "Custom", …) + icon?: string; // lucide icon name as a STRING (never a component import) + filterable: boolean; + sortable?: boolean; + sortColumns?: string[]; // multi-column sort (e.g. first_name -> [first_name, last_name]) + columnKey?: string; // back-reference into a rendering columns-registry + accessor?: string; + visibleTo?: (p: ResolvedPermissions) => boolean; +} + +export type FieldRegistry = Record; + +export class FilterCompileError extends Error { + constructor( + message: string, + public readonly code: "unknown_field" | "not_filterable" | "operator_not_allowed" | "invalid_value" | "unsupported" + ) { + super(message); + this.name = "FilterCompileError"; + } +} diff --git a/supabase/migrations/200_class_managers.sql b/supabase/migrations/200_class_managers.sql new file mode 100644 index 00000000..1cc3eb73 --- /dev/null +++ b/supabase/migrations/200_class_managers.sql @@ -0,0 +1,85 @@ +-- Migration 200: Class Managers (education_consultancy) +-- Replaces the two hardcoded classes-access mechanisms (class_attendance_markers +-- allowlist + CLASS_ENROLL_POSITIONS position-slug list in src/lib/api/permissions.ts) +-- with a single admin-managed, per-user, per-capability grant table. +-- +-- Additive + idempotent. Two backfills into the new table: +-- 1. class_attendance_markers rows -> mark_attendance=true, view_roster=true, +-- enroll_students=false (existing markers keep exactly what they had — no +-- new enroll grant, per explicit product decision). +-- 2. Every education_consultancy tenant_user whose position slug was in the +-- old CLASS_ENROLL_POSITIONS set (branch-manager, lead-executive, +-- counselor, application-executive) -> enroll_students=true, preserving +-- whatever mark_attendance/view_roster they already have (does not +-- downgrade backfill #1). Without this, replacing the position-slug check +-- with class_managers would silently revoke enroll access from everyone +-- who held it by position, which is a real regression, not an intended +-- tightening — the product decision was "admin manages it going forward", +-- not "everyone's existing access is revoked on cutover". +-- class_attendance_markers is left in place; it becomes dead once the app-code +-- cutover (separate change) ships reads/writes against class_managers instead. +-- Do not drop it here. +-- +-- Expected before/after row counts: class_managers 0 -> N +-- (N = markers rows + position-holder rows, deduped by (tenant_id,user_id)). +-- Rollback: DROP TABLE IF EXISTS class_managers; +-- Applied: stage TBD / prod TBD. + +BEGIN; + +CREATE TABLE IF NOT EXISTS class_managers ( + tenant_id UUID NOT NULL REFERENCES tenants(id) ON DELETE CASCADE, + user_id UUID NOT NULL REFERENCES auth.users(id) ON DELETE CASCADE, + enroll_students BOOLEAN NOT NULL DEFAULT false, + mark_attendance BOOLEAN NOT NULL DEFAULT false, + view_roster BOOLEAN NOT NULL DEFAULT false, + granted_by UUID REFERENCES auth.users(id), + created_at TIMESTAMPTZ NOT NULL DEFAULT now(), + updated_at TIMESTAMPTZ NOT NULL DEFAULT now(), + PRIMARY KEY (tenant_id, user_id) +); + +ALTER TABLE class_managers ENABLE ROW LEVEL SECURITY; + +DROP POLICY IF EXISTS "class_managers_select" ON class_managers; +CREATE POLICY "class_managers_select" ON class_managers + FOR SELECT USING (tenant_id IN (SELECT get_user_tenant_ids())); + +DROP POLICY IF EXISTS "class_managers_insert" ON class_managers; +CREATE POLICY "class_managers_insert" ON class_managers + FOR INSERT WITH CHECK (is_tenant_admin(tenant_id)); + +DROP POLICY IF EXISTS "class_managers_update" ON class_managers; +CREATE POLICY "class_managers_update" ON class_managers + FOR UPDATE USING (is_tenant_admin(tenant_id)) WITH CHECK (is_tenant_admin(tenant_id)); + +DROP POLICY IF EXISTS "class_managers_delete" ON class_managers; +CREATE POLICY "class_managers_delete" ON class_managers + FOR DELETE USING (is_tenant_admin(tenant_id)); + +DROP TRIGGER IF EXISTS trigger_class_managers_updated_at ON class_managers; +CREATE TRIGGER trigger_class_managers_updated_at + BEFORE UPDATE ON class_managers FOR EACH ROW EXECUTE FUNCTION update_updated_at(); + +-- ── Backfill 1: class_attendance_markers ──────────────────────────────────── +INSERT INTO class_managers (tenant_id, user_id, enroll_students, mark_attendance, view_roster) +SELECT tenant_id, user_id, false, true, true +FROM class_attendance_markers +ON CONFLICT (tenant_id, user_id) DO UPDATE + SET mark_attendance = true, view_roster = true; + +-- ── Backfill 2: old position-slug enroll access (parity with pre-cutover behavior) ── +INSERT INTO class_managers (tenant_id, user_id, enroll_students, mark_attendance, view_roster) +SELECT tu.tenant_id, tu.user_id, true, false, false +FROM tenant_users tu +JOIN positions p ON p.id = tu.position_id +JOIN tenants t ON t.id = tu.tenant_id +WHERE t.industry_id = 'education_consultancy' + AND p.slug IN ('branch-manager', 'lead-executive', 'counselor', 'application-executive') +ON CONFLICT (tenant_id, user_id) DO UPDATE + SET enroll_students = true; + +INSERT INTO public.schema_migrations (version) VALUES ('200_class_managers.sql') + ON CONFLICT (version) DO NOTHING; + +COMMIT; diff --git a/supabase/migrations/201_leads_tags_other_partial_index.sql b/supabase/migrations/201_leads_tags_other_partial_index.sql new file mode 100644 index 00000000..f2e174d9 --- /dev/null +++ b/supabase/migrations/201_leads_tags_other_partial_index.sql @@ -0,0 +1,30 @@ +-- Migration 201: partial index so the "exclude tags @> {other}" predicate stops +-- forcing a seq scan on every /api/v1/leads request. +-- +-- Additive only (index-only, no table rewrite). Expected before/after row counts: +-- leads: 0 rows touched (index creation does not modify data). +-- Rollback: DROP INDEX CONCURRENTLY IF EXISTS idx_leads_tenant_created_active_nonother; +-- Applied: stage 2026-08-07 / prod HELD (promotion gate). +-- +-- NOT in a transaction: CREATE INDEX CONCURRENTLY cannot run inside BEGIN/COMMIT +-- (see supabase/migrations/085_unique_display_id.sql for the precedent). +-- +-- src/app/(main)/api/v1/leads/route.ts:314 runs `NOT (tags @> '{"other"}')` +-- unconditionally on every request. This is a negated GIN predicate — Postgres +-- cannot use idx_leads_tags for a negation, so the exact-count query (and any +-- other query that isn't LIMIT-bounded against idx_leads_tenant_created_active) +-- falls back to a full seq scan. Measured on stage (dymeudcddasqpomfpjvt), +-- Admizz tenant (16,683 active rows): exact-count query 1467.9ms via Seq Scan. +-- +-- This partial index carries the exact same predicate (tenant_id, deleted_at IS +-- NULL, converted_at IS NULL, NOT tags @> '{other}') as the WHERE clause, so the +-- planner can prove the query predicate implies the index predicate and use an +-- Index Only Scan instead. Verified in a rolled-back transaction against stage +-- before committing to this approach (see PR body for before/after EXPLAIN +-- plans) — count query dropped to ~11.9ms. +CREATE INDEX CONCURRENTLY IF NOT EXISTS idx_leads_tenant_created_active_nonother + ON public.leads (tenant_id, created_at DESC, id DESC) + WHERE deleted_at IS NULL AND converted_at IS NULL AND NOT (tags @> ARRAY['other']::text[]); + +INSERT INTO public.schema_migrations (version) VALUES ('201_leads_tags_other_partial_index.sql') + ON CONFLICT (version) DO NOTHING;