aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorJimmy Praet <jimmy.praet@telenet.be>2024-01-15 08:20:01 +0100
committerGitHub <noreply@github.com>2024-01-15 07:20:01 +0000
commit5d3fdd121279c758f247a76e020799aa5e548feb (patch)
treee27b631cfdfd35d673da1130c9639633a1e3bc10
parentce0225c1b87d682f53b87496c8dd6ccee0396f0b (diff)
downloadgitea-5d3fdd121279c758f247a76e020799aa5e548feb.tar.gz
gitea-5d3fdd121279c758f247a76e020799aa5e548feb.zip
Add branch protection setting for ignoring stale approvals (#28498)
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>
-rw-r--r--models/git/protected_branch.go1
-rw-r--r--models/issues/pull.go2
-rw-r--r--models/migrations/v1_22/v284.go14
-rw-r--r--modules/structs/repo_branch.go3
-rw-r--r--options/locale/locale_en-US.ini2
-rw-r--r--routers/api/v1/repo/branch.go5
-rw-r--r--routers/web/repo/setting/protected_branch.go1
-rw-r--r--services/convert/convert.go1
-rw-r--r--services/forms/repo_form.go1
-rw-r--r--templates/repo/settings/protected_branch.tmpl9
-rw-r--r--templates/swagger/v1_json.tmpl12
-rw-r--r--web_src/js/features/repo-settings.js4
12 files changed, 53 insertions, 2 deletions
diff --git a/models/git/protected_branch.go b/models/git/protected_branch.go
index 66a4b52b17..e0ff4d1542 100644
--- a/models/git/protected_branch.go
+++ b/models/git/protected_branch.go
@@ -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"`
diff --git a/models/issues/pull.go b/models/issues/pull.go
index 34bea921a0..614ee9a87c 100644
--- a/models/issues/pull.go
+++ b/models/issues/pull.go
@@ -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))
diff --git a/models/migrations/v1_22/v284.go b/models/migrations/v1_22/v284.go
new file mode 100644
index 0000000000..1a4c786964
--- /dev/null
+++ b/models/migrations/v1_22/v284.go
@@ -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))
+}
diff --git a/modules/structs/repo_branch.go b/modules/structs/repo_branch.go
index e9927aec27..e96d276b29 100644
--- a/modules/structs/repo_branch.go
+++ b/modules/structs/repo_branch.go
@@ -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"`
diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini
index 95148bb644..90e3ac503a 100644
--- a/options/locale/locale_en-US.ini
+++ b/options/locale/locale_en-US.ini
@@ -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
diff --git a/routers/api/v1/repo/branch.go b/routers/api/v1/repo/branch.go
index 36c85c8a57..edbfbcc568 100644
--- a/routers/api/v1/repo/branch.go
+++ b/routers/api/v1/repo/branch.go
@@ -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
}
diff --git a/routers/web/repo/setting/protected_branch.go b/routers/web/repo/setting/protected_branch.go
index 73adfec95a..98d6977b81 100644
--- a/routers/web/repo/setting/protected_branch.go
+++ b/routers/web/repo/setting/protected_branch.go
@@ -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
diff --git a/services/convert/convert.go b/services/convert/convert.go
index 860c3ef03a..ca3ec32a40 100644
--- a/services/convert/convert.go
+++ b/services/convert/convert.go
@@ -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,
diff --git a/services/forms/repo_form.go b/services/forms/repo_form.go
index 86599000a5..780fc88000 100644
--- a/services/forms/repo_form.go
+++ b/services/forms/repo_form.go
@@ -212,6 +212,7 @@ type ProtectBranchForm struct {
BlockOnOfficialReviewRequests bool
BlockOnOutdatedBranch bool
DismissStaleApprovals bool
+ IgnoreStaleApprovals bool
RequireSignedCommits bool
ProtectedFilePatterns string
UnprotectedFilePatterns string
diff --git a/templates/repo/settings/protected_branch.tmpl b/templates/repo/settings/protected_branch.tmpl
index a1e45d805c..9c0fbddf06 100644
--- a/templates/repo/settings/protected_branch.tmpl
+++ b/templates/repo/settings/protected_branch.tmpl
@@ -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">
diff --git a/templates/swagger/v1_json.tmpl b/templates/swagger/v1_json.tmpl
index c5d939fee5..094a0e9aec 100644
--- a/templates/swagger/v1_json.tmpl
+++ b/templates/swagger/v1_json.tmpl
@@ -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": {
diff --git a/web_src/js/features/repo-settings.js b/web_src/js/features/repo-settings.js
index 74cc308354..04974200bb 100644
--- a/web_src/js/features/repo-settings.js
+++ b/web_src/js/features/repo-settings.js
@@ -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 = () => {