Skip to content

Commit 846f618

Browse files
65436543
authored
Add priority to protected branch (#32286)
## Solves Currently for rules to re-order them you have to alter the creation date. so you basicly have to delete and recreate them in the right order. This is more than just inconvinient ... ## Solution Add a new col for prioritization ## Demo WebUI Video https://github.com/user-attachments/assets/92182a31-9705-4ac5-b6e3-9bb74108cbd1 --- *Sponsored by Kithara Software GmbH*
1 parent 3fc1bbe commit 846f618

22 files changed

+454
-13
lines changed

‎models/git/protected_branch.go

Lines changed: 33 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ type ProtectedBranch struct {
3434
RepoID int64 `xorm:"UNIQUE(s)"`
3535
Repo *repo_model.Repository `xorm:"-"`
3636
RuleName string `xorm:"'branch_name' UNIQUE(s)"` // a branch name or a glob match to branch name
37+
Priority int64 `xorm:"NOT NULL DEFAULT 0"`
3738
globRule glob.Glob `xorm:"-"`
3839
isPlainName bool `xorm:"-"`
3940
CanPush bool `xorm:"NOT NULL DEFAULT false"`
@@ -413,21 +414,52 @@ func UpdateProtectBranch(ctx context.Context, repo *repo_model.Repository, prote
413414
}
414415
protectBranch.ApprovalsWhitelistTeamIDs = whitelist
415416

416-
// Make sure protectBranch.ID is not 0 for whitelists
417+
// Looks like it's a new rule
417418
if protectBranch.ID == 0 {
419+
// as it's a new rule and if priority was not set, we need to calc it.
420+
if protectBranch.Priority == 0 {
421+
var lowestPrio int64
422+
// because of mssql we can not use builder or save xorm syntax, so raw sql it is
423+
if _, err := db.GetEngine(ctx).SQL(`SELECT MAX(priority) FROM protected_branch WHERE repo_id = ?`, protectBranch.RepoID).
424+
Get(&lowestPrio); err != nil {
425+
return err
426+
}
427+
log.Trace("Create new ProtectedBranch at repo[%d] and detect current lowest priority '%d'", protectBranch.RepoID, lowestPrio)
428+
protectBranch.Priority = lowestPrio + 1
429+
}
430+
418431
if _, err = db.GetEngine(ctx).Insert(protectBranch); err != nil {
419432
return fmt.Errorf("Insert: %v", err)
420433
}
421434
return nil
422435
}
423436

437+
// update the rule
424438
if _, err = db.GetEngine(ctx).ID(protectBranch.ID).AllCols().Update(protectBranch); err != nil {
425439
return fmt.Errorf("Update: %v", err)
426440
}
427441

428442
return nil
429443
}
430444

445+
func UpdateProtectBranchPriorities(ctx context.Context, repo *repo_model.Repository, ids []int64) error {
446+
prio := int64(1)
447+
return db.WithTx(ctx, func(ctx context.Context) error {
448+
for _, id := range ids {
449+
if _, err := db.GetEngine(ctx).
450+
ID(id).Where("repo_id = ?", repo.ID).
451+
Cols("priority").
452+
Update(&ProtectedBranch{
453+
Priority: prio,
454+
}); err != nil {
455+
return err
456+
}
457+
prio++
458+
}
459+
return nil
460+
})
461+
}
462+
431463
// updateApprovalWhitelist checks whether the user whitelist changed and returns a whitelist with
432464
// the users from newWhitelist which have explicit read or write access to the repo.
433465
func updateApprovalWhitelist(ctx context.Context, repo *repo_model.Repository, currentWhitelist, newWhitelist []int64) (whitelist []int64, err error) {

‎models/git/protected_branch_list.go

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,13 @@ func (rules ProtectedBranchRules) sort() {
2828
sort.Slice(rules, func(i, j int) bool {
2929
rules[i].loadGlob()
3030
rules[j].loadGlob()
31+
32+
// if priority differ, use that to sort
33+
if rules[i].Priority != rules[j].Priority {
34+
return rules[i].Priority < rules[j].Priority
35+
}
36+
37+
// now we sort the old way
3138
if rules[i].isPlainName != rules[j].isPlainName {
3239
return rules[i].isPlainName // plain name comes first, so plain name means "less"
3340
}

‎models/git/protected_branch_list_test.go

Lines changed: 35 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ func TestBranchRuleMatchPriority(t *testing.T) {
7575
}
7676
}
7777

78-
func TestBranchRuleSort(t *testing.T) {
78+
func TestBranchRuleSortLegacy(t *testing.T) {
7979
in := []*ProtectedBranch{{
8080
RuleName: "b",
8181
CreatedUnix: 1,
@@ -103,3 +103,37 @@ func TestBranchRuleSort(t *testing.T) {
103103
}
104104
assert.Equal(t, expect, got)
105105
}
106+
107+
func TestBranchRuleSortPriority(t *testing.T) {
108+
in := []*ProtectedBranch{{
109+
RuleName: "b",
110+
CreatedUnix: 1,
111+
Priority: 4,
112+
}, {
113+
RuleName: "b/*",
114+
CreatedUnix: 3,
115+
Priority: 2,
116+
}, {
117+
RuleName: "a/*",
118+
CreatedUnix: 2,
119+
Priority: 1,
120+
}, {
121+
RuleName: "c",
122+
CreatedUnix: 0,
123+
Priority: 0,
124+
}, {
125+
RuleName: "a",
126+
CreatedUnix: 4,
127+
Priority: 3,
128+
}}
129+
expect := []string{"c", "a/*", "b/*", "a", "b"}
130+
131+
pbr := ProtectedBranchRules(in)
132+
pbr.sort()
133+
134+
var got []string
135+
for i := range pbr {
136+
got = append(got, pbr[i].RuleName)
137+
}
138+
assert.Equal(t, expect, got)
139+
}

‎models/git/protected_branch_test.go

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,10 @@ import (
77
"fmt"
88
"testing"
99

10+
"code.gitea.io/gitea/models/db"
11+
repo_model "code.gitea.io/gitea/models/repo"
12+
"code.gitea.io/gitea/models/unittest"
13+
1014
"github.com/stretchr/testify/assert"
1115
)
1216

@@ -76,3 +80,77 @@ func TestBranchRuleMatch(t *testing.T) {
7680
)
7781
}
7882
}
83+
84+
func TestUpdateProtectBranchPriorities(t *testing.T) {
85+
assert.NoError(t, unittest.PrepareTestDatabase())
86+
87+
repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1})
88+
89+
// Create some test protected branches with initial priorities
90+
protectedBranches := []*ProtectedBranch{
91+
{
92+
RepoID: repo.ID,
93+
RuleName: "master",
94+
Priority: 1,
95+
},
96+
{
97+
RepoID: repo.ID,
98+
RuleName: "develop",
99+
Priority: 2,
100+
},
101+
{
102+
RepoID: repo.ID,
103+
RuleName: "feature/*",
104+
Priority: 3,
105+
},
106+
}
107+
108+
for _, pb := range protectedBranches {
109+
_, err := db.GetEngine(db.DefaultContext).Insert(pb)
110+
assert.NoError(t, err)
111+
}
112+
113+
// Test updating priorities
114+
newPriorities := []int64{protectedBranches[2].ID, protectedBranches[0].ID, protectedBranches[1].ID}
115+
err := UpdateProtectBranchPriorities(db.DefaultContext, repo, newPriorities)
116+
assert.NoError(t, err)
117+
118+
// Verify new priorities
119+
pbs, err := FindRepoProtectedBranchRules(db.DefaultContext, repo.ID)
120+
assert.NoError(t, err)
121+
122+
expectedPriorities := map[string]int64{
123+
"feature/*": 1,
124+
"master": 2,
125+
"develop": 3,
126+
}
127+
128+
for _, pb := range pbs {
129+
assert.Equal(t, expectedPriorities[pb.RuleName], pb.Priority)
130+
}
131+
}
132+
133+
func TestNewProtectBranchPriority(t *testing.T) {
134+
assert.NoError(t, unittest.PrepareTestDatabase())
135+
repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1})
136+
137+
err := UpdateProtectBranch(db.DefaultContext, repo, &ProtectedBranch{
138+
RepoID: repo.ID,
139+
RuleName: "branch-1",
140+
Priority: 1,
141+
}, WhitelistOptions{})
142+
assert.NoError(t, err)
143+
144+
newPB := &ProtectedBranch{
145+
RepoID: repo.ID,
146+
RuleName: "branch-2",
147+
// Priority intentionally omitted
148+
}
149+
150+
err = UpdateProtectBranch(db.DefaultContext, repo, newPB, WhitelistOptions{})
151+
assert.NoError(t, err)
152+
153+
savedPB2, err := GetFirstMatchProtectedBranchRule(db.DefaultContext, repo.ID, "branch-2")
154+
assert.NoError(t, err)
155+
assert.Equal(t, int64(2), savedPB2.Priority)
156+
}

‎models/migrations/migrations.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -367,6 +367,7 @@ func prepareMigrationTasks() []*migration {
367367
newMigration(307, "Fix milestone deadline_unix when there is no due date", v1_23.FixMilestoneNoDueDate),
368368
newMigration(308, "Add index(user_id, is_deleted) for action table", v1_23.AddNewIndexForUserDashboard),
369369
newMigration(309, "Improve Notification table indices", v1_23.ImproveNotificationTableIndices),
370+
newMigration(310, "Add Priority to ProtectedBranch", v1_23.AddPriorityToProtectedBranch),
370371
}
371372
return preparedMigrations
372373
}

‎models/migrations/v1_23/v310.go

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
// Copyright 2024 The Gitea Authors. All rights reserved.
2+
// SPDX-License-Identifier: MIT
3+
4+
package v1_23 //nolint
5+
6+
import (
7+
"xorm.io/xorm"
8+
)
9+
10+
func AddPriorityToProtectedBranch(x *xorm.Engine) error {
11+
type ProtectedBranch struct {
12+
Priority int64 `xorm:"NOT NULL DEFAULT 0"`
13+
}
14+
15+
return x.Sync(new(ProtectedBranch))
16+
}

‎modules/structs/repo_branch.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ type BranchProtection struct {
2525
// Deprecated: true
2626
BranchName string `json:"branch_name"`
2727
RuleName string `json:"rule_name"`
28+
Priority int64 `json:"priority"`
2829
EnablePush bool `json:"enable_push"`
2930
EnablePushWhitelist bool `json:"enable_push_whitelist"`
3031
PushWhitelistUsernames []string `json:"push_whitelist_usernames"`
@@ -64,6 +65,7 @@ type CreateBranchProtectionOption struct {
6465
// Deprecated: true
6566
BranchName string `json:"branch_name"`
6667
RuleName string `json:"rule_name"`
68+
Priority int64 `json:"priority"`
6769
EnablePush bool `json:"enable_push"`
6870
EnablePushWhitelist bool `json:"enable_push_whitelist"`
6971
PushWhitelistUsernames []string `json:"push_whitelist_usernames"`
@@ -96,6 +98,7 @@ type CreateBranchProtectionOption struct {
9698

9799
// EditBranchProtectionOption options for editing a branch protection
98100
type EditBranchProtectionOption struct {
101+
Priority *int64 `json:"priority"`
99102
EnablePush *bool `json:"enable_push"`
100103
EnablePushWhitelist *bool `json:"enable_push_whitelist"`
101104
PushWhitelistUsernames []string `json:"push_whitelist_usernames"`
@@ -125,3 +128,8 @@ type EditBranchProtectionOption struct {
125128
UnprotectedFilePatterns *string `json:"unprotected_file_patterns"`
126129
BlockAdminMergeOverride *bool `json:"block_admin_merge_override"`
127130
}
131+
132+
// UpdateBranchProtectionPriories a list to update the branch protection rule priorities
133+
type UpdateBranchProtectionPriories struct {
134+
IDs []int64 `json:"ids"`
135+
}

‎routers/api/v1/api.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1204,6 +1204,7 @@ func Routes() *web.Router {
12041204
m.Patch("", bind(api.EditBranchProtectionOption{}), mustNotBeArchived, repo.EditBranchProtection)
12051205
m.Delete("", repo.DeleteBranchProtection)
12061206
})
1207+
m.Post("/priority", bind(api.UpdateBranchProtectionPriories{}), mustNotBeArchived, repo.UpdateBranchProtectionPriories)
12071208
}, reqToken(), reqAdmin())
12081209
m.Group("/tags", func() {
12091210
m.Get("", repo.ListTags)

‎routers/api/v1/repo/branch.go

Lines changed: 52 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -618,6 +618,7 @@ func CreateBranchProtection(ctx *context.APIContext) {
618618
protectBranch = &git_model.ProtectedBranch{
619619
RepoID: ctx.Repo.Repository.ID,
620620
RuleName: ruleName,
621+
Priority: form.Priority,
621622
CanPush: form.EnablePush,
622623
EnableWhitelist: form.EnablePush && form.EnablePushWhitelist,
623624
WhitelistDeployKeys: form.EnablePush && form.EnablePushWhitelist && form.PushWhitelistDeployKeys,
@@ -640,7 +641,7 @@ func CreateBranchProtection(ctx *context.APIContext) {
640641
BlockAdminMergeOverride: form.BlockAdminMergeOverride,
641642
}
642643

643-
err = git_model.UpdateProtectBranch(ctx, ctx.Repo.Repository, protectBranch, git_model.WhitelistOptions{
644+
if err := git_model.UpdateProtectBranch(ctx, ctx.Repo.Repository, protectBranch, git_model.WhitelistOptions{
644645
UserIDs: whitelistUsers,
645646
TeamIDs: whitelistTeams,
646647
ForcePushUserIDs: forcePushAllowlistUsers,
@@ -649,14 +650,13 @@ func CreateBranchProtection(ctx *context.APIContext) {
649650
MergeTeamIDs: mergeWhitelistTeams,
650651
ApprovalsUserIDs: approvalsWhitelistUsers,
651652
ApprovalsTeamIDs: approvalsWhitelistTeams,
652-
})
653-
if err != nil {
653+
}); err != nil {
654654
ctx.Error(http.StatusInternalServerError, "UpdateProtectBranch", err)
655655
return
656656
}
657657

658658
if isBranchExist {
659-
if err = pull_service.CheckPRsForBaseBranch(ctx, ctx.Repo.Repository, ruleName); err != nil {
659+
if err := pull_service.CheckPRsForBaseBranch(ctx, ctx.Repo.Repository, ruleName); err != nil {
660660
ctx.Error(http.StatusInternalServerError, "CheckPRsForBaseBranch", err)
661661
return
662662
}
@@ -796,6 +796,10 @@ func EditBranchProtection(ctx *context.APIContext) {
796796
}
797797
}
798798

799+
if form.Priority != nil {
800+
protectBranch.Priority = *form.Priority
801+
}
802+
799803
if form.EnableMergeWhitelist != nil {
800804
protectBranch.EnableMergeWhitelist = *form.EnableMergeWhitelist
801805
}
@@ -1080,3 +1084,47 @@ func DeleteBranchProtection(ctx *context.APIContext) {
10801084

10811085
ctx.Status(http.StatusNoContent)
10821086
}
1087+
1088+
// UpdateBranchProtectionPriories updates the priorities of branch protections for a repo
1089+
func UpdateBranchProtectionPriories(ctx *context.APIContext) {
1090+
// swagger:operation POST /repos/{owner}/{repo}/branch_protections/priority repository repoUpdateBranchProtectionPriories
1091+
// ---
1092+
// summary: Update the priorities of branch protections for a repository.
1093+
// consumes:
1094+
// - application/json
1095+
// produces:
1096+
// - application/json
1097+
// parameters:
1098+
// - name: owner
1099+
// in: path
1100+
// description: owner of the repo
1101+
// type: string
1102+
// required: true
1103+
// - name: repo
1104+
// in: path
1105+
// description: name of the repo
1106+
// type: string
1107+
// required: true
1108+
// - name: body
1109+
// in: body
1110+
// schema:
1111+
// "$ref": "#/definitions/UpdateBranchProtectionPriories"
1112+
// responses:
1113+
// "204":
1114+
// "$ref": "#/responses/empty"
1115+
// "404":
1116+
// "$ref": "#/responses/notFound"
1117+
// "422":
1118+
// "$ref": "#/responses/validationError"
1119+
// "423":
1120+
// "$ref": "#/responses/repoArchivedError"
1121+
form := web.GetForm(ctx).(*api.UpdateBranchProtectionPriories)
1122+
repo := ctx.Repo.Repository
1123+
1124+
if err := git_model.UpdateProtectBranchPriorities(ctx, repo, form.IDs); err != nil {
1125+
ctx.Error(http.StatusInternalServerError, "UpdateProtectBranchPriorities", err)
1126+
return
1127+
}
1128+
1129+
ctx.Status(http.StatusNoContent)
1130+
}

‎routers/api/v1/swagger/options.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -146,6 +146,9 @@ type swaggerParameterBodies struct {
146146
// in:body
147147
EditBranchProtectionOption api.EditBranchProtectionOption
148148

149+
// in:body
150+
UpdateBranchProtectionPriories api.UpdateBranchProtectionPriories
151+
149152
// in:body
150153
CreateOAuth2ApplicationOptions api.CreateOAuth2ApplicationOptions
151154

‎routers/web/repo/setting/protected_branch.go

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -322,6 +322,16 @@ func DeleteProtectedBranchRulePost(ctx *context.Context) {
322322
ctx.JSONRedirect(fmt.Sprintf("%s/settings/branches", ctx.Repo.RepoLink))
323323
}
324324

325+
func UpdateBranchProtectionPriories(ctx *context.Context) {
326+
form := web.GetForm(ctx).(*forms.ProtectBranchPriorityForm)
327+
repo := ctx.Repo.Repository
328+
329+
if err := git_model.UpdateProtectBranchPriorities(ctx, repo, form.IDs); err != nil {
330+
ctx.ServerError("UpdateProtectBranchPriorities", err)
331+
return
332+
}
333+
}
334+
325335
// RenameBranchPost responses for rename a branch
326336
func RenameBranchPost(ctx *context.Context) {
327337
form := web.GetForm(ctx).(*forms.RenameBranchForm)

0 commit comments

Comments
 (0)