Merge pull request #2053 from Kobzol/work-show-render

Store PR title in the workqueue and output it in `work show`

Author: Kobzol

src/handlers/assign/tests/tests_candidates.rs

@@ -3,7 +3,7 @@
 use super::super::*;
 use crate::db::review_prefs::{upsert_review_prefs, RotationMode};
 use crate::github::{PullRequestNumber, User};
-use crate::handlers::pr_tracking::ReviewerWorkqueue;
+use crate::handlers::pr_tracking::{AssignedPullRequest, ReviewerWorkqueue};
 use crate::tests::github::{issue, user};
 use crate::tests::{run_db_test, TestContext};
 
@@ -13,7 +13,7 @@ struct AssignCtx {
     teams: Teams,
     config: AssignConfig,
     issue: Issue,
-    reviewer_workqueue: HashMap<UserId, HashSet<PullRequestNumber>>,
+    reviewer_workqueue: HashMap<UserId, HashMap<PullRequestNumber, AssignedPullRequest>>,
 }
 
 impl AssignCtx {
@@ -50,7 +50,16 @@ impl AssignCtx {
     }
 
     fn assign_prs(mut self, user_id: UserId, count: u64) -> Self {
-        let prs: HashSet<PullRequestNumber> = (0..count).collect();
+        let prs = (0..count)
+            .map(|pr_number| {
+                (
+                    pr_number,
+                    AssignedPullRequest {
+                        title: format!("PR {pr_number}"),
+                    },
+                )
+            })
+            .collect();
         self.reviewer_workqueue.insert(user_id, prs);
         self
     }

src/handlers/pr_tracking.rs

@@ -18,22 +18,29 @@ use octocrab::models::IssueState;
 use octocrab::params::pulls::Sort;
 use octocrab::params::{Direction, State};
 use octocrab::Octocrab;
-use std::collections::{HashMap, HashSet};
+use std::collections::HashMap;
 use tokio::sync::RwLockWriteGuard;
 use tracing as log;
 
+#[derive(Clone, Debug)]
+pub struct AssignedPullRequest {
+    pub title: String,
+}
+
 /// Maps users to a set of currently assigned open non-draft pull requests.
 /// We store this map in memory, rather than in the DB, because it can get desynced when webhooks
 /// are missed.
 /// It is thus reloaded when triagebot starts and also periodically, so it is not needed to store it
 /// in the DB.
 #[derive(Debug, Default)]
 pub struct ReviewerWorkqueue {
-    reviewers: HashMap<UserId, HashSet<PullRequestNumber>>,
+    reviewers: HashMap<UserId, HashMap<PullRequestNumber, AssignedPullRequest>>,
 }
 
 impl ReviewerWorkqueue {
-    pub fn new(reviewers: HashMap<UserId, HashSet<PullRequestNumber>>) -> Self {
+    pub fn new(
+        reviewers: HashMap<UserId, HashMap<PullRequestNumber, AssignedPullRequest>>,
+    ) -> Self {
         Self { reviewers }
     }
 
@@ -112,6 +119,10 @@ pub(super) async fn handle_input<'a>(
         return Ok(());
     }
 
+    let assigned_pr = AssignedPullRequest {
+        title: pr.title.clone(),
+    };
+
     match input {
         // The PR was assigned to a specific user, and it is waiting for a review.
         ReviewPrefsInput::Assigned { assignee } => {
@@ -120,7 +131,7 @@ pub(super) async fn handle_input<'a>(
                 assignee.login
             );
 
-            upsert_pr_into_user_queue(&mut workqueue, assignee.id, pr_number);
+            upsert_pr_into_user_queue(&mut workqueue, assignee.id, pr_number, assigned_pr);
         }
         ReviewPrefsInput::Unassigned { assignee } => {
             log::info!(
@@ -139,7 +150,12 @@ pub(super) async fn handle_input<'a>(
         // receive.
         ReviewPrefsInput::OtherChange => {
             for assignee in &event.issue.assignees {
-                if upsert_pr_into_user_queue(&mut workqueue, assignee.id, pr_number) {
+                if upsert_pr_into_user_queue(
+                    &mut workqueue,
+                    assignee.id,
+                    pr_number,
+                    assigned_pr.clone(),
+                ) {
                     log::info!("Adding PR {pr_number} to workqueue of {}.", assignee.login);
                 }
             }
@@ -155,10 +171,11 @@ pub async fn load_workqueue(client: &Octocrab) -> anyhow::Result<ReviewerWorkque
     let prs = retrieve_pull_request_assignments("rust-lang", "rust", &client).await?;
 
     // Aggregate PRs by user
-    let aggregated: HashMap<UserId, HashSet<PullRequestNumber>> =
-        prs.into_iter().fold(HashMap::new(), |mut acc, (user, pr)| {
+    let aggregated: HashMap<UserId, HashMap<PullRequestNumber, AssignedPullRequest>> = prs
+        .into_iter()
+        .fold(HashMap::new(), |mut acc, (user, pr_number, pr)| {
             let prs = acc.entry(user.id).or_default();
-            prs.insert(pr);
+            prs.insert(pr_number, pr);
             acc
         });
     tracing::debug!("PR assignments\n{aggregated:?}");
@@ -174,7 +191,7 @@ pub async fn retrieve_pull_request_assignments(
     owner: &str,
     repository: &str,
     client: &Octocrab,
-) -> anyhow::Result<Vec<(User, PullRequestNumber)>> {
+) -> anyhow::Result<Vec<(User, PullRequestNumber, AssignedPullRequest)>> {
     let mut assignments = vec![];
 
     // We use the REST API to fetch open pull requests, as it is much (~5-10x)
@@ -224,6 +241,9 @@ pub async fn retrieve_pull_request_assignments(
                         id: (*user.id).into(),
                     },
                     pr.number,
+                    AssignedPullRequest {
+                        title: pr.title.clone().unwrap_or_default(),
+                    },
                 ));
             }
         }
@@ -234,7 +254,10 @@ pub async fn retrieve_pull_request_assignments(
 }
 
 /// Get pull request assignments for a team member
-pub async fn get_assigned_prs(ctx: &Context, user_id: UserId) -> HashSet<PullRequestNumber> {
+pub async fn get_assigned_prs(
+    ctx: &Context,
+    user_id: UserId,
+) -> HashMap<PullRequestNumber, AssignedPullRequest> {
     ctx.workqueue
         .read()
         .await
@@ -245,15 +268,22 @@ pub async fn get_assigned_prs(ctx: &Context, user_id: UserId) -> HashSet<PullReq
 }
 
 /// Add a PR to the workqueue of a team member.
+/// Updates data of the pull request if it already was in the workqueue.
 /// Ensures no accidental PR duplicates.
 ///
 /// Returns true if the PR was actually inserted.
 fn upsert_pr_into_user_queue(
     workqueue: &mut RwLockWriteGuard<ReviewerWorkqueue>,
     user_id: UserId,
     pr: PullRequestNumber,
+    assigned_pr: AssignedPullRequest,
 ) -> bool {
-    workqueue.reviewers.entry(user_id).or_default().insert(pr)
+    workqueue
+        .reviewers
+        .entry(user_id)
+        .or_default()
+        .insert(pr, assigned_pr)
+        .is_none()
 }
 
 /// Delete a PR from the workqueue of a team member.
@@ -270,7 +300,7 @@ fn delete_pr_from_user_queue(
 /// Delete a PR from the workqueue completely.
 fn delete_pr_from_all_queues(workqueue: &mut ReviewerWorkqueue, pr: PullRequestNumber) {
     for queue in workqueue.reviewers.values_mut() {
-        queue.retain(|pr_number| *pr_number != pr);
+        queue.retain(|pr_number, _| *pr_number != pr);
     }
 }
 
@@ -308,7 +338,9 @@ mod tests {
     use crate::config::Config;
     use crate::github::{Issue, IssuesAction, IssuesEvent, Repository, User};
     use crate::github::{Label, PullRequestNumber};
-    use crate::handlers::pr_tracking::{handle_input, parse_input, upsert_pr_into_user_queue};
+    use crate::handlers::pr_tracking::{
+        handle_input, parse_input, upsert_pr_into_user_queue, AssignedPullRequest,
+    };
     use crate::tests::github::{default_test_user, issue, pull_request, user};
     use crate::tests::{run_db_test, TestContext};
 
@@ -504,7 +536,7 @@ mod tests {
             .get(&user.id)
             .cloned()
             .unwrap_or_default()
-            .into_iter()
+            .into_keys()
             .collect::<Vec<_>>();
         assigned.sort();
         assert_eq!(assigned, expected_prs);
@@ -514,7 +546,14 @@ mod tests {
         {
             let mut workqueue = ctx.handler_ctx().workqueue.write().await;
             for &pr in prs {
-                upsert_pr_into_user_queue(&mut workqueue, user.id, pr);
+                upsert_pr_into_user_queue(
+                    &mut workqueue,
+                    user.id,
+                    pr,
+                    AssignedPullRequest {
+                        title: format!("PR {pr}"),
+                    },
+                );
             }
         }
         check_assigned_prs(&ctx, user, prs).await;

src/zulip.rs

@@ -348,13 +348,7 @@ async fn workqueue_commands(
                 .await
                 .into_iter()
                 .collect::<Vec<_>>();
-            assigned_prs.sort();
-
-            let prs = assigned_prs
-                .iter()
-                .map(|pr| format!("#{pr}"))
-                .collect::<Vec<String>>()
-                .join(", ");
+            assigned_prs.sort_by_key(|(pr_number, _)| *pr_number);
 
             let review_prefs = get_review_prefs(&db_client, gh_id)
                 .await
@@ -372,8 +366,18 @@ async fn workqueue_commands(
                 RotationMode::OffRotation => "off rotation",
             };
 
+            let prs = assigned_prs
+                .iter()
+                .map(|(pr_number, pr)| {
+                    format!(
+                        "- [#{pr_number}](https://github.com/rust-lang/rust/pull/{pr_number}) {}",
+                        pr.title
+                    )
+                })
+                .collect::<Vec<String>>()
+                .join("\n");
             let mut response = format!(
-                "`rust-lang/rust` PRs in your review queue: {prs} ({} {})\n",
+                "`rust-lang/rust` PRs in your review queue ({} {}):\n{prs}\n",
                 assigned_prs.len(),
                 pluralize("PR", assigned_prs.len())
             );