Skip to content

Commit 9c26dc1

Browse files
a1012112796lafriks
a1012112796
and
lafriks
authored
Add block on official review requests branch protection (#13705)
Signed-off-by: a1012112796 <[email protected]> Co-authored-by: Lauris BH <[email protected]>
1 parent 7ed5bf8 commit 9c26dc1

File tree

14 files changed

+228
-141
lines changed

14 files changed

+228
-141
lines changed

‎models/branches.go

Lines changed: 42 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -21,28 +21,29 @@ import (
2121

2222
// ProtectedBranch struct
2323
type ProtectedBranch struct {
24-
ID int64 `xorm:"pk autoincr"`
25-
RepoID int64 `xorm:"UNIQUE(s)"`
26-
BranchName string `xorm:"UNIQUE(s)"`
27-
CanPush bool `xorm:"NOT NULL DEFAULT false"`
28-
EnableWhitelist bool
29-
WhitelistUserIDs []int64 `xorm:"JSON TEXT"`
30-
WhitelistTeamIDs []int64 `xorm:"JSON TEXT"`
31-
EnableMergeWhitelist bool `xorm:"NOT NULL DEFAULT false"`
32-
WhitelistDeployKeys bool `xorm:"NOT NULL DEFAULT false"`
33-
MergeWhitelistUserIDs []int64 `xorm:"JSON TEXT"`
34-
MergeWhitelistTeamIDs []int64 `xorm:"JSON TEXT"`
35-
EnableStatusCheck bool `xorm:"NOT NULL DEFAULT false"`
36-
StatusCheckContexts []string `xorm:"JSON TEXT"`
37-
EnableApprovalsWhitelist bool `xorm:"NOT NULL DEFAULT false"`
38-
ApprovalsWhitelistUserIDs []int64 `xorm:"JSON TEXT"`
39-
ApprovalsWhitelistTeamIDs []int64 `xorm:"JSON TEXT"`
40-
RequiredApprovals int64 `xorm:"NOT NULL DEFAULT 0"`
41-
BlockOnRejectedReviews bool `xorm:"NOT NULL DEFAULT false"`
42-
BlockOnOutdatedBranch bool `xorm:"NOT NULL DEFAULT false"`
43-
DismissStaleApprovals bool `xorm:"NOT NULL DEFAULT false"`
44-
RequireSignedCommits bool `xorm:"NOT NULL DEFAULT false"`
45-
ProtectedFilePatterns string `xorm:"TEXT"`
24+
ID int64 `xorm:"pk autoincr"`
25+
RepoID int64 `xorm:"UNIQUE(s)"`
26+
BranchName string `xorm:"UNIQUE(s)"`
27+
CanPush bool `xorm:"NOT NULL DEFAULT false"`
28+
EnableWhitelist bool
29+
WhitelistUserIDs []int64 `xorm:"JSON TEXT"`
30+
WhitelistTeamIDs []int64 `xorm:"JSON TEXT"`
31+
EnableMergeWhitelist bool `xorm:"NOT NULL DEFAULT false"`
32+
WhitelistDeployKeys bool `xorm:"NOT NULL DEFAULT false"`
33+
MergeWhitelistUserIDs []int64 `xorm:"JSON TEXT"`
34+
MergeWhitelistTeamIDs []int64 `xorm:"JSON TEXT"`
35+
EnableStatusCheck bool `xorm:"NOT NULL DEFAULT false"`
36+
StatusCheckContexts []string `xorm:"JSON TEXT"`
37+
EnableApprovalsWhitelist bool `xorm:"NOT NULL DEFAULT false"`
38+
ApprovalsWhitelistUserIDs []int64 `xorm:"JSON TEXT"`
39+
ApprovalsWhitelistTeamIDs []int64 `xorm:"JSON TEXT"`
40+
RequiredApprovals int64 `xorm:"NOT NULL DEFAULT 0"`
41+
BlockOnRejectedReviews bool `xorm:"NOT NULL DEFAULT false"`
42+
BlockOnOfficialReviewRequests bool `xorm:"NOT NULL DEFAULT false"`
43+
BlockOnOutdatedBranch bool `xorm:"NOT NULL DEFAULT false"`
44+
DismissStaleApprovals bool `xorm:"NOT NULL DEFAULT false"`
45+
RequireSignedCommits bool `xorm:"NOT NULL DEFAULT false"`
46+
ProtectedFilePatterns string `xorm:"TEXT"`
4647

4748
CreatedUnix timeutil.TimeStamp `xorm:"created"`
4849
UpdatedUnix timeutil.TimeStamp `xorm:"updated"`
@@ -171,13 +172,12 @@ func (protectBranch *ProtectedBranch) GetGrantedApprovalsCount(pr *PullRequest)
171172
}
172173

173174
// MergeBlockedByRejectedReview returns true if merge is blocked by rejected reviews
174-
// An official ReviewRequest should also block Merge like Reject
175175
func (protectBranch *ProtectedBranch) MergeBlockedByRejectedReview(pr *PullRequest) bool {
176176
if !protectBranch.BlockOnRejectedReviews {
177177
return false
178178
}
179179
rejectExist, err := x.Where("issue_id = ?", pr.IssueID).
180-
And("type in ( ?, ?)", ReviewTypeReject, ReviewTypeRequest).
180+
And("type = ?", ReviewTypeReject).
181181
And("official = ?", true).
182182
Exist(new(Review))
183183
if err != nil {
@@ -188,6 +188,24 @@ func (protectBranch *ProtectedBranch) MergeBlockedByRejectedReview(pr *PullReque
188188
return rejectExist
189189
}
190190

191+
// MergeBlockedByOfficialReviewRequests block merge because of some review request to official reviewer
192+
// of from official review
193+
func (protectBranch *ProtectedBranch) MergeBlockedByOfficialReviewRequests(pr *PullRequest) bool {
194+
if !protectBranch.BlockOnOfficialReviewRequests {
195+
return false
196+
}
197+
has, err := x.Where("issue_id = ?", pr.IssueID).
198+
And("type = ?", ReviewTypeRequest).
199+
And("official = ?", true).
200+
Exist(new(Review))
201+
if err != nil {
202+
log.Error("MergeBlockedByOfficialReviewRequests: %v", err)
203+
return true
204+
}
205+
206+
return has
207+
}
208+
191209
// MergeBlockedByOutdatedBranch returns true if merge is blocked by an outdated head branch
192210
func (protectBranch *ProtectedBranch) MergeBlockedByOutdatedBranch(pr *PullRequest) bool {
193211
return protectBranch.BlockOnOutdatedBranch && pr.CommitsBehind > 0

‎models/migrations/migrations.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -254,6 +254,8 @@ var migrations = []Migration{
254254
NewMigration("code comment replies should have the commitID of the review they are replying to", updateCodeCommentReplies),
255255
// v159 -> v160
256256
NewMigration("update reactions constraint", updateReactionConstraint),
257+
// v160 -> v161
258+
NewMigration("Add block on official review requests branch protection", addBlockOnOfficialReviewRequests),
257259
}
258260

259261
// GetCurrentDBVersion returns the current db version

‎models/migrations/v160.go

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
// Copyright 2020 The Gitea Authors. All rights reserved.
2+
// Use of this source code is governed by a MIT-style
3+
// license that can be found in the LICENSE file.
4+
5+
package migrations
6+
7+
import (
8+
"xorm.io/xorm"
9+
)
10+
11+
func addBlockOnOfficialReviewRequests(x *xorm.Engine) error {
12+
type ProtectedBranch struct {
13+
BlockOnOfficialReviewRequests bool `xorm:"NOT NULL DEFAULT false"`
14+
}
15+
16+
return x.Sync2(new(ProtectedBranch))
17+
}

‎modules/auth/repo_form.go

Lines changed: 20 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -178,25 +178,26 @@ func (f *RepoSettingForm) Validate(ctx *macaron.Context, errs binding.Errors) bi
178178

179179
// ProtectBranchForm form for changing protected branch settings
180180
type ProtectBranchForm struct {
181-
Protected bool
182-
EnablePush string
183-
WhitelistUsers string
184-
WhitelistTeams string
185-
WhitelistDeployKeys bool
186-
EnableMergeWhitelist bool
187-
MergeWhitelistUsers string
188-
MergeWhitelistTeams string
189-
EnableStatusCheck bool `xorm:"NOT NULL DEFAULT false"`
190-
StatusCheckContexts []string
191-
RequiredApprovals int64
192-
EnableApprovalsWhitelist bool
193-
ApprovalsWhitelistUsers string
194-
ApprovalsWhitelistTeams string
195-
BlockOnRejectedReviews bool
196-
BlockOnOutdatedBranch bool
197-
DismissStaleApprovals bool
198-
RequireSignedCommits bool
199-
ProtectedFilePatterns string
181+
Protected bool
182+
EnablePush string
183+
WhitelistUsers string
184+
WhitelistTeams string
185+
WhitelistDeployKeys bool
186+
EnableMergeWhitelist bool
187+
MergeWhitelistUsers string
188+
MergeWhitelistTeams string
189+
EnableStatusCheck bool
190+
StatusCheckContexts []string
191+
RequiredApprovals int64
192+
EnableApprovalsWhitelist bool
193+
ApprovalsWhitelistUsers string
194+
ApprovalsWhitelistTeams string
195+
BlockOnRejectedReviews bool
196+
BlockOnOfficialReviewRequests bool
197+
BlockOnOutdatedBranch bool
198+
DismissStaleApprovals bool
199+
RequireSignedCommits bool
200+
ProtectedFilePatterns string
200201
}
201202

202203
// Validate validates the fields

‎modules/convert/convert.go

Lines changed: 23 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -105,28 +105,29 @@ func ToBranchProtection(bp *models.ProtectedBranch) *api.BranchProtection {
105105
}
106106

107107
return &api.BranchProtection{
108-
BranchName: bp.BranchName,
109-
EnablePush: bp.CanPush,
110-
EnablePushWhitelist: bp.EnableWhitelist,
111-
PushWhitelistUsernames: pushWhitelistUsernames,
112-
PushWhitelistTeams: pushWhitelistTeams,
113-
PushWhitelistDeployKeys: bp.WhitelistDeployKeys,
114-
EnableMergeWhitelist: bp.EnableMergeWhitelist,
115-
MergeWhitelistUsernames: mergeWhitelistUsernames,
116-
MergeWhitelistTeams: mergeWhitelistTeams,
117-
EnableStatusCheck: bp.EnableStatusCheck,
118-
StatusCheckContexts: bp.StatusCheckContexts,
119-
RequiredApprovals: bp.RequiredApprovals,
120-
EnableApprovalsWhitelist: bp.EnableApprovalsWhitelist,
121-
ApprovalsWhitelistUsernames: approvalsWhitelistUsernames,
122-
ApprovalsWhitelistTeams: approvalsWhitelistTeams,
123-
BlockOnRejectedReviews: bp.BlockOnRejectedReviews,
124-
BlockOnOutdatedBranch: bp.BlockOnOutdatedBranch,
125-
DismissStaleApprovals: bp.DismissStaleApprovals,
126-
RequireSignedCommits: bp.RequireSignedCommits,
127-
ProtectedFilePatterns: bp.ProtectedFilePatterns,
128-
Created: bp.CreatedUnix.AsTime(),
129-
Updated: bp.UpdatedUnix.AsTime(),
108+
BranchName: bp.BranchName,
109+
EnablePush: bp.CanPush,
110+
EnablePushWhitelist: bp.EnableWhitelist,
111+
PushWhitelistUsernames: pushWhitelistUsernames,
112+
PushWhitelistTeams: pushWhitelistTeams,
113+
PushWhitelistDeployKeys: bp.WhitelistDeployKeys,
114+
EnableMergeWhitelist: bp.EnableMergeWhitelist,
115+
MergeWhitelistUsernames: mergeWhitelistUsernames,
116+
MergeWhitelistTeams: mergeWhitelistTeams,
117+
EnableStatusCheck: bp.EnableStatusCheck,
118+
StatusCheckContexts: bp.StatusCheckContexts,
119+
RequiredApprovals: bp.RequiredApprovals,
120+
EnableApprovalsWhitelist: bp.EnableApprovalsWhitelist,
121+
ApprovalsWhitelistUsernames: approvalsWhitelistUsernames,
122+
ApprovalsWhitelistTeams: approvalsWhitelistTeams,
123+
BlockOnRejectedReviews: bp.BlockOnRejectedReviews,
124+
BlockOnOfficialReviewRequests: bp.BlockOnOfficialReviewRequests,
125+
BlockOnOutdatedBranch: bp.BlockOnOutdatedBranch,
126+
DismissStaleApprovals: bp.DismissStaleApprovals,
127+
RequireSignedCommits: bp.RequireSignedCommits,
128+
ProtectedFilePatterns: bp.ProtectedFilePatterns,
129+
Created: bp.CreatedUnix.AsTime(),
130+
Updated: bp.UpdatedUnix.AsTime(),
130131
}
131132
}
132133

‎modules/structs/repo_branch.go

Lines changed: 62 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -23,26 +23,27 @@ type Branch struct {
2323

2424
// BranchProtection represents a branch protection for a repository
2525
type BranchProtection struct {
26-
BranchName string `json:"branch_name"`
27-
EnablePush bool `json:"enable_push"`
28-
EnablePushWhitelist bool `json:"enable_push_whitelist"`
29-
PushWhitelistUsernames []string `json:"push_whitelist_usernames"`
30-
PushWhitelistTeams []string `json:"push_whitelist_teams"`
31-
PushWhitelistDeployKeys bool `json:"push_whitelist_deploy_keys"`
32-
EnableMergeWhitelist bool `json:"enable_merge_whitelist"`
33-
MergeWhitelistUsernames []string `json:"merge_whitelist_usernames"`
34-
MergeWhitelistTeams []string `json:"merge_whitelist_teams"`
35-
EnableStatusCheck bool `json:"enable_status_check"`
36-
StatusCheckContexts []string `json:"status_check_contexts"`
37-
RequiredApprovals int64 `json:"required_approvals"`
38-
EnableApprovalsWhitelist bool `json:"enable_approvals_whitelist"`
39-
ApprovalsWhitelistUsernames []string `json:"approvals_whitelist_username"`
40-
ApprovalsWhitelistTeams []string `json:"approvals_whitelist_teams"`
41-
BlockOnRejectedReviews bool `json:"block_on_rejected_reviews"`
42-
BlockOnOutdatedBranch bool `json:"block_on_outdated_branch"`
43-
DismissStaleApprovals bool `json:"dismiss_stale_approvals"`
44-
RequireSignedCommits bool `json:"require_signed_commits"`
45-
ProtectedFilePatterns string `json:"protected_file_patterns"`
26+
BranchName string `json:"branch_name"`
27+
EnablePush bool `json:"enable_push"`
28+
EnablePushWhitelist bool `json:"enable_push_whitelist"`
29+
PushWhitelistUsernames []string `json:"push_whitelist_usernames"`
30+
PushWhitelistTeams []string `json:"push_whitelist_teams"`
31+
PushWhitelistDeployKeys bool `json:"push_whitelist_deploy_keys"`
32+
EnableMergeWhitelist bool `json:"enable_merge_whitelist"`
33+
MergeWhitelistUsernames []string `json:"merge_whitelist_usernames"`
34+
MergeWhitelistTeams []string `json:"merge_whitelist_teams"`
35+
EnableStatusCheck bool `json:"enable_status_check"`
36+
StatusCheckContexts []string `json:"status_check_contexts"`
37+
RequiredApprovals int64 `json:"required_approvals"`
38+
EnableApprovalsWhitelist bool `json:"enable_approvals_whitelist"`
39+
ApprovalsWhitelistUsernames []string `json:"approvals_whitelist_username"`
40+
ApprovalsWhitelistTeams []string `json:"approvals_whitelist_teams"`
41+
BlockOnRejectedReviews bool `json:"block_on_rejected_reviews"`
42+
BlockOnOfficialReviewRequests bool `json:"block_on_official_review_requests"`
43+
BlockOnOutdatedBranch bool `json:"block_on_outdated_branch"`
44+
DismissStaleApprovals bool `json:"dismiss_stale_approvals"`
45+
RequireSignedCommits bool `json:"require_signed_commits"`
46+
ProtectedFilePatterns string `json:"protected_file_patterns"`
4647
// swagger:strfmt date-time
4748
Created time.Time `json:"created_at"`
4849
// swagger:strfmt date-time
@@ -51,47 +52,49 @@ type BranchProtection struct {
5152

5253
// CreateBranchProtectionOption options for creating a branch protection
5354
type CreateBranchProtectionOption struct {
54-
BranchName string `json:"branch_name"`
55-
EnablePush bool `json:"enable_push"`
56-
EnablePushWhitelist bool `json:"enable_push_whitelist"`
57-
PushWhitelistUsernames []string `json:"push_whitelist_usernames"`
58-
PushWhitelistTeams []string `json:"push_whitelist_teams"`
59-
PushWhitelistDeployKeys bool `json:"push_whitelist_deploy_keys"`
60-
EnableMergeWhitelist bool `json:"enable_merge_whitelist"`
61-
MergeWhitelistUsernames []string `json:"merge_whitelist_usernames"`
62-
MergeWhitelistTeams []string `json:"merge_whitelist_teams"`
63-
EnableStatusCheck bool `json:"enable_status_check"`
64-
StatusCheckContexts []string `json:"status_check_contexts"`
65-
RequiredApprovals int64 `json:"required_approvals"`
66-
EnableApprovalsWhitelist bool `json:"enable_approvals_whitelist"`
67-
ApprovalsWhitelistUsernames []string `json:"approvals_whitelist_username"`
68-
ApprovalsWhitelistTeams []string `json:"approvals_whitelist_teams"`
69-
BlockOnRejectedReviews bool `json:"block_on_rejected_reviews"`
70-
BlockOnOutdatedBranch bool `json:"block_on_outdated_branch"`
71-
DismissStaleApprovals bool `json:"dismiss_stale_approvals"`
72-
RequireSignedCommits bool `json:"require_signed_commits"`
73-
ProtectedFilePatterns string `json:"protected_file_patterns"`
55+
BranchName string `json:"branch_name"`
56+
EnablePush bool `json:"enable_push"`
57+
EnablePushWhitelist bool `json:"enable_push_whitelist"`
58+
PushWhitelistUsernames []string `json:"push_whitelist_usernames"`
59+
PushWhitelistTeams []string `json:"push_whitelist_teams"`
60+
PushWhitelistDeployKeys bool `json:"push_whitelist_deploy_keys"`
61+
EnableMergeWhitelist bool `json:"enable_merge_whitelist"`
62+
MergeWhitelistUsernames []string `json:"merge_whitelist_usernames"`
63+
MergeWhitelistTeams []string `json:"merge_whitelist_teams"`
64+
EnableStatusCheck bool `json:"enable_status_check"`
65+
StatusCheckContexts []string `json:"status_check_contexts"`
66+
RequiredApprovals int64 `json:"required_approvals"`
67+
EnableApprovalsWhitelist bool `json:"enable_approvals_whitelist"`
68+
ApprovalsWhitelistUsernames []string `json:"approvals_whitelist_username"`
69+
ApprovalsWhitelistTeams []string `json:"approvals_whitelist_teams"`
70+
BlockOnRejectedReviews bool `json:"block_on_rejected_reviews"`
71+
BlockOnOfficialReviewRequests bool `json:"block_on_official_review_requests"`
72+
BlockOnOutdatedBranch bool `json:"block_on_outdated_branch"`
73+
DismissStaleApprovals bool `json:"dismiss_stale_approvals"`
74+
RequireSignedCommits bool `json:"require_signed_commits"`
75+
ProtectedFilePatterns string `json:"protected_file_patterns"`
7476
}
7577

7678
// EditBranchProtectionOption options for editing a branch protection
7779
type EditBranchProtectionOption struct {
78-
EnablePush *bool `json:"enable_push"`
79-
EnablePushWhitelist *bool `json:"enable_push_whitelist"`
80-
PushWhitelistUsernames []string `json:"push_whitelist_usernames"`
81-
PushWhitelistTeams []string `json:"push_whitelist_teams"`
82-
PushWhitelistDeployKeys *bool `json:"push_whitelist_deploy_keys"`
83-
EnableMergeWhitelist *bool `json:"enable_merge_whitelist"`
84-
MergeWhitelistUsernames []string `json:"merge_whitelist_usernames"`
85-
MergeWhitelistTeams []string `json:"merge_whitelist_teams"`
86-
EnableStatusCheck *bool `json:"enable_status_check"`
87-
StatusCheckContexts []string `json:"status_check_contexts"`
88-
RequiredApprovals *int64 `json:"required_approvals"`
89-
EnableApprovalsWhitelist *bool `json:"enable_approvals_whitelist"`
90-
ApprovalsWhitelistUsernames []string `json:"approvals_whitelist_username"`
91-
ApprovalsWhitelistTeams []string `json:"approvals_whitelist_teams"`
92-
BlockOnRejectedReviews *bool `json:"block_on_rejected_reviews"`
93-
BlockOnOutdatedBranch *bool `json:"block_on_outdated_branch"`
94-
DismissStaleApprovals *bool `json:"dismiss_stale_approvals"`
95-
RequireSignedCommits *bool `json:"require_signed_commits"`
96-
ProtectedFilePatterns *string `json:"protected_file_patterns"`
80+
EnablePush *bool `json:"enable_push"`
81+
EnablePushWhitelist *bool `json:"enable_push_whitelist"`
82+
PushWhitelistUsernames []string `json:"push_whitelist_usernames"`
83+
PushWhitelistTeams []string `json:"push_whitelist_teams"`
84+
PushWhitelistDeployKeys *bool `json:"push_whitelist_deploy_keys"`
85+
EnableMergeWhitelist *bool `json:"enable_merge_whitelist"`
86+
MergeWhitelistUsernames []string `json:"merge_whitelist_usernames"`
87+
MergeWhitelistTeams []string `json:"merge_whitelist_teams"`
88+
EnableStatusCheck *bool `json:"enable_status_check"`
89+
StatusCheckContexts []string `json:"status_check_contexts"`
90+
RequiredApprovals *int64 `json:"required_approvals"`
91+
EnableApprovalsWhitelist *bool `json:"enable_approvals_whitelist"`
92+
ApprovalsWhitelistUsernames []string `json:"approvals_whitelist_username"`
93+
ApprovalsWhitelistTeams []string `json:"approvals_whitelist_teams"`
94+
BlockOnRejectedReviews *bool `json:"block_on_rejected_reviews"`
95+
BlockOnOfficialReviewRequests *bool `json:"block_on_official_review_requests"`
96+
BlockOnOutdatedBranch *bool `json:"block_on_outdated_branch"`
97+
DismissStaleApprovals *bool `json:"dismiss_stale_approvals"`
98+
RequireSignedCommits *bool `json:"require_signed_commits"`
99+
ProtectedFilePatterns *string `json:"protected_file_patterns"`
97100
}

0 commit comments

Comments
 (0)