Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make manual merge autodetection optional and add manual merge as merge method #12543

Merged
merged 34 commits into from
Mar 4, 2021
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
6f17944
Make auto check manual merge as a chooseable mod and add manual merge…
a1012112796 Aug 20, 2020
adab611
Merge branch 'master' into empty_pr
a1012112796 Aug 20, 2020
52d0a6b
make swager
a1012112796 Aug 20, 2020
510cc6b
api support
a1012112796 Aug 20, 2020
6edd86e
ping ci
a1012112796 Aug 20, 2020
609e9da
fix TestPullCreate_EmptyChangesWithCommits
a1012112796 Aug 20, 2020
0a566a3
Apply suggestions from code review
a1012112796 Aug 21, 2020
fe98c68
Apply review suggestions and add test
a1012112796 Aug 21, 2020
e483115
Apply suggestions from code review
a1012112796 Aug 22, 2020
2e71aff
Merge branch 'master' into empty_pr
a1012112796 Aug 22, 2020
7c97116
fix build
a1012112796 Aug 22, 2020
ed8f276
test error message
a1012112796 Aug 22, 2020
c14facd
make fmt
a1012112796 Aug 22, 2020
b958762
Fix indentation issues identified by @silverwind
zeripath Aug 23, 2020
c816cc0
Merge remote-tracking branch 'origin/master' into PR-12543-empty-pr
zeripath Aug 23, 2020
9b3d4b2
Merge remote-tracking branch 'origin/master' into PR-12543-empty-pr
zeripath Aug 23, 2020
5d1b12c
Fix tests and make manually merged disabled error on API the same
zeripath Aug 23, 2020
2cde228
a small nit
a1012112796 Aug 24, 2020
a9fa07e
Merge branch 'master' into empty_pr
zeripath Aug 24, 2020
a5fcf91
fix wrong commit id error
a1012112796 Aug 24, 2020
18e8627
Merge branch 'master' into empty_pr
a1012112796 Oct 23, 2020
7863e53
Merge branch 'master' into empty_pr
a1012112796 Jan 25, 2021
3fbdb5b
fix bug
a1012112796 Jan 25, 2021
c6864ab
simple test
a1012112796 Jan 25, 2021
149cc8b
fix test
a1012112796 Jan 25, 2021
b883e5a
Merge branch 'master' into empty_pr
a1012112796 Jan 26, 2021
14f7d33
Merge branch 'master' into empty_pr
a1012112796 Jan 27, 2021
bfcfb44
Merge branch 'master' into empty_pr
a1012112796 Jan 30, 2021
16ff24f
Merge branch 'master' into empty_pr
a1012112796 Feb 2, 2021
59f225f
Merge branch 'master' into empty_pr
a1012112796 Feb 7, 2021
eea1645
Merge branch 'master' into empty_pr
a1012112796 Feb 15, 2021
f80d860
Merge branch 'master' into empty_pr
a1012112796 Feb 26, 2021
2abc0d4
Merge branch 'master' into empty_pr
a1012112796 Mar 4, 2021
c15efe7
Merge branch 'master' into empty_pr
techknowlogick Mar 4, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions integrations/pull_status_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ func TestPullCreate_EmptyChangesWithCommits(t *testing.T) {
resp := session.MakeRequest(t, req, http.StatusOK)
doc := NewHTMLParser(t, resp.Body)

text := strings.TrimSpace(doc.doc.Find(".item.text.green").Text())
assert.EqualValues(t, "This pull request can be merged automatically.", text)
text := strings.TrimSpace(doc.doc.Find(".item.text.grey").Text())
assert.EqualValues(t, "This branche is equal with target branch.", text)
a1012112796 marked this conversation as resolved.
Show resolved Hide resolved
})
}
8 changes: 8 additions & 0 deletions models/pull.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ const (
PullRequestStatusMergeable
PullRequestStatusManuallyMerged
PullRequestStatusError
PullRequestStatusEmpty
)

// PullRequest represents relation between pull request and repositories.
Expand Down Expand Up @@ -329,6 +330,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

Expand All @@ -341,6 +347,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
Expand Down
5 changes: 4 additions & 1 deletion models/repo_unit.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,8 @@ type PullRequestsConfig struct {
AllowRebase bool
AllowRebaseMerge bool
AllowSquash bool
AllowManualMerge bool
AutoCheckManualMerge bool
a1012112796 marked this conversation as resolved.
Show resolved Hide resolved
}

// FromDB fills up a PullRequestsConfig from serialized format.
Expand All @@ -110,7 +112,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
}

// BeforeSet is invoked from XORM before setting the value of a field of this object.
Expand Down
9 changes: 6 additions & 3 deletions modules/auth/repo_form.go
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,8 @@ type RepoSettingForm struct {
PullsAllowRebase bool
PullsAllowRebaseMerge bool
PullsAllowSquash bool
PullsAllowManualMerge bool
EnableAutoCheckManualMerge bool
EnableTimetracker bool
AllowOnlyContributorsToTrackTime bool
EnableIssueDependencies bool
Expand Down Expand Up @@ -513,11 +515,12 @@ func (f *InitializeLabelsForm) Validate(ctx *macaron.Context, errs binding.Error
// 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
Expand Down
4 changes: 4 additions & 0 deletions modules/structs/repo.go
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,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 AutoCheckManualMerge, or `false` to prevent it. `has_pull_requests` must be `true`, Note: In some special cases, misjudgments can occur.
AutoCheckManualMerge *bool `json:"auto_check_manual_merge,omitempty"`
// set to `true` to archive this repository.
Archived *bool `json:"archived,omitempty"`
}
Expand Down
10 changes: 10 additions & 0 deletions options/locale/locale_en-US.ini
Original file line number Diff line number Diff line change
Expand Up @@ -1004,6 +1004,7 @@ issues.context.delete = Delete
issues.no_content = There is no content yet.
issues.close_issue = Close
issues.pull_merged_at = `merged commit <a href="%[1]s">%[2]s</a> into <b>%[3]s</b> %[4]s`
issues.manually_pull_merged_at = `merged commit <a href="%[1]s">%[2]s</a> into <b>%[3]s</b> %[4]s manually`
issues.close_comment_issue = Comment and Close
issues.reopen_issue = Reopen
issues.reopen_comment_issue = Comment and Reopen
Expand Down Expand Up @@ -1164,6 +1165,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. But you still can create pr from it
a1012112796 marked this conversation as resolved.
Show resolved Hide resolved
pulls.has_pull_request = `A pull request between these branches already exists: <a href="%[1]s/pulls/%[3]d">%[2]s#%[3]d</a>`
pulls.create = Create Pull Request
pulls.title_desc = wants to merge %[1]d commits from <code>%[2]s</code> into <code id="branch_target">%[3]s</code>
Expand All @@ -1176,13 +1178,16 @@ 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 <a rel="nofollow" class="ui sha" href="%[1]s"><code>%[2]s</code></a>.
pulls.manually_merged = Merged manually
a1012112796 marked this conversation as resolved.
Show resolved Hide resolved
pulls.manually_merged_as = The pull request has been merged as <a rel="nofollow" class="ui sha" href="%[1]s"><code>%[2]s</code></a> manually.
a1012112796 marked this conversation as resolved.
Show resolved Hide resolved
pulls.is_closed = The pull request has been closed.
pulls.has_merged = The pull request has been merged.
pulls.title_wip_desc = `<a href="#">Start the title with <strong>%s</strong></a> to prevent the pull request from being merged accidentally.`
pulls.cannot_merge_work_in_progress = This pull request is marked as a work in progress. Remove the <strong>%s</strong> prefix from the title when it's ready
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 branche is equal with target branch."
a1012112796 marked this conversation as resolved.
Show resolved Hide resolved
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.
Expand All @@ -1200,6 +1205,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 is wrong, need real commit id on base branch"
a1012112796 marked this conversation as resolved.
Show resolved Hide resolved

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.
Expand All @@ -1210,6 +1216,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: %[1]s<br>%[2]s<br>Hint: Try a different strategy
Expand Down Expand Up @@ -1419,6 +1427,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 merged manually
settings.pulls.enable_auto_check_manual_merge = Enable auto check manual merge (Note: In some special cases, misjudgments can occur)
a1012112796 marked this conversation as resolved.
Show resolved Hide resolved
settings.projects_desc = Enable Repository Projects
settings.admin_settings = Administrator Settings
settings.admin_enable_health_check = Enable Repository Health Checks (git fsck)
Expand Down
65 changes: 61 additions & 4 deletions routers/api/v1/repo/pull.go
Original file line number Diff line number Diff line change
Expand Up @@ -759,13 +759,70 @@ func MergePullRequest(ctx *context.APIContext, form auth.MergePullRequestForm) {
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 {
prUnit, err := pr.BaseRepo.GetUnit(models.UnitTypePullRequests)
if err != nil {
ctx.ServerError("pr.BaseRepo.GetUnit(models.UnitTypePullRequests): %v", err)
return
}
prConfig := prUnit.PullRequestsConfig()

// Check if merge style is correct and allowed
if !prConfig.IsMergeStyleAllowed(models.MergeStyleManuallyMerged) {
ctx.Error(http.StatusMethodNotAllowed, "manually-merged is not allowed", "")
return
}

if len(form.MergeCommitID) < 40 {
ctx.Error(http.StatusMethodNotAllowed, "commit id is wrong, need real commit id on base branch", "")
return
}

commit, err := ctx.Repo.GitRepo.GetCommit(form.MergeCommitID)
if err != nil {
ctx.ServerError("ctx.Repo.GitRepo.GetCommit", err)
return
}

branchName, err := commit.GetBranchName()
if err != nil {
ctx.ServerError("commit.GetBranchName", err)
return
}

if branchName != pr.BaseBranch {
ctx.Error(http.StatusMethodNotAllowed, "commit id is wrong, need real commit id on base branch", "")
return
}
zeripath marked this conversation as resolved.
Show resolved Hide resolved

pr.MergedCommitID = commit.ID.String()
pr.MergedUnix = timeutil.TimeStamp(commit.Author.When.Unix())
pr.Status = models.PullRequestStatusManuallyMerged
pr.Merger = ctx.User
pr.MergerID = ctx.User.ID

if merged, err := pr.SetMerged(); err != nil {
log.Error("PullRequest[%d].setMerged : %v", pr.ID, err)
ctx.Status(500)
return
} else if !merged {
return
}

notification.NotifyMergePullRequest(pr, ctx.User)
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())
ctx.Status(http.StatusOK)
a1012112796 marked this conversation as resolved.
Show resolved Hide resolved
return
}

if !pr.CanAutoMerge() {
ctx.Error(http.StatusMethodNotAllowed, "PR not in mergeable state", "Please try again later")
return
}

Expand Down
8 changes: 8 additions & 0 deletions routers/api/v1/repo/repo.go
Original file line number Diff line number Diff line change
Expand Up @@ -688,6 +688,8 @@ func updateRepoUnits(ctx *context.APIContext, opts api.EditRepoOption) error {
AllowRebase: true,
AllowRebaseMerge: true,
AllowSquash: true,
AllowManualMerge: true,
AutoCheckManualMerge: false,
}
} else {
config = unit.PullRequestsConfig()
Expand All @@ -708,6 +710,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.AutoCheckManualMerge != nil {
config.AutoCheckManualMerge = *opts.AutoCheckManualMerge
}

units = append(units, models.RepoUnit{
RepoID: repo.ID,
Expand Down
8 changes: 8 additions & 0 deletions routers/repo/compare.go
Original file line number Diff line number Diff line change
Expand Up @@ -437,6 +437,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.AutoCheckManualMerge {
ctx.Data["AllowEmptyPr"] = true
} else {
ctx.Data["AllowEmptyPr"] = false
}
}
return true
}

Expand Down
6 changes: 6 additions & 0 deletions routers/repo/issue.go
Original file line number Diff line number Diff line change
Expand Up @@ -1183,6 +1183,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"] = ""
}
Expand Down Expand Up @@ -1223,6 +1225,10 @@ func ViewIssue(ctx *context.Context) {
ctx.ServerError("GetReviewersByIssueID", err)
return
}

ctx.Data["StillCanManualMerge"] = !pull.CanAutoMerge() && !pull.IsChecking() &&
!pull.IsWorkInProgress() && !pull.HasMerged && !issue.IsClosed &&
(ctx.IsSigned && (ctx.Repo.IsAdmin() || ctx.User.IsAdmin)) && prConfig.AllowManualMerge
}

// Get Dependencies
Expand Down
68 changes: 64 additions & 4 deletions routers/repo/pull.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import (
"code.gitea.io/gitea/modules/repofiles"
"code.gitea.io/gitea/modules/setting"
"code.gitea.io/gitea/modules/structs"
"code.gitea.io/gitea/modules/timeutil"
"code.gitea.io/gitea/modules/util"
"code.gitea.io/gitea/routers/utils"
"code.gitea.io/gitea/services/gitdiff"
Expand Down Expand Up @@ -754,14 +755,73 @@ func MergePullRequest(ctx *context.Context, form auth.MergePullRequestForm) {
return
}

if !pr.CanAutoMerge() {
ctx.Flash.Error(ctx.Tr("repo.pulls.no_merge_not_ready"))
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"))
// handle manually-merged mark
if models.MergeStyle(form.Do) == models.MergeStyleManuallyMerged {
prUnit, err := pr.BaseRepo.GetUnit(models.UnitTypePullRequests)
if err != nil {
ctx.ServerError("pr.BaseRepo.GetUnit(models.UnitTypePullRequests): %v", err)
return
}
prConfig := prUnit.PullRequestsConfig()

// Check if merge style is correct and allowed
if !prConfig.IsMergeStyleAllowed(models.MergeStyleManuallyMerged) {
ctx.Status(403)
return
}

if len(form.MergeCommitID) < 40 {
ctx.Flash.Error(ctx.Tr("repo.pulls.wrong_commit_id"))
ctx.Redirect(ctx.Repo.RepoLink + "/pulls/" + com.ToStr(issue.Index))
return
}

commit, err := ctx.Repo.GitRepo.GetCommit(form.MergeCommitID)
if err != nil {
ctx.ServerError("ctx.Repo.GitRepo.GetCommit", err)
return
}

branchName, err := commit.GetBranchName()
if err != nil {
ctx.ServerError("commit.GetBranchName", err)
return
}

if branchName != pr.BaseBranch {
ctx.Flash.Error(ctx.Tr("repo.pulls.wrong_commit_id"))
ctx.Redirect(ctx.Repo.RepoLink + "/pulls/" + com.ToStr(issue.Index))
return
}

pr.MergedCommitID = commit.ID.String()
pr.MergedUnix = timeutil.TimeStamp(commit.Author.When.Unix())
pr.Status = models.PullRequestStatusManuallyMerged
pr.Merger = ctx.User
pr.MergerID = ctx.User.ID

if merged, err := pr.SetMerged(); err != nil {
log.Error("PullRequest[%d].setMerged : %v", pr.ID, err)
ctx.Status(500)
return
} else if !merged {
return
}

notification.NotifyMergePullRequest(pr, ctx.User)
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())
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
}
Expand Down
2 changes: 2 additions & 0 deletions routers/repo/setting.go
Original file line number Diff line number Diff line change
Expand Up @@ -303,6 +303,8 @@ func SettingsPost(ctx *context.Context, form auth.RepoSettingForm) {
AllowRebase: form.PullsAllowRebase,
AllowRebaseMerge: form.PullsAllowRebaseMerge,
AllowSquash: form.PullsAllowSquash,
AllowManualMerge: form.PullsAllowManualMerge,
AutoCheckManualMerge: form.EnableAutoCheckManualMerge,
},
})
} else if !models.UnitTypePullRequests.UnitGlobalDisabled() {
Expand Down
Loading