4aa710bd18
fixes hyperhive/hyperhive#2839. pr_assign_reviewer's own doc-comment claimed full idempotency (already-requested = no-op), but conflated 'still pending' with 'already reviewed' - Forgejo clears a fulfilled reviewer from requested_reviewers, so re-requesting them isn't a no-op, it dismisses the standing review. now checks latest_reviews for a non-superseded review from the target user first and skips the request instead of blindly posting it.
74 lines
2.9 KiB
Rust
74 lines
2.9 KiB
Rust
//! `pr assign-reviewer <number> <user> [--remove]` — add or remove a
|
|
//! requested reviewer on a PR. Unlike `pr assign-committer` (assignees,
|
|
//! mutate + PATCH the issue) Forgejo has a dedicated endpoint pair for review
|
|
//! requests: `POST /repos/{repo}/pulls/{index}/requested_reviewers`
|
|
//! adds, `DELETE` (same path + body) removes.
|
|
//!
|
|
//! Requesting a review from a user who is already *pending* (requested but
|
|
//! hasn't reviewed yet) is a Forgejo-side no-op. Requesting from a user who
|
|
//! has already **submitted** a review is a different story: Forgejo doesn't
|
|
//! no-op that case, it dismisses the existing review (including an
|
|
//! `APPROVED` one), because the reviewer is no longer in the pending-request
|
|
//! list once they've reviewed. A caller re-requesting on the strength of
|
|
//! "already requested = no-op" (the old, wrong claim this module made)
|
|
//! silently destroys a standing approval — this is how a coverage sweep that
|
|
//! re-requests reviews indiscriminately can dismiss a ready PR's approval
|
|
//! with no new commit in sight. So this verb checks for an existing
|
|
//! non-superseded review from `user` first and skips the request instead of
|
|
//! blindly posting it.
|
|
|
|
use anyhow::Result;
|
|
use clap::Args as ClapArgs;
|
|
use forgejo_api::structs::PullReviewRequestOptions;
|
|
|
|
use crate::client::{Client, index};
|
|
|
|
#[derive(ClapArgs)]
|
|
pub struct Args {
|
|
/// PR number.
|
|
number: u64,
|
|
/// User login to request a review from (or withdraw with `--remove`).
|
|
user: String,
|
|
/// Withdraw the review request instead of adding it.
|
|
#[arg(long)]
|
|
remove: bool,
|
|
}
|
|
|
|
pub fn run(client: &Client, args: Args) -> Result<()> {
|
|
let (owner, name) = client.owner_repo()?;
|
|
let idx = index(args.number)?;
|
|
let body = PullReviewRequestOptions {
|
|
reviewers: Some(vec![args.user.clone()]),
|
|
team_reviewers: None,
|
|
};
|
|
if args.remove {
|
|
client
|
|
.api()
|
|
.repo_delete_pull_review_requests(owner, name, idx, body)
|
|
.send()?;
|
|
println!(
|
|
"review request withdrawn: {} on #{}",
|
|
args.user, args.number
|
|
);
|
|
return Ok(());
|
|
}
|
|
// Guard the add path only — withdrawing a request is always safe.
|
|
// Re-requesting from someone who already reviewed dismisses that review
|
|
// instead of no-op'ing (see the module doc-comment above).
|
|
if let Some(existing) = super::latest_reviews(client, client.repo(), args.number)?
|
|
.into_iter()
|
|
.find(|r| r.login == args.user && !r.superseded())
|
|
{
|
|
println!(
|
|
"not re-requesting: {} already has a standing {} review on #{} (re-requesting would dismiss it)",
|
|
args.user, existing.state, args.number
|
|
);
|
|
return Ok(());
|
|
}
|
|
client
|
|
.api()
|
|
.repo_create_pull_review_requests(owner, name, idx, body)
|
|
.send()?;
|
|
println!("review requested: {} on #{}", args.user, args.number);
|
|
Ok(())
|
|
}
|