diff --git a/docs/backend-asks/tag-ids-run-filter.md b/docs/backend-asks/tag-ids-run-filter.md new file mode 100644 index 00000000..3212d50c --- /dev/null +++ b/docs/backend-asks/tag-ids-run-filter.md @@ -0,0 +1,73 @@ +# Backend ask — resolve a `tag_ids` Run-filter key + +**For:** the distributor (`POST /v1/modelling/trigger-run`) +**From:** the portfolio tagging system (app), ADR-0013 (extends ADR-0008) +**Status:** app side built; backend change outstanding + +## What's needed + +The modelling **Run filter** gains one optional key, `tag_ids` — a list of +portfolio Tag ids. The distributor already receives +`{ task_id, portfolio_id, scenario_ids, filters }` and resolves `filters` +against the shared Postgres. It must now also resolve `tag_ids`. + +```jsonc +// dispatch payload (unchanged except the new optional filters key) +{ + "portfolio_id": 814, + "scenario_ids": [12, 34], + "filters": { + "postcodes": ["B93 8SU"], // existing + "property_types": ["House"], // existing + "built_forms": ["Detached"], // existing + "tag_ids": [7, 3] // NEW — optional + } +} +``` + +## Resolution rule + +Tags are a **direct membership** lookup, not the override→EPC precedence used for +type/built-form. A property matches `tag_ids` when it has a row in +`property_tag` for any of the ids (**any-of**). Combine with the other filter +dimensions the same way they combine today: **AND across dimensions, OR within**. + +```sql +-- add to the property-matching WHERE, only when tag_ids is present: +p.id IN ( + SELECT pt.property_id FROM property_tag pt + WHERE pt.tag_id IN (:tag_ids) +) +``` + +Tables (already migrated — see PR #387 / migration `0271`): + +- `portfolio_tag(id, portfolio_id, name, colour, …)` +- `property_tag(property_id, tag_id)` — PK `(property_id, tag_id)`, index + `ix_property_tag_tag` on `tag_id` for exactly this reverse lookup. + +Ids that don't belong to the portfolio simply match nothing (no validation +needed). An absent/empty `tag_ids` means unconstrained, like the other keys. + +## Reference implementation (must stay in lock-step) + +The app's in-app **preview count** is the reference implementation ADR-0008 +requires — the distributor's resolution must match it. See +`previewModellingRun` in `src/lib/modellingRuns/server.ts`: the tag clause is + +```ts +conditions.push( + sql`resolved.id IN (SELECT pt.property_id FROM property_tag pt + WHERE pt.tag_id IN (${inList(args.filters.tagIds)}))`, +); +``` + +Divergence between the app preview and the distributor is defined as a bug +(ADR-0008). + +## Until this lands + +The run UI can show and preview a Tag selection (the app resolves the preview), +but a triggered run won't scope by tags until the distributor resolves +`tag_ids`. No contract-shape change is needed on either side beyond reading the +new key. diff --git a/src/app/api/portfolio/[portfolioId]/modelling-runs/preview/route.ts b/src/app/api/portfolio/[portfolioId]/modelling-runs/preview/route.ts index 4817d91c..fd5af586 100644 --- a/src/app/api/portfolio/[portfolioId]/modelling-runs/preview/route.ts +++ b/src/app/api/portfolio/[portfolioId]/modelling-runs/preview/route.ts @@ -12,6 +12,7 @@ const bodySchema = z.object({ postcodes: z.array(z.string()).optional(), propertyTypes: z.array(z.string()).optional(), builtForms: z.array(z.string()).optional(), + tagIds: z.array(z.union([z.string(), z.number()])).optional(), }) .default({}), }); diff --git a/src/app/api/portfolio/[portfolioId]/modelling-runs/route.ts b/src/app/api/portfolio/[portfolioId]/modelling-runs/route.ts index b62f8078..46f70890 100644 --- a/src/app/api/portfolio/[portfolioId]/modelling-runs/route.ts +++ b/src/app/api/portfolio/[portfolioId]/modelling-runs/route.ts @@ -12,6 +12,7 @@ const filtersSchema = z.object({ postcodes: z.array(z.string()).optional(), propertyTypes: z.array(z.string()).optional(), builtForms: z.array(z.string()).optional(), + tagIds: z.array(z.union([z.string(), z.number()])).optional(), }); const bodySchema = z.object({ diff --git a/src/app/api/portfolio/[portfolioId]/tags/[tagId]/assignments/route.ts b/src/app/api/portfolio/[portfolioId]/tags/[tagId]/assignments/route.ts index 92370a44..64db0dba 100644 --- a/src/app/api/portfolio/[portfolioId]/tags/[tagId]/assignments/route.ts +++ b/src/app/api/portfolio/[portfolioId]/tags/[tagId]/assignments/route.ts @@ -8,15 +8,27 @@ import { getServerSession } from "next-auth"; import { AuthOptions } from "@/app/api/auth/[...nextauth]/authOptions"; import { canWriteTags, getPortfolioRole } from "@/lib/tags/server"; import { MAX_BULK_TAG_ROWS } from "@/lib/tags/bulkAssign"; +import { getFilteredPropertyIds } from "@/app/portfolio/[slug]/utils"; +import type { FilterGroups } from "@/app/utils/propertyFilters"; type Params = { params: Promise<{ portfolioId: string; tagId: string }> }; +/** Batch size for membership writes — keeps a whole-portfolio op under the bind-param cap. */ +const CHUNK = 5000; + const bodySchema = z.discriminatedUnion("mode", [ z.object({ mode: z.literal("properties"), action: z.enum(["add", "remove"]), propertyIds: z.array(z.string()).min(1), }), + z.object({ + mode: z.literal("filter"), + action: z.enum(["add", "remove"]), + // The property-table filter groups; resolved to ids server-side so the whole + // matching set is tagged, not just the visible page (ADR-0013). + filterGroups: z.array(z.unknown()), + }), z.object({ mode: z.literal("identifiers"), key: z.enum(["landlord_property_id", "uprn"]), @@ -64,39 +76,60 @@ export async function POST(req: NextRequest, props: Params) { return NextResponse.json({ error: "Invalid body" }, { status: 400 }); } - if (body.mode === "properties") { - // Only ids that actually belong to this portfolio (guards spoofed ids). - const owned = await db - .select({ id: property.id }) - .from(property) - .where( - and( - eq(property.portfolioId, pId), - inArray( - property.id, - body.propertyIds.map((id) => BigInt(id)), + if (body.mode === "properties" || body.mode === "filter") { + let ids: bigint[]; + if (body.mode === "properties") { + // Only ids that actually belong to this portfolio (guards spoofed ids). + const owned = await db + .select({ id: property.id }) + .from(property) + .where( + and( + eq(property.portfolioId, pId), + inArray( + property.id, + body.propertyIds.map((id) => BigInt(id)), + ), ), - ), - ); - const ids = owned.map((o) => o.id); + ); + ids = owned.map((o) => o.id); + } else { + // Resolve the current property-table filter to its whole matching set. + ids = await getFilteredPropertyIds(portfolioId, body.filterGroups as FilterGroups); + } if (ids.length === 0) return NextResponse.json({ changed: 0 }); - if (body.action === "add") { - const inserted = await db - .insert(propertyTag) - .values(ids.map((propertyId) => ({ propertyId, tagId: tId }))) - .onConflictDoNothing() - .returning({ propertyId: propertyTag.propertyId }); - return NextResponse.json({ changed: inserted.length }); + // Chunk so a whole-portfolio selection stays under Postgres' bind-param cap. + let changed = 0; + for (let i = 0; i < ids.length; i += CHUNK) { + const batch = ids.slice(i, i + CHUNK); + if (body.action === "add") { + const inserted = await db + .insert(propertyTag) + .values(batch.map((propertyId) => ({ propertyId, tagId: tId }))) + .onConflictDoNothing() + .returning({ propertyId: propertyTag.propertyId }); + changed += inserted.length; + } else { + const removed = await db + .delete(propertyTag) + .where( + and(eq(propertyTag.tagId, tId), inArray(propertyTag.propertyId, batch)), + ) + .returning({ propertyId: propertyTag.propertyId }); + changed += removed.length; + } } - const removed = await db - .delete(propertyTag) - .where(and(eq(propertyTag.tagId, tId), inArray(propertyTag.propertyId, ids))) - .returning({ propertyId: propertyTag.propertyId }); - return NextResponse.json({ changed: removed.length }); + return NextResponse.json({ changed }); } // identifiers mode — Bulk tag assignment (add only) with a match summary. + if (body.mode !== "identifiers") { + return NextResponse.json({ error: "Invalid body" }, { status: 400 }); + } + // Capture the narrowed discriminant in a const — TS widens `let body` back to + // the full union inside the closures below (e.g. matches.map). + const key = body.key; const wanted = [...new Set(body.identifiers.map((s) => s.trim()).filter(Boolean))]; const matches = await db .select({ @@ -108,7 +141,7 @@ export async function POST(req: NextRequest, props: Params) { .where( and( eq(property.portfolioId, pId), - body.key === "uprn" + key === "uprn" ? inArray( property.uprn, wanted @@ -121,20 +154,21 @@ export async function POST(req: NextRequest, props: Params) { const matchedValues = new Set( matches.map((m) => - body.key === "uprn" ? String(m.uprn) : String(m.landlordPropertyId), + key === "uprn" ? String(m.uprn) : String(m.landlordPropertyId), ), ); const unmatched = wanted.filter((v) => !matchedValues.has(v)); const matchedPropertyIds = matches.map((m) => m.id); let taggedCount = 0; - if (matchedPropertyIds.length > 0) { + for (let i = 0; i < matchedPropertyIds.length; i += CHUNK) { + const batch = matchedPropertyIds.slice(i, i + CHUNK); const inserted = await db .insert(propertyTag) - .values(matchedPropertyIds.map((propertyId) => ({ propertyId, tagId: tId }))) + .values(batch.map((propertyId) => ({ propertyId, tagId: tId }))) .onConflictDoNothing() .returning({ propertyId: propertyTag.propertyId }); - taggedCount = inserted.length; + taggedCount += inserted.length; } return NextResponse.json({ diff --git a/src/app/db/schema/property.ts b/src/app/db/schema/property.ts index c81ff31e..d05097c8 100644 --- a/src/app/db/schema/property.ts +++ b/src/app/db/schema/property.ts @@ -416,6 +416,15 @@ export interface PropertyWithRelations extends Record { co2Emissions: number | null; mainfuel: string | null; lexiscore: number | null; + // Portfolio Tags on this property (ADR-0013), name-ordered; [] when untagged. + tags: PropertyTag[]; +} + +/** A Tag as it appears on a property row (id serialised to a string). */ +export interface PropertyTag { + id: string; + name: string; + colour: string; } export type NonIntrusiveSurveyNotes = InferModel< diff --git a/src/app/portfolio/[slug]/(portfolio)/settings/layout.tsx b/src/app/portfolio/[slug]/(portfolio)/settings/layout.tsx index db1a38bf..11eafddd 100644 --- a/src/app/portfolio/[slug]/(portfolio)/settings/layout.tsx +++ b/src/app/portfolio/[slug]/(portfolio)/settings/layout.tsx @@ -1,7 +1,7 @@ import { getServerSession } from "next-auth"; import { AuthOptions } from "@/app/api/auth/[...nextauth]/authOptions"; import { SettingsSidebarLink } from "./SettingsSidebarLink"; -import { Settings2, Users, Building2, ScrollText } from "lucide-react"; +import { Settings2, Users, Building2, ScrollText, Tag } from "lucide-react"; export default async function SettingsLayout({ children, @@ -33,6 +33,12 @@ export default async function SettingsLayout({ > User Access + } + > + Tags + {isDomnaUser && ( { + const res = await fetch(`/api/portfolio/${portfolioId}/tags`); + if (!res.ok) throw new Error("Failed to load tags"); + return res.json(); +} + +export default function TagsCard({ portfolioId }: { portfolioId: string }) { + const queryClient = useQueryClient(); + const queryKey = ["portfolio-tags", portfolioId]; + + const { data: tags = [], isLoading } = useQuery({ + queryKey, + queryFn: () => fetchTags(portfolioId), + refetchOnWindowFocus: false, + }); + + const [name, setName] = useState(""); + const [colour, setColour] = useState(DEFAULT_COLOUR); + const [formError, setFormError] = useState(null); + const [editingId, setEditingId] = useState(null); + + const invalidate = () => queryClient.invalidateQueries({ queryKey }); + + const createTag = useMutation({ + mutationFn: async (input: { name: string; colour: string }) => { + const res = await fetch(`/api/portfolio/${portfolioId}/tags`, { + method: "POST", + body: JSON.stringify(input), + }); + const body = await res.json(); + if (!res.ok) throw new Error(body.error ?? "Failed to create tag"); + return body; + }, + onSuccess: () => { + setName(""); + setColour(DEFAULT_COLOUR); + setFormError(null); + invalidate(); + }, + onError: (e: Error) => setFormError(e.message), + }); + + return ( +
+

Tags

+

+ Group properties to run scenarios on a subset or filter your reporting. + Assign tags from the property table. +

+ + {/* Create */} +
{ + e.preventDefault(); + createTag.mutate({ name, colour }); + }} + > + + + +
+ {formError &&

{formError}

} + + {/* List */} + {isLoading ? ( +

Loading…

+ ) : tags.length === 0 ? ( +

No tags yet.

+ ) : ( +
    + {tags.map((tag) => + editingId === tag.id ? ( + { + setEditingId(null); + invalidate(); + }} + onCancel={() => setEditingId(null)} + /> + ) : ( + setEditingId(tag.id)} + onDeleted={invalidate} + /> + ), + )} +
+ )} +
+ ); +} + +function TagChip({ colour, name }: { colour: string; name: string }) { + return ( + + + {name} + + ); +} + +function TagRow({ + portfolioId, + tag, + onEdit, + onDeleted, +}: { + portfolioId: string; + tag: Tag; + onEdit: () => void; + onDeleted: () => void; +}) { + const del = useMutation({ + mutationFn: async () => { + const res = await fetch(`/api/portfolio/${portfolioId}/tags/${tag.id}`, { + method: "DELETE", + }); + if (!res.ok) throw new Error("Failed to delete tag"); + }, + onSuccess: onDeleted, + }); + + return ( +
  • + + + {tag.propertyCount} propert{tag.propertyCount === 1 ? "y" : "ies"} + +
    + + +
    +
    • + ); +} + +function EditRow({ + portfolioId, + tag, + onDone, + onCancel, +}: { + portfolioId: string; + tag: Tag; + onDone: () => void; + onCancel: () => void; +}) { + const [name, setName] = useState(tag.name); + const [colour, setColour] = useState(tag.colour); + const [error, setError] = useState(null); + + const save = useMutation({ + mutationFn: async () => { + const res = await fetch(`/api/portfolio/${portfolioId}/tags/${tag.id}`, { + method: "PATCH", + body: JSON.stringify({ name, colour }), + }); + const body = await res.json(); + if (!res.ok) throw new Error(body.error ?? "Failed to save"); + return body; + }, + onSuccess: onDone, + onError: (e: Error) => setError(e.message), + }); + + return ( +
  • + setColour(e.target.value)} + className="h-8 w-9 rounded-md border border-gray-300 bg-white p-0.5 cursor-pointer" + aria-label="Tag colour" + /> + setName(e.target.value)} + className="flex-1" + /> + {error && {error}} + + +
    • + ); +} diff --git a/src/app/portfolio/[slug]/(portfolio)/settings/tags/page.tsx b/src/app/portfolio/[slug]/(portfolio)/settings/tags/page.tsx new file mode 100644 index 00000000..d2042156 --- /dev/null +++ b/src/app/portfolio/[slug]/(portfolio)/settings/tags/page.tsx @@ -0,0 +1,12 @@ +import TagsCard from "./TagsCard"; + +export default async function TagsSettingsPage(props: { + params: Promise<{ slug: string }>; +}) { + const { slug } = await props.params; + return ( +
    + +
    + ); +} diff --git a/src/app/portfolio/[slug]/utils.ts b/src/app/portfolio/[slug]/utils.ts index 0eb1b88e..c6f5f326 100644 --- a/src/app/portfolio/[slug]/utils.ts +++ b/src/app/portfolio/[slug]/utils.ts @@ -656,6 +656,38 @@ function buildConditionSql(filter: PropertyFilter): ReturnType | nul if (filter.operator === "num_equals") return sql`${carbon} = ${n}`; return null; } + + case "tags": { + // Value is a JSON array of selected tag ids, optionally with the synthetic + // "__untagged__" bucket. Any-of within tags (ADR-0013). EXISTS on + // property_tag is PK-indexed by property_id — no join to add to the count. + if (filter.operator !== "enum_one_of") return null; + let selected: string[]; + try { + selected = JSON.parse(filter.value); + } catch { + return null; + } + if (selected.length === 0) return null; + + const untagged = selected.includes("__untagged__"); + const ids = selected.filter((s) => /^\d+$/.test(s)); + const parts: ReturnType[] = []; + if (ids.length > 0) { + const placeholders = ids.map((id) => sql`${id}::bigint`); + parts.push( + sql`EXISTS (SELECT 1 FROM property_tag pt WHERE pt.property_id = p.id AND pt.tag_id IN (${sql.join(placeholders, sql`, `)}))`, + ); + } + if (untagged) { + parts.push( + sql`NOT EXISTS (SELECT 1 FROM property_tag pt WHERE pt.property_id = p.id)`, + ); + } + if (parts.length === 0) return null; + if (parts.length === 1) return parts[0]; + return sql`(${sql.join(parts, sql` OR `)})`; + } } return null; } @@ -748,6 +780,49 @@ export async function getPropertiesCount( return parseInt(result.rows[0]?.count ?? "0", 10); } +/** + * The ids of every Property matching a filter selection — used by filter-driven + * Bulk tag assignment ("tag all N matching the current filter", ADR-0013). Same + * resolution + conditional joins as getPropertiesCount, but returns ids across + * the whole matching set, not a display page. Mirrors the table's `uprn IS NOT + * NULL` exclusion so the count the user saw and the set that gets tagged agree. + */ +export async function getFilteredPropertyIds( + portfolioId: string, + filterGroups: FilterGroups = [], +): Promise { + const combinedWhere = buildWhereClause(filterGroups); + const fields = filterFieldsInUse(filterGroups); + const needsEpcJoins = [...fields].some((f) => EPC_JOIN_FILTER_FIELDS.has(f)); + const needsPlanJoin = [...fields].some((f) => PLAN_JOIN_FILTER_FIELDS.has(f)); + + const epcJoins = needsEpcJoins + ? sql`LEFT JOIN property_details_epc epc ON epc.property_id = p.id + ${newApproachJoins}` + : sql``; + const planJoin = needsPlanJoin + ? sql`LEFT JOIN LATERAL ( + SELECT id, post_sap_points FROM plan + WHERE property_id = p.id + AND portfolio_id = p.portfolio_id + AND is_default = true + ORDER BY created_at DESC + LIMIT 1 + ) pl ON true` + : sql``; + + const result = await db.execute<{ id: string }>(sql` + SELECT DISTINCT p.id AS id + FROM property p + ${epcJoins} + ${planJoin} + WHERE p.portfolio_id = ${portfolioId} + AND p.uprn IS NOT NULL + ${combinedWhere} + `); + return result.rows.map((r) => BigInt(r.id)); +} + export async function getProperties( portfolioId: string, limit: number = 1000, @@ -798,7 +873,19 @@ export async function getProperties( ${totalFloorAreaSql(sql`epc`)} AS "totalFloorArea", ${carbonSql(sql`epc`)} AS "co2Emissions", ${mainfuelSql(sql`epc`)} AS mainfuel, - p.lexiscore AS lexiscore + p.lexiscore AS lexiscore, + -- Portfolio Tags on this property, name-ordered, as JSON (ADR-0013). A + -- correlated subquery keeps the main query at one row per property (a JOIN + -- would multiply rows and break the LATERAL aggregates above). + COALESCE(( + SELECT json_agg( + json_build_object('id', t.id::text, 'name', t.name, 'colour', t.colour) + ORDER BY t.name + ) + FROM property_tag pt + JOIN portfolio_tag t ON t.id = pt.tag_id + WHERE pt.property_id = p.id + ), '[]'::json) AS tags FROM property p -- LATERAL one-row lookups keep this query at "N properties × index probe" -- instead of hash-joining the multi-million-row plan/recommendation tables. diff --git a/src/app/utils/propertyFilters.ts b/src/app/utils/propertyFilters.ts index 1f85675e..d85609ef 100644 --- a/src/app/utils/propertyFilters.ts +++ b/src/app/utils/propertyFilters.ts @@ -13,7 +13,8 @@ export type FilterField = | "provenance" | "floorArea" | "co2Emissions" - | "mainfuel"; + | "mainfuel" + | "tags"; export type FilterOperator = | "contains" diff --git a/src/lib/modellingRuns/server.ts b/src/lib/modellingRuns/server.ts index 63f03c31..990915a7 100644 --- a/src/lib/modellingRuns/server.ts +++ b/src/lib/modellingRuns/server.ts @@ -141,6 +141,13 @@ export async function previewModellingRun(args: { if (args.filters.builtForms) { conditions.push(sql`resolved.bform IN (${inList(args.filters.builtForms)})`); } + if (args.filters.tagIds) { + // Tags resolve by direct membership (any-of), not the override→EPC rule — + // ADR-0013. The distributor mirrors this join against the shared DB. + conditions.push( + sql`resolved.id IN (SELECT pt.property_id FROM property_tag pt WHERE pt.tag_id IN (${inList(args.filters.tagIds)}))`, + ); + } const result = await db.execute<{ matched: number;