From 846f6187168c807e1353d46d5d2260bf077b43cd Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Wed, 27 Nov 2024 05:41:06 +0100 Subject: [PATCH] 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* --- models/git/protected_branch.go | 34 +++++++- models/git/protected_branch_list.go | 7 ++ models/git/protected_branch_list_test.go | 36 +++++++- models/git/protected_branch_test.go | 78 ++++++++++++++++++ models/migrations/migrations.go | 1 + models/migrations/v1_23/v310.go | 16 ++++ modules/structs/repo_branch.go | 8 ++ routers/api/v1/api.go | 1 + routers/api/v1/repo/branch.go | 56 ++++++++++++- routers/api/v1/swagger/options.go | 3 + routers/web/repo/setting/protected_branch.go | 10 +++ routers/web/web.go | 1 + services/convert/convert.go | 1 + services/forms/repo_form.go | 4 + templates/repo/settings/branches.tmpl | 7 +- templates/swagger/v1_json.tmpl | 82 +++++++++++++++++++ tests/integration/api_branch_test.go | 9 +- web_src/js/features/repo-issue-list.ts | 6 +- .../features/repo-settings-branches.test.ts | 71 ++++++++++++++++ web_src/js/features/repo-settings-branches.ts | 32 ++++++++ web_src/js/features/repo-settings.ts | 2 + web_src/js/svg.ts | 2 + 22 files changed, 454 insertions(+), 13 deletions(-) create mode 100644 models/migrations/v1_23/v310.go create mode 100644 web_src/js/features/repo-settings-branches.test.ts create mode 100644 web_src/js/features/repo-settings-branches.ts 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)) +} diff --git a/modules/structs/repo_branch.go b/modules/structs/repo_branch.go index 12a8344e87..a9aa1d330a 100644 --- a/modules/structs/repo_branch.go +++ b/modules/structs/repo_branch.go @@ -25,6 +25,7 @@ type BranchProtection struct { // Deprecated: true BranchName string `json:"branch_name"` RuleName string `json:"rule_name"` + Priority int64 `json:"priority"` EnablePush bool `json:"enable_push"` EnablePushWhitelist bool `json:"enable_push_whitelist"` PushWhitelistUsernames []string `json:"push_whitelist_usernames"` @@ -64,6 +65,7 @@ type CreateBranchProtectionOption struct { // Deprecated: true BranchName string `json:"branch_name"` RuleName string `json:"rule_name"` + Priority int64 `json:"priority"` EnablePush bool `json:"enable_push"` EnablePushWhitelist bool `json:"enable_push_whitelist"` PushWhitelistUsernames []string `json:"push_whitelist_usernames"` @@ -96,6 +98,7 @@ type CreateBranchProtectionOption struct { // EditBranchProtectionOption options for editing a branch protection type EditBranchProtectionOption struct { + Priority *int64 `json:"priority"` EnablePush *bool `json:"enable_push"` EnablePushWhitelist *bool `json:"enable_push_whitelist"` PushWhitelistUsernames []string `json:"push_whitelist_usernames"` @@ -125,3 +128,8 @@ type EditBranchProtectionOption struct { UnprotectedFilePatterns *string `json:"unprotected_file_patterns"` BlockAdminMergeOverride *bool `json:"block_admin_merge_override"` } + +// UpdateBranchProtectionPriories a list to update the branch protection rule priorities +type UpdateBranchProtectionPriories struct { + IDs []int64 `json:"ids"` +} diff --git a/routers/api/v1/api.go b/routers/api/v1/api.go index aee76325a8..f28ee980e1 100644 --- a/routers/api/v1/api.go +++ b/routers/api/v1/api.go @@ -1204,6 +1204,7 @@ func Routes() *web.Router { m.Patch("", bind(api.EditBranchProtectionOption{}), mustNotBeArchived, repo.EditBranchProtection) m.Delete("", repo.DeleteBranchProtection) }) + m.Post("/priority", bind(api.UpdateBranchProtectionPriories{}), mustNotBeArchived, repo.UpdateBranchProtectionPriories) }, reqToken(), reqAdmin()) m.Group("/tags", func() { m.Get("", repo.ListTags) diff --git a/routers/api/v1/repo/branch.go b/routers/api/v1/repo/branch.go index 1cea7d8c72..45c5c1cd14 100644 --- a/routers/api/v1/repo/branch.go +++ b/routers/api/v1/repo/branch.go @@ -618,6 +618,7 @@ func CreateBranchProtection(ctx *context.APIContext) { protectBranch = &git_model.ProtectedBranch{ RepoID: ctx.Repo.Repository.ID, RuleName: ruleName, + Priority: form.Priority, CanPush: form.EnablePush, EnableWhitelist: form.EnablePush && form.EnablePushWhitelist, WhitelistDeployKeys: form.EnablePush && form.EnablePushWhitelist && form.PushWhitelistDeployKeys, @@ -640,7 +641,7 @@ func CreateBranchProtection(ctx *context.APIContext) { BlockAdminMergeOverride: form.BlockAdminMergeOverride, } - err = git_model.UpdateProtectBranch(ctx, ctx.Repo.Repository, protectBranch, git_model.WhitelistOptions{ + if err := git_model.UpdateProtectBranch(ctx, ctx.Repo.Repository, protectBranch, git_model.WhitelistOptions{ UserIDs: whitelistUsers, TeamIDs: whitelistTeams, ForcePushUserIDs: forcePushAllowlistUsers, @@ -649,14 +650,13 @@ func CreateBranchProtection(ctx *context.APIContext) { MergeTeamIDs: mergeWhitelistTeams, ApprovalsUserIDs: approvalsWhitelistUsers, ApprovalsTeamIDs: approvalsWhitelistTeams, - }) - if err != nil { + }); err != nil { ctx.Error(http.StatusInternalServerError, "UpdateProtectBranch", err) return } if isBranchExist { - if err = pull_service.CheckPRsForBaseBranch(ctx, ctx.Repo.Repository, ruleName); err != nil { + if err := pull_service.CheckPRsForBaseBranch(ctx, ctx.Repo.Repository, ruleName); err != nil { ctx.Error(http.StatusInternalServerError, "CheckPRsForBaseBranch", err) return } @@ -796,6 +796,10 @@ func EditBranchProtection(ctx *context.APIContext) { } } + if form.Priority != nil { + protectBranch.Priority = *form.Priority + } + if form.EnableMergeWhitelist != nil { protectBranch.EnableMergeWhitelist = *form.EnableMergeWhitelist } @@ -1080,3 +1084,47 @@ func DeleteBranchProtection(ctx *context.APIContext) { ctx.Status(http.StatusNoContent) } + +// UpdateBranchProtectionPriories updates the priorities of branch protections for a repo +func UpdateBranchProtectionPriories(ctx *context.APIContext) { + // swagger:operation POST /repos/{owner}/{repo}/branch_protections/priority repository repoUpdateBranchProtectionPriories + // --- + // summary: Update the priorities of branch protections for a repository. + // consumes: + // - application/json + // produces: + // - application/json + // parameters: + // - name: owner + // in: path + // description: owner of the repo + // type: string + // required: true + // - name: repo + // in: path + // description: name of the repo + // type: string + // required: true + // - name: body + // in: body + // schema: + // "$ref": "#/definitions/UpdateBranchProtectionPriories" + // responses: + // "204": + // "$ref": "#/responses/empty" + // "404": + // "$ref": "#/responses/notFound" + // "422": + // "$ref": "#/responses/validationError" + // "423": + // "$ref": "#/responses/repoArchivedError" + form := web.GetForm(ctx).(*api.UpdateBranchProtectionPriories) + repo := ctx.Repo.Repository + + if err := git_model.UpdateProtectBranchPriorities(ctx, repo, form.IDs); err != nil { + ctx.Error(http.StatusInternalServerError, "UpdateProtectBranchPriorities", err) + return + } + + ctx.Status(http.StatusNoContent) +} diff --git a/routers/api/v1/swagger/options.go b/routers/api/v1/swagger/options.go index 1de58632d5..39c98c666e 100644 --- a/routers/api/v1/swagger/options.go +++ b/routers/api/v1/swagger/options.go @@ -146,6 +146,9 @@ type swaggerParameterBodies struct { // in:body EditBranchProtectionOption api.EditBranchProtectionOption + // in:body + UpdateBranchProtectionPriories api.UpdateBranchProtectionPriories + // in:body CreateOAuth2ApplicationOptions api.CreateOAuth2ApplicationOptions diff --git a/routers/web/repo/setting/protected_branch.go b/routers/web/repo/setting/protected_branch.go index 940a138aff..f651d8f318 100644 --- a/routers/web/repo/setting/protected_branch.go +++ b/routers/web/repo/setting/protected_branch.go @@ -322,6 +322,16 @@ func DeleteProtectedBranchRulePost(ctx *context.Context) { ctx.JSONRedirect(fmt.Sprintf("%s/settings/branches", ctx.Repo.RepoLink)) } +func UpdateBranchProtectionPriories(ctx *context.Context) { + form := web.GetForm(ctx).(*forms.ProtectBranchPriorityForm) + repo := ctx.Repo.Repository + + if err := git_model.UpdateProtectBranchPriorities(ctx, repo, form.IDs); err != nil { + ctx.ServerError("UpdateProtectBranchPriorities", err) + return + } +} + // RenameBranchPost responses for rename a branch func RenameBranchPost(ctx *context.Context) { form := web.GetForm(ctx).(*forms.RenameBranchForm) diff --git a/routers/web/web.go b/routers/web/web.go index b96d06ed66..a2c14993ac 100644 --- a/routers/web/web.go +++ b/routers/web/web.go @@ -1081,6 +1081,7 @@ func registerRoutes(m *web.Router) { m.Combo("/edit").Get(repo_setting.SettingsProtectedBranch). Post(web.Bind(forms.ProtectBranchForm{}), context.RepoMustNotBeArchived(), repo_setting.SettingsProtectedBranchPost) m.Post("/{id}/delete", repo_setting.DeleteProtectedBranchRulePost) + m.Post("/priority", web.Bind(forms.ProtectBranchPriorityForm{}), context.RepoMustNotBeArchived(), repo_setting.UpdateBranchProtectionPriories) }) m.Group("/tags", func() { diff --git a/services/convert/convert.go b/services/convert/convert.go index 8dc311dae9..c8cad2a2ad 100644 --- a/services/convert/convert.go +++ b/services/convert/convert.go @@ -158,6 +158,7 @@ func ToBranchProtection(ctx context.Context, bp *git_model.ProtectedBranch, repo return &api.BranchProtection{ BranchName: branchName, RuleName: bp.RuleName, + Priority: bp.Priority, EnablePush: bp.CanPush, EnablePushWhitelist: bp.EnableWhitelist, PushWhitelistUsernames: pushWhitelistUsernames, diff --git a/services/forms/repo_form.go b/services/forms/repo_form.go index 8e663084f8..7647c74e46 100644 --- a/services/forms/repo_form.go +++ b/services/forms/repo_form.go @@ -228,6 +228,10 @@ func (f *ProtectBranchForm) Validate(req *http.Request, errs binding.Errors) bin return middleware.Validate(errs, ctx.Data, f, ctx.Locale) } +type ProtectBranchPriorityForm struct { + IDs []int64 +} + // __ __ ___. .__ __ // / \ / \ ____\_ |__ | |__ ____ ____ | | __ // \ \/\/ // __ \| __ \| | \ / _ \ / _ \| |/ / diff --git a/templates/repo/settings/branches.tmpl b/templates/repo/settings/branches.tmpl index 6f070ba61c..57d9f2c5a8 100644 --- a/templates/repo/settings/branches.tmpl +++ b/templates/repo/settings/branches.tmpl @@ -37,9 +37,12 @@
-
+
{{range .ProtectedBranches}} -
+
+
+ {{svg "octicon-grabber" 16}} +
{{.RuleName}}
diff --git a/templates/swagger/v1_json.tmpl b/templates/swagger/v1_json.tmpl index a6dcba4f19..c06c0ad154 100644 --- a/templates/swagger/v1_json.tmpl +++ b/templates/swagger/v1_json.tmpl @@ -4666,6 +4666,58 @@ } } }, + "/repos/{owner}/{repo}/branch_protections/priority": { + "post": { + "consumes": [ + "application/json" + ], + "produces": [ + "application/json" + ], + "tags": [ + "repository" + ], + "summary": "Update the priorities of branch protections for a repository.", + "operationId": "repoUpdateBranchProtectionPriories", + "parameters": [ + { + "type": "string", + "description": "owner of the repo", + "name": "owner", + "in": "path", + "required": true + }, + { + "type": "string", + "description": "name of the repo", + "name": "repo", + "in": "path", + "required": true + }, + { + "name": "body", + "in": "body", + "schema": { + "$ref": "#/definitions/UpdateBranchProtectionPriories" + } + } + ], + "responses": { + "204": { + "$ref": "#/responses/empty" + }, + "404": { + "$ref": "#/responses/notFound" + }, + "422": { + "$ref": "#/responses/validationError" + }, + "423": { + "$ref": "#/responses/repoArchivedError" + } + } + } + }, "/repos/{owner}/{repo}/branch_protections/{name}": { "get": { "produces": [ @@ -18874,6 +18926,11 @@ }, "x-go-name": "MergeWhitelistUsernames" }, + "priority": { + "type": "integer", + "format": "int64", + "x-go-name": "Priority" + }, "protected_file_patterns": { "type": "string", "x-go-name": "ProtectedFilePatterns" @@ -19568,6 +19625,11 @@ }, "x-go-name": "MergeWhitelistUsernames" }, + "priority": { + "type": "integer", + "format": "int64", + "x-go-name": "Priority" + }, "protected_file_patterns": { "type": "string", "x-go-name": "ProtectedFilePatterns" @@ -20800,6 +20862,11 @@ }, "x-go-name": "MergeWhitelistUsernames" }, + "priority": { + "type": "integer", + "format": "int64", + "x-go-name": "Priority" + }, "protected_file_patterns": { "type": "string", "x-go-name": "ProtectedFilePatterns" @@ -24886,6 +24953,21 @@ }, "x-go-package": "code.gitea.io/gitea/modules/structs" }, + "UpdateBranchProtectionPriories": { + "description": "UpdateBranchProtectionPriories a list to update the branch protection rule priorities", + "type": "object", + "properties": { + "ids": { + "type": "array", + "items": { + "type": "integer", + "format": "int64" + }, + "x-go-name": "IDs" + } + }, + "x-go-package": "code.gitea.io/gitea/modules/structs" + }, "UpdateFileOptions": { "description": "UpdateFileOptions options for updating files\nNote: `author` and `committer` are optional (if only one is given, it will be used for the other, otherwise the authenticated user will be used)", "type": "object", diff --git a/tests/integration/api_branch_test.go b/tests/integration/api_branch_test.go index dc1aaec2a2..8e49516aa7 100644 --- a/tests/integration/api_branch_test.go +++ b/tests/integration/api_branch_test.go @@ -49,7 +49,7 @@ func testAPIGetBranchProtection(t *testing.T, branchName string, expectedHTTPSta return nil } -func testAPICreateBranchProtection(t *testing.T, branchName string, expectedHTTPStatus int) { +func testAPICreateBranchProtection(t *testing.T, branchName string, expectedPriority, expectedHTTPStatus int) { token := getUserToken(t, "user2", auth_model.AccessTokenScopeWriteRepository) req := NewRequestWithJSON(t, "POST", "/api/v1/repos/user2/repo1/branch_protections", &api.BranchProtection{ RuleName: branchName, @@ -60,6 +60,7 @@ func testAPICreateBranchProtection(t *testing.T, branchName string, expectedHTTP var branchProtection api.BranchProtection DecodeJSON(t, resp, &branchProtection) assert.EqualValues(t, branchName, branchProtection.RuleName) + assert.EqualValues(t, expectedPriority, branchProtection.Priority) } } @@ -189,13 +190,13 @@ func TestAPIBranchProtection(t *testing.T) { defer tests.PrepareTestEnv(t)() // Branch protection on branch that not exist - testAPICreateBranchProtection(t, "master/doesnotexist", http.StatusCreated) + testAPICreateBranchProtection(t, "master/doesnotexist", 1, http.StatusCreated) // Get branch protection on branch that exist but not branch protection testAPIGetBranchProtection(t, "master", http.StatusNotFound) - testAPICreateBranchProtection(t, "master", http.StatusCreated) + testAPICreateBranchProtection(t, "master", 2, http.StatusCreated) // Can only create once - testAPICreateBranchProtection(t, "master", http.StatusForbidden) + testAPICreateBranchProtection(t, "master", 0, http.StatusForbidden) // Can't delete a protected branch testAPIDeleteBranch(t, "master", http.StatusForbidden) diff --git a/web_src/js/features/repo-issue-list.ts b/web_src/js/features/repo-issue-list.ts index 931122db3c..a7185e5f99 100644 --- a/web_src/js/features/repo-issue-list.ts +++ b/web_src/js/features/repo-issue-list.ts @@ -196,7 +196,11 @@ async function initIssuePinSort() { createSortable(pinDiv, { group: 'shared', - onEnd: pinMoveEnd, // eslint-disable-line @typescript-eslint/no-misused-promises + onEnd: (e) => { + (async () => { + await pinMoveEnd(e); + })(); + }, }); } diff --git a/web_src/js/features/repo-settings-branches.test.ts b/web_src/js/features/repo-settings-branches.test.ts new file mode 100644 index 0000000000..023039334f --- /dev/null +++ b/web_src/js/features/repo-settings-branches.test.ts @@ -0,0 +1,71 @@ +import {beforeEach, describe, expect, test, vi} from 'vitest'; +import {initRepoBranchesSettings} from './repo-settings-branches.ts'; +import {POST} from '../modules/fetch.ts'; +import {createSortable} from '../modules/sortable.ts'; + +vi.mock('../modules/fetch.ts', () => ({ + POST: vi.fn(), +})); + +vi.mock('../modules/sortable.ts', () => ({ + createSortable: vi.fn(), +})); + +describe('Repository Branch Settings', () => { + beforeEach(() => { + document.body.innerHTML = ` +
+
+
+
+
+
+
+
+
+
+
+ `; + + vi.clearAllMocks(); + }); + + test('should initialize sortable for protected branches list', () => { + initRepoBranchesSettings(); + + expect(createSortable).toHaveBeenCalledWith( + document.querySelector('#protected-branches-list'), + expect.objectContaining({ + handle: '.drag-handle', + animation: 150, + }), + ); + }); + + test('should not initialize if protected branches list is not present', () => { + document.body.innerHTML = ''; + + initRepoBranchesSettings(); + + expect(createSortable).not.toHaveBeenCalled(); + }); + + test('should post new order after sorting', async () => { + vi.mocked(POST).mockResolvedValue({ok: true} as Response); + + // Mock createSortable to capture and execute the onEnd callback + vi.mocked(createSortable).mockImplementation((_el, options) => { + options.onEnd(); + return {destroy: vi.fn()}; + }); + + initRepoBranchesSettings(); + + expect(POST).toHaveBeenCalledWith( + 'some/repo/branches/priority', + expect.objectContaining({ + data: {ids: [1, 2, 3]}, + }), + ); + }); +}); diff --git a/web_src/js/features/repo-settings-branches.ts b/web_src/js/features/repo-settings-branches.ts new file mode 100644 index 0000000000..43b98f79b3 --- /dev/null +++ b/web_src/js/features/repo-settings-branches.ts @@ -0,0 +1,32 @@ +import {createSortable} from '../modules/sortable.ts'; +import {POST} from '../modules/fetch.ts'; +import {showErrorToast} from '../modules/toast.ts'; +import {queryElemChildren} from '../utils/dom.ts'; + +export function initRepoBranchesSettings() { + const protectedBranchesList = document.querySelector('#protected-branches-list'); + if (!protectedBranchesList) return; + + createSortable(protectedBranchesList, { + handle: '.drag-handle', + animation: 150, + + onEnd: () => { + (async () => { + const itemElems = queryElemChildren(protectedBranchesList, '.item[data-id]'); + const itemIds = Array.from(itemElems, (el) => parseInt(el.getAttribute('data-id'))); + + try { + await POST(protectedBranchesList.getAttribute('data-update-priority-url'), { + data: { + ids: itemIds, + }, + }); + } catch (err) { + const errorMessage = String(err); + showErrorToast(`Failed to update branch protection rule priority:, error: ${errorMessage}`); + } + })(); + }, + }); +} diff --git a/web_src/js/features/repo-settings.ts b/web_src/js/features/repo-settings.ts index 72213f794a..5a009cfea4 100644 --- a/web_src/js/features/repo-settings.ts +++ b/web_src/js/features/repo-settings.ts @@ -3,6 +3,7 @@ import {minimatch} from 'minimatch'; import {createMonaco} from './codeeditor.ts'; import {onInputDebounce, queryElems, toggleElem} from '../utils/dom.ts'; import {POST} from '../modules/fetch.ts'; +import {initRepoBranchesSettings} from './repo-settings-branches.ts'; const {appSubUrl, csrfToken} = window.config; @@ -154,4 +155,5 @@ export function initRepoSettings() { initRepoSettingsCollaboration(); initRepoSettingsSearchTeamBox(); initRepoSettingsGitHook(); + initRepoBranchesSettings(); } diff --git a/web_src/js/svg.ts b/web_src/js/svg.ts index d04f63793f..cbb1af4ba1 100644 --- a/web_src/js/svg.ts +++ b/web_src/js/svg.ts @@ -34,6 +34,7 @@ import octiconGitCommit from '../../public/assets/img/svg/octicon-git-commit.svg import octiconGitMerge from '../../public/assets/img/svg/octicon-git-merge.svg'; import octiconGitPullRequest from '../../public/assets/img/svg/octicon-git-pull-request.svg'; import octiconGitPullRequestDraft from '../../public/assets/img/svg/octicon-git-pull-request-draft.svg'; +import octiconGrabber from '../../public/assets/img/svg/octicon-grabber.svg'; import octiconHeading from '../../public/assets/img/svg/octicon-heading.svg'; import octiconHorizontalRule from '../../public/assets/img/svg/octicon-horizontal-rule.svg'; import octiconImage from '../../public/assets/img/svg/octicon-image.svg'; @@ -107,6 +108,7 @@ const svgs = { 'octicon-git-merge': octiconGitMerge, 'octicon-git-pull-request': octiconGitPullRequest, 'octicon-git-pull-request-draft': octiconGitPullRequestDraft, + 'octicon-grabber': octiconGrabber, 'octicon-heading': octiconHeading, 'octicon-horizontal-rule': octiconHorizontalRule, 'octicon-image': octiconImage,