From a5279b74b63248902a6572df5afa3745367e6cb9 Mon Sep 17 00:00:00 2001 From: a1012112796 <1012112796@qq.com> Date: Thu, 4 Mar 2021 11:41:23 +0800 Subject: [PATCH] Make manual merge autodetection optional and add manual merge as merge method (#12543) * Make auto check manual merge as a chooseable mod and add manual merge way on ui as title, Before this pr, we use same way with GH to check manually merge. It good, but in some special cases, misjudgments can occur. and it's hard to fix this bug. So I add option to allow repo manager block "auto check manual merge" function, Then it will have same style like gitlab(allow empty pr). and to compensate for not being able to detect THE PR merge automatically, I added a manual approach. Signed-off-by: a1012112796 <1012112796@qq.com> * make swager * api support * ping ci * fix TestPullCreate_EmptyChangesWithCommits * Apply suggestions from code review Co-authored-by: zeripath * Apply review suggestions and add test * Apply suggestions from code review Co-authored-by: zeripath * fix build * test error message * make fmt * Fix indentation issues identified by @silverwind Co-authored-by: silverwind * Fix tests and make manually merged disabled error on API the same Signed-off-by: Andrew Thornton * a small nit * fix wrong commit id error * fix bug * simple test * fix test Co-authored-by: zeripath Co-authored-by: silverwind Co-authored-by: techknowlogick --- .../api_helper_for_declarative_test.go | 35 +++++++++++ integrations/git_test.go | 30 ++++++++++ integrations/pull_status_test.go | 2 +- models/pull.go | 8 +++ models/repo_unit.go | 5 +- modules/forms/repo_form.go | 9 ++- modules/git/repo_commit.go | 9 +++ modules/git/repo_commit_test.go | 15 +++++ modules/structs/repo.go | 4 ++ options/locale/locale_en-US.ini | 10 ++++ routers/api/v1/repo/pull.go | 26 +++++++-- routers/api/v1/repo/repo.go | 8 +++ routers/repo/compare.go | 8 +++ routers/repo/issue.go | 18 ++++++ routers/repo/pull.go | 34 +++++++++-- routers/repo/setting.go | 2 + services/pull/check.go | 17 +++++- services/pull/merge.go | 51 ++++++++++++++++ services/pull/patch.go | 5 +- templates/repo/diff/compare.tmpl | 12 +++- .../repo/issue/view_content/comments.tmpl | 6 +- templates/repo/issue/view_content/pull.tmpl | 58 ++++++++++++++++++- templates/repo/issue/view_title.tmpl | 2 +- templates/repo/settings/options.tmpl | 12 ++++ templates/swagger/v1_json.tmpl | 16 ++++- 25 files changed, 379 insertions(+), 23 deletions(-) diff --git a/integrations/api_helper_for_declarative_test.go b/integrations/api_helper_for_declarative_test.go index f1d57e717c..913fce1577 100644 --- a/integrations/api_helper_for_declarative_test.go +++ b/integrations/api_helper_for_declarative_test.go @@ -9,6 +9,7 @@ import ( "fmt" "io/ioutil" "net/http" + "net/url" "testing" "time" @@ -71,6 +72,23 @@ func doAPICreateRepository(ctx APITestContext, empty bool, callback ...func(*tes } } +func doAPIEditRepository(ctx APITestContext, editRepoOption *api.EditRepoOption, callback ...func(*testing.T, api.Repository)) func(*testing.T) { + return func(t *testing.T) { + req := NewRequestWithJSON(t, "PATCH", fmt.Sprintf("/api/v1/repos/%s/%s?token=%s", url.PathEscape(ctx.Username), url.PathEscape(ctx.Reponame), ctx.Token), editRepoOption) + if ctx.ExpectedCode != 0 { + ctx.Session.MakeRequest(t, req, ctx.ExpectedCode) + return + } + resp := ctx.Session.MakeRequest(t, req, http.StatusOK) + + var repository api.Repository + DecodeJSON(t, resp, &repository) + if len(callback) > 0 { + callback[0](t, repository) + } + } +} + func doAPIAddCollaborator(ctx APITestContext, username string, mode models.AccessMode) func(*testing.T) { return func(t *testing.T) { permission := "read" @@ -256,6 +274,23 @@ func doAPIMergePullRequest(ctx APITestContext, owner, repo string, index int64) } } +func doAPIManuallyMergePullRequest(ctx APITestContext, owner, repo, commitID string, index int64) func(*testing.T) { + return func(t *testing.T) { + urlStr := fmt.Sprintf("/api/v1/repos/%s/%s/pulls/%d/merge?token=%s", + owner, repo, index, ctx.Token) + req := NewRequestWithJSON(t, http.MethodPost, urlStr, &auth.MergePullRequestForm{ + Do: string(models.MergeStyleManuallyMerged), + MergeCommitID: commitID, + }) + + if ctx.ExpectedCode != 0 { + ctx.Session.MakeRequest(t, req, ctx.ExpectedCode) + return + } + ctx.Session.MakeRequest(t, req, 200) + } +} + func doAPIGetBranch(ctx APITestContext, branch string, callback ...func(*testing.T, api.Branch)) func(*testing.T) { return func(t *testing.T) { req := NewRequestf(t, "GET", "/api/v1/repos/%s/%s/branches/%s?token=%s", ctx.Username, ctx.Reponame, branch, ctx.Token) diff --git a/integrations/git_test.go b/integrations/git_test.go index c3c1126829..705bd08c11 100644 --- a/integrations/git_test.go +++ b/integrations/git_test.go @@ -69,6 +69,7 @@ func testGit(t *testing.T, u *url.URL) { mediaTest(t, &httpContext, little, big, littleLFS, bigLFS) t.Run("BranchProtectMerge", doBranchProtectPRMerge(&httpContext, dstPath)) + t.Run("CreatePRAndSetManuallyMerged", doCreatePRAndSetManuallyMerged(httpContext, httpContext, dstPath, "master", "test-manually-merge")) t.Run("MergeFork", func(t *testing.T) { defer PrintCurrentTest(t)() t.Run("CreatePRAndMerge", doMergeFork(httpContext, forkedUserCtx, "master", httpContext.Username+":master")) @@ -468,6 +469,35 @@ func doMergeFork(ctx, baseCtx APITestContext, baseBranch, headBranch string) fun } } +func doCreatePRAndSetManuallyMerged(ctx, baseCtx APITestContext, dstPath, baseBranch, headBranch string) func(t *testing.T) { + return func(t *testing.T) { + defer PrintCurrentTest(t)() + var ( + pr api.PullRequest + err error + lastCommitID string + ) + + trueBool := true + falseBool := false + + t.Run("AllowSetManuallyMergedAndSwitchOffAutodetectManualMerge", doAPIEditRepository(baseCtx, &api.EditRepoOption{ + HasPullRequests: &trueBool, + AllowManualMerge: &trueBool, + AutodetectManualMerge: &falseBool, + })) + + t.Run("CreateHeadBranch", doGitCreateBranch(dstPath, headBranch)) + t.Run("PushToHeadBranch", doGitPushTestRepository(dstPath, "origin", headBranch)) + t.Run("CreateEmptyPullRequest", func(t *testing.T) { + pr, err = doAPICreatePullRequest(ctx, baseCtx.Username, baseCtx.Reponame, baseBranch, headBranch)(t) + assert.NoError(t, err) + }) + lastCommitID = pr.Base.Sha + t.Run("ManuallyMergePR", doAPIManuallyMergePullRequest(ctx, baseCtx.Username, baseCtx.Reponame, lastCommitID, pr.Index)) + } +} + func doEnsureCanSeePull(ctx APITestContext, pr api.PullRequest) func(t *testing.T) { return func(t *testing.T) { req := NewRequest(t, "GET", fmt.Sprintf("/%s/%s/pulls/%d", url.PathEscape(ctx.Username), url.PathEscape(ctx.Reponame), pr.Index)) diff --git a/integrations/pull_status_test.go b/integrations/pull_status_test.go index fc4c7ca33d..7c6f3d44c5 100644 --- a/integrations/pull_status_test.go +++ b/integrations/pull_status_test.go @@ -115,6 +115,6 @@ func TestPullCreate_EmptyChangesWithCommits(t *testing.T) { doc := NewHTMLParser(t, resp.Body) text := strings.TrimSpace(doc.doc.Find(".merge-section").Text()) - assert.Contains(t, text, "This pull request can be merged automatically.") + assert.Contains(t, text, "This branch is equal with the target branch.") }) } diff --git a/models/pull.go b/models/pull.go index 0eba65db4f..7dacf6a8d7 100644 --- a/models/pull.go +++ b/models/pull.go @@ -35,6 +35,7 @@ const ( PullRequestStatusMergeable PullRequestStatusManuallyMerged PullRequestStatusError + PullRequestStatusEmpty ) // PullRequest represents relation between pull request and repositories. @@ -332,6 +333,11 @@ func (pr *PullRequest) CanAutoMerge() bool { return pr.Status == PullRequestStatusMergeable } +// IsEmpty returns true if this pull request is empty. +func (pr *PullRequest) IsEmpty() bool { + return pr.Status == PullRequestStatusEmpty +} + // MergeStyle represents the approach to merge commits into base branch. type MergeStyle string @@ -344,6 +350,8 @@ const ( MergeStyleRebaseMerge MergeStyle = "rebase-merge" // MergeStyleSquash squash commits into single commit before merging MergeStyleSquash MergeStyle = "squash" + // MergeStyleManuallyMerged pr has been merged manually, just mark it as merged directly + MergeStyleManuallyMerged MergeStyle = "manually-merged" ) // SetMerged sets a pull request to merged and closes the corresponding issue diff --git a/models/repo_unit.go b/models/repo_unit.go index 3ef3904833..0feddfe2ea 100644 --- a/models/repo_unit.go +++ b/models/repo_unit.go @@ -101,6 +101,8 @@ type PullRequestsConfig struct { AllowRebase bool AllowRebaseMerge bool AllowSquash bool + AllowManualMerge bool + AutodetectManualMerge bool } // FromDB fills up a PullRequestsConfig from serialized format. @@ -120,7 +122,8 @@ func (cfg *PullRequestsConfig) IsMergeStyleAllowed(mergeStyle MergeStyle) bool { return mergeStyle == MergeStyleMerge && cfg.AllowMerge || mergeStyle == MergeStyleRebase && cfg.AllowRebase || mergeStyle == MergeStyleRebaseMerge && cfg.AllowRebaseMerge || - mergeStyle == MergeStyleSquash && cfg.AllowSquash + mergeStyle == MergeStyleSquash && cfg.AllowSquash || + mergeStyle == MergeStyleManuallyMerged && cfg.AllowManualMerge } // AllowedMergeStyleCount returns the total count of allowed merge styles for the PullRequestsConfig diff --git a/modules/forms/repo_form.go b/modules/forms/repo_form.go index 2793acdd5b..ab88aef571 100644 --- a/modules/forms/repo_form.go +++ b/modules/forms/repo_form.go @@ -156,6 +156,8 @@ type RepoSettingForm struct { PullsAllowRebase bool PullsAllowRebaseMerge bool PullsAllowSquash bool + PullsAllowManualMerge bool + EnableAutodetectManualMerge bool EnableTimetracker bool AllowOnlyContributorsToTrackTime bool EnableIssueDependencies bool @@ -556,11 +558,12 @@ func (f *InitializeLabelsForm) Validate(req *http.Request, errs binding.Errors) // swagger:model MergePullRequestOption type MergePullRequestForm struct { // required: true - // enum: merge,rebase,rebase-merge,squash - Do string `binding:"Required;In(merge,rebase,rebase-merge,squash)"` + // enum: merge,rebase,rebase-merge,squash,manually-merged + Do string `binding:"Required;In(merge,rebase,rebase-merge,squash,manually-merged)"` MergeTitleField string MergeMessageField string - ForceMerge *bool `json:"force_merge,omitempty"` + MergeCommitID string // only used for manually-merged + ForceMerge *bool `json:"force_merge,omitempty"` } // Validate validates the fields diff --git a/modules/git/repo_commit.go b/modules/git/repo_commit.go index 5bf113ba49..ea0aeeb35d 100644 --- a/modules/git/repo_commit.go +++ b/modules/git/repo_commit.go @@ -456,3 +456,12 @@ func (repo *Repository) GetCommitsFromIDs(commitIDs []string) (commits *list.Lis return commits } + +// IsCommitInBranch check if the commit is on the branch +func (repo *Repository) IsCommitInBranch(commitID, branch string) (r bool, err error) { + stdout, err := NewCommand("branch", "--contains", commitID, branch).RunInDir(repo.Path) + if err != nil { + return false, err + } + return len(stdout) > 0, err +} diff --git a/modules/git/repo_commit_test.go b/modules/git/repo_commit_test.go index 87dd6763b3..3eedaa6b6e 100644 --- a/modules/git/repo_commit_test.go +++ b/modules/git/repo_commit_test.go @@ -63,3 +63,18 @@ func TestGetCommitWithBadCommitID(t *testing.T) { assert.Error(t, err) assert.True(t, IsErrNotExist(err)) } + +func TestIsCommitInBranch(t *testing.T) { + bareRepo1Path := filepath.Join(testReposDir, "repo1_bare") + bareRepo1, err := OpenRepository(bareRepo1Path) + assert.NoError(t, err) + defer bareRepo1.Close() + + result, err := bareRepo1.IsCommitInBranch("2839944139e0de9737a044f78b0e4b40d989a9e3", "branch1") + assert.NoError(t, err) + assert.Equal(t, true, result) + + result, err = bareRepo1.IsCommitInBranch("2839944139e0de9737a044f78b0e4b40d989a9e3", "branch2") + assert.NoError(t, err) + assert.Equal(t, false, result) +} diff --git a/modules/structs/repo.go b/modules/structs/repo.go index d588813b21..c47700cd00 100644 --- a/modules/structs/repo.go +++ b/modules/structs/repo.go @@ -167,6 +167,10 @@ type EditRepoOption struct { AllowRebaseMerge *bool `json:"allow_rebase_explicit,omitempty"` // either `true` to allow squash-merging pull requests, or `false` to prevent squash-merging. `has_pull_requests` must be `true`. AllowSquash *bool `json:"allow_squash_merge,omitempty"` + // either `true` to allow mark pr as merged manually, or `false` to prevent it. `has_pull_requests` must be `true`. + AllowManualMerge *bool `json:"allow_manual_merge,omitempty"` + // either `true` to enable AutodetectManualMerge, or `false` to prevent it. `has_pull_requests` must be `true`, Note: In some special cases, misjudgments can occur. + AutodetectManualMerge *bool `json:"autodetect_manual_merge,omitempty"` // set to `true` to archive this repository. Archived *bool `json:"archived,omitempty"` // set to a string like `8h30m0s` to set the mirror interval time diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index 0ee8e7ab0c..08b202d192 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -1105,6 +1105,7 @@ issues.context.delete = Delete issues.no_content = There is no content yet. issues.close_issue = Close issues.pull_merged_at = `merged commit %[2]s into %[3]s %[4]s` +issues.manually_pull_merged_at = `merged commit %[2]s into %[3]s %[4]s manually` issues.close_comment_issue = Comment and Close issues.reopen_issue = Reopen issues.reopen_comment_issue = Comment and Reopen @@ -1273,6 +1274,7 @@ pulls.compare_compare = pull from pulls.filter_branch = Filter branch pulls.no_results = No results found. pulls.nothing_to_compare = These branches are equal. There is no need to create a pull request. +pulls.nothing_to_compare_and_allow_empty_pr = These branches are equal. This PR will be empty. pulls.has_pull_request = `A pull request between these branches already exists: %[2]s#%[3]d` pulls.create = Create Pull Request pulls.title_desc = wants to merge %[1]d commits from %[2]s into %[3]s @@ -1285,6 +1287,8 @@ pulls.reopen_to_merge = Please reopen this pull request to perform a merge. pulls.cant_reopen_deleted_branch = This pull request cannot be reopened because the branch was deleted. pulls.merged = Merged pulls.merged_as = The pull request has been merged as %[2]s. +pulls.manually_merged = Manually merged +pulls.manually_merged_as = The pull request has been manually merged as %[2]s. pulls.is_closed = The pull request has been closed. pulls.has_merged = The pull request has been merged. pulls.title_wip_desc = `Start the title with %s to prevent the pull request from being merged accidentally.` @@ -1292,6 +1296,7 @@ pulls.cannot_merge_work_in_progress = This pull request is marked as a work in p pulls.data_broken = This pull request is broken due to missing fork information. pulls.files_conflicted = This pull request has changes conflicting with the target branch. pulls.is_checking = "Merge conflict checking is in progress. Try again in few moments." +pulls.is_empty = "This branch is equal with the target branch." pulls.required_status_check_failed = Some required checks were not successful. pulls.required_status_check_missing = Some required checks are missing. pulls.required_status_check_administrator = As an administrator, you may still merge this pull request. @@ -1312,6 +1317,7 @@ pulls.reject_count_1 = "%d change request" pulls.reject_count_n = "%d change requests" pulls.waiting_count_1 = "%d waiting review" pulls.waiting_count_n = "%d waiting reviews" +pulls.wrong_commit_id = "commit id must be a commit id on the target branch" pulls.no_merge_desc = This pull request cannot be merged because all repository merge options are disabled. pulls.no_merge_helper = Enable merge options in the repository settings or merge the pull request manually. @@ -1322,6 +1328,8 @@ pulls.merge_pull_request = Merge Pull Request pulls.rebase_merge_pull_request = Rebase and Merge pulls.rebase_merge_commit_pull_request = Rebase and Merge (--no-ff) pulls.squash_merge_pull_request = Squash and Merge +pulls.merge_manually = Manually merged +pulls.merge_commit_id = The merge commit ID pulls.require_signed_wont_sign = The branch requires signed commits but this merge will not be signed pulls.invalid_merge_option = You cannot use this merge option for this pull request. pulls.merge_conflict = Merge Failed: There was a conflict whilst merging. Hint: Try a different strategy @@ -1545,6 +1553,8 @@ settings.pulls.allow_merge_commits = Enable Commit Merging settings.pulls.allow_rebase_merge = Enable Rebasing to Merge Commits settings.pulls.allow_rebase_merge_commit = Enable Rebasing with explicit merge commits (--no-ff) settings.pulls.allow_squash_commits = Enable Squashing to Merge Commits +settings.pulls.allow_manual_merge = Enable Mark PR as manually merged +settings.pulls.enable_autodetect_manual_merge = Enable autodetect manual merge (Note: In some special cases, misjudgments can occur) settings.projects_desc = Enable Repository Projects settings.admin_settings = Administrator Settings settings.admin_enable_health_check = Enable Repository Health Checks (git fsck) diff --git a/routers/api/v1/repo/pull.go b/routers/api/v1/repo/pull.go index 38dac36553..8eda949652 100644 --- a/routers/api/v1/repo/pull.go +++ b/routers/api/v1/repo/pull.go @@ -769,13 +769,31 @@ func MergePullRequest(ctx *context.APIContext) { return } - if !pr.CanAutoMerge() { - ctx.Error(http.StatusMethodNotAllowed, "PR not in mergeable state", "Please try again later") + if pr.HasMerged { + ctx.Error(http.StatusMethodNotAllowed, "PR already merged", "") return } - if pr.HasMerged { - ctx.Error(http.StatusMethodNotAllowed, "PR already merged", "") + // handle manually-merged mark + if models.MergeStyle(form.Do) == models.MergeStyleManuallyMerged { + 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", models.MergeStyle(form.Do))) + return + } + if strings.Contains(err.Error(), "Wrong commit ID") { + ctx.JSON(http.StatusConflict, err) + return + } + ctx.Error(http.StatusInternalServerError, "Manually-Merged", err) + return + } + ctx.Status(http.StatusOK) + return + } + + if !pr.CanAutoMerge() { + ctx.Error(http.StatusMethodNotAllowed, "PR not in mergeable state", "Please try again later") return } diff --git a/routers/api/v1/repo/repo.go b/routers/api/v1/repo/repo.go index b84c5993e4..b12797f83b 100644 --- a/routers/api/v1/repo/repo.go +++ b/routers/api/v1/repo/repo.go @@ -725,6 +725,8 @@ func updateRepoUnits(ctx *context.APIContext, opts api.EditRepoOption) error { AllowRebase: true, AllowRebaseMerge: true, AllowSquash: true, + AllowManualMerge: true, + AutodetectManualMerge: false, } } else { config = unit.PullRequestsConfig() @@ -745,6 +747,12 @@ func updateRepoUnits(ctx *context.APIContext, opts api.EditRepoOption) error { if opts.AllowSquash != nil { config.AllowSquash = *opts.AllowSquash } + if opts.AllowManualMerge != nil { + config.AllowManualMerge = *opts.AllowManualMerge + } + if opts.AutodetectManualMerge != nil { + config.AutodetectManualMerge = *opts.AutodetectManualMerge + } units = append(units, models.RepoUnit{ RepoID: repo.ID, diff --git a/routers/repo/compare.go b/routers/repo/compare.go index 2eef20f5ff..a8f4f8add8 100644 --- a/routers/repo/compare.go +++ b/routers/repo/compare.go @@ -429,6 +429,14 @@ func PrepareCompareDiff( if headCommitID == compareInfo.MergeBase { ctx.Data["IsNothingToCompare"] = true + if unit, err := repo.GetUnit(models.UnitTypePullRequests); err == nil { + config := unit.PullRequestsConfig() + if !config.AutodetectManualMerge { + ctx.Data["AllowEmptyPr"] = !(baseBranch == headBranch && ctx.Repo.Repository.Name == headRepo.Name) + } else { + ctx.Data["AllowEmptyPr"] = false + } + } return true } diff --git a/routers/repo/issue.go b/routers/repo/issue.go index a9459a10ed..99df9db183 100644 --- a/routers/repo/issue.go +++ b/routers/repo/issue.go @@ -1491,6 +1491,8 @@ func ViewIssue(ctx *context.Context) { ctx.Data["MergeStyle"] = models.MergeStyleRebaseMerge } else if prConfig.AllowSquash { ctx.Data["MergeStyle"] = models.MergeStyleSquash + } else if prConfig.AllowManualMerge { + ctx.Data["MergeStyle"] = models.MergeStyleManuallyMerged } else { ctx.Data["MergeStyle"] = "" } @@ -1531,6 +1533,22 @@ func ViewIssue(ctx *context.Context) { pull.HeadRepo != nil && git.IsBranchExist(pull.HeadRepo.RepoPath(), pull.HeadBranch) && (!pull.HasMerged || ctx.Data["HeadBranchCommitID"] == ctx.Data["PullHeadCommitID"]) + + stillCanManualMerge := func() bool { + if pull.HasMerged || issue.IsClosed || !ctx.IsSigned { + return false + } + if pull.CanAutoMerge() || pull.IsWorkInProgress() || pull.IsChecking() { + return false + } + if (ctx.User.IsAdmin || ctx.Repo.IsAdmin()) && prConfig.AllowManualMerge { + return true + } + + return false + } + + ctx.Data["StillCanManualMerge"] = stillCanManualMerge() } // Get Dependencies diff --git a/routers/repo/pull.go b/routers/repo/pull.go index 15af2a2a3f..2ed47605f8 100644 --- a/routers/repo/pull.go +++ b/routers/repo/pull.go @@ -33,6 +33,7 @@ import ( "code.gitea.io/gitea/services/gitdiff" pull_service "code.gitea.io/gitea/services/pull" repo_service "code.gitea.io/gitea/services/repository" + "github.com/unknwon/com" ) const ( @@ -794,15 +795,36 @@ func MergePullRequest(ctx *context.Context) { return } - if !pr.CanAutoMerge() { - ctx.Flash.Error(ctx.Tr("repo.pulls.no_merge_not_ready")) - ctx.Redirect(ctx.Repo.RepoLink + "/pulls/" + fmt.Sprint(issue.Index)) + if pr.HasMerged { + ctx.Flash.Error(ctx.Tr("repo.pulls.has_merged")) + ctx.Redirect(ctx.Repo.RepoLink + "/pulls/" + com.ToStr(issue.Index)) return } - if pr.HasMerged { - ctx.Flash.Error(ctx.Tr("repo.pulls.has_merged")) - ctx.Redirect(ctx.Repo.RepoLink + "/pulls/" + fmt.Sprint(issue.Index)) + // handle manually-merged mark + if models.MergeStyle(form.Do) == models.MergeStyleManuallyMerged { + 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(ctx.Repo.RepoLink + "/pulls/" + com.ToStr(issue.Index)) + return + } else if strings.Contains(err.Error(), "Wrong commit ID") { + ctx.Flash.Error(ctx.Tr("repo.pulls.wrong_commit_id")) + ctx.Redirect(ctx.Repo.RepoLink + "/pulls/" + com.ToStr(issue.Index)) + return + } + + ctx.ServerError("MergedManually", err) + return + } + + ctx.Redirect(ctx.Repo.RepoLink + "/pulls/" + com.ToStr(issue.Index)) + return + } + + if !pr.CanAutoMerge() { + ctx.Flash.Error(ctx.Tr("repo.pulls.no_merge_not_ready")) + ctx.Redirect(ctx.Repo.RepoLink + "/pulls/" + com.ToStr(issue.Index)) return } diff --git a/routers/repo/setting.go b/routers/repo/setting.go index b35828d7b1..692d65b44c 100644 --- a/routers/repo/setting.go +++ b/routers/repo/setting.go @@ -321,6 +321,8 @@ func SettingsPost(ctx *context.Context) { AllowRebase: form.PullsAllowRebase, AllowRebaseMerge: form.PullsAllowRebaseMerge, AllowSquash: form.PullsAllowSquash, + AllowManualMerge: form.PullsAllowManualMerge, + AutodetectManualMerge: form.EnableAutodetectManualMerge, }, }) } else if !models.UnitTypePullRequests.UnitGlobalDisabled() { diff --git a/services/pull/check.go b/services/pull/check.go index 5acee8174b..3ec76de5e8 100644 --- a/services/pull/check.go +++ b/services/pull/check.go @@ -116,7 +116,7 @@ func getMergeCommit(pr *models.PullRequest) (*git.Commit, error) { if err != nil { return nil, fmt.Errorf("git rev-list --ancestry-path --merges --reverse: %v", err) } else if len(mergeCommit) < 40 { - // PR was fast-forwarded, so just use last commit of PR + // PR was maybe fast-forwarded, so just use last commit of PR mergeCommit = commitID[:40] } @@ -137,6 +137,21 @@ func getMergeCommit(pr *models.PullRequest) (*git.Commit, error) { // manuallyMerged checks if a pull request got manually merged // When a pull request got manually merged mark the pull request as merged func manuallyMerged(pr *models.PullRequest) bool { + if err := pr.LoadBaseRepo(); err != nil { + log.Error("PullRequest[%d].LoadBaseRepo: %v", pr.ID, err) + return false + } + + if unit, err := pr.BaseRepo.GetUnit(models.UnitTypePullRequests); err == nil { + config := unit.PullRequestsConfig() + if !config.AutodetectManualMerge { + return false + } + } else { + log.Error("PullRequest[%d].BaseRepo.GetUnit(models.UnitTypePullRequests): %v", pr.ID, err) + return false + } + commit, err := getMergeCommit(pr) if err != nil { log.Error("PullRequest[%d].getMergeCommit: %v", pr.ID, err) diff --git a/services/pull/merge.go b/services/pull/merge.go index 9d6eadc524..bbe631f68c 100644 --- a/services/pull/merge.go +++ b/services/pull/merge.go @@ -615,3 +615,54 @@ func CheckPRReadyToMerge(pr *models.PullRequest, skipProtectedFilesCheck bool) ( return nil } + +// MergedManually mark pr as merged manually +func MergedManually(pr *models.PullRequest, doer *models.User, baseGitRepo *git.Repository, commitID string) (err error) { + prUnit, err := pr.BaseRepo.GetUnit(models.UnitTypePullRequests) + if err != nil { + return + } + prConfig := prUnit.PullRequestsConfig() + + // Check if merge style is correct and allowed + if !prConfig.IsMergeStyleAllowed(models.MergeStyleManuallyMerged) { + return models.ErrInvalidMergeStyle{ID: pr.BaseRepo.ID, Style: models.MergeStyleManuallyMerged} + } + + if len(commitID) < 40 { + return fmt.Errorf("Wrong commit ID") + } + + commit, err := baseGitRepo.GetCommit(commitID) + if err != nil { + if git.IsErrNotExist(err) { + return fmt.Errorf("Wrong commit ID") + } + return + } + + ok, err := baseGitRepo.IsCommitInBranch(commitID, pr.BaseBranch) + if err != nil { + return + } + if !ok { + return fmt.Errorf("Wrong commit ID") + } + + pr.MergedCommitID = commitID + pr.MergedUnix = timeutil.TimeStamp(commit.Author.When.Unix()) + pr.Status = models.PullRequestStatusManuallyMerged + pr.Merger = doer + pr.MergerID = doer.ID + + merged := false + if merged, err = pr.SetMerged(); err != nil { + return + } else if !merged { + return fmt.Errorf("SetMerged failed") + } + + notification.NotifyMergePullRequest(pr, doer) + log.Info("manuallyMerged[%d]: Marked as manually merged into %s/%s by commit id: %s", pr.ID, pr.BaseRepo.Name, pr.BaseBranch, commit.ID.String()) + return nil +} diff --git a/services/pull/patch.go b/services/pull/patch.go index 2692d848c4..72b459bf2c 100644 --- a/services/pull/patch.go +++ b/services/pull/patch.go @@ -80,7 +80,7 @@ func TestPatch(pr *models.PullRequest) error { pr.MergeBase = strings.TrimSpace(pr.MergeBase) // 2. Check for conflicts - if conflicts, err := checkConflicts(pr, gitRepo, tmpBasePath); err != nil || conflicts { + if conflicts, err := checkConflicts(pr, gitRepo, tmpBasePath); err != nil || conflicts || pr.Status == models.PullRequestStatusEmpty { return err } @@ -125,8 +125,9 @@ func checkConflicts(pr *models.PullRequest, gitRepo *git.Repository, tmpBasePath // 1a. if the size of that patch is 0 - there can be no conflicts! if stat.Size() == 0 { log.Debug("PullRequest[%d]: Patch is empty - ignoring", pr.ID) - pr.Status = models.PullRequestStatusMergeable + pr.Status = models.PullRequestStatusEmpty pr.ConflictedFiles = []string{} + pr.ChangedProtectedFiles = []string{} return false, nil } diff --git a/templates/repo/diff/compare.tmpl b/templates/repo/diff/compare.tmpl index 245bdaa542..071a790457 100644 --- a/templates/repo/diff/compare.tmpl +++ b/templates/repo/diff/compare.tmpl @@ -83,7 +83,17 @@ {{end}} {{if .IsNothingToCompare}} -
{{.i18n.Tr "repo.pulls.nothing_to_compare"}}
+ {{if and $.IsSigned $.AllowEmptyPr (not .Repository.IsArchived) }} +
{{.i18n.Tr "repo.pulls.nothing_to_compare_and_allow_empty_pr"}}
+
+ +
+ + {{else}} +
{{.i18n.Tr "repo.pulls.nothing_to_compare"}}
+ {{end}} {{else if and .PageIsComparePull (gt .CommitCount 0)}} {{if .HasPullRequest}}
diff --git a/templates/repo/issue/view_content/comments.tmpl b/templates/repo/issue/view_content/comments.tmpl index cfacde9648..b8d78f5697 100644 --- a/templates/repo/issue/view_content/comments.tmpl +++ b/templates/repo/issue/view_content/comments.tmpl @@ -124,7 +124,11 @@ {{.Poster.GetDisplayName}} {{$link := printf "%s/commit/%s" $.Repository.HTMLURL $.Issue.PullRequest.MergedCommitID}} - {{$.i18n.Tr "repo.issues.pull_merged_at" $link (ShortSha $.Issue.PullRequest.MergedCommitID) ($.BaseTarget|Escape) $createdStr | Str2html}} + {{if eq $.Issue.PullRequest.Status 3}} + {{$.i18n.Tr "repo.issues.manually_pull_merged_at" $link (ShortSha $.Issue.PullRequest.MergedCommitID) $.BaseTarget $createdStr | Str2html}} + {{else}} + {{$.i18n.Tr "repo.issues.pull_merged_at" $link (ShortSha $.Issue.PullRequest.MergedCommitID) $.BaseTarget $createdStr | Str2html}} + {{end}}
{{else if eq .Type 3 5 6}} diff --git a/templates/repo/issue/view_content/pull.tmpl b/templates/repo/issue/view_content/pull.tmpl index 9e883c0a93..2175fad067 100644 --- a/templates/repo/issue/view_content/pull.tmpl +++ b/templates/repo/issue/view_content/pull.tmpl @@ -118,6 +118,7 @@ {{- else if and .EnableStatusCheck (or (not $.LatestCommitStatus) .RequiredStatusCheckState.IsPending .RequiredStatusCheckState.IsWarning)}}yellow {{- else if and .AllowMerge .RequireSigned (not .WillSign)}}red {{- else if .Issue.PullRequest.IsChecking}}yellow + {{- else if .Issue.PullRequest.IsEmpty}}grey {{- else if .Issue.PullRequest.CanAutoMerge}}green {{- else}}red{{end}}">{{svg "octicon-git-merge" 32}}
@@ -128,7 +129,11 @@
{{if .Issue.PullRequest.MergedCommitID}} {{$link := printf "%s/commit/%s" $.Repository.HTMLURL .Issue.PullRequest.MergedCommitID}} - {{$.i18n.Tr "repo.pulls.merged_as" $link (ShortSha .Issue.PullRequest.MergedCommitID) | Safe}} + {{if eq $.Issue.PullRequest.Status 3}} + {{$.i18n.Tr "repo.pulls.manually_merged_as" $link (ShortSha .Issue.PullRequest.MergedCommitID) | Safe}} + {{else}} + {{$.i18n.Tr "repo.pulls.merged_as" $link (ShortSha .Issue.PullRequest.MergedCommitID) | Safe}} + {{end}} {{else}} {{$.i18n.Tr "repo.pulls.has_merged"}} {{end}} @@ -176,6 +181,11 @@ {{svg "octicon-sync"}} {{$.i18n.Tr "repo.pulls.is_checking"}}
+ {{else if .Issue.PullRequest.IsEmpty}} +
+ {{svg "octicon-alert" 16}} + {{$.i18n.Tr "repo.pulls.is_empty"}} +
{{else if .Issue.PullRequest.CanAutoMerge}} {{if .IsBlockedByApprovals}}
@@ -350,6 +360,22 @@
{{end}} + {{if and $prUnit.PullRequestsConfig.AllowManualMerge $.IsRepoAdmin}} + + {{end}}
{{if gt $prUnit.PullRequestsConfig.AllowedMergeStyleCount 1}} @@ -385,6 +414,9 @@ {{if $prUnit.PullRequestsConfig.AllowSquash}}
{{$.i18n.Tr "repo.pulls.squash_merge_pull_request"}}
{{end}} + {{if and $prUnit.PullRequestsConfig.AllowManualMerge $.IsRepoAdmin}} +
{{$.i18n.Tr "repo.pulls.merge_manually"}}
+ {{end}}
{{end}} @@ -492,6 +524,30 @@ {{end}}
{{end}} + + {{if $.StillCanManualMerge}} +
+ + +
+ +
+ {{end}} diff --git a/templates/repo/issue/view_title.tmpl b/templates/repo/issue/view_title.tmpl index 2f2787d67c..f6cbb9206c 100644 --- a/templates/repo/issue/view_title.tmpl +++ b/templates/repo/issue/view_title.tmpl @@ -20,7 +20,7 @@ {{end}} {{if .HasMerged}} -
{{svg "octicon-git-merge"}} {{.i18n.Tr "repo.pulls.merged"}}
+
{{svg "octicon-git-merge" 16}} {{if eq .Issue.PullRequest.Status 3}}{{.i18n.Tr "repo.pulls.manually_merged"}}{{else}}{{.i18n.Tr "repo.pulls.merged"}}{{end}}
{{else if .Issue.IsClosed}}
{{if .Issue.IsPull}}{{svg "octicon-git-pull-request"}}{{else}}{{svg "octicon-issue-closed"}}{{end}} {{.i18n.Tr "repo.issues.closed_title"}}
{{else if .Issue.IsPull}} diff --git a/templates/repo/settings/options.tmpl b/templates/repo/settings/options.tmpl index b69f90f9c5..9d87101671 100644 --- a/templates/repo/settings/options.tmpl +++ b/templates/repo/settings/options.tmpl @@ -330,6 +330,18 @@ +
+
+ + +
+
+
+
+ + +
+
{{end}} diff --git a/templates/swagger/v1_json.tmpl b/templates/swagger/v1_json.tmpl index 1b1d9e5c97..930af907ea 100644 --- a/templates/swagger/v1_json.tmpl +++ b/templates/swagger/v1_json.tmpl @@ -13586,6 +13586,11 @@ "description": "EditRepoOption options when editing a repository's properties", "type": "object", "properties": { + "allow_manual_merge": { + "description": "either `true` to allow mark pr as merged manually, or `false` to prevent it. `has_pull_requests` must be `true`.", + "type": "boolean", + "x-go-name": "AllowManualMerge" + }, "allow_merge_commits": { "description": "either `true` to allow merging pull requests with a merge commit, or `false` to prevent merging pull requests with merge commits. `has_pull_requests` must be `true`.", "type": "boolean", @@ -13611,6 +13616,11 @@ "type": "boolean", "x-go-name": "Archived" }, + "autodetect_manual_merge": { + "description": "either `true` to enable AutodetectManualMerge, or `false` to prevent it. `has_pull_requests` must be `true`, Note: In some special cases, misjudgments can occur.", + "type": "boolean", + "x-go-name": "AutodetectManualMerge" + }, "default_branch": { "description": "sets the default branch for this repository.", "type": "string", @@ -14596,9 +14606,13 @@ "merge", "rebase", "rebase-merge", - "squash" + "squash", + "manually-merged" ] }, + "MergeCommitID": { + "type": "string" + }, "MergeMessageField": { "type": "string" },