Skip to content

Commit cbf05c3

Browse files
a1012112796a1012112796
authored
Add option to update pull request by rebase (#16125)
* add option to update pull request by `rebase` Signed-off-by: a1012112796 <[email protected]>
1 parent 2bb3200 commit cbf05c3

File tree

10 files changed

+184
-26
lines changed

10 files changed

+184
-26
lines changed

‎integrations/pull_update_test.go

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,34 @@ func TestAPIPullUpdate(t *testing.T) {
4747
})
4848
}
4949

50+
func TestAPIPullUpdateByRebase(t *testing.T) {
51+
onGiteaRun(t, func(t *testing.T, giteaURL *url.URL) {
52+
//Create PR to test
53+
user := models.AssertExistsAndLoadBean(t, &models.User{ID: 2}).(*models.User)
54+
org26 := models.AssertExistsAndLoadBean(t, &models.User{ID: 26}).(*models.User)
55+
pr := createOutdatedPR(t, user, org26)
56+
57+
//Test GetDiverging
58+
diffCount, err := pull_service.GetDiverging(pr)
59+
assert.NoError(t, err)
60+
assert.EqualValues(t, 1, diffCount.Behind)
61+
assert.EqualValues(t, 1, diffCount.Ahead)
62+
assert.NoError(t, pr.LoadBaseRepo())
63+
assert.NoError(t, pr.LoadIssue())
64+
65+
session := loginUser(t, "user2")
66+
token := getTokenForLoggedInUser(t, session)
67+
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)
68+
session.MakeRequest(t, req, http.StatusOK)
69+
70+
//Test GetDiverging after update
71+
diffCount, err = pull_service.GetDiverging(pr)
72+
assert.NoError(t, err)
73+
assert.EqualValues(t, 0, diffCount.Behind)
74+
assert.EqualValues(t, 1, diffCount.Ahead)
75+
})
76+
}
77+
5078
func createOutdatedPR(t *testing.T, actor, forkOrg *models.User) *models.PullRequest {
5179
baseRepo, err := repo_service.CreateRepository(actor, actor, models.CreateRepoOptions{
5280
Name: "repo-pr-update",

‎models/pull.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -373,6 +373,8 @@ const (
373373
MergeStyleSquash MergeStyle = "squash"
374374
// MergeStyleManuallyMerged pr has been merged manually, just mark it as merged directly
375375
MergeStyleManuallyMerged MergeStyle = "manually-merged"
376+
// MergeStyleRebaseUpdate not a merge style, used to update pull head by rebase
377+
MergeStyleRebaseUpdate MergeStyle = "rebase-update-only"
376378
)
377379

378380
// SetMerged sets a pull request to merged and closes the corresponding issue

‎options/locale/locale_en-US.ini

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1468,7 +1468,8 @@ pulls.status_checks_failure = Some checks failed
14681468
pulls.status_checks_error = Some checks reported errors
14691469
pulls.status_checks_requested = Required
14701470
pulls.status_checks_details = Details
1471-
pulls.update_branch = Update branch
1471+
pulls.update_branch = Update branch by merge
1472+
pulls.update_branch_rebase = Update branch by rebase
14721473
pulls.update_branch_success = Branch update was successful
14731474
pulls.update_not_allowed = You are not allowed to update branch
14741475
pulls.outdated_with_base_branch = This branch is out-of-date with the base branch

‎routers/api/v1/repo/pull.go

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1065,6 +1065,11 @@ func UpdatePullRequest(ctx *context.APIContext) {
10651065
// type: integer
10661066
// format: int64
10671067
// required: true
1068+
// - name: style
1069+
// in: query
1070+
// description: how to update pull request
1071+
// type: string
1072+
// enum: [merge, rebase]
10681073
// responses:
10691074
// "200":
10701075
// "$ref": "#/responses/empty"
@@ -1111,21 +1116,23 @@ func UpdatePullRequest(ctx *context.APIContext) {
11111116
return
11121117
}
11131118

1114-
allowedUpdate, err := pull_service.IsUserAllowedToUpdate(pr, ctx.User)
1119+
rebase := ctx.FormString("style") == "rebase"
1120+
1121+
allowedUpdateByMerge, allowedUpdateByRebase, err := pull_service.IsUserAllowedToUpdate(pr, ctx.User)
11151122
if err != nil {
11161123
ctx.Error(http.StatusInternalServerError, "IsUserAllowedToMerge", err)
11171124
return
11181125
}
11191126

1120-
if !allowedUpdate {
1127+
if (!allowedUpdateByMerge && !rebase) || (rebase && !allowedUpdateByRebase) {
11211128
ctx.Status(http.StatusForbidden)
11221129
return
11231130
}
11241131

11251132
// default merge commit message
11261133
message := fmt.Sprintf("Merge branch '%s' into %s", pr.BaseBranch, pr.HeadBranch)
11271134

1128-
if err = pull_service.Update(pr, ctx.User, message); err != nil {
1135+
if err = pull_service.Update(pr, ctx.User, message, rebase); err != nil {
11291136
if models.IsErrMergeConflicts(err) {
11301137
ctx.Error(http.StatusConflict, "Update", "merge failed because of conflict")
11311138
return

‎routers/web/repo/pull.go

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -450,7 +450,7 @@ func PrepareViewPullInfo(ctx *context.Context, issue *models.Issue) *git.Compare
450450
}
451451

452452
if headBranchExist {
453-
ctx.Data["UpdateAllowed"], err = pull_service.IsUserAllowedToUpdate(pull, ctx.User)
453+
ctx.Data["UpdateAllowed"], ctx.Data["UpdateByRebaseAllowed"], err = pull_service.IsUserAllowedToUpdate(pull, ctx.User)
454454
if err != nil {
455455
ctx.ServerError("IsUserAllowedToUpdate", err)
456456
return nil
@@ -724,6 +724,8 @@ func UpdatePullRequest(ctx *context.Context) {
724724
return
725725
}
726726

727+
rebase := ctx.FormString("style") == "rebase"
728+
727729
if err := issue.PullRequest.LoadBaseRepo(); err != nil {
728730
ctx.ServerError("LoadBaseRepo", err)
729731
return
@@ -733,14 +735,14 @@ func UpdatePullRequest(ctx *context.Context) {
733735
return
734736
}
735737

736-
allowedUpdate, err := pull_service.IsUserAllowedToUpdate(issue.PullRequest, ctx.User)
738+
allowedUpdateByMerge, allowedUpdateByRebase, err := pull_service.IsUserAllowedToUpdate(issue.PullRequest, ctx.User)
737739
if err != nil {
738740
ctx.ServerError("IsUserAllowedToMerge", err)
739741
return
740742
}
741743

742744
// ToDo: add check if maintainers are allowed to change branch ... (need migration & co)
743-
if !allowedUpdate {
745+
if (!allowedUpdateByMerge && !rebase) || (rebase && !allowedUpdateByRebase) {
744746
ctx.Flash.Error(ctx.Tr("repo.pulls.update_not_allowed"))
745747
ctx.Redirect(ctx.Repo.RepoLink + "/pulls/" + fmt.Sprint(issue.Index))
746748
return
@@ -749,7 +751,7 @@ func UpdatePullRequest(ctx *context.Context) {
749751
// default merge commit message
750752
message := fmt.Sprintf("Merge branch '%s' into %s", issue.PullRequest.BaseBranch, issue.PullRequest.HeadBranch)
751753

752-
if err = pull_service.Update(issue.PullRequest, ctx.User, message); err != nil {
754+
if err = pull_service.Update(issue.PullRequest, ctx.User, message, rebase); err != nil {
753755
if models.IsErrMergeConflicts(err) {
754756
conflictError := err.(models.ErrMergeConflicts)
755757
flashError, err := ctx.HTMLString(string(tplAlertDetails), map[string]interface{}{

‎services/pull/merge.go

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -253,6 +253,8 @@ func rawMerge(pr *models.PullRequest, doer *models.User, mergeStyle models.Merge
253253
}
254254
case models.MergeStyleRebase:
255255
fallthrough
256+
case models.MergeStyleRebaseUpdate:
257+
fallthrough
256258
case models.MergeStyleRebaseMerge:
257259
// Checkout head branch
258260
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
305307
outbuf.Reset()
306308
errbuf.Reset()
307309

310+
// not need merge, just update by rebase. so skip
311+
if mergeStyle == models.MergeStyleRebaseUpdate {
312+
break
313+
}
314+
308315
// Checkout base branch again
309316
if err := git.NewCommand("checkout", baseBranch).RunInDirPipeline(tmpBasePath, &outbuf, &errbuf); err != nil {
310317
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
410417
pr.ID,
411418
)
412419

420+
var pushCmd *git.Command
421+
if mergeStyle == models.MergeStyleRebaseUpdate {
422+
// force push the rebase result to head brach
423+
pushCmd = git.NewCommand("push", "-f", "head_repo", stagingBranch+":refs/heads/"+pr.HeadBranch)
424+
} else {
425+
pushCmd = git.NewCommand("push", "origin", baseBranch+":refs/heads/"+pr.BaseBranch)
426+
}
427+
413428
// Push back to upstream.
414-
if err := git.NewCommand("push", "origin", baseBranch+":refs/heads/"+pr.BaseBranch).RunInDirTimeoutEnvPipeline(env, -1, tmpBasePath, &outbuf, &errbuf); err != nil {
429+
if err := pushCmd.RunInDirTimeoutEnvPipeline(env, -1, tmpBasePath, &outbuf, &errbuf); err != nil {
415430
if strings.Contains(errbuf.String(), "non-fast-forward") {
416431
return "", &git.ErrPushOutOfDate{
417432
StdOut: outbuf.String(),

‎services/pull/update.go

Lines changed: 40 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -13,13 +13,24 @@ import (
1313
)
1414

1515
// Update updates pull request with base branch.
16-
func Update(pull *models.PullRequest, doer *models.User, message string) error {
17-
//use merge functions but switch repo's and branch's
18-
pr := &models.PullRequest{
19-
HeadRepoID: pull.BaseRepoID,
20-
BaseRepoID: pull.HeadRepoID,
21-
HeadBranch: pull.BaseBranch,
22-
BaseBranch: pull.HeadBranch,
16+
func Update(pull *models.PullRequest, doer *models.User, message string, rebase bool) error {
17+
var (
18+
pr *models.PullRequest
19+
style models.MergeStyle
20+
)
21+
22+
if rebase {
23+
pr = pull
24+
style = models.MergeStyleRebaseUpdate
25+
} else {
26+
//use merge functions but switch repo's and branch's
27+
pr = &models.PullRequest{
28+
HeadRepoID: pull.BaseRepoID,
29+
BaseRepoID: pull.HeadRepoID,
30+
HeadBranch: pull.BaseBranch,
31+
BaseBranch: pull.HeadBranch,
32+
}
33+
style = models.MergeStyleMerge
2334
}
2435

2536
if pull.Flow == models.PullRequestFlowAGit {
@@ -42,27 +53,31 @@ func Update(pull *models.PullRequest, doer *models.User, message string) error {
4253
return fmt.Errorf("HeadBranch of PR %d is up to date", pull.Index)
4354
}
4455

45-
_, err = rawMerge(pr, doer, models.MergeStyleMerge, message)
56+
_, err = rawMerge(pr, doer, style, message)
4657

4758
defer func() {
59+
if rebase {
60+
go AddTestPullRequestTask(doer, pr.BaseRepo.ID, pr.BaseBranch, false, "", "")
61+
return
62+
}
4863
go AddTestPullRequestTask(doer, pr.HeadRepo.ID, pr.HeadBranch, false, "", "")
4964
}()
5065

5166
return err
5267
}
5368

5469
// IsUserAllowedToUpdate check if user is allowed to update PR with given permissions and branch protections
55-
func IsUserAllowedToUpdate(pull *models.PullRequest, user *models.User) (bool, error) {
70+
func IsUserAllowedToUpdate(pull *models.PullRequest, user *models.User) (mergeAllowed, rebaseAllowed bool, err error) {
5671
if pull.Flow == models.PullRequestFlowAGit {
57-
return false, nil
72+
return false, false, nil
5873
}
5974

6075
if user == nil {
61-
return false, nil
76+
return false, false, nil
6277
}
6378
headRepoPerm, err := models.GetUserRepoPermission(pull.HeadRepo, user)
6479
if err != nil {
65-
return false, err
80+
return false, false, err
6681
}
6782

6883
pr := &models.PullRequest{
@@ -74,15 +89,25 @@ func IsUserAllowedToUpdate(pull *models.PullRequest, user *models.User) (bool, e
7489

7590
err = pr.LoadProtectedBranch()
7691
if err != nil {
77-
return false, err
92+
return false, false, err
93+
}
94+
95+
// can't do rebase on protected branch because need force push
96+
if pr.ProtectedBranch == nil {
97+
rebaseAllowed = true
7898
}
7999

80100
// Update function need push permission
81101
if pr.ProtectedBranch != nil && !pr.ProtectedBranch.CanUserPush(user.ID) {
82-
return false, nil
102+
return false, false, nil
103+
}
104+
105+
mergeAllowed, err = IsUserAllowedToMerge(pr, headRepoPerm, user)
106+
if err != nil {
107+
return false, false, err
83108
}
84109

85-
return IsUserAllowedToMerge(pr, headRepoPerm, user)
110+
return mergeAllowed, rebaseAllowed, nil
86111
}
87112

88113
// GetDiverging determines how many commits a PR is ahead or behind the PR base branch

‎templates/repo/issue/view_content/pull.tmpl

Lines changed: 40 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -281,7 +281,26 @@
281281
{{$.i18n.Tr "repo.pulls.outdated_with_base_branch"}}
282282
</div>
283283
<div class="item-section-right">
284-
{{if .UpdateAllowed}}
284+
{{if and .UpdateAllowed .UpdateByRebaseAllowed }}
285+
<div class="dib">
286+
<div class="ui buttons update-button">
287+
<button class="ui button" data-do="{{.Link}}/update" data-redirect="{{.Link}}">
288+
<span class="button-text">
289+
{{$.i18n.Tr "repo.pulls.update_branch"}}
290+
</span>
291+
</button>
292+
293+
<div class="ui dropdown icon button no-text">
294+
{{svg "octicon-triangle-down" 14 "dropdown icon"}}
295+
<div class="menu">
296+
<div class="item active selected" data-do="{{.Link}}/update">{{$.i18n.Tr "repo.pulls.update_branch"}}</div>
297+
<div class="item" data-do="{{.Link}}/update?style=rebase">{{$.i18n.Tr "repo.pulls.update_branch_rebase"}}</div>
298+
</div>
299+
</div>
300+
</div>
301+
</div>
302+
{{end}}
303+
{{if and .UpdateAllowed (not .UpdateByRebaseAllowed)}}
285304
<form action="{{.Link}}/update" method="post" class="ui update-branch-form">
286305
{{.CsrfTokenHtml}}
287306
<button class="ui compact button" data-do="update">
@@ -560,7 +579,26 @@
560579
{{$.i18n.Tr "repo.pulls.outdated_with_base_branch"}}
561580
</div>
562581
<div>
563-
{{if .UpdateAllowed}}
582+
{{if and .UpdateAllowed .UpdateByRebaseAllowed }}
583+
<div class="dib">
584+
<div class="ui buttons update-button">
585+
<button class="ui button" data-do="{{.Link}}/update" data-redirect="{{.Link}}">
586+
<span class="button-text">
587+
{{$.i18n.Tr "repo.pulls.update_branch"}}
588+
</span>
589+
</button>
590+
591+
<div class="ui dropdown icon button no-text">
592+
{{svg "octicon-triangle-down" 14 "dropdown icon"}}
593+
<div class="menu">
594+
<div class="item active selected" data-do="{{.Link}}/update">{{$.i18n.Tr "repo.pulls.update_branch"}}</div>
595+
<div class="item" data-do="{{.Link}}/update?style=rebase">{{$.i18n.Tr "repo.pulls.update_branch_rebase"}}</div>
596+
</div>
597+
</div>
598+
</div>
599+
</div>
600+
{{end}}
601+
{{if and .UpdateAllowed (not .UpdateByRebaseAllowed)}}
564602
<form action="{{.Link}}/update" method="post">
565603
{{.CsrfTokenHtml}}
566604
<button class="ui compact button" data-do="update">

‎templates/swagger/v1_json.tmpl

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8128,6 +8128,16 @@
81288128
"name": "index",
81298129
"in": "path",
81308130
"required": true
8131+
},
8132+
{
8133+
"enum": [
8134+
"merge",
8135+
"rebase"
8136+
],
8137+
"type": "string",
8138+
"description": "how to update pull request",
8139+
"name": "style",
8140+
"in": "query"
81318141
}
81328142
],
81338143
"responses": {

‎web_src/js/index.js

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1267,6 +1267,36 @@ async function initRepository() {
12671267
$('.instruct-toggle').show();
12681268
});
12691269

1270+
// Pull Request update button
1271+
const $pullUpdateButton = $('.update-button > button');
1272+
$pullUpdateButton.on('click', function (e) {
1273+
e.preventDefault();
1274+
const $this = $(this);
1275+
const redirect = $this.data('redirect');
1276+
$this.addClass('loading');
1277+
$.post($this.data('do'), {
1278+
_csrf: csrf
1279+
}).done((data) => {
1280+
if (data.redirect) {
1281+
window.location.href = data.redirect;
1282+
} else if (redirect) {
1283+
window.location.href = redirect;
1284+
} else {
1285+
window.location.reload();
1286+
}
1287+
});
1288+
});
1289+
1290+
$('.update-button > .dropdown').dropdown({
1291+
onChange(_text, _value, $choice) {
1292+
const $url = $choice.data('do');
1293+
if ($url) {
1294+
$pullUpdateButton.find('.button-text').text($choice.text());
1295+
$pullUpdateButton.data('do', $url);
1296+
}
1297+
}
1298+
});
1299+
12701300
initReactionSelector();
12711301
}
12721302

0 commit comments

Comments
 (0)