Skip to content

Get rules by id when editing branch protection rule #22932

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.

Sign up for GitHub

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

Merged
merged 15 commits into from
Feb 20, 2023
Merged

Get rules by id when editing branch protection rule #22932

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
ad020df
get protected branch rule by id instead of name
Zettat123 Feb 16, 2023
4097e3e
add the rule_id field
Zettat123 Feb 16, 2023
8d1f1c9
fix err declaration
Zettat123 Feb 16, 2023
fc0c63c
get rule by id after getting rule by name failed
Zettat123 Feb 16, 2023
96e3dd5
fix error msg
Zettat123 Feb 16, 2023
c8fcf2f
update comment
Zettat123 Feb 16, 2023
59f2fb1
update comment
Zettat123 Feb 16, 2023
f03b608
get rule by id first
Zettat123 Feb 17, 2023
2dad832
check rule name
Zettat123 Feb 20, 2023
9ef32f7
Merge branch 'main' into bugfix/edit-branch-protection
Zettat123 Feb 20, 2023
4b9823e
add error message
Zettat123 Feb 20, 2023
1ec3b6b
update fixme message
Zettat123 Feb 20, 2023
cd5688f
remove unnecessary code
Zettat123 Feb 20, 2023
9be8b2d
remove empty line
Zettat123 Feb 20, 2023
5ecda72
Merge branch 'main' into bugfix/edit-branch-protection
lunny Feb 20, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions modules/structs/repo_branch.go
View file
Open in desktop
Original file line number Diff line number Diff line change
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
13 changes: 13 additions & 0 deletions routers/web/repo/setting_protected_branch.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -165,12 +165,25 @@ func SettingsProtectedBranchPost(ctx *context.Context) {
return
}

// FIXME: If a new ProtectBranch has a duplicate RuleName, an error should be returned.

// Currently, if a new ProtectBranch with the same name is created,
// the existing ProtectBranch will be updated.
// But we cannot modify this logic now because many unit tests rely on it.

Copy link
Member

@wolfogre wolfogre Feb 17, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it should be:

  • When creating a new ProtectBranch with the same name, the RuleID should be 0, so it's fine to update the existing one. (Update an existing ProtectBranch by name)
  • If the RuleID isn't 0 and the RuleName is duplicate, it should return an error. (Update the name of an existing ProtectBranch by id)

var err error
protectBranch, err = git_model.GetProtectedBranchRuleByName(ctx, ctx.Repo.Repository.ID, f.RuleName)
if err != nil {
ctx.ServerError("GetProtectBranchOfRepoByName", err)
return
}
if protectBranch == nil && f.RuleID > 0 {
protectBranch, err = git_model.GetProtectedBranchRuleByID(ctx, ctx.Repo.Repository.ID, f.RuleID)
if err != nil {
ctx.ServerError("GetProtectBranchOfRepoByID", err)
return
}
}
if protectBranch == nil {
// No options found, create defaults.
protectBranch = &git_model.ProtectedBranch{
Expand Down
1 change: 1 addition & 0 deletions services/forms/repo_form.go
View file
Open in desktop
Original file line number Diff line number Diff line change
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
View file
Open in desktop
Original file line number Diff line number Diff line change
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