crates/cli/ents-web/src/pages/reviews.rs
reviews.rshistorycomment on this file
| 1 | //! `GET /reviews`, `GET /reviews/{target}/{member}`, |
| 2 | //! `POST /reviews/{target}/{member}/withdraw`: the review surface's own |
| 3 | //! aggregate list and per-review detail page -- a read-only aggregate |
| 4 | //! across commits, alongside `crate::pages::commits`'s own per-commit |
| 5 | //! `reviews_section` rather than replacing it. Starting a review still only |
| 6 | //! ever posts through `commits`'s own route (`POST /commit/{oid}/review`); |
| 7 | //! commenting on one (`POST /reviews/{target}/{member}/comment`) is |
| 8 | //! `commits::review_comment`, shared verbatim by both pages that render a |
| 9 | //! review's thread. Withdrawing one is this module's own mutation: every |
| 10 | //! read is `ents_forge::review::{list,show}` and the withdraw write is |
| 11 | //! `ents_forge::review::withdraw` -- the web is another caller of that one |
| 12 | //! library func, never a second review-state machine (`lens.parity`). |
| 13 | //! |
| 14 | //! A review's own page ([`show`]) renders even when the review is |
| 15 | //! [`ents_forge::review::ReviewState::Withdrawn`] -- a direct link stays |
| 16 | //! live -- while [`list`] and [`reviews_sidebar`] both filter withdrawn |
| 17 | //! rows out, mirroring `commits::reviews_section`'s own stance: withdrawal |
| 18 | //! is append-only (`model.review`), it retracts a verdict from the |
| 19 | //! aggregate views, never from history or from the one page a direct link |
| 20 | //! still reaches. |
| 21 | |
| 22 | use std::sync::Arc; |
| 23 | |
| 24 | use axum::Form; |
| 25 | use axum::extract::{Path, State}; |
| 26 | use axum::response::{IntoResponse, Redirect}; |
| 27 | use ents_forge::review::{self, Review, ReviewState}; |
| 28 | use ents_model::MemberId; |
| 29 | use gix::bstr::ByteSlice as _; |
| 30 | use gix_object::{Find, Write}; |
| 31 | use maud::{Markup, html}; |
| 32 | use serde::Deserialize; |
| 33 | |
| 34 | use crate::error::Result; |
| 35 | use crate::session::Session; |
| 36 | use crate::state::AppState; |
| 37 | |
| 38 | /// Every non-withdrawn review recorded in this repository |
| 39 | /// (`ents_forge::review::list`, no `target` filter), each paired with the |
| 40 | /// review ref's own tip-commit time when it could be read (`model.review` |
| 41 | /// stores no timestamp field of its own), newest first. The one aggregate |
| 42 | /// read [`list`]'s cards and [`reviews_sidebar`]'s rows both build from, so |
| 43 | /// the withdrawn filter and the ordering are computed in exactly one place. |
| 44 | /// Best effort throughout, mirroring `commits::reviews_section`'s own |
| 45 | /// stance: a review whose reviewer-commit chain fails to read still sorts |
| 46 | /// (last, its time treated as `0`) rather than dropping the row. |
| 47 | fn active_reviews<O: Find + Write>( |
| 48 | state: &AppState<O>, |
| 49 | ) -> Vec<(String, MemberId, Review, Option<i64>)> { |
| 50 | let mut rows = review::list(state.refs.as_ref(), &*state.objects(), &state.path, None) |
| 51 | .unwrap_or_default(); |
| 52 | rows.retain(|(_, review)| review.state != ReviewState::Withdrawn); |
| 53 | let mut with_time: Vec<(String, MemberId, Review, Option<i64>)> = rows |
| 54 | .into_iter() |
| 55 | .map(|((target, member), review)| { |
| 56 | let seconds = ents_model::namespace::review_ref(&target, &member) |
| 57 | .ok() |
| 58 | .and_then(|ref_name| state.refs.get(ref_name.as_ref()).ok().flatten()) |
| 59 | .and_then(|tip| super::commit_authorship(&*state.objects(), tip).ok()) |
| 60 | .map(|(_author, seconds)| seconds); |
| 61 | (target, member, review, seconds) |
| 62 | }) |
| 63 | .collect(); |
| 64 | with_time.sort_by_key(|(.., seconds)| std::cmp::Reverse(seconds.unwrap_or(0))); |
| 65 | with_time |
| 66 | } |
| 67 | |
| 68 | /// `GET /reviews`: [`active_reviews`]'s full result rendered as one card |
| 69 | /// per review -- verdict, reviewer, and the target commit's own subject -- |
| 70 | /// beside [`reviews_sidebar`]'s compact newest-first nav (`crate::pages::layout_split`). |
| 71 | /// |
| 72 | /// # Errors |
| 73 | /// |
| 74 | /// Propagates a ref-store or object read failure enumerating the reviews |
| 75 | /// themselves ([`ents_forge::review::list`]); a per-row read failure |
| 76 | /// degrades that row instead of failing the page. |
| 77 | pub async fn list<O>(State(state): State<Arc<AppState<O>>>) -> Result<Markup> |
| 78 | where |
| 79 | O: Find + Write + Send + 'static, |
| 80 | { |
| 81 | let rows = active_reviews(&state); |
| 82 | let repo = gix::open(&state.path).ok(); |
| 83 | |
| 84 | let cards: Vec<Markup> = rows |
| 85 | .iter() |
| 86 | .map(|(target, member, review, seconds)| { |
| 87 | let subject = repo.as_ref().and_then(|repo| { |
| 88 | let oid = gix_hash::ObjectId::from_hex(target.as_bytes()).ok()?; |
| 89 | let commit = repo.find_commit(oid).ok()?; |
| 90 | let message = commit.message().ok()?; |
| 91 | Some(message.title.to_str_lossy().into_owned()) |
| 92 | }); |
| 93 | let short = target.get(..7).unwrap_or(target).to_owned(); |
| 94 | html! { |
| 95 | div.card { |
| 96 | div.comment-meta { |
| 97 | (super::verdict_chip(review.verdict)) |
| 98 | (super::avatar(member.as_str())) |
| 99 | span.author { (member) } |
| 100 | span.spacer {} |
| 101 | a href={ "/reviews/" (target) "/" (member) } { code { (short) } } |
| 102 | @if let Some(subject) = &subject { |
| 103 | span.muted { (subject) } |
| 104 | } |
| 105 | } |
| 106 | @if let Some(seconds) = seconds { |
| 107 | span.entry-size { (super::ago(*seconds)) } |
| 108 | } |
| 109 | } |
| 110 | } |
| 111 | }) |
| 112 | .collect(); |
| 113 | |
| 114 | Ok(super::layout_split( |
| 115 | &super::RepoHeader::from_state(&state), |
| 116 | &super::identity_label(&state), |
| 117 | super::Tab::Reviews, |
| 118 | "Reviews", |
| 119 | false, |
| 120 | reviews_sidebar(&rows, None), |
| 121 | html! { |
| 122 | div.readable { |
| 123 | @if cards.is_empty() { |
| 124 | (super::blankslate( |
| 125 | "No reviews yet", |
| 126 | html! { "Record one from a commit's own page." }, |
| 127 | )) |
| 128 | } @else { |
| 129 | @for card in &cards { |
| 130 | (card) |
| 131 | } |
| 132 | } |
| 133 | } |
| 134 | }, |
| 135 | )) |
| 136 | } |
| 137 | |
| 138 | /// The Reviews split's `.tree` sidebar (mirrors `issues::issues_sidebar`): |
| 139 | /// every [`active_reviews`] row as a two-line `.side-row` -- its verdict and |
| 140 | /// reviewer on the title line, the target commit's abbreviated id on the |
| 141 | /// locator line -- linking to [`show`]'s own page, `.active` naming the |
| 142 | /// viewed `(target, member)` pair. Withdrawn reviews are already filtered |
| 143 | /// out of `rows` by [`active_reviews`]; they stay reachable only by a |
| 144 | /// direct link to [`show`], never from this nav. |
| 145 | fn reviews_sidebar( |
| 146 | rows: &[(String, MemberId, Review, Option<i64>)], |
| 147 | active: Option<(&str, &str)>, |
| 148 | ) -> Markup { |
| 149 | html! { |
| 150 | div.tree-head { |
| 151 | span { "Reviews" } |
| 152 | } |
| 153 | @if rows.is_empty() { |
| 154 | span.tree-note { "No reviews yet." } |
| 155 | } |
| 156 | @for (target, member, review, _seconds) in rows { |
| 157 | a.side-row.active[active == Some((target.as_str(), member.as_str()))] |
| 158 | href={ "/reviews/" (target) "/" (member) } |
| 159 | { |
| 160 | span.side-title { |
| 161 | (super::verdict_chip(review.verdict)) |
| 162 | " " (member.as_str()) |
| 163 | } |
| 164 | span.side-meta { |
| 165 | span.locator { "on " (ents_forge::abbreviate_id(target)) } |
| 166 | } |
| 167 | } |
| 168 | } |
| 169 | } |
| 170 | } |
| 171 | |
| 172 | /// `GET /reviews/{target}/{member}`: one review |
| 173 | /// (`ents_forge::review::show`), its verdict/state/target/reviewer metadata |
| 174 | /// card, its body rendered as AsciiDoc, its discussion thread, a comment |
| 175 | /// composer, and -- only for the review's own author while it is still |
| 176 | /// [`ReviewState::Active`] -- a withdraw control. Renders even for a |
| 177 | /// withdrawn review (this module's own doc: a direct link stays live; only |
| 178 | /// [`list`]/[`reviews_sidebar`] hide a withdrawn row). The metadata |
| 179 | /// `dl.entity-view` stays hand-rolled rather than |
| 180 | /// [`crate::render::view`]'s generic dump: every row is a domain widget or |
| 181 | /// derived value (verdict chip, state badge, a link to the target commit, |
| 182 | /// the reviewer's avatar, a relative time read from the review ref's own |
| 183 | /// tip commit -- not a field on the entity at all). |
| 184 | /// |
| 185 | /// # Errors |
| 186 | /// |
| 187 | /// [`crate::Error::Forge`] (wrapping [`ents_forge::Error::NotFound`]) if |
| 188 | /// `target`/`member` has no review ref at all; otherwise propagates a |
| 189 | /// ref-store or object read failure. |
| 190 | // @relation(model.review, model.comment-context, lens.parity, scope=function) |
| 191 | pub async fn show<O>( |
| 192 | State(state): State<Arc<AppState<O>>>, |
| 193 | axum::Extension(session): axum::Extension<Session>, |
| 194 | Path((target, member)): Path<(String, String)>, |
| 195 | ) -> Result<Markup> |
| 196 | where |
| 197 | O: Find + Write + Send + 'static, |
| 198 | { |
| 199 | let member = MemberId::new(member); |
| 200 | let (review, thread) = |
| 201 | review::show(state.refs.as_ref(), &*state.objects(), &target, &member)?; |
| 202 | let reviewer = ents_model::namespace::review_ref(&target, &member) |
| 203 | .ok() |
| 204 | .and_then(|ref_name| state.refs.get(ref_name.as_ref()).ok().flatten()) |
| 205 | .and_then(|tip| super::commit_authorship(&*state.objects(), tip).ok()); |
| 206 | let body = |
| 207 | crate::asciidoc::to_html(&review.body).unwrap_or_else(|_| html! { p { (review.body) } }); |
| 208 | let return_to = format!("/reviews/{target}/{member}"); |
| 209 | let is_author = super::reviewer_member_id(&state) == member; |
| 210 | // Best-effort: the sidebar listing every other review beside this one |
| 211 | // is navigation chrome, never a reason to fail this review's own page. |
| 212 | let rows = active_reviews(&state); |
| 213 | |
| 214 | Ok(super::layout_split( |
| 215 | &super::RepoHeader::from_state(&state), |
| 216 | &super::identity_label(&state), |
| 217 | super::Tab::Reviews, |
| 218 | &format!("Review of {}", ents_forge::abbreviate_id(&target)), |
| 219 | false, |
| 220 | reviews_sidebar(&rows, Some((&target, member.as_str()))), |
| 221 | html! { |
| 222 | (super::child_crumbs("reviews", "/reviews", ents_forge::abbreviate_id(&target))) |
| 223 | div.readable { |
| 224 | div.card { |
| 225 | dl.entity-view { |
| 226 | dt { "verdict" } |
| 227 | dd { (super::verdict_chip(review.verdict)) } |
| 228 | dt { "state" } |
| 229 | dd { (state_badge(review.state)) } |
| 230 | dt { "target" } |
| 231 | dd { a href={ "/commit/" (target) } { code { (target) } } } |
| 232 | dt { "reviewer" } |
| 233 | dd { (super::avatar(member.as_str())) " @" (member.as_str()) } |
| 234 | dt { "reviewed" } |
| 235 | dd { |
| 236 | @if let Some((_author, seconds)) = &reviewer { |
| 237 | (super::ago(*seconds)) |
| 238 | } @else { |
| 239 | span.muted { "unknown" } |
| 240 | } |
| 241 | } |
| 242 | } |
| 243 | div.doc-body { (body) } |
| 244 | } |
| 245 | @if review.state == ReviewState::Active { |
| 246 | @if is_author { |
| 247 | (withdraw_form(&session, &target, &member)) |
| 248 | } |
| 249 | } @else { |
| 250 | p.muted { "This review has been withdrawn." } |
| 251 | } |
| 252 | h2 { "Discussion" } |
| 253 | @if thread.is_empty() { |
| 254 | (super::blankslate( |
| 255 | "No comments yet", |
| 256 | html! { "Start the discussion below." }, |
| 257 | )) |
| 258 | } @else { |
| 259 | (crate::pages::comments::thread_section(&state, &session, &thread, &return_to)) |
| 260 | } |
| 261 | div.card { |
| 262 | div.card-header { "Add a comment" } |
| 263 | (super::commits::review_comment_form(&session, &target, &member, &return_to)) |
| 264 | } |
| 265 | } |
| 266 | }, |
| 267 | )) |
| 268 | } |
| 269 | |
| 270 | /// The review detail card's `state` `dd` (see [`show`]): a plain neutral |
| 271 | /// `.chip.chip-pill` naming `active`, or the same grey `.state-closed` |
| 272 | /// treatment `issues::state_chip` gives a closed issue naming `withdrawn` |
| 273 | /// instead -- so a direct link to a retracted verdict states plainly, at a |
| 274 | /// glance, that it no longer stands. |
| 275 | fn state_badge(state: ReviewState) -> Markup { |
| 276 | match state { |
| 277 | ReviewState::Active => html! { |
| 278 | span.chip.chip-pill { "active" } |
| 279 | }, |
| 280 | ReviewState::Withdrawn => html! { |
| 281 | span.chip.chip-pill.state-closed { "withdrawn" } |
| 282 | }, |
| 283 | } |
| 284 | } |
| 285 | |
| 286 | /// The withdraw-this-review control (`POST /reviews/{target}/{member}/withdraw`), |
| 287 | /// rendered by [`show`] only for the review's own author while it is still |
| 288 | /// [`ReviewState::Active`] -- retracting a verdict stays a decision only |
| 289 | /// its author can make, the same way `ents-gate`'s own `owner_mutation` |
| 290 | /// check refuses anyone else's attempt at the ref level |
| 291 | /// (`ents_forge::review::withdraw`'s own doc). |
| 292 | fn withdraw_form(session: &Session, target: &str, member: &MemberId) -> Markup { |
| 293 | html! { |
| 294 | form method="post" action=(format!("/reviews/{target}/{member}/withdraw")) { |
| 295 | (super::csrf_input(session)) |
| 296 | button type="submit" { "Withdraw review" } |
| 297 | } |
| 298 | } |
| 299 | } |
| 300 | |
| 301 | /// The form fields `POST /reviews/{target}/{member}/withdraw` accepts. |
| 302 | #[derive(Debug, Deserialize)] |
| 303 | pub struct WithdrawForm { |
| 304 | /// The per-session CSRF token (`roots.web-session`). |
| 305 | csrf: String, |
| 306 | } |
| 307 | |
| 308 | /// `POST /reviews/{target}/{member}/withdraw`: retract the signed-in |
| 309 | /// member's own review of `target` (`ents_forge::review::withdraw`) -- the |
| 310 | /// web is another caller of that one library func, driving the identical |
| 311 | /// mutation `git ents review withdraw` does. The path's own `member` |
| 312 | /// segment names whose review [`show`] rendered, but the write always |
| 313 | /// targets the *signed-in* identity's own member id |
| 314 | /// ([`super::reviewer_member_id`]), never the path's: this handler can only |
| 315 | /// ever build and write `reviews/<target>/<the signer>`. A member who is |
| 316 | /// not the review's author therefore has no matching |
| 317 | /// `refs/meta/reviews/<target>/<member>` of their own to advance, and the |
| 318 | /// mutation fails with [`ents_forge::Error::NotFound`] rather than |
| 319 | /// touching anyone else's ref -- no divergent ownership check is added |
| 320 | /// here; `ents-gate`'s own `identity_binding`/`owner_mutation` checks on |
| 321 | /// this namespace back the same refusal up independently (see |
| 322 | /// [`ents_forge::review::withdraw`]'s own doc). |
| 323 | /// |
| 324 | /// # Errors |
| 325 | /// |
| 326 | /// [`crate::Error::BadCsrf`] if `form.csrf` does not match; otherwise |
| 327 | /// propagates [`ents_forge::review::withdraw`]'s own failures (including |
| 328 | /// [`ents_forge::Error::NotFound`] when the signed-in member has no review |
| 329 | /// reaching `target`). |
| 330 | // @relation(model.review, roots.web-signing, roots.web-session, lens.parity, scope=function) |
| 331 | pub async fn withdraw<O>( |
| 332 | State(state): State<Arc<AppState<O>>>, |
| 333 | axum::Extension(session): axum::Extension<Session>, |
| 334 | Path((target, _member)): Path<(String, String)>, |
| 335 | Form(form): Form<WithdrawForm>, |
| 336 | ) -> Result<impl IntoResponse> |
| 337 | where |
| 338 | O: Find + Write + Send + 'static, |
| 339 | { |
| 340 | super::require_csrf(&session, &form.csrf)?; |
| 341 | let member = super::reviewer_member_id(&state); |
| 342 | let identity = state.identity.as_ref(); |
| 343 | let (target_hex, outcome) = review::withdraw( |
| 344 | state.refs.as_ref(), |
| 345 | &*state.objects(), |
| 346 | state.events.as_ref(), |
| 347 | &state.path, |
| 348 | &target, |
| 349 | &member, |
| 350 | &crate::receive_identity!(identity, crate::pages::member_author(&session)), |
| 351 | state.mode, |
| 352 | )?; |
| 353 | crate::error::outcome_to_result(outcome)?; |
| 354 | Ok(Redirect::to(&format!("/reviews/{target_hex}/{member}"))) |
| 355 | } |