ReviewOS

stacks/reportshq

Drag-and-drop report builder

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

Context

The flagship surface: a visual drag-and-drop report builder. A user composes a report from blocks on a 12-column grid, configures each block's query and viz, and sees live data as they build. This is the page that sells the product - read stacks-design-taste (and stacks-design-output) first, budget real time for interaction polish, and be picky.

Drag interactions come from @ts-charts/drag (our D3 rewrite's drag package) plus pointer events - no third-party drag-and-drop library.

Scope

  • Builder route per report (/reports/{slug}/edit), edit access per #4 authz.
  • Canvas: 12-column grid, drag from a block palette to place, drag to move, resize handles, collision push-down, snap guides, 8px baseline; selection state with keyboard movement (arrows) and delete - the grid math mirrors ReportBlock.{x,y,w,h} (#9) exactly.
  • Block config panel: event/measure/dimension/filter/grain/comparison pickers driven by the same @stacksjs/validation schema as the server (#9); friendly names from the taxonomy doc (#6); viz options per block kind (#11).
  • Live preview: debounced POST /api/projects/{id}/query (#10) per edited block; skeleton while in flight; errors surface inline in the block, never as toasts alone.
  • Autosave to ReportRevision (#9) with visible save-state indicator; undo/redo across layout + config edits (bounded stack, in-memory, seeded from revisions on load).
  • Draft vs published: publishing snapshots the draft into the served report; viewers (#13) only ever see published.
  • Empty state: a designed "start from a template or a blank grid" moment (templates arrive with #14; wire the hook now).
  • Reduced motion: all drag/snap affordances usable with prefers-reduced-motion; no scroll-hijack; touch works (pointer events, not mouse-only).
  • Feature tests for the server surface (revision writes, publish snapshot, authz); browser QA via stacks-browse for drag/resize/undo flows.

Acceptance

  • Build a three-block report from scratch on the seeded project, publish, reload: layout and data identical. Pre-flight passes in both themes at mobile and desktop widths (builder may be read-only on small screens if honest about it).

Depends on #9, #10, #11. Part of #1 (M4 Builder and viewing). Ground rules: see #1.

8 days ago

Partly shipped in b7be716, and I am leaving this open because one thing is genuinely unverified: I could not confirm the save endpoint persists a drag over HTTP before running out of session. Everything else below is built and checked.

In: the server-rendered grid (7 blocks of the seeded report render correctly), the block palette, pointer-driven drag and resize, keyboard move and delete, selection, the batched debounced save, publish, and the whole write surface behind auth with per-project resolution.

Not verified: an end-to-end drag actually landing in the database through /api/reports/blocks/save. The last curl against it returned 404 from editableReport, and I fixed two real causes on the way (below) but did not get a green round trip afterwards. Treat the save path as unproven until someone reproduces it. The helper underneath it, updateBlocks, is straightforward and validated, so my expectation is that it works now, but expectation is not verification and I would rather say so.

Two root causes found and fixed, both worth knowing:

The session cookie name was mine, not the framework's. I had picked reportshq_token; the auth middleware resolves the name from config via authCookieName(), which returns auth-token. So pages authenticated happily from the cookie while every API request they made came back 401. The app now resolves the name from the framework in one place (sessionCookieName()), and the pre-paint guard reads it off the root element rather than spelling it out, since a plain script cannot import. Two spellings of a session name is a failure mode that looks like a permissions bug and is not one.

request.input() does not surface JSON body fields on this path. Handlers reading request.input('project') saw nothing and answered "not found" for a report that was right there. The routes now parse the body explicitly, which is what the ingest route already did; I should have followed that precedent from the start.

Design decisions worth review:

  • Drag is pointer events written directly, not signals. The design rules ban driving continuous values through state re-read every frame, which is precisely what a drag is, and pointer capture means a fast gesture leaving the tile does not drop it. Same documented exemption AuthGuard takes.
  • Saves are batched and debounced. One drag can move several blocks, and saving those as separate requests is several chances to end up with a layout nobody arranged.
  • The save state is a word, not a spinner, and it says "Not saved" on failure. A builder that reports success on a failed write is how a session's work disappears quietly.
  • A new block lands below the others, never at 0,0. A builder that covers your existing work the moment you add to it teaches people not to add to it.
  • Validation happens before anything is written, and layout and query are checked together, so a rejected block leaves the grid exactly as it was rather than half-moved.

Still to do on this issue: the block config panel (choosing measure, events, dimension, grain, filters against the shared schema), undo and redo across edits, collision push-down when a block is dropped onto another, and the draft-versus-published split in the builder UI. The revision machinery those need is already built and tested in #9.

8 days ago

The save round trip is now verified. The 404 I flagged as unproven had a real cause, fixed in 1a0fd0d.

The auth middleware stamps request._authenticatedUser, not request.user. Every route I had written read the latter, which is undefined, so an authenticated request was answered 404 or 401. The middleware passed and the route refused, and those two facts look completely unrelated when you are staring at them: the request has a valid cookie, the middleware clearly let it through, and the handler insists the report does not exist. It was silently wrong on the project routes too, which had never been exercised over HTTP.

There is now one resolver in app/Support/session.ts that checks both properties, coerces the id once (the framework types it string | number), and returns null rather than a zero id for an anonymous request, since zero sails through a truthiness check and then matches nothing. Six regression tests cover it, because a bug whose symptom is "authenticated request looks anonymous" is worth catching in a suite rather than in a browser.

Verified against a running server, end to end:

save a move                      HTTP 200   block 77 now: 6,8,3,2
off-grid move                    HTTP 422   "A block at x=10 cannot be 6 wide; the grid is 12 columns."
                                            block unchanged at 6,8,3,2
another tenant's project         HTTP 404
no cookie                        HTTP 401
add a block                      HTTP 201
publish                          HTTP 200   blocks: 8, status: published, publish revisions recorded

The off-grid case is the one worth noting: validation runs before anything is written, so a rejected block leaves the grid exactly as it was rather than half-moved, and the message names the grid rather than blaming x, which is what the builder will surface when a drag hits the right edge.

237 tests, CI green.

Still open on this issue: the block config panel (measure, events, dimension, grain and filters against the shared schema), undo and redo, collision push-down when a block is dropped onto another, and the draft-versus-published split in the builder. The revision machinery all of that needs is already built and tested in #9, and the write surface it will call is now proven.

8 days ago

Done across d5d2520, 6091df2 and 258e999. CI green, 307 tests.

Canvas. Twelve columns, drag to move, corner handle to resize, arrow keys and Delete for the keyboard, pointer events throughout so a finger and a stylus work and a fast drag that leaves the tile keeps the gesture.

Collision push-down lives in app/Reports/layout.ts and runs on the server on every write, so the API cannot be talked into an overlap even by a client that skips the push-down. That matters more than it sounds: CSS grid neither enforces nor complains about overlap, so a stored overlap looks fine in the builder and turns up as a chart drawn over a table in somebody else's browser. The dragged block keeps where it was dropped and the others give way, because moving the dragged block instead means it does not end up where you let go. Nothing is ever pulled up - compaction would rearrange spacing somebody chose deliberately.

The client mirrors the algorithm for feedback while dragging, packing from the layout as it was when the drag started rather than from the previous frame. Packing already-pushed positions again pushes them again, so blocks drifted further down the longer you held the pointer; caught in the browser, where the metrics landed on row 6 with row 5 free. The save response returns the stored layout and the client adopts it, so there is one authority and one preview rather than two implementations that eventually disagree.

Undo and redo, bounded to 50 in memory, on Cmd+Z / Shift+Cmd+Z and two buttons, recorded before a change so undo restores what was on screen when you started. Deeper history is still the revision log.

Draft and published are now different things. The model has promised this in a comment since #9 while the viewer read live blocks, meaning every keystroke in the builder was public the moment it was typed. Viewers are served the last publish revision. unpublished_changes is a flag rather than a timestamp comparison, because both sides store to the second and a publish landing in the same second as its edit would read as stale forever.

Settings panel with title, events, measure, field, grouping, grain, comparison and filters, offering what the project has actually sent rather than what the schema allows in the abstract. Events read as 'Order placed', not commerce.order.created. Only controls that mean something for the block appear. The query is merged onto the stored one, not rebuilt: the panel does not render every field a block can carry, and rebuilding deleted a funnel's steps the first time somebody renamed it.

Live preview redraws the tile by fetching this page and swapping that one block in. Charts are server-rendered SVG, and a client-side redraw would be a second implementation of every chart in JavaScript, which is exactly how a builder previews something the viewer will not render. Rejected configs surface in the panel and on the block, carrying the server's own message.

Small screens stack the blocks full width and say that arranging needs a wider screen, per the issue's allowance. Selecting, adding and publishing still work.

Two bugs found only because this was finally opened in a browser rather than curled: a plain click was a drag of zero distance and wrote a revision plus a 'Not published' badge for selecting a block; and the auth guard bounced every signed-in visitor to /login on every page (fixed separately in e6598e3). Both are the same lesson, now in AGENTS.md: curl cannot verify anything that depends on JavaScript.

Not carried over from the original scope: the URL is /report-edit?project=N&slug=S rather than /reports/{slug}/edit, matching the viewer's existing /report?project=N&slug=S. Consistency with the surface built in #13 seemed worth more than the shape written down here; say if you would rather both moved.

Sign in to comment on this issue.