feat(forms): request-form builder #486

Merged

longjacksonle merged 4 commits from feat/PMS-731-admin-form-builder into main

2026-08-06 21:38:09 +02:00

longjacksonle commented

2026-08-06 21:35:44 +02:00

Owner

Second slice of the client-request frontend, after the public page in #485: the admin surface that defines what a client is asked for.

/admin/forms lists the tenant's request forms and edits one in a single modal covering the definition, its ordered field set, and its conditional rules.

Why one editor, not the parent-then-children split

sla.rs creates a policy and then edits its targets in a second modal. That shape does not fit here, for two server-side reasons:

mokosh-server requires at least one field to CREATE a definition, so a metadata-only create step would always fail.
A PATCH carrying fields REPLACES the whole set rather than merging, because field identity is the payload key and a merge cannot express a rename unambiguously.

A whole-form editor matches both facts exactly. It also opens with one empty field row rather than an empty list the operator has to discover the "Add field" button to escape.

Details worth reviewing

Position is the sort order. Fields are reordered by moving rows, and sort_order is assigned from the index on save, so an operator never types numbers that can collide.

Three checks run client-side purely to save a round trip, with the server still the authority: a duplicate reference name, a choice list with no options, and a rule missing either of its fields. Each is something the server also rejects. Per docs/form-conventions.md every check runs and every slot is set before bailing once, so one problem never masks another.

Server field errors land in the banner on purpose. The server keys them fields[3].options and rules[0].field, which have no inline slot in a dynamic row editor, so routing them by key would drop them. They surface in the form-level banner where they stay readable.

The slug follows the name until touched, then stops, and is read-only once the definition exists. It is the link-stable identifier, and links already emailed to clients have to keep resolving. The client-side slugify mirrors the server's slug shape rather than approximating it, and is unit-tested against it.

An unknown rule kind blocks the save rather than silently dropping it. FormRule decodes an unrecognised kind to Other so a definition authored by a newer server still loads and lists, but saving would write the loss back, so the editor refuses and says why. Unit-tested.

A typed PATCH was missing. Only the String-error patch_authed existed, which discards the error.errors[] envelope the editor needs. Added alongside put_authed_typed.

Verification

cargo check --target wasm32-unknown-unknown, cargo fmt --all --check, cargo test (258 passed, 0 failed, including 3 new unit tests), and both guard scripts (check-theme-tokens, check-runner-labels).

just check-clippy still reports the two pre-existing failures in src/pages/projects.rs (manual_div_ceil) and src/pages/time.rs (unnecessary_sort_by). Both reproduce on unmodified main with a local clippy newer than CI's, so they are not from this branch; nothing in the new code trips a lint. Flagged in #485 too, and worth its own fix before whoever upgrades the runner hits it.

Next slices

The send-a-link action (issue a request link to a client from the company or contact screen), then the measured-duration readout on the KB article page.

Second slice of the client-request frontend, after the public page in #485: the admin surface that defines what a client is asked for. `/admin/forms` lists the tenant's request forms and edits one in a single modal covering the definition, its ordered field set, and its conditional rules. ## Why one editor, not the parent-then-children split `sla.rs` creates a policy and then edits its targets in a second modal. That shape does not fit here, for two server-side reasons: - mokosh-server requires at least one field to CREATE a definition, so a metadata-only create step would always fail. - A PATCH carrying `fields` REPLACES the whole set rather than merging, because field identity is the payload key and a merge cannot express a rename unambiguously. A whole-form editor matches both facts exactly. It also opens with one empty field row rather than an empty list the operator has to discover the "Add field" button to escape. ## Details worth reviewing **Position is the sort order.** Fields are reordered by moving rows, and `sort_order` is assigned from the index on save, so an operator never types numbers that can collide. **Three checks run client-side purely to save a round trip**, with the server still the authority: a duplicate reference name, a choice list with no options, and a rule missing either of its fields. Each is something the server also rejects. Per `docs/form-conventions.md` every check runs and every slot is set before bailing once, so one problem never masks another. **Server field errors land in the banner on purpose.** The server keys them `fields[3].options` and `rules[0].field`, which have no inline slot in a dynamic row editor, so routing them by key would drop them. They surface in the form-level banner where they stay readable. **The slug follows the name until touched, then stops, and is read-only once the definition exists.** It is the link-stable identifier, and links already emailed to clients have to keep resolving. The client-side `slugify` mirrors the server's slug shape rather than approximating it, and is unit-tested against it. **An unknown rule kind blocks the save rather than silently dropping it.** `FormRule` decodes an unrecognised kind to `Other` so a definition authored by a newer server still loads and lists, but saving would write the loss back, so the editor refuses and says why. Unit-tested. **A typed PATCH was missing.** Only the `String`-error `patch_authed` existed, which discards the `error.errors[]` envelope the editor needs. Added alongside `put_authed_typed`. ## Verification `cargo check --target wasm32-unknown-unknown`, `cargo fmt --all --check`, `cargo test` (258 passed, 0 failed, including 3 new unit tests), and both guard scripts (`check-theme-tokens`, `check-runner-labels`). `just check-clippy` still reports the two pre-existing failures in `src/pages/projects.rs` (`manual_div_ceil`) and `src/pages/time.rs` (`unnecessary_sort_by`). Both reproduce on unmodified `main` with a local clippy newer than CI's, so they are not from this branch; nothing in the new code trips a lint. Flagged in #485 too, and worth its own fix before whoever upgrades the runner hits it. ## Next slices The send-a-link action (issue a request link to a client from the company or contact screen), then the measured-duration readout on the KB article page.

longjacksonle added 4 commits

2026-08-06 21:35:44 +02:00

feat(fetch): add a typed PATCH 95dc128ec7

The forms surface is PATCH rather than PUT, and only the `String`-error `patch_authed` existed. That variant loses the `error.errors[]` envelope, so an editor saving a bad field set could not tell the operator which field the server objected to. Mirrors `put_authed_typed`.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011myibMMwyb6za3GVWJGkiX

feat(forms): add form-definition client DTOs d8fb9e7800

Mirrors mokosh-server's `src/modules/forms/models.rs`, carrying only what the builder reads or sends.

`FieldType` is a typed enum rather than a bare string so the builder cannot author a type the server would reject with a CHECK-constraint violation, and it carries the two predicates the editor needs to decide what to render: whether a type requires an option set, and whether a length bound means anything for it.

`FormRule` decodes an unrecognised kind to `Other` rather than failing, so a definition authored by a newer server still loads and lists. Because saving would silently drop such a rule, the editor treats its presence as a blocking condition rather than writing the loss back.

The public client page keeps its own narrower shapes on purpose: the server sends a deliberately reduced view to an unauthenticated visitor (no ids, no author, no KB article), so one shared struct would mean optional fields that are always absent on one of the two surfaces.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011myibMMwyb6za3GVWJGkiX

feat(forms): add the request-form builder page 48bbe2425f

`/admin/forms`: define the forms clients fill in, their ordered field set, each field's type and validation, the conditional rules, and the KB article that documents how to perform the request.

One editor rather than the parent-then-children split `sla.rs` uses, because the server requires at least one field to CREATE a definition and a PATCH carrying `fields` REPLACES the whole set (field identity is the payload key, so a merge cannot express a rename). A whole-form editor matches both facts exactly, and the editor opens with one empty field row rather than an empty list the operator has to find the "Add field" button to escape.

Position in the list IS the sort order, so an operator reorders by moving rows instead of typing numbers that can collide.

Three things are caught client-side purely to save a round trip, with the server still the authority: a duplicate reference name, a choice list with no options, and a rule missing either of its fields. Per docs/form-conventions.md every check runs before bailing once, so one problem never masks another. Server-side field errors are keyed `fields[3].options`, which has no inline slot here, so they surface in the form-level banner where they stay readable rather than being dropped.

The slug follows the name until the operator edits it, then stops moving, and is read-only once the definition exists: it is the link-stable identifier and links already emailed to clients have to keep resolving.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011myibMMwyb6za3GVWJGkiX

feat(routes): route /admin/forms and add its nav entry

Check / fmt + clippy + tests (pull_request) Successful in 2m24s

Details

Create release / Create release from merged PR (pull_request) Has been skipped

Details

3f6e8d6287

Sits with the other /admin/* surfaces, role-gated the same way, and appears in the admin nav block next to SLA Management so the builder is reachable rather than only typeable.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011myibMMwyb6za3GVWJGkiX

longjacksonle scheduled this pull request to auto merge when all checks succeed

2026-08-06 21:37:35 +02:00