Repository: scala-steward-org/scala-steward (scala)

Issue:
Support for removing source branch on GitLab merge requests
We are currently using existing Gitlab flags (`gitlab-merge-when-pipeline-succeeds` and `gitlab-required-reviewers`) to merge successful merge request automatically. While this flow works great, there is a small nuisance - source branch hangs around after the merge request is merged. Luckily, there is a `remove_source_branch` parameter which we can pass as part of `create merge request` payload to remove source branches once the merge request is merged! 

I already added `remove_source_branch` flag and created a pull request. Happy to address any feedback. Let me know what you think! 

As side note, we already have the following box checked in Gitlab which is a repository level setting but it doesn't apply to merge requests created by scala-steward for some reason;

<img width="479" alt="image" src="https://user-images.githubusercontent.com/15850565/223233672-8dca27f7-39fc-4995-8378-67648cba6db8.png">

Pull request:
Fixes #3006 

Once we agree that this is a useful feature to add, I can update the relevant tests etc.

Patch:
diff --git a/docs/help.md b/docs/help.md
index 3f2a8ae9e8..78ea0316a9 100644
--- a/docs/help.md
+++ b/docs/help.md
@@ -5,7 +5,7 @@ All command line arguments for the `scala-steward` application.
 ```
 Usage:
     scala-steward validate-repo-config
-    scala-steward --workspace <file> --repos-file <file> [--git-author-name <string>] --git-author-email <string> [--git-author-signing-key <string>] --git-ask-pass <file> [--sign-commits] [--forge-type <forge-type>] [--forge-api-host <uri>] --forge-login <string> [--do-not-fork] [--add-labels] [--ignore-opts-files] [--env-var <name=value>]... [--process-timeout <duration>] [--whitelist <string>]... [--read-only <string>]... [--enable-sandbox | --disable-sandbox] [--max-buffer-size <integer>] [--repo-config <uri>]... [--disable-default-repo-config] [--scalafix-migrations <uri>]... [--disable-default-scalafix-migrations] [--artifact-migrations <uri>]... [--disable-default-artifact-migrations] [--cache-ttl <duration>] [--bitbucket-use-default-reviewers] [--bitbucket-server-use-default-reviewers] [--gitlab-merge-when-pipeline-succeeds] [--gitlab-required-reviewers <integer>] [--azure-repos-organization <string>] [--github-app-id <integer> --github-app-key-file <file>] [--url-checker-test-url <uri>]... [--default-maven-repo <string>] [--refresh-backoff-period <duration>]
+    scala-steward --workspace <file> --repos-file <file> [--git-author-name <string>] --git-author-email <string> [--git-author-signing-key <string>] --git-ask-pass <file> [--sign-commits] [--forge-type <forge-type>] [--forge-api-host <uri>] --forge-login <string> [--do-not-fork] [--add-labels] [--ignore-opts-files] [--env-var <name=value>]... [--process-timeout <duration>] [--whitelist <string>]... [--read-only <string>]... [--enable-sandbox | --disable-sandbox] [--max-buffer-size <integer>] [--repo-config <uri>]... [--disable-default-repo-config] [--scalafix-migrations <uri>]... [--disable-default-scalafix-migrations] [--artifact-migrations <uri>]... [--disable-default-artifact-migrations] [--cache-ttl <duration>] [--bitbucket-use-default-reviewers] [--bitbucket-server-use-default-reviewers] [--gitlab-merge-when-pipeline-succeeds] [--gitlab-required-reviewers <integer>] [--gitlab-remove-source-branch] [--azure-repos-organization <string>] [--github-app-id <integer> --github-app-key-file <file>] [--url-checker-test-url <uri>]... [--default-maven-repo <string>] [--refresh-backoff-period <duration>]
 
 
 
@@ -80,6 +80,8 @@ Options and flags:
         Whether to merge a gitlab merge request when the pipeline succeeds
     --gitlab-required-reviewers <integer>
         When set, the number of required reviewers for a merge request will be set to this number (non-negative integer).  Is only used in the context of gitlab-merge-when-pipeline-succeeds being enabled, and requires that the configured access token have the appropriate privileges.  Also requires a Gitlab Premium subscription.
+    --gitlab-remove-source-branch
+        Flag indicating if a merge request should remove the source branch when merging.
     --azure-repos-organization <string>
         The Azure organization (required when --forge-type is azure-repos)
     --github-app-id <integer>
diff --git a/modules/core/src/main/scala/org/scalasteward/core/application/Cli.scala b/modules/core/src/main/scala/org/scalasteward/core/application/Cli.scala
index 074b66bfcc..3bbccaa37d 100644
--- a/modules/core/src/main/scala/org/scalasteward/core/application/Cli.scala
+++ b/modules/core/src/main/scala/org/scalasteward/core/application/Cli.scala
@@ -280,8 +280,16 @@ object Cli {
       "When set, the number of required reviewers for a merge request will be set to this number (non-negative integer).  Is only used in the context of gitlab-merge-when-pipeline-succeeds being enabled, and requires that the configured access token have the appropriate privileges.  Also requires a Gitlab Premium subscription."
     ).validate("Required reviewers must be non-negative")(_ >= 0).orNone
 
+  private val gitlabRemoveSourceBranch: Opts[Boolean] =
+    flag(
+      "gitlab-remove-source-branch",
+      "Flag indicating if a merge request should remove the source branch when merging."
+    ).orFalse
+
   private val gitLabCfg: Opts[GitLabCfg] =
-    (gitlabMergeWhenPipelineSucceeds, gitlabRequiredReviewers).mapN(GitLabCfg.apply)
+    (gitlabMergeWhenPipelineSucceeds, gitlabRequiredReviewers, gitlabRemoveSourceBranch).mapN(
+      GitLabCfg.apply
+    )
 
   private val githubAppId: Opts[Long] =
     option[Long](
diff --git a/modules/core/src/main/scala/org/scalasteward/core/application/Config.scala b/modules/core/src/main/scala/org/scalasteward/core/application/Config.scala
index 2ced3f8a64..2ad3116a02 100644
--- a/modules/core/src/main/scala/org/scalasteward/core/application/Config.scala
+++ b/modules/core/src/main/scala/org/scalasteward/core/application/Config.scala
@@ -158,7 +158,8 @@ object Config {
 
   final case class GitLabCfg(
       mergeWhenPipelineSucceeds: Boolean,
-      requiredReviewers: Option[Int]
+      requiredReviewers: Option[Int],
+      removeSourceBranch: Boolean
   ) extends ForgeSpecificCfg
 
   final case class GiteaCfg(
diff --git a/modules/core/src/main/scala/org/scalasteward/core/forge/gitlab/GitLabApiAlg.scala b/modules/core/src/main/scala/org/scalasteward/core/forge/gitlab/GitLabApiAlg.scala
index 9f333d9b39..e062f14005 100644
--- a/modules/core/src/main/scala/org/scalasteward/core/forge/gitlab/GitLabApiAlg.scala
+++ b/modules/core/src/main/scala/org/scalasteward/core/forge/gitlab/GitLabApiAlg.scala
@@ -40,6 +40,7 @@ final private[gitlab] case class MergeRequestPayload(
     assignee_ids: Option[List[Int]],
     reviewer_ids: Option[List[Int]],
     target_project_id: Long,
+    remove_source_branch: Option[Boolean],
     source_branch: String,
     target_branch: Branch
 )
@@ -49,7 +50,8 @@ private[gitlab] object MergeRequestPayload {
       id: String,
       projectId: Long,
       data: NewPullRequestData,
-      usernamesToUserIdsMapping: Map[String, Int]
+      usernamesToUserIdsMapping: Map[String, Int],
+      removeSourceBranch: Boolean
   ): MergeRequestPayload = {
     val assignees = data.assignees.flatMap(usernamesToUserIdsMapping.get)
     val reviewers = data.reviewers.flatMap(usernamesToUserIdsMapping.get)
@@ -61,6 +63,7 @@ private[gitlab] object MergeRequestPayload {
       reviewer_ids = Option.when(reviewers.nonEmpty)(reviewers),
       labels = Option.when(data.labels.nonEmpty)(data.labels),
       target_project_id = projectId,
+      remove_source_branch = Option.when(removeSourceBranch)(removeSourceBranch),
       source_branch = data.head,
       target_branch = data.base
     )
@@ -135,7 +138,9 @@ private[gitlab] object GitLabJsonCodec {
     }
 
   implicit val projectIdDecoder: Decoder[ProjectId] = deriveDecoder
-  implicit val mergeRequestPayloadEncoder: Encoder[MergeRequestPayload] = deriveEncoder
+  implicit val mergeRequestPayloadEncoder: Encoder[MergeRequestPayload] =
+    deriveEncoder[MergeRequestPayload].mapJson(_.dropNullValues)
+
   implicit val updateStateEncoder: Encoder[UpdateState] = Encoder.instance { newState =>
     val encoded = newState.state match {
       case PullRequestState.Open   => "open"
@@ -188,7 +193,8 @@ final class GitLabApiAlg[F[_]: Parallel](
         id = url.encodedProjectId(targetRepo),
         projectId = projectId.id,
         data = data,
-        usernamesToUserIdsMapping = usernameMapping
+        usernamesToUserIdsMapping = usernameMapping,
+        removeSourceBranch = gitLabCfg.removeSourceBranch
       )
       res <- client.postWithBody[MergeRequestOut, MergeRequestPayload](
         uri = url.mergeRequest(targetRepo),


Test patch:
diff --git a/modules/core/src/test/scala/org/scalasteward/core/forge/gitlab/GitLabApiAlgTest.scala b/modules/core/src/test/scala/org/scalasteward/core/forge/gitlab/GitLabApiAlgTest.scala
index 2a1a5aaf74..b6a224456a 100644
--- a/modules/core/src/test/scala/org/scalasteward/core/forge/gitlab/GitLabApiAlgTest.scala
+++ b/modules/core/src/test/scala/org/scalasteward/core/forge/gitlab/GitLabApiAlgTest.scala
@@ -122,25 +122,61 @@ class GitLabApiAlgTest extends CatsEffectSuite with Http4sDsl[MockEff] {
 
   private val gitlabApiAlg = ForgeSelection.forgeApiAlg[MockEff](
     config.forgeCfg.copy(tpe = ForgeType.GitLab),
-    GitLabCfg(mergeWhenPipelineSucceeds = false, requiredReviewers = None),
+    GitLabCfg(
+      mergeWhenPipelineSucceeds = false,
+      requiredReviewers = None,
+      removeSourceBranch = false
+    ),
     user
   )
 
   private val gitlabApiAlgNoFork = ForgeSelection.forgeApiAlg[MockEff](
     config.forgeCfg.copy(tpe = ForgeType.GitLab, doNotFork = true),
-    GitLabCfg(mergeWhenPipelineSucceeds = false, requiredReviewers = None),
+    GitLabCfg(
+      mergeWhenPipelineSucceeds = false,
+      requiredReviewers = None,
+      removeSourceBranch = false
+    ),
+    user
+  )
+
+  private val gitlabApiAlgAutoMerge = ForgeSelection.forgeApiAlg[MockEff](
+    config.forgeCfg.copy(tpe = ForgeType.GitLab, doNotFork = true),
+    GitLabCfg(
+      mergeWhenPipelineSucceeds = true,
+      requiredReviewers = None,
+      removeSourceBranch = false
+    ),
+    user
+  )
+
+  private val gitlabApiAlgRemoveSourceBranch = ForgeSelection.forgeApiAlg[MockEff](
+    config.forgeCfg.copy(tpe = ForgeType.GitLab, doNotFork = true),
+    GitLabCfg(
+      mergeWhenPipelineSucceeds = false,
+      requiredReviewers = None,
+      removeSourceBranch = true
+    ),
     user
   )
 
   private val gitlabApiAlgLessReviewersRequired = ForgeSelection.forgeApiAlg[MockEff](
     config.forgeCfg.copy(tpe = ForgeType.GitLab, doNotFork = true),
-    GitLabCfg(mergeWhenPipelineSucceeds = true, requiredReviewers = Some(0)),
+    GitLabCfg(
+      mergeWhenPipelineSucceeds = true,
+      requiredReviewers = Some(0),
+      removeSourceBranch = false
+    ),
     user
   )
 
   private val gitlabApiAlgWithAssigneeAndReviewers = ForgeSelection.forgeApiAlg[MockEff](
     config.forgeCfg.copy(tpe = ForgeType.GitLab, doNotFork = true),
-    GitLabCfg(mergeWhenPipelineSucceeds = true, requiredReviewers = Some(0)),
+    GitLabCfg(
+      mergeWhenPipelineSucceeds = true,
+      requiredReviewers = Some(0),
+      removeSourceBranch = false
+    ),
     user
   )
 
@@ -232,7 +268,7 @@ class GitLabApiAlgTest extends CatsEffectSuite with Http4sDsl[MockEff] {
   }
 
   test("createPullRequest -- auto merge") {
-    val prOut = gitlabApiAlgNoFork
+    val prOut = gitlabApiAlgAutoMerge
       .createPullRequest(Repo("foo", "bar"), newPRData)
       .runA(state)
 
@@ -345,6 +381,21 @@ class GitLabApiAlgTest extends CatsEffectSuite with Http4sDsl[MockEff] {
     assertIO(prOut, expected)
   }
 
+  test("createPullRequest -- remove source branch") {
+    val prOut = gitlabApiAlgRemoveSourceBranch
+      .createPullRequest(Repo("foo", "bar"), newPRData)
+      .runA(state)
+
+    val expected = PullRequestOut(
+      uri"https://gitlab.com/foo/bar/merge_requests/150",
+      PullRequestState.Open,
+      PullRequestNumber(150),
+      "title"
+    )
+
+    assertIO(prOut, expected)
+  }
+
   test("referencePullRequest") {
     val reference = gitlabApiAlg.referencePullRequest(PullRequestNumber(1347))
     assertEquals(reference, "!1347")
diff --git a/modules/core/src/test/scala/org/scalasteward/core/forge/gitlab/MergeRequestPayloadTest.scala b/modules/core/src/test/scala/org/scalasteward/core/forge/gitlab/MergeRequestPayloadTest.scala
index 67b9dc1bbc..4ac82ad3d6 100644
--- a/modules/core/src/test/scala/org/scalasteward/core/forge/gitlab/MergeRequestPayloadTest.scala
+++ b/modules/core/src/test/scala/org/scalasteward/core/forge/gitlab/MergeRequestPayloadTest.scala
@@ -22,15 +22,13 @@ class MergeRequestPayloadTest extends FunSuite {
   private val projectId = 321L
 
   test("asJson") {
-    val obtained = MergeRequestPayload(id, projectId, data, Map.empty).asJson
+    val obtained =
+      MergeRequestPayload(id, projectId, data, Map.empty, removeSourceBranch = false).asJson
     val expected =
       json"""{
                "id" : "123",
                "title" : "Test MR title",
                "description" : "Test MR body",
-               "labels" : null,
-               "assignee_ids" : null,
-               "reviewer_ids" : null,
                "target_project_id" : 321,
                "source_branch" : "source",
                "target_branch" : "master"
@@ -39,15 +37,18 @@ class MergeRequestPayloadTest extends FunSuite {
   }
 
   test("asJson for draft MR") {
-    val obtained = MergeRequestPayload(id, projectId, data.copy(draft = true), Map.empty).asJson
+    val obtained = MergeRequestPayload(
+      id,
+      projectId,
+      data.copy(draft = true),
+      Map.empty,
+      removeSourceBranch = false
+    ).asJson
     val expected =
       json"""{
                "id" : "123",
                "title" : "Draft: Test MR title",
                "description" : "Test MR body",
-               "labels" : null,
-               "assignee_ids" : null,
-               "reviewer_ids" : null,
                "target_project_id" : 321,
                "source_branch" : "source",
                "target_branch" : "master"
@@ -57,15 +58,19 @@ class MergeRequestPayloadTest extends FunSuite {
 
   test("asJson with labels") {
     val obtained =
-      MergeRequestPayload(id, projectId, data.copy(labels = List("foo", "bar")), Map.empty).asJson
+      MergeRequestPayload(
+        id,
+        projectId,
+        data.copy(labels = List("foo", "bar")),
+        Map.empty,
+        removeSourceBranch = false
+      ).asJson
     val expected =
       json"""{
                "id" : "123",
                "title" : "Test MR title",
                "description" : "Test MR body",
                "labels" : [ "foo", "bar" ],
-               "assignee_ids" : null,
-               "reviewer_ids" : null,
                "target_project_id" : 321,
                "source_branch" : "source",
                "target_branch" : "master"
@@ -78,14 +83,14 @@ class MergeRequestPayloadTest extends FunSuite {
       id = id,
       projectId = projectId,
       data = data.copy(assignees = List("foo"), reviewers = List("bar")),
-      usernamesToUserIdsMapping = Map("foo" -> 1, "bar" -> 2)
+      usernamesToUserIdsMapping = Map("foo" -> 1, "bar" -> 2),
+      removeSourceBranch = false
     ).asJson
     val expected =
       json"""{
                "id" : "123",
                "title" : "Test MR title",
                "description" : "Test MR body",
-               "labels" : null,
                "assignee_ids": [ 1 ],
                "reviewer_ids": [ 2 ],
                "target_project_id" : 321,
@@ -94,4 +99,25 @@ class MergeRequestPayloadTest extends FunSuite {
              }"""
     assertEquals(obtained, expected)
   }
+
+  test("asJson with remove source branch") {
+    val obtained = MergeRequestPayload(
+      id = id,
+      projectId = projectId,
+      data = data,
+      usernamesToUserIdsMapping = Map.empty,
+      removeSourceBranch = true
+    ).asJson
+    val expected =
+      json"""{
+               "id" : "123",
+               "title" : "Test MR title",
+               "description" : "Test MR body",
+               "target_project_id" : 321,
+               "remove_source_branch": true,
+               "source_branch" : "source",
+               "target_branch" : "master"
+             }"""
+    assertEquals(obtained, expected)
+  }
 }