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

Backport #35488 by @kemzeb

Fix #35463.

---------

Co-authored-by: Kemal Zebari <60799661+kemzeb@users.noreply.github.com>
Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>
This commit is contained in:
Giteabot
2025-10-23 00:41:40 +08:00
committed by GitHub
parent c84d17b1bb
commit 0925089b5e
20 changed files with 335 additions and 265 deletions

View File

@@ -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 {

View File

@@ -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
}
func ErrorAsLocale(err error) *LocaleWrapper {
var e LocaleWrapper
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 ErrorAsTranslatable(err error) ErrorTranslatable {
var e *errorTranslatableWrapper
if errors.As(err, &e) {
return &e
return e
}
return nil
}

View File

@@ -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)
}

View File

@@ -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)
}
}

View File

@@ -850,8 +850,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)

View File

@@ -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)

View File

@@ -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 {

View File

@@ -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 {

View File

@@ -1095,11 +1095,17 @@ func MergePullRequest(ctx *context.Context) {
message += "\n\n" + form.MergeMessageField
}
deleteBranchAfterMerge, err := pull_service.ShouldDeleteBranchAfterMerge(ctx, form.DeleteBranchAfterMerge, ctx.Repo.Repository, pr)
if err != nil {
ctx.ServerError("ShouldDeleteBranchAfterMerge", err)
return
}
if form.MergeWhenChecksSucceed {
// delete all scheduled auto merges
_ = pull_model.DeleteScheduledAutoMerge(ctx, pr.ID)
// schedule auto merge
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 {
ctx.ServerError("ScheduleAutoMerge", err)
return
@@ -1185,35 +1191,27 @@ func MergePullRequest(ctx *context.Context) {
log.Trace("Pull request merged: %d", pr.ID)
if !form.DeleteBranchAfterMerge {
ctx.JSONRedirect(issue.Link())
if deleteBranchAfterMerge {
deleteBranchAfterMergeAndFlashMessage(ctx, pr.ID)
if ctx.Written() {
return
}
}
ctx.JSONRedirect(issue.Link())
}
// Don't cleanup when other pr use this branch as head branch
exist, err := issues_model.HasUnmergedPullRequestsByHeadInfo(ctx, pr.HeadRepoID, pr.HeadBranch)
if err != nil {
ctx.ServerError("HasUnmergedPullRequestsByHeadInfo", err)
func deleteBranchAfterMergeAndFlashMessage(ctx *context.Context, prID int64) {
var fullBranchName string
err := repo_service.DeleteBranchAfterMerge(ctx, ctx.Doer, prID, &fullBranchName)
if errTr := util.ErrorAsTranslatable(err); errTr != nil {
ctx.Flash.Error(errTr.Translate(ctx.Locale))
return
} else if err == nil {
ctx.Flash.Success(ctx.Tr("repo.branch.deletion_success", fullBranchName))
return
}
if exist {
ctx.JSONRedirect(issue.Link())
return
}
var headRepo *git.Repository
if ctx.Repo != nil && ctx.Repo.Repository != nil && pr.HeadRepoID == ctx.Repo.Repository.ID && ctx.Repo.GitRepo != nil {
headRepo = ctx.Repo.GitRepo
} else {
headRepo, err = gitrepo.OpenRepository(ctx, pr.HeadRepo)
if err != nil {
ctx.ServerError(fmt.Sprintf("OpenRepository[%s]", pr.HeadRepo.FullName()), err)
return
}
defer headRepo.Close()
}
deleteBranch(ctx, pr, headRepo)
ctx.JSONRedirect(issue.Link())
// catch unknown errors
ctx.ServerError("DeleteBranchAfterMerge", err)
}
// CancelAutoMergePullRequest cancels a scheduled pr
@@ -1402,131 +1400,17 @@ func CompareAndPullRequestPost(ctx *context.Context) {
}
// CleanUpPullRequest responses for delete merged branch when PR has been merged
// Used by "DeleteBranchLink" for "delete branch" button
func CleanUpPullRequest(ctx *context.Context) {
issue, ok := getPullInfo(ctx)
if !ok {
return
}
pr := issue.PullRequest
// Don't cleanup unmerged and unclosed PRs and agit PRs
if !pr.HasMerged && !issue.IsClosed && pr.Flow != issues_model.PullRequestFlowGithub {
ctx.NotFound(nil)
deleteBranchAfterMergeAndFlashMessage(ctx, issue.PullRequest.ID)
if ctx.Written() {
return
}
// 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.ServerError("HasUnmergedPullRequestsByHeadInfo", err)
return
}
if exist {
ctx.NotFound(nil)
return
}
if err := pr.LoadHeadRepo(ctx); err != nil {
ctx.ServerError("LoadHeadRepo", err)
return
} else if pr.HeadRepo == nil {
// Forked repository has already been deleted
ctx.NotFound(nil)
return
} else if err = pr.LoadBaseRepo(ctx); err != nil {
ctx.ServerError("LoadBaseRepo", err)
return
} else if err = pr.HeadRepo.LoadOwner(ctx); err != nil {
ctx.ServerError("HeadRepo.LoadOwner", err)
return
}
if err := repo_service.CanDeleteBranch(ctx, pr.HeadRepo, pr.HeadBranch, ctx.Doer); err != nil {
if errors.Is(err, util.ErrPermissionDenied) {
ctx.NotFound(nil)
} else {
ctx.ServerError("CanDeleteBranch", err)
}
return
}
fullBranchName := pr.HeadRepo.Owner.Name + "/" + pr.HeadBranch
var gitBaseRepo *git.Repository
// Assume that the base repo is the current context (almost certainly)
if ctx.Repo != nil && ctx.Repo.Repository != nil && ctx.Repo.Repository.ID == pr.BaseRepoID && ctx.Repo.GitRepo != nil {
gitBaseRepo = ctx.Repo.GitRepo
} else {
// If not just open it
gitBaseRepo, err = gitrepo.OpenRepository(ctx, pr.BaseRepo)
if err != nil {
ctx.ServerError(fmt.Sprintf("OpenRepository[%s]", pr.BaseRepo.FullName()), err)
return
}
defer gitBaseRepo.Close()
}
// Now assume that the head repo is the same as the base repo (reasonable chance)
gitRepo := gitBaseRepo
// But if not: is it the same as the context?
if pr.BaseRepoID != pr.HeadRepoID && ctx.Repo != nil && ctx.Repo.Repository != nil && ctx.Repo.Repository.ID == pr.HeadRepoID && ctx.Repo.GitRepo != nil {
gitRepo = ctx.Repo.GitRepo
} else if pr.BaseRepoID != pr.HeadRepoID {
// Otherwise just load it up
gitRepo, err = gitrepo.OpenRepository(ctx, pr.HeadRepo)
if err != nil {
ctx.ServerError(fmt.Sprintf("OpenRepository[%s]", pr.HeadRepo.FullName()), err)
return
}
defer gitRepo.Close()
}
defer func() {
ctx.JSONRedirect(issue.Link())
}()
// Check if branch has no new commits
headCommitID, err := gitBaseRepo.GetRefCommitID(pr.GetGitHeadRefName())
if err != nil {
log.Error("GetRefCommitID: %v", err)
ctx.Flash.Error(ctx.Tr("repo.branch.deletion_failed", fullBranchName))
return
}
branchCommitID, err := gitRepo.GetBranchCommitID(pr.HeadBranch)
if err != nil {
log.Error("GetBranchCommitID: %v", err)
ctx.Flash.Error(ctx.Tr("repo.branch.deletion_failed", fullBranchName))
return
}
if headCommitID != branchCommitID {
ctx.Flash.Error(ctx.Tr("repo.branch.delete_branch_has_new_commits", fullBranchName))
return
}
deleteBranch(ctx, pr, gitRepo)
}
func deleteBranch(ctx *context.Context, pr *issues_model.PullRequest, gitRepo *git.Repository) {
fullBranchName := pr.HeadRepo.FullName() + ":" + pr.HeadBranch
if err := repo_service.DeleteBranch(ctx, ctx.Doer, pr.HeadRepo, gitRepo, pr.HeadBranch, pr); err != nil {
switch {
case git.IsErrBranchNotExist(err):
ctx.Flash.Error(ctx.Tr("repo.branch.deletion_failed", fullBranchName))
case errors.Is(err, repo_service.ErrBranchIsDefault):
ctx.Flash.Error(ctx.Tr("repo.branch.deletion_failed", fullBranchName))
case errors.Is(err, git_model.ErrBranchIsProtected):
ctx.Flash.Error(ctx.Tr("repo.branch.deletion_failed", fullBranchName))
default:
log.Error("DeleteBranch: %v", err)
ctx.Flash.Error(ctx.Tr("repo.branch.deletion_failed", fullBranchName))
}
return
}
ctx.Flash.Success(ctx.Tr("repo.branch.deletion_success", fullBranchName))
}
// DownloadPullDiff render a pull's raw diff

View File

@@ -49,14 +49,14 @@ func EnableOrDisableWorkflow(ctx *context.APIContext, workflowID string, isEnabl
func DispatchActionWorkflow(ctx reqctx.RequestContext, doer *user_model.User, repo *repo_model.Repository, gitRepo *git.Repository, workflowID, ref string, processInputs func(model *model.WorkflowDispatch, inputs map[string]any) error) error {
if workflowID == "" {
return util.ErrorWrapLocale(
return util.ErrorWrapTranslatable(
util.NewNotExistErrorf("workflowID is empty"),
"actions.workflow.not_found", workflowID,
)
}
if ref == "" {
return util.ErrorWrapLocale(
return util.ErrorWrapTranslatable(
util.NewNotExistErrorf("ref is empty"),
"form.target_ref_not_exist", ref,
)
@@ -66,7 +66,7 @@ func DispatchActionWorkflow(ctx reqctx.RequestContext, doer *user_model.User, re
cfgUnit := repo.MustGetUnit(ctx, unit.TypeActions)
cfg := cfgUnit.ActionsConfig()
if cfg.IsWorkflowDisabled(workflowID) {
return util.ErrorWrapLocale(
return util.ErrorWrapTranslatable(
util.NewPermissionDeniedErrorf("workflow is disabled"),
"actions.workflow.disabled",
)
@@ -85,7 +85,7 @@ func DispatchActionWorkflow(ctx reqctx.RequestContext, doer *user_model.User, re
runTargetCommit, err = gitRepo.GetBranchCommit(ref)
}
if err != nil {
return util.ErrorWrapLocale(
return util.ErrorWrapTranslatable(
util.NewNotExistErrorf("ref %q doesn't exist", ref),
"form.target_ref_not_exist", ref,
)
@@ -126,7 +126,7 @@ func DispatchActionWorkflow(ctx reqctx.RequestContext, doer *user_model.User, re
}
if entry == nil {
return util.ErrorWrapLocale(
return util.ErrorWrapTranslatable(
util.NewNotExistErrorf("workflow %q doesn't exist", workflowID),
"actions.workflow.not_found", workflowID,
)
@@ -164,7 +164,7 @@ func DispatchActionWorkflow(ctx reqctx.RequestContext, doer *user_model.User, re
}
if len(workflows) == 0 {
return util.ErrorWrapLocale(
return util.ErrorWrapTranslatable(
util.NewNotExistErrorf("workflow %q doesn't exist", workflowID),
"actions.workflow.not_found", workflowID,
)

View File

@@ -205,18 +205,6 @@ func handlePullRequestAutoMerge(pullID int64, sha string) {
return
}
var headGitRepo *git.Repository
if pr.BaseRepoID == pr.HeadRepoID {
headGitRepo = baseGitRepo
} else {
headGitRepo, err = gitrepo.OpenRepository(ctx, pr.HeadRepo)
if err != nil {
log.Error("OpenRepository %-v: %v", pr.HeadRepo, err)
return
}
defer headGitRepo.Close()
}
switch pr.Flow {
case issues_model.PullRequestFlowGithub:
headBranchExist := pr.HeadRepo != nil && gitrepo.IsBranchExist(ctx, pr.HeadRepo, pr.HeadBranch)
@@ -276,9 +264,12 @@ func handlePullRequestAutoMerge(pullID int64, sha string) {
return
}
if pr.Flow == issues_model.PullRequestFlowGithub && scheduledPRM.DeleteBranchAfterMerge {
if err := repo_service.DeleteBranch(ctx, doer, pr.HeadRepo, headGitRepo, pr.HeadBranch, pr); err != nil {
log.Error("DeletePullRequestHeadBranch: %v", err)
deleteBranchAfterMerge, err := pull_service.ShouldDeleteBranchAfterMerge(ctx, &scheduledPRM.DeleteBranchAfterMerge, pr.BaseRepo, pr)
if err != nil {
log.Error("ShouldDeleteBranchAfterMerge: %v", err)
} else if deleteBranchAfterMerge {
if err = repo_service.DeleteBranchAfterMerge(ctx, doer, pr.ID, nil); err != nil {
log.Error("DeleteBranchAfterMerge: %v", err)
}
}
}

View File

@@ -540,7 +540,7 @@ type MergePullRequestForm struct {
HeadCommitID string `json:"head_commit_id,omitempty"`
ForceMerge bool `json:"force_merge,omitempty"`
MergeWhenChecksSucceed bool `json:"merge_when_checks_succeed,omitempty"`
DeleteBranchAfterMerge bool `json:"delete_branch_after_merge,omitempty"`
DeleteBranchAfterMerge *bool `json:"delete_branch_after_merge,omitempty"`
}
// Validate validates the fields

View File

@@ -730,3 +730,24 @@ func SetMerged(ctx context.Context, pr *issues_model.PullRequest, mergedCommitID
return true, nil
})
}
func ShouldDeleteBranchAfterMerge(ctx context.Context, userOption *bool, repo *repo_model.Repository, pr *issues_model.PullRequest) (bool, error) {
if pr.Flow != issues_model.PullRequestFlowGithub {
// only support GitHub workflow (branch-based)
// for agit workflow, there is no branch, so nothing to delete
// TODO: maybe in the future, it should delete the "agit reference (refs/for/xxxx)"?
return false, nil
}
// if user has set an option, respect it
if userOption != nil {
return *userOption, nil
}
// otherwise, use repository default
prUnit, err := repo.GetUnit(ctx, unit.TypePullRequests)
if err != nil {
return false, err
}
return prUnit.PullRequestsConfig().DefaultDeleteBranchAfterMerge, nil
}

View File

@@ -485,10 +485,7 @@ func RenameBranch(ctx context.Context, repo *repo_model.Repository, doer *user_m
return "", nil
}
// enmuerates all branch related errors
var (
ErrBranchIsDefault = errors.New("branch is default")
)
var ErrBranchIsDefault = util.ErrorWrap(util.ErrPermissionDenied, "branch is default")
func CanDeleteBranch(ctx context.Context, repo *repo_model.Repository, branchName string, doer *user_model.User) error {
if branchName == repo.DefaultBranch {
@@ -748,3 +745,89 @@ func GetBranchDivergingInfo(ctx reqctx.RequestContext, baseRepo *repo_model.Repo
info.BaseHasNewCommits = info.HeadCommitsBehind > 0
return info, nil
}
func DeleteBranchAfterMerge(ctx context.Context, doer *user_model.User, prID int64, outFullBranchName *string) error {
pr, err := issues_model.GetPullRequestByID(ctx, prID)
if err != nil {
return err
}
if err = pr.LoadIssue(ctx); err != nil {
return err
}
if err = pr.LoadBaseRepo(ctx); err != nil {
return err
}
if err := pr.LoadHeadRepo(ctx); err != nil {
return err
}
if pr.HeadRepo == nil {
// Forked repository has already been deleted
return util.ErrorWrapTranslatable(util.ErrNotExist, "repo.branch.deletion_failed", "(deleted-repo):"+pr.HeadBranch)
}
if err = pr.HeadRepo.LoadOwner(ctx); err != nil {
return err
}
fullBranchName := pr.HeadRepo.FullName() + ":" + pr.HeadBranch
if outFullBranchName != nil {
*outFullBranchName = fullBranchName
}
errFailedToDelete := func(err error) error {
return util.ErrorWrapTranslatable(err, "repo.branch.deletion_failed", fullBranchName)
}
// Don't clean up unmerged and unclosed PRs and agit PRs
if !pr.HasMerged && !pr.Issue.IsClosed && pr.Flow != issues_model.PullRequestFlowGithub {
return errFailedToDelete(util.ErrUnprocessableContent)
}
// Don't clean up 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 {
return err
}
if exist {
return errFailedToDelete(util.ErrUnprocessableContent)
}
if err := CanDeleteBranch(ctx, pr.HeadRepo, pr.HeadBranch, doer); err != nil {
if errors.Is(err, util.ErrPermissionDenied) {
return errFailedToDelete(err)
}
return err
}
gitBaseRepo, gitBaseCloser, err := gitrepo.RepositoryFromContextOrOpen(ctx, pr.BaseRepo)
if err != nil {
return err
}
defer gitBaseCloser.Close()
gitHeadRepo, gitHeadCloser, err := gitrepo.RepositoryFromContextOrOpen(ctx, pr.HeadRepo)
if err != nil {
return err
}
defer gitHeadCloser.Close()
// Check if branch has no new commits
headCommitID, err := gitBaseRepo.GetRefCommitID(pr.GetGitHeadRefName())
if err != nil {
log.Error("GetRefCommitID: %v", err)
return errFailedToDelete(err)
}
branchCommitID, err := gitHeadRepo.GetBranchCommitID(pr.HeadBranch)
if err != nil {
log.Error("GetBranchCommitID: %v", err)
return errFailedToDelete(err)
}
if headCommitID != branchCommitID {
return util.ErrorWrapTranslatable(util.ErrUnprocessableContent, "repo.branch.delete_branch_has_new_commits", fullBranchName)
}
err = DeleteBranch(ctx, doer, pr.HeadRepo, gitHeadRepo, pr.HeadBranch, pr)
if errors.Is(err, util.ErrPermissionDenied) || errors.Is(err, util.ErrNotExist) {
return errFailedToDelete(err)
}
return err
}

View File

@@ -51,7 +51,7 @@
</div>
</div>
<div class="item-section-right">
<button class="delete-button ui button" data-url="{{.DeleteBranchLink}}">{{ctx.Locale.Tr "repo.branch.delete_html"}}</button>
<button class="ui button link-action delete-branch-after-merge" data-url="{{.DeleteBranchLink}}">{{ctx.Locale.Tr "repo.branch.delete_html"}}</button>
</div>
</div>
{{end}}
@@ -69,7 +69,7 @@
</div>
{{if and .IsPullBranchDeletable (not .IsPullRequestBroken)}}
<div class="item-section-right">
<button class="delete-button ui button" data-url="{{.DeleteBranchLink}}">{{ctx.Locale.Tr "repo.branch.delete_html"}}</button>
<button class="ui button link-action delete-branch-after-merge" data-url="{{.DeleteBranchLink}}">{{ctx.Locale.Tr "repo.branch.delete_html"}}</button>
</div>
{{end}}
</div>

View File

@@ -129,7 +129,7 @@ func TestAPIRepoIssueConfigPaths(t *testing.T) {
configData, err := yaml.Marshal(configMap)
assert.NoError(t, err)
_, err = createFileInBranch(owner, repo, fullPath, repo.DefaultBranch, string(configData))
_, err = createFile(owner, repo, fullPath, string(configData))
assert.NoError(t, err)
issueConfig := getIssueConfig(t, owner.Name, repo.Name)

View File

@@ -17,19 +17,23 @@ import (
issues_model "code.gitea.io/gitea/models/issues"
"code.gitea.io/gitea/models/perm"
repo_model "code.gitea.io/gitea/models/repo"
unit_model "code.gitea.io/gitea/models/unit"
"code.gitea.io/gitea/models/unittest"
user_model "code.gitea.io/gitea/models/user"
"code.gitea.io/gitea/modules/setting"
api "code.gitea.io/gitea/modules/structs"
"code.gitea.io/gitea/modules/util"
"code.gitea.io/gitea/services/convert"
"code.gitea.io/gitea/services/forms"
"code.gitea.io/gitea/services/gitdiff"
issue_service "code.gitea.io/gitea/services/issue"
pull_service "code.gitea.io/gitea/services/pull"
repo_service "code.gitea.io/gitea/services/repository"
files_service "code.gitea.io/gitea/services/repository/files"
"code.gitea.io/gitea/tests"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
func TestAPIViewPulls(t *testing.T) {
@@ -186,6 +190,76 @@ func TestAPIMergePullWIP(t *testing.T) {
MakeRequest(t, req, http.StatusMethodNotAllowed)
}
func TestAPIMergePull(t *testing.T) {
onGiteaRun(t, func(t *testing.T, giteaURL *url.URL) {
repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1})
owner := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: repo.OwnerID})
apiCtx := NewAPITestContext(t, repo.OwnerName, repo.Name, auth_model.AccessTokenScopeWriteRepository)
checkBranchExists := func(t *testing.T, branchName string, status int) {
req := NewRequest(t, "GET", fmt.Sprintf("/api/v1/repos/%s/%s/branches/%s", owner.Name, repo.Name, branchName)).AddTokenAuth(apiCtx.Token)
MakeRequest(t, req, status)
}
createTestBranchPR := func(t *testing.T, branchName string) *api.PullRequest {
testCreateFileInBranch(t, owner, repo, createFileInBranchOptions{NewBranch: branchName}, map[string]string{"a-new-file-" + branchName + ".txt": "dummy content"})
prDTO, err := doAPICreatePullRequest(apiCtx, repo.OwnerName, repo.Name, repo.DefaultBranch, branchName)(t)
require.NoError(t, err)
return &prDTO
}
performMerge := func(t *testing.T, prIndex int64, params map[string]any, optExpectedStatus ...int) {
req := NewRequestWithJSON(t, http.MethodPost, fmt.Sprintf("/api/v1/repos/%s/%s/pulls/%d/merge", owner.Name, repo.Name, prIndex), params).AddTokenAuth(apiCtx.Token)
expectedStatus := util.OptionalArg(optExpectedStatus, http.StatusOK)
MakeRequest(t, req, expectedStatus)
}
t.Run("Normal", func(t *testing.T) {
newBranch := "test-pull-1"
prDTO := createTestBranchPR(t, newBranch)
performMerge(t, prDTO.Index, map[string]any{"do": "merge"})
checkBranchExists(t, newBranch, http.StatusOK)
// try to merge again, make sure we cannot perform a merge on the same PR
performMerge(t, prDTO.Index, map[string]any{"do": "merge"}, http.StatusMethodNotAllowed)
})
t.Run("DeleteBranchAfterMergePassedByFormField", func(t *testing.T) {
newBranch := "test-pull-2"
prDTO := createTestBranchPR(t, newBranch)
performMerge(t, prDTO.Index, map[string]any{"do": "merge", "delete_branch_after_merge": true})
checkBranchExists(t, newBranch, http.StatusNotFound)
})
updateRepoUnitDefaultDeleteBranchAfterMerge := func(t *testing.T, repo *repo_model.Repository, value bool) {
prUnit, err := repo.GetUnit(t.Context(), unit_model.TypePullRequests)
require.NoError(t, err)
prUnit.PullRequestsConfig().DefaultDeleteBranchAfterMerge = value
require.NoError(t, repo_service.UpdateRepositoryUnits(t.Context(), repo, []repo_model.RepoUnit{{
RepoID: repo.ID,
Type: unit_model.TypePullRequests,
Config: prUnit.PullRequestsConfig(),
}}, nil))
}
t.Run("DeleteBranchAfterMergePassedByRepoSettings", func(t *testing.T) {
newBranch := "test-pull-3"
prDTO := createTestBranchPR(t, newBranch)
updateRepoUnitDefaultDeleteBranchAfterMerge(t, repo, true)
performMerge(t, prDTO.Index, map[string]any{"do": "merge"})
checkBranchExists(t, newBranch, http.StatusNotFound)
})
t.Run("DeleteBranchAfterMergeFormFieldIsSetButNotRepoSettings", func(t *testing.T) {
newBranch := "test-pull-4"
prDTO := createTestBranchPR(t, newBranch)
updateRepoUnitDefaultDeleteBranchAfterMerge(t, repo, false)
performMerge(t, prDTO.Index, map[string]any{"do": "merge", "delete_branch_after_merge": true})
checkBranchExists(t, newBranch, http.StatusNotFound)
})
})
}
func TestAPICreatePullSuccess(t *testing.T) {
defer tests.PrepareTestEnv(t)()
repo10 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 10})

View File

@@ -133,7 +133,7 @@ func BenchmarkAPICreateFileSmall(b *testing.B) {
b.ResetTimer()
for n := 0; b.Loop(); n++ {
treePath := fmt.Sprintf("update/file%d.txt", n)
_, _ = createFileInBranch(user2, repo1, treePath, repo1.DefaultBranch, treePath)
_, _ = createFile(user2, repo1, treePath)
}
})
}
@@ -149,7 +149,7 @@ func BenchmarkAPICreateFileMedium(b *testing.B) {
for n := 0; b.Loop(); n++ {
treePath := fmt.Sprintf("update/file%d.txt", n)
copy(data, treePath)
_, _ = createFileInBranch(user2, repo1, treePath, repo1.DefaultBranch, treePath)
_, _ = createFile(user2, repo1, treePath)
}
})
}

View File

@@ -6,26 +6,36 @@ package integration
import (
"context"
"strings"
"testing"
repo_model "code.gitea.io/gitea/models/repo"
user_model "code.gitea.io/gitea/models/user"
api "code.gitea.io/gitea/modules/structs"
"code.gitea.io/gitea/modules/util"
files_service "code.gitea.io/gitea/services/repository/files"
"github.com/stretchr/testify/require"
)
func createFileInBranch(user *user_model.User, repo *repo_model.Repository, treePath, branchName, content string) (*api.FilesResponse, error) {
type createFileInBranchOptions struct {
OldBranch, NewBranch string
}
func testCreateFileInBranch(t *testing.T, user *user_model.User, repo *repo_model.Repository, createOpts createFileInBranchOptions, files map[string]string) *api.FilesResponse {
resp, err := createFileInBranch(user, repo, createOpts, files)
require.NoError(t, err)
return resp
}
func createFileInBranch(user *user_model.User, repo *repo_model.Repository, createOpts createFileInBranchOptions, files map[string]string) (*api.FilesResponse, error) {
ctx := context.TODO()
opts := &files_service.ChangeRepoFilesOptions{
Files: []*files_service.ChangeRepoFile{
{
opts := &files_service.ChangeRepoFilesOptions{OldBranch: createOpts.OldBranch, NewBranch: createOpts.NewBranch}
for path, content := range files {
opts.Files = append(opts.Files, &files_service.ChangeRepoFile{
Operation: "create",
TreePath: treePath,
TreePath: path,
ContentReader: strings.NewReader(content),
},
},
OldBranch: branchName,
Author: nil,
Committer: nil,
})
}
return files_service.ChangeRepoFiles(ctx, repo, user, opts)
}
@@ -53,10 +63,12 @@ func createOrReplaceFileInBranch(user *user_model.User, repo *repo_model.Reposit
return err
}
_, err = createFileInBranch(user, repo, treePath, branchName, content)
_, err = createFileInBranch(user, repo, createFileInBranchOptions{OldBranch: branchName}, map[string]string{treePath: content})
return err
}
func createFile(user *user_model.User, repo *repo_model.Repository, treePath string) (*api.FilesResponse, error) {
return createFileInBranch(user, repo, treePath, repo.DefaultBranch, "This is a NEW file")
// TODO: replace all usages of this function with testCreateFileInBranch or testCreateFile
func createFile(user *user_model.User, repo *repo_model.Repository, treePath string, optContent ...string) (*api.FilesResponse, error) {
content := util.OptionalArg(optContent, "This is a NEW file") // some tests need this default content because its SHA is hardcoded
return createFileInBranch(user, repo, createFileInBranchOptions{}, map[string]string{treePath: content})
}

View File

@@ -79,7 +79,7 @@ func testPullCleanUp(t *testing.T, session *TestSession, user, repo, pullnum str
// Click the little button to create a pull
htmlDoc := NewHTMLParser(t, resp.Body)
link, exists := htmlDoc.doc.Find(".timeline-item .delete-button").Attr("data-url")
link, exists := htmlDoc.doc.Find(".timeline-item .delete-branch-after-merge").Attr("data-url")
assert.True(t, exists, "The template has changed, can not find delete button url")
req = NewRequestWithValues(t, "POST", link, map[string]string{
"_csrf": htmlDoc.GetCSRF(),