Stowage/forgejo
2
0
Fork 0
mirror of https://codeberg.org/forgejo/forgejo.git synced 2025-12-07 14:09:47 +00:00
Code Issues Projects Releases Packages Wiki Activity

feat: show link to pull requests targeting a non-default branch when pushing (#10079)

Browse source 
This resolves #10057 by showing a list of links to pull requests with the head branch being the one just pushed.

Since there may be multiple pull requests with different base branches, we find all of them and print them.

Here is a comparison table for pushing to the `feature` branch when having 2 pull requests: `feature -> dev`, and `feature -> prod`. `main` being the default branch.

## Before

remote:
remote: Create a new pull request for 'feature':
remote:   http://localhost:3000/user1/repo1/compare/main...feature
remote:

## After

remote:
remote: Create a new pull request for 'feature':
remote:   http://localhost:3000/user1/repo1/compare/main...feature
remote: Visit the existing pull requests:
remote:   http://localhost:3000/user1/repo1/pulls/1 merges into dev
remote:   http://localhost:3000/user1/repo1/pulls/3 merges into prod
remote:

Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/10079
Reviewed-by: Gusted <gusted@noreply.codeberg.org>
Co-authored-by: Calixte Pernot <cpernot@praksys.net>
Co-committed-by: Calixte Pernot <cpernot@praksys.net>
This commit is contained in:
Calixte Pernot 2025-11-19 14:59:13 +01:00 • committed by Gusted
parent 37f8fcf66d
commit 4d0c7db6cd
6 changed files with 193 additions and 33 deletions
Download patch file Download diff file Expand all files Collapse all files

15
cmd/hook.go
View file

@ -452,10 +452,17 @@ func hookPrintResults(results []private.HookPostReceiveBranchResult) {
fmt.Fprintln(os.Stderr, "")
if res.Create {
fmt.Fprintf(os.Stderr, "Create a new pull request for '%s':\n", res.Branch)
fmt.Fprintf(os.Stderr, " %s\n", res.URL)
} else {
fmt.Fprint(os.Stderr, "Visit the existing pull request:\n")
fmt.Fprintf(os.Stderr, " %s\n", res.URL)
fmt.Fprintf(os.Stderr, " %s\n", res.CreateURL)
}
if len(res.PullURLS) != 0 {
if len(res.PullURLS) >= 2 {
fmt.Fprint(os.Stderr, "Visit the existing pull requests:\n")
} else {
fmt.Fprint(os.Stderr, "Visit the existing pull request:\n")
}
for _, url := range res.PullURLS {
fmt.Fprintf(os.Stderr, " %s\n", url)
}
}
fmt.Fprintln(os.Stderr, "")
_ = os.Stderr.Sync()

15
models/issues/pull.go
View file

@ -632,6 +632,21 @@ func GetUnmergedPullRequest(ctx context.Context, headRepoID, baseRepoID int64, h
return pr, nil
}
// GetUnmergedPullRequestsAnyTarget returns a pull request that is open and has not been merged
// by given head repo and branch and targeting any other branch on the baseRepo
func GetUnmergedPullRequestsAnyTarget(ctx context.Context, headRepoID, baseRepoID int64, headBranch string, flow PullRequestFlow) (PullRequestList, error) {
var pr PullRequestList
err := db.GetEngine(ctx).
Where("head_repo_id=? AND head_branch=? AND base_repo_id=? AND has_merged=? AND flow = ? AND issue.is_closed=?",
headRepoID, headBranch, baseRepoID, false, flow, false).
Join("INNER", "issue", "issue.id=pull_request.issue_id").
Find(&pr)
if err != nil {
return nil, err
}
return pr, nil
}
// GetLatestPullRequestByHeadInfo returns the latest pull request (regardless of its status)
// by given head information (repo and branch).
func GetLatestPullRequestByHeadInfo(ctx context.Context, repoID int64, branch string) (*PullRequest, error) {

5
models/issues/pull_test.go
View file

@ -172,6 +172,11 @@ func TestGetUnmergedPullRequest(t *testing.T) {
require.NoError(t, err)
assert.Equal(t, int64(2), pr.ID)
prList, err := issues_model.GetUnmergedPullRequestsAnyTarget(db.DefaultContext, 1, 1, "branch2", issues_model.PullRequestFlowGithub)
require.NoError(t, err)
assert.Len(t, prList, 1)
assert.Equal(t, int64(2), prList[0].ID)
_, err = issues_model.GetUnmergedPullRequest(db.DefaultContext, 1, 9223372036854775807, "branch1", "master", issues_model.PullRequestFlowGithub)
require.Error(t, err)
assert.True(t, issues_model.IsErrPullRequestNotExist(err))

9
modules/private/hook.go
View file

@ -60,10 +60,11 @@ type HookPostReceiveResult struct {
// HookPostReceiveBranchResult represents an individual branch result from PostReceive
type HookPostReceiveBranchResult struct {
Message bool
Create bool
Branch string
URL string
Message bool
Create bool
Branch string
CreateURL string
PullURLS []string
}
// HookProcReceiveResult represents an individual result from ProcReceive

73
routers/private/hook_post_receive.go
View file

@ -231,10 +231,11 @@ func HookPostReceive(ctx *app_context.PrivateContext) {
}
results = append(results, private.HookPostReceiveBranchResult{
Message: setting.Git.PullRequestPushMessage && repo.AllowsPulls(ctx),
Create: false,
Branch: "",
URL: fmt.Sprintf("%s/pulls/%d", repo.HTMLURL(), pr.Index),
Message: setting.Git.PullRequestPushMessage && repo.AllowsPulls(ctx),
Create: false,
Branch: "",
CreateURL: "",
PullURLS: []string{fmt.Sprintf("%s/pulls/%d", repo.HTMLURL(), pr.Index)},
})
continue
}
@ -281,35 +282,57 @@ func HookPostReceive(ctx *app_context.PrivateContext) {
continue
}
pr, err := issues_model.GetUnmergedPullRequest(ctx, repo.ID, baseRepo.ID, branch, baseRepo.DefaultBranch, issues_model.PullRequestFlowGithub)
if err != nil && !issues_model.IsErrPullRequestNotExist(err) {
log.Error("Failed to get active PR in: %-v Branch: %s to: %-v Branch: %s Error: %v", repo, branch, baseRepo, baseRepo.DefaultBranch, err)
// Check if there is an existing pull request for this branch.
prList, err := issues_model.GetUnmergedPullRequestsAnyTarget(ctx, repo.ID, baseRepo.ID, branch, issues_model.PullRequestFlowGithub)
if err != nil {
log.Error("Failed to get active PR in: %-v Branch: %s to: %-v Error: %v", repo, branch, baseRepo, err)
ctx.JSON(http.StatusInternalServerError, private.HookPostReceiveResult{
Err: fmt.Sprintf(
"Failed to get active PR in: %-v Branch: %s to: %-v Branch: %s Error: %v", repo, branch, baseRepo, baseRepo.DefaultBranch, err),
"Failed to get active PR in: %-v Branch: %s to: %-v Error: %v", repo, branch, baseRepo, err),
RepoWasEmpty: wasEmpty,
})
return
}
err = prList.LoadRepositories(ctx)
if err != nil {
log.Error("Failed to load repositories for PullRequestList: %s", err)
ctx.JSON(http.StatusInternalServerError, private.HookPostReceiveResult{
Err: fmt.Sprintf("Failed to load repositories for PullRequestList: %s", err),
RepoWasEmpty: wasEmpty,
})
return
}
if pr == nil {
if repo.IsFork {
branch = fmt.Sprintf("%s:%s", repo.OwnerName, branch)
}
results = append(results, private.HookPostReceiveBranchResult{
Message: setting.Git.PullRequestPushMessage && baseRepo.AllowsPulls(ctx),
Create: true,
Branch: branch,
URL: fmt.Sprintf("%s/compare/%s...%s", baseRepo.HTMLURL(), util.PathEscapeSegments(baseRepo.DefaultBranch), util.PathEscapeSegments(branch)),
})
} else {
results = append(results, private.HookPostReceiveBranchResult{
Message: setting.Git.PullRequestPushMessage && baseRepo.AllowsPulls(ctx),
Create: false,
Branch: branch,
URL: fmt.Sprintf("%s/pulls/%d", baseRepo.HTMLURL(), pr.Index),
})
if repo.IsFork {
branch = fmt.Sprintf("%s:%s", repo.OwnerName, branch)
}
createURL := fmt.Sprintf("%s/compare/%s...%s", baseRepo.HTMLURL(), util.PathEscapeSegments(baseRepo.DefaultBranch), util.PathEscapeSegments(branch))
var urls []string
foundDefaultBranch := false
for _, pr := range prList {
var baseBranchDisplay string
if pr.HeadRepoID == pr.BaseRepoID {
// Inside the same repository: just show base branch name
baseBranchDisplay = pr.BaseBranch
} else {
// We are merging this into another repo: display user/repo:branch
baseBranchDisplay = fmt.Sprintf("%s:%s", pr.BaseRepo.FullName(), pr.BaseBranch)
}
urls = append(urls, fmt.Sprintf("%s/pulls/%d merges into %s", baseRepo.HTMLURL(), pr.Index, baseBranchDisplay))
if pr.BaseBranch == baseRepo.DefaultBranch {
foundDefaultBranch = true
}
}
if foundDefaultBranch {
createURL = ""
}
results = append(results, private.HookPostReceiveBranchResult{
Message: setting.Git.PullRequestPushMessage && baseRepo.AllowsPulls(ctx),
Create: !foundDefaultBranch,
Branch: branch,
CreateURL: createURL,
PullURLS: urls,
})
}
}
ctx.JSON(http.StatusOK, private.HookPostReceiveResult{

109
tests/integration/git_test.go
View file

@ -116,6 +116,8 @@ func testGit(t *testing.T, u *url.URL) {
rawTest(t, &httpContext, little, big, littleLFS, bigLFS)
mediaTest(t, &httpContext, little, big, littleLFS, bigLFS)
t.Run("PushRemoteMessages", doTestPushMessages(httpContext, u, objectFormat))
t.Run("PushForkRemoteMessages", doTestForkPushMessages(httpContext, dstPath))
t.Run("CreateAgitFlowPull", doCreateAgitFlowPull(dstPath, &httpContext, "test/head"))
t.Run("InternalReferences", doInternalReferences(&httpContext, dstPath))
t.Run("BranchProtect", doBranchProtect(&httpContext, dstPath))
@ -163,6 +165,8 @@ func testGit(t *testing.T, u *url.URL) {
rawTest(t, &sshContext, little, big, littleLFS, bigLFS)
mediaTest(t, &sshContext, little, big, littleLFS, bigLFS)
t.Run("PushRemoteMessages", doTestPushMessages(sshContext, u, objectFormat))
t.Run("PushForkRemoteMessages", doTestForkPushMessages(sshContext, dstPath))
t.Run("CreateAgitFlowPull", doCreateAgitFlowPull(dstPath, &sshContext, "test/head2"))
t.Run("InternalReferences", doInternalReferences(&sshContext, dstPath))
t.Run("BranchProtect", doBranchProtect(&sshContext, dstPath))
@ -1156,3 +1160,108 @@ func doLFSNoAccess(ctx APITestContext, publicKeyID int64, objectFormat git.Objec
}
}
}
func extractRemoteMessages(stderr string) string {
var remoteMsg string
for line := range strings.SplitSeq(stderr, "\n") {
msg, found := strings.CutPrefix(line, "remote: ")
if found {
remoteMsg += msg
remoteMsg += "\n"
}
}
return remoteMsg
}
func doTestForkPushMessages(apictx APITestContext, dstPath string) func(*testing.T) {
return func(t *testing.T) {
defer tests.PrintCurrentTest(t)()
doGitCheckoutBranch(dstPath, "-b", "test_msg")(t)
// Commit/Push on test_msg branch
generateCommitWithNewData(t, littleSize, dstPath, "user2@example.com", "User Two", "testmsg-file")
_, stderr, err := git.NewCommand(git.DefaultContext, "push", "-u", "origin", "test_msg").RunStdString(&git.RunOpts{Dir: dstPath}) // Push
require.NoError(t, err)
messages := extractRemoteMessages(stderr)
// Remote server should suggest the creation of a pull request to the default branch "master"
require.Contains(t, messages, "Create a new pull request for 'user2:test_msg':")
// But shouldn't show "Visit existing pull requests..."
require.NotContains(t, messages, "Visit")
// Shouldn't contain links to pull requests
require.NotContains(t, messages, "/pulls")
require.NotContains(t, messages, "merges into")
// Create PR to default branch and push new commit
pr, giterr := doAPICreatePullRequest(apictx, "user2", apictx.Reponame, "master", "test_msg")(t)
require.NoError(t, giterr)
generateCommitWithNewData(t, littleSize, dstPath, "user2@example.com", "User Two", "testmsg-file")
_, stderr, err = git.NewCommand(git.DefaultContext, "push").RunStdString(&git.RunOpts{Dir: dstPath})
require.NoError(t, err)
messages = extractRemoteMessages(stderr)
// However, a pull request to the base repo doesn't exist
// Because we are a fork, only PRs to the base repo are displayed
require.Contains(t, messages, "Create a new pull request for 'user2:test_msg':")
require.Contains(t, messages, apictx.Reponame+"/compare/master...user2")
require.NotContains(t, messages, "merges into")
require.NotContains(t, messages, pr.HTMLURL)
// Finally merge the pull request and pull back changes to avoid polluting next tests
baseRepo := pr.Base.Repository
doAPIMergePullRequest(apictx, baseRepo.Owner.UserName, baseRepo.Name, pr.Index)(t)
doGitCheckoutBranch(dstPath, "master")(t)
doGitPull(dstPath)(t)
}
}
func doTestPushMessages(ctx APITestContext, u *url.URL, objectFormat git.ObjectFormat) func(*testing.T) {
return func(t *testing.T) {
defer tests.PrintCurrentTest(t)()
ctx.Reponame = fmt.Sprintf("repo-test-pushmsg-%s", objectFormat.Name())
dstPath := t.TempDir()
// Create/Clone new repo
doAPICreateRepository(ctx, nil, objectFormat)(t)
u.Path = ctx.GitPath()
doGitClone(dstPath, u)(t)
// Push to master
generateCommitWithNewData(t, littleSize, dstPath, "user2@example.com", "User Two", "testmsg-file")
_, stderr, err := git.NewCommand(git.DefaultContext, "push", "-u", "origin", "master").RunStdString(&git.RunOpts{Dir: dstPath}) // Push
require.NoError(t, err)
messages := extractRemoteMessages(stderr)
// Remote server shouldn't suggest the creation of a pull request: we pushed to the default branch
require.NotContains(t, messages, "Create a new pull request for 'user2:testmsg':")
// ...and shouldn't give links to pull request: there is no PR yet
require.NotContains(t, messages, "Visit the existing")
// Shouldn't contain links to pull requests
require.NotContains(t, messages, "/pulls")
require.NotContains(t, messages, "merges into")
// Create a branch and push to it
doGitCheckoutBranch(dstPath, "-b", "test_msg")(t)
generateCommitWithNewData(t, littleSize, dstPath, "user2@example.com", "User Two", "testmsg-file")
_, stderr, err = git.NewCommand(git.DefaultContext, "push", "-u", "origin", "test_msg").RunStdString(&git.RunOpts{Dir: dstPath})
require.NoError(t, err)
messages = extractRemoteMessages(stderr)
// We pushed to a new branch and there is no PR yet: a link to create one should be given
require.Contains(t, messages, ctx.Reponame+"/compare/master...test_msg")
require.NotContains(t, messages, "/pulls")
require.NotContains(t, messages, "merges into")
// Create PR to default branch and push new commit
pr, giterr := doAPICreatePullRequest(ctx, "user2", ctx.Reponame, "master", "test_msg")(t)
require.NoError(t, giterr)
generateCommitWithNewData(t, littleSize, dstPath, "user2@example.com", "User Two", "testmsg-file")
_, stderr, err = git.NewCommand(git.DefaultContext, "push", "origin", "test_msg").RunStdString(&git.RunOpts{Dir: dstPath})
require.NoError(t, err)
messages = extractRemoteMessages(stderr)
// A pull request to the base branch already exist
require.NotContains(t, messages, "Create a new pull request")
require.Contains(t, messages, "Visit the existing pull request:")
require.Contains(t, messages, pr.HTMLURL+" merges into master")
// Delete this test repository
doAPIDeleteRepository(ctx)(t)
}
}