Merge pull request #521 from Kobzol/rollup-duplicate

Filter out duplicate PRs when making a rollup

Author: Kobzol

src/github/rollup.rs

@@ -10,6 +10,7 @@ use axum::http::StatusCode;
 use axum::response::{IntoResponse, Redirect, Response};
 use futures::StreamExt;
 use rand::{Rng, distr::Alphanumeric};
+use std::collections::HashSet;
 use std::sync::Arc;
 use tracing::Instrument;
 
@@ -151,16 +152,23 @@ async fn create_rollup(
     let OAuthRollupState {
         repo_name,
         repo_owner,
-        pr_nums,
+        mut pr_nums,
     } = rollup_state;
 
     let username = user_client.username;
 
     tracing::info!("User {username} is creating a rollup with PRs: {pr_nums:?}");
 
+    // Filter out duplicates
+    let mut pr_set = HashSet::new();
+    pr_nums.retain(|&pr| pr_set.insert(pr));
+
     if pr_nums.len() as u64 > ROLLUP_PR_LIMIT {
         return Err(RollupError::TooManyPullRequests);
     }
+    if pr_nums.is_empty() {
+        return Err(RollupError::NoPullRequestsSelected);
+    }
 
     // Ensure user has a fork
     match user_client.client.get_repo().await {
@@ -198,10 +206,6 @@ async fn create_rollup(
         }
     }
 
-    if rollup_prs.is_empty() {
-        return Err(RollupError::NoPullRequestsSelected);
-    }
-
     // Sort PRs by priority and then PR number
     // We want to try to merge PRs with higher priority first, so that in case of conflicts they
     // get in, rather than being left out.
@@ -553,6 +557,31 @@ mod tests {
         ");
     }
 
+    #[sqlx::test]
+    async fn rollup_duplicates(pool: sqlx::PgPool) {
+        let gh = run_test((pool, rollup_state()), async |ctx: &mut BorsTester| {
+            let pr2 = ctx.open_pr(default_repo_name(), |_| {}).await?;
+            let pr3 = ctx.open_pr(default_repo_name(), |_| {}).await?;
+            ctx.approve(pr2.id()).await?;
+            ctx.approve(pr3.id()).await?;
+
+            make_rollup(ctx, &[&pr3, &pr2, &pr3, &pr2])
+                .await?
+                .assert_status(StatusCode::TEMPORARY_REDIRECT);
+            Ok(())
+        })
+        .await;
+        let repo = gh.get_repo(());
+        insta::assert_snapshot!(repo.lock().get_pr(4).description, @r"
+        Successful merges:
+
+         - #2 (Title of PR 2)
+         - #3 (Title of PR 3)
+
+        r? @ghost
+        ");
+    }
+
     async fn make_rollup(
         ctx: &mut BorsTester,
         prs: &[&PullRequest],