Honor delete branch on merge repo setting when using merge API

routers/api/v1/repo/pull.go

@@ -25,6 +25,7 @@ import (
 	"code.gitea.io/gitea/modules/gitrepo"
 	"code.gitea.io/gitea/modules/graceful"
 	"code.gitea.io/gitea/modules/log"
+	"code.gitea.io/gitea/modules/optional"
 	"code.gitea.io/gitea/modules/setting"
 	api "code.gitea.io/gitea/modules/structs"
 	"code.gitea.io/gitea/modules/timeutil"
@@ -990,7 +991,7 @@ func MergePullRequest(ctx *context.APIContext) {
 	}
 
 	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, optional.FromPtr(form.DeleteBranchAfterMerge).ValueOrDefault(false))
 		if err != nil {
 			if pull_model.IsErrAlreadyScheduledToAutoMerge(err) {
 				ctx.APIError(http.StatusConflict, err)
@@ -1035,8 +1036,23 @@ func MergePullRequest(ctx *context.APIContext) {
 	}
 	log.Trace("Pull request merged: %d", pr.ID)
 
+	prUnit, err := ctx.Repo.Repository.GetUnit(ctx, unit.TypePullRequests)
+	if err != nil {
+		ctx.APIErrorInternal(err)
+		return
+	}
+
 	// for agit flow, we should not delete the agit reference after merge
-	if form.DeleteBranchAfterMerge && pr.Flow == issues_model.PullRequestFlowGithub {
+	shouldDeleteBranch := pr.Flow == issues_model.PullRequestFlowGithub
+	if form.DeleteBranchAfterMerge != nil {
+		// if the form field is defined, it takes precedence over the repo setting equivalent
+		shouldDeleteBranch = shouldDeleteBranch && *form.DeleteBranchAfterMerge
+	} else {
+		// otherwise, we look at the repo setting to make the determination
+		shouldDeleteBranch = shouldDeleteBranch && prUnit.PullRequestsConfig().DefaultDeleteBranchAfterMerge
+	}
+
+	if shouldDeleteBranch {
 		// 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 {

routers/web/repo/pull.go

@@ -32,6 +32,7 @@ import (
 	"code.gitea.io/gitea/modules/graceful"
 	issue_template "code.gitea.io/gitea/modules/issue/template"
 	"code.gitea.io/gitea/modules/log"
+	"code.gitea.io/gitea/modules/optional"
 	"code.gitea.io/gitea/modules/setting"
 	"code.gitea.io/gitea/modules/templates"
 	"code.gitea.io/gitea/modules/util"
@@ -1106,7 +1107,7 @@ func MergePullRequest(ctx *context.Context) {
 		// 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, optional.FromPtr(form.DeleteBranchAfterMerge).ValueOrDefault(false))
 		if err != nil {
 			ctx.ServerError("ScheduleAutoMerge", err)
 			return
@@ -1192,7 +1193,7 @@ func MergePullRequest(ctx *context.Context) {
 
 	log.Trace("Pull request merged: %d", pr.ID)
 
-	if !form.DeleteBranchAfterMerge {
+	if !optional.FromPtr(form.DeleteBranchAfterMerge).ValueOrDefault(false) {
 		ctx.JSONRedirect(issue.Link())
 		return
 	}

services/forms/repo_form.go

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

tests/integration/api_pull_test.go

@@ -17,6 +17,7 @@ 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"
@@ -26,10 +27,12 @@ import (
 	"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 +189,107 @@ func TestAPIMergePullWIP(t *testing.T) {
 	MakeRequest(t, req, http.StatusMethodNotAllowed)
 }
 
+func TestAPIMergePull(t *testing.T) {
+	doCheckBranchExists := func(t *testing.T, user *user_model.User, token, branchName string, repo *repo_model.Repository, status int) {
+		req := NewRequest(t, "GET", fmt.Sprintf("/api/v1/repos/%s/%s/branches/%s", user.Name, repo.Name, branchName)).AddTokenAuth(token)
+		MakeRequest(t, req, status)
+	}
+
+	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})
+		session := loginUser(t, owner.Name)
+		token := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeWriteRepository)
+
+		t.Run("Normal", func(t *testing.T) {
+			newBranch := "test-pull-1"
+			prDTO := createPullRequestWithCommit(t, owner, newBranch, repo)
+
+			req := NewRequestWithJSON(t, http.MethodPost, fmt.Sprintf("/api/v1/repos/%s/%s/pulls/%d/merge", owner.Name, repo.Name, prDTO.Index), &forms.MergePullRequestForm{
+				Do: "merge",
+			}).AddTokenAuth(token)
+
+			MakeRequest(t, req, http.StatusOK)
+			doCheckBranchExists(t, owner, token, newBranch, repo, http.StatusOK)
+			// make sure we cannot perform a merge on the same PR
+			MakeRequest(t, req, http.StatusUnprocessableEntity)
+			doCheckBranchExists(t, owner, token, newBranch, repo, http.StatusOK)
+		})
+
+		t.Run("DeleteBranchAfterMergePassedByFormField", func(t *testing.T) {
+			newBranch := "test-pull-2"
+			prDTO := createPullRequestWithCommit(t, owner, newBranch, repo)
+			deleteBranch := true
+
+			req := NewRequestWithJSON(t, http.MethodPost, fmt.Sprintf("/api/v1/repos/%s/%s/pulls/%d/merge", owner.Name, repo.Name, prDTO.Index), &forms.MergePullRequestForm{
+				Do:                     "merge",
+				DeleteBranchAfterMerge: &deleteBranch,
+			}).AddTokenAuth(token)
+
+			MakeRequest(t, req, http.StatusOK)
+			doCheckBranchExists(t, owner, token, newBranch, repo, http.StatusNotFound)
+		})
+
+		t.Run("DeleteBranchAfterMergePassedByRepoSettings", func(t *testing.T) {
+			newBranch := "test-pull-3"
+			prDTO := createPullRequestWithCommit(t, owner, newBranch, repo)
+
+			// set the default branch after merge setting at the repo level
+			prUnit, err := repo.GetUnit(t.Context(), unit_model.TypePullRequests)
+			require.NoError(t, err)
+
+			prConfig := prUnit.PullRequestsConfig()
+			prConfig.DefaultDeleteBranchAfterMerge = true
+
+			var units []repo_model.RepoUnit
+			units = append(units, repo_model.RepoUnit{
+				RepoID: repo.ID,
+				Type:   unit_model.TypePullRequests,
+				Config: prConfig,
+			})
+			require.NoError(t, repo_service.UpdateRepositoryUnits(t.Context(), repo, units, nil))
+
+			// perform merge
+			req := NewRequestWithJSON(t, http.MethodPost, fmt.Sprintf("/api/v1/repos/%s/%s/pulls/%d/merge", owner.Name, repo.Name, prDTO.Index), &forms.MergePullRequestForm{
+				Do: "merge",
+			}).AddTokenAuth(token)
+
+			MakeRequest(t, req, http.StatusOK)
+			doCheckBranchExists(t, owner, token, newBranch, repo, http.StatusNotFound)
+		})
+
+		t.Run("DeleteBranchAfterMergeFormFieldIsSetButNotRepoSettings", func(t *testing.T) {
+			newBranch := "test-pull-4"
+			prDTO := createPullRequestWithCommit(t, owner, newBranch, repo)
+
+			// make sure the default branch after merge setting is unset at the repo level
+			prUnit, err := repo.GetUnit(t.Context(), unit_model.TypePullRequests)
+			require.NoError(t, err)
+
+			prConfig := prUnit.PullRequestsConfig()
+			prConfig.DefaultDeleteBranchAfterMerge = false
+
+			var units []repo_model.RepoUnit
+			units = append(units, repo_model.RepoUnit{
+				RepoID: repo.ID,
+				Type:   unit_model.TypePullRequests,
+				Config: prConfig,
+			})
+			require.NoError(t, repo_service.UpdateRepositoryUnits(t.Context(), repo, units, nil))
+
+			// perform merge
+			deleteBranch := true
+			req := NewRequestWithJSON(t, http.MethodPost, fmt.Sprintf("/api/v1/repos/%s/%s/pulls/%d/merge", owner.Name, repo.Name, prDTO.Index), &forms.MergePullRequestForm{
+				Do:                     "merge",
+				DeleteBranchAfterMerge: &deleteBranch,
+			}).AddTokenAuth(token)
+
+			MakeRequest(t, req, http.StatusOK)
+			doCheckBranchExists(t, owner, token, newBranch, repo, http.StatusNotFound)
+		})
+	})
+}
+
 func TestAPICreatePullSuccess(t *testing.T) {
 	defer tests.PrepareTestEnv(t)()
 	repo10 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 10})
@@ -520,3 +624,41 @@ func TestAPIViewPullFilesWithHeadRepoDeleted(t *testing.T) {
 		})(t)
 	})
 }
+
+func createPullRequestWithCommit(t *testing.T, user *user_model.User, newBranch string, repo *repo_model.Repository) *api.PullRequest {
+	// create a new branch with a commit
+	filesResp, err := files_service.ChangeRepoFiles(t.Context(), repo, user, &files_service.ChangeRepoFilesOptions{
+		Files: []*files_service.ChangeRepoFile{
+			{
+				Operation:     "create",
+				TreePath:      strings.ReplaceAll(newBranch, " ", "_"),
+				ContentReader: strings.NewReader("This is a test."),
+			},
+		},
+		Message:   "Add test file using branch name as its name",
+		OldBranch: repo.DefaultBranch,
+		NewBranch: newBranch,
+		Author: &files_service.IdentityOptions{
+			GitUserName:  user.Name,
+			GitUserEmail: user.Email,
+		},
+		Committer: &files_service.IdentityOptions{
+			GitUserName:  user.Name,
+			GitUserEmail: user.Email,
+		},
+		Dates: &files_service.CommitDateOptions{
+			Author:    time.Now(),
+			Committer: time.Now(),
+		},
+	})
+
+	require.NoError(t, err)
+	require.NotEmpty(t, filesResp)
+
+	// create a corresponding PR
+	apiCtx := NewAPITestContext(t, repo.OwnerName, repo.Name, auth_model.AccessTokenScopeWriteRepository)
+	prDTO, err := doAPICreatePullRequest(apiCtx, repo.OwnerName, repo.Name, repo.DefaultBranch, newBranch)(t)
+	require.NoError(t, err)
+
+	return &prDTO
+}