Add block on official review requests branch protection (#13705)

Signed-off-by: a1012112796 <1012112796@qq.com>

Co-authored-by: Lauris BH <lauris@nix.lv>
This commit is contained in:
a1012112796 2020-11-29 03:30:46 +08:00 committed by GitHub
parent 7ed5bf8cbe
commit 9c26dc1f3a
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
14 changed files with 228 additions and 141 deletions

View File

@ -39,6 +39,7 @@ type ProtectedBranch struct {
ApprovalsWhitelistTeamIDs []int64 `xorm:"JSON TEXT"` ApprovalsWhitelistTeamIDs []int64 `xorm:"JSON TEXT"`
RequiredApprovals int64 `xorm:"NOT NULL DEFAULT 0"` RequiredApprovals int64 `xorm:"NOT NULL DEFAULT 0"`
BlockOnRejectedReviews bool `xorm:"NOT NULL DEFAULT false"` BlockOnRejectedReviews bool `xorm:"NOT NULL DEFAULT false"`
BlockOnOfficialReviewRequests bool `xorm:"NOT NULL DEFAULT false"`
BlockOnOutdatedBranch bool `xorm:"NOT NULL DEFAULT false"` BlockOnOutdatedBranch bool `xorm:"NOT NULL DEFAULT false"`
DismissStaleApprovals bool `xorm:"NOT NULL DEFAULT false"` DismissStaleApprovals bool `xorm:"NOT NULL DEFAULT false"`
RequireSignedCommits bool `xorm:"NOT NULL DEFAULT false"` RequireSignedCommits bool `xorm:"NOT NULL DEFAULT false"`
@ -171,13 +172,12 @@ func (protectBranch *ProtectedBranch) GetGrantedApprovalsCount(pr *PullRequest)
} }
// MergeBlockedByRejectedReview returns true if merge is blocked by rejected reviews // 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 { func (protectBranch *ProtectedBranch) MergeBlockedByRejectedReview(pr *PullRequest) bool {
if !protectBranch.BlockOnRejectedReviews { if !protectBranch.BlockOnRejectedReviews {
return false return false
} }
rejectExist, err := x.Where("issue_id = ?", pr.IssueID). rejectExist, err := x.Where("issue_id = ?", pr.IssueID).
And("type in ( ?, ?)", ReviewTypeReject, ReviewTypeRequest). And("type = ?", ReviewTypeReject).
And("official = ?", true). And("official = ?", true).
Exist(new(Review)) Exist(new(Review))
if err != nil { if err != nil {
@ -188,6 +188,24 @@ func (protectBranch *ProtectedBranch) MergeBlockedByRejectedReview(pr *PullReque
return rejectExist 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 // MergeBlockedByOutdatedBranch returns true if merge is blocked by an outdated head branch
func (protectBranch *ProtectedBranch) MergeBlockedByOutdatedBranch(pr *PullRequest) bool { func (protectBranch *ProtectedBranch) MergeBlockedByOutdatedBranch(pr *PullRequest) bool {
return protectBranch.BlockOnOutdatedBranch && pr.CommitsBehind > 0 return protectBranch.BlockOnOutdatedBranch && pr.CommitsBehind > 0

View File

@ -254,6 +254,8 @@ var migrations = []Migration{
NewMigration("code comment replies should have the commitID of the review they are replying to", updateCodeCommentReplies), NewMigration("code comment replies should have the commitID of the review they are replying to", updateCodeCommentReplies),
// v159 -> v160 // v159 -> v160
NewMigration("update reactions constraint", updateReactionConstraint), NewMigration("update reactions constraint", updateReactionConstraint),
// v160 -> v161
NewMigration("Add block on official review requests branch protection", addBlockOnOfficialReviewRequests),
} }
// GetCurrentDBVersion returns the current db version // GetCurrentDBVersion returns the current db version

17
models/migrations/v160.go Normal file
View File

@ -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))
}

View File

@ -186,13 +186,14 @@ type ProtectBranchForm struct {
EnableMergeWhitelist bool EnableMergeWhitelist bool
MergeWhitelistUsers string MergeWhitelistUsers string
MergeWhitelistTeams string MergeWhitelistTeams string
EnableStatusCheck bool `xorm:"NOT NULL DEFAULT false"` EnableStatusCheck bool
StatusCheckContexts []string StatusCheckContexts []string
RequiredApprovals int64 RequiredApprovals int64
EnableApprovalsWhitelist bool EnableApprovalsWhitelist bool
ApprovalsWhitelistUsers string ApprovalsWhitelistUsers string
ApprovalsWhitelistTeams string ApprovalsWhitelistTeams string
BlockOnRejectedReviews bool BlockOnRejectedReviews bool
BlockOnOfficialReviewRequests bool
BlockOnOutdatedBranch bool BlockOnOutdatedBranch bool
DismissStaleApprovals bool DismissStaleApprovals bool
RequireSignedCommits bool RequireSignedCommits bool

View File

@ -121,6 +121,7 @@ func ToBranchProtection(bp *models.ProtectedBranch) *api.BranchProtection {
ApprovalsWhitelistUsernames: approvalsWhitelistUsernames, ApprovalsWhitelistUsernames: approvalsWhitelistUsernames,
ApprovalsWhitelistTeams: approvalsWhitelistTeams, ApprovalsWhitelistTeams: approvalsWhitelistTeams,
BlockOnRejectedReviews: bp.BlockOnRejectedReviews, BlockOnRejectedReviews: bp.BlockOnRejectedReviews,
BlockOnOfficialReviewRequests: bp.BlockOnOfficialReviewRequests,
BlockOnOutdatedBranch: bp.BlockOnOutdatedBranch, BlockOnOutdatedBranch: bp.BlockOnOutdatedBranch,
DismissStaleApprovals: bp.DismissStaleApprovals, DismissStaleApprovals: bp.DismissStaleApprovals,
RequireSignedCommits: bp.RequireSignedCommits, RequireSignedCommits: bp.RequireSignedCommits,

View File

@ -39,6 +39,7 @@ type BranchProtection struct {
ApprovalsWhitelistUsernames []string `json:"approvals_whitelist_username"` ApprovalsWhitelistUsernames []string `json:"approvals_whitelist_username"`
ApprovalsWhitelistTeams []string `json:"approvals_whitelist_teams"` ApprovalsWhitelistTeams []string `json:"approvals_whitelist_teams"`
BlockOnRejectedReviews bool `json:"block_on_rejected_reviews"` BlockOnRejectedReviews bool `json:"block_on_rejected_reviews"`
BlockOnOfficialReviewRequests bool `json:"block_on_official_review_requests"`
BlockOnOutdatedBranch bool `json:"block_on_outdated_branch"` BlockOnOutdatedBranch bool `json:"block_on_outdated_branch"`
DismissStaleApprovals bool `json:"dismiss_stale_approvals"` DismissStaleApprovals bool `json:"dismiss_stale_approvals"`
RequireSignedCommits bool `json:"require_signed_commits"` RequireSignedCommits bool `json:"require_signed_commits"`
@ -67,6 +68,7 @@ type CreateBranchProtectionOption struct {
ApprovalsWhitelistUsernames []string `json:"approvals_whitelist_username"` ApprovalsWhitelistUsernames []string `json:"approvals_whitelist_username"`
ApprovalsWhitelistTeams []string `json:"approvals_whitelist_teams"` ApprovalsWhitelistTeams []string `json:"approvals_whitelist_teams"`
BlockOnRejectedReviews bool `json:"block_on_rejected_reviews"` BlockOnRejectedReviews bool `json:"block_on_rejected_reviews"`
BlockOnOfficialReviewRequests bool `json:"block_on_official_review_requests"`
BlockOnOutdatedBranch bool `json:"block_on_outdated_branch"` BlockOnOutdatedBranch bool `json:"block_on_outdated_branch"`
DismissStaleApprovals bool `json:"dismiss_stale_approvals"` DismissStaleApprovals bool `json:"dismiss_stale_approvals"`
RequireSignedCommits bool `json:"require_signed_commits"` RequireSignedCommits bool `json:"require_signed_commits"`
@ -90,6 +92,7 @@ type EditBranchProtectionOption struct {
ApprovalsWhitelistUsernames []string `json:"approvals_whitelist_username"` ApprovalsWhitelistUsernames []string `json:"approvals_whitelist_username"`
ApprovalsWhitelistTeams []string `json:"approvals_whitelist_teams"` ApprovalsWhitelistTeams []string `json:"approvals_whitelist_teams"`
BlockOnRejectedReviews *bool `json:"block_on_rejected_reviews"` BlockOnRejectedReviews *bool `json:"block_on_rejected_reviews"`
BlockOnOfficialReviewRequests *bool `json:"block_on_official_review_requests"`
BlockOnOutdatedBranch *bool `json:"block_on_outdated_branch"` BlockOnOutdatedBranch *bool `json:"block_on_outdated_branch"`
DismissStaleApprovals *bool `json:"dismiss_stale_approvals"` DismissStaleApprovals *bool `json:"dismiss_stale_approvals"`
RequireSignedCommits *bool `json:"require_signed_commits"` RequireSignedCommits *bool `json:"require_signed_commits"`

View File

@ -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.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_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_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_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_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:" 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.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 = 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_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 = 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.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: settings.default_branch_desc = Select a default repository branch for pull requests and code commits:

View File

@ -520,6 +520,7 @@ func CreateBranchProtection(ctx *context.APIContext, form api.CreateBranchProtec
EnableApprovalsWhitelist: form.EnableApprovalsWhitelist, EnableApprovalsWhitelist: form.EnableApprovalsWhitelist,
RequiredApprovals: requiredApprovals, RequiredApprovals: requiredApprovals,
BlockOnRejectedReviews: form.BlockOnRejectedReviews, BlockOnRejectedReviews: form.BlockOnRejectedReviews,
BlockOnOfficialReviewRequests: form.BlockOnOfficialReviewRequests,
DismissStaleApprovals: form.DismissStaleApprovals, DismissStaleApprovals: form.DismissStaleApprovals,
RequireSignedCommits: form.RequireSignedCommits, RequireSignedCommits: form.RequireSignedCommits,
ProtectedFilePatterns: form.ProtectedFilePatterns, ProtectedFilePatterns: form.ProtectedFilePatterns,
@ -652,6 +653,10 @@ func EditBranchProtection(ctx *context.APIContext, form api.EditBranchProtection
protectBranch.BlockOnRejectedReviews = *form.BlockOnRejectedReviews protectBranch.BlockOnRejectedReviews = *form.BlockOnRejectedReviews
} }
if form.BlockOnOfficialReviewRequests != nil {
protectBranch.BlockOnOfficialReviewRequests = *form.BlockOnOfficialReviewRequests
}
if form.DismissStaleApprovals != nil { if form.DismissStaleApprovals != nil {
protectBranch.DismissStaleApprovals = *form.DismissStaleApprovals protectBranch.DismissStaleApprovals = *form.DismissStaleApprovals
} }

View File

@ -1455,6 +1455,7 @@ func ViewIssue(ctx *context.Context) {
cnt := pull.ProtectedBranch.GetGrantedApprovalsCount(pull) cnt := pull.ProtectedBranch.GetGrantedApprovalsCount(pull)
ctx.Data["IsBlockedByApprovals"] = !pull.ProtectedBranch.HasEnoughApprovals(pull) ctx.Data["IsBlockedByApprovals"] = !pull.ProtectedBranch.HasEnoughApprovals(pull)
ctx.Data["IsBlockedByRejection"] = pull.ProtectedBranch.MergeBlockedByRejectedReview(pull) ctx.Data["IsBlockedByRejection"] = pull.ProtectedBranch.MergeBlockedByRejectedReview(pull)
ctx.Data["IsBlockedByOfficialReviewRequests"] = pull.ProtectedBranch.MergeBlockedByOfficialReviewRequests(pull)
ctx.Data["IsBlockedByOutdatedBranch"] = pull.ProtectedBranch.MergeBlockedByOutdatedBranch(pull) ctx.Data["IsBlockedByOutdatedBranch"] = pull.ProtectedBranch.MergeBlockedByOutdatedBranch(pull)
ctx.Data["GrantedApprovals"] = cnt ctx.Data["GrantedApprovals"] = cnt
ctx.Data["RequireSigned"] = pull.ProtectedBranch.RequireSignedCommits ctx.Data["RequireSigned"] = pull.ProtectedBranch.RequireSignedCommits

View File

@ -246,6 +246,7 @@ func SettingsProtectedBranchPost(ctx *context.Context, f auth.ProtectBranchForm)
} }
} }
protectBranch.BlockOnRejectedReviews = f.BlockOnRejectedReviews protectBranch.BlockOnRejectedReviews = f.BlockOnRejectedReviews
protectBranch.BlockOnOfficialReviewRequests = f.BlockOnOfficialReviewRequests
protectBranch.DismissStaleApprovals = f.DismissStaleApprovals protectBranch.DismissStaleApprovals = f.DismissStaleApprovals
protectBranch.RequireSignedCommits = f.RequireSignedCommits protectBranch.RequireSignedCommits = f.RequireSignedCommits
protectBranch.ProtectedFilePatterns = f.ProtectedFilePatterns protectBranch.ProtectedFilePatterns = f.ProtectedFilePatterns

View File

@ -591,6 +591,11 @@ func CheckPRReadyToMerge(pr *models.PullRequest, skipProtectedFilesCheck bool) (
Reason: "There are requested changes", Reason: "There are requested changes",
} }
} }
if pr.ProtectedBranch.MergeBlockedByOfficialReviewRequests(pr) {
return models.ErrNotAllowedToMerge{
Reason: "There are official review requests",
}
}
if pr.ProtectedBranch.MergeBlockedByOutdatedBranch(pr) { if pr.ProtectedBranch.MergeBlockedByOutdatedBranch(pr) {
return models.ErrNotAllowedToMerge{ return models.ErrNotAllowedToMerge{

View File

@ -84,6 +84,7 @@
{{- else if .IsPullRequestBroken}}red {{- else if .IsPullRequestBroken}}red
{{- else if .IsBlockedByApprovals}}red {{- else if .IsBlockedByApprovals}}red
{{- else if .IsBlockedByRejection}}red {{- else if .IsBlockedByRejection}}red
{{- else if .IsBlockedByOfficialReviewRequests}}red
{{- else if .IsBlockedByOutdatedBranch}}red {{- else if .IsBlockedByOutdatedBranch}}red
{{- else if .IsBlockedByChangedProtectedFiles}}red {{- else if .IsBlockedByChangedProtectedFiles}}red
{{- else if and .EnableStatusCheck (or .RequiredStatusCheckState.IsFailure .RequiredStatusCheckState.IsError)}}red {{- else if and .EnableStatusCheck (or .RequiredStatusCheckState.IsFailure .RequiredStatusCheckState.IsError)}}red
@ -159,6 +160,11 @@
<i class="icon icon-octicon">{{svg "octicon-x"}}</i> <i class="icon icon-octicon">{{svg "octicon-x"}}</i>
{{$.i18n.Tr "repo.pulls.blocked_by_rejection"}} {{$.i18n.Tr "repo.pulls.blocked_by_rejection"}}
</div> </div>
{{else if .IsBlockedByOfficialReviewRequests}}
<div class="item">
<i class="icon icon-octicon">{{svg "octicon-x"}}</i>
{{$.i18n.Tr "repo.pulls.blocked_by_official_review_requests"}}
</div>
{{else if .IsBlockedByOutdatedBranch}} {{else if .IsBlockedByOutdatedBranch}}
<div class="item"> <div class="item">
<i class="icon icon-octicon">{{svg "octicon-x"}}</i> <i class="icon icon-octicon">{{svg "octicon-x"}}</i>
@ -194,7 +200,7 @@
{{$.i18n.Tr (printf "repo.signing.wont_sign.%s" .WontSignReason) }} {{$.i18n.Tr (printf "repo.signing.wont_sign.%s" .WontSignReason) }}
</div> </div>
{{end}} {{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 and (or $.IsRepoAdmin (not $notAllOverridableChecksOk)) (or (not .AllowMerge) (not .RequireSigned) .WillSign)}}
{{if $notAllOverridableChecksOk}} {{if $notAllOverridableChecksOk}}
<div class="item"> <div class="item">
@ -386,6 +392,11 @@
{{svg "octicon-x"}} {{svg "octicon-x"}}
{{$.i18n.Tr "repo.pulls.blocked_by_rejection"}} {{$.i18n.Tr "repo.pulls.blocked_by_rejection"}}
</div> </div>
{{else if .IsBlockedByOfficialReviewRequests}}
<div class="item text red">
{{svg "octicon-x"}}
{{$.i18n.Tr "repo.pulls.blocked_by_official_review_requests"}}
</div>
{{else if .IsBlockedByOutdatedBranch}} {{else if .IsBlockedByOutdatedBranch}}
<div class="item text red"> <div class="item text red">
<i class="icon icon-octicon">{{svg "octicon-x"}}</i> <i class="icon icon-octicon">{{svg "octicon-x"}}</i>

View File

@ -211,6 +211,13 @@
<p class="help">{{.i18n.Tr "repo.settings.block_rejected_reviews_desc"}}</p> <p class="help">{{.i18n.Tr "repo.settings.block_rejected_reviews_desc"}}</p>
</div> </div>
</div> </div>
<div class="field">
<div class="ui checkbox">
<input name="block_on_official_review_requests" type="checkbox" {{if .Branch.BlockOnOfficialReviewRequests}}checked{{end}}>
<label for="block_on_official_review_requests">{{.i18n.Tr "repo.settings.block_on_official_review_requests"}}</label>
<p class="help">{{.i18n.Tr "repo.settings.block_on_official_review_requests_desc"}}</p>
</div>
</div>
<div class="field"> <div class="field">
<div class="ui checkbox"> <div class="ui checkbox">
<input name="dismiss_stale_approvals" type="checkbox" {{if .Branch.DismissStaleApprovals}}checked{{end}}> <input name="dismiss_stale_approvals" type="checkbox" {{if .Branch.DismissStaleApprovals}}checked{{end}}>

View File

@ -11326,6 +11326,10 @@
}, },
"x-go-name": "ApprovalsWhitelistUsernames" "x-go-name": "ApprovalsWhitelistUsernames"
}, },
"block_on_official_review_requests": {
"type": "boolean",
"x-go-name": "BlockOnOfficialReviewRequests"
},
"block_on_outdated_branch": { "block_on_outdated_branch": {
"type": "boolean", "type": "boolean",
"x-go-name": "BlockOnOutdatedBranch" "x-go-name": "BlockOnOutdatedBranch"
@ -11660,6 +11664,10 @@
}, },
"x-go-name": "ApprovalsWhitelistUsernames" "x-go-name": "ApprovalsWhitelistUsernames"
}, },
"block_on_official_review_requests": {
"type": "boolean",
"x-go-name": "BlockOnOfficialReviewRequests"
},
"block_on_outdated_branch": { "block_on_outdated_branch": {
"type": "boolean", "type": "boolean",
"x-go-name": "BlockOnOutdatedBranch" "x-go-name": "BlockOnOutdatedBranch"
@ -12605,6 +12613,10 @@
}, },
"x-go-name": "ApprovalsWhitelistUsernames" "x-go-name": "ApprovalsWhitelistUsernames"
}, },
"block_on_official_review_requests": {
"type": "boolean",
"x-go-name": "BlockOnOfficialReviewRequests"
},
"block_on_outdated_branch": { "block_on_outdated_branch": {
"type": "boolean", "type": "boolean",
"x-go-name": "BlockOnOutdatedBranch" "x-go-name": "BlockOnOutdatedBranch"