Add option to update pull request by rebase (#16125)

* add option to update pull request by `rebase`

Signed-off-by: a1012112796 <1012112796@qq.com>

integrations/pull_update_test.go

@@ -47,6 +47,34 @@ func TestAPIPullUpdate(t *testing.T) {
 	})
 }
 
+func TestAPIPullUpdateByRebase(t *testing.T) {
+	onGiteaRun(t, func(t *testing.T, giteaURL *url.URL) {
+		//Create PR to test
+		user := models.AssertExistsAndLoadBean(t, &models.User{ID: 2}).(*models.User)
+		org26 := models.AssertExistsAndLoadBean(t, &models.User{ID: 26}).(*models.User)
+		pr := createOutdatedPR(t, user, org26)
+
+		//Test GetDiverging
+		diffCount, err := pull_service.GetDiverging(pr)
+		assert.NoError(t, err)
+		assert.EqualValues(t, 1, diffCount.Behind)
+		assert.EqualValues(t, 1, diffCount.Ahead)
+		assert.NoError(t, pr.LoadBaseRepo())
+		assert.NoError(t, pr.LoadIssue())
+
+		session := loginUser(t, "user2")
+		token := getTokenForLoggedInUser(t, session)
+		req := NewRequestf(t, "POST", "/api/v1/repos/%s/%s/pulls/%d/update?style=rebase&token="+token, pr.BaseRepo.OwnerName, pr.BaseRepo.Name, pr.Issue.Index)
+		session.MakeRequest(t, req, http.StatusOK)
+
+		//Test GetDiverging after update
+		diffCount, err = pull_service.GetDiverging(pr)
+		assert.NoError(t, err)
+		assert.EqualValues(t, 0, diffCount.Behind)
+		assert.EqualValues(t, 1, diffCount.Ahead)
+	})
+}
+
 func createOutdatedPR(t *testing.T, actor, forkOrg *models.User) *models.PullRequest {
 	baseRepo, err := repo_service.CreateRepository(actor, actor, models.CreateRepoOptions{
 		Name:        "repo-pr-update",

models/pull.go

@@ -373,6 +373,8 @@ const (
 	MergeStyleSquash MergeStyle = "squash"
 	// MergeStyleManuallyMerged pr has been merged manually, just mark it as merged directly
 	MergeStyleManuallyMerged MergeStyle = "manually-merged"
+	// MergeStyleRebaseUpdate not a merge style, used to update pull head by rebase
+	MergeStyleRebaseUpdate MergeStyle = "rebase-update-only"
 )
 
 // SetMerged sets a pull request to merged and closes the corresponding issue

options/locale/locale_en-US.ini

@@ -1468,7 +1468,8 @@ pulls.status_checks_failure = Some checks failed
 pulls.status_checks_error = Some checks reported errors
 pulls.status_checks_requested = Required
 pulls.status_checks_details = Details
-pulls.update_branch = Update branch
+pulls.update_branch = Update branch by merge
+pulls.update_branch_rebase = Update branch by rebase
 pulls.update_branch_success = Branch update was successful
 pulls.update_not_allowed = You are not allowed to update branch
 pulls.outdated_with_base_branch = This branch is out-of-date with the base branch

routers/api/v1/repo/pull.go

@@ -1065,6 +1065,11 @@ func UpdatePullRequest(ctx *context.APIContext) {
 	//   type: integer
 	//   format: int64
 	//   required: true
+	// - name: style
+	//   in: query
+	//   description: how to update pull request
+	//   type: string
+	//   enum: [merge, rebase]
 	// responses:
 	//   "200":
 	//     "$ref": "#/responses/empty"
@@ -1111,13 +1116,15 @@ func UpdatePullRequest(ctx *context.APIContext) {
 		return
 	}
 
-	allowedUpdate, err := pull_service.IsUserAllowedToUpdate(pr, ctx.User)
+	rebase := ctx.FormString("style") == "rebase"
+
+	allowedUpdateByMerge, allowedUpdateByRebase, err := pull_service.IsUserAllowedToUpdate(pr, ctx.User)
 	if err != nil {
 		ctx.Error(http.StatusInternalServerError, "IsUserAllowedToMerge", err)
 		return
 	}
 
-	if !allowedUpdate {
+	if (!allowedUpdateByMerge && !rebase) || (rebase && !allowedUpdateByRebase) {
 		ctx.Status(http.StatusForbidden)
 		return
 	}
@@ -1125,7 +1132,7 @@ func UpdatePullRequest(ctx *context.APIContext) {
 	// default merge commit message
 	message := fmt.Sprintf("Merge branch '%s' into %s", pr.BaseBranch, pr.HeadBranch)
 
-	if err = pull_service.Update(pr, ctx.User, message); err != nil {
+	if err = pull_service.Update(pr, ctx.User, message, rebase); err != nil {
 		if models.IsErrMergeConflicts(err) {
 			ctx.Error(http.StatusConflict, "Update", "merge failed because of conflict")
 			return

routers/web/repo/pull.go

@@ -450,7 +450,7 @@ func PrepareViewPullInfo(ctx *context.Context, issue *models.Issue) *git.Compare
 	}
 
 	if headBranchExist {
-		ctx.Data["UpdateAllowed"], err = pull_service.IsUserAllowedToUpdate(pull, ctx.User)
+		ctx.Data["UpdateAllowed"], ctx.Data["UpdateByRebaseAllowed"], err = pull_service.IsUserAllowedToUpdate(pull, ctx.User)
 		if err != nil {
 			ctx.ServerError("IsUserAllowedToUpdate", err)
 			return nil
@@ -724,6 +724,8 @@ func UpdatePullRequest(ctx *context.Context) {
 		return
 	}
 
+	rebase := ctx.FormString("style") == "rebase"
+
 	if err := issue.PullRequest.LoadBaseRepo(); err != nil {
 		ctx.ServerError("LoadBaseRepo", err)
 		return
@@ -733,14 +735,14 @@ func UpdatePullRequest(ctx *context.Context) {
 		return
 	}
 
-	allowedUpdate, err := pull_service.IsUserAllowedToUpdate(issue.PullRequest, ctx.User)
+	allowedUpdateByMerge, allowedUpdateByRebase, err := pull_service.IsUserAllowedToUpdate(issue.PullRequest, ctx.User)
 	if err != nil {
 		ctx.ServerError("IsUserAllowedToMerge", err)
 		return
 	}
 
 	// ToDo: add check if maintainers are allowed to change branch ... (need migration & co)
-	if !allowedUpdate {
+	if (!allowedUpdateByMerge && !rebase) || (rebase && !allowedUpdateByRebase) {
 		ctx.Flash.Error(ctx.Tr("repo.pulls.update_not_allowed"))
 		ctx.Redirect(ctx.Repo.RepoLink + "/pulls/" + fmt.Sprint(issue.Index))
 		return
@@ -749,7 +751,7 @@ func UpdatePullRequest(ctx *context.Context) {
 	// default merge commit message
 	message := fmt.Sprintf("Merge branch '%s' into %s", issue.PullRequest.BaseBranch, issue.PullRequest.HeadBranch)
 
-	if err = pull_service.Update(issue.PullRequest, ctx.User, message); err != nil {
+	if err = pull_service.Update(issue.PullRequest, ctx.User, message, rebase); err != nil {
 		if models.IsErrMergeConflicts(err) {
 			conflictError := err.(models.ErrMergeConflicts)
 			flashError, err := ctx.HTMLString(string(tplAlertDetails), map[string]interface{}{

services/pull/merge.go

@@ -253,6 +253,8 @@ func rawMerge(pr *models.PullRequest, doer *models.User, mergeStyle models.Merge
 		}
 	case models.MergeStyleRebase:
 		fallthrough
+	case models.MergeStyleRebaseUpdate:
+		fallthrough
 	case models.MergeStyleRebaseMerge:
 		// Checkout head branch
 		if err := git.NewCommand("checkout", "-b", stagingBranch, trackingBranch).RunInDirPipeline(tmpBasePath, &outbuf, &errbuf); err != nil {
@@ -305,6 +307,11 @@ func rawMerge(pr *models.PullRequest, doer *models.User, mergeStyle models.Merge
 		outbuf.Reset()
 		errbuf.Reset()
 
+		// not need merge, just update by rebase. so skip
+		if mergeStyle == models.MergeStyleRebaseUpdate {
+			break
+		}
+
 		// Checkout base branch again
 		if err := git.NewCommand("checkout", baseBranch).RunInDirPipeline(tmpBasePath, &outbuf, &errbuf); err != nil {
 			log.Error("git checkout base prior to merge post staging rebase [%s:%s -> %s:%s]: %v\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String())
@@ -410,8 +417,16 @@ func rawMerge(pr *models.PullRequest, doer *models.User, mergeStyle models.Merge
 		pr.ID,
 	)
 
+	var pushCmd *git.Command
+	if mergeStyle == models.MergeStyleRebaseUpdate {
+		// force push the rebase result to head brach
+		pushCmd = git.NewCommand("push", "-f", "head_repo", stagingBranch+":refs/heads/"+pr.HeadBranch)
+	} else {
+		pushCmd = git.NewCommand("push", "origin", baseBranch+":refs/heads/"+pr.BaseBranch)
+	}
+
 	// Push back to upstream.
-	if err := git.NewCommand("push", "origin", baseBranch+":refs/heads/"+pr.BaseBranch).RunInDirTimeoutEnvPipeline(env, -1, tmpBasePath, &outbuf, &errbuf); err != nil {
+	if err := pushCmd.RunInDirTimeoutEnvPipeline(env, -1, tmpBasePath, &outbuf, &errbuf); err != nil {
 		if strings.Contains(errbuf.String(), "non-fast-forward") {
 			return "", &git.ErrPushOutOfDate{
 				StdOut: outbuf.String(),

services/pull/update.go

@@ -13,13 +13,24 @@ import (
 )
 
 // Update updates pull request with base branch.
-func Update(pull *models.PullRequest, doer *models.User, message string) error {
-	//use merge functions but switch repo's and branch's
-	pr := &models.PullRequest{
-		HeadRepoID: pull.BaseRepoID,
-		BaseRepoID: pull.HeadRepoID,
-		HeadBranch: pull.BaseBranch,
-		BaseBranch: pull.HeadBranch,
+func Update(pull *models.PullRequest, doer *models.User, message string, rebase bool) error {
+	var (
+		pr    *models.PullRequest
+		style models.MergeStyle
+	)
+
+	if rebase {
+		pr = pull
+		style = models.MergeStyleRebaseUpdate
+	} else {
+		//use merge functions but switch repo's and branch's
+		pr = &models.PullRequest{
+			HeadRepoID: pull.BaseRepoID,
+			BaseRepoID: pull.HeadRepoID,
+			HeadBranch: pull.BaseBranch,
+			BaseBranch: pull.HeadBranch,
+		}
+		style = models.MergeStyleMerge
 	}
 
 	if pull.Flow == models.PullRequestFlowAGit {
@@ -42,9 +53,13 @@ func Update(pull *models.PullRequest, doer *models.User, message string) error {
 		return fmt.Errorf("HeadBranch of PR %d is up to date", pull.Index)
 	}
 
-	_, err = rawMerge(pr, doer, models.MergeStyleMerge, message)
+	_, err = rawMerge(pr, doer, style, message)
 
 	defer func() {
+		if rebase {
+			go AddTestPullRequestTask(doer, pr.BaseRepo.ID, pr.BaseBranch, false, "", "")
+			return
+		}
 		go AddTestPullRequestTask(doer, pr.HeadRepo.ID, pr.HeadBranch, false, "", "")
 	}()
 
@@ -52,17 +67,17 @@ func Update(pull *models.PullRequest, doer *models.User, message string) error {
 }
 
 // IsUserAllowedToUpdate check if user is allowed to update PR with given permissions and branch protections
-func IsUserAllowedToUpdate(pull *models.PullRequest, user *models.User) (bool, error) {
+func IsUserAllowedToUpdate(pull *models.PullRequest, user *models.User) (mergeAllowed, rebaseAllowed bool, err error) {
 	if pull.Flow == models.PullRequestFlowAGit {
-		return false, nil
+		return false, false, nil
 	}
 
 	if user == nil {
-		return false, nil
+		return false, false, nil
 	}
 	headRepoPerm, err := models.GetUserRepoPermission(pull.HeadRepo, user)
 	if err != nil {
-		return false, err
+		return false, false, err
 	}
 
 	pr := &models.PullRequest{
@@ -74,15 +89,25 @@ func IsUserAllowedToUpdate(pull *models.PullRequest, user *models.User) (bool, e
 
 	err = pr.LoadProtectedBranch()
 	if err != nil {
-		return false, err
+		return false, false, err
+	}
+
+	// can't do rebase on protected branch because need force push
+	if pr.ProtectedBranch == nil {
+		rebaseAllowed = true
 	}
 
 	// Update function need push permission
 	if pr.ProtectedBranch != nil && !pr.ProtectedBranch.CanUserPush(user.ID) {
-		return false, nil
+		return false, false, nil
 	}
 
-	return IsUserAllowedToMerge(pr, headRepoPerm, user)
+	mergeAllowed, err = IsUserAllowedToMerge(pr, headRepoPerm, user)
+	if err != nil {
+		return false, false, err
+	}
+
+	return mergeAllowed, rebaseAllowed, nil
 }
 
 // GetDiverging determines how many commits a PR is ahead or behind the PR base branch

templates/repo/issue/view_content/pull.tmpl

@@ -281,7 +281,26 @@
 							{{$.i18n.Tr "repo.pulls.outdated_with_base_branch"}}
 						</div>
 						<div class="item-section-right">
-							{{if .UpdateAllowed}}
+							{{if and .UpdateAllowed .UpdateByRebaseAllowed }}
+								<div class="dib">
+									<div class="ui buttons update-button">
+										<button class="ui button" data-do="{{.Link}}/update" data-redirect="{{.Link}}">
+											<span class="button-text">
+												{{$.i18n.Tr "repo.pulls.update_branch"}}
+											</span>
+										</button>
+
+										<div class="ui dropdown icon button no-text">
+											{{svg "octicon-triangle-down" 14 "dropdown icon"}}
+											<div class="menu">
+												<div class="item active selected" data-do="{{.Link}}/update">{{$.i18n.Tr "repo.pulls.update_branch"}}</div>
+												<div class="item" data-do="{{.Link}}/update?style=rebase">{{$.i18n.Tr "repo.pulls.update_branch_rebase"}}</div>
+											</div>
+										</div>
+									</div>
+								</div>
+							{{end}}
+							{{if and .UpdateAllowed (not .UpdateByRebaseAllowed)}}
 								<form action="{{.Link}}/update" method="post" class="ui update-branch-form">
 									{{.CsrfTokenHtml}}
 									<button class="ui compact button" data-do="update">
@@ -560,7 +579,26 @@
 						{{$.i18n.Tr "repo.pulls.outdated_with_base_branch"}}
 					</div>
 					<div>
-						{{if .UpdateAllowed}}
+						{{if and .UpdateAllowed .UpdateByRebaseAllowed }}
+							<div class="dib">
+								<div class="ui buttons update-button">
+									<button class="ui button" data-do="{{.Link}}/update" data-redirect="{{.Link}}">
+										<span class="button-text">
+											{{$.i18n.Tr "repo.pulls.update_branch"}}
+										</span>
+									</button>
+
+									<div class="ui dropdown icon button no-text">
+										{{svg "octicon-triangle-down" 14 "dropdown icon"}}
+										<div class="menu">
+											<div class="item active selected" data-do="{{.Link}}/update">{{$.i18n.Tr "repo.pulls.update_branch"}}</div>
+											<div class="item" data-do="{{.Link}}/update?style=rebase">{{$.i18n.Tr "repo.pulls.update_branch_rebase"}}</div>
+										</div>
+									</div>
+								</div>
+							</div>
+						{{end}}
+						{{if and .UpdateAllowed (not .UpdateByRebaseAllowed)}}
 							<form action="{{.Link}}/update" method="post">
 								{{.CsrfTokenHtml}}
 								<button class="ui compact button" data-do="update">

templates/swagger/v1_json.tmpl

@@ -8128,6 +8128,16 @@
             "name": "index",
             "in": "path",
             "required": true
+          },
+          {
+            "enum": [
+              "merge",
+              "rebase"
+            ],
+            "type": "string",
+            "description": "how to update pull request",
+            "name": "style",
+            "in": "query"
           }
         ],
         "responses": {

web_src/js/index.js

@@ -1267,6 +1267,36 @@ async function initRepository() {
       $('.instruct-toggle').show();
     });
 
+    // Pull Request update button
+    const $pullUpdateButton = $('.update-button > button');
+    $pullUpdateButton.on('click', function (e) {
+      e.preventDefault();
+      const $this = $(this);
+      const redirect = $this.data('redirect');
+      $this.addClass('loading');
+      $.post($this.data('do'), {
+        _csrf: csrf
+      }).done((data) => {
+        if (data.redirect) {
+          window.location.href = data.redirect;
+        } else if (redirect) {
+          window.location.href = redirect;
+        } else {
+          window.location.reload();
+        }
+      });
+    });
+
+    $('.update-button > .dropdown').dropdown({
+      onChange(_text, _value, $choice) {
+        const $url = $choice.data('do');
+        if ($url) {
+          $pullUpdateButton.find('.button-text').text($choice.text());
+          $pullUpdateButton.data('do', $url);
+        }
+      }
+    });
+
     initReactionSelector();
   }