0
0
mirror of https://github.com/go-gitea/gitea.git synced 2026-05-22 04:13:04 +02:00

fix(pull): handle empty pull request files view to allow reviews (#37783)

Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>
This commit is contained in:
Elisei Roca 2026-05-19 20:06:29 +02:00 committed by GitHub
parent 7e436972f9
commit 9c8d55daf8
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 55 additions and 35 deletions

View File

@ -683,6 +683,8 @@ func indexCommit(commits []*git.Commit, commitID string) *git.Commit {
// ViewPullFiles render pull request changed files list page
func viewPullFiles(ctx *context.Context, beforeCommitID, afterCommitID string) {
var err error
ctx.Data["PageIsPullList"] = true
ctx.Data["PageIsPullFiles"] = true
@ -707,44 +709,53 @@ func viewPullFiles(ctx *context.Context, beforeCommitID, afterCommitID string) {
headCommitID := prCompareInfo.HeadCommitID
isSingleCommit := beforeCommitID == "" && afterCommitID != ""
ctx.Data["IsShowingOnlySingleCommit"] = isSingleCommit
// FIXME: when afterCommitID==headCommitID, isSingleCommit and isShowAllCommits can be both true, which doesn't seem right
isShowAllCommits := (beforeCommitID == "" || beforeCommitID == prCompareInfo.CompareBase) && (afterCommitID == "" || afterCommitID == headCommitID)
ctx.Data["IsShowingOnlySingleCommit"] = isSingleCommit
ctx.Data["IsShowingAllCommits"] = isShowAllCommits
if afterCommitID == "" || afterCommitID == headCommitID {
afterCommitID = headCommitID
}
// "commits list" is half-open, half-closed: (base, head]
// * base commit is not in the list
// * if the PR is empty, the list is also empty (head commit is not in the list)
afterCommitID = util.IfZero(afterCommitID, headCommitID)
afterCommit := indexCommit(prCompareInfo.Commits, afterCommitID)
if afterCommit == nil && afterCommitID == headCommitID {
afterCommit, err = gitRepo.GetCommit(afterCommitID)
if err != nil {
ctx.ServerError("GetCommit(afterCommitID)", err)
return
}
}
if afterCommit == nil {
ctx.HTTPError(http.StatusBadRequest, "after commit not found in PR commits")
ctx.NotFound(nil)
return
}
var beforeCommit *git.Commit
var err error
if !isSingleCommit {
if beforeCommitID == "" || beforeCommitID == prCompareInfo.CompareBase {
beforeCommitID = prCompareInfo.CompareBase
// merge base commit is not in the list of the pull request commits
beforeCommit, err = gitRepo.GetCommit(beforeCommitID)
if err != nil {
ctx.ServerError("GetCommit", err)
return
}
} else {
beforeCommit = indexCommit(prCompareInfo.Commits, beforeCommitID)
if beforeCommit == nil {
ctx.HTTPError(http.StatusBadRequest, "before commit not found in PR commits")
return
}
}
} else {
if isSingleCommit {
beforeCommit, err = afterCommit.Parent(0)
if err != nil {
ctx.ServerError("Parent", err)
ctx.ServerError("afterCommit.Parent", err)
return
}
beforeCommitID = beforeCommit.ID.String()
} else {
beforeCommitID = util.IfZero(beforeCommitID, prCompareInfo.CompareBase)
beforeCommit = indexCommit(prCompareInfo.Commits, beforeCommitID)
if beforeCommit == nil && beforeCommitID == prCompareInfo.CompareBase {
// base commit is not in the list of the pull request commits
beforeCommit, err = gitRepo.GetCommit(beforeCommitID)
if err != nil {
ctx.ServerError("GetCommit(beforeCommitID)", err)
return
}
}
}
if beforeCommit == nil {
ctx.NotFound(nil)
return
}
ctx.Data["CompareInfo"] = prCompareInfo

View File

@ -140,20 +140,29 @@ func TestPullCreate_EmptyChangesWithDifferentCommits(t *testing.T) {
func TestPullCreate_EmptyChangesWithSameCommits(t *testing.T) {
onGiteaRun(t, func(t *testing.T, u *url.URL) {
session := loginUser(t, "user1")
testRepoFork(t, session, "user2", "repo1", "user1", "repo1", "")
testCreateBranch(t, session, "user1", "repo1", "branch/master", "status1", http.StatusSeeOther)
req := NewRequestWithValues(t, "POST", "/user1/repo1/compare/master...status1",
map[string]string{
"title": "pull request from status1",
},
)
session.MakeRequest(t, req, http.StatusOK)
req = NewRequest(t, "GET", "/user1/repo1/pulls/1")
resp := session.MakeRequest(t, req, http.StatusOK)
doc := NewHTMLParser(t, resp.Body)
testCreateBranch(t, session, "user2", "repo1", "branch/master", "empty-pr-branch", http.StatusSeeOther)
resp := testPullCreateDirectly(t, session, createPullRequestOptions{
BaseRepoOwner: "user2",
BaseRepoName: "repo1",
BaseBranch: "master",
HeadBranch: "empty-pr-branch",
Title: "empty pr test",
})
prURL := test.RedirectURL(resp)
// check the "merge box" text
req := NewRequest(t, "GET", prURL)
resp = session.MakeRequest(t, req, http.StatusOK)
doc := NewHTMLParser(t, resp.Body)
text := strings.TrimSpace(doc.doc.Find(".merge-section").Text())
assert.Contains(t, text, "This branch is already included in the target branch. There is nothing to merge.")
// check the "files" tab content
req = NewRequest(t, "GET", prURL+"/files")
resp = session.MakeRequest(t, req, http.StatusOK)
doc = NewHTMLParser(t, resp.Body)
assert.Equal(t, "Diff Content Not Available", strings.TrimSpace(doc.Find("#diff-container").Text()))
})
}