From 4973d23ef8b06adc48ff430267c57db6689a6627 Mon Sep 17 00:00:00 2001 From: Antonin Delpeuch Date: Fri, 6 Dec 2024 23:42:14 +0100 Subject: [PATCH] fix: Do not offer duplicating a PR for a recently pushed branch Fixes #6187. --- models/git/branch.go | 11 ++++++--- tests/integration/pull_create_test.go | 35 +++++++++++++++++++++++++++ 2 files changed, 42 insertions(+), 4 deletions(-) diff --git a/models/git/branch.go b/models/git/branch.go index f004d502ac..8fadf945ae 100644 --- a/models/git/branch.go +++ b/models/git/branch.go @@ -417,10 +417,13 @@ func FindRecentlyPushedNewBranches(ctx context.Context, repoID, userID int64, ex branches := make(BranchList, 0, 2) subQuery := builder.Select("head_branch").From("pull_request"). InnerJoin("issue", "issue.id = pull_request.issue_id"). - Where(builder.Eq{ - "pull_request.head_repo_id": repoID, - "issue.is_closed": false, - }) + Where(builder.And( + builder.Eq{"pull_request.head_repo_id": repoID}, + builder.Or( + builder.Eq{"pull_request.has_merged": true}, + builder.Eq{"issue.is_closed": false}, + ), + )) err := db.GetEngine(ctx). Where("pusher_id=? AND is_deleted=?", userID, false). And("name <> ?", excludeBranchName). diff --git a/tests/integration/pull_create_test.go b/tests/integration/pull_create_test.go index b814642bc7..7ccbce2eae 100644 --- a/tests/integration/pull_create_test.go +++ b/tests/integration/pull_create_test.go @@ -336,6 +336,10 @@ func TestRecentlyPushed(t *testing.T) { []repo_model.RepoUnit{{ RepoID: repo.ID, Type: unit_model.TypePullRequests, + Config: &repo_model.PullRequestsConfig{ + AllowMerge: true, + AllowSquash: true, + }, }}, nil) require.NoError(t, err) @@ -527,6 +531,37 @@ func TestRecentlyPushed(t *testing.T) { link, _ := htmlDoc.Find(".ui.message a[href*='/src/branch/']").Attr("href") assert.Equal(t, "/user1/repo1/src/branch/recent-push", link) }) + + // Test that visiting the base repo does not show any banner if + // the branches have corresponding PRs (open or merged) + t.Run("branches with merged or open PRs are not shown", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + respChildPR := testPullCreateDirectly(t, session, "user2", "repo1", "master", "user1", "repo1", "recent-push", "Child Pull Request") + elemChildPR := strings.Split(test.RedirectURL(respChildPR), "/") + assert.EqualValues(t, "user2", elemChildPR[1]) + assert.EqualValues(t, "repo1", elemChildPR[2]) + assert.EqualValues(t, "pulls", elemChildPR[3]) + session2 := loginUser(t, "user2") + // Merge the PR from the fork + testPullMerge(t, session2, elemChildPR[1], elemChildPR[2], elemChildPR[4], repo_model.MergeStyleSquash, false) + + respBasePR := testPullCreate(t, session, "user2", "repo1", true, "master", "recent-push-base", "Base Pull Request") + elemBasePR := strings.Split(test.RedirectURL(respBasePR), "/") + assert.EqualValues(t, "pulls", elemBasePR[3]) + // Leave the PR from the base repo open (it conflicts with the PR from the fork anyway) + + // Count recently pushed branches on the base repo + req := NewRequest(t, "GET", "/user2/repo1") + resp := session.MakeRequest(t, req, http.StatusOK) + htmlDoc := NewHTMLParser(t, resp.Body) + + messages := htmlDoc.Find(".ui.message") + + // None of the branches should be shown, as they have either already been merged already, + // or have an open PR, so it doesn't make sense to make a new PR for any of them. + assert.Equal(t, 0, messages.Length()) + }) }) }