Skip to content

Commit c8c2a31

Browse files
authored
Add force_merge to merge request and fix checking mergable (#23010)
Fix #23000.
1 parent 1fcf96a commit c8c2a31

File tree

6 files changed

+59
-20
lines changed

6 files changed

+59
-20
lines changed

routers/api/v1/repo/pull.go

+11-4
Original file line numberDiff line numberDiff line change
@@ -767,11 +767,18 @@ func MergePullRequest(ctx *context.APIContext) {
767767
}
768768
}
769769

770-
manuallMerge := repo_model.MergeStyle(form.Do) == repo_model.MergeStyleManuallyMerged
771-
force := form.ForceMerge != nil && *form.ForceMerge
770+
manuallyMerged := repo_model.MergeStyle(form.Do) == repo_model.MergeStyleManuallyMerged
771+
772+
mergeCheckType := pull_service.MergeCheckTypeGeneral
773+
if form.MergeWhenChecksSucceed {
774+
mergeCheckType = pull_service.MergeCheckTypeAuto
775+
}
776+
if manuallyMerged {
777+
mergeCheckType = pull_service.MergeCheckTypeManually
778+
}
772779

773780
// start with merging by checking
774-
if err := pull_service.CheckPullMergable(ctx, ctx.Doer, &ctx.Repo.Permission, pr, manuallMerge, force); err != nil {
781+
if err := pull_service.CheckPullMergable(ctx, ctx.Doer, &ctx.Repo.Permission, pr, mergeCheckType, form.ForceMerge); err != nil {
775782
if errors.Is(err, pull_service.ErrIsClosed) {
776783
ctx.NotFound()
777784
} else if errors.Is(err, pull_service.ErrUserNotAllowedToMerge) {
@@ -793,7 +800,7 @@ func MergePullRequest(ctx *context.APIContext) {
793800
}
794801

795802
// handle manually-merged mark
796-
if manuallMerge {
803+
if manuallyMerged {
797804
if err := pull_service.MergedManually(pr, ctx.Doer, ctx.Repo.GitRepo, form.MergeCommitID); err != nil {
798805
if models.IsErrInvalidMergeStyle(err) {
799806
ctx.Error(http.StatusMethodNotAllowed, "Invalid merge style", fmt.Errorf("%s is not allowed an allowed merge style for this repository", repo_model.MergeStyle(form.Do)))

routers/web/repo/pull.go

+12-4
Original file line numberDiff line numberDiff line change
@@ -926,11 +926,19 @@ func MergePullRequest(ctx *context.Context) {
926926
pr := issue.PullRequest
927927
pr.Issue = issue
928928
pr.Issue.Repo = ctx.Repo.Repository
929-
manualMerge := repo_model.MergeStyle(form.Do) == repo_model.MergeStyleManuallyMerged
930-
forceMerge := form.ForceMerge != nil && *form.ForceMerge
929+
930+
manuallyMerged := repo_model.MergeStyle(form.Do) == repo_model.MergeStyleManuallyMerged
931+
932+
mergeCheckType := pull_service.MergeCheckTypeGeneral
933+
if form.MergeWhenChecksSucceed {
934+
mergeCheckType = pull_service.MergeCheckTypeAuto
935+
}
936+
if manuallyMerged {
937+
mergeCheckType = pull_service.MergeCheckTypeManually
938+
}
931939

932940
// start with merging by checking
933-
if err := pull_service.CheckPullMergable(ctx, ctx.Doer, &ctx.Repo.Permission, pr, manualMerge, forceMerge); err != nil {
941+
if err := pull_service.CheckPullMergable(ctx, ctx.Doer, &ctx.Repo.Permission, pr, mergeCheckType, form.ForceMerge); err != nil {
934942
switch {
935943
case errors.Is(err, pull_service.ErrIsClosed):
936944
if issue.IsPull {
@@ -962,7 +970,7 @@ func MergePullRequest(ctx *context.Context) {
962970
}
963971

964972
// handle manually-merged mark
965-
if manualMerge {
973+
if manuallyMerged {
966974
if err := pull_service.MergedManually(pr, ctx.Doer, ctx.Repo.GitRepo, form.MergeCommitID); err != nil {
967975
switch {
968976

services/automerge/automerge.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -230,7 +230,7 @@ func handlePull(pullID int64, sha string) {
230230
return
231231
}
232232

233-
if err := pull_service.CheckPullMergable(ctx, doer, &perm, pr, false, false); err != nil {
233+
if err := pull_service.CheckPullMergable(ctx, doer, &perm, pr, pull_service.MergeCheckTypeGeneral, false); err != nil {
234234
if errors.Is(pull_service.ErrUserNotAllowedToMerge, err) {
235235
log.Info("%-v was scheduled to automerge by an unauthorized user", pr)
236236
return

services/forms/repo_form.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -604,7 +604,7 @@ type MergePullRequestForm struct {
604604
MergeMessageField string
605605
MergeCommitID string // only used for manually-merged
606606
HeadCommitID string `json:"head_commit_id,omitempty"`
607-
ForceMerge *bool `json:"force_merge,omitempty"`
607+
ForceMerge bool `json:"force_merge,omitempty"`
608608
MergeWhenChecksSucceed bool `json:"merge_when_checks_succeed,omitempty"`
609609
DeleteBranchAfterMerge bool `json:"delete_branch_after_merge,omitempty"`
610610
}

services/pull/check.go

+28-9
Original file line numberDiff line numberDiff line change
@@ -59,8 +59,16 @@ func AddToTaskQueue(pr *issues_model.PullRequest) {
5959
}
6060
}
6161

62+
type MergeCheckType int
63+
64+
const (
65+
MergeCheckTypeGeneral MergeCheckType = iota // general merge checks for "merge", "rebase", "squash", etc
66+
MergeCheckTypeManually // Manually Merged button (mark a PR as merged manually)
67+
MergeCheckTypeAuto // Auto Merge (Scheduled Merge) After Checks Succeed
68+
)
69+
6270
// CheckPullMergable check if the pull mergable based on all conditions (branch protection, merge options, ...)
63-
func CheckPullMergable(stdCtx context.Context, doer *user_model.User, perm *access_model.Permission, pr *issues_model.PullRequest, manuallMerge, force bool) error {
71+
func CheckPullMergable(stdCtx context.Context, doer *user_model.User, perm *access_model.Permission, pr *issues_model.PullRequest, mergeCheckType MergeCheckType, adminSkipProtectionCheck bool) error {
6472
return db.WithTx(stdCtx, func(ctx context.Context) error {
6573
if pr.HasMerged {
6674
return ErrHasMerged
@@ -80,8 +88,8 @@ func CheckPullMergable(stdCtx context.Context, doer *user_model.User, perm *acce
8088
return ErrUserNotAllowedToMerge
8189
}
8290

83-
if manuallMerge {
84-
// don't check rules to "auto merge", doer is going to mark this pull as merged manually
91+
if mergeCheckType == MergeCheckTypeManually {
92+
// if doer is doing "manually merge" (mark as merged manually), do not check anything
8593
return nil
8694
}
8795

@@ -103,14 +111,25 @@ func CheckPullMergable(stdCtx context.Context, doer *user_model.User, perm *acce
103111
return err
104112
}
105113

106-
if !force {
107-
return err
114+
// Now the branch protection check failed, check whether the failure could be skipped (skip by setting err = nil)
115+
116+
// * when doing Auto Merge (Scheduled Merge After Checks Succeed), skip the branch protection check
117+
if mergeCheckType == MergeCheckTypeAuto {
118+
err = nil
119+
}
120+
121+
// * if the doer is admin, they could skip the branch protection check
122+
if adminSkipProtectionCheck {
123+
if isRepoAdmin, errCheckAdmin := access_model.IsUserRepoAdmin(ctx, pr.BaseRepo, doer); errCheckAdmin != nil {
124+
log.Error("Unable to check if %-v is a repo admin in %-v: %v", doer, pr.BaseRepo, errCheckAdmin)
125+
return errCheckAdmin
126+
} else if isRepoAdmin {
127+
err = nil // repo admin can skip the check, so clear the error
128+
}
108129
}
109130

110-
if isRepoAdmin, err2 := access_model.IsUserRepoAdmin(ctx, pr.BaseRepo, doer); err2 != nil {
111-
log.Error("Unable to check if %-v is a repo admin in %-v: %v", doer, pr.BaseRepo, err2)
112-
return err2
113-
} else if !isRepoAdmin {
131+
// If there is still a branch protection check error, return it
132+
if err != nil {
114133
return err
115134
}
116135
}

web_src/js/components/PullRequestMergeForm.vue

+6-1
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
<input type="hidden" name="_csrf" :value="csrfToken">
1919
<input type="hidden" name="head_commit_id" v-model="mergeForm.pullHeadCommitID">
2020
<input type="hidden" name="merge_when_checks_succeed" v-model="autoMergeWhenSucceed">
21+
<input type="hidden" name="force_merge" v-model="forceMerge">
2122

2223
<template v-if="!mergeStyleDetail.hideMergeMessageTexts">
2324
<div class="field">
@@ -131,6 +132,7 @@ export default {
131132
textDoMerge: '',
132133
mergeTitleFieldText: '',
133134
mergeMessageFieldText: '',
135+
hideAutoMerge: false,
134136
},
135137
mergeStyleAllowedCount: 0,
136138
@@ -141,7 +143,10 @@ export default {
141143
mergeButtonStyleClass() {
142144
if (this.mergeForm.allOverridableChecksOk) return 'green';
143145
return this.autoMergeWhenSucceed ? 'blue' : 'red';
144-
}
146+
},
147+
forceMerge() {
148+
return this.mergeForm.canMergeNow && !this.mergeForm.allOverridableChecksOk;
149+
},
145150
},
146151
watch: {
147152
mergeStyle(val) {

0 commit comments

Comments
 (0)