From 9c26dc1f3a742280baff4e9578545bc822016764 Mon Sep 17 00:00:00 2001 From: a1012112796 <1012112796@qq.com> Date: Sun, 29 Nov 2020 03:30:46 +0800 Subject: [PATCH] Add block on official review requests branch protection (#13705) Signed-off-by: a1012112796 <1012112796@qq.com> Co-authored-by: Lauris BH --- models/branches.go | 66 ++++++---- models/migrations/migrations.go | 2 + models/migrations/v160.go | 17 +++ modules/auth/repo_form.go | 39 +++--- modules/convert/convert.go | 45 +++---- modules/structs/repo_branch.go | 121 +++++++++--------- options/locale/locale_en-US.ini | 3 + routers/api/v1/repo/branch.go | 35 ++--- routers/repo/issue.go | 1 + routers/repo/setting_protected_branch.go | 1 + services/pull/merge.go | 5 + templates/repo/issue/view_content/pull.tmpl | 15 ++- templates/repo/settings/protected_branch.tmpl | 7 + templates/swagger/v1_json.tmpl | 12 ++ 14 files changed, 228 insertions(+), 141 deletions(-) create mode 100644 models/migrations/v160.go diff --git a/models/branches.go b/models/branches.go index 420a4b663a..144767ee95 100644 --- a/models/branches.go +++ b/models/branches.go @@ -21,28 +21,29 @@ import ( // ProtectedBranch struct type ProtectedBranch struct { - ID int64 `xorm:"pk autoincr"` - RepoID int64 `xorm:"UNIQUE(s)"` - BranchName string `xorm:"UNIQUE(s)"` - CanPush bool `xorm:"NOT NULL DEFAULT false"` - EnableWhitelist bool - WhitelistUserIDs []int64 `xorm:"JSON TEXT"` - WhitelistTeamIDs []int64 `xorm:"JSON TEXT"` - EnableMergeWhitelist bool `xorm:"NOT NULL DEFAULT false"` - WhitelistDeployKeys 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"` - EnableApprovalsWhitelist bool `xorm:"NOT NULL DEFAULT false"` - ApprovalsWhitelistUserIDs []int64 `xorm:"JSON TEXT"` - ApprovalsWhitelistTeamIDs []int64 `xorm:"JSON TEXT"` - RequiredApprovals int64 `xorm:"NOT NULL DEFAULT 0"` - BlockOnRejectedReviews bool `xorm:"NOT NULL DEFAULT false"` - BlockOnOutdatedBranch bool `xorm:"NOT NULL DEFAULT false"` - DismissStaleApprovals bool `xorm:"NOT NULL DEFAULT false"` - RequireSignedCommits bool `xorm:"NOT NULL DEFAULT false"` - ProtectedFilePatterns string `xorm:"TEXT"` + ID int64 `xorm:"pk autoincr"` + RepoID int64 `xorm:"UNIQUE(s)"` + BranchName string `xorm:"UNIQUE(s)"` + CanPush bool `xorm:"NOT NULL DEFAULT false"` + EnableWhitelist bool + WhitelistUserIDs []int64 `xorm:"JSON TEXT"` + WhitelistTeamIDs []int64 `xorm:"JSON TEXT"` + EnableMergeWhitelist bool `xorm:"NOT NULL DEFAULT false"` + WhitelistDeployKeys 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"` + EnableApprovalsWhitelist bool `xorm:"NOT NULL DEFAULT false"` + ApprovalsWhitelistUserIDs []int64 `xorm:"JSON TEXT"` + ApprovalsWhitelistTeamIDs []int64 `xorm:"JSON TEXT"` + RequiredApprovals int64 `xorm:"NOT NULL DEFAULT 0"` + BlockOnRejectedReviews bool `xorm:"NOT NULL DEFAULT false"` + BlockOnOfficialReviewRequests bool `xorm:"NOT NULL DEFAULT false"` + BlockOnOutdatedBranch bool `xorm:"NOT NULL DEFAULT false"` + DismissStaleApprovals bool `xorm:"NOT NULL DEFAULT false"` + RequireSignedCommits bool `xorm:"NOT NULL DEFAULT false"` + ProtectedFilePatterns string `xorm:"TEXT"` CreatedUnix timeutil.TimeStamp `xorm:"created"` UpdatedUnix timeutil.TimeStamp `xorm:"updated"` @@ -171,13 +172,12 @@ func (protectBranch *ProtectedBranch) GetGrantedApprovalsCount(pr *PullRequest) } // MergeBlockedByRejectedReview returns true if merge is blocked by rejected reviews -// An official ReviewRequest should also block Merge like Reject func (protectBranch *ProtectedBranch) MergeBlockedByRejectedReview(pr *PullRequest) bool { if !protectBranch.BlockOnRejectedReviews { return false } rejectExist, err := x.Where("issue_id = ?", pr.IssueID). - And("type in ( ?, ?)", ReviewTypeReject, ReviewTypeRequest). + And("type = ?", ReviewTypeReject). And("official = ?", true). Exist(new(Review)) if err != nil { @@ -188,6 +188,24 @@ func (protectBranch *ProtectedBranch) MergeBlockedByRejectedReview(pr *PullReque return rejectExist } +// MergeBlockedByOfficialReviewRequests block merge because of some review request to official reviewer +// of from official review +func (protectBranch *ProtectedBranch) MergeBlockedByOfficialReviewRequests(pr *PullRequest) bool { + if !protectBranch.BlockOnOfficialReviewRequests { + return false + } + has, err := x.Where("issue_id = ?", pr.IssueID). + And("type = ?", ReviewTypeRequest). + And("official = ?", true). + Exist(new(Review)) + if err != nil { + log.Error("MergeBlockedByOfficialReviewRequests: %v", err) + return true + } + + return has +} + // MergeBlockedByOutdatedBranch returns true if merge is blocked by an outdated head branch func (protectBranch *ProtectedBranch) MergeBlockedByOutdatedBranch(pr *PullRequest) bool { return protectBranch.BlockOnOutdatedBranch && pr.CommitsBehind > 0 diff --git a/models/migrations/migrations.go b/models/migrations/migrations.go index 5a2646474c..6b13719b44 100644 --- a/models/migrations/migrations.go +++ b/models/migrations/migrations.go @@ -254,6 +254,8 @@ var migrations = []Migration{ NewMigration("code comment replies should have the commitID of the review they are replying to", updateCodeCommentReplies), // v159 -> v160 NewMigration("update reactions constraint", updateReactionConstraint), + // v160 -> v161 + NewMigration("Add block on official review requests branch protection", addBlockOnOfficialReviewRequests), } // GetCurrentDBVersion returns the current db version diff --git a/models/migrations/v160.go b/models/migrations/v160.go new file mode 100644 index 0000000000..e1a4b4821d --- /dev/null +++ b/models/migrations/v160.go @@ -0,0 +1,17 @@ +// Copyright 2020 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 ( + "xorm.io/xorm" +) + +func addBlockOnOfficialReviewRequests(x *xorm.Engine) error { + type ProtectedBranch struct { + BlockOnOfficialReviewRequests bool `xorm:"NOT NULL DEFAULT false"` + } + + return x.Sync2(new(ProtectedBranch)) +} diff --git a/modules/auth/repo_form.go b/modules/auth/repo_form.go index 2d6f89b6ed..24c2478fa4 100644 --- a/modules/auth/repo_form.go +++ b/modules/auth/repo_form.go @@ -178,25 +178,26 @@ func (f *RepoSettingForm) Validate(ctx *macaron.Context, errs binding.Errors) bi // ProtectBranchForm form for changing protected branch settings type ProtectBranchForm struct { - Protected bool - EnablePush string - WhitelistUsers string - WhitelistTeams string - WhitelistDeployKeys bool - EnableMergeWhitelist bool - MergeWhitelistUsers string - MergeWhitelistTeams string - EnableStatusCheck bool `xorm:"NOT NULL DEFAULT false"` - StatusCheckContexts []string - RequiredApprovals int64 - EnableApprovalsWhitelist bool - ApprovalsWhitelistUsers string - ApprovalsWhitelistTeams string - BlockOnRejectedReviews bool - BlockOnOutdatedBranch bool - DismissStaleApprovals bool - RequireSignedCommits bool - ProtectedFilePatterns string + Protected bool + EnablePush string + WhitelistUsers string + WhitelistTeams string + WhitelistDeployKeys bool + EnableMergeWhitelist bool + MergeWhitelistUsers string + MergeWhitelistTeams string + EnableStatusCheck bool + StatusCheckContexts []string + RequiredApprovals int64 + EnableApprovalsWhitelist bool + ApprovalsWhitelistUsers string + ApprovalsWhitelistTeams string + BlockOnRejectedReviews bool + BlockOnOfficialReviewRequests bool + BlockOnOutdatedBranch bool + DismissStaleApprovals bool + RequireSignedCommits bool + ProtectedFilePatterns string } // Validate validates the fields diff --git a/modules/convert/convert.go b/modules/convert/convert.go index 5d056c3795..9b20c683be 100644 --- a/modules/convert/convert.go +++ b/modules/convert/convert.go @@ -105,28 +105,29 @@ func ToBranchProtection(bp *models.ProtectedBranch) *api.BranchProtection { } return &api.BranchProtection{ - BranchName: bp.BranchName, - EnablePush: bp.CanPush, - EnablePushWhitelist: bp.EnableWhitelist, - PushWhitelistUsernames: pushWhitelistUsernames, - PushWhitelistTeams: pushWhitelistTeams, - PushWhitelistDeployKeys: bp.WhitelistDeployKeys, - EnableMergeWhitelist: bp.EnableMergeWhitelist, - MergeWhitelistUsernames: mergeWhitelistUsernames, - MergeWhitelistTeams: mergeWhitelistTeams, - EnableStatusCheck: bp.EnableStatusCheck, - StatusCheckContexts: bp.StatusCheckContexts, - RequiredApprovals: bp.RequiredApprovals, - EnableApprovalsWhitelist: bp.EnableApprovalsWhitelist, - ApprovalsWhitelistUsernames: approvalsWhitelistUsernames, - ApprovalsWhitelistTeams: approvalsWhitelistTeams, - BlockOnRejectedReviews: bp.BlockOnRejectedReviews, - BlockOnOutdatedBranch: bp.BlockOnOutdatedBranch, - DismissStaleApprovals: bp.DismissStaleApprovals, - RequireSignedCommits: bp.RequireSignedCommits, - ProtectedFilePatterns: bp.ProtectedFilePatterns, - Created: bp.CreatedUnix.AsTime(), - Updated: bp.UpdatedUnix.AsTime(), + BranchName: bp.BranchName, + EnablePush: bp.CanPush, + EnablePushWhitelist: bp.EnableWhitelist, + PushWhitelistUsernames: pushWhitelistUsernames, + PushWhitelistTeams: pushWhitelistTeams, + PushWhitelistDeployKeys: bp.WhitelistDeployKeys, + EnableMergeWhitelist: bp.EnableMergeWhitelist, + MergeWhitelistUsernames: mergeWhitelistUsernames, + MergeWhitelistTeams: mergeWhitelistTeams, + EnableStatusCheck: bp.EnableStatusCheck, + StatusCheckContexts: bp.StatusCheckContexts, + RequiredApprovals: bp.RequiredApprovals, + EnableApprovalsWhitelist: bp.EnableApprovalsWhitelist, + ApprovalsWhitelistUsernames: approvalsWhitelistUsernames, + ApprovalsWhitelistTeams: approvalsWhitelistTeams, + BlockOnRejectedReviews: bp.BlockOnRejectedReviews, + BlockOnOfficialReviewRequests: bp.BlockOnOfficialReviewRequests, + BlockOnOutdatedBranch: bp.BlockOnOutdatedBranch, + DismissStaleApprovals: bp.DismissStaleApprovals, + RequireSignedCommits: bp.RequireSignedCommits, + ProtectedFilePatterns: bp.ProtectedFilePatterns, + Created: bp.CreatedUnix.AsTime(), + Updated: bp.UpdatedUnix.AsTime(), } } diff --git a/modules/structs/repo_branch.go b/modules/structs/repo_branch.go index 490c63070a..efb4caecb0 100644 --- a/modules/structs/repo_branch.go +++ b/modules/structs/repo_branch.go @@ -23,26 +23,27 @@ type Branch struct { // BranchProtection represents a branch protection for a repository type BranchProtection struct { - BranchName string `json:"branch_name"` - EnablePush bool `json:"enable_push"` - EnablePushWhitelist bool `json:"enable_push_whitelist"` - PushWhitelistUsernames []string `json:"push_whitelist_usernames"` - PushWhitelistTeams []string `json:"push_whitelist_teams"` - PushWhitelistDeployKeys bool `json:"push_whitelist_deploy_keys"` - EnableMergeWhitelist bool `json:"enable_merge_whitelist"` - MergeWhitelistUsernames []string `json:"merge_whitelist_usernames"` - MergeWhitelistTeams []string `json:"merge_whitelist_teams"` - EnableStatusCheck bool `json:"enable_status_check"` - StatusCheckContexts []string `json:"status_check_contexts"` - RequiredApprovals int64 `json:"required_approvals"` - EnableApprovalsWhitelist bool `json:"enable_approvals_whitelist"` - ApprovalsWhitelistUsernames []string `json:"approvals_whitelist_username"` - ApprovalsWhitelistTeams []string `json:"approvals_whitelist_teams"` - BlockOnRejectedReviews bool `json:"block_on_rejected_reviews"` - BlockOnOutdatedBranch bool `json:"block_on_outdated_branch"` - DismissStaleApprovals bool `json:"dismiss_stale_approvals"` - RequireSignedCommits bool `json:"require_signed_commits"` - ProtectedFilePatterns string `json:"protected_file_patterns"` + BranchName string `json:"branch_name"` + EnablePush bool `json:"enable_push"` + EnablePushWhitelist bool `json:"enable_push_whitelist"` + PushWhitelistUsernames []string `json:"push_whitelist_usernames"` + PushWhitelistTeams []string `json:"push_whitelist_teams"` + PushWhitelistDeployKeys bool `json:"push_whitelist_deploy_keys"` + EnableMergeWhitelist bool `json:"enable_merge_whitelist"` + MergeWhitelistUsernames []string `json:"merge_whitelist_usernames"` + MergeWhitelistTeams []string `json:"merge_whitelist_teams"` + EnableStatusCheck bool `json:"enable_status_check"` + StatusCheckContexts []string `json:"status_check_contexts"` + RequiredApprovals int64 `json:"required_approvals"` + EnableApprovalsWhitelist bool `json:"enable_approvals_whitelist"` + ApprovalsWhitelistUsernames []string `json:"approvals_whitelist_username"` + ApprovalsWhitelistTeams []string `json:"approvals_whitelist_teams"` + BlockOnRejectedReviews bool `json:"block_on_rejected_reviews"` + BlockOnOfficialReviewRequests bool `json:"block_on_official_review_requests"` + BlockOnOutdatedBranch bool `json:"block_on_outdated_branch"` + DismissStaleApprovals bool `json:"dismiss_stale_approvals"` + RequireSignedCommits bool `json:"require_signed_commits"` + ProtectedFilePatterns string `json:"protected_file_patterns"` // swagger:strfmt date-time Created time.Time `json:"created_at"` // swagger:strfmt date-time @@ -51,47 +52,49 @@ type BranchProtection struct { // CreateBranchProtectionOption options for creating a branch protection type CreateBranchProtectionOption struct { - BranchName string `json:"branch_name"` - EnablePush bool `json:"enable_push"` - EnablePushWhitelist bool `json:"enable_push_whitelist"` - PushWhitelistUsernames []string `json:"push_whitelist_usernames"` - PushWhitelistTeams []string `json:"push_whitelist_teams"` - PushWhitelistDeployKeys bool `json:"push_whitelist_deploy_keys"` - EnableMergeWhitelist bool `json:"enable_merge_whitelist"` - MergeWhitelistUsernames []string `json:"merge_whitelist_usernames"` - MergeWhitelistTeams []string `json:"merge_whitelist_teams"` - EnableStatusCheck bool `json:"enable_status_check"` - StatusCheckContexts []string `json:"status_check_contexts"` - RequiredApprovals int64 `json:"required_approvals"` - EnableApprovalsWhitelist bool `json:"enable_approvals_whitelist"` - ApprovalsWhitelistUsernames []string `json:"approvals_whitelist_username"` - ApprovalsWhitelistTeams []string `json:"approvals_whitelist_teams"` - BlockOnRejectedReviews bool `json:"block_on_rejected_reviews"` - BlockOnOutdatedBranch bool `json:"block_on_outdated_branch"` - DismissStaleApprovals bool `json:"dismiss_stale_approvals"` - RequireSignedCommits bool `json:"require_signed_commits"` - ProtectedFilePatterns string `json:"protected_file_patterns"` + BranchName string `json:"branch_name"` + EnablePush bool `json:"enable_push"` + EnablePushWhitelist bool `json:"enable_push_whitelist"` + PushWhitelistUsernames []string `json:"push_whitelist_usernames"` + PushWhitelistTeams []string `json:"push_whitelist_teams"` + PushWhitelistDeployKeys bool `json:"push_whitelist_deploy_keys"` + EnableMergeWhitelist bool `json:"enable_merge_whitelist"` + MergeWhitelistUsernames []string `json:"merge_whitelist_usernames"` + MergeWhitelistTeams []string `json:"merge_whitelist_teams"` + EnableStatusCheck bool `json:"enable_status_check"` + StatusCheckContexts []string `json:"status_check_contexts"` + RequiredApprovals int64 `json:"required_approvals"` + EnableApprovalsWhitelist bool `json:"enable_approvals_whitelist"` + ApprovalsWhitelistUsernames []string `json:"approvals_whitelist_username"` + ApprovalsWhitelistTeams []string `json:"approvals_whitelist_teams"` + BlockOnRejectedReviews bool `json:"block_on_rejected_reviews"` + BlockOnOfficialReviewRequests bool `json:"block_on_official_review_requests"` + BlockOnOutdatedBranch bool `json:"block_on_outdated_branch"` + DismissStaleApprovals bool `json:"dismiss_stale_approvals"` + RequireSignedCommits bool `json:"require_signed_commits"` + ProtectedFilePatterns string `json:"protected_file_patterns"` } // EditBranchProtectionOption options for editing a branch protection type EditBranchProtectionOption struct { - EnablePush *bool `json:"enable_push"` - EnablePushWhitelist *bool `json:"enable_push_whitelist"` - PushWhitelistUsernames []string `json:"push_whitelist_usernames"` - PushWhitelistTeams []string `json:"push_whitelist_teams"` - PushWhitelistDeployKeys *bool `json:"push_whitelist_deploy_keys"` - EnableMergeWhitelist *bool `json:"enable_merge_whitelist"` - MergeWhitelistUsernames []string `json:"merge_whitelist_usernames"` - MergeWhitelistTeams []string `json:"merge_whitelist_teams"` - EnableStatusCheck *bool `json:"enable_status_check"` - StatusCheckContexts []string `json:"status_check_contexts"` - RequiredApprovals *int64 `json:"required_approvals"` - EnableApprovalsWhitelist *bool `json:"enable_approvals_whitelist"` - ApprovalsWhitelistUsernames []string `json:"approvals_whitelist_username"` - ApprovalsWhitelistTeams []string `json:"approvals_whitelist_teams"` - BlockOnRejectedReviews *bool `json:"block_on_rejected_reviews"` - BlockOnOutdatedBranch *bool `json:"block_on_outdated_branch"` - DismissStaleApprovals *bool `json:"dismiss_stale_approvals"` - RequireSignedCommits *bool `json:"require_signed_commits"` - ProtectedFilePatterns *string `json:"protected_file_patterns"` + EnablePush *bool `json:"enable_push"` + EnablePushWhitelist *bool `json:"enable_push_whitelist"` + PushWhitelistUsernames []string `json:"push_whitelist_usernames"` + PushWhitelistTeams []string `json:"push_whitelist_teams"` + PushWhitelistDeployKeys *bool `json:"push_whitelist_deploy_keys"` + EnableMergeWhitelist *bool `json:"enable_merge_whitelist"` + MergeWhitelistUsernames []string `json:"merge_whitelist_usernames"` + MergeWhitelistTeams []string `json:"merge_whitelist_teams"` + EnableStatusCheck *bool `json:"enable_status_check"` + StatusCheckContexts []string `json:"status_check_contexts"` + RequiredApprovals *int64 `json:"required_approvals"` + EnableApprovalsWhitelist *bool `json:"enable_approvals_whitelist"` + ApprovalsWhitelistUsernames []string `json:"approvals_whitelist_username"` + ApprovalsWhitelistTeams []string `json:"approvals_whitelist_teams"` + BlockOnRejectedReviews *bool `json:"block_on_rejected_reviews"` + BlockOnOfficialReviewRequests *bool `json:"block_on_official_review_requests"` + BlockOnOutdatedBranch *bool `json:"block_on_outdated_branch"` + DismissStaleApprovals *bool `json:"dismiss_stale_approvals"` + RequireSignedCommits *bool `json:"require_signed_commits"` + ProtectedFilePatterns *string `json:"protected_file_patterns"` } diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index 016211c7f8..1500bf73e9 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -1244,6 +1244,7 @@ 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. pulls.blocked_by_approvals = "This Pull Request doesn't have enough approvals yet. %d of %d approvals granted." pulls.blocked_by_rejection = "This Pull Request has changes requested by an official reviewer." +pulls.blocked_by_official_review_requests = "This Pull Request has official review requests." pulls.blocked_by_outdated_branch = "This Pull Request is blocked because it's outdated." pulls.blocked_by_changed_protected_files_1= "This Pull Request is blocked because it changes a protected file:" pulls.blocked_by_changed_protected_files_n= "This Pull Request is blocked because it changes protected files:" @@ -1707,6 +1708,8 @@ settings.protected_branch_deletion = Disable Branch Protection settings.protected_branch_deletion_desc = Disabling branch protection allows users with write permission to push to the branch. Continue? settings.block_rejected_reviews = Block merge on rejected reviews settings.block_rejected_reviews_desc = Merging will not be possible when changes are requested by official reviewers, even if there are enough approvals. +settings.block_on_official_review_requests = Block merge on official review requests +settings.block_on_official_review_requests_desc = Merging will not be possible when it has official review requests, even if there are enough approvals. settings.block_outdated_branch = Block merge if pull request is outdated settings.block_outdated_branch_desc = Merging will not be possible when head branch is behind base branch. settings.default_branch_desc = Select a default repository branch for pull requests and code commits: diff --git a/routers/api/v1/repo/branch.go b/routers/api/v1/repo/branch.go index 2b20ab048d..9a0a552bad 100644 --- a/routers/api/v1/repo/branch.go +++ b/routers/api/v1/repo/branch.go @@ -509,21 +509,22 @@ func CreateBranchProtection(ctx *context.APIContext, form api.CreateBranchProtec } protectBranch = &models.ProtectedBranch{ - RepoID: ctx.Repo.Repository.ID, - BranchName: form.BranchName, - CanPush: form.EnablePush, - EnableWhitelist: form.EnablePush && form.EnablePushWhitelist, - EnableMergeWhitelist: form.EnableMergeWhitelist, - WhitelistDeployKeys: form.EnablePush && form.EnablePushWhitelist && form.PushWhitelistDeployKeys, - EnableStatusCheck: form.EnableStatusCheck, - StatusCheckContexts: form.StatusCheckContexts, - EnableApprovalsWhitelist: form.EnableApprovalsWhitelist, - RequiredApprovals: requiredApprovals, - BlockOnRejectedReviews: form.BlockOnRejectedReviews, - DismissStaleApprovals: form.DismissStaleApprovals, - RequireSignedCommits: form.RequireSignedCommits, - ProtectedFilePatterns: form.ProtectedFilePatterns, - BlockOnOutdatedBranch: form.BlockOnOutdatedBranch, + RepoID: ctx.Repo.Repository.ID, + BranchName: form.BranchName, + CanPush: form.EnablePush, + EnableWhitelist: form.EnablePush && form.EnablePushWhitelist, + EnableMergeWhitelist: form.EnableMergeWhitelist, + WhitelistDeployKeys: form.EnablePush && form.EnablePushWhitelist && form.PushWhitelistDeployKeys, + EnableStatusCheck: form.EnableStatusCheck, + StatusCheckContexts: form.StatusCheckContexts, + EnableApprovalsWhitelist: form.EnableApprovalsWhitelist, + RequiredApprovals: requiredApprovals, + BlockOnRejectedReviews: form.BlockOnRejectedReviews, + BlockOnOfficialReviewRequests: form.BlockOnOfficialReviewRequests, + DismissStaleApprovals: form.DismissStaleApprovals, + RequireSignedCommits: form.RequireSignedCommits, + ProtectedFilePatterns: form.ProtectedFilePatterns, + BlockOnOutdatedBranch: form.BlockOnOutdatedBranch, } err = models.UpdateProtectBranch(ctx.Repo.Repository, protectBranch, models.WhitelistOptions{ @@ -652,6 +653,10 @@ func EditBranchProtection(ctx *context.APIContext, form api.EditBranchProtection protectBranch.BlockOnRejectedReviews = *form.BlockOnRejectedReviews } + if form.BlockOnOfficialReviewRequests != nil { + protectBranch.BlockOnOfficialReviewRequests = *form.BlockOnOfficialReviewRequests + } + if form.DismissStaleApprovals != nil { protectBranch.DismissStaleApprovals = *form.DismissStaleApprovals } diff --git a/routers/repo/issue.go b/routers/repo/issue.go index ded9b3c208..6ebc5d6ecb 100644 --- a/routers/repo/issue.go +++ b/routers/repo/issue.go @@ -1455,6 +1455,7 @@ func ViewIssue(ctx *context.Context) { cnt := pull.ProtectedBranch.GetGrantedApprovalsCount(pull) ctx.Data["IsBlockedByApprovals"] = !pull.ProtectedBranch.HasEnoughApprovals(pull) ctx.Data["IsBlockedByRejection"] = pull.ProtectedBranch.MergeBlockedByRejectedReview(pull) + ctx.Data["IsBlockedByOfficialReviewRequests"] = pull.ProtectedBranch.MergeBlockedByOfficialReviewRequests(pull) ctx.Data["IsBlockedByOutdatedBranch"] = pull.ProtectedBranch.MergeBlockedByOutdatedBranch(pull) ctx.Data["GrantedApprovals"] = cnt ctx.Data["RequireSigned"] = pull.ProtectedBranch.RequireSignedCommits diff --git a/routers/repo/setting_protected_branch.go b/routers/repo/setting_protected_branch.go index f864e8a75c..c2e7bc8fac 100644 --- a/routers/repo/setting_protected_branch.go +++ b/routers/repo/setting_protected_branch.go @@ -246,6 +246,7 @@ func SettingsProtectedBranchPost(ctx *context.Context, f auth.ProtectBranchForm) } } protectBranch.BlockOnRejectedReviews = f.BlockOnRejectedReviews + protectBranch.BlockOnOfficialReviewRequests = f.BlockOnOfficialReviewRequests protectBranch.DismissStaleApprovals = f.DismissStaleApprovals protectBranch.RequireSignedCommits = f.RequireSignedCommits protectBranch.ProtectedFilePatterns = f.ProtectedFilePatterns diff --git a/services/pull/merge.go b/services/pull/merge.go index 8e51583e91..3d856724cd 100644 --- a/services/pull/merge.go +++ b/services/pull/merge.go @@ -591,6 +591,11 @@ func CheckPRReadyToMerge(pr *models.PullRequest, skipProtectedFilesCheck bool) ( Reason: "There are requested changes", } } + if pr.ProtectedBranch.MergeBlockedByOfficialReviewRequests(pr) { + return models.ErrNotAllowedToMerge{ + Reason: "There are official review requests", + } + } if pr.ProtectedBranch.MergeBlockedByOutdatedBranch(pr) { return models.ErrNotAllowedToMerge{ diff --git a/templates/repo/issue/view_content/pull.tmpl b/templates/repo/issue/view_content/pull.tmpl index dbecf644d9..9d8ea7ba8a 100644 --- a/templates/repo/issue/view_content/pull.tmpl +++ b/templates/repo/issue/view_content/pull.tmpl @@ -84,6 +84,7 @@ {{- else if .IsPullRequestBroken}}red {{- else if .IsBlockedByApprovals}}red {{- else if .IsBlockedByRejection}}red + {{- else if .IsBlockedByOfficialReviewRequests}}red {{- else if .IsBlockedByOutdatedBranch}}red {{- else if .IsBlockedByChangedProtectedFiles}}red {{- else if and .EnableStatusCheck (or .RequiredStatusCheckState.IsFailure .RequiredStatusCheckState.IsError)}}red @@ -159,6 +160,11 @@ {{svg "octicon-x"}} {{$.i18n.Tr "repo.pulls.blocked_by_rejection"}} + {{else if .IsBlockedByOfficialReviewRequests}} +
+ {{svg "octicon-x"}} + {{$.i18n.Tr "repo.pulls.blocked_by_official_review_requests"}} +
{{else if .IsBlockedByOutdatedBranch}}
{{svg "octicon-x"}} @@ -194,7 +200,7 @@ {{$.i18n.Tr (printf "repo.signing.wont_sign.%s" .WontSignReason) }}
{{end}} - {{$notAllOverridableChecksOk := or .IsBlockedByApprovals .IsBlockedByRejection .IsBlockedByOutdatedBranch .IsBlockedByChangedProtectedFiles (and .EnableStatusCheck (not .RequiredStatusCheckState.IsSuccess))}} + {{$notAllOverridableChecksOk := or .IsBlockedByApprovals .IsBlockedByRejection .IsBlockedByOfficialReviewRequests .IsBlockedByOutdatedBranch .IsBlockedByChangedProtectedFiles (and .EnableStatusCheck (not .RequiredStatusCheckState.IsSuccess))}} {{if and (or $.IsRepoAdmin (not $notAllOverridableChecksOk)) (or (not .AllowMerge) (not .RequireSigned) .WillSign)}} {{if $notAllOverridableChecksOk}}
@@ -384,7 +390,12 @@ {{else if .IsBlockedByRejection}}
{{svg "octicon-x"}} - {{$.i18n.Tr "repo.pulls.blocked_by_rejection"}} + {{$.i18n.Tr "repo.pulls.blocked_by_rejection"}} +
+ {{else if .IsBlockedByOfficialReviewRequests}} +
+ {{svg "octicon-x"}} + {{$.i18n.Tr "repo.pulls.blocked_by_official_review_requests"}}
{{else if .IsBlockedByOutdatedBranch}}
diff --git a/templates/repo/settings/protected_branch.tmpl b/templates/repo/settings/protected_branch.tmpl index 53a978e068..1fbd28d8bd 100644 --- a/templates/repo/settings/protected_branch.tmpl +++ b/templates/repo/settings/protected_branch.tmpl @@ -211,6 +211,13 @@

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

+
+
+ + +

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

+
+
diff --git a/templates/swagger/v1_json.tmpl b/templates/swagger/v1_json.tmpl index 8bcfc43d73..b121451ba6 100644 --- a/templates/swagger/v1_json.tmpl +++ b/templates/swagger/v1_json.tmpl @@ -11326,6 +11326,10 @@ }, "x-go-name": "ApprovalsWhitelistUsernames" }, + "block_on_official_review_requests": { + "type": "boolean", + "x-go-name": "BlockOnOfficialReviewRequests" + }, "block_on_outdated_branch": { "type": "boolean", "x-go-name": "BlockOnOutdatedBranch" @@ -11660,6 +11664,10 @@ }, "x-go-name": "ApprovalsWhitelistUsernames" }, + "block_on_official_review_requests": { + "type": "boolean", + "x-go-name": "BlockOnOfficialReviewRequests" + }, "block_on_outdated_branch": { "type": "boolean", "x-go-name": "BlockOnOutdatedBranch" @@ -12605,6 +12613,10 @@ }, "x-go-name": "ApprovalsWhitelistUsernames" }, + "block_on_official_review_requests": { + "type": "boolean", + "x-go-name": "BlockOnOfficialReviewRequests" + }, "block_on_outdated_branch": { "type": "boolean", "x-go-name": "BlockOnOutdatedBranch"