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>
This commit is contained in:
parent
3596df52c0
commit
9a83aa28a3
|
@ -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
|
||||
|
|
|
@ -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.
|
||||
|
|
|
@ -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
|
||||
|
|
Loading…
Reference in New Issue