Partly fix incorrect code review comment line number

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
 }

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)

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) {

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 {

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
 	}

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

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,
+			})
 		}
 	}
 

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
 					})

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)
 			}
 		}
 	}

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)
 		}
 	}
 

templates/repo/issue/view_content/comments.tmpl

@@ -456,10 +456,8 @@
 
 				{{if and .Review .Review.CodeComments}}
 				<div class="timeline-item event">
-					{{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}}
 				</div>
 				{{end}}