Merge pull request #12379 from gitbutlerapp/merge-review

forge: merge API

Author: estib-vega

crates/but-api/src/legacy/forge.rs

@@ -221,6 +221,31 @@ pub async fn publish_review(
     )
     .await
 }
+
+/// Merge a review on the forge.
+#[but_api]
+#[instrument(err(Debug))]
+pub async fn merge_review(ctx: ThreadSafeContext, review_id: usize) -> Result<()> {
+    let (storage, base_branch, preferred_forge_user) = {
+        let ctx = ctx.into_thread_local();
+        let base_branch = gitbutler_branch_actions::base::get_base_branch_data(&ctx)?;
+        (
+            but_forge_storage::Controller::from_path(but_path::app_data_dir()?),
+            base_branch,
+            ctx.legacy_project.preferred_forge_user.clone(),
+        )
+    };
+    but_forge::merge_review(
+        &preferred_forge_user,
+        &base_branch
+            .forge_repo_info
+            .context("No forge could be determined for this repository branch")?,
+        review_id,
+        &storage,
+    )
+    .await
+}
+
 /// Update the stacked review descriptions to have the correct footers.
 #[but_api]
 #[instrument(err(Debug))]

crates/but-forge/src/lib.rs

@@ -11,7 +11,7 @@ pub use review::{
     CacheConfig, CreateForgeReviewParams, ForgeAccountValidity, ForgeReview, ForgeReviewDescriptionUpdate,
     ForgeReviewFilter, ReviewTemplateFunctions, available_review_templates, check_forge_account_is_valid,
     create_forge_review, get_forge_review, get_review_template_functions, list_forge_reviews_for_branch,
-    list_forge_reviews_with_cache, update_review_description_tables,
+    list_forge_reviews_with_cache, merge_review, update_review_description_tables,
 };
 
 fn determine_forge_from_host(host: &str) -> Option<ForgeName> {

crates/but-forge/src/review.rs

@@ -3,7 +3,7 @@ use std::{
     path::{self},
 };
 
-use anyhow::{Error, Result};
+use anyhow::{Context as _, Error, Result};
 use but_fs::list_files;
 use but_github::CredentialCheckResult;
 use but_gitlab::GitLabProjectId;
@@ -733,6 +733,50 @@ pub async fn get_forge_review(
     }
 }
 
+/// Merge a review to it's target branch
+pub async fn merge_review(
+    preferred_forge_user: &Option<crate::ForgeUser>,
+    forge_repo_info: &crate::forge::ForgeRepoInfo,
+    review_number: usize,
+    storage: &but_forge_storage::Controller,
+) -> Result<()> {
+    let crate::forge::ForgeRepoInfo { forge, owner, repo, .. } = forge_repo_info;
+    match forge {
+        ForgeName::GitHub => {
+            let preferred_account = preferred_forge_user.as_ref().and_then(|user| user.github());
+            let pr_number = review_number
+                .try_into()
+                .context("PR: Failed to cast usize to i64, somehow")?;
+            let params = but_github::MergePullRequestParams {
+                owner,
+                repo,
+                pr_number,
+                commit_message: None,
+                commit_title: None,
+                merge_method: None,
+            };
+            but_github::pr::merge(preferred_account, params, storage).await
+        }
+        ForgeName::GitLab => {
+            let preferred_account = preferred_forge_user.as_ref().and_then(|user| user.gitlab());
+            let project_id = GitLabProjectId::new(owner, repo);
+            let mr_iid = review_number
+                .try_into()
+                .context("MR: Failed to cast usize to i64, somehow")?;
+            let params = but_gitlab::MergeMergeRequestParams {
+                project_id,
+                mr_iid,
+                squash: None,
+            };
+
+            but_gitlab::mr::merge(preferred_account, params, storage).await
+        }
+        _ => Err(Error::msg(format!(
+            "Merging reviews for forge {forge:?} is not implemented yet.",
+        ))),
+    }
+}
+
 #[derive(Debug, Clone, Serialize, Deserialize)]
 #[serde(rename_all = "camelCase")]
 pub struct CreateForgeReviewParams {

crates/but-github/src/client.rs

@@ -232,6 +232,39 @@ impl GitHubClient {
         let pr: GitHubPullRequest = response.json().await?;
         Ok(pr.into())
     }
+
+    pub async fn merge_pull_request(&self, params: &MergePullRequestParams<'_>) -> Result<()> {
+        #[derive(Serialize)]
+        struct MergePullRequestBody<'a> {
+            #[serde(skip_serializing_if = "Option::is_none")]
+            commit_title: Option<&'a str>,
+            #[serde(skip_serializing_if = "Option::is_none")]
+            commit_message: Option<&'a str>,
+            #[serde(skip_serializing_if = "Option::is_none")]
+            merge_method: Option<&'a str>,
+        }
+
+        let url = format!(
+            "{}/repos/{}/{}/pulls/{}/merge",
+            self.base_url, params.owner, params.repo, params.pr_number
+        );
+
+        let merge_method = params.merge_method.as_ref().map(Into::into);
+
+        let body = MergePullRequestBody {
+            commit_title: params.commit_title,
+            commit_message: params.commit_message,
+            merge_method,
+        };
+
+        let response = self.client.put(&url).json(&body).send().await?;
+
+        if !response.status().is_success() {
+            bail!("Failed to merge pull request: {}", response.status());
+        }
+
+        Ok(())
+    }
 }
 
 pub struct CreatePullRequestParams<'a> {
@@ -254,6 +287,35 @@ pub struct UpdatePullRequestParams<'a> {
     pub state: Option<&'a str>,
 }
 
+#[derive(Debug, Clone, Serialize, Deserialize, Default)]
+#[serde(rename_all = "lowercase")]
+pub enum MergeMethod {
+    #[default]
+    Merge,
+    Squash,
+    Rebase,
+}
+
+impl From<&MergeMethod> for &str {
+    fn from(val: &MergeMethod) -> Self {
+        match val {
+            MergeMethod::Merge => "merge",
+            MergeMethod::Rebase => "rebase",
+            MergeMethod::Squash => "squash",
+        }
+    }
+}
+
+#[derive(Debug, Clone)]
+pub struct MergePullRequestParams<'a> {
+    pub owner: &'a str,
+    pub repo: &'a str,
+    pub pr_number: i64,
+    pub commit_title: Option<&'a str>,
+    pub commit_message: Option<&'a str>,
+    pub merge_method: Option<MergeMethod>,
+}
+
 #[derive(Debug, Serialize)]
 pub struct AuthenticatedUser {
     pub login: String,

crates/but-github/src/lib.rs

@@ -8,7 +8,8 @@ use serde::{Deserialize, Serialize};
 mod client;
 pub mod pr;
 pub use client::{
-    CheckRun, CreatePullRequestParams, GitHubClient, GitHubPrLabel, GitHubUser, PullRequest, UpdatePullRequestParams,
+    CheckRun, CreatePullRequestParams, GitHubClient, GitHubPrLabel, GitHubUser, MergeMethod, MergePullRequestParams,
+    PullRequest, UpdatePullRequestParams,
 };
 mod token;
 pub use token::GithubAccountIdentifier;

crates/but-github/src/pr.rs

@@ -70,3 +70,14 @@ pub async fn update(
         .context("Failed to update pull request")?;
     Ok(pr)
 }
+
+pub async fn merge(
+    preferred_account: Option<&crate::GithubAccountIdentifier>,
+    params: crate::client::MergePullRequestParams<'_>,
+    storage: &but_forge_storage::Controller,
+) -> Result<()> {
+    GitHubClient::from_storage(storage, preferred_account)?
+        .merge_pull_request(&params)
+        .await
+        .context("Failed to merge PR")
+}

crates/but-gitlab/src/client.rs

@@ -183,6 +183,29 @@ impl GitLabClient {
         let mr: GitLabMergeRequest = response.json().await?;
         Ok(mr.into())
     }
+
+    pub async fn merge_merge_request(&self, params: &MergeMergeRequestParams) -> Result<()> {
+        #[derive(Serialize)]
+        struct MergeMergeRequestBody {
+            #[serde(skip_serializing_if = "Option::is_none")]
+            squash: Option<bool>,
+        }
+
+        let url = format!(
+            "{}/projects/{}/merge_requests/{}/merge",
+            self.base_url, params.project_id, params.mr_iid
+        );
+
+        let body = MergeMergeRequestBody { squash: params.squash };
+
+        let response = self.client.put(&url).json(&body).send().await?;
+
+        if !response.status().is_success() {
+            bail!("Failed to merge merge request: {}", response.status());
+        }
+
+        Ok(())
+    }
 }
 
 pub struct CreateMergeRequestParams<'a> {
@@ -192,6 +215,11 @@ pub struct CreateMergeRequestParams<'a> {
     pub target_branch: &'a str,
     pub project_id: GitLabProjectId,
 }
+pub struct MergeMergeRequestParams {
+    pub project_id: GitLabProjectId,
+    pub mr_iid: i64,
+    pub squash: Option<bool>,
+}
 
 #[derive(Debug, Serialize)]
 pub struct AuthenticatedUser {

crates/but-gitlab/src/lib.rs

@@ -4,7 +4,9 @@ use but_secret::Sensitive;
 mod client;
 pub mod mr;
 mod project;
-pub use client::{CreateMergeRequestParams, GitLabClient, GitLabLabel, GitLabUser, MergeRequest};
+pub use client::{
+    CreateMergeRequestParams, GitLabClient, GitLabLabel, GitLabUser, MergeMergeRequestParams, MergeRequest,
+};
 pub use project::GitLabProjectId;
 mod token;
 use serde::Serialize;

crates/but-gitlab/src/mr.rs

@@ -56,3 +56,14 @@ pub async fn get(
         .context("Failed to get merge request")?;
     Ok(mr)
 }
+
+pub async fn merge(
+    preferred_account: Option<&crate::GitlabAccountIdentifier>,
+    params: crate::client::MergeMergeRequestParams,
+    storage: &but_forge_storage::Controller,
+) -> Result<()> {
+    GitLabClient::from_storage(storage, preferred_account)?
+        .merge_merge_request(&params)
+        .await
+        .context("Faile to merge MR")
+}

crates/but-server/src/lib.rs

@@ -927,6 +927,16 @@ async fn handle_command(
                 Err(e) => Err(e),
             }
         }
+        "merge_review" => {
+            let params = deserialize_json(request.params);
+            match params {
+                Ok(params) => {
+                    let result = legacy::forge::merge_review_cmd(params).await;
+                    result.map(|_| json!({"result": "success"}))
+                }
+                Err(e) => Err(e),
+            }
+        }
         "update_review_footers" => {
             let params = deserialize_json(request.params);
             match params {