Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Get rules by id when editing branch protection rule #22932

Merged
merged 15 commits into from Feb 20, 2023
1 change: 1 addition & 0 deletions modules/structs/repo_branch.go
Expand Up @@ -25,6 +25,7 @@ type BranchProtection struct {
// Deprecated: true
BranchName string `json:"branch_name"`
RuleName string `json:"rule_name"`
RuleID string `json:"rule_id"`
EnablePush bool `json:"enable_push"`
EnablePushWhitelist bool `json:"enable_push_whitelist"`
PushWhitelistUsernames []string `json:"push_whitelist_usernames"`
Expand Down
1 change: 1 addition & 0 deletions options/locale/locale_en-US.ini
Expand Up @@ -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
Expand Down
35 changes: 31 additions & 4 deletions routers/web/repo/setting_protected_branch.go
Expand Up @@ -166,10 +166,37 @@ 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 {
Zettat123 marked this conversation as resolved.
Show resolved Hide resolved
// 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.
Expand Down
1 change: 1 addition & 0 deletions services/forms/repo_form.go
Expand Up @@ -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
Expand Down
4 changes: 4 additions & 0 deletions templates/swagger/v1_json.tmpl
Expand Up @@ -14498,6 +14498,10 @@
"format": "int64",
"x-go-name": "RequiredApprovals"
},
"rule_id": {
"type": "string",
"x-go-name": "RuleID"
},
"rule_name": {
"type": "string",
"x-go-name": "RuleName"
Expand Down