Retarget depending pulls when the parent branch is deleted

custom/conf/app.example.ini

@@ -1067,6 +1067,9 @@ LEVEL = Info
 ;;
 ;; 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 child pull requests to the parent pull request branch target on merge of parent pull request
+;RETARGET_CHILDREN_ON_MERGE = true
 
 ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
 ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;

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

@@ -135,6 +135,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`: **true**: Retarget child pull requests to the parent pull request branch target on merge of parent pull request.
 
 ### Repository - Issue (`repository.issue`)
 

modules/setting/repository.go

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

routers/api/v1/repo/pull.go

@@ -913,6 +913,10 @@ func MergePullRequest(ctx *context.APIContext) {
  }
  defer headRepo.Close()
  }
+ if err := pull_service.RetargetChildrenOnMerge(ctx, ctx.Doer, pr); err != nil {
+ ctx.Error(http.StatusInternalServerError, "RetargetChildrenOnMerge", err)
+ return
+ }
  if err := repo_service.DeleteBranch(ctx, ctx.Doer, pr.HeadRepo, headRepo, pr.HeadBranch); err != nil {
  switch {
  case git.IsErrBranchNotExist(err):

routers/web/repo/pull.go

@@ -1587,6 +1587,12 @@ func CleanUpPullRequest(ctx *context.Context) {
 
 func deleteBranch(ctx *context.Context, pr *issues_model.PullRequest, gitRepo *git.Repository) {
  fullBranchName := pr.HeadRepo.FullName() + ":" + pr.HeadBranch
+
+ if err := pull_service.RetargetChildrenOnMerge(ctx, ctx.Doer, pr); err != nil {
+ 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 {
  switch {
  case git.IsErrBranchNotExist(err):

services/pull/pull.go

@@ -546,6 +546,43 @@ func (errs errlist) Error() string {
  return ""
 }
 
+// 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 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 {
+ prs, err := issues_model.GetUnmergedPullRequestsByBaseInfo(ctx, repoID, branch)
+ if err != nil {
+ return err
+ }
+
+ if err := issues_model.PullRequestList(prs).LoadAttributes(ctx); err != nil {
+ return err
+ }
+
+ var errs errlist
+ for _, pr := range prs {
+ if err = pr.Issue.LoadRepo(ctx); err != nil {
+ errs = append(errs, err)
+ } else if err = ChangeTargetBranch(ctx, pr, doer, targetBranch); err != nil &&
+ !issues_model.IsErrIssueIsClosed(err) && !models.IsErrPullRequestHasMerged(err) &&
+ !issues_model.IsErrPullRequestAlreadyExists(err) {
+ errs = append(errs, err)
+ }
+ }
+
+ if len(errs) > 0 {
+ return errs
+ }
+ return nil
+}
+
 // CloseBranchPulls close all the pull requests who's head branch is the branch
 func CloseBranchPulls(ctx context.Context, doer *user_model.User, repoID int64, branch string) error {
  prs, err := issues_model.GetUnmergedPullRequestsByHeadInfo(ctx, repoID, branch)

tests/integration/pull_create_test.go

@@ -17,16 +17,23 @@ import (
  "github.com/stretchr/testify/assert"
 )
 
-func testPullCreate(t *testing.T, session *TestSession, user, repo, branch, title string) *httptest.ResponseRecorder {
+func testPullCreate(t *testing.T, session *TestSession, user, repo, targetBranch, sourceBranch, title string) *httptest.ResponseRecorder {
  req := NewRequest(t, "GET", path.Join(user, repo))
  resp := session.MakeRequest(t, req, http.StatusOK)
 
  // Click the PR button to create a pull
  htmlDoc := NewHTMLParser(t, resp.Body)
  link, exists := htmlDoc.doc.Find("#new-pull-request").Attr("href")
  assert.True(t, exists, "The template has changed")
- if branch != "master" {
- link = strings.Replace(link, ":master", ":"+branch, 1)
+ if targetBranch != "master" {
+ link = strings.Replace(link, "master...", targetBranch+"...", 1)
+ }
+ if sourceBranch != "master" {
+ if strings.Split(link, "/")[1] == user {
+ link = strings.Replace(link, "...master", "..."+sourceBranch, 1)
+ } else {
+ link = strings.Replace(link, ":master", ":"+sourceBranch, 1)
+ }
  }
 
  req = NewRequest(t, "GET", link)
@@ -49,7 +56,7 @@ func TestPullCreate(t *testing.T) {
  session := loginUser(t, "user1")
  testRepoFork(t, session, "user2", "repo1", "user1", "repo1")
  testEditFile(t, session, "user1", "repo1", "master", "README.md", "Hello, World (Edited)\n")
- resp := testPullCreate(t, session, "user1", "repo1", "master", "This is a pull title")
+ resp := testPullCreate(t, session, "user1", "repo1", "master", "master", "This is a pull title")
 
  // check the redirected URL
  url := test.RedirectURL(resp)
@@ -77,7 +84,7 @@ func TestPullCreate_TitleEscape(t *testing.T) {
  session := loginUser(t, "user1")
  testRepoFork(t, session, "user2", "repo1", "user1", "repo1")
  testEditFile(t, session, "user1", "repo1", "master", "README.md", "Hello, World (Edited)\n")
- resp := testPullCreate(t, session, "user1", "repo1", "master", "<i>XSS PR</i>")
+ resp := testPullCreate(t, session, "user1", "repo1", "master", "master", "<i>XSS PR</i>")
 
  // check the redirected URL
  url := test.RedirectURL(resp)
@@ -142,7 +149,7 @@ func TestPullBranchDelete(t *testing.T) {
  testRepoFork(t, session, "user2", "repo1", "user1", "repo1")
  testCreateBranch(t, session, "user1", "repo1", "branch/master", "master1", http.StatusSeeOther)
  testEditFile(t, session, "user1", "repo1", "master1", "README.md", "Hello, World (Edited)\n")
- resp := testPullCreate(t, session, "user1", "repo1", "master1", "This is a pull title")
+ resp := testPullCreate(t, session, "user1", "repo1", "master", "master1", "This is a pull title")
 
  // check the redirected URL
  url := test.RedirectURL(resp)

tests/integration/pull_merge_test.go

@@ -35,16 +35,23 @@ import (
  "github.com/stretchr/testify/assert"
 )
 
-func testPullMerge(t *testing.T, session *TestSession, user, repo, pullnum string, mergeStyle repo_model.MergeStyle) *httptest.ResponseRecorder {
+func testPullMerge(t *testing.T, session *TestSession, user, repo, pullnum string, mergeStyle repo_model.MergeStyle, deleteBranch bool) *httptest.ResponseRecorder {
  req := NewRequest(t, "GET", path.Join(user, repo, "pulls", pullnum))
  resp := session.MakeRequest(t, req, http.StatusOK)
 
  htmlDoc := NewHTMLParser(t, resp.Body)
  link := path.Join(user, repo, "pulls", pullnum, "merge")
- req = NewRequestWithValues(t, "POST", link, map[string]string{
+
+ options := map[string]string{
  "_csrf": htmlDoc.GetCSRF(),
  "do": string(mergeStyle),
- })
+ }
+
+ if deleteBranch {
+ options["delete_branch_after_merge"] = "on"
+ }
+
+ req = NewRequestWithValues(t, "POST", link, options)
  resp = session.MakeRequest(t, req, http.StatusOK)
 
  respJSON := struct {
@@ -83,11 +90,11 @@ func TestPullMerge(t *testing.T) {
  testRepoFork(t, session, "user2", "repo1", "user1", "repo1")
  testEditFile(t, session, "user1", "repo1", "master", "README.md", "Hello, World (Edited)\n")
 
- resp := testPullCreate(t, session, "user1", "repo1", "master", "This is a pull title")
+ resp := testPullCreate(t, session, "user1", "repo1", "master", "master", "This is a pull title")
 
  elem := strings.Split(test.RedirectURL(resp), "/")
  assert.EqualValues(t, "pulls", elem[3])
- testPullMerge(t, session, elem[1], elem[2], elem[4], repo_model.MergeStyleMerge)
+ testPullMerge(t, session, elem[1], elem[2], elem[4], repo_model.MergeStyleMerge, false)
 
  hookTasks, err = webhook.HookTasks(db.DefaultContext, 1, 1)
  assert.NoError(t, err)
@@ -105,11 +112,11 @@ func TestPullRebase(t *testing.T) {
  testRepoFork(t, session, "user2", "repo1", "user1", "repo1")
  testEditFile(t, session, "user1", "repo1", "master", "README.md", "Hello, World (Edited)\n")
 
- resp := testPullCreate(t, session, "user1", "repo1", "master", "This is a pull title")
+ resp := testPullCreate(t, session, "user1", "repo1", "master", "master", "This is a pull title")
 
  elem := strings.Split(test.RedirectURL(resp), "/")
  assert.EqualValues(t, "pulls", elem[3])
- testPullMerge(t, session, elem[1], elem[2], elem[4], repo_model.MergeStyleRebase)
+ testPullMerge(t, session, elem[1], elem[2], elem[4], repo_model.MergeStyleRebase, false)
 
  hookTasks, err = webhook.HookTasks(db.DefaultContext, 1, 1)
  assert.NoError(t, err)
@@ -127,11 +134,11 @@ func TestPullRebaseMerge(t *testing.T) {
  testRepoFork(t, session, "user2", "repo1", "user1", "repo1")
  testEditFile(t, session, "user1", "repo1", "master", "README.md", "Hello, World (Edited)\n")
 
- resp := testPullCreate(t, session, "user1", "repo1", "master", "This is a pull title")
+ resp := testPullCreate(t, session, "user1", "repo1", "master", "master", "This is a pull title")
 
  elem := strings.Split(test.RedirectURL(resp), "/")
  assert.EqualValues(t, "pulls", elem[3])
- testPullMerge(t, session, elem[1], elem[2], elem[4], repo_model.MergeStyleRebaseMerge)
+ testPullMerge(t, session, elem[1], elem[2], elem[4], repo_model.MergeStyleRebaseMerge, false)
 
  hookTasks, err = webhook.HookTasks(db.DefaultContext, 1, 1)
  assert.NoError(t, err)
@@ -150,11 +157,11 @@ func TestPullSquash(t *testing.T) {
  testEditFile(t, session, "user1", "repo1", "master", "README.md", "Hello, World (Edited)\n")
  testEditFile(t, session, "user1", "repo1", "master", "README.md", "Hello, World (Edited!)\n")
 
- resp := testPullCreate(t, session, "user1", "repo1", "master", "This is a pull title")
+ resp := testPullCreate(t, session, "user1", "repo1", "master", "master", "This is a pull title")
 
  elem := strings.Split(test.RedirectURL(resp), "/")
  assert.EqualValues(t, "pulls", elem[3])
- testPullMerge(t, session, elem[1], elem[2], elem[4], repo_model.MergeStyleSquash)
+ testPullMerge(t, session, elem[1], elem[2], elem[4], repo_model.MergeStyleSquash, false)
 
  hookTasks, err = webhook.HookTasks(db.DefaultContext, 1, 1)
  assert.NoError(t, err)
@@ -168,11 +175,11 @@ func TestPullCleanUpAfterMerge(t *testing.T) {
  testRepoFork(t, session, "user2", "repo1", "user1", "repo1")
  testEditFileToNewBranch(t, session, "user1", "repo1", "master", "feature/test", "README.md", "Hello, World (Edited - TestPullCleanUpAfterMerge)\n")
 
- resp := testPullCreate(t, session, "user1", "repo1", "feature/test", "This is a pull title")
+ resp := testPullCreate(t, session, "user1", "repo1", "master", "feature/test", "This is a pull title")
 
  elem := strings.Split(test.RedirectURL(resp), "/")
  assert.EqualValues(t, "pulls", elem[3])
- testPullMerge(t, session, elem[1], elem[2], elem[4], repo_model.MergeStyleMerge)
+ testPullMerge(t, session, elem[1], elem[2], elem[4], repo_model.MergeStyleMerge, false)
 
  // Check PR branch deletion
  resp = testPullCleanUp(t, session, elem[1], elem[2], elem[4])
@@ -203,7 +210,7 @@ func TestCantMergeWorkInProgress(t *testing.T) {
  testRepoFork(t, session, "user2", "repo1", "user1", "repo1")
  testEditFile(t, session, "user1", "repo1", "master", "README.md", "Hello, World (Edited)\n")
 
- resp := testPullCreate(t, session, "user1", "repo1", "master", "[wip] This is a pull title")
+ resp := testPullCreate(t, session, "user1", "repo1", "master", "master", "[wip] This is a pull title")
 
  req := NewRequest(t, "GET", test.RedirectURL(resp))
  resp = session.MakeRequest(t, req, http.StatusOK)
@@ -435,3 +442,33 @@ func TestConflictChecking(t *testing.T) {
  assert.False(t, conflictingPR.Mergeable(db.DefaultContext))
  })
 }
+
+func TestPullRetargetChildOnBranchDelete(t *testing.T) {
+ onGiteaRun(t, func(t *testing.T, giteaURL *url.URL) {
+ session := loginUser(t, "user1")
+ testEditFileToNewBranch(t, session, "user2", "repo1", "master", "base-pr", "README.md", "Hello, World\n(Edited - TestPullRetargetOnCleanup - base PR)\n")
+ testRepoFork(t, session, "user2", "repo1", "user1", "repo1")
+ testEditFileToNewBranch(t, session, "user1", "repo1", "base-pr", "child-pr", "README.md", "Hello, World\n(Edited - TestPullRetargetOnCleanup - base PR)\n(Edited - TestPullRetargetOnCleanup - child PR)")
+
+ respBasePR := testPullCreate(t, session, "user2", "repo1", "master", "base-pr", "Base Pull Request")
+ elemBasePR := strings.Split(test.RedirectURL(respBasePR), "/")
+ assert.EqualValues(t, "pulls", elemBasePR[3])
+
+ respChildPR := testPullCreate(t, session, "user1", "repo1", "base-pr", "child-pr", "Child Pull Request")
+ elemChildPR := strings.Split(test.RedirectURL(respChildPR), "/")
+ assert.EqualValues(t, "pulls", elemChildPR[3])
+
+ testPullMerge(t, session, elemBasePR[1], elemBasePR[2], elemBasePR[4], repo_model.MergeStyleMerge, true)
+
+ // Check child PR
+ req := NewRequest(t, "GET", test.RedirectURL(respChildPR))
+ resp := session.MakeRequest(t, req, http.StatusOK)
+
+ htmlDoc := NewHTMLParser(t, resp.Body)
+ targetBranch := htmlDoc.doc.Find("#branch_target>a").Text()
+ prStatus := strings.TrimSpace(htmlDoc.doc.Find(".issue-title-meta>.issue-state-label").Text())
+
+ assert.EqualValues(t, "master", targetBranch)
+ assert.EqualValues(t, "Open", prStatus)
+ })
+}

tests/integration/repo_activity_test.go

@@ -22,16 +22,16 @@ func TestRepoActivity(t *testing.T) {
  // Create PRs (1 merged & 2 proposed)
  testRepoFork(t, session, "user2", "repo1", "user1", "repo1")
  testEditFile(t, session, "user1", "repo1", "master", "README.md", "Hello, World (Edited)\n")
- resp := testPullCreate(t, session, "user1", "repo1", "master", "This is a pull title")
+ resp := testPullCreate(t, session, "user1", "repo1", "master", "master", "This is a pull title")
  elem := strings.Split(test.RedirectURL(resp), "/")
  assert.EqualValues(t, "pulls", elem[3])
- testPullMerge(t, session, elem[1], elem[2], elem[4], repo_model.MergeStyleMerge)
+ testPullMerge(t, session, elem[1], elem[2], elem[4], repo_model.MergeStyleMerge, false)
 
  testEditFileToNewBranch(t, session, "user1", "repo1", "master", "feat/better_readme", "README.md", "Hello, World (Edited Again)\n")
- testPullCreate(t, session, "user1", "repo1", "feat/better_readme", "This is a pull title")
+ testPullCreate(t, session, "user1", "repo1", "master", "feat/better_readme", "This is a pull title")
 
  testEditFileToNewBranch(t, session, "user1", "repo1", "master", "feat/much_better_readme", "README.md", "Hello, World (Edited More)\n")
- testPullCreate(t, session, "user1", "repo1", "feat/much_better_readme", "This is a pull title")
+ testPullCreate(t, session, "user1", "repo1", "master", "feat/much_better_readme", "This is a pull title")
 
  // Create issues (3 new issues)
  testNewIssue(t, session, "user2", "repo1", "Issue 1", "Description 1")