git-ents.gitmain
⌘K
foforge
commit da51411
web, forge, model: drive web rendering and entity forms from ents attributes

The web’s own reflection walk now consumes ents_forge::present’s attribute-driven walk (present::fields, newly public): skip, skip_empty, id-abbreviation, and head/col column selection are declared once on the entity’s fields and obeyed by both surfaces, with render.rs keeping only the web-shaped parts (Markup wrapping, href columns, long-token cells, unreadable degradation). Redaction’s refname-bound target gains ents::skip, matching Effect.name and ResultRecord.target.

ents-web/src/form.rs derives the issue create/edit and start-review forms from the same Facet action variants the CLI parses (IssueAction::New/Edit, ReviewAction::New): one control per web-suppliable field (Vec → comma input, bool → checkbox, ents::compose → textarea), per-field overrides for the pickers, and parse_action reading the post back into the action variant itself, which per-entity dispatch maps onto the same ents_forge calls as before. CSRF, PRG, and routes unchanged; issue form fields now post the action shape’s own names (label, assignee).

The hand-rolled dl.entity-view blocks on issues/reviews stay custom, now with their justification recorded: every row is a domain widget or a value derived from outside the entity.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Joseph D. Carpinelli · 28 days ago

Reviews

No reviews of this commit yet — record a verdict below.

Start a review

verdict

crates/cli/ents-web/src/lib.rs @@ -111,6 +111,7 @@ pub mod auth; pub(crate) mod editor; pub mod error; +pub mod form; pub mod identity; pub(crate) mod markdown; pub mod pages;
crates/cli/ents-web/src/render.rs @@ -2,7 +2,13 @@ //! analog of the gate executor named in this crate's development-plan //! row: one reflection walk over any `#[derive(Facet)]` entity's //! [`facet::Shape`], reused for every kernel entity this crate lists or -//! shows, rather than one hand-written renderer per entity type. +//! shows, rather than one hand-written renderer per entity type. The walk +//! itself is [`ents_forge::present`]'s: presentation policy (the `ents` +//! attributes -- skip, `skip_empty`, id-abbreviation, head/col column +//! selection) is declared once on the entity's own fields and obeyed here +//! exactly as the CLI obeys it; this module keeps only what is genuinely +//! web-shaped -- Markup wrapping, href columns, long-token cell breaking, +//! and the unreadable-entity degradation cards. //! //! The binding rule this module exists to uphold: nothing here ever //! matches on *which* concrete type it was handed. [`fields`] walks @@ -23,18 +29,17 @@ pub type FieldRow = (&'static str, String); /// Reflect over `value`'s [`facet::Shape`] and return one `(name, value)` -/// pair per field, in declaration order. +/// pair per field the entity's own `ents` attributes present in a view: +/// `ents::skip` fields are omitted, `ents::skip_empty` fields omitted when +/// empty, id-valued fields abbreviated, and the `ents::body` field ordered +/// last -- [`ents_forge::present::view`]'s policy, one walk shared with +/// the CLI's `show` ([`ents_forge::present::fields`]). /// /// A field's value renders via its own `Display` impl when it has one -/// (plain text, no `Type::Foo(...)` wrapper -- what a `String`, a -/// `MemberId` newtype, or an enum like [`ents_model::MemberState`] with its -/// own canonical `Display` gives), and falls back to `Debug` otherwise -/// (every entity struct in `ents-model`/`ents-forge`/`ents-kiln` derives -/// `Debug`, so a field of an enum with no `Display` still renders its -/// variant name rather than an opaque placeholder). A field this crate -/// cannot even walk as a struct (called on a non-struct `T`) renders as an -/// empty list, not a panic -- reflection is a UI convenience, never a -/// correctness path. +/// (plain text, no `Type::Foo(...)` wrapper), falling back to `Debug` so +/// an enum without `Display` still shows its variant name rather than an +/// opaque placeholder. A non-struct `T` renders as an empty list, not a +/// panic -- reflection is a UI convenience, never a correctness path. /// /// # Examples /// @@ -51,37 +56,37 @@ /// ``` #[must_use] pub fn fields<T: Facet<'static>>(value: &T) -> Vec<FieldRow> { - let peek = facet_reflect::Peek::new(value); - let Ok(structure) = peek.into_struct() else { - return Vec::new(); - }; - structure - .ty() - .fields - .iter() - .enumerate() - .map(|(index, field)| { - let rendered = structure - .field(index) - .map(render_scalar) - .unwrap_or_default(); - (field.name, rendered) - }) + let walked = ents_forge::present::fields(value); + let (body, lines): (Vec<_>, Vec<_>) = walked + .into_iter() + .filter(|field| !(field.skip_empty && field.empty)) + .partition(|field| field.body); + lines + .into_iter() + .chain(body) + .map(|field| (field.name, field.value)) .collect() } -/// Render one field's [`facet_reflect::Peek`] as plain text: its own -/// `Display` if it has one, else `Debug`, so an enum still shows a variant -/// name instead of this crate's opaque `⟨TypeName⟩` placeholder. -fn render_scalar(peek: facet_reflect::Peek<'_, '_>) -> String { - if let Some(s) = peek.as_str() { - return s.to_owned(); - } - let displayed = format!("{peek}"); - if displayed.starts_with('⟨') { - format!("{peek:?}") +/// The `(name, value)` list columns `value`'s own `ents` attributes +/// select: `ents::head` fields first, then `ents::col` fields, matching +/// [`ents_forge::present::columns`]'s order -- or, for an entity declaring +/// no column at all, every walked field, so an unannotated entity still +/// lists in full rather than as a bare id column. +fn list_columns<T: Facet<'static>>(value: &T) -> Vec<FieldRow> { + let walked = ents_forge::present::fields(value); + if walked.iter().any(|field| field.head || field.col) { + let heads = walked.iter().filter(|field| field.head); + let cols = walked.iter().filter(|field| field.col && !field.head); + heads + .chain(cols) + .map(|field| (field.name, field.value.clone())) + .collect() } else { - displayed + walked + .into_iter() + .map(|field| (field.name, field.value)) + .collect() } } @@ -114,7 +119,11 @@ /// A table listing `rows`, one row per `(id, entity)` pair, columns taken /// from the first entity's own reflected field names -- the generic -/// "list" page every kernel entity this crate exposes uses. +/// "list" page every kernel entity this crate exposes uses. Which columns +/// render is the entity's own `ents` declaration ([`list_columns`]): its +/// `ents::head` fields lead, its `ents::col` fields follow -- the same +/// selection and order the CLI's `list` derives -- with the full field +/// walk as the fallback for an entity declaring no column. /// /// `id_header` names the leading column holding each entry's key (a /// username, an effect name, a redaction id -- whatever names the ref this @@ -147,7 +156,12 @@ ) -> Markup { let field_names: Vec<&'static str> = rows .first() - .map(|(_, entity)| fields(entity).into_iter().map(|(name, _)| name).collect()) + .map(|(_, entity)| { + list_columns(entity) + .into_iter() + .map(|(name, _)| name) + .collect() + }) .unwrap_or_default(); html! { div.card { @@ -164,7 +178,7 @@ @for (id, entity) in rows { tr { td { a href=(href_for(id)) { (id) } } - @for (_, rendered) in fields(entity) { + @for (_, rendered) in list_columns(entity) { td.long-token[has_long_token(&rendered)] { (rendered) } } } @@ -386,6 +400,31 @@ ); } + /// The `ents` attributes declared on the entity's own fields govern + /// the web exactly as they govern the CLI: `ents::skip` hides the + /// refname-bound name, `ents::skip_empty` hides an empty set, and the + /// list narrows to the declared `ents::col` columns. + #[rstest] + // @relation(roots.web-agnostic, scope=function, role=Verifies) + fn ents_attributes_govern_the_view_and_the_list_columns() { + let effect = Effect { + name: "unit".to_owned(), + trigger: "rev(refs/heads/main)".to_owned(), + toolchains: vec![], + run: "true".to_owned(), + }; + let names: Vec<_> = fields(&effect).into_iter().map(|(name, _)| name).collect(); + assert_eq!(names, vec!["trigger", "run"]); + + let rows = vec![("unit".to_owned(), effect)]; + let markup = list_table(&rows, "name", |id| format!("/effects/{id}")).into_string(); + assert!(markup.contains("trigger") && markup.contains("rev(refs/heads/main)")); + assert!( + !markup.contains("<th>run</th>"), + "only the declared columns render: {markup}" + ); + } + #[rstest] // @relation(roots.web-agnostic, scope=function, role=Verifies) fn list_table_derives_its_columns_from_the_first_rows_own_shape() {
crates/cli/ents-web/tests/router.rs @@ -2249,8 +2249,10 @@ labels: &str, ) -> String { let (cookie, csrf) = session_cookie_and_csrf(router, state, "/issues").await; + // Field names are `IssueAction::New`'s own (`ents_web::form`): the + // form's controls and its parse both derive from the action shape. let form = format!( - "title={}&state={issue_state}&assignees={assignees}&labels={labels}&body=the+full+body&csrf={csrf}", + "title={}&state={issue_state}&assignee={assignees}&label={labels}&body=the+full+body&csrf={csrf}", title.replace(' ', "+") ); let response = router
crates/forge/ents-forge/src/present.rs @@ -156,6 +156,46 @@ out } +/// One walked field with its declared presentation roles — for a surface +/// (the web) that renders its own markup over the same attribute-driven +/// policy [`view`] and [`columns`] apply. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct PresentedField { + /// The field's declared name, exactly as the struct spells it. + pub name: &'static str, + /// The field's rendered value, human audience: ids abbreviated. + pub value: String, + /// Whether the value counts as empty for `ents::skip_empty`. + pub empty: bool, + /// `ents::head`: a leading list column. + pub head: bool, + /// `ents::col`: a list column after the head columns. + pub col: bool, + /// `ents::skip_empty`: omit from a view when empty. + pub skip_empty: bool, + /// `ents::body`: the message body, rendered last. + pub body: bool, +} + +/// Walk `value` for a human audience: every non-`ents::skip` field with +/// its rendered value and declared roles, in declaration order. A +/// non-struct `T` yields no fields. +#[must_use] +pub fn fields<T: Facet<'static>>(value: &T) -> Vec<PresentedField> { + rows(value, Audience::Human) + .into_iter() + .map(|row| PresentedField { + name: row.name, + value: row.value, + empty: row.empty, + head: row.policy.head, + col: row.policy.col, + skip_empty: row.policy.skip_empty, + body: row.policy.body, + }) + .collect() +} + /// [`record`] over every `(id, entity)` row, records separated by one /// blank line — the whole `--porcelain` output for a listing. #[must_use]
crates/kernel/ents-model/src/redaction.rs @@ -2,6 +2,7 @@ //! //! Spec coverage: `model.redaction`. +use ents_attrs as ents; use facet::Facet; use gix_hash::ObjectId; @@ -39,6 +40,7 @@ // @relation(model.redaction, meta-ref.typed-tree, model.extensibility, scope=file) #[derive(Debug, Clone, PartialEq, Eq, Facet)] pub struct Redaction { + #[facet(ents::skip)] target: [u8; 20], /// A human-readable reason for the redaction. pub reason: String,
crates/cli/ents-web/src/pages/commits.rs @@ -601,12 +601,6 @@ Ok(Redirect::to(&target)) } -/// The start-a-review form (`POST /commit/{oid}/review`): a verdict and -/// a body. The verdict is a closed `.picker` (README's `VerdictPicker`) of -/// radio inputs over [`ents_forge::review::Verdict`]'s three variants -- -/// `model.review` makes it a hard enum, unlike issue and comment states -- -/// defaulting to `approve`, the same default a bare `select`'s first option -/// would submit. /// The commit-level comment composer's own hidden `<template>` /// (`crate::pages::files::composer_template`'s counterpart for a commit, /// which has no file of its own to anchor a path-based comment to): posts @@ -696,78 +690,101 @@ Ok(Redirect::to(&target)) } +/// The start-a-review form (`POST /commit/{oid}/review`), its fields +/// [`ents_forge::review::ReviewAction::New`]'s own +/// ([`crate::form::action_form`]): `target` is the page's commit, so no +/// control renders for it, and the verdict is a closed `.picker` +/// (README's `VerdictPicker`) of radio inputs over +/// [`ents_forge::review::Verdict`]'s three variants -- `model.review` +/// makes it a hard enum, unlike issue and comment states -- defaulting to +/// `approve`, the same default a bare `select`'s first option would +/// submit. The body stays the derived compose-field textarea. fn start_review_form(session: &Session, oid: &str) -> Markup { html! { h3 { "Start a review" } - form method="post" action=(format!("/commit/{oid}/review")) { - (super::csrf_input(session)) - p.muted { "verdict" } - div.picker { - label.opt { - input type="radio" name="verdict" value="approve" checked; - span.dot {} - "approve" - } - label.opt { - input type="radio" name="verdict" value="request-changes"; - span.dot {} - "request-changes" - } - label.opt { - input type="radio" name="verdict" value="comment"; - span.dot {} - "comment" - } - } - label { "Body" textarea name="body" {} } - button type="submit" { "Start a Review" } - } + (crate::form::action_form::<ents_forge::review::ReviewAction>( + "New", + session, + &crate::form::Spec { + action: &format!("/commit/{oid}/review"), + submit: "Start a Review", + cancel: None, + values: &[], + overrides: &[ + ("target", html! {}), + ("verdict", html! { + p.muted { "verdict" } + div.picker { + label.opt { + input type="radio" name="verdict" value="approve" checked; + span.dot {} + "approve" + } + label.opt { + input type="radio" name="verdict" value="request-changes"; + span.dot {} + "request-changes" + } + label.opt { + input type="radio" name="verdict" value="comment"; + span.dot {} + "comment" + } + } + }), + ], + }, + )) } } -/// The form fields `POST /commit/{oid}/review` accepts. -#[derive(Debug, Deserialize)] -pub struct ReviewForm { - /// The review's verdict. - verdict: String, - /// The review's body text. - #[serde(default)] - body: String, - /// The per-session CSRF token (`roots.web-session`). - csrf: String, -} - /// `POST /commit/{oid}/review`: review the commit at `oid` /// (`ents_forge::review::new`), which writes both the review's entity ref /// and its retention pin (`model.review`, `model.review-pin`) -- the web is /// another caller of that one library func, never a second review or /// pin-writing path. Signed (`roots.web-signing`) on behalf of the current -/// session (`roots.web-session`). +/// session (`roots.web-session`). The posted fields are +/// [`ents_forge::review::ReviewAction::New`]'s own +/// ([`crate::form::parse_action`]), the path's `oid` standing in for the +/// CLI's `--target`. /// /// # Errors /// -/// [`Error::BadCsrf`] if `form.csrf` does not match; otherwise propagates -/// [`ents_forge::review::new`]'s own failures (including an unresolvable -/// target commit). +/// [`Error::BadCsrf`] if the posted token does not match; otherwise +/// propagates [`ents_forge::review::new`]'s own failures (including an +/// unresolvable target commit). // @relation(model.review, model.review-pin, roots.web-signing, roots.web-session, scope=function) pub async fn review<O>( State(state): State<Arc<AppState<O>>>, axum::Extension(session): axum::Extension<Session>, Path(oid): Path<String>, - Form(form): Form<ReviewForm>, + Form(mut pairs): Form<Vec<(String, String)>>, ) -> Result<impl IntoResponse> where O: Find + Write + Send + 'static, { - super::require_csrf(&session, &form.csrf)?; + super::require_csrf(&session, crate::form::posted_csrf(&pairs))?; + pairs.retain(|(name, _)| name != "target"); + pairs.push(("target".to_owned(), oid.clone())); + let ents_forge::review::ReviewAction::New { + target, + verdict, + body, + key: _, + } = crate::form::parse_action("New", &pairs)? + else { + return Err(Error::InvalidArgument( + "not a form-backed review action".to_owned(), + )); + }; let member = super::reviewer_member_id(&state); let identity = state.identity.as_ref(); let new = ents_forge::review::NewReview { - target: oid.clone(), - verdict: form.verdict.parse().map_err(|_unknown| { - Error::InvalidArgument(format!("unknown verdict: {}", form.verdict)) - })?, - body: form.body, + target, + verdict: verdict + .parse() + .map_err(|_unknown| Error::InvalidArgument(format!("unknown verdict: {verdict}")))?, + body: body.unwrap_or_default(), }; let (_target, outcome) = ents_forge::review::new( state.refs.as_ref(),
crates/cli/ents-web/src/pages/issues.rs @@ -19,7 +19,7 @@ use axum::Form; use axum::extract::{Path, State}; use axum::response::{IntoResponse, Redirect}; -use ents_forge::issue::{self, EditIssue, NewIssue}; +use ents_forge::issue::{self, EditIssue, IssueAction, NewIssue}; use ents_model::MemberId; use gix_object::{Find, Write}; use maud::{Markup, html}; @@ -117,7 +117,10 @@ /// for its state/assignees/labels, and its discussion thread -- the /// comments naming `issues/<id>` as their context /// (`ents_forge::comment::thread`, `model.comment-context`), rendered like -/// every other conversation in this crate. +/// every other conversation in this crate. The metadata `dl.entity-view` +/// stays hand-rolled rather than [`crate::render::view`]'s generic dump: +/// every row is a domain widget (state chip, assignee avatars, label +/// chips, an "unassigned"/"none" placeholder), not a field's plain text. /// /// # Errors /// @@ -225,127 +228,124 @@ )) } -/// The form fields `POST /issues` accepts. -#[derive(Debug, Deserialize)] -pub struct NewForm { - /// The issue's title. - title: String, - /// The issue's body. - #[serde(default)] - body: String, - /// The issue's initial state; defaults to `open` (`model.issue`: the - /// platform has no default of its own, so the frontend chooses one). - #[serde(default = "default_state")] - state: String, - /// Comma- or whitespace-separated assignee usernames. - #[serde(default)] - assignees: String, - /// Comma- or whitespace-separated labels. - #[serde(default)] - labels: String, - /// The per-session CSRF token (`roots.web-session`). - csrf: String, -} - -fn default_state() -> String { - "open".to_owned() -} - /// `POST /issues`: open an issue at a freshly generated /// `refs/meta/issues/<id>` (`ents_forge::issue::new`), signed /// (`roots.web-signing`) on behalf of the current session -/// (`roots.web-session`). +/// (`roots.web-session`). The posted fields are +/// [`IssueAction::New`]'s own ([`crate::form::parse_action`]), so the +/// form's shape and this handler's parse are one declaration. /// /// # Errors /// -/// [`crate::Error::BadCsrf`] if `form.csrf` does not match; otherwise -/// propagates [`ents_forge::issue::new`]'s own failures. +/// [`crate::Error::BadCsrf`] if the posted token does not match; +/// otherwise propagates [`ents_forge::issue::new`]'s own failures. // @relation(model.issue, roots.web-signing, roots.web-session, scope=function) pub async fn create<O>( State(state): State<Arc<AppState<O>>>, axum::Extension(session): axum::Extension<Session>, - Form(form): Form<NewForm>, + Form(pairs): Form<Vec<(String, String)>>, ) -> Result<impl IntoResponse> where O: Find + Write + Send + 'static, { - super::require_csrf(&session, &form.csrf)?; - let identity = state.identity.as_ref(); - let new = NewIssue { - title: form.title, - body: form.body, - state: form.state, - assignees: parse_members(&form.assignees), - labels: parse_labels(&form.labels), - }; - let (id, outcome) = issue::new( - state.refs.as_ref(), - &*state.objects(), - state.events.as_ref(), - new, - &crate::receive_identity!(identity, crate::pages::member_author(&session)), - state.mode, - )?; - crate::error::outcome_to_result(outcome)?; - Ok(Redirect::to(&format!("/issues/{id}"))) -} - -/// The form fields `POST /issues/{id}` accepts. Each field replaces its -/// counterpart on the issue; an empty `assignees`/`labels` leaves that set -/// unchanged (matching `git ents issue edit`'s own semantics), while `state` -/// is always applied. -#[derive(Debug, Deserialize)] -pub struct EditForm { - /// Replace the issue's state. - state: String, - /// Comma- or whitespace-separated assignees; empty leaves them. - #[serde(default)] - assignees: String, - /// Comma- or whitespace-separated labels; empty leaves them. - #[serde(default)] - labels: String, - /// The per-session CSRF token (`roots.web-session`). - csrf: String, + super::require_csrf(&session, crate::form::posted_csrf(&pairs))?; + dispatch(&state, &session, crate::form::parse_action("New", &pairs)?) } /// `POST /issues/{id}`: mutate `id`'s state, assignees, and/or labels -/// (`ents_forge::issue::edit`) as a signed mutation on the issue's own ref. +/// (`ents_forge::issue::edit`) as a signed mutation on the issue's own +/// ref. The posted fields are [`IssueAction::Edit`]'s own, the path's +/// `id` standing in for the CLI's positional argument. /// /// # Errors /// -/// [`crate::Error::BadCsrf`] if `form.csrf` does not match; otherwise -/// propagates [`ents_forge::issue::edit`]'s own failures (including -/// [`ents_forge::Error::NotFound`] when `id` names no issue). +/// [`crate::Error::BadCsrf`] if the posted token does not match; +/// otherwise propagates [`ents_forge::issue::edit`]'s own failures +/// (including [`ents_forge::Error::NotFound`] when `id` names no issue). // @relation(model.issue, roots.web-signing, roots.web-session, scope=function) pub async fn edit<O>( State(state): State<Arc<AppState<O>>>, axum::Extension(session): axum::Extension<Session>, Path(id): Path<String>, - Form(form): Form<EditForm>, + Form(mut pairs): Form<Vec<(String, String)>>, ) -> Result<impl IntoResponse> where O: Find + Write + Send + 'static, { - super::require_csrf(&session, &form.csrf)?; + super::require_csrf(&session, crate::form::posted_csrf(&pairs))?; + pairs.retain(|(name, _)| name != "id"); + pairs.push(("id".to_owned(), id)); + dispatch(&state, &session, crate::form::parse_action("Edit", &pairs)?) +} + +/// The issue dispatch table: each mutating [`IssueAction`] variant mapped +/// to the same `ents_forge::issue` call `git ents issue`'s own command +/// module makes, with the same edit semantics (an empty label/assignee +/// set leaves the field unchanged) -- `lens.parity`, the web as another +/// caller of the one business-logic path. +// @relation(model.issue, lens.parity, scope=function) +fn dispatch<O>(state: &AppState<O>, session: &Session, action: IssueAction) -> Result<Redirect> +where + O: Find + Write + Send + 'static, +{ let identity = state.identity.as_ref(); - let assignees = parse_members(&form.assignees); - let labels = parse_labels(&form.labels); - let edit = EditIssue { - state: Some(form.state), - assignees: (!assignees.is_empty()).then_some(assignees), - labels: (!labels.is_empty()).then_some(labels), - }; - let outcome = issue::edit( - state.refs.as_ref(), - &*state.objects(), - state.events.as_ref(), - &id, - edit, - &crate::receive_identity!(identity, crate::pages::member_author(&session)), - state.mode, - )?; - crate::error::outcome_to_result(outcome)?; - Ok(Redirect::to(&format!("/issues/{id}"))) + let identity = crate::receive_identity!(identity, crate::pages::member_author(session)); + match action { + IssueAction::New { + title, + body, + state: issue_state, + label, + assignee, + key: _, + } => { + let new = NewIssue { + title: title.unwrap_or_default(), + body: body.unwrap_or_default(), + state: issue_state, + assignees: assignee.into_iter().map(MemberId::new).collect(), + labels: label, + }; + let (id, outcome) = issue::new( + state.refs.as_ref(), + &*state.objects(), + state.events.as_ref(), + new, + &identity, + state.mode, + )?; + crate::error::outcome_to_result(outcome)?; + Ok(Redirect::to(&format!("/issues/{id}"))) + } + IssueAction::Edit { + id, + state: issue_state, + label, + assignee, + key: _, + } => { + let edit = EditIssue { + state: issue_state, + labels: (!label.is_empty()).then_some(label), + assignees: (!assignee.is_empty()) + .then(|| assignee.into_iter().map(MemberId::new).collect()), + }; + let outcome = issue::edit( + state.refs.as_ref(), + &*state.objects(), + state.events.as_ref(), + &id, + edit, + &identity, + state.mode, + )?; + crate::error::outcome_to_result(outcome)?; + Ok(Redirect::to(&format!("/issues/{id}"))) + } + _ => Err(crate::Error::InvalidArgument( + "not a form-backed issue action".to_owned(), + )), + } } /// The form fields `POST /issues/{id}/comment` accepts. @@ -410,45 +410,70 @@ /// a state outside the trio stays reachable through `git ents issue edit` /// or a direct edit, same as any other schema-level custom field. fn new_form(session: &Session, known_labels: &[String]) -> Markup { - html! { - form method="post" action="/issues" { - (super::csrf_input(session)) - label { "Title" input type="text" name="title"; } - div { - label { "State" } - (state_picker("open")) - } - label { "Assignees" input type="text" name="assignees" placeholder="alice, bob" list="members"; } - (label_picker(known_labels, &[])) - label { "Body" textarea name="body" {} } - div.composer-buttons { - a.composer-cancel href="/issues" { "Cancel" } - button type="submit" { "Open Issue" } - } - } - } + crate::form::action_form::<IssueAction>( + "New", + session, + &crate::form::Spec { + action: "/issues", + submit: "Open Issue", + cancel: Some("/issues"), + values: &[], + overrides: &[ + ( + "title", + html! { label { "Title" input type="text" name="title"; } }, + ), + ( + "state", + html! { div { label { "State" } (state_picker("open")) } }, + ), + ("label", label_picker(known_labels, &[])), + ( + "assignee", + html! { + label { + "Assignees" + input type="text" name="assignee" placeholder="alice, bob" list="members"; + } + }, + ), + ], + }, + ) } -/// The edit-issue form (`POST /issues/{id}`), its fields pre-filled from -/// the current issue. Its `state` field carries the same -/// [`state_picker`] as [`new_form`]'s, for the same reason (see -/// [`new_form`]'s own doc). +/// The edit-issue form (`POST /issues/{id}`), its fields derived from +/// [`IssueAction::Edit`]'s own shape (the positional `id` is the route's +/// path segment, so no control renders for it) and pre-filled from the +/// current issue. Its `state` field carries the same [`state_picker`] as +/// [`new_form`]'s, for the same reason (see [`new_form`]'s own doc). fn edit_form(session: &Session, issue: &ents_forge::Issue, known_labels: &[String]) -> Markup { - html! { - form method="post" action="" { - (super::csrf_input(session)) - div { - label { "State" } - (state_picker(&issue.state)) - } - label { - "Assignees" - input type="text" name="assignees" value=(join_members(&issue.assignees)) list="members"; - } - (label_picker(known_labels, &issue.labels)) - button type="submit" { "Save" } - } - } + crate::form::action_form::<IssueAction>( + "Edit", + session, + &crate::form::Spec { + action: "", + submit: "Save", + cancel: None, + values: &[], + overrides: &[ + ( + "state", + html! { div { label { "State" } (state_picker(&issue.state)) } }, + ), + ("label", label_picker(known_labels, &issue.labels)), + ( + "assignee", + html! { + label { + "Assignees" + input type="text" name="assignee" value=(join_members(&issue.assignees)) list="members"; + } + }, + ), + ], + }, + ) } /// The comment-on-this-issue form (`POST /issues/{id}/comment`). @@ -570,19 +595,16 @@ /// The labels field both [`new_form`] and [`edit_form`] render: `known`'s /// labels previewed as `.label-chip` (`.on` for one already in `current`), -/// then the actual control -- a single free-text `input[name=labels]` -/// pre-filled from `current` and completed by a [`label_datalist`] of -/// `known`'s own names. `model.issue`'s labels stay this one -/// comma/whitespace-separated string field end to end (`EditForm`/ -/// `NewForm`'s own `labels: String`, `parse_labels`), so unlike -/// [`state_picker`]'s single-choice radios, real independently-toggleable -/// checkboxes are not this control's shape here: a checkbox group needs -/// either a repeating-key field (breaking the scalar `labels` the create/ -/// edit handlers and `tests/router.rs`'s own `seed_issue` already commit -/// to) or client-side script to merge checkbox state into one text value -/// (excluded -- this crate works with no JS at all). The datalist gives -/// "pick existing" a real, working affordance; typing any other word is -/// "type-and-create". +/// then the actual control -- a single free-text `input[name=label]` +/// (spelled as [`IssueAction`]'s own `label` field, which +/// [`crate::form::parse_action`] splits on commas/whitespace) pre-filled +/// from `current` and completed by a [`label_datalist`] of `known`'s own +/// names. Unlike [`state_picker`]'s single-choice radios, real +/// independently-toggleable checkboxes are not this control's shape here: +/// a checkbox group needs client-side script to merge checkbox state into +/// the text value a no-JS post also carries (excluded -- this crate works +/// with no JS at all). The datalist gives "pick existing" a real, working +/// affordance; typing any other word is "type-and-create". fn label_picker(known: &[String], current: &[String]) -> Markup { html! { div { @@ -597,7 +619,7 @@ } input type="text" - name="labels" + name="label" value=(current.join(", ")) placeholder="bug, gate" list="issue-labels"; @@ -629,19 +651,3 @@ .join(", ") } -/// Parse a comma- or whitespace-separated list into members, dropping empty -/// segments so a trailing comma or extra spacing does not enroll a blank -/// assignee. -fn parse_members(text: &str) -> Vec<MemberId> { - parse_labels(text).into_iter().map(MemberId::new).collect() -} - -/// Parse a comma- or whitespace-separated list into labels, dropping empty -/// segments. -fn parse_labels(text: &str) -> Vec<String> { - text.split([',', ' ', '\t', '\n']) - .map(str::trim) - .filter(|segment| !segment.is_empty()) - .map(str::to_owned) - .collect() -}
crates/cli/ents-web/src/pages/reviews.rs @@ -175,7 +175,12 @@ /// composer, and -- only for the review's own author while it is still /// [`ReviewState::Active`] -- a withdraw control. Renders even for a /// withdrawn review (this module's own doc: a direct link stays live; only -/// [`list`]/[`reviews_sidebar`] hide a withdrawn row). +/// [`list`]/[`reviews_sidebar`] hide a withdrawn row). The metadata +/// `dl.entity-view` stays hand-rolled rather than +/// [`crate::render::view`]'s generic dump: every row is a domain widget or +/// derived value (verdict chip, state badge, a link to the target commit, +/// the reviewer's avatar, a relative time read from the review ref's own +/// tip commit -- not a field on the entity at all). /// /// # Errors ///
crates/cli/ents-web/src/form.rs @@ -1,0 +1,344 @@ +//! Action-shape-derived entity forms: the same `#[derive(Facet)]` action +//! enums the CLI parses (`ents_forge::issue::IssueAction`, +//! `ents_forge::review::ReviewAction`) drive a web form's controls and its +//! parse — one field list, declared on the action variant, obeyed by both +//! frontends (`lens.parity`) instead of a hand-declared form struct per +//! route. [`action_form`] renders one control per variant field +//! (`Vec` → comma-separated input, `bool` → checkbox, an `ents::compose` +//! field → textarea, anything else → text input), with per-field +//! overrides for the controls a page legitimately customizes (a state +//! picker, a verdict picker); [`parse_action`] reads the posted pairs +//! back into the action variant itself. CSRF stays +//! [`crate::pages::csrf_input`]'s hidden field and the handler's +//! `require_csrf` check; the PRG redirect stays the handler's own. +//! +//! Path-bound values (an `args::positional` id, a review's target) never +//! render as controls — the caller appends them to the posted pairs +//! before parsing. `PathBuf`-shaped fields (`--key`, a local signing-key +//! path no browser can supply) are skipped by both directions. + +use std::path::PathBuf; + +use facet::{Facet, Field, Type, UserType}; +use facet_reflect::Partial; +use maud::{Markup, html}; + +use crate::error::{Error, Result}; +use crate::session::Session; + +/// The web-varying parts of one derived form: where it posts, what its +/// submit control says, and the per-field prefills and overrides. +pub struct Spec<'a> { + /// The form's POST target. + pub action: &'a str, + /// The submit button's label. + pub submit: &'a str, + /// A cancel link's href, rendered beside the submit button. + pub cancel: Option<&'a str>, + /// Prefill values by field name (a `Vec` field prefills comma-joined). + pub values: &'a [(&'a str, String)], + /// Custom controls by field name; an empty override omits the field. + pub overrides: &'a [(&'a str, Markup)], +} + +/// Render the form `T`'s variant `variant` declares: one derived control +/// per web-suppliable field in declaration order, `spec.overrides` +/// slotted in place, the session's CSRF hidden field leading. +#[must_use] +pub fn action_form<T: Facet<'static>>(variant: &str, session: &Session, spec: &Spec<'_>) -> Markup { + let fields = variant_fields::<T>(variant).unwrap_or_default(); + html! { + form method="post" action=(spec.action) { + (crate::pages::csrf_input(session)) + @for field in fields { + @if let Some((_, markup)) = spec.overrides.iter().find(|(name, _)| *name == field.name) { + (markup) + } @else if let Some(markup) = control(field, value_of(spec, field.name)) { + (markup) + } + } + @if let Some(cancel) = spec.cancel { + div.composer-buttons { + a.composer-cancel href=(cancel) { "Cancel" } + button type="submit" { (spec.submit) } + } + } @else { + button type="submit" { (spec.submit) } + } + } + } +} + +/// Parse posted `pairs` (path-bound values appended by the caller) into +/// `T`'s variant `variant`, by the same shape-to-control mapping +/// [`action_form`] renders: a `Vec` splits on commas/whitespace across +/// every posted occurrence, an empty `Option<String>` is `None`, a `bool` +/// is its checkbox's presence, and an unposted field takes its declared +/// default. Unknown pairs (the CSRF token, a `return_to`) are ignored. +/// +/// # Errors +/// +/// [`Error::InvalidArgument`] if `T` has no such variant, a field's shape +/// is not one this mapping speaks, or the value cannot be set. +pub fn parse_action<T: Facet<'static>>(variant: &str, pairs: &[(String, String)]) -> Result<T> { + let malformed = |source: &dyn std::fmt::Display| { + Error::InvalidArgument(format!("malformed {variant} form: {source}")) + }; + let fields = variant_fields::<T>(variant)?; + let mut partial = Partial::alloc::<T>() + .map_err(|source| malformed(&source))? + .select_variant_named(variant) + .map_err(|source| malformed(&source))?; + for (index, field) in fields.iter().enumerate() { + let posted: Vec<&str> = pairs + .iter() + .filter(|(name, _)| name == field.name) + .map(|(_, value)| value.as_str()) + .collect(); + let shape = field.shape(); + partial = if shape.is_type::<Vec<String>>() { + partial.set_field(field.name, split_list(&posted)) + } else if let Some(first) = posted.first() { + if shape.is_type::<String>() { + partial.set_field(field.name, (*first).to_owned()) + } else if shape.is_type::<Option<String>>() { + let value = posted.iter().find(|value| !value.trim().is_empty()); + partial.set_field(field.name, value.map(|value| (*value).to_owned())) + } else if shape.is_type::<bool>() { + partial.set_field(field.name, matches!(*first, "true" | "on" | "1")) + } else { + return Err(Error::InvalidArgument(format!( + "unsupported form field: {}", + field.name + ))); + } + } else { + partial.set_nth_field_to_default(index) + } + .map_err(|source| malformed(&source))?; + } + partial + .build() + .map_err(|source| malformed(&source))? + .materialize::<T>() + .map_err(|source| malformed(&source)) +} + +/// The posted CSRF token, or empty when the field is absent — feeding +/// `require_csrf`, which then refuses the empty token like any other +/// mismatch. +#[must_use] +pub fn posted_csrf(pairs: &[(String, String)]) -> &str { + pairs + .iter() + .find(|(name, _)| name == crate::session::CSRF_FIELD) + .map_or("", |(_, value)| value.as_str()) +} + +/// `variant`'s field list on the action enum `T`. +fn variant_fields<T: Facet<'static>>(variant: &str) -> Result<&'static [Field]> { + let Type::User(UserType::Enum(shape)) = T::SHAPE.ty else { + return Err(Error::InvalidArgument(format!( + "{} is not an action enum", + T::SHAPE + ))); + }; + shape + .variants + .iter() + .find(|candidate| candidate.name == variant) + .map(|found| found.data.fields) + .ok_or_else(|| Error::InvalidArgument(format!("no such action: {variant}"))) +} + +/// `field`'s derived control, or `None` for a field the web never +/// renders: an `args::positional` value (bound into the route's own +/// path) or a `PathBuf` (a local file path no browser form supplies). +fn control(field: &Field, value: Option<&str>) -> Option<Markup> { + if field.has_attr(Some("args"), "positional") { + return None; + } + let shape = field.shape(); + if shape.is_type::<PathBuf>() || shape.is_type::<Option<PathBuf>>() { + return None; + } + let name = field.name; + let label = title_case(name); + Some(if shape.is_type::<bool>() { + html! { + label { + input type="checkbox" name=(name) checked[value == Some("true")]; + " " (label) + } + } + } else if shape.is_type::<Vec<String>>() { + html! { + label { (label) input type="text" name=(name) value=[value] placeholder="a, b"; } + } + } else if field.has_attr(Some("ents"), "compose") { + html! { + label { (label) textarea name=(name) { @if let Some(value) = value { (value) } } } + } + } else { + html! { + label { (label) input type="text" name=(name) value=[value]; } + } + }) +} + +/// `spec.values`'s prefill for `name`, if any. +fn value_of<'a>(spec: &'a Spec<'_>, name: &str) -> Option<&'a str> { + spec.values + .iter() + .find(|(field, _)| *field == name) + .map(|(_, value)| value.as_str()) +} + +/// Every posted occurrence split on commas and whitespace, trimmed, +/// empties dropped — one text input carries a whole `Vec` field. +fn split_list(posted: &[&str]) -> Vec<String> { + posted + .iter() + .flat_map(|value| value.split([',', ' ', '\t', '\n'])) + .map(str::trim) + .filter(|segment| !segment.is_empty()) + .map(str::to_owned) + .collect() +} + +/// A field name as its control's label: first letter upper-cased. +fn title_case(name: &str) -> String { + let mut chars = name.chars(); + chars.next().map_or_else(String::new, |first| { + first.to_uppercase().chain(chars).collect() + }) +} + +#[cfg(test)] +mod tests { + #![allow(clippy::expect_used, clippy::panic, reason = "unit test")] + + use ents_forge::issue::IssueAction; + use ents_forge::review::ReviewAction; + use rstest::rstest; + + use super::*; + + fn session() -> Session { + Session { + csrf: "tok".to_owned(), + member: None, + } + } + + fn pairs(entries: &[(&str, &str)]) -> Vec<(String, String)> { + entries + .iter() + .map(|(name, value)| ((*name).to_owned(), (*value).to_owned())) + .collect() + } + + /// The action variant's own shape decides the controls: a compose + /// field is a textarea, a `Vec` a comma input, a `PathBuf` (`--key`) + /// and a positional id never render, and an override slots in place. + #[rstest] + // @relation(lens.parity, scope=function, role=Verifies) + fn action_form_derives_controls_from_the_variant_shape() { + let markup = action_form::<IssueAction>( + "New", + &session(), + &Spec { + action: "/issues", + submit: "Open Issue", + cancel: None, + values: &[], + overrides: &[("state", html! { span.custom-state {} })], + }, + ) + .into_string(); + assert!(markup.contains("<textarea name=\"body\">")); + assert!(markup.contains("name=\"label\"") && markup.contains("name=\"assignee\"")); + assert!(markup.contains("custom-state") && !markup.contains("name=\"state\"")); + assert!(!markup.contains("name=\"key\""), "{markup}"); + assert!(markup.contains("name=\"csrf\" value=\"tok\"")); + + let edit = action_form::<IssueAction>( + "Edit", + &session(), + &Spec { + action: "", + submit: "Save", + cancel: None, + values: &[("label", "bug, gate".to_owned())], + overrides: &[], + }, + ) + .into_string(); + assert!(!edit.contains("name=\"id\""), "positional ids are path-bound"); + assert!(edit.contains("value=\"bug, gate\"")); + } + + /// The same shape parses the post back: `Vec` fields split on commas + /// and whitespace, an empty optional is `None`, the unposted `--key` + /// defaults, and unknown pairs (csrf) are ignored. + #[rstest] + // @relation(lens.parity, scope=function, role=Verifies) + fn parse_action_reads_the_posted_pairs_into_the_variant() { + let action: IssueAction = parse_action( + "New", + &pairs(&[ + ("title", "gate rejects a valid signature"), + ("body", ""), + ("state", "open"), + ("label", "bug, gate"), + ("assignee", "jdc alice"), + ("csrf", "tok"), + ]), + ) + .expect("parses"); + let IssueAction::New { + title, + body, + state, + label, + assignee, + key, + } = action + else { + panic!("wrong variant"); + }; + assert_eq!(title.as_deref(), Some("gate rejects a valid signature")); + assert_eq!(body, None); + assert_eq!(state, "open"); + assert_eq!(label, vec!["bug".to_owned(), "gate".to_owned()]); + assert_eq!(assignee, vec!["jdc".to_owned(), "alice".to_owned()]); + assert_eq!(key, None); + } + + /// An unposted field takes the declaration's own default — the same + /// `default = "open"` the CLI applies to an omitted flag. + #[rstest] + // @relation(lens.parity, scope=function, role=Verifies) + fn parse_action_defaults_an_unposted_field_from_the_declaration() { + let action: ReviewAction = + parse_action("New", &pairs(&[("verdict", "approve")])).expect("parses"); + let ReviewAction::New { + target, + verdict, + body, + key, + } = action + else { + panic!("wrong variant"); + }; + assert_eq!(target, "HEAD"); + assert_eq!(verdict, "approve"); + assert_eq!(body, None); + assert_eq!(key, None); + } + + #[rstest] + fn parse_action_refuses_an_unknown_variant() { + assert!(parse_action::<IssueAction>("Explode", &[]).is_err()); + } +}