From 123c254b843e2c9bf6f44c37b28b1b41958e40ee Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Thu, 31 Mar 2022 16:57:13 +0200 Subject: [PATCH] Move checks for pulls before merge into own function (#19271) (#19277) Backport #19271 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/pull.go | 27 +++---- models/pull_test.go | 16 +++- routers/api/v1/repo/pull.go | 104 +++++++----------------- routers/web/repo/pull.go | 155 +++++++++++++----------------------- services/forms/repo_form.go | 25 ++++++ services/pull/check.go | 84 +++++++++++++++++++ services/pull/merge.go | 19 +---- 7 files changed, 216 insertions(+), 214 deletions(-) diff --git a/models/pull.go b/models/pull.go index 243d40b1fa..0511d29dfc 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 2567984cc1..9098b61161 100644 --- a/models/pull_test.go +++ b/models/pull_test.go @@ -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) } diff --git a/routers/api/v1/repo/pull.go b/routers/api/v1/repo/pull.go index 158abd4651..06641b3f84 100644 --- a/routers/api/v1/repo/pull.go +++ b/routers/api/v1/repo/pull.go @@ -95,7 +95,6 @@ func ListPullRequests(ctx *context.APIContext) { Labels: ctx.FormStrings("labels"), MilestoneID: ctx.FormInt64("milestone"), }) - if err != nil { ctx.Error(http.StatusInternalServerError, "PullRequests", err) return @@ -724,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 } @@ -744,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.User) - 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.User, &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.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.User, ctx.Repo.GitRepo, form.MergeCommitID); err != nil { + if manuallMerge { + if err := pull_service.MergedManually(pr, ctx.User, 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 @@ -782,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(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.User); 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(pr, ctx.User); 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(pr, ctx.User, ctx.Repo.GitRepo, repo_model.MergeStyle(form.Do), form.HeadCommitID, message); err != nil { + if err := pull_service.Merge(pr, ctx.User, 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 e9364ce654..cdf130abf4 100644 --- a/routers/web/repo/pull.go +++ b/routers/web/repo/pull.go @@ -33,6 +33,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" @@ -48,16 +49,14 @@ const ( pullRequestTemplateKey = "PullRequestTemplate" ) -var ( - pullRequestTemplateCandidates = []string{ - "PULL_REQUEST_TEMPLATE.md", - "pull_request_template.md", - ".gitea/PULL_REQUEST_TEMPLATE.md", - ".gitea/pull_request_template.md", - ".github/PULL_REQUEST_TEMPLATE.md", - ".github/pull_request_template.md", - } -) +var pullRequestTemplateCandidates = []string{ + "PULL_REQUEST_TEMPLATE.md", + "pull_request_template.md", + ".gitea/PULL_REQUEST_TEMPLATE.md", + ".gitea/pull_request_template.md", + ".github/PULL_REQUEST_TEMPLATE.md", + ".github/pull_request_template.md", +} func getRepository(ctx *context.Context, repoID int64) *repo_model.Repository { repo, err := repo_model.GetRepositoryByID(repoID) @@ -125,7 +124,7 @@ func getForkRepository(ctx *context.Context) *repo_model.Repository { } } - var traverseParentRepo = forkRepo + traverseParentRepo := forkRepo for { if ctx.User.ID == traverseParentRepo.OwnerID { canForkToUser = false @@ -195,7 +194,7 @@ func ForkPost(ctx *context.Context) { } var err error - var traverseParentRepo = forkRepo + traverseParentRepo := forkRepo for { if ctxUser.ID == traverseParentRepo.OwnerID { ctx.RenderWithErr(ctx.Tr("repo.settings.new_owner_has_same_repo"), tplFork, &form) @@ -853,39 +852,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.User) - 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.User, &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.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 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.User, ctx.Repo.GitRepo, form.MergeCommitID); err != nil { + if manuallMerge { + if err := pull_service.MergedManually(pr, ctx.User, 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()) @@ -904,72 +917,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()) + // 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.Flash.Error(ctx.Tr("repo.pulls.no_merge_wip")) - ctx.Redirect(issue.Link()) - return - } - - if err := pull_service.CheckPRReadyToMerge(pr, false); err != nil { - if !models.IsErrNotAllowedToMerge(err) { - ctx.ServerError("Merge PR status", err) - return - } - if isRepoAdmin, err := models.IsUserRepoAdmin(pr.BaseRepo, ctx.User); 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()) - return - } - - if err = pull_service.Merge(pr, ctx.User, ctx.Repo.GitRepo, repo_model.MergeStyle(form.Do), form.HeadCommitID, message); err != nil { + if err := pull_service.Merge(pr, ctx.User, 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()) @@ -1079,7 +1033,6 @@ func MergePullRequest(ctx *context.Context) { } func stopTimerIfAvailable(user *user_model.User, issue *models.Issue) error { - if models.StopwatchExists(user.ID, issue.ID) { if err := models.CreateOrStopIssueStopwatch(user, issue); err != nil { return err diff --git a/services/forms/repo_form.go b/services/forms/repo_form.go index 19b5a37664..e78559912e 100644 --- a/services/forms/repo_form.go +++ b/services/forms/repo_form.go @@ -582,6 +582,31 @@ func (f *MergePullRequestForm) Validate(req *http.Request, errs binding.Errors) return middleware.Validate(errs, ctx.Data, f, ctx.Locale) } +// SetDefaults if not provided for mergestyle and commit message +func (f *MergePullRequestForm) SetDefaults(pr *models.PullRequest) (err error) { + if f.Do == "" { + f.Do = "merge" + } + + f.MergeTitleField = strings.TrimSpace(f.MergeTitleField) + if len(f.MergeTitleField) == 0 { + switch f.Do { + case "merge", "rebase-merge": + f.MergeTitleField, err = pr.GetDefaultMergeMessage() + case "squash": + f.MergeTitleField, err = pr.GetDefaultSquashMessage() + } + } + + f.MergeMessageField = strings.TrimSpace(f.MergeMessageField) + if len(f.MergeMessageField) > 0 { + f.MergeTitleField += "\n\n" + f.MergeMessageField + f.MergeMessageField = "" + } + + return +} + // CodeCommentForm form for adding code comments for PRs type CodeCommentForm struct { Origin string `binding:"Required;In(timeline,diff)"` diff --git a/services/pull/check.go b/services/pull/check.go index 363a716b28..f3ffa02656 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(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 := 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(pr, doer, pr.BaseRepo.RepoPath(), pr.BaseBranch, pr.GetGitRefName()) + + return sign, err +} + // checkAndUpdateStatus checks if pull request is possible to leaving checking status, // and set to be either conflict or mergeable. func checkAndUpdateStatus(pr *models.PullRequest) { diff --git a/services/pull/merge.go b/services/pull/merge.go index f6a6415bff..37269b0c78 100644 --- a/services/pull/merge.go +++ b/services/pull/merge.go @@ -164,13 +164,13 @@ func rawMerge(pr *models.PullRequest, doer *user_model.User, mergeStyle repo_mod } infoPath := filepath.Join(tmpBasePath, ".git", "info") - if err := os.MkdirAll(infoPath, 0700); err != nil { + if err := os.MkdirAll(infoPath, 0o700); err != nil { log.Error("Unable to create .git/info in %s: %v", tmpBasePath, err) return "", fmt.Errorf("Unable to create .git/info in tmpBasePath: %v", err) } sparseCheckoutListPath := filepath.Join(infoPath, "sparse-checkout") - if err := os.WriteFile(sparseCheckoutListPath, []byte(sparseCheckoutList), 0600); err != nil { + if err := os.WriteFile(sparseCheckoutListPath, []byte(sparseCheckoutList), 0o600); err != nil { log.Error("Unable to write .git/info/sparse-checkout file in %s: %v", tmpBasePath, err) return "", fmt.Errorf("Unable to write .git/info/sparse-checkout file in tmpBasePath: %v", err) } @@ -561,21 +561,6 @@ func getDiffTree(repoPath, baseBranch, headBranch string) (string, error) { return out.String(), nil } -// IsSignedIfRequired check if merge will be signed if required -func IsSignedIfRequired(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(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 {