diff options
author | 6543 <6543@obermui.de> | 2022-03-31 16:53:08 +0200 |
---|---|---|
committer | GitHub <noreply@github.com> | 2022-03-31 16:53:08 +0200 |
commit | 9c349a4277926bfd3ff0360301765ad7abd9f10b (patch) | |
tree | 6a3859850e61aca261738438deaf6075b1d30612 /services | |
parent | f6145a69c4814d9aaaeac5d73c743b2f65bc586c (diff) | |
download | gitea-9c349a4277926bfd3ff0360301765ad7abd9f10b.tar.gz gitea-9c349a4277926bfd3ff0360301765ad7abd9f10b.zip |
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 ...
Diffstat (limited to 'services')
-rw-r--r-- | services/forms/repo_form.go | 25 | ||||
-rw-r--r-- | services/pull/check.go | 84 | ||||
-rw-r--r-- | services/pull/merge.go | 27 |
3 files changed, 115 insertions, 21 deletions
diff --git a/services/forms/repo_form.go b/services/forms/repo_form.go index 33c7658640..80123e9af3 100644 --- a/services/forms/repo_form.go +++ b/services/forms/repo_form.go @@ -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)"` diff --git a/services/pull/check.go b/services/pull/check.go index f920688f5d..a4bbd3bb89 100644 --- a/services/pull/check.go +++ b/services/pull/check.go @@ -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) { diff --git a/services/pull/merge.go b/services/pull/merge.go index 6108a7956e..fb18be27c7 100644 --- a/services/pull/merge.go +++ b/services/pull/merge.go @@ -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", } } |