Review requests: ask someone to review a proposal (closes #68) cc1cd197df1d

-- Review requests (hub plan 0.4): who was asked to review a proposal, by

-- whom and when; a request goes when that reviewer reviews.

CREATE TABLE proposal_review_requests (

    proposal_id  INTEGER NOT NULL REFERENCES proposals(id) ON DELETE CASCADE,

    account_id   INTEGER NOT NULL REFERENCES accounts(id) ON DELETE CASCADE,

    requested_by TEXT NOT NULL,

    requested_ms INTEGER NOT NULL,

    PRIMARY KEY (proposal_id, account_id)

);

    /// Asked to review and not yet reviewing, in the order asked.

    #[serde(default)]

    pub requested_reviewers: Vec<String>,

}

/// `POST .../proposals/{number}/reviewers`: accounts to ask for a review.

#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize, JsonSchema)]

pub struct ReviewRequest {

    pub reviewers: Vec<String>,

        .route("/repos/{owner}/{name}/proposals/{number}/reviewers", post(request_reviewers))

        .route("/repos/{owner}/{name}/proposals/{number}/reviewers/{username}", axum::routing::delete(withdraw_reviewer))

    crate::web::proposals::reviewed(&app, &row, &p, &me, &body.verdict).await;

/// `POST .../proposals/{number}/reviewers`: ask accounts to review (the

/// author, or a writer). Answers with everyone asked now.

async fn request_reviewers(

    State(app): State<Shared>,

    headers: HeaderMap,

    Path((owner, name, number)): Path<(String, String, i64)>,

    Json(body): Json<ReviewRequest>,

) -> Result<Json<Vec<String>>, ApiError> {

    let c = caller(&app, &headers).await?;

    let me = c.require("proposals:write").or_else(|_| c.require("repo:write"))?.clone();

    let (_, row, role) = reach(&app, &c, &owner, &name, Access::Read).await?;

    let p = db::proposals::get(&app.db, row.id, number).await?.ok_or_else(|| not_found("no such proposal"))?;

    if Some(me.id) != p.author_id && role < Role::Write {

        return Err(forbidden("only the author or a writer asks for reviews"));

    }

    if !p.is_open() {

        return Err(ApiError(StatusCode::CONFLICT, "the proposal is not open".into()));

    }

    crate::web::proposals::request_reviewers(&app, &row, &p, &me, &body.reviewers)

        .await

        .map_err(|e| ApiError(StatusCode::UNPROCESSABLE_ENTITY, e))?;

    Ok(Json(db::proposals::review_requests(&app.db, p.id).await?))

}

/// `DELETE .../proposals/{number}/reviewers/{username}`: withdraw a request.

async fn withdraw_reviewer(

    State(app): State<Shared>,

    headers: HeaderMap,

    Path((owner, name, number, username)): Path<(String, String, i64, String)>,

) -> Result<StatusCode, ApiError> {

    let c = caller(&app, &headers).await?;

    let me = c.require("proposals:write").or_else(|_| c.require("repo:write"))?.clone();

    let (_, row, role) = reach(&app, &c, &owner, &name, Access::Read).await?;

    let p = db::proposals::get(&app.db, row.id, number).await?.ok_or_else(|| not_found("no such proposal"))?;

    if Some(me.id) != p.author_id && role < Role::Write {

        return Err(forbidden("only the author or a writer withdraws review requests"));

    }

    let a = db::accounts::account_by_name(&app.db, &username).await?.ok_or_else(|| not_found("no such account"))?;

    if db::proposals::unrequest_review(&app.db, p.id, a.id).await? {

        db::proposals::add_event(&app.db, p.id, &me.username, "review request withdrawn", &a.username).await.ok();

    }

    Ok(StatusCode::NO_CONTENT)

}

// ------------------------------------------------------- review requests

/// Ask an account to review a proposal; false when it was already asked.

pub async fn request_review(pool: &Pool, proposal_id: i64, account_id: i64, by: &str) -> sqlx::Result<bool> {

    let r = sqlx::query(

        "INSERT OR IGNORE INTO proposal_review_requests (proposal_id, account_id, requested_by, requested_ms) VALUES (?, ?, ?, ?)",

    )

    .bind(proposal_id)

    .bind(account_id)

    .bind(by)

    .bind(now_ms())

    .execute(pool)

    .await?;

    Ok(r.rows_affected() == 1)

}

/// Withdraw a request, or mark it answered; false when there was none.

pub async fn unrequest_review(pool: &Pool, proposal_id: i64, account_id: i64) -> sqlx::Result<bool> {

    let r = sqlx::query("DELETE FROM proposal_review_requests WHERE proposal_id = ? AND account_id = ?")

        .bind(proposal_id)

        .bind(account_id)

        .execute(pool)

        .await?;

    Ok(r.rows_affected() == 1)

}

/// The usernames asked to review a proposal and not yet reviewing, in the

/// order they were asked.

pub async fn review_requests(pool: &Pool, proposal_id: i64) -> sqlx::Result<Vec<String>> {

    sqlx::query_scalar(

        "SELECT a.username FROM proposal_review_requests r JOIN accounts a ON a.id = r.account_id \

         WHERE r.proposal_id = ? ORDER BY r.requested_ms, a.username",

    )

    .bind(proposal_id)

    .fetch_all(pool)

    .await

}

        .route("/{owner}/{name}/proposal/{number}/reviewers", post(proposals::reviewers))

    /// Asked to review and not yet reviewing.

    requested: Vec<String>,

    /// The viewer may ask for reviews (the author, or a writer).

    can_request: bool,

    let can_request = p.is_open() && me.is_some_and(|a| Some(a.id) == p.author_id || role >= Role::Write);

        requested: pdb::review_requests(&app.db, loaded.row.id).await.unwrap_or_default(),

        can_request,

    reviewed(&app, &row, &p, &me.account, &f.verdict).await;

        requested_reviewers: pdb::review_requests(&app.db, p.id).await.unwrap_or_default(),

// ------------------------------------------------------- review requests

/// Ask accounts to review a proposal (hub plan 0.4): each must exist, be

/// someone other than the author, and be able to read the repository. Each

/// newly asked is told, and the proposal records it. The usernames newly

/// asked, or why none could be.

pub async fn request_reviewers(

    app: &Shared,

    row: &db::repos::RepoRow,

    p: &pdb::ProposalRow,

    by: &accounts::Account,

    names: &[String],

) -> Result<Vec<String>, String> {

    let mut found = Vec::new();

    for name in names.iter().map(|n| n.trim().trim_start_matches('@')).filter(|n| !n.is_empty()) {

        let Some(a) = accounts::account_by_name(&app.db, name).await.ok().flatten() else {

            return Err(format!("There is no account {name}."));

        };

        if Some(a.id) == p.author_id {

            return Err(format!("{name} wrote this proposal, and cannot review it."));

        }

        let role = crate::auth::roles::role_for(&app.db, Some(&a), row).await.ok().flatten();

        if !crate::auth::roles::allows(role, crate::auth::Access::Read) {

            return Err(format!("{name} cannot read this repository."));

        }

        found.push(a);

    }

    let mut asked = Vec::new();

    for a in found {

        if !pdb::request_review(&app.db, p.id, a.id, &by.username).await.map_err(|e| e.to_string())? {

            continue;

        }

        pdb::add_event(&app.db, p.id, &by.username, "review requested", &a.username).await.ok();

        crate::notify::deliver(

            app,

            crate::notify::Event {

                kind: "proposal.review_requested",

                title: format!("{}/{}#{}: {} (review requested from {})", row.owner, row.name, p.number, p.title, a.username),

                url: format!("/{}/{}/proposal/{}", row.owner, row.name, p.number),

                actor: by,

                repo_id: row.id,

                thread: None,

                mention_text: "",

                direct: vec![a.id],

            },

        )

        .await;

        asked.push(a.username);

    }

    Ok(asked)

}

/// After a review: it answers the reviewer's request, if there was one, and

/// the proposal's author is told.

pub async fn reviewed(app: &Shared, row: &db::repos::RepoRow, p: &pdb::ProposalRow, by: &accounts::Account, verdict: &str) {

    pdb::unrequest_review(&app.db, p.id, by.id).await.ok();

    let what = match verdict {

        "approve" => "approved",

        "changes" => "changes requested",

        _ => "reviewed",

    };

    crate::notify::deliver(

        app,

        crate::notify::Event {

            kind: "proposal.review",

            title: format!("{}/{}#{}: {} ({what} by {})", row.owner, row.name, p.number, p.title, by.username),

            url: format!("/{}/{}/proposal/{}", row.owner, row.name, p.number),

            actor: by,

            repo_id: row.id,

            thread: Some(("proposal", p.id)),

            mention_text: "",

            direct: p.author_id.into_iter().collect(),

        },

    )

    .await;

}

#[derive(Deserialize)]

pub struct ReviewersForm {

    #[serde(rename = "_csrf")]

    csrf_token: String,

    /// `add` or `remove`.

    action: String,

    username: String,

}

/// `POST /{owner}/{name}/proposal/{number}/reviewers`: the author or a

/// writer asks someone to review, or withdraws the request.

pub async fn reviewers(

    State(app): State<Shared>,

    viewer: Viewer,

    Path((owner, name, number)): Path<(String, String, i64)>,

    Form(f): Form<ReviewersForm>,

) -> Result<Response, Error> {

    let shell = Shell::new(&app, &format!("/{owner}/{name}/proposal/{number}"), &viewer);

    let me = require(&viewer, &shell)?;

    csrf(&viewer, &f.csrf_token, &shell)?;

    let (_, row, role) = lookup(&app, &shell, &viewer, &owner, &name).await?;

    let p = pdb::get(&app.db, row.id, number)

        .await

        .map_err(|e| Error::from_db(shell.clone(), e))?

        .ok_or_else(|| Error::not_found(shell.clone()))?;

    if Some(me.account.id) != p.author_id && role < Role::Write {

        return Err(Error::not_found(shell));

    }

    let who = f.username.trim().trim_start_matches('@').to_string();

    if f.action == "remove" {

        if let Some(a) = accounts::account_by_name(&app.db, &who).await.ok().flatten() {

            if pdb::unrequest_review(&app.db, p.id, a.id).await.map_err(|e| Error::from_db(shell.clone(), e))? {

                pdb::add_event(&app.db, p.id, &me.account.username, "review request withdrawn", &a.username).await.ok();

            }

        }

        return Ok(back(&owner, &name, number, "conversation", None, None));

    }

    match request_reviewers(&app, &row, &p, &me.account, &[who]).await {

        Ok(asked) if asked.is_empty() => Ok(back(&owner, &name, number, "conversation", Some("Already asked.".into()), None)),

        Ok(asked) => Ok(back(&owner, &name, number, "conversation", Some(format!("Asked {} to review.", asked.join(", "))), None)),

        Err(e) => Ok(back(&owner, &name, number, "conversation", None, Some(e))),

    }

}

    // Review requests: the author asks andy; andy is told, reviews, and the

    // request is answered; the author hears of the review.

    let andy_user = mint(andy.clone(), "scopes=user%3Aread").await;

    let andy_pw = mint(andy.clone(), "scopes=proposals%3Awrite").await;

    oxygenhub::db::notify::mark_read(&app.db, oxygenhub::db::accounts::account_by_name(&app.db, "andy").await.unwrap().unwrap().id, None)

        .await

        .unwrap();

    let path = "/api/v1/repos/andy/demo/proposals/2/reviewers";

    let (st, v) = api(app, "POST", path, Some(&bob_rw), Some(r#"{"reviewers":["nobody"]}"#)).await;

    assert_eq!(st, StatusCode::UNPROCESSABLE_ENTITY, "{v}");

    let (st, v) = api(app, "POST", path, Some(&bob_rw), Some(r#"{"reviewers":["bob"]}"#)).await;

    assert_eq!(st, StatusCode::UNPROCESSABLE_ENTITY, "the author cannot review: {v}");

    let (st, v) = api(app, "POST", path, Some(&bob_rw), Some(r#"{"reviewers":["@andy"]}"#)).await;

    assert_eq!((st, v.clone()), (StatusCode::OK, serde_json::json!(["andy"])));

    let (_, v) = api(app, "GET", "/api/v1/repos/andy/demo/proposals/2", None, None).await;

    assert_eq!(v["requested_reviewers"], serde_json::json!(["andy"]));

    let (_, v) = api(app, "GET", "/api/v1/notifications", Some(&andy_user), None).await;

    assert!(v["items"].as_array().unwrap().iter().any(|n| n["kind"] == "proposal.review_requested" && n["read"] == false), "{v}");

    let page = call(app, "GET", "/andy/demo/proposal/2", None, Some(&andy), None, None).await;

    assert!(page.body.contains("Reviewers") && page.body.contains("review requested"), "{}", page.body);

    let (st, _) =

        api(app, "POST", "/api/v1/repos/andy/demo/proposals/2/reviews", Some(&andy_pw), Some(r#"{"verdict":"comment","body":"ok"}"#)).await;

    assert_eq!(st, StatusCode::CREATED);

    let (_, v) = api(app, "GET", "/api/v1/repos/andy/demo/proposals/2", None, None).await;

    assert_eq!(v["requested_reviewers"], serde_json::json!([]), "answered by the review");

    let (_, v) = api(app, "GET", "/api/v1/notifications", Some(&bob_user), None).await;

    assert!(v["items"].as_array().unwrap().iter().any(|n| n["kind"] == "proposal.review" && n["read"] == false), "{v}");

    // Asking again and withdrawing it.

    api(app, "POST", path, Some(&bob_rw), Some(r#"{"reviewers":["andy"]}"#)).await;

    let (st, _) = api(app, "DELETE", &format!("{path}/andy"), Some(&bob_rw), None).await;

    assert_eq!(st, StatusCode::NO_CONTENT);

    let (_, v) = api(app, "GET", "/api/v1/repos/andy/demo/proposals/2", None, None).await;

    assert_eq!(v["requested_reviewers"], serde_json::json!([]));

- [x] Notifications for review requests, mentions and landings (0.4). Mentions and landings were in already; review requests (ask, withdraw, answered by a review, the author told of each review) added 2026-10-01.

The author or a writer can **ask for a review** from anyone who can read the repository (not the author), under Reviewers on the proposal page or with `POST /api/v1/repos/{o}/{n}/proposals/{number}/reviewers` and `{"reviewers": [...]}`; `DELETE .../reviewers/{username}` withdraws a request. Each person asked gets a notification (`proposal.review_requested`), the timeline records it, the proposal's JSON lists `requested_reviewers`, and a review from that person answers the request. Every review notifies the proposal's author (`proposal.review`).

  <h3>Reviewers</h3>

  {% if requested.is_empty() %}<p class="small mute">No review requested.</p>{% else %}<ul class="plain">{% for r in requested %}<li><img class="tl-mini" src="/avatar/{{ r }}" alt="" width="18" height="18"> <a href="/{{ r }}">{{ r }}</a> <span class="small mute">asked</span>{% if can_request %} <form class="tiny inline" method="post" action="{{ repo.url }}/proposal/{{ p.number }}/reviewers"><input type="hidden" name="_csrf" value="{{ shell.csrf }}"><input type="hidden" name="action" value="remove"><input type="hidden" name="username" value="{{ r }}"><button title="Withdraw the request">×</button></form>{% endif %}</li>{% endfor %}</ul>{% endif %}

  {% if can_request %}<form class="side-picker-inline" method="post" action="{{ repo.url }}/proposal/{{ p.number }}/reviewers"><input type="hidden" name="_csrf" value="{{ shell.csrf }}"><input type="hidden" name="action" value="add"><input type="text" name="username" placeholder="username" aria-label="Ask someone to review" size="12" required> <button class="small">Ask</button></form>{% endif %}