From b1e326d09e86037435e3184b655a22e2d8841bef Mon Sep 17 00:00:00 2001
From: wxiaoguang <wxiaoguang@gmail.com>
Date: Mon, 24 Mar 2025 22:28:02 +0800
Subject: [PATCH] Auto expand "New PR" form (#33971)
Follow GitHub's behavior: use `?expand=1` to expand the "New PR" form
---
routers/private/hook_post_receive.go | 5 +-
routers/web/repo/compare.go | 21 +++-----
routers/web/repo/pull.go | 15 ++++++
routers/web/web.go | 1 +
templates/repo/branch/list.tmpl | 4 +-
.../code/recently_pushed_new_branches.tmpl | 2 +-
templates/repo/diff/compare.tmpl | 4 +-
templates/repo/view_content.tmpl | 2 +-
tests/integration/pull_compare_test.go | 49 ++++++++++++-------
9 files changed, 62 insertions(+), 41 deletions(-)
diff --git a/routers/private/hook_post_receive.go b/routers/private/hook_post_receive.go
index dba6aef9a3..442d0a76c9 100644
--- a/routers/private/hook_post_receive.go
+++ b/routers/private/hook_post_receive.go
@@ -303,14 +303,11 @@ func HookPostReceive(ctx *gitea_context.PrivateContext) {
}
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)),
+ URL: fmt.Sprintf("%s/pulls/new/%s", repo.HTMLURL(), util.PathEscapeSegments(branch)),
})
} else {
results = append(results, private.HookPostReceiveBranchResult{
diff --git a/routers/web/repo/compare.go b/routers/web/repo/compare.go
index 3e9cdb5df8..4d4969fa87 100644
--- a/routers/web/repo/compare.go
+++ b/routers/web/repo/compare.go
@@ -569,19 +569,13 @@ func PrepareCompareDiff(
ctx *context.Context,
ci *common.CompareInfo,
whitespaceBehavior git.TrustedCmdArgs,
-) bool {
- var (
- repo = ctx.Repo.Repository
- err error
- title string
- )
-
- // Get diff information.
- ctx.Data["CommitRepoLink"] = ci.HeadRepo.Link()
-
+) (nothingToCompare bool) {
+ repo := ctx.Repo.Repository
headCommitID := ci.CompareInfo.HeadCommitID
+ ctx.Data["CommitRepoLink"] = ci.HeadRepo.Link()
ctx.Data["AfterCommitID"] = headCommitID
+ ctx.Data["ExpandNewPrForm"] = ctx.FormBool("expand")
if (headCommitID == ci.CompareInfo.MergeBase && !ci.DirectComparison) ||
headCommitID == ci.CompareInfo.BaseCommitID {
@@ -670,6 +664,7 @@ func PrepareCompareDiff(
ctx.Data["Commits"] = commits
ctx.Data["CommitCount"] = len(commits)
+ title := ci.HeadBranch
if len(commits) == 1 {
c := commits[0]
title = strings.TrimSpace(c.UserCommit.Summary())
@@ -678,9 +673,8 @@ func PrepareCompareDiff(
if len(body) > 1 {
ctx.Data["content"] = strings.Join(body[1:], "\n")
}
- } else {
- title = ci.HeadBranch
}
+
if len(title) > 255 {
var trailer string
title, trailer = util.EllipsisDisplayStringX(title, 255)
@@ -745,8 +739,7 @@ func CompareDiff(ctx *context.Context) {
ctx.Data["OtherCompareSeparator"] = "..."
}
- nothingToCompare := PrepareCompareDiff(ctx, ci,
- gitdiff.GetWhitespaceFlag(ctx.Data["WhitespaceBehavior"].(string)))
+ nothingToCompare := PrepareCompareDiff(ctx, ci, gitdiff.GetWhitespaceFlag(ctx.Data["WhitespaceBehavior"].(string)))
if ctx.Written() {
return
}
diff --git a/routers/web/repo/pull.go b/routers/web/repo/pull.go
index e12798f93d..c72664f8e9 100644
--- a/routers/web/repo/pull.go
+++ b/routers/web/repo/pull.go
@@ -1269,6 +1269,21 @@ func stopTimerIfAvailable(ctx *context.Context, user *user_model.User, issue *is
return nil
}
+func PullsNewRedirect(ctx *context.Context) {
+ branch := ctx.PathParam("*")
+ redirectRepo := ctx.Repo.Repository
+ repo := ctx.Repo.Repository
+ if repo.IsFork {
+ if err := repo.GetBaseRepo(ctx); err != nil {
+ ctx.ServerError("GetBaseRepo", err)
+ return
+ }
+ redirectRepo = repo.BaseRepo
+ branch = fmt.Sprintf("%s:%s", repo.OwnerName, branch)
+ }
+ ctx.Redirect(fmt.Sprintf("%s/compare/%s...%s?expand=1", redirectRepo.Link(), util.PathEscapeSegments(redirectRepo.DefaultBranch), util.PathEscapeSegments(branch)))
+}
+
// CompareAndPullRequestPost response for creating pull request
func CompareAndPullRequestPost(ctx *context.Context) {
form := web.GetForm(ctx).(*forms.CreateIssueForm)
diff --git a/routers/web/web.go b/routers/web/web.go
index f4bd3ef4bc..463f486250 100644
--- a/routers/web/web.go
+++ b/routers/web/web.go
@@ -1185,6 +1185,7 @@ func registerRoutes(m *web.Router) {
m.Combo("/compare/*", repo.MustBeNotEmpty, repo.SetEditorconfigIfExists).
Get(repo.SetDiffViewStyle, repo.SetWhitespaceBehavior, repo.CompareDiff).
Post(reqSignIn, context.RepoMustNotBeArchived(), reqUnitPullsReader, repo.MustAllowPulls, web.Bind(forms.CreateIssueForm{}), repo.SetWhitespaceBehavior, repo.CompareAndPullRequestPost)
+ m.Get("/pulls/new/*", repo.PullsNewRedirect)
}, optSignIn, context.RepoAssignment, reqUnitCodeReader)
// end "/{username}/{reponame}": repo code: find, compare, list
diff --git a/templates/repo/branch/list.tmpl b/templates/repo/branch/list.tmpl
index f4f3c2e5c5..c0accf16fa 100644
--- a/templates/repo/branch/list.tmpl
+++ b/templates/repo/branch/list.tmpl
@@ -128,13 +128,13 @@
{{svg "octicon-git-pull-request"}} {{ctx.Locale.Tr "repo.branch.included"}}
</span>
{{else if and (not .DBBranch.IsDeleted) $.AllowsPulls (gt .CommitsAhead 0)}}
- <a href="{{$.RepoLink}}/compare/{{PathEscapeSegments $.DefaultBranchBranch.DBBranch.Name}}...{{if ne $.Repository.Owner.Name $.Owner.Name}}{{PathEscape $.Owner.Name}}:{{end}}{{PathEscapeSegments .DBBranch.Name}}">
+ <a href="{{$.RepoLink}}/compare/{{PathEscapeSegments $.DefaultBranchBranch.DBBranch.Name}}...{{if ne $.Repository.Owner.Name $.Owner.Name}}{{PathEscape $.Owner.Name}}:{{end}}{{PathEscapeSegments .DBBranch.Name}}?expand=1">
<button id="new-pull-request" class="ui compact basic button tw-mr-0">{{if $.CanPull}}{{ctx.Locale.Tr "repo.pulls.compare_changes"}}{{else}}{{ctx.Locale.Tr "action.compare_branch"}}{{end}}</button>
</a>
{{end}}
{{else if and .LatestPullRequest.HasMerged .MergeMovedOn}}
{{if and (not .DBBranch.IsDeleted) $.AllowsPulls (gt .CommitsAhead 0)}}
- <a href="{{$.RepoLink}}/compare/{{PathEscapeSegments $.DefaultBranchBranch.DBBranch.Name}}...{{if ne $.Repository.Owner.Name $.Owner.Name}}{{PathEscape $.Owner.Name}}:{{end}}{{PathEscapeSegments .DBBranch.Name}}">
+ <a href="{{$.RepoLink}}/compare/{{PathEscapeSegments $.DefaultBranchBranch.DBBranch.Name}}...{{if ne $.Repository.Owner.Name $.Owner.Name}}{{PathEscape $.Owner.Name}}:{{end}}{{PathEscapeSegments .DBBranch.Name}}?expand=1">
<button id="new-pull-request" class="ui compact basic button tw-mr-0">{{if $.CanPull}}{{ctx.Locale.Tr "repo.pulls.compare_changes"}}{{else}}{{ctx.Locale.Tr "action.compare_branch"}}{{end}}</button>
</a>
{{end}}
diff --git a/templates/repo/code/recently_pushed_new_branches.tmpl b/templates/repo/code/recently_pushed_new_branches.tmpl
index f0edf6065b..4a864ba756 100644
--- a/templates/repo/code/recently_pushed_new_branches.tmpl
+++ b/templates/repo/code/recently_pushed_new_branches.tmpl
@@ -5,7 +5,7 @@
{{$branchLink := HTMLFormat `<a href="%s">%s</a>` .BranchLink .BranchDisplayName}}
{{ctx.Locale.Tr "repo.pulls.recently_pushed_new_branches" $branchLink $timeSince}}
</div>
- <a role="button" class="ui compact green button tw-m-0" href="{{.BranchCompareURL}}">
+ <a role="button" class="ui compact green button tw-m-0" href="{{QueryBuild .BranchCompareURL "expand" 1}}">
{{ctx.Locale.Tr "repo.pulls.compare_changes"}}
</a>
</div>
diff --git a/templates/repo/diff/compare.tmpl b/templates/repo/diff/compare.tmpl
index 9a7a04a328..05cfffd2b7 100644
--- a/templates/repo/diff/compare.tmpl
+++ b/templates/repo/diff/compare.tmpl
@@ -205,10 +205,10 @@
{{end}}
</div>
{{else if $allowCreatePR}}
- <div class="ui info message pullrequest-form-toggle {{if .Flash}}tw-hidden{{end}}">
+ <div class="ui info message pullrequest-form-toggle {{if .ExpandNewPrForm}}tw-hidden{{end}}">
<button class="ui button primary show-panel toggle" data-panel=".pullrequest-form-toggle, .pullrequest-form">{{ctx.Locale.Tr "repo.pulls.new"}}</button>
</div>
- <div class="pullrequest-form {{if not .Flash}}tw-hidden{{end}}">
+ <div class="pullrequest-form {{if not .ExpandNewPrForm}}tw-hidden{{end}}">
{{template "repo/issue/new_form" .}}
</div>
{{end}}
diff --git a/templates/repo/view_content.tmpl b/templates/repo/view_content.tmpl
index 06e9f8515c..292a2f878c 100644
--- a/templates/repo/view_content.tmpl
+++ b/templates/repo/view_content.tmpl
@@ -30,7 +30,7 @@
{{end}}
{{$cmpBranch = print $cmpBranch (.BranchName|PathEscapeSegments)}}
{{$compareLink := printf "%s/compare/%s...%s" .BaseRepo.Link (.BaseRepo.DefaultBranch|PathEscapeSegments) $cmpBranch}}
- <a id="new-pull-request" role="button" class="ui compact basic button" href="{{$compareLink}}"
+ <a id="new-pull-request" role="button" class="ui compact basic button" href="{{QueryBuild $compareLink "expand" 1}}"
data-tooltip-content="{{if .PullRequestCtx.Allowed}}{{ctx.Locale.Tr "repo.pulls.compare_changes"}}{{else}}{{ctx.Locale.Tr "action.compare_branch"}}{{end}}">
{{svg "octicon-git-pull-request"}}
</a>
diff --git a/tests/integration/pull_compare_test.go b/tests/integration/pull_compare_test.go
index 106774aa54..4ac5be18be 100644
--- a/tests/integration/pull_compare_test.go
+++ b/tests/integration/pull_compare_test.go
@@ -24,23 +24,38 @@ import (
func TestPullCompare(t *testing.T) {
defer tests.PrepareTestEnv(t)()
- session := loginUser(t, "user2")
- req := NewRequest(t, "GET", "/user2/repo1/pulls")
- resp := session.MakeRequest(t, req, http.StatusOK)
- htmlDoc := NewHTMLParser(t, resp.Body)
- link, exists := htmlDoc.doc.Find(".new-pr-button").Attr("href")
- assert.True(t, exists, "The template has changed")
+ t.Run("PullsNewRedirect", func(t *testing.T) {
+ req := NewRequest(t, "GET", "/user2/repo1/pulls/new/foo")
+ resp := MakeRequest(t, req, http.StatusSeeOther)
+ redirect := test.RedirectURL(resp)
+ assert.Equal(t, "/user2/repo1/compare/master...foo?expand=1", redirect)
- req = NewRequest(t, "GET", link)
- resp = session.MakeRequest(t, req, http.StatusOK)
- assert.EqualValues(t, http.StatusOK, resp.Code)
+ req = NewRequest(t, "GET", "/user13/repo11/pulls/new/foo")
+ resp = MakeRequest(t, req, http.StatusSeeOther)
+ redirect = test.RedirectURL(resp)
+ assert.Equal(t, "/user12/repo10/compare/master...user13:foo?expand=1", redirect)
+ })
- // test the edit button in the PR diff view
- req = NewRequest(t, "GET", "/user2/repo1/pulls/3/files")
- resp = session.MakeRequest(t, req, http.StatusOK)
- doc := NewHTMLParser(t, resp.Body)
- editButtonCount := doc.doc.Find(".diff-file-header-actions a[href*='/_edit/']").Length()
- assert.Positive(t, editButtonCount, "Expected to find a button to edit a file in the PR diff view but there were none")
+ t.Run("ButtonsExist", func(t *testing.T) {
+ session := loginUser(t, "user2")
+
+ // test the "New PR" button
+ req := NewRequest(t, "GET", "/user2/repo1/pulls")
+ resp := session.MakeRequest(t, req, http.StatusOK)
+ htmlDoc := NewHTMLParser(t, resp.Body)
+ link, exists := htmlDoc.doc.Find(".new-pr-button").Attr("href")
+ assert.True(t, exists, "The template has changed")
+ req = NewRequest(t, "GET", link)
+ resp = session.MakeRequest(t, req, http.StatusOK)
+ assert.EqualValues(t, http.StatusOK, resp.Code)
+
+ // test the edit button in the PR diff view
+ req = NewRequest(t, "GET", "/user2/repo1/pulls/3/files")
+ resp = session.MakeRequest(t, req, http.StatusOK)
+ doc := NewHTMLParser(t, resp.Body)
+ editButtonCount := doc.doc.Find(".diff-file-header-actions a[href*='/_edit/']").Length()
+ assert.Positive(t, editButtonCount, "Expected to find a button to edit a file in the PR diff view but there were none")
+ })
onGiteaRun(t, func(t *testing.T, u *url.URL) {
defer tests.PrepareTestEnv(t)()
@@ -54,8 +69,8 @@ func TestPullCompare(t *testing.T) {
repo1 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{OwnerName: "user2", Name: "repo1"})
issueIndex := unittest.AssertExistsAndLoadBean(t, &issues_model.IssueIndex{GroupID: repo1.ID}, unittest.OrderBy("group_id ASC"))
prFilesURL := fmt.Sprintf("/user2/repo1/pulls/%d/files", issueIndex.MaxIndex)
- req = NewRequest(t, "GET", prFilesURL)
- resp = session.MakeRequest(t, req, http.StatusOK)
+ req := NewRequest(t, "GET", prFilesURL)
+ resp := session.MakeRequest(t, req, http.StatusOK)
doc := NewHTMLParser(t, resp.Body)
editButtonCount := doc.doc.Find(".diff-file-header-actions a[href*='/_edit/']").Length()
assert.Positive(t, editButtonCount, "Expected to find a button to edit a file in the PR diff view but there were none")