Repository: MarcoIeni/release-plz (rust)

Issue:
Release pr command updates the PR if it already exists
## Motivations

At the moment the `release-pr` command creates a GitHub PR where it updates the version of the packages.
The problem is that, when you run it again, this command will create a new GitHub pr instead of updating the existing one.

## Solution

Before creating a pr, release-plz should check if a "release" branch already exists.
In that case it should commit the changes to that branch, instead of creating a new one and raising a new pr.


Pull request description:
feat: close old release-plz prs
<!-- Please explain the changes you made -->

<!--
Please, make sure:
- you have read the contributing guidelines:
  https://github.com/MarcoIeni/release-plz/blob/main/docs/CONTRIBUTING.md
- you have formatted the code using rustfmt:
  https://github.com/rust-lang/rustfmt
- you have checked that all tests pass, by running `cargo test --all`
- you have updated the changelog (if needed):
  https://github.com/MarcoIeni/release-plz/blob/main/CHANGELOG.md
-->
Close #12

Patch:
diff --git a/Cargo.lock b/Cargo.lock
index ab3f6ffd..1208a64e 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -2308,6 +2308,7 @@ dependencies = [
  "next_version",
  "octocrab",
  "secrecy",
+ "serde_json",
  "tempfile",
  "test_logs",
  "tokio",
diff --git a/crates/release_plz/src/args.rs b/crates/release_plz/src/args.rs
index ed1d91ce..cee5da83 100644
--- a/crates/release_plz/src/args.rs
+++ b/crates/release_plz/src/args.rs
@@ -18,7 +18,8 @@ pub enum Command {
     /// Update packages version based on commit messages.
     Update(Update),
     /// Create a Pull Request representing the next release.
-    /// The Pull request contains updated packages version based on commit messages.
+    /// The Pull request contains updated packages version and changelog based on commit messages.
+    /// Close old PRs opened by release-plz, too.
     ReleasePr(ReleasePr),
 }
 
diff --git a/crates/release_plz_core/Cargo.toml b/crates/release_plz_core/Cargo.toml
index f57ffff9..1f3f8581 100644
--- a/crates/release_plz_core/Cargo.toml
+++ b/crates/release_plz_core/Cargo.toml
@@ -39,3 +39,4 @@ tokio = { version = "1", features = ["macros"] }
 wiremock = "0.5"
 fake = "2"
 claim = "0.5"
+serde_json = "1"
diff --git a/crates/release_plz_core/src/github_client.rs b/crates/release_plz_core/src/github_client.rs
new file mode 100644
index 00000000..27a1c9fd
--- /dev/null
+++ b/crates/release_plz_core/src/github_client.rs
@@ -0,0 +1,140 @@
+use anyhow::Context;
+use octocrab::{models::IssueState, params, Octocrab, OctocrabBuilder};
+use secrecy::{ExposeSecret, SecretString};
+use tracing::{info, instrument, Span};
+use url::Url;
+
+#[derive(Debug)]
+pub struct GitHubClient<'a> {
+    github: &'a GitHub,
+    client: Octocrab,
+}
+
+#[derive(Debug)]
+pub struct Pr {
+    pub branch: String,
+    pub title: String,
+}
+
+impl<'a> GitHubClient<'a> {
+    pub fn new(github: &'a GitHub) -> anyhow::Result<Self> {
+        let mut octocrab_builder =
+            OctocrabBuilder::new().personal_token(github.token.expose_secret().clone());
+
+        if let Some(base_url) = &github.base_url {
+            octocrab_builder = octocrab_builder
+                .base_url(base_url.clone())
+                .context("Invalid GitHub base url")?;
+        }
+
+        let client = octocrab_builder
+            .build()
+            .context("Failed to build GitHub client")?;
+
+        Ok(Self { github, client })
+    }
+
+    /// Close all Prs which branch starts with the given `branch_prefix`.
+    pub async fn close_prs_on_branches(&self, branch_prefix: &str) -> anyhow::Result<()> {
+        let pulls = self.client.pulls(&self.github.owner, &self.github.repo);
+
+        let mut i: u32 = 1;
+        let page_size = 30;
+        loop {
+            let prs = pulls
+                .list()
+                .state(params::State::Open)
+                .per_page(page_size)
+                .page(i)
+                .send()
+                .await
+                .context("Failed to retrieve PRs")?
+                .take_items();
+            let release_prs = prs
+                .iter()
+                .filter(|&pr| pr.head.ref_field.starts_with(branch_prefix))
+                .map(|pr| pr.number);
+            for release_pr in release_prs {
+                self.close_pr(release_pr).await?;
+            }
+
+            if prs.len() < page_size as usize {
+                break;
+            }
+            i += 1;
+        }
+        Ok(())
+    }
+
+    async fn close_pr(&self, pr_number: u64) -> anyhow::Result<()> {
+        self.client
+            .issues(&self.github.owner, &self.github.repo)
+            .update(pr_number)
+            .state(IssueState::Closed)
+            .send()
+            .await
+            .with_context(|| format!("cannot close pr {pr_number}"))?;
+        Ok(())
+    }
+
+    #[instrument(
+        fields(
+            default_branch = tracing::field::Empty,
+        )
+    )]
+    pub async fn open_pr(&self, pr: &Pr) -> anyhow::Result<()> {
+        let default_branch = self
+            .client
+            .repos(&self.github.owner, &self.github.repo)
+            .get()
+            .await
+            .context(format!(
+                "failed to retrieve GitHub repository {}/{}",
+                self.github.owner, self.github.repo
+            ))?
+            .default_branch
+            .context("failed to retrieve default branch")?;
+        Span::current().record("default_branch", &default_branch.as_str());
+
+        let pr = self
+            .client
+            .pulls(&self.github.owner, &self.github.repo)
+            .create(&pr.title, &pr.branch, default_branch)
+            .body("release-plz automatic bot")
+            .send()
+            .await
+            .context("Failed to open PR")?;
+
+        if let Some(url) = pr.html_url {
+            info!("opened pr: {}", url);
+        }
+
+        Ok(())
+    }
+}
+
+#[derive(Debug)]
+pub struct GitHub {
+    pub owner: String,
+    pub repo: String,
+    pub token: SecretString,
+    base_url: Option<Url>,
+}
+
+impl GitHub {
+    pub fn new(owner: String, repo: String, token: SecretString) -> Self {
+        Self {
+            owner,
+            repo,
+            token,
+            base_url: None,
+        }
+    }
+
+    pub fn with_base_url(self, base_url: Url) -> Self {
+        Self {
+            base_url: Some(base_url),
+            ..self
+        }
+    }
+}
diff --git a/crates/release_plz_core/src/lib.rs b/crates/release_plz_core/src/lib.rs
index 4b752467..064a9a8b 100644
--- a/crates/release_plz_core/src/lib.rs
+++ b/crates/release_plz_core/src/lib.rs
@@ -1,6 +1,7 @@
 mod changelog;
 mod diff;
 mod download;
+mod github_client;
 mod next_ver;
 mod registry_packages;
 mod release_pr;
@@ -10,6 +11,7 @@ mod version;
 
 pub use changelog::*;
 pub use download::read_package;
+pub use github_client::GitHub;
 pub use next_ver::*;
 pub use release_pr::*;
 pub use update::*;
diff --git a/crates/release_plz_core/src/release_pr.rs b/crates/release_plz_core/src/release_pr.rs
index fa4743d0..3cf019f1 100644
--- a/crates/release_plz_core/src/release_pr.rs
+++ b/crates/release_plz_core/src/release_pr.rs
@@ -5,12 +5,15 @@ use chrono::SecondsFormat;
 use git_cmd::Repo;
 
 use anyhow::{anyhow, Context};
-use octocrab::OctocrabBuilder;
-use secrecy::{ExposeSecret, SecretString};
-use tracing::{info, instrument, Span};
-use url::Url;
+use tracing::instrument;
 
-use crate::{copy_to_temp_dir, update, UpdateRequest, UpdateResult, CARGO_TOML};
+use crate::{
+    copy_to_temp_dir,
+    github_client::{GitHub, GitHubClient, Pr},
+    update, UpdateRequest, UpdateResult, CARGO_TOML,
+};
+
+const BRANCH_PREFIX: &str = "release-plz/";
 
 #[derive(Debug)]
 pub struct ReleasePrRequest {
@@ -18,32 +21,6 @@ pub struct ReleasePrRequest {
     pub update_request: UpdateRequest,
 }
 
-#[derive(Debug)]
-pub struct GitHub {
-    pub owner: String,
-    pub repo: String,
-    pub token: SecretString,
-    base_url: Option<Url>,
-}
-
-impl GitHub {
-    pub fn new(owner: String, repo: String, token: SecretString) -> Self {
-        Self {
-            owner,
-            repo,
-            token,
-            base_url: None,
-        }
-    }
-
-    pub fn with_base_url(self, base_url: Url) -> Self {
-        Self {
-            base_url: Some(base_url),
-            ..self
-        }
-    }
-}
-
 /// Open a pull request with the next packages versions of a local rust project
 #[instrument]
 pub async fn release_pr(input: &ReleasePrRequest) -> anyhow::Result<()> {
@@ -66,24 +43,23 @@ pub async fn release_pr(input: &ReleasePrRequest) -> anyhow::Result<()> {
         .set_local_manifest(local_manifest)
         .context("can't find temporary project")?;
     let (packages_to_update, _repository) = update(&new_update_request)?;
+    let gh_client = GitHubClient::new(&input.github)?;
+    gh_client
+        .close_prs_on_branches(BRANCH_PREFIX)
+        .await
+        .context("cannot close old release-plz prs")?;
     if !packages_to_update.is_empty() {
         let repo = Repo::new(new_manifest_dir)?;
-        let pr = Pr::new(&packages_to_update);
+        let pr = Pr::from(packages_to_update.as_ref());
         create_release_branch(&repo, &pr.branch)?;
-        open_pr(&pr, &input.github).await?;
+        gh_client.open_pr(&pr).await?;
     }
 
     Ok(())
 }
 
-#[derive(Debug)]
-struct Pr {
-    branch: String,
-    title: String,
-}
-
-impl Pr {
-    fn new(packages_to_update: &[(Package, UpdateResult)]) -> Self {
+impl From<&[(Package, UpdateResult)]> for Pr {
+    fn from(packages_to_update: &[(Package, UpdateResult)]) -> Self {
         Self {
             branch: release_branch(),
             title: pr_title(packages_to_update),
@@ -97,7 +73,7 @@ fn release_branch() -> String {
     let now = now.to_rfc3339_opts(SecondsFormat::Secs, true);
     // ':' is not a valid character for a branch name.
     let now = now.replace(':', "-");
-    format!("release-plz/{now}")
+    format!("{BRANCH_PREFIX}{now}")
 }
 
 fn pr_title(packages_to_update: &[(Package, UpdateResult)]) -> String {
@@ -109,51 +85,6 @@ fn pr_title(packages_to_update: &[(Package, UpdateResult)]) -> String {
     }
 }
 
-#[instrument(
-    fields(
-        default_branch = tracing::field::Empty,
-    )
-)]
-async fn open_pr(pr: &Pr, github: &GitHub) -> anyhow::Result<()> {
-    let mut octocrab_builder =
-        OctocrabBuilder::new().personal_token(github.token.expose_secret().clone());
-
-    if let Some(base_url) = &github.base_url {
-        octocrab_builder = octocrab_builder
-            .base_url(base_url.clone())
-            .context("Invalid GitHub base url")?;
-    }
-
-    let client = octocrab_builder
-        .build()
-        .context("Failed to build GitHub client")?;
-
-    let default_branch = client
-        .repos(&github.owner, &github.repo)
-        .get()
-        .await
-        .context(format!(
-            "failed to retrieve GitHub repository {}/{}",
-            github.owner, github.repo
-        ))?
-        .default_branch
-        .context("failed to retrieve default branch")?;
-    Span::current().record("default_branch", &default_branch.as_str());
-
-    let pr = client
-        .pulls(&github.owner, &github.repo)
-        .create(&pr.title, &pr.branch, default_branch)
-        .body("release-plz automatic bot")
-        .send()
-        .await?;
-
-    if let Some(url) = pr.html_url {
-        info!("opened pr: {}", url);
-    }
-
-    Ok(())
-}
-
 fn create_release_branch(repository: &Repo, release_branch: &str) -> anyhow::Result<()> {
     repository.checkout_new_branch(release_branch)?;
     repository.add_all_and_commit("chore: release")?;


Test patch:
diff --git a/crates/release_plz_core/tests/all/helpers/comparison_test.rs b/crates/release_plz_core/tests/all/helpers/comparison_test.rs
index 43db6852..76294679 100644
--- a/crates/release_plz_core/tests/all/helpers/comparison_test.rs
+++ b/crates/release_plz_core/tests/all/helpers/comparison_test.rs
@@ -9,10 +9,13 @@ use secrecy::Secret;
 use tempfile::{tempdir, TempDir};
 use url::Url;
 
+use super::github_mock_server::GitHubMockServer;
+
 /// Compare local project with the one in cargo registry
 pub struct ComparisonTest {
     local_project: TempDir,
     registry_project: TempDir,
+    github_mock_server: GitHubMockServer,
 }
 
 const PROJECT_NAME: &str = "myproject";
@@ -20,7 +23,7 @@ pub const OWNER: &str = "owner";
 pub const REPO: &str = "repo";
 
 impl ComparisonTest {
-    pub fn new() -> Self {
+    pub async fn new() -> Self {
         test_logs::init();
         let local_project_dir = tempdir().unwrap();
         let local_project = local_project_dir.as_ref().join(PROJECT_NAME);
@@ -30,6 +33,7 @@ impl ComparisonTest {
         let comparison = Self {
             local_project: local_project_dir,
             registry_project,
+            github_mock_server: GitHubMockServer::start(OWNER, REPO).await,
         };
         fs::copy(
             comparison.registry_project().join(CARGO_TOML),
@@ -67,7 +71,8 @@ impl ComparisonTest {
         }
     }
 
-    pub async fn open_release_pr(&self, base_url: Url) -> anyhow::Result<()> {
+    pub async fn open_release_pr(&self) -> anyhow::Result<()> {
+        let base_url = self.github_mock_server.base_url();
         let release_pr_request = self.release_pr_request(base_url);
         release_plz_core::release_pr(&release_pr_request).await
     }
@@ -105,4 +110,10 @@ impl ComparisonTest {
     fn local_project_changelog_path(&self) -> PathBuf {
         self.local_project().join(CHANGELOG_FILENAME)
     }
+
+    /// Get a reference to the comparison test's github mock server.
+    #[must_use]
+    pub fn github_mock_server(&self) -> &GitHubMockServer {
+        &self.github_mock_server
+    }
 }
diff --git a/crates/release_plz_core/tests/all/helpers/github_mock_server.rs b/crates/release_plz_core/tests/all/helpers/github_mock_server.rs
index c506aead..646b965b 100644
--- a/crates/release_plz_core/tests/all/helpers/github_mock_server.rs
+++ b/crates/release_plz_core/tests/all/helpers/github_mock_server.rs
@@ -1,14 +1,21 @@
 use url::Url;
-use wiremock::{matchers::any, Mock, MockServer, ResponseTemplate};
+use wiremock::{
+    matchers::{method, path},
+    Mock, MockServer, ResponseTemplate,
+};
 
 pub struct GitHubMockServer {
     server: MockServer,
+    owner: &'static str,
+    repo: &'static str,
 }
 
 impl GitHubMockServer {
-    pub async fn start() -> Self {
+    pub async fn start(owner: &'static str, repo: &'static str) -> Self {
         Self {
             server: MockServer::start().await,
+            owner,
+            repo,
         }
     }
 
@@ -16,12 +23,31 @@ impl GitHubMockServer {
         Url::parse(&self.server.uri()).unwrap()
     }
 
-    /// Api was never called
-    pub async fn radio_silence(&self) {
-        Mock::given(any())
+    /// Nobody tried to open a PR.
+    pub async fn expect_no_created_prs(&self) {
+        Mock::given(method("POST"))
+            .and(path(self.pulls_path()))
             .respond_with(ResponseTemplate::new(200))
             .expect(0)
             .mount(&self.server)
             .await;
     }
+
+    /// Return an empty list of PRs.
+    pub async fn no_prs(&self) {
+        let no_prs = {
+            let empty_body = serde_json::Value::Array(vec![]);
+            ResponseTemplate::new(200).set_body_json(empty_body)
+        };
+        Mock::given(method("GET"))
+            .and(path(self.pulls_path()))
+            .respond_with(no_prs)
+            .expect(1)
+            .mount(&self.server)
+            .await;
+    }
+
+    fn pulls_path(&self) -> String {
+        format!("/repos/{}/{}/pulls", self.owner, self.repo)
+    }
 }
diff --git a/crates/release_plz_core/tests/all/release_pr.rs b/crates/release_plz_core/tests/all/release_pr.rs
index d9eb6c95..a81cbf2b 100644
--- a/crates/release_plz_core/tests/all/release_pr.rs
+++ b/crates/release_plz_core/tests/all/release_pr.rs
@@ -1,12 +1,12 @@
-use crate::helpers::{comparison_test::ComparisonTest, github_mock_server::GitHubMockServer};
+use crate::helpers::comparison_test::ComparisonTest;
 
 #[tokio::test]
 async fn up_to_date_project_should_not_raise_pr() {
-    let comparison_test = ComparisonTest::new();
-    let github_mock_server = GitHubMockServer::start().await;
+    let comparison_test = ComparisonTest::new().await;
     comparison_test
-        .open_release_pr(github_mock_server.base_url())
-        .await
-        .unwrap();
-    github_mock_server.radio_silence().await;
+        .github_mock_server()
+        .expect_no_created_prs()
+        .await;
+    comparison_test.github_mock_server().no_prs().await;
+    comparison_test.open_release_pr().await.unwrap();
 }
diff --git a/crates/release_plz_core/tests/all/update.rs b/crates/release_plz_core/tests/all/update.rs
index 05da3846..accc56b1 100644
--- a/crates/release_plz_core/tests/all/update.rs
+++ b/crates/release_plz_core/tests/all/update.rs
@@ -3,9 +3,9 @@ use release_plz_core::{read_package, CHANGELOG_HEADER};
 
 use crate::helpers::{comparison_test::ComparisonTest, user_mock};
 
-#[test]
-fn up_to_date_project_is_not_touched() {
-    let comparison_test = ComparisonTest::new();
+#[tokio::test]
+async fn up_to_date_project_is_not_touched() {
+    let comparison_test = ComparisonTest::new().await;
 
     comparison_test.run_update();
 
@@ -13,9 +13,9 @@ fn up_to_date_project_is_not_touched() {
     assert!(comparison_test.are_projects_equal());
 }
 
-#[test]
-fn version_is_updated_when_project_changed() {
-    let comparison_test = ComparisonTest::new();
+#[tokio::test]
+async fn version_is_updated_when_project_changed() {
+    let comparison_test = ComparisonTest::new().await;
     let feature_message = "do awesome stuff";
     user_mock::add_feature(&comparison_test.local_project(), feature_message);
 
@@ -41,15 +41,15 @@ fn version_is_updated_when_project_changed() {
     .assert_eq(&comparison_test.local_project_changelog());
 }
 
-#[test]
-fn changelog_is_updated_if_changelog_already_exists() {
+#[tokio::test]
+async fn changelog_is_updated_if_changelog_already_exists() {
     let old_body = r#"
 ## [0.1.0] - 1970-01-01
 
 ### Fixed
 - fix important bug
 "#;
-    let comparison_test = ComparisonTest::new();
+    let comparison_test = ComparisonTest::new().await;
     let old_changelog = format!("{CHANGELOG_HEADER}{old_body}");
     comparison_test.write_local_project_changelog(&old_changelog);
     let feature_message = "do awesome stuff";