Make backend code respond correct JSON when creating PR (#25353)

Fix #25351
This commit is contained in:
wxiaoguang 2023-06-19 16:25:36 +08:00 committed by GitHub
parent c09d0b4952
commit b4e4b7ad51
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
4 changed files with 25 additions and 50 deletions

View file

@ -37,7 +37,6 @@ import (
"code.gitea.io/gitea/modules/upload" "code.gitea.io/gitea/modules/upload"
"code.gitea.io/gitea/modules/util" "code.gitea.io/gitea/modules/util"
"code.gitea.io/gitea/modules/web" "code.gitea.io/gitea/modules/web"
"code.gitea.io/gitea/modules/web/middleware"
"code.gitea.io/gitea/routers/utils" "code.gitea.io/gitea/routers/utils"
asymkey_service "code.gitea.io/gitea/services/asymkey" asymkey_service "code.gitea.io/gitea/services/asymkey"
"code.gitea.io/gitea/services/automerge" "code.gitea.io/gitea/services/automerge"
@ -1206,36 +1205,12 @@ func CompareAndPullRequestPost(ctx *context.Context) {
} }
if ctx.HasError() { if ctx.HasError() {
middleware.AssignForm(form, ctx.Data) ctx.JSONError(ctx.GetErrMsg())
// This stage is already stop creating new pull request, so it does not matter if it has
// something to compare or not.
PrepareCompareDiff(ctx, ci,
gitdiff.GetWhitespaceFlag(ctx.Data["WhitespaceBehavior"].(string)))
if ctx.Written() {
return
}
if len(form.Title) > 255 {
var trailer string
form.Title, trailer = util.SplitStringAtByteN(form.Title, 255)
form.Content = trailer + "\n\n" + form.Content
}
middleware.AssignForm(form, ctx.Data)
ctx.HTML(http.StatusOK, tplCompareDiff)
return return
} }
if util.IsEmptyString(form.Title) { if util.IsEmptyString(form.Title) {
PrepareCompareDiff(ctx, ci, ctx.JSONError(ctx.Tr("repo.issues.new.title_empty"))
gitdiff.GetWhitespaceFlag(ctx.Data["WhitespaceBehavior"].(string)))
if ctx.Written() {
return
}
ctx.RenderWithErr(ctx.Tr("repo.issues.new.title_empty"), tplCompareDiff, form)
return return
} }
@ -1278,20 +1253,20 @@ func CompareAndPullRequestPost(ctx *context.Context) {
pushrejErr := err.(*git.ErrPushRejected) pushrejErr := err.(*git.ErrPushRejected)
message := pushrejErr.Message message := pushrejErr.Message
if len(message) == 0 { if len(message) == 0 {
ctx.Flash.Error(ctx.Tr("repo.pulls.push_rejected_no_message")) ctx.JSONError(ctx.Tr("repo.pulls.push_rejected_no_message"))
} else { return
flashError, err := ctx.RenderToString(tplAlertDetails, map[string]interface{}{
"Message": ctx.Tr("repo.pulls.push_rejected"),
"Summary": ctx.Tr("repo.pulls.push_rejected_summary"),
"Details": utils.SanitizeFlashErrorString(pushrejErr.Message),
})
if err != nil {
ctx.ServerError("CompareAndPullRequest.HTMLString", err)
return
}
ctx.Flash.Error(flashError)
} }
ctx.Redirect(pullIssue.Link()) flashError, err := ctx.RenderToString(tplAlertDetails, map[string]interface{}{
"Message": ctx.Tr("repo.pulls.push_rejected"),
"Summary": ctx.Tr("repo.pulls.push_rejected_summary"),
"Details": utils.SanitizeFlashErrorString(pushrejErr.Message),
})
if err != nil {
ctx.ServerError("CompareAndPullRequest.HTMLString", err)
return
}
ctx.Flash.Error(flashError)
ctx.JSONRedirect(pullIssue.Link()) // FIXME: it's unfriendly, and will make the content lost
return return
} }
ctx.ServerError("NewPullRequest", err) ctx.ServerError("NewPullRequest", err)
@ -1299,7 +1274,7 @@ func CompareAndPullRequestPost(ctx *context.Context) {
} }
log.Trace("Pull request created: %d/%d", repo.ID, pullIssue.ID) log.Trace("Pull request created: %d/%d", repo.ID, pullIssue.ID)
ctx.Redirect(pullIssue.Link()) ctx.JSONRedirect(pullIssue.Link())
} }
// CleanUpPullRequest responses for delete merged branch when PR has been merged // CleanUpPullRequest responses for delete merged branch when PR has been merged

View file

@ -11,6 +11,7 @@ import (
"strings" "strings"
"testing" "testing"
"code.gitea.io/gitea/modules/test"
"code.gitea.io/gitea/tests" "code.gitea.io/gitea/tests"
"github.com/stretchr/testify/assert" "github.com/stretchr/testify/assert"
@ -39,8 +40,7 @@ func testPullCreate(t *testing.T, session *TestSession, user, repo, branch, titl
"_csrf": htmlDoc.GetCSRF(), "_csrf": htmlDoc.GetCSRF(),
"title": title, "title": title,
}) })
resp = session.MakeRequest(t, req, http.StatusSeeOther) resp = session.MakeRequest(t, req, http.StatusOK)
return resp return resp
} }
@ -52,7 +52,7 @@ func TestPullCreate(t *testing.T) {
resp := testPullCreate(t, session, "user1", "repo1", "master", "This is a pull title") resp := testPullCreate(t, session, "user1", "repo1", "master", "This is a pull title")
// check the redirected URL // check the redirected URL
url := resp.Header().Get("Location") url := test.RedirectURL(resp)
assert.Regexp(t, "^/user2/repo1/pulls/[0-9]*$", url) assert.Regexp(t, "^/user2/repo1/pulls/[0-9]*$", url)
// check .diff can be accessed and matches performed change // check .diff can be accessed and matches performed change
@ -80,7 +80,7 @@ func TestPullCreate_TitleEscape(t *testing.T) {
resp := testPullCreate(t, session, "user1", "repo1", "master", "<i>XSS PR</i>") resp := testPullCreate(t, session, "user1", "repo1", "master", "<i>XSS PR</i>")
// check the redirected URL // check the redirected URL
url := resp.Header().Get("Location") url := test.RedirectURL(resp)
assert.Regexp(t, "^/user2/repo1/pulls/[0-9]*$", url) assert.Regexp(t, "^/user2/repo1/pulls/[0-9]*$", url)
// Edit title // Edit title
@ -145,7 +145,7 @@ func TestPullBranchDelete(t *testing.T) {
resp := testPullCreate(t, session, "user1", "repo1", "master1", "This is a pull title") resp := testPullCreate(t, session, "user1", "repo1", "master1", "This is a pull title")
// check the redirected URL // check the redirected URL
url := resp.Header().Get("Location") url := test.RedirectURL(resp)
assert.Regexp(t, "^/user2/repo1/pulls/[0-9]*$", url) assert.Regexp(t, "^/user2/repo1/pulls/[0-9]*$", url)
req := NewRequest(t, "GET", url) req := NewRequest(t, "GET", url)
session.MakeRequest(t, req, http.StatusOK) session.MakeRequest(t, req, http.StatusOK)

View file

@ -199,7 +199,7 @@ func TestCantMergeWorkInProgress(t *testing.T) {
resp := testPullCreate(t, session, "user1", "repo1", "master", "[wip] This is a pull title") resp := testPullCreate(t, session, "user1", "repo1", "master", "[wip] This is a pull title")
req := NewRequest(t, "GET", resp.Header().Get("Location")) req := NewRequest(t, "GET", test.RedirectURL(resp))
resp = session.MakeRequest(t, req, http.StatusOK) resp = session.MakeRequest(t, req, http.StatusOK)
htmlDoc := NewHTMLParser(t, resp.Body) htmlDoc := NewHTMLParser(t, resp.Body)
text := strings.TrimSpace(htmlDoc.doc.Find(".merge-section > .item").Last().Text()) text := strings.TrimSpace(htmlDoc.doc.Find(".merge-section > .item").Last().Text())

View file

@ -30,7 +30,7 @@ func TestPullCreate_CommitStatus(t *testing.T) {
"title": "pull request from status1", "title": "pull request from status1",
}, },
) )
session.MakeRequest(t, req, http.StatusSeeOther) session.MakeRequest(t, req, http.StatusOK)
req = NewRequest(t, "GET", "/user1/repo1/pulls") req = NewRequest(t, "GET", "/user1/repo1/pulls")
resp := session.MakeRequest(t, req, http.StatusOK) resp := session.MakeRequest(t, req, http.StatusOK)
@ -127,7 +127,7 @@ func TestPullCreate_EmptyChangesWithDifferentCommits(t *testing.T) {
"title": "pull request from status1", "title": "pull request from status1",
}, },
) )
session.MakeRequest(t, req, http.StatusSeeOther) session.MakeRequest(t, req, http.StatusOK)
req = NewRequest(t, "GET", "/user1/repo1/pulls/1") req = NewRequest(t, "GET", "/user1/repo1/pulls/1")
resp := session.MakeRequest(t, req, http.StatusOK) resp := session.MakeRequest(t, req, http.StatusOK)
@ -150,7 +150,7 @@ func TestPullCreate_EmptyChangesWithSameCommits(t *testing.T) {
"title": "pull request from status1", "title": "pull request from status1",
}, },
) )
session.MakeRequest(t, req, http.StatusSeeOther) session.MakeRequest(t, req, http.StatusOK)
req = NewRequest(t, "GET", "/user1/repo1/pulls/1") req = NewRequest(t, "GET", "/user1/repo1/pulls/1")
resp := session.MakeRequest(t, req, http.StatusOK) resp := session.MakeRequest(t, req, http.StatusOK)
doc := NewHTMLParser(t, resp.Body) doc := NewHTMLParser(t, resp.Body)