Merge pull request #1969 from Kobzol/pr-tracking-improvements

Handle drafts better in PR tracking and do not consider self-assigned PRs for the workqueue

Author: Kobzol

src/github.rs

@@ -4,6 +4,7 @@ use bytes::Bytes;
 use chrono::{DateTime, FixedOffset, Utc};
 use futures::{future::BoxFuture, FutureExt};
 use hyper::header::HeaderValue;
+use octocrab::models::Author;
 use regex::Regex;
 use reqwest::header::{AUTHORIZATION, USER_AGENT};
 use reqwest::{Client, Request, RequestBuilder, Response, StatusCode};
@@ -24,6 +25,15 @@ pub struct User {
     pub id: UserId,
 }
 
+impl From<&Author> for User {
+    fn from(author: &Author) -> Self {
+        Self {
+            id: author.id.0,
+            login: author.login.clone(),
+        }
+    }
+}
+
 impl GithubClient {
     async fn send_req(&self, req: RequestBuilder) -> anyhow::Result<(Bytes, String)> {
         const MAX_ATTEMPTS: u32 = 2;

src/handlers/pr_tracking.rs

@@ -82,6 +82,8 @@ pub(super) async fn parse_input(
         })),
         // We don't need to handle Opened explicitly, because that will trigger the Assigned event
         IssuesAction::Reopened
+        | IssuesAction::ReadyForReview
+        | IssuesAction::ConvertedToDraft
         | IssuesAction::Closed
         | IssuesAction::Deleted
         | IssuesAction::Transferred
@@ -107,7 +109,7 @@ pub(super) async fn handle_input<'a>(
     // If the PR doesn't wait for a review, remove it from the workqueue completely.
     // This handles situations such as labels being modified, which make the PR no longer to be
     // in the "waiting for a review" state, or the PR being closed/merged.
-    if !waits_for_a_review(&pr.labels, pr.is_open(), pr.draft) {
+    if !waits_for_a_review(&pr.labels, &pr.assignees, &pr.user, pr.is_open(), pr.draft) {
         log::info!(
             "Removing PR {pr_number} from workqueue, because it is not waiting for a review.",
         );
@@ -200,8 +202,23 @@ pub async fn retrieve_pull_request_assignments(
             .into_iter()
             .map(|l| Label { name: l.name })
             .collect::<Vec<Label>>();
+        let assignees = pr
+            .assignees
+            .as_ref()
+            .map(|authors| authors.iter().map(User::from).collect::<Vec<_>>())
+            .unwrap_or_default();
+        let author = pr
+            .user
+            .as_ref()
+            .map(|author| User::from(author.as_ref()))
+            .unwrap_or_else(|| User {
+                login: "ghost".to_string(),
+                id: 0,
+            });
         if waits_for_a_review(
             &labels,
+            &assignees,
+            &author,
             pr.state == Some(IssueState::Open),
             pr.draft.unwrap_or_default(),
         ) {
@@ -269,14 +286,26 @@ fn delete_pr_from_all_queues(workqueue: &mut ReviewerWorkqueue, pr: PullRequestN
 ///
 /// Note: this functionality is currently hardcoded for rust-lang/rust, other repos might use
 /// different labels.
-fn waits_for_a_review(labels: &[Label], is_open: bool, is_draft: bool) -> bool {
+fn waits_for_a_review(
+    labels: &[Label],
+    assignees: &[User],
+    author: &User,
+    is_open: bool,
+    is_draft: bool,
+) -> bool {
     let is_blocked = labels
         .iter()
         .any(|l| l.name == "S-blocked" || l.name == "S-inactive");
     let is_rollup = labels.iter().any(|l| l.name == "rollup");
     let is_waiting_for_reviewer = labels.iter().any(|l| l.name == "S-waiting-on-review");
-
-    is_open && !is_draft && !is_blocked && !is_rollup && is_waiting_for_reviewer
+    let is_assigned_to_author = assignees.contains(author);
+
+    is_open
+        && !is_draft
+        && !is_blocked
+        && !is_rollup
+        && is_waiting_for_reviewer
+        && !is_assigned_to_author
 }
 
 #[cfg(test)]

src/zulip.rs

@@ -298,7 +298,7 @@ async fn workqueue_commands(
                 pluralize("PR", assigned_prs.len())
             );
             writeln!(response, "Review capacity: {capacity}\n")?;
-            writeln!(response, "*Note that only selected PRs that are assigned to you are considered as being in the review queue.*")?;
+            writeln!(response, "*Note that only certain PRs that are assigned to you are included in your review queue.*")?;
             response
         }
         "set-pr-limit" => {