fedi/forgejo
1
0
Fork 0
mirror of https://codeberg.org/forgejo/forgejo.git synced 2024-12-20 23:49:23 +00:00
Code Issues Packages Projects Releases Wiki Activity

Add branch protection setting for ignoring stale approvals (#28498)

Browse source 
Fixes #27114.

* In Gitea 1.12 (#9532), a "dismiss stale approvals" branch protection
setting was introduced, for ignoring stale reviews when verifying the
approval count of a pull request.
* In Gitea 1.14 (#12674), the "dismiss review" feature was added.
* This caused confusion with users (#25858), as "dismiss" now means 2
different things.
* In Gitea 1.20 (#25882), the behavior of the "dismiss stale approvals"
branch protection was modified to actually dismiss the stale review.

For some users this new behavior of dismissing the stale reviews is not
desirable.

So this PR reintroduces the old behavior as a new "ignore stale
approvals" branch protection setting.

---------

Co-authored-by: delvh <dev.lh@web.de>
This commit is contained in:
Jimmy Praet 2024-01-15 08:20:01 +01:00 committed by GitHub
parent ce0225c1b8
commit 5d3fdd1212
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
12 changed files with 53 additions and 2 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

1
models/git/protected_branch.go
View file

@ -54,6 +54,7 @@ type ProtectedBranch struct {
BlockOnOfficialReviewRequests bool `xorm:"NOT NULL DEFAULT false"`
BlockOnOutdatedBranch bool `xorm:"NOT NULL DEFAULT false"`
DismissStaleApprovals bool `xorm:"NOT NULL DEFAULT false"`
IgnoreStaleApprovals bool `xorm:"NOT NULL DEFAULT false"`
RequireSignedCommits bool `xorm:"NOT NULL DEFAULT false"`
ProtectedFilePatterns string `xorm:"TEXT"`
UnprotectedFilePatterns string `xorm:"TEXT"`

2
models/issues/pull.go
View file

@ -801,7 +801,7 @@ func GetGrantedApprovalsCount(ctx context.Context, protectBranch *git_model.Prot
And("type = ?", ReviewTypeApprove).
And("official = ?", true).
And("dismissed = ?", false)
if protectBranch.DismissStaleApprovals {
if protectBranch.IgnoreStaleApprovals {
sess = sess.And("stale = ?", false)
}
approvals, err := sess.Count(new(Review))

14
models/migrations/v1_22/v284.go Normal file
View file

@ -0,0 +1,14 @@
// Copyright 2023 The Gitea Authors. All rights reserved.
// SPDX-License-Identifier: MIT
package v1_22 //nolint
import (
"xorm.io/xorm"
)
func AddIgnoreStaleApprovalsColumnToProtectedBranchTable(x *xorm.Engine) error {
type ProtectedBranch struct {
IgnoreStaleApprovals bool `xorm:"NOT NULL DEFAULT false"`
}
return x.Sync(new(ProtectedBranch))
}

3
modules/structs/repo_branch.go
View file

@ -43,6 +43,7 @@ type BranchProtection struct {
BlockOnOfficialReviewRequests bool `json:"block_on_official_review_requests"`
BlockOnOutdatedBranch bool `json:"block_on_outdated_branch"`
DismissStaleApprovals bool `json:"dismiss_stale_approvals"`
IgnoreStaleApprovals bool `json:"ignore_stale_approvals"`
RequireSignedCommits bool `json:"require_signed_commits"`
ProtectedFilePatterns string `json:"protected_file_patterns"`
UnprotectedFilePatterns string `json:"unprotected_file_patterns"`
@ -75,6 +76,7 @@ type CreateBranchProtectionOption struct {
BlockOnOfficialReviewRequests bool `json:"block_on_official_review_requests"`
BlockOnOutdatedBranch bool `json:"block_on_outdated_branch"`
DismissStaleApprovals bool `json:"dismiss_stale_approvals"`
IgnoreStaleApprovals bool `json:"ignore_stale_approvals"`
RequireSignedCommits bool `json:"require_signed_commits"`
ProtectedFilePatterns string `json:"protected_file_patterns"`
UnprotectedFilePatterns string `json:"unprotected_file_patterns"`
@ -100,6 +102,7 @@ type EditBranchProtectionOption struct {
BlockOnOfficialReviewRequests *bool `json:"block_on_official_review_requests"`
BlockOnOutdatedBranch *bool `json:"block_on_outdated_branch"`
DismissStaleApprovals *bool `json:"dismiss_stale_approvals"`
IgnoreStaleApprovals *bool `json:"ignore_stale_approvals"`
RequireSignedCommits *bool `json:"require_signed_commits"`
ProtectedFilePatterns *string `json:"protected_file_patterns"`
UnprotectedFilePatterns *string `json:"unprotected_file_patterns"`

2
options/locale/locale_en-US.ini
View file

@ -2315,6 +2315,8 @@ settings.protect_approvals_whitelist_users = Whitelisted reviewers:
settings.protect_approvals_whitelist_teams = Whitelisted teams for reviews:
settings.dismiss_stale_approvals = Dismiss stale approvals
settings.dismiss_stale_approvals_desc = When new commits that change the content of the pull request are pushed to the branch, old approvals will be dismissed.
settings.ignore_stale_approvals = Ignore stale approvals
settings.ignore_stale_approvals_desc = Do not count approvals that were made on older commits (stale reviews) towards how many approvals the PR has. Irrelevant if stale reviews are already dismissed.
settings.require_signed_commits = Require Signed Commits
settings.require_signed_commits_desc = Reject pushes to this branch if they are unsigned or unverifiable.
settings.protect_branch_name_pattern = Protected Branch Name Pattern

5
routers/api/v1/repo/branch.go
View file

@ -615,6 +615,7 @@ func CreateBranchProtection(ctx *context.APIContext) {
BlockOnRejectedReviews: form.BlockOnRejectedReviews,
BlockOnOfficialReviewRequests: form.BlockOnOfficialReviewRequests,
DismissStaleApprovals: form.DismissStaleApprovals,
IgnoreStaleApprovals: form.IgnoreStaleApprovals,
RequireSignedCommits: form.RequireSignedCommits,
ProtectedFilePatterns: form.ProtectedFilePatterns,
UnprotectedFilePatterns: form.UnprotectedFilePatterns,
@ -786,6 +787,10 @@ func EditBranchProtection(ctx *context.APIContext) {
protectBranch.DismissStaleApprovals = *form.DismissStaleApprovals
}
if form.IgnoreStaleApprovals != nil {
protectBranch.IgnoreStaleApprovals = *form.IgnoreStaleApprovals
}
if form.RequireSignedCommits != nil {
protectBranch.RequireSignedCommits = *form.RequireSignedCommits
}

1
routers/web/repo/setting/protected_branch.go
View file

@ -228,6 +228,7 @@ func SettingsProtectedBranchPost(ctx *context.Context) {
protectBranch.BlockOnRejectedReviews = f.BlockOnRejectedReviews
protectBranch.BlockOnOfficialReviewRequests = f.BlockOnOfficialReviewRequests
protectBranch.DismissStaleApprovals = f.DismissStaleApprovals
protectBranch.IgnoreStaleApprovals = f.IgnoreStaleApprovals
protectBranch.RequireSignedCommits = f.RequireSignedCommits
protectBranch.ProtectedFilePatterns = f.ProtectedFilePatterns
protectBranch.UnprotectedFilePatterns = f.UnprotectedFilePatterns

1
services/convert/convert.go
View file

@ -158,6 +158,7 @@ func ToBranchProtection(ctx context.Context, bp *git_model.ProtectedBranch) *api
BlockOnOfficialReviewRequests: bp.BlockOnOfficialReviewRequests,
BlockOnOutdatedBranch: bp.BlockOnOutdatedBranch,
DismissStaleApprovals: bp.DismissStaleApprovals,
IgnoreStaleApprovals: bp.IgnoreStaleApprovals,
RequireSignedCommits: bp.RequireSignedCommits,
ProtectedFilePatterns: bp.ProtectedFilePatterns,
UnprotectedFilePatterns: bp.UnprotectedFilePatterns,

1
services/forms/repo_form.go
View file

@ -212,6 +212,7 @@ type ProtectBranchForm struct {
BlockOnOfficialReviewRequests bool
BlockOnOutdatedBranch bool
DismissStaleApprovals bool
IgnoreStaleApprovals bool
RequireSignedCommits bool
ProtectedFilePatterns string
UnprotectedFilePatterns string

9
templates/repo/settings/protected_branch.tmpl
View file

@ -144,11 +144,18 @@
</div>
<div class="field">
<div class="ui checkbox">
<input name="dismiss_stale_approvals" type="checkbox" {{if .Rule.DismissStaleApprovals}}checked{{end}}>
<input id="dismiss_stale_approvals" name="dismiss_stale_approvals" type="checkbox" {{if .Rule.DismissStaleApprovals}}checked{{end}}>
<label>{{ctx.Locale.Tr "repo.settings.dismiss_stale_approvals"}}</label>
<p class="help">{{ctx.Locale.Tr "repo.settings.dismiss_stale_approvals_desc"}}</p>
</div>
</div>
<div id="ignore_stale_approvals_box" class="field {{if .Rule.DismissStaleApprovals}}disabled{{end}}">
<div class="ui checkbox">
<input id="ignore_stale_approvals" name="ignore_stale_approvals" type="checkbox" {{if .Rule.IgnoreStaleApprovals}}checked{{end}}>
<label>{{ctx.Locale.Tr "repo.settings.ignore_stale_approvals"}}</label>
<p class="help">{{ctx.Locale.Tr "repo.settings.ignore_stale_approvals_desc"}}</p>
</div>
</div>
<div class="grouped fields">
<div class="field">
<div class="ui checkbox">

12
templates/swagger/v1_json.tmpl generated
View file

@ -17020,6 +17020,10 @@
"type": "boolean",
"x-go-name": "EnableStatusCheck"
},
"ignore_stale_approvals": {
"type": "boolean",
"x-go-name": "IgnoreStaleApprovals"
},
"merge_whitelist_teams": {
"type": "array",
"items": {
@ -17661,6 +17665,10 @@
"type": "boolean",
"x-go-name": "EnableStatusCheck"
},
"ignore_stale_approvals": {
"type": "boolean",
"x-go-name": "IgnoreStaleApprovals"
},
"merge_whitelist_teams": {
"type": "array",
"items": {
@ -18792,6 +18800,10 @@
"type": "boolean",
"x-go-name": "EnableStatusCheck"
},
"ignore_stale_approvals": {
"type": "boolean",
"x-go-name": "IgnoreStaleApprovals"
},
"merge_whitelist_teams": {
"type": "array",
"items": {

4
web_src/js/features/repo-settings.js
View file

@ -82,6 +82,10 @@ export function initRepoSettingBranches() {
const $target = $($(this).attr('data-target'));
if (this.checked) $target.addClass('disabled'); // only disable, do not auto enable
});
$('#dismiss_stale_approvals').on('change', function () {
const $target = $('#ignore_stale_approvals_box');
$target.toggleClass('disabled', this.checked);
});
// show the `Matched` mark for the status checks that match the pattern
const markMatchedStatusChecks = () => {