Refactor pull request view (5) (#37517)

Clean up templates, remove various CSS patches.

By the way, fix incorrect NewRequest URLs in tests.
This commit is contained in:
wxiaoguang
2026-05-04 02:53:24 +08:00
committed by GitHub
parent c4c50bee7f
commit f26f71f1b2
23 changed files with 144 additions and 167 deletions
@@ -149,7 +149,7 @@ func TestActionsArtifactDownload(t *testing.T) {
assert.Contains(t, listResp.Value[artifactIdx].FileContainerResourceURL, "/api/actions_pipeline/_apis/pipelines/workflows/791/artifacts")
idx := strings.Index(listResp.Value[artifactIdx].FileContainerResourceURL, "/api/actions_pipeline/_apis/pipelines/")
url := listResp.Value[artifactIdx].FileContainerResourceURL[idx+1:] + "?itemPath=artifact-download"
url := listResp.Value[artifactIdx].FileContainerResourceURL[idx:] + "?itemPath=artifact-download"
req = NewRequest(t, "GET", url).
AddTokenAuth("8061e833a55f6fc0157c98b883e91fcfeeb1a71a")
resp = MakeRequest(t, req, http.StatusOK)
@@ -245,7 +245,7 @@ func TestActionsArtifactDownloadMultiFiles(t *testing.T) {
assert.Contains(t, fileContainerResourceURL, "/api/actions_pipeline/_apis/pipelines/workflows/791/artifacts")
idx := strings.Index(fileContainerResourceURL, "/api/actions_pipeline/_apis/pipelines/")
url := fileContainerResourceURL[idx+1:] + "?itemPath=" + testArtifactName
url := fileContainerResourceURL[idx:] + "?itemPath=" + testArtifactName
req = NewRequest(t, "GET", url).
AddTokenAuth("8061e833a55f6fc0157c98b883e91fcfeeb1a71a")
resp = MakeRequest(t, req, http.StatusOK)
@@ -323,7 +323,7 @@ func TestActionsArtifactOverwrite(t *testing.T) {
listResp := DecodeJSON(t, resp, &listArtifactsResponse{})
idx := strings.Index(listResp.Value[0].FileContainerResourceURL, "/api/actions_pipeline/_apis/pipelines/")
url := listResp.Value[0].FileContainerResourceURL[idx+1:] + "?itemPath=artifact-download"
url := listResp.Value[0].FileContainerResourceURL[idx:] + "?itemPath=artifact-download"
req = NewRequest(t, "GET", url).
AddTokenAuth("8061e833a55f6fc0157c98b883e91fcfeeb1a71a")
resp = MakeRequest(t, req, http.StatusOK)
@@ -380,7 +380,7 @@ func TestActionsArtifactOverwrite(t *testing.T) {
assert.Equal(t, "artifact-download", uploadedItem.Name)
idx := strings.Index(uploadedItem.FileContainerResourceURL, "/api/actions_pipeline/_apis/pipelines/")
url := uploadedItem.FileContainerResourceURL[idx+1:] + "?itemPath=artifact-download"
url := uploadedItem.FileContainerResourceURL[idx:] + "?itemPath=artifact-download"
req = NewRequest(t, "GET", url).
AddTokenAuth("8061e833a55f6fc0157c98b883e91fcfeeb1a71a")
resp = MakeRequest(t, req, http.StatusOK)
+1 -1
View File
@@ -348,7 +348,7 @@ func TestAPIUpdateBranchReference(t *testing.T) {
func testAPIRenameBranch(t *testing.T, doerName, ownerName, repoName, from, to string, expectedHTTPStatus int) *httptest.ResponseRecorder {
token := getUserToken(t, doerName, auth_model.AccessTokenScopeWriteRepository)
req := NewRequestWithJSON(t, "PATCH", "api/v1/repos/"+ownerName+"/"+repoName+"/branches/"+from, &api.RenameBranchRepoOption{
req := NewRequestWithJSON(t, "PATCH", "/api/v1/repos/"+ownerName+"/"+repoName+"/branches/"+from, &api.RenameBranchRepoOption{
Name: to,
}).AddTokenAuth(token)
return MakeRequest(t, req, expectedHTTPStatus)
+6 -6
View File
@@ -96,18 +96,18 @@ func testUploadAttachmentDeleteTemp(t *testing.T) {
defer web.RouteMock(route_web.RouterMockPointBeforeWebRoutes, func(resp http.ResponseWriter, req *http.Request) {
tmpFileCountDuringUpload = countTmpFile()
})()
_ = testCreateIssueAttachment(t, session, "user2/repo1", "image.png", testGeneratePngBytes(), http.StatusOK)
_ = testCreateIssueAttachment(t, session, "/user2/repo1", "image.png", testGeneratePngBytes(), http.StatusOK)
assert.Equal(t, 1, tmpFileCountDuringUpload, "the temp file should exist when uploaded size exceeds the parse form's max memory")
assert.Equal(t, 0, countTmpFile(), "the temp file should be deleted after upload")
}
func testCreateAnonymousAttachment(t *testing.T) {
session := emptyTestSession(t)
testCreateIssueAttachment(t, session, "user2/repo1", "image.png", testGeneratePngBytes(), http.StatusSeeOther)
testCreateIssueAttachment(t, session, "/user2/repo1", "image.png", testGeneratePngBytes(), http.StatusSeeOther)
}
func testCreateUser2IssueAttachment(t *testing.T) {
const repoURL = "user2/repo1"
const repoURL = "/user2/repo1"
session := loginUser(t, "user2")
uuid := testCreateIssueAttachment(t, session, repoURL, "image.png", testGeneratePngBytes(), http.StatusOK)
@@ -177,7 +177,7 @@ func testGetAttachment(t *testing.T) {
}
func testDeleteAttachmentPermissions(t *testing.T) {
const repoURL = "user2/repo1"
const repoURL = "/user2/repo1"
ownerSession := loginUser(t, "user2")
readonlySession := loginUser(t, "user5")
@@ -191,12 +191,12 @@ func testDeleteAttachmentPermissions(t *testing.T) {
testCreateReleaseAttachment(t, readonlySession, repoURL, "reader-release.png", testGeneratePngBytes(), http.StatusNotFound)
crossRepoUUID := testCreateIssueAttachment(t, ownerSession, repoURL, "cross-repo.png", testGeneratePngBytes(), http.StatusOK)
testDeleteIssueAttachment(t, ownerSession, "user2/repo2", crossRepoUUID, http.StatusBadRequest)
testDeleteIssueAttachment(t, ownerSession, "/user2/repo2", crossRepoUUID, http.StatusBadRequest)
testDeleteIssueAttachment(t, ownerSession, repoURL, crossRepoUUID, http.StatusOK)
releaseUUID := testCreateReleaseAttachment(t, ownerSession, repoURL, "reader-release.png", testGeneratePngBytes(), http.StatusOK)
testDeleteReleaseAttachment(t, ownerSession, repoURL, releaseUUID, http.StatusOK)
// test deleting release attachment from another repo
testDeleteReleaseAttachment(t, ownerSession, "user2/repo2", crossRepoUUID, http.StatusBadRequest)
testDeleteReleaseAttachment(t, ownerSession, "/user2/repo2", crossRepoUUID, http.StatusBadRequest)
}
+12 -12
View File
@@ -124,7 +124,7 @@ func testEditorActionEdit(t *testing.T, session *TestSession, user, repo, editor
resp := testEditorActionPostRequest(t, session, fmt.Sprintf("/%s/%s/%s/%s/%s", user, repo, editorAction, branch, filePath), params)
assert.Equal(t, http.StatusOK, resp.Code)
assert.NotEmpty(t, test.RedirectURL(resp))
req := NewRequest(t, "GET", path.Join(user, repo, "raw/branch", newBranchName, params["tree_path"]))
req := NewRequest(t, "GET", "/"+path.Join(user, repo, "raw/branch", newBranchName, params["tree_path"]))
resp = session.MakeRequest(t, req, http.StatusOK)
assert.Equal(t, params["content"], resp.Body.String())
return resp
@@ -330,18 +330,18 @@ index 0000000000..bbbbbbbbbb
func testForkToEditFile(t *testing.T, session *TestSession, user, owner, repo, branch, filePath string) {
forkToEdit := func(t *testing.T, session *TestSession, owner, repo, operation, branch, filePath string) {
// visit the base repo, see the "Add File" button
req := NewRequest(t, "GET", path.Join(owner, repo))
req := NewRequest(t, "GET", "/"+path.Join(owner, repo))
resp := session.MakeRequest(t, req, http.StatusOK)
htmlDoc := NewHTMLParser(t, resp.Body)
AssertHTMLElement(t, htmlDoc, ".repo-add-file", 1)
// attempt to edit a file, see the guideline page
req = NewRequest(t, "GET", path.Join(owner, repo, operation, branch, filePath))
req = NewRequest(t, "GET", "/"+path.Join(owner, repo, operation, branch, filePath))
resp = session.MakeRequest(t, req, http.StatusOK)
assert.Contains(t, resp.Body.String(), "Fork Repository to Propose Changes")
// fork the repository
req = NewRequest(t, "POST", path.Join(owner, repo, "_fork", branch))
req = NewRequest(t, "POST", "/"+path.Join(owner, repo, "_fork", branch))
resp = session.MakeRequest(t, req, http.StatusOK)
assert.JSONEq(t, `{"redirect":""}`, resp.Body.String())
}
@@ -351,7 +351,7 @@ func testForkToEditFile(t *testing.T, session *TestSession, user, owner, repo, b
forkToEdit(t, session, owner, repo, "_edit", branch, filePath)
// Archive the repository
req := NewRequestWithValues(t, "POST", path.Join(user, repo, "settings"),
req := NewRequestWithValues(t, "POST", "/"+path.Join(user, repo, "settings"),
map[string]string{
"repo_name": repo,
"action": "archive",
@@ -360,12 +360,12 @@ func testForkToEditFile(t *testing.T, session *TestSession, user, owner, repo, b
session.MakeRequest(t, req, http.StatusSeeOther)
// Check editing archived repository is disabled
req = NewRequest(t, "GET", path.Join(owner, repo, "_edit", branch, filePath)).SetHeader("Accept", "text/html")
req = NewRequest(t, "GET", "/"+path.Join(owner, repo, "_edit", branch, filePath)).SetHeader("Accept", "text/html")
resp := session.MakeRequest(t, req, http.StatusNotFound)
assert.Contains(t, resp.Body.String(), "You have forked this repository but your fork is not editable.")
// Unfork the repository
req = NewRequestWithValues(t, "POST", path.Join(user, repo, "settings"),
req = NewRequestWithValues(t, "POST", "/"+path.Join(user, repo, "settings"),
map[string]string{
"repo_name": repo,
"action": "convert_fork",
@@ -381,7 +381,7 @@ func testForkToEditFile(t *testing.T, session *TestSession, user, owner, repo, b
t.Run("CheckBaseRepoForm", func(t *testing.T) {
// the base repo's edit form should have the correct action and upload links (pointing to the forked repo)
req := NewRequest(t, "GET", path.Join(owner, repo, "_upload", branch, filePath)+"?foo=bar")
req := NewRequest(t, "GET", "/"+path.Join(owner, repo, "_upload", branch, filePath)+"?foo=bar")
resp := session.MakeRequest(t, req, http.StatusOK)
htmlDoc := NewHTMLParser(t, resp.Body)
@@ -399,7 +399,7 @@ func testForkToEditFile(t *testing.T, session *TestSession, user, owner, repo, b
})
t.Run("ViewBaseEditFormAndCommitToFork", func(t *testing.T) {
req := NewRequest(t, "GET", path.Join(owner, repo, "_edit", branch, filePath))
req := NewRequest(t, "GET", "/"+path.Join(owner, repo, "_edit", branch, filePath))
resp := session.MakeRequest(t, req, http.StatusOK)
htmlDoc := NewHTMLParser(t, resp.Body)
editRequestForm := map[string]string{
@@ -437,16 +437,16 @@ func testEditFileNotAllowed(t *testing.T) {
for _, operation := range operations {
t.Run(operation, func(t *testing.T) {
// Branch does not exist
targetLink := path.Join("user2", "repo1", operation, "missing", "README.md")
targetLink := path.Join("/user2/repo1", operation, "missing", "README.md")
sessionUser1.MakeRequest(t, NewRequest(t, "GET", targetLink), http.StatusNotFound)
// Private repository
targetLink = path.Join("user2", "repo2", operation, "master", "Home.md")
targetLink = path.Join("/user2/repo2", operation, "master", "Home.md")
sessionUser1.MakeRequest(t, NewRequest(t, "GET", targetLink), http.StatusOK)
sessionUser4.MakeRequest(t, NewRequest(t, "GET", targetLink), http.StatusNotFound)
// Empty repository
targetLink = path.Join("org41", "repo61", operation, "master", "README.md")
targetLink = path.Join("/org41/repo61", operation, "master", "README.md")
sessionUser1.MakeRequest(t, NewRequest(t, "GET", targetLink), http.StatusNotFound)
})
}
+6 -2
View File
@@ -326,14 +326,18 @@ func NewRequestWithJSON(t testing.TB, method, urlStr string, v any) *RequestWrap
func NewRequestWithBody(t testing.TB, method, urlStr string, body io.Reader) *RequestWrapper {
t.Helper()
if !strings.HasPrefix(urlStr, "http") && !strings.HasPrefix(urlStr, "/") {
urlStr = "/" + urlStr
if !strings.HasPrefix(urlStr, "http:") && !strings.HasPrefix(urlStr, "https:") && !strings.HasPrefix(urlStr, "/") {
t.Fatalf("invalid url str: %s", urlStr)
}
req, err := http.NewRequest(method, urlStr, body)
require.NoError(t, err)
if req.URL.User != nil {
req.Header.Set("Authorization", "Basic "+base64.StdEncoding.EncodeToString([]byte(req.URL.User.String())))
}
req.RequestURI = req.URL.Path
if req.URL.RawQuery != "" {
req.RequestURI += "?" + req.URL.RawQuery
}
return &RequestWrapper{req}
}
+3 -3
View File
@@ -123,7 +123,7 @@ func TestNoLoginViewIssue(t *testing.T) {
}
func testNewIssue(t *testing.T, session *TestSession, user, repo, title, content string) string {
req := NewRequest(t, "GET", path.Join(user, repo, "issues", "new"))
req := NewRequest(t, "GET", "/"+path.Join(user, repo, "issues", "new"))
resp := session.MakeRequest(t, req, http.StatusOK)
htmlDoc := NewHTMLParser(t, resp.Body)
@@ -667,7 +667,7 @@ func TestUpdateIssueDeadline(t *testing.T) {
assert.Equal(t, api.StateOpen, issueBefore.State())
session := loginUser(t, owner.Name)
urlStr := fmt.Sprintf("%s/%s/issues/%d/deadline", owner.Name, repoBefore.Name, issueBefore.Index)
urlStr := fmt.Sprintf("/%s/%s/issues/%d/deadline", owner.Name, repoBefore.Name, issueBefore.Index)
req := NewRequestWithValues(t, "POST", urlStr, map[string]string{"deadline": "2022-04-06"})
session.MakeRequest(t, req, http.StatusOK)
@@ -687,7 +687,7 @@ func TestIssueReferenceURL(t *testing.T) {
issue := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{ID: 1})
repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: issue.RepoID})
req := NewRequest(t, "GET", fmt.Sprintf("%s/issues/%d", repo.FullName(), issue.Index))
req := NewRequest(t, "GET", fmt.Sprintf("%s/issues/%d", repo.Link(), issue.Index))
resp := session.MakeRequest(t, req, http.StatusOK)
htmlDoc := NewHTMLParser(t, resp.Body)
+2 -2
View File
@@ -66,7 +66,7 @@ func testOrgProfile(t *testing.T, u *url.URL) {
createTestProfile(t, "org3", user.RepoNameProfilePrivate, contentPrivateReadme)
// Anonymous User
req := NewRequest(t, "GET", "org3")
req := NewRequest(t, "GET", "/org3")
resp := MakeRequest(t, req, http.StatusOK)
bodyString := util.UnsafeBytesToString(resp.Body.Bytes())
assert.Contains(t, bodyString, contentPublicReadme)
@@ -74,7 +74,7 @@ func testOrgProfile(t *testing.T, u *url.URL) {
// Logged in but not member
session := loginUser(t, "user24")
req = NewRequest(t, "GET", "org3")
req = NewRequest(t, "GET", "/org3")
resp = session.MakeRequest(t, req, http.StatusOK)
bodyString = util.UnsafeBytesToString(resp.Body.Bytes())
assert.Contains(t, bodyString, contentPublicReadme)
+1 -1
View File
@@ -26,7 +26,7 @@ import (
)
func testPullCreate(t *testing.T, session *TestSession, user, repo string, toSelf bool, targetBranch, sourceBranch, title string) *httptest.ResponseRecorder {
req := NewRequest(t, "GET", path.Join(user, repo))
req := NewRequest(t, "GET", "/"+path.Join(user, repo))
resp := session.MakeRequest(t, req, http.StatusOK)
// Click the PR button to create a pull
+3 -6
View File
@@ -80,7 +80,7 @@ func testPullMerge(t *testing.T, session *TestSession, user, repo, pullNum strin
}
func testPullCleanUp(t *testing.T, session *TestSession, user, repo, pullnum string) *httptest.ResponseRecorder {
req := NewRequest(t, "GET", path.Join(user, repo, "pulls", pullnum))
req := NewRequest(t, "GET", "/"+path.Join(user, repo, "pulls", pullnum))
resp := session.MakeRequest(t, req, http.StatusOK)
// Click the little button to create a pull
@@ -322,11 +322,8 @@ func TestCantMergeWorkInProgress(t *testing.T) {
req := NewRequest(t, "GET", test.RedirectURL(resp))
resp = session.MakeRequest(t, req, http.StatusOK)
htmlDoc := NewHTMLParser(t, resp.Body)
text := strings.TrimSpace(htmlDoc.doc.Find(".merge-section > .item").Last().Text())
assert.NotEmpty(t, text, "Can't find WIP text")
assert.Contains(t, text, translation.NewLocale("en-US").TrString("repo.pulls.cannot_merge_work_in_progress"), "Unable to find WIP text")
assert.Contains(t, text, "[wip]", "Unable to find WIP text")
wipToggleButtonCount := htmlDoc.Find(`.merge-section > .item button[data-global-init="initPullRequestWipToggle"]`).Length()
assert.Equal(t, 1, wipToggleButtonCount)
})
}
+2 -2
View File
@@ -264,13 +264,13 @@ func testSubmitReview(t *testing.T, session *TestSession, owner, repo, pullNumbe
"type": reviewType,
}
submitURL := path.Join(owner, repo, "pulls", pullNumber, "files", "reviews", "submit")
submitURL := "/" + path.Join(owner, repo, "pulls", pullNumber, "files", "reviews", "submit")
req := NewRequestWithValues(t, "POST", submitURL, options)
return session.MakeRequest(t, req, expectedSubmitStatus)
}
func testIssueClose(t *testing.T, session *TestSession, owner, repo, issueNumber string) *httptest.ResponseRecorder {
closeURL := path.Join(owner, repo, "issues", issueNumber, "comments")
closeURL := "/" + path.Join(owner, repo, "issues", issueNumber, "comments")
options := map[string]string{
"status": "close",
+3 -3
View File
@@ -31,7 +31,7 @@ func TestPullCreate_CommitStatus(t *testing.T) {
testRepoFork(t, session, "user2", "repo1", "user1", "repo1", "")
testEditFileToNewBranch(t, session, "user1", "repo1", "master", "status1", "README.md", "status1")
url := path.Join("user1", "repo1", "compare", "master...status1")
url := "/" + path.Join("user1", "repo1", "compare", "master...status1")
req := NewRequestWithValues(t, "POST", url,
map[string]string{
"title": "pull request from status1",
@@ -121,7 +121,7 @@ func TestPullCreate_EmptyChangesWithDifferentCommits(t *testing.T) {
testEditFileToNewBranch(t, session, "user1", "repo1", "master", "status1", "README.md", "status1")
testEditFile(t, session, "user1", "repo1", "status1", "README.md", "# repo1\n\nDescription for repo1")
url := path.Join("user1", "repo1", "compare", "master...status1")
url := "/" + path.Join("user1", "repo1", "compare", "master...status1")
req := NewRequestWithValues(t, "POST", url,
map[string]string{
"title": "pull request from status1",
@@ -143,7 +143,7 @@ func TestPullCreate_EmptyChangesWithSameCommits(t *testing.T) {
session := loginUser(t, "user1")
testRepoFork(t, session, "user2", "repo1", "user1", "repo1", "")
testCreateBranch(t, session, "user1", "repo1", "branch/master", "status1", http.StatusSeeOther)
url := path.Join("user1", "repo1", "compare", "master...status1")
url := "/" + path.Join("user1", "repo1", "compare", "master...status1")
req := NewRequestWithValues(t, "POST", url,
map[string]string{
"title": "pull request from status1",
+2 -2
View File
@@ -21,7 +21,7 @@ import (
)
func testCreateBranch(t testing.TB, session *TestSession, user, repo, oldRefSubURL, newBranchName string, expectedStatus int) string {
req := NewRequestWithValues(t, "POST", path.Join(user, repo, "branches/_new", oldRefSubURL), map[string]string{
req := NewRequestWithValues(t, "POST", "/"+path.Join(user, repo, "branches/_new", oldRefSubURL), map[string]string{
"new_branch_name": newBranchName,
})
resp := session.MakeRequest(t, req, expectedStatus)
@@ -221,7 +221,7 @@ func prepareRepoPR(t *testing.T, baseSession, headSession *TestSession, baseRepo
func checkRecentlyPushedNewBranches(t *testing.T, session *TestSession, repoPath string, expected []string) {
branches := make([]string, 0, 2)
req := NewRequest(t, "GET", repoPath)
req := NewRequest(t, "GET", "/"+repoPath)
resp := session.MakeRequest(t, req, http.StatusOK)
doc := NewHTMLParser(t, resp.Body)
doc.doc.Find(".ui.positive.message div a").Each(func(index int, branch *goquery.Selection) {
+2 -2
View File
@@ -37,7 +37,7 @@ func TestViewTimetrackingControls(t *testing.T) {
}
func testViewTimetrackingControls(t *testing.T, session *TestSession, user, repo, issue string, canTrackTime bool) {
req := NewRequest(t, "GET", path.Join(user, repo, "issues", issue))
req := NewRequest(t, "GET", "/"+path.Join(user, repo, "issues", issue))
resp := session.MakeRequest(t, req, http.StatusOK)
htmlDoc := NewHTMLParser(t, resp.Body)
@@ -45,7 +45,7 @@ func testViewTimetrackingControls(t *testing.T, session *TestSession, user, repo
AssertHTMLElement(t, htmlDoc, ".issue-start-time", canTrackTime)
AssertHTMLElement(t, htmlDoc, ".issue-add-time", canTrackTime)
issueLink := path.Join(user, repo, "issues", issue)
issueLink := "/" + path.Join(user, repo, "issues", issue)
reqStart := NewRequest(t, "POST", path.Join(issueLink, "times", "stopwatch", "start"))
if canTrackTime {
session.MakeRequest(t, reqStart, http.StatusOK)