summaryrefslogtreecommitdiffstats
path: root/services
diff options
context:
space:
mode:
author6543 <6543@obermui.de>2022-03-31 16:53:08 +0200
committerGitHub <noreply@github.com>2022-03-31 16:53:08 +0200
commit9c349a4277926bfd3ff0360301765ad7abd9f10b (patch)
tree6a3859850e61aca261738438deaf6075b1d30612 /services
parentf6145a69c4814d9aaaeac5d73c743b2f65bc586c (diff)
downloadgitea-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.go25
-rw-r--r--services/pull/check.go84
-rw-r--r--services/pull/merge.go27
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",
}
}