From 1fd16f91fb27caf8f52395f31b98b85788154e5c Mon Sep 17 00:00:00 2001 From: Viktor Kuzmin <kvaster@gmail.com> Date: Sun, 8 May 2022 23:46:00 +0300 Subject: [PATCH 01/14] Try to keep branch PRs open when branch is deleted cause of it's PR merged and closed, fixes #18408 --- routers/api/v1/repo/pull.go | 7 +++++++ routers/web/repo/pull.go | 9 +++++++++ services/pull/pull.go | 28 ++++++++++++++++++++++++++++ 3 files changed, 44 insertions(+) diff --git a/routers/api/v1/repo/pull.go b/routers/api/v1/repo/pull.go index d6b9dddd9d7ba..2f3a0d9dc021c 100644 --- a/routers/api/v1/repo/pull.go +++ b/routers/api/v1/repo/pull.go @@ -913,6 +913,13 @@ func MergePullRequest(ctx *context.APIContext) { } defer headRepo.Close() } + if 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 := 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 39f9cefa5c669..e519f9d1a40f7 100644 --- a/routers/web/repo/pull.go +++ b/routers/web/repo/pull.go @@ -1579,6 +1579,15 @@ func CleanUpPullRequest(ctx *context.Context) { func deleteBranch(ctx *context.Context, pr *issues_model.PullRequest, gitRepo *git.Repository) { fullBranchName := pr.HeadRepo.FullName() + ":" + pr.HeadBranch + + if 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 := 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 6094a4ed31b9e..ea9e087668538 100644 --- a/services/pull/pull.go +++ b/services/pull/pull.go @@ -542,6 +542,34 @@ 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) + if err != nil { + return err + } + + if err := models.PullRequestList(prs).LoadAttributes(); err != nil { + return err + } + + var errs errlist + for _, pr := range prs { + if err = pr.Issue.LoadAttributes(); 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) { + 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) From badb941bd114be8935b5472e198eb00c63e199dd Mon Sep 17 00:00:00 2001 From: Viktor Kuzmin <kvaster@gmail.com> Date: Mon, 9 May 2022 00:09:55 +0300 Subject: [PATCH 02/14] Make retarget optional, keep default behaviour without option --- modules/setting/repository.go | 2 ++ routers/api/v1/repo/pull.go | 2 +- routers/web/repo/pull.go | 2 +- 3 files changed, 4 insertions(+), 2 deletions(-) diff --git a/modules/setting/repository.go b/modules/setting/repository.go index 9697a851d3ba9..287337679235e 100644 --- a/modules/setting/repository.go +++ b/modules/setting/repository.go @@ -85,6 +85,7 @@ var ( PopulateSquashCommentWithCommitMessages bool AddCoCommitterTrailers bool TestConflictingPatchesWithGitApply bool + RetargetChildsOnClose bool } `ini:"repository.pull-request"` // Issue Setting @@ -209,6 +210,7 @@ var ( PopulateSquashCommentWithCommitMessages bool AddCoCommitterTrailers bool TestConflictingPatchesWithGitApply bool + RetargetChildsOnClose bool }{ WorkInProgressPrefixes: []string{"WIP:", "[WIP]"}, // Same as GitHub. See diff --git a/routers/api/v1/repo/pull.go b/routers/api/v1/repo/pull.go index 2f3a0d9dc021c..b52a159710efc 100644 --- a/routers/api/v1/repo/pull.go +++ b/routers/api/v1/repo/pull.go @@ -913,7 +913,7 @@ func MergePullRequest(ctx *context.APIContext) { } defer headRepo.Close() } - if pr.BaseRepoID == pr.HeadRepoID { + 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) diff --git a/routers/web/repo/pull.go b/routers/web/repo/pull.go index e519f9d1a40f7..54d126ac5f432 100644 --- a/routers/web/repo/pull.go +++ b/routers/web/repo/pull.go @@ -1580,7 +1580,7 @@ func CleanUpPullRequest(ctx *context.Context) { func deleteBranch(ctx *context.Context, pr *issues_model.PullRequest, gitRepo *git.Repository) { fullBranchName := pr.HeadRepo.FullName() + ":" + pr.HeadBranch - if pr.BaseRepoID == pr.HeadRepoID { + 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)) From 79e7630f0088de75ad6e285df96160d27c8fa933 Mon Sep 17 00:00:00 2001 From: Viktor Kuzmin <kvaster@gmail.com> Date: Sun, 26 Mar 2023 17:15:53 +0300 Subject: [PATCH 03/14] Cleanup error handling, add docs --- custom/conf/app.example.ini | 3 +++ .../config-cheat-sheet.en-us.md | 1 + modules/setting/repository.go | 5 +++-- routers/api/v1/repo/pull.go | 10 ++++----- routers/web/repo/pull.go | 10 ++++----- services/pull/pull.go | 22 +++++++++++++------ 6 files changed, 30 insertions(+), 21 deletions(-) diff --git a/custom/conf/app.example.ini b/custom/conf/app.example.ini index d58309f141bd5..c7fb40651f2d8 100644 --- a/custom/conf/app.example.ini +++ b/custom/conf/app.example.ini @@ -1065,6 +1065,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 the child pull request to the parent pull request branch target +;RETARGET_CHILDREN_ON_MERGE = false ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; diff --git a/docs/content/administration/config-cheat-sheet.en-us.md b/docs/content/administration/config-cheat-sheet.en-us.md index e111ff6db62e0..aed26e95911b4 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`: **false**: Retarget the child pull request to the parent pull request branch target. ### Repository - Issue (`repository.issue`) diff --git a/modules/setting/repository.go b/modules/setting/repository.go index 287337679235e..8979f3e8fede5 100644 --- a/modules/setting/repository.go +++ b/modules/setting/repository.go @@ -85,7 +85,7 @@ var ( PopulateSquashCommentWithCommitMessages bool AddCoCommitterTrailers bool TestConflictingPatchesWithGitApply bool - RetargetChildsOnClose bool + RetargetChildrenOnMerge bool } `ini:"repository.pull-request"` // Issue Setting @@ -210,7 +210,7 @@ var ( PopulateSquashCommentWithCommitMessages bool AddCoCommitterTrailers bool TestConflictingPatchesWithGitApply bool - RetargetChildsOnClose bool + RetargetChildrenOnMerge bool }{ WorkInProgressPrefixes: []string{"WIP:", "[WIP]"}, // Same as GitHub. See @@ -225,6 +225,7 @@ var ( DefaultMergeMessageOfficialApproversOnly: true, PopulateSquashCommentWithCommitMessages: false, AddCoCommitterTrailers: true, + RetargetChildrenOnMerge: false, }, // Issue settings diff --git a/routers/api/v1/repo/pull.go b/routers/api/v1/repo/pull.go index b52a159710efc..ad9933d306fb2 100644 --- a/routers/api/v1/repo/pull.go +++ b/routers/api/v1/repo/pull.go @@ -913,12 +913,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 { diff --git a/routers/web/repo/pull.go b/routers/web/repo/pull.go index 54d126ac5f432..94199040474b7 100644 --- a/routers/web/repo/pull.go +++ b/routers/web/repo/pull.go @@ -1580,12 +1580,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 { diff --git a/services/pull/pull.go b/services/pull/pull.go index ea9e087668538..7b8894158a82f 100644 --- a/services/pull/pull.go +++ b/services/pull/pull.go @@ -542,24 +542,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) } } From edfb3a357f5c876437e4904b9c6b29b225c19006 Mon Sep 17 00:00:00 2001 From: Viktor Kuzmin <kvaster@gmail.com> Date: Sun, 26 Mar 2023 19:52:14 +0300 Subject: [PATCH 04/14] Docs and logs cleanup --- custom/conf/app.example.ini | 2 +- docs/content/administration/config-cheat-sheet.en-us.md | 2 +- routers/api/v1/repo/pull.go | 1 - routers/web/repo/pull.go | 1 - 4 files changed, 2 insertions(+), 4 deletions(-) diff --git a/custom/conf/app.example.ini b/custom/conf/app.example.ini index c7fb40651f2d8..4b17cc4edaa34 100644 --- a/custom/conf/app.example.ini +++ b/custom/conf/app.example.ini @@ -1066,7 +1066,7 @@ 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 the child pull request to the parent pull request branch target +;; Retarget child pull requests to the parent pull request branch target on merge of parent pull request ;RETARGET_CHILDREN_ON_MERGE = false ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; diff --git a/docs/content/administration/config-cheat-sheet.en-us.md b/docs/content/administration/config-cheat-sheet.en-us.md index aed26e95911b4..6e4bc897c57fc 100644 --- a/docs/content/administration/config-cheat-sheet.en-us.md +++ b/docs/content/administration/config-cheat-sheet.en-us.md @@ -135,7 +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`: **false**: Retarget the child pull request to the parent pull request branch target. +- `RETARGET_CHILDREN_ON_MERGE`: **false**: Retarget child pull requests to the parent pull request branch target on merge of parent pull request. ### Repository - Issue (`repository.issue`) diff --git a/routers/api/v1/repo/pull.go b/routers/api/v1/repo/pull.go index ad9933d306fb2..6ac374ab3a783 100644 --- a/routers/api/v1/repo/pull.go +++ b/routers/api/v1/repo/pull.go @@ -914,7 +914,6 @@ func MergePullRequest(ctx *context.APIContext) { defer headRepo.Close() } if err := pull_service.RetargetChildrenOnMerge(ctx, ctx.Doer, pr); err != nil { - log.Error("RetargetChildrenOnMerge: %v", err) ctx.Error(http.StatusInternalServerError, "RetargetChildrenOnMerge", err) return } diff --git a/routers/web/repo/pull.go b/routers/web/repo/pull.go index 94199040474b7..471f8e27587aa 100644 --- a/routers/web/repo/pull.go +++ b/routers/web/repo/pull.go @@ -1581,7 +1581,6 @@ func deleteBranch(ctx *context.Context, pr *issues_model.PullRequest, gitRepo *g fullBranchName := pr.HeadRepo.FullName() + ":" + pr.HeadBranch 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 } From 64a201908fb26e6b693f5633f3d15de689b8460c Mon Sep 17 00:00:00 2001 From: Viktor Kuzmin <kvaster@gmail.com> Date: Wed, 3 Jan 2024 17:46:15 +0200 Subject: [PATCH 05/14] Rebase to main, do not load all issue atributes - repo is enough --- services/pull/pull.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/services/pull/pull.go b/services/pull/pull.go index 7b8894158a82f..48b8fbc8696f6 100644 --- a/services/pull/pull.go +++ b/services/pull/pull.go @@ -552,18 +552,18 @@ func RetargetChildrenOnMerge(ctx context.Context, doer *user_model.User, pr *iss // 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) + prs, err := issues_model.GetUnmergedPullRequestsByBaseInfo(ctx, repoID, branch) if err != nil { return err } - if err := issues_model.PullRequestList(prs).LoadAttributes(); err != nil { + if err := issues_model.PullRequestList(prs).LoadAttributes(ctx); err != nil { return err } var errs errlist for _, pr := range prs { - if err = pr.Issue.LoadAttributes(ctx); err != nil { + 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) && From d8b13f1417751e4b5945f6bde0183b840f259348 Mon Sep 17 00:00:00 2001 From: Denys Konovalov <kontakt@denyskon.de> Date: Mon, 15 Jan 2024 16:49:35 +0100 Subject: [PATCH 06/14] Update modules/setting/repository.go --- modules/setting/repository.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/setting/repository.go b/modules/setting/repository.go index 8979f3e8fede5..a6f0ed8833589 100644 --- a/modules/setting/repository.go +++ b/modules/setting/repository.go @@ -225,7 +225,7 @@ var ( DefaultMergeMessageOfficialApproversOnly: true, PopulateSquashCommentWithCommitMessages: false, AddCoCommitterTrailers: true, - RetargetChildrenOnMerge: false, + RetargetChildrenOnMerge: true, }, // Issue settings From fe010bd175611804e3ae6eed04bad30fe338d288 Mon Sep 17 00:00:00 2001 From: Denys Konovalov <kontakt@denyskon.de> Date: Mon, 15 Jan 2024 16:49:43 +0100 Subject: [PATCH 07/14] Update docs/content/administration/config-cheat-sheet.en-us.md --- docs/content/administration/config-cheat-sheet.en-us.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/content/administration/config-cheat-sheet.en-us.md b/docs/content/administration/config-cheat-sheet.en-us.md index 6e4bc897c57fc..dc52463504224 100644 --- a/docs/content/administration/config-cheat-sheet.en-us.md +++ b/docs/content/administration/config-cheat-sheet.en-us.md @@ -135,7 +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`: **false**: Retarget child pull requests to the parent pull request branch target on merge of parent pull request. +- `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`) From e2cfe09b7f9f34eab2df9943b733266c00bfc8cf Mon Sep 17 00:00:00 2001 From: Denys Konovalov <kontakt@denyskon.de> Date: Mon, 15 Jan 2024 16:49:49 +0100 Subject: [PATCH 08/14] Update custom/conf/app.example.ini --- custom/conf/app.example.ini | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/custom/conf/app.example.ini b/custom/conf/app.example.ini index 4b17cc4edaa34..32bcdf8154479 100644 --- a/custom/conf/app.example.ini +++ b/custom/conf/app.example.ini @@ -1067,7 +1067,7 @@ LEVEL = Info ;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 = false +;RETARGET_CHILDREN_ON_MERGE = true ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; From 6a70807c13a8828cb211d91b000e4986f09a7089 Mon Sep 17 00:00:00 2001 From: Denys Konovalov <kontakt@denyskon.de> Date: Tue, 16 Jan 2024 18:48:39 +0100 Subject: [PATCH 09/14] Update services/pull/pull.go Co-authored-by: delvh <dev.lh@web.de> --- services/pull/pull.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/services/pull/pull.go b/services/pull/pull.go index d4c3a1945cd44..869f9824aa927 100644 --- a/services/pull/pull.go +++ b/services/pull/pull.go @@ -554,7 +554,8 @@ func RetargetChildrenOnMerge(ctx context.Context, doer *user_model.User, pr *iss return nil } -// RetargetBranchPulls change target branch for all pull requests who's 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 { prs, err := issues_model.GetUnmergedPullRequestsByBaseInfo(ctx, repoID, branch) if err != nil { From 5b2e813a73b06f5bfb5db67b1b49fc740f584a8f Mon Sep 17 00:00:00 2001 From: Denys Konovalov <kontakt@denyskon.de> Date: Tue, 16 Jan 2024 20:18:41 +0100 Subject: [PATCH 10/14] add test --- tests/integration/pull_create_test.go | 19 +++++--- tests/integration/pull_merge_test.go | 65 +++++++++++++++++++------ tests/integration/repo_activity_test.go | 8 +-- 3 files changed, 68 insertions(+), 24 deletions(-) diff --git a/tests/integration/pull_create_test.go b/tests/integration/pull_create_test.go index a6ee0d9dfa1ef..412d2dcc764eb 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, targetBranch, sourceBranch, title string) *httptest.ResponseRecorder { req := NewRequest(t, "GET", path.Join(user, repo)) resp := session.MakeRequest(t, req, http.StatusOK) @@ -25,8 +25,15 @@ 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) + 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) diff --git a/tests/integration/pull_merge_test.go b/tests/integration/pull_merge_test.go index 35af47f802fd6..43a8ca533c8c9 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", "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) + }) +} diff --git a/tests/integration/repo_activity_test.go b/tests/integration/repo_activity_test.go index c8d0c46d6455e..cc729e4a5712f 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", "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") From 05ea478e2173c15933ced7e84cf93973164138f9 Mon Sep 17 00:00:00 2001 From: Denys Konovalov <kontakt@denyskon.de> Date: Tue, 16 Jan 2024 20:20:40 +0100 Subject: [PATCH 11/14] make fmt --- services/pull/pull.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/pull/pull.go b/services/pull/pull.go index 869f9824aa927..930954bdfdfa1 100644 --- a/services/pull/pull.go +++ b/services/pull/pull.go @@ -555,7 +555,7 @@ func RetargetChildrenOnMerge(ctx context.Context, doer *user_model.User, pr *iss } // 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) +// 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 { From 48d74512d7860657e38e7aff219463087ab22332 Mon Sep 17 00:00:00 2001 From: Denys Konovalov <kontakt@denyskon.de> Date: Tue, 16 Jan 2024 20:46:57 +0100 Subject: [PATCH 12/14] Update docs/content/administration/config-cheat-sheet.en-us.md Co-authored-by: delvh <dev.lh@web.de> --- docs/content/administration/config-cheat-sheet.en-us.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/content/administration/config-cheat-sheet.en-us.md b/docs/content/administration/config-cheat-sheet.en-us.md index 41a4f5ef57ccb..eb9b8d1ae9a43 100644 --- a/docs/content/administration/config-cheat-sheet.en-us.md +++ b/docs/content/administration/config-cheat-sheet.en-us.md @@ -135,7 +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. +- `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`) From ea60e5870ca38403e0e481cdfd420090e191456b Mon Sep 17 00:00:00 2001 From: Denys Konovalov <kontakt@denyskon.de> Date: Tue, 16 Jan 2024 20:47:04 +0100 Subject: [PATCH 13/14] Update custom/conf/app.example.ini Co-authored-by: delvh <dev.lh@web.de> --- custom/conf/app.example.ini | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/custom/conf/app.example.ini b/custom/conf/app.example.ini index d80567377d257..b0875123b7b8b 100644 --- a/custom/conf/app.example.ini +++ b/custom/conf/app.example.ini @@ -1068,7 +1068,7 @@ 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 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 ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; From 0f6cddafd8f8e4201385724f995df3e309c94fa3 Mon Sep 17 00:00:00 2001 From: Denys Konovalov <kontakt@denyskon.de> Date: Tue, 16 Jan 2024 21:04:27 +0100 Subject: [PATCH 14/14] add don't retarget test --- tests/integration/pull_create_test.go | 16 ++++++--- tests/integration/pull_merge_test.go | 46 ++++++++++++++++++++----- tests/integration/repo_activity_test.go | 6 ++-- 3 files changed, 52 insertions(+), 16 deletions(-) diff --git a/tests/integration/pull_create_test.go b/tests/integration/pull_create_test.go index 412d2dcc764eb..029ea65d7190f 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, targetBranch, sourceBranch, 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,11 +25,17 @@ func testPullCreate(t *testing.T, session *TestSession, user, repo, targetBranch htmlDoc := NewHTMLParser(t, resp.Body) link, exists := htmlDoc.doc.Find("#new-pull-request").Attr("href") assert.True(t, exists, "The template has changed") + + 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 strings.Split(link, "/")[1] == user { + if targetUser == user { link = strings.Replace(link, "...master", "..."+sourceBranch, 1) } else { link = strings.Replace(link, ":master", ":"+sourceBranch, 1) @@ -56,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", "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) @@ -84,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", "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) @@ -149,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", "master", "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 43a8ca533c8c9..2aa6742a56ac0 100644 --- a/tests/integration/pull_merge_test.go +++ b/tests/integration/pull_merge_test.go @@ -90,7 +90,7 @@ 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", "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]) @@ -112,7 +112,7 @@ 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", "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]) @@ -134,7 +134,7 @@ 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", "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]) @@ -157,7 +157,7 @@ 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", "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]) @@ -175,7 +175,7 @@ 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", "master", "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]) @@ -210,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", "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) @@ -450,11 +450,11 @@ func TestPullRetargetChildOnBranchDelete(t *testing.T) { 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") + 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", "base-pr", "child-pr", "Child Pull Request") + 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]) @@ -472,3 +472,33 @@ func TestPullRetargetChildOnBranchDelete(t *testing.T) { 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 cc729e4a5712f..792554db4bc93 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", "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, false) testEditFileToNewBranch(t, session, "user1", "repo1", "master", "feat/better_readme", "README.md", "Hello, World (Edited Again)\n") - testPullCreate(t, session, "user1", "repo1", "master", "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", "master", "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")