From 9a83aa28a351c9af1731a94ce0e1b5d3842db4e8 Mon Sep 17 00:00:00 2001
From: Zettat123 <zettat123@gmail.com>
Date: Mon, 20 Feb 2023 19:30:41 +0800
Subject: [PATCH] Get rules by id when editing branch protection rule (#22932)
When users rename an existing branch protection rule, a new rule with
the new name will be created and the old rule will still exist.
---
The reason is that the `SettingsProtectedBranchPost` function only get
branch protection rule by name before updating or creating a rule. When
the rule name changes, the function cannot find the existing rule so it
will create a new rule rather than update the existing rule. To fix the
bug, the function should get rule by id first.
---------
Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com>
---
options/locale/locale_en-US.ini | 1 +
routers/web/repo/setting_protected_branch.go | 34 +++++++++++++++++---
services/forms/repo_form.go | 1 +
3 files changed, 32 insertions(+), 4 deletions(-)
diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini
index 411a585c81..92ca5be8d3 100644
--- a/options/locale/locale_en-US.ini
+++ b/options/locale/locale_en-US.ini
@@ -2152,6 +2152,7 @@ settings.choose_branch = Choose a branch…
settings.no_protected_branch = There are no protected branches.
settings.edit_protected_branch = Edit
settings.protected_branch_required_rule_name = Required rule name
+settings.protected_branch_duplicate_rule_name = Duplicate rule name
settings.protected_branch_required_approvals_min = Required approvals cannot be negative.
settings.tags = Tags
settings.tags.protection = Tag Protection
diff --git a/routers/web/repo/setting_protected_branch.go b/routers/web/repo/setting_protected_branch.go
index a54565c1f1..0a8c39fef0 100644
--- a/routers/web/repo/setting_protected_branch.go
+++ b/routers/web/repo/setting_protected_branch.go
@@ -166,10 +166,36 @@ func SettingsProtectedBranchPost(ctx *context.Context) {
}
var err error
- protectBranch, err = git_model.GetProtectedBranchRuleByName(ctx, ctx.Repo.Repository.ID, f.RuleName)
- if err != nil {
- ctx.ServerError("GetProtectBranchOfRepoByName", err)
- return
+ if f.RuleID > 0 {
+ // If the RuleID isn't 0, it must be an edit operation. So we get rule by id.
+ protectBranch, err = git_model.GetProtectedBranchRuleByID(ctx, ctx.Repo.Repository.ID, f.RuleID)
+ if err != nil {
+ ctx.ServerError("GetProtectBranchOfRepoByID", err)
+ return
+ }
+ if protectBranch != nil && protectBranch.RuleName != f.RuleName {
+ // RuleName changed. We need to check if there is a rule with the same name.
+ // If a rule with the same name exists, an error should be returned.
+ sameNameProtectBranch, err := git_model.GetProtectedBranchRuleByName(ctx, ctx.Repo.Repository.ID, f.RuleName)
+ if err != nil {
+ ctx.ServerError("GetProtectBranchOfRepoByName", err)
+ return
+ }
+ if sameNameProtectBranch != nil {
+ ctx.Flash.Error(ctx.Tr("repo.settings.protected_branch_duplicate_rule_name"))
+ ctx.Redirect(fmt.Sprintf("%s/settings/branches/edit?rule_name=%s", ctx.Repo.RepoLink, protectBranch.RuleName))
+ return
+ }
+ }
+ } else {
+ // FIXME: If a new ProtectBranch has a duplicate RuleName, an error should be returned.
+ // Currently, if a new ProtectBranch with a duplicate RuleName is created, the existing ProtectBranch will be updated.
+ // But we cannot modify this logic now because many unit tests rely on it.
+ protectBranch, err = git_model.GetProtectedBranchRuleByName(ctx, ctx.Repo.Repository.ID, f.RuleName)
+ if err != nil {
+ ctx.ServerError("GetProtectBranchOfRepoByName", err)
+ return
+ }
}
if protectBranch == nil {
// No options found, create defaults.
diff --git a/services/forms/repo_form.go b/services/forms/repo_form.go
index ff0916f8e1..374ae0ac5c 100644
--- a/services/forms/repo_form.go
+++ b/services/forms/repo_form.go
@@ -190,6 +190,7 @@ func (f *RepoSettingForm) Validate(req *http.Request, errs binding.Errors) bindi
// ProtectBranchForm form for changing protected branch settings
type ProtectBranchForm struct {
RuleName string `binding:"Required"`
+ RuleID int64
EnablePush string
WhitelistUsers string
WhitelistTeams string