Merge pull request 'fix: Do not offer duplicating a PR for a recently pushed branch' (#6191) from wetneb/forgejo:6187-recent-branches into forgejo

Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/6191
Reviewed-by: Otto <otto@codeberg.org>

models/git/branch.go

@@ -417,15 +417,18 @@ 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).
 		And("repo_id = ?", repoID).
-		And("commit_time >= ?", time.Now().Add(-time.Hour*6).Unix()).
+		And("commit_time >= ?", time.Now().Add(-time.Minute*30).Unix()).
 		NotIn("name", subQuery).
 		OrderBy("branch.commit_time DESC").
 		Limit(2).

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())
+		})
 	})
 }