Merge pull request #1883 from gitbucket/disable-merge-strategies

Add a option to enable/disable merge strategies
2025-11-09 15:05:50 +01:00 · 2018-02-11 13:51:45 +09:00
parent c8a7ef2fdb 737b0a9bdf
commit 88427b893c
8 changed files with 181 additions and 90 deletions
--- a/src/main/resources/update/gitbucket-core_4.22.xml
+++ b/src/main/resources/update/gitbucket-core_4.22.xml
@@ -0,0 +1,6 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<changeSet>
+  <addColumn tableName="REPOSITORY">
+    <column name="MERGE_OPTIONS" type="varchar(200)" nullable="false" defaultValue="merge-commit,squash,rebase"/>
+  </addColumn>
+</changeSet>
--- a/src/main/scala/gitbucket/core/GitBucketCoreModule.scala
+++ b/src/main/scala/gitbucket/core/GitBucketCoreModule.scala
@@ -53,5 +53,8 @@ object GitBucketCoreModule extends Module("gitbucket-core",
    new LiquibaseMigration("update/gitbucket-core_4.21.xml")
  ),
  new Version("4.21.1"),
-  new Version("4.21.2")
+  new Version("4.21.2"),
+  new Version("4.22.0",
+    new LiquibaseMigration("update/gitbucket-core_4.22.xml")
+  )
 )
--- a/src/main/scala/gitbucket/core/controller/PullRequestsController.scala
+++ b/src/main/scala/gitbucket/core/controller/PullRequestsController.scala
@@ -17,6 +17,7 @@ import org.scalatra.forms._
 import org.eclipse.jgit.api.Git
 import org.eclipse.jgit.lib.PersonIdent
 import org.eclipse.jgit.revwalk.RevWalk
+import org.scalatra.BadRequest

 import scala.collection.JavaConverters._

@@ -248,67 +249,69 @@ trait PullRequestsControllerBase extends ControllerBase {
    params("id").toIntOpt.flatMap { issueId =>
      val owner = repository.owner
      val name  = repository.name
-      LockUtil.lock(s"${owner}/${name}"){
-        getPullRequest(owner, name, issueId).map { case (issue, pullreq) =>
-          using(Git.open(getRepositoryDir(owner, name))) { git =>
-            // mark issue as merged and close.
-            val loginAccount = context.loginAccount.get
-            val commentId = createComment(owner, name, loginAccount.userName, issueId, form.message, "merge")
-            createComment(owner, name, loginAccount.userName, issueId, "Close", "close")
-            updateClosed(owner, name, issueId, true)
+      if(repository.repository.options.mergeOptions.split(",").contains(form.strategy)){
+        LockUtil.lock(s"${owner}/${name}"){
+          getPullRequest(owner, name, issueId).map { case (issue, pullreq) =>
+            using(Git.open(getRepositoryDir(owner, name))) { git =>
+              // mark issue as merged and close.
+              val loginAccount = context.loginAccount.get
+              val commentId = createComment(owner, name, loginAccount.userName, issueId, form.message, "merge")
+              createComment(owner, name, loginAccount.userName, issueId, "Close", "close")
+              updateClosed(owner, name, issueId, true)

-            // record activity
-            recordMergeActivity(owner, name, loginAccount.userName, issueId, form.message)
+              // record activity
+              recordMergeActivity(owner, name, loginAccount.userName, issueId, form.message)

-            val (commits, _) = getRequestCompareInfo(owner, name, pullreq.commitIdFrom,
-              pullreq.requestUserName, pullreq.requestRepositoryName, pullreq.commitIdTo)
+              val (commits, _) = getRequestCompareInfo(owner, name, pullreq.commitIdFrom,
+                pullreq.requestUserName, pullreq.requestRepositoryName, pullreq.commitIdTo)

-            val revCommits = using(new RevWalk( git.getRepository )){ revWalk =>
-              commits.flatten.map { commit =>
-                revWalk.parseCommit(git.getRepository.resolve(commit.id))
+              val revCommits = using(new RevWalk( git.getRepository )){ revWalk =>
+                commits.flatten.map { commit =>
+                  revWalk.parseCommit(git.getRepository.resolve(commit.id))
+                }
+              }.reverse
+
+              // merge git repository
+              form.strategy match {
+                case "merge-commit" =>
+                  mergePullRequest(git, pullreq.branch, issueId,
+                    s"Merge pull request #${issueId} from ${pullreq.requestUserName}/${pullreq.requestBranch}\n\n" + form.message,
+                    new PersonIdent(loginAccount.fullName, loginAccount.mailAddress))
+                case "rebase" =>
+                  rebasePullRequest(git, pullreq.branch, issueId, revCommits,
+                    new PersonIdent(loginAccount.fullName, loginAccount.mailAddress))
+                case "squash" =>
+                  squashPullRequest(git, pullreq.branch, issueId,
+                    s"${issue.title} (#${issueId})\n\n" + form.message,
+                    new PersonIdent(loginAccount.fullName, loginAccount.mailAddress))
              }
-            }.reverse

-            // merge git repository
-            form.strategy match {
-              case "merge-commit" =>
-                mergePullRequest(git, pullreq.branch, issueId,
-                  s"Merge pull request #${issueId} from ${pullreq.requestUserName}/${pullreq.requestBranch}\n\n" + form.message,
-                  new PersonIdent(loginAccount.fullName, loginAccount.mailAddress))
-              case "rebase" =>
-                rebasePullRequest(git, pullreq.branch, issueId, revCommits,
-                  new PersonIdent(loginAccount.fullName, loginAccount.mailAddress))
-              case "squash" =>
-                squashPullRequest(git, pullreq.branch, issueId,
-                  s"${issue.title} (#${issueId})\n\n" + form.message,
-                  new PersonIdent(loginAccount.fullName, loginAccount.mailAddress))
-            }
-
-            // close issue by content of pull request
-            val defaultBranch = getRepository(owner, name).get.repository.defaultBranch
-            if(pullreq.branch == defaultBranch){
-              commits.flatten.foreach { commit =>
-                closeIssuesFromMessage(commit.fullMessage, loginAccount.userName, owner, name)
+              // close issue by content of pull request
+              val defaultBranch = getRepository(owner, name).get.repository.defaultBranch
+              if(pullreq.branch == defaultBranch){
+                commits.flatten.foreach { commit =>
+                  closeIssuesFromMessage(commit.fullMessage, loginAccount.userName, owner, name)
+                }
+                closeIssuesFromMessage(issue.title + " " + issue.content.getOrElse(""), loginAccount.userName, owner, name)
+                closeIssuesFromMessage(form.message, loginAccount.userName, owner, name)
              }
-              closeIssuesFromMessage(issue.title + " " + issue.content.getOrElse(""), loginAccount.userName, owner, name)
-              closeIssuesFromMessage(form.message, loginAccount.userName, owner, name)
+
+              updatePullRequests(owner, name, pullreq.branch)
+
+              // call web hook
+              callPullRequestWebHook("closed", repository, issueId, context.baseUrl, context.loginAccount.get)
+
+              // call hooks
+              PluginRegistry().getPullRequestHooks.foreach{ h =>
+                h.addedComment(commentId, form.message, issue, repository)
+                h.merged(issue, repository)
+              }
+
+              redirect(s"/${owner}/${name}/pull/${issueId}")
            }
-
-            updatePullRequests(owner, name, pullreq.branch)
-
-            // call web hook
-            callPullRequestWebHook("closed", repository, issueId, context.baseUrl, context.loginAccount.get)
-
-            // call hooks
-            PluginRegistry().getPullRequestHooks.foreach{ h =>
-              h.addedComment(commentId, form.message, issue, repository)
-              h.merged(issue, repository)
-            }
-
-            redirect(s"/${owner}/${name}/pull/${issueId}")
          }
        }
-      }
+      } else Some(BadRequest())
    } getOrElse NotFound()
  })

--- a/src/main/scala/gitbucket/core/controller/RepositorySettingsController.scala
+++ b/src/main/scala/gitbucket/core/controller/RepositorySettingsController.scala
@@ -39,20 +39,29 @@ trait RepositorySettingsControllerBase extends ControllerBase {
    externalIssuesUrl: Option[String],
    wikiOption: String,
    externalWikiUrl: Option[String],
-    allowFork: Boolean
+    allowFork: Boolean,
+    mergeOptions: Seq[String]
  )

  val optionsForm = mapping(
-    "repositoryName"    -> trim(label("Repository Name"    , text(required, maxlength(100), repository, renameRepositoryName))),
-    "description"       -> trim(label("Description"        , optional(text()))),
-    "isPrivate"         -> trim(label("Repository Type"    , boolean())),
-    "issuesOption"      -> trim(label("Issues Option"      , text(required, featureOption))),
-    "externalIssuesUrl" -> trim(label("External Issues URL", optional(text(maxlength(200))))),
-    "wikiOption"        -> trim(label("Wiki Option"        , text(required, featureOption))),
-    "externalWikiUrl"   -> trim(label("External Wiki URL"  , optional(text(maxlength(200))))),
-    "allowFork"         -> trim(label("Allow Forking"      , boolean()))
+    "repositoryName"     -> trim(label("Repository Name"    , text(required, maxlength(100), repository, renameRepositoryName))),
+    "description"        -> trim(label("Description"        , optional(text()))),
+    "isPrivate"          -> trim(label("Repository Type"    , boolean())),
+    "issuesOption"       -> trim(label("Issues Option"      , text(required, featureOption))),
+    "externalIssuesUrl"  -> trim(label("External Issues URL", optional(text(maxlength(200))))),
+    "wikiOption"         -> trim(label("Wiki Option"        , text(required, featureOption))),
+    "externalWikiUrl"    -> trim(label("External Wiki URL"  , optional(text(maxlength(200))))),
+    "allowFork"          -> trim(label("Allow Forking"      , boolean())),
+    "mergeOptions"       -> new ValueType[Seq[String]]{
+      override def convert(name: String, params: Map[String, Seq[String]], messages: Messages): Seq[String] =
+        params.get("mergeOptions").getOrElse(Nil)
+      override def validate(name: String, params: Map[String, Seq[String]], messages: Messages): Seq[(String, String)] =
+        if(params.get("mergeOptions").getOrElse(Nil).isEmpty) Seq("mergeOptions" -> "At least one option must be enabled.") else Nil
+    },
  )(OptionsForm.apply)

+
+
  // for default branch
  case class DefaultBranchForm(defaultBranch: String)

@@ -118,7 +127,8 @@ trait RepositorySettingsControllerBase extends ControllerBase {
      form.externalIssuesUrl,
      form.wikiOption,
      form.externalWikiUrl,
-      form.allowFork
+      form.allowFork,
+      form.mergeOptions
    )
    // Change repository name
    if(repository.name != form.repositoryName){
--- a/src/main/scala/gitbucket/core/model/Repository.scala
+++ b/src/main/scala/gitbucket/core/model/Repository.scala
@@ -22,11 +22,12 @@ trait RepositoryComponent extends TemplateComponent { self: Profile =>
    val wikiOption           = column[String]("WIKI_OPTION")
    val externalWikiUrl      = column[String]("EXTERNAL_WIKI_URL")
    val allowFork            = column[Boolean]("ALLOW_FORK")
+    val mergeOptions         = column[String]("MERGE_OPTIONS")

    def * = (
      (userName, repositoryName, isPrivate, description.?, defaultBranch,
      registeredDate, updatedDate, lastActivityDate, originUserName.?, originRepositoryName.?, parentUserName.?, parentRepositoryName.?),
-      (issuesOption, externalIssuesUrl.?, wikiOption, externalWikiUrl.?, allowFork)
+      (issuesOption, externalIssuesUrl.?, wikiOption, externalWikiUrl.?, allowFork, mergeOptions)
    ).shaped <> (
      { case (repository, options) =>
        Repository(
@@ -88,5 +89,6 @@ case class RepositoryOptions(
  externalIssuesUrl: Option[String],
  wikiOption: String,
  externalWikiUrl: Option[String],
-  allowFork: Boolean
+  allowFork: Boolean,
+  mergeOptions: String
 )
--- a/src/main/scala/gitbucket/core/service/RepositoryService.scala
+++ b/src/main/scala/gitbucket/core/service/RepositoryService.scala
@@ -46,7 +46,8 @@ trait RepositoryService { self: AccountService =>
          externalIssuesUrl    = None,
          wikiOption           = "PUBLIC", // TODO DISABLE for the forked repository?
          externalWikiUrl      = None,
-          allowFork            = true
+          allowFork            = true,
+          mergeOptions         = "merge-commit,squash,rebase"
        )
      )

@@ -362,14 +363,34 @@ trait RepositoryService { self: AccountService =>
  /**
   * Save repository options.
   */
-  def saveRepositoryOptions(userName: String, repositoryName: String,
-      description: Option[String], isPrivate: Boolean,
-      issuesOption: String, externalIssuesUrl: Option[String],
-      wikiOption: String, externalWikiUrl: Option[String],
-      allowFork: Boolean)(implicit s: Session): Unit =
+  def saveRepositoryOptions(userName: String, repositoryName: String, description: Option[String], isPrivate: Boolean,
+      issuesOption: String, externalIssuesUrl: Option[String], wikiOption: String, externalWikiUrl: Option[String],
+      allowFork: Boolean, mergeOptions: Seq[String])(implicit s: Session): Unit = {
+
    Repositories.filter(_.byRepository(userName, repositoryName))
-      .map { r => (r.description.?, r.isPrivate, r.issuesOption, r.externalIssuesUrl.?, r.wikiOption, r.externalWikiUrl.?, r.allowFork, r.updatedDate) }
-      .update (description, isPrivate, issuesOption, externalIssuesUrl, wikiOption, externalWikiUrl, allowFork, currentDate)
+      .map { r => (
+        r.description.?,
+        r.isPrivate,
+        r.issuesOption,
+        r.externalIssuesUrl.?,
+        r.wikiOption,
+        r.externalWikiUrl.?,
+        r.allowFork,
+        r.mergeOptions,
+        r.updatedDate
+      ) }
+      .update (
+        description,
+        isPrivate,
+        issuesOption,
+        externalIssuesUrl,
+        wikiOption,
+        externalWikiUrl,
+        allowFork,
+        mergeOptions.mkString(","),
+        currentDate
+      )
+  }

  def saveRepositoryDefaultBranch(userName: String, repositoryName: String,
      defaultBranch: String)(implicit s: Session): Unit =
--- a/src/main/twirl/gitbucket/core/pulls/mergeguide.scala.html
+++ b/src/main/twirl/gitbucket/core/pulls/mergeguide.scala.html
@@ -143,34 +143,48 @@
        <span id="error-message" class="error"></span>
        <textarea name="message" style="height: 80px; margin-top: 8px; margin-bottom: 8px;" class="form-control">@issue.title</textarea>
        <div>
+        @defining(originRepository.repository.options.mergeOptions.split(",")){ mergeOptions =>
          <div class="btn-group">
            <button id="merge-strategy-btn" class="dropdown-toggle btn btn-default" data-toggle="dropdown">
-              <span class="strong">Merge commit</span>
+              <span class="strong">
+                @(if(mergeOptions.contains("merge-commit")) "Merge commit"
+                  else if(mergeOptions.contains("squash")) "Squash"
+                  else if(mergeOptions.contains("rebase")) "Rebase")
+              </span>
              <span class="caret"></span>
            </button>
            <ul class="dropdown-menu">
-              <li>
-                <a href="javascript:void(0);" class="merge-strategy" data-value="merge-commit">
-                  <strong>Merge commit</strong><br>These commits will be added to the base branch via a merge commit.
-                </a>
-              </li>
-              <li>
-                <a href="javascript:void(0);" class="merge-strategy" data-value="squash">
-                  <strong>Squash</strong><br>These commits will be combined into one commit in the base branch.
-                </a>
-              </li>
-              <li>
-                <a href="javascript:void(0);" class="merge-strategy" data-value="rebase">
-                  <strong>Rebase</strong><br>These commits will be rebased and added to the base branch.
-                </a>
-              </li>
+              @if(mergeOptions.contains("merge-commit")){
+                <li>
+                  <a href="javascript:void(0);" class="merge-strategy" data-value="merge-commit">
+                    <strong>Merge commit</strong><br>These commits will be added to the base branch via a merge commit.
+                  </a>
+                </li>
+              }
+              @if(mergeOptions.contains("squash")){
+                <li>
+                  <a href="javascript:void(0);" class="merge-strategy" data-value="squash">
+                    <strong>Squash</strong><br>These commits will be combined into one commit in the base branch.
+                  </a>
+                </li>
+              }
+              @if(mergeOptions.contains("rebase")){
+                <li>
+                  <a href="javascript:void(0);" class="merge-strategy" data-value="rebase">
+                    <strong>Rebase</strong><br>These commits will be rebased and added to the base branch.
+                  </a>
+                </li>
+              }
            </ul>
          </div>
          <div class="pull-right">
            <input type="button" class="btn btn-default" value="Cancel" id="cancel-merge-pull-request"/>
            <input type="submit" class="btn btn-success" value="Confirm merge"/>
-            <input type="hidden" name="strategy" value="merge-commit"/>
+            <input type="hidden" name="strategy" value="@(if(mergeOptions.contains("merge-commit")) "merge-commit"
+                                                          else if(mergeOptions.contains("squash")) "squash"
+                                                          else if(mergeOptions.contains("rebase")) "rebase")"/>
          </div>
+        }
        </div>
      </form>
    </div>
--- a/src/main/twirl/gitbucket/core/settings/options.scala.html
+++ b/src/main/twirl/gitbucket/core/settings/options.scala.html
@@ -112,6 +112,38 @@
            </fieldset>
          </div>
        </div>
+        <div class="panel panel-default">
+          <div class="panel-heading strong">Merge strategy</div>
+          <div class="panel-body">
+            Select pull request merge strategies which are available in this repository. At least one option must be enabled.
+            <fieldset class="form-group">
+            @defining(repository.repository.options.mergeOptions.split(",")){ mergeOptions =>
+              <div class="checkbox">
+                <label for="mergeOptions_MergeCommit">
+                  <input type="checkbox" name="mergeOptions" id="mergeOptions_MergeCommit" value="merge-commit" @if(mergeOptions.contains("merge-commit")){checked}>
+                <span class="strong">Merge commit</span>
+                <div class="normal muted">Commits will be added to the base branch via a merge commit.</div>
+                </label>
+              </div>
+              <div class="checkbox">
+                <label for="mergeOptions_Squash">
+                  <input type="checkbox" name="mergeOptions" id="mergeOptions_Squash" value="squash" @if(mergeOptions.contains("squash")){checked}>
+                  <span class="strong">Squash</span>
+                  <div class="normal muted">Commits will be combined into one commit in the base branch.</div>
+                </label>
+              </div>
+              <div class="checkbox">
+                <label for="mergeOptions_Rebase">
+                  <input type="checkbox" name="mergeOptions" id="mergeOptions_Rebase" value="rebase" @if(mergeOptions.contains("rebase")){checked}>
+                  <span class="strong">Rebase</span>
+                  <div class="normal muted">Commits will be rebased and added to the base branch.</div>
+                </label>
+              </div>
+              <span id="error-mergeOptions" class="error"></span>
+            }
+            </fieldset>
+          </div>
+        </div>
        <div class="align-right" style="margin-top: 20px;">
          <input type="submit" class="btn btn-success" value="Apply changes"/>
        </div>