Cleanup error handling, add docs

Author: kvaster

custom/conf/app.example.ini

@@ -1062,6 +1062,9 @@ ROUTER = console
 ;;
 ;; In addition to testing patches using the three-way merge method, re-test conflicting patches with git apply
 ;TEST_CONFLICTING_PATCHES_WITH_GIT_APPLY = false
+;;
+;; Retarget the child pull request to the parent pull request branch target
+;RETARGET_CHILDREN_ON_MERGE = false
 
 ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
 ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;

docs/content/doc/administration/config-cheat-sheet.en-us.md

@@ -137,6 +137,7 @@ In addition there is _`StaticRootPath`_ which can be set as a built-in at build
 - `POPULATE_SQUASH_COMMENT_WITH_COMMIT_MESSAGES`: **false**: In default squash-merge messages include the commit message of all commits comprising the pull request.
 - `ADD_CO_COMMITTER_TRAILERS`: **true**: Add co-authored-by and co-committed-by trailers to merge commit messages if committer does not match author.
 - `TEST_CONFLICTING_PATCHES_WITH_GIT_APPLY`: **false**: PR patches are tested using a three-way merge method to discover if there are conflicts. If this setting is set to **true**, conflicting patches will be retested using `git apply` - This was the previous behaviour in 1.18 (and earlier) but is somewhat inefficient. Please report if you find that this setting is required.
+- `RETARGET_CHILDREN_ON_MERGE`: **false**: Retarget the child pull request to the parent pull request branch target.
 
 ### Repository - Issue (`repository.issue`)
 

modules/setting/repository.go

@@ -84,7 +84,7 @@ var (
 			PopulateSquashCommentWithCommitMessages  bool
 			AddCoCommitterTrailers                   bool
 			TestConflictingPatchesWithGitApply       bool
-			RetargetChildsOnClose                    bool
+			RetargetChildrenOnMerge                  bool
 		} `ini:"repository.pull-request"`
 
 		// Issue Setting
@@ -208,7 +208,7 @@ var (
 			PopulateSquashCommentWithCommitMessages  bool
 			AddCoCommitterTrailers                   bool
 			TestConflictingPatchesWithGitApply       bool
-			RetargetChildsOnClose                    bool
+			RetargetChildrenOnMerge                  bool
 		}{
 			WorkInProgressPrefixes: []string{"WIP:", "[WIP]"},
 			// Same as GitHub. See
@@ -223,6 +223,7 @@ var (
 			DefaultMergeMessageOfficialApproversOnly: true,
 			PopulateSquashCommentWithCommitMessages:  false,
 			AddCoCommitterTrailers:                   true,
+			RetargetChildrenOnMerge:                  false,
 		},
 
 		// Issue settings

routers/api/v1/repo/pull.go

@@ -904,12 +904,10 @@ func MergePullRequest(ctx *context.APIContext) {
 			}
 			defer headRepo.Close()
 		}
-		if setting.Repository.PullRequest.RetargetChildsOnClose && pr.BaseRepoID == pr.HeadRepoID {
-			if err := pull_service.RebaseBranchPulls(ctx, ctx.Doer, pr.HeadRepoID, pr.HeadBranch, pr.BaseBranch); err != nil {
-				log.Error("RebaseBranchPulls: %v", err)
-				ctx.Error(http.StatusInternalServerError, "RebaseBranchPulls", err)
-				return
-			}
+		if err := pull_service.RetargetChildrenOnMerge(ctx, ctx.Doer, pr); err != nil {
+			log.Error("RetargetChildrenOnMerge: %v", err)
+			ctx.Error(http.StatusInternalServerError, "RetargetChildrenOnMerge", err)
+			return
 		}
 		if err := repo_service.DeleteBranch(ctx, ctx.Doer, pr.HeadRepo, headRepo, pr.HeadBranch); err != nil {
 			switch {

routers/web/repo/pull.go

@@ -1400,12 +1400,10 @@ func CleanUpPullRequest(ctx *context.Context) {
 func deleteBranch(ctx *context.Context, pr *issues_model.PullRequest, gitRepo *git.Repository) {
 	fullBranchName := pr.HeadRepo.FullName() + ":" + pr.HeadBranch
 
-	if setting.Repository.PullRequest.RetargetChildsOnClose && pr.BaseRepoID == pr.HeadRepoID {
-		if err := pull_service.RebaseBranchPulls(ctx, ctx.Doer, pr.HeadRepoID, pr.HeadBranch, pr.BaseBranch); err != nil {
-			log.Error("RebaseBranchPulls: %v", err)
-			ctx.Flash.Error(ctx.Tr("repo.branch.deletion_failed", fullBranchName))
-			return
-		}
+	if err := pull_service.RetargetChildrenOnMerge(ctx, ctx.Doer, pr); err != nil {
+		log.Error("RetargetChildrenOnMerge: %v", err)
+		ctx.Flash.Error(ctx.Tr("repo.branch.deletion_failed", fullBranchName))
+		return
 	}
 
 	if err := repo_service.DeleteBranch(ctx, ctx.Doer, pr.HeadRepo, gitRepo, pr.HeadBranch); err != nil {

services/pull/pull.go

@@ -497,24 +497,32 @@ func (errs errlist) Error() string {
 	return ""
 }
 
-// RebaseBranchPulls change target branch for all pull requests who's base branch is the branch
-func RebaseBranchPulls(ctx context.Context, doer *user_model.User, repoID int64, branch, targetBranch string) error {
-	prs, err := models.GetUnmergedPullRequestsByBaseInfo(repoID, branch)
+// RetargetChildrenOnMerge retarget children pull requests on merge if possible
+func RetargetChildrenOnMerge(ctx context.Context, doer *user_model.User, pr *issues_model.PullRequest) error {
+	if setting.Repository.PullRequest.RetargetChildrenOnMerge && pr.BaseRepoID == pr.HeadRepoID {
+		return RetargetBranchPulls(ctx, doer, pr.HeadRepoID, pr.HeadBranch, pr.BaseBranch)
+	}
+	return nil
+}
+
+// RetargetBranchPulls change target branch for all pull requests who's base branch is the branch
+func RetargetBranchPulls(ctx context.Context, doer *user_model.User, repoID int64, branch, targetBranch string) error {
+	prs, err := issues_model.GetUnmergedPullRequestsByBaseInfo(repoID, branch)
 	if err != nil {
 		return err
 	}
 
-	if err := models.PullRequestList(prs).LoadAttributes(); err != nil {
+	if err := issues_model.PullRequestList(prs).LoadAttributes(); err != nil {
 		return err
 	}
 
 	var errs errlist
 	for _, pr := range prs {
-		if err = pr.Issue.LoadAttributes(); err != nil {
+		if err = pr.Issue.LoadAttributes(ctx); err != nil {
 			errs = append(errs, err)
 		} else if err = ChangeTargetBranch(ctx, pr, doer, targetBranch); err != nil &&
-			!models.IsErrIssueIsClosed(err) && !models.IsErrPullRequestHasMerged(err) &&
-			!models.IsErrPullRequestAlreadyExists(err) {
+			!issues_model.IsErrIssueIsClosed(err) && !models.IsErrPullRequestHasMerged(err) &&
+			!issues_model.IsErrPullRequestAlreadyExists(err) {
 			errs = append(errs, err)
 		}
 	}