fix

Author: wxiaoguang

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
@@ -39,22 +40,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 {
@@ -83,15 +68,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
 }

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"
@@ -989,25 +988,12 @@ func MergePullRequest(ctx *context.APIContext) {
 		message += "\n\n" + form.MergeMessageField
 	}
 
-	prUnit, err := ctx.Repo.Repository.GetUnit(ctx, unit.TypePullRequests)
+	deleteBranchAfterMerge, err := pull_service.ShouldDeleteBranchAfterMerge(ctx, form.DeleteBranchAfterMerge, ctx.Repo.Repository, pr)
 	if err != nil {
 		ctx.APIErrorInternal(err)
 		return
 	}
 
-	// for agit flow, we should not delete the agit reference after merge
-	// FIXME: old code has that comment above. Is that comment valid? What would go wrong if a agit branch is deleted after merge?
-	// * If a agit branch can be deleted after merge, then fix the comment and maybe other related code
-	// * If a agit branch should not be deleted, then we need to fix the logic and add more tests
-	deleteBranchAfterMerge := pr.Flow == issues_model.PullRequestFlowGithub
-	if form.DeleteBranchAfterMerge != nil {
-		// if the form field is defined, it takes precedence over the repo setting equivalent
-		deleteBranchAfterMerge = deleteBranchAfterMerge && *form.DeleteBranchAfterMerge
-	} else {
-		// otherwise, we look at the repo setting to make the determination
-		deleteBranchAfterMerge = deleteBranchAfterMerge && prUnit.PullRequestsConfig().DefaultDeleteBranchAfterMerge
-	}
-
 	if form.MergeWhenChecksSucceed {
 		scheduled, err := automerge.ScheduleAutoMerge(ctx, ctx.Doer, pr, repo_model.MergeStyle(form.Do), message, deleteBranchAfterMerge)
 		if err != nil {
@@ -1055,45 +1041,9 @@ func MergePullRequest(ctx *context.APIContext) {
 	log.Trace("Pull request merged: %d", pr.ID)
 
 	if deleteBranchAfterMerge {
-		// 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
-			}
+		// no way to tell users that what error happens, and the PR has been merged, so ignore the error
+		if err = repo_service.DeleteBranchAfterMerge(ctx, ctx.Doer, pr.ID, nil); err != nil {
+			log.Debug("DeleteBranchAfterMerge: pr %d, err: %v", pr.ID, err)
 		}
 	}
 

routers/web/repo/actions/view.go

@@ -887,8 +887,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 {