Server-side drafts for the request-form builder (PMS-759) #514

Merged
longjacksonle merged 3 commits from feat/PMS-759-server-side-form-drafts into main 2026-08-11 17:10:59 +02:00

Server half of PMS-759. The SPA half is psa-systems/mokosh-apps (linked below once open).

PMS-754 and PMS-756 already shipped autosave, restore and a discard confirmation for the request-form builder, all of it in localStorage. Vas asked for storage that survives a different machine, and localStorage is per-origin, per-profile, per-device: the demo works perfectly on one laptop and loses everything on the next one.

What this adds

form_definition_drafts: an opaque JSONB snapshot of the editor, keyed by tenant, by the user who owns it, and by the definition being edited (null while the form is new). Plus GET, PUT and DELETE /api/v1/forms/drafts.

A draft is not a form definition. The alternative, a status column on form_definitions, was rejected: every existing query filters is_active, the request-link issuer resolves a definition by id and would happily send a half-built one, and form_fields carries NOT NULL constraints a half-built form cannot satisfy. As its own record, "a draft cannot be sent to a client" needs no enforcement code at all.

Two partial unique indexes, not one plain UNIQUE. NULLs are distinct in a unique index and form_definition_id is NULL while a form is new, so a plain constraint would let one user accumulate an unbounded pile of new-form drafts, one per keystroke batch. Split this way each half is also a valid ON CONFLICT inference target, which is what makes the autosave an upsert rather than a read-then-write race.

The payload stays opaque. It is the editor's own shape; a second copy of it in Rust is a thing to keep in step for no benefit, since the only thing read inside it is name, for the drafts list. It is size-capped instead, and rejected rather than truncated: a truncated draft restores as a corrupted form.

Ownership is a predicate, not an afterthought. user_id is in the WHERE clause of every read and of the delete, so another user's draft is a 404 rather than a 403 and a caller cannot learn the id exists. Tenant isolation is the usual fail-closed RLS policy on top.

Saving the form clears its draft server-side, on both create and update, rather than leaving it to the SPA. A draft exists to survive the browser going away, so it cannot depend on the browser to tidy up.

Verification

just check clean, full Postgres integration suite green, seven new tests in tests/form_drafts.rs: repeated autosaves leaving one row, new-form and edit drafts coexisting, cross-user invisibility on both read and delete, a cross-tenant definition id being refused, retirement on save, and the size cap.

🤖 Generated with Claude Code

https://claude.ai/code/session_01X29MwLvt6mgSKrpqzh5zqB

Server half of PMS-759. The SPA half is psa-systems/mokosh-apps (linked below once open). PMS-754 and PMS-756 already shipped autosave, restore and a discard confirmation for the request-form builder, all of it in `localStorage`. Vas asked for storage that survives a different machine, and `localStorage` is per-origin, per-profile, per-device: the demo works perfectly on one laptop and loses everything on the next one. ## What this adds `form_definition_drafts`: an opaque JSONB snapshot of the editor, keyed by tenant, by the user who owns it, and by the definition being edited (null while the form is new). Plus `GET`, `PUT` and `DELETE /api/v1/forms/drafts`. **A draft is not a form definition.** The alternative, a `status` column on `form_definitions`, was rejected: every existing query filters `is_active`, the request-link issuer resolves a definition by id and would happily send a half-built one, and `form_fields` carries NOT NULL constraints a half-built form cannot satisfy. As its own record, "a draft cannot be sent to a client" needs no enforcement code at all. **Two partial unique indexes, not one plain UNIQUE.** NULLs are distinct in a unique index and `form_definition_id` is NULL while a form is new, so a plain constraint would let one user accumulate an unbounded pile of new-form drafts, one per keystroke batch. Split this way each half is also a valid `ON CONFLICT` inference target, which is what makes the autosave an upsert rather than a read-then-write race. **The payload stays opaque.** It is the editor's own shape; a second copy of it in Rust is a thing to keep in step for no benefit, since the only thing read inside it is `name`, for the drafts list. It is size-capped instead, and rejected rather than truncated: a truncated draft restores as a corrupted form. **Ownership is a predicate, not an afterthought.** `user_id` is in the WHERE clause of every read and of the delete, so another user's draft is a 404 rather than a 403 and a caller cannot learn the id exists. Tenant isolation is the usual fail-closed RLS policy on top. **Saving the form clears its draft server-side**, on both create and update, rather than leaving it to the SPA. A draft exists to survive the browser going away, so it cannot depend on the browser to tidy up. ## Verification `just check` clean, full Postgres integration suite green, seven new tests in `tests/form_drafts.rs`: repeated autosaves leaving one row, new-form and edit drafts coexisting, cross-user invisibility on both read and delete, a cross-tenant definition id being refused, retirement on save, and the size cap. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01X29MwLvt6mgSKrpqzh5zqB
PMS-754 autosaves the request-form builder to `localStorage` on every change, which covers a closed tab and a crash and nothing else. Vas asked for storage that survives a different machine, and `localStorage` is per-origin, per-profile, per-device: a user who starts a form on a laptop and finishes on a desktop still sees nothing.

A draft is deliberately not a `form_definitions` row with a status column. Every existing query filters `is_active`, the request-link issuer resolves a definition by id and would happily send a half-built one, and `form_fields` carries NOT NULL constraints a half-built form cannot satisfy. As its own record, "a draft cannot be sent to a client" needs no enforcement code.

Two partial unique indexes rather than one plain UNIQUE, because NULLs are distinct in a unique index and `form_definition_id` is NULL while a form is still new: a plain constraint would let one user accumulate an unbounded pile of new-form drafts, one per keystroke batch. Split this way each half is also a valid ON CONFLICT inference target, which is what lets the autosave be an upsert instead of a read-then-write race.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01X29MwLvt6mgSKrpqzh5zqB
`GET /forms/drafts` lists the caller's own, `PUT /forms/drafts` upserts one, `DELETE /forms/drafts/{id}` discards it. Admin-gated to match create and update: a draft is a half-written definition, so the people who can author one are the people who can hold one.

The payload stays opaque. It is the editor's own shape, and a second copy of that shape in Rust would be a thing to keep in step for no benefit: the only thing read inside it is `name`, so the drafts list has a label. Being opaque and client-supplied is why it is size-capped instead, and rejected rather than truncated, because a truncated draft restores as a corrupted form.

Ownership is a predicate on every read and on the delete, not a check applied afterwards, so another user's draft is a 404 rather than a 403: a caller should not learn that the id exists. The definition named by a draft is verified inside the tenant transaction for the same reason.

Saving the form clears its draft server-side rather than leaving it to the SPA. A draft exists to survive the browser going away, so it cannot depend on the browser to tidy up.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01X29MwLvt6mgSKrpqzh5zqB
test(forms): cover draft upsert, ownership and retirement
All checks were successful
Check / fmt + clippy + build + tests (pull_request) Successful in 2m15s
E2E / Playwright against staging (pull_request) Successful in 2m14s
Create release / Gate (release-branch merges only) (pull_request) Successful in 2s
Create release / Create release from merged PR (pull_request) Has been skipped
Integration / integration tests (pull_request) Successful in 7m42s
4ae1149638
Autosave fires on a debounce while someone types, so the same draft is written over and over: the test that matters most is that the second write replaces the first rather than accumulating, since the failure there is a drafts list nobody can use.

Also covered: a new-form draft and an edit draft coexisting rather than overwriting each other, another admin in the same tenant seeing neither the draft nor a way to delete it, a draft naming another tenant's form being refused, saving the form retiring its draft on both the create and the update path, and the payload cap.

The cross-tenant test inserts the other tenant's form directly rather than through the API: a tenant-less login resolves against the default tenant (PMS-138), so there is no token to be had for a second tenant's admin, and the test only needs an id that belongs somewhere else.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01X29MwLvt6mgSKrpqzh5zqB
longjacksonle deleted branch feat/PMS-759-server-side-form-drafts 2026-08-11 17:10:59 +02:00
Sign in to join this conversation.
No reviewers
No labels
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
psa-systems/mokosh-server!514
No description provided.