Fix #23000.tags/v1.19.0-rc0
@@ -767,11 +767,18 @@ func MergePullRequest(ctx *context.APIContext) { | |||
} | |||
} | |||
manuallMerge := repo_model.MergeStyle(form.Do) == repo_model.MergeStyleManuallyMerged | |||
force := form.ForceMerge != nil && *form.ForceMerge | |||
manuallyMerged := repo_model.MergeStyle(form.Do) == repo_model.MergeStyleManuallyMerged | |||
mergeCheckType := pull_service.MergeCheckTypeGeneral | |||
if form.MergeWhenChecksSucceed { | |||
mergeCheckType = pull_service.MergeCheckTypeAuto | |||
} | |||
if manuallyMerged { | |||
mergeCheckType = pull_service.MergeCheckTypeManually | |||
} | |||
// start with merging by checking | |||
if err := pull_service.CheckPullMergable(ctx, ctx.Doer, &ctx.Repo.Permission, pr, manuallMerge, force); err != nil { | |||
if err := pull_service.CheckPullMergable(ctx, ctx.Doer, &ctx.Repo.Permission, pr, mergeCheckType, form.ForceMerge); err != nil { | |||
if errors.Is(err, pull_service.ErrIsClosed) { | |||
ctx.NotFound() | |||
} else if errors.Is(err, pull_service.ErrUserNotAllowedToMerge) { | |||
@@ -793,7 +800,7 @@ func MergePullRequest(ctx *context.APIContext) { | |||
} | |||
// handle manually-merged mark | |||
if manuallMerge { | |||
if manuallyMerged { | |||
if err := pull_service.MergedManually(pr, ctx.Doer, ctx.Repo.GitRepo, form.MergeCommitID); err != nil { | |||
if models.IsErrInvalidMergeStyle(err) { | |||
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))) |
@@ -926,11 +926,19 @@ func MergePullRequest(ctx *context.Context) { | |||
pr := issue.PullRequest | |||
pr.Issue = issue | |||
pr.Issue.Repo = ctx.Repo.Repository | |||
manualMerge := repo_model.MergeStyle(form.Do) == repo_model.MergeStyleManuallyMerged | |||
forceMerge := form.ForceMerge != nil && *form.ForceMerge | |||
manuallyMerged := repo_model.MergeStyle(form.Do) == repo_model.MergeStyleManuallyMerged | |||
mergeCheckType := pull_service.MergeCheckTypeGeneral | |||
if form.MergeWhenChecksSucceed { | |||
mergeCheckType = pull_service.MergeCheckTypeAuto | |||
} | |||
if manuallyMerged { | |||
mergeCheckType = pull_service.MergeCheckTypeManually | |||
} | |||
// start with merging by checking | |||
if err := pull_service.CheckPullMergable(ctx, ctx.Doer, &ctx.Repo.Permission, pr, manualMerge, forceMerge); err != nil { | |||
if err := pull_service.CheckPullMergable(ctx, ctx.Doer, &ctx.Repo.Permission, pr, mergeCheckType, form.ForceMerge); err != nil { | |||
switch { | |||
case errors.Is(err, pull_service.ErrIsClosed): | |||
if issue.IsPull { | |||
@@ -962,7 +970,7 @@ func MergePullRequest(ctx *context.Context) { | |||
} | |||
// handle manually-merged mark | |||
if manualMerge { | |||
if manuallyMerged { | |||
if err := pull_service.MergedManually(pr, ctx.Doer, ctx.Repo.GitRepo, form.MergeCommitID); err != nil { | |||
switch { | |||
@@ -230,7 +230,7 @@ func handlePull(pullID int64, sha string) { | |||
return | |||
} | |||
if err := pull_service.CheckPullMergable(ctx, doer, &perm, pr, false, false); err != nil { | |||
if err := pull_service.CheckPullMergable(ctx, doer, &perm, pr, pull_service.MergeCheckTypeGeneral, false); err != nil { | |||
if errors.Is(pull_service.ErrUserNotAllowedToMerge, err) { | |||
log.Info("%-v was scheduled to automerge by an unauthorized user", pr) | |||
return |
@@ -604,7 +604,7 @@ type MergePullRequestForm struct { | |||
MergeMessageField string | |||
MergeCommitID string // only used for manually-merged | |||
HeadCommitID string `json:"head_commit_id,omitempty"` | |||
ForceMerge *bool `json:"force_merge,omitempty"` | |||
ForceMerge bool `json:"force_merge,omitempty"` | |||
MergeWhenChecksSucceed bool `json:"merge_when_checks_succeed,omitempty"` | |||
DeleteBranchAfterMerge bool `json:"delete_branch_after_merge,omitempty"` | |||
} |
@@ -59,8 +59,16 @@ func AddToTaskQueue(pr *issues_model.PullRequest) { | |||
} | |||
} | |||
type MergeCheckType int | |||
const ( | |||
MergeCheckTypeGeneral MergeCheckType = iota // general merge checks for "merge", "rebase", "squash", etc | |||
MergeCheckTypeManually // Manually Merged button (mark a PR as merged manually) | |||
MergeCheckTypeAuto // Auto Merge (Scheduled Merge) After Checks Succeed | |||
) | |||
// CheckPullMergable check if the pull mergable based on all conditions (branch protection, merge options, ...) | |||
func CheckPullMergable(stdCtx context.Context, doer *user_model.User, perm *access_model.Permission, pr *issues_model.PullRequest, manuallMerge, force bool) error { | |||
func CheckPullMergable(stdCtx context.Context, doer *user_model.User, perm *access_model.Permission, pr *issues_model.PullRequest, mergeCheckType MergeCheckType, adminSkipProtectionCheck bool) error { | |||
return db.WithTx(stdCtx, func(ctx context.Context) error { | |||
if pr.HasMerged { | |||
return ErrHasMerged | |||
@@ -80,8 +88,8 @@ func CheckPullMergable(stdCtx context.Context, doer *user_model.User, perm *acce | |||
return ErrUserNotAllowedToMerge | |||
} | |||
if manuallMerge { | |||
// don't check rules to "auto merge", doer is going to mark this pull as merged manually | |||
if mergeCheckType == MergeCheckTypeManually { | |||
// if doer is doing "manually merge" (mark as merged manually), do not check anything | |||
return nil | |||
} | |||
@@ -103,14 +111,25 @@ func CheckPullMergable(stdCtx context.Context, doer *user_model.User, perm *acce | |||
return err | |||
} | |||
if !force { | |||
return err | |||
// Now the branch protection check failed, check whether the failure could be skipped (skip by setting err = nil) | |||
// * when doing Auto Merge (Scheduled Merge After Checks Succeed), skip the branch protection check | |||
if mergeCheckType == MergeCheckTypeAuto { | |||
err = nil | |||
} | |||
// * if the doer is admin, they could skip the branch protection check | |||
if adminSkipProtectionCheck { | |||
if isRepoAdmin, errCheckAdmin := access_model.IsUserRepoAdmin(ctx, pr.BaseRepo, doer); errCheckAdmin != nil { | |||
log.Error("Unable to check if %-v is a repo admin in %-v: %v", doer, pr.BaseRepo, errCheckAdmin) | |||
return errCheckAdmin | |||
} else if isRepoAdmin { | |||
err = nil // repo admin can skip the check, so clear the error | |||
} | |||
} | |||
if isRepoAdmin, err2 := access_model.IsUserRepoAdmin(ctx, pr.BaseRepo, doer); err2 != nil { | |||
log.Error("Unable to check if %-v is a repo admin in %-v: %v", doer, pr.BaseRepo, err2) | |||
return err2 | |||
} else if !isRepoAdmin { | |||
// If there is still a branch protection check error, return it | |||
if err != nil { | |||
return err | |||
} | |||
} |
@@ -18,6 +18,7 @@ | |||
<input type="hidden" name="_csrf" :value="csrfToken"> | |||
<input type="hidden" name="head_commit_id" v-model="mergeForm.pullHeadCommitID"> | |||
<input type="hidden" name="merge_when_checks_succeed" v-model="autoMergeWhenSucceed"> | |||
<input type="hidden" name="force_merge" v-model="forceMerge"> | |||
<template v-if="!mergeStyleDetail.hideMergeMessageTexts"> | |||
<div class="field"> | |||
@@ -131,6 +132,7 @@ export default { | |||
textDoMerge: '', | |||
mergeTitleFieldText: '', | |||
mergeMessageFieldText: '', | |||
hideAutoMerge: false, | |||
}, | |||
mergeStyleAllowedCount: 0, | |||
@@ -141,7 +143,10 @@ export default { | |||
mergeButtonStyleClass() { | |||
if (this.mergeForm.allOverridableChecksOk) return 'green'; | |||
return this.autoMergeWhenSucceed ? 'blue' : 'red'; | |||
} | |||
}, | |||
forceMerge() { | |||
return this.mergeForm.canMergeNow && !this.mergeForm.allOverridableChecksOk; | |||
}, | |||
}, | |||
watch: { | |||
mergeStyle(val) { |