Give form field higher precedence then repo setting equivalent

Author: kemzeb

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)
@@ -1042,8 +1043,14 @@ func MergePullRequest(ctx *context.APIContext) {
 	}
 
 	// for agit flow, we should not delete the agit reference after merge
-	shouldDeleteBranch := (prUnit.PullRequestsConfig().DefaultDeleteBranchAfterMerge || 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

routers/web/repo/pull.go

@@ -30,6 +30,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"
@@ -1099,7 +1100,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
@@ -1185,7 +1186,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

@@ -219,10 +219,11 @@ func TestAPIMergePull(t *testing.T) {
 		t.Run("DeleteBranchAfterMergePassedByFormField", func(t *testing.T) {
 			newBranch := "test-pull-2"
 			prResp := creatPullRequestWithCommit(t, owner, token, newBranch, repo)
+			deleteBranch := true
 
 			req := NewRequestWithJSON(t, http.MethodPost, fmt.Sprintf("/api/v1/repos/%s/%s/pulls/%d/merge", owner.Name, repo.Name, prResp.Index), &forms.MergePullRequestForm{
 				Do:                     "merge",
-				DeleteBranchAfterMerge: true,
+				DeleteBranchAfterMerge: &deleteBranch,
 			}).AddTokenAuth(token)
 
 			MakeRequest(t, req, http.StatusOK)
@@ -256,6 +257,36 @@ func TestAPIMergePull(t *testing.T) {
 			MakeRequest(t, req, http.StatusOK)
 			doCheckBranchExists(owner, token, newBranch, repo, http.StatusNotFound)
 		})
+
+		t.Run("DeleteBranchAfterMergeFormFieldIsSetButNotRepoSettings", func(t *testing.T) {
+			newBranch := "test-pull-4"
+			prResp := creatPullRequestWithCommit(t, owner, token, 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, prResp.Index), &forms.MergePullRequestForm{
+				Do:                     "merge",
+				DeleteBranchAfterMerge: &deleteBranch,
+			}).AddTokenAuth(token)
+
+			MakeRequest(t, req, http.StatusOK)
+			doCheckBranchExists(owner, token, newBranch, repo, http.StatusNotFound)
+		})
 	})
 }