From d612e38923ca144cb0f826fc099a38dac0bbdb7d Mon Sep 17 00:00:00 2001 From: yp05327 <576951401@qq.com> Date: Mon, 10 Jun 2024 00:36:49 +0000 Subject: [PATCH 01/11] fix --- models/issues/comment_code.go | 10 +++++++--- services/gitdiff/gitdiff.go | 4 ++-- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/models/issues/comment_code.go b/models/issues/comment_code.go index 6f23d3326a223..a18e8b880617a 100644 --- a/models/issues/comment_code.go +++ b/models/issues/comment_code.go @@ -5,6 +5,7 @@ package issues import ( "context" + "strings" "code.gitea.io/gitea/models/db" user_model "code.gitea.io/gitea/models/user" @@ -15,7 +16,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]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 +41,12 @@ 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(map[string][]*Comment) } - pathToLineToComment[comment.TreePath][comment.Line] = append(pathToLineToComment[comment.TreePath][comment.Line], comment) + // always show the comment at the last line + patchLines := strings.Split(comment.Patch, "\n") + lineContent := patchLines[len(patchLines)-1] + pathToLineToComment[comment.TreePath][lineContent] = append(pathToLineToComment[comment.TreePath][lineContent], comment) } return pathToLineToComment, nil } diff --git a/services/gitdiff/gitdiff.go b/services/gitdiff/gitdiff.go index 0ddd5a48e2148..824aa022b9b43 100644 --- a/services/gitdiff/gitdiff.go +++ b/services/gitdiff/gitdiff.go @@ -475,10 +475,10 @@ func (diff *Diff) LoadComments(ctx context.Context, issue *issues_model.Issue, c 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 { + if comments, ok := lineCommits[line.Content]; ok { line.Comments = append(line.Comments, comments...) } - if comments, ok := lineCommits[int64(line.RightIdx)]; ok { + if comments, ok := lineCommits[""]; ok { line.Comments = append(line.Comments, comments...) } sort.SliceStable(line.Comments, func(i, j int) bool { From 20e93625b71f71fd1bd57b5334ecfa80ebdfe88d Mon Sep 17 00:00:00 2001 From: yp05327 <576951401@qq.com> Date: Mon, 10 Jun 2024 01:39:31 +0000 Subject: [PATCH 02/11] fix web ui --- models/issues/comment.go | 11 +++++++++ models/issues/comment_code.go | 24 ++++++++++++------- routers/web/repo/pull_review.go | 8 +++---- services/forms/repo_form.go | 1 + templates/repo/diff/comment_form.tmpl | 1 + .../repo/diff/comment_form_datahandler.tmpl | 2 +- templates/repo/diff/conversation.tmpl | 2 +- templates/repo/diff/section_split.tmpl | 8 +++---- templates/repo/diff/section_unified.tmpl | 2 +- .../repo/issue/view_content/conversation.tmpl | 2 +- web_src/js/features/repo-diff.js | 3 ++- web_src/js/features/repo-issue.js | 2 ++ 12 files changed, 44 insertions(+), 22 deletions(-) diff --git a/models/issues/comment.go b/models/issues/comment.go index c6c5dc24321d1..233f4b0f98aba 100644 --- a/models/issues/comment.go +++ b/models/issues/comment.go @@ -10,6 +10,7 @@ import ( "fmt" "html/template" "strconv" + "strings" "unicode/utf8" "code.gitea.io/gitea/models/db" @@ -783,6 +784,15 @@ func (c *Comment) LoadPushCommits(ctx context.Context) (err error) { return err } +func (c *Comment) GetLineContent() string { + // always show the comment at the last line + patchLines := strings.Split(c.Patch, "\n") + if len(patchLines) == 0 { + return "" + } + return patchLines[len(patchLines)-1] +} + // CreateComment creates comment with context func CreateComment(ctx context.Context, opts *CreateCommentOptions) (_ *Comment, err error) { ctx, committer, err := db.TxContext(ctx) @@ -1033,6 +1043,7 @@ type FindCommentsOptions struct { Since int64 Before int64 Line int64 + LineContent string TreePath string Type CommentType IssueIDs []int64 diff --git a/models/issues/comment_code.go b/models/issues/comment_code.go index a18e8b880617a..fd183582b776d 100644 --- a/models/issues/comment_code.go +++ b/models/issues/comment_code.go @@ -5,7 +5,6 @@ package issues import ( "context" - "strings" "code.gitea.io/gitea/models/db" user_model "code.gitea.io/gitea/models/user" @@ -43,9 +42,7 @@ func fetchCodeCommentsByReview(ctx context.Context, issue *Issue, currentUser *u if pathToLineToComment[comment.TreePath] == nil { pathToLineToComment[comment.TreePath] = make(map[string][]*Comment) } - // always show the comment at the last line - patchLines := strings.Split(comment.Patch, "\n") - lineContent := patchLines[len(patchLines)-1] + lineContent := comment.GetLineContent() pathToLineToComment[comment.TreePath][lineContent] = append(pathToLineToComment[comment.TreePath][lineContent], comment) } return pathToLineToComment, nil @@ -70,6 +67,14 @@ func findCodeComments(ctx context.Context, opts FindCommentsOptions, issue *Issu return nil, err } + if opts.LineContent != "" { + for index, comment := range comments { + if comment.GetLineContent() != opts.LineContent { + comments = append(comments[:index], comments[index+1:]...) + } + } + } + if err := issue.LoadRepo(ctx); err != nil { return nil, err } @@ -131,12 +136,13 @@ func findCodeComments(ctx context.Context, opts FindCommentsOptions, issue *Issu } // FetchCodeCommentsByLine fetches the code comments for a given treePath and line number -func FetchCodeCommentsByLine(ctx context.Context, issue *Issue, currentUser *user_model.User, treePath string, line int64, showOutdatedComments bool) (CommentList, error) { +func FetchCodeCommentsByLine(ctx context.Context, issue *Issue, currentUser *user_model.User, treePath string, line int64, lineContent string, showOutdatedComments bool) (CommentList, error) { opts := FindCommentsOptions{ - Type: CommentTypeCode, - IssueID: issue.ID, - TreePath: treePath, - Line: line, + Type: CommentTypeCode, + IssueID: issue.ID, + TreePath: treePath, + Line: line, + LineContent: lineContent, } return findCodeComments(ctx, opts, issue, currentUser, nil, showOutdatedComments) } diff --git a/routers/web/repo/pull_review.go b/routers/web/repo/pull_review.go index 62f6d71c5e5cf..1fb0d7cdb7804 100644 --- a/routers/web/repo/pull_review.go +++ b/routers/web/repo/pull_review.go @@ -110,7 +110,7 @@ func CreateCodeComment(ctx *context.Context) { log.Trace("Comment created: %-v #%d[%d] Comment[%d]", ctx.Repo.Repository, issue.Index, issue.ID, comment.ID) - renderConversation(ctx, comment, form.Origin) + renderConversation(ctx, comment, form.Origin, form.LineContent) } // UpdateResolveConversation add or remove an Conversation resolved mark @@ -161,14 +161,14 @@ func UpdateResolveConversation(ctx *context.Context) { return } - renderConversation(ctx, comment, origin) + renderConversation(ctx, comment, origin, ctx.FormString("line_content")) } -func renderConversation(ctx *context.Context, comment *issues_model.Comment, origin string) { +func renderConversation(ctx *context.Context, comment *issues_model.Comment, origin, lineContent string) { ctx.Data["PageIsPullFiles"] = origin == "diff" showOutdatedComments := origin == "timeline" || ctx.Data["ShowOutdatedComments"].(bool) - comments, err := issues_model.FetchCodeCommentsByLine(ctx, comment.Issue, ctx.Doer, comment.TreePath, comment.Line, showOutdatedComments) + comments, err := issues_model.FetchCodeCommentsByLine(ctx, comment.Issue, ctx.Doer, comment.TreePath, comment.Line, lineContent, showOutdatedComments) if err != nil { ctx.ServerError("FetchCodeCommentsByLine", err) return diff --git a/services/forms/repo_form.go b/services/forms/repo_form.go index 32d96abf4d272..fde0b1145a46c 100644 --- a/services/forms/repo_form.go +++ b/services/forms/repo_form.go @@ -587,6 +587,7 @@ type CodeCommentForm struct { Content string `binding:"Required"` Side string `binding:"Required;In(previous,proposed)"` Line int64 + LineContent string `binding:"Required"` TreePath string `form:"path" binding:"Required"` SingleReview bool `form:"single_review"` Reply int64 `form:"reply"` diff --git a/templates/repo/diff/comment_form.tmpl b/templates/repo/diff/comment_form.tmpl index 856b3da01a345..9abcc2121a5f4 100644 --- a/templates/repo/diff/comment_form.tmpl +++ b/templates/repo/diff/comment_form.tmpl @@ -5,6 +5,7 @@ + diff --git a/templates/repo/diff/comment_form_datahandler.tmpl b/templates/repo/diff/comment_form_datahandler.tmpl index d0e493488df0a..88e2a8a9793db 100644 --- a/templates/repo/diff/comment_form_datahandler.tmpl +++ b/templates/repo/diff/comment_form_datahandler.tmpl @@ -1,5 +1,5 @@ {{if $.comment}} - {{template "repo/diff/comment_form" dict "root" $.root "hidden" $.hidden "reply" $.reply "Line" $.comment.UnsignedLine "File" $.comment.TreePath "Side" $.comment.DiffSide "HasComments" true}} + {{template "repo/diff/comment_form" dict "root" $.root "hidden" $.hidden "reply" $.reply "Line" $.comment.UnsignedLine "LineContent" $.comment.GetLineContent "File" $.comment.TreePath "Side" $.comment.DiffSide "HasComments" true}} {{else if $.root}} {{template "repo/diff/comment_form" $}} {{else}} diff --git a/templates/repo/diff/conversation.tmpl b/templates/repo/diff/conversation.tmpl index 08f60644b3b52..4eee1490eb152 100644 --- a/templates/repo/diff/conversation.tmpl +++ b/templates/repo/diff/conversation.tmpl @@ -50,7 +50,7 @@ {{if and $.CanMarkConversation $hasReview (not $isReviewPending)}} - {{/* */}}{{end}}{{/* @@ -64,7 +64,7 @@ {{if $match.RightIdx}}{{end}} {{/* */}}{{if and $.root.SignedUserID $.root.PageIsPullFiles}}{{/* - */}}{{/* */}}{{end}}{{/* @@ -81,7 +81,7 @@ {{if $line.LeftIdx}}{{end}} {{/* */}}{{if and $.root.SignedUserID $.root.PageIsPullFiles (not (eq .GetType 2))}}{{/* - */}}{{/* */}}{{end}}{{/* @@ -96,7 +96,7 @@ {{if $line.RightIdx}}{{end}} {{/* */}}{{if and $.root.SignedUserID $.root.PageIsPullFiles (not (eq .GetType 3))}}{{/* - */}}{{/* */}}{{end}}{{/* diff --git a/templates/repo/diff/section_unified.tmpl b/templates/repo/diff/section_unified.tmpl index 708b333291641..dffa4eda45543 100644 --- a/templates/repo/diff/section_unified.tmpl +++ b/templates/repo/diff/section_unified.tmpl @@ -54,7 +54,7 @@ {{else}} {{/* */}}{{if and $.root.SignedUserID $.root.PageIsPullFiles}}{{/* - */}}{{/* */}}{{end}}{{/* diff --git a/templates/repo/issue/view_content/conversation.tmpl b/templates/repo/issue/view_content/conversation.tmpl index ccea9b690d7e8..648e2fe932a45 100644 --- a/templates/repo/issue/view_content/conversation.tmpl +++ b/templates/repo/issue/view_content/conversation.tmpl @@ -120,7 +120,7 @@
{{if and $.CanMarkConversation $hasReview (not $isReviewPending)}} -
{{if and $.CanMarkConversation $hasReview (not $isReviewPending)}} - {{/* */}}{{end}}{{/* @@ -64,7 +64,7 @@ {{if $match.RightIdx}}{{end}} {{/* */}}{{if and $.root.SignedUserID $.root.PageIsPullFiles}}{{/* - */}}{{/* */}}{{end}}{{/* @@ -81,7 +81,7 @@ {{if $line.LeftIdx}}{{end}} {{/* */}}{{if and $.root.SignedUserID $.root.PageIsPullFiles (not (eq .GetType 2))}}{{/* - */}}{{/* */}}{{end}}{{/* @@ -96,7 +96,7 @@ {{if $line.RightIdx}}{{end}} {{/* */}}{{if and $.root.SignedUserID $.root.PageIsPullFiles (not (eq .GetType 3))}}{{/* - */}}{{/* */}}{{end}}{{/* diff --git a/templates/repo/diff/section_unified.tmpl b/templates/repo/diff/section_unified.tmpl index dffa4eda45543..708b333291641 100644 --- a/templates/repo/diff/section_unified.tmpl +++ b/templates/repo/diff/section_unified.tmpl @@ -54,7 +54,7 @@ {{else}} {{/* */}}{{if and $.root.SignedUserID $.root.PageIsPullFiles}}{{/* - */}}{{/* */}}{{end}}{{/* diff --git a/web_src/js/features/repo-diff.js b/web_src/js/features/repo-diff.js index ba526d3691551..00f74515df6a8 100644 --- a/web_src/js/features/repo-diff.js +++ b/web_src/js/features/repo-diff.js @@ -100,11 +100,10 @@ function initRepoDiffConversationForm() { const comment_id = $(this).data('comment-id'); const origin = $(this).data('origin'); const action = $(this).data('action'); - const line_content = $(this).data('line-content'); const url = $(this).data('update-url'); try { - const response = await POST(url, {data: new URLSearchParams({origin, action, comment_id, line_content})}); + const response = await POST(url, {data: new URLSearchParams({origin, action, comment_id})}); const data = await response.text(); if ($(this).closest('.conversation-holder').length) { diff --git a/web_src/js/features/repo-issue.js b/web_src/js/features/repo-issue.js index e35c125c8397a..95910e34bc9af 100644 --- a/web_src/js/features/repo-issue.js +++ b/web_src/js/features/repo-issue.js @@ -524,7 +524,6 @@ export function initRepoPullRequestReview() { const isSplit = this.closest('.code-diff')?.classList.contains('code-diff-split'); const side = this.getAttribute('data-side'); const idx = this.getAttribute('data-idx'); - const content = this.getAttribute('data-content'); const path = this.closest('[data-path]')?.getAttribute('data-path'); const tr = this.closest('tr'); const lineType = tr.getAttribute('data-line-type'); @@ -552,7 +551,6 @@ export function initRepoPullRequestReview() { const html = await response.text(); $td.html(html); $td.find("input[name='line']").val(idx); - $td.find("input[name='line_content']").val(content); $td.find("input[name='side']").val(side === 'left' ? 'previous' : 'proposed'); $td.find("input[name='path']").val(path); From 9ab94b0627684806e0ba79be5e93e48d33da7f7c Mon Sep 17 00:00:00 2001 From: yp05327 <576951401@qq.com> Date: Mon, 10 Jun 2024 07:50:17 +0000 Subject: [PATCH 10/11] fix unit test --- models/issues/comment_test.go | 11 +++-------- models/issues/review_test.go | 3 +-- 2 files changed, 4 insertions(+), 10 deletions(-) diff --git a/models/issues/comment_test.go b/models/issues/comment_test.go index 1844aae0739fd..c32278add0077 100644 --- a/models/issues/comment_test.go +++ b/models/issues/comment_test.go @@ -51,16 +51,11 @@ func TestFetchCodeComments(t *testing.T) { issue := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{ID: 2}) user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1}) res, err := issues_model.FetchCodeComments(db.DefaultContext, issue, user, false) - lineContent := "+ # repo1" assert.NoError(t, err) assert.Contains(t, res, "README.md") - assert.Contains(t, res["README.md"], lineContent) - assert.Len(t, res["README.md"][lineContent], 1) - assert.Equal(t, int64(4), res["README.md"][lineContent][0].ID) - lineContent = "- " - assert.Contains(t, res["README.md"], lineContent) - assert.Len(t, res["README.md"][lineContent], 1) - assert.Equal(t, int64(5), res["README.md"][lineContent][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 cfeed0e704834..ab9f56a734c18 100644 --- a/models/issues/review_test.go +++ b/models/issues/review_test.go @@ -48,8 +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) - lineContent := "+ # repo1" - assert.Equal(t, int64(4), review.CodeComments["README.md"][lineContent][0].ID) + assert.Equal(t, int64(4), review.CodeComments["README.md"][0].Line) } func TestReviewType_Icon(t *testing.T) { From 293da205e6324953b0f6c94f985b00d042b5cc80 Mon Sep 17 00:00:00 2001 From: yp05327 <576951401@qq.com> Date: Mon, 10 Jun 2024 07:51:21 +0000 Subject: [PATCH 11/11] revert changes in tests --- routers/web/repo/pull_review_test.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/routers/web/repo/pull_review_test.go b/routers/web/repo/pull_review_test.go index 464b56498cf64..8344ff409113f 100644 --- a/routers/web/repo/pull_review_test.go +++ b/routers/web/repo/pull_review_test.go @@ -56,29 +56,29 @@ func TestRenderConversation(t *testing.T) { } run("diff with outdated", func(t *testing.T, ctx *context.Context, resp *httptest.ResponseRecorder) { ctx.Data["ShowOutdatedComments"] = true - renderConversation(ctx, preparedComment, "diff", preparedComment.GetLineContent()) + renderConversation(ctx, preparedComment, "diff") assert.Contains(t, resp.Body.String(), `