diff --git a/module/AUDIT_METRICS.md b/module/AUDIT_METRICS.md index e1249bc64..02fdd4225 100644 --- a/module/AUDIT_METRICS.md +++ b/module/AUDIT_METRICS.md @@ -195,7 +195,8 @@ never the SDK binding input. Regenerate the export before generating the SDK. ## Unknown and partial results -Empty queries have no points and render **No data yet**. Numeric sums with no +Empty queries have no points and render as no data, in the form the widget's +visualization has for it (see *How a resolved series is drawn*). Numeric sums with no numeric observations omit their value, just like averages/percentiles; an emitted zero stays zero. `samples[alias]` reports observations before reduction (so repeated logical IDs are not mistaken for missing telemetry). Fewer samples than @@ -213,6 +214,60 @@ The host DSL also withholds totals after top-N truncation. Authorization/RPC errors remain errors, never empty successful results. Existing consumers of the SDK must handle `total: null` when adopting this contract. +## How a resolved series is drawn + +Every dashboard built from these series — the host's own pages, a solution's +declared dashboard, and a solution remote that mounts the kit itself — is drawn +by one renderer, `` from `@codefly-dev/ui/dashboard`, with the kit's +**metric tier**. There is no second, plainer rendering: a consuming solution +does not have to ask which half of the kit its dashboard will get. + +| Widget | Drawn as | Empty series | `coverage: "partial"` | `total: null` | +| --- | --- | --- | --- | --- | +| `number` | `StatTile`: the series total, in the widget's `format` and `unit`; a sparkline and a delta against the previous bucket when the metric is bucketed over **time** | `—` with a **No data** badge, never `0` | `—` with a **Partial data** badge | `—` with a **No total** badge when coverage is complete | +| `line` / `area` / `bar` | the kit's metric chart over a `ChartSeries` — axes from the appearance palette, a crosshair readout, a legend for two or more series, and every exact value in a visually-hidden data table | the chart's own **No data** plot, plus a **No data** badge on the card | the observed points still plot, with a **Partial data** badge on the card | nothing: a series widget shows no total | +| `table` | key/value rows | **No data yet.** | a **Partial data** badge on the card | — | + +A scalar widget's `format` names how its value is written, and `percent` takes a +value that is **already a percentage**: 45 renders "45%". It is not a share — +0.4 renders "0.4%". A producer whose own `percent` means a 0-to-1 share scales it +before it reaches the view, because the field is bound structurally and nothing +on the wire distinguishes the two conventions. + +Two of those cells are the contract, not presentation. A `null` total renders as +a dash and never as `0`, because "cannot say" and "nothing happened" are +different answers and only one of them is in the data; and a series with no +points is no data rather than a zero, because the aggregate omits a bucket +instead of emitting an empty one. A sparkline and a bucket delta are drawn +**only** for a time-bucketed series: a line through counts grouped by event type +or region would assert an order, and a delta between two of them a change over +time, that a categorical grouping does not have. Both read the series in **time** +order, taken from the parsed bucket keys rather than from the order the +aggregate returned them in, so a series delivered newest-first reports the same +change — a delta's sign is drawn as a colour, and getting it from an array +position would paint a decline as a green rise. + +A widget whose resolution **failed** says so only when it has nothing else to +say. With a series still in hand — a refresh that did not land — the tile draws +that series and carries a **Stale** badge instead, because values that are old +are still better than a blank card. Only a failure with nothing behind it reads +as "Unable to load." + +A widget whose resolution was **refused** is the other case, and it keeps +nothing. "No access" is drawn before the series is read, and the host that +resolved the widget deletes the values it had cached — every revision of that +widget's declaration, at the read that was refused rather than in the component +displaying it, so a refusal arriving after the viewer moved on still lands. +Numbers this viewer is no longer allowed to read are a wrong answer, not a stale +one, and only a read that succeeds puts any back. + +A host composes its own features around that renderer — ``'s slots +take header actions, a layout (the host's solution dashboard passes a +drag-to-reorder grid), a per-widget renderer for a host that resolves each +widget in its own query, and an empty state. A consumer re-implementing any part +of the drawing is refused by a gate: `src/components/__tests__/kit.test.ts` in +the frontend. + ## Independent validation Run `go test -race ./pkg/infra/auditmetricstest` from accounts/code for pure diff --git a/module/services/frontend/AGENTS.md b/module/services/frontend/AGENTS.md index 00759fb49..0a257e52b 100644 --- a/module/services/frontend/AGENTS.md +++ b/module/services/frontend/AGENTS.md @@ -115,6 +115,40 @@ whether `/s/{id}` renders, and it stays installation-blind. observation speaks for the present is the host's to decide, and a copy of that window here would drift from it. +## Drawing a solution's declared dashboard + +`src/solutions/SolutionDashboard.tsx` **mounts** the shared kit's +`` (`@codefly-dev/ui/dashboard`); it draws nothing itself. Which +tier each widget and each coverage state is drawn with is the module's +[AUDIT_METRICS.md](../../AUDIT_METRICS.md) ("How a resolved series is drawn") — +one renderer for the host's pages, a declared solution dashboard and a solution +remote alike. + +What this file owns is only what is the host's, composed through that +renderer's slots: + +- **the audit queries**, one per widget, so a widget whose metric errors fails + alone instead of blanking its siblings. Each lives in its own component + (`SolutionWidget`) that resolves the series and then mounts the kit's + `DashboardWidget`, because React fixes the number of hooks a component may + call and a renderer taking the whole view cannot hold a varying number of + queries; +- **the viewer's arrangement** — reorder by drag or arrow keys, remove, add + back, saved per viewer and organization (`dashboard-layout.ts`), handed to + the renderer's `layout` slot as the kit's `SortableGrid`; +- **the ⓘ** that says what a tile counts, from which audit events, over what + range and who caused the newest ones (`metric-info.ts`, the principal + directory, the recent-events drill-down), handed to each widget as its + header actions. + +Re-implementing any part of the drawing is refused by +`src/components/__tests__/kit.test.ts`: a declaration taking a name +`@codefly-dev/ui` exports must render the kit's component of that name or carry +a reviewed entry, `class-variance-authority` is not a dependency of this app so +a kit-shaped primitive cannot be built here, and the two chart surfaces +(`src/components/charts/chart.tsx`, +`src/features/dashboard/ui/charts/line-chart.tsx`) stay re-exports. + ## Loading a remote, and its CSP `/s/[solutionId]` (`src/app/(dashboard)/s/[solutionId]/page.tsx`) loads the @@ -218,3 +252,18 @@ failures part company here, which is what `mayKeepRetainedRows` and those very rows, so they go; a **transient failure** is not, so they stay with a note that they may be out of date. Blanking a good table because one request did not come back is worse than the staleness. + +**Where a refusal acts is the read, not the component.** Withholding rows at +render answers for the error in hand and leaves the rows in the cache, so the +next transient failure serves them again — authority is a property of the read, +and so is what the read may leave behind. A refused read therefore deletes the +retained snapshots for every cached key of what it was reading, in the query +function where `readOutcome` classifies it, before the rejection reaches any +component. `src/solutions/SolutionDashboard.tsx` is the worked example: one +widget, several cached revisions of its declaration, and a refusal that may +arrive after the viewer has navigated away or the page has unmounted, where no +effect of theirs is left to run. Delete the data and nothing else: removing the +entry cancels the read reporting the refusal and strands recovery, clearing it +restarts every active read of it so a standing 403 never settles, and +`setQueryData(key, undefined)` is a documented no-op. Recovery is then exactly +one thing — a read that succeeds. diff --git a/module/services/frontend/code/package-lock.json b/module/services/frontend/code/package-lock.json index 76aa8ac38..1cc6a8c4d 100644 --- a/module/services/frontend/code/package-lock.json +++ b/module/services/frontend/code/package-lock.json @@ -16,8 +16,8 @@ "@codefly-dev/saas-plugin-contract": "2.4.2", "@codefly-dev/saas-plugin-react": "0.4.3", "@codefly-dev/saas-sdk": "0.3.23", - "@codefly-dev/saas-ui": "0.12.6", - "@codefly-dev/ui": "0.12.6", + "@codefly-dev/saas-ui": "0.12.7", + "@codefly-dev/ui": "0.12.7", "@codefly/saas-plugin-manifest": "0.1.1", "@codefly/saas-settings": "0.1.0", "@connectrpc/connect": "^2.1.1", @@ -34,7 +34,6 @@ "@simplewebauthn/browser": "^13.3.0", "@tanstack/react-query": "^5.60.0", "@tanstack/react-table": "^8.21.3", - "class-variance-authority": "^0.7.1", "clsx": "^2.1.1", "codefly": "https://github.com/codefly-dev/sdk-js/archive/9f3bb78682a0c2b9a70631d35c2ae0094c281ac5.tar.gz", "js-yaml": "^4.3.2", @@ -15234,7 +15233,7 @@ }, "packages/codefly-ui": { "name": "@codefly-dev/ui", - "version": "0.12.6", + "version": "0.12.7", "license": "Apache-2.0", "dependencies": { "@base-ui/react": "^1.3.0", @@ -15382,7 +15381,7 @@ }, "packages/saas-ui": { "name": "@codefly-dev/saas-ui", - "version": "0.12.6", + "version": "0.12.7", "license": "Apache-2.0", "devDependencies": { "@bufbuild/protobuf": "^2.11.0", diff --git a/module/services/frontend/code/package.json b/module/services/frontend/code/package.json index cfae0c638..ca15dbe96 100644 --- a/module/services/frontend/code/package.json +++ b/module/services/frontend/code/package.json @@ -39,8 +39,8 @@ "@codefly-dev/saas-plugin-contract": "2.4.2", "@codefly-dev/saas-plugin-react": "0.4.3", "@codefly-dev/saas-sdk": "0.3.23", - "@codefly-dev/saas-ui": "0.12.6", - "@codefly-dev/ui": "0.12.6", + "@codefly-dev/saas-ui": "0.12.7", + "@codefly-dev/ui": "0.12.7", "@codefly/saas-plugin-manifest": "0.1.1", "@codefly/saas-settings": "0.1.0", "@connectrpc/connect": "^2.1.1", @@ -57,7 +57,6 @@ "@simplewebauthn/browser": "^13.3.0", "@tanstack/react-query": "^5.60.0", "@tanstack/react-table": "^8.21.3", - "class-variance-authority": "^0.7.1", "clsx": "^2.1.1", "codefly": "https://github.com/codefly-dev/sdk-js/archive/9f3bb78682a0c2b9a70631d35c2ae0094c281ac5.tar.gz", "js-yaml": "^4.3.2", diff --git a/module/services/frontend/code/packages/codefly-ui/README.md b/module/services/frontend/code/packages/codefly-ui/README.md index 070eee474..95543fdca 100644 --- a/module/services/frontend/code/packages/codefly-ui/README.md +++ b/module/services/frontend/code/packages/codefly-ui/README.md @@ -36,6 +36,15 @@ and the v1 subset — is [CATALOG.md](./CATALOG.md). (`Banner`, `EmptyState`, `ErrorState`) and overlays (`Dialog`, `AlertDialog`, `Notice`, `Tooltip`, `DropdownMenu`); `dashboard` is ``, charts, `fromDashboardData`; `chat` is ``. React only: no plugin runtime, no host context. + `` paints with the **metric tier** — `StatTile`/`KPIRow` for scalar + widgets, `metric-chart`'s line/area/bar over `ChartSeries` for series ones — + which is the tier the host's own operations pages use, so a solution's + dashboard and the host's own are the same picture. `DashboardWidget` is that + renderer for one widget, exported for a consumer that resolves each widget in + its own query; `slots` take header actions, a layout, a per-widget renderer + and an empty state, so a consumer composes its own features around the + renderer rather than beside it. What each widget and each coverage state + draws is tabulated in the module's `AUDIT_METRICS.md`. This is the surface a solution fe-remote consumes. `` is fed by `@codefly-dev/saas-sdk`'s `useChatStream` — the hook owns the SSE/WS transport, the component stays pure, the same split as `runDashboard` → ``. @@ -106,7 +115,7 @@ solution measured where it had to reach past the kit. | `@codefly-dev/ui/plugin-host/ui` | Client UI adapters (`PluginErrorBoundary`) | | `@codefly-dev/ui/skin` | `resolveSkin`, skin types | | `@codefly-dev/ui/layout` | `Card`/`Section`/`Tabs` + shadcn primitives (React-only) | -| `@codefly-dev/ui/dashboard` | `Dashboard`, charts, `fromDashboardData` (React-only) | +| `@codefly-dev/ui/dashboard` | `Dashboard`, `DashboardWidget`, metric tiles + charts, `fromDashboardData` (React-only) | | `@codefly-dev/ui/chat` | `Chat` (React-only) | | `@codefly-dev/ui/content` | `Content`, `Markdown`, `JsonView`, `CodeBlock`, `TextBlock` (React-only) | | `@codefly-dev/ui/board` | `Board` (React-only) | diff --git a/module/services/frontend/code/packages/codefly-ui/package.json b/module/services/frontend/code/packages/codefly-ui/package.json index 18f4fc37c..056f89478 100644 --- a/module/services/frontend/code/packages/codefly-ui/package.json +++ b/module/services/frontend/code/packages/codefly-ui/package.json @@ -1,6 +1,6 @@ { "name": "@codefly-dev/ui", - "version": "0.12.6", + "version": "0.12.7", "description": "The shared, versioned Codefly SaaS frontend kit: the plugin host and the tokens-as-data skin mechanism, standalone so the host and Module-Federation remotes import and dedupe one copy.", "repository": { "type": "git", diff --git a/module/services/frontend/code/packages/codefly-ui/src/dashboard/__tests__/atoms.test.tsx b/module/services/frontend/code/packages/codefly-ui/src/dashboard/__tests__/atoms.test.tsx index fb6d0aeb2..36d76d9f8 100644 --- a/module/services/frontend/code/packages/codefly-ui/src/dashboard/__tests__/atoms.test.tsx +++ b/module/services/frontend/code/packages/codefly-ui/src/dashboard/__tests__/atoms.test.tsx @@ -2,18 +2,30 @@ import { describe, expect, it } from "vitest"; import { Axis, Gridline } from "../atoms.js"; import { linearScale, sparkPlot } from "../geometry.js"; -type ElementLike = { type?: unknown; props?: { children?: unknown } & Record }; +type ElementLike = { + type?: unknown; + props?: { children?: unknown } & Record; +}; // Collect every element of a given SVG tag (a rendered component is a plain // object tree, so no DOM is needed) with its props and text child. Recurses into // child arrays — an axis holds one array of y labels and one of x labels. -function byTag(node: unknown, tag: string): { props: Record; text: string }[] { +function byTag( + node: unknown, + tag: string, +): { props: Record; text: string }[] { if (Array.isArray(node)) return node.flatMap((child) => byTag(child, tag)); if (!node || typeof node !== "object") return []; const el = node as ElementLike; const self = el.type === tag && el.props - ? [{ props: el.props, text: typeof el.props.children === "string" ? el.props.children : "" }] + ? [ + { + props: el.props, + text: + typeof el.props.children === "string" ? el.props.children : "", + }, + ] : []; return self.concat(byTag(el.props?.children, tag)); } @@ -34,13 +46,18 @@ describe("Gridline", () => { describe("Axis", () => { it("labels each y tick with the formatted value", () => { const scale = linearScale(0, 10, plot.bottom, plot.top); - const labels = byTag(Axis({ plot, y: { ticks: [0, 5, 10], scale } }), "text").map((t) => t.text); + const labels = byTag( + Axis({ plot, y: { ticks: [0, 5, 10], scale } }), + "text", + ).map((t) => t.text); expect(labels).toEqual(["0", "5", "10"]); }); it("time-formats ISO bucket keys on the x axis", () => { const keys = ["2026-09-01T00:00:00+00", "2026-09-02T00:00:00+00"]; - const labels = byTag(Axis({ plot, x: { keys } }), "text").map((t) => t.text); + const labels = byTag(Axis({ plot, x: { keys } }), "text").map( + (t) => t.text, + ); expect(labels).toEqual(["Sep 1", "Sep 2"]); }); diff --git a/module/services/frontend/code/packages/codefly-ui/src/dashboard/__tests__/charts.test.tsx b/module/services/frontend/code/packages/codefly-ui/src/dashboard/__tests__/charts.test.tsx index 600a9200d..42a18a171 100644 --- a/module/services/frontend/code/packages/codefly-ui/src/dashboard/__tests__/charts.test.tsx +++ b/module/services/frontend/code/packages/codefly-ui/src/dashboard/__tests__/charts.test.tsx @@ -3,7 +3,10 @@ import { Axis, Svg } from "../atoms.js"; import { AreaChart, LineChart, StatChart } from "../charts.js"; import type { SeriesPoint } from "../types.js"; -type ElementLike = { type?: unknown; props?: { children?: unknown } & Record }; +type ElementLike = { + type?: unknown; + props?: { children?: unknown } & Record; +}; // Walk a React element tree (no DOM needed — a rendered component is a plain // object) collecting every 's props, so a test can assert what color the @@ -36,7 +39,9 @@ const pts: SeriesPoint[] = [ // a text-color class (and the Dashboard accent behind `text-primary`) theme it. describe("chart color inherits via currentColor", () => { it("LineChart strokes with currentColor", () => { - const strokes = pathProps(LineChart({ points: pts })).filter((p) => p.stroke && p.stroke !== "none"); + const strokes = pathProps(LineChart({ points: pts })).filter( + (p) => p.stroke && p.stroke !== "none", + ); expect(strokes.length).toBeGreaterThan(0); for (const p of strokes) expect(p.stroke).toBe("currentColor"); }); @@ -69,7 +74,9 @@ describe("axes gating", () => { expect(tree.props.stretch).toBe(false); expect(byType(tree, Axis)).toHaveLength(1); // The data path still colors via currentColor, not a hardcoded hue. - const strokes = pathProps(tree).filter((p) => p.stroke && p.stroke !== "none"); + const strokes = pathProps(tree).filter( + (p) => p.stroke && p.stroke !== "none", + ); expect(strokes.length).toBeGreaterThan(0); for (const p of strokes) expect(p.stroke).toBe("currentColor"); }); diff --git a/module/services/frontend/code/packages/codefly-ui/src/dashboard/__tests__/dashboard.test.tsx b/module/services/frontend/code/packages/codefly-ui/src/dashboard/__tests__/dashboard.test.tsx index cd9ffb4a2..981a3be7b 100644 --- a/module/services/frontend/code/packages/codefly-ui/src/dashboard/__tests__/dashboard.test.tsx +++ b/module/services/frontend/code/packages/codefly-ui/src/dashboard/__tests__/dashboard.test.tsx @@ -1,77 +1,719 @@ // @vitest-environment happy-dom -import { cleanup, render, screen } from "@testing-library/react"; +import { cleanup, render, screen, within } from "@testing-library/react"; import { afterEach, describe, expect, it } from "vitest"; import { Dashboard } from "../dashboard.js"; -import type { DashboardView } from "../types.js"; +import type { + DashboardView, + DashboardWidgetView, + WidgetSeries, +} from "../types.js"; afterEach(cleanup); -const emptySeries = { points: [], total: 0 }; +// Two daily buckets: a time-bucketed series, so it earns a trend. +const daily: WidgetSeries = { + points: [ + { key: "2026-09-01T00:00:00Z", value: 3 }, + { key: "2026-09-02T00:00:00Z", value: 5 }, + ], + total: 8, +}; + +// The same counts grouped by a category: an ordered line through these would +// assert a direction the data does not have. +const byCategory: WidgetSeries = { + points: [ + { key: "identity", value: 3 }, + { key: "security", value: 5 }, + ], + total: 8, +}; + +const empty: WidgetSeries = { points: [], total: 0, coverage: "empty" }; + +function view(...widgets: DashboardWidgetView[]): DashboardView { + return { widgets }; +} + +function show(...widgets: DashboardWidgetView[]) { + return render(); +} describe("Dashboard", () => { - it("renders its header through Section and each widget through Card", () => { - const data: DashboardView = { - title: "Traffic", - description: "last 7 days", - widgets: [ - { - id: "w1", - title: "Visits", - visualization: "number", - series: emptySeries, - }, - ], - }; - const { container } = render(); + it("renders its header through Section and each series widget through Card", () => { + const { container } = render( + , + ); - // Section owns the header markup — its
root and heading classes. - const section = container.querySelector("section"); - expect(section).not.toBeNull(); + // Section owns the header markup — its
root and heading. + expect(container.querySelector("section")).not.toBeNull(); expect( screen.getByRole("heading", { name: "Traffic", level: 2 }), ).toBeTruthy(); expect(screen.getByText("last 7 days")).toBeTruthy(); - // Card owns the surface — one card per widget, painted by the primitive. - const card = container.querySelector( - ".rounded-lg.border.bg-card.p-4.text-card-foreground.shadow-sm", - ); - expect(card).not.toBeNull(); + // Card owns the surface — one card per series widget, painted by the + // primitive, with the widget's title as its heading. + expect( + container.querySelector( + ".rounded-lg.border.bg-card.p-4.text-card-foreground.shadow-sm", + ), + ).not.toBeNull(); expect( screen.getByRole("heading", { name: "Visits", level: 3 }), ).toBeTruthy(); }); + it("reaches the token the metric charts actually paint with its accent", () => { + // The tier change moved where a mark's colour comes from: a metric + // chart colours its first series `var(--chart-1)`, not `--primary`, + // so an accent that set only `--primary` stopped colouring the charts + // it exists for. Asserted on the rendered mark, not just the variable. + const { container } = render( + , + ); + expect( + container.querySelector('path[stroke="var(--chart-1)"]'), + ).not.toBeNull(); + const section = container.querySelector("section") as HTMLElement; + expect(section.style.getPropertyValue("--chart-1")).toBe("hotpink"); + expect(section.style.getPropertyValue("--primary")).toBe("hotpink"); + }); + + it("reaches a scalar's own trend with the accent, not just the charts", () => { + // A declared accent that coloured the line charts but not the + // sparklines beside them drew one dashboard in two palettes. + const { container } = render( + , + ); + // The delta draws a lucide arrow, so the sparkline is not the first svg. + const [trend] = [...container.querySelectorAll("svg")].filter( + (svg) => !svg.classList.contains("lucide"), + ); + expect((trend as SVGElement).style.color).toBe("var(--chart-1)"); + expect( + ( + container.querySelector("section") as HTMLElement + ).style.getPropertyValue("--chart-1"), + ).toBe("hotpink"); + }); + + it("keeps a titled scalar reachable by heading navigation", () => { + // It was a Card with an

; a StatTile's label is a span, so the + // title left the document outline when the tier changed. + render( + , + ); + expect( + screen.getByRole("heading", { name: "Total events", level: 3 }), + ).toBeTruthy(); + }); + it("scopes the accent override onto the Section root", () => { - const data: DashboardView = { - accent: "hotpink", - widgets: [{ id: "w1", visualization: "number", series: emptySeries }], - }; - const { container } = render(); + const { container } = render( + , + ); const section = container.querySelector("section") as HTMLElement; expect(section.style.getPropertyValue("--primary")).toBe("hotpink"); }); + + it("says so, rather than drawing an empty grid, when the view has no widgets", () => { + show(); + expect(screen.getByText("No widgets.")).toBeTruthy(); + }); }); -it("preserves partial telemetry and unavailable totals from the SDK view", () => { - render( - { + it("draws a metric chart with an accessible data table", () => { + const { container } = render( + , + ); + const table = screen.getByRole("table", { name: "Events" }); + expect(within(table).getByRole("row", { name: /identity/ })).toBeTruthy(); + expect(within(table).getByText("3")).toBeTruthy(); + expect(within(table).getByText("5")).toBeTruthy(); + expect(container.querySelector("svg")?.getAttribute("aria-hidden")).toBe( + "true", + ); + }); + + it("draws the chart's own empty state instead of an axis with nothing on it", () => { + const { container } = render( + , + ); + expect( + container.querySelector("figure")?.getAttribute("aria-label"), + ).toBe("Events: no data"); + expect(container.querySelector("svg")).toBeNull(); + // Said twice on purpose, and in two registers: the card badges the + // coverage beside its title, and the plot says it where the geometry + // would have been. + expect(screen.getAllByText("No data")).toHaveLength(2); + }); + + it("badges a partial series on the card, from `series.coverage`", () => { + render( + , + ); + expect(screen.getByText("Partial data")).toBeTruthy(); + // A partial series keeps its observed points: the chart still draws. + expect(screen.getByRole("table", { name: "Events" })).toBeTruthy(); + }); + }, +); + +describe("a number widget", () => { + it("is a stat tile showing the series total in the widget's format", () => { + show({ + id: "w", + title: "Revenue", + visualization: "number", + series: { points: [{ key: "all", value: 4_200_000 }], total: 4_200_000 }, + format: "currency", + }); + expect(screen.getByText("Revenue")).toBeTruthy(); + expect(screen.getByText("$4.2M")).toBeTruthy(); + }); + + it("appends the widget's unit", () => { + show({ + id: "w", + title: "Throughput", + visualization: "number", + series: { points: [{ key: "all", value: 42 }], total: 42 }, + unit: "req/s", + }); + expect(screen.getByText("req/s")).toBeTruthy(); + }); + + it("sparklines a time-bucketed metric and deltas its last bucket", () => { + const { container } = show({ + id: "w", + title: "Logins", + visualization: "number", + series: daily, + }); + expect(screen.getByText("8")).toBeTruthy(); + expect(container.querySelector("svg")).not.toBeNull(); + // 3 → 5 between the last two buckets. + expect(screen.getByText(/66\.7%/).textContent).toContain( + "vs previous bucket", + ); + }); + + it("draws neither a trend nor a delta for a metric grouped by category", () => { + // A sparkline over "identity, security" would read as a direction, and a + // delta between them as a change over time. Neither is true of a + // categorical grouping, whose order is the aggregate's, not time's. + const { container } = show({ + id: "w", + title: "Logins", + visualization: "number", + series: byCategory, + }); + expect(screen.getByText("8")).toBeTruthy(); + expect(container.querySelector("svg")).toBeNull(); + expect(screen.queryByText(/%/)).toBeNull(); + }); + + it("deltas the two latest buckets whatever order the series arrives in", () => { + // The resolver appends one point per bucket in the order the aggregate + // returned them, with no sort, and a delta's sign is drawn as a colour + // — so reading "previous" and "latest" off the last two array positions + // would paint a decline green for a series delivered newest-first. + const forward = [ + { key: "2026-09-01T00:00:00Z", value: 10 }, + { key: "2026-09-02T00:00:00Z", value: 5 }, + ]; + const reading = (points: typeof forward) => { + const { unmount } = show({ + id: "w", + title: "Logins", + visualization: "number", + series: { points, total: 15 }, + }); + const text = screen.getByText(/%/).textContent; + unmount(); + return text; + }; + const ordered = reading(forward); + expect(ordered).toContain("50%"); + // A halving, drawn as a fall, not a doubling drawn as a rise. + expect(ordered).toContain("−"); + expect(reading([...forward].reverse())).toBe(ordered); + }); + + it("orders the sparkline by time too, so the trend and the delta agree", () => { + const { container } = show({ + id: "w", + visualization: "number", + series: { + points: [ + { key: "2026-09-03T00:00:00Z", value: 9 }, + { key: "2026-09-01T00:00:00Z", value: 1 }, + { key: "2026-09-02T00:00:00Z", value: 5 }, ], - }} - />, - ); - expect(screen.getByRole("status").textContent).toBe("Partial telemetry"); - expect(screen.getByText("Total unavailable")).toBeTruthy(); - expect(screen.queryByText("0")).toBeNull(); + total: 15, + }, + }); + // The sparkline's path rises monotonically: 1, 5, 9 in time order, not + // 9, 1, 5 in array order. + const paths = [...container.querySelectorAll("svg path")]; + const d = paths[paths.length - 1]?.getAttribute("d") ?? ""; + const ys = [...d.matchAll(/[ML][\d.]+,([\d.]+)/g)].map((m) => Number(m[1])); + expect(ys).toHaveLength(3); + // Smaller y is higher on screen, so a rising series falls in y. + expect(ys[0]).toBeGreaterThan(ys[1]); + expect(ys[1]).toBeGreaterThan(ys[2]); + }); + + it("reads `format: percent` as an already-scaled percentage, not a share", () => { + // The field is bound structurally, so nothing on the wire distinguishes + // a producer whose own `percent` means a 0-to-1 share. This pins the + // kit's convention: a producer with the other one must scale first, and + // finds out here rather than by shipping a hundredfold error. + show({ + id: "w", + title: "MFA adoption", + visualization: "number", + series: { points: [{ key: "all", value: 45 }], total: 45 }, + format: "percent", + }); + expect(screen.getByText("45%")).toBeTruthy(); + expect(screen.queryByText("4,500%")).toBeNull(); + }); + + it("gives no delta for a rise from zero, which is not a percentage", () => { + show({ + id: "w", + visualization: "number", + series: { + points: [ + { key: "2026-09-01T00:00:00Z", value: 0 }, + { key: "2026-09-02T00:00:00Z", value: 5 }, + ], + total: 5, + }, + }); + expect(screen.queryByText(/%/)).toBeNull(); + }); + + it("shows a dash and says no total, never a 0, when the total is withheld", () => { + // `total: null` means the pipeline cannot name one number — a distinct + // count or an average across groups. Rendering it as 0 would report + // "nothing happened" where the answer is "cannot say". + show({ + id: "w", + title: "Distinct actors", + visualization: "number", + series: { ...byCategory, total: null }, + }); + expect(screen.getByText("—")).toBeTruthy(); + expect(screen.getByText("No total")).toBeTruthy(); + expect(screen.queryByText("0")).toBeNull(); + }); + + it("badges a partial series, which is why its total is withheld", () => { + show({ + id: "w", + title: "Logins", + visualization: "number", + series: { ...byCategory, total: null, coverage: "partial" }, + }); + expect(screen.getByText("Partial data")).toBeTruthy(); + expect(screen.getByText("—")).toBeTruthy(); + expect(screen.queryByText("No total")).toBeNull(); + }); + + it("reads an empty series as no data, not as a zero", () => { + // The audit aggregate omits a bucket rather than emitting a zero one, so + // a series with no points means nothing matched. + show({ id: "w", title: "Logins", visualization: "number", series: empty }); + expect(screen.getByText("No data")).toBeTruthy(); + expect(screen.getByText("—")).toBeTruthy(); + expect(screen.queryByText("0")).toBeNull(); + }); + + it("keeps a real zero a zero", () => { + show({ + id: "w", + title: "Failures", + visualization: "number", + series: { points: [{ key: "all", value: 0 }], total: 0 }, + }); + expect(screen.getByText("0")).toBeTruthy(); + expect(screen.queryByText("—")).toBeNull(); + }); +}); + +describe("a table widget", () => { + it("lists each point's key and value", () => { + show({ + id: "w", + title: "By type", + visualization: "table", + series: byCategory, + }); + const table = screen.getByRole("table", { name: "By type" }); + expect(within(table).getByText("identity")).toBeTruthy(); + expect(within(table).getByText("5")).toBeTruthy(); + }); + + it("says there is nothing yet for an empty series", () => { + show({ id: "w", title: "By type", visualization: "table", series: empty }); + expect(screen.getByText("No data yet.")).toBeTruthy(); + }); +}); + +describe("a widget still being resolved, or that failed", () => { + it("waits on a null series rather than drawing it as empty", () => { + const { container } = show( + { id: "chart", title: "Logins", visualization: "line", series: null }, + { id: "tile", title: "Total", visualization: "number", series: null }, + ); + expect(container.querySelectorAll('[data-slot="skeleton"]')).toHaveLength( + 2, + ); + expect(screen.queryByText("No data")).toBeNull(); + }); + + it("keeps a series a failed refresh left behind, and marks it stale", () => { + // A refresh that did not land is not an absence of data. Blanking the + // tile would throw away values that are still the best available. + const { container } = show( + { + id: "chart", + title: "Logins", + visualization: "line", + series: byCategory, + failure: "failed", + }, + { + id: "tile", + title: "Total", + visualization: "number", + series: daily, + failure: "failed", + }, + ); + expect(screen.queryByText("Unable to load.")).toBeNull(); + expect(screen.getAllByText("Stale")).toHaveLength(2); + expect(screen.getByRole("table", { name: "Logins" })).toBeTruthy(); + expect(screen.getByText("8")).toBeTruthy(); + expect(container.querySelector('[data-slot="skeleton"]')).toBeNull(); + }); + + it("keeps an empty series valueless through a failed refresh", () => { + // The badge is one slot and freshness takes it, so deriving the value's + // presence from the state turned a known-empty scalar into a reported + // zero the moment its refresh failed. + const widget: DashboardWidgetView = { + id: "total", + visualization: "number", + series: empty, + }; + const { rerender } = render(); + expect(screen.getByText("—")).toBeTruthy(); + expect(screen.getByText("No data")).toBeTruthy(); + + rerender(); + expect(screen.getByText("Stale")).toBeTruthy(); + expect(screen.getByText("—")).toBeTruthy(); + expect(screen.queryByText("0")).toBeNull(); + }); + + it("keeps a partial series identifiable through a failed refresh", () => { + show({ + id: "w", + title: "Logins", + visualization: "number", + series: { ...byCategory, total: null, coverage: "partial" }, + failure: "failed", + }); + // Freshness takes the badge, but the withheld total still reads as one. + expect(screen.getByText("Stale")).toBeTruthy(); + expect(screen.getByText("—")).toBeTruthy(); + expect(screen.queryByText("8")).toBeNull(); + }); + + it("discards a series the caller retained on a refused read", () => { + // A number this viewer may not see is a WRONG answer, not an old one. + // The host is expected to withhold it; the renderer that owns what a + // refusal looks like fails closed too, so a caller that forwards + // retained values by mistake cannot show them through this. + const { container } = show( + { + id: "chart", + title: "Logins", + visualization: "line", + series: byCategory, + failure: "refused", + }, + { + id: "tile", + title: "Total", + visualization: "number", + series: daily, + failure: "refused", + }, + ); + expect(screen.queryByText("8")).toBeNull(); + expect(container.querySelector("svg")).toBeNull(); + expect(screen.queryByRole("table", { name: "Logins" })).toBeNull(); + expect(screen.getAllByText("No access")).toHaveLength(2); + expect( + screen.getByText("You don't have permission to see this."), + ).toBeTruthy(); + expect(screen.queryByText("Unable to load.")).toBeNull(); + }); + + it("says a failed widget failed, and does not wait forever on its missing series", () => { + // A failed resolution leaves no series behind, so a renderer that tested + // for the series first would draw every failure as an endless wait. + const { container } = show( + { + id: "chart", + title: "Logins", + visualization: "line", + series: null, + failure: "failed", + }, + { + id: "tile", + title: "Total", + visualization: "number", + series: null, + failure: "failed", + }, + ); + expect(screen.getByText("Unable to load.")).toBeTruthy(); + expect(screen.getByText("Provider unavailable")).toBeTruthy(); + expect(container.querySelector('[data-slot="skeleton"]')).toBeNull(); + }); +}); + +describe("the default layout", () => { + it("gathers adjacent scalars into one KPI row and keeps declared order", () => { + const { container } = show( + { id: "a", title: "A", visualization: "number", series: daily }, + { id: "b", title: "B", visualization: "number", series: daily }, + { id: "c", title: "C", visualization: "line", series: daily }, + { id: "d", title: "D", visualization: "number", series: daily }, + ); + // Two tiles, then a chart, then a tile: three runs, the KPI row's own + // four-up geometry only around the adjacent pair. + const rows = [ + ...container.querySelectorAll('section > div > div[class*="grid"]'), + ]; + expect(rows).toHaveLength(3); + expect(rows[0].className).toContain("lg:grid-cols-4"); + expect(rows[0].children).toHaveLength(2); + expect(rows[1].className).toContain("sm:grid-cols-2"); + expect(rows[2].children).toHaveLength(1); + expect( + [...container.querySelectorAll('[data-slot="metric-label"], h3')].map( + (node) => node.textContent, + ), + ).toEqual(["A", "B", "C", "D"]); + }); + + it("keeps a scalar that declares a span in the grid, with its span", () => { + // A span is a request for grid width, which the KPI row's own geometry + // cannot grant, so a scalar that declares one stays a grid item rather + // than having the field silently dropped. + const { container } = render( + , + ); + const rows = [ + ...container.querySelectorAll('section > div > div[class*="grid"]'), + ]; + expect(rows).toHaveLength(2); + // The spanning scalar is a card in the grid... + expect(rows[0].className).toContain("sm:grid-cols-2"); + expect(rows[0].firstElementChild?.className).toContain("sm:col-span-2"); + // ...and only the one that asked for nothing becomes a headline number. + expect(rows[1].className).toContain("lg:grid-cols-2"); + }); + + it("caps the KPI row at the dashboard's declared column count", () => { + const { container } = render( + , + ); + const row = container.querySelector( + 'section > div > div[class*="grid"]', + ) as HTMLElement; + expect(row.className).toContain("lg:grid-cols-2"); + expect(row.className).not.toContain("lg:grid-cols-4"); + }); + + it("stacks every widget in a row of its own for a stack layout", () => { + const { container } = render( + , + ); + expect(container.querySelector("section > div")?.className).toContain( + "flex flex-col", + ); + expect(container.querySelector('[class*="grid-cols-4"]')).toBeNull(); + }); +}); + +// The seams a host composes its own features through. They are what let the +// host's solution dashboard keep its reorderable grid, its + menu and its +// per-widget queries while this renderer still does the drawing. +describe("slots", () => { + const widgets: DashboardWidgetView[] = [ + { id: "a", title: "A", visualization: "line", series: daily }, + { id: "b", title: "B", visualization: "number", series: daily }, + ]; + + it("puts actions in the dashboard's own header", () => { + render( + Add }} + />, + ); + expect(screen.getByRole("button", { name: "Add" })).toBeTruthy(); + }); + + it("hands every widget to renderWidget, in the view's order", () => { + render( +
  • {widget.id}
    • }} + />, + ); + expect( + screen.getAllByRole("listitem").map((item) => item.textContent), + ).toEqual(["a", "b"]); + }); + + it("hands the tiles, each with its widget, to a layout of the host's own", () => { + render( + ( +
      + {tiles.map((tile) => ( +
    • + {tile.node} +
      • + ))} +
    + ), + }} + />, + ); + const items = screen.getAllByRole("listitem"); + expect(items.map((item) => item.dataset.visualization)).toEqual([ + "line", + "number", + ]); + // The nodes are the kit's own cards: the chart's data table is in them. + expect(within(items[0]).getByRole("table", { name: "A" })).toBeTruthy(); + }); + + it("shows the host's own empty state in place of the layout", () => { + render( + Add one back with +.

    }} + />, + ); + expect(screen.getByText("Add one back with +.")).toBeTruthy(); + expect(screen.queryByText("No widgets.")).toBeNull(); + }); }); diff --git a/module/services/frontend/code/packages/codefly-ui/src/dashboard/__tests__/geometry.test.ts b/module/services/frontend/code/packages/codefly-ui/src/dashboard/__tests__/geometry.test.ts index ff04c06ff..de9854fbb 100644 --- a/module/services/frontend/code/packages/codefly-ui/src/dashboard/__tests__/geometry.test.ts +++ b/module/services/frontend/code/packages/codefly-ui/src/dashboard/__tests__/geometry.test.ts @@ -1,5 +1,12 @@ import { describe, expect, it } from "vitest"; -import { areaPath, linearScale, linePath, niceTicks, scaleX, sparkPlot } from "../geometry.js"; +import { + areaPath, + linearScale, + linePath, + niceTicks, + scaleX, + sparkPlot, +} from "../geometry.js"; import type { SeriesPoint } from "../types.js"; // Path coordinates are emitted as "M x y L x y …"; pull them out as alternating @@ -12,7 +19,8 @@ function coords(d: string): { xs: number[]; ys: number[] } { }; } -const pts = (...values: number[]): SeriesPoint[] => values.map((value, i) => ({ key: String(i), value })); +const pts = (...values: number[]): SeriesPoint[] => + values.map((value, i) => ({ key: String(i), value })); describe("linePath", () => { // Regression: scaleY used to close over the constant full viewBox height, so a @@ -118,7 +126,8 @@ describe("niceTicks", () => { const ticks = niceTicks(0, 95); expect(ticks[0]).toBe(0); expect(ticks[ticks.length - 1]).toBeGreaterThanOrEqual(95); - for (let i = 1; i < ticks.length; i++) expect(ticks[i]).toBeGreaterThan(ticks[i - 1]); + for (let i = 1; i < ticks.length; i++) + expect(ticks[i]).toBeGreaterThan(ticks[i - 1]); }); // A degenerate span (all-zero series, or a single point anchored at its own diff --git a/module/services/frontend/code/packages/codefly-ui/src/dashboard/__tests__/types.test.ts b/module/services/frontend/code/packages/codefly-ui/src/dashboard/__tests__/types.test.ts index 02f7d839d..397e29328 100644 --- a/module/services/frontend/code/packages/codefly-ui/src/dashboard/__tests__/types.test.ts +++ b/module/services/frontend/code/packages/codefly-ui/src/dashboard/__tests__/types.test.ts @@ -25,8 +25,14 @@ describe("fromDashboardData", () => { expect(view.columns).toBe(3); expect(view.accent).toBe("#7c3aed"); expect(view.widgets).toHaveLength(1); - expect(view.widgets[0]).toMatchObject({ id: "trend", visualization: "line" }); - expect(view.widgets[0].series.total).toBe(3); + expect(view.widgets[0]).toMatchObject({ + id: "trend", + visualization: "line", + }); + // `fromDashboardData` bridges an ALREADY-resolved result, so the series + // it produces is never the `null` the view model allows for a widget + // still being resolved. + expect(view.widgets[0].series?.total).toBe(3); }); it("defaults layout to grid when the source omits it", () => { diff --git a/module/services/frontend/code/packages/codefly-ui/src/dashboard/charts.tsx b/module/services/frontend/code/packages/codefly-ui/src/dashboard/charts.tsx index 003f3267e..325b130fe 100644 --- a/module/services/frontend/code/packages/codefly-ui/src/dashboard/charts.tsx +++ b/module/services/frontend/code/packages/codefly-ui/src/dashboard/charts.tsx @@ -14,17 +14,17 @@ import type * as React from "react"; import { Axis, Gridline, Svg } from "./atoms.js"; import { cn } from "./cn.js"; import { - areaPath, AXIS_MARGIN, + areaPath, linearScale, linePath, niceTicks, PAD, plot, sparkPlot, - valueDomain, VIEW_H, VIEW_W, + valueDomain, } from "./geometry.js"; import type { SeriesPoint } from "./types.js"; @@ -66,8 +66,21 @@ function cartesian({ const p = sparkPlot(height); return ( - {fill && } - + {fill && ( + + )} + ); } @@ -86,10 +99,28 @@ function cartesian({ const singleY = points.length === 1 ? y(points[0].value) : undefined; return ( - + {showY && } - {fill && } - + {fill && ( + + )} + {points.map((p) => (
    - + {format ? format(p.key) : p.key} - {p.value.toLocaleString()} + + {p.value.toLocaleString()} +
    ))} @@ -154,7 +217,9 @@ export function StatChart({ const style: React.CSSProperties = { color: "var(--primary)" }; return (
    - {total.toLocaleString()} + + {total.toLocaleString()} + {/* A trend needs two points. One point (a single all-time bucket, say) would draw as a flat line that reads as "no change". */} {points.length >= 2 && ( diff --git a/module/services/frontend/code/packages/codefly-ui/src/dashboard/dashboard.tsx b/module/services/frontend/code/packages/codefly-ui/src/dashboard/dashboard.tsx index 978723a30..91ede8d7e 100644 --- a/module/services/frontend/code/packages/codefly-ui/src/dashboard/dashboard.tsx +++ b/module/services/frontend/code/packages/codefly-ui/src/dashboard/dashboard.tsx @@ -6,13 +6,35 @@ // solution's Module-Federation remote render identical dashboards from the same // package instance. Data resolution (metric → audit query) is the job of // `@codefly-dev/saas-sdk`'s `runDashboard`; use `fromDashboardData` to bridge. +// +// It paints with the kit's METRIC tier — `StatTile`/`KPIRow` for scalars, +// `metric-chart`'s line/area/bar over `ChartSeries` for series — which is the +// tier the host's own operations pages are drawn with. Before, this renderer +// and the host's solution dashboard both used the older `charts.tsx` tier, so a +// solution's Dashboard tab read plainer than the host's own pages from the same +// kit, and there were two renderers to keep in step rather than one. import type * as React from "react"; +import { Fragment, type ReactNode } from "react"; import { Card } from "../layout/card.js"; import { Section } from "../layout/page.js"; -import { AreaChart, BarList, LineChart, StatChart } from "./charts.js"; +import { Skeleton } from "../layout/skeleton.js"; import { cn } from "./cn.js"; -import type { DashboardView, DashboardWidgetView } from "./types.js"; +import { parseTimeKey } from "./format.js"; +import { + AreaChart as MetricAreaChart, + BarChart as MetricBarChart, + LineChart as MetricLineChart, +} from "./metric-chart.js"; +import type { ChartSeries } from "./metric-geometry.js"; +import { type MetricState, MetricStateBadge } from "./metric-state.js"; +import { KPIRow, type Metric, StatTile } from "./metric-tiles.js"; +import type { + DashboardView, + DashboardWidgetView, + SeriesPoint, + WidgetSeries, +} from "./types.js"; // Column-span utility classes, matching the responsive grid below so a spanning // card widens in step with it. A span is clamped to the grid's column count by @@ -24,6 +46,19 @@ const COL_SPAN: Record<1 | 2 | 3 | 4, string> = { 4: "sm:col-span-2 lg:col-span-4", }; +// The KPI row's own column count, capped at the dashboard's DECLARED one. +// `KPIRow` is four across, which is what headline numbers want; a dashboard +// that declared fewer columns did not ask for more, and one that declared none +// did not ask for two either — so this is applied only when there is a +// declaration to honour. `cn` resolves these against the row's own defaults +// per breakpoint. +const KPI_COLS: Record<1 | 2 | 3 | 4, string> = { + 1: "grid-cols-1 sm:grid-cols-1 lg:grid-cols-1", + 2: "grid-cols-1 sm:grid-cols-2 lg:grid-cols-2", + 3: "grid-cols-1 sm:grid-cols-2 lg:grid-cols-3", + 4: "grid-cols-1 sm:grid-cols-2 lg:grid-cols-4", +}; + const GRID_COLS: Record<1 | 2 | 3 | 4, string> = { 1: "grid-cols-1", 2: "grid-cols-1 sm:grid-cols-2", @@ -31,75 +66,480 @@ const GRID_COLS: Record<1 | 2 | 3 | 4, string> = { 4: "grid-cols-1 sm:grid-cols-2 lg:grid-cols-4", }; -function WidgetBody({ widget }: { widget: DashboardWidgetView }) { - const { series, visualization } = widget; - if (series.points.length === 0) { - return
    No data yet.
    ; +/** The widget's own title, or its id when the declaration gave none. */ +function widgetTitle(widget: DashboardWidgetView): string { + return widget.title ?? widget.id; +} + +/** + * The series' values in time order, or `null` when it is not a trend at all. + * + * Only a series whose every key is a time bucket earns a sparkline and a + * bucket-over-bucket delta: drawing a line through counts grouped by event type + * or region would assert an order and a direction the data does not have. + * + * The order comes from the parsed timestamps rather than from the array, + * because nothing guarantees the array is chronological — the resolver appends + * one point per bucket in whatever order the aggregate returned them, and + * `ChartSeries` documents x-order as the producer's responsibility. Reading + * "previous" and "latest" off the last two positions would, for a series + * delivered newest-first, report the change with the wrong sign — and a delta's + * sign is drawn as a colour, so a decline would read as a green rise. + */ +function trendValues(points: SeriesPoint[]): number[] | null { + if (points.length < 2) return null; + const dated: { at: number; value: number }[] = []; + for (const point of points) { + const at = parseTimeKey(point.key); + if (at === null) return null; + dated.push({ at: at.getTime(), value: point.value }); + } + dated.sort((a, b) => a.at - b.at); + return dated.map((point) => point.value); +} + +/** + * The badge a resolved series earns, or `undefined` when it is simply ready. + * + * `partial` outranks a withheld total because it is the *reason* the total is + * withheld (a partial series keeps its observed points and gives up its total), + * and a reader who sees "No total" alone learns less than one who sees + * "Partial data". An empty series is `no_data`, never a zero: the audit + * aggregate omits a bucket rather than emitting one, so no points means nothing + * matched. + */ +function seriesState(series: WidgetSeries): MetricState | undefined { + if (series.coverage === "partial") return "partial"; + if (series.coverage === "empty" || series.points.length === 0) { + return "no_data"; } - switch (visualization) { + if (series.total === null) return "no_total"; + return undefined; +} + +/** + * Change from the second-latest bucket to the latest, as the signed fraction + * `StatTile` renders. A change from zero is not a percentage, so it is no + * delta at all rather than an infinite one. + */ +function lastBucketDelta(trend: number[]): number | undefined { + if (trend.length < 2) return undefined; + const previous = trend[trend.length - 2]; + const latest = trend[trend.length - 1]; + if (previous === 0) return undefined; + return (latest - previous) / Math.abs(previous); +} + +/** + * Whether this series has a single number to show. An empty result and a + * withheld total are both "no number"; an emitted zero is a number. + */ +function hasScalar(series: WidgetSeries): boolean { + return ( + series.total !== null && + series.points.length > 0 && + series.coverage !== "empty" + ); +} + +/** A resolved scalar widget as the `Metric` a tile binds to. */ +function widgetMetric( + widget: DashboardWidgetView, + series: WidgetSeries, +): Metric { + const trend = trendValues(series.points); + const delta = trend ? lastBucketDelta(trend) : undefined; + return { + id: widget.id, + label: widgetTitle(widget), + // Whether a number exists is read off the SERIES, never off the badge. + // The badge is one slot and freshness can take it (a failed refresh + // says "Stale"), so deriving the value's presence from the state made + // an empty series render its 0 the moment it also went stale. The two + // are different questions about the same tile and are answered apart. + value: hasScalar(series) ? series.total : null, + format: widget.format, + unit: widget.unit, + higherIsBetter: widget.higherIsBetter, + series: trend ?? undefined, + delta, + // Only named when there is a delta: `MetricDelta` reads a label with no + // delta as "no data
    ); } diff --git a/module/services/frontend/code/packages/codefly-ui/src/dashboard/sparkline.tsx b/module/services/frontend/code/packages/codefly-ui/src/dashboard/sparkline.tsx index 005664923..72ea0380e 100644 --- a/module/services/frontend/code/packages/codefly-ui/src/dashboard/sparkline.tsx +++ b/module/services/frontend/code/packages/codefly-ui/src/dashboard/sparkline.tsx @@ -18,6 +18,7 @@ * dividing by zero; the visual "no change" answer is correct. */ +import type { CSSProperties } from "react"; import { useId } from "react"; import { linearScale } from "./geometry.js"; @@ -26,6 +27,12 @@ interface SparklineProps { width?: number; height?: number; className?: string; + /** + * Inline style for the svg. The line strokes `currentColor`, so this is how + * a caller gives it a colour from a token rather than from a utility class + * — which is what lets a themed subtree's own accent reach it. + */ + style?: CSSProperties; // strokeWidth defaults to 1.5 — looks crisp at 1× and 2× DPR. strokeWidth?: number; } @@ -36,6 +43,7 @@ export function Sparkline({ height = 24, strokeWidth = 1.5, className, + style, }: SparklineProps) { const gradId = `sparkline-grad-${useId().replaceAll(":", "")}`; @@ -46,6 +54,7 @@ export function Sparkline({ height={height} viewBox={`0 0 ${width} ${height}`} className={className} + style={style} aria-hidden /> ); @@ -81,6 +90,7 @@ export function Sparkline({ height={height} viewBox={`0 0 ${width} ${height}`} className={className} + style={style} aria-hidden > diff --git a/module/services/frontend/code/packages/codefly-ui/src/dashboard/types.ts b/module/services/frontend/code/packages/codefly-ui/src/dashboard/types.ts index 55be73394..9d1b1c7ec 100644 --- a/module/services/frontend/code/packages/codefly-ui/src/dashboard/types.ts +++ b/module/services/frontend/code/packages/codefly-ui/src/dashboard/types.ts @@ -8,6 +8,15 @@ // own types here is what lets `@codefly-dev/ui` stay a pure component library with no // dependency on the SDK's transport stack. +import type { MetricFormat } from "./metric-tiles.js"; + +/** + * How a widget's read ended when it did not succeed. Presentational: a caller + * classifies its own transport (the host app maps its `ReadOutcome` onto this) + * and the renderer only decides what to draw. + */ +export type WidgetReadFailure = "failed" | "refused"; + /** How a widget draws its series. */ export type WidgetVisualization = "line" | "bar" | "area" | "number" | "table"; @@ -32,9 +41,43 @@ export interface DashboardWidgetView { id: string; visualization: WidgetVisualization; title?: string; - series: WidgetSeries; + /** + * The resolved series, or `null` while it is still being resolved. A + * renderer draws a placeholder for `null` rather than an empty series, + * because "not yet" and "nothing matched" are different answers and the + * second one is a fact about the data. + */ + series: WidgetSeries | null; + /** + * How the read ended, when it did not succeed. A caller that resolves each + * widget separately sets it per widget, so one failure says so in its own + * card instead of blanking its siblings. + * + * The two are not a severity ordering, they are different questions. + * `"failed"` says the read did not complete and says nothing about + * authority, so a series already in hand is still the best answer and is + * drawn as stale. `"refused"` says this viewer is not allowed to see the + * number, which makes a previously fetched one a WRONG answer rather than + * an old one — so the renderer discards it, whatever the caller passed. + * A boolean cannot express that difference, which is why this is not one. + */ + failure?: WidgetReadFailure; /** Grid columns this widget spans (1–4); ignored in a stack. */ span?: 1 | 2 | 3 | 4; + /** + * How a scalar widget's value is rendered. Defaults to a plain number. + * + * `"percent"` takes a value that is ALREADY a percentage: 45 renders "45%". + * It does not take a share — 0.4 renders "0.4%", not "40%". A producer whose + * own `percent` means a 0-to-1 share must scale it before it reaches here, + * because this field is bound structurally (see `fromDashboardData`) and + * nothing on the wire can tell the two conventions apart. + */ + format?: MetricFormat; + /** Suffix after a scalar widget's value, e.g. "req/s". */ + unit?: string; + /** When false a rising value reads as bad (an error count). Defaults true. */ + higherIsBetter?: boolean; } /** The fully-resolved dashboard the renderer paints. */ @@ -57,6 +100,9 @@ interface ResolvedWidgetLike { visualization: WidgetVisualization; title?: string; series: WidgetSeries; + format?: MetricFormat; + unit?: string; + higherIsBetter?: boolean; } interface DashboardDataLike { title?: string; @@ -84,6 +130,9 @@ export function fromDashboardData( visualization: widget.visualization, title: widget.title, series: widget.series, + format: widget.format, + unit: widget.unit, + higherIsBetter: widget.higherIsBetter, })), }; } diff --git a/module/services/frontend/code/packages/codefly-ui/src/skin/preview.generated.css b/module/services/frontend/code/packages/codefly-ui/src/skin/preview.generated.css index 79c8e1603..20b57248a 100644 --- a/module/services/frontend/code/packages/codefly-ui/src/skin/preview.generated.css +++ b/module/services/frontend/code/packages/codefly-ui/src/skin/preview.generated.css @@ -4438,6 +4438,10 @@ max-width: var(--container-sm); } + .sm\:grid-cols-1 { + grid-template-columns: repeat(1, minmax(0, 1fr)); + } + .sm\:grid-cols-2 { grid-template-columns: repeat(2, minmax(0, 1fr)); } @@ -4553,6 +4557,14 @@ grid-column: span 4 / span 4; } + .lg\:grid-cols-1 { + grid-template-columns: repeat(1, minmax(0, 1fr)); + } + + .lg\:grid-cols-2 { + grid-template-columns: repeat(2, minmax(0, 1fr)); + } + .lg\:grid-cols-3 { grid-template-columns: repeat(3, minmax(0, 1fr)); } diff --git a/module/services/frontend/code/packages/codefly-ui/stories/metrics.stories.tsx b/module/services/frontend/code/packages/codefly-ui/stories/metrics.stories.tsx index f5066f219..70cbd65e5 100644 --- a/module/services/frontend/code/packages/codefly-ui/stories/metrics.stories.tsx +++ b/module/services/frontend/code/packages/codefly-ui/stories/metrics.stories.tsx @@ -1,12 +1,12 @@ -import { TrendLineChart } from "../src/dashboard/index.js"; import { - MetricCard, - StatTile, KPIRow, MetricAreaChart, MetricBarChart, + MetricCard, MetricLineChart, type MetricState, + StatTile, + TrendLineChart, } from "../src/dashboard/index.js"; export default { title: "Shared UI/Metrics" }; @@ -56,6 +56,7 @@ export const Availability = { "stale", "provider_unavailable", "not_configured", + "no_total", ] satisfies MetricState[] ).map((state) => ( diff --git a/module/services/frontend/code/packages/saas-ui/package.json b/module/services/frontend/code/packages/saas-ui/package.json index 505377c0c..d62870fe0 100644 --- a/module/services/frontend/code/packages/saas-ui/package.json +++ b/module/services/frontend/code/packages/saas-ui/package.json @@ -1,6 +1,6 @@ { "name": "@codefly-dev/saas-ui", - "version": "0.12.6", + "version": "0.12.7", "description": "Reusable SaaS-domain frontend components (datasources, …) the portal and solutions both import, built on @codefly-dev/ui primitives and driven by an injected typed client.", "repository": { "type": "git", diff --git a/module/services/frontend/code/src/components/__tests__/fixtures/README.md b/module/services/frontend/code/src/components/__tests__/fixtures/README.md new file mode 100644 index 000000000..ddb89f971 --- /dev/null +++ b/module/services/frontend/code/src/components/__tests__/fixtures/README.md @@ -0,0 +1,17 @@ +# Fixtures + +`deleted-solution-dashboard.txt` is `src/solutions/SolutionDashboard.tsx` as it +stood at `8590d329`, before #1041 replaced its hand-rolled widget dispatch with +a mount of the kit's renderer. The duplication gate asserts that it would be +refused today, so the rule is pinned against the real duplication rather than a +paraphrase of it. + +It is committed rather than read with `git show` because the suite ships inside +an immutable module package: a consuming repository carries this source tree and +not this repository's history, so a fixture that reaches for an upstream commit +fails there with `fatal: not a git repository` while passing in full-history CI +— a portability failure that CI is structurally unable to see. + +The `.txt` extension is deliberate. The file is a byte fixture, not a module: +as `.tsx` it would be typechecked against today's imports, linted, and counted +by `ui-inventory.json`. diff --git a/module/services/frontend/code/src/components/__tests__/fixtures/deleted-solution-dashboard.txt b/module/services/frontend/code/src/components/__tests__/fixtures/deleted-solution-dashboard.txt new file mode 100644 index 000000000..526feeade --- /dev/null +++ b/module/services/frontend/code/src/components/__tests__/fixtures/deleted-solution-dashboard.txt @@ -0,0 +1,695 @@ +"use client"; + +import type { + Dashboard, + DataGraph, + MetricWidget, +} from "@codefly/saas-plugin-manifest"; +import { + createSaasClient, + type ResolvedWidget, + runDashboard, +} from "@codefly-dev/saas-sdk"; +// Charts come from the shared kit, not host-internal components: the same +// primitives a solution's own remote would render with, so host-rendered and +// solution-rendered dashboards look identical and there is one charting +// implementation to maintain. +import { + AreaChart, + BarList, + LineChart, + SortableGrid, + StatChart, +} from "@codefly-dev/ui/dashboard"; +import { useQuery } from "@tanstack/react-query"; +import { GripVertical, Info, Plus, X } from "lucide-react"; +import { type ReactNode, useState, useSyncExternalStore } from "react"; +import { flushSync } from "react-dom"; +import { resolveActor } from "@/features/audit/model/transforms"; +import { + useAuditEventTypes, + usePrincipalDirectory, +} from "@/features/audit/service/queries"; +import { scopedDashboardDraftKey } from "@/features/dashboard/service/use-dashboard-authoring"; +import { useAuth } from "@/lib/auth"; +import { apiTransport } from "@/lib/connect/transport"; +import { + Button, + Card, + CardContent, + CardHeader, + CardTitle, + DropdownMenu, + DropdownMenuContent, + DropdownMenuItem, + DropdownMenuTrigger, + Popover, + PopoverContent, + PopoverTitle, + PopoverTrigger, + Skeleton, +} from "@/shared/ui"; +import { + addableTiles, + addTile, + layoutKey, + moveBy, + parseLayout, + readSavedLayout, + removeTile, + serializeLayout, + swapTiles, + tileWidget, + writeSavedLayout, +} from "./dashboard-layout"; +import { + ACTIVITY_DASHBOARD, + activityGraph, + activitySummary, + describeMetric, + formatDay, + formatMoment, + newestEvents, + recentEventsQueries, +} from "./metric-info"; + +// One audit-backed SDK client for every solution dashboard, built on the host's +// shared Connect transport so a solution's declared graph resolves through the +// same auth'd, org-scoped gateway path the rest of the app uses. The host owns +// the client, the bearer token, and the org scope; the solution supplies only +// the DataGraph — it can never widen a query past the viewer's own org. +const sdk = createSaasClient(apiTransport); + +// Dashboard id of the throwaway single-widget graph each card resolves. The card +// resolves its own widget in isolation, so this id is never surfaced. +const SINGLE_WIDGET_DASHBOARD = "widget"; + +// A graph carrying every event and metric but exactly one widget, so runDashboard +// resolves only that widget's metric (and its derived inputs). Resolving each +// widget through its own call is what isolates failures: a widget whose metric's +// audit query errors fails alone, instead of blanking every sibling that shares +// the dashboard's single batched resolution. +function singleWidgetGraph(graph: DataGraph, widget: MetricWidget): DataGraph { + return { + events: graph.events, + metrics: graph.metrics, + dashboards: [ + { id: SINGLE_WIDGET_DASHBOARD, layout: "grid", widgets: [widget] }, + ], + }; +} + +// Render a resolved widget's series in the shape its visualization asks for. A +// series with no points is "no data yet" for every kind — the audit RPC omits a +// bucket rather than emitting a zero, so an empty series means nothing matched, +// not a real zero worth plotting. +function WidgetBody({ widget }: { widget: ResolvedWidget }) { + return ( + <> + {widget.series.coverage === "partial" && ( +

    + Partial telemetry +

    + )} + + + ); +} + +function WidgetValues({ widget }: { widget: ResolvedWidget }) { + const { series, visualization } = widget; + if (series.points.length === 0) { + return ( +

    + {series.coverage === "partial" + ? "Telemetry unavailable or incomplete." + : "No data yet."} +

    + ); + } + switch (visualization) { + case "line": + return ( + + ); + case "area": + return ( + + ); + case "bar": + return ; + case "number": + return series.total === null ? ( +

    + {series.coverage === "partial" + ? "Incomplete telemetry; total unavailable." + : "Total unavailable across groups."} +

    + ) : ( + + ); + case "table": + return ( + + + {series.points.map((point) => ( + + + + + ))} + +
    {point.key} + {point.value.toLocaleString()} +
    + ); + default: { + // Compile-time exhaustiveness: a new WidgetVisualization must be + // handled here or this assignment fails to type-check. + const _exhaustive: never = visualization; + return _exhaustive; + } + } +} + +function WidgetCard({ + graph, + widget, + solutionId, + dashboardId, + orgId, + grip, + info, + remove, +}: { + graph: DataGraph; + widget: MetricWidget; + solutionId: string; + dashboardId: string; + orgId: string; + grip: ReactNode; + info: ReactNode; + remove: ReactNode; +}) { + // The graph is part of the key so a solution that redeploys with a changed + // declaration refetches instead of serving another graph's cached series that + // happens to share the same solution/dashboard/widget/org ids. Disabled until + // the org resolves so the pre-org window reads as loading, never as empty. + const { data, isPending, isError } = useQuery({ + queryKey: [ + "solution-widget", + solutionId, + dashboardId, + widget.id, + orgId, + graph, + ], + queryFn: () => + runDashboard( + sdk.audit, + singleWidgetGraph(graph, widget), + SINGLE_WIDGET_DASHBOARD, + { orgId }, + ).then((resolved) => resolved.widgets[0]), + enabled: orgId !== "", + }); + + return ( + + + {grip} + + {widget.title ?? widget.metric} + + {info} + {remove} + + + {isError ? ( +

    Unable to load.

    + ) : isPending || !data ? ( + + ) : ( + + )} +     +     + ); +} + +// Reading `window.localStorage` can itself throw: blocked site data or a +// sandboxed frame raise a SecurityError from the getter, before any getItem. No +// storage means every viewer sees the declared default. +function browserStorage(): Storage | null { + if (typeof window === "undefined") return null; + try { + return window.localStorage; + } catch { + return null; + } +} + +// The saved layout is read from storage on each render, so there is nothing to +// subscribe to; a change made on this page is held in state instead. +function noSubscription() { + return () => {}; +} + +/** + * One viewer's layout of one dashboard, kept in this browser. The server + * cannot know what the browser saved, so it renders the declared default and + * React swaps in the saved layout after hydration; `useSyncExternalStore` is + * what makes that a defined re-render rather than a hydration mismatch. + * + * The layout is saved only on the viewer's own change, so a viewer who never + * customizes keeps following the declared default. + * + * STOPGAP (labelled): browser storage is an interim home, so a layout does not + * follow the viewer to another browser or device. ADR 0007 puts per-user + * dashboard preferences in a composed `user_settings` field keyed by dashboard + * id; that field and its typed settings-catalog entry do not exist yet (no + * tracking issue yet). When they land, this hook's read and write move there + * and nothing else changes. + */ +function useDashboardLayout( + storageKey: string, + dashboard: Dashboard, +): [string[], (next: readonly string[]) => void] { + const stored = useSyncExternalStore( + noSubscription, + () => readSavedLayout(browserStorage(), storageKey), + () => null, + ); + // The viewer's latest change on this page. It holds even when storage + // refuses the write; it just will not outlive a reload. + const [edited, setEdited] = useState<{ key: string; raw: string } | null>( + null, + ); + const layout = parseLayout( + edited?.key === storageKey ? edited.raw : stored, + dashboard, + ); + const save = (next: readonly string[]) => { + const raw = serializeLayout(next, dashboard); + setEdited({ key: storageKey, raw }); + writeSavedLayout(browserStorage(), storageKey, raw); + }; + return [layout, save]; +} + +const ARROW_STEP: Partial> = { + ArrowUp: -1, + ArrowLeft: -1, + ArrowDown: 1, + ArrowRight: 1, +}; + +// How many of the newest events the ⓘ lists. +const RECENT_EVENTS = 5; + +// Where a tile's number comes from: what it counts and from which audit +// events, read from the solution's declaration; how many events there are and +// the days they span, from per-day counts; and the newest events themselves, +// from the audit log. Nothing is asked of the audit trail until the viewer +// opens it. +function MetricInfo({ + graph, + widget, + orgId, +}: { + graph: DataGraph; + widget: MetricWidget; + orgId: string; +}) { + const [open, setOpen] = useState(false); + const title = widget.title ?? widget.metric; + const { counts, sources, note } = describeMetric( + graph, + widget.metric, + widget.visualization, + ); + const { data: registered } = useAuditEventTypes({ enabled: open }); + const activity = useQuery({ + queryKey: ["solution-metric-activity", widget.metric, orgId, graph], + queryFn: () => + runDashboard( + sdk.audit, + activityGraph(graph, widget.metric), + ACTIVITY_DASHBOARD, + { orgId }, + ).then((resolved) => + activitySummary(resolved.widgets.map((w) => w.series)), + ), + enabled: open && orgId !== "", + }); + const searches = recentEventsQueries(graph, widget.metric); + const recent = useQuery({ + queryKey: ["solution-metric-recent", widget.metric, orgId, graph], + queryFn: async () => + newestEvents( + await Promise.all( + (searches ?? []).map((search) => + sdk.audit + .queryAuditLog({ orgId, pageSize: RECENT_EVENTS, ...search }) + .then((response) => response.events), + ), + ), + RECENT_EVENTS, + ), + enabled: open && orgId !== "" && searches !== null, + }); + const { directory } = usePrincipalDirectory( + orgId, + recent.data?.map((event) => event.actorId) ?? [], + ); + const who = (actorId: string) => resolveActor(actorId, directory).label; + + const summary = activity.data; + const counted = activity.isPending + ? "Loading…" + : activity.isError + ? "Unavailable" + : null; + const dayFreshness = + counted ?? + (summary ? `Latest event on ${formatDay(summary.last)}` : "No events yet"); + // A metric the audit log search cannot narrow like the metric does (by + // collection) keeps the day its per-day counts give. + const newest = recent.data?.[0]; + const freshness = + searches === null || recent.isError + ? dayFreshness + : recent.isPending + ? "Loading…" + : newest + ? `${formatMoment(newest.at)}, by ${who(newest.actorId)}` + : "No events yet"; + + return ( + + + } + > + + + + {title} +
    +
    Counts
    +
    {counts}
    +
    Counted from
    +
    + {sources.map(({ type, declared }) => { + // The audit service's type list holds only the platform's own + // types, so a type a module registers falls back to what the + // solution's declaration says about it. + const description = + registered?.find((t) => t.name === type)?.description || + declared; + return ( +

    + {description && {description}} + + {type} + +

    + ); + })} +
    +
    Scope
    +
    Your organization
    +
    Period
    +
    All time
    +
    Based on
    +
    + {counted ?? + (summary + ? `${summary.events.toLocaleString()} ${summary.events === 1 ? "event" : "events"}` + : "No events yet")} +
    +
    Data range
    +
    + {counted ?? + (summary + ? formatDay(summary.first) === formatDay(summary.last) + ? formatDay(summary.first) + : `${formatDay(summary.first)} – ${formatDay(summary.last)}` + : "No events yet")} +
    +
    Freshness
    +
    {freshness}
    + {recent.data && recent.data.length > 0 && ( + <> +
    Recent events
    +
    +
      + {recent.data.map((event) => ( +
    • + {formatMoment(event.at, { year: false })} ·{" "} + {who(event.actorId)} +
      • + ))} +
    +
    + + )} + {note && ( + <> +
    Note
    +
    {note}
    + + )} +
    +     +     + ); +} + +// Lists what the viewer can put back on the dashboard: the declared widgets +// they removed. A preference never adds a metric the dashboard does not draw +// (ADR 0007). +function AddTileMenu({ + label, + addable, + onAdd, +}: { + label: string; + addable: { id: string; label: string }[]; + onAdd: (tileId: string) => void; +}) { + return ( + + + } + > + + + + {addable.length === 0 ? ( + + Every widget is on this dashboard + + ) : ( + addable.map((tile) => ( + onAdd(tile.id)}> + {tile.label} + + )) + )} + + + ); +} + +// A dashboard the viewer can arrange: drag a tile to reorder (or focus its grip +// and use the arrow keys), × to remove one, + to add one back. +function SolutionDashboard({ + graph, + dashboard, + solutionId, +}: { + graph: DataGraph; + dashboard: Dashboard; + solutionId: string; +}) { + const { user, organizationId } = useAuth(); + const orgId = organizationId ?? ""; + const storageKey = scopedDashboardDraftKey( + layoutKey(solutionId, dashboard.id), + { organizationId, userId: user?.id }, + ); + const [layout, save] = useDashboardLayout(storageKey, dashboard); + const shown = layout.flatMap((tileId) => { + const widget = tileWidget(dashboard, tileId); + return widget ? [{ tileId, widget }] : []; + }); + const widgets = new Map(shown.map(({ tileId, widget }) => [tileId, widget])); + const titleOf = (tileId: string) => { + const widget = widgets.get(tileId); + return widget?.title ?? widget?.metric ?? tileId; + }; + const renderTile = (tileId: string) => { + const widget = widgets.get(tileId); + if (!widget) return null; + const title = titleOf(tileId); + return ( + { + const step = ARROW_STEP[event.key]; + if (step === undefined) return; + event.preventDefault(); + const grip = event.currentTarget; + // Moving a tile can move its DOM node, which drops focus; + // commit first so the grip can take focus back. + flushSync(() => save(moveBy(layout, tileId, step))); + grip.focus(); + }} + > + + + } + info={} + remove={ + + } + /> + ); + }; + // The floating copy while a tile is dragged: the same card, but with no + // info popover (so no second panel or audit query exists mid-drag) and no + // controls a keyboard or screen reader could reach. The icons stay so the + // copy is drawn the same size as the tile it follows. + const renderDragged = (tileId: string) => { + const widget = widgets.get(tileId); + if (!widget) return null; + const glyph = (icon: ReactNode) => ( + + ); + return ( + + ); + }; + + return ( +
    +
    + {dashboard.title && ( +

    + {dashboard.title} +

    + )} + save(addTile(layout, tileId))} + /> +
    + {shown.length === 0 ? ( +

    + No widgets are shown. Add one back with the + button. +

    + ) : ( + // The kit's SortableGrid rather than its Grid/Stack: the tiles are + // dragged onto each other to swap. The classes are those Grid + // cols={2} and Stack draw with. + tileId)} + onSwap={(dragged, target) => save(swapTiles(layout, dragged, target))} + itemLabel={titleOf} + renderItem={renderTile} + renderOverlay={renderDragged} + className={ + dashboard.layout === "stack" + ? "flex flex-col gap-4" + : "grid grid-cols-1 gap-4 sm:grid-cols-2" + } + /> + )} +
    + ); +} + +/** + * Renders every dashboard a registered solution declares in its data-graph slot. + * Each widget resolves against the audit trail through its own query, so one + * widget's failure never blanks its siblings. A solution ships only the + * declaration; all charting lives here in the host. Each viewer arranges each + * dashboard for themselves, starting from the declared widgets in declared + * order. + */ +export function SolutionDashboards({ + graph, + solutionId, +}: { + graph: DataGraph; + solutionId: string; +}) { + return ( +
    + {graph.dashboards.map((dashboard) => ( + + ))} +
    + ); +} diff --git a/module/services/frontend/code/src/components/__tests__/kit.test.ts b/module/services/frontend/code/src/components/__tests__/kit.test.ts new file mode 100644 index 000000000..755cc806a --- /dev/null +++ b/module/services/frontend/code/src/components/__tests__/kit.test.ts @@ -0,0 +1,1875 @@ +import { readdirSync, readFileSync } from "node:fs"; +import { dirname, join, relative } from "node:path"; +import { fileURLToPath } from "node:url"; +import ts from "typescript"; +import { describe, expect, it } from "vitest"; +import packageJson from "../../../package.json" with { type: "json" }; + +/** + * The host app composes `@codefly-dev/ui`; it does not re-implement it. + * + * The audit that opened issue #1041 found a solution's Dashboard tab drawn with + * the kit's older basic chart tier while the host's own operations pages used + * its metric tier — because `src/solutions/SolutionDashboard.tsx` had its own + * copy of the widget-to-chart dispatch, the responsive grid, the card shell and + * the empty state beside the kit's ``. Two renderers of one thing do + * not stay equal: one gets the sparkline, the crosshair readout and the + * screen-reader data table a release later, and the other does not. The owner + * asked for a linter. + * + * The rule is not invented here: another module that composes this kit already + * carries it, in a gate of the same shape, and that one is copied rather than + * replaced. Three legs, because no single one of them is sufficient: + * + * - **A name the kit exports** may be declared here only by a wrapper that + * renders the kit's component of that name, or with a reviewed entry in + * {@link ALLOWED}. Both forms of the declaration count — exported and + * module-local — because a duplicate hidden as a file-local helper is the + * commonest way one arrives (two of them were sitting here when this gate + * was written, and the upstream version of this rule, which looked only at + * exported declarations, would have walked past both). + * - **`class-variance-authority`** — the library the kit builds its variants + * with — is not a dependency of this package, so a kit-shaped primitive + * cannot be built here at all. This leg is a fact, not a convention, which + * is why it is worth more than the first. + * - **The chart surfaces stay re-exports.** `src/components/charts/chart.tsx` + * and `src/features/dashboard/ui/charts/line-chart.tsx` are each a single + * `export … from "@codefly-dev/ui/dashboard"`. Issue #1041 asked what they + * were; they are already right, and this keeps them so. + * + * It resolves names with the TypeScript checker, so every spelling that reaches + * the same component reaches the same verdict, and it reads what a component + * RETURNS rather than what a file mentions. Where it cannot resolve a name the + * kit owns — published from a third-party module, or from a chain whose end it + * cannot read — it refuses rather than passing: "could not tell" is not a + * verdict it is allowed to reach, and four rounds of adversarial fixtures each + * found the next way through the version that skipped instead. + * + * WHAT IT CANNOT SEE. This is a tripwire for the common shape, not a proof, and + * these are limits rather than bugs — each is a place where deciding would mean + * following the program further than reading one component's returned + * expression: + * + * - **A duplicate under a NEW name**, built by hand, passes every leg. + * {@link ALLOWED} and {@link DISPATCHERS} are the backstop — labels a + * reviewer reads and asks about, not detectors. + * - **Kit JSX handed to another component through an attribute.** What the + * recipient does with a prop is beyond this; a kit element in one is no + * evidence that this component renders it. Children are not attributes and + * do count. + * - **Anything behind a call.** A mark rendered by a helper function, or + * chosen from a lookup table (`MARKS[kind]`), is a dispatch this cannot + * read. The table is plain to a reviewer and invisible here. + * - **A dispatch split across components**, one visualization each, composed + * by a parent that only forwards. + * + * Classifying files by their names instead would fail on the next file anyone + * adds, and a gate that cries wolf is a gate that gets deleted. + */ + +const here = dirname(fileURLToPath(import.meta.url)); +const appRoot = join(here, "..", "..", ".."); + +/** + * A name this package declares although the kit exports it, and why. Each entry + * says what the two things are, why neither can stand in for the other, and + * what would have to change for the entry to go. + */ +const ALLOWED: Record = { + "Dashboard in src/features/dashboard/ui/dashboard.tsx": + "the host's dashboard DSL surface: it reads the viewer's organization from host auth and resolves each declared metric through `useMetric` as it draws. The kit's `` is pure and takes an already-resolved `DashboardView`, so neither can stand in for the other. Goes when the DSL resolves its metrics outside the renderer and hands the kit a view, as `src/solutions/SolutionDashboard.tsx` now does.", + "Dashboard in src/components/dashboard.tsx": + "the widget-slot renderer: a discriminated union of host-supplied widgets (sparkline, bars, series, arbitrary node), each carrying its own loading/error/empty state, rather than a declared metric view. Goes when the audit page's widgets are expressible as a `DashboardView` and it mounts the kit's renderer instead.", + "Layout in src/features/dashboard/ui/layout.tsx": + "page rhythm for a dashboard page — an `

    ` and a vertical stack. The kit's `Layout` is the application shell (sidebar, header, content region); this is what goes inside one. Goes when it is re-expressed on the kit's `Page`/`PageHeader`.", + "MetricCard in src/features/dashboard/ui/metric-card.tsx": + "resolves one declared metric through `useMetric` and then paints whichever chart the declaration asked for; the kit's `MetricCard` takes an already-resolved `Metric`. Goes with the DSL's resolution moving outside the card, the same change that retires the `Dashboard` entry above.", + "MetricCard in src/features/event-operations/ui/event-operations-page.tsx": + "a tile over an already-formatted string value plus a caption line. The kit's `StatTile`/`MetricCard` take a numeric `Metric` and have no caption slot, so this is not yet expressible as one. Goes when `StatTile` carries a description, or when these counts are expressed as `Metric`s with a `format`.", + "MetricCard in src/features/job-operations/ui/job-operations-page.tsx": + "the same tile as the event-operations page's, over the job queue's counts, and it goes with it — the two are each other's duplicate as much as the kit's, and both should become one call to the kit's tile.", +}; + +/** + * Chart surfaces that are, and stay, thin re-exports of the kit. + * + * Issue #1041 asked what these two are, since both carry a kit export's name. + * They are each a single `export … from "@codefly-dev/ui/dashboard"`: they ship + * the kit's component, not another one, so neither is a duplicate and neither + * needs an entry in {@link ALLOWED}. What they do carry is a hazard this gate + * cannot see, recorded here rather than left for the next reader to find: both + * publish the name `LineChart`, and they publish DIFFERENT kit components under + * it — `src/components/charts` re-exports the metric-tier `LineChart`, while + * `src/features/dashboard/ui/charts` re-exports `TrendLineChart`. An import of + * `LineChart` therefore means one of two charts depending on which path it came + * from. Retiring the second alias belongs with the dashboard-DSL work that + * retires `MetricCard in src/features/dashboard/ui/metric-card.tsx` below, its + * only consumer. + */ +const CHART_RE_EXPORTS = [ + "src/components/charts/chart.tsx", + "src/features/dashboard/ui/charts/line-chart.tsx", +]; + +function walk(dir: string, out: string[] = []): string[] { + for (const entry of readdirSync(dir, { withFileTypes: true })) { + const path = join(dir, entry.name); + if (entry.isDirectory()) { + if (entry.name !== "__tests__" && entry.name !== "node_modules") { + walk(path, out); + } + continue; + } + if (/\.tsx?$/.test(entry.name) && !/\.(test|stories)\./.test(entry.name)) { + out.push(path); + } + } + return out; +} + +/** + * `export *` clauses in the kit's entry points, by the module they name. + * + * The export scan below reads `export { … }` blocks, so a star re-export is + * invisible to it: the kit's root and plugin-host entry points use one, and + * four capitalised names reach consumers only that way. None collides with a + * host declaration today, but a partial miss is indistinguishable from success + * — the `kit.size` sanity check passes at 309 whether those four are in it or + * not — so the gate refuses a star it cannot resolve rather than quietly + * covering less than it claims. + */ +function starReExports( + entryPoints: { path: string; source: string }[], +): string[] { + const unresolved: string[] = []; + for (const { path, source } of entryPoints) { + for (const star of source.matchAll( + /export\s+\*\s+from\s*["']([^"']+)["']/g, + )) { + // A star onto another of the kit's own entry points resolves: that + // file is itself in this list and its named exports are read there. + if (star[1].startsWith("./") || star[1].startsWith("../")) continue; + unresolved.push(`${path} → ${star[1]}`); + } + } + return unresolved; +} + +/** Every capitalised name the kit's entry points export. */ +function kitExports(entryPoints: string[]): Set { + const names = new Set(); + for (const source of entryPoints) { + for (const block of source.matchAll(/export\s*\{([^}]*)\}/g)) { + for (const raw of block[1].split(",")) { + const name = + raw + .trim() + .replace(/^type\s+/, "") + .split(/\s+as\s+/) + .pop() + ?.trim() ?? ""; + if (/^[A-Z][A-Za-z0-9]*$/.test(name)) names.add(name); + } + } + } + return names; +} + +/** + * Modules of this app that exist only to re-export the kit — the host's own + * import surface. `src/shared/ui/index.ts` says so in its first line ("Host + * convenience imports; presentation is implemented by the shared kit"), and + * most of the app reaches the kit through it rather than through + * `@codefly-dev/ui` directly. + * + * They are kit sources for the purposes of the rule below. Without this, the + * gate's verdict depended on which path a wrapper imported through: the same + * `Badge` wrapper passed when it imported `@codefly-dev/ui/layout` and failed + * when it imported the identical component from `@/shared/ui`. A gate that + * rejects the house style is one that gets an exception entry per correct file, + * and then gets deleted. + */ +const KIT_SURFACES = [ + "src/shared/ui/index.ts", + "src/components/charts/chart.tsx", + "src/components/charts/index.ts", + "src/components/sparkline.tsx", + "src/components/metric-state.tsx", +]; + +const KIT_SPECIFIER = /^@codefly-dev\/(?:ui|saas-ui)(?:\/|$)/; + +/** + * Whether a module specifier names one of the host's kit surfaces, by alias + * (`@/shared/ui`) or by a relative path that lands on the same file. Both + * spellings reach the same module, so judging only the alias rejected a real + * wrapper for how it spelled its import. + */ +function isHostSurface(specifier: string, fromPath: string): boolean { + const resolved = specifier.startsWith("@/") + ? `src/${specifier.slice(2)}` + : specifier.startsWith(".") + ? join(dirname(fromPath), specifier).replaceAll("\\", "/") + : null; + if (resolved === null) return false; + return KIT_SURFACES.some( + (surface) => surface === resolved || surface.startsWith(`${resolved}/`), + ); +} + +/** + * What a local name in a file refers to. + * + * `{ kit }` is a kit component, under the kit's OWN name whatever the local + * spelling. `{ namespace }` is a whole module imported as one object, so + * `` has to be resolved through it at the use site. + */ +/** + * A program over just these sources, so a symbol can be resolved the way the + * compiler resolves it. + * + * Hand-rolled scope analysis is what kept misjudging this: it collected names + * across scopes (so a nested parameter shadowed an outer tag), missed + * destructured bindings (so a parameter did NOT shadow when it should have), + * and could not follow an export chain at all. The checker answers all of + * those from one question — what does this identifier refer to — so the rules + * below ask it instead of re-deriving it. + * + * Only these files are in the program: relative specifiers resolve between + * them and everything else (`react`, the kit itself) stays unresolved on + * purpose. A kit binding is recognised from the import declaration in the + * source, which is present whether or not the package is installed, so the + * gate needs no `node_modules` and cannot be weakened by a resolution failure. + */ +function programOver(files: { path: string; source: string }[]): { + program: ts.Program; + checker: ts.TypeChecker; + sources: Map; +} { + const sources = new Map( + files.map(({ path, source }) => [path, parse(path, source)]), + ); + const host: ts.CompilerHost = { + fileExists: (name) => sources.has(name), + readFile: (name) => sources.get(name)?.text, + getSourceFile: (name) => sources.get(name), + getDefaultLibFileName: () => "lib.d.ts", + writeFile: () => {}, + getCurrentDirectory: () => "", + getCanonicalFileName: (name) => name, + useCaseSensitiveFileNames: () => true, + getNewLine: () => "\n", + resolveModuleNames: (names, containingFile) => + names.map((name) => { + // The app's own two spellings of its own modules: a relative path + // and the `@/` alias its tsconfig maps to `src/`. Resolving only + // the first let a copy hide behind an export chain written in the + // second — the gate read the barrel and then stopped. + if (!name.startsWith(".") && !name.startsWith("@/")) return undefined; + const base = name.startsWith("@/") + ? `src/${name.slice(2)}` + : join(dirname(containingFile), name).replaceAll("\\", "/"); + for (const candidate of [ + base, + `${base}.tsx`, + `${base}.ts`, + `${base}/index.tsx`, + `${base}/index.ts`, + ]) { + if (sources.has(candidate)) { + return { + resolvedFileName: candidate, + extension: candidate.endsWith(".tsx") + ? ts.Extension.Tsx + : ts.Extension.Ts, + }; + } + } + return undefined; + }), + }; + const program = ts.createProgram( + [...sources.keys()], + { + jsx: ts.JsxEmit.Preserve, + noLib: true, + noResolve: false, + allowJs: false, + target: ts.ScriptTarget.Latest, + module: ts.ModuleKind.ESNext, + }, + host, + ); + return { program, checker: program.getTypeChecker(), sources }; +} + +const KIT_NAMESPACE = "*kit"; +const SURFACE_NAMESPACE = "*surface"; + +/** + * An alias chain from a symbol to its end, every hop included. + * + * Cycle-detected, not hop-capped. A chain that is long is not a chain that is + * suspicious, and abandoning one at an arbitrary depth was itself an escape: + * nine re-exports in a row resolved to nothing and the copy at the end of them + * went unexamined. A cycle is the only thing that can stop this, and it stops + * it without a verdict. + */ +function aliasChain(symbol: ts.Symbol, checker: ts.TypeChecker): ts.Symbol[] { + const chain = [symbol]; + const seen = new Set([symbol]); + let current = symbol; + while (current.flags & ts.SymbolFlags.Alias) { + const next = checker.getImmediateAliasedSymbol(current); + if (!next || seen.has(next)) break; + seen.add(next); + chain.push(next); + current = next; + } + return chain; +} + +/** + * Where a declaration comes from, when it comes from another module. + * + * `kit` is the kit's own component under the kit's own name, however the local + * spelling renamed it, whether it arrived by `import` or by `export … from`. + * `namespace` is a whole kit or surface module, to be resolved at the use site. + * `external` is a module that is neither — including a third-party one, which + * is as much a second implementation of a kit name as a hand-written copy. + * `undefined` means the declaration is local, so the chain continues. + */ +type KitBinding = + | { kind: "kit"; name: string } + | { kind: "namespace"; marker: string } + | { kind: "type" } + | { kind: "external"; from: string }; + +function kitBindingOf( + declaration: ts.Node, + surface: Map, +): KitBinding | undefined { + let from: ts.ImportDeclaration | ts.ExportDeclaration | undefined; + for (let node: ts.Node | undefined = declaration; node; node = node.parent) { + if (ts.isImportDeclaration(node) || ts.isExportDeclaration(node)) { + from = node; + break; + } + } + const specifier = from?.moduleSpecifier; + if (!from || !specifier || !ts.isStringLiteral(specifier)) return undefined; + if ( + (ts.isImportDeclaration(from) && from.importClause?.isTypeOnly) || + (ts.isExportDeclaration(from) && from.isTypeOnly) + ) { + return { kind: "type" }; + } + const fromKit = KIT_SPECIFIER.test(specifier.text); + const fromSurface = isHostSurface( + specifier.text, + from.getSourceFile().fileName, + ); + if (!fromKit && !fromSurface) { + return { kind: "external", from: specifier.text }; + } + if (ts.isNamespaceImport(declaration) || ts.isNamespaceExport(declaration)) { + return { + kind: "namespace", + marker: fromKit ? KIT_NAMESPACE : SURFACE_NAMESPACE, + }; + } + if (ts.isImportSpecifier(declaration) || ts.isExportSpecifier(declaration)) { + // A type is not a component. It cannot be rendered, it cannot be a + // second implementation of one, and treating a type-only binding as + // something unreadable would offend on every `export type` that shares + // a kit name. + if (declaration.isTypeOnly || typeOnlyClause(declaration)) { + return { kind: "type" }; + } + const published = (declaration.propertyName ?? declaration.name).text; + const name = fromKit ? published : surface.get(published); + return name + ? { kind: "kit", name } + : { kind: "external", from: specifier.text }; + } + return { kind: "external", from: specifier.text }; +} + +/** Whether the clause a specifier sits in is type-only. */ +function typeOnlyClause( + declaration: ts.ImportSpecifier | ts.ExportSpecifier, +): boolean { + const clause = declaration.parent.parent; + return ts.isImportClause(clause) + ? clause.isTypeOnly + : ts.isExportDeclaration(clause) && clause.isTypeOnly; +} + +/** + * The kit component an identifier refers to, by the kit's OWN name, or a + * namespace marker for a whole kit module, or undefined for anything else — + * including anything that cannot be resolved, which is the conservative + * answer: unresolved is not composition. + * + * The checker answers this, so every spelling that reaches the same component + * reaches the same verdict: an import alias, a local variable alias, a chain of + * re-exports through the app's own barrels, a member of a namespace import of + * either. Hand-rolled scope analysis is what kept misjudging it — it collected + * names across scopes, missed destructured bindings, and could not follow an + * export chain at all. + */ +function kitComponentOf( + node: ts.Expression | ts.JsxTagNameExpression, + checker: ts.TypeChecker, + surface: Map, + seen = new Set(), +): string | undefined { + if (seen.has(node)) return undefined; + seen.add(node); + + // The whole expression first, which is what resolves `UI.Card` when `UI` is + // a namespace of one of the app's own barrels: the checker walks the barrel's + // export to the kit import behind it. + const symbol = checker.getSymbolAtLocation(node); + if (symbol) { + for (const hop of aliasChain(symbol, checker)) { + const declaration = hop.declarations?.[0]; + if (!declaration) continue; + const binding = kitBindingOf(declaration, surface); + if (binding?.kind === "kit") return binding.name; + if (binding?.kind === "namespace") return binding.marker; + // Another module of the app's own: the chain walks through it. Only + // its end can say the name is not the kit's, which is why nothing + // here stops at the first hop that is not. + if (binding) continue; + if (ts.isVariableDeclaration(declaration) && declaration.initializer) { + return kitComponentOf(declaration.initializer, checker, surface, seen); + } + } + } + // `UI.Card` where `UI` is the kit itself: the kit is not in the program, so + // there is no symbol for the member and the namespace is the evidence. + if (ts.isPropertyAccessExpression(node)) { + const base = kitComponentOf(node.expression, checker, surface, seen); + if (base === KIT_NAMESPACE) return node.name.text; + if (base === SURFACE_NAMESPACE) return surface.get(node.name.text); + } + return undefined; +} + +/** + * What a component renders, as the alternative pictures it can draw: one set of + * kit marks per branch the renderer can take. + * + * Expression forms, and nothing beyond them. A conditional is two + * alternatives wherever it sits — nested inside returned JSX included, which + * is where a dispatch is usually written. `&&` adds the empty alternative, + * because the mark may be absent. `||` and `??` are their two sides. A cast, + * a parenthesis and a non-null assertion are their inner expression. A comma + * expression is its RIGHT operand alone: the others are evaluated and thrown + * away. Children of an element render together, so they multiply out. + * + * Nothing else is descended into, which is how two things end up not counting + * as rendered — both limits rather than oversights. A function body is not what + * the expression holding it draws, so a render prop and a callback in children + * are values, not pictures. And JSX handed to another component through an + * attribute is a value passed too: what the recipient does with it is beyond + * anything this can see, so a kit element in a prop is no evidence that this + * component renders it. Children are not attributes and do count. + */ +function alternatives( + node: ts.Node, + checker: ts.TypeChecker, + surface: Map, + seen = new Set(), +): Set[] { + const NOTHING = [new Set()]; + if (seen.has(node)) return NOTHING; + seen.add(node); + const of = (next: ts.Node) => alternatives(next, checker, surface, seen); + + if ( + ts.isParenthesizedExpression(node) || + ts.isAsExpression(node) || + ts.isSatisfiesExpression(node) || + ts.isTypeAssertionExpression(node) || + ts.isNonNullExpression(node) + ) { + return of(node.expression); + } + if (ts.isJsxExpression(node)) { + return node.expression ? of(node.expression) : NOTHING; + } + if (ts.isConditionalExpression(node)) { + return distinct([...of(node.whenTrue), ...of(node.whenFalse)]); + } + if (ts.isBinaryExpression(node)) { + switch (node.operatorToken.kind) { + case ts.SyntaxKind.CommaToken: + return of(node.right); + case ts.SyntaxKind.AmpersandAmpersandToken: + return distinct([new Set(), ...of(node.right)]); + case ts.SyntaxKind.BarBarToken: + case ts.SyntaxKind.QuestionQuestionToken: + return distinct([...of(node.left), ...of(node.right)]); + default: + return NOTHING; + } + } + if (ts.isJsxElement(node)) { + return together([ + [markOf(node.openingElement.tagName, checker, surface)], + ...node.children.map(of), + ]); + } + if (ts.isJsxSelfClosingElement(node)) { + return [markOf(node.tagName, checker, surface)]; + } + if (ts.isJsxFragment(node)) { + return together(node.children.map(of)); + } + if (ts.isArrayLiteralExpression(node)) { + return together(node.elements.map(of)); + } + // A local name holding a picture: `const content = ; return content`. + if (ts.isIdentifier(node)) { + const symbol = checker.getSymbolAtLocation(node); + for (const hop of symbol ? aliasChain(symbol, checker) : []) { + const declaration = hop.declarations?.[0]; + if ( + declaration && + ts.isVariableDeclaration(declaration) && + declaration.initializer + ) { + return of(declaration.initializer); + } + } + } + return NOTHING; +} + +/** Whether a node is a function, whose body is not this component's picture. */ +function isFunctionLike(node: ts.Node): boolean { + return ( + ts.isFunctionDeclaration(node) || + ts.isFunctionExpression(node) || + ts.isArrowFunction(node) || + ts.isMethodDeclaration(node) + ); +} + +/** The kit mark a JSX tag draws, as a set of none or one. */ +function markOf( + tag: ts.JsxTagNameExpression, + checker: ts.TypeChecker, + surface: Map, +): Set { + const kit = kitComponentOf(tag, checker, surface); + return kit && !kit.startsWith("*") ? new Set([kit]) : new Set(); +} + +/** One alternative per distinct set of marks. */ +function distinct(paths: Set[]): Set[] { + const byShape = new Map>(); + for (const path of paths) { + byShape.set([...path].sort().join("+"), path); + } + return [...byShape.values()]; +} + +/** + * Parts that all render: every combination of one alternative from each. Kept + * distinct at each step, so the count is bounded by the mark sets that can + * actually occur rather than by the size of the tree. + */ +function together(parts: Set[][]): Set[] { + let combined: Set[] = [new Set()]; + for (const part of parts) { + const next: Set[] = []; + for (const left of combined) { + for (const right of part) { + next.push(new Set([...left, ...right])); + } + } + combined = distinct(next); + } + return combined; +} + +/** + * The alternatives a declaration's body returns. + * + * Every `return` in the body, plus a concise arrow body, which is one. A + * statement after a `return` or a `throw` in the same block is unreachable and + * is not one: a picture no execution reaches is not drawn. + */ +function returnedAlternatives( + declaration: ts.Node, + checker: ts.TypeChecker, + surface: Map, +): Set[] { + const body = functionBodyOf(declaration); + if (!body) return []; + if (!ts.isBlock(body)) { + return alternatives(body, checker, surface); + } + const paths: Set[] = []; + const visit = (node: ts.Node) => { + if (isFunctionLike(node) && node !== body) return; + if (ts.isBlock(node) || ts.isCaseClause(node) || ts.isDefaultClause(node)) { + for (const statement of node.statements) { + visit(statement); + if (ts.isReturnStatement(statement) || ts.isThrowStatement(statement)) { + return; + } + } + return; + } + if (ts.isReturnStatement(node)) { + if (node.expression) { + paths.push(...alternatives(node.expression, checker, surface)); + } + return; + } + ts.forEachChild(node, visit); + }; + visit(body); + return paths; +} + +/** Every kit component this declaration returns, on any alternative. */ +function returnedKitComponents( + declaration: ts.Node, + checker: ts.TypeChecker, + surface: Map, +): Set { + return new Set( + returnedAlternatives(declaration, checker, surface).flatMap((path) => [ + ...path, + ]), + ); +} + +/** The body of the function a declaration is, if it is one. */ +function functionBodyOf(declaration: ts.Node): ts.Node | undefined { + if (ts.isFunctionDeclaration(declaration)) return declaration.body; + if ( + ts.isVariableDeclaration(declaration) && + declaration.initializer && + isFunctionLike(declaration.initializer) + ) { + return (declaration.initializer as ts.ArrowFunction).body; + } + if (isFunctionLike(declaration)) { + return (declaration as ts.ArrowFunction).body; + } + return undefined; +} + +/** + * What a published name resolves to. + * + * `declaration` is code in this set, to be examined. `kit` is the kit's own + * component, however renamed — a re-export ships the kit's component rather + * than a second one. `unresolved` is everything else, and it is an OFFENCE + * rather than a skip: a name the kit owns, published here from somewhere this + * cannot read, is a second implementation of it as surely as a hand-written + * copy, and "could not tell" was the hole that four rounds of fixtures each + * found the next way through. + */ +type Resolution = + | { kind: "declaration"; node: ts.Node } + | { kind: "kit"; name: string } + | { kind: "type" } + | { kind: "unresolved"; why: string }; + +/** + * Every component a file publishes, by published name — following an export + * chain of any length into whichever file actually declares it, so splitting a + * copy from its kit-named export hides nothing and a genuine wrapper + * re-exported under the kit's name is not mistaken for one. Declarations the + * file does not export are still declarations here: a duplicate hidden as a + * file-local helper is the commonest way one arrives. + */ +function publishedComponents( + file: ts.SourceFile, + checker: ts.TypeChecker, + surface: Map, +): Map { + const published = new Map(); + const resolve = (symbol: ts.Symbol): Resolution => { + const chain = aliasChain(symbol, checker); + for (const [hop, resolved] of chain.entries()) { + const declarations = resolved.declarations ?? []; + const value = declarations.find( + (declaration) => + ts.isFunctionDeclaration(declaration) || + ts.isClassDeclaration(declaration) || + ts.isVariableDeclaration(declaration), + ); + if (value) return { kind: "declaration", node: value }; + if ( + declarations.length > 0 && + declarations.every( + (declaration) => + ts.isTypeAliasDeclaration(declaration) || + ts.isInterfaceDeclaration(declaration), + ) + ) { + return { kind: "type" }; + } + const binding = declarations[0] + ? kitBindingOf(declarations[0], surface) + : undefined; + if (binding?.kind === "kit" || binding?.kind === "namespace") { + return { + kind: "kit", + name: binding.kind === "kit" ? binding.name : binding.marker, + }; + } + if (binding?.kind === "type") return { kind: "type" }; + // An `external` hop in the middle is another of the app's own + // modules, which the chain walks through. At the END of the chain it + // is a module this cannot read, and a kit name coming out of one is + // a second implementation of it as much as a hand-written copy. + if (hop === chain.length - 1) { + return binding?.kind === "external" + ? { kind: "unresolved", why: `published from ${binding.from}` } + : { kind: "unresolved", why: "the chain reaches no declaration" }; + } + } + return { kind: "unresolved", why: "the chain reaches no declaration" }; + }; + + const moduleSymbol = checker.getSymbolAtLocation(file); + if (moduleSymbol) { + for (const exported of checker.getExportsOfModule(moduleSymbol)) { + if (!/^[A-Z][A-Za-z0-9]*$/.test(exported.name)) continue; + published.set(exported.name, resolve(exported)); + } + } + const visit = (node: ts.Node) => { + const name = + (ts.isFunctionDeclaration(node) || ts.isClassDeclaration(node)) && + node.name + ? node.name.text + : ts.isVariableDeclaration(node) && ts.isIdentifier(node.name) + ? node.name.text + : undefined; + if (name && /^[A-Z][A-Za-z0-9]*$/.test(name) && !published.has(name)) { + published.set(name, { kind: "declaration", node }); + } + ts.forEachChild(node, visit); + }; + visit(file); + return published; +} + +/** + * The kit's dashboard marks. A surface may compose several of these on one + * page — a KPI row above a chart is a page, not a renderer. What it may not do + * is BRANCH between them: choosing a mark by a widget's kind is a dispatch over + * visualizations, and that is ``'s whole job. + * + * Branching is the test, not the count, because counting file-wide imports + * caught a page that composed two marks side by side and could be escaped by a + * namespace import or a local alias. This is also the only leg that catches a + * duplicate under a NEW name: the renderer #1041 deleted declared + * `WidgetBody`, `WidgetValues` and `WidgetCard` — not one of them a name the + * kit exports — so every name-based leg passed it while it was the duplication + * itself. + */ +const DASHBOARD_MARKS = new Set([ + "LineChart", + "AreaChart", + "BarList", + "StatChart", + "TrendLineChart", + "MetricLineChart", + "MetricAreaChart", + "MetricBarChart", + "StatTile", + "MetricCard", + "KPIRow", + "Sparkline", +]); + +/** + * Whether this declaration CHOOSES between dashboard marks: two alternatives + * whose marks are each non-empty and neither contained in the other. + * + * Containment is what separates the two shapes that look alike. A page that + * always draws a chart and sometimes adds a KPI row has `{chart}` and + * `{row, chart}` — one is the other plus an optional extra, and it is one + * picture with a variation. A renderer that draws a line for one kind and an + * area for another has `{line}` and `{area}` — neither is the other, and that + * is the dispatch over visualizations `` owns. + * + * Choosing is the test, not branching and not counting: counting file-wide + * imports flagged a page that composed two marks side by side, and comparing + * only top-level returns walked past the same dispatch written one line deeper, + * inside the JSX it returns. + */ +function dispatchedMarks( + declaration: ts.Node, + checker: ts.TypeChecker, + surface: Map, +): string[] { + const paths = distinct( + returnedAlternatives(declaration, checker, surface).map( + (path) => new Set([...path].filter((mark) => DASHBOARD_MARKS.has(mark))), + ), + ).filter((path) => path.size > 0); + const within = (small: Set, large: Set) => + [...small].every((mark) => large.has(mark)); + for (const left of paths) { + for (const right of paths) { + if (left === right) continue; + if (!within(left, right) && !within(right, left)) { + return [...new Set([...left, ...right])].sort(); + } + } + } + return []; +} + +const DISPATCHERS: Record = { + "src/components/dashboard.tsx": + "the widget-slot renderer: a union of host-supplied widgets, each with its own loading/error/empty state. Goes when the audit page's widgets become a DashboardView and it mounts the kit's renderer.", + "src/features/dashboard/ui/metric-card.tsx": + "the dashboard DSL's card: it resolves one declared metric through useMetric and then draws whichever chart the declaration asked for. Goes with the DSL resolving outside the card.", +}; + +function appSources(): { path: string; source: string }[] { + return walk(join(appRoot, "src")) + .map((file) => relative(appRoot, file).replaceAll("\\", "/")) + .filter( + (path) => + !path.startsWith("src/gen/") && !path.startsWith("src/generated/"), + ) + .map((path) => ({ + path, + source: readFileSync(join(appRoot, path), "utf8"), + })); +} + +function kitEntryPoints(): { path: string; source: string }[] { + return walk(join(appRoot, "packages/codefly-ui/src")) + .filter((file) => /[/\\]index\.ts$/.test(file)) + .map((file) => ({ + path: relative(appRoot, file).replaceAll("\\", "/"), + source: readFileSync(file, "utf8"), + })); +} + +function kitNames(): Set { + return kitExports(kitEntryPoints().map((entry) => entry.source)); +} + +/** Each host surface's published name → the kit name behind it. */ +function hostSurface(): Map { + const surface = new Map(); + for (const path of KIT_SURFACES) { + const file = parse(path, readFileSync(join(appRoot, path), "utf8")); + for (const statement of file.statements) { + if ( + !ts.isExportDeclaration(statement) && + !ts.isImportDeclaration(statement) + ) { + continue; + } + const specifier = statement.moduleSpecifier; + if (!specifier || !ts.isStringLiteral(specifier)) continue; + if (!KIT_SPECIFIER.test(specifier.text)) continue; + const clause = ts.isExportDeclaration(statement) + ? statement.exportClause + : statement.importClause?.namedBindings; + if (!clause) continue; + if (ts.isNamedExports(clause) || ts.isNamedImports(clause)) { + for (const element of clause.elements) { + surface.set( + element.name.text, + (element.propertyName ?? element.name).text, + ); + } + } + } + } + return surface; +} + +function parse(path: string, source: string): ts.SourceFile { + return ts.createSourceFile( + path, + source, + ts.ScriptTarget.Latest, + true, + ts.ScriptKind.TSX, + ); +} + +interface Offence { + name: string; + path: string; + /** + * Why it is an offence: a name the kit owns, a widget dispatch, or a name + * the kit owns published from somewhere this cannot read. + */ + kind: "name" | "dispatch" | "unresolved"; +} + +/** + * Files whose default export's NAME is fixed by the framework, not chosen: + * Next's route conventions make every route component a `Page`, every nested + * shell a `Layout`, and so on, and the kit exports primitives called `Page` and + * `Layout`. A route component is not a second copy of either — it is the thing + * that mounts them — so its default export is not read as a declaration taking + * the kit's name. A NAMED declaration in such a file still is. + * + * The root route (`src/app/page.tsx`) counts: it is as conventional as a nested + * one, and requiring a directory between `app/` and `page.tsx` rejected it. + */ +const ROUTE_CONVENTION = + /^src\/app\/(?:.*\/)?(page|layout|template|default|error|global-error|loading|not-found|forbidden|unauthorized)\.tsx?$/; + +/** What a file's default export resolves to, in every spelling of it. */ +function defaultExportNames( + file: ts.SourceFile, + checker: ts.TypeChecker, +): Set { + const names = new Set(); + const moduleSymbol = checker.getSymbolAtLocation(file); + const exported = moduleSymbol + ? checker + .getExportsOfModule(moduleSymbol) + .find((symbol) => symbol.name === "default") + : undefined; + for (const symbol of exported ? aliasChain(exported, checker) : []) { + for (const declaration of symbol.declarations ?? []) { + if ( + (ts.isFunctionDeclaration(declaration) || + ts.isClassDeclaration(declaration) || + ts.isVariableDeclaration(declaration)) && + declaration.name && + ts.isIdentifier(declaration.name) + ) { + names.add(declaration.name.text); + } + if ( + ts.isExportAssignment(declaration) && + ts.isIdentifier(declaration.expression) + ) { + names.add(declaration.expression.text); + } + } + } + return names; +} + +/** + * Declarations that take a kit export's name without RETURNING the kit's + * component of that name, names the kit owns that this cannot resolve at all, + * and declarations that choose between the kit's dashboard marks. Pure, so the + * fixtures below can exercise it on sources that are not in the tree. + */ +function findOffences( + files: { path: string; source: string }[], + kit: Set, + allowed: Record = ALLOWED, + /** Published name → kit name for the host's own surfaces. */ + surface: Map = new Map(), + dispatchers: Record = DISPATCHERS, +): Offence[] { + const { checker, sources } = programOver(files); + const offences: Offence[] = []; + + for (const [path, file] of sources) { + if (KIT_SURFACES.includes(path)) continue; + const routeDefaults = ROUTE_CONVENTION.test(path) + ? defaultExportNames(file, checker) + : new Set(); + const components = publishedComponents(file, checker, surface); + + for (const [name, resolution] of components) { + if (!kit.has(name) || routeDefaults.has(name)) continue; + if (resolution.kind === "kit" || resolution.kind === "type") continue; + // An exception is recorded where the component is DECLARED, so a + // barrel that re-exports it does not need a second one for the + // same component. The published path is checked too, for a name a + // file publishes without declaring anywhere in the set. + const declaredIn = + resolution.kind === "declaration" + ? resolution.node.getSourceFile().fileName + : path; + if (`${name} in ${path}` in allowed) continue; + if (`${name} in ${declaredIn}` in allowed) continue; + if (resolution.kind === "unresolved") { + offences.push({ name, path, kind: "unresolved" }); + continue; + } + if (returnedKitComponents(resolution.node, checker, surface).has(name)) { + continue; + } + offences.push({ name, path, kind: "name" }); + } + + if (path in dispatchers) continue; + for (const [, resolution] of components) { + if (resolution.kind !== "declaration") continue; + const marks = dispatchedMarks(resolution.node, checker, surface); + if (marks.length > 1) { + offences.push({ name: marks.join("+"), path, kind: "dispatch" }); + break; + } + } + } + return offences; +} + +describe("the host app composes the kit rather than re-implementing it", () => { + it("declares no component the kit already exports", () => { + const kit = kitNames(); + expect( + kit.size, + "the kit's entry points were not readable, so this gate proves nothing", + ).toBeGreaterThan(100); + + const offences = findOffences( + appSources(), + kit, + ALLOWED, + hostSurface(), + ).map(({ name, path, kind }) => { + if (kind === "name") { + return `${name} in ${path} — @codefly-dev/ui already exports it. Compose the kit's component, or add "${name} in ${path}" to ALLOWED with what the two things are and why neither can stand in for the other.`; + } + if (kind === "unresolved") { + return `${name} in ${path} — @codefly-dev/ui exports that name and this publishes it from somewhere this gate cannot read, so it cannot tell the kit's component from a second one. Publish the kit's own, or declare it here where it can be read.`; + } + return `${path} draws with ${name} — more than one of the kit's dashboard marks, which is a widget dispatch and 's job. Mount or , or add ${path} to DISPATCHERS with what it is and what would retire it.`; + }); + expect(offences).toEqual([]); + }); + + it("reads every name the kit exports, star re-exports included", () => { + // The gate covers what it can parse. A star onto a package outside the + // kit's own tree is not parsed, so it is refused rather than missed. + expect(starReExports(kitEntryPoints())).toEqual([ + "packages/codefly-ui/src/plugin-host/index.ts → @codefly-dev/saas-plugin-react", + ]); + }); + + it("cannot build a kit-shaped primitive: the library for building them is not a dependency", () => { + // The strongest leg, because it is a fact rather than a convention: `cva` + // is how the kit composes its variants, and it left this package's + // dependencies with issue #1041. The kit declares it for itself. + expect(Object.keys(packageJson.dependencies)).not.toContain( + "class-variance-authority", + ); + const importers = appSources() + .filter(({ source }) => /["']class-variance-authority["']/.test(source)) + .map(({ path }) => path); + expect(importers).toEqual([]); + }); + + it.each(CHART_RE_EXPORTS)( + "%s stays a re-export of the kit's charts", + (path) => { + const source = readFileSync(join(appRoot, path), "utf8"); + expect(source).toContain('from "@codefly-dev/ui/dashboard"'); + // No component of its own: no JSX, no class strings, no variants. + expect(source).not.toMatch(/\bfunction\b|=>|cva\(|cn\(/); + }, + ); + + it("keeps every entry in ALLOWED a real one", () => { + const kit = kitNames(); + for (const [key, reason] of Object.entries(ALLOWED)) { + const [name, path] = key.split(" in "); + expect( + kit.has(name), + `${key}: ${name} is not a kit export any more, so this entry is stale — delete it`, + ).toBe(true); + expect( + new RegExp( + `^(?:export\\s+)?(?:function|const|class)\\s+${name}\\b`, + "m", + ).test(readFileSync(join(appRoot, path), "utf8")), + `${key} is not declared there any more, so this entry is stale — delete it`, + ).toBe(true); + expect( + reason.length, + `${key} needs what the two things are and why, not a listing`, + ).toBeGreaterThan(80); + } + }); +}); + +// Three directions, because a gate is only as good as what it is known to +// catch, to let through, and to catch when the same duplication arrives by +// another route. Each fixture is a source that is not in the tree, so these +// hold whatever the tree happens to contain. +describe("the detector", () => { + const kit = new Set(["Card", "CardRoot", "StatTile", "Page", "Badge"]); + // What the host's surfaces publish, as `src/shared/ui` really does it: one + // name straight through, one renaming a kit primitive to another kit + // component's name. + const surface = new Map([ + ["Badge", "Badge"], + ["Card", "CardRoot"], + ]); + const deletedRenderer = () => + readFileSync( + join( + appRoot, + "src/components/__tests__/fixtures/deleted-solution-dashboard.txt", + ), + "utf8", + ); + const detect = (path: string, source: string) => + findOffences([{ path, source }], kit, {}, surface, {}).map( + (o) => `${o.name} in ${o.path}`, + ); + + it("catches a component taking a kit export's name", () => { + expect( + detect( + "src/features/x/ui/card.tsx", + `export function Card({ children }: { children: ReactNode }) { + return
    {children}
    ; + }`, + ), + ).toEqual(["Card in src/features/x/ui/card.tsx"]); + }); + + it("lets a wrapper that renders the kit's own component through", () => { + expect( + detect( + "src/features/x/ui/card.tsx", + `import { Card as KitCard } from "@codefly-dev/ui/layout"; + export function Card(props: ComponentProps) { + return ; + }`, + ), + ).toEqual([]); + }); + + it("catches the same duplication declared module-locally instead of exported", () => { + // How both duplicates in this tree arrived: not as a new file anyone + // would review as a component, but as a helper at the bottom of a page. + // The upstream version of this rule matched only `export function`, and + // let this shape past. + expect( + detect( + "src/features/x/ui/page.tsx", + `function Card({ title }: { title: string }) { + return
    {title}
    ; + } + export function Deliveries() { return ; }`, + ), + ).toEqual(["Card in src/features/x/ui/page.tsx"]); + }); + + it("catches an arrow-function component, which is the same declaration", () => { + expect( + detect( + "src/features/x/ui/tile.tsx", + `const StatTile = ({ value }: { value: number }) => {value};`, + ), + ).toEqual(["StatTile in src/features/x/ui/tile.tsx"]); + }); + + it("catches a declaration that imports a DIFFERENT kit component under the name it takes", () => { + // Importing `CardRoot` and calling the result `Card` is not a wrapper + // around `Card`: it is a second `Card` built on a kit primitive. This is + // how a duplicate escapes a plain name gate — the kit and the consumer + // call the same thing by different names, so neither name collides. + expect( + detect( + "src/features/x/ui/card.tsx", + `import { CardRoot } from "@codefly-dev/ui/layout"; + export function Card({ title }: { title: string }) { + return

    {title}

    ; + }`, + ), + ).toEqual(["Card in src/features/x/ui/card.tsx"]); + }); + + it("is not fooled by an alias: a wrapper is recognised by the kit's own spelling", () => { + expect( + detect( + "src/features/x/ui/card.tsx", + `import { StatTile as Tile } from "@codefly-dev/ui/dashboard"; + export const StatTile = (props: ComponentProps) => ;`, + ), + ).toEqual([]); + }); + + it("ignores a re-export, which ships the kit's component rather than another one", () => { + expect( + detect( + "src/components/charts/chart.tsx", + `export { StatTile, Card } from "@codefly-dev/ui/dashboard";`, + ), + ).toEqual([]); + }); + + it("lets a Next route component keep the name the framework gives it", () => { + expect( + detect( + "src/app/admin/users/page.tsx", + `export default function Page() { return ; }`, + ), + ).toEqual([]); + }); + + it("still catches a NAMED kit-shaped component inside a route file", () => { + // The exemption is for the one export whose name Next fixes, not for the + // file: a second `Card` beside the route component is still a second Card. + expect( + detect( + "src/app/admin/users/page.tsx", + `function Card({ title }: { title: string }) { return
    {title}
    ; } + export default function Page() { return ; }`, + ), + ).toEqual(["Card in src/app/admin/users/page.tsx"]); + }); + + it("lets a wrapper compose the kit through the host's own import surface", () => { + // The path a wrapper imports through must not change the verdict. This + // is the same component, and the same wrapper, as the kit-direct case + // above; `@/shared/ui` publishes `Badge` straight from the kit. + expect( + detect( + "src/features/x/ui/badge.tsx", + `import { Badge as KitBadge } from "@/shared/ui"; + export function Badge(props: ComponentProps) { + return ; + }`, + ), + ).toEqual([]); + }); + + it("resolves the surface's alias, so one import is judged one way either way", () => { + // `@/shared/ui` publishes the kit's `CardRoot` under the name `Card`, so + // importing `Card` from there composes `CardRoot` — and a declaration + // named `Card` is a second `Card`, exactly as the kit-direct form of + // this is caught above. Without resolving the alias the two spellings + // of one import would get opposite verdicts. + expect( + detect( + "src/features/x/ui/card.tsx", + `import { Card as HostCard } from "@/shared/ui"; + export function Card({ title }: { title: string }) { + return {title}; + }`, + ), + ).toEqual(["Card in src/features/x/ui/card.tsx"]); + }); + + it("does not read the host's surfaces themselves as duplicates", () => { + // A surface file re-exports under a kit name by construction. + expect( + detect( + "src/shared/ui/index.ts", + `export { CardRoot as Card, Badge } from "@codefly-dev/ui/layout";`, + ), + ).toEqual([]); + }); + + it("catches a same-name copy sitting beside an unrelated kit re-export", () => { + // The shape a duplicate actually arrives in: somebody re-exports the + // kit's component for a neighbour and writes their own beside it. A + // file-wide import check read that re-export as proof of composition + // and exempted the copy. + expect( + detect( + "src/features/x/ui/card.tsx", + `export { Card as KitCard } from "@codefly-dev/ui/layout"; +export const Card = () =>
    copy
    ;`, + ), + ).toEqual(["Card in src/features/x/ui/card.tsx"]); + }); + + it("catches an implementation published under a kit name by an export alias", () => { + // The declared name is not the shipped name. Reading only declarations + // let this straight through. + expect( + detect( + "src/features/x/ui/panel.tsx", + `const LocalPanel = () =>
    copy
    ; +export { LocalPanel as Card };`, + ), + ).toEqual(["Card in src/features/x/ui/panel.tsx"]); + }); + + it("allows a wrapper that reaches the kit through a namespace import", () => { + expect( + detect( + "src/features/x/ui/badge.tsx", + `import * as UI from "@/shared/ui"; +export const Badge = (props) => ;`, + ), + ).toEqual([]); + }); + + it("allows a wrapper that reaches a host surface by relative path", () => { + // `@/shared/ui` and `../../../shared/ui` are the same module; judging + // only the alias rejected a real wrapper for how it spelled its import. + expect( + detect( + "src/features/x/ui/badge.tsx", + `import { Badge as KitBadge } from "../../../shared/ui"; +export const Badge = (props) => ;`, + ), + ).toEqual([]); + }); + + it("applies the Next route exception to the ROOT route, not only nested ones", () => { + expect( + detect( + "src/app/page.tsx", + "export default function Page() { return
    ; }", + ), + ).toEqual([]); + }); + + it("rejects the exact renderer this gate was written to prevent", () => { + // The case that shows the name legs are not enough on their own: the + // renderer #1041 deleted declared WidgetBody, WidgetValues and + // WidgetCard — not one of them a name the kit exports — so every + // name-based leg passed it while it was the duplication itself. The + // dispatch leg is what catches it. + // + // Read from a committed fixture, not with `git show`: this suite ships + // inside an immutable module package, where a consuming repository + // carries the source tree and not this repository's history, so a + // fixture reaching for an upstream commit fails there while passing in + // full-history CI. + expect( + detect("src/solutions/SolutionDashboard.tsx", deletedRenderer()).length, + ).toBeGreaterThan(0); + }); + + it("still rejects that renderer when its imports become a namespace", () => { + // Changing only how the marks are imported, and qualifying the JSX, + // must not change the verdict: a gate that reads local names rather + // than what they resolve to is escaped by one edit. + const named = + /import \{\s*AreaChart,\s*BarList,\s*LineChart,\s*SortableGrid,\s*StatChart,\s*\} from "@codefly-dev\/ui\/dashboard";/; + const original = deletedRenderer(); + expect(named.test(original)).toBe(true); + const namespaced = original + .replace(named, 'import * as Charts from "@codefly-dev/ui/dashboard";') + .replace( + /<(\/?)(AreaChart|BarList|LineChart|SortableGrid|StatChart)\b/g, + "<$1Charts.$2", + ); + expect( + detect("src/solutions/SolutionDashboard.tsx", namespaced).length, + ).toBeGreaterThan(0); + }); + + it("rejects that renderer when its marks are local aliases", () => { + expect( + detect( + "src/features/x/ui/widget.tsx", + `import { LineChart as L, AreaChart as A } from "@codefly-dev/ui/dashboard"; +export function WidgetBody({ kind }) { + return kind === "line" ? : ; +}`, + ).length, + ).toBeGreaterThan(0); + }); + + it("does not read a type reference as composition", () => { + // A parameter typed ComponentProps mentions the kit and + // draws nothing; reading plain identifiers exempted a Card whose body + // was a bare
    . + expect( + detect( + "src/features/x/ui/card.tsx", + `import { Card as KitCard } from "@codefly-dev/ui/layout"; +export const Card = (props: React.ComponentProps) => ( +
    {props.children}
    +);`, + ), + ).toEqual(["Card in src/features/x/ui/card.tsx"]); + }); + + it("does not let a local shadow stand in for the kit's component", () => { + expect( + detect( + "src/features/x/ui/card.tsx", + `import { Card as KitCard } from "@codefly-dev/ui/layout"; +export function Card() { + const KitCard = () =>
    ; + return ; +}`, + ), + ).toEqual(["Card in src/features/x/ui/card.tsx"]); + }); + + it("resolves a surface namespace member to the kit name behind it", () => { + // `@/shared/ui` publishes CardRoot as Card, so composes + // CardRoot and a declaration named Card is still a second Card. + expect( + detect( + "src/features/x/ui/card.tsx", + `import * as UI from "@/shared/ui"; +export const Card = () =>

    copy

    ;`, + ), + ).toEqual(["Card in src/features/x/ui/card.tsx"]); + }); + + it("follows a local alias of a kit import", () => { + expect( + detect( + "src/features/x/ui/card.tsx", + `import { Card as KitCard } from "@codefly-dev/ui/layout"; +const Local = KitCard; +export const Card = (props) => ;`, + ), + ).toEqual([]); + }); + + it("allows a wrapper that renders through a kit namespace", () => { + expect( + detect( + "src/features/x/ui/badge.tsx", + `import * as UI from "@codefly-dev/ui/layout"; +export default function Badge(props) { return ; }`, + ), + ).toEqual([]); + }); + + it.each([ + [ + "a default identifier", + "const Page = () =>
    ; export default Page;", + ], + [ + "an aliased default", + "const Page = () =>
    ; export { Page as default };", + ], + ])("applies the route exception to %s", (_name, source) => { + expect(detect("src/app/page.tsx", source)).toEqual([]); + }); + + it("does not read two marks composed side by side as a dispatch", () => { + // A KPI row above a chart is a page. What a page may not do is BRANCH + // between marks, which is the dispatch owns. + expect( + detect( + "src/features/x/ui/overview.tsx", + `import { KPIRow, MetricLineChart } from "@codefly-dev/ui/dashboard"; +export const Overview = () => ( + <> + + + +);`, + ), + ).toEqual([]); + }); + + it("catches a copy split across two files by an aliasing re-export", () => { + expect( + findOffences( + [ + { + path: "src/features/x/ui/local.tsx", + source: "export const LocalPanel = () =>
    copy
    ;", + }, + { + path: "src/features/x/ui/index.ts", + source: 'export { LocalPanel as Card } from "./local";', + }, + ], + kit, + {}, + surface, + {}, + ).length, + ).toBeGreaterThan(0); + }); + + it("lets a page draw ONE kit mark, which is composition and not a dispatch", () => { + expect( + detect( + "src/features/x/ui/page.tsx", + `import { MetricLineChart } from "@codefly-dev/ui/dashboard"; +export const Trend = () => ;`, + ), + ).toEqual([]); + }); + + it.each([ + [ + "a guard-return dispatch", + `import { LineChart as L, AreaChart as A } from "@codefly-dev/ui/dashboard"; +export function WidgetBody({ kind }) { + if (kind === "line") return ; + return     ; +}`, + true, + ], + [ + "a destructured parameter shadowing the import", + `import { Card as KitCard } from "@codefly-dev/ui/layout"; +export function Card({ KitCard }) { return ; }`, + true, + ], + [ + "an unused nested wrapper beside a plain return", + `import { Card as KitCard } from "@codefly-dev/ui/layout"; +export function Card() { + const Unused = () => ; + return
    copy
    ; +}`, + true, + ], + [ + "a function-local alias of the kit component", + `import { Card as KitCard } from "@codefly-dev/ui/layout"; +export function Card(props) { + const Local = KitCard; + return ; +}`, + false, + ], + [ + "an alias of a namespace member", + `import * as UI from "@codefly-dev/ui/layout"; +const Local = UI.Card; +export const Card = (props) => ;`, + false, + ], + [ + "a nested shadow that does not reach the outer JSX", + `import { Card as KitCard } from "@codefly-dev/ui/layout"; +export function Card(props) { + const helper = (KitCard) => KitCard; + return ; +}`, + false, + ], + [ + "two layout branches drawing the same pair of marks", + `import { KPIRow, MetricLineChart } from "@codefly-dev/ui/dashboard"; +export function Overview({ compact }) { + return compact ? ( +
    + ) : ( +
    + ); +}`, + false, + ], + ])("%s", (_name, source, rejected) => { + expect(detect("src/features/x/ui/example.tsx", source).length > 0).toBe( + rejected, + ); + }); + + it("does not count kit JSX inside a callback the component passes down", () => { + // A render prop is not what this component returns, and the skip that + // keeps a nested function out of a returned expression is what says so. + expect( + detect( + "src/features/x/ui/card.tsx", + `import { Card as KitCard } from "@codefly-dev/ui/layout"; +export function Card() { + return } />; +}`, + ), + ).toEqual(["Card in src/features/x/ui/card.tsx"]); + }); + + it("accepts a genuine wrapper re-exported under the kit's name", () => { + // Its implementation composes the kit, which is the question — the name + // it is published under is not evidence either way. + expect( + findOffences( + [ + { + path: "src/features/x/ui/local.tsx", + source: `import { Card as KitCard } from "@codefly-dev/ui/layout"; +export const LocalPanel = (props) => ;`, + }, + { + path: "src/features/x/ui/index.ts", + source: 'export { LocalPanel as Card } from "./local";', + }, + ], + kit, + {}, + surface, + {}, + ), + ).toEqual([]); + }); + + it("catches a copy published under a kit name two hops away", () => { + expect( + findOffences( + [ + { + path: "src/features/x/ui/local.tsx", + source: "export const LocalPanel = () =>
    copy
    ;", + }, + { + path: "src/features/x/ui/bridge.ts", + source: 'export { LocalPanel } from "./local";', + }, + { + path: "src/features/x/ui/index.ts", + source: 'export { LocalPanel as Card } from "./bridge";', + }, + ], + kit, + {}, + surface, + {}, + ).length, + ).toBeGreaterThan(0); + }); + + // What the gate must see through, and what it must not mistake for a copy. + // Each of these was found by an independent review of an earlier shape of + // this detector, and each named a rule rather than a trick: these say what + // the rule now is. + it.each([ + [ + "a dispatch written one line inside the JSX it returns", + `import { LineChart as L, AreaChart as A } from "@codefly-dev/ui/dashboard"; +export function WidgetBody({ kind }) { + return
    {kind === "line" ? : }
    ; +}`, + true, + ], + [ + "the same dispatch behind a cast", + `import { LineChart as L, AreaChart as A } from "@codefly-dev/ui/dashboard"; +export function WidgetBody({ kind }) { + return (kind === "line" ? : ) as React.ReactNode; +}`, + true, + ], + [ + "kit JSX in a comma operand, which is evaluated and discarded", + `import { Card as KitCard } from "@codefly-dev/ui/layout"; +export function Card() { return (,
    copy
    ); }`, + true, + ], + [ + "kit JSX in a return no execution reaches", + `import { Card as KitCard } from "@codefly-dev/ui/layout"; +export function Card() { + return
    copy
    ; + return ; +}`, + true, + ], + [ + "kit JSX handed to another component through a prop", + `import { Card as KitCard } from "@codefly-dev/ui/layout"; +function Ignore(props) { return
    copy
    ; } +export function Card() { return } />; }`, + true, + ], + [ + "a dispatch whose arms are fragments", + `import { LineChart as L, AreaChart as A } from "@codefly-dev/ui/dashboard"; +export function WidgetBody({ kind }) { + return kind === "line" ? <> : <>    ; +}`, + true, + ], + [ + "kit JSX returned by a nested function the component only declares", + `import { Card as KitCard } from "@codefly-dev/ui/layout"; +export function Card() { + function Unused() { return ; } + return
    copy
    ; +}`, + true, + ], + [ + "kit JSX in a callback the component puts in its children", + `import { Card as KitCard } from "@codefly-dev/ui/layout"; +export function Card() { return {() => }; }`, + true, + ], + [ + "returned kit JSX in an array, which React renders", + `import { Card as KitCard } from "@codefly-dev/ui/layout"; +export function Card(props) { return []; }`, + false, + ], + [ + "returned kit JSX held in a local first", + `import { Card as KitCard } from "@codefly-dev/ui/layout"; +export function Card(props) { + const content = ; + return content; +}`, + false, + ], + [ + "a mark drawn always beside one drawn only sometimes", + `import { KPIRow, MetricLineChart } from "@codefly-dev/ui/dashboard"; +export function Overview({ withRow }) { + return ( +
    + {withRow ? : null} + +
    + ); +}`, + false, + ], + ])("%s", (_name, source, rejected) => { + expect(detect("src/features/x/ui/example.tsx", source).length > 0).toBe( + rejected, + ); + }); + + it("catches a copy re-exported through the app's own path alias", () => { + // `@/…` and `./…` reach the same module. Resolving only the second read + // the barrel and stopped, so the copy at the end of it went unexamined. + expect( + findOffences( + [ + { + path: "src/features/x/ui/local.tsx", + source: "export const LocalPanel = () =>
    copy
    ;", + }, + { + path: "src/features/x/ui/index.ts", + source: + 'export { LocalPanel as Card } from "@/features/x/ui/local";', + }, + ], + kit, + {}, + surface, + {}, + ).map(({ name, kind }) => `${kind}:${name}`), + // `name`, not `unresolved`: the copy was READ. An alias the gate + // cannot follow would also be refused, but for not knowing, and that + // is a weaker answer than the one available here. + ).toEqual(["name:Card"]); + }); + + it("catches a copy however many re-exports stand between", () => { + // A chain is followed to its end, cycle-detected rather than cut off at + // a depth: ten hops is not more suspicious than two, and a gate that + // gave up at eight could be escaped by writing a ninth. + const sources = [ + { + path: "src/features/x/ui/local.tsx", + source: "export const LocalPanel = () =>
    copy
    ;", + }, + ]; + for (let hop = 0; hop < 9; hop += 1) { + sources.push({ + path: `src/features/x/ui/bridge${hop}.ts`, + source: `export { LocalPanel } from "./${hop ? `bridge${hop - 1}` : "local"}";`, + }); + } + sources.push({ + path: "src/features/x/ui/index.ts", + source: 'export { LocalPanel as Card } from "./bridge8";', + }); + expect( + findOffences(sources, kit, {}, surface, {}).map( + ({ name, kind }) => `${kind}:${name}`, + ), + ).toEqual(["name:Card"]); + }); + + it.each([ + [ + "a named import", + 'export { Card as Panel } from "@codefly-dev/ui/layout";', + 'import { Panel } from "./bridge";\nexport const Card = (props) => ;', + ], + [ + "a namespace import", + 'export { Card } from "@codefly-dev/ui/layout";', + 'import * as UI from "./bridge";\nexport const Card = (props) => ;', + ], + ])( + "accepts a wrapper reaching the kit through the app's own barrel by %s", + (_name, bridge, local) => { + // The barrel is the app's, the component behind it is the kit's. A + // gate that stopped at the barrel rejected a genuine wrapper for how + // it spelled its import. + expect( + findOffences( + [ + { path: "src/features/x/ui/bridge.ts", source: bridge }, + { path: "src/features/x/ui/local.tsx", source: local }, + ], + kit, + {}, + surface, + {}, + ), + ).toEqual([]); + }, + ); + + it("refuses a kit name published from outside anything it can read", () => { + // Not a hand-written copy and not the kit either: a second Card from a + // third-party package is the same two-renderers problem, and "could not + // tell" is not a verdict this gate is allowed to reach. + expect( + findOffences( + [ + { + path: "src/features/x/ui/index.ts", + source: 'export { Panel as Card } from "some-other-ui-kit";', + }, + ], + kit, + {}, + surface, + {}, + ).length, + ).toBeGreaterThan(0); + }); + + it("does not refuse a TYPE that shares a kit name", () => { + // A type cannot be rendered and cannot stand in for a component, so it + // is neither a copy nor something unreadable. + expect( + findOffences( + [ + { + path: "src/features/x/model/types.ts", + source: "export interface Card { id: string }", + }, + { + path: "src/features/x/ui/index.ts", + source: 'export type { Card } from "../model/types";', + }, + ], + kit, + {}, + surface, + {}, + ), + ).toEqual([]); + }); + + it("ignores a name the kit does not export", () => { + expect( + detect( + "src/features/x/ui/panel.tsx", + `export function PayloadBlock({ title }: { title: string }) { return
    {title}
    ; }`, + ), + ).toEqual([]); + }); +}); diff --git a/module/services/frontend/code/src/features/webhooks/ui/webhook-deliveries-panel.tsx b/module/services/frontend/code/src/features/webhooks/ui/webhook-deliveries-panel.tsx index 972c11c46..dc57752f5 100644 --- a/module/services/frontend/code/src/features/webhooks/ui/webhook-deliveries-panel.tsx +++ b/module/services/frontend/code/src/features/webhooks/ui/webhook-deliveries-panel.tsx @@ -295,14 +295,14 @@ function DeliveryDetail({ {/* Request payload */} -
    {/* Response body */} -
    { expect(screen.getByText("No data")).toBeTruthy(); }); + it.each([ + ["StatTile", StatTile], + ["MetricCard", MetricCard], + ])( + "%s withholds everything derived from a forbidden value, not just the value", + (_name, Tile) => { + // "No access" beside "+170%" and a trend line publishes the figure + // twice over, in the two forms hardest to notice. The renderer + // builds a fresh metric and avoids this; a direct consumer of the + // exported primitive does not. + const { container } = render( + , + ); + expect(screen.getByText("No access")).toBeTruthy(); + expect(screen.getByText("—")).toBeTruthy(); + expect(screen.queryByText("37")).toBeNull(); + expect(screen.queryByText(/170%/)).toBeNull(); + expect(container.querySelector("svg")).toBeNull(); + }, + ); + + it("colours its trend from the chart token when asked, so an accent reaches it", () => { + // The sparkline strokes currentColor under a muted utility, so neither + // of a dashboard's accent overrides could reach it. + const { container } = render( + , + ); + const svg = container.querySelector("svg") as SVGElement; + expect(svg.style.color).toBe("var(--chart-1)"); + expect(svg.getAttribute("class")).not.toContain("text-muted-foreground"); + }); + it("shows a skeleton instead of a value while loading", () => { const { container } = render( , diff --git a/module/services/frontend/code/src/solutions/SolutionDashboard.tsx b/module/services/frontend/code/src/solutions/SolutionDashboard.tsx index 526feeade..330d45656 100644 --- a/module/services/frontend/code/src/solutions/SolutionDashboard.tsx +++ b/module/services/frontend/code/src/solutions/SolutionDashboard.tsx @@ -1,27 +1,30 @@ "use client"; import type { - Dashboard, + Dashboard as DashboardDeclaration, DataGraph, MetricWidget, } from "@codefly/saas-plugin-manifest"; +import { createSaasClient, runDashboard } from "@codefly-dev/saas-sdk"; +// The dashboard is DRAWN by the shared kit's `` — the same renderer, +// from the same package instance, that a solution's own remote mounts and that +// the host's operations pages are built from. Nothing here re-implements it: +// this file owns only what is genuinely the host's (the per-widget audit +// queries, the viewer's saved arrangement of the tiles, and the ⓘ that says +// where a number comes from) and composes it AROUND the renderer through the +// renderer's own slots. import { - createSaasClient, - type ResolvedWidget, - runDashboard, -} from "@codefly-dev/saas-sdk"; -// Charts come from the shared kit, not host-internal components: the same -// primitives a solution's own remote would render with, so host-rendered and -// solution-rendered dashboards look identical and there is one charting -// implementation to maintain. -import { - AreaChart, - BarList, - LineChart, + Dashboard, + DashboardWidget, + type DashboardWidgetView, SortableGrid, - StatChart, + type WidgetReadFailure, } from "@codefly-dev/ui/dashboard"; -import { useQuery } from "@tanstack/react-query"; +import { + type QueryClient, + useQuery, + useQueryClient, +} from "@tanstack/react-query"; import { GripVertical, Info, Plus, X } from "lucide-react"; import { type ReactNode, useState, useSyncExternalStore } from "react"; import { flushSync } from "react-dom"; @@ -33,12 +36,13 @@ import { import { scopedDashboardDraftKey } from "@/features/dashboard/service/use-dashboard-authoring"; import { useAuth } from "@/lib/auth"; import { apiTransport } from "@/lib/connect/transport"; +import { + mayKeepRetainedRows, + type ReadOutcome, + readOutcome, +} from "@/shared/lib/read-outcome"; import { Button, - Card, - CardContent, - CardHeader, - CardTitle, DropdownMenu, DropdownMenuContent, DropdownMenuItem, @@ -47,7 +51,6 @@ import { PopoverContent, PopoverTitle, PopoverTrigger, - Skeleton, } from "@/shared/ui"; import { addableTiles, @@ -99,144 +102,160 @@ function singleWidgetGraph(graph: DataGraph, widget: MetricWidget): DataGraph { }; } -// Render a resolved widget's series in the shape its visualization asks for. A -// series with no points is "no data yet" for every kind — the audit RPC omits a -// bucket rather than emitting a zero, so an empty series means nothing matched, -// not a real zero worth plotting. -function WidgetBody({ widget }: { widget: ResolvedWidget }) { - return ( - <> - {widget.series.coverage === "partial" && ( -

    - Partial telemetry -

    - )} - - - ); -} - -function WidgetValues({ widget }: { widget: ResolvedWidget }) { - const { series, visualization } = widget; - if (series.points.length === 0) { - return ( -

    - {series.coverage === "partial" - ? "Telemetry unavailable or incomplete." - : "No data yet."} -

    - ); - } - switch (visualization) { - case "line": - return ( - - ); - case "area": - return ( - - ); - case "bar": - return ; - case "number": - return series.total === null ? ( -

    - {series.coverage === "partial" - ? "Incomplete telemetry; total unavailable." - : "Total unavailable across groups."} -

    - ) : ( - - ); - case "table": - return ( - - - {series.points.map((point) => ( - - - - - ))} - -
    {point.key} - {point.value.toLocaleString()} -
    - ); - default: { - // Compile-time exhaustiveness: a new WidgetVisualization must be - // handled here or this assignment fails to type-check. - const _exhaustive: never = visualization; - return _exhaustive; - } - } -} - -function WidgetCard({ +// One declared widget, resolved and drawn. +// +// The query lives here, in a component of its own, because each widget is +// resolved in isolation: a widget whose metric's audit query errors fails +// alone instead of blanking every sibling that would share one batched +// resolution. React fixes the number of hooks a component may call, so these +// queries cannot be hoisted into the renderer that takes the whole view — +// which is why the kit exports `DashboardWidget` beside `` and this +// mounts that, rather than drawing a card of its own. +function SolutionWidget({ graph, widget, solutionId, dashboardId, orgId, - grip, - info, - remove, + actions, }: { graph: DataGraph; widget: MetricWidget; solutionId: string; dashboardId: string; orgId: string; - grip: ReactNode; - info: ReactNode; - remove: ReactNode; + actions: ReactNode; }) { // The graph is part of the key so a solution that redeploys with a changed // declaration refetches instead of serving another graph's cached series that // happens to share the same solution/dashboard/widget/org ids. Disabled until // the org resolves so the pre-org window reads as loading, never as empty. - const { data, isPending, isError } = useQuery({ - queryKey: [ - "solution-widget", - solutionId, - dashboardId, - widget.id, - orgId, - graph, - ], + const client = useQueryClient(); + const source = widgetSource({ solutionId, dashboardId, widget, orgId }); + const { data, isError, error } = useQuery({ + queryKey: [...source, graph], queryFn: () => runDashboard( sdk.audit, singleWidgetGraph(graph, widget), SINGLE_WIDGET_DASHBOARD, { orgId }, - ).then((resolved) => resolved.widgets[0]), + ) + .then((resolved) => resolved.widgets[0]) + .catch((cause: unknown) => { + // The read itself answers for what it leaves behind. A refused + // read deletes every snapshot of this widget's source before the + // rejection reaches anyone, so there is nothing retained for a + // later error to hand back and nothing to keep in step with it. + if (!mayKeepRetainedRows(readOutcome(true, cause))) { + discardSnapshots(client, source); + } + throw cause; + }), enabled: orgId !== "", }); + const outcome = readOutcome(isError, error); + // Whatever the cache still holds. A refusal has already emptied it, which is + // why this reads the snapshot without asking whether the viewer may keep it: + // the question was put at the read boundary, where the answer could be acted + // on once for every revision instead of per mounted component. + const retained = data?.series ?? null; + return ( - - - {grip} - - {widget.title ?? widget.metric} - - {info} - {remove} - - - {isError ? ( -

    Unable to load.

    - ) : isPending || !data ? ( - - ) : ( - - )} -     -     + ); } +/** + * Every cached revision of one widget's source — the query key without the + * declaration that produced it. + * + * A dashboard whose declaration has changed since keeps its own cache entry: a + * different title is enough for another key, holding the same numbers. So a + * rule about this widget's values is a rule about the whole prefix, never about + * the one entry a render happens to have resolved. + */ +function widgetSource({ + solutionId, + dashboardId, + widget, + orgId, +}: { + solutionId: string; + dashboardId: string; + widget: MetricWidget; + orgId: string; +}): readonly unknown[] { + return ["solution-widget", solutionId, dashboardId, widget.id, orgId]; +} + +/** + * Delete the retained snapshots under a widget's source: the values go, the + * queries and their states stay. + * + * This is what `mayKeepRetainedRows` says must happen to rows a refused read + * leaves behind, done where the refusal is classified rather than in a + * component. The difference is not tidiness. A component effect runs only for + * the revision that is mounted and only while it is: a refusal that arrives + * after the viewer switched dashboards, or after the page unmounted, left the + * numbers in the cache for the next render to serve. The read is the one thing + * that happens exactly once per refusal, whoever is watching. + * + * Writing the state is the whole of it, deliberately: + * + * - `removeQueries` destroys the entry, which silently cancels the very read + * that is reporting the refusal (`Query.destroy` → `cancel({silent:true})`) + * and leaves a mounted observer holding a detached query that no longer + * re-renders — the values stay on screen and recovery has nothing to + * refetch into. + * - `setQueryData(key, undefined)` cannot express this at all: the client + * returns early when the new data is `undefined`, so the call does nothing. + * - `resetQueries` clears the error with the data and refetches every active + * entry, so a standing refusal re-asks and re-clears without end. + * + * Setting `data` notifies the observers, cancels nothing, keeps the refusal, + * and starts no fetch. An entry left with no data renders as a wait and is + * stale, so the next read of it is a real read — which is the only way values + * come back. + */ +function discardSnapshots(client: QueryClient, source: readonly unknown[]) { + for (const query of client + .getQueryCache() + .findAll({ queryKey: source, exact: false })) { + if (query.state.data === undefined) continue; + query.setState({ data: undefined, dataUpdatedAt: 0 }); + } +} + +// The host's read classification as the renderer's presentational one. A read +// that succeeded is no failure at all, which is what `undefined` says. +const WIDGET_FAILURE: Record = { + empty: undefined, + forbidden: "refused", + failed: "failed", +}; + +// A declared widget as the renderer's view model, with no series yet. The +// series is bound by `SolutionWidget`, the one place that has resolved it; a +// `null` series is the honest value here and the renderer draws it as a wait +// rather than as an empty result. +function declaredWidget(widget: MetricWidget): DashboardWidgetView { + return { + id: widget.id, + visualization: widget.visualization, + title: widget.title ?? widget.metric, + series: null, + }; +} + // Reading `window.localStorage` can itself throw: blocked site data or a // sandboxed frame raise a SecurityError from the getter, before any getItem. No // storage means every viewer sees the declared default. @@ -273,7 +292,7 @@ function noSubscription() { */ function useDashboardLayout( storageKey: string, - dashboard: Dashboard, + dashboard: DashboardDeclaration, ): [string[], (next: readonly string[]) => void] { const stored = useSyncExternalStore( noSubscription, @@ -525,7 +544,7 @@ function SolutionDashboard({ solutionId, }: { graph: DataGraph; - dashboard: Dashboard; + dashboard: DashboardDeclaration; solutionId: string; }) { const { user, organizationId } = useAuth(); @@ -544,52 +563,45 @@ function SolutionDashboard({ const widget = widgets.get(tileId); return widget?.title ?? widget?.metric ?? tileId; }; - const renderTile = (tileId: string) => { - const widget = widgets.get(tileId); - if (!widget) return null; + // The grip, the ⓘ and the × — the three host-only controls — go into the + // renderer's own per-widget actions slot, so they sit in the card header the + // kit draws instead of a header this file draws. + const controls = (tileId: string) => { const title = titleOf(tileId); + const widget = widgets.get(tileId); return ( - { - const step = ARROW_STEP[event.key]; - if (step === undefined) return; - event.preventDefault(); - const grip = event.currentTarget; - // Moving a tile can move its DOM node, which drops focus; - // commit first so the grip can take focus back. - flushSync(() => save(moveBy(layout, tileId, step))); - grip.focus(); - }} - > - - - } - info={} - remove={ - - } - /> + <> + + {widget && } + + ); }; // The floating copy while a tile is dragged: the same card, but with no @@ -612,56 +624,82 @@ function SolutionDashboard({ ); return ( ); }; return ( -
    -
    - {dashboard.title && ( -

    - {dashboard.title} -

    - )} - save(addTile(layout, tileId))} - /> -
    - {shown.length === 0 ? ( -

    - No widgets are shown. Add one back with the + button. -

    - ) : ( - // The kit's SortableGrid rather than its Grid/Stack: the tiles are - // dragged onto each other to swap. The classes are those Grid - // cols={2} and Stack draw with. - tileId)} - onSwap={(dragged, target) => save(swapTiles(layout, dragged, target))} - itemLabel={titleOf} - renderItem={renderTile} - renderOverlay={renderDragged} - className={ - dashboard.layout === "stack" - ? "flex flex-col gap-4" - : "grid grid-cols-1 gap-4 sm:grid-cols-2" - } - /> - )} -
    + declaredWidget(widget)), + }} + slots={{ + actions: ( + save(addTile(layout, tileId))} + /> + ), + renderWidget: (view) => { + const widget = widgets.get(view.id); + if (!widget) return null; + return ( + + ); + }, + // The kit's SortableGrid rather than its default grid: the tiles + // are dragged onto each other to swap. The classes are those the + // kit's grid and stack draw with. + layout: (tiles) => { + const nodes = new Map(tiles.map((tile) => [tile.id, tile.node])); + return ( + tile.id)} + onSwap={(dragged, target) => + save(swapTiles(layout, dragged, target)) + } + itemLabel={titleOf} + renderItem={(id) => nodes.get(id) ?? null} + renderOverlay={renderDragged} + className={ + dashboard.layout === "stack" + ? "flex flex-col gap-4" + : "grid grid-cols-1 gap-4 sm:grid-cols-2" + } + /> + ); + }, + empty: ( +

    + No widgets are shown. Add one back with the + button. +

    + ), + }} + /> ); } diff --git a/module/services/frontend/code/src/solutions/__tests__/SolutionDashboard.test.tsx b/module/services/frontend/code/src/solutions/__tests__/SolutionDashboard.test.tsx index 8a64814bb..e1376b1f3 100644 --- a/module/services/frontend/code/src/solutions/__tests__/SolutionDashboard.test.tsx +++ b/module/services/frontend/code/src/solutions/__tests__/SolutionDashboard.test.tsx @@ -1,5 +1,12 @@ import type { DataGraph } from "@codefly/saas-plugin-manifest"; -import { cleanup, fireEvent, screen, within } from "@testing-library/react"; +import { Code, ConnectError } from "@connectrpc/connect"; +import { + act, + cleanup, + fireEvent, + screen, + within, +} from "@testing-library/react"; import { HttpResponse, http } from "msw"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { renderInApp, rpc } from "@/test/container"; @@ -114,15 +121,25 @@ function aggregateHandler( ); } +// A rejected refetch reaches the component a tick after `refetchQueries` +// resolves. Without this wait an assertion can run while the previous state is +// still rendered, which makes the retention tests pass whatever the code does. +const settle = () => new Promise((resolve) => setTimeout(resolve, 20)); + const DEFAULT_ORDER = ["Logins over time", "Top event types", "Total logins"]; -// The tile titles, in the order the dashboard shows them. +// The tile titles, in the order the dashboard shows them. The kit's renderer +// titles a series widget through its Card (`card-heading`) and a scalar one +// through its StatTile (`metric-label`), so a tile is named by whichever of +// the two it carries. function tileTitles(): string[] { return screen .getAllByRole("listitem") .map( (tile) => - tile.querySelector('[data-slot="card-title"]')?.textContent ?? "", + tile.querySelector( + '[data-slot="card-heading"], [data-slot="metric-label"]', + )?.textContent ?? "", ); } @@ -155,13 +172,22 @@ describe("SolutionDashboards", () => { renderInApp(); - // Bar widget: each event_type bucket becomes a labelled bar. - expect(await screen.findByText("saas.auth.login")).toBeTruthy(); - expect(screen.getByText("42")).toBeTruthy(); + // Bar widget: each event_type bucket becomes a bar, and every exact + // value is reachable in the chart's accessible data table — which is + // what a reader who cannot see the geometry gets. + const bars = await screen.findByRole("table", { name: "Top event types" }); + expect( + within(bars).getByRole("row", { name: /saas\.auth\.login/ }), + ).toBeTruthy(); + expect(within(bars).getByText("42")).toBeTruthy(); // Number widget: the total-logins stat sums the time buckets (3 + 5). expect(screen.getByText("8")).toBeTruthy(); - // Each widget's title is rendered from the declaration. - expect(screen.getByText("Logins over time")).toBeTruthy(); + // Each widget's title is rendered from the declaration. A chart widget + // says it three times: once visibly as its card's heading, and twice in + // the accessible data table that is the chart's screen-reader + // representation — as the table's caption and as its one series' column + // header, there being no other name for the series than the widget's. + expect(screen.getAllByText("Logins over time")).toHaveLength(3); expect(screen.getByText("Total logins")).toBeTruthy(); }); @@ -196,6 +222,449 @@ describe("SolutionDashboards", () => { expect(await screen.findByText("Unable to load.")).toBeTruthy(); }); + // The sequence that matters: a read that SUCCEEDED, then one that did not. + // TanStack Query hands back the last successful data when a refetch + // rejects, so a cold failure cannot show this — there is nothing retained + // to leak. + describe.each([ + [403, "permission_denied", Code.PermissionDenied], + [503, "unavailable", Code.Unavailable], + ] as const)("a %d on a refetch", (status, code, expected) => { + const refused = status === 403; + + it(`${refused ? "discards" : "keeps"} the values already on screen`, async () => { + server.use( + http.post(rpc("AuditService", "AggregateAuditLog"), () => + HttpResponse.json({ buckets: [{ key: "2026-09-01", count: "37" }] }), + ), + ); + const { client } = renderDashboards(); + expect(await screen.findAllByText("37")).not.toHaveLength(0); + + server.use( + http.post(rpc("AuditService", "AggregateAuditLog"), () => + HttpResponse.json({ code, message: "read rejected" }, { status }), + ), + ); + await act(async () => { + await client.refetchQueries({ queryKey: ["solution-widget"] }); + }); + + const query = client + .getQueryCache() + .find({ queryKey: ["solution-widget"], exact: false }); + expect(ConnectError.from(query?.state.error).code).toBe(expected); + + if (refused) { + // A number this viewer is no longer allowed to read is a wrong + // answer, not a stale one. + expect(await screen.findAllByText("No access")).not.toHaveLength(0); + expect(screen.queryAllByText("37")).toHaveLength(0); + expect(screen.queryByText("Stale")).toBeNull(); + } else { + // A transient failure says nothing about authority, so the + // values stay and read as out of date. + expect(await screen.findAllByText("Stale")).not.toHaveLength(0); + expect(screen.getAllByText("37").length).toBeGreaterThan(0); + } + }); + }); + + // The longer sequence, and the one that matters most: a refusal followed by + // an ordinary failure, with no authorized read in between. A snapshot that + // outlived the refusal would be eligible again at the next error, which + // hands those values straight back. + describe.each(["line", "area", "bar", "number", "table"] as const)( + "a refused %s widget", + (visualization) => { + const oneWidget: DataGraph = { + ...graph, + dashboards: [ + { + ...graph.dashboards[0], + widgets: [{ ...graph.dashboards[0].widgets[0], visualization }], + }, + ], + }; + + it.each([ + [ + "a 503", + () => HttpResponse.json({ code: "unavailable" }, { status: 503 }), + ], + ["a dropped connection", () => HttpResponse.error()], + ])("does not resurrect its values on %s", async (_name, fail) => { + server.use( + http.post(rpc("AuditService", "AggregateAuditLog"), () => + HttpResponse.json({ + buckets: [{ key: "2026-09-01", count: "37" }], + }), + ), + ); + const { client } = renderInApp( + , + ); + expect(await screen.findAllByText("37")).not.toHaveLength(0); + + server.use( + http.post(rpc("AuditService", "AggregateAuditLog"), () => + HttpResponse.json({ code: "permission_denied" }, { status: 403 }), + ), + ); + await act(async () => { + await client.refetchQueries({ queryKey: ["solution-widget"] }); + await settle(); + }); + expect(await screen.findByText("No access")).toBeTruthy(); + expect(screen.queryAllByText("37")).toHaveLength(0); + + server.use(http.post(rpc("AuditService", "AggregateAuditLog"), fail)); + await act(async () => { + await client.refetchQueries({ queryKey: ["solution-widget"] }); + await settle(); + }); + expect(screen.queryAllByText("37")).toHaveLength(0); + }); + }, + ); + + it("does not resurrect them once the caches have had time to expire", async () => { + // The shape this replaced marked the snapshot ineligible from a second + // cache entry, which had its own garbage-collection clock: five + // minutes later the mark was gone and the values were not, so a retry + // handed them back. Deleting the snapshot leaves nothing with a + // lifetime of its own to fall out of step. + server.use( + http.post(rpc("AuditService", "AggregateAuditLog"), () => + HttpResponse.json({ buckets: [{ key: "2026-09-01", count: "37" }] }), + ), + ); + const { client } = renderDashboards(); + expect(await screen.findAllByText("37")).not.toHaveLength(0); + + server.use( + http.post(rpc("AuditService", "AggregateAuditLog"), () => + HttpResponse.json({ code: "permission_denied" }, { status: 403 }), + ), + ); + await act(async () => { + await client.refetchQueries({ queryKey: ["solution-widget"] }); + await settle(); + }); + expect(await screen.findAllByText("No access")).not.toHaveLength(0); + + // Past the default five-minute gcTime, on the cache's own clock. + vi.useFakeTimers({ toFake: ["setTimeout", "clearTimeout"] }); + try { + await act(async () => { + await vi.advanceTimersByTimeAsync(300_001); + }); + } finally { + vi.useRealTimers(); + } + + server.use( + http.post(rpc("AuditService", "AggregateAuditLog"), () => + HttpResponse.json({ code: "unavailable" }, { status: 503 }), + ), + ); + await act(async () => { + await client.refetchQueries({ queryKey: ["solution-widget"] }); + await settle(); + }); + expect(screen.queryAllByText("37")).toHaveLength(0); + }); + + it("does not resurrect them on a remount", async () => { + server.use( + http.post(rpc("AuditService", "AggregateAuditLog"), () => + HttpResponse.json({ buckets: [{ key: "2026-09-01", count: "37" }] }), + ), + ); + const { client, unmount } = renderDashboards(); + expect(await screen.findAllByText("37")).not.toHaveLength(0); + + server.use( + http.post(rpc("AuditService", "AggregateAuditLog"), () => + HttpResponse.json({ code: "permission_denied" }, { status: 403 }), + ), + ); + await act(async () => { + await client.refetchQueries({ queryKey: ["solution-widget"] }); + await settle(); + }); + expect(await screen.findAllByText("No access")).not.toHaveLength(0); + + unmount(); + server.use( + http.post(rpc("AuditService", "AggregateAuditLog"), () => + HttpResponse.json({ code: "unavailable" }, { status: 503 }), + ), + ); + renderInApp( + , + client, + ); + await act(async () => { + await settle(); + }); + expect(screen.queryAllByText("37")).toHaveLength(0); + }); + + it("does not leave the same numbers behind under another graph revision", async () => { + // The graph is the last part of the query key, so a declaration that + // differs only in a dashboard title has its own cache entry holding + // the same numbers. A refusal has to reach every revision of them. + const renamed: DataGraph = { + ...graph, + dashboards: [{ ...graph.dashboards[0], title: "Activity (renamed)" }], + }; + server.use( + http.post(rpc("AuditService", "AggregateAuditLog"), () => + HttpResponse.json({ buckets: [{ key: "2026-09-01", count: "37" }] }), + ), + ); + const { client, unmount } = renderDashboards(); + expect(await screen.findAllByText("37")).not.toHaveLength(0); + unmount(); + + // A second revision, cached under its own key. + const second = renderInApp( + , + client, + ); + expect(await screen.findAllByText("37")).not.toHaveLength(0); + + server.use( + http.post(rpc("AuditService", "AggregateAuditLog"), () => + HttpResponse.json({ code: "permission_denied" }, { status: 403 }), + ), + ); + await act(async () => { + await client.refetchQueries({ queryKey: ["solution-widget"] }); + await settle(); + }); + expect(await screen.findAllByText("No access")).not.toHaveLength(0); + second.unmount(); + + // Back to the first revision, whose refresh fails transiently. + server.use( + http.post(rpc("AuditService", "AggregateAuditLog"), () => + HttpResponse.json({ code: "unavailable" }, { status: 503 }), + ), + ); + renderInApp( + , + client, + ); + await act(async () => { + await settle(); + }); + expect(screen.queryAllByText("37")).toHaveLength(0); + }); + + it("lets a successful authorized read lift the refusal", async () => { + server.use( + http.post(rpc("AuditService", "AggregateAuditLog"), () => + HttpResponse.json({ buckets: [{ key: "2026-09-01", count: "37" }] }), + ), + ); + const { client } = renderDashboards(); + expect(await screen.findAllByText("37")).not.toHaveLength(0); + + server.use( + http.post(rpc("AuditService", "AggregateAuditLog"), () => + HttpResponse.json({ code: "permission_denied" }, { status: 403 }), + ), + ); + await act(async () => { + await client.refetchQueries({ queryKey: ["solution-widget"] }); + }); + expect(await screen.findAllByText("No access")).not.toHaveLength(0); + + // Authority restored: a read succeeds and the NEW values are drawn. + server.use( + http.post(rpc("AuditService", "AggregateAuditLog"), () => + HttpResponse.json({ buckets: [{ key: "2026-09-01", count: "42" }] }), + ), + ); + await act(async () => { + await client.refetchQueries({ queryKey: ["solution-widget"] }); + }); + expect(await screen.findAllByText("42")).not.toHaveLength(0); + expect(screen.queryByText("No access")).toBeNull(); + }); + + // Everything above drives the refusal through a mounted component. These + // drive it where no mounted component can answer for it: the refusal is a + // property of the READ, and the read outlives whoever started it. + describe("a refusal with nobody watching it", () => { + // A handler that holds its request open until the test releases it, so a + // refusal can be in flight across an unmount or a change of dashboard. + function heldRefusal() { + let enter!: () => void; + let release!: () => void; + const entered = new Promise((resolve) => { + enter = resolve; + }); + const released = new Promise((resolve) => { + release = resolve; + }); + server.use( + http.post(rpc("AuditService", "AggregateAuditLog"), async () => { + enter(); + await released; + return HttpResponse.json( + { code: "permission_denied" }, + { status: 403 }, + ); + }), + ); + return { entered, release }; + } + + it.each([ + [ + "a 503", + () => HttpResponse.json({ code: "unavailable" }, { status: 503 }), + ], + ["a dropped connection", () => HttpResponse.error()], + ])( + "deletes the values when it completes after the page is gone, then %s", + async (_name, fail) => { + server.use( + http.post(rpc("AuditService", "AggregateAuditLog"), () => + HttpResponse.json({ + buckets: [{ key: "2026-09-01", count: "37" }], + }), + ), + ); + const { client, unmount } = renderDashboards(); + expect(await screen.findAllByText("37")).not.toHaveLength(0); + + const { entered, release } = heldRefusal(); + const refusal = client.refetchQueries({ + queryKey: ["solution-widget"], + }); + await entered; + // The viewer leaves while the request is still open, so no effect + // of theirs is left to run when the answer comes back. + unmount(); + release(); + await refusal; + + // A retry nobody is watching either. It must not hand the values + // back to the next mount. + server.use(http.post(rpc("AuditService", "AggregateAuditLog"), fail)); + await client.refetchQueries({ queryKey: ["solution-widget"] }); + + renderInApp( + , + client, + ); + await act(async () => { + await settle(); + }); + expect(screen.queryAllByText("37")).toHaveLength(0); + }, + ); + + it("deletes the revision on screen when it lands on the one the viewer left", async () => { + // The refusal is for the revision the viewer has navigated away from; + // the one mounted is another revision of the same widget, with its own + // cached values on screen. Both are this widget's source, and nothing + // in React connects the refused read to the component drawing them. + const renamed: DataGraph = { + ...graph, + dashboards: [{ ...graph.dashboards[0], title: "Activity (renamed)" }], + }; + server.use( + http.post(rpc("AuditService", "AggregateAuditLog"), () => + HttpResponse.json({ buckets: [{ key: "2026-09-01", count: "37" }] }), + ), + ); + const { client, rerender } = renderDashboards(); + expect(await screen.findAllByText("37")).not.toHaveLength(0); + + // The second revision reads its own values, under its own key. + server.use( + http.post(rpc("AuditService", "AggregateAuditLog"), () => + HttpResponse.json({ buckets: [{ key: "2026-09-01", count: "42" }] }), + ), + ); + rerender(); + expect(await screen.findAllByText("42")).not.toHaveLength(0); + rerender(); + await act(async () => { + await settle(); + }); + + const { entered, release } = heldRefusal(); + const refusal = client.refetchQueries({ + queryKey: ["solution-widget"], + type: "active", + }); + await entered; + + // Mid-refusal the viewer switches back, where the other revision's + // values are cached and its refresh fails transiently: 42, stale. + server.use( + http.post(rpc("AuditService", "AggregateAuditLog"), () => + HttpResponse.json({ code: "unavailable" }, { status: 503 }), + ), + ); + rerender(); + expect(await screen.findAllByText("Stale")).not.toHaveLength(0); + expect(screen.getAllByText("42").length).toBeGreaterThan(0); + + await act(async () => { + release(); + await refusal; + await settle(); + }); + expect(screen.queryAllByText("42")).toHaveLength(0); + }); + + it("asks once, rather than clearing its own error and asking again", async () => { + // Clearing the entry to drop its values restarts every active read of + // it, whose next answer is the same refusal: a standing 403 became an + // unbounded loop. Deleting the values touches neither the error nor + // the fetch, so the retry policy is the client's alone. + server.use( + http.post(rpc("AuditService", "AggregateAuditLog"), () => + HttpResponse.json({ buckets: [{ key: "2026-09-01", count: "37" }] }), + ), + ); + const { client } = renderDashboards(); + expect(await screen.findAllByText("37")).not.toHaveLength(0); + + let refusals = 0; + server.use( + http.post(rpc("AuditService", "AggregateAuditLog"), async () => { + refusals += 1; + // Answered a tick late, so the component sees the pending state + // in between — the window a self-restarting clear re-entered. + await new Promise((resolve) => setTimeout(resolve, 15)); + return HttpResponse.json( + { code: "permission_denied" }, + { status: 403 }, + ); + }), + ); + await act(async () => { + await client.refetchQueries({ queryKey: ["solution-widget"] }); + }); + expect(await screen.findAllByText("No access")).not.toHaveLength(0); + for (let round = 0; round < 8; round += 1) { + await act(async () => { + await settle(); + }); + } + // Three widgets, one refresh, `retry: false` in tests. + expect(refusals).toBe(3); + }); + }); + it("reads as loading until the org context resolves, never as empty", () => { authState.organizationId = undefined; diff --git a/module/services/frontend/code/src/test/container.tsx b/module/services/frontend/code/src/test/container.tsx index af7459594..acf8a8e7e 100644 --- a/module/services/frontend/code/src/test/container.tsx +++ b/module/services/frontend/code/src/test/container.tsx @@ -10,16 +10,35 @@ export function rpc(service: string, method: string): string { // Render an admin feature container with the single provider its data hooks // need. retry is off so a rejected ancillary query surfaces at once rather // than stalling the test. -export function renderInApp(ui: React.ReactElement) { - const client = new QueryClient({ - defaultOptions: { queries: { retry: false } }, - }); +export function renderInApp(ui: React.ReactElement, reuse?: QueryClient) { + // `reuse` mounts into an EXISTING cache, which is what a remount or a + // second revision of the same declaration actually is: the component goes + // away and the cached answers do not. A test that built a fresh client + // could not tell a snapshot that was deleted from one that was never + // there. + const client = + reuse ?? + new QueryClient({ + defaultOptions: { queries: { retry: false } }, + }); // The client comes back alongside the render result so a test can drive a // SECOND read of the same query — the success-then-refused sequence, which is // the one that proves a denial is not hidden behind rows already on screen. // A cold denied read cannot show that, because there is nothing retained. + const mounted = render( + {ui}, + ); return { - ...render({ui}), + ...mounted, client, + // Re-render under the SAME provider. React Testing Library's own + // `rerender` replaces the whole tree with what it is given, so a test + // that passed the bare container would lose the client — which is the + // one thing a test of a second revision of the same declaration needs + // to keep. + rerender: (next: React.ReactElement) => + mounted.rerender( + {next}, + ), }; }