Skip to content

Commit b1e326d

Browse files
wxiaoguangwxiaoguang
authored
Auto expand "New PR" form (#33971)
Follow GitHub's behavior: use `?expand=1` to expand the "New PR" form
1 parent 25b6f38 commit b1e326d

File tree

9 files changed

+64
-43
lines changed

9 files changed

+64
-43
lines changed

‎routers/private/hook_post_receive.go

+1-4
Original file line numberDiff line numberDiff line change
@@ -303,14 +303,11 @@ func HookPostReceive(ctx *gitea_context.PrivateContext) {
303303
}
304304

305305
if pr == nil {
306-
if repo.IsFork {
307-
branch = fmt.Sprintf("%s:%s", repo.OwnerName, branch)
308-
}
309306
results = append(results, private.HookPostReceiveBranchResult{
310307
Message: setting.Git.PullRequestPushMessage && baseRepo.AllowsPulls(ctx),
311308
Create: true,
312309
Branch: branch,
313-
URL: fmt.Sprintf("%s/compare/%s...%s", baseRepo.HTMLURL(), util.PathEscapeSegments(baseRepo.DefaultBranch), util.PathEscapeSegments(branch)),
310+
URL: fmt.Sprintf("%s/pulls/new/%s", repo.HTMLURL(), util.PathEscapeSegments(branch)),
314311
})
315312
} else {
316313
results = append(results, private.HookPostReceiveBranchResult{

‎routers/web/repo/compare.go

+7-14
Original file line numberDiff line numberDiff line change
@@ -569,19 +569,13 @@ func PrepareCompareDiff(
569569
ctx *context.Context,
570570
ci *common.CompareInfo,
571571
whitespaceBehavior git.TrustedCmdArgs,
572-
) bool {
573-
var (
574-
repo = ctx.Repo.Repository
575-
err error
576-
title string
577-
)
578-
579-
// Get diff information.
580-
ctx.Data["CommitRepoLink"] = ci.HeadRepo.Link()
581-
572+
) (nothingToCompare bool) {
573+
repo := ctx.Repo.Repository
582574
headCommitID := ci.CompareInfo.HeadCommitID
583575

576+
ctx.Data["CommitRepoLink"] = ci.HeadRepo.Link()
584577
ctx.Data["AfterCommitID"] = headCommitID
578+
ctx.Data["ExpandNewPrForm"] = ctx.FormBool("expand")
585579

586580
if (headCommitID == ci.CompareInfo.MergeBase && !ci.DirectComparison) ||
587581
headCommitID == ci.CompareInfo.BaseCommitID {
@@ -670,6 +664,7 @@ func PrepareCompareDiff(
670664
ctx.Data["Commits"] = commits
671665
ctx.Data["CommitCount"] = len(commits)
672666

667+
title := ci.HeadBranch
673668
if len(commits) == 1 {
674669
c := commits[0]
675670
title = strings.TrimSpace(c.UserCommit.Summary())
@@ -678,9 +673,8 @@ func PrepareCompareDiff(
678673
if len(body) > 1 {
679674
ctx.Data["content"] = strings.Join(body[1:], "\n")
680675
}
681-
} else {
682-
title = ci.HeadBranch
683676
}
677+
684678
if len(title) > 255 {
685679
var trailer string
686680
title, trailer = util.EllipsisDisplayStringX(title, 255)
@@ -745,8 +739,7 @@ func CompareDiff(ctx *context.Context) {
745739
ctx.Data["OtherCompareSeparator"] = "..."
746740
}
747741

748-
nothingToCompare := PrepareCompareDiff(ctx, ci,
749-
gitdiff.GetWhitespaceFlag(ctx.Data["WhitespaceBehavior"].(string)))
742+
nothingToCompare := PrepareCompareDiff(ctx, ci, gitdiff.GetWhitespaceFlag(ctx.Data["WhitespaceBehavior"].(string)))
750743
if ctx.Written() {
751744
return
752745
}

‎routers/web/repo/pull.go

+15
Original file line numberDiff line numberDiff line change
@@ -1269,6 +1269,21 @@ func stopTimerIfAvailable(ctx *context.Context, user *user_model.User, issue *is
12691269
return nil
12701270
}
12711271

1272+
func PullsNewRedirect(ctx *context.Context) {
1273+
branch := ctx.PathParam("*")
1274+
redirectRepo := ctx.Repo.Repository
1275+
repo := ctx.Repo.Repository
1276+
if repo.IsFork {
1277+
if err := repo.GetBaseRepo(ctx); err != nil {
1278+
ctx.ServerError("GetBaseRepo", err)
1279+
return
1280+
}
1281+
redirectRepo = repo.BaseRepo
1282+
branch = fmt.Sprintf("%s:%s", repo.OwnerName, branch)
1283+
}
1284+
ctx.Redirect(fmt.Sprintf("%s/compare/%s...%s?expand=1", redirectRepo.Link(), util.PathEscapeSegments(redirectRepo.DefaultBranch), util.PathEscapeSegments(branch)))
1285+
}
1286+
12721287
// CompareAndPullRequestPost response for creating pull request
12731288
func CompareAndPullRequestPost(ctx *context.Context) {
12741289
form := web.GetForm(ctx).(*forms.CreateIssueForm)

‎routers/web/web.go

+1
Original file line numberDiff line numberDiff line change
@@ -1185,6 +1185,7 @@ func registerRoutes(m *web.Router) {
11851185
m.Combo("/compare/*", repo.MustBeNotEmpty, repo.SetEditorconfigIfExists).
11861186
Get(repo.SetDiffViewStyle, repo.SetWhitespaceBehavior, repo.CompareDiff).
11871187
Post(reqSignIn, context.RepoMustNotBeArchived(), reqUnitPullsReader, repo.MustAllowPulls, web.Bind(forms.CreateIssueForm{}), repo.SetWhitespaceBehavior, repo.CompareAndPullRequestPost)
1188+
m.Get("/pulls/new/*", repo.PullsNewRedirect)
11881189
}, optSignIn, context.RepoAssignment, reqUnitCodeReader)
11891190
// end "/{username}/{reponame}": repo code: find, compare, list
11901191

‎templates/repo/branch/list.tmpl

+2-2
Original file line numberDiff line numberDiff line change
@@ -128,13 +128,13 @@
128128
{{svg "octicon-git-pull-request"}} {{ctx.Locale.Tr "repo.branch.included"}}
129129
</span>
130130
{{else if and (not .DBBranch.IsDeleted) $.AllowsPulls (gt .CommitsAhead 0)}}
131-
<a href="{{$.RepoLink}}/compare/{{PathEscapeSegments $.DefaultBranchBranch.DBBranch.Name}}...{{if ne $.Repository.Owner.Name $.Owner.Name}}{{PathEscape $.Owner.Name}}:{{end}}{{PathEscapeSegments .DBBranch.Name}}">
131+
<a href="{{$.RepoLink}}/compare/{{PathEscapeSegments $.DefaultBranchBranch.DBBranch.Name}}...{{if ne $.Repository.Owner.Name $.Owner.Name}}{{PathEscape $.Owner.Name}}:{{end}}{{PathEscapeSegments .DBBranch.Name}}?expand=1">
132132
<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>
133133
</a>
134134
{{end}}
135135
{{else if and .LatestPullRequest.HasMerged .MergeMovedOn}}
136136
{{if and (not .DBBranch.IsDeleted) $.AllowsPulls (gt .CommitsAhead 0)}}
137-
<a href="{{$.RepoLink}}/compare/{{PathEscapeSegments $.DefaultBranchBranch.DBBranch.Name}}...{{if ne $.Repository.Owner.Name $.Owner.Name}}{{PathEscape $.Owner.Name}}:{{end}}{{PathEscapeSegments .DBBranch.Name}}">
137+
<a href="{{$.RepoLink}}/compare/{{PathEscapeSegments $.DefaultBranchBranch.DBBranch.Name}}...{{if ne $.Repository.Owner.Name $.Owner.Name}}{{PathEscape $.Owner.Name}}:{{end}}{{PathEscapeSegments .DBBranch.Name}}?expand=1">
138138
<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>
139139
</a>
140140
{{end}}

‎templates/repo/code/recently_pushed_new_branches.tmpl

+1-1
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
{{$branchLink := HTMLFormat `<a href="%s">%s</a>` .BranchLink .BranchDisplayName}}
66
{{ctx.Locale.Tr "repo.pulls.recently_pushed_new_branches" $branchLink $timeSince}}
77
</div>
8-
<a role="button" class="ui compact green button tw-m-0" href="{{.BranchCompareURL}}">
8+
<a role="button" class="ui compact green button tw-m-0" href="{{QueryBuild .BranchCompareURL "expand" 1}}">
99
{{ctx.Locale.Tr "repo.pulls.compare_changes"}}
1010
</a>
1111
</div>

‎templates/repo/diff/compare.tmpl

+2-2
Original file line numberDiff line numberDiff line change
@@ -205,10 +205,10 @@
205205
{{end}}
206206
</div>
207207
{{else if $allowCreatePR}}
208-
<div class="ui info message pullrequest-form-toggle {{if .Flash}}tw-hidden{{end}}">
208+
<div class="ui info message pullrequest-form-toggle {{if .ExpandNewPrForm}}tw-hidden{{end}}">
209209
<button class="ui button primary show-panel toggle" data-panel=".pullrequest-form-toggle, .pullrequest-form">{{ctx.Locale.Tr "repo.pulls.new"}}</button>
210210
</div>
211-
<div class="pullrequest-form {{if not .Flash}}tw-hidden{{end}}">
211+
<div class="pullrequest-form {{if not .ExpandNewPrForm}}tw-hidden{{end}}">
212212
{{template "repo/issue/new_form" .}}
213213
</div>
214214
{{end}}

‎templates/repo/view_content.tmpl

+1-1
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@
3030
{{end}}
3131
{{$cmpBranch = print $cmpBranch (.BranchName|PathEscapeSegments)}}
3232
{{$compareLink := printf "%s/compare/%s...%s" .BaseRepo.Link (.BaseRepo.DefaultBranch|PathEscapeSegments) $cmpBranch}}
33-
<a id="new-pull-request" role="button" class="ui compact basic button" href="{{$compareLink}}"
33+
<a id="new-pull-request" role="button" class="ui compact basic button" href="{{QueryBuild $compareLink "expand" 1}}"
3434
data-tooltip-content="{{if .PullRequestCtx.Allowed}}{{ctx.Locale.Tr "repo.pulls.compare_changes"}}{{else}}{{ctx.Locale.Tr "action.compare_branch"}}{{end}}">
3535
{{svg "octicon-git-pull-request"}}
3636
</a>

‎tests/integration/pull_compare_test.go

+34-19
Original file line numberDiff line numberDiff line change
@@ -24,23 +24,38 @@ import (
2424
func TestPullCompare(t *testing.T) {
2525
defer tests.PrepareTestEnv(t)()
2626

27-
session := loginUser(t, "user2")
28-
req := NewRequest(t, "GET", "/user2/repo1/pulls")
29-
resp := session.MakeRequest(t, req, http.StatusOK)
30-
htmlDoc := NewHTMLParser(t, resp.Body)
31-
link, exists := htmlDoc.doc.Find(".new-pr-button").Attr("href")
32-
assert.True(t, exists, "The template has changed")
33-
34-
req = NewRequest(t, "GET", link)
35-
resp = session.MakeRequest(t, req, http.StatusOK)
36-
assert.EqualValues(t, http.StatusOK, resp.Code)
37-
38-
// test the edit button in the PR diff view
39-
req = NewRequest(t, "GET", "/user2/repo1/pulls/3/files")
40-
resp = session.MakeRequest(t, req, http.StatusOK)
41-
doc := NewHTMLParser(t, resp.Body)
42-
editButtonCount := doc.doc.Find(".diff-file-header-actions a[href*='/_edit/']").Length()
43-
assert.Positive(t, editButtonCount, "Expected to find a button to edit a file in the PR diff view but there were none")
27+
t.Run("PullsNewRedirect", func(t *testing.T) {
28+
req := NewRequest(t, "GET", "/user2/repo1/pulls/new/foo")
29+
resp := MakeRequest(t, req, http.StatusSeeOther)
30+
redirect := test.RedirectURL(resp)
31+
assert.Equal(t, "/user2/repo1/compare/master...foo?expand=1", redirect)
32+
33+
req = NewRequest(t, "GET", "/user13/repo11/pulls/new/foo")
34+
resp = MakeRequest(t, req, http.StatusSeeOther)
35+
redirect = test.RedirectURL(resp)
36+
assert.Equal(t, "/user12/repo10/compare/master...user13:foo?expand=1", redirect)
37+
})
38+
39+
t.Run("ButtonsExist", func(t *testing.T) {
40+
session := loginUser(t, "user2")
41+
42+
// test the "New PR" button
43+
req := NewRequest(t, "GET", "/user2/repo1/pulls")
44+
resp := session.MakeRequest(t, req, http.StatusOK)
45+
htmlDoc := NewHTMLParser(t, resp.Body)
46+
link, exists := htmlDoc.doc.Find(".new-pr-button").Attr("href")
47+
assert.True(t, exists, "The template has changed")
48+
req = NewRequest(t, "GET", link)
49+
resp = session.MakeRequest(t, req, http.StatusOK)
50+
assert.EqualValues(t, http.StatusOK, resp.Code)
51+
52+
// test the edit button in the PR diff view
53+
req = NewRequest(t, "GET", "/user2/repo1/pulls/3/files")
54+
resp = session.MakeRequest(t, req, http.StatusOK)
55+
doc := NewHTMLParser(t, resp.Body)
56+
editButtonCount := doc.doc.Find(".diff-file-header-actions a[href*='/_edit/']").Length()
57+
assert.Positive(t, editButtonCount, "Expected to find a button to edit a file in the PR diff view but there were none")
58+
})
4459

4560
onGiteaRun(t, func(t *testing.T, u *url.URL) {
4661
defer tests.PrepareTestEnv(t)()
@@ -54,8 +69,8 @@ func TestPullCompare(t *testing.T) {
5469
repo1 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{OwnerName: "user2", Name: "repo1"})
5570
issueIndex := unittest.AssertExistsAndLoadBean(t, &issues_model.IssueIndex{GroupID: repo1.ID}, unittest.OrderBy("group_id ASC"))
5671
prFilesURL := fmt.Sprintf("/user2/repo1/pulls/%d/files", issueIndex.MaxIndex)
57-
req = NewRequest(t, "GET", prFilesURL)
58-
resp = session.MakeRequest(t, req, http.StatusOK)
72+
req := NewRequest(t, "GET", prFilesURL)
73+
resp := session.MakeRequest(t, req, http.StatusOK)
5974
doc := NewHTMLParser(t, resp.Body)
6075
editButtonCount := doc.doc.Find(".diff-file-header-actions a[href*='/_edit/']").Length()
6176
assert.Positive(t, editButtonCount, "Expected to find a button to edit a file in the PR diff view but there were none")

0 commit comments

Comments
 (0)