Refactor: move part of updating protected branch logic to service layer (#33742)

Author: lunny

routers/api/v1/repo/branch.go

@@ -594,12 +594,6 @@ func CreateBranchProtection(ctx *context.APIContext) {
 		return
 	}
 
-	isPlainRule := !git_model.IsRuleNameSpecial(ruleName)
-	var isBranchExist bool
-	if isPlainRule {
-		isBranchExist = git.IsBranchExist(ctx.Req.Context(), ctx.Repo.Repository.RepoPath(), ruleName)
-	}
-
 	protectBranch, err := git_model.GetProtectedBranchRuleByName(ctx, repo.ID, ruleName)
 	if err != nil {
 		ctx.APIErrorInternal(err)
@@ -716,7 +710,7 @@ func CreateBranchProtection(ctx *context.APIContext) {
 		BlockAdminMergeOverride:       form.BlockAdminMergeOverride,
 	}
 
-	if err := git_model.UpdateProtectBranch(ctx, ctx.Repo.Repository, protectBranch, git_model.WhitelistOptions{
+	if err := pull_service.CreateOrUpdateProtectedBranch(ctx, ctx.Repo.Repository, protectBranch, git_model.WhitelistOptions{
 		UserIDs:          whitelistUsers,
 		TeamIDs:          whitelistTeams,
 		ForcePushUserIDs: forcePushAllowlistUsers,
@@ -730,36 +724,6 @@ func CreateBranchProtection(ctx *context.APIContext) {
 		return
 	}
 
-	if isBranchExist {
-		if err := pull_service.CheckPRsForBaseBranch(ctx, ctx.Repo.Repository, ruleName); err != nil {
-			ctx.APIErrorInternal(err)
-			return
-		}
-	} else {
-		if !isPlainRule {
-			if ctx.Repo.GitRepo == nil {
-				ctx.Repo.GitRepo, err = gitrepo.RepositoryFromRequestContextOrOpen(ctx, ctx.Repo.Repository)
-				if err != nil {
-					ctx.APIErrorInternal(err)
-					return
-				}
-			}
-			// FIXME: since we only need to recheck files protected rules, we could improve this
-			matchedBranches, err := git_model.FindAllMatchedBranches(ctx, ctx.Repo.Repository.ID, ruleName)
-			if err != nil {
-				ctx.APIErrorInternal(err)
-				return
-			}
-
-			for _, branchName := range matchedBranches {
-				if err = pull_service.CheckPRsForBaseBranch(ctx, ctx.Repo.Repository, branchName); err != nil {
-					ctx.APIErrorInternal(err)
-					return
-				}
-			}
-		}
-	}
-
 	// Reload from db to get all whitelists
 	bp, err := git_model.GetProtectedBranchRuleByName(ctx, ctx.Repo.Repository.ID, ruleName)
 	if err != nil {

routers/web/repo/setting/protected_branch.go

@@ -261,7 +261,7 @@ func SettingsProtectedBranchPost(ctx *context.Context) {
 	protectBranch.BlockOnOutdatedBranch = f.BlockOnOutdatedBranch
 	protectBranch.BlockAdminMergeOverride = f.BlockAdminMergeOverride
 
-	err = git_model.UpdateProtectBranch(ctx, ctx.Repo.Repository, protectBranch, git_model.WhitelistOptions{
+	if err = pull_service.CreateOrUpdateProtectedBranch(ctx, ctx.Repo.Repository, protectBranch, git_model.WhitelistOptions{
 		UserIDs:          whitelistUsers,
 		TeamIDs:          whitelistTeams,
 		ForcePushUserIDs: forcePushAllowlistUsers,
@@ -270,24 +270,10 @@ func SettingsProtectedBranchPost(ctx *context.Context) {
 		MergeTeamIDs:     mergeWhitelistTeams,
 		ApprovalsUserIDs: approvalsWhitelistUsers,
 		ApprovalsTeamIDs: approvalsWhitelistTeams,
-	})
-	if err != nil {
-		ctx.ServerError("UpdateProtectBranch", err)
-		return
-	}
-
-	// FIXME: since we only need to recheck files protected rules, we could improve this
-	matchedBranches, err := git_model.FindAllMatchedBranches(ctx, ctx.Repo.Repository.ID, protectBranch.RuleName)
-	if err != nil {
-		ctx.ServerError("FindAllMatchedBranches", err)
+	}); err != nil {
+		ctx.ServerError("CreateOrUpdateProtectedBranch", err)
 		return
 	}
-	for _, branchName := range matchedBranches {
-		if err = pull_service.CheckPRsForBaseBranch(ctx, ctx.Repo.Repository, branchName); err != nil {
-			ctx.ServerError("CheckPRsForBaseBranch", err)
-			return
-		}
-	}
 
 	ctx.Flash.Success(ctx.Tr("repo.settings.update_protect_branch_success", protectBranch.RuleName))
 	ctx.Redirect(fmt.Sprintf("%s/settings/branches?rule_name=%s", ctx.Repo.RepoLink, protectBranch.RuleName))

services/forms/repo_form.go

@@ -170,13 +170,6 @@ func (f *RepoSettingForm) Validate(req *http.Request, errs binding.Errors) bindi
 	return middleware.Validate(errs, ctx.Data, f, ctx.Locale)
 }
 
-// __________                             .__
-// \______   \____________    ____   ____ |  |__
-//  |    |  _/\_  __ \__  \  /    \_/ ___\|  |  \
-//  |    |   \ |  | \// __ \|   |  \  \___|   Y  \
-//  |______  / |__|  (____  /___|  /\___  >___|  /
-//         \/             \/     \/     \/     \/
-
 // ProtectBranchForm form for changing protected branch settings
 type ProtectBranchForm struct {
 	RuleName                      string `binding:"Required"`

services/pull/protected_branch.go

@@ -0,0 +1,48 @@
+// Copyright 2025 The Gitea Authors. All rights reserved.
+// SPDX-License-Identifier: MIT
+
+package pull
+
+import (
+	"context"
+
+	git_model "code.gitea.io/gitea/models/git"
+	repo_model "code.gitea.io/gitea/models/repo"
+	"code.gitea.io/gitea/modules/git"
+)
+
+func CreateOrUpdateProtectedBranch(ctx context.Context, repo *repo_model.Repository,
+	protectBranch *git_model.ProtectedBranch, whitelistOptions git_model.WhitelistOptions,
+) error {
+	err := git_model.UpdateProtectBranch(ctx, repo, protectBranch, whitelistOptions)
+	if err != nil {
+		return err
+	}
+
+	isPlainRule := !git_model.IsRuleNameSpecial(protectBranch.RuleName)
+	var isBranchExist bool
+	if isPlainRule {
+		isBranchExist = git.IsBranchExist(ctx, repo.RepoPath(), protectBranch.RuleName)
+	}
+
+	if isBranchExist {
+		if err := CheckPRsForBaseBranch(ctx, repo, protectBranch.RuleName); err != nil {
+			return err
+		}
+	} else {
+		if !isPlainRule {
+			// FIXME: since we only need to recheck files protected rules, we could improve this
+			matchedBranches, err := git_model.FindAllMatchedBranches(ctx, repo.ID, protectBranch.RuleName)
+			if err != nil {
+				return err
+			}
+			for _, branchName := range matchedBranches {
+				if err = CheckPRsForBaseBranch(ctx, repo, branchName); err != nil {
+					return err
+				}
+			}
+		}
+	}
+
+	return nil
+}