diff options
author | a1012112796 <1012112796@qq.com> | 2021-08-31 22:03:45 +0800 |
---|---|---|
committer | GitHub <noreply@github.com> | 2021-08-31 16:03:45 +0200 |
commit | cbf05c3f795c1eed999dafe8d0757495e07f1ee2 (patch) | |
tree | e9b227ef8dd8d836b04bd126806c8a1264b750ee | |
parent | 2bb32006fd560af44426a06f63f83e3c70c3f258 (diff) | |
download | gitea-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.go | 28 | ||||
-rw-r--r-- | models/pull.go | 2 | ||||
-rw-r--r-- | options/locale/locale_en-US.ini | 3 | ||||
-rw-r--r-- | routers/api/v1/repo/pull.go | 13 | ||||
-rw-r--r-- | routers/web/repo/pull.go | 10 | ||||
-rw-r--r-- | services/pull/merge.go | 17 | ||||
-rw-r--r-- | services/pull/update.go | 55 | ||||
-rw-r--r-- | templates/repo/issue/view_content/pull.tmpl | 42 | ||||
-rw-r--r-- | templates/swagger/v1_json.tmpl | 10 | ||||
-rw-r--r-- | web_src/js/index.js | 30 |
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(); } |