Move checks for pulls before merge into own function (#19271)
This make checks in one single place so they dont differ and maintainer can not forget a check in one place while adding it to the other .... ( as it's atm ) Fix: * The API does ignore issue dependencies where Web does not * The API checks if "IsSignedIfRequired" where Web does not - UI probably do but nothing will some to craft custom requests * Default merge message is crafted a bit different between API and Web if not set on specific cases ...
This commit is contained in:
parent
f6145a69c4
commit
9c349a4277
9 changed files with 217 additions and 211 deletions
|
@ -599,6 +599,31 @@ func (f *MergePullRequestForm) Validate(req *http.Request, errs binding.Errors)
|
|||
return middleware.Validate(errs, ctx.Data, f, ctx.Locale)
|
||||
}
|
||||
|
||||
// SetDefaults if not provided for mergestyle and commit message
|
||||
func (f *MergePullRequestForm) SetDefaults(pr *models.PullRequest) (err error) {
|
||||
if f.Do == "" {
|
||||
f.Do = "merge"
|
||||
}
|
||||
|
||||
f.MergeTitleField = strings.TrimSpace(f.MergeTitleField)
|
||||
if len(f.MergeTitleField) == 0 {
|
||||
switch f.Do {
|
||||
case "merge", "rebase-merge":
|
||||
f.MergeTitleField, err = pr.GetDefaultMergeMessage()
|
||||
case "squash":
|
||||
f.MergeTitleField, err = pr.GetDefaultSquashMessage()
|
||||
}
|
||||
}
|
||||
|
||||
f.MergeMessageField = strings.TrimSpace(f.MergeMessageField)
|
||||
if len(f.MergeMessageField) > 0 {
|
||||
f.MergeTitleField += "\n\n" + f.MergeMessageField
|
||||
f.MergeMessageField = ""
|
||||
}
|
||||
|
||||
return
|
||||
}
|
||||
|
||||
// CodeCommentForm form for adding code comments for PRs
|
||||
type CodeCommentForm struct {
|
||||
Origin string `binding:"Required;In(timeline,diff)"`
|
||||
|
|
|
@ -7,6 +7,7 @@ package pull
|
|||
|
||||
import (
|
||||
"context"
|
||||
"errors"
|
||||
"fmt"
|
||||
"os"
|
||||
"strconv"
|
||||
|
@ -24,11 +25,21 @@ import (
|
|||
"code.gitea.io/gitea/modules/queue"
|
||||
"code.gitea.io/gitea/modules/timeutil"
|
||||
"code.gitea.io/gitea/modules/util"
|
||||
asymkey_service "code.gitea.io/gitea/services/asymkey"
|
||||
)
|
||||
|
||||
// prQueue represents a queue to handle update pull request tests
|
||||
var prQueue queue.UniqueQueue
|
||||
|
||||
var (
|
||||
ErrIsClosed = errors.New("pull is cosed")
|
||||
ErrUserNotAllowedToMerge = errors.New("user not allowed to merge")
|
||||
ErrHasMerged = errors.New("has already been merged")
|
||||
ErrIsWorkInProgress = errors.New("work in progress PRs cannot be merged")
|
||||
ErrNotMergableState = errors.New("not in mergeable state")
|
||||
ErrDependenciesLeft = errors.New("is blocked by an open dependency")
|
||||
)
|
||||
|
||||
// AddToTaskQueue adds itself to pull request test task queue.
|
||||
func AddToTaskQueue(pr *models.PullRequest) {
|
||||
err := prQueue.PushFunc(strconv.FormatInt(pr.ID, 10), func() error {
|
||||
|
@ -46,6 +57,79 @@ func AddToTaskQueue(pr *models.PullRequest) {
|
|||
}
|
||||
}
|
||||
|
||||
// CheckPullMergable check if the pull mergable based on all conditions (branch protection, merge options, ...)
|
||||
func CheckPullMergable(ctx context.Context, doer *user_model.User, perm *models.Permission, pr *models.PullRequest, manuallMerge, force bool) error {
|
||||
if pr.HasMerged {
|
||||
return ErrHasMerged
|
||||
}
|
||||
|
||||
if err := pr.LoadIssue(); err != nil {
|
||||
return err
|
||||
} else if pr.Issue.IsClosed {
|
||||
return ErrIsClosed
|
||||
}
|
||||
|
||||
if allowedMerge, err := IsUserAllowedToMerge(pr, *perm, doer); err != nil {
|
||||
return err
|
||||
} else if !allowedMerge {
|
||||
return ErrUserNotAllowedToMerge
|
||||
}
|
||||
|
||||
if manuallMerge {
|
||||
// don't check rules to "auto merge", doer is going to mark this pull as merged manually
|
||||
return nil
|
||||
}
|
||||
|
||||
if pr.IsWorkInProgress() {
|
||||
return ErrIsWorkInProgress
|
||||
}
|
||||
|
||||
if !pr.CanAutoMerge() {
|
||||
return ErrNotMergableState
|
||||
}
|
||||
|
||||
if err := CheckPRReadyToMerge(ctx, pr, false); err != nil {
|
||||
if models.IsErrDisallowedToMerge(err) {
|
||||
if force {
|
||||
if isRepoAdmin, err := models.IsUserRepoAdmin(pr.BaseRepo, doer); err != nil {
|
||||
return err
|
||||
} else if !isRepoAdmin {
|
||||
return ErrUserNotAllowedToMerge
|
||||
}
|
||||
}
|
||||
} else {
|
||||
return err
|
||||
}
|
||||
}
|
||||
|
||||
if _, err := isSignedIfRequired(ctx, pr, doer); err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
if noDeps, err := models.IssueNoDependenciesLeft(pr.Issue); err != nil {
|
||||
return err
|
||||
} else if !noDeps {
|
||||
return ErrDependenciesLeft
|
||||
}
|
||||
|
||||
return nil
|
||||
}
|
||||
|
||||
// isSignedIfRequired check if merge will be signed if required
|
||||
func isSignedIfRequired(ctx context.Context, pr *models.PullRequest, doer *user_model.User) (bool, error) {
|
||||
if err := pr.LoadProtectedBranch(); err != nil {
|
||||
return false, err
|
||||
}
|
||||
|
||||
if pr.ProtectedBranch == nil || !pr.ProtectedBranch.RequireSignedCommits {
|
||||
return true, nil
|
||||
}
|
||||
|
||||
sign, _, _, err := asymkey_service.SignMerge(ctx, pr, doer, pr.BaseRepo.RepoPath(), pr.BaseBranch, pr.GetGitRefName())
|
||||
|
||||
return sign, err
|
||||
}
|
||||
|
||||
// checkAndUpdateStatus checks if pull request is possible to leaving checking status,
|
||||
// and set to be either conflict or mergeable.
|
||||
func checkAndUpdateStatus(pr *models.PullRequest) {
|
||||
|
|
|
@ -660,21 +660,6 @@ func getDiffTree(ctx context.Context, repoPath, baseBranch, headBranch string) (
|
|||
return out.String(), nil
|
||||
}
|
||||
|
||||
// IsSignedIfRequired check if merge will be signed if required
|
||||
func IsSignedIfRequired(ctx context.Context, pr *models.PullRequest, doer *user_model.User) (bool, error) {
|
||||
if err := pr.LoadProtectedBranch(); err != nil {
|
||||
return false, err
|
||||
}
|
||||
|
||||
if pr.ProtectedBranch == nil || !pr.ProtectedBranch.RequireSignedCommits {
|
||||
return true, nil
|
||||
}
|
||||
|
||||
sign, _, _, err := asymkey_service.SignMerge(ctx, pr, doer, pr.BaseRepo.RepoPath(), pr.BaseBranch, pr.GetGitRefName())
|
||||
|
||||
return sign, err
|
||||
}
|
||||
|
||||
// IsUserAllowedToMerge check if user is allowed to merge PR with given permissions and branch protections
|
||||
func IsUserAllowedToMerge(pr *models.PullRequest, p models.Permission, user *user_model.User) (bool, error) {
|
||||
if user == nil {
|
||||
|
@ -711,29 +696,29 @@ func CheckPRReadyToMerge(ctx context.Context, pr *models.PullRequest, skipProtec
|
|||
return err
|
||||
}
|
||||
if !isPass {
|
||||
return models.ErrNotAllowedToMerge{
|
||||
return models.ErrDisallowedToMerge{
|
||||
Reason: "Not all required status checks successful",
|
||||
}
|
||||
}
|
||||
|
||||
if !pr.ProtectedBranch.HasEnoughApprovals(pr) {
|
||||
return models.ErrNotAllowedToMerge{
|
||||
return models.ErrDisallowedToMerge{
|
||||
Reason: "Does not have enough approvals",
|
||||
}
|
||||
}
|
||||
if pr.ProtectedBranch.MergeBlockedByRejectedReview(pr) {
|
||||
return models.ErrNotAllowedToMerge{
|
||||
return models.ErrDisallowedToMerge{
|
||||
Reason: "There are requested changes",
|
||||
}
|
||||
}
|
||||
if pr.ProtectedBranch.MergeBlockedByOfficialReviewRequests(pr) {
|
||||
return models.ErrNotAllowedToMerge{
|
||||
return models.ErrDisallowedToMerge{
|
||||
Reason: "There are official review requests",
|
||||
}
|
||||
}
|
||||
|
||||
if pr.ProtectedBranch.MergeBlockedByOutdatedBranch(pr) {
|
||||
return models.ErrNotAllowedToMerge{
|
||||
return models.ErrDisallowedToMerge{
|
||||
Reason: "The head branch is behind the base branch",
|
||||
}
|
||||
}
|
||||
|
@ -743,7 +728,7 @@ func CheckPRReadyToMerge(ctx context.Context, pr *models.PullRequest, skipProtec
|
|||
}
|
||||
|
||||
if pr.ProtectedBranch.MergeBlockedByProtectedFiles(pr) {
|
||||
return models.ErrNotAllowedToMerge{
|
||||
return models.ErrDisallowedToMerge{
|
||||
Reason: "Changed protected files",
|
||||
}
|
||||
}
|
||||
|
|
Loading…
Add table
Add a link
Reference in a new issue