Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add attachment support for code review comments #29220

Merged
merged 15 commits into from
Feb 25, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 22 additions & 15 deletions models/issues/comment.go
Original file line number Diff line number Diff line change
Expand Up @@ -855,6 +855,9 @@ func updateCommentInfos(ctx context.Context, opts *CreateCommentOptions, comment
// Check comment type.
switch opts.Type {
case CommentTypeCode:
if err = updateAttachments(ctx, opts, comment); err != nil {
return err
}
if comment.ReviewID != 0 {
if comment.Review == nil {
if err := comment.loadReview(ctx); err != nil {
Expand All @@ -872,22 +875,9 @@ func updateCommentInfos(ctx context.Context, opts *CreateCommentOptions, comment
}
fallthrough
case CommentTypeReview:
// Check attachments
attachments, err := repo_model.GetAttachmentsByUUIDs(ctx, opts.Attachments)
if err != nil {
return fmt.Errorf("getAttachmentsByUUIDs [uuids: %v]: %w", opts.Attachments, err)
}

for i := range attachments {
attachments[i].IssueID = opts.Issue.ID
attachments[i].CommentID = comment.ID
// No assign value could be 0, so ignore AllCols().
if _, err = db.GetEngine(ctx).ID(attachments[i].ID).Update(attachments[i]); err != nil {
return fmt.Errorf("update attachment [%d]: %w", attachments[i].ID, err)
}
if err = updateAttachments(ctx, opts, comment); err != nil {
return err
}

comment.Attachments = attachments
case CommentTypeReopen, CommentTypeClose:
if err = repo_model.UpdateRepoIssueNumbers(ctx, opts.Issue.RepoID, opts.Issue.IsPull, true); err != nil {
return err
Expand All @@ -897,6 +887,23 @@ func updateCommentInfos(ctx context.Context, opts *CreateCommentOptions, comment
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)
if err != nil {
return fmt.Errorf("getAttachmentsByUUIDs [uuids: %v]: %w", opts.Attachments, err)
}
for i := range attachments {
attachments[i].IssueID = opts.Issue.ID
attachments[i].CommentID = comment.ID
// No assign value could be 0, so ignore AllCols().
if _, err = db.GetEngine(ctx).ID(attachments[i].ID).Update(attachments[i]); err != nil {
return fmt.Errorf("update attachment [%d]: %w", attachments[i].ID, err)
}
}
comment.Attachments = attachments
return nil
}

func createDeadlineComment(ctx context.Context, doer *user_model.User, issue *Issue, newDeadlineUnix timeutil.TimeStamp) (*Comment, error) {
var content string
var commentType CommentType
Expand Down
1 change: 1 addition & 0 deletions routers/api/v1/repo/pull_review.go
Original file line number Diff line number Diff line change
Expand Up @@ -362,6 +362,7 @@ func CreatePullReview(ctx *context.APIContext) {
true, // pending review
0, // no reply
opts.CommitID,
nil,
); err != nil {
ctx.Error(http.StatusInternalServerError, "CreateCodeComment", err)
return
Expand Down
4 changes: 4 additions & 0 deletions routers/web/repo/issue.go
Original file line number Diff line number Diff line change
Expand Up @@ -1718,6 +1718,10 @@ func ViewIssue(ctx *context.Context) {
for _, codeComments := range comment.Review.CodeComments {
for _, lineComments := range codeComments {
for _, c := range lineComments {
if err := c.LoadAttachments(ctx); err != nil {
ctx.ServerError("LoadAttachments", err)
return
}
// Check tag.
role, ok = marked[c.PosterID]
if ok {
Expand Down
13 changes: 13 additions & 0 deletions routers/web/repo/pull.go
Original file line number Diff line number Diff line change
Expand Up @@ -970,6 +970,19 @@ func viewPullFiles(ctx *context.Context, specifiedStartCommit, specifiedEndCommi
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)
if err != nil {
ctx.ServerError("LoadProtectedBranch", err)
Expand Down
19 changes: 19 additions & 0 deletions routers/web/repo/pull_review.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (
"code.gitea.io/gitea/modules/json"
"code.gitea.io/gitea/modules/log"
"code.gitea.io/gitea/modules/setting"
"code.gitea.io/gitea/modules/upload"
"code.gitea.io/gitea/modules/web"
"code.gitea.io/gitea/services/forms"
pull_service "code.gitea.io/gitea/services/pull"
Expand Down Expand Up @@ -50,6 +51,8 @@ func RenderNewCodeCommentForm(ctx *context.Context) {
return
}
ctx.Data["AfterCommitID"] = pullHeadCommitID
ctx.Data["IsAttachmentEnabled"] = setting.Attachment.Enabled
upload.AddUploadContext(ctx, "comment")
ctx.HTML(http.StatusOK, tplNewComment)
}

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

var attachments []string
if setting.Attachment.Enabled {
attachments = form.Files
}

wxiaoguang marked this conversation as resolved.
Show resolved Hide resolved
comment, err := pull_service.CreateCodeComment(ctx,
ctx.Doer,
ctx.Repo.GitRepo,
Expand All @@ -85,6 +93,7 @@ func CreateCodeComment(ctx *context.Context) {
!form.SingleReview,
form.Reply,
form.LatestCommitID,
attachments,
wxiaoguang marked this conversation as resolved.
Show resolved Hide resolved
)
if err != nil {
ctx.ServerError("CreateCodeComment", err)
Expand Down Expand Up @@ -168,6 +177,16 @@ func renderConversation(ctx *context.Context, comment *issues_model.Comment, ori
return
}

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
if ctx.Data["CanMarkConversation"], err = issues_model.CanMarkConversation(ctx, comment.Issue, ctx.Doer); err != nil {
ctx.ServerError("CanMarkConversation", err)
Expand Down
2 changes: 1 addition & 1 deletion routers/web/repo/pull_review_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ func TestRenderConversation(t *testing.T) {

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)
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
}
Expand Down
1 change: 1 addition & 0 deletions services/forms/repo_form.go
Original file line number Diff line number Diff line change
Expand Up @@ -626,6 +626,7 @@ type CodeCommentForm struct {
SingleReview bool `form:"single_review"`
Reply int64 `form:"reply"`
LatestCommitID string
Files []string
}

// Validate validates the fields
Expand Down
1 change: 1 addition & 0 deletions services/mailer/incoming/incoming_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,7 @@ func (h *ReplyHandler) Handle(ctx context.Context, content *MailContent, doer *u
false, // not pending review but a single review
comment.ReviewID,
"",
nil,
)
if err != nil {
return fmt.Errorf("CreateCodeComment failed: %w", err)
Expand Down
7 changes: 5 additions & 2 deletions services/pull/review.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ func InvalidateCodeComments(ctx context.Context, prs issues_model.PullRequestLis
}

// 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 (
existsReview bool
err error
Expand Down Expand Up @@ -104,6 +104,7 @@ func CreateCodeComment(ctx context.Context, doer *user_model.User, gitRepo *git.
treePath,
line,
replyReviewID,
attachments,
)
if err != nil {
return nil, err
Expand Down Expand Up @@ -144,6 +145,7 @@ func CreateCodeComment(ctx context.Context, doer *user_model.User, gitRepo *git.
treePath,
line,
review.ID,
attachments,
)
if err != nil {
return nil, err
Expand All @@ -162,7 +164,7 @@ func CreateCodeComment(ctx context.Context, doer *user_model.User, gitRepo *git.
}

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

Expand Down
5 changes: 5 additions & 0 deletions templates/repo/diff/box.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -237,6 +237,11 @@
"TextareaName" "content"
"DropzoneParentContainer" ".ui.form"
)}}
{{if .IsAttachmentEnabled}}
<div class="field">
{{template "repo/upload" .}}
</div>
{{end}}
<div class="text right edit buttons">
<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>
Expand Down
6 changes: 6 additions & 0 deletions templates/repo/diff/comment_form.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,12 @@
"DisableAutosize" "true"
)}}

{{if $.root.IsAttachmentEnabled}}
<div class="field">
{{template "repo/upload" $.root}}
</div>
{{end}}

<div class="field footer gt-mx-3">
<span class="markup-info">{{svg "octicon-markup"}} {{ctx.Locale.Tr "repo.diff.comment.markdown_info"}}</span>
<div class="gt-text-right">
Expand Down
5 changes: 4 additions & 1 deletion templates/repo/diff/comments.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,10 @@
{{end}}
</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>
{{$reactions := .Reactions.GroupByType}}
{{if $reactions}}
Expand Down
3 changes: 3 additions & 0 deletions templates/repo/issue/view_content/conversation.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,9 @@
</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>
{{if .Attachments}}
{{template "repo/issue/view_content/attachments" dict "ctxData" $ "Attachments" .Attachments "Content" .RenderedContent}}
{{end}}
</div>
{{$reactions := .Reactions.GroupByType}}
{{if $reactions}}
Expand Down
111 changes: 57 additions & 54 deletions web_src/js/features/common-global.js
Original file line number Diff line number Diff line change
Expand Up @@ -200,63 +200,66 @@ export function initGlobalCommon() {
}

export function initGlobalDropzone() {
// Dropzone
for (const el of document.querySelectorAll('.dropzone')) {
const $dropzone = $(el);
const _promise = createDropzone(el, {
url: $dropzone.data('upload-url'),
headers: {'X-Csrf-Token': csrfToken},
maxFiles: $dropzone.data('max-file'),
maxFilesize: $dropzone.data('max-size'),
acceptedFiles: (['*/*', ''].includes($dropzone.data('accepts'))) ? null : $dropzone.data('accepts'),
addRemoveLinks: true,
dictDefaultMessage: $dropzone.data('default-message'),
dictInvalidFileType: $dropzone.data('invalid-input-type'),
dictFileTooBig: $dropzone.data('file-too-big'),
dictRemoveFile: $dropzone.data('remove-file'),
timeout: 0,
thumbnailMethod: 'contain',
thumbnailWidth: 480,
thumbnailHeight: 480,
init() {
this.on('success', (file, data) => {
file.uuid = data.uuid;
const input = $(`<input id="${data.uuid}" name="files" type="hidden">`).val(data.uuid);
$dropzone.find('.files').append(input);
// Create a "Copy Link" element, to conveniently copy the image
// or file link as Markdown to the clipboard
const copyLinkElement = document.createElement('div');
copyLinkElement.className = 'gt-text-center';
// The a element has a hardcoded cursor: pointer because the default is overridden by .dropzone
copyLinkElement.innerHTML = `<a href="#" style="cursor: pointer;">${svg('octicon-copy', 14, 'copy link')} Copy link</a>`;
copyLinkElement.addEventListener('click', async (e) => {
e.preventDefault();
let fileMarkdown = `[${file.name}](/attachments/${file.uuid})`;
if (file.type.startsWith('image/')) {
fileMarkdown = `!${fileMarkdown}`;
} else if (file.type.startsWith('video/')) {
fileMarkdown = `<video src="/attachments/${file.uuid}" title="${htmlEscape(file.name)}" controls></video>`;
}
const success = await clippie(fileMarkdown);
showTemporaryTooltip(e.target, success ? i18n.copy_success : i18n.copy_error);
});
file.previewTemplate.append(copyLinkElement);
});
this.on('removedfile', (file) => {
$(`#${file.uuid}`).remove();
if ($dropzone.data('remove-url')) {
POST($dropzone.data('remove-url'), {
data: new URLSearchParams({file: file.uuid}),
});
initDropzone(el);
}
}

export function initDropzone(el) {
const $dropzone = $(el);
const _promise = createDropzone(el, {
url: $dropzone.data('upload-url'),
headers: {'X-Csrf-Token': csrfToken},
maxFiles: $dropzone.data('max-file'),
maxFilesize: $dropzone.data('max-size'),
acceptedFiles: (['*/*', ''].includes($dropzone.data('accepts'))) ? null : $dropzone.data('accepts'),
addRemoveLinks: true,
dictDefaultMessage: $dropzone.data('default-message'),
dictInvalidFileType: $dropzone.data('invalid-input-type'),
dictFileTooBig: $dropzone.data('file-too-big'),
dictRemoveFile: $dropzone.data('remove-file'),
timeout: 0,
thumbnailMethod: 'contain',
thumbnailWidth: 480,
thumbnailHeight: 480,
init() {
this.on('success', (file, data) => {
file.uuid = data.uuid;
const input = $(`<input id="${data.uuid}" name="files" type="hidden">`).val(data.uuid);
$dropzone.find('.files').append(input);
// Create a "Copy Link" element, to conveniently copy the image
// or file link as Markdown to the clipboard
const copyLinkElement = document.createElement('div');
copyLinkElement.className = 'gt-text-center';
// The a element has a hardcoded cursor: pointer because the default is overridden by .dropzone
copyLinkElement.innerHTML = `<a href="#" style="cursor: pointer;">${svg('octicon-copy', 14, 'copy link')} Copy link</a>`;
copyLinkElement.addEventListener('click', async (e) => {
e.preventDefault();
let fileMarkdown = `[${file.name}](/attachments/${file.uuid})`;
if (file.type.startsWith('image/')) {
fileMarkdown = `!${fileMarkdown}`;
} else if (file.type.startsWith('video/')) {
fileMarkdown = `<video src="/attachments/${file.uuid}" title="${htmlEscape(file.name)}" controls></video>`;
}
const success = await clippie(fileMarkdown);
showTemporaryTooltip(e.target, success ? i18n.copy_success : i18n.copy_error);
});
this.on('error', function (file, message) {
showErrorToast(message);
this.removeFile(file);
});
},
});
}
file.previewTemplate.append(copyLinkElement);
});
this.on('removedfile', (file) => {
$(`#${file.uuid}`).remove();
if ($dropzone.data('remove-url')) {
POST($dropzone.data('remove-url'), {
data: new URLSearchParams({file: file.uuid}),
});
}
});
this.on('error', function (file, message) {
showErrorToast(message);
this.removeFile(file);
});
},
});
}

async function linkAction(e) {
Expand Down
Loading
Loading