summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authora1012112796 <1012112796@qq.com>2021-08-31 22:03:45 +0800
committerGitHub <noreply@github.com>2021-08-31 16:03:45 +0200
commitcbf05c3f795c1eed999dafe8d0757495e07f1ee2 (patch)
treee9b227ef8dd8d836b04bd126806c8a1264b750ee
parent2bb32006fd560af44426a06f63f83e3c70c3f258 (diff)
downloadgitea-cbf05c3f795c1eed999dafe8d0757495e07f1ee2.tar.gz
gitea-cbf05c3f795c1eed999dafe8d0757495e07f1ee2.zip
Add option to update pull request by `rebase` (#16125)
* add option to update pull request by `rebase` Signed-off-by: a1012112796 <1012112796@qq.com>
-rw-r--r--integrations/pull_update_test.go28
-rw-r--r--models/pull.go2
-rw-r--r--options/locale/locale_en-US.ini3
-rw-r--r--routers/api/v1/repo/pull.go13
-rw-r--r--routers/web/repo/pull.go10
-rw-r--r--services/pull/merge.go17
-rw-r--r--services/pull/update.go55
-rw-r--r--templates/repo/issue/view_content/pull.tmpl42
-rw-r--r--templates/swagger/v1_json.tmpl10
-rw-r--r--web_src/js/index.js30
10 files changed, 184 insertions, 26 deletions
diff --git a/integrations/pull_update_test.go b/integrations/pull_update_test.go
index 46a1e04b25..3e3b987b64 100644
--- a/integrations/pull_update_test.go
+++ b/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",
diff --git a/models/pull.go b/models/pull.go
index e8e815385d..ea8f289993 100644
--- a/models/pull.go
+++ b/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
diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini
index 76536f2d49..3462a15cca 100644
--- a/options/locale/locale_en-US.ini
+++ b/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
diff --git a/routers/api/v1/repo/pull.go b/routers/api/v1/repo/pull.go
index 0a903101c7..e493e720fb 100644
--- a/routers/api/v1/repo/pull.go
+++ b/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
diff --git a/routers/web/repo/pull.go b/routers/web/repo/pull.go
index 8ff2ddf394..885ac3391a 100644
--- a/routers/web/repo/pull.go
+++ b/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{}{
diff --git a/services/pull/merge.go b/services/pull/merge.go
index 7e6a214b87..ef797e1ca4 100644
--- a/services/pull/merge.go
+++ b/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(),
diff --git a/services/pull/update.go b/services/pull/update.go
index c2c13845e3..4b4e67797e 100644
--- a/services/pull/update.go
+++ b/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
+ }
+
+ mergeAllowed, err = IsUserAllowedToMerge(pr, headRepoPerm, user)
+ if err != nil {
+ return false, false, err
}
- return IsUserAllowedToMerge(pr, headRepoPerm, user)
+ return mergeAllowed, rebaseAllowed, nil
}
// GetDiverging determines how many commits a PR is ahead or behind the PR base branch
diff --git a/templates/repo/issue/view_content/pull.tmpl b/templates/repo/issue/view_content/pull.tmpl
index 15ce63b4d3..2e624a9fbf 100644
--- a/templates/repo/issue/view_content/pull.tmpl
+++ b/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">
diff --git a/templates/swagger/v1_json.tmpl b/templates/swagger/v1_json.tmpl
index ee759ba6b1..90a32e28af 100644
--- a/templates/swagger/v1_json.tmpl
+++ b/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": {
diff --git a/web_src/js/index.js b/web_src/js/index.js
index 7e4970c3a0..ba025cfade 100644
--- a/web_src/js/index.js
+++ b/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();
}