From 298c1e5ea281f6bf8b9bca8907191aa7dcdfa1ac Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Tue, 16 Jul 2019 00:41:37 +0800 Subject: [PATCH 01/18] refuse merge until ci successfully --- models/branches.go | 2 ++ models/commit_status.go | 21 ++++++++++++++++++ modules/auth/repo_form.go | 2 ++ options/locale/locale_en-US.ini | 1 + routers/repo/setting_protected_branch.go | 16 ++++++++++++++ templates/repo/settings/protected_branch.tmpl | 22 +++++++++++++++++++ 6 files changed, 64 insertions(+) diff --git a/models/branches.go b/models/branches.go index 2a99d98955da1..9daaa487e79cd 100644 --- a/models/branches.go +++ b/models/branches.go @@ -36,6 +36,8 @@ type ProtectedBranch struct { EnableMergeWhitelist bool `xorm:"NOT NULL DEFAULT false"` MergeWhitelistUserIDs []int64 `xorm:"JSON TEXT"` MergeWhitelistTeamIDs []int64 `xorm:"JSON TEXT"` + EnableStatusCheck bool `xorm:"NOT NULL DEFAULT false"` + StatusCheckContexts []string `xorm:"JSON TEXT"` ApprovalsWhitelistUserIDs []int64 `xorm:"JSON TEXT"` ApprovalsWhitelistTeamIDs []int64 `xorm:"JSON TEXT"` RequiredApprovals int64 `xorm:"NOT NULL DEFAULT 0"` diff --git a/models/commit_status.go b/models/commit_status.go index 9f0a32cdfb81e..11618b0b2c6a6 100644 --- a/models/commit_status.go +++ b/models/commit_status.go @@ -9,6 +9,7 @@ import ( "crypto/sha1" "fmt" "strings" + "time" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/setting" @@ -205,6 +206,26 @@ func GetLatestCommitStatus(repo *Repository, sha string, page int) ([]*CommitSta return statuses, x.In("id", ids).Find(&statuses) } +// FindRepoStatusCheckContexts returns repo's status check contexts +func FindRepoStatusCheckContexts(repoID int64, before time.Duration) ([]string, error) { + start := timeutil.TimeStampNow().AddDuration(-before) + ids := make([]int64, 0, 10) + err := x.Table("commit_status"). + Where("repo_id = ?", repoID). + And("updated_unix >= ?", start). + Select("max( id ) as id"). + GroupBy("context_hash").OrderBy("max( id ) desc").Find(&ids) + if err != nil { + return nil, err + } + var contexts = make([]string, 0, len(ids)) + if len(ids) == 0 { + return contexts, nil + } + return contexts, x.Select("context").Table("commit_status").In("id", ids).Find(&contexts) + +} + // NewCommitStatusOptions holds options for creating a CommitStatus type NewCommitStatusOptions struct { Repo *Repository diff --git a/modules/auth/repo_form.go b/modules/auth/repo_form.go index 56ae77a7f74dc..dea1b33fe52c5 100644 --- a/modules/auth/repo_form.go +++ b/modules/auth/repo_form.go @@ -155,6 +155,8 @@ type ProtectBranchForm struct { EnableMergeWhitelist bool MergeWhitelistUsers string MergeWhitelistTeams string + EnableStatusCheck bool `xorm:"NOT NULL DEFAULT false"` + StatusCheckContexts string RequiredApprovals int64 ApprovalsWhitelistUsers string ApprovalsWhitelistTeams string diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index 8a1dc95b9fa18..273437e12c090 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -1311,6 +1311,7 @@ settings.protect_merge_whitelist_committers = Enable Merge Whitelist settings.protect_merge_whitelist_committers_desc = Allow only whitelisted users or teams to merge pull requests into this branch. settings.protect_merge_whitelist_users = Whitelisted users for merging: settings.protect_merge_whitelist_teams = Whitelisted teams for merging: +settings.protect_check_status_contexts = Status checks found in the last week for this repository settings.protect_required_approvals = Required approvals: settings.protect_required_approvals_desc = Allow only to merge pull request with enough positive reviews of whitelisted users or teams. settings.protect_approvals_whitelist_users = Whitelisted reviewers: diff --git a/routers/repo/setting_protected_branch.go b/routers/repo/setting_protected_branch.go index b5a115b6a464d..175b167997cb5 100644 --- a/routers/repo/setting_protected_branch.go +++ b/routers/repo/setting_protected_branch.go @@ -7,6 +7,7 @@ package repo import ( "fmt" "strings" + "time" "code.gitea.io/gitea/models" "code.gitea.io/gitea/modules/auth" @@ -125,6 +126,15 @@ func SettingsProtectedBranch(c *context.Context) { c.Data["whitelist_users"] = strings.Join(base.Int64sToStrings(protectBranch.WhitelistUserIDs), ",") c.Data["merge_whitelist_users"] = strings.Join(base.Int64sToStrings(protectBranch.MergeWhitelistUserIDs), ",") c.Data["approvals_whitelist_users"] = strings.Join(base.Int64sToStrings(protectBranch.ApprovalsWhitelistUserIDs), ",") + c.Data["branch_status_check_contexts"], _ = models.FindRepoStatusCheckContexts(c.Repo.Repository.ID, 4*7*24*time.Hour) // Find last week status check contexts + c.Data["is_context_required"] = func(context string) bool { + for _, c := range protectBranch.StatusCheckContexts { + if c == context { + return true + } + } + return false + } if c.Repo.Owner.IsOrganization() { teams, err := c.Repo.Owner.TeamsWithAccessToRepo(c.Repo.Repository.ID, models.AccessModeRead) @@ -183,6 +193,12 @@ func SettingsProtectedBranchPost(ctx *context.Context, f auth.ProtectBranchForm) if strings.TrimSpace(f.MergeWhitelistUsers) != "" { mergeWhitelistUsers, _ = base.StringsToInt64s(strings.Split(f.MergeWhitelistUsers, ",")) } + if strings.TrimSpace(f.MergeWhitelistTeams) != "" { + mergeWhitelistTeams, _ = base.StringsToInt64s(strings.Split(f.MergeWhitelistTeams, ",")) + } + protectBranch.EnableStatusCheck = f.EnableStatusCheck + protectBranch.StatusCheckContexts = strings.Split(f.StatusCheckContexts, ",") + if strings.TrimSpace(f.MergeWhitelistTeams) != "" { mergeWhitelistTeams, _ = base.StringsToInt64s(strings.Split(f.MergeWhitelistTeams, ",")) } diff --git a/templates/repo/settings/protected_branch.tmpl b/templates/repo/settings/protected_branch.tmpl index 066350f97a50f..c1a85609ee0ed 100644 --- a/templates/repo/settings/protected_branch.tmpl +++ b/templates/repo/settings/protected_branch.tmpl @@ -104,6 +104,28 @@ {{end}} +
+ + + + + + + {{range $.branch_status_check_contexts}} + + {{end}} + +
+ {{.i18n.Tr "repo.settings.protect_check_status_contexts"}} +
+ + + + {{.}} + {{if call $.is_context_required .}}
Required
{{end}} +
+
+
From afad473544abcf86a666eff33d325abcad35137f Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Tue, 16 Jul 2019 11:18:29 +0800 Subject: [PATCH 02/18] deny merge request when required status checkes not succeed on merge Post and API --- modules/auth/repo_form.go | 2 +- modules/pull/commit_status.go | 29 ++++++++++ options/locale/locale_en-US.ini | 3 ++ routers/api/v1/repo/pull.go | 36 +++++++++++++ routers/repo/pull.go | 54 +++++++++++++++++++ routers/repo/setting_protected_branch.go | 7 +-- templates/repo/issue/view_content/pull.tmpl | 6 +++ templates/repo/pulls/status.tmpl | 5 +- templates/repo/settings/protected_branch.tmpl | 50 ++++++++++------- 9 files changed, 167 insertions(+), 25 deletions(-) create mode 100644 modules/pull/commit_status.go diff --git a/modules/auth/repo_form.go b/modules/auth/repo_form.go index dea1b33fe52c5..8d10fc1570d3d 100644 --- a/modules/auth/repo_form.go +++ b/modules/auth/repo_form.go @@ -156,7 +156,7 @@ type ProtectBranchForm struct { MergeWhitelistUsers string MergeWhitelistTeams string EnableStatusCheck bool `xorm:"NOT NULL DEFAULT false"` - StatusCheckContexts string + StatusCheckContexts []string RequiredApprovals int64 ApprovalsWhitelistUsers string ApprovalsWhitelistTeams string diff --git a/modules/pull/commit_status.go b/modules/pull/commit_status.go new file mode 100644 index 0000000000000..2ef5514c7a9a0 --- /dev/null +++ b/modules/pull/commit_status.go @@ -0,0 +1,29 @@ +// Copyright 2019 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 "code.gitea.io/gitea/models" + +// IsCommitStatusContextSuccess returns true if all required status check contexts succeed. +func IsCommitStatusContextSuccess(commitStatuses []*models.CommitStatus, requiredContexts []string) bool { + for _, ctx := range requiredContexts { + var found bool + for _, commitStatus := range commitStatuses { + if commitStatus.Context == ctx { + if commitStatus.State != models.CommitStatusSuccess { + return false + } + + found = true + break + } + } + if !found { + return false + } + } + return true +} diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index 273437e12c090..1048475ea5511 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -981,6 +981,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.required_status_check_failed = Some required checks were not successful. pulls.blocked_by_approvals = "This Pull Request doesn't have enough approvals yet. %d of %d approvals granted." pulls.can_auto_merge_desc = This pull request can be merged automatically. pulls.cannot_auto_merge_desc = This pull request cannot be merged automatically due to conflicts. @@ -988,6 +989,7 @@ pulls.cannot_auto_merge_helper = Merge manually to resolve the conflicts. 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. pulls.no_merge_wip = This pull request can not be merged because it is marked as being a work in progress. +pulls.no_merge_status_check = This pull request cannot be merged because not all required status checkes are successful. 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) @@ -1312,6 +1314,7 @@ settings.protect_merge_whitelist_committers_desc = Allow only whitelisted users settings.protect_merge_whitelist_users = Whitelisted users for merging: settings.protect_merge_whitelist_teams = Whitelisted teams for merging: settings.protect_check_status_contexts = Status checks found in the last week for this repository +settings.protect_check_status_contexts_desc = Require status checks to pass before merging Choose which status checks must pass before branches can be merged into a branch that matches this rule. When enabled, commits must first be pushed to another branch, then merged or pushed directly to a branch that matches this rule after status checks have passed. settings.protect_required_approvals = Required approvals: settings.protect_required_approvals_desc = Allow only to merge pull request with enough positive reviews of whitelisted users or teams. settings.protect_approvals_whitelist_users = Whitelisted reviewers: diff --git a/routers/api/v1/repo/pull.go b/routers/api/v1/repo/pull.go index 5ac542ec207b2..63ef1acbe8469 100644 --- a/routers/api/v1/repo/pull.go +++ b/routers/api/v1/repo/pull.go @@ -16,6 +16,7 @@ import ( "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/notification" "code.gitea.io/gitea/modules/pull" + pull_service "code.gitea.io/gitea/modules/pull" api "code.gitea.io/gitea/modules/structs" "code.gitea.io/gitea/modules/timeutil" ) @@ -570,6 +571,41 @@ func MergePullRequest(ctx *context.APIContext, form auth.MergePullRequestForm) { return } + // check if all required status checks are successful + headGitRepo, err := git.OpenRepository(pr.HeadRepo.RepoPath()) + if err != nil { + ctx.Error(500, "OpenRepository", err) + return + } + + headBranchExist := headGitRepo.IsBranchExist(pr.HeadBranch) + if !headBranchExist { + ctx.Error(500, "HeadBranchExist is not exist, cannot merge", nil) + return + } + + sha, err := headGitRepo.GetBranchCommitID(pr.HeadBranch) + if err != nil { + ctx.Error(500, "GetBranchCommitID", err) + return + } + + commitStatuses, err := models.GetLatestCommitStatus(ctx.Repo.Repository, sha, 0) + if err != nil { + ctx.Error(500, "GetLatestCommitStatus", err) + return + } + + if err = pr.LoadProtectedBranch(); err != nil { + ctx.Error(500, "GetLatestCommitStatus", err) + return + } + + if pr.ProtectedBranch.EnableStatusCheck && !pull_service.IsCommitStatusContextSuccess(commitStatuses, pr.ProtectedBranch.StatusCheckContexts) { + ctx.Status(405) + return + } + if len(form.Do) == 0 { form.Do = string(models.MergeStyleMerge) } diff --git a/routers/repo/pull.go b/routers/repo/pull.go index 14b8670a207fd..7b3abed27623b 100644 --- a/routers/repo/pull.go +++ b/routers/repo/pull.go @@ -22,6 +22,7 @@ import ( "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/notification" "code.gitea.io/gitea/modules/pull" + pull_service "code.gitea.io/gitea/modules/pull" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/util" "code.gitea.io/gitea/services/gitdiff" @@ -350,6 +351,23 @@ func PrepareViewPullInfo(ctx *context.Context, issue *models.Issue) *git.Compare ctx.Data["LatestCommitStatuses"] = commitStatuses ctx.Data["LatestCommitStatus"] = models.CalcCommitStatus(commitStatuses) } + + if err = pull.LoadProtectedBranch(); err != nil { + ctx.ServerError("GetLatestCommitStatus", err) + return nil + } + + if pull.ProtectedBranch.EnableStatusCheck { + ctx.Data["is_context_required"] = func(context string) bool { + for _, c := range pull.ProtectedBranch.StatusCheckContexts { + if c == context { + return true + } + } + return false + } + ctx.Data["IsRequiredStatusCheckSuccess"] = pull_service.IsCommitStatusContextSuccess(commitStatuses, pull.ProtectedBranch.StatusCheckContexts) + } } } @@ -608,6 +626,42 @@ func MergePullRequest(ctx *context.Context, form auth.MergePullRequestForm) { return } + // check if all required status checks are successful + headGitRepo, err := git.OpenRepository(pr.HeadRepo.RepoPath()) + if err != nil { + ctx.ServerError("OpenRepository", err) + return + } + + headBranchExist := headGitRepo.IsBranchExist(pr.HeadBranch) + if !headBranchExist { + ctx.ServerError("HeadBranchExist is not exist, cannot merge", nil) + return + } + + sha, err := headGitRepo.GetBranchCommitID(pr.HeadBranch) + if err != nil { + ctx.ServerError("GetBranchCommitID", err) + return + } + + commitStatuses, err := models.GetLatestCommitStatus(ctx.Repo.Repository, sha, 0) + if err != nil { + ctx.ServerError("GetLatestCommitStatus", err) + return + } + + if err = pr.LoadProtectedBranch(); err != nil { + ctx.ServerError("GetLatestCommitStatus", err) + return + } + + if pr.ProtectedBranch.EnableStatusCheck && !pull_service.IsCommitStatusContextSuccess(commitStatuses, pr.ProtectedBranch.StatusCheckContexts) { + ctx.Flash.Error(ctx.Tr("repo.pulls.no_merge_status_check")) + ctx.Redirect(ctx.Repo.RepoLink + "/pulls/" + com.ToStr(pr.Index)) + return + } + if ctx.HasError() { ctx.Flash.Error(ctx.Data["ErrorMsg"].(string)) ctx.Redirect(ctx.Repo.RepoLink + "/pulls/" + com.ToStr(pr.Index)) diff --git a/routers/repo/setting_protected_branch.go b/routers/repo/setting_protected_branch.go index 175b167997cb5..8bfc88c205766 100644 --- a/routers/repo/setting_protected_branch.go +++ b/routers/repo/setting_protected_branch.go @@ -196,12 +196,13 @@ func SettingsProtectedBranchPost(ctx *context.Context, f auth.ProtectBranchForm) if strings.TrimSpace(f.MergeWhitelistTeams) != "" { mergeWhitelistTeams, _ = base.StringsToInt64s(strings.Split(f.MergeWhitelistTeams, ",")) } - protectBranch.EnableStatusCheck = f.EnableStatusCheck - protectBranch.StatusCheckContexts = strings.Split(f.StatusCheckContexts, ",") - if strings.TrimSpace(f.MergeWhitelistTeams) != "" { mergeWhitelistTeams, _ = base.StringsToInt64s(strings.Split(f.MergeWhitelistTeams, ",")) } + + protectBranch.EnableStatusCheck = f.EnableStatusCheck + protectBranch.StatusCheckContexts = f.StatusCheckContexts + protectBranch.RequiredApprovals = f.RequiredApprovals if strings.TrimSpace(f.ApprovalsWhitelistUsers) != "" { approvalsWhitelistUsers, _ = base.StringsToInt64s(strings.Split(f.ApprovalsWhitelistUsers, ",")) diff --git a/templates/repo/issue/view_content/pull.tmpl b/templates/repo/issue/view_content/pull.tmpl index f0e39d5e37a5f..97c01235853a9 100644 --- a/templates/repo/issue/view_content/pull.tmpl +++ b/templates/repo/issue/view_content/pull.tmpl @@ -41,6 +41,7 @@ {{else if .IsFilesConflicted}}grey {{else if .IsPullRequestBroken}}red {{else if .IsBlockedByApprovals}}red + {[else if not .IsRequiredStatusCheckSuccess}}red {{else if .Issue.PullRequest.IsChecking}}yellow {{else if .Issue.PullRequest.CanAutoMerge}}green {{else}}red{{end}}"> @@ -104,6 +105,11 @@ {{$.i18n.Tr "repo.pulls.is_checking"}}
+ {{else if not .IsRequiredStatusCheckSuccess}} +
+ + {{$.i18n.Tr "repo.pulls.required_status_check_failed"}} +
{{else if .Issue.PullRequest.CanAutoMerge}}
diff --git a/templates/repo/pulls/status.tmpl b/templates/repo/pulls/status.tmpl index e1401aa8bb83a..6e88c64b9425e 100644 --- a/templates/repo/pulls/status.tmpl +++ b/templates/repo/pulls/status.tmpl @@ -15,7 +15,10 @@
{{template "repo/commit_status" .}} {{.Context}} {{.Description}} -
{{if .TargetURL}}Details{{end}}
+
+ {{if call $.is_context_required .Context}}
Required
{{end}} + {{if .TargetURL}}Details{{end}} +
{{end}} {{end}} \ No newline at end of file diff --git a/templates/repo/settings/protected_branch.tmpl b/templates/repo/settings/protected_branch.tmpl index c1a85609ee0ed..3b5718623738d 100644 --- a/templates/repo/settings/protected_branch.tmpl +++ b/templates/repo/settings/protected_branch.tmpl @@ -104,26 +104,36 @@ {{end}}
-
- - - - - - - {{range $.branch_status_check_contexts}} - - {{end}} - -
- {{.i18n.Tr "repo.settings.protect_check_status_contexts"}} -
- - - - {{.}} - {{if call $.is_context_required .}}
Required
{{end}} -
+
+
+ + +

{{.i18n.Tr "repo.settings.protect_check_status_contexts_desc"}}

+
+
+ +
+
+ + + + + + + {{range $.branch_status_check_contexts}} + + {{end}} + +
+ {{.i18n.Tr "repo.settings.protect_check_status_contexts"}} +
+ + + + {{.}} + {{if call $.is_context_required .}}
Required
{{end}} +
+
From a9bdf391a39d343a1d7ad4acc4dcc118865099da Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Tue, 16 Jul 2019 11:34:11 +0800 Subject: [PATCH 03/18] add database migration for added columns on protected_branch --- models/migrations/migrations.go | 2 ++ models/migrations/v94.go | 24 ++++++++++++++++++++++++ 2 files changed, 26 insertions(+) create mode 100644 models/migrations/v94.go diff --git a/models/migrations/migrations.go b/models/migrations/migrations.go index 15e021c05a796..885043dce51d2 100644 --- a/models/migrations/migrations.go +++ b/models/migrations/migrations.go @@ -242,6 +242,8 @@ var migrations = []Migration{ NewMigration("remove orphaned repository index statuses", removeLingeringIndexStatus), // v93 -> v94 NewMigration("add email notification enabled preference to user", addEmailNotificationEnabledToUser), + // v94 -> v95 + NewMigration("add enable_status_check, status_check_contexts to protected_branch", addStatusCheckColumnsForProtectedBranches), } // Migrate database to current version diff --git a/models/migrations/v94.go b/models/migrations/v94.go new file mode 100644 index 0000000000000..88f188213281e --- /dev/null +++ b/models/migrations/v94.go @@ -0,0 +1,24 @@ +// Copyright 2019 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 migrations + +import "github.com/go-xorm/xorm" + +func addStatusCheckColumnsForProtectedBranches(x *xorm.Engine) error { + type ProtectedBranch struct { + EnableStatusCheck bool `xorm:"NOT NULL DEFAULT false"` + StatusCheckContexts []string `xorm:"JSON TEXT"` + } + + if err := x.Sync2(new(ProtectedBranch)); err != nil { + return err + } + + _, err := x.Update(&ProtectedBranch{ + EnableStatusCheck: false, + StatusCheckContexts: []string{}, + }) + return err +} From 41673ccd240d97b3d7ef9ffc03d7a55b10fb374a Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Tue, 16 Jul 2019 12:43:25 +0800 Subject: [PATCH 04/18] fix migration --- models/migrations/v94.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/models/migrations/v94.go b/models/migrations/v94.go index 88f188213281e..32c7d28203c29 100644 --- a/models/migrations/v94.go +++ b/models/migrations/v94.go @@ -16,7 +16,7 @@ func addStatusCheckColumnsForProtectedBranches(x *xorm.Engine) error { return err } - _, err := x.Update(&ProtectedBranch{ + _, err := x.AllCols().Update(&ProtectedBranch{ EnableStatusCheck: false, StatusCheckContexts: []string{}, }) From 64de970b18a4eb2f3168d579db2168094eeeed96 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Tue, 16 Jul 2019 15:28:31 +0800 Subject: [PATCH 05/18] fix protected branch check bug --- routers/api/v1/repo/pull.go | 4 +++- routers/repo/pull.go | 6 ++++-- templates/repo/pulls/status.tmpl | 2 +- templates/repo/settings/protected_branch.tmpl | 4 ++-- 4 files changed, 10 insertions(+), 6 deletions(-) diff --git a/routers/api/v1/repo/pull.go b/routers/api/v1/repo/pull.go index 63ef1acbe8469..fabf0f1f289b8 100644 --- a/routers/api/v1/repo/pull.go +++ b/routers/api/v1/repo/pull.go @@ -601,7 +601,9 @@ func MergePullRequest(ctx *context.APIContext, form auth.MergePullRequestForm) { return } - if pr.ProtectedBranch.EnableStatusCheck && !pull_service.IsCommitStatusContextSuccess(commitStatuses, pr.ProtectedBranch.StatusCheckContexts) { + if pr.ProtectedBranch != nil && + pr.ProtectedBranch.EnableStatusCheck && + !pull_service.IsCommitStatusContextSuccess(commitStatuses, pr.ProtectedBranch.StatusCheckContexts) { ctx.Status(405) return } diff --git a/routers/repo/pull.go b/routers/repo/pull.go index 7b3abed27623b..79358c583560f 100644 --- a/routers/repo/pull.go +++ b/routers/repo/pull.go @@ -357,7 +357,7 @@ func PrepareViewPullInfo(ctx *context.Context, issue *models.Issue) *git.Compare return nil } - if pull.ProtectedBranch.EnableStatusCheck { + if pull.ProtectedBranch != nil && pull.ProtectedBranch.EnableStatusCheck { ctx.Data["is_context_required"] = func(context string) bool { for _, c := range pull.ProtectedBranch.StatusCheckContexts { if c == context { @@ -656,7 +656,9 @@ func MergePullRequest(ctx *context.Context, form auth.MergePullRequestForm) { return } - if pr.ProtectedBranch.EnableStatusCheck && !pull_service.IsCommitStatusContextSuccess(commitStatuses, pr.ProtectedBranch.StatusCheckContexts) { + if pr.ProtectedBranch != nil && + pr.ProtectedBranch.EnableStatusCheck && + !pull_service.IsCommitStatusContextSuccess(commitStatuses, pr.ProtectedBranch.StatusCheckContexts) { ctx.Flash.Error(ctx.Tr("repo.pulls.no_merge_status_check")) ctx.Redirect(ctx.Repo.RepoLink + "/pulls/" + com.ToStr(pr.Index)) return diff --git a/templates/repo/pulls/status.tmpl b/templates/repo/pulls/status.tmpl index 6e88c64b9425e..a1fb30e47bc39 100644 --- a/templates/repo/pulls/status.tmpl +++ b/templates/repo/pulls/status.tmpl @@ -16,7 +16,7 @@ {{template "repo/commit_status" .}} {{.Context}} {{.Description}}
- {{if call $.is_context_required .Context}}
Required
{{end}} + {{if and $.is_context_required (call $.is_context_required .Context)}}
Required
{{end}} {{if .TargetURL}}Details{{end}}
diff --git a/templates/repo/settings/protected_branch.tmpl b/templates/repo/settings/protected_branch.tmpl index 3b5718623738d..a3e319c582b76 100644 --- a/templates/repo/settings/protected_branch.tmpl +++ b/templates/repo/settings/protected_branch.tmpl @@ -125,10 +125,10 @@ {{range $.branch_status_check_contexts}} - + {{.}} - {{if call $.is_context_required .}}
Required
{{end}} + {{if and $.is_context_required (call $.is_context_required .)}}
Required
{{end}} {{end}} From 7b38636094817a73d25924d46caffeb686abe97e Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Wed, 17 Jul 2019 14:50:32 +0800 Subject: [PATCH 06/18] fix protected branch settings --- options/locale/locale_en-US.ini | 3 ++- routers/repo/setting_protected_branch.go | 16 +++++++++++++++- templates/repo/pulls/status.tmpl | 4 +++- templates/repo/settings/protected_branch.tmpl | 6 +++--- 4 files changed, 23 insertions(+), 6 deletions(-) diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index 1048475ea5511..e2bdf5d13fd79 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -1313,8 +1313,9 @@ settings.protect_merge_whitelist_committers = Enable Merge Whitelist settings.protect_merge_whitelist_committers_desc = Allow only whitelisted users or teams to merge pull requests into this branch. settings.protect_merge_whitelist_users = Whitelisted users for merging: settings.protect_merge_whitelist_teams = Whitelisted teams for merging: -settings.protect_check_status_contexts = Status checks found in the last week for this repository +settings.protect_check_status_contexts = Enable Status Check settings.protect_check_status_contexts_desc = Require status checks to pass before merging Choose which status checks must pass before branches can be merged into a branch that matches this rule. When enabled, commits must first be pushed to another branch, then merged or pushed directly to a branch that matches this rule after status checks have passed. +settings.protect_check_status_contexts_list = Status checks found in the last week for this repository settings.protect_required_approvals = Required approvals: settings.protect_required_approvals_desc = Allow only to merge pull request with enough positive reviews of whitelisted users or teams. settings.protect_approvals_whitelist_users = Whitelisted reviewers: diff --git a/routers/repo/setting_protected_branch.go b/routers/repo/setting_protected_branch.go index 8bfc88c205766..5f43da33d246c 100644 --- a/routers/repo/setting_protected_branch.go +++ b/routers/repo/setting_protected_branch.go @@ -126,7 +126,21 @@ func SettingsProtectedBranch(c *context.Context) { c.Data["whitelist_users"] = strings.Join(base.Int64sToStrings(protectBranch.WhitelistUserIDs), ",") c.Data["merge_whitelist_users"] = strings.Join(base.Int64sToStrings(protectBranch.MergeWhitelistUserIDs), ",") c.Data["approvals_whitelist_users"] = strings.Join(base.Int64sToStrings(protectBranch.ApprovalsWhitelistUserIDs), ",") - c.Data["branch_status_check_contexts"], _ = models.FindRepoStatusCheckContexts(c.Repo.Repository.ID, 4*7*24*time.Hour) // Find last week status check contexts + contexts, _ := models.FindRepoStatusCheckContexts(c.Repo.Repository.ID, 7*24*time.Hour) // Find last week status check contexts + for _, context := range protectBranch.StatusCheckContexts { + var found bool + for _, ctx := range contexts { + if ctx == context { + found = true + break + } + } + if !found { + contexts = append(contexts, context) + } + } + + c.Data["branch_status_check_contexts"] = contexts c.Data["is_context_required"] = func(context string) bool { for _, c := range protectBranch.StatusCheckContexts { if c == context { diff --git a/templates/repo/pulls/status.tmpl b/templates/repo/pulls/status.tmpl index a1fb30e47bc39..5e6b0226d1604 100644 --- a/templates/repo/pulls/status.tmpl +++ b/templates/repo/pulls/status.tmpl @@ -16,7 +16,9 @@ {{template "repo/commit_status" .}} {{.Context}} {{.Description}}
- {{if and $.is_context_required (call $.is_context_required .Context)}}
Required
{{end}} + {{if $.is_context_required}} + {{if (call $.is_context_required .Context)}}
Required
{{end}} + {{end}} {{if .TargetURL}}Details{{end}}
diff --git a/templates/repo/settings/protected_branch.tmpl b/templates/repo/settings/protected_branch.tmpl index a3e319c582b76..067d1d97613ec 100644 --- a/templates/repo/settings/protected_branch.tmpl +++ b/templates/repo/settings/protected_branch.tmpl @@ -117,7 +117,7 @@ @@ -125,10 +125,10 @@ {{range $.branch_status_check_contexts}} {{end}} From 5dacbff82b2d51760d13cf4fb24bebb773856bee Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Wed, 17 Jul 2019 15:10:32 +0800 Subject: [PATCH 07/18] remove duplicated code on check pull request's required commit statuses pass --- modules/pull/commit_status.go | 43 ++++++++++++++++++++++++++++++++++- routers/api/v1/repo/pull.go | 32 +++----------------------- routers/repo/pull.go | 33 +++------------------------ 3 files changed, 48 insertions(+), 60 deletions(-) diff --git a/modules/pull/commit_status.go b/modules/pull/commit_status.go index 2ef5514c7a9a0..61def1feb87fc 100644 --- a/modules/pull/commit_status.go +++ b/modules/pull/commit_status.go @@ -5,7 +5,12 @@ package pull -import "code.gitea.io/gitea/models" +import ( + "code.gitea.io/gitea/models" + "code.gitea.io/gitea/modules/git" + + "github.com/pkg/errors" +) // IsCommitStatusContextSuccess returns true if all required status check contexts succeed. func IsCommitStatusContextSuccess(commitStatuses []*models.CommitStatus, requiredContexts []string) bool { @@ -27,3 +32,39 @@ func IsCommitStatusContextSuccess(commitStatuses []*models.CommitStatus, require } return true } + +// IsPullCommitStatusPass returns if all required status checks PASS +func IsPullCommitStatusPass(pr *models.PullRequest) (bool, error) { + // check if all required status checks are successful + headGitRepo, err := git.OpenRepository(pr.HeadRepo.RepoPath()) + if err != nil { + return false, errors.Wrap(err, "OpenRepository") + } + + headBranchExist := headGitRepo.IsBranchExist(pr.HeadBranch) + if !headBranchExist { + return false, errors.New("HeadBranchExist is not exist, cannot merge") + } + + sha, err := headGitRepo.GetBranchCommitID(pr.HeadBranch) + if err != nil { + return false, errors.Wrap(err, "GetBranchCommitID") + } + + commitStatuses, err := models.GetLatestCommitStatus(pr.BaseRepo, sha, 0) + if err != nil { + return false, errors.Wrap(err, "GetLatestCommitStatus") + } + + if err = pr.LoadProtectedBranch(); err != nil { + return false, errors.Wrap(err, "GetLatestCommitStatus") + } + + if pr.ProtectedBranch != nil && + pr.ProtectedBranch.EnableStatusCheck && + !IsCommitStatusContextSuccess(commitStatuses, pr.ProtectedBranch.StatusCheckContexts) { + return false, nil + } + + return true, nil +} diff --git a/routers/api/v1/repo/pull.go b/routers/api/v1/repo/pull.go index fabf0f1f289b8..ac2db07a005bf 100644 --- a/routers/api/v1/repo/pull.go +++ b/routers/api/v1/repo/pull.go @@ -571,39 +571,13 @@ func MergePullRequest(ctx *context.APIContext, form auth.MergePullRequestForm) { return } - // check if all required status checks are successful - headGitRepo, err := git.OpenRepository(pr.HeadRepo.RepoPath()) + isPass, err := pull_service.IsPullCommitStatusPass(pr) if err != nil { - ctx.Error(500, "OpenRepository", err) + ctx.Error(500, "IsPullCommitStatusPass", err) return } - headBranchExist := headGitRepo.IsBranchExist(pr.HeadBranch) - if !headBranchExist { - ctx.Error(500, "HeadBranchExist is not exist, cannot merge", nil) - return - } - - sha, err := headGitRepo.GetBranchCommitID(pr.HeadBranch) - if err != nil { - ctx.Error(500, "GetBranchCommitID", err) - return - } - - commitStatuses, err := models.GetLatestCommitStatus(ctx.Repo.Repository, sha, 0) - if err != nil { - ctx.Error(500, "GetLatestCommitStatus", err) - return - } - - if err = pr.LoadProtectedBranch(); err != nil { - ctx.Error(500, "GetLatestCommitStatus", err) - return - } - - if pr.ProtectedBranch != nil && - pr.ProtectedBranch.EnableStatusCheck && - !pull_service.IsCommitStatusContextSuccess(commitStatuses, pr.ProtectedBranch.StatusCheckContexts) { + if !isPass { ctx.Status(405) return } diff --git a/routers/repo/pull.go b/routers/repo/pull.go index 79358c583560f..4b189b11a6dd9 100644 --- a/routers/repo/pull.go +++ b/routers/repo/pull.go @@ -626,39 +626,12 @@ func MergePullRequest(ctx *context.Context, form auth.MergePullRequestForm) { return } - // check if all required status checks are successful - headGitRepo, err := git.OpenRepository(pr.HeadRepo.RepoPath()) - if err != nil { - ctx.ServerError("OpenRepository", err) - return - } - - headBranchExist := headGitRepo.IsBranchExist(pr.HeadBranch) - if !headBranchExist { - ctx.ServerError("HeadBranchExist is not exist, cannot merge", nil) - return - } - - sha, err := headGitRepo.GetBranchCommitID(pr.HeadBranch) + isPass, err := pull_service.IsPullCommitStatusPass(pr) if err != nil { - ctx.ServerError("GetBranchCommitID", err) + ctx.ServerError("IsPullCommitStatusPass", err) return } - - commitStatuses, err := models.GetLatestCommitStatus(ctx.Repo.Repository, sha, 0) - if err != nil { - ctx.ServerError("GetLatestCommitStatus", err) - return - } - - if err = pr.LoadProtectedBranch(); err != nil { - ctx.ServerError("GetLatestCommitStatus", err) - return - } - - if pr.ProtectedBranch != nil && - pr.ProtectedBranch.EnableStatusCheck && - !pull_service.IsCommitStatusContextSuccess(commitStatuses, pr.ProtectedBranch.StatusCheckContexts) { + if !isPass { ctx.Flash.Error(ctx.Tr("repo.pulls.no_merge_status_check")) ctx.Redirect(ctx.Repo.RepoLink + "/pulls/" + com.ToStr(pr.Index)) return From 98410341e36b59b25853ad49ef890e727290659d Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Wed, 17 Jul 2019 15:18:57 +0800 Subject: [PATCH 08/18] remove unused codes --- routers/repo/setting_protected_branch.go | 6 ------ 1 file changed, 6 deletions(-) diff --git a/routers/repo/setting_protected_branch.go b/routers/repo/setting_protected_branch.go index 5f43da33d246c..c3a4addd7d879 100644 --- a/routers/repo/setting_protected_branch.go +++ b/routers/repo/setting_protected_branch.go @@ -207,12 +207,6 @@ func SettingsProtectedBranchPost(ctx *context.Context, f auth.ProtectBranchForm) if strings.TrimSpace(f.MergeWhitelistUsers) != "" { mergeWhitelistUsers, _ = base.StringsToInt64s(strings.Split(f.MergeWhitelistUsers, ",")) } - if strings.TrimSpace(f.MergeWhitelistTeams) != "" { - mergeWhitelistTeams, _ = base.StringsToInt64s(strings.Split(f.MergeWhitelistTeams, ",")) - } - if strings.TrimSpace(f.MergeWhitelistTeams) != "" { - mergeWhitelistTeams, _ = base.StringsToInt64s(strings.Split(f.MergeWhitelistTeams, ",")) - } protectBranch.EnableStatusCheck = f.EnableStatusCheck protectBranch.StatusCheckContexts = f.StatusCheckContexts From e4d001a7dbd71d5012f1bb23248230ffe8480bba Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Tue, 30 Jul 2019 10:15:11 +0800 Subject: [PATCH 09/18] fix migration --- go.mod | 1 + 1 file changed, 1 insertion(+) diff --git a/go.mod b/go.mod index f5d49294d4361..0b5dd381de921 100644 --- a/go.mod +++ b/go.mod @@ -80,6 +80,7 @@ require ( github.com/nfnt/resize v0.0.0-20160724205520-891127d8d1b5 github.com/oliamb/cutter v0.2.2 github.com/philhofer/fwd v1.0.0 // indirect + github.com/pkg/errors v0.8.1 github.com/pquerna/otp v0.0.0-20160912161815-54653902c20e github.com/prometheus/client_golang v1.1.0 github.com/prometheus/procfs v0.0.4 // indirect From f94f1ffadb1f7e87b0fce07899c2414c555647e7 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sat, 7 Sep 2019 11:11:23 +0800 Subject: [PATCH 10/18] add newline for template file --- templates/repo/pulls/status.tmpl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/templates/repo/pulls/status.tmpl b/templates/repo/pulls/status.tmpl index 5e6b0226d1604..76a4eb54230c8 100644 --- a/templates/repo/pulls/status.tmpl +++ b/templates/repo/pulls/status.tmpl @@ -23,4 +23,4 @@ {{end}} -{{end}} \ No newline at end of file +{{end}} From 9d940fdee72e97777957285479b89796ecd36196 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sat, 7 Sep 2019 11:22:30 +0800 Subject: [PATCH 11/18] fix go mod --- go.mod | 2 ++ 1 file changed, 2 insertions(+) diff --git a/go.mod b/go.mod index 0b5dd381de921..154c7772a19d8 100644 --- a/go.mod +++ b/go.mod @@ -75,6 +75,8 @@ require ( github.com/mattn/go-sqlite3 v1.11.0 github.com/mcuadros/go-version v0.0.0-20190308113854-92cdf37c5b75 github.com/microcosm-cc/bluemonday v0.0.0-20161012083705-f77f16ffc87a + github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd // indirect + github.com/modern-go/reflect2 v1.0.1 // indirect github.com/mschoch/smat v0.0.0-20160514031455-90eadee771ae // indirect github.com/msteinert/pam v0.0.0-20151204160544-02ccfbfaf0cc github.com/nfnt/resize v0.0.0-20160724205520-891127d8d1b5 From 9d272603b614407f40025b85de90cbbea4c858c2 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Thu, 12 Sep 2019 23:26:17 +0800 Subject: [PATCH 12/18] rename function name and some other fixes --- models/commit_status.go | 4 ++-- models/pull.go | 15 +++++++++++++++ modules/pull/commit_status.go | 23 ++++++++++++----------- routers/repo/setting_protected_branch.go | 2 +- 4 files changed, 30 insertions(+), 14 deletions(-) diff --git a/models/commit_status.go b/models/commit_status.go index 11618b0b2c6a6..a6d91c4e28bc2 100644 --- a/models/commit_status.go +++ b/models/commit_status.go @@ -206,8 +206,8 @@ func GetLatestCommitStatus(repo *Repository, sha string, page int) ([]*CommitSta return statuses, x.In("id", ids).Find(&statuses) } -// FindRepoStatusCheckContexts returns repo's status check contexts -func FindRepoStatusCheckContexts(repoID int64, before time.Duration) ([]string, error) { +// FindRepoRecentCommitStatusContexts returns repository's recent commit status contexts +func FindRepoRecentCommitStatusContexts(repoID int64, before time.Duration) ([]string, error) { start := timeutil.TimeStampNow().AddDuration(-before) ids := make([]int64, 0, 10) err := x.Table("commit_status"). diff --git a/models/pull.go b/models/pull.go index ecb5c1345e578..11b8909607a1a 100644 --- a/models/pull.go +++ b/models/pull.go @@ -99,6 +99,21 @@ func (pr *PullRequest) LoadAttributes() error { return pr.loadAttributes(x) } +// LoadBaseRepo loads pull request base repository from database +func (pr *PullRequest) LoadBaseRepo() error { + if pr.BaseRepo == nil { + var repo Repository + has, err := x.ID(pr.BaseRepoID).Get(&repo) + if err != nil { + return err + } else if !has { + return ErrRepoNotExist{ID: pr.BaseRepoID} + } + pr.BaseRepo = &repo + } + return nil +} + // LoadIssue loads issue information from database func (pr *PullRequest) LoadIssue() (err error) { return pr.loadIssue(x) diff --git a/modules/pull/commit_status.go b/modules/pull/commit_status.go index 61def1feb87fc..6502b24dda01f 100644 --- a/modules/pull/commit_status.go +++ b/modules/pull/commit_status.go @@ -35,6 +35,13 @@ func IsCommitStatusContextSuccess(commitStatuses []*models.CommitStatus, require // IsPullCommitStatusPass returns if all required status checks PASS func IsPullCommitStatusPass(pr *models.PullRequest) (bool, error) { + if err := pr.LoadProtectedBranch(); err != nil { + return false, errors.Wrap(err, "GetLatestCommitStatus") + } + if pr.ProtectedBranch == nil || !pr.ProtectedBranch.EnableStatusCheck { + return true, nil + } + // check if all required status checks are successful headGitRepo, err := git.OpenRepository(pr.HeadRepo.RepoPath()) if err != nil { @@ -51,20 +58,14 @@ func IsPullCommitStatusPass(pr *models.PullRequest) (bool, error) { return false, errors.Wrap(err, "GetBranchCommitID") } - commitStatuses, err := models.GetLatestCommitStatus(pr.BaseRepo, sha, 0) - if err != nil { - return false, errors.Wrap(err, "GetLatestCommitStatus") + if err := pr.LoadBaseRepo(); err != nil { + return false, errors.Wrap(err, "LoadBaseRepo") } - if err = pr.LoadProtectedBranch(); err != nil { + commitStatuses, err := models.GetLatestCommitStatus(pr.BaseRepo, sha, 0) + if err != nil { return false, errors.Wrap(err, "GetLatestCommitStatus") } - if pr.ProtectedBranch != nil && - pr.ProtectedBranch.EnableStatusCheck && - !IsCommitStatusContextSuccess(commitStatuses, pr.ProtectedBranch.StatusCheckContexts) { - return false, nil - } - - return true, nil + return IsCommitStatusContextSuccess(commitStatuses, pr.ProtectedBranch.StatusCheckContexts), nil } diff --git a/routers/repo/setting_protected_branch.go b/routers/repo/setting_protected_branch.go index c3a4addd7d879..3d055e23ff98b 100644 --- a/routers/repo/setting_protected_branch.go +++ b/routers/repo/setting_protected_branch.go @@ -126,7 +126,7 @@ func SettingsProtectedBranch(c *context.Context) { c.Data["whitelist_users"] = strings.Join(base.Int64sToStrings(protectBranch.WhitelistUserIDs), ",") c.Data["merge_whitelist_users"] = strings.Join(base.Int64sToStrings(protectBranch.MergeWhitelistUserIDs), ",") c.Data["approvals_whitelist_users"] = strings.Join(base.Int64sToStrings(protectBranch.ApprovalsWhitelistUserIDs), ",") - contexts, _ := models.FindRepoStatusCheckContexts(c.Repo.Repository.ID, 7*24*time.Hour) // Find last week status check contexts + contexts, _ := models.FindRepoRecentCommitStatusContexts(c.Repo.Repository.ID, 7*24*time.Hour) // Find last week status check contexts for _, context := range protectBranch.StatusCheckContexts { var found bool for _, ctx := range contexts { From e7d604a9be48904732dbbf7ab9e9f9797a96bae8 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Fri, 13 Sep 2019 00:16:57 +0800 Subject: [PATCH 13/18] fix template --- templates/repo/issue/view_content/pull.tmpl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/templates/repo/issue/view_content/pull.tmpl b/templates/repo/issue/view_content/pull.tmpl index 97c01235853a9..91b08b41b585e 100644 --- a/templates/repo/issue/view_content/pull.tmpl +++ b/templates/repo/issue/view_content/pull.tmpl @@ -41,7 +41,7 @@ {{else if .IsFilesConflicted}}grey {{else if .IsPullRequestBroken}}red {{else if .IsBlockedByApprovals}}red - {[else if not .IsRequiredStatusCheckSuccess}}red + {{else if not .IsRequiredStatusCheckSuccess}}red {{else if .Issue.PullRequest.IsChecking}}yellow {{else if .Issue.PullRequest.CanAutoMerge}}green {{else}}red{{end}}"> From d033abada214d472aff210fd4e60d268046d9cd3 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Fri, 13 Sep 2019 20:14:05 +0800 Subject: [PATCH 14/18] fix bug pull view --- routers/repo/pull.go | 11 ++++++----- templates/repo/issue/view_content/pull.tmpl | 4 ++-- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/routers/repo/pull.go b/routers/repo/pull.go index 4b189b11a6dd9..5a112bd08e435 100644 --- a/routers/repo/pull.go +++ b/routers/repo/pull.go @@ -323,6 +323,12 @@ func PrepareViewPullInfo(ctx *context.Context, issue *models.Issue) *git.Compare setMergeTarget(ctx, pull) + if err = pull.LoadProtectedBranch(); err != nil { + ctx.ServerError("GetLatestCommitStatus", err) + return nil + } + ctx.Data["EnableStatusCheck"] = pull.ProtectedBranch != nil && pull.ProtectedBranch.EnableStatusCheck + var headGitRepo *git.Repository var headBranchExist bool // HeadRepo may be missing @@ -352,11 +358,6 @@ func PrepareViewPullInfo(ctx *context.Context, issue *models.Issue) *git.Compare ctx.Data["LatestCommitStatus"] = models.CalcCommitStatus(commitStatuses) } - if err = pull.LoadProtectedBranch(); err != nil { - ctx.ServerError("GetLatestCommitStatus", err) - return nil - } - if pull.ProtectedBranch != nil && pull.ProtectedBranch.EnableStatusCheck { ctx.Data["is_context_required"] = func(context string) bool { for _, c := range pull.ProtectedBranch.StatusCheckContexts { diff --git a/templates/repo/issue/view_content/pull.tmpl b/templates/repo/issue/view_content/pull.tmpl index 91b08b41b585e..50b60d24745f9 100644 --- a/templates/repo/issue/view_content/pull.tmpl +++ b/templates/repo/issue/view_content/pull.tmpl @@ -41,7 +41,7 @@ {{else if .IsFilesConflicted}}grey {{else if .IsPullRequestBroken}}red {{else if .IsBlockedByApprovals}}red - {{else if not .IsRequiredStatusCheckSuccess}}red + {{else if and .EnableStatusCheck (not .IsRequiredStatusCheckSuccess)}}red {{else if .Issue.PullRequest.IsChecking}}yellow {{else if .Issue.PullRequest.CanAutoMerge}}green {{else}}red{{end}}"> @@ -105,7 +105,7 @@ {{$.i18n.Tr "repo.pulls.is_checking"}} - {{else if not .IsRequiredStatusCheckSuccess}} + {{else if and .EnableStatusCheck (not .IsRequiredStatusCheckSuccess)}}
{{$.i18n.Tr "repo.pulls.required_status_check_failed"}} From 11804b3e8c1359cc3a03e10a627a7c5e91e28a6f Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sat, 14 Sep 2019 10:06:13 +0800 Subject: [PATCH 15/18] remove go1.12 wrong dependencies --- go.mod | 2 -- 1 file changed, 2 deletions(-) diff --git a/go.mod b/go.mod index 154c7772a19d8..0b5dd381de921 100644 --- a/go.mod +++ b/go.mod @@ -75,8 +75,6 @@ require ( github.com/mattn/go-sqlite3 v1.11.0 github.com/mcuadros/go-version v0.0.0-20190308113854-92cdf37c5b75 github.com/microcosm-cc/bluemonday v0.0.0-20161012083705-f77f16ffc87a - github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd // indirect - github.com/modern-go/reflect2 v1.0.1 // indirect github.com/mschoch/smat v0.0.0-20160514031455-90eadee771ae // indirect github.com/msteinert/pam v0.0.0-20151204160544-02ccfbfaf0cc github.com/nfnt/resize v0.0.0-20160724205520-891127d8d1b5 From 91ccdd4e5bbbb7c786387d9ffece5836ba320414 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Tue, 17 Sep 2019 16:51:03 +0800 Subject: [PATCH 16/18] add administrator bypass when protected branch status check enabled --- options/locale/locale_en-US.ini | 1 + routers/api/v1/repo/pull.go | 2 +- routers/repo/pull.go | 2 +- templates/repo/issue/view_content/pull.tmpl | 245 +++++++++++--------- 4 files changed, 133 insertions(+), 117 deletions(-) diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index e2bdf5d13fd79..843252dc34656 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -982,6 +982,7 @@ 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.required_status_check_failed = Some required checks were not successful. +pulls.required_status_check_administrator = As an administrator, you may still merge this pull request. pulls.blocked_by_approvals = "This Pull Request doesn't have enough approvals yet. %d of %d approvals granted." pulls.can_auto_merge_desc = This pull request can be merged automatically. pulls.cannot_auto_merge_desc = This pull request cannot be merged automatically due to conflicts. diff --git a/routers/api/v1/repo/pull.go b/routers/api/v1/repo/pull.go index ac2db07a005bf..d3156bc2c21a2 100644 --- a/routers/api/v1/repo/pull.go +++ b/routers/api/v1/repo/pull.go @@ -577,7 +577,7 @@ func MergePullRequest(ctx *context.APIContext, form auth.MergePullRequestForm) { return } - if !isPass { + if !isPass && !ctx.IsUserRepoAdmin() { ctx.Status(405) return } diff --git a/routers/repo/pull.go b/routers/repo/pull.go index 5a112bd08e435..180d592e3dbf8 100644 --- a/routers/repo/pull.go +++ b/routers/repo/pull.go @@ -632,7 +632,7 @@ func MergePullRequest(ctx *context.Context, form auth.MergePullRequestForm) { ctx.ServerError("IsPullCommitStatusPass", err) return } - if !isPass { + if !isPass && !ctx.IsUserRepoAdmin() { ctx.Flash.Error(ctx.Tr("repo.pulls.no_merge_status_check")) ctx.Redirect(ctx.Repo.RepoLink + "/pulls/" + com.ToStr(pr.Index)) return diff --git a/templates/repo/issue/view_content/pull.tmpl b/templates/repo/issue/view_content/pull.tmpl index 50b60d24745f9..f5ce8e0886c39 100644 --- a/templates/repo/issue/view_content/pull.tmpl +++ b/templates/repo/issue/view_content/pull.tmpl @@ -105,135 +105,150 @@ {{$.i18n.Tr "repo.pulls.is_checking"}}
- {{else if and .EnableStatusCheck (not .IsRequiredStatusCheckSuccess)}} + {{else if and (not .Issue.PullRequest.CanAutoMerge) .EnableStatusCheck (not .IsRequiredStatusCheckSuccess)}}
{{$.i18n.Tr "repo.pulls.required_status_check_failed"}}
{{else if .Issue.PullRequest.CanAutoMerge}} -
- - {{$.i18n.Tr "repo.pulls.can_auto_merge_desc"}} + {{if and .EnableStatusCheck (not .IsRequiredStatusCheckSuccess)}} +
+ + {{$.i18n.Tr "repo.pulls.required_status_check_failed"}}
- {{if .AllowMerge}} - {{$prUnit := .Repository.MustGetUnit $.UnitTypePullRequests}} - {{if or $prUnit.PullRequestsConfig.AllowMerge $prUnit.PullRequestsConfig.AllowRebase $prUnit.PullRequestsConfig.AllowRebaseMerge $prUnit.PullRequestsConfig.AllowSquash}} -
- {{if $prUnit.PullRequestsConfig.AllowMerge}} - - {{end}} - {{if $prUnit.PullRequestsConfig.AllowRebase}} - - {{end}} - {{if $prUnit.PullRequestsConfig.AllowRebaseMerge}} -
- {{.i18n.Tr "repo.settings.protect_check_status_contexts"}} + {{.i18n.Tr "repo.settings.protect_check_status_contexts_list"}}
- + {{.}} - {{if and $.is_context_required (call $.is_context_required .)}}
Required
{{end}} + {{if $.is_context_required}}{{if call $.is_context_required .}}
Required
{{end}}{{end}}