Add attachment support for code review comments (#29220)

Fixes #27960, #24411, #12183

---------

Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>
This commit is contained in:
Jimmy Praet 2024-02-25 07:00:55 +01:00 committed by Earl Warren
parent 590f86796b
commit f95fb8cc44
15 changed files with 227 additions and 74 deletions

View file

@ -863,6 +863,9 @@ func updateCommentInfos(ctx context.Context, opts *CreateCommentOptions, comment
// Check comment type. // Check comment type.
switch opts.Type { switch opts.Type {
case CommentTypeCode: case CommentTypeCode:
if err = updateAttachments(ctx, opts, comment); err != nil {
return err
}
if comment.ReviewID != 0 { if comment.ReviewID != 0 {
if comment.Review == nil { if comment.Review == nil {
if err := comment.loadReview(ctx); err != nil { if err := comment.loadReview(ctx); err != nil {
@ -880,12 +883,23 @@ func updateCommentInfos(ctx context.Context, opts *CreateCommentOptions, comment
} }
fallthrough fallthrough
case CommentTypeReview: case CommentTypeReview:
// Check attachments if err = updateAttachments(ctx, opts, comment); err != nil {
return err
}
case CommentTypeReopen, CommentTypeClose:
if err = repo_model.UpdateRepoIssueNumbers(ctx, opts.Issue.RepoID, opts.Issue.IsPull, true); err != nil {
return err
}
}
// update the issue's updated_unix column
return UpdateIssueCols(ctx, opts.Issue, "updated_unix")
}
func updateAttachments(ctx context.Context, opts *CreateCommentOptions, comment *Comment) error {
attachments, err := repo_model.GetAttachmentsByUUIDs(ctx, opts.Attachments) attachments, err := repo_model.GetAttachmentsByUUIDs(ctx, opts.Attachments)
if err != nil { if err != nil {
return fmt.Errorf("getAttachmentsByUUIDs [uuids: %v]: %w", opts.Attachments, err) return fmt.Errorf("getAttachmentsByUUIDs [uuids: %v]: %w", opts.Attachments, err)
} }
for i := range attachments { for i := range attachments {
attachments[i].IssueID = opts.Issue.ID attachments[i].IssueID = opts.Issue.ID
attachments[i].CommentID = comment.ID attachments[i].CommentID = comment.ID
@ -894,15 +908,8 @@ func updateCommentInfos(ctx context.Context, opts *CreateCommentOptions, comment
return fmt.Errorf("update attachment [%d]: %w", attachments[i].ID, err) return fmt.Errorf("update attachment [%d]: %w", attachments[i].ID, err)
} }
} }
comment.Attachments = attachments comment.Attachments = attachments
case CommentTypeReopen, CommentTypeClose: return nil
if err = repo_model.UpdateRepoIssueNumbers(ctx, opts.Issue.RepoID, opts.Issue.IsPull, true); err != nil {
return err
}
}
// update the issue's updated_unix column
return UpdateIssueCols(ctx, opts.Issue, "updated_unix")
} }
func createDeadlineComment(ctx context.Context, doer *user_model.User, issue *Issue, newDeadlineUnix timeutil.TimeStamp) (*Comment, error) { func createDeadlineComment(ctx context.Context, doer *user_model.User, issue *Issue, newDeadlineUnix timeutil.TimeStamp) (*Comment, error) {

View file

@ -508,6 +508,7 @@ func CreatePullReview(ctx *context.APIContext) {
true, // pending review true, // pending review
0, // no reply 0, // no reply
opts.CommitID, opts.CommitID,
nil,
); err != nil { ); err != nil {
ctx.Error(http.StatusInternalServerError, "CreateCodeComment", err) ctx.Error(http.StatusInternalServerError, "CreateCodeComment", err)
return return

View file

@ -1727,6 +1727,10 @@ func ViewIssue(ctx *context.Context) {
for _, codeComments := range comment.Review.CodeComments { for _, codeComments := range comment.Review.CodeComments {
for _, lineComments := range codeComments { for _, lineComments := range codeComments {
for _, c := range lineComments { for _, c := range lineComments {
if err := c.LoadAttachments(ctx); err != nil {
ctx.ServerError("LoadAttachments", err)
return
}
// Check tag. // Check tag.
role, ok = marked[c.PosterID] role, ok = marked[c.PosterID]
if ok { if ok {

View file

@ -986,6 +986,19 @@ func viewPullFiles(ctx *context.Context, specifiedStartCommit, specifiedEndCommi
return return
} }
for _, file := range diff.Files {
for _, section := range file.Sections {
for _, line := range section.Lines {
for _, comment := range line.Comments {
if err := comment.LoadAttachments(ctx); err != nil {
ctx.ServerError("LoadAttachments", err)
return
}
}
}
}
}
pb, err := git_model.GetFirstMatchProtectedBranchRule(ctx, pull.BaseRepoID, pull.BaseBranch) pb, err := git_model.GetFirstMatchProtectedBranchRule(ctx, pull.BaseRepoID, pull.BaseBranch)
if err != nil { if err != nil {
ctx.ServerError("LoadProtectedBranch", err) ctx.ServerError("LoadProtectedBranch", err)

View file

@ -15,6 +15,7 @@ import (
"code.gitea.io/gitea/modules/json" "code.gitea.io/gitea/modules/json"
"code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/log"
"code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/setting"
"code.gitea.io/gitea/modules/upload"
"code.gitea.io/gitea/modules/web" "code.gitea.io/gitea/modules/web"
"code.gitea.io/gitea/services/forms" "code.gitea.io/gitea/services/forms"
pull_service "code.gitea.io/gitea/services/pull" pull_service "code.gitea.io/gitea/services/pull"
@ -49,6 +50,8 @@ func RenderNewCodeCommentForm(ctx *context.Context) {
return return
} }
ctx.Data["AfterCommitID"] = pullHeadCommitID ctx.Data["AfterCommitID"] = pullHeadCommitID
ctx.Data["IsAttachmentEnabled"] = setting.Attachment.Enabled
upload.AddUploadContext(ctx, "comment")
ctx.HTML(http.StatusOK, tplNewComment) ctx.HTML(http.StatusOK, tplNewComment)
} }
@ -74,6 +77,11 @@ func CreateCodeComment(ctx *context.Context) {
signedLine *= -1 signedLine *= -1
} }
var attachments []string
if setting.Attachment.Enabled {
attachments = form.Files
}
comment, err := pull_service.CreateCodeComment(ctx, comment, err := pull_service.CreateCodeComment(ctx,
ctx.Doer, ctx.Doer,
ctx.Repo.GitRepo, ctx.Repo.GitRepo,
@ -84,6 +92,7 @@ func CreateCodeComment(ctx *context.Context) {
!form.SingleReview, !form.SingleReview,
form.Reply, form.Reply,
form.LatestCommitID, form.LatestCommitID,
attachments,
) )
if err != nil { if err != nil {
ctx.ServerError("CreateCodeComment", err) ctx.ServerError("CreateCodeComment", err)
@ -159,6 +168,17 @@ func renderConversation(ctx *context.Context, comment *issues_model.Comment, ori
return return
} }
ctx.Data["PageIsPullFiles"] = (origin == "diff") ctx.Data["PageIsPullFiles"] = (origin == "diff")
for _, c := range comments {
if err := c.LoadAttachments(ctx); err != nil {
ctx.ServerError("LoadAttachments", err)
return
}
}
ctx.Data["IsAttachmentEnabled"] = setting.Attachment.Enabled
upload.AddUploadContext(ctx, "comment")
ctx.Data["comments"] = comments ctx.Data["comments"] = comments
if ctx.Data["CanMarkConversation"], err = issues_model.CanMarkConversation(ctx, comment.Issue, ctx.Doer); err != nil { if ctx.Data["CanMarkConversation"], err = issues_model.CanMarkConversation(ctx, comment.Issue, ctx.Doer); err != nil {
ctx.ServerError("CanMarkConversation", err) ctx.ServerError("CanMarkConversation", err)

View file

@ -0,0 +1,76 @@
// Copyright 2024 The Gitea Authors. All rights reserved.
// SPDX-License-Identifier: MIT
package repo
import (
"net/http/httptest"
"testing"
"code.gitea.io/gitea/models/db"
issues_model "code.gitea.io/gitea/models/issues"
"code.gitea.io/gitea/models/unittest"
"code.gitea.io/gitea/modules/context"
"code.gitea.io/gitea/modules/contexttest"
"code.gitea.io/gitea/modules/templates"
"code.gitea.io/gitea/services/pull"
"github.com/stretchr/testify/assert"
)
func TestRenderConversation(t *testing.T) {
unittest.PrepareTestEnv(t)
pr, _ := issues_model.GetPullRequestByID(db.DefaultContext, 2)
_ = pr.LoadIssue(db.DefaultContext)
_ = pr.Issue.LoadPoster(db.DefaultContext)
_ = pr.Issue.LoadRepo(db.DefaultContext)
run := func(name string, cb func(t *testing.T, ctx *context.Context, resp *httptest.ResponseRecorder)) {
t.Run(name, func(t *testing.T) {
ctx, resp := contexttest.MockContext(t, "/", contexttest.MockContextOption{Render: templates.HTMLRenderer()})
contexttest.LoadUser(t, ctx, pr.Issue.PosterID)
contexttest.LoadRepo(t, ctx, pr.BaseRepoID)
contexttest.LoadGitRepo(t, ctx)
defer ctx.Repo.GitRepo.Close()
cb(t, ctx, resp)
})
}
var preparedComment *issues_model.Comment
run("prepare", func(t *testing.T, ctx *context.Context, resp *httptest.ResponseRecorder) {
comment, err := pull.CreateCodeComment(ctx, pr.Issue.Poster, ctx.Repo.GitRepo, pr.Issue, 1, "content", "", false, 0, pr.HeadCommitID, nil)
if !assert.NoError(t, err) {
return
}
comment.Invalidated = true
err = issues_model.UpdateCommentInvalidate(ctx, comment)
if !assert.NoError(t, err) {
return
}
preparedComment = comment
})
if !assert.NotNil(t, preparedComment) {
return
}
run("diff with outdated", func(t *testing.T, ctx *context.Context, resp *httptest.ResponseRecorder) {
ctx.Data["ShowOutdatedComments"] = true
renderConversation(ctx, preparedComment, "diff")
assert.Contains(t, resp.Body.String(), `<div class="content comment-container"`)
})
run("diff without outdated", func(t *testing.T, ctx *context.Context, resp *httptest.ResponseRecorder) {
ctx.Data["ShowOutdatedComments"] = false
renderConversation(ctx, preparedComment, "diff")
assert.Contains(t, resp.Body.String(), `conversation-not-existing`)
})
run("timeline with outdated", func(t *testing.T, ctx *context.Context, resp *httptest.ResponseRecorder) {
ctx.Data["ShowOutdatedComments"] = true
renderConversation(ctx, preparedComment, "timeline")
assert.Contains(t, resp.Body.String(), `<div id="code-comments-`)
})
run("timeline is not affected by ShowOutdatedComments=false", func(t *testing.T, ctx *context.Context, resp *httptest.ResponseRecorder) {
ctx.Data["ShowOutdatedComments"] = false
renderConversation(ctx, preparedComment, "timeline")
assert.Contains(t, resp.Body.String(), `<div id="code-comments-`)
})
}

View file

@ -637,6 +637,7 @@ type CodeCommentForm struct {
SingleReview bool `form:"single_review"` SingleReview bool `form:"single_review"`
Reply int64 `form:"reply"` Reply int64 `form:"reply"`
LatestCommitID string LatestCommitID string
Files []string
} }
// Validate validates the fields // Validate validates the fields

View file

@ -130,6 +130,7 @@ func (h *ReplyHandler) Handle(ctx context.Context, content *MailContent, doer *u
false, // not pending review but a single review false, // not pending review but a single review
comment.ReviewID, comment.ReviewID,
"", "",
nil,
) )
if err != nil { if err != nil {
return fmt.Errorf("CreateCodeComment failed: %w", err) return fmt.Errorf("CreateCodeComment failed: %w", err)

View file

@ -71,7 +71,7 @@ func InvalidateCodeComments(ctx context.Context, prs issues_model.PullRequestLis
} }
// CreateCodeComment creates a comment on the code line // CreateCodeComment creates a comment on the code line
func CreateCodeComment(ctx context.Context, doer *user_model.User, gitRepo *git.Repository, issue *issues_model.Issue, line int64, content, treePath string, pendingReview bool, replyReviewID int64, latestCommitID string) (*issues_model.Comment, error) { func CreateCodeComment(ctx context.Context, doer *user_model.User, gitRepo *git.Repository, issue *issues_model.Issue, line int64, content, treePath string, pendingReview bool, replyReviewID int64, latestCommitID string, attachments []string) (*issues_model.Comment, error) {
var ( var (
existsReview bool existsReview bool
err error err error
@ -104,6 +104,7 @@ func CreateCodeComment(ctx context.Context, doer *user_model.User, gitRepo *git.
treePath, treePath,
line, line,
replyReviewID, replyReviewID,
attachments,
) )
if err != nil { if err != nil {
return nil, err return nil, err
@ -144,6 +145,7 @@ func CreateCodeComment(ctx context.Context, doer *user_model.User, gitRepo *git.
treePath, treePath,
line, line,
review.ID, review.ID,
attachments,
) )
if err != nil { if err != nil {
return nil, err return nil, err
@ -161,8 +163,8 @@ func CreateCodeComment(ctx context.Context, doer *user_model.User, gitRepo *git.
return comment, nil return comment, nil
} }
// createCodeComment creates a plain code comment at the specified line / path // CreateCodeCommentKnownReviewID creates a plain code comment at the specified line / path
func CreateCodeCommentKnownReviewID(ctx context.Context, doer *user_model.User, repo *repo_model.Repository, issue *issues_model.Issue, content, treePath string, line, reviewID int64) (*issues_model.Comment, error) { func CreateCodeCommentKnownReviewID(ctx context.Context, doer *user_model.User, repo *repo_model.Repository, issue *issues_model.Issue, content, treePath string, line, reviewID int64, attachments []string) (*issues_model.Comment, error) {
var commitID, patch string var commitID, patch string
if err := issue.LoadPullRequest(ctx); err != nil { if err := issue.LoadPullRequest(ctx); err != nil {
return nil, fmt.Errorf("LoadPullRequest: %w", err) return nil, fmt.Errorf("LoadPullRequest: %w", err)
@ -260,6 +262,7 @@ func CreateCodeCommentKnownReviewID(ctx context.Context, doer *user_model.User,
ReviewID: reviewID, ReviewID: reviewID,
Patch: patch, Patch: patch,
Invalidated: invalidated, Invalidated: invalidated,
Attachments: attachments,
}) })
} }

View file

@ -240,6 +240,11 @@
"TextareaName" "content" "TextareaName" "content"
"DropzoneParentContainer" ".ui.form" "DropzoneParentContainer" ".ui.form"
)}} )}}
{{if .IsAttachmentEnabled}}
<div class="field">
{{template "repo/upload" .}}
</div>
{{end}}
<div class="text right edit buttons"> <div class="text right edit buttons">
<button class="ui cancel button">{{ctx.Locale.Tr "repo.issues.cancel"}}</button> <button class="ui cancel button">{{ctx.Locale.Tr "repo.issues.cancel"}}</button>
<button class="ui primary save button">{{ctx.Locale.Tr "repo.issues.save"}}</button> <button class="ui primary save button">{{ctx.Locale.Tr "repo.issues.save"}}</button>

View file

@ -19,6 +19,12 @@
"DisableAutosize" "true" "DisableAutosize" "true"
)}} )}}
{{if $.root.IsAttachmentEnabled}}
<div class="field">
{{template "repo/upload" $.root}}
</div>
{{end}}
<div class="field footer gt-mx-3"> <div class="field footer gt-mx-3">
<span class="markup-info">{{svg "octicon-markup"}} {{ctx.Locale.Tr "repo.diff.comment.markdown_info"}}</span> <span class="markup-info">{{svg "octicon-markup"}} {{ctx.Locale.Tr "repo.diff.comment.markdown_info"}}</span>
<div class="gt-text-right"> <div class="gt-text-right">

View file

@ -61,7 +61,10 @@
{{end}} {{end}}
</div> </div>
<div id="issuecomment-{{.ID}}-raw" class="raw-content gt-hidden">{{.Content}}</div> <div id="issuecomment-{{.ID}}-raw" class="raw-content gt-hidden">{{.Content}}</div>
<div class="edit-content-zone gt-hidden" data-update-url="{{$.root.RepoLink}}/comments/{{.ID}}" data-context="{{$.root.RepoLink}}"></div> <div class="edit-content-zone gt-hidden" data-update-url="{{$.root.RepoLink}}/comments/{{.ID}}" data-context="{{$.root.RepoLink}}" data-attachment-url="{{$.root.RepoLink}}/comments/{{.ID}}/attachments"></div>
{{if .Attachments}}
{{template "repo/issue/view_content/attachments" dict "ctxData" $ "Attachments" .Attachments "Content" .RenderedContent}}
{{end}}
</div> </div>
{{$reactions := .Reactions.GroupByType}} {{$reactions := .Reactions.GroupByType}}
{{if $reactions}} {{if $reactions}}

View file

@ -94,6 +94,9 @@
</div> </div>
<div id="issuecomment-{{.ID}}-raw" class="raw-content gt-hidden">{{.Content}}</div> <div id="issuecomment-{{.ID}}-raw" class="raw-content gt-hidden">{{.Content}}</div>
<div class="edit-content-zone gt-hidden" data-update-url="{{$.RepoLink}}/comments/{{.ID}}" data-context="{{$.RepoLink}}" data-attachment-url="{{$.RepoLink}}/comments/{{.ID}}/attachments"></div> <div class="edit-content-zone gt-hidden" data-update-url="{{$.RepoLink}}/comments/{{.ID}}" data-context="{{$.RepoLink}}" data-attachment-url="{{$.RepoLink}}/comments/{{.ID}}/attachments"></div>
{{if .Attachments}}
{{template "repo/issue/view_content/attachments" dict "ctxData" $ "Attachments" .Attachments "Content" .RenderedContent}}
{{end}}
</div> </div>
{{$reactions := .Reactions.GroupByType}} {{$reactions := .Reactions.GroupByType}}
{{if $reactions}} {{if $reactions}}

View file

@ -200,8 +200,12 @@ export function initGlobalCommon() {
} }
export function initGlobalDropzone() { export function initGlobalDropzone() {
// Dropzone
for (const el of document.querySelectorAll('.dropzone')) { for (const el of document.querySelectorAll('.dropzone')) {
initDropzone(el);
}
}
export function initDropzone(el) {
const $dropzone = $(el); const $dropzone = $(el);
const _promise = createDropzone(el, { const _promise = createDropzone(el, {
url: $dropzone.data('upload-url'), url: $dropzone.data('upload-url'),
@ -257,7 +261,6 @@ export function initGlobalDropzone() {
}, },
}); });
} }
}
async function linkAction(e) { async function linkAction(e) {
// A "link-action" can post AJAX request to its "data-url" // A "link-action" can post AJAX request to its "data-url"

View file

@ -5,6 +5,7 @@ import {hideElem, showElem, toggleElem} from '../utils/dom.js';
import {setFileFolding} from './file-fold.js'; import {setFileFolding} from './file-fold.js';
import {getComboMarkdownEditor, initComboMarkdownEditor} from './comp/ComboMarkdownEditor.js'; import {getComboMarkdownEditor, initComboMarkdownEditor} from './comp/ComboMarkdownEditor.js';
import {toAbsoluteUrl} from '../utils.js'; import {toAbsoluteUrl} from '../utils.js';
import {initDropzone} from './common-global.js';
const {appSubUrl, csrfToken} = window.config; const {appSubUrl, csrfToken} = window.config;
@ -382,6 +383,11 @@ export async function handleReply($el) {
const $textarea = form.find('textarea'); const $textarea = form.find('textarea');
let editor = getComboMarkdownEditor($textarea); let editor = getComboMarkdownEditor($textarea);
if (!editor) { if (!editor) {
// FIXME: the initialization of the dropzone is not consistent.
// When the page is loaded, the dropzone is initialized by initGlobalDropzone, but the editor is not initialized.
// When the form is submitted and partially reload, none of them is initialized.
const dropzone = form.find('.dropzone')[0];
if (!dropzone.dropzone) initDropzone(dropzone);
editor = await initComboMarkdownEditor(form.find('.combo-markdown-editor')); editor = await initComboMarkdownEditor(form.find('.combo-markdown-editor'));
} }
editor.focus(); editor.focus();
@ -511,6 +517,7 @@ export function initRepoPullRequestReview() {
td.find("input[name='side']").val(side === 'left' ? 'previous' : 'proposed'); td.find("input[name='side']").val(side === 'left' ? 'previous' : 'proposed');
td.find("input[name='path']").val(path); td.find("input[name='path']").val(path);
initDropzone(td.find('.dropzone')[0]);
const editor = await initComboMarkdownEditor(td.find('.combo-markdown-editor')); const editor = await initComboMarkdownEditor(td.find('.combo-markdown-editor'));
editor.focus(); editor.focus();
} }