Skip to content

Commit 0453177

Browse files
authored
Refactor: move part of updating protected branch logic to service layer (#33742)
1 parent df7b61c commit 0453177

File tree

4 files changed

+52
-61
lines changed

4 files changed

+52
-61
lines changed

routers/api/v1/repo/branch.go

Lines changed: 1 addition & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -594,12 +594,6 @@ func CreateBranchProtection(ctx *context.APIContext) {
594594
return
595595
}
596596

597-
isPlainRule := !git_model.IsRuleNameSpecial(ruleName)
598-
var isBranchExist bool
599-
if isPlainRule {
600-
isBranchExist = git.IsBranchExist(ctx.Req.Context(), ctx.Repo.Repository.RepoPath(), ruleName)
601-
}
602-
603597
protectBranch, err := git_model.GetProtectedBranchRuleByName(ctx, repo.ID, ruleName)
604598
if err != nil {
605599
ctx.APIErrorInternal(err)
@@ -716,7 +710,7 @@ func CreateBranchProtection(ctx *context.APIContext) {
716710
BlockAdminMergeOverride: form.BlockAdminMergeOverride,
717711
}
718712

719-
if err := git_model.UpdateProtectBranch(ctx, ctx.Repo.Repository, protectBranch, git_model.WhitelistOptions{
713+
if err := pull_service.CreateOrUpdateProtectedBranch(ctx, ctx.Repo.Repository, protectBranch, git_model.WhitelistOptions{
720714
UserIDs: whitelistUsers,
721715
TeamIDs: whitelistTeams,
722716
ForcePushUserIDs: forcePushAllowlistUsers,
@@ -730,36 +724,6 @@ func CreateBranchProtection(ctx *context.APIContext) {
730724
return
731725
}
732726

733-
if isBranchExist {
734-
if err := pull_service.CheckPRsForBaseBranch(ctx, ctx.Repo.Repository, ruleName); err != nil {
735-
ctx.APIErrorInternal(err)
736-
return
737-
}
738-
} else {
739-
if !isPlainRule {
740-
if ctx.Repo.GitRepo == nil {
741-
ctx.Repo.GitRepo, err = gitrepo.RepositoryFromRequestContextOrOpen(ctx, ctx.Repo.Repository)
742-
if err != nil {
743-
ctx.APIErrorInternal(err)
744-
return
745-
}
746-
}
747-
// FIXME: since we only need to recheck files protected rules, we could improve this
748-
matchedBranches, err := git_model.FindAllMatchedBranches(ctx, ctx.Repo.Repository.ID, ruleName)
749-
if err != nil {
750-
ctx.APIErrorInternal(err)
751-
return
752-
}
753-
754-
for _, branchName := range matchedBranches {
755-
if err = pull_service.CheckPRsForBaseBranch(ctx, ctx.Repo.Repository, branchName); err != nil {
756-
ctx.APIErrorInternal(err)
757-
return
758-
}
759-
}
760-
}
761-
}
762-
763727
// Reload from db to get all whitelists
764728
bp, err := git_model.GetProtectedBranchRuleByName(ctx, ctx.Repo.Repository.ID, ruleName)
765729
if err != nil {

routers/web/repo/setting/protected_branch.go

Lines changed: 3 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -261,7 +261,7 @@ func SettingsProtectedBranchPost(ctx *context.Context) {
261261
protectBranch.BlockOnOutdatedBranch = f.BlockOnOutdatedBranch
262262
protectBranch.BlockAdminMergeOverride = f.BlockAdminMergeOverride
263263

264-
err = git_model.UpdateProtectBranch(ctx, ctx.Repo.Repository, protectBranch, git_model.WhitelistOptions{
264+
if err = pull_service.CreateOrUpdateProtectedBranch(ctx, ctx.Repo.Repository, protectBranch, git_model.WhitelistOptions{
265265
UserIDs: whitelistUsers,
266266
TeamIDs: whitelistTeams,
267267
ForcePushUserIDs: forcePushAllowlistUsers,
@@ -270,24 +270,10 @@ func SettingsProtectedBranchPost(ctx *context.Context) {
270270
MergeTeamIDs: mergeWhitelistTeams,
271271
ApprovalsUserIDs: approvalsWhitelistUsers,
272272
ApprovalsTeamIDs: approvalsWhitelistTeams,
273-
})
274-
if err != nil {
275-
ctx.ServerError("UpdateProtectBranch", err)
276-
return
277-
}
278-
279-
// FIXME: since we only need to recheck files protected rules, we could improve this
280-
matchedBranches, err := git_model.FindAllMatchedBranches(ctx, ctx.Repo.Repository.ID, protectBranch.RuleName)
281-
if err != nil {
282-
ctx.ServerError("FindAllMatchedBranches", err)
273+
}); err != nil {
274+
ctx.ServerError("CreateOrUpdateProtectedBranch", err)
283275
return
284276
}
285-
for _, branchName := range matchedBranches {
286-
if err = pull_service.CheckPRsForBaseBranch(ctx, ctx.Repo.Repository, branchName); err != nil {
287-
ctx.ServerError("CheckPRsForBaseBranch", err)
288-
return
289-
}
290-
}
291277

292278
ctx.Flash.Success(ctx.Tr("repo.settings.update_protect_branch_success", protectBranch.RuleName))
293279
ctx.Redirect(fmt.Sprintf("%s/settings/branches?rule_name=%s", ctx.Repo.RepoLink, protectBranch.RuleName))

services/forms/repo_form.go

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -170,13 +170,6 @@ func (f *RepoSettingForm) Validate(req *http.Request, errs binding.Errors) bindi
170170
return middleware.Validate(errs, ctx.Data, f, ctx.Locale)
171171
}
172172

173-
// __________ .__
174-
// \______ \____________ ____ ____ | |__
175-
// | | _/\_ __ \__ \ / \_/ ___\| | \
176-
// | | \ | | \// __ \| | \ \___| Y \
177-
// |______ / |__| (____ /___| /\___ >___| /
178-
// \/ \/ \/ \/ \/
179-
180173
// ProtectBranchForm form for changing protected branch settings
181174
type ProtectBranchForm struct {
182175
RuleName string `binding:"Required"`

services/pull/protected_branch.go

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
// Copyright 2025 The Gitea Authors. All rights reserved.
2+
// SPDX-License-Identifier: MIT
3+
4+
package pull
5+
6+
import (
7+
"context"
8+
9+
git_model "code.gitea.io/gitea/models/git"
10+
repo_model "code.gitea.io/gitea/models/repo"
11+
"code.gitea.io/gitea/modules/git"
12+
)
13+
14+
func CreateOrUpdateProtectedBranch(ctx context.Context, repo *repo_model.Repository,
15+
protectBranch *git_model.ProtectedBranch, whitelistOptions git_model.WhitelistOptions,
16+
) error {
17+
err := git_model.UpdateProtectBranch(ctx, repo, protectBranch, whitelistOptions)
18+
if err != nil {
19+
return err
20+
}
21+
22+
isPlainRule := !git_model.IsRuleNameSpecial(protectBranch.RuleName)
23+
var isBranchExist bool
24+
if isPlainRule {
25+
isBranchExist = git.IsBranchExist(ctx, repo.RepoPath(), protectBranch.RuleName)
26+
}
27+
28+
if isBranchExist {
29+
if err := CheckPRsForBaseBranch(ctx, repo, protectBranch.RuleName); err != nil {
30+
return err
31+
}
32+
} else {
33+
if !isPlainRule {
34+
// FIXME: since we only need to recheck files protected rules, we could improve this
35+
matchedBranches, err := git_model.FindAllMatchedBranches(ctx, repo.ID, protectBranch.RuleName)
36+
if err != nil {
37+
return err
38+
}
39+
for _, branchName := range matchedBranches {
40+
if err = CheckPRsForBaseBranch(ctx, repo, branchName); err != nil {
41+
return err
42+
}
43+
}
44+
}
45+
}
46+
47+
return nil
48+
}

0 commit comments

Comments
 (0)