Skip to content

Commit 9744887

Browse files
lunnylunny
committed
Fix bug
1 parent c03055c commit 9744887

File tree

32 files changed

+186
-125
lines changed

32 files changed

+186
-125
lines changed

‎models/issues/comment_test.go

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -71,9 +71,14 @@ func TestFetchCodeComments(t *testing.T) {
7171
res, err := issues_model.FetchCodeComments(db.DefaultContext, issue.Repo, issue.ID, user, false)
7272
assert.NoError(t, err)
7373
assert.Contains(t, res, "README.md")
74-
assert.Contains(t, res["README.md"], int64(4))
75-
assert.Len(t, res["README.md"][4], 1)
76-
assert.Equal(t, int64(4), res["README.md"][0].ID)
74+
fourthLineComments := []*issues_model.Comment{}
75+
for _, comment := range res["README.md"] {
76+
if comment.Line == 4 {
77+
fourthLineComments = append(fourthLineComments, comment)
78+
}
79+
}
80+
assert.Len(t, fourthLineComments, 1)
81+
assert.Equal(t, int64(4), fourthLineComments[0].ID)
7782

7883
user2 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2})
7984
res, err = issues_model.FetchCodeComments(db.DefaultContext, issue.Repo, issue.ID, user2, false)

‎models/issues/pull.go

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -392,11 +392,16 @@ func (pr *PullRequest) getReviewedByLines(ctx context.Context, writer io.Writer)
392392
return committer.Commit()
393393
}
394394

395-
// GetGitRefName returns git ref for hidden pull request branch
396-
func (pr *PullRequest) GetGitRefName() string {
395+
// GetGitHeadRefName returns git head commit id ref for the pull request's branch
396+
func (pr *PullRequest) GetGitHeadRefName() string {
397397
return fmt.Sprintf("%s%d/head", git.PullPrefix, pr.Index)
398398
}
399399

400+
// GetGitMergeRefName returns git merged commit id ref for the pull request
401+
func (pr *PullRequest) GetGitMergeRefName() string {
402+
return fmt.Sprintf("%s%d/merge", git.PullPrefix, pr.Index)
403+
}
404+
400405
func (pr *PullRequest) GetGitHeadBranchRefName() string {
401406
return fmt.Sprintf("%s%s", git.BranchPrefix, pr.HeadBranch)
402407
}

‎modules/migration/pullrequest.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,8 +49,8 @@ func (p *PullRequest) IsForkPullRequest() bool {
4949
return p.Head.RepoFullName() != p.Base.RepoFullName()
5050
}
5151

52-
// GetGitRefName returns pull request relative path to head
53-
func (p PullRequest) GetGitRefName() string {
52+
// GetGitHeadRefName returns pull request relative path to head
53+
func (p PullRequest) GetGitHeadRefName() string {
5454
return fmt.Sprintf("%s%d/head", git.PullPrefix, p.Number)
5555
}
5656

‎routers/api/v1/repo/pull.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1461,9 +1461,9 @@ func GetPullRequestCommits(ctx *context.APIContext) {
14611461
defer closer.Close()
14621462

14631463
if pr.HasMerged {
1464-
prInfo, err = baseGitRepo.GetCompareInfo(pr.BaseRepo.RepoPath(), pr.MergeBase, pr.GetGitRefName(), false, false)
1464+
prInfo, err = baseGitRepo.GetCompareInfo(pr.BaseRepo.RepoPath(), pr.MergeBase, pr.GetGitHeadRefName(), false, false)
14651465
} else {
1466-
prInfo, err = baseGitRepo.GetCompareInfo(pr.BaseRepo.RepoPath(), pr.BaseBranch, pr.GetGitRefName(), false, false)
1466+
prInfo, err = baseGitRepo.GetCompareInfo(pr.BaseRepo.RepoPath(), pr.BaseBranch, pr.GetGitHeadRefName(), false, false)
14671467
}
14681468
if err != nil {
14691469
ctx.APIErrorInternal(err)
@@ -1584,16 +1584,16 @@ func GetPullRequestFiles(ctx *context.APIContext) {
15841584

15851585
var prInfo *git.CompareInfo
15861586
if pr.HasMerged {
1587-
prInfo, err = baseGitRepo.GetCompareInfo(pr.BaseRepo.RepoPath(), pr.MergeBase, pr.GetGitRefName(), true, false)
1587+
prInfo, err = baseGitRepo.GetCompareInfo(pr.BaseRepo.RepoPath(), pr.MergeBase, pr.GetGitHeadRefName(), true, false)
15881588
} else {
1589-
prInfo, err = baseGitRepo.GetCompareInfo(pr.BaseRepo.RepoPath(), pr.BaseBranch, pr.GetGitRefName(), true, false)
1589+
prInfo, err = baseGitRepo.GetCompareInfo(pr.BaseRepo.RepoPath(), pr.BaseBranch, pr.GetGitHeadRefName(), true, false)
15901590
}
15911591
if err != nil {
15921592
ctx.APIErrorInternal(err)
15931593
return
15941594
}
15951595

1596-
headCommitID, err := baseGitRepo.GetRefCommitID(pr.GetGitRefName())
1596+
headCommitID, err := baseGitRepo.GetRefCommitID(pr.GetGitHeadRefName())
15971597
if err != nil {
15981598
ctx.APIErrorInternal(err)
15991599
return

‎routers/api/v1/repo/pull_review.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -328,7 +328,7 @@ func CreatePullReview(ctx *context.APIContext) {
328328

329329
// if CommitID is empty, set it as lastCommitID
330330
if opts.CommitID == "" {
331-
headCommitID, err := ctx.Repo.GitRepo.GetRefCommitID(pr.GetGitRefName())
331+
headCommitID, err := ctx.Repo.GitRepo.GetRefCommitID(pr.GetGitHeadRefName())
332332
if err != nil {
333333
ctx.APIErrorInternal(err)
334334
return
@@ -448,7 +448,7 @@ func SubmitPullReview(ctx *context.APIContext) {
448448
return
449449
}
450450

451-
headCommitID, err := ctx.Repo.GitRepo.GetRefCommitID(pr.GetGitRefName())
451+
headCommitID, err := ctx.Repo.GitRepo.GetRefCommitID(pr.GetGitHeadRefName())
452452
if err != nil {
453453
ctx.APIErrorInternal(err)
454454
return

‎routers/web/repo/issue_comment.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,7 @@ func NewComment(ctx *context.Context) {
103103
// check whether the ref of PR <refs/pulls/pr_index/head> in base repo is consistent with the head commit of head branch in the head repo
104104
// get head commit of PR
105105
if pull.Flow == issues_model.PullRequestFlowGithub {
106-
prHeadRef := pull.GetGitRefName()
106+
prHeadRef := pull.GetGitHeadRefName()
107107
if err := pull.LoadBaseRepo(ctx); err != nil {
108108
ctx.ServerError("Unable to load base repo", err)
109109
return

‎routers/web/repo/issue_view.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -492,7 +492,7 @@ func preparePullViewSigning(ctx *context.Context, issue *issues_model.Issue) {
492492
pull := issue.PullRequest
493493
ctx.Data["WillSign"] = false
494494
if ctx.Doer != nil {
495-
sign, key, _, err := asymkey_service.SignMerge(ctx, pull, ctx.Doer, pull.BaseRepo.RepoPath(), pull.BaseBranch, pull.GetGitRefName())
495+
sign, key, _, err := asymkey_service.SignMerge(ctx, pull, ctx.Doer, pull.BaseRepo.RepoPath(), pull.BaseBranch, pull.GetGitHeadRefName())
496496
ctx.Data["WillSign"] = sign
497497
ctx.Data["SigningKey"] = key
498498
if err != nil {

‎routers/web/repo/pull.go

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -196,7 +196,7 @@ func GetPullDiffStats(ctx *context.Context) {
196196
}
197197

198198
// do not report 500 server error to end users if error occurs, otherwise a PR missing ref won't be able to view.
199-
headCommitID, err := ctx.Repo.GitRepo.GetRefCommitID(pull.GetGitRefName())
199+
headCommitID, err := ctx.Repo.GitRepo.GetRefCommitID(pull.GetGitHeadRefName())
200200
if err != nil {
201201
log.Error("Failed to GetRefCommitID: %v, repo: %v", err, ctx.Repo.Repository.FullName())
202202
return
@@ -249,7 +249,7 @@ func prepareMergedViewPullInfo(ctx *context.Context, issue *issues_model.Issue)
249249
}
250250

251251
compareInfo, err := ctx.Repo.GitRepo.GetCompareInfo(ctx.Repo.Repository.RepoPath(),
252-
baseCommit, pull.GetGitRefName(), false, false)
252+
baseCommit, pull.GetGitHeadRefName(), false, false)
253253
if err != nil {
254254
if strings.Contains(err.Error(), "fatal: Not a valid object name") || strings.Contains(err.Error(), "unknown revision or path not in the working tree") {
255255
ctx.Data["IsPullRequestBroken"] = true
@@ -329,9 +329,9 @@ func prepareViewPullInfo(ctx *context.Context, issue *issues_model.Issue) *git.C
329329
ctx.Data["BaseTarget"] = pull.BaseBranch
330330
ctx.Data["HeadTarget"] = pull.HeadBranch
331331

332-
sha, err := baseGitRepo.GetRefCommitID(pull.GetGitRefName())
332+
sha, err := baseGitRepo.GetRefCommitID(pull.GetGitHeadRefName())
333333
if err != nil {
334-
ctx.ServerError(fmt.Sprintf("GetRefCommitID(%s)", pull.GetGitRefName()), err)
334+
ctx.ServerError(fmt.Sprintf("GetRefCommitID(%s)", pull.GetGitHeadRefName()), err)
335335
return nil
336336
}
337337
commitStatuses, err := git_model.GetLatestCommitStatus(ctx, repo.ID, sha, db.ListOptionsAll)
@@ -349,7 +349,7 @@ func prepareViewPullInfo(ctx *context.Context, issue *issues_model.Issue) *git.C
349349
}
350350

351351
compareInfo, err := baseGitRepo.GetCompareInfo(pull.BaseRepo.RepoPath(),
352-
pull.MergeBase, pull.GetGitRefName(), false, false)
352+
pull.MergeBase, pull.GetGitHeadRefName(), false, false)
353353
if err != nil {
354354
if strings.Contains(err.Error(), "fatal: Not a valid object name") {
355355
ctx.Data["IsPullRequestBroken"] = true
@@ -382,12 +382,12 @@ func prepareViewPullInfo(ctx *context.Context, issue *issues_model.Issue) *git.C
382382
if pull.Flow == issues_model.PullRequestFlowGithub {
383383
headBranchExist = gitrepo.IsBranchExist(ctx, pull.HeadRepo, pull.HeadBranch)
384384
} else {
385-
headBranchExist = gitrepo.IsReferenceExist(ctx, pull.BaseRepo, pull.GetGitRefName())
385+
headBranchExist = gitrepo.IsReferenceExist(ctx, pull.BaseRepo, pull.GetGitHeadRefName())
386386
}
387387

388388
if headBranchExist {
389389
if pull.Flow != issues_model.PullRequestFlowGithub {
390-
headBranchSha, err = baseGitRepo.GetRefCommitID(pull.GetGitRefName())
390+
headBranchSha, err = baseGitRepo.GetRefCommitID(pull.GetGitHeadRefName())
391391
} else {
392392
headBranchSha, err = headGitRepo.GetBranchCommitID(pull.HeadBranch)
393393
}
@@ -410,7 +410,7 @@ func prepareViewPullInfo(ctx *context.Context, issue *issues_model.Issue) *git.C
410410
ctx.Data["GetCommitMessages"] = ""
411411
}
412412

413-
sha, err := baseGitRepo.GetRefCommitID(pull.GetGitRefName())
413+
sha, err := baseGitRepo.GetRefCommitID(pull.GetGitHeadRefName())
414414
if err != nil {
415415
if git.IsErrNotExist(err) {
416416
ctx.Data["IsPullRequestBroken"] = true
@@ -426,7 +426,7 @@ func prepareViewPullInfo(ctx *context.Context, issue *issues_model.Issue) *git.C
426426
ctx.Data["NumFiles"] = 0
427427
return nil
428428
}
429-
ctx.ServerError(fmt.Sprintf("GetRefCommitID(%s)", pull.GetGitRefName()), err)
429+
ctx.ServerError(fmt.Sprintf("GetRefCommitID(%s)", pull.GetGitHeadRefName()), err)
430430
return nil
431431
}
432432

@@ -497,7 +497,7 @@ func prepareViewPullInfo(ctx *context.Context, issue *issues_model.Issue) *git.C
497497
}
498498

499499
compareInfo, err := baseGitRepo.GetCompareInfo(pull.BaseRepo.RepoPath(),
500-
git.BranchPrefix+pull.BaseBranch, pull.GetGitRefName(), false, false)
500+
git.BranchPrefix+pull.BaseBranch, pull.GetGitHeadRefName(), false, false)
501501
if err != nil {
502502
if strings.Contains(err.Error(), "fatal: Not a valid object name") {
503503
ctx.Data["IsPullRequestBroken"] = true
@@ -647,7 +647,7 @@ func viewPullFiles(ctx *context.Context, beforeCommitID, afterCommitID string) {
647647
return
648648
}
649649

650-
headCommitID, err := gitRepo.GetRefCommitID(pull.GetGitRefName())
650+
headCommitID, err := gitRepo.GetRefCommitID(pull.GetGitHeadRefName())
651651
if err != nil {
652652
ctx.ServerError("GetRefCommitID", err)
653653
return
@@ -1462,7 +1462,7 @@ func CleanUpPullRequest(ctx *context.Context) {
14621462
}()
14631463

14641464
// Check if branch has no new commits
1465-
headCommitID, err := gitBaseRepo.GetRefCommitID(pr.GetGitRefName())
1465+
headCommitID, err := gitBaseRepo.GetRefCommitID(pr.GetGitHeadRefName())
14661466
if err != nil {
14671467
log.Error("GetRefCommitID: %v", err)
14681468
ctx.Flash.Error(ctx.Tr("repo.branch.deletion_failed", fullBranchName))

‎routers/web/repo/pull_review.go

Lines changed: 39 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -7,11 +7,13 @@ import (
77
"errors"
88
"fmt"
99
"net/http"
10+
"slices"
1011

1112
issues_model "code.gitea.io/gitea/models/issues"
1213
"code.gitea.io/gitea/models/organization"
1314
pull_model "code.gitea.io/gitea/models/pull"
1415
user_model "code.gitea.io/gitea/models/user"
16+
"code.gitea.io/gitea/modules/git"
1517
"code.gitea.io/gitea/modules/json"
1618
"code.gitea.io/gitea/modules/log"
1719
"code.gitea.io/gitea/modules/setting"
@@ -170,20 +172,50 @@ func renderConversation(ctx *context.Context, comment *issues_model.Comment, ori
170172
ctx.ServerError("comment.Issue.LoadPullRequest", err)
171173
return
172174
}
173-
if beforeCommitID == "" {
174-
beforeCommitID = comment.Issue.PullRequest.MergeBase
175+
176+
headCommitID, err := ctx.Repo.GitRepo.GetRefCommitID(comment.Issue.PullRequest.GetGitHeadRefName())
177+
if err != nil {
178+
ctx.ServerError("GetRefCommitID", err)
179+
return
175180
}
176-
if afterCommitID == "" {
177-
var err error
178-
afterCommitID, err = ctx.Repo.GitRepo.GetRefCommitID(comment.Issue.PullRequest.GetGitRefName())
181+
182+
prCommitIDs, err := git.CommitIDsBetween(ctx, ctx.Repo.GitRepo.Path, comment.Issue.PullRequest.MergeBase, headCommitID)
183+
if err != nil {
184+
ctx.ServerError("CommitIDsBetween", err)
185+
return
186+
}
187+
188+
if beforeCommitID == "" || beforeCommitID == comment.Issue.PullRequest.MergeBase {
189+
beforeCommitID = comment.Issue.PullRequest.MergeBase
190+
} else {
191+
// beforeCommitID must be one of the pull request commits
192+
if !slices.Contains(prCommitIDs, beforeCommitID) {
193+
ctx.HTTPError(http.StatusBadRequest, fmt.Sprintf("beforeCommitID[%s] is not a valid pull request commit", beforeCommitID))
194+
return
195+
}
196+
197+
beforeCommit, err := ctx.Repo.GitRepo.GetCommit(beforeCommitID)
179198
if err != nil {
180-
ctx.ServerError("GetRefCommitID", err)
199+
ctx.ServerError("GetCommit", err)
181200
return
182201
}
202+
203+
beforeCommit, err = beforeCommit.Parent(0)
204+
if err != nil {
205+
ctx.ServerError("GetParent", err)
206+
return
207+
}
208+
beforeCommitID = beforeCommit.ID.String()
209+
}
210+
if afterCommitID == "" {
211+
afterCommitID = headCommitID
212+
} else if !slices.Contains(prCommitIDs, afterCommitID) { // afterCommitID must be one of the pull request commits
213+
ctx.HTTPError(http.StatusBadRequest, fmt.Sprintf("afterCommitID[%s] is not a valid pull request commit", afterCommitID))
214+
return
183215
}
184216

185217
showOutdatedComments := origin == "timeline" || ctx.Data["ShowOutdatedComments"].(bool)
186-
comments, err := pull_service.FetchCodeCommentsByLine(ctx, ctx.Repo.Repository, comment.IssueID,
218+
comments, err := pull_service.FetchCodeCommentsByLine(ctx, ctx.Repo.GitRepo, ctx.Repo.Repository, comment.IssueID,
187219
ctx.Doer, beforeCommitID, afterCommitID, comment.TreePath, comment.Line, showOutdatedComments)
188220
if err != nil {
189221
ctx.ServerError("FetchCodeCommentsByLine", err)

‎services/actions/notifier_helper.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,7 @@ func (input *notifyInput) WithPayload(payload api.Payloader) *notifyInput {
104104
func (input *notifyInput) WithPullRequest(pr *issues_model.PullRequest) *notifyInput {
105105
input.PullRequest = pr
106106
if input.Ref == "" {
107-
input.Ref = git.RefName(pr.GetGitRefName())
107+
input.Ref = git.RefName(pr.GetGitHeadRefName())
108108
}
109109
return input
110110
}

0 commit comments

Comments
 (0)