git ents: web, review: issue-shaped review detail page

crates/cli/ents-web/src/router.rs @@ -74,7 +74,15 @@ "/reviews/{target}/{member}/comment", post(pages::commits::review_comment::<O>), ) + .route( + "/reviews/{target}/{member}/withdraw", + post(pages::reviews::withdraw::<O>), + ) .route("/reviews", get(pages::reviews::list::<O>)) + .route( + "/reviews/{target}/{member}", + get(pages::reviews::show::<O>), + ) .route("/files", get(pages::files::root::<O>)) .route("/files/{*path}", get(pages::files::show::<O>)) .route("/meta", get(pages::meta::show::<O>))

crates/cli/ents-web/tests/router.rs @@ -4249,3 +4249,209 @@ assert!(body.contains("alice"), "active review still lists: {body}"); assert!(!body.contains("bob"), "withdrawn review must not render: {body}"); } + +/// `GET /reviews/{target}/{member}` (`crate::pages::reviews::show`): a +/// review started through the commit page's own form (`POST +/// /commit/{oid}/review`) gets its own detail page -- the verdict chip, an +/// `active` state badge, the rendered body, and (since the viewing +/// identity is the review's own author) a withdraw control -- and the +/// Reviews split's own sidebar (`reviews_sidebar`) links to it, newest +/// first, beside `GET /reviews`'s own aggregate listing. +#[tokio::test] +// @relation(model.review, lens.parity, scope=function, role=Verifies) +async fn review_detail_page_renders_for_its_own_author_with_a_withdraw_control() { + let dir = seed_repo(&[("src/main.rs", "fn main() {}\n")]); + let oid = head_oid(dir.path()); + let state = build_state_at( + FixtureIdentity { + name: "reviewer", + key: Keypair::from_seed(1), + }, + dir.path().to_owned(), + ); + let router = ents_web::router(state.clone()); + + let (cookie, csrf) = session_cookie_and_csrf(&router, &state, &format!("/commit/{oid}")).await; + let started = router + .clone() + .oneshot( + Request::post(format!("/commit/{oid}/review")) + .header(header::CONTENT_TYPE, "application/x-www-form-urlencoded") + .header(header::COOKIE, cookie.clone()) + .body(Body::from(format!( + "verdict=approve&body=looks+good+to+me&csrf={csrf}" + ))) + .expect("request"), + ) + .await + .expect("in-process call"); + assert!(started.status().is_redirection(), "{:?}", started.status()); + + let commit_page = get_body(&router, &format!("/commit/{oid}")).await; + let review_id = commit_page + .split_once("/reviews/") + .and_then(|(_, rest)| rest.split_once("/comment")) + .map(|(id, _)| id) + .expect("a review comment form links in") + .to_owned(); + + // The sidebar/list both link to the review's own detail page. + let list = get_body(&router, "/reviews").await; + assert!( + list.contains(&format!("href=\"/reviews/{review_id}\"")), + "the sidebar links the active review's own detail page: {list}" + ); + + let detail = get_body(&router, &format!("/reviews/{review_id}")).await; + assert!( + detail.contains("class=\"verdict verdict-approve\""), + "the verdict chip renders: {detail}" + ); + assert!( + detail.contains("looks good to me"), + "the review body renders as its own doc-body: {detail}" + ); + assert!( + detail.contains(">active<"), + "the state badge names the review active: {detail}" + ); + assert!( + detail.contains(&format!("action=\"/reviews/{review_id}/withdraw\"")), + "the review's own author sees a withdraw control: {detail}" + ); + + // Withdrawing redirects back to the same detail page, which still + // renders -- but now with the withdrawn indicator instead of the + // button -- while the sidebar and the aggregate list both drop it. + let (cookie, csrf) = session_cookie_and_csrf(&router, &state, &detail_path(&review_id)).await; + let withdrawn = router + .clone() + .oneshot( + Request::post(format!("/reviews/{review_id}/withdraw")) + .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!( + withdrawn.status().is_redirection(), + "{:?}", + withdrawn.status() + ); + + let detail_after = get_body(&router, &format!("/reviews/{review_id}")).await; + assert!( + detail_after.contains("withdrawn"), + "the withdrawn review's own page states so plainly: {detail_after}" + ); + assert!( + !detail_after.contains(&format!("action=\"/reviews/{review_id}/withdraw\"")), + "a withdrawn review no longer offers the withdraw control: {detail_after}" + ); + + let list_after = get_body(&router, "/reviews").await; + assert!( + !list_after.contains(&format!("href=\"/reviews/{review_id}\"")), + "the withdrawn review drops out of the aggregate list: {list_after}" + ); +} + +/// `format!("/reviews/{{review_id}}")`, spelled once so +/// [`review_detail_page_renders_for_its_own_author_with_a_withdraw_control`] +/// can reuse the same detail path both to fetch a fresh CSRF token and to +/// `POST` the withdraw form against it. +fn detail_path(review_id: &str) -> String { + format!("/reviews/{review_id}") +} + +/// `crate::pages::reviews::show`'s withdraw control only ever renders for +/// the review's *own* author -- a second identity viewing the same +/// still-active review sees the metadata card and the thread, but no +/// withdraw form at all, exactly as `commits::reviews_section` never lets +/// one member's page render a button that would fail +/// `ents_forge::review::withdraw`'s own author check. +#[tokio::test] +async fn review_detail_page_hides_the_withdraw_control_from_a_non_author() { + let refs = MemRefStore::default(); + let objects = ObjectStore::default(); + let target = "0123456789abcdef0123456789abcdef01234567"; + let reviewed = gix_hash::ObjectId::from_hex(target.as_bytes()).expect("valid hex"); + + let review_ref = + ents_model::namespace::review_ref(target, &MemberId::new("carol")).expect("valid"); + write_meta_entity( + &refs, + &objects, + review_ref, + &ents_forge::review::Review::new( + reviewed, + ents_forge::review::Verdict::Approve, + "review body from carol", + ), + None, + 100, + ); + + let state = build_state_with( + FixtureIdentity { + name: "onlooker", + key: Keypair::from_seed(2), + }, + refs, + objects, + ); + let router = ents_web::router(state); + + let detail = get_body(&router, &format!("/reviews/{target}/carol")).await; + assert!( + detail.contains("review body from carol"), + "the review still renders for a non-author viewer: {detail}" + ); + assert!( + !detail.contains("/withdraw\""), + "a non-author viewer sees no withdraw control: {detail}" + ); +} + +/// `POST /reviews/{target}/{member}/withdraw` is a state-changing route +/// gated the same way every other mutation in this crate is +/// (`roots.web-session`): no CSRF field at all is rejected outright, +/// regardless of whether the named review even exists. +#[tokio::test] +async fn review_withdraw_is_rejected_without_a_valid_csrf_token() { + let state = build_state(FixtureIdentity { + name: "local-user", + key: Keypair::from_seed(1), + }); + let router = ents_web::router(Arc::clone(&state)); + + let no_csrf = router + .clone() + .oneshot( + Request::post("/reviews/0123456789abcdef0123456789abcdef01234567/carol/withdraw") + .header(header::CONTENT_TYPE, "application/x-www-form-urlencoded") + .body(Body::empty()) + .expect("request"), + ) + .await + .expect("in-process call"); + assert!( + !no_csrf.status().is_success() && !no_csrf.status().is_redirection(), + "a POST with no csrf field must not withdraw a review" + ); + + let (cookie, _csrf) = session_cookie_and_csrf(&router, &state, "/reviews").await; + let wrong = router + .oneshot( + Request::post("/reviews/0123456789abcdef0123456789abcdef01234567/carol/withdraw") + .header(header::CONTENT_TYPE, "application/x-www-form-urlencoded") + .header(header::COOKIE, cookie) + .body(Body::from("csrf=not-the-token")) + .expect("request"), + ) + .await + .expect("in-process call"); + assert_eq!(wrong.status(), StatusCode::BAD_REQUEST); +}

crates/cli/ents-web/src/pages/commits.rs @@ -534,8 +534,11 @@ /// The comment-on-this-review form (`POST /reviews/{target}/{member}/comment`): /// a contextual comment naming `reviews/<target>/<member>` /// (`model.comment-context`), so a review's discussion can start from the -/// web and not only the CLI or lens. -fn review_comment_form( +/// web and not only the CLI or lens. Shared with +/// [`super::reviews::show`]'s own review detail page -- the same composer, +/// not a second one, whether the review's thread renders on its home +/// commit's page or on its own. +pub(crate) fn review_comment_form( session: &Session, target: &str, member: &ents_model::MemberId, @@ -771,7 +774,7 @@ O: Find + Write + Send + 'static, { super::require_csrf(&session, &form.csrf)?; - let member = reviewer_member_id(&state); + let member = super::reviewer_member_id(&state); let identity = state.identity.as_ref(); let new = ents_forge::review::NewReview { target: oid.clone(), @@ -794,37 +797,6 @@ Ok(Redirect::to(&format!("/commit/{oid}"))) } -/// The acting session's member id -- the composite review key's -/// `<member>` segment -- resolved the same way -/// [`super::account::resolve_member_by_key`] does, falling back to a short -/// hash of the public key when no enrolled member matches: mirrors -/// `git_ents::commands::serve::build_state`'s identical fallback -/// (`roots.web-signing`: an unenrolled local identity may still review, -/// exactly as it may still browse and comment). -fn reviewer_member_id<O: Find>(state: &AppState<O>) -> ents_model::MemberId { - let pubkey = state.identity.public_openssh(); - super::account::resolve_member_by_key(state, &pubkey) - .map(|(id, _member)| id) - .unwrap_or_else(|_source| ents_model::MemberId::new(short_key_fingerprint(&pubkey))) -} - -/// The first twelve characters of `pubkey`'s key-material token -- mirrors -/// `git_ents::commands::short_fingerprint`'s identical fallback label. -fn short_key_fingerprint(pubkey: &str) -> String { - let hex: String = pubkey - .split_whitespace() - .nth(1) - .unwrap_or(pubkey) - .chars() - .take(12) - .collect(); - if hex.is_empty() { - "member".to_owned() - } else { - hex - } -} - /// Validate `text` as a full, well-formed object id -- hex characters only, /// at the exact length the served repository's hash kind expects (this /// page does not resolve abbreviated prefixes; [`super::commits::list`]'s

crates/cli/ents-web/src/pages/mod.rs @@ -19,12 +19,17 @@ //! and [`agents`] are rail items of their own -- `Tab::Commits`, //! `Tab::Reviews`, `Tab::Issues`, and `Tab::Agents` (Agents, //! `docs/agent-sessions-plan.adoc`'s Phase 3) in [`layout`]'s icon rail, -//! alongside the dashboard, code, threads, and meta items. `reviews::list` -//! (`GET /reviews`) is a read-only aggregate across every commit's own -//! reviews (`commits::reviews_section` renders the same -//! [`ents_forge::review`] entities scoped to one commit; this module has no -//! writes of its own -- every mutation still posts through `commits`'s own -//! routes). [`search`] +//! alongside the dashboard, code, threads, and meta items. [`reviews`]'s +//! `list` (`GET /reviews`) and `show` (`GET /reviews/{target}/{member}`) +//! are a read-only aggregate and a per-review detail page over the same +//! [`ents_forge::review`] entities [`commits::reviews_section`] renders +//! scoped to one commit; starting a review still only ever posts through +//! `commits`'s own route (`POST /commit/{oid}/review`), but withdrawing one +//! (`POST /reviews/{target}/{member}/withdraw`) and commenting on one +//! (`POST /reviews/{target}/{member}/comment`, `commits::review_comment`) +//! are reachable from either page -- a review's own page and its home +//! commit's page render the identical thread and composer, never two. +//! [`search`] //! renders with no rail item active at all; it is reached from the //! `.wb-bar`'s own `.palette` search form rather than any rail item. @@ -651,3 +656,39 @@ }); format!("scope-c{}", hash.checked_rem(6).unwrap_or(0)) } + +/// The acting session's member id -- the composite review key's +/// `<member>` segment -- resolved the same way +/// [`account::resolve_member_by_key`] does, falling back to a short hash of +/// the public key when no enrolled member matches: mirrors +/// `git_ents::commands::serve::build_state`'s identical fallback +/// (`roots.web-signing`: an unenrolled local identity may still review or +/// withdraw a review, exactly as it may still browse and comment). Shared +/// by [`super::commits`] (starting a review) and [`super::reviews`] +/// (withdrawing one) -- both need the same "which member is this session, +/// as far as the review namespace is concerned" answer, so it lives here +/// rather than in either page module. +pub(crate) fn reviewer_member_id<O: Find>(state: &AppState<O>) -> ents_model::MemberId { + let pubkey = state.identity.public_openssh(); + account::resolve_member_by_key(state, &pubkey) + .map(|(id, _member)| id) + .unwrap_or_else(|_source| ents_model::MemberId::new(short_key_fingerprint(&pubkey))) +} + +/// The first twelve characters of `pubkey`'s key-material token -- +/// mirrors `git_ents::commands::short_fingerprint`'s identical fallback +/// label. [`reviewer_member_id`]'s own helper. +fn short_key_fingerprint(pubkey: &str) -> String { + let hex: String = pubkey + .split_whitespace() + .nth(1) + .unwrap_or(pubkey) + .chars() + .take(12) + .collect(); + if hex.is_empty() { + "member".to_owned() + } else { + hex + } +}

crates/cli/ents-web/src/pages/reviews.rs @@ -1,99 +1,132 @@ -//! `GET /reviews`: every review recorded in this repository, newest -//! first -- a read-only aggregate across commits, alongside -//! `crate::pages::commits`'s own per-commit `reviews_section` rather than -//! replacing it. Every mutation (starting a review, commenting on one) -//! still posts through `commits`'s own routes; this module has none of -//! its own. +//! `GET /reviews`, `GET /reviews/{target}/{member}`, +//! `POST /reviews/{target}/{member}/withdraw`: the review surface's own +//! aggregate list and per-review detail page -- a read-only aggregate +//! across commits, alongside `crate::pages::commits`'s own per-commit +//! `reviews_section` rather than replacing it. Starting a review still only +//! ever posts through `commits`'s own route (`POST /commit/{oid}/review`); +//! commenting on one (`POST /reviews/{target}/{member}/comment`) is +//! `commits::review_comment`, shared verbatim by both pages that render a +//! review's thread. Withdrawing one is this module's own mutation: every +//! read is `ents_forge::review::{list,show}` and the withdraw write is +//! `ents_forge::review::withdraw` -- the web is another caller of that one +//! library func, never a second review-state machine (`lens.parity`). +//! +//! A review's own page ([`show`]) renders even when the review is +//! [`ents_forge::review::ReviewState::Withdrawn`] -- a direct link stays +//! live -- while [`list`] and [`reviews_sidebar`] both filter withdrawn +//! rows out, mirroring `commits::reviews_section`'s own stance: withdrawal +//! is append-only (`model.review`), it retracts a verdict from the +//! aggregate views, never from history or from the one page a direct link +//! still reaches. use std::sync::Arc; +use axum::Form; +use axum::extract::{Path, State}; +use axum::response::{IntoResponse, Redirect}; +use ents_forge::review::{self, Review, ReviewState}; +use ents_model::MemberId; use gix::bstr::ByteSlice as _; use gix_object::{Find, Write}; use maud::{Markup, html}; +use serde::Deserialize; use crate::error::Result; +use crate::session::Session; use crate::state::AppState; -/// `GET /reviews`: list [`ents_forge::review::list`]'s full result (no -/// `target` filter), newest reviewer-commit first. Best effort per row, -/// mirroring `crate::pages::commits::reviews_section`'s own stance: a -/// review whose reviewer-commit chain or target subject fails to read -/// still renders, just without the piece that failed, rather than -/// dropping the row or failing the page. +/// Every non-withdrawn review recorded in this repository +/// (`ents_forge::review::list`, no `target` filter), each paired with the +/// review ref's own tip-commit time when it could be read (`model.review` +/// stores no timestamp field of its own), newest first. The one aggregate +/// read [`list`]'s cards and [`reviews_sidebar`]'s rows both build from, so +/// the withdrawn filter and the ordering are computed in exactly one place. +/// Best effort throughout, mirroring `commits::reviews_section`'s own +/// stance: a review whose reviewer-commit chain fails to read still sorts +/// (last, its time treated as `0`) rather than dropping the row. +fn active_reviews<O: Find + Write>( + state: &AppState<O>, +) -> Vec<(String, MemberId, Review, Option<i64>)> { + let mut rows = review::list(state.refs.as_ref(), &*state.objects(), &state.path, None) + .unwrap_or_default(); + rows.retain(|(_, review)| review.state != ReviewState::Withdrawn); + let mut with_time: Vec<(String, MemberId, Review, Option<i64>)> = rows + .into_iter() + .map(|((target, member), review)| { + let seconds = ents_model::namespace::review_ref(&target, &member) + .ok() + .and_then(|ref_name| state.refs.get(ref_name.as_ref()).ok().flatten()) + .and_then(|tip| super::commit_authorship(&*state.objects(), tip).ok()) + .map(|(_author, seconds)| seconds); + (target, member, review, seconds) + }) + .collect(); + with_time.sort_by_key(|(.., seconds)| std::cmp::Reverse(seconds.unwrap_or(0))); + with_time +} + +/// `GET /reviews`: [`active_reviews`]'s full result rendered as one card +/// per review -- verdict, reviewer, and the target commit's own subject -- +/// beside [`reviews_sidebar`]'s compact newest-first nav (`crate::pages::layout_split`). /// /// # Errors /// /// Propagates a ref-store or object read failure enumerating the reviews /// themselves ([`ents_forge::review::list`]); a per-row read failure /// degrades that row instead of failing the page. -pub async fn list<O>( - axum::extract::State(state): axum::extract::State<Arc<AppState<O>>>, -) -> Result<Markup> +pub async fn list<O>(State(state): State<Arc<AppState<O>>>) -> Result<Markup> where O: Find + Write + Send + 'static, { - let mut rows = ents_forge::review::list(state.refs.as_ref(), &*state.objects(), &state.path, None) - .unwrap_or_default(); - // Withdrawn reviews stay in `refs/meta/reviews/*`'s own history - // (append-only, `model.review`) but drop out of this aggregate listing - // — a later item's review detail page is where a withdrawn verdict - // still surfaces. - rows.retain(|(_, review)| review.state != ents_forge::review::ReviewState::Withdrawn); + let rows = active_reviews(&state); let repo = gix::open(&state.path).ok(); - let mut with_time: Vec<(i64, Markup)> = rows - .drain(..) - .map(|((target, member), review)| { - let reviewer = ents_model::namespace::review_ref(&target, &member) - .ok() - .and_then(|ref_name| state.refs.get(ref_name.as_ref()).ok().flatten()) - .and_then(|tip| super::commit_authorship(&*state.objects(), tip).ok()); - let seconds = reviewer.as_ref().map_or(0, |(_author, seconds)| *seconds); + let cards: Vec<Markup> = rows + .iter() + .map(|(target, member, review, seconds)| { let subject = repo.as_ref().and_then(|repo| { let oid = gix_hash::ObjectId::from_hex(target.as_bytes()).ok()?; let commit = repo.find_commit(oid).ok()?; let message = commit.message().ok()?; Some(message.title.to_str_lossy().into_owned()) }); - let short = target.get(..7).unwrap_or(&target).to_owned(); - ( - seconds, - html! { - div.card { - div.comment-meta { - span class={ "verdict verdict-" (review.verdict) } { (review.verdict) } - (super::avatar(member.as_str())) - span.author { (member) } - span.spacer {} - a href={ "/commit/" (target) } { code { (short) } } - @if let Some(subject) = &subject { - span.muted { (subject) } - } - } - @if let Some((_author, seconds)) = &reviewer { - span.entry-size { (super::ago(*seconds)) } + let short = target.get(..7).unwrap_or(target).to_owned(); + html! { + div.card { + div.comment-meta { + span class={ "verdict verdict-" (review.verdict) } { (review.verdict) } + (super::avatar(member.as_str())) + span.author { (member) } + span.spacer {} + a href={ "/reviews/" (target) "/" (member) } { code { (short) } } + @if let Some(subject) = &subject { + span.muted { (subject) } } } - }, - ) + @if let Some(seconds) = seconds { + span.entry-size { (super::ago(*seconds)) } + } + } + } }) .collect(); - with_time.sort_by(|(a, _), (b, _)| b.cmp(a)); - Ok(super::layout( + Ok(super::layout_split( &super::RepoHeader::from_state(&state), &super::identity_label(&state), super::Tab::Reviews, "Reviews", + false, + reviews_sidebar(&rows, None), html! { div.readable { - @if with_time.is_empty() { + @if cards.is_empty() { (super::blankslate( "No reviews yet", html! { "Record one from a commit's own page." }, )) } @else { - @for (_seconds, card) in &with_time { + @for card in &cards { (card) } } @@ -101,3 +134,217 @@ }, )) } + +/// The Reviews split's `.tree` sidebar (mirrors `issues::issues_sidebar`): +/// every [`active_reviews`] row as a two-line `.side-row` -- its verdict and +/// reviewer on the title line, the target commit's abbreviated id on the +/// locator line -- linking to [`show`]'s own page, `.active` naming the +/// viewed `(target, member)` pair. Withdrawn reviews are already filtered +/// out of `rows` by [`active_reviews`]; they stay reachable only by a +/// direct link to [`show`], never from this nav. +fn reviews_sidebar( + rows: &[(String, MemberId, Review, Option<i64>)], + active: Option<(&str, &str)>, +) -> Markup { + html! { + div.tree-head { + span { "Reviews" } + } + @if rows.is_empty() { + span.tree-note { "No reviews yet." } + } + @for (target, member, review, _seconds) in rows { + a.side-row.active[active == Some((target.as_str(), member.as_str()))] + href={ "/reviews/" (target) "/" (member) } + { + span.side-title { + span class={ "verdict verdict-" (review.verdict) } { (review.verdict) } + " " (member.as_str()) + } + span.side-meta { + span.locator { "on " (ents_forge::abbreviate_id(target)) } + } + } + } + } +} + +/// `GET /reviews/{target}/{member}`: one review +/// (`ents_forge::review::show`), its verdict/state/target/reviewer metadata +/// card, its body rendered as AsciiDoc, its discussion thread, a comment +/// 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). +/// +/// # Errors +/// +/// [`crate::Error::Forge`] (wrapping [`ents_forge::Error::NotFound`]) if +/// `target`/`member` has no review ref at all; otherwise propagates a +/// ref-store or object read failure. +// @relation(model.review, model.comment-context, lens.parity, scope=function) +pub async fn show<O>( + State(state): State<Arc<AppState<O>>>, + axum::Extension(session): axum::Extension<Session>, + Path((target, member)): Path<(String, String)>, +) -> Result<Markup> +where + O: Find + Write + Send + 'static, +{ + let member = MemberId::new(member); + let (review, thread) = + review::show(state.refs.as_ref(), &*state.objects(), &target, &member)?; + let reviewer = ents_model::namespace::review_ref(&target, &member) + .ok() + .and_then(|ref_name| state.refs.get(ref_name.as_ref()).ok().flatten()) + .and_then(|tip| super::commit_authorship(&*state.objects(), tip).ok()); + let body = + crate::asciidoc::to_html(&review.body).unwrap_or_else(|_| html! { p { (review.body) } }); + let return_to = format!("/reviews/{target}/{member}"); + let is_author = super::reviewer_member_id(&state) == member; + // Best-effort: the sidebar listing every other review beside this one + // is navigation chrome, never a reason to fail this review's own page. + let rows = active_reviews(&state); + + Ok(super::layout_split( + &super::RepoHeader::from_state(&state), + &super::identity_label(&state), + super::Tab::Reviews, + &format!("Review of {}", ents_forge::abbreviate_id(&target)), + false, + reviews_sidebar(&rows, Some((&target, member.as_str()))), + html! { + (super::child_crumbs("reviews", "/reviews", ents_forge::abbreviate_id(&target))) + div.readable { + div.card { + dl.entity-view { + dt { "verdict" } + dd { span class={ "verdict verdict-" (review.verdict) } { (review.verdict) } } + dt { "state" } + dd { (state_badge(review.state)) } + dt { "target" } + dd { a href={ "/commit/" (target) } { code { (target) } } } + dt { "reviewer" } + dd { (super::avatar(member.as_str())) " @" (member.as_str()) } + dt { "reviewed" } + dd { + @if let Some((_author, seconds)) = &reviewer { + (super::ago(*seconds)) + } @else { + span.muted { "unknown" } + } + } + } + div.doc-body { (body) } + } + @if review.state == ReviewState::Active { + @if is_author { + (withdraw_form(&session, &target, &member)) + } + } @else { + p.muted { "This review has been withdrawn." } + } + h2 { "Discussion" } + @if thread.is_empty() { + (super::blankslate( + "No comments yet", + html! { "Start the discussion below." }, + )) + } @else { + (crate::pages::comments::thread_section(&state, &session, &thread, &return_to)) + } + div.card { + div.card-header { "Add a comment" } + (super::commits::review_comment_form(&session, &target, &member, &return_to)) + } + } + }, + )) +} + +/// The review detail card's `state` `dd` (see [`show`]): a plain neutral +/// `.chip.chip-pill` naming `active`, or the same grey `.state-closed` +/// treatment `issues::state_chip` gives a closed issue naming `withdrawn` +/// instead -- so a direct link to a retracted verdict states plainly, at a +/// glance, that it no longer stands. +fn state_badge(state: ReviewState) -> Markup { + match state { + ReviewState::Active => html! { + span.chip.chip-pill { "active" } + }, + ReviewState::Withdrawn => html! { + span.chip.chip-pill.state-closed { "withdrawn" } + }, + } +} + +/// The withdraw-this-review control (`POST /reviews/{target}/{member}/withdraw`), +/// rendered by [`show`] only for the review's own author while it is still +/// [`ReviewState::Active`] -- retracting a verdict stays a decision only +/// its author can make, the same way `ents-gate`'s own `owner_mutation` +/// check refuses anyone else's attempt at the ref level +/// (`ents_forge::review::withdraw`'s own doc). +fn withdraw_form(session: &Session, target: &str, member: &MemberId) -> Markup { + html! { + form method="post" action=(format!("/reviews/{target}/{member}/withdraw")) { + (super::csrf_input(session)) + button type="submit" { "Withdraw review" } + } + } +} + +/// The form fields `POST /reviews/{target}/{member}/withdraw` accepts. +#[derive(Debug, Deserialize)] +pub struct WithdrawForm { + /// The per-session CSRF token (`roots.web-session`). + csrf: String, +} + +/// `POST /reviews/{target}/{member}/withdraw`: retract the signed-in +/// member's own review of `target` (`ents_forge::review::withdraw`) -- the +/// web is another caller of that one library func, driving the identical +/// mutation `git ents review withdraw` does. The path's own `member` +/// segment names whose review [`show`] rendered, but the write always +/// targets the *signed-in* identity's own member id +/// ([`super::reviewer_member_id`]), never the path's: this handler can only +/// ever build and write `reviews/<target>/<the signer>`. A member who is +/// not the review's author therefore has no matching +/// `refs/meta/reviews/<target>/<member>` of their own to advance, and the +/// mutation fails with [`ents_forge::Error::NotFound`] rather than +/// touching anyone else's ref -- no divergent ownership check is added +/// here; `ents-gate`'s own `identity_binding`/`owner_mutation` checks on +/// this namespace back the same refusal up independently (see +/// [`ents_forge::review::withdraw`]'s own doc). +/// +/// # Errors +/// +/// [`crate::Error::BadCsrf`] if `form.csrf` does not match; otherwise +/// propagates [`ents_forge::review::withdraw`]'s own failures (including +/// [`ents_forge::Error::NotFound`] when the signed-in member has no review +/// reaching `target`). +// @relation(model.review, roots.web-signing, roots.web-session, lens.parity, scope=function) +pub async fn withdraw<O>( + State(state): State<Arc<AppState<O>>>, + axum::Extension(session): axum::Extension<Session>, + Path((target, _member)): Path<(String, String)>, + Form(form): Form<WithdrawForm>, +) -> Result<impl IntoResponse> +where + O: Find + Write + Send + 'static, +{ + super::require_csrf(&session, &form.csrf)?; + let member = super::reviewer_member_id(&state); + let identity = state.identity.as_ref(); + let (target_hex, outcome) = review::withdraw( + state.refs.as_ref(), + &*state.objects(), + state.events.as_ref(), + &state.path, + &target, + &member, + &crate::receive_identity!(identity, crate::pages::member_author(&session)), + state.mode, + )?; + crate::error::outcome_to_result(outcome)?; + Ok(Redirect::to(&format!("/reviews/{target_hex}/{member}"))) +}

web, review: issue-shaped review detail page

Reviews

Start a review