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

Fix issue attachment handling #24202

Merged
merged 7 commits into from
Apr 20, 2023
Merged
Show file tree
Hide file tree
Changes from 2 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
149 changes: 71 additions & 78 deletions models/issues/comment.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,84 +52,61 @@ func (err ErrCommentNotExist) Unwrap() error {
// CommentType defines whether a comment is just a simple comment, an action (like close) or a reference.
type CommentType int

// define unknown comment type
const (
CommentTypeUnknown CommentType = -1
)
// CommentTypeUndefined is used to search for comments of any type
const CommentTypeUndefined CommentType = -1

// Enumerate all the comment types
const (
// 0 Plain comment, can be associated with a commit (CommitID > 0) and a line (LineNum > 0)
CommentTypeComment CommentType = iota
CommentTypeReopen // 1
CommentTypeClose // 2

// 3 References.
CommentTypeIssueRef
// 4 Reference from a commit (not part of a pull request)
CommentTypeCommitRef
// 5 Reference from a comment
CommentTypeCommentRef
// 6 Reference from a pull request
CommentTypePullRef
// 7 Labels changed
CommentTypeLabel
// 8 Milestone changed
CommentTypeMilestone
// 9 Assignees changed
CommentTypeAssignees
// 10 Change Title
CommentTypeChangeTitle
// 11 Delete Branch
CommentTypeDeleteBranch
// 12 Start a stopwatch for time tracking
CommentTypeStartTracking
// 13 Stop a stopwatch for time tracking
CommentTypeStopTracking
// 14 Add time manual for time tracking
CommentTypeAddTimeManual
// 15 Cancel a stopwatch for time tracking
CommentTypeCancelTracking
// 16 Added a due date
CommentTypeAddedDeadline
// 17 Modified the due date
CommentTypeModifiedDeadline
// 18 Removed a due date
CommentTypeRemovedDeadline
// 19 Dependency added
CommentTypeAddDependency
// 20 Dependency removed
CommentTypeRemoveDependency
// 21 Comment a line of code
CommentTypeCode
// 22 Reviews a pull request by giving general feedback
CommentTypeReview
// 23 Lock an issue, giving only collaborators access
CommentTypeLock
// 24 Unlocks a previously locked issue
CommentTypeUnlock
// 25 Change pull request's target branch
CommentTypeChangeTargetBranch
// 26 Delete time manual for time tracking
CommentTypeDeleteTimeManual
// 27 add or remove Request from one
CommentTypeReviewRequest
// 28 merge pull request
CommentTypeMergePull
// 29 push to PR head branch
CommentTypePullRequestPush
// 30 Project changed
CommentTypeProject
// 31 Project board changed
CommentTypeProjectBoard
// 32 Dismiss Review
CommentTypeDismissReview
// 33 Change issue ref
CommentTypeChangeIssueRef
// 34 pr was scheduled to auto merge when checks succeed
CommentTypePRScheduledToAutoMerge
// 35 pr was un scheduled to auto merge when checks succeed
CommentTypePRUnScheduledToAutoMerge
CommentTypeComment CommentType = iota // 0 Plain comment, can be associated with a commit (CommitID > 0) and a line (LineNum > 0)

CommentTypeReopen // 1
CommentTypeClose // 2

CommentTypeIssueRef // 3 References.
CommentTypeCommitRef // 4 Reference from a commit (not part of a pull request)
CommentTypeCommentRef // 5 Reference from a comment
CommentTypePullRef // 6 Reference from a pull request

CommentTypeLabel // 7 Labels changed
CommentTypeMilestone // 8 Milestone changed
CommentTypeAssignees // 9 Assignees changed
CommentTypeChangeTitle // 10 Change Title
CommentTypeDeleteBranch // 11 Delete Branch

CommentTypeStartTracking // 12 Start a stopwatch for time tracking
CommentTypeStopTracking // 13 Stop a stopwatch for time tracking
CommentTypeAddTimeManual // 14 Add time manual for time tracking
CommentTypeCancelTracking // 15 Cancel a stopwatch for time tracking
CommentTypeAddedDeadline // 16 Added a due date
CommentTypeModifiedDeadline // 17 Modified the due date
CommentTypeRemovedDeadline // 18 Removed a due date

CommentTypeAddDependency // 19 Dependency added
CommentTypeRemoveDependency // 20 Dependency removed

CommentTypeCode // 21 Comment a line of code
CommentTypeReview // 22 Reviews a pull request by giving general feedback

CommentTypeLock // 23 Lock an issue, giving only collaborators access
CommentTypeUnlock // 24 Unlocks a previously locked issue

CommentTypeChangeTargetBranch // 25 Change pull request's target branch

CommentTypeDeleteTimeManual // 26 Delete time manual for time tracking

CommentTypeReviewRequest // 27 add or remove Request from one
CommentTypeMergePull // 28 merge pull request
CommentTypePullRequestPush // 29 push to PR head branch

CommentTypeProject // 30 Project changed
CommentTypeProjectBoard // 31 Project board changed

CommentTypeDismissReview // 32 Dismiss Review

CommentTypeChangeIssueRef // 33 Change issue ref

CommentTypePRScheduledToAutoMerge // 34 pr was scheduled to auto merge when checks succeed
CommentTypePRUnScheduledToAutoMerge // 35 pr was un scheduled to auto merge when checks succeed

)

var commentStrings = []string{
Expand Down Expand Up @@ -181,7 +158,23 @@ func AsCommentType(typeName string) CommentType {
return CommentType(index)
}
}
return CommentTypeUnknown
return CommentTypeUndefined
}

func (t CommentType) HasContentSupport() bool {
switch t {
case CommentTypeComment, CommentTypeCode, CommentTypeReview:
return true
}
return false
}

func (t CommentType) HasAttachmentSupport() bool {
switch t {
case CommentTypeComment, CommentTypeCode, CommentTypeReview:
return true
}
return false
}

// RoleDescriptor defines comment tag type
Expand Down Expand Up @@ -1039,7 +1032,7 @@ func (opts *FindCommentsOptions) ToConds() builder.Cond {
if opts.Before > 0 {
cond = cond.And(builder.Lte{"comment.updated_unix": opts.Before})
}
if opts.Type != CommentTypeUnknown {
if opts.Type != CommentTypeUndefined {
cond = cond.And(builder.Eq{"comment.type": opts.Type})
}
if opts.Line != 0 {
Expand Down
5 changes: 3 additions & 2 deletions models/issues/comment_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,8 +64,9 @@ func TestFetchCodeComments(t *testing.T) {
}

func TestAsCommentType(t *testing.T) {
assert.Equal(t, issues_model.CommentTypeUnknown, issues_model.AsCommentType(""))
assert.Equal(t, issues_model.CommentTypeUnknown, issues_model.AsCommentType("nonsense"))
assert.Equal(t, issues_model.CommentType(0), issues_model.CommentTypeComment)
assert.Equal(t, issues_model.CommentTypeUndefined, issues_model.AsCommentType(""))
assert.Equal(t, issues_model.CommentTypeUndefined, issues_model.AsCommentType("nonsense"))
assert.Equal(t, issues_model.CommentTypeComment, issues_model.AsCommentType("comment"))
assert.Equal(t, issues_model.CommentTypePRUnScheduledToAutoMerge, issues_model.AsCommentType("pull_cancel_scheduled_merge"))
}
2 changes: 1 addition & 1 deletion models/issues/issue.go
Original file line number Diff line number Diff line change
Expand Up @@ -269,7 +269,7 @@ func (issue *Issue) LoadPullRequest(ctx context.Context) (err error) {
}

func (issue *Issue) loadComments(ctx context.Context) (err error) {
return issue.loadCommentsByType(ctx, CommentTypeUnknown)
return issue.loadCommentsByType(ctx, CommentTypeUndefined)
}

// LoadDiscussComments loads discuss comments
Expand Down
4 changes: 2 additions & 2 deletions routers/api/v1/repo/issue_comment.go
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ func ListIssueCommentsAndTimeline(ctx *context.APIContext) {
IssueID: issue.ID,
Since: since,
Before: before,
Type: issues_model.CommentTypeUnknown,
Type: issues_model.CommentTypeUndefined,
}

comments, err := issues_model.FindComments(ctx, opts)
Expand Down Expand Up @@ -549,7 +549,7 @@ func editIssueComment(ctx *context.APIContext, form api.EditIssueCommentOption)
return
}

if comment.Type != issues_model.CommentTypeComment && comment.Type != issues_model.CommentTypeReview && comment.Type != issues_model.CommentTypeCode {
if !comment.Type.HasContentSupport() {
ctx.Status(http.StatusNoContent)
return
}
Expand Down
28 changes: 16 additions & 12 deletions routers/web/repo/issue.go
Original file line number Diff line number Diff line change
Expand Up @@ -1557,7 +1557,7 @@ func ViewIssue(ctx *context.Context) {
return
}
}
} else if comment.Type == issues_model.CommentTypeCode || comment.Type == issues_model.CommentTypeReview || comment.Type == issues_model.CommentTypeDismissReview {
} else if comment.Type.HasContentSupport() {
comment.RenderedContent, err = markdown.RenderString(&markup.RenderContext{
URLPrefix: ctx.Repo.RepoLink,
Metas: ctx.Repo.Repository.ComposeMetas(),
Expand Down Expand Up @@ -2849,7 +2849,7 @@ func UpdateCommentContent(ctx *context.Context) {
return
}

if comment.Type != issues_model.CommentTypeComment && comment.Type != issues_model.CommentTypeReview && comment.Type != issues_model.CommentTypeCode {
if !comment.Type.HasContentSupport() {
ctx.Error(http.StatusNoContent)
return
}
Expand Down Expand Up @@ -2913,7 +2913,7 @@ func DeleteComment(ctx *context.Context) {
if !ctx.IsSigned || (ctx.Doer.ID != comment.PosterID && !ctx.Repo.CanWriteIssuesOrPulls(comment.Issue.IsPull)) {
ctx.Error(http.StatusForbidden)
return
} else if comment.Type != issues_model.CommentTypeComment && comment.Type != issues_model.CommentTypeCode {
} else if !comment.Type.HasContentSupport() {
ctx.Error(http.StatusNoContent)
return
}
Expand Down Expand Up @@ -3059,7 +3059,7 @@ func ChangeCommentReaction(ctx *context.Context) {
return
}

if comment.Type != issues_model.CommentTypeComment && comment.Type != issues_model.CommentTypeCode && comment.Type != issues_model.CommentTypeReview {
if !comment.Type.HasContentSupport() {
ctx.Error(http.StatusNoContent)
return
}
Expand Down Expand Up @@ -3175,15 +3175,19 @@ func GetCommentAttachments(ctx *context.Context) {
ctx.NotFoundOrServerError("GetCommentByID", issues_model.IsErrCommentNotExist, err)
return
}

if !comment.Type.HasAttachmentSupport() {
ctx.ServerError("GetCommentAttachments", fmt.Errorf("comment type %v does not support attachments", comment.Type))
return
}

attachments := make([]*api.Attachment, 0)
if comment.Type == issues_model.CommentTypeComment {
if err := comment.LoadAttachments(ctx); err != nil {
ctx.ServerError("LoadAttachments", err)
return
}
for i := 0; i < len(comment.Attachments); i++ {
attachments = append(attachments, convert.ToAttachment(comment.Attachments[i]))
}
if err := comment.LoadAttachments(ctx); err != nil {
ctx.ServerError("LoadAttachments", err)
return
}
for i := 0; i < len(comment.Attachments); i++ {
attachments = append(attachments, convert.ToAttachment(comment.Attachments[i]))
}
ctx.JSON(http.StatusOK, attachments)
}
Expand Down
3 changes: 1 addition & 2 deletions services/issue/comments.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,8 +90,7 @@ func CreateIssueComment(ctx context.Context, doer *user_model.User, repo *repo_m

// UpdateComment updates information of comment.
func UpdateComment(ctx context.Context, c *issues_model.Comment, doer *user_model.User, oldContent string) error {
needsContentHistory := c.Content != oldContent &&
(c.Type == issues_model.CommentTypeComment || c.Type == issues_model.CommentTypeReview || c.Type == issues_model.CommentTypeCode)
needsContentHistory := c.Content != oldContent && c.Type.HasContentSupport()
if needsContentHistory {
hasContentHistory, err := issues_model.HasIssueContentHistory(ctx, c.IssueID, c.ID)
if err != nil {
Expand Down
2 changes: 1 addition & 1 deletion templates/repo/diff/comments.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@
{{end}}
</div>
<div id="issuecomment-{{.ID}}-raw" class="raw-content gt-hidden">{{.Content}}</div>
<div class="edit-content-zone gt-hidden" data-write="issuecomment-{{.ID}}-write" data-preview="issuecomment-{{.ID}}-preview" 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}}"></div>
</div>
{{$reactions := .Reactions.GroupByType}}
{{if $reactions}}
Expand Down
6 changes: 4 additions & 2 deletions templates/repo/issue/view_content.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@
{{end}}
</div>
<div id="issue-{{.Issue.ID}}-raw" class="raw-content gt-hidden">{{.Issue.Content}}</div>
<div class="edit-content-zone gt-hidden" data-write="issue-{{.Issue.ID}}-write" data-preview="issue-{{.Issue.ID}}-preview" data-update-url="{{$.RepoLink}}/issues/{{.Issue.Index}}/content" data-context="{{.RepoLink}}" data-attachment-url="{{$.RepoLink}}/issues/{{.Issue.Index}}/attachments" data-view-attachment-url="{{$.RepoLink}}/issues/{{.Issue.Index}}/view-attachments"></div>
<div class="edit-content-zone gt-hidden" data-update-url="{{$.RepoLink}}/issues/{{.Issue.Index}}/content" data-context="{{.RepoLink}}" data-attachment-url="{{$.RepoLink}}/issues/{{.Issue.Index}}/attachments" data-view-attachment-url="{{$.RepoLink}}/issues/{{.Issue.Index}}/view-attachments"></div>
{{if .Issue.Attachments}}
{{template "repo/issue/view_content/attachments" dict "ctxData" $ "Attachments" .Issue.Attachments "Content" .Issue.RenderedContent}}
{{end}}
Expand Down Expand Up @@ -166,21 +166,23 @@

<template id="issue-comment-editor-template">
<div class="ui comment form">
<div class="field">
{{template "shared/combomarkdowneditor" (dict
"locale" $.locale
"MarkdownPreviewUrl" (print .Repository.Link "/markup")
"MarkdownPreviewContext" .RepoLink
"TextareaName" "content"
"DropzoneParentContainer" ".ui.form"
)}}
</div>
wxiaoguang marked this conversation as resolved.
Show resolved Hide resolved

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

<div class="field footer">
<div class="field">
<div class="text right edit">
<button class="ui basic secondary cancel button" tabindex="3">{{.locale.Tr "repo.issues.cancel"}}</button>
<button class="ui primary save button" tabindex="2">{{.locale.Tr "repo.issues.save"}}</button>
Expand Down
18 changes: 9 additions & 9 deletions templates/repo/issue/view_content/attachments.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,10 @@
{{if .Attachments}}
<div class="ui clearing divider"></div>
{{end}}
<div class="ui middle aligned padded grid">
{{$hasThumbnails := false}}
{{- range .Attachments -}}
<div class="twelve wide column" style="padding: 6px;">
{{$hasThumbnails := false}}
{{- range .Attachments -}}
<div class="gt-df">
<div class="gt-f1 gt-p-3">
<a target="_blank" rel="noopener noreferrer" href="{{.DownloadURL}}" title='{{$.ctxData.locale.Tr "repo.issues.attachment.open_tab" .Name}}'>
{{if FilenameIsImage .Name}}
{{if not (containGeneric $.Content .UUID)}}
Expand All @@ -18,20 +18,20 @@
<span><strong>{{.Name}}</strong></span>
</a>
</div>
<div class="four wide column" style="padding: 0px;">
<div class="gt-p-3 gt-df gt-ac">
<span class="ui text grey right">{{.Size | FileSize}}</span>
</div>
{{end -}}
</div>
</div>
{{end -}}

{{if $hasThumbnails}}
<div class="ui clearing divider"></div>
<div class="ui divider"></div>
<div class="ui small thumbnails">
{{- range .Attachments -}}
{{if FilenameIsImage .Name}}
{{if not (containGeneric $.Content .UUID)}}
<a target="_blank" rel="noopener noreferrer" href="{{.DownloadURL}}">
<img src="{{.DownloadURL}}" title='{{$.ctxData.locale.Tr "repo.issues.attachment.open_tab" .Name}}'>
<img alt="{{.Name}}" src="{{.DownloadURL}}" title='{{$.ctxData.locale.Tr "repo.issues.attachment.open_tab" .Name}}'>
</a>
{{end}}
{{end}}
Expand Down
6 changes: 3 additions & 3 deletions templates/repo/issue/view_content/comments.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@
{{end}}
</div>
<div id="issuecomment-{{.ID}}-raw" class="raw-content gt-hidden">{{.Content}}</div>
<div class="edit-content-zone gt-hidden" data-write="issuecomment-{{.ID}}-write" data-preview="issuecomment-{{.ID}}-preview" 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}}
Expand Down Expand Up @@ -450,7 +450,7 @@
{{end}}
</div>
<div id="issuecomment-{{.ID}}-raw" class="raw-content gt-hidden">{{.Content}}</div>
<div class="edit-content-zone gt-hidden" data-write="issuecomment-{{.ID}}-write" data-preview="issuecomment-{{.ID}}-preview" 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}}
Expand Down Expand Up @@ -577,7 +577,7 @@
{{end}}
</div>
<div id="issuecomment-{{.ID}}-raw" class="raw-content gt-hidden">{{.Content}}</div>
<div class="edit-content-zone gt-hidden" data-write="issuecomment-{{.ID}}-write" data-preview="issuecomment-{{.ID}}-preview" 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>
</div>
{{$reactions := .Reactions.GroupByType}}
{{if $reactions}}
Expand Down
Loading