Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add force_merge to merge request and fix checking mergable #23010

Merged
merged 8 commits into from
Feb 21, 2023
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
introduce MergeCheckType
wxiaoguang committed Feb 21, 2023

Verified

This commit was signed with the committer’s verified signature.
simeonschaub Simeon David Schaub
commit 570d3764880702e7de74010c5729bb1d5f3978fa
10 changes: 8 additions & 2 deletions routers/api/v1/repo/pull.go
Original file line number Diff line number Diff line change
@@ -767,10 +767,16 @@ func MergePullRequest(ctx *context.APIContext) {
}
}

manuallMerge := repo_model.MergeStyle(form.Do) == repo_model.MergeStyleManuallyMerged
mergeCheckType := pull_service.MergeCheckTypeGeneral
if form.MergeWhenChecksSucceed {
mergeCheckType = pull_service.MergeCheckTypeAuto
}
if repo_model.MergeStyle(form.Do) == repo_model.MergeStyleManuallyMerged {
mergeCheckType = pull_service.MergeCheckTypeManually
}

// start with merging by checking
if err := pull_service.CheckPullMergable(ctx, ctx.Doer, &ctx.Repo.Permission, pr, manuallMerge, form.ForceMerge); 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) {
13 changes: 10 additions & 3 deletions routers/web/repo/pull.go
Original file line number Diff line number Diff line change
@@ -926,10 +926,17 @@ 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

mergeCheckType := pull_service.MergeCheckTypeGeneral
if form.MergeWhenChecksSucceed {
mergeCheckType = pull_service.MergeCheckTypeAuto
}
if repo_model.MergeStyle(form.Do) == repo_model.MergeStyleManuallyMerged {
mergeCheckType = pull_service.MergeCheckTypeManually
}

// start with merging by checking
if err := pull_service.CheckPullMergable(ctx, ctx.Doer, &ctx.Repo.Permission, pr, manualMerge, form.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 {
@@ -961,7 +968,7 @@ func MergePullRequest(ctx *context.Context) {
}

// handle manually-merged mark
if manualMerge {
if mergeCheckType == pull_service.MergeCheckTypeManually {
if err := pull_service.MergedManually(pr, ctx.Doer, ctx.Repo.GitRepo, form.MergeCommitID); err != nil {
switch {

2 changes: 1 addition & 1 deletion services/automerge/automerge.go
Original file line number Diff line number Diff line change
@@ -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
37 changes: 28 additions & 9 deletions services/pull/check.go
Original file line number Diff line number Diff line change
@@ -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
}
}
1 change: 1 addition & 0 deletions web_src/js/components/PullRequestMergeForm.vue
Original file line number Diff line number Diff line change
@@ -128,6 +128,7 @@ export default {
textDoMerge: '',
mergeTitleFieldText: '',
mergeMessageFieldText: '',
hideAutoMerge: false,
},
mergeStyleAllowedCount: 0,