From bec5eaaf813e0dc8c65ec9cad176aff08c3d854d Mon Sep 17 00:00:00 2001 From: important-new Date: Wed, 29 Jul 2026 21:42:15 +0800 Subject: [PATCH 01/40] Two surfaces that were telling the reader something untrue (IA-89, IA-82) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit IA-89 — a client who had just paid $450 saw "BALANCE DUE $450" in the largest type on the page, a SENT badge, and a small green "Payment received" note. The paid layout was already well designed; the optimistic window simply reused the UNPAID one and bolted a reassurance box underneath. It now renders the paid layout with its wording swapped, so the only change the client sees when the webhook lands is "Processing" -> "Paid". The balance slot deliberately carries no amount while processing: the balance is not zero until the webhook says so, and restating $450 there is the contradiction this state exists to remove. Nothing is written — the webhook remains the settlement authority. IA-82 — /metrics had two aggregations with no reader. serviceBreakdown was computed on every request and did not appear in the page's own response interface; findings-heatmap was a fully defined route with no caller anywhere in the app. The second half was not the wire-up the backlog assumed. summariseHeatmap read `item.sectionName` and grouped on the raw `rating` string, and the persisted envelope has neither: it is keyed by composite findingKey, and `rating` holds a rating-level id. Wired as-is, every row would have landed under section "Unknown" with uuids for column headers. Its unit tests passed because they invented the input. So it is rewritten. Sections come from parsing the findingKey and resolving the id against the tenant's templates; columns are the tenant's own rating levels, minus Not Inspected / Not Present, which record the absence of a condition rather than a finding. That also dissolves the "fold 6 buckets into 3" product question the register had been carrying — no fold is needed, and folding on severity would have merged Monitor into Marginal, since both carry severity `marginal` and Marginal is the most common rating in real commercial data. Ratings matching no known level are counted as `unresolved` rather than invented into a column of their own. The endpoint takes the same `period` the page's selector uses, and the loader fetches it separately so a slow findings read cannot blank the revenue KPIs. Also moves the 21 existing `metrics_*` keys into `messages/en/metrics.json`, which until now held only its `$schema`. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_019iXRoN1WME2DJpssV5dzwA --- .../portal/sections/InvoiceDisplay.test.tsx | 98 +++++++++++ .../portal/sections/InvoiceDisplay.tsx | 72 +++++--- app/routes/metrics.test.tsx | 94 +++++++++- app/routes/metrics.tsx | 94 +++++++++- messages/en/library.json | 22 --- messages/en/metrics.json | 31 +++- messages/en/reports.json | 3 + server/api/analytics.ts | 13 +- server/lib/analytics.ts | 163 +++++++++++++++--- server/services/analytics.service.ts | 73 +++++++- tests/unit/analytics/analytics.spec.ts | 140 ++++++++++++--- 11 files changed, 684 insertions(+), 119 deletions(-) create mode 100644 app/components/portal/sections/InvoiceDisplay.test.tsx diff --git a/app/components/portal/sections/InvoiceDisplay.test.tsx b/app/components/portal/sections/InvoiceDisplay.test.tsx new file mode 100644 index 000000000..7bbbe904c --- /dev/null +++ b/app/components/portal/sections/InvoiceDisplay.test.tsx @@ -0,0 +1,98 @@ +/** + * IA-89 — what a client sees in the seconds between paying and the webhook + * settling the invoice. + * + * Stripe redirects back to the Hub with `?redirect_status=succeeded` while the + * invoice row still says `sent`. That window used to render the UNPAID layout — + * a `SENT` badge and "BALANCE DUE $450" in the largest type on the page — with a + * small green "Payment received" note underneath. The three signals contradicted + * each other, and the loudest one told a client who had just been charged that + * they still owed the money. + * + * These assertions pin the fix: the optimistic window renders the PAID layout + * with its wording swapped, so the only change the client sees when the webhook + * lands is "Processing" → "Paid". + */ +import { describe, it, expect } from "vitest"; +import { render } from "@testing-library/react"; +import { createRoutesStub } from "react-router"; + +import { InvoiceDisplay } from "./InvoiceDisplay"; +import type { InvoiceData } from "./payment-helpers"; +import type { TenantBrand } from "~/lib/brand"; + +const BRAND = { companyName: "Acme Inspections", primaryColor: "#2563eb" } as TenantBrand; + +const UNPAID: InvoiceData = { + number: "INV-1042", + date: "2026-07-26", + dueDate: null, + status: "sent", + clientName: "Dana Client", + inspectorName: "Sam Inspector", + lineItems: [{ description: "Home inspection", amount: 450 }], + total: 450, +}; + +/** + * Rendered inside a data router because the unpaid branch mounts + * , which reads the session context off the route loader data. + */ +function renderInvoice(invoice: InvoiceData, justPaid: boolean) { + const Stub = createRoutesStub([ + { + path: "/", + Component: () => ( + + ), + }, + ]); + return render(); +} + +describe("InvoiceDisplay optimistic post-payment state", () => { + it("never shows a balance due after a successful redirect", () => { + const { queryByText, getByText } = renderInvoice(UNPAID, true); + + // The contradiction, stated as an assertion: the loudest element must not + // be asking for money that was just paid. + expect(queryByText("Balance due")).toBeNull(); + expect(getByText("Balance")).toBeTruthy(); + expect(getByText("Finalizing receipt")).toBeTruthy(); + }); + + it("credits the payment in the totals block, as the paid layout does", () => { + const { getByText } = renderInvoice(UNPAID, true); + expect(getByText("Amount paid")).toBeTruthy(); + expect(getByText("−$450")).toBeTruthy(); + }); + + it("replaces the SENT badge and stamps the document Processing", () => { + const { queryByText, getByText } = renderInvoice(UNPAID, true); + expect(queryByText("sent")).toBeNull(); + expect(getByText("processing")).toBeTruthy(); + expect(getByText("Processing")).toBeTruthy(); + expect(queryByText("Paid")).toBeNull(); + }); + + it("hides the pay form so the client cannot pay twice", () => { + const { queryByText } = renderInvoice(UNPAID, true); + expect(queryByText(/Pay this invoice/i)).toBeNull(); + }); + + it("still renders the unpaid layout before the redirect", () => { + const { getByText, queryByText } = renderInvoice(UNPAID, false); + expect(getByText("Balance due")).toBeTruthy(); + expect(getByText("sent")).toBeTruthy(); + expect(queryByText("Finalizing receipt")).toBeNull(); + expect(queryByText("Amount paid")).toBeNull(); + }); + + it("settles into the paid layout once the invoice says paid", () => { + const { getByText, queryByText } = renderInvoice({ ...UNPAID, status: "paid" }, false); + expect(getByText("Paid")).toBeTruthy(); + expect(getByText("Amount paid")).toBeTruthy(); + expect(getByText("$0")).toBeTruthy(); + expect(queryByText("Finalizing receipt")).toBeNull(); + }); +}); diff --git a/app/components/portal/sections/InvoiceDisplay.tsx b/app/components/portal/sections/InvoiceDisplay.tsx index 61af7a191..2771e0893 100644 --- a/app/components/portal/sections/InvoiceDisplay.tsx +++ b/app/components/portal/sections/InvoiceDisplay.tsx @@ -31,20 +31,38 @@ export function InvoiceDisplay({ invoice, brand, inspectionId, portalToken, just const total = invoice.total; const isPaid = invoice.status === "paid"; const isVoid = invoice.status === "void"; - const amountPaid = isPaid ? total : 0; const balanceDue = isPaid ? 0 : total; const payable = !isPaid && !isVoid && balanceDue > 0; + // IA-89 — Stripe has redirected back but the webhook has not settled the + // invoice yet. This is a PRESENTATION state only: nothing is written, the + // webhook stays the settlement authority. + const processing = payable && justPaid; + // Everything below keys off `settled` rather than `isPaid`, so the optimistic + // state renders the PAID layout (stamp, "Amount paid", zero balance) with its + // wording swapped — not the unpaid layout with a reassurance box bolted on. + // It used to render the latter: a client who had just paid $450 saw + // "BALANCE DUE $450" in the largest type on the page, a `SENT` badge, and a + // small green note claiming payment was received. The three signals + // contradicted each other at the most trust-sensitive moment in the journey, + // and the largest one was the wrong one. + const settled = isPaid || processing; + const amountPaid = settled ? total : 0; // Phase B — every amount on this document renders in the invoice's snapshot // currency, not the tenant's live setting. `money()` defaults to USD when absent. const cur = { currency: invoice.currency }; return (
- {/* PAID stamp */} - {isPaid && ( + {/* PAID stamp — "PROCESSING" until the webhook settles, so the client + watches one element change wording rather than the page change shape. */} + {settled && (
- - {m.portal_invoice_paid_stamp()} + + {processing ? m.portal_invoice_processing_stamp() : m.portal_invoice_paid_stamp()}
)} @@ -58,8 +76,8 @@ export function InvoiceDisplay({ invoice, brand, inspectionId, portalToken, just {invoice.number}
- - {invoice.status} + + {processing ? m.portal_invoice_status_processing() : invoice.status} @@ -92,39 +110,41 @@ export function InvoiceDisplay({ invoice, brand, inspectionId, portalToken, just {discountTotal < 0 && } - {isPaid && } + {settled && }
- {isPaid ? m.portal_invoice_balance() : m.portal_invoice_balance_due()} - 0 ? "text-ih-fg-1" : "text-ih-ok-fg"}`}> - {money(balanceDue, cur)} - + {settled ? m.portal_invoice_balance() : m.portal_invoice_balance_due()} + {processing ? ( + // The amount is deliberately absent: the balance is not zero until + // the webhook says so, and restating "$450" here is exactly the + // contradiction this state exists to remove. + + {m.portal_invoice_finalizing_short()} + + ) : ( + 0 ? "text-ih-fg-1" : "text-ih-ok-fg"}`}> + {money(balanceDue, cur)} + + )}
{/* Pay panel — Stripe Payment Element (bring-your-own-keys) */} - {payable && !justPaid && ( + {payable && !processing && (
)} - {/* Optimistic post-redirect state — webhook settles the invoice async */} - {payable && justPaid && ( -
-
-

{m.portal_invoice_payment_received()}

-

{m.portal_invoice_finalizing()}

-
-
- )} - - {/* Paid confirmation */} - {isPaid && ( + {/* Confirmation — one block for both settled states; only the second line + differs (what happens next vs. what to do with the receipt). */} + {settled && (

{m.portal_invoice_payment_received()}

-

{m.portal_invoice_keep_receipt()}

+

+ {processing ? m.portal_invoice_finalizing() : m.portal_invoice_keep_receipt()} +

)} diff --git a/app/routes/metrics.test.tsx b/app/routes/metrics.test.tsx index 197e0eab0..57a4c486f 100644 --- a/app/routes/metrics.test.tsx +++ b/app/routes/metrics.test.tsx @@ -21,12 +21,15 @@ const SERVER_MONTHLY = [ { month: "2026-05", count: 7, revenue: 4200 }, ]; -function renderMetrics(data: Record | null) { +function renderMetrics( + data: Record | null, + findings: Record | null = null, +) { const Stub = createRoutesStub([ { path: "/metrics", Component: MetricsPage, - loader: () => ({ data, period: "6m" }), + loader: () => ({ data, findings, period: "6m" }), }, ]); return render(); @@ -91,3 +94,90 @@ describe("MetricsPage monthly charts", () => { expect(empties.length).toBeGreaterThan(0); }); }); + +/** + * IA-82 — two server aggregations reached the page and rendered nothing. + * + * `serviceBreakdown` was computed by `/api/metrics` on every request and did not + * appear in the page's own response interface, let alone its markup. The + * findings matrix had a whole endpoint (`GET /api/analytics/findings-heatmap`) + * with no caller anywhere in the app. A fully-specified route with no reader is + * indistinguishable from a feature until someone checks. + */ +describe("MetricsPage findings matrix", () => { + const FINDINGS = { + columns: [ + { key: "satisfactory", label: "Satisfactory", color: "#10b981" }, + { key: "monitor", label: "Monitor", color: "#f59e0b" }, + { key: "defect", label: "Defect", color: "#ef4444" }, + ], + rows: [ + { section: "Roof", counts: { satisfactory: 4, defect: 2 }, total: 6 }, + { section: "Electrical", counts: { monitor: 1 }, total: 1 }, + ], + total: 7, + }; + + it("renders a column per rating level and a row per section", async () => { + const { findByText, queryByText } = renderMetrics( + { totalInspections: 2, totalRevenue: 0, avgOrderValue: 0, monthly: [], topAgents: [], byInspector: [] }, + FINDINGS, + ); + + // Column headers come from the tenant's own levels, not a hardcoded scale. + await findByText("Satisfactory"); + await findByText("Monitor"); + await findByText("Defect"); + // Section rows. + await findByText("Roof"); + await findByText("Electrical"); + expect(queryByText(/No rated findings in this period/i)).toBeNull(); + }); + + it("falls back to the empty state when the findings fetch failed", async () => { + // The loader returns null for findings when its endpoint errors — the page + // must still render (the KPIs are the primary content), just without a matrix. + const { findByText } = renderMetrics( + { totalInspections: 2, totalRevenue: 0, avgOrderValue: 0, monthly: [], topAgents: [], byInspector: [] }, + null, + ); + await findByText(/No rated findings in this period/i); + }); +}); + +describe("MetricsPage service mix", () => { + it("renders the serviceBreakdown the endpoint has always returned", async () => { + const { findByText, queryByText } = renderMetrics({ + totalInspections: 3, + totalRevenue: 90_000, + avgOrderValue: 30_000, + monthly: [], + topAgents: [], + byInspector: [], + serviceBreakdown: [ + { serviceName: "Radon Test", count: 2, revenue: 25_000 }, + { serviceName: "Sewer Scope", count: 1, revenue: 65_000 }, + ], + }); + + await findByText("Radon Test"); + await findByText("Sewer Scope"); + // Revenue is integer cents, same as every other figure on this page. + await findByText("$250"); + await findByText("$650"); + expect(queryByText(/No service data yet/i)).toBeNull(); + }); + + it("shows the empty state when no services are attached", async () => { + const { findByText } = renderMetrics({ + totalInspections: 0, + totalRevenue: 0, + avgOrderValue: 0, + monthly: [], + topAgents: [], + byInspector: [], + serviceBreakdown: [], + }); + await findByText(/No service data yet/i); + }); +}); diff --git a/app/routes/metrics.tsx b/app/routes/metrics.tsx index 7d7c6e807..e8fd9e2c8 100644 --- a/app/routes/metrics.tsx +++ b/app/routes/metrics.tsx @@ -21,28 +21,58 @@ interface MetricsData { monthly: { month: string; count: number; revenue: number }[]; topAgents: { agentName: string; count: number; revenue: number }[]; byInspector: { inspectorId: string | null; inspectorName: string; count: number; revenue: number; avgTurnaroundDays: number | null }[]; + // IA-82 — the endpoint has always computed and returned this; nothing rendered + // it, so the aggregation ran for no reader. + serviceBreakdown: { serviceName: string; count: number; revenue: number }[]; } +/** Mirrors `FindingsMatrix` from server/lib/analytics.ts. */ +interface FindingsData { + columns: { key: string; label: string; color: string }[]; + rows: { section: string; counts: Record; total: number }[]; + total: number; +} + +type FindingsRow = FindingsData["rows"][number]; + export async function loader({ request, context }: Route.LoaderArgs) { const token = await requireToken(context, request); const url = new URL(request.url); const periodParam = url.searchParams.get("period") ?? "6m"; const period = (["3m", "6m", "12m"].includes(periodParam) ? periodParam : "6m") as "3m" | "6m" | "12m"; + const api = createApi(context, { token }); + + let data: MetricsData | null = null; try { - const api = createApi(context, { token }); const res = await api.metrics.index.$get({ query: { period } }); const body = res.ok ? ((await res.json()) as Record) : {}; const d = (body.data ?? {}) as Record; - return { data: (Object.keys(d).length > 0 ? d : null) as MetricsData | null, period }; + data = (Object.keys(d).length > 0 ? d : null) as MetricsData | null; + } catch { + data = null; + } + + // IA-82 — the findings matrix is a second aggregation with its own endpoint. + // It is fetched separately (and fails alone) so a slow or erroring findings + // read cannot blank the revenue KPIs, which are the page's primary content. + let findings: FindingsData | null = null; + try { + const res = await api.analytics["findings-heatmap"].$get({ query: { period } }); + if (res.ok) { + const body = (await res.json()) as { data?: FindingsData }; + findings = body.data ?? null; + } } catch { - return { data: null, period }; + findings = null; } + + return { data, findings, period }; } const PERIODS = ["3m", "6m", "12m"] as const; export default function MetricsPage() { - const { data, period: initialPeriod } = useLoaderData(); + const { data, findings, period: initialPeriod } = useLoaderData(); const navigate = useNavigate(); const locale = useDisplayLocale(); const currency = useDisplayCurrency(); @@ -178,6 +208,62 @@ export default function MetricsPage() { )} + {/* Findings by section — the tenant's own rating levels as columns. + Not Inspected / Not Present are excluded server-side: they record the + absence of a condition, so counting them would let a mostly-unbuilt + section outrank one full of real defects. */} + +

{m.metrics_findings_title()}

+ {findings && findings.rows.length > 0 && findings.columns.length > 0 ? ( +
+ + rows={findings.rows} + getRowKey={(row) => row.section} + columns={[ + { label: m.metrics_col_section(), cell: (row) => {row.section} }, + ...findings.columns.map((col) => ({ + label: ( + + + {col.label} + + ), + align: "center" as const, + cell: (row: FindingsRow) => ( + + {row.counts[col.key] ?? "—"} + + ), + })), + { label: m.metrics_col_total(), align: "right", cell: (row) => {row.total} }, + ]} + /> +
+ ) : ( +

{m.metrics_no_findings()}

+ )} + + + {/* Service mix */} + +

{m.metrics_services_title()}

+ {data && data.serviceBreakdown?.length > 0 ? ( +
+ + rows={data.serviceBreakdown} + getRowKey={(row) => row.serviceName} + columns={[ + { label: m.metrics_col_service(), cell: (row) => {row.serviceName} }, + { label: m.metrics_col_inspections(), align: "center", cell: (row) => {row.count} }, + { label: m.metrics_col_revenue(), align: "right", cell: (row) => {fmt(row.revenue)} }, + ]} + /> +
+ ) : ( +

{m.metrics_no_services()}

+ )} + + {/* Top agents */}

{m.metrics_top_agents()}

diff --git a/messages/en/library.json b/messages/en/library.json index 9fa944e19..8628599c5 100644 --- a/messages/en/library.json +++ b/messages/en/library.json @@ -131,28 +131,6 @@ "notifications_meta": "{count} notifications", "notifications_empty_title": "No notifications", "notifications_empty_desc": "You're all caught up.", - "metrics_meta_title": "Metrics - OpenInspection", - "metrics_heading": "Metrics", - "metrics_meta": "{count} inspections", - "metrics_loading": "Loading...", - "metrics_kpi_revenue": "Total Revenue", - "metrics_kpi_inspections": "Total Inspections", - "metrics_kpi_aov": "Avg Order Value", - "metrics_chart_inspections": "Inspections per Month", - "metrics_no_data": "No data available for this period.", - "metrics_chart_revenue": "Revenue per Month", - "metrics_no_revenue": "No revenue data available for this period.", - "metrics_top_agents": "Top Referring Agents", - "metrics_agent_count": "{count} insp", - "metrics_no_agents": "No agent data yet.", - "metrics_by_inspector": "By Inspector", - "metrics_col_inspector": "Inspector", - "metrics_col_inspections": "Inspections", - "metrics_col_revenue": "Revenue", - "metrics_col_turnaround": "Avg turnaround", - "metrics_turnaround_days": "{days}d", - "metrics_turnaround_na": "—", - "metrics_no_inspectors": "No inspector data yet.", "docs_meta_title": "API Docs - OpenInspection", "misc_not_found_meta_title": "Page Not Found - OpenInspection", "misc_not_found_heading": "Page not found", diff --git a/messages/en/metrics.json b/messages/en/metrics.json index 006f618aa..0a9716b7c 100644 --- a/messages/en/metrics.json +++ b/messages/en/metrics.json @@ -1,3 +1,32 @@ { - "$schema": "https://inlang.com/schema/inlang-message-format" + "$schema": "https://inlang.com/schema/inlang-message-format", + "metrics_meta_title": "Metrics - OpenInspection", + "metrics_heading": "Metrics", + "metrics_meta": "{count} inspections", + "metrics_loading": "Loading...", + "metrics_kpi_revenue": "Total Revenue", + "metrics_kpi_inspections": "Total Inspections", + "metrics_kpi_aov": "Avg Order Value", + "metrics_chart_inspections": "Inspections per Month", + "metrics_no_data": "No data available for this period.", + "metrics_chart_revenue": "Revenue per Month", + "metrics_no_revenue": "No revenue data available for this period.", + "metrics_top_agents": "Top Referring Agents", + "metrics_agent_count": "{count} insp", + "metrics_no_agents": "No agent data yet.", + "metrics_by_inspector": "By Inspector", + "metrics_col_inspector": "Inspector", + "metrics_col_inspections": "Inspections", + "metrics_col_revenue": "Revenue", + "metrics_col_turnaround": "Avg turnaround", + "metrics_turnaround_days": "{days}d", + "metrics_turnaround_na": "—", + "metrics_no_inspectors": "No inspector data yet.", + "metrics_findings_title": "Findings by Section", + "metrics_col_section": "Section", + "metrics_col_total": "Total", + "metrics_no_findings": "No rated findings in this period.", + "metrics_services_title": "Service Mix", + "metrics_col_service": "Service", + "metrics_no_services": "No service data yet." } diff --git a/messages/en/reports.json b/messages/en/reports.json index 1480ae0eb..89061bca7 100644 --- a/messages/en/reports.json +++ b/messages/en/reports.json @@ -170,6 +170,9 @@ "portal_repair_copied": "Copied!", "portal_repair_copy_failed": "Copy failed", "portal_invoice_paid_stamp": "Paid", + "portal_invoice_processing_stamp": "Processing", + "portal_invoice_status_processing": "processing", + "portal_invoice_finalizing_short": "Finalizing receipt", "portal_invoice_eyebrow": "Invoice", "portal_invoice_field_from": "From", "portal_invoice_field_bill_to": "Bill to", diff --git a/server/api/analytics.ts b/server/api/analytics.ts index ff226b02c..e54ff4177 100644 --- a/server/api/analytics.ts +++ b/server/api/analytics.ts @@ -27,10 +27,13 @@ const heatmapRoute = createRoute(withMcpMetadata({ method: 'get', path: '/findings-heatmap', tags: ["metrics"], - summary: 'Section × rating bucket counts across this tenant\'s inspections', + summary: 'Section × rating-level counts across this tenant\'s inspections', + request: { query: z.object({ + period: z.enum(['3m', '6m', '12m']).default('12m').describe('Trailing window the counts cover, matching the /metrics period selector.'), + }).describe('Trailing-window selector.') }, responses: { 200: { description: 'ok' } }, operationId: "listAnalyticFindingsHeatmap", - description: "Auto-generated placeholder for listAnalyticFindingsHeatmap (GET /findings-heatmap, metrics domain). TODO: replace with a real description sourced from the handler." + description: "Counts rated items by template section and rating level over the trailing period. Columns are the tenant's own rating levels minus Not Inspected / Not Present; rows are template sections, ordered by volume." }, { scopes: ['read'], tier: 'extended' })); const analyticsRoutes = createApiRouter() @@ -44,7 +47,11 @@ const analyticsRoutes = createApiRouter() .openapi(heatmapRoute, async (c) => { const tenantId = c.get('tenantId'); if (!tenantId) throw Errors.Unauthorized('Missing tenant scope'); - const out = await c.var.services.analytics.findingsHeatmap(tenantId); + const { period } = c.req.valid('query'); + const months = period === '3m' ? 3 : period === '6m' ? 6 : 12; + const from = new Date(); + from.setMonth(from.getMonth() - months); + const out = await c.var.services.analytics.findingsHeatmap(tenantId, from.toISOString().slice(0, 10)); return c.json({ success: true as const, data: out }, 200); }); diff --git a/server/lib/analytics.ts b/server/lib/analytics.ts index 51eab529e..9153428c0 100644 --- a/server/lib/analytics.ts +++ b/server/lib/analytics.ts @@ -8,14 +8,16 @@ * `anchorYm` (YYYY-MM). Missing months are surfaced as zero * counts so the chart renders continuous gridlines. * - * • summariseHeatmap(resultsRows) - * Flattens the per-inspection results.data envelope into - * (section, category, count) cells. Missing sectionName lands - * under "Unknown"; missing rating is skipped entirely. + * • summariseFindings(resultsRows, ctx) + * Flattens the per-inspection results.data envelopes into a + * section × rating-level matrix. See its own doc comment for why + * it needs a resolution context rather than reading the envelope + * alone. * * Splitting these out keeps the SQL-touching service surface * minimal and the logic deterministic / unit-testable. */ +import { parseFindingKey } from './finding-key'; export interface InspectionRow { createdAt: string | Date; @@ -71,36 +73,147 @@ export function groupInspectionsByMonth( } export interface HeatmapItem { - sectionName?: string; - rating?: string; + rating?: unknown; } -export interface HeatmapCell { - section: string; - category: string; - count: number; +/** The subset of a rating level this aggregator needs. */ +export interface HeatmapLevel { + id: string; + label: string; + abbreviation: string; + color: string; + severity: 'good' | 'marginal' | 'significant' | 'minor'; + isDefect: boolean; + order?: number; } -export function summariseHeatmap( +export interface FindingsColumn { + /** Stable slug of the level label — the key inside every row's `counts`. */ + key: string; + label: string; + color: string; +} + +export interface FindingsRow { + section: string; + counts: Record; + total: number; +} + +export interface FindingsMatrix { + columns: FindingsColumn[]; + rows: FindingsRow[]; + /** Rated items counted into the matrix (excludes the NI/NP levels). */ + total: number; + /** Rated items dropped because their rating matched no known level. */ + unresolved: number; +} + +export const UNKNOWN_SECTION = 'Unknown'; + +/** Level label → the `counts` key. Lowercase, non-alphanumerics collapsed. */ +export function findingsColumnKey(label: string): string { + return label.trim().toLowerCase().replace(/[^a-z0-9]+/g, '_').replace(/^_|_$/g, '') || 'unlabelled'; +} + +/** + * Is this level a "not a condition" level — Not Inspected / Not Present? + * + * Mirrors `getNaKind` in report-utils (same abbreviation-then-label test) but + * takes the level directly instead of an id + list, because this aggregator has + * already resolved the level and importing report-utils would drag the whole + * report stats surface into the analytics path. + */ +function isNonCondition(level: HeatmapLevel): boolean { + if (level.isDefect || level.severity !== 'minor') return false; + const abbr = level.abbreviation.trim().toUpperCase(); + if (abbr === 'NP' || abbr === 'NI') return true; + const label = level.label.trim().toLowerCase(); + return /not\s*present/.test(label) || /not\s*inspected/.test(label); +} + +/** + * Build the section × rating-level matrix behind the /metrics findings card. + * + * **Why this needs a resolution context.** The persisted envelope + * (`inspection_results.data`) is keyed by composite findingKey and each entry + * holds only the fields the editor writes — `rating`, `notes`, `value`, + * `canned`, `defectFields`, `itemAttribute`. It carries neither a section name + * nor a human-readable rating: `rating` is a rating-level **id**. So the section + * comes from parsing the key and looking the id up in `sectionTitles`, and the + * column vocabulary comes from the tenant's own rating levels. + * + * **Why the columns are the tenant's levels rather than a fixed set.** Rating + * systems are per-tenant and differ in arity — the residential seed has + * Satisfactory/Monitor/Defect, the commercial one adds Marginal, Low + * Maintenance and Hazard. Folding those onto a fixed 3- or 4-column scale would + * silently merge distinct levels (Monitor and Marginal both carry severity + * `marginal`, so a severity-keyed fold loses one of them outright). Competitors + * publish 4–5 levels for the same reason. Levels that describe the absence of a + * condition — Not Inspected / Not Present — are excluded: they are not findings. + * + * Legacy envelopes wrote the level's label (`"Satisfactory"`) rather than its + * id, so a rating is matched against id, then label, then abbreviation. + * Anything still unmatched is counted in `unresolved` rather than invented into + * a column of its own. + */ +export function summariseFindings( inspectionResultsRows: Array>, -): { cells: HeatmapCell[] } { - const counts: Record> = {}; + ctx: { sectionTitles: Record; levels: HeatmapLevel[] }, +): FindingsMatrix { + const conditionLevels = ctx.levels.filter((l) => !isNonCondition(l)); - for (const row of inspectionResultsRows) { - for (const item of Object.values(row)) { - const section = item.sectionName ?? 'Unknown'; - const cat = item.rating; - if (!cat) continue; - counts[section] ??= {}; - counts[section][cat] = (counts[section][cat] ?? 0) + 1; + // One column per distinct label. Two rating systems in the same tenant can + // both define "Monitor"; they are the same column to a reader. + const columns: FindingsColumn[] = []; + const columnKeyByLabel = new Map(); + for (const level of [...conditionLevels].sort((a, b) => (a.order ?? 0) - (b.order ?? 0))) { + const label = level.label.trim(); + if (columnKeyByLabel.has(label.toLowerCase())) continue; + const key = findingsColumnKey(label); + columnKeyByLabel.set(label.toLowerCase(), key); + columns.push({ key, label, color: level.color }); + } + + // rating value (id | label | abbreviation) -> column key. Non-condition + // levels map to null so their ratings are dropped, not counted as unresolved. + const columnByRating = new Map(); + for (const level of ctx.levels) { + const key = isNonCondition(level) ? null : (columnKeyByLabel.get(level.label.trim().toLowerCase()) ?? null); + for (const alias of [level.id, level.label, level.abbreviation]) { + const norm = String(alias ?? '').trim().toLowerCase(); + if (norm && !columnByRating.has(norm)) columnByRating.set(norm, key); } } - const cells: HeatmapCell[] = []; - for (const [section, byCat] of Object.entries(counts)) { - for (const [category, count] of Object.entries(byCat)) { - cells.push({ section, category, count }); + const bySection = new Map>(); + let total = 0; + let unresolved = 0; + + for (const row of inspectionResultsRows) { + for (const [key, item] of Object.entries(row)) { + const rating = typeof item?.rating === 'string' ? item.rating.trim() : ''; + if (!rating) continue; + const column = columnByRating.get(rating.toLowerCase()); + if (column === undefined) { unresolved++; continue; } // no such level + if (column === null) continue; // Not Inspected / Not Present — not a finding. + + const { sectionId } = parseFindingKey(key); + const section = ctx.sectionTitles[sectionId] ?? UNKNOWN_SECTION; + const counts = bySection.get(section) ?? {}; + counts[column] = (counts[column] ?? 0) + 1; + bySection.set(section, counts); + total++; } } - return { cells }; + + const rows: FindingsRow[] = [...bySection.entries()] + .map(([section, counts]) => ({ + section, + counts, + total: Object.values(counts).reduce((s, n) => s + n, 0), + })) + .sort((a, b) => b.total - a.total || a.section.localeCompare(b.section)); + + return { columns, rows, total, unresolved }; } diff --git a/server/services/analytics.service.ts b/server/services/analytics.service.ts index 36f6fea49..d48dd2ca2 100644 --- a/server/services/analytics.service.ts +++ b/server/services/analytics.service.ts @@ -10,11 +10,13 @@ * be unit-tested without a Hono context. */ import { drizzle } from 'drizzle-orm/d1'; -import { eq } from 'drizzle-orm'; -import { inspections, inspectionResults } from '../lib/db/schema'; +import { and, eq, gte } from 'drizzle-orm'; +import { inspections, inspectionResults, ratingSystems, templates } from '../lib/db/schema'; import { groupInspectionsByMonth, - summariseHeatmap, + summariseFindings, + type FindingsMatrix, + type HeatmapLevel, type MonthBucket, type HeatmapItem, } from '../lib/analytics'; @@ -53,15 +55,70 @@ export class AnalyticsService { return { months: buckets }; } - async findingsHeatmap(tenantId: string) { + /** + * Section × rating-level counts for the /metrics findings card. + * + * Three reads, because the result envelope alone cannot answer the + * question (see `summariseFindings`): the envelopes themselves, the + * tenant's templates (section id → title), and the tenant's rating systems + * (level id → label/colour). Templates and rating systems are both small, + * per-tenant tables — the envelopes are the only unbounded read, and + * `fromDate` bounds them to the requested window. + * + * Section titles come from the live templates rather than each inspection's + * `template_snapshot`: section ids survive the snapshot copy, so the live + * template resolves the same ids without loading one large JSON blob per + * inspection. An inspection whose section was since deleted from its + * template falls into the "Unknown" row. + */ + async findingsHeatmap(tenantId: string, fromDate?: string): Promise { const db = this.getDrizzle(); - const rows = await db.select({ data: inspectionResults.data }) + + const resultRows = await db.select({ data: inspectionResults.data }) .from(inspectionResults) - .where(eq(inspectionResults.tenantId, tenantId)) + .innerJoin(inspections, and( + eq(inspections.id, inspectionResults.inspectionId), + eq(inspections.tenantId, inspectionResults.tenantId), + )) + .where(fromDate + ? and(eq(inspectionResults.tenantId, tenantId), gte(inspections.date, fromDate)) + : eq(inspectionResults.tenantId, tenantId)) + .all(); + + const templateRows = await db.select({ schema: templates.schema }) + .from(templates) + .where(eq(templates.tenantId, tenantId)) + .all(); + + const ratingRows = await db.select({ levels: ratingSystems.levels }) + .from(ratingSystems) + .where(eq(ratingSystems.tenantId, tenantId)) .all(); - const envelopes = rows.map(r => + + const sectionTitles: Record = {}; + for (const row of templateRows) { + const schema = safeJsonParse<{ sections?: Array<{ id?: unknown; title?: unknown; name?: unknown }> }>(row.schema, {}); + for (const section of schema.sections ?? []) { + const id = typeof section?.id === 'string' ? section.id : ''; + const title = typeof section?.title === 'string' && section.title + ? section.title + : typeof section?.name === 'string' ? section.name : ''; + if (id && title) sectionTitles[id] ??= title; + } + } + + const levels: HeatmapLevel[] = []; + for (const row of ratingRows) { + for (const level of safeJsonParse(row.levels, [])) { + if (level && typeof level.id === 'string' && typeof level.label === 'string') { + levels.push({ ...level, abbreviation: level.abbreviation ?? '' }); + } + } + } + + const envelopes = resultRows.map(r => safeJsonParse>(r.data, {}), ); - return summariseHeatmap(envelopes); + return summariseFindings(envelopes, { sectionTitles, levels }); } } diff --git a/tests/unit/analytics/analytics.spec.ts b/tests/unit/analytics/analytics.spec.ts index b92d00b1d..badf51744 100644 --- a/tests/unit/analytics/analytics.spec.ts +++ b/tests/unit/analytics/analytics.spec.ts @@ -6,7 +6,7 @@ * here without any DB plumbing. */ import { describe, it, expect } from 'vitest'; -import { groupInspectionsByMonth, summariseHeatmap } from '../../../server/lib/analytics'; +import { groupInspectionsByMonth, summariseFindings, type HeatmapLevel } from '../../../server/lib/analytics'; describe('groupInspectionsByMonth (subsystem E P7.1)', () => { it('returns N empty buckets when no inspections exist', () => { @@ -61,42 +61,126 @@ describe('groupInspectionsByMonth (subsystem E P7.1)', () => { }); }); -describe('summariseHeatmap (subsystem E P7.1)', () => { - it('returns empty cells when no inspections exist', () => { - expect(summariseHeatmap([])).toEqual({ cells: [] }); +/** + * IA-82 — the findings matrix behind /metrics. + * + * The predecessor of this aggregator read `item.sectionName` off each result + * entry and grouped by the raw `rating` string. Neither field exists in a real + * envelope: `applyResultsBatch` (and the single-field patch path) writes only + * `rating | notes | value | canned | defectFields | itemAttribute`, keyed by the + * composite findingKey, and `rating` holds a rating-level **id**. Its tests + * passed because they invented the input. Every fixture below is shaped like + * what the database actually stores. + */ +const LEVELS: HeatmapLevel[] = [ + { id: 'lv-sat', label: 'Satisfactory', abbreviation: 'Sat', color: '#10b981', severity: 'good', isDefect: false, order: 0 }, + { id: 'lv-mon', label: 'Monitor', abbreviation: 'Mon', color: '#f59e0b', severity: 'marginal', isDefect: false, order: 1 }, + { id: 'lv-mar', label: 'Marginal', abbreviation: 'Mar', color: '#f59e0b', severity: 'marginal', isDefect: false, order: 2 }, + { id: 'lv-def', label: 'Defect', abbreviation: 'D', color: '#ef4444', severity: 'significant', isDefect: true, order: 3 }, + { id: 'lv-ni', label: 'Not Inspected', abbreviation: 'NI', color: '#94a3b8', severity: 'minor', isDefect: false, order: 4 }, + { id: 'lv-np', label: 'Not Present', abbreviation: 'NP', color: '#cbd5e1', severity: 'minor', isDefect: false, order: 5 }, +]; + +const SECTIONS = { 'sec-roof': 'Roof', 'sec-elec': 'Electrical' }; +const CTX = { sectionTitles: SECTIONS, levels: LEVELS }; + +/** A findingKey as `applyResultsBatch` writes it: `unit:section:item`. */ +const key = (section: string, item: string) => `_default:${section}:${item}`; + +describe('summariseFindings (IA-82)', () => { + it('returns an empty matrix when no inspections exist', () => { + const out = summariseFindings([], CTX); + expect(out.rows).toEqual([]); + expect(out.total).toBe(0); }); - it('counts ratings per (section, category) bucket', () => { - const out = summariseHeatmap([ + it('counts rating-level ids per template section', () => { + const out = summariseFindings([ { - 'i-1': { sectionName: 'Roof', rating: 'Defect' }, - 'i-2': { sectionName: 'Roof', rating: 'Defect' }, - 'i-3': { sectionName: 'Roof', rating: 'Satisfactory' }, + [key('sec-roof', 'i-1')]: { rating: 'lv-def' }, + [key('sec-roof', 'i-2')]: { rating: 'lv-def' }, + [key('sec-roof', 'i-3')]: { rating: 'lv-sat' }, }, + { [key('sec-elec', 'i-4')]: { rating: 'lv-mon' } }, + ], CTX); + + const roof = out.rows.find(r => r.section === 'Roof'); + const elec = out.rows.find(r => r.section === 'Electrical'); + expect(roof?.counts.defect).toBe(2); + expect(roof?.counts.satisfactory).toBe(1); + expect(roof?.total).toBe(3); + expect(elec?.counts.monitor).toBe(1); + expect(out.total).toBe(4); + }); + + it('keeps Monitor and Marginal as separate columns', () => { + // Both carry severity 'marginal', so any severity-keyed fold would merge + // them — and Marginal is the most common rating in real commercial data. + const out = summariseFindings([ + { [key('sec-roof', 'i-1')]: { rating: 'lv-mon' }, [key('sec-roof', 'i-2')]: { rating: 'lv-mar' } }, + ], CTX); + expect(out.columns.map(c => c.key)).toEqual(['satisfactory', 'monitor', 'marginal', 'defect']); + const roof = out.rows.find(r => r.section === 'Roof'); + expect(roof?.counts.monitor).toBe(1); + expect(roof?.counts.marginal).toBe(1); + }); + + it('excludes Not Inspected / Not Present — they are not findings', () => { + const out = summariseFindings([ { - 'i-4': { sectionName: 'Electrical', rating: 'Monitor' }, + [key('sec-roof', 'i-1')]: { rating: 'lv-ni' }, + [key('sec-roof', 'i-2')]: { rating: 'lv-np' }, + [key('sec-roof', 'i-3')]: { rating: 'lv-sat' }, }, - ]); - const roofDef = out.cells.find(c => c.section === 'Roof' && c.category === 'Defect'); - const roofSat = out.cells.find(c => c.section === 'Roof' && c.category === 'Satisfactory'); - const elecMon = out.cells.find(c => c.section === 'Electrical' && c.category === 'Monitor'); - expect(roofDef?.count).toBe(2); - expect(roofSat?.count).toBe(1); - expect(elecMon?.count).toBe(1); + ], CTX); + expect(out.columns.map(c => c.label)).not.toContain('Not Inspected'); + expect(out.columns.map(c => c.label)).not.toContain('Not Present'); + expect(out.rows.find(r => r.section === 'Roof')?.total).toBe(1); + expect(out.unresolved).toBe(0); + }); + + it('resolves legacy envelopes that stored the label instead of the id', () => { + const out = summariseFindings([ + { [key('sec-roof', 'i-1')]: { rating: 'Satisfactory' }, [key('sec-roof', 'i-2')]: { rating: 'D' } }, + ], CTX); + const roof = out.rows.find(r => r.section === 'Roof'); + expect(roof?.counts.satisfactory).toBe(1); + expect(roof?.counts.defect).toBe(1); + expect(out.unresolved).toBe(0); + }); + + it('files a section the templates no longer define under "Unknown"', () => { + const out = summariseFindings([ + { [key('sec-deleted', 'i-1')]: { rating: 'lv-sat' } }, + ], CTX); + expect(out.rows.map(r => r.section)).toEqual(['Unknown']); }); - it('groups items without a sectionName under "Unknown"', () => { - const out = summariseHeatmap([ - { 'i-1': { rating: 'Satisfactory' } }, - ]); - const cell = out.cells.find(c => c.section === 'Unknown'); - expect(cell?.count).toBe(1); + it('counts unmatched ratings as unresolved rather than inventing a column', () => { + const out = summariseFindings([ + { [key('sec-roof', 'i-1')]: { rating: 'lv-from-a-deleted-system' } }, + ], CTX); + expect(out.unresolved).toBe(1); + expect(out.total).toBe(0); + expect(out.rows).toEqual([]); }); - it('ignores items without a rating', () => { - const out = summariseHeatmap([ - { 'i-1': { sectionName: 'Roof' /* no rating */ } }, - ]); - expect(out.cells).toEqual([]); + it('ignores items with no rating', () => { + const out = summariseFindings([ + { [key('sec-roof', 'i-1')]: { rating: undefined } }, + ], CTX); + expect(out.total).toBe(0); + expect(out.unresolved).toBe(0); + }); + + it('orders rows by volume so the busiest section reads first', () => { + const out = summariseFindings([ + { [key('sec-elec', 'a')]: { rating: 'lv-sat' } }, + { + [key('sec-roof', 'b')]: { rating: 'lv-sat' }, + [key('sec-roof', 'c')]: { rating: 'lv-def' }, + }, + ], CTX); + expect(out.rows.map(r => r.section)).toEqual(['Roof', 'Electrical']); }); }); From 22013b3910757d7405a93506266aa4be37c7ea5c Mon Sep 17 00:00:00 2001 From: important-new Date: Wed, 29 Jul 2026 23:06:45 +0800 Subject: [PATCH 02/40] A date range you can read, and a popover that stays with its trigger MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The /metrics window was a three-button `3m · 6m · 12m` group. "3m" is the system's shorthand, not anything a reader says, and those three windows were the only three questions the page could answer — "how did last week go?" was unaskable. It is now a range: seven named presets (7 / 14 / 30 days, 3 / 6 / 12 months, year to date) plus an explicit custom range, with the resolved dates shown beside every one of them. `period` is replaced by `from`/`to` rather than joined by it. Two ways to say the same thing is the drift this audit keeps finding; the enum was three weeks old and its vocabulary is the thing being fixed. Two bugs found by opening the page rather than by reading it: `serviceBreakdown` filtered on tenant alone, ignoring the window entirely. That was invisible while nothing rendered it, and would now read as an all-time card sitting inside a page about a chosen date range. `inspections.date` holds a bare civil date on some rows and a full ISO instant on others. An inclusive upper bound of `2026-07-29` sorts BEFORE `2026-07-29T07:40`, so a naive `lte` drops everything created today — "Last 7 days" would quietly omit today's work. `inclusiveUpperBound` appends a sentinel that sorts after any time-of-day. Popover, shared, first open only: the panel's initial style carried no `position`, so for one layout pass it sat in normal flow — inside the flex row its own trigger lives in, which pushed that trigger sideways by the panel's width. The positioning effect then measured the anchor where it had been pushed to and pinned the panel there: 1209 − 338 − 8 = 863, a menu adrift mid-page with no visible owner. Reopening looked fine, which is what made it confusing. Fixed by being `fixed` from the first render, measuring in a layout effect, and re-measuring on scroll and resize so a panel anchored in a scrolling page header keeps up. The regression test asserts on server-rendered markup, the only view of the panel before an effect has run. Findings by Section now returns one matrix per rating system instead of a union. Systems are not commensurable: `Defect`, `Deficient` and `Deficiency` name one severity band in three vocabularies, and a level's `order` is an index within its own system, so a merged header loses the severity gradient that makes the table readable. The card shows the busiest system, offers a selector when more than one is in use, and states how many findings sit behind the others — a filtered view that does not say what it filtered is how a reader concludes their data has gone missing. Also: only rating systems a template can actually resolve to become columns. The four seeded systems have ten distinct level labels between them, and the union rendered seven columns no template in the tenant could ever fill. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_019iXRoN1WME2DJpssV5dzwA --- app/components/metrics/DateRangePicker.tsx | 174 +++++++++++++++++ .../metrics/FindingsBySection.test.tsx | 107 +++++++++++ app/components/metrics/FindingsBySection.tsx | 116 ++++++++++++ app/lib/metrics-range.test.ts | 124 ++++++++++++ app/lib/metrics-range.ts | 163 ++++++++++++++++ app/routes/metrics.test.tsx | 33 ++-- app/routes/metrics.tsx | 97 +++------- messages/en/metrics.json | 26 ++- packages/shared-ui/src/Popover.test.tsx | 26 +++ packages/shared-ui/src/Popover.tsx | 74 +++++++- server/api/analytics.ts | 15 +- server/api/metrics.ts | 41 ++-- server/lib/analytics.ts | 179 +++++++++++++----- server/lib/metrics-window.ts | 73 +++++++ server/lib/validations/metrics.schema.ts | 15 +- server/services/analytics.service.ts | 41 +++- tests/unit/analytics/analytics.spec.ts | 130 +++++++++++-- .../unit/metrics/metrics-by-inspector.spec.ts | 4 +- .../metrics/metrics-top-agents-people.spec.ts | 4 +- tests/unit/metrics/metrics-window.spec.ts | 66 +++++++ 20 files changed, 1310 insertions(+), 198 deletions(-) create mode 100644 app/components/metrics/DateRangePicker.tsx create mode 100644 app/components/metrics/FindingsBySection.test.tsx create mode 100644 app/components/metrics/FindingsBySection.tsx create mode 100644 app/lib/metrics-range.test.ts create mode 100644 app/lib/metrics-range.ts create mode 100644 server/lib/metrics-window.ts create mode 100644 tests/unit/metrics/metrics-window.spec.ts diff --git a/app/components/metrics/DateRangePicker.tsx b/app/components/metrics/DateRangePicker.tsx new file mode 100644 index 000000000..3a92722bb --- /dev/null +++ b/app/components/metrics/DateRangePicker.tsx @@ -0,0 +1,174 @@ +/** + * — the window control on /metrics. + * + * It replaced a three-button `3m · 6m · 12m` group. That control had two + * problems and no way to grow: "3m" is the system's own shorthand rather than + * anything a reader says, and the three windows it offered were the only three + * questions the page could answer — "how did last week go?" was unaskable. + * + * Shape: one trigger that states the window in words plus the dates it resolves + * to, opening a panel of named presets with a custom range beneath them. The + * presets are the questions people actually ask; the two date fields are the + * escape hatch, not the primary path, so they sit below a rule rather than + * competing with the list. + * + * Built from and + + setOpen(false)} anchorRef={anchorRef}> + {/* Width is set by the longest row — "Last 12 months" beside + "Jul 29, 2025 – Jul 29, 2026", the only preset whose span crosses a + year and so prints both. At w-64 that label wrapped to two lines and + broke the list's rhythm. */} +
+
    + {PRESET_IDS.map((id) => { + const isActive = id === active; + return ( +
  • + +
    • + ); + })} +
+ +
+

+ {m.metrics_range_custom_heading()} +

+
+ + + +
+
+ +
+
+
+ + + ); +} diff --git a/app/components/metrics/FindingsBySection.test.tsx b/app/components/metrics/FindingsBySection.test.tsx new file mode 100644 index 000000000..af2f10659 --- /dev/null +++ b/app/components/metrics/FindingsBySection.test.tsx @@ -0,0 +1,107 @@ +/** + * What a tenant running more than one rating system sees. + * + * The rule this pins: systems are never merged, one is shown at a time, and the + * findings behind the other systems are COUNTED IN THE UI rather than silently + * dropped. A filtered view that does not say what it filtered is how a reader + * concludes their data has gone missing. + */ +import { describe, it, expect } from "vitest"; +import { render, fireEvent } from "@testing-library/react"; + +import { FindingsBySection, type FindingsData } from "./FindingsBySection"; + +const DEFAULT_SYSTEM = { + systemId: "rs-default", + systemName: "OpenInspection Default", + columns: [ + { key: "satisfactory", label: "Satisfactory", color: "#10b981" }, + { key: "defect", label: "Defect", color: "#ef4444" }, + ], + rows: [{ section: "Roof", counts: { satisfactory: 4, defect: 2 }, total: 6 }], + total: 6, +}; + +const TREC_SYSTEM = { + systemId: "rs-trec", + systemName: "TREC (Texas REC)", + columns: [ + { key: "inspected", label: "Inspected", color: "#10b981" }, + { key: "deficient", label: "Deficient", color: "#ef4444" }, + ], + rows: [{ section: "Foundation", counts: { inspected: 3, deficient: 1 }, total: 4 }], + total: 4, +}; + +const MULTI: FindingsData = { systems: [DEFAULT_SYSTEM, TREC_SYSTEM], total: 10 }; +const SINGLE: FindingsData = { systems: [DEFAULT_SYSTEM], total: 6 }; + +describe("FindingsBySection with one rating system", () => { + it("shows no selector — there is nothing to choose between", () => { + const { container, queryByText } = render(); + expect(container.querySelector("select")).toBeNull(); + expect(queryByText(/more findings were rated under a different rating system/i)).toBeNull(); + }); +}); + +describe("FindingsBySection with several rating systems", () => { + it("defaults to the busiest system and never merges the vocabularies", () => { + const { getByText, queryByText } = render(); + // The server orders systems by volume, so [0] is the default view. + expect(getByText("Satisfactory")).toBeTruthy(); + expect(getByText("Roof")).toBeTruthy(); + // TREC's columns and rows must NOT appear alongside the default system's. + expect(queryByText("Deficient")).toBeNull(); + expect(queryByText("Foundation")).toBeNull(); + }); + + it("states how many findings the current view is not showing", () => { + const { getByText } = render(); + // 10 total − 6 in the active system = 4 behind the other system. + expect(getByText(/^4 more findings were rated under a different rating system/)).toBeTruthy(); + }); + + it("switches the whole matrix when another system is chosen", () => { + const { container, getByText, queryByText } = render(); + const select = container.querySelector("select")!; + fireEvent.change(select, { target: { value: "rs-trec" } }); + + expect(getByText("Deficient")).toBeTruthy(); + expect(getByText("Foundation")).toBeTruthy(); + // The previous system's column set is fully gone, not appended. + expect(queryByText("Satisfactory")).toBeNull(); + expect(queryByText("Roof")).toBeNull(); + // …and the hidden count follows the selection: 10 − 4 = 6. + expect(getByText(/^6 more findings were rated under a different rating system/)).toBeTruthy(); + }); + + it("labels each option with its own finding count", () => { + const { container } = render(); + const options = Array.from(container.querySelectorAll("option")).map((o) => o.textContent); + expect(options).toEqual(["OpenInspection Default · 6", "TREC (Texas REC) · 4"]); + }); + + it("falls back to the busiest system when the selected one leaves the window", () => { + // The date-range picker refetches; a system with findings last range may + // have none in this one, and a pinned id would render an empty card. + const { container, rerender, getByText } = render(); + fireEvent.change(container.querySelector("select")!, { target: { value: "rs-trec" } }); + expect(getByText("Foundation")).toBeTruthy(); + + rerender(); + expect(getByText("Roof")).toBeTruthy(); + }); +}); + +describe("FindingsBySection with nothing to show", () => { + it("renders the empty state rather than an empty table", () => { + const { getByText, container } = render(); + expect(getByText(/No rated findings in this date range/i)).toBeTruthy(); + expect(container.querySelector("table")).toBeNull(); + }); + + it("renders the empty state when the findings fetch failed outright", () => { + const { getByText } = render(); + expect(getByText(/No rated findings in this date range/i)).toBeTruthy(); + }); +}); diff --git a/app/components/metrics/FindingsBySection.tsx b/app/components/metrics/FindingsBySection.tsx new file mode 100644 index 000000000..70472e8b9 --- /dev/null +++ b/app/components/metrics/FindingsBySection.tsx @@ -0,0 +1,116 @@ +/** + * — the section × rating-level matrix on /metrics. + * + * **One rating system at a time, never a merged table.** Rating systems are + * per-tenant and not commensurable: `Defect`, `Deficient` and `Deficiency` name + * one severity band in three vocabularies, so a union renders them as three + * sparse columns; and a level's `order` is a per-system index, so a merged + * header loses the left-to-right severity gradient that makes the table + * readable at a glance. A row total spanning two systems counts real findings + * but describes a distribution nobody can compare. + * + * So the server returns one self-contained matrix per system and this component + * shows the busiest by default. When a tenant has only one system in use — the + * common case — there is no selector and nothing to notice. When there is more + * than one, the selector appears AND the count hidden behind it is stated: a + * filtered view that does not say what it filtered is how a reader concludes + * their data is missing. + * + * lint:ds — `ih-*` tokens only. + */ +import { useState } from "react"; +import { Card, Select, Table } from "@core/shared-ui"; +import { m } from "~/paraglide/messages"; + +export interface FindingsSystem { + systemId: string; + systemName: string; + columns: { key: string; label: string; color: string }[]; + rows: { section: string; counts: Record; total: number; unresolvedSection?: true }[]; + total: number; +} + +export interface FindingsData { + /** Ordered by volume server-side, so `[0]` is the default view. */ + systems: FindingsSystem[]; + total: number; +} + +type FindingsRow = FindingsSystem["rows"][number]; + +export function FindingsBySection({ findings }: { findings: FindingsData | null }) { + const systems = findings?.systems ?? []; + const [selectedId, setSelectedId] = useState(null); + // Fall back to the busiest rather than pinning an id in state on mount: the + // range picker refetches, and a system that had findings last range may have + // none in this one. + const active = systems.find((s) => s.systemId === selectedId) ?? systems[0] ?? null; + const hidden = (findings?.total ?? 0) - (active?.total ?? 0); + + return ( + +
+

{m.metrics_findings_title()}

+ {systems.length > 1 && ( + setRecipientId(e.target.value)} + aria-label={m.comm_compose_to_aria()} + className="h-7 px-2 rounded-lg border border-ih-border bg-ih-bg-card text-[12px] text-ih-fg-1 outline-none focus:border-ih-primary" + > + {threadOptions.map((o) => ( + + ))} + +
+ ) : undefined} + /> + )} + + )} + + {/* ── Outbox — the record of what the platform sent ─────────── */} +
+ + {outboxOpen && ( + payload.state === "loading" && !loaded ? ( +

{m.comm_loading()}

+ ) : groups.length > 0 ? ( + + ) : ( +

{outboxEmpty}

+ ) + )} +
+ + ); +} diff --git a/app/components/inspection-hub/OutboxList.tsx b/app/components/inspection-hub/OutboxList.tsx new file mode 100644 index 000000000..4f741bc29 --- /dev/null +++ b/app/components/inspection-hub/OutboxList.tsx @@ -0,0 +1,145 @@ +/** + * — the record of what the platform sent (design §3.3). + * + * One row = one NOTICE, grouped on `(automation_id, send_at)` — a report + * publish to four people over two channels is one row, not eight. The + * grouping itself lives in `groupDeliveries` (app/lib/communication-view.ts). + * + * The row's signature is per-channel delivered/total counts. An icon never + * carries state alone: every count is text beside the icon plus a + * visually-hidden sentence, and colour is never the only signal. Everything + * not delivered is what takes colour; a clean row stays quiet. + * + * The channel cell renders whatever `channel` arrives — no `email|sms` + * switch. That is the zero-cost concession that keeps a future `in_app` + * channel a drop-in instead of a rewrite. + */ +import { useState } from "react"; +import { m } from "~/paraglide/messages"; +import { useDisplayLocale, useDisplayTimeZone } from "~/hooks/useSessionContext"; +import { reasonText, type NoticeGroup, type NoticeChannel } from "~/lib/communication-view"; + +function channelIcon(channel: string) { + // Known channels get a glyph; anything else gets a neutral dot. The LABEL + // always renders as text, so an unknown channel is ugly, never invisible. + if (channel === "email") { + return ( + + ); + } + if (channel === "sms") { + return ( + + ); + } + return