ReviewOS

stacks/reportshq

Report domain models

#9
Closed chris opened this 8 days ago ·
8 days ago

Context

The reporting domain, modeled entirely in app/Models/ (schema via buddy generate:migrations, #5). Keep the shapes boring and explicit; the query engine (#10), builder (#12), and templates (#14) all consume these.

Scope

  • Report: project-scoped, name, slug, description, status (draft | published), origin (user | template), template_key (nullable, for #14 idempotency), default time range, layout version. useTimestamps, useSoftDeletes.
  • ReportBlock: belongs to Report; kind (line | area | bar | donut | big_number | table | funnel | heatmap | text); grid placement {x, y, w, h} on a 12-column grid; query JSON (measure(s), event name(s), dimension, filters, time grain, comparison range) validated with @stacksjs/validation schemas - the same schema the builder UI and query engine share; viz JSON (axes, palette slot, number format).
  • ReportRevision: autosave snapshots of layout + blocks (bounded count per report; builder undo/restore in #12).
  • ReportSchedule: belongs to Report; cadence (daily | weekly | monthly), hour + timezone, recipients, format (link | csv | xlsx); consumed by #15.
  • ReportShare: revocable public token, optional expiry; consumed by #16.
  • ReportExport: requested export bookkeeping (status, file path via @stacksjs/storage, expiry); consumed by #15.
  • Relationships, factories, and seeders for all of the above - the seeded dev database must contain a convincing demo project with events (#6 factories) and two seeded reports, because every UI issue develops against the seed.
  • Owner/member authorization helpers consistent with #4 (app/Support/access.ts pattern).
  • Feature tests: model CRUD through actions, block query-JSON validation rejects malformed configs, revision pruning.

Acceptance

  • buddy migrate:fresh --seed yields a demo project with seeded reports renderable once #10/#11 land; zero hand-written SQL.

Depends on #4, #5. Blocks #10, #12, #13, #14, #15, #16. Part of #1 (M3 Reporting core). Ground rules: see #1.

8 days ago

Shipped in 6cd30fb. 134 tests, CI green.

Six models, all generated from app/Models with no SQL written by hand: Report, ReportBlock, ReportRevision, ReportSchedule, ReportShare, ReportExport. Each was checked for the duplicate-relationship-column mistake that bit Project and ProjectInvite in #4, so created_by_id, requested_by_id and report_id are declared through the relation with an explicit foreignKey rather than alongside one.

app/Reports/schema.ts is the single definition of what a block asks for. The models validate against it, the builder's config panel (#12) will be generated from it, and the engine (#10) reads it to build a query. Writing that three times is exactly how a measure ends up meaning one thing in the UI and another in the SQL.

The part worth reviewing is the field allowlist. A block config names fields that reach a query builder, and it is the one place in this product where client-supplied strings flow toward SQL. So field, dimension and every filter.field are checked against an explicit list of event columns; anything under properties. is open, because that bag is the customer's own and is read through a JSON accessor, but quotes and backslashes are refused there too and the key length matches what ingestion would have stored. A pattern like "anything shaped like an identifier" would have been easier and is how a column name becomes an injection point the day someone rewrites the query with a template string.

Validation returns every problem rather than the first. A config panel that reveals the next error only after you fix this one is a worse experience than one that shows both at once.

Layout validation names what actually happened. A block at x: 8 that is 6 wide reports "a block at x=8 cannot be 6 wide; the grid is 12 columns" rather than blaming x, because that is the shape a drag produces at the right-hand edge and the builder will surface the message verbatim.

Revisions are whole-report snapshots, not per-block: a layout change moves several blocks at once, and restoring half of one is worse than not restoring. Autosaves are pruned to 30, ordered by id rather than timestamp because several land inside the same second and CURRENT_TIMESTAMP would order them arbitrarily. Publishes survive that pruning, since "restore what was last live" is a different promise from undo and there are few of them.

A few smaller decisions recorded in the code: default_range is a relative token (last_30_days) rather than dates, because a report saved in June should not still open on June when read in September. A schedule stores its cadence as parts rather than a cron expression, since weekly on Tuesday at 08:00 in Europe/Lisbon is what a person configures and reconstructing the day-of-week from 0 8 * * * is lossy. A schedule copies the project's timezone at creation rather than joining it, so moving a project's timezone does not silently reschedule every existing delivery. And revoking a share sets revoked_at rather than deleting the row, because a deleted row makes a link that was withdrawn indistinguishable from one that never existed.

The demo seed is the part everything downstream depends on. bun scripts/seed-demo.ts writes a believable month rather than a fixture: weekday and weekend traffic differ, a trend drifts upward across the month, revenue varies per order, about 6% of orders are refunded a day or two later, and a pool of returning customers reappears across days so retention and repeat rate are not empty. It writes through the real ingest path, so it exercises normalisation rather than bypassing it, and it is deterministic and idempotent, because every UI issue from here on develops against it.

Seeded Northwind Supply
  1603 events across 30 days
  2 published reports: revenue-overview (7 blocks), customers (5 blocks)

commerce.product.viewed=454, commerce.order.created=454, commerce.checkout.started=454, user.registered=215, commerce.order.refunded=26. Running it twice leaves one project, not two.

Not done here, deliberately: the per-model useSeeder factories exist but the acceptance criterion mentioned migrate:fresh --seed, which is broken upstream (stacksjs/stacks#2323) and would leave duplicate migrations behind. The demo script is the reliable path until that lands, and it produces a far more useful database than random factory rows would.

Sign in to comment on this issue.