git-ents.gitmain
⌘K
foforge
commit c62202a
roots: add reply, resolve, and reopen actions to comment views

Every comment now carries reply and resolve/reopen actions on its own GET /comments/{id} page, as CSRF-checked signed POSTs that call ents_forge::comment::{reply,resolve,reopen} — the web is another caller of the same library funcs, never a second thread-building or state-mutation path. The comment page shows its state (open/resolved) and offers the opposite action. action_forms is factored out for reuse by the issue and review threads that follow.

Implements model.comment-state (state shown; resolve/reopen) and model.comment-thread (reply) in the web layer, each signed through the injected identity (roots.web-signing) on behalf of an authenticated session (roots.web-session).

Assisted-by: Claude:claude-opus-4-8

Joseph D. Carpinelli · 1 month ago

Reviews

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

Start a review

verdict

crates/cli/ents-web/src/router.rs @@ -20,7 +20,7 @@ use axum::http::{HeaderValue, header}; use axum::middleware::{self, Next}; use axum::response::{IntoResponse, Response}; -use axum::routing::get; +use axum::routing::{get, post}; use gix_object::{Find, Write}; use crate::assets; @@ -63,6 +63,12 @@ get(pages::comments::list::<O>).post(pages::comments::add::<O>), ) .route("/comments/{id}", get(pages::comments::show::<O>)) + .route("/comments/{id}/reply", post(pages::comments::reply::<O>)) + .route( + "/comments/{id}/resolve", + post(pages::comments::resolve::<O>), + ) + .route("/comments/{id}/reopen", post(pages::comments::reopen::<O>)) .route("/inbox", get(pages::inbox::list::<O>)) .route("/style.css", get(style)) .route("/ents.js", get(script))
crates/cli/ents-web/tests/router.rs @@ -233,6 +233,25 @@ ]); } +/// `GET path` and return its body decoded as UTF-8, asserting a 200 -- +/// the read-back half of the many "mutate, then observe" tests below, so +/// each does not re-spell the collect/decode dance inline. +async fn get_body(router: &axum::Router, path: &str) -> String { + let response = router + .clone() + .oneshot(Request::get(path).body(Body::empty()).expect("request")) + .await + .expect("in-process call"); + assert_eq!(response.status(), StatusCode::OK, "GET {path}"); + let bytes = response + .into_body() + .collect() + .await + .expect("body") + .to_bytes(); + String::from_utf8(bytes.to_vec()).expect("utf8 html") +} + /// Establish a session against `router` via a `GET` to `path`, returning /// its cookie header and CSRF token -- the same extraction /// `csrf_is_required_and_checked_on_every_state_changing_request` performs @@ -274,7 +293,9 @@ /// at `rev` -- what the comment tests below seed a real comment through, /// exercising the actual signed-write path (`ents_forge::comment::add`) /// rather than poking the ref store directly. Asserts the write succeeded -/// (a redirect to the new comment's own page). +/// (a redirect to the new comment's own page) and returns the new comment's +/// id, read from that redirect's `Location` (`/comments/<id>`) -- what the +/// thread-action tests below drive reply/resolve/reopen against. async fn seed_comment( router: &axum::Router, state: &AppState<ObjectStore>, @@ -282,7 +303,7 @@ body: &str, lines: &str, rev: &str, -) { +) -> String { let (cookie, csrf) = session_cookie_and_csrf(router, state, "/comments").await; let form = format!( "path={path}&body={}&lines={lines}&rev={rev}&csrf={csrf}", @@ -304,6 +325,15 @@ "comment write did not succeed: {:?}", response.status() ); + response + .headers() + .get(header::LOCATION) + .expect("a successful comment write redirects to the new comment") + .to_str() + .expect("ascii") + .strip_prefix("/comments/") + .expect("redirect targets /comments/<id>") + .to_owned() } /// `roots.local`: this crate's route table never exposes git's own @@ -1184,6 +1214,123 @@ assert!(body.contains("class=\"outdated\"")); } +/// `model.comment-state`, `roots.web-session`: a comment resolves and +/// reopens through CSRF-checked, signed `POST`s -- each an +/// `ents_forge::comment::{resolve,reopen}` call -- and its `GET +/// /comments/{id}` page reflects the new state and offers the opposite +/// action each time. The wrong CSRF token is refused, exactly as every +/// other state-changing route in this crate refuses one. +#[tokio::test] +// @relation(model.comment-state, roots.web-signing, roots.web-session, scope=function, role=Verifies) +async fn a_comment_resolves_and_reopens_through_csrf_checked_posts() { + let dir = seed_repo(&[("src/main.rs", "line 1\nline 2\nline 3\n")]); + let state = build_state_at( + FixtureIdentity { + name: "commenter", + key: Keypair::from_seed(1), + }, + dir.path().to_owned(), + ); + let router = ents_web::router(state.clone()); + let id = seed_comment(&router, &state, "src/main.rs", "look here", "2:2", "HEAD").await; + let page = format!("/comments/{id}"); + let (cookie, csrf) = session_cookie_and_csrf(&router, &state, &page).await; + + // A fresh comment lists open and offers "resolve". + let body = get_body(&router, &page).await; + assert!(body.contains("resolve"), "an open comment offers resolve"); + + // The wrong token is refused. + let refused = router + .clone() + .oneshot( + Request::post(format!("/comments/{id}/resolve")) + .header(header::CONTENT_TYPE, "application/x-www-form-urlencoded") + .header(header::COOKIE, cookie.clone()) + .body(Body::from("csrf=not-the-token")) + .expect("request"), + ) + .await + .expect("in-process call"); + assert_eq!(refused.status(), StatusCode::BAD_REQUEST); + + // The right token resolves it. + let resolved = router + .clone() + .oneshot( + Request::post(format!("/comments/{id}/resolve")) + .header(header::CONTENT_TYPE, "application/x-www-form-urlencoded") + .header(header::COOKIE, cookie.clone()) + .body(Body::from(format!("csrf={csrf}"))) + .expect("request"), + ) + .await + .expect("in-process call"); + assert!(resolved.status().is_redirection()); + let body = get_body(&router, &page).await; + assert!(body.contains("resolved"), "the comment now reads resolved"); + assert!(body.contains("reopen"), "a resolved comment offers reopen"); + + // Reopen returns it to open. + let reopened = router + .clone() + .oneshot( + Request::post(format!("/comments/{id}/reopen")) + .header(header::CONTENT_TYPE, "application/x-www-form-urlencoded") + .header(header::COOKIE, cookie) + .body(Body::from(format!("csrf={csrf}"))) + .expect("request"), + ) + .await + .expect("in-process call"); + assert!(reopened.status().is_redirection()); + let body = get_body(&router, &page).await; + assert!( + body.contains(">open<") || body.contains("resolve"), + "the comment offers resolve again once reopened" + ); +} + +/// `model.comment-thread`: a reply through `POST /comments/{id}/reply` is a +/// second comment (`ents_forge::comment::reply`) -- after it lands, the +/// comment index lists two comments where the seed left one. +#[tokio::test] +// @relation(model.comment-thread, roots.web-signing, roots.web-session, scope=function, role=Verifies) +async fn a_reply_creates_a_threaded_comment_through_a_signed_post() { + let dir = seed_repo(&[("src/main.rs", "line 1\nline 2\nline 3\n")]); + let state = build_state_at( + FixtureIdentity { + name: "commenter", + key: Keypair::from_seed(1), + }, + dir.path().to_owned(), + ); + let router = ents_web::router(state.clone()); + let id = seed_comment(&router, &state, "src/main.rs", "the parent", "2:2", "HEAD").await; + let (cookie, csrf) = session_cookie_and_csrf(&router, &state, "/comments").await; + + let reply = router + .clone() + .oneshot( + Request::post(format!("/comments/{id}/reply")) + .header(header::CONTENT_TYPE, "application/x-www-form-urlencoded") + .header(header::COOKIE, cookie) + .body(Body::from(format!("body=a+reply+here&csrf={csrf}"))) + .expect("request"), + ) + .await + .expect("in-process call"); + assert!(reply.status().is_redirection(), "{:?}", reply.status()); + + let body = get_body(&router, "/comments").await; + let count = body.matches("/comments/").count(); + assert!( + count >= 2, + "the index lists the parent and its reply, got {count} links" + ); + assert!(body.contains("a reply here"), "the reply's body renders"); +} + /// A raw-source blob view carries the client-side hooks `assets/ents.js` /// needs: `div.blob`'s own `data-path`/`data-rev` (the latter a full /// 40-hex `HEAD` commit oid, not the string `"HEAD"`), and a
crates/cli/ents-web/src/pages/comments.rs @@ -110,7 +110,11 @@ } /// `GET /comments/{id}?rev=...`: the comment's body, its anchor, and the -/// projection of that anchor onto `rev` (`anchor.projection`). +/// projection of that anchor onto `rev` (`anchor.projection`). Its state +/// (`model.comment-state`) and the reply/resolve/reopen actions +/// ([`action_forms`], `model.comment-thread`, `model.comment-state`) render +/// alongside, so a comment is a conversation from its own page and not only +/// from an issue's or a review's. /// /// # Errors /// @@ -118,6 +122,7 @@ /// `id` has no comment ref. pub async fn show<O>( State(state): State<Arc<AppState<O>>>, + axum::Extension(session): axum::Extension<Session>, Path(id): Path<String>, PathQuery(query): PathQuery<ShowQuery>, ) -> Result<maud::Markup> @@ -132,6 +137,8 @@ &query.rev, false, )?; + let resolved = comment.state == "resolved"; + let return_to = format!("/comments/{id}"); Ok(super::layout( &super::RepoHeader::from_state(&state), &super::identity_label(&state), @@ -153,10 +160,183 @@ } dt { "body" } dd { (comment.body) } } + (action_forms(&session, &id, resolved, &return_to)) }, )) } +/// The form fields the reply route accepts. +#[derive(Debug, Deserialize)] +pub struct ReplyForm { + /// The reply's body text. + body: String, + /// The per-session CSRF token (`roots.web-session`). + csrf: String, + /// Where to send the browser back to after the reply lands + /// ([`redirect_back`]) -- the issue, review, or comment page the reply + /// was composed on. + #[serde(default)] + return_to: String, +} + +/// The form fields the resolve and reopen routes accept: a CSRF token and a +/// return path, no body. +#[derive(Debug, Deserialize)] +pub struct ActionForm { + /// The per-session CSRF token (`roots.web-session`). + csrf: String, + /// Where to send the browser back to ([`redirect_back`]). + #[serde(default)] + return_to: String, +} + +/// `POST /comments/{id}/reply`: a reply to `id` (`model.comment-thread`), +/// signed (`roots.web-signing`) on behalf of the current session +/// (`roots.web-session`) -- a caller of [`ents_forge::comment::reply`], +/// never a second thread-building path. +/// +/// # Errors +/// +/// [`crate::Error::BadCsrf`] if `form.csrf` does not match; otherwise +/// propagates [`ents_forge::comment::reply`]'s own failures (including +/// [`ents_forge::Error::NotFound`] when `id` names no comment). +// @relation(model.comment-thread, roots.web-signing, roots.web-session, scope=function) +pub async fn reply<O>( + State(state): State<Arc<AppState<O>>>, + axum::Extension(session): axum::Extension<Session>, + Path(id): Path<String>, + Form(form): Form<ReplyForm>, +) -> Result<impl IntoResponse> +where + O: Find + Write + Send + 'static, +{ + super::require_csrf(&session, &form.csrf)?; + let identity = state.identity.as_ref(); + let (_reply_id, outcome) = comment::reply( + state.refs.as_ref(), + &*state.objects(), + state.events.as_ref(), + &id, + form.body, + &crate::receive_identity!(identity), + state.mode, + )?; + crate::error::outcome_to_result(outcome)?; + Ok(redirect_back(&form.return_to, &id)) +} + +/// `POST /comments/{id}/resolve`: record state `resolved` on `id` +/// (`model.comment-state`), signed on behalf of the current session. +/// +/// # Errors +/// +/// [`crate::Error::BadCsrf`] if `form.csrf` does not match; otherwise +/// propagates [`ents_forge::comment::resolve`]'s own failures. +// @relation(model.comment-state, roots.web-signing, roots.web-session, scope=function) +pub async fn resolve<O>( + State(state): State<Arc<AppState<O>>>, + axum::Extension(session): axum::Extension<Session>, + Path(id): Path<String>, + Form(form): Form<ActionForm>, +) -> Result<impl IntoResponse> +where + O: Find + Write + Send + 'static, +{ + super::require_csrf(&session, &form.csrf)?; + let identity = state.identity.as_ref(); + let outcome = comment::resolve( + state.refs.as_ref(), + &*state.objects(), + state.events.as_ref(), + &id, + &crate::receive_identity!(identity), + state.mode, + )?; + crate::error::outcome_to_result(outcome)?; + Ok(redirect_back(&form.return_to, &id)) +} + +/// `POST /comments/{id}/reopen`: record state `open` on `id` again +/// (`model.comment-state`), the way [`resolve`] records `resolved`. +/// +/// # Errors +/// +/// [`crate::Error::BadCsrf`] if `form.csrf` does not match; otherwise +/// propagates [`ents_forge::comment::reopen`]'s own failures. +// @relation(model.comment-state, roots.web-signing, roots.web-session, scope=function) +pub async fn reopen<O>( + State(state): State<Arc<AppState<O>>>, + axum::Extension(session): axum::Extension<Session>, + Path(id): Path<String>, + Form(form): Form<ActionForm>, +) -> Result<impl IntoResponse> +where + O: Find + Write + Send + 'static, +{ + super::require_csrf(&session, &form.csrf)?; + let identity = state.identity.as_ref(); + let outcome = comment::reopen( + state.refs.as_ref(), + &*state.objects(), + state.events.as_ref(), + &id, + &crate::receive_identity!(identity), + state.mode, + )?; + crate::error::outcome_to_result(outcome)?; + Ok(redirect_back(&form.return_to, &id)) +} + +/// Where a reply/resolve/reopen sends the browser after the mutation lands: +/// back to `return_to` when it is a same-origin path (the issue, review, or +/// comment page the action was taken on), or the comment's own page as a +/// safe fallback. Only a value beginning with `/` is honored, so a crafted +/// `return_to` can never redirect off-site. +fn redirect_back(return_to: &str, id: &str) -> Redirect { + if return_to.starts_with('/') { + Redirect::to(return_to) + } else { + Redirect::to(&format!("/comments/{id}")) + } +} + +/// The reply and resolve/reopen action forms every comment carries, on its +/// own page and in an issue's or review's thread alike: a reply composer +/// (`model.comment-thread`) and a single state toggle showing `resolve` +/// when open or `reopen` when resolved (`model.comment-state`). `return_to` +/// is echoed into a hidden field so [`redirect_back`] can return to +/// whichever page rendered these forms. +pub(crate) fn action_forms( + session: &Session, + id: &str, + resolved: bool, + return_to: &str, +) -> maud::Markup { + html! { + div.comment-actions { + form method="post" action=(format!("/comments/{id}/reply")) { + (super::csrf_input(session)) + input type="hidden" name="return_to" value=(return_to); + label { "reply" textarea name="body" {} } + button type="submit" { "reply" } + } + @if resolved { + form method="post" action=(format!("/comments/{id}/reopen")) { + (super::csrf_input(session)) + input type="hidden" name="return_to" value=(return_to); + button type="submit" { "reopen" } + } + } @else { + form method="post" action=(format!("/comments/{id}/resolve")) { + (super::csrf_input(session)) + input type="hidden" name="return_to" value=(return_to); + button type="submit" { "resolve" } + } + } + } + } +} + /// The form fields `POST /comments` accepts. #[derive(Debug, Deserialize)] pub struct AddForm {