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*

6 files changed, 170 insertions, 2 deletions

diff --git a/models/git/protected_branch.go b/models/git/protected_branch.go
index 37d933a982..a3caed73c4 100644
--- a/models/git/protected_branch.go
+++ b/models/git/protected_branch.go
@@ -34,6 +34,7 @@ type ProtectedBranch struct {
 	RepoID                        int64                  `xorm:"UNIQUE(s)"`
 	Repo                          *repo_model.Repository `xorm:"-"`
 	RuleName                      string                 `xorm:"'branch_name' UNIQUE(s)"` // a branch name or a glob match to branch name
+	Priority                      int64                  `xorm:"NOT NULL DEFAULT 0"`
 	globRule                      glob.Glob              `xorm:"-"`
 	isPlainName                   bool                   `xorm:"-"`
 	CanPush                       bool                   `xorm:"NOT NULL DEFAULT false"`
@@ -413,14 +414,27 @@ func UpdateProtectBranch(ctx context.Context, repo *repo_model.Repository, prote
 	}
 	protectBranch.ApprovalsWhitelistTeamIDs = whitelist
 
-	// Make sure protectBranch.ID is not 0 for whitelists
+	// Looks like it's a new rule
 	if protectBranch.ID == 0 {
+		// as it's a new rule and if priority was not set, we need to calc it.
+		if protectBranch.Priority == 0 {
+			var lowestPrio int64
+			// because of mssql we can not use builder or save xorm syntax, so raw sql it is
+			if _, err := db.GetEngine(ctx).SQL(`SELECT MAX(priority) FROM protected_branch WHERE repo_id = ?`, protectBranch.RepoID).
+				Get(&lowestPrio); err != nil {
+				return err
+			}
+			log.Trace("Create new ProtectedBranch at repo[%d] and detect current lowest priority '%d'", protectBranch.RepoID, lowestPrio)
+			protectBranch.Priority = lowestPrio + 1
+		}
+
 		if _, err = db.GetEngine(ctx).Insert(protectBranch); err != nil {
 			return fmt.Errorf("Insert: %v", err)
 		}
 		return nil
 	}
 
+	// update the rule
 	if _, err = db.GetEngine(ctx).ID(protectBranch.ID).AllCols().Update(protectBranch); err != nil {
 		return fmt.Errorf("Update: %v", err)
 	}
@@ -428,6 +442,24 @@ func UpdateProtectBranch(ctx context.Context, repo *repo_model.Repository, prote
 	return nil
 }
 
+func UpdateProtectBranchPriorities(ctx context.Context, repo *repo_model.Repository, ids []int64) error {
+	prio := int64(1)
+	return db.WithTx(ctx, func(ctx context.Context) error {
+		for _, id := range ids {
+			if _, err := db.GetEngine(ctx).
+				ID(id).Where("repo_id = ?", repo.ID).
+				Cols("priority").
+				Update(&ProtectedBranch{
+					Priority: prio,
+				}); err != nil {
+				return err
+			}
+			prio++
+		}
+		return nil
+	})
+}
+
 // updateApprovalWhitelist checks whether the user whitelist changed and returns a whitelist with
 // the users from newWhitelist which have explicit read or write access to the repo.
 func updateApprovalWhitelist(ctx context.Context, repo *repo_model.Repository, currentWhitelist, newWhitelist []int64) (whitelist []int64, err error) {
diff --git a/models/git/protected_branch_list.go b/models/git/protected_branch_list.go
index 613333a5a2..16f8500672 100644
--- a/models/git/protected_branch_list.go
+++ b/models/git/protected_branch_list.go
@@ -28,6 +28,13 @@ func (rules ProtectedBranchRules) sort() {
 	sort.Slice(rules, func(i, j int) bool {
 		rules[i].loadGlob()
 		rules[j].loadGlob()
+
+		// if priority differ, use that to sort
+		if rules[i].Priority != rules[j].Priority {
+			return rules[i].Priority < rules[j].Priority
+		}
+
+		// now we sort the old way
 		if rules[i].isPlainName != rules[j].isPlainName {
 			return rules[i].isPlainName // plain name comes first, so plain name means "less"
 		}
diff --git a/models/git/protected_branch_list_test.go b/models/git/protected_branch_list_test.go
index 94a48f37e6..a46402c543 100644
--- a/models/git/protected_branch_list_test.go
+++ b/models/git/protected_branch_list_test.go
@@ -75,7 +75,7 @@ func TestBranchRuleMatchPriority(t *testing.T) {
 	}
 }
 
-func TestBranchRuleSort(t *testing.T) {
+func TestBranchRuleSortLegacy(t *testing.T) {
 	in := []*ProtectedBranch{{
 		RuleName:    "b",
 		CreatedUnix: 1,
@@ -103,3 +103,37 @@ func TestBranchRuleSort(t *testing.T) {
 	}
 	assert.Equal(t, expect, got)
 }
+
+func TestBranchRuleSortPriority(t *testing.T) {
+	in := []*ProtectedBranch{{
+		RuleName:    "b",
+		CreatedUnix: 1,
+		Priority:    4,
+	}, {
+		RuleName:    "b/*",
+		CreatedUnix: 3,
+		Priority:    2,
+	}, {
+		RuleName:    "a/*",
+		CreatedUnix: 2,
+		Priority:    1,
+	}, {
+		RuleName:    "c",
+		CreatedUnix: 0,
+		Priority:    0,
+	}, {
+		RuleName:    "a",
+		CreatedUnix: 4,
+		Priority:    3,
+	}}
+	expect := []string{"c", "a/*", "b/*", "a", "b"}
+
+	pbr := ProtectedBranchRules(in)
+	pbr.sort()
+
+	var got []string
+	for i := range pbr {
+		got = append(got, pbr[i].RuleName)
+	}
+	assert.Equal(t, expect, got)
+}
diff --git a/models/git/protected_branch_test.go b/models/git/protected_branch_test.go
index 1962859a8c..49d433f845 100644
--- a/models/git/protected_branch_test.go
+++ b/models/git/protected_branch_test.go
@@ -7,6 +7,10 @@ import (
 	"fmt"
 	"testing"
 
+	"code.gitea.io/gitea/models/db"
+	repo_model "code.gitea.io/gitea/models/repo"
+	"code.gitea.io/gitea/models/unittest"
+
 	"github.com/stretchr/testify/assert"
 )
 
@@ -76,3 +80,77 @@ func TestBranchRuleMatch(t *testing.T) {
 		)
 	}
 }
+
+func TestUpdateProtectBranchPriorities(t *testing.T) {
+	assert.NoError(t, unittest.PrepareTestDatabase())
+
+	repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1})
+
+	// Create some test protected branches with initial priorities
+	protectedBranches := []*ProtectedBranch{
+		{
+			RepoID:   repo.ID,
+			RuleName: "master",
+			Priority: 1,
+		},
+		{
+			RepoID:   repo.ID,
+			RuleName: "develop",
+			Priority: 2,
+		},
+		{
+			RepoID:   repo.ID,
+			RuleName: "feature/*",
+			Priority: 3,
+		},
+	}
+
+	for _, pb := range protectedBranches {
+		_, err := db.GetEngine(db.DefaultContext).Insert(pb)
+		assert.NoError(t, err)
+	}
+
+	// Test updating priorities
+	newPriorities := []int64{protectedBranches[2].ID, protectedBranches[0].ID, protectedBranches[1].ID}
+	err := UpdateProtectBranchPriorities(db.DefaultContext, repo, newPriorities)
+	assert.NoError(t, err)
+
+	// Verify new priorities
+	pbs, err := FindRepoProtectedBranchRules(db.DefaultContext, repo.ID)
+	assert.NoError(t, err)
+
+	expectedPriorities := map[string]int64{
+		"feature/*": 1,
+		"master":    2,
+		"develop":   3,
+	}
+
+	for _, pb := range pbs {
+		assert.Equal(t, expectedPriorities[pb.RuleName], pb.Priority)
+	}
+}
+
+func TestNewProtectBranchPriority(t *testing.T) {
+	assert.NoError(t, unittest.PrepareTestDatabase())
+	repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1})
+
+	err := UpdateProtectBranch(db.DefaultContext, repo, &ProtectedBranch{
+		RepoID:   repo.ID,
+		RuleName: "branch-1",
+		Priority: 1,
+	}, WhitelistOptions{})
+	assert.NoError(t, err)
+
+	newPB := &ProtectedBranch{
+		RepoID:   repo.ID,
+		RuleName: "branch-2",
+		// Priority intentionally omitted
+	}
+
+	err = UpdateProtectBranch(db.DefaultContext, repo, newPB, WhitelistOptions{})
+	assert.NoError(t, err)
+
+	savedPB2, err := GetFirstMatchProtectedBranchRule(db.DefaultContext, repo.ID, "branch-2")
+	assert.NoError(t, err)
+	assert.Equal(t, int64(2), savedPB2.Priority)
+}
diff --git a/models/migrations/migrations.go b/models/migrations/migrations.go
index e0361af86b..4c3cefde7b 100644
--- a/models/migrations/migrations.go
+++ b/models/migrations/migrations.go
@@ -367,6 +367,7 @@ func prepareMigrationTasks() []*migration {
 		newMigration(307, "Fix milestone deadline_unix when there is no due date", v1_23.FixMilestoneNoDueDate),
 		newMigration(308, "Add index(user_id, is_deleted) for action table", v1_23.AddNewIndexForUserDashboard),
 		newMigration(309, "Improve Notification table indices", v1_23.ImproveNotificationTableIndices),
+		newMigration(310, "Add Priority to ProtectedBranch", v1_23.AddPriorityToProtectedBranch),
 	}
 	return preparedMigrations
 }
diff --git a/models/migrations/v1_23/v310.go b/models/migrations/v1_23/v310.go
new file mode 100644
index 0000000000..394417f5a0
--- /dev/null
+++ b/models/migrations/v1_23/v310.go
@@ -0,0 +1,16 @@
+// Copyright 2024 The Gitea Authors. All rights reserved.
+// SPDX-License-Identifier: MIT
+
+package v1_23 //nolint
+
+import (
+	"xorm.io/xorm"
+)
+
+func AddPriorityToProtectedBranch(x *xorm.Engine) error {
+	type ProtectedBranch struct {
+		Priority int64 `xorm:"NOT NULL DEFAULT 0"`
+	}
+
+	return x.Sync(new(ProtectedBranch))
+}