diff options
author | Zettat123 | 2023-02-20 19:30:41 +0800 |
---|---|---|
committer | GitHub | 2023-02-20 19:30:41 +0800 |
commit | 9a83aa28a351c9af1731a94ce0e1b5d3842db4e8 (patch) | |
tree | 009e05aba7fb05586abf1ce4283373e3965807f6 /routers | |
parent | 3596df52c09831f7f39f8416264ff267954f35a0 (diff) |
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.
![image](https://user-images.githubusercontent.com/15528715/219276442-d3c001ad-e693-44ec-9ad2-b33f2666b49b.png)
---
![image](https://user-images.githubusercontent.com/15528715/219276478-547c3b93-b3f1-4292-a1ef-c1b7747fe1bb.png)
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>
Diffstat (limited to 'routers')
-rw-r--r-- | routers/web/repo/setting_protected_branch.go | 34 |
1 files changed, 30 insertions, 4 deletions
diff --git a/routers/web/repo/setting_protected_branch.go b/routers/web/repo/setting_protected_branch.go index a54565c1f..0a8c39fef 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. |