diff --git a/models/issues/comment_code.go b/models/issues/comment_code.go index 6f23d3326a223..63564f4d5ac6d 100644 --- a/models/issues/comment_code.go +++ b/models/issues/comment_code.go @@ -15,7 +15,7 @@ import ( ) // CodeComments represents comments on code by using this structure: FILENAME -> LINE (+ == proposed; - == previous) -> COMMENTS -type CodeComments map[string]map[int64][]*Comment +type CodeComments map[string][]*Comment // FetchCodeComments will return a 2d-map: ["Path"]["Line"] = Comments at line func FetchCodeComments(ctx context.Context, issue *Issue, currentUser *user_model.User, showOutdatedComments bool) (CodeComments, error) { @@ -40,9 +40,9 @@ func fetchCodeCommentsByReview(ctx context.Context, issue *Issue, currentUser *u for _, comment := range comments { if pathToLineToComment[comment.TreePath] == nil { - pathToLineToComment[comment.TreePath] = make(map[int64][]*Comment) + pathToLineToComment[comment.TreePath] = make([]*Comment, 0) } - pathToLineToComment[comment.TreePath][comment.Line] = append(pathToLineToComment[comment.TreePath][comment.Line], comment) + pathToLineToComment[comment.TreePath] = append(pathToLineToComment[comment.TreePath], comment) } return pathToLineToComment, nil } diff --git a/models/issues/comment_test.go b/models/issues/comment_test.go index c5bbfdedc28ec..c32278add0077 100644 --- a/models/issues/comment_test.go +++ b/models/issues/comment_test.go @@ -53,9 +53,9 @@ func TestFetchCodeComments(t *testing.T) { res, err := issues_model.FetchCodeComments(db.DefaultContext, issue, user, false) assert.NoError(t, err) assert.Contains(t, res, "README.md") - assert.Contains(t, res["README.md"], int64(4)) - assert.Len(t, res["README.md"][4], 1) - assert.Equal(t, int64(4), res["README.md"][4][0].ID) + assert.Len(t, res["README.md"], 2) + assert.Equal(t, res["README.md"][0].ID, int64(4)) + assert.Equal(t, res["README.md"][1].ID, int64(5)) user2 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) res, err = issues_model.FetchCodeComments(db.DefaultContext, issue, user2, false) diff --git a/models/issues/review_test.go b/models/issues/review_test.go index ac1b84adebcc2..ab9f56a734c18 100644 --- a/models/issues/review_test.go +++ b/models/issues/review_test.go @@ -48,7 +48,7 @@ func TestReview_LoadCodeComments(t *testing.T) { assert.NoError(t, review.LoadAttributes(db.DefaultContext)) assert.NoError(t, review.LoadCodeComments(db.DefaultContext)) assert.Len(t, review.CodeComments, 1) - assert.Equal(t, int64(4), review.CodeComments["README.md"][int64(4)][0].Line) + assert.Equal(t, int64(4), review.CodeComments["README.md"][0].Line) } func TestReviewType_Icon(t *testing.T) { diff --git a/routers/web/repo/issue.go b/routers/web/repo/issue.go index e7ad02c0c25ec..26dac7940736f 100644 --- a/routers/web/repo/issue.go +++ b/routers/web/repo/issue.go @@ -1728,23 +1728,21 @@ func ViewIssue(ctx *context.Context) { return } for _, codeComments := range comment.Review.CodeComments { - for _, lineComments := range codeComments { - for _, c := range lineComments { - // Check tag. - role, ok = marked[c.PosterID] - if ok { - c.ShowRole = role - continue - } + for _, c := range codeComments { + // Check tag. + role, ok = marked[c.PosterID] + if ok { + c.ShowRole = role + continue + } - c.ShowRole, err = roleDescriptor(ctx, repo, c.Poster, issue, c.HasOriginalAuthor()) - if err != nil { - ctx.ServerError("roleDescriptor", err) - return - } - marked[c.PosterID] = c.ShowRole - participants = addParticipant(c.Poster, participants) + c.ShowRole, err = roleDescriptor(ctx, repo, c.Poster, issue, c.HasOriginalAuthor()) + if err != nil { + ctx.ServerError("roleDescriptor", err) + return } + marked[c.PosterID] = c.ShowRole + participants = addParticipant(c.Poster, participants) } } if err = comment.LoadResolveDoer(ctx); err != nil { diff --git a/routers/web/repo/pull.go b/routers/web/repo/pull.go index 92e0a1674e080..e85636e9f0dea 100644 --- a/routers/web/repo/pull.go +++ b/routers/web/repo/pull.go @@ -752,7 +752,7 @@ func viewPullFiles(ctx *context.Context, specifiedStartCommit, specifiedEndCommi "numberOfViewedFiles": diff.NumViewedFiles, } - if err = diff.LoadComments(ctx, issue, ctx.Doer, ctx.Data["ShowOutdatedComments"].(bool)); err != nil { + if err = diff.LoadComments(ctx, issue, gitRepo, diffOptions, ctx.Doer, ctx.Data["ShowOutdatedComments"].(bool)); err != nil { ctx.ServerError("LoadComments", err) return } diff --git a/services/convert/pull_review.go b/services/convert/pull_review.go index 29a5ab7466b13..7b56fd39f2443 100644 --- a/services/convert/pull_review.go +++ b/services/convert/pull_review.go @@ -90,31 +90,30 @@ func ToPullReviewCommentList(ctx context.Context, review *issues_model.Review, d apiComments := make([]*api.PullReviewComment, 0, len(review.CodeComments)) for _, lines := range review.CodeComments { - for _, comments := range lines { - for _, comment := range comments { - apiComment := &api.PullReviewComment{ - ID: comment.ID, - Body: comment.Content, - Poster: ToUser(ctx, comment.Poster, doer), - Resolver: ToUser(ctx, comment.ResolveDoer, doer), - ReviewID: review.ID, - Created: comment.CreatedUnix.AsTime(), - Updated: comment.UpdatedUnix.AsTime(), - Path: comment.TreePath, - CommitID: comment.CommitSHA, - OrigCommitID: comment.OldRef, - DiffHunk: patch2diff(comment.Patch), - HTMLURL: comment.HTMLURL(ctx), - HTMLPullURL: review.Issue.HTMLURL(), - } + for _, comment := range lines { + apiComment := &api.PullReviewComment{ + ID: comment.ID, + Body: comment.Content, + Poster: ToUser(ctx, comment.Poster, doer), + Resolver: ToUser(ctx, comment.ResolveDoer, doer), + ReviewID: review.ID, + Created: comment.CreatedUnix.AsTime(), + Updated: comment.UpdatedUnix.AsTime(), + Path: comment.TreePath, + CommitID: comment.CommitSHA, + OrigCommitID: comment.OldRef, + DiffHunk: patch2diff(comment.Patch), + HTMLURL: comment.HTMLURL(ctx), + HTMLPullURL: review.Issue.HTMLURL(), + } - if comment.Line < 0 { - apiComment.OldLineNum = comment.UnsignedLine() - } else { - apiComment.LineNum = comment.UnsignedLine() - } - apiComments = append(apiComments, apiComment) + if comment.Line < 0 { + apiComment.OldLineNum = comment.UnsignedLine() + } else { + apiComment.LineNum = comment.UnsignedLine() } + apiComments = append(apiComments, apiComment) + } } return apiComments, nil diff --git a/services/feed/action.go b/services/feed/action.go index 83daaa1438fd5..4c3187ae031d7 100644 --- a/services/feed/action.go +++ b/services/feed/action.go @@ -224,20 +224,18 @@ func (a *actionNotifier) PullRequestReview(ctx context.Context, pr *issues_model actions := make([]*activities_model.Action, 0, 10) for _, lines := range review.CodeComments { - for _, comments := range lines { - for _, comm := range comments { - actions = append(actions, &activities_model.Action{ - ActUserID: review.Reviewer.ID, - ActUser: review.Reviewer, - Content: fmt.Sprintf("%d|%s", review.Issue.Index, strings.Split(comm.Content, "\n")[0]), - OpType: activities_model.ActionCommentPull, - RepoID: review.Issue.RepoID, - Repo: review.Issue.Repo, - IsPrivate: review.Issue.Repo.IsPrivate, - Comment: comm, - CommentID: comm.ID, - }) - } + for _, comment := range lines { + actions = append(actions, &activities_model.Action{ + ActUserID: review.Reviewer.ID, + ActUser: review.Reviewer, + Content: fmt.Sprintf("%d|%s", review.Issue.Index, strings.Split(comment.Content, "\n")[0]), + OpType: activities_model.ActionCommentPull, + RepoID: review.Issue.RepoID, + Repo: review.Issue.Repo, + IsPrivate: review.Issue.Repo.IsPrivate, + Comment: comment, + CommentID: comment.ID, + }) } } diff --git a/services/gitdiff/gitdiff.go b/services/gitdiff/gitdiff.go index 0ddd5a48e2148..a7a5402ea9bc1 100644 --- a/services/gitdiff/gitdiff.go +++ b/services/gitdiff/gitdiff.go @@ -466,21 +466,59 @@ type Diff struct { } // LoadComments loads comments into each line -func (diff *Diff) LoadComments(ctx context.Context, issue *issues_model.Issue, currentUser *user_model.User, showOutdatedComments bool) error { +func (diff *Diff) LoadComments(ctx context.Context, issue *issues_model.Issue, gitRepo *git.Repository, diffOptions *DiffOptions, currentUser *user_model.User, showOutdatedComments bool) error { allComments, err := issues_model.FetchCodeComments(ctx, issue, currentUser, showOutdatedComments) if err != nil { return err } for _, file := range diff.Files { - if lineCommits, ok := allComments[file.Name]; ok { - for _, section := range file.Sections { - for _, line := range section.Lines { - if comments, ok := lineCommits[int64(line.LeftIdx*-1)]; ok { - line.Comments = append(line.Comments, comments...) + if comments, ok := allComments[file.Name]; ok { + for _, comment := range comments { + diffOptions.BeforeCommitID = comment.CommitSHA + diffOptions.MaxFiles = 1 + d, err := GetDiff(ctx, gitRepo, diffOptions, file.Name) + if err != nil { + return err + } + + deleted := false + done := false + if len(d.Files) > 0 { + f := d.Files[0] + for _, s := range f.Sections { + for _, l := range s.Lines { + if l.LeftIdx == int(comment.Line) { + if l.Type == DiffLineDel { + // this line has been deleted so skip it + deleted = true + } else { + // update new line + comment.Line = int64(l.RightIdx) + } + done = true + break + } + } + if done { + break + } } - if comments, ok := lineCommits[int64(line.RightIdx)]; ok { - line.Comments = append(line.Comments, comments...) + } + + if deleted { + continue + } + + for _, section := range file.Sections { + for _, line := range section.Lines { + if comment.Line == int64(line.RightIdx) { + line.Comments = append(line.Comments, comment) + } } + } + } + for _, section := range file.Sections { + for _, line := range section.Lines { sort.SliceStable(line.Comments, func(i, j int) bool { return line.Comments[i].CreatedUnix < line.Comments[j].CreatedUnix }) diff --git a/services/mailer/mail.go b/services/mailer/mail.go index 000cc835c8b77..83e3ca1729f7a 100644 --- a/services/mailer/mail.go +++ b/services/mailer/mail.go @@ -242,8 +242,8 @@ func composeIssueCommentMessages(ctx *mailCommentContext, lang string, recipient if ctx.Comment != nil && ctx.Comment.Review != nil { reviewComments = make([]*issues_model.Comment, 0, 10) for _, lines := range ctx.Comment.Review.CodeComments { - for _, comments := range lines { - reviewComments = append(reviewComments, comments...) + for _, comment := range lines { + reviewComments = append(reviewComments, comment) } } } diff --git a/services/pull/review.go b/services/pull/review.go index 3d5eca779f8e9..b510a7b5bf112 100644 --- a/services/pull/review.go +++ b/services/pull/review.go @@ -329,14 +329,12 @@ func SubmitReview(ctx context.Context, doer *user_model.User, gitRepo *git.Repos notify_service.PullRequestReview(ctx, pr, review, comm, mentions) for _, lines := range review.CodeComments { - for _, comments := range lines { - for _, codeComment := range comments { - mentions, err := issues_model.FindAndUpdateIssueMentions(ctx, issue, doer, codeComment.Content) - if err != nil { - return nil, nil, err - } - notify_service.PullRequestCodeComment(ctx, pr, codeComment, mentions) + for _, comment := range lines { + mentions, err := issues_model.FindAndUpdateIssueMentions(ctx, issue, doer, comment.Content) + if err != nil { + return nil, nil, err } + notify_service.PullRequestCodeComment(ctx, pr, comment, mentions) } } diff --git a/templates/repo/issue/view_content/comments.tmpl b/templates/repo/issue/view_content/comments.tmpl index 3da2f3815ee3f..9196b6a1dc05e 100644 --- a/templates/repo/issue/view_content/comments.tmpl +++ b/templates/repo/issue/view_content/comments.tmpl @@ -456,10 +456,8 @@ {{if and .Review .Review.CodeComments}}
- {{range $filename, $lines := .Review.CodeComments}} - {{range $line, $comms := $lines}} - {{template "repo/issue/view_content/conversation" dict "." $ "comments" $comms}} - {{end}} + {{range $filename, $comms := .Review.CodeComments}} + {{template "repo/issue/view_content/conversation" dict "." $ "comments" $comms}} {{end}}
{{end}}