We don't need to duplicated operation when delete branch

models/issues/pull_list.go

@@ -165,6 +165,23 @@ func (prs PullRequestList) getRepositoryIDs() []int64 {
 	return repoIDs.Values()
 }
 
+func (prs PullRequestList) SetBaseRepo(baseRepo *repo_model.Repository) {
+	for _, pr := range prs {
+		if pr.BaseRepo == nil {
+			pr.BaseRepo = baseRepo
+		}
+	}
+}
+
+func (prs PullRequestList) SetHeadRepo(headRepo *repo_model.Repository) {
+	for _, pr := range prs {
+		if pr.HeadRepo == nil {
+			pr.HeadRepo = headRepo
+			pr.isHeadRepoLoaded = true
+		}
+	}
+}
+
 func (prs PullRequestList) LoadRepositories(ctx context.Context) error {
 	repoIDs := prs.getRepositoryIDs()
 	reposMap := make(map[int64]*repo_model.Repository, len(repoIDs))

routers/api/v1/repo/pull.go

@@ -1085,13 +1085,6 @@ func MergePullRequest(ctx *context.APIContext) {
 				defer headRepo.Close()
 			}
 
-			// TODO: why only retarget same repository pull requests?
-			if setting.Repository.PullRequest.RetargetChildrenOnMerge && pr.BaseRepoID == pr.HeadRepoID {
-				if err := pull_service.RetargetBranchPulls(ctx, ctx.Doer, pr.HeadRepoID, pr.HeadBranch, pr.BaseBranch); err != nil {
-					ctx.Error(http.StatusInternalServerError, "RetargetBranchPulls", err)
-					return
-				}
-			}
 			if err := repo_service.DeleteBranch(ctx, ctx.Doer, pr.HeadRepo, headRepo, pr.HeadBranch, pr); err != nil {
 				switch {
 				case git.IsErrBranchNotExist(err):
@@ -1105,10 +1098,6 @@ func MergePullRequest(ctx *context.APIContext) {
 				}
 				return
 			}
-			if err := issues_model.AddDeletePRBranchComment(ctx, ctx.Doer, pr.BaseRepo, pr.Issue.ID, pr.HeadBranch); err != nil {
-				// Do not fail here as branch has already been deleted
-				log.Error("DeleteBranch: %v", err)
-			}
 		}
 	}
 

services/pull/pull.go

@@ -631,9 +631,9 @@ func (errs errlist) Error() string {
 
 var _ error = &errlist{}
 
-// RetargetBranchPulls change target branch for all pull requests whose base branch is the branch
+// retargetBranchPulls change target branch for all pull requests whose base branch is the branch
 // Both branch and targetBranch must be in the same repo (for security reasons)
-func RetargetBranchPulls(ctx context.Context, doer *user_model.User, repoID int64, branch, targetBranch string) error {
+func retargetBranchPulls(ctx context.Context, doer *user_model.User, repoID int64, branch, targetBranch string) error {
 	prs, err := issues_model.GetUnmergedPullRequestsByBaseInfo(ctx, repoID, branch)
 	if err != nil {
 		return err
@@ -660,11 +660,12 @@ func RetargetBranchPulls(ctx context.Context, doer *user_model.User, repoID int6
 	return nil
 }
 
-// ClosePullsCausedByBranchDeleted close all the pull requests who's head branch is the branch
+// AdjustPullsCausedByBranchDeleted close all the pull requests who's head branch is the branch
-// Or who's base branch is the branch if setting.Repository.PullRequest.RetargetChildrenOnMerge is true
+// Or Close all the plls who's base branch is the branch if setting.Repository.PullRequest.RetargetChildrenOnMerge is false.
-func ClosePullsCausedByBranchDeleted(ctx context.Context, doer *user_model.User, repoID int64, branch string) error {
+// If it's true, Retarget all these pulls to the default branch.
+func AdjustPullsCausedByBranchDeleted(ctx context.Context, doer *user_model.User, repo *repo_model.Repository, branch string) error {
 	// branch as head branch
-	prs, err := issues_model.GetUnmergedPullRequestsByHeadInfo(ctx, repoID, branch)
+	prs, err := issues_model.GetUnmergedPullRequestsByHeadInfo(ctx, repo.ID, branch)
 	if err != nil {
 		return err
 	}
@@ -672,6 +673,7 @@ func ClosePullsCausedByBranchDeleted(ctx context.Context, doer *user_model.User,
 	if err := issues_model.PullRequestList(prs).LoadAttributes(ctx); err != nil {
 		return err
 	}
+	issues_model.PullRequestList(prs).SetHeadRepo(repo)
 	if err := issues_model.PullRequestList(prs).LoadRepositories(ctx); err != nil {
 		return err
 	}
@@ -690,11 +692,15 @@ func ClosePullsCausedByBranchDeleted(ctx context.Context, doer *user_model.User,
 	}
 
 	if setting.Repository.PullRequest.RetargetChildrenOnMerge {
+		if err := retargetBranchPulls(ctx, doer, repo.ID, branch, repo.DefaultBranch); err != nil {
+			log.Error("retargetBranchPulls failed: %v", err)
+			errs = append(errs, err)
+		}
 		return errs
 	}
 
 	// branch as base branch
-	prs, err = issues_model.GetUnmergedPullRequestsByBaseInfo(ctx, repoID, branch)
+	prs, err = issues_model.GetUnmergedPullRequestsByBaseInfo(ctx, repo.ID, branch)
 	if err != nil {
 		return err
 	}
@@ -702,6 +708,7 @@ func ClosePullsCausedByBranchDeleted(ctx context.Context, doer *user_model.User,
 	if err := issues_model.PullRequestList(prs).LoadAttributes(ctx); err != nil {
 		return err
 	}
+	issues_model.PullRequestList(prs).SetBaseRepo(repo)
 	if err := issues_model.PullRequestList(prs).LoadRepositories(ctx); err != nil {
 		return err
 	}

services/repository/push.go

@@ -276,13 +276,7 @@ func pushUpdates(optsList []*repo_module.PushUpdateOptions) error {
 			} else {
 				notify_service.DeleteRef(ctx, pusher, repo, opts.RefFullName)
 
-				if setting.Repository.PullRequest.RetargetChildrenOnMerge {
+				if err := pull_service.AdjustPullsCausedByBranchDeleted(ctx, pusher, repo, branch); err != nil {
-					if err := pull_service.RetargetBranchPulls(ctx, pusher, repo.ID, branch, repo.DefaultBranch); err != nil {
-						log.Error("retargetBranchPulls failed: %v", err)
-					}
-				}
-
-				if err := pull_service.ClosePullsCausedByBranchDeleted(ctx, pusher, repo.ID, branch); err != nil {
 					// close all related pulls
 					log.Error("close related pull request failed: %v", err)
 				}