]> source.dussan.org Git - gitea.git/commitdiff
Move checks for pulls before merge into own function (#19271)
author6543 <6543@obermui.de>
Thu, 31 Mar 2022 14:53:08 +0000 (16:53 +0200)
committerGitHub <noreply@github.com>
Thu, 31 Mar 2022 14:53:08 +0000 (16:53 +0200)
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 ...

models/error.go
models/pull.go
models/pull_test.go
routers/api/v1/repo/pull.go
routers/private/hook_pre_receive.go
routers/web/repo/pull.go
services/forms/repo_form.go
services/pull/check.go
services/pull/merge.go

index fbd2f971857a46217fd6784b0a9d82f6db95ac54..6233b2ea853d4284f35e5306cd96a3cade3ad287 100644 (file)
@@ -586,18 +586,18 @@ func (err ErrBranchesEqual) Error() string {
        return fmt.Sprintf("branches are equal [head: %sm base: %s]", err.HeadBranchName, err.BaseBranchName)
 }
 
-// ErrNotAllowedToMerge represents an error that a branch is protected and the current user is not allowed to modify it.
-type ErrNotAllowedToMerge struct {
+// ErrDisallowedToMerge represents an error that a branch is protected and the current user is not allowed to modify it.
+type ErrDisallowedToMerge struct {
        Reason string
 }
 
-// IsErrNotAllowedToMerge checks if an error is an ErrNotAllowedToMerge.
-func IsErrNotAllowedToMerge(err error) bool {
-       _, ok := err.(ErrNotAllowedToMerge)
+// IsErrDisallowedToMerge checks if an error is an ErrDisallowedToMerge.
+func IsErrDisallowedToMerge(err error) bool {
+       _, ok := err.(ErrDisallowedToMerge)
        return ok
 }
 
-func (err ErrNotAllowedToMerge) Error() string {
+func (err ErrDisallowedToMerge) Error() string {
        return fmt.Sprintf("not allowed to merge [reason: %s]", err.Reason)
 }
 
index ec1fde02595b0301dd39cae6b9394cffa14f4404..6abd9f04b24f8ab6197b208c19bbf3babe86a795 100644 (file)
@@ -222,22 +222,19 @@ func (pr *PullRequest) loadProtectedBranch(ctx context.Context) (err error) {
 }
 
 // GetDefaultMergeMessage returns default message used when merging pull request
-func (pr *PullRequest) GetDefaultMergeMessage() string {
+func (pr *PullRequest) GetDefaultMergeMessage() (string, error) {
        if pr.HeadRepo == nil {
                var err error
                pr.HeadRepo, err = repo_model.GetRepositoryByID(pr.HeadRepoID)
                if err != nil {
-                       log.Error("GetRepositoryById[%d]: %v", pr.HeadRepoID, err)
-                       return ""
+                       return "", fmt.Errorf("GetRepositoryById[%d]: %v", pr.HeadRepoID, err)
                }
        }
        if err := pr.LoadIssue(); err != nil {
-               log.Error("Cannot load issue %d for PR id %d: Error: %v", pr.IssueID, pr.ID, err)
-               return ""
+               return "", fmt.Errorf("Cannot load issue %d for PR id %d: Error: %v", pr.IssueID, pr.ID, err)
        }
        if err := pr.LoadBaseRepo(); err != nil {
-               log.Error("LoadBaseRepo: %v", err)
-               return ""
+               return "", fmt.Errorf("LoadBaseRepo: %v", err)
        }
 
        issueReference := "#"
@@ -246,10 +243,10 @@ func (pr *PullRequest) GetDefaultMergeMessage() string {
        }
 
        if pr.BaseRepoID == pr.HeadRepoID {
-               return fmt.Sprintf("Merge pull request '%s' (%s%d) from %s into %s", pr.Issue.Title, issueReference, pr.Issue.Index, pr.HeadBranch, pr.BaseBranch)
+               return fmt.Sprintf("Merge pull request '%s' (%s%d) from %s into %s", pr.Issue.Title, issueReference, pr.Issue.Index, pr.HeadBranch, pr.BaseBranch), nil
        }
 
-       return fmt.Sprintf("Merge pull request '%s' (%s%d) from %s:%s into %s", pr.Issue.Title, issueReference, pr.Issue.Index, pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseBranch)
+       return fmt.Sprintf("Merge pull request '%s' (%s%d) from %s:%s into %s", pr.Issue.Title, issueReference, pr.Issue.Index, pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseBranch), nil
 }
 
 // ReviewCount represents a count of Reviews
@@ -335,19 +332,17 @@ func (pr *PullRequest) getReviewedByLines(writer io.Writer) error {
 }
 
 // GetDefaultSquashMessage returns default message used when squash and merging pull request
-func (pr *PullRequest) GetDefaultSquashMessage() string {
+func (pr *PullRequest) GetDefaultSquashMessage() (string, error) {
        if err := pr.LoadIssue(); err != nil {
-               log.Error("LoadIssue: %v", err)
-               return ""
+               return "", fmt.Errorf("LoadIssue: %v", err)
        }
        if err := pr.LoadBaseRepo(); err != nil {
-               log.Error("LoadBaseRepo: %v", err)
-               return ""
+               return "", fmt.Errorf("LoadBaseRepo: %v", err)
        }
        if pr.BaseRepo.UnitEnabled(unit.TypeExternalTracker) {
-               return fmt.Sprintf("%s (!%d)", pr.Issue.Title, pr.Issue.Index)
+               return fmt.Sprintf("%s (!%d)", pr.Issue.Title, pr.Issue.Index), nil
        }
-       return fmt.Sprintf("%s (#%d)", pr.Issue.Title, pr.Issue.Index)
+       return fmt.Sprintf("%s (#%d)", pr.Issue.Title, pr.Issue.Index), nil
 }
 
 // GetGitRefName returns git ref for hidden pull request branch
index 2567984cc1331662d3d36ebbb15b2ed86d253961..9098b611617a63407ed57079053c797e18eff25b 100644 (file)
@@ -261,11 +261,15 @@ func TestPullRequest_GetDefaultMergeMessage_InternalTracker(t *testing.T) {
        assert.NoError(t, unittest.PrepareTestDatabase())
        pr := unittest.AssertExistsAndLoadBean(t, &PullRequest{ID: 2}).(*PullRequest)
 
-       assert.Equal(t, "Merge pull request 'issue3' (#3) from branch2 into master", pr.GetDefaultMergeMessage())
+       msg, err := pr.GetDefaultMergeMessage()
+       assert.NoError(t, err)
+       assert.Equal(t, "Merge pull request 'issue3' (#3) from branch2 into master", msg)
 
        pr.BaseRepoID = 1
        pr.HeadRepoID = 2
-       assert.Equal(t, "Merge pull request 'issue3' (#3) from user2/repo1:branch2 into master", pr.GetDefaultMergeMessage())
+       msg, err = pr.GetDefaultMergeMessage()
+       assert.NoError(t, err)
+       assert.Equal(t, "Merge pull request 'issue3' (#3) from user2/repo1:branch2 into master", msg)
 }
 
 func TestPullRequest_GetDefaultMergeMessage_ExternalTracker(t *testing.T) {
@@ -283,9 +287,13 @@ func TestPullRequest_GetDefaultMergeMessage_ExternalTracker(t *testing.T) {
 
        pr := unittest.AssertExistsAndLoadBean(t, &PullRequest{ID: 2, BaseRepo: baseRepo}).(*PullRequest)
 
-       assert.Equal(t, "Merge pull request 'issue3' (!3) from branch2 into master", pr.GetDefaultMergeMessage())
+       msg, err := pr.GetDefaultMergeMessage()
+       assert.NoError(t, err)
+       assert.Equal(t, "Merge pull request 'issue3' (!3) from branch2 into master", msg)
 
        pr.BaseRepoID = 1
        pr.HeadRepoID = 2
-       assert.Equal(t, "Merge pull request 'issue3' (!3) from user2/repo1:branch2 into master", pr.GetDefaultMergeMessage())
+       msg, err = pr.GetDefaultMergeMessage()
+       assert.NoError(t, err)
+       assert.Equal(t, "Merge pull request 'issue3' (!3) from user2/repo1:branch2 into master", msg)
 }
index 65544b80b3d4ac9c165ae434064a810bd7d221cc..bb922ddb23bedf5addbfa0fda488613e88944a36 100644 (file)
@@ -723,13 +723,12 @@ func MergePullRequest(ctx *context.APIContext) {
                return
        }
 
-       if err = pr.LoadHeadRepo(); err != nil {
+       if err := pr.LoadHeadRepo(); err != nil {
                ctx.Error(http.StatusInternalServerError, "LoadHeadRepo", err)
                return
        }
 
-       err = pr.LoadIssue()
-       if err != nil {
+       if err := pr.LoadIssue(); err != nil {
                ctx.Error(http.StatusInternalServerError, "LoadIssue", err)
                return
        }
@@ -743,29 +742,33 @@ func MergePullRequest(ctx *context.APIContext) {
                }
        }
 
-       if pr.Issue.IsClosed {
-               ctx.NotFound()
-               return
-       }
+       manuallMerge := repo_model.MergeStyle(form.Do) == repo_model.MergeStyleManuallyMerged
+       force := form.ForceMerge != nil && *form.ForceMerge
 
-       allowedMerge, err := pull_service.IsUserAllowedToMerge(pr, ctx.Repo.Permission, ctx.Doer)
-       if err != nil {
-               ctx.Error(http.StatusInternalServerError, "IsUSerAllowedToMerge", err)
-               return
-       }
-       if !allowedMerge {
-               ctx.Error(http.StatusMethodNotAllowed, "Merge", "User not allowed to merge PR")
-               return
-       }
-
-       if pr.HasMerged {
-               ctx.Error(http.StatusMethodNotAllowed, "PR already merged", "")
+       if err := pull_service.CheckPullMergable(ctx, ctx.Doer, &ctx.Repo.Permission, pr, manuallMerge, force); err != nil {
+               if errors.Is(err, pull_service.ErrIsClosed) {
+                       ctx.NotFound()
+               } else if errors.Is(err, pull_service.ErrUserNotAllowedToMerge) {
+                       ctx.Error(http.StatusMethodNotAllowed, "Merge", "User not allowed to merge PR")
+               } else if errors.Is(err, pull_service.ErrHasMerged) {
+                       ctx.Error(http.StatusMethodNotAllowed, "PR already merged", "")
+               } else if errors.Is(err, pull_service.ErrIsWorkInProgress) {
+                       ctx.Error(http.StatusMethodNotAllowed, "PR is a work in progress", "Work in progress PRs cannot be merged")
+               } else if errors.Is(err, pull_service.ErrNotMergableState) {
+                       ctx.Error(http.StatusMethodNotAllowed, "PR not in mergeable state", "Please try again later")
+               } else if models.IsErrDisallowedToMerge(err) {
+                       ctx.Error(http.StatusMethodNotAllowed, "PR is not ready to be merged", err)
+               } else if asymkey_service.IsErrWontSign(err) {
+                       ctx.Error(http.StatusMethodNotAllowed, fmt.Sprintf("Protected branch %s requires signed commits but this merge would not be signed", pr.BaseBranch), err)
+               } else {
+                       ctx.InternalServerError(err)
+               }
                return
        }
 
        // handle manually-merged mark
-       if repo_model.MergeStyle(form.Do) == repo_model.MergeStyleManuallyMerged {
-               if err = pull_service.MergedManually(pr, ctx.Doer, ctx.Repo.GitRepo, form.MergeCommitID); err != nil {
+       if manuallMerge {
+               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)))
                                return
@@ -781,63 +784,13 @@ func MergePullRequest(ctx *context.APIContext) {
                return
        }
 
-       if !pr.CanAutoMerge() {
-               ctx.Error(http.StatusMethodNotAllowed, "PR not in mergeable state", "Please try again later")
+       // set defaults to propagate needed fields
+       if err := form.SetDefaults(pr); err != nil {
+               ctx.ServerError("SetDefaults", fmt.Errorf("SetDefaults: %v", err))
                return
        }
 
-       if pr.IsWorkInProgress() {
-               ctx.Error(http.StatusMethodNotAllowed, "PR is a work in progress", "Work in progress PRs cannot be merged")
-               return
-       }
-
-       if err := pull_service.CheckPRReadyToMerge(ctx, pr, false); err != nil {
-               if !models.IsErrNotAllowedToMerge(err) {
-                       ctx.Error(http.StatusInternalServerError, "CheckPRReadyToMerge", err)
-                       return
-               }
-               if form.ForceMerge != nil && *form.ForceMerge {
-                       if isRepoAdmin, err := models.IsUserRepoAdmin(pr.BaseRepo, ctx.Doer); err != nil {
-                               ctx.Error(http.StatusInternalServerError, "IsUserRepoAdmin", err)
-                               return
-                       } else if !isRepoAdmin {
-                               ctx.Error(http.StatusMethodNotAllowed, "Merge", "Only repository admin can merge if not all checks are ok (force merge)")
-                       }
-               } else {
-                       ctx.Error(http.StatusMethodNotAllowed, "PR is not ready to be merged", err)
-                       return
-               }
-       }
-
-       if _, err := pull_service.IsSignedIfRequired(ctx, pr, ctx.Doer); err != nil {
-               if !asymkey_service.IsErrWontSign(err) {
-                       ctx.Error(http.StatusInternalServerError, "IsSignedIfRequired", err)
-                       return
-               }
-               ctx.Error(http.StatusMethodNotAllowed, fmt.Sprintf("Protected branch %s requires signed commits but this merge would not be signed", pr.BaseBranch), err)
-               return
-       }
-
-       if len(form.Do) == 0 {
-               form.Do = string(repo_model.MergeStyleMerge)
-       }
-
-       message := strings.TrimSpace(form.MergeTitleField)
-       if len(message) == 0 {
-               if repo_model.MergeStyle(form.Do) == repo_model.MergeStyleMerge {
-                       message = pr.GetDefaultMergeMessage()
-               }
-               if repo_model.MergeStyle(form.Do) == repo_model.MergeStyleSquash {
-                       message = pr.GetDefaultSquashMessage()
-               }
-       }
-
-       form.MergeMessageField = strings.TrimSpace(form.MergeMessageField)
-       if len(form.MergeMessageField) > 0 {
-               message += "\n\n" + form.MergeMessageField
-       }
-
-       if err := pull_service.Merge(ctx, pr, ctx.Doer, ctx.Repo.GitRepo, repo_model.MergeStyle(form.Do), form.HeadCommitID, message); err != nil {
+       if err := pull_service.Merge(ctx, pr, ctx.Doer, ctx.Repo.GitRepo, repo_model.MergeStyle(form.Do), form.HeadCommitID, form.MergeTitleField); 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)))
                        return
index c6ea422287ede5c7f33a49ca688bbac503309db0..f3f876edcaf2856d8affe4fddfda70ea4e24afc5 100644 (file)
@@ -340,7 +340,7 @@ func preReceiveBranch(ctx *preReceiveContext, oldCommitID, newCommitID, refFullN
 
                // Check all status checks and reviews are ok
                if err := pull_service.CheckPRReadyToMerge(ctx, pr, true); err != nil {
-                       if models.IsErrNotAllowedToMerge(err) {
+                       if models.IsErrDisallowedToMerge(err) {
                                log.Warn("Forbidden: User %d is not allowed push to protected branch %s in %-v and pr #%d is not ready to be merged: %s", ctx.opts.UserID, branchName, repo, pr.Index, err.Error())
                                ctx.JSON(http.StatusForbidden, private.Response{
                                        Err: fmt.Sprintf("Not allowed to push to protected branch %s and pr #%d is not ready to be merged: %s", branchName, ctx.opts.PullRequestID, err.Error()),
index 7b24e5dabb6ed60c7290773f416178c24d1e239f..32d9a65cbdbb94bf39e56bd3a75d029201c1541b 100644 (file)
@@ -34,6 +34,7 @@ import (
        "code.gitea.io/gitea/modules/web"
        "code.gitea.io/gitea/modules/web/middleware"
        "code.gitea.io/gitea/routers/utils"
+       asymkey_service "code.gitea.io/gitea/services/asymkey"
        "code.gitea.io/gitea/services/forms"
        "code.gitea.io/gitea/services/gitdiff"
        pull_service "code.gitea.io/gitea/services/pull"
@@ -858,39 +859,53 @@ func MergePullRequest(ctx *context.Context) {
        if ctx.Written() {
                return
        }
-       if issue.IsClosed {
-               if issue.IsPull {
-                       ctx.Flash.Error(ctx.Tr("repo.pulls.is_closed"))
-                       ctx.Redirect(issue.Link())
-                       return
-               }
-               ctx.Flash.Error(ctx.Tr("repo.issues.closed_title"))
-               ctx.Redirect(issue.Link())
-               return
-       }
 
        pr := issue.PullRequest
+       pr.Issue = issue
+       pr.Issue.Repo = ctx.Repo.Repository
+       manuallMerge := repo_model.MergeStyle(form.Do) == repo_model.MergeStyleManuallyMerged
+       forceMerge := form.ForceMerge != nil && *form.ForceMerge
 
-       allowedMerge, err := pull_service.IsUserAllowedToMerge(pr, ctx.Repo.Permission, ctx.Doer)
-       if err != nil {
-               ctx.ServerError("IsUserAllowedToMerge", err)
-               return
-       }
-       if !allowedMerge {
-               ctx.Flash.Error(ctx.Tr("repo.pulls.update_not_allowed"))
-               ctx.Redirect(issue.Link())
-               return
-       }
+       if err := pull_service.CheckPullMergable(ctx, ctx.Doer, &ctx.Repo.Permission, pr, manuallMerge, forceMerge); err != nil {
+               if errors.Is(err, pull_service.ErrIsClosed) {
+                       if issue.IsPull {
+                               ctx.Flash.Error(ctx.Tr("repo.pulls.is_closed"))
+                               ctx.Redirect(issue.Link())
+                       } else {
+                               ctx.Flash.Error(ctx.Tr("repo.issues.closed_title"))
+                               ctx.Redirect(issue.Link())
+                       }
+               } else if errors.Is(err, pull_service.ErrUserNotAllowedToMerge) {
+                       ctx.Flash.Error(ctx.Tr("repo.pulls.update_not_allowed"))
+                       ctx.Redirect(issue.Link())
+               } else if errors.Is(err, pull_service.ErrHasMerged) {
+                       ctx.Flash.Error(ctx.Tr("repo.pulls.has_merged"))
+                       ctx.Redirect(issue.Link())
+               } else if errors.Is(err, pull_service.ErrIsWorkInProgress) {
+                       ctx.Flash.Error(ctx.Tr("repo.pulls.no_merge_wip"))
+                       ctx.Redirect(issue.Link())
+               } else if errors.Is(err, pull_service.ErrNotMergableState) {
+                       ctx.Flash.Error(ctx.Tr("repo.pulls.no_merge_not_ready"))
+                       ctx.Redirect(issue.Link())
+               } else if models.IsErrDisallowedToMerge(err) {
+                       ctx.Flash.Error(ctx.Tr("repo.pulls.no_merge_not_ready"))
+                       ctx.Redirect(issue.Link())
+               } else if asymkey_service.IsErrWontSign(err) {
+                       ctx.Flash.Error(err.Error()) // has not translation ...
+                       ctx.Redirect(issue.Link())
+               } else if errors.Is(err, pull_service.ErrDependenciesLeft) {
+                       ctx.Flash.Error(ctx.Tr("repo.issues.dependency.pr_close_blocked"))
+                       ctx.Redirect(issue.Link())
+               } else {
+                       ctx.ServerError("WebCheck", err)
+               }
 
-       if pr.HasMerged {
-               ctx.Flash.Error(ctx.Tr("repo.pulls.has_merged"))
-               ctx.Redirect(issue.Link())
                return
        }
 
        // handle manually-merged mark
-       if repo_model.MergeStyle(form.Do) == repo_model.MergeStyleManuallyMerged {
-               if err = pull_service.MergedManually(pr, ctx.Doer, ctx.Repo.GitRepo, form.MergeCommitID); err != nil {
+       if manuallMerge {
+               if err := pull_service.MergedManually(pr, ctx.Doer, ctx.Repo.GitRepo, form.MergeCommitID); err != nil {
                        if models.IsErrInvalidMergeStyle(err) {
                                ctx.Flash.Error(ctx.Tr("repo.pulls.invalid_merge_option"))
                                ctx.Redirect(issue.Link())
@@ -909,72 +924,13 @@ func MergePullRequest(ctx *context.Context) {
                return
        }
 
-       if !pr.CanAutoMerge() {
-               ctx.Flash.Error(ctx.Tr("repo.pulls.no_merge_not_ready"))
-               ctx.Redirect(issue.Link())
-               return
-       }
-
-       if pr.IsWorkInProgress() {
-               ctx.Flash.Error(ctx.Tr("repo.pulls.no_merge_wip"))
-               ctx.Redirect(issue.Link())
-               return
-       }
-
-       if err := pull_service.CheckPRReadyToMerge(ctx, pr, false); err != nil {
-               if !models.IsErrNotAllowedToMerge(err) {
-                       ctx.ServerError("Merge PR status", err)
-                       return
-               }
-               if isRepoAdmin, err := models.IsUserRepoAdmin(pr.BaseRepo, ctx.Doer); err != nil {
-                       ctx.ServerError("IsUserRepoAdmin", err)
-                       return
-               } else if !isRepoAdmin {
-                       ctx.Flash.Error(ctx.Tr("repo.pulls.no_merge_not_ready"))
-                       ctx.Redirect(issue.Link())
-                       return
-               }
-       }
-
-       if ctx.HasError() {
-               ctx.Flash.Error(ctx.Data["ErrorMsg"].(string))
-               ctx.Redirect(issue.Link())
-               return
-       }
-
-       message := strings.TrimSpace(form.MergeTitleField)
-       if len(message) == 0 {
-               if repo_model.MergeStyle(form.Do) == repo_model.MergeStyleMerge {
-                       message = pr.GetDefaultMergeMessage()
-               }
-               if repo_model.MergeStyle(form.Do) == repo_model.MergeStyleRebaseMerge {
-                       message = pr.GetDefaultMergeMessage()
-               }
-               if repo_model.MergeStyle(form.Do) == repo_model.MergeStyleSquash {
-                       message = pr.GetDefaultSquashMessage()
-               }
-       }
-
-       form.MergeMessageField = strings.TrimSpace(form.MergeMessageField)
-       if len(form.MergeMessageField) > 0 {
-               message += "\n\n" + form.MergeMessageField
-       }
-
-       pr.Issue = issue
-       pr.Issue.Repo = ctx.Repo.Repository
-
-       noDeps, err := models.IssueNoDependenciesLeft(issue)
-       if err != nil {
-               return
-       }
-
-       if !noDeps {
-               ctx.Flash.Error(ctx.Tr("repo.issues.dependency.pr_close_blocked"))
-               ctx.Redirect(issue.Link())
+       // set defaults to propagate needed fields
+       if err := form.SetDefaults(pr); err != nil {
+               ctx.ServerError("SetDefaults", fmt.Errorf("SetDefaults: %v", err))
                return
        }
 
-       if err = pull_service.Merge(ctx, pr, ctx.Doer, ctx.Repo.GitRepo, repo_model.MergeStyle(form.Do), form.HeadCommitID, message); err != nil {
+       if err := pull_service.Merge(ctx, pr, ctx.Doer, ctx.Repo.GitRepo, repo_model.MergeStyle(form.Do), form.HeadCommitID, form.MergeTitleField); err != nil {
                if models.IsErrInvalidMergeStyle(err) {
                        ctx.Flash.Error(ctx.Tr("repo.pulls.invalid_merge_option"))
                        ctx.Redirect(issue.Link())
index 33c7658640581fab4333477316420558df5a4451..80123e9af3228e6016062f1d49b25084802369a8 100644 (file)
@@ -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)"`
index f920688f5d9bbee17ee69fa4bc1d11135b2e1ff9..a4bbd3bb891bed3389c2efa9e5a5814531ce3621 100644 (file)
@@ -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) {
index 6108a7956e1d64b08063f02371f59e690568b6f3..fb18be27c74be7532758d643907982b762c086e5 100644 (file)
@@ -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",
                }
        }