Skip to content

Commit 345ae04

Browse files
lunnylunny
committed
Allow code review comments display cross commits even if the head branch has a force-push
1 parent 3531e9d commit 345ae04

File tree

24 files changed

+559
-381
lines changed

24 files changed

+559
-381
lines changed

‎models/issues/comment.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1007,6 +1007,7 @@ type FindCommentsOptions struct {
10071007
RepoID int64
10081008
IssueID int64
10091009
ReviewID int64
1010+
CommitSHA string
10101011
Since int64
10111012
Before int64
10121013
Line int64
@@ -1052,6 +1053,9 @@ func (opts FindCommentsOptions) ToConds() builder.Cond {
10521053
if opts.IsPull.Has() {
10531054
cond = cond.And(builder.Eq{"issue.is_pull": opts.IsPull.Value()})
10541055
}
1056+
if opts.CommitSHA != "" {
1057+
cond = cond.And(builder.Eq{"comment.commit_sha": opts.CommitSHA})
1058+
}
10551059
return cond
10561060
}
10571061

‎models/issues/comment_code.go

Lines changed: 21 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -9,46 +9,52 @@ import (
99

1010
"code.gitea.io/gitea/models/db"
1111
"code.gitea.io/gitea/models/renderhelper"
12+
repo_model "code.gitea.io/gitea/models/repo"
1213
user_model "code.gitea.io/gitea/models/user"
1314
"code.gitea.io/gitea/modules/markup/markdown"
1415

1516
"xorm.io/builder"
1617
)
1718

1819
// CodeComments represents comments on code by using this structure: FILENAME -> LINE (+ == proposed; - == previous) -> COMMENTS
19-
type CodeComments map[string]map[int64][]*Comment
20+
type CodeComments map[string][]*Comment
21+
22+
func (cc CodeComments) AllComments() []*Comment {
23+
var allComments []*Comment
24+
for _, comments := range cc {
25+
allComments = append(allComments, comments...)
26+
}
27+
return allComments
28+
}
2029

2130
// FetchCodeComments will return a 2d-map: ["Path"]["Line"] = Comments at line
22-
func FetchCodeComments(ctx context.Context, issue *Issue, currentUser *user_model.User, showOutdatedComments bool) (CodeComments, error) {
23-
return fetchCodeCommentsByReview(ctx, issue, currentUser, nil, showOutdatedComments)
31+
func FetchCodeComments(ctx context.Context, repo *repo_model.Repository, issueID int64, currentUser *user_model.User, showOutdatedComments bool) (CodeComments, error) {
32+
return fetchCodeCommentsByReview(ctx, repo, issueID, currentUser, nil, showOutdatedComments)
2433
}
2534

26-
func fetchCodeCommentsByReview(ctx context.Context, issue *Issue, currentUser *user_model.User, review *Review, showOutdatedComments bool) (CodeComments, error) {
27-
pathToLineToComment := make(CodeComments)
35+
func fetchCodeCommentsByReview(ctx context.Context, repo *repo_model.Repository, issueID int64, currentUser *user_model.User, review *Review, showOutdatedComments bool) (CodeComments, error) {
36+
codeCommentsPathMap := make(CodeComments)
2837
if review == nil {
2938
review = &Review{ID: 0}
3039
}
3140
opts := FindCommentsOptions{
3241
Type: CommentTypeCode,
33-
IssueID: issue.ID,
42+
IssueID: issueID,
3443
ReviewID: review.ID,
3544
}
3645

37-
comments, err := findCodeComments(ctx, opts, issue, currentUser, review, showOutdatedComments)
46+
comments, err := FindCodeComments(ctx, opts, repo, currentUser, review, showOutdatedComments)
3847
if err != nil {
3948
return nil, err
4049
}
4150

4251
for _, comment := range comments {
43-
if pathToLineToComment[comment.TreePath] == nil {
44-
pathToLineToComment[comment.TreePath] = make(map[int64][]*Comment)
45-
}
46-
pathToLineToComment[comment.TreePath][comment.Line] = append(pathToLineToComment[comment.TreePath][comment.Line], comment)
52+
codeCommentsPathMap[comment.TreePath] = append(codeCommentsPathMap[comment.TreePath], comment)
4753
}
48-
return pathToLineToComment, nil
54+
return codeCommentsPathMap, nil
4955
}
5056

51-
func findCodeComments(ctx context.Context, opts FindCommentsOptions, issue *Issue, currentUser *user_model.User, review *Review, showOutdatedComments bool) ([]*Comment, error) {
57+
func FindCodeComments(ctx context.Context, opts FindCommentsOptions, repo *repo_model.Repository, currentUser *user_model.User, review *Review, showOutdatedComments bool) ([]*Comment, error) {
5258
var comments CommentList
5359
if review == nil {
5460
review = &Review{ID: 0}
@@ -67,10 +73,6 @@ func findCodeComments(ctx context.Context, opts FindCommentsOptions, issue *Issu
6773
return nil, err
6874
}
6975

70-
if err := issue.LoadRepo(ctx); err != nil {
71-
return nil, err
72-
}
73-
7476
if err := comments.LoadPosters(ctx); err != nil {
7577
return nil, err
7678
}
@@ -110,12 +112,12 @@ func findCodeComments(ctx context.Context, opts FindCommentsOptions, issue *Issu
110112
return nil, err
111113
}
112114

113-
if err := comment.LoadReactions(ctx, issue.Repo); err != nil {
115+
if err := comment.LoadReactions(ctx, repo); err != nil {
114116
return nil, err
115117
}
116118

117119
var err error
118-
rctx := renderhelper.NewRenderContextRepoComment(ctx, issue.Repo, renderhelper.RepoCommentOptions{
120+
rctx := renderhelper.NewRenderContextRepoComment(ctx, repo, renderhelper.RepoCommentOptions{
119121
FootnoteContextID: strconv.FormatInt(comment.ID, 10),
120122
})
121123
if comment.RenderedContent, err = markdown.RenderString(rctx, comment.Content); err != nil {
@@ -124,14 +126,3 @@ func findCodeComments(ctx context.Context, opts FindCommentsOptions, issue *Issu
124126
}
125127
return comments[:n], nil
126128
}
127-
128-
// FetchCodeCommentsByLine fetches the code comments for a given treePath and line number
129-
func FetchCodeCommentsByLine(ctx context.Context, issue *Issue, currentUser *user_model.User, treePath string, line int64, showOutdatedComments bool) (CommentList, error) {
130-
opts := FindCommentsOptions{
131-
Type: CommentTypeCode,
132-
IssueID: issue.ID,
133-
TreePath: treePath,
134-
Line: line,
135-
}
136-
return findCodeComments(ctx, opts, issue, currentUser, nil, showOutdatedComments)
137-
}

‎models/issues/comment_test.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -68,15 +68,15 @@ func TestFetchCodeComments(t *testing.T) {
6868

6969
issue := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{ID: 2})
7070
user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1})
71-
res, err := issues_model.FetchCodeComments(db.DefaultContext, issue, user, false)
71+
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")
7474
assert.Contains(t, res["README.md"], int64(4))
7575
assert.Len(t, res["README.md"][4], 1)
76-
assert.Equal(t, int64(4), res["README.md"][4][0].ID)
76+
assert.Equal(t, int64(4), res["README.md"][0].ID)
7777

7878
user2 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2})
79-
res, err = issues_model.FetchCodeComments(db.DefaultContext, issue, user2, false)
79+
res, err = issues_model.FetchCodeComments(db.DefaultContext, issue.Repo, issue.ID, user2, false)
8080
assert.NoError(t, err)
8181
assert.Len(t, res, 1)
8282
}

‎models/issues/review.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -159,7 +159,7 @@ func (r *Review) LoadCodeComments(ctx context.Context) (err error) {
159159
if err = r.LoadIssue(ctx); err != nil {
160160
return err
161161
}
162-
r.CodeComments, err = fetchCodeCommentsByReview(ctx, r.Issue, nil, r, false)
162+
r.CodeComments, err = fetchCodeCommentsByReview(ctx, r.Issue.Repo, r.Issue.ID, nil, r, false)
163163
return err
164164
}
165165

‎models/issues/review_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ func TestReview_LoadCodeComments(t *testing.T) {
4848
assert.NoError(t, review.LoadAttributes(db.DefaultContext))
4949
assert.NoError(t, review.LoadCodeComments(db.DefaultContext))
5050
assert.Len(t, review.CodeComments, 1)
51-
assert.Equal(t, int64(4), review.CodeComments["README.md"][int64(4)][0].Line)
51+
assert.Equal(t, int64(4), review.CodeComments["README.md"][0].Line)
5252
}
5353

5454
func TestReviewType_Icon(t *testing.T) {

‎modules/git/diff.go

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import (
1515
"strings"
1616

1717
"code.gitea.io/gitea/modules/log"
18+
"code.gitea.io/gitea/modules/util"
1819
)
1920

2021
// RawDiffType type of a raw diff.
@@ -107,12 +108,16 @@ func ParseDiffHunkString(diffHunk string) (leftLine, leftHunk, rightLine, rightH
107108
leftLine, _ = strconv.Atoi(leftRange[0][1:])
108109
if len(leftRange) > 1 {
109110
leftHunk, _ = strconv.Atoi(leftRange[1])
111+
} else {
112+
leftHunk = util.Iif(leftLine > 0, leftLine, -leftLine)
110113
}
111114
if len(ranges) > 1 {
112115
rightRange := strings.Split(ranges[1], ",")
113116
rightLine, _ = strconv.Atoi(rightRange[0])
114117
if len(rightRange) > 1 {
115118
rightHunk, _ = strconv.Atoi(rightRange[1])
119+
} else {
120+
rightHunk = rightLine
116121
}
117122
} else {
118123
log.Debug("Parse line number failed: %v", diffHunk)
@@ -342,3 +347,55 @@ func GetAffectedFiles(repo *Repository, branchName, oldCommitID, newCommitID str
342347

343348
return affectedFiles, err
344349
}
350+
351+
type HunkInfo struct {
352+
LeftLine int64 // Line number in the old file
353+
LeftHunk int64 // Number of lines in the old file
354+
RightLine int64 // Line number in the new file
355+
RightHunk int64 // Number of lines in the new file
356+
}
357+
358+
// GetAffectedHunksForTwoCommitsSpecialFile returns the affected hunks between two commits for a special file
359+
// git diff --unified=0 abc123 def456 -- src/main.go
360+
func GetAffectedHunksForTwoCommitsSpecialFile(ctx context.Context, repoPath, oldCommitID, newCommitID, filePath string) ([]*HunkInfo, error) {
361+
reader, writer := io.Pipe()
362+
defer func() {
363+
_ = reader.Close()
364+
_ = writer.Close()
365+
}()
366+
go func() {
367+
if err := NewCommand("diff", "--unified=0", "--no-color").
368+
AddDynamicArguments(oldCommitID, newCommitID).
369+
AddDashesAndList(filePath).
370+
Run(ctx, &RunOpts{
371+
Dir: repoPath,
372+
Stdout: writer,
373+
}); err != nil {
374+
_ = writer.CloseWithError(fmt.Errorf("GetAffectedHunksForTwoCommitsSpecialFile[%s, %s, %s, %s]: %w", repoPath, oldCommitID, newCommitID, filePath, err))
375+
return
376+
}
377+
_ = writer.Close()
378+
}()
379+
380+
scanner := bufio.NewScanner(reader)
381+
hunks := make([]*HunkInfo, 0, 32)
382+
for scanner.Scan() {
383+
lof := scanner.Text()
384+
if !strings.HasPrefix(lof, "@@") {
385+
continue
386+
}
387+
// Parse the hunk header
388+
leftLine, leftHunk, rightLine, rightHunk := ParseDiffHunkString(lof)
389+
hunks = append([]*HunkInfo{}, &HunkInfo{
390+
LeftLine: int64(leftLine),
391+
LeftHunk: int64(leftHunk),
392+
RightLine: int64(rightLine),
393+
RightHunk: int64(rightHunk),
394+
})
395+
}
396+
if scanner.Err() != nil {
397+
return nil, fmt.Errorf("GetAffectedHunksForTwoCommitsSpecialFile[%s, %s, %s, %s]: %w", repoPath, oldCommitID, newCommitID, filePath, scanner.Err())
398+
}
399+
400+
return hunks, nil
401+
}

‎modules/git/diff_test.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -181,4 +181,10 @@ func TestParseDiffHunkString(t *testing.T) {
181181
assert.Equal(t, 3, leftHunk)
182182
assert.Equal(t, 19, rightLine)
183183
assert.Equal(t, 5, rightHunk)
184+
185+
leftLine, leftHunk, rightLine, rightHunk = ParseDiffHunkString("@@ -1 +0,0 @@")
186+
assert.Equal(t, 1, leftLine)
187+
assert.Equal(t, 1, leftHunk)
188+
assert.Equal(t, 0, rightLine)
189+
assert.Equal(t, 0, rightHunk)
184190
}

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

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@ import (
1313
"code.gitea.io/gitea/models/organization"
1414
access_model "code.gitea.io/gitea/models/perm/access"
1515
user_model "code.gitea.io/gitea/models/user"
16-
"code.gitea.io/gitea/modules/gitrepo"
1716
api "code.gitea.io/gitea/modules/structs"
1817
"code.gitea.io/gitea/modules/web"
1918
"code.gitea.io/gitea/routers/api/v1/utils"
@@ -329,14 +328,7 @@ func CreatePullReview(ctx *context.APIContext) {
329328

330329
// if CommitID is empty, set it as lastCommitID
331330
if opts.CommitID == "" {
332-
gitRepo, closer, err := gitrepo.RepositoryFromContextOrOpen(ctx, pr.Issue.Repo)
333-
if err != nil {
334-
ctx.APIErrorInternal(err)
335-
return
336-
}
337-
defer closer.Close()
338-
339-
headCommitID, err := gitRepo.GetRefCommitID(pr.GetGitHeadRefName())
331+
headCommitID, err := ctx.Repo.GitRepo.GetRefCommitID(pr.GetGitHeadRefName())
340332
if err != nil {
341333
ctx.APIErrorInternal(err)
342334
return
@@ -357,11 +349,12 @@ func CreatePullReview(ctx *context.APIContext) {
357349
ctx.Repo.GitRepo,
358350
pr.Issue,
359351
line,
352+
pr.MergeBase,
353+
opts.CommitID,
360354
c.Body,
361355
c.Path,
362356
true, // pending review
363357
0, // no reply
364-
opts.CommitID,
365358
nil,
366359
); err != nil {
367360
ctx.APIErrorInternal(err)

‎routers/web/repo/issue_view.go

Lines changed: 14 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -730,28 +730,23 @@ func prepareIssueViewCommentsAndSidebarParticipants(ctx *context.Context, issue
730730
}
731731
comment.Review.Reviewer = user_model.NewGhostUser()
732732
}
733-
if err = comment.Review.LoadCodeComments(ctx); err != nil {
734-
ctx.ServerError("Review.LoadCodeComments", err)
735-
return
736-
}
733+
737734
for _, codeComments := range comment.Review.CodeComments {
738-
for _, lineComments := range codeComments {
739-
for _, c := range lineComments {
740-
// Check tag.
741-
role, ok = marked[c.PosterID]
742-
if ok {
743-
c.ShowRole = role
744-
continue
745-
}
735+
for _, c := range codeComments {
736+
// Check tag.
737+
role, ok = marked[c.PosterID]
738+
if ok {
739+
c.ShowRole = role
740+
continue
741+
}
746742

747-
c.ShowRole, err = roleDescriptor(ctx, issue.Repo, c.Poster, permCache, issue, c.HasOriginalAuthor())
748-
if err != nil {
749-
ctx.ServerError("roleDescriptor", err)
750-
return
751-
}
752-
marked[c.PosterID] = c.ShowRole
753-
participants = addParticipant(c.Poster, participants)
743+
c.ShowRole, err = roleDescriptor(ctx, issue.Repo, c.Poster, permCache, issue, c.HasOriginalAuthor())
744+
if err != nil {
745+
ctx.ServerError("roleDescriptor", err)
746+
return
754747
}
748+
marked[c.PosterID] = c.ShowRole
749+
participants = addParticipant(c.Poster, participants)
755750
}
756751
}
757752
if err = comment.LoadResolveDoer(ctx); err != nil {

0 commit comments

Comments
 (0)