aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
Diffstat
-rw-r--r--custom/conf/app.example.ini3
-rw-r--r--docs/content/administration/config-cheat-sheet.en-us.md1
-rw-r--r--modules/setting/repository.go3
-rw-r--r--routers/api/v1/repo/pull.go4
-rw-r--r--routers/web/repo/pull.go6
-rw-r--r--services/pull/pull.go37
-rw-r--r--tests/integration/pull_create_test.go25
-rw-r--r--tests/integration/pull_merge_test.go95
-rw-r--r--tests/integration/repo_activity_test.go8
9 files changed, 158 insertions, 24 deletions
diff --git a/custom/conf/app.example.ini b/custom/conf/app.example.ini
index 301e88b14e..b0875123b7 100644
--- a/custom/conf/app.example.ini
+++ b/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. It only works on merged PRs where the head and base branch target the same repo.
+;RETARGET_CHILDREN_ON_MERGE = true
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
diff --git a/docs/content/administration/config-cheat-sheet.en-us.md b/docs/content/administration/config-cheat-sheet.en-us.md
index c9e99ea54f..eb9b8d1ae9 100644
--- a/docs/content/administration/config-cheat-sheet.en-us.md
+++ b/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. It only works on merged PRs where the head and base branch target the same repo.
### Repository - Issue (`repository.issue`)
diff --git a/modules/setting/repository.go b/modules/setting/repository.go
index 9697a851d3..a6f0ed8833 100644
--- a/modules/setting/repository.go
+++ b/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
diff --git a/routers/api/v1/repo/pull.go b/routers/api/v1/repo/pull.go
index 34129ad595..b1cb7011f1 100644
--- a/routers/api/v1/repo/pull.go
+++ b/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):
diff --git a/routers/web/repo/pull.go b/routers/web/repo/pull.go
index 32d82b2a64..e36d7092af 100644
--- a/routers/web/repo/pull.go
+++ b/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):
diff --git a/services/pull/pull.go b/services/pull/pull.go
index d1630f3792..930954bdfd 100644
--- a/services/pull/pull.go
+++ b/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)
diff --git a/tests/integration/pull_create_test.go b/tests/integration/pull_create_test.go
index a6ee0d9dfa..029ea65d71 100644
--- a/tests/integration/pull_create_test.go
+++ b/tests/integration/pull_create_test.go
@@ -17,7 +17,7 @@ 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 string, toSelf bool, targetBranch, sourceBranch, title string) *httptest.ResponseRecorder {
req := NewRequest(t, "GET", path.Join(user, repo))
resp := session.MakeRequest(t, req, http.StatusOK)
@@ -25,8 +25,21 @@ func testPullCreate(t *testing.T, session *TestSession, user, repo, branch, titl
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)
+
+ targetUser := strings.Split(link, "/")[1]
+ if toSelf && targetUser != user {
+ link = strings.Replace(link, targetUser, user, 1)
+ }
+
+ if targetBranch != "master" {
+ link = strings.Replace(link, "master...", targetBranch+"...", 1)
+ }
+ if sourceBranch != "master" {
+ if targetUser == user {
+ link = strings.Replace(link, "...master", "..."+sourceBranch, 1)
+ } else {
+ link = strings.Replace(link, ":master", ":"+sourceBranch, 1)
+ }
}
req = NewRequest(t, "GET", link)
@@ -49,7 +62,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", false, "master", "master", "This is a pull title")
// check the redirected URL
url := test.RedirectURL(resp)
@@ -77,7 +90,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", false, "master", "master", "<i>XSS PR</i>")
// check the redirected URL
url := test.RedirectURL(resp)
@@ -142,7 +155,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", false, "master", "master1", "This is a pull title")
// check the redirected URL
url := test.RedirectURL(resp)
diff --git a/tests/integration/pull_merge_test.go b/tests/integration/pull_merge_test.go
index 35af47f802..2aa6742a56 100644
--- a/tests/integration/pull_merge_test.go
+++ b/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", false, "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", false, "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", false, "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", false, "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", false, "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", false, "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,63 @@ 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", true, "master", "base-pr", "Base Pull Request")
+ elemBasePR := strings.Split(test.RedirectURL(respBasePR), "/")
+ assert.EqualValues(t, "pulls", elemBasePR[3])
+
+ respChildPR := testPullCreate(t, session, "user1", "repo1", false, "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)
+ })
+}
+
+func TestPullDontRetargetChildOnWrongRepo(t *testing.T) {
+ onGiteaRun(t, func(t *testing.T, giteaURL *url.URL) {
+ session := loginUser(t, "user1")
+ testRepoFork(t, session, "user2", "repo1", "user1", "repo1")
+ testEditFileToNewBranch(t, session, "user1", "repo1", "master", "base-pr", "README.md", "Hello, World\n(Edited - TestPullDontRetargetChildOnWrongRepo - base PR)\n")
+ testEditFileToNewBranch(t, session, "user1", "repo1", "base-pr", "child-pr", "README.md", "Hello, World\n(Edited - TestPullDontRetargetChildOnWrongRepo - base PR)\n(Edited - TestPullDontRetargetChildOnWrongRepo - child PR)")
+
+ respBasePR := testPullCreate(t, session, "user1", "repo1", false, "master", "base-pr", "Base Pull Request")
+ elemBasePR := strings.Split(test.RedirectURL(respBasePR), "/")
+ assert.EqualValues(t, "pulls", elemBasePR[3])
+
+ respChildPR := testPullCreate(t, session, "user1", "repo1", true, "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, "base-pr", targetBranch)
+ assert.EqualValues(t, "Closed", prStatus)
+ })
+}
diff --git a/tests/integration/repo_activity_test.go b/tests/integration/repo_activity_test.go
index c8d0c46d64..792554db4b 100644
--- a/tests/integration/repo_activity_test.go
+++ b/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", false, "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", false, "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", false, "master", "feat/much_better_readme", "This is a pull title")
// Create issues (3 new issues)
testNewIssue(t, session, "user2", "repo1", "Issue 1", "Description 1")
generated by cgit v1.2.3 (git 2.39.1) at 2024-12-25 06:06:05 +0000