From d8e06865e1bfddbad4d426f7215e8536e72efc6c Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Wed, 30 Mar 2022 21:53:18 +0200 Subject: [PATCH 01/11] move checks for branchprotection into own package --- routers/api/v1/repo/pull.go | 110 ++++++++-------------- routers/web/repo/pull.go | 143 ++++++++++++----------------- services/branchprotection/check.go | 71 ++++++++++++++ services/branchprotection/error.go | 71 ++++++++++++++ 4 files changed, 239 insertions(+), 156 deletions(-) create mode 100644 services/branchprotection/check.go create mode 100644 services/branchprotection/error.go diff --git a/routers/api/v1/repo/pull.go b/routers/api/v1/repo/pull.go index 65544b80b3d4a..180986df4f439 100644 --- a/routers/api/v1/repo/pull.go +++ b/routers/api/v1/repo/pull.go @@ -27,6 +27,7 @@ import ( "code.gitea.io/gitea/modules/web" "code.gitea.io/gitea/routers/api/v1/utils" asymkey_service "code.gitea.io/gitea/services/asymkey" + "code.gitea.io/gitea/services/branchprotection" "code.gitea.io/gitea/services/forms" issue_service "code.gitea.io/gitea/services/issue" pull_service "code.gitea.io/gitea/services/pull" @@ -723,13 +724,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 +743,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 := branchprotection.Check(ctx, ctx.Doer, &ctx.Repo.Permission, pr, manuallMerge, force); err != nil { + if branchprotection.IsErrIsClosed(err) { + ctx.NotFound() + } else if branchprotection.IsErrUserNotAllowedToMerge(err) { + ctx.Error(http.StatusMethodNotAllowed, "Merge", "User not allowed to merge PR") + } else if branchprotection.IsErrHasMerged(err) { + ctx.Error(http.StatusMethodNotAllowed, "PR already merged", "") + } else if branchprotection.IsErrIsWorkInProgress(err) { + ctx.Error(http.StatusMethodNotAllowed, "PR is a work in progress", "Work in progress PRs cannot be merged") + } else if branchprotection.IsErrNotMergableState(err) { + ctx.Error(http.StatusMethodNotAllowed, "PR not in mergeable state", "Please try again later") + } else if models.IsErrNotAllowedToMerge(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,62 +785,28 @@ func MergePullRequest(ctx *context.APIContext) { return } - if !pr.CanAutoMerge() { - ctx.Error(http.StatusMethodNotAllowed, "PR not in mergeable state", "Please try again later") - 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 + // TODO: move func to propergate defaults into own func + message := strings.TrimSpace(form.MergeTitleField) + { // Set defaults if not given + if len(form.Do) == 0 { + form.Do = string(repo_model.MergeStyleMerge) } - } - if _, err := pull_service.IsSignedIfRequired(ctx, pr, ctx.Doer); err != nil { - if !asymkey_service.IsErrWontSign(err) { - ctx.Error(http.StatusInternalServerError, "IsSignedIfRequired", err) - return + 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() + } } - 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 } } - 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 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))) diff --git a/routers/web/repo/pull.go b/routers/web/repo/pull.go index 7b24e5dabb6ed..5f8e530437b38 100644 --- a/routers/web/repo/pull.go +++ b/routers/web/repo/pull.go @@ -34,6 +34,8 @@ 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/branchprotection" "code.gitea.io/gitea/services/forms" "code.gitea.io/gitea/services/gitdiff" pull_service "code.gitea.io/gitea/services/pull" @@ -858,39 +860,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 := branchprotection.Check(ctx, ctx.Doer, &ctx.Repo.Permission, pr, manuallMerge, forceMerge); err != nil { + if branchprotection.IsErrIsClosed(err) { + 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 branchprotection.IsErrUserNotAllowedToMerge(err) { + ctx.Flash.Error(ctx.Tr("repo.pulls.update_not_allowed")) + ctx.Redirect(issue.Link()) + } else if branchprotection.IsErrHasMerged(err) { + ctx.Flash.Error(ctx.Tr("repo.pulls.has_merged")) + ctx.Redirect(issue.Link()) + } else if branchprotection.IsErrIsWorkInProgress(err) { + ctx.Flash.Error(ctx.Tr("repo.pulls.no_merge_wip")) + ctx.Redirect(issue.Link()) + } else if branchprotection.IsErrNotMergableState(err) { + ctx.Flash.Error(ctx.Tr("repo.pulls.no_merge_not_ready")) + ctx.Redirect(issue.Link()) + } else if models.IsErrNotAllowedToMerge(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 branchprotection.IsErrDependenciesLeft(err) { + 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 +925,27 @@ 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 - } - + // TODO: move func to propergate defaults into own func 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() + { // Set defaults if not given + 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() + } } - 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 } } - 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()) - 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, message); err != nil { if models.IsErrInvalidMergeStyle(err) { ctx.Flash.Error(ctx.Tr("repo.pulls.invalid_merge_option")) ctx.Redirect(issue.Link()) diff --git a/services/branchprotection/check.go b/services/branchprotection/check.go new file mode 100644 index 0000000000000..4ce5839d9e948 --- /dev/null +++ b/services/branchprotection/check.go @@ -0,0 +1,71 @@ +// Copyright 2022 The Gitea Authors. All rights reserved. +// Use of this source code is governed by a MIT-style +// license that can be found in the LICENSE file. + +package branchprotection + +import ( + "context" + + "code.gitea.io/gitea/models" + user_model "code.gitea.io/gitea/models/user" + pull_service "code.gitea.io/gitea/services/pull" +) + +// Check if pull is mergable +func Check(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 := pull_service.IsUserAllowedToMerge(pr, *perm, doer); err != nil { + return err + } else if !allowedMerge { + return ErrUserNotAllowedToMerge{} + } + + if manuallMerge { + // dont check rules to "auto merge", doer is goint to mark this pull as merged manually + return nil + } + + if pr.IsWorkInProgress() { + return ErrIsWorkInProgress{} + } + + if !pr.CanAutoMerge() { + return ErrNotMergableState{} + } + + if err := pull_service.CheckPRReadyToMerge(ctx, pr, false); err != nil { + if models.IsErrNotAllowedToMerge(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 := pull_service.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 +} diff --git a/services/branchprotection/error.go b/services/branchprotection/error.go new file mode 100644 index 0000000000000..0738b76c361f7 --- /dev/null +++ b/services/branchprotection/error.go @@ -0,0 +1,71 @@ +// Copyright 2022 The Gitea Authors. All rights reserved. +// Use of this source code is governed by a MIT-style +// license that can be found in the LICENSE file. + +package branchprotection + +type ErrIsClosed struct{} + +func IsErrIsClosed(err error) bool { + _, ok := err.(ErrIsClosed) + return ok +} + +func (err ErrIsClosed) Error() string { + return "pull is cosed" +} + +type ErrUserNotAllowedToMerge struct{} + +func IsErrUserNotAllowedToMerge(err error) bool { + _, ok := err.(ErrUserNotAllowedToMerge) + return ok +} + +func (err ErrUserNotAllowedToMerge) Error() string { + return "user not allowed to merge" +} + +type ErrHasMerged struct{} + +func IsErrHasMerged(err error) bool { + _, ok := err.(ErrHasMerged) + return ok +} + +func (err ErrHasMerged) Error() string { + return "has already been merged" +} + +type ErrIsWorkInProgress struct{} + +func IsErrIsWorkInProgress(err error) bool { + _, ok := err.(ErrIsWorkInProgress) + return ok +} + +func (err ErrIsWorkInProgress) Error() string { + return "work in progress PRs cannot be merged" +} + +type ErrNotMergableState struct{} + +func IsErrNotMergableState(err error) bool { + _, ok := err.(ErrNotMergableState) + return ok +} + +func (err ErrNotMergableState) Error() string { + return "not in mergeable state" +} + +type ErrDependenciesLeft struct{} + +func IsErrDependenciesLeft(err error) bool { + _, ok := err.(ErrDependenciesLeft) + return ok +} + +func (err ErrDependenciesLeft) Error() string { + return "is blocked by an open dependency" +} From 6f15ec8e704e5735ffbd6c7f9ff857e6cfaf1d5c Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Wed, 30 Mar 2022 22:35:14 +0200 Subject: [PATCH 02/11] resolve todo now ... --- routers/api/v1/repo/pull.go | 25 +++---------------------- routers/web/repo/pull.go | 23 +++-------------------- services/forms/repo_form.go | 26 ++++++++++++++++++++++++++ 3 files changed, 32 insertions(+), 42 deletions(-) diff --git a/routers/api/v1/repo/pull.go b/routers/api/v1/repo/pull.go index 180986df4f439..34e9ac12558ae 100644 --- a/routers/api/v1/repo/pull.go +++ b/routers/api/v1/repo/pull.go @@ -785,29 +785,10 @@ func MergePullRequest(ctx *context.APIContext) { return } - // TODO: move func to propergate defaults into own func - message := strings.TrimSpace(form.MergeTitleField) - { // Set defaults if not given - if len(form.Do) == 0 { - form.Do = string(repo_model.MergeStyleMerge) - } - - 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 - } - } + // set defaults to propagate needed fields + form.SetDefaults(pr) - 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 diff --git a/routers/web/repo/pull.go b/routers/web/repo/pull.go index 5f8e530437b38..2451dc54b1e3b 100644 --- a/routers/web/repo/pull.go +++ b/routers/web/repo/pull.go @@ -925,27 +925,10 @@ func MergePullRequest(ctx *context.Context) { return } - // TODO: move func to propergate defaults into own func - message := strings.TrimSpace(form.MergeTitleField) - { // Set defaults if not given - 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 - } - } + // set defaults to propagate needed fields + form.SetDefaults(pr) - 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()) diff --git a/services/forms/repo_form.go b/services/forms/repo_form.go index 33c7658640581..04e2cfe69ffc4 100644 --- a/services/forms/repo_form.go +++ b/services/forms/repo_form.go @@ -599,6 +599,32 @@ func (f *MergePullRequestForm) Validate(req *http.Request, errs binding.Errors) return middleware.Validate(errs, ctx.Data, f, ctx.Locale) } +// SetDefaults for options who n +func (f *MergePullRequestForm) SetDefaults(pr *models.PullRequest) { + if len(f.Do) == 0 { + f.Do = "merge" + } + + f.MergeTitleField = strings.TrimSpace(f.MergeTitleField) + if len(f.MergeTitleField) == 0 { + switch f.Do { + case "merge": + f.MergeTitleField = pr.GetDefaultMergeMessage() + case "rebase-merge": + f.MergeTitleField = pr.GetDefaultMergeMessage() + case "squash": + f.MergeTitleField = pr.GetDefaultSquashMessage() + + } + } + + f.MergeMessageField = strings.TrimSpace(f.MergeMessageField) + if len(f.MergeMessageField) > 0 { + f.MergeTitleField += "\n\n" + f.MergeMessageField + f.MergeMessageField = "" + } +} + // CodeCommentForm form for adding code comments for PRs type CodeCommentForm struct { Origin string `binding:"Required;In(timeline,diff)"` From f17abd1e6963ed3293ba0c6723601b2b755bb7e2 Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Wed, 30 Mar 2022 23:04:26 +0200 Subject: [PATCH 03/11] move & rename --- routers/api/v1/repo/pull.go | 13 ++++++------- routers/web/repo/pull.go | 15 +++++++-------- .../check.go => pull/protection_check.go} | 13 ++++++------- .../error.go => pull/protection_error.go} | 2 +- 4 files changed, 20 insertions(+), 23 deletions(-) rename services/{branchprotection/check.go => pull/protection_check.go} (71%) rename services/{branchprotection/error.go => pull/protection_error.go} (98%) diff --git a/routers/api/v1/repo/pull.go b/routers/api/v1/repo/pull.go index 34e9ac12558ae..344b2af7f008e 100644 --- a/routers/api/v1/repo/pull.go +++ b/routers/api/v1/repo/pull.go @@ -27,7 +27,6 @@ import ( "code.gitea.io/gitea/modules/web" "code.gitea.io/gitea/routers/api/v1/utils" asymkey_service "code.gitea.io/gitea/services/asymkey" - "code.gitea.io/gitea/services/branchprotection" "code.gitea.io/gitea/services/forms" issue_service "code.gitea.io/gitea/services/issue" pull_service "code.gitea.io/gitea/services/pull" @@ -746,16 +745,16 @@ func MergePullRequest(ctx *context.APIContext) { manuallMerge := repo_model.MergeStyle(form.Do) == repo_model.MergeStyleManuallyMerged force := form.ForceMerge != nil && *form.ForceMerge - if err := branchprotection.Check(ctx, ctx.Doer, &ctx.Repo.Permission, pr, manuallMerge, force); err != nil { - if branchprotection.IsErrIsClosed(err) { + if err := pull_service.CheckPullProtection(ctx, ctx.Doer, &ctx.Repo.Permission, pr, manuallMerge, force); err != nil { + if pull_service.IsErrIsClosed(err) { ctx.NotFound() - } else if branchprotection.IsErrUserNotAllowedToMerge(err) { + } else if pull_service.IsErrUserNotAllowedToMerge(err) { ctx.Error(http.StatusMethodNotAllowed, "Merge", "User not allowed to merge PR") - } else if branchprotection.IsErrHasMerged(err) { + } else if pull_service.IsErrHasMerged(err) { ctx.Error(http.StatusMethodNotAllowed, "PR already merged", "") - } else if branchprotection.IsErrIsWorkInProgress(err) { + } else if pull_service.IsErrIsWorkInProgress(err) { ctx.Error(http.StatusMethodNotAllowed, "PR is a work in progress", "Work in progress PRs cannot be merged") - } else if branchprotection.IsErrNotMergableState(err) { + } else if pull_service.IsErrNotMergableState(err) { ctx.Error(http.StatusMethodNotAllowed, "PR not in mergeable state", "Please try again later") } else if models.IsErrNotAllowedToMerge(err) { ctx.Error(http.StatusMethodNotAllowed, "PR is not ready to be merged", err) diff --git a/routers/web/repo/pull.go b/routers/web/repo/pull.go index 2451dc54b1e3b..09976a368e34d 100644 --- a/routers/web/repo/pull.go +++ b/routers/web/repo/pull.go @@ -35,7 +35,6 @@ import ( "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/branchprotection" "code.gitea.io/gitea/services/forms" "code.gitea.io/gitea/services/gitdiff" pull_service "code.gitea.io/gitea/services/pull" @@ -867,8 +866,8 @@ func MergePullRequest(ctx *context.Context) { manuallMerge := repo_model.MergeStyle(form.Do) == repo_model.MergeStyleManuallyMerged forceMerge := form.ForceMerge != nil && *form.ForceMerge - if err := branchprotection.Check(ctx, ctx.Doer, &ctx.Repo.Permission, pr, manuallMerge, forceMerge); err != nil { - if branchprotection.IsErrIsClosed(err) { + if err := pull_service.CheckPullProtection(ctx, ctx.Doer, &ctx.Repo.Permission, pr, manuallMerge, forceMerge); err != nil { + if pull_service.IsErrIsClosed(err) { if issue.IsPull { ctx.Flash.Error(ctx.Tr("repo.pulls.is_closed")) ctx.Redirect(issue.Link()) @@ -876,16 +875,16 @@ func MergePullRequest(ctx *context.Context) { ctx.Flash.Error(ctx.Tr("repo.issues.closed_title")) ctx.Redirect(issue.Link()) } - } else if branchprotection.IsErrUserNotAllowedToMerge(err) { + } else if pull_service.IsErrUserNotAllowedToMerge(err) { ctx.Flash.Error(ctx.Tr("repo.pulls.update_not_allowed")) ctx.Redirect(issue.Link()) - } else if branchprotection.IsErrHasMerged(err) { + } else if pull_service.IsErrHasMerged(err) { ctx.Flash.Error(ctx.Tr("repo.pulls.has_merged")) ctx.Redirect(issue.Link()) - } else if branchprotection.IsErrIsWorkInProgress(err) { + } else if pull_service.IsErrIsWorkInProgress(err) { ctx.Flash.Error(ctx.Tr("repo.pulls.no_merge_wip")) ctx.Redirect(issue.Link()) - } else if branchprotection.IsErrNotMergableState(err) { + } else if pull_service.IsErrNotMergableState(err) { ctx.Flash.Error(ctx.Tr("repo.pulls.no_merge_not_ready")) ctx.Redirect(issue.Link()) } else if models.IsErrNotAllowedToMerge(err) { @@ -894,7 +893,7 @@ func MergePullRequest(ctx *context.Context) { } else if asymkey_service.IsErrWontSign(err) { ctx.Flash.Error(err.Error()) // has not translation ... ctx.Redirect(issue.Link()) - } else if branchprotection.IsErrDependenciesLeft(err) { + } else if pull_service.IsErrDependenciesLeft(err) { ctx.Flash.Error(ctx.Tr("repo.issues.dependency.pr_close_blocked")) ctx.Redirect(issue.Link()) } else { diff --git a/services/branchprotection/check.go b/services/pull/protection_check.go similarity index 71% rename from services/branchprotection/check.go rename to services/pull/protection_check.go index 4ce5839d9e948..0722e726707be 100644 --- a/services/branchprotection/check.go +++ b/services/pull/protection_check.go @@ -2,18 +2,17 @@ // Use of this source code is governed by a MIT-style // license that can be found in the LICENSE file. -package branchprotection +package pull import ( "context" "code.gitea.io/gitea/models" user_model "code.gitea.io/gitea/models/user" - pull_service "code.gitea.io/gitea/services/pull" ) -// Check if pull is mergable -func Check(ctx context.Context, doer *user_model.User, perm *models.Permission, pr *models.PullRequest, manuallMerge, force bool) error { +// CheckPullProtection check if the pull mergable based on all conditions (branch protection, merge options, ...) +func CheckPullProtection(ctx context.Context, doer *user_model.User, perm *models.Permission, pr *models.PullRequest, manuallMerge, force bool) error { if pr.HasMerged { return ErrHasMerged{} } @@ -24,7 +23,7 @@ func Check(ctx context.Context, doer *user_model.User, perm *models.Permission, return ErrIsClosed{} } - if allowedMerge, err := pull_service.IsUserAllowedToMerge(pr, *perm, doer); err != nil { + if allowedMerge, err := IsUserAllowedToMerge(pr, *perm, doer); err != nil { return err } else if !allowedMerge { return ErrUserNotAllowedToMerge{} @@ -43,7 +42,7 @@ func Check(ctx context.Context, doer *user_model.User, perm *models.Permission, return ErrNotMergableState{} } - if err := pull_service.CheckPRReadyToMerge(ctx, pr, false); err != nil { + if err := CheckPRReadyToMerge(ctx, pr, false); err != nil { if models.IsErrNotAllowedToMerge(err) { if force { if isRepoAdmin, err := models.IsUserRepoAdmin(pr.BaseRepo, doer); err != nil { @@ -57,7 +56,7 @@ func Check(ctx context.Context, doer *user_model.User, perm *models.Permission, } } - if _, err := pull_service.IsSignedIfRequired(ctx, pr, doer); err != nil { + if _, err := IsSignedIfRequired(ctx, pr, doer); err != nil { return err } diff --git a/services/branchprotection/error.go b/services/pull/protection_error.go similarity index 98% rename from services/branchprotection/error.go rename to services/pull/protection_error.go index 0738b76c361f7..a4cd4f0296bab 100644 --- a/services/branchprotection/error.go +++ b/services/pull/protection_error.go @@ -2,7 +2,7 @@ // Use of this source code is governed by a MIT-style // license that can be found in the LICENSE file. -package branchprotection +package pull type ErrIsClosed struct{} From 2a66e96a8a1810f9b28b42c31f908c1d9e8dc536 Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Wed, 30 Mar 2022 23:08:00 +0200 Subject: [PATCH 04/11] unexport if posible --- services/pull/merge.go | 15 --------------- services/pull/protection_check.go | 18 +++++++++++++++++- 2 files changed, 17 insertions(+), 16 deletions(-) diff --git a/services/pull/merge.go b/services/pull/merge.go index 6108a7956e1d6..a1ac18789d9be 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 { diff --git a/services/pull/protection_check.go b/services/pull/protection_check.go index 0722e726707be..f846e1476fc8a 100644 --- a/services/pull/protection_check.go +++ b/services/pull/protection_check.go @@ -9,6 +9,7 @@ import ( "code.gitea.io/gitea/models" user_model "code.gitea.io/gitea/models/user" + asymkey_service "code.gitea.io/gitea/services/asymkey" ) // CheckPullProtection check if the pull mergable based on all conditions (branch protection, merge options, ...) @@ -56,7 +57,7 @@ func CheckPullProtection(ctx context.Context, doer *user_model.User, perm *model } } - if _, err := IsSignedIfRequired(ctx, pr, doer); err != nil { + if _, err := isSignedIfRequired(ctx, pr, doer); err != nil { return err } @@ -68,3 +69,18 @@ func CheckPullProtection(ctx context.Context, doer *user_model.User, perm *model 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 +} From 52cb7c463049c606a5806758d074a311fecd517c Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Thu, 31 Mar 2022 04:11:25 +0200 Subject: [PATCH 05/11] wording --- services/pull/protection_check.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/pull/protection_check.go b/services/pull/protection_check.go index f846e1476fc8a..60d6ce714aa3c 100644 --- a/services/pull/protection_check.go +++ b/services/pull/protection_check.go @@ -31,7 +31,7 @@ func CheckPullProtection(ctx context.Context, doer *user_model.User, perm *model } if manuallMerge { - // dont check rules to "auto merge", doer is goint to mark this pull as merged manually + // don't check rules to "auto merge", doer is going to mark this pull as merged manually return nil } From c0e932ab3642937c5f2d459ecb1d82e42e5f61d8 Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Thu, 31 Mar 2022 12:02:43 +0200 Subject: [PATCH 06/11] improve SetDefaults & nits --- models/pull.go | 27 +++++++++++---------------- models/pull_test.go | 12 +++++++++--- routers/api/v1/repo/pull.go | 5 ++++- routers/web/repo/pull.go | 5 ++++- services/forms/repo_form.go | 17 ++++++++--------- 5 files changed, 36 insertions(+), 30 deletions(-) diff --git a/models/pull.go b/models/pull.go index ec1fde02595b0..6abd9f04b24f8 100644 --- a/models/pull.go +++ b/models/pull.go @@ -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 diff --git a/models/pull_test.go b/models/pull_test.go index 2567984cc1331..195de3e5cbdb0 100644 --- a/models/pull_test.go +++ b/models/pull_test.go @@ -261,7 +261,9 @@ 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 @@ -283,9 +285,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) } diff --git a/routers/api/v1/repo/pull.go b/routers/api/v1/repo/pull.go index 344b2af7f008e..3269c36f7b83c 100644 --- a/routers/api/v1/repo/pull.go +++ b/routers/api/v1/repo/pull.go @@ -785,7 +785,10 @@ func MergePullRequest(ctx *context.APIContext) { } // set defaults to propagate needed fields - form.SetDefaults(pr) + 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, form.MergeTitleField); err != nil { if models.IsErrInvalidMergeStyle(err) { diff --git a/routers/web/repo/pull.go b/routers/web/repo/pull.go index 09976a368e34d..53fb1454d9bc0 100644 --- a/routers/web/repo/pull.go +++ b/routers/web/repo/pull.go @@ -925,7 +925,10 @@ func MergePullRequest(ctx *context.Context) { } // set defaults to propagate needed fields - form.SetDefaults(pr) + 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, form.MergeTitleField); err != nil { if models.IsErrInvalidMergeStyle(err) { diff --git a/services/forms/repo_form.go b/services/forms/repo_form.go index 04e2cfe69ffc4..80123e9af3228 100644 --- a/services/forms/repo_form.go +++ b/services/forms/repo_form.go @@ -599,22 +599,19 @@ func (f *MergePullRequestForm) Validate(req *http.Request, errs binding.Errors) return middleware.Validate(errs, ctx.Data, f, ctx.Locale) } -// SetDefaults for options who n -func (f *MergePullRequestForm) SetDefaults(pr *models.PullRequest) { - if len(f.Do) == 0 { +// 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": - f.MergeTitleField = pr.GetDefaultMergeMessage() - case "rebase-merge": - f.MergeTitleField = pr.GetDefaultMergeMessage() + case "merge", "rebase-merge": + f.MergeTitleField, err = pr.GetDefaultMergeMessage() case "squash": - f.MergeTitleField = pr.GetDefaultSquashMessage() - + f.MergeTitleField, err = pr.GetDefaultSquashMessage() } } @@ -623,6 +620,8 @@ func (f *MergePullRequestForm) SetDefaults(pr *models.PullRequest) { f.MergeTitleField += "\n\n" + f.MergeMessageField f.MergeMessageField = "" } + + return } // CodeCommentForm form for adding code comments for PRs From 6959e2734fc6c7f70f18ccc1a47839e336a2e712 Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Thu, 31 Mar 2022 12:05:53 +0200 Subject: [PATCH 07/11] NotAllowedToMerge -> DisallowedToMerge --- models/error.go | 12 ++++++------ routers/api/v1/repo/pull.go | 2 +- routers/private/hook_pre_receive.go | 2 +- routers/web/repo/pull.go | 2 +- services/pull/merge.go | 12 ++++++------ services/pull/protection_check.go | 2 +- 6 files changed, 16 insertions(+), 16 deletions(-) diff --git a/models/error.go b/models/error.go index fbd2f971857a4..6233b2ea853d4 100644 --- a/models/error.go +++ b/models/error.go @@ -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) } diff --git a/routers/api/v1/repo/pull.go b/routers/api/v1/repo/pull.go index 3269c36f7b83c..a363c65ea311a 100644 --- a/routers/api/v1/repo/pull.go +++ b/routers/api/v1/repo/pull.go @@ -756,7 +756,7 @@ func MergePullRequest(ctx *context.APIContext) { ctx.Error(http.StatusMethodNotAllowed, "PR is a work in progress", "Work in progress PRs cannot be merged") } else if pull_service.IsErrNotMergableState(err) { ctx.Error(http.StatusMethodNotAllowed, "PR not in mergeable state", "Please try again later") - } else if models.IsErrNotAllowedToMerge(err) { + } 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) diff --git a/routers/private/hook_pre_receive.go b/routers/private/hook_pre_receive.go index c6ea422287ede..f3f876edcaf28 100644 --- a/routers/private/hook_pre_receive.go +++ b/routers/private/hook_pre_receive.go @@ -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()), diff --git a/routers/web/repo/pull.go b/routers/web/repo/pull.go index 53fb1454d9bc0..fb5e96089785b 100644 --- a/routers/web/repo/pull.go +++ b/routers/web/repo/pull.go @@ -887,7 +887,7 @@ func MergePullRequest(ctx *context.Context) { } else if pull_service.IsErrNotMergableState(err) { ctx.Flash.Error(ctx.Tr("repo.pulls.no_merge_not_ready")) ctx.Redirect(issue.Link()) - } else if models.IsErrNotAllowedToMerge(err) { + } 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) { diff --git a/services/pull/merge.go b/services/pull/merge.go index a1ac18789d9be..fb18be27c74be 100644 --- a/services/pull/merge.go +++ b/services/pull/merge.go @@ -696,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", } } @@ -728,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", } } diff --git a/services/pull/protection_check.go b/services/pull/protection_check.go index 60d6ce714aa3c..fce8c2122006c 100644 --- a/services/pull/protection_check.go +++ b/services/pull/protection_check.go @@ -44,7 +44,7 @@ func CheckPullProtection(ctx context.Context, doer *user_model.User, perm *model } if err := CheckPRReadyToMerge(ctx, pr, false); err != nil { - if models.IsErrNotAllowedToMerge(err) { + if models.IsErrDisallowedToMerge(err) { if force { if isRepoAdmin, err := models.IsUserRepoAdmin(pr.BaseRepo, doer); err != nil { return err From 4775c3ab837aeba286fd9f3d5b25986597b15337 Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Thu, 31 Mar 2022 12:33:37 +0200 Subject: [PATCH 08/11] fix test --- models/pull_test.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/models/pull_test.go b/models/pull_test.go index 195de3e5cbdb0..9098b611617a6 100644 --- a/models/pull_test.go +++ b/models/pull_test.go @@ -267,7 +267,9 @@ func TestPullRequest_GetDefaultMergeMessage_InternalTracker(t *testing.T) { 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) { From 943d1f25864d4adf43f2ad2303df774b14655449 Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Thu, 31 Mar 2022 15:28:16 +0200 Subject: [PATCH 09/11] merge files --- routers/api/v1/repo/pull.go | 2 +- routers/web/repo/pull.go | 2 +- services/pull/protection_check.go | 70 +++++++++++++++++++++++++++++- services/pull/protection_error.go | 71 ------------------------------- 4 files changed, 70 insertions(+), 75 deletions(-) delete mode 100644 services/pull/protection_error.go diff --git a/routers/api/v1/repo/pull.go b/routers/api/v1/repo/pull.go index a363c65ea311a..5a84a7b3fbefe 100644 --- a/routers/api/v1/repo/pull.go +++ b/routers/api/v1/repo/pull.go @@ -745,7 +745,7 @@ func MergePullRequest(ctx *context.APIContext) { manuallMerge := repo_model.MergeStyle(form.Do) == repo_model.MergeStyleManuallyMerged force := form.ForceMerge != nil && *form.ForceMerge - if err := pull_service.CheckPullProtection(ctx, ctx.Doer, &ctx.Repo.Permission, pr, manuallMerge, force); err != nil { + if err := pull_service.CheckPullMergable(ctx, ctx.Doer, &ctx.Repo.Permission, pr, manuallMerge, force); err != nil { if pull_service.IsErrIsClosed(err) { ctx.NotFound() } else if pull_service.IsErrUserNotAllowedToMerge(err) { diff --git a/routers/web/repo/pull.go b/routers/web/repo/pull.go index fb5e96089785b..1cbbc3c960a16 100644 --- a/routers/web/repo/pull.go +++ b/routers/web/repo/pull.go @@ -866,7 +866,7 @@ func MergePullRequest(ctx *context.Context) { manuallMerge := repo_model.MergeStyle(form.Do) == repo_model.MergeStyleManuallyMerged forceMerge := form.ForceMerge != nil && *form.ForceMerge - if err := pull_service.CheckPullProtection(ctx, ctx.Doer, &ctx.Repo.Permission, pr, manuallMerge, forceMerge); err != nil { + if err := pull_service.CheckPullMergable(ctx, ctx.Doer, &ctx.Repo.Permission, pr, manuallMerge, forceMerge); err != nil { if pull_service.IsErrIsClosed(err) { if issue.IsPull { ctx.Flash.Error(ctx.Tr("repo.pulls.is_closed")) diff --git a/services/pull/protection_check.go b/services/pull/protection_check.go index fce8c2122006c..a742c7952afc1 100644 --- a/services/pull/protection_check.go +++ b/services/pull/protection_check.go @@ -12,8 +12,74 @@ import ( asymkey_service "code.gitea.io/gitea/services/asymkey" ) -// CheckPullProtection check if the pull mergable based on all conditions (branch protection, merge options, ...) -func CheckPullProtection(ctx context.Context, doer *user_model.User, perm *models.Permission, pr *models.PullRequest, manuallMerge, force bool) error { +type ErrIsClosed struct{} + +func IsErrIsClosed(err error) bool { + _, ok := err.(ErrIsClosed) + return ok +} + +func (err ErrIsClosed) Error() string { + return "pull is cosed" +} + +type ErrUserNotAllowedToMerge struct{} + +func IsErrUserNotAllowedToMerge(err error) bool { + _, ok := err.(ErrUserNotAllowedToMerge) + return ok +} + +func (err ErrUserNotAllowedToMerge) Error() string { + return "user not allowed to merge" +} + +type ErrHasMerged struct{} + +func IsErrHasMerged(err error) bool { + _, ok := err.(ErrHasMerged) + return ok +} + +func (err ErrHasMerged) Error() string { + return "has already been merged" +} + +type ErrIsWorkInProgress struct{} + +func IsErrIsWorkInProgress(err error) bool { + _, ok := err.(ErrIsWorkInProgress) + return ok +} + +func (err ErrIsWorkInProgress) Error() string { + return "work in progress PRs cannot be merged" +} + +type ErrNotMergableState struct{} + +func IsErrNotMergableState(err error) bool { + _, ok := err.(ErrNotMergableState) + return ok +} + +func (err ErrNotMergableState) Error() string { + return "not in mergeable state" +} + +type ErrDependenciesLeft struct{} + +func IsErrDependenciesLeft(err error) bool { + _, ok := err.(ErrDependenciesLeft) + return ok +} + +func (err ErrDependenciesLeft) Error() string { + return "is blocked by an open dependency" +} + +// 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{} } diff --git a/services/pull/protection_error.go b/services/pull/protection_error.go deleted file mode 100644 index a4cd4f0296bab..0000000000000 --- a/services/pull/protection_error.go +++ /dev/null @@ -1,71 +0,0 @@ -// Copyright 2022 The Gitea Authors. All rights reserved. -// Use of this source code is governed by a MIT-style -// license that can be found in the LICENSE file. - -package pull - -type ErrIsClosed struct{} - -func IsErrIsClosed(err error) bool { - _, ok := err.(ErrIsClosed) - return ok -} - -func (err ErrIsClosed) Error() string { - return "pull is cosed" -} - -type ErrUserNotAllowedToMerge struct{} - -func IsErrUserNotAllowedToMerge(err error) bool { - _, ok := err.(ErrUserNotAllowedToMerge) - return ok -} - -func (err ErrUserNotAllowedToMerge) Error() string { - return "user not allowed to merge" -} - -type ErrHasMerged struct{} - -func IsErrHasMerged(err error) bool { - _, ok := err.(ErrHasMerged) - return ok -} - -func (err ErrHasMerged) Error() string { - return "has already been merged" -} - -type ErrIsWorkInProgress struct{} - -func IsErrIsWorkInProgress(err error) bool { - _, ok := err.(ErrIsWorkInProgress) - return ok -} - -func (err ErrIsWorkInProgress) Error() string { - return "work in progress PRs cannot be merged" -} - -type ErrNotMergableState struct{} - -func IsErrNotMergableState(err error) bool { - _, ok := err.(ErrNotMergableState) - return ok -} - -func (err ErrNotMergableState) Error() string { - return "not in mergeable state" -} - -type ErrDependenciesLeft struct{} - -func IsErrDependenciesLeft(err error) bool { - _, ok := err.(ErrDependenciesLeft) - return ok -} - -func (err ErrDependenciesLeft) Error() string { - return "is blocked by an open dependency" -} From 120d84dab969742e8101484ec6267f3698a495b2 Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Thu, 31 Mar 2022 16:07:33 +0200 Subject: [PATCH 10/11] use package "errors" --- routers/api/v1/repo/pull.go | 10 ++-- routers/web/repo/pull.go | 12 ++--- services/pull/protection_check.go | 88 ++++++------------------------- 3 files changed, 27 insertions(+), 83 deletions(-) diff --git a/routers/api/v1/repo/pull.go b/routers/api/v1/repo/pull.go index 5a84a7b3fbefe..bb922ddb23bed 100644 --- a/routers/api/v1/repo/pull.go +++ b/routers/api/v1/repo/pull.go @@ -746,15 +746,15 @@ func MergePullRequest(ctx *context.APIContext) { force := form.ForceMerge != nil && *form.ForceMerge if err := pull_service.CheckPullMergable(ctx, ctx.Doer, &ctx.Repo.Permission, pr, manuallMerge, force); err != nil { - if pull_service.IsErrIsClosed(err) { + if errors.Is(err, pull_service.ErrIsClosed) { ctx.NotFound() - } else if pull_service.IsErrUserNotAllowedToMerge(err) { + } else if errors.Is(err, pull_service.ErrUserNotAllowedToMerge) { ctx.Error(http.StatusMethodNotAllowed, "Merge", "User not allowed to merge PR") - } else if pull_service.IsErrHasMerged(err) { + } else if errors.Is(err, pull_service.ErrHasMerged) { ctx.Error(http.StatusMethodNotAllowed, "PR already merged", "") - } else if pull_service.IsErrIsWorkInProgress(err) { + } 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 pull_service.IsErrNotMergableState(err) { + } 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) diff --git a/routers/web/repo/pull.go b/routers/web/repo/pull.go index 1cbbc3c960a16..32d9a65cbdbb9 100644 --- a/routers/web/repo/pull.go +++ b/routers/web/repo/pull.go @@ -867,7 +867,7 @@ func MergePullRequest(ctx *context.Context) { forceMerge := form.ForceMerge != nil && *form.ForceMerge if err := pull_service.CheckPullMergable(ctx, ctx.Doer, &ctx.Repo.Permission, pr, manuallMerge, forceMerge); err != nil { - if pull_service.IsErrIsClosed(err) { + if errors.Is(err, pull_service.ErrIsClosed) { if issue.IsPull { ctx.Flash.Error(ctx.Tr("repo.pulls.is_closed")) ctx.Redirect(issue.Link()) @@ -875,16 +875,16 @@ func MergePullRequest(ctx *context.Context) { ctx.Flash.Error(ctx.Tr("repo.issues.closed_title")) ctx.Redirect(issue.Link()) } - } else if pull_service.IsErrUserNotAllowedToMerge(err) { + } else if errors.Is(err, pull_service.ErrUserNotAllowedToMerge) { ctx.Flash.Error(ctx.Tr("repo.pulls.update_not_allowed")) ctx.Redirect(issue.Link()) - } else if pull_service.IsErrHasMerged(err) { + } else if errors.Is(err, pull_service.ErrHasMerged) { ctx.Flash.Error(ctx.Tr("repo.pulls.has_merged")) ctx.Redirect(issue.Link()) - } else if pull_service.IsErrIsWorkInProgress(err) { + } else if errors.Is(err, pull_service.ErrIsWorkInProgress) { ctx.Flash.Error(ctx.Tr("repo.pulls.no_merge_wip")) ctx.Redirect(issue.Link()) - } else if pull_service.IsErrNotMergableState(err) { + } 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) { @@ -893,7 +893,7 @@ func MergePullRequest(ctx *context.Context) { } else if asymkey_service.IsErrWontSign(err) { ctx.Flash.Error(err.Error()) // has not translation ... ctx.Redirect(issue.Link()) - } else if pull_service.IsErrDependenciesLeft(err) { + } else if errors.Is(err, pull_service.ErrDependenciesLeft) { ctx.Flash.Error(ctx.Tr("repo.issues.dependency.pr_close_blocked")) ctx.Redirect(issue.Link()) } else { diff --git a/services/pull/protection_check.go b/services/pull/protection_check.go index a742c7952afc1..c559daf905f6a 100644 --- a/services/pull/protection_check.go +++ b/services/pull/protection_check.go @@ -6,94 +6,38 @@ package pull import ( "context" + "errors" "code.gitea.io/gitea/models" user_model "code.gitea.io/gitea/models/user" asymkey_service "code.gitea.io/gitea/services/asymkey" ) -type ErrIsClosed struct{} - -func IsErrIsClosed(err error) bool { - _, ok := err.(ErrIsClosed) - return ok -} - -func (err ErrIsClosed) Error() string { - return "pull is cosed" -} - -type ErrUserNotAllowedToMerge struct{} - -func IsErrUserNotAllowedToMerge(err error) bool { - _, ok := err.(ErrUserNotAllowedToMerge) - return ok -} - -func (err ErrUserNotAllowedToMerge) Error() string { - return "user not allowed to merge" -} - -type ErrHasMerged struct{} - -func IsErrHasMerged(err error) bool { - _, ok := err.(ErrHasMerged) - return ok -} - -func (err ErrHasMerged) Error() string { - return "has already been merged" -} - -type ErrIsWorkInProgress struct{} - -func IsErrIsWorkInProgress(err error) bool { - _, ok := err.(ErrIsWorkInProgress) - return ok -} - -func (err ErrIsWorkInProgress) Error() string { - return "work in progress PRs cannot be merged" -} - -type ErrNotMergableState struct{} - -func IsErrNotMergableState(err error) bool { - _, ok := err.(ErrNotMergableState) - return ok -} - -func (err ErrNotMergableState) Error() string { - return "not in mergeable state" -} - -type ErrDependenciesLeft struct{} - -func IsErrDependenciesLeft(err error) bool { - _, ok := err.(ErrDependenciesLeft) - return ok -} - -func (err ErrDependenciesLeft) Error() string { - return "is blocked by an open dependency" -} +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") +) // 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{} + return ErrHasMerged } if err := pr.LoadIssue(); err != nil { return err } else if pr.Issue.IsClosed { - return ErrIsClosed{} + return ErrIsClosed } if allowedMerge, err := IsUserAllowedToMerge(pr, *perm, doer); err != nil { return err } else if !allowedMerge { - return ErrUserNotAllowedToMerge{} + return ErrUserNotAllowedToMerge } if manuallMerge { @@ -102,11 +46,11 @@ func CheckPullMergable(ctx context.Context, doer *user_model.User, perm *models. } if pr.IsWorkInProgress() { - return ErrIsWorkInProgress{} + return ErrIsWorkInProgress } if !pr.CanAutoMerge() { - return ErrNotMergableState{} + return ErrNotMergableState } if err := CheckPRReadyToMerge(ctx, pr, false); err != nil { @@ -115,7 +59,7 @@ func CheckPullMergable(ctx context.Context, doer *user_model.User, perm *models. if isRepoAdmin, err := models.IsUserRepoAdmin(pr.BaseRepo, doer); err != nil { return err } else if !isRepoAdmin { - return ErrUserNotAllowedToMerge{} + return ErrUserNotAllowedToMerge } } } else { @@ -130,7 +74,7 @@ func CheckPullMergable(ctx context.Context, doer *user_model.User, perm *models. if noDeps, err := models.IssueNoDependenciesLeft(pr.Issue); err != nil { return err } else if !noDeps { - return ErrDependenciesLeft{} + return ErrDependenciesLeft } return nil From d0a23125cd7c9f44a9a81ff1739ac02aa2451583 Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Thu, 31 Mar 2022 16:11:25 +0200 Subject: [PATCH 11/11] merge files --- services/pull/check.go | 84 +++++++++++++++++++++++++++ services/pull/protection_check.go | 96 ------------------------------- 2 files changed, 84 insertions(+), 96 deletions(-) delete mode 100644 services/pull/protection_check.go diff --git a/services/pull/check.go b/services/pull/check.go index f920688f5d9bb..a4bbd3bb891be 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/protection_check.go b/services/pull/protection_check.go deleted file mode 100644 index c559daf905f6a..0000000000000 --- a/services/pull/protection_check.go +++ /dev/null @@ -1,96 +0,0 @@ -// Copyright 2022 The Gitea Authors. All rights reserved. -// Use of this source code is governed by a MIT-style -// license that can be found in the LICENSE file. - -package pull - -import ( - "context" - "errors" - - "code.gitea.io/gitea/models" - user_model "code.gitea.io/gitea/models/user" - asymkey_service "code.gitea.io/gitea/services/asymkey" -) - -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") -) - -// 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 -}