Skip to content

Commit 2e33671

Browse files
jpraetwxiaoguang
jpraet
and
wxiaoguang
authored
Add attachment support for code review comments (#29220)
Fixes #27960, #24411, #12183 --------- Co-authored-by: wxiaoguang <[email protected]>
1 parent 1ef8777 commit 2e33671

File tree

15 files changed

+149
-73
lines changed

15 files changed

+149
-73
lines changed

‎models/issues/comment.go

Lines changed: 22 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -855,6 +855,9 @@ func updateCommentInfos(ctx context.Context, opts *CreateCommentOptions, comment
855855
// Check comment type.
856856
switch opts.Type {
857857
case CommentTypeCode:
858+
if err = updateAttachments(ctx, opts, comment); err != nil {
859+
return err
860+
}
858861
if comment.ReviewID != 0 {
859862
if comment.Review == nil {
860863
if err := comment.loadReview(ctx); err != nil {
@@ -872,22 +875,9 @@ func updateCommentInfos(ctx context.Context, opts *CreateCommentOptions, comment
872875
}
873876
fallthrough
874877
case CommentTypeReview:
875-
// Check attachments
876-
attachments, err := repo_model.GetAttachmentsByUUIDs(ctx, opts.Attachments)
877-
if err != nil {
878-
return fmt.Errorf("getAttachmentsByUUIDs [uuids: %v]: %w", opts.Attachments, err)
879-
}
880-
881-
for i := range attachments {
882-
attachments[i].IssueID = opts.Issue.ID
883-
attachments[i].CommentID = comment.ID
884-
// No assign value could be 0, so ignore AllCols().
885-
if _, err = db.GetEngine(ctx).ID(attachments[i].ID).Update(attachments[i]); err != nil {
886-
return fmt.Errorf("update attachment [%d]: %w", attachments[i].ID, err)
887-
}
878+
if err = updateAttachments(ctx, opts, comment); err != nil {
879+
return err
888880
}
889-
890-
comment.Attachments = attachments
891881
case CommentTypeReopen, CommentTypeClose:
892882
if err = repo_model.UpdateRepoIssueNumbers(ctx, opts.Issue.RepoID, opts.Issue.IsPull, true); err != nil {
893883
return err
@@ -897,6 +887,23 @@ func updateCommentInfos(ctx context.Context, opts *CreateCommentOptions, comment
897887
return UpdateIssueCols(ctx, opts.Issue, "updated_unix")
898888
}
899889

890+
func updateAttachments(ctx context.Context, opts *CreateCommentOptions, comment *Comment) error {
891+
attachments, err := repo_model.GetAttachmentsByUUIDs(ctx, opts.Attachments)
892+
if err != nil {
893+
return fmt.Errorf("getAttachmentsByUUIDs [uuids: %v]: %w", opts.Attachments, err)
894+
}
895+
for i := range attachments {
896+
attachments[i].IssueID = opts.Issue.ID
897+
attachments[i].CommentID = comment.ID
898+
// No assign value could be 0, so ignore AllCols().
899+
if _, err = db.GetEngine(ctx).ID(attachments[i].ID).Update(attachments[i]); err != nil {
900+
return fmt.Errorf("update attachment [%d]: %w", attachments[i].ID, err)
901+
}
902+
}
903+
comment.Attachments = attachments
904+
return nil
905+
}
906+
900907
func createDeadlineComment(ctx context.Context, doer *user_model.User, issue *Issue, newDeadlineUnix timeutil.TimeStamp) (*Comment, error) {
901908
var content string
902909
var commentType CommentType

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -362,6 +362,7 @@ func CreatePullReview(ctx *context.APIContext) {
362362
true, // pending review
363363
0, // no reply
364364
opts.CommitID,
365+
nil,
365366
); err != nil {
366367
ctx.Error(http.StatusInternalServerError, "CreateCodeComment", err)
367368
return

‎routers/web/repo/issue.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1718,6 +1718,10 @@ func ViewIssue(ctx *context.Context) {
17181718
for _, codeComments := range comment.Review.CodeComments {
17191719
for _, lineComments := range codeComments {
17201720
for _, c := range lineComments {
1721+
if err := c.LoadAttachments(ctx); err != nil {
1722+
ctx.ServerError("LoadAttachments", err)
1723+
return
1724+
}
17211725
// Check tag.
17221726
role, ok = marked[c.PosterID]
17231727
if ok {

‎routers/web/repo/pull.go

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -970,6 +970,19 @@ func viewPullFiles(ctx *context.Context, specifiedStartCommit, specifiedEndCommi
970970
return
971971
}
972972

973+
for _, file := range diff.Files {
974+
for _, section := range file.Sections {
975+
for _, line := range section.Lines {
976+
for _, comment := range line.Comments {
977+
if err := comment.LoadAttachments(ctx); err != nil {
978+
ctx.ServerError("LoadAttachments", err)
979+
return
980+
}
981+
}
982+
}
983+
}
984+
}
985+
973986
pb, err := git_model.GetFirstMatchProtectedBranchRule(ctx, pull.BaseRepoID, pull.BaseBranch)
974987
if err != nil {
975988
ctx.ServerError("LoadProtectedBranch", err)

‎routers/web/repo/pull_review.go

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import (
1515
"code.gitea.io/gitea/modules/json"
1616
"code.gitea.io/gitea/modules/log"
1717
"code.gitea.io/gitea/modules/setting"
18+
"code.gitea.io/gitea/modules/upload"
1819
"code.gitea.io/gitea/modules/web"
1920
"code.gitea.io/gitea/services/forms"
2021
pull_service "code.gitea.io/gitea/services/pull"
@@ -50,6 +51,8 @@ func RenderNewCodeCommentForm(ctx *context.Context) {
5051
return
5152
}
5253
ctx.Data["AfterCommitID"] = pullHeadCommitID
54+
ctx.Data["IsAttachmentEnabled"] = setting.Attachment.Enabled
55+
upload.AddUploadContext(ctx, "comment")
5356
ctx.HTML(http.StatusOK, tplNewComment)
5457
}
5558

@@ -75,6 +78,11 @@ func CreateCodeComment(ctx *context.Context) {
7578
signedLine *= -1
7679
}
7780

81+
var attachments []string
82+
if setting.Attachment.Enabled {
83+
attachments = form.Files
84+
}
85+
7886
comment, err := pull_service.CreateCodeComment(ctx,
7987
ctx.Doer,
8088
ctx.Repo.GitRepo,
@@ -85,6 +93,7 @@ func CreateCodeComment(ctx *context.Context) {
8593
!form.SingleReview,
8694
form.Reply,
8795
form.LatestCommitID,
96+
attachments,
8897
)
8998
if err != nil {
9099
ctx.ServerError("CreateCodeComment", err)
@@ -168,6 +177,16 @@ func renderConversation(ctx *context.Context, comment *issues_model.Comment, ori
168177
return
169178
}
170179

180+
for _, c := range comments {
181+
if err := c.LoadAttachments(ctx); err != nil {
182+
ctx.ServerError("LoadAttachments", err)
183+
return
184+
}
185+
}
186+
187+
ctx.Data["IsAttachmentEnabled"] = setting.Attachment.Enabled
188+
upload.AddUploadContext(ctx, "comment")
189+
171190
ctx.Data["comments"] = comments
172191
if ctx.Data["CanMarkConversation"], err = issues_model.CanMarkConversation(ctx, comment.Issue, ctx.Doer); err != nil {
173192
ctx.ServerError("CanMarkConversation", err)

‎routers/web/repo/pull_review_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ func TestRenderConversation(t *testing.T) {
3939

4040
var preparedComment *issues_model.Comment
4141
run("prepare", func(t *testing.T, ctx *context.Context, resp *httptest.ResponseRecorder) {
42-
comment, err := pull.CreateCodeComment(ctx, pr.Issue.Poster, ctx.Repo.GitRepo, pr.Issue, 1, "content", "", false, 0, pr.HeadCommitID)
42+
comment, err := pull.CreateCodeComment(ctx, pr.Issue.Poster, ctx.Repo.GitRepo, pr.Issue, 1, "content", "", false, 0, pr.HeadCommitID, nil)
4343
if !assert.NoError(t, err) {
4444
return
4545
}

‎services/forms/repo_form.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -626,6 +626,7 @@ type CodeCommentForm struct {
626626
SingleReview bool `form:"single_review"`
627627
Reply int64 `form:"reply"`
628628
LatestCommitID string
629+
Files []string
629630
}
630631

631632
// Validate validates the fields

‎services/mailer/incoming/incoming_handler.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,7 @@ func (h *ReplyHandler) Handle(ctx context.Context, content *MailContent, doer *u
130130
false, // not pending review but a single review
131131
comment.ReviewID,
132132
"",
133+
nil,
133134
)
134135
if err != nil {
135136
return fmt.Errorf("CreateCodeComment failed: %w", err)

‎services/pull/review.go

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ func InvalidateCodeComments(ctx context.Context, prs issues_model.PullRequestLis
7171
}
7272

7373
// CreateCodeComment creates a comment on the code line
74-
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) {
74+
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) {
7575
var (
7676
existsReview bool
7777
err error
@@ -104,6 +104,7 @@ func CreateCodeComment(ctx context.Context, doer *user_model.User, gitRepo *git.
104104
treePath,
105105
line,
106106
replyReviewID,
107+
attachments,
107108
)
108109
if err != nil {
109110
return nil, err
@@ -144,6 +145,7 @@ func CreateCodeComment(ctx context.Context, doer *user_model.User, gitRepo *git.
144145
treePath,
145146
line,
146147
review.ID,
148+
attachments,
147149
)
148150
if err != nil {
149151
return nil, err
@@ -162,7 +164,7 @@ func CreateCodeComment(ctx context.Context, doer *user_model.User, gitRepo *git.
162164
}
163165

164166
// createCodeComment creates a plain code comment at the specified line / path
165-
func createCodeComment(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) {
167+
func createCodeComment(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) {
166168
var commitID, patch string
167169
if err := issue.LoadPullRequest(ctx); err != nil {
168170
return nil, fmt.Errorf("LoadPullRequest: %w", err)
@@ -260,6 +262,7 @@ func createCodeComment(ctx context.Context, doer *user_model.User, repo *repo_mo
260262
ReviewID: reviewID,
261263
Patch: patch,
262264
Invalidated: invalidated,
265+
Attachments: attachments,
263266
})
264267
}
265268

‎templates/repo/diff/box.tmpl

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -237,6 +237,11 @@
237237
"TextareaName" "content"
238238
"DropzoneParentContainer" ".ui.form"
239239
)}}
240+
{{if .IsAttachmentEnabled}}
241+
<div class="field">
242+
{{template "repo/upload" .}}
243+
</div>
244+
{{end}}
240245
<div class="text right edit buttons">
241246
<button class="ui cancel button">{{ctx.Locale.Tr "repo.issues.cancel"}}</button>
242247
<button class="ui primary save button">{{ctx.Locale.Tr "repo.issues.save"}}</button>

‎templates/repo/diff/comment_form.tmpl

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,12 @@
1919
"DisableAutosize" "true"
2020
)}}
2121

22+
{{if $.root.IsAttachmentEnabled}}
23+
<div class="field">
24+
{{template "repo/upload" $.root}}
25+
</div>
26+
{{end}}
27+
2228
<div class="field footer gt-mx-3">
2329
<span class="markup-info">{{svg "octicon-markup"}} {{ctx.Locale.Tr "repo.diff.comment.markdown_info"}}</span>
2430
<div class="gt-text-right">

‎templates/repo/diff/comments.tmpl

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,10 @@
6161
{{end}}
6262
</div>
6363
<div id="issuecomment-{{.ID}}-raw" class="raw-content gt-hidden">{{.Content}}</div>
64-
<div class="edit-content-zone gt-hidden" data-update-url="{{$.root.RepoLink}}/comments/{{.ID}}" data-context="{{$.root.RepoLink}}"></div>
64+
<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>
65+
{{if .Attachments}}
66+
{{template "repo/issue/view_content/attachments" dict "ctxData" $ "Attachments" .Attachments "Content" .RenderedContent}}
67+
{{end}}
6568
</div>
6669
{{$reactions := .Reactions.GroupByType}}
6770
{{if $reactions}}

‎templates/repo/issue/view_content/conversation.tmpl

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,9 @@
9494
</div>
9595
<div id="issuecomment-{{.ID}}-raw" class="raw-content gt-hidden">{{.Content}}</div>
9696
<div class="edit-content-zone gt-hidden" data-update-url="{{$.RepoLink}}/comments/{{.ID}}" data-context="{{$.RepoLink}}" data-attachment-url="{{$.RepoLink}}/comments/{{.ID}}/attachments"></div>
97+
{{if .Attachments}}
98+
{{template "repo/issue/view_content/attachments" dict "ctxData" $ "Attachments" .Attachments "Content" .RenderedContent}}
99+
{{end}}
97100
</div>
98101
{{$reactions := .Reactions.GroupByType}}
99102
{{if $reactions}}

‎web_src/js/features/common-global.js

Lines changed: 57 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -200,63 +200,66 @@ export function initGlobalCommon() {
200200
}
201201

202202
export function initGlobalDropzone() {
203-
// Dropzone
204203
for (const el of document.querySelectorAll('.dropzone')) {
205-
const $dropzone = $(el);
206-
const _promise = createDropzone(el, {
207-
url: $dropzone.data('upload-url'),
208-
headers: {'X-Csrf-Token': csrfToken},
209-
maxFiles: $dropzone.data('max-file'),
210-
maxFilesize: $dropzone.data('max-size'),
211-
acceptedFiles: (['*/*', ''].includes($dropzone.data('accepts'))) ? null : $dropzone.data('accepts'),
212-
addRemoveLinks: true,
213-
dictDefaultMessage: $dropzone.data('default-message'),
214-
dictInvalidFileType: $dropzone.data('invalid-input-type'),
215-
dictFileTooBig: $dropzone.data('file-too-big'),
216-
dictRemoveFile: $dropzone.data('remove-file'),
217-
timeout: 0,
218-
thumbnailMethod: 'contain',
219-
thumbnailWidth: 480,
220-
thumbnailHeight: 480,
221-
init() {
222-
this.on('success', (file, data) => {
223-
file.uuid = data.uuid;
224-
const input = $(`<input id="${data.uuid}" name="files" type="hidden">`).val(data.uuid);
225-
$dropzone.find('.files').append(input);
226-
// Create a "Copy Link" element, to conveniently copy the image
227-
// or file link as Markdown to the clipboard
228-
const copyLinkElement = document.createElement('div');
229-
copyLinkElement.className = 'gt-text-center';
230-
// The a element has a hardcoded cursor: pointer because the default is overridden by .dropzone
231-
copyLinkElement.innerHTML = `<a href="#" style="cursor: pointer;">${svg('octicon-copy', 14, 'copy link')} Copy link</a>`;
232-
copyLinkElement.addEventListener('click', async (e) => {
233-
e.preventDefault();
234-
let fileMarkdown = `[${file.name}](/attachments/${file.uuid})`;
235-
if (file.type.startsWith('image/')) {
236-
fileMarkdown = `!${fileMarkdown}`;
237-
} else if (file.type.startsWith('video/')) {
238-
fileMarkdown = `<video src="/attachments/${file.uuid}" title="${htmlEscape(file.name)}" controls></video>`;
239-
}
240-
const success = await clippie(fileMarkdown);
241-
showTemporaryTooltip(e.target, success ? i18n.copy_success : i18n.copy_error);
242-
});
243-
file.previewTemplate.append(copyLinkElement);
244-
});
245-
this.on('removedfile', (file) => {
246-
$(`#${file.uuid}`).remove();
247-
if ($dropzone.data('remove-url')) {
248-
POST($dropzone.data('remove-url'), {
249-
data: new URLSearchParams({file: file.uuid}),
250-
});
204+
initDropzone(el);
205+
}
206+
}
207+
208+
export function initDropzone(el) {
209+
const $dropzone = $(el);
210+
const _promise = createDropzone(el, {
211+
url: $dropzone.data('upload-url'),
212+
headers: {'X-Csrf-Token': csrfToken},
213+
maxFiles: $dropzone.data('max-file'),
214+
maxFilesize: $dropzone.data('max-size'),
215+
acceptedFiles: (['*/*', ''].includes($dropzone.data('accepts'))) ? null : $dropzone.data('accepts'),
216+
addRemoveLinks: true,
217+
dictDefaultMessage: $dropzone.data('default-message'),
218+
dictInvalidFileType: $dropzone.data('invalid-input-type'),
219+
dictFileTooBig: $dropzone.data('file-too-big'),
220+
dictRemoveFile: $dropzone.data('remove-file'),
221+
timeout: 0,
222+
thumbnailMethod: 'contain',
223+
thumbnailWidth: 480,
224+
thumbnailHeight: 480,
225+
init() {
226+
this.on('success', (file, data) => {
227+
file.uuid = data.uuid;
228+
const input = $(`<input id="${data.uuid}" name="files" type="hidden">`).val(data.uuid);
229+
$dropzone.find('.files').append(input);
230+
// Create a "Copy Link" element, to conveniently copy the image
231+
// or file link as Markdown to the clipboard
232+
const copyLinkElement = document.createElement('div');
233+
copyLinkElement.className = 'gt-text-center';
234+
// The a element has a hardcoded cursor: pointer because the default is overridden by .dropzone
235+
copyLinkElement.innerHTML = `<a href="#" style="cursor: pointer;">${svg('octicon-copy', 14, 'copy link')} Copy link</a>`;
236+
copyLinkElement.addEventListener('click', async (e) => {
237+
e.preventDefault();
238+
let fileMarkdown = `[${file.name}](/attachments/${file.uuid})`;
239+
if (file.type.startsWith('image/')) {
240+
fileMarkdown = `!${fileMarkdown}`;
241+
} else if (file.type.startsWith('video/')) {
242+
fileMarkdown = `<video src="/attachments/${file.uuid}" title="${htmlEscape(file.name)}" controls></video>`;
251243
}
244+
const success = await clippie(fileMarkdown);
245+
showTemporaryTooltip(e.target, success ? i18n.copy_success : i18n.copy_error);
252246
});
253-
this.on('error', function (file, message) {
254-
showErrorToast(message);
255-
this.removeFile(file);
256-
});
257-
},
258-
});
259-
}
247+
file.previewTemplate.append(copyLinkElement);
248+
});
249+
this.on('removedfile', (file) => {
250+
$(`#${file.uuid}`).remove();
251+
if ($dropzone.data('remove-url')) {
252+
POST($dropzone.data('remove-url'), {
253+
data: new URLSearchParams({file: file.uuid}),
254+
});
255+
}
256+
});
257+
this.on('error', function (file, message) {
258+
showErrorToast(message);
259+
this.removeFile(file);
260+
});
261+
},
262+
});
260263
}
261264

262265
async function linkAction(e) {

0 commit comments

Comments
 (0)