Honor delete branch on merge repo setting when using merge API (#35488)

Fix #35463.

---------

Co-authored-by: wxiaoguang <[email protected]>

Author: kemzeb

models/git/protected_branch.go

@@ -5,7 +5,6 @@ package git
 
 import (
 	"context"
-	"errors"
 	"fmt"
 	"slices"
 	"strings"
@@ -25,7 +24,7 @@ import (
 	"xorm.io/builder"
 )
 
-var ErrBranchIsProtected = errors.New("branch is protected")
+var ErrBranchIsProtected = util.ErrorWrap(util.ErrPermissionDenied, "branch is protected")
 
 // ProtectedBranch struct
 type ProtectedBranch struct {

modules/util/error.go

@@ -6,6 +6,7 @@ package util
 import (
 	"errors"
 	"fmt"
+	"html/template"
 )
 
 // Common Errors forming the base of our error system
@@ -40,22 +41,6 @@ func (w errorWrapper) Unwrap() error {
 	return w.Err
 }
 
-type LocaleWrapper struct {
-	err    error
-	TrKey  string
-	TrArgs []any
-}
-
-// Error returns the message
-func (w LocaleWrapper) Error() string {
-	return w.err.Error()
-}
-
-// Unwrap returns the underlying error
-func (w LocaleWrapper) Unwrap() error {
-	return w.err
-}
-
 // ErrorWrap returns an error that formats as the given text but unwraps as the provided error
 func ErrorWrap(unwrap error, message string, args ...any) error {
 	if len(args) == 0 {
@@ -84,15 +69,39 @@ func NewNotExistErrorf(message string, args ...any) error {
 	return ErrorWrap(ErrNotExist, message, args...)
 }
 
-// ErrorWrapLocale wraps an err with a translation key and arguments
-func ErrorWrapLocale(err error, trKey string, trArgs ...any) error {
-	return LocaleWrapper{err: err, TrKey: trKey, TrArgs: trArgs}
+// ErrorTranslatable wraps an error with translation information
+type ErrorTranslatable interface {
+	error
+	Unwrap() error
+	Translate(ErrorLocaleTranslator) template.HTML
+}
+
+type errorTranslatableWrapper struct {
+	err    error
+	trKey  string
+	trArgs []any
+}
+
+type ErrorLocaleTranslator interface {
+	Tr(key string, args ...any) template.HTML
+}
+
+func (w *errorTranslatableWrapper) Error() string { return w.err.Error() }
+
+func (w *errorTranslatableWrapper) Unwrap() error { return w.err }
+
+func (w *errorTranslatableWrapper) Translate(t ErrorLocaleTranslator) template.HTML {
+	return t.Tr(w.trKey, w.trArgs...)
+}
+
+func ErrorWrapTranslatable(err error, trKey string, trArgs ...any) ErrorTranslatable {
+	return &errorTranslatableWrapper{err: err, trKey: trKey, trArgs: trArgs}
 }
 
-func ErrorAsLocale(err error) *LocaleWrapper {
-	var e LocaleWrapper
+func ErrorAsTranslatable(err error) ErrorTranslatable {
+	var e *errorTranslatableWrapper
 	if errors.As(err, &e) {
-		return &e
+		return e
 	}
 	return nil
 }

modules/util/error_test.go

@@ -0,0 +1,29 @@
+// Copyright 2025 The Gitea Authors. All rights reserved.
+// SPDX-License-Identifier: MIT
+
+package util
+
+import (
+	"io"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestErrorTranslatable(t *testing.T) {
+	var err error
+
+	err = ErrorWrapTranslatable(io.EOF, "key", 1)
+	assert.ErrorIs(t, err, io.EOF)
+	assert.Equal(t, "EOF", err.Error())
+	assert.Equal(t, "key", err.(*errorTranslatableWrapper).trKey)
+	assert.Equal(t, []any{1}, err.(*errorTranslatableWrapper).trArgs)
+
+	err = ErrorWrap(err, "new msg %d", 100)
+	assert.ErrorIs(t, err, io.EOF)
+	assert.Equal(t, "new msg 100", err.Error())
+
+	errTr := ErrorAsTranslatable(err)
+	assert.Equal(t, "EOF", errTr.Error())
+	assert.Equal(t, "key", errTr.(*errorTranslatableWrapper).trKey)
+}

routers/api/v1/repo/pull.go

@@ -13,7 +13,6 @@ import (
 	"time"
 
 	activities_model "code.gitea.io/gitea/models/activities"
-	git_model "code.gitea.io/gitea/models/git"
 	issues_model "code.gitea.io/gitea/models/issues"
 	access_model "code.gitea.io/gitea/models/perm/access"
 	pull_model "code.gitea.io/gitea/models/pull"
@@ -938,7 +937,7 @@ func MergePullRequest(ctx *context.APIContext) {
 		} else if errors.Is(err, pull_service.ErrNoPermissionToMerge) {
 			ctx.APIError(http.StatusMethodNotAllowed, "User not allowed to merge PR")
 		} else if errors.Is(err, pull_service.ErrHasMerged) {
-			ctx.APIError(http.StatusMethodNotAllowed, "")
+			ctx.APIError(http.StatusMethodNotAllowed, "The PR is already merged")
 		} else if errors.Is(err, pull_service.ErrIsWorkInProgress) {
 			ctx.APIError(http.StatusMethodNotAllowed, "Work in progress PRs cannot be merged")
 		} else if errors.Is(err, pull_service.ErrNotMergeableState) {
@@ -989,8 +988,14 @@ func MergePullRequest(ctx *context.APIContext) {
 		message += "\n\n" + form.MergeMessageField
 	}
 
+	deleteBranchAfterMerge, err := pull_service.ShouldDeleteBranchAfterMerge(ctx, form.DeleteBranchAfterMerge, ctx.Repo.Repository, pr)
+	if err != nil {
+		ctx.APIErrorInternal(err)
+		return
+	}
+
 	if form.MergeWhenChecksSucceed {
-		scheduled, err := automerge.ScheduleAutoMerge(ctx, ctx.Doer, pr, repo_model.MergeStyle(form.Do), message, form.DeleteBranchAfterMerge)
+		scheduled, err := automerge.ScheduleAutoMerge(ctx, ctx.Doer, pr, repo_model.MergeStyle(form.Do), message, deleteBranchAfterMerge)
 		if err != nil {
 			if pull_model.IsErrAlreadyScheduledToAutoMerge(err) {
 				ctx.APIError(http.StatusConflict, err)
@@ -1035,47 +1040,10 @@ func MergePullRequest(ctx *context.APIContext) {
 	}
 	log.Trace("Pull request merged: %d", pr.ID)
 
-	// for agit flow, we should not delete the agit reference after merge
-	if form.DeleteBranchAfterMerge && pr.Flow == issues_model.PullRequestFlowGithub {
-		// check permission even it has been checked in repo_service.DeleteBranch so that we don't need to
-		// do RetargetChildrenOnMerge
-		if err := repo_service.CanDeleteBranch(ctx, pr.HeadRepo, pr.HeadBranch, ctx.Doer); err == nil {
-			// Don't cleanup when there are other PR's that use this branch as head branch.
-			exist, err := issues_model.HasUnmergedPullRequestsByHeadInfo(ctx, pr.HeadRepoID, pr.HeadBranch)
-			if err != nil {
-				ctx.APIErrorInternal(err)
-				return
-			}
-			if exist {
-				ctx.Status(http.StatusOK)
-				return
-			}
-
-			var headRepo *git.Repository
-			if ctx.Repo != nil && ctx.Repo.Repository != nil && ctx.Repo.Repository.ID == pr.HeadRepoID && ctx.Repo.GitRepo != nil {
-				headRepo = ctx.Repo.GitRepo
-			} else {
-				headRepo, err = gitrepo.OpenRepository(ctx, pr.HeadRepo)
-				if err != nil {
-					ctx.APIErrorInternal(err)
-					return
-				}
-				defer headRepo.Close()
-			}
-
-			if err := repo_service.DeleteBranch(ctx, ctx.Doer, pr.HeadRepo, headRepo, pr.HeadBranch, pr); err != nil {
-				switch {
-				case git.IsErrBranchNotExist(err):
-					ctx.APIErrorNotFound(err)
-				case errors.Is(err, repo_service.ErrBranchIsDefault):
-					ctx.APIError(http.StatusForbidden, errors.New("can not delete default branch"))
-				case errors.Is(err, git_model.ErrBranchIsProtected):
-					ctx.APIError(http.StatusForbidden, errors.New("branch protected"))
-				default:
-					ctx.APIErrorInternal(err)
-				}
-				return
-			}
+	if deleteBranchAfterMerge {
+		if err = repo_service.DeleteBranchAfterMerge(ctx, ctx.Doer, pr.ID, nil); err != nil {
+			// no way to tell users that what error happens, and the PR has been merged, so ignore the error
+			log.Debug("DeleteBranchAfterMerge: pr %d, err: %v", pr.ID, err)
 		}
 	}
 

routers/web/repo/actions/view.go

@@ -943,8 +943,8 @@ func Run(ctx *context_module.Context) {
 		return nil
 	})
 	if err != nil {
-		if errLocale := util.ErrorAsLocale(err); errLocale != nil {
-			ctx.Flash.Error(ctx.Tr(errLocale.TrKey, errLocale.TrArgs...))
+		if errTr := util.ErrorAsTranslatable(err); errTr != nil {
+			ctx.Flash.Error(errTr.Translate(ctx.Locale))
 			ctx.Redirect(redirectURL)
 		} else {
 			ctx.ServerError("DispatchActionWorkflow", err)

routers/web/repo/editor_apply_patch.go

@@ -41,7 +41,7 @@ func NewDiffPatchPost(ctx *context.Context) {
 		Committer:    parsed.GitCommitter,
 	})
 	if err != nil {
-		err = util.ErrorWrapLocale(err, "repo.editor.fail_to_apply_patch")
+		err = util.ErrorWrapTranslatable(err, "repo.editor.fail_to_apply_patch")
 	}
 	if err != nil {
 		editorHandleFileOperationError(ctx, parsed.NewBranchName, err)

routers/web/repo/editor_cherry_pick.go

@@ -74,7 +74,7 @@ func CherryPickPost(ctx *context.Context) {
 			opts.Content = buf.String()
 			_, err = files.ApplyDiffPatch(ctx, ctx.Repo.Repository, ctx.Doer, opts)
 			if err != nil {
-				err = util.ErrorWrapLocale(err, "repo.editor.fail_to_apply_patch")
+				err = util.ErrorWrapTranslatable(err, "repo.editor.fail_to_apply_patch")
 			}
 		}
 		if err != nil {

routers/web/repo/editor_error.go

@@ -38,8 +38,8 @@ func editorHandleFileOperationErrorRender(ctx *context_service.Context, message,
 }
 
 func editorHandleFileOperationError(ctx *context_service.Context, targetBranchName string, err error) {
-	if errAs := util.ErrorAsLocale(err); errAs != nil {
-		ctx.JSONError(ctx.Tr(errAs.TrKey, errAs.TrArgs...))
+	if errAs := util.ErrorAsTranslatable(err); errAs != nil {
+		ctx.JSONError(errAs.Translate(ctx.Locale))
 	} else if errAs, ok := errorAs[git.ErrNotExist](err); ok {
 		ctx.JSONError(ctx.Tr("repo.editor.file_modifying_no_longer_exists", errAs.RelPath))
 	} else if errAs, ok := errorAs[git_model.ErrLFSFileLocked](err); ok {