Skip to content

Commit de2268f

Browse files
wxiaoguangsilverwindGiteaBot
authored
Fix issue attachment handling (#24202)
Close #24195 Some of the changes are taken from my another fix f07b0de in #20147 (although that PR was discarded ....) The bug is: 1. The old code doesn't handle `removedfile` event correctly 2. The old code doesn't provide attachments for type=CommentTypeReview This PR doesn't intend to refactor the "upload" code to a perfect state (to avoid making the review difficult), so some legacy styles are kept. --------- Co-authored-by: silverwind <me@silverwind.io> Co-authored-by: Giteabot <teabot@gitea.io>
1 parent 92e07f2 commit de2268f

File tree

12 files changed

+149
-154
lines changed

12 files changed

+149
-154
lines changed

Diff for: ‎models/issues/comment.go

+71-78
Original file line numberDiff line numberDiff line change
@@ -52,84 +52,61 @@ func (err ErrCommentNotExist) Unwrap() error {
5252
// CommentType defines whether a comment is just a simple comment, an action (like close) or a reference.
5353
type CommentType int
5454

55-
// define unknown comment type
56-
const (
57-
CommentTypeUnknown CommentType = -1
58-
)
55+
// CommentTypeUndefined is used to search for comments of any type
56+
const CommentTypeUndefined CommentType = -1
5957

60-
// Enumerate all the comment types
6158
const (
62-
// 0 Plain comment, can be associated with a commit (CommitID > 0) and a line (LineNum > 0)
63-
CommentTypeComment CommentType = iota
64-
CommentTypeReopen // 1
65-
CommentTypeClose // 2
66-
67-
// 3 References.
68-
CommentTypeIssueRef
69-
// 4 Reference from a commit (not part of a pull request)
70-
CommentTypeCommitRef
71-
// 5 Reference from a comment
72-
CommentTypeCommentRef
73-
// 6 Reference from a pull request
74-
CommentTypePullRef
75-
// 7 Labels changed
76-
CommentTypeLabel
77-
// 8 Milestone changed
78-
CommentTypeMilestone
79-
// 9 Assignees changed
80-
CommentTypeAssignees
81-
// 10 Change Title
82-
CommentTypeChangeTitle
83-
// 11 Delete Branch
84-
CommentTypeDeleteBranch
85-
// 12 Start a stopwatch for time tracking
86-
CommentTypeStartTracking
87-
// 13 Stop a stopwatch for time tracking
88-
CommentTypeStopTracking
89-
// 14 Add time manual for time tracking
90-
CommentTypeAddTimeManual
91-
// 15 Cancel a stopwatch for time tracking
92-
CommentTypeCancelTracking
93-
// 16 Added a due date
94-
CommentTypeAddedDeadline
95-
// 17 Modified the due date
96-
CommentTypeModifiedDeadline
97-
// 18 Removed a due date
98-
CommentTypeRemovedDeadline
99-
// 19 Dependency added
100-
CommentTypeAddDependency
101-
// 20 Dependency removed
102-
CommentTypeRemoveDependency
103-
// 21 Comment a line of code
104-
CommentTypeCode
105-
// 22 Reviews a pull request by giving general feedback
106-
CommentTypeReview
107-
// 23 Lock an issue, giving only collaborators access
108-
CommentTypeLock
109-
// 24 Unlocks a previously locked issue
110-
CommentTypeUnlock
111-
// 25 Change pull request's target branch
112-
CommentTypeChangeTargetBranch
113-
// 26 Delete time manual for time tracking
114-
CommentTypeDeleteTimeManual
115-
// 27 add or remove Request from one
116-
CommentTypeReviewRequest
117-
// 28 merge pull request
118-
CommentTypeMergePull
119-
// 29 push to PR head branch
120-
CommentTypePullRequestPush
121-
// 30 Project changed
122-
CommentTypeProject
123-
// 31 Project board changed
124-
CommentTypeProjectBoard
125-
// 32 Dismiss Review
126-
CommentTypeDismissReview
127-
// 33 Change issue ref
128-
CommentTypeChangeIssueRef
129-
// 34 pr was scheduled to auto merge when checks succeed
130-
CommentTypePRScheduledToAutoMerge
131-
// 35 pr was un scheduled to auto merge when checks succeed
132-
CommentTypePRUnScheduledToAutoMerge
59+
CommentTypeComment CommentType = iota // 0 Plain comment, can be associated with a commit (CommitID > 0) and a line (LineNum > 0)
60+
61+
CommentTypeReopen // 1
62+
CommentTypeClose // 2
63+
64+
CommentTypeIssueRef // 3 References.
65+
CommentTypeCommitRef // 4 Reference from a commit (not part of a pull request)
66+
CommentTypeCommentRef // 5 Reference from a comment
67+
CommentTypePullRef // 6 Reference from a pull request
68+
69+
CommentTypeLabel // 7 Labels changed
70+
CommentTypeMilestone // 8 Milestone changed
71+
CommentTypeAssignees // 9 Assignees changed
72+
CommentTypeChangeTitle // 10 Change Title
73+
CommentTypeDeleteBranch // 11 Delete Branch
74+
75+
CommentTypeStartTracking // 12 Start a stopwatch for time tracking
76+
CommentTypeStopTracking // 13 Stop a stopwatch for time tracking
77+
CommentTypeAddTimeManual // 14 Add time manual for time tracking
78+
CommentTypeCancelTracking // 15 Cancel a stopwatch for time tracking
79+
CommentTypeAddedDeadline // 16 Added a due date
80+
CommentTypeModifiedDeadline // 17 Modified the due date
81+
CommentTypeRemovedDeadline // 18 Removed a due date
82+
83+
CommentTypeAddDependency // 19 Dependency added
84+
CommentTypeRemoveDependency // 20 Dependency removed
85+
86+
CommentTypeCode // 21 Comment a line of code
87+
CommentTypeReview // 22 Reviews a pull request by giving general feedback
88+
89+
CommentTypeLock // 23 Lock an issue, giving only collaborators access
90+
CommentTypeUnlock // 24 Unlocks a previously locked issue
91+
92+
CommentTypeChangeTargetBranch // 25 Change pull request's target branch
93+
94+
CommentTypeDeleteTimeManual // 26 Delete time manual for time tracking
95+
96+
CommentTypeReviewRequest // 27 add or remove Request from one
97+
CommentTypeMergePull // 28 merge pull request
98+
CommentTypePullRequestPush // 29 push to PR head branch
99+
100+
CommentTypeProject // 30 Project changed
101+
CommentTypeProjectBoard // 31 Project board changed
102+
103+
CommentTypeDismissReview // 32 Dismiss Review
104+
105+
CommentTypeChangeIssueRef // 33 Change issue ref
106+
107+
CommentTypePRScheduledToAutoMerge // 34 pr was scheduled to auto merge when checks succeed
108+
CommentTypePRUnScheduledToAutoMerge // 35 pr was un scheduled to auto merge when checks succeed
109+
133110
)
134111

135112
var commentStrings = []string{
@@ -181,7 +158,23 @@ func AsCommentType(typeName string) CommentType {
181158
return CommentType(index)
182159
}
183160
}
184-
return CommentTypeUnknown
161+
return CommentTypeUndefined
162+
}
163+
164+
func (t CommentType) HasContentSupport() bool {
165+
switch t {
166+
case CommentTypeComment, CommentTypeCode, CommentTypeReview:
167+
return true
168+
}
169+
return false
170+
}
171+
172+
func (t CommentType) HasAttachmentSupport() bool {
173+
switch t {
174+
case CommentTypeComment, CommentTypeCode, CommentTypeReview:
175+
return true
176+
}
177+
return false
185178
}
186179

187180
// RoleDescriptor defines comment tag type
@@ -1039,7 +1032,7 @@ func (opts *FindCommentsOptions) ToConds() builder.Cond {
10391032
if opts.Before > 0 {
10401033
cond = cond.And(builder.Lte{"comment.updated_unix": opts.Before})
10411034
}
1042-
if opts.Type != CommentTypeUnknown {
1035+
if opts.Type != CommentTypeUndefined {
10431036
cond = cond.And(builder.Eq{"comment.type": opts.Type})
10441037
}
10451038
if opts.Line != 0 {

Diff for: ‎models/issues/comment_test.go

+3-2
Original file line numberDiff line numberDiff line change
@@ -64,8 +64,9 @@ func TestFetchCodeComments(t *testing.T) {
6464
}
6565

6666
func TestAsCommentType(t *testing.T) {
67-
assert.Equal(t, issues_model.CommentTypeUnknown, issues_model.AsCommentType(""))
68-
assert.Equal(t, issues_model.CommentTypeUnknown, issues_model.AsCommentType("nonsense"))
67+
assert.Equal(t, issues_model.CommentType(0), issues_model.CommentTypeComment)
68+
assert.Equal(t, issues_model.CommentTypeUndefined, issues_model.AsCommentType(""))
69+
assert.Equal(t, issues_model.CommentTypeUndefined, issues_model.AsCommentType("nonsense"))
6970
assert.Equal(t, issues_model.CommentTypeComment, issues_model.AsCommentType("comment"))
7071
assert.Equal(t, issues_model.CommentTypePRUnScheduledToAutoMerge, issues_model.AsCommentType("pull_cancel_scheduled_merge"))
7172
}

Diff for: ‎models/issues/issue.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -269,7 +269,7 @@ func (issue *Issue) LoadPullRequest(ctx context.Context) (err error) {
269269
}
270270

271271
func (issue *Issue) loadComments(ctx context.Context) (err error) {
272-
return issue.loadCommentsByType(ctx, CommentTypeUnknown)
272+
return issue.loadCommentsByType(ctx, CommentTypeUndefined)
273273
}
274274

275275
// LoadDiscussComments loads discuss comments

Diff for: ‎routers/api/v1/repo/issue_comment.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -173,7 +173,7 @@ func ListIssueCommentsAndTimeline(ctx *context.APIContext) {
173173
IssueID: issue.ID,
174174
Since: since,
175175
Before: before,
176-
Type: issues_model.CommentTypeUnknown,
176+
Type: issues_model.CommentTypeUndefined,
177177
}
178178

179179
comments, err := issues_model.FindComments(ctx, opts)
@@ -549,7 +549,7 @@ func editIssueComment(ctx *context.APIContext, form api.EditIssueCommentOption)
549549
return
550550
}
551551

552-
if comment.Type != issues_model.CommentTypeComment && comment.Type != issues_model.CommentTypeReview && comment.Type != issues_model.CommentTypeCode {
552+
if !comment.Type.HasContentSupport() {
553553
ctx.Status(http.StatusNoContent)
554554
return
555555
}

Diff for: ‎routers/web/repo/issue.go

+16-12
Original file line numberDiff line numberDiff line change
@@ -1557,7 +1557,7 @@ func ViewIssue(ctx *context.Context) {
15571557
return
15581558
}
15591559
}
1560-
} else if comment.Type == issues_model.CommentTypeCode || comment.Type == issues_model.CommentTypeReview || comment.Type == issues_model.CommentTypeDismissReview {
1560+
} else if comment.Type.HasContentSupport() {
15611561
comment.RenderedContent, err = markdown.RenderString(&markup.RenderContext{
15621562
URLPrefix: ctx.Repo.RepoLink,
15631563
Metas: ctx.Repo.Repository.ComposeMetas(),
@@ -2849,7 +2849,7 @@ func UpdateCommentContent(ctx *context.Context) {
28492849
return
28502850
}
28512851

2852-
if comment.Type != issues_model.CommentTypeComment && comment.Type != issues_model.CommentTypeReview && comment.Type != issues_model.CommentTypeCode {
2852+
if !comment.Type.HasContentSupport() {
28532853
ctx.Error(http.StatusNoContent)
28542854
return
28552855
}
@@ -2913,7 +2913,7 @@ func DeleteComment(ctx *context.Context) {
29132913
if !ctx.IsSigned || (ctx.Doer.ID != comment.PosterID && !ctx.Repo.CanWriteIssuesOrPulls(comment.Issue.IsPull)) {
29142914
ctx.Error(http.StatusForbidden)
29152915
return
2916-
} else if comment.Type != issues_model.CommentTypeComment && comment.Type != issues_model.CommentTypeCode {
2916+
} else if !comment.Type.HasContentSupport() {
29172917
ctx.Error(http.StatusNoContent)
29182918
return
29192919
}
@@ -3059,7 +3059,7 @@ func ChangeCommentReaction(ctx *context.Context) {
30593059
return
30603060
}
30613061

3062-
if comment.Type != issues_model.CommentTypeComment && comment.Type != issues_model.CommentTypeCode && comment.Type != issues_model.CommentTypeReview {
3062+
if !comment.Type.HasContentSupport() {
30633063
ctx.Error(http.StatusNoContent)
30643064
return
30653065
}
@@ -3175,15 +3175,19 @@ func GetCommentAttachments(ctx *context.Context) {
31753175
ctx.NotFoundOrServerError("GetCommentByID", issues_model.IsErrCommentNotExist, err)
31763176
return
31773177
}
3178+
3179+
if !comment.Type.HasAttachmentSupport() {
3180+
ctx.ServerError("GetCommentAttachments", fmt.Errorf("comment type %v does not support attachments", comment.Type))
3181+
return
3182+
}
3183+
31783184
attachments := make([]*api.Attachment, 0)
3179-
if comment.Type == issues_model.CommentTypeComment {
3180-
if err := comment.LoadAttachments(ctx); err != nil {
3181-
ctx.ServerError("LoadAttachments", err)
3182-
return
3183-
}
3184-
for i := 0; i < len(comment.Attachments); i++ {
3185-
attachments = append(attachments, convert.ToAttachment(comment.Attachments[i]))
3186-
}
3185+
if err := comment.LoadAttachments(ctx); err != nil {
3186+
ctx.ServerError("LoadAttachments", err)
3187+
return
3188+
}
3189+
for i := 0; i < len(comment.Attachments); i++ {
3190+
attachments = append(attachments, convert.ToAttachment(comment.Attachments[i]))
31873191
}
31883192
ctx.JSON(http.StatusOK, attachments)
31893193
}

Diff for: ‎services/issue/comments.go

+1-2
Original file line numberDiff line numberDiff line change
@@ -90,8 +90,7 @@ func CreateIssueComment(ctx context.Context, doer *user_model.User, repo *repo_m
9090

9191
// UpdateComment updates information of comment.
9292
func UpdateComment(ctx context.Context, c *issues_model.Comment, doer *user_model.User, oldContent string) error {
93-
needsContentHistory := c.Content != oldContent &&
94-
(c.Type == issues_model.CommentTypeComment || c.Type == issues_model.CommentTypeReview || c.Type == issues_model.CommentTypeCode)
93+
needsContentHistory := c.Content != oldContent && c.Type.HasContentSupport()
9594
if needsContentHistory {
9695
hasContentHistory, err := issues_model.HasIssueContentHistory(ctx, c.IssueID, c.ID)
9796
if err != nil {

Diff for: ‎templates/repo/diff/comments.tmpl

+1-1
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@
5555
{{end}}
5656
</div>
5757
<div id="issuecomment-{{.ID}}-raw" class="raw-content gt-hidden">{{.Content}}</div>
58-
<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>
58+
<div class="edit-content-zone gt-hidden" data-update-url="{{$.root.RepoLink}}/comments/{{.ID}}" data-context="{{$.root.RepoLink}}"></div>
5959
</div>
6060
{{$reactions := .Reactions.GroupByType}}
6161
{{if $reactions}}

Diff for: ‎templates/repo/issue/view_content.tmpl

+11-9
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@
7878
{{end}}
7979
</div>
8080
<div id="issue-{{.Issue.ID}}-raw" class="raw-content gt-hidden">{{.Issue.Content}}</div>
81-
<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>
81+
<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>
8282
{{if .Issue.Attachments}}
8383
{{template "repo/issue/view_content/attachments" dict "ctxData" $ "Attachments" .Issue.Attachments "Content" .Issue.RenderedContent}}
8484
{{end}}
@@ -166,21 +166,23 @@
166166

167167
<template id="issue-comment-editor-template">
168168
<div class="ui comment form">
169-
{{template "shared/combomarkdowneditor" (dict
170-
"locale" $.locale
171-
"MarkdownPreviewUrl" (print .Repository.Link "/markup")
172-
"MarkdownPreviewContext" .RepoLink
173-
"TextareaName" "content"
174-
"DropzoneParentContainer" ".ui.form"
175-
)}}
169+
<div class="field">
170+
{{template "shared/combomarkdowneditor" (dict
171+
"locale" $.locale
172+
"MarkdownPreviewUrl" (print .Repository.Link "/markup")
173+
"MarkdownPreviewContext" .RepoLink
174+
"TextareaName" "content"
175+
"DropzoneParentContainer" ".ui.form"
176+
)}}
177+
</div>
176178

177179
{{if .IsAttachmentEnabled}}
178180
<div class="field">
179181
{{template "repo/upload" .}}
180182
</div>
181183
{{end}}
182184

183-
<div class="field footer">
185+
<div class="field">
184186
<div class="text right edit">
185187
<button class="ui basic secondary cancel button" tabindex="3">{{.locale.Tr "repo.issues.cancel"}}</button>
186188
<button class="ui primary save button" tabindex="2">{{.locale.Tr "repo.issues.save"}}</button>

Diff for: ‎templates/repo/issue/view_content/attachments.tmpl

+10-10
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,11 @@
11
<div class="dropzone-attachments">
22
{{if .Attachments}}
3-
<div class="ui clearing divider"></div>
3+
<div class="ui divider"></div>
44
{{end}}
5-
<div class="ui middle aligned padded grid">
6-
{{$hasThumbnails := false}}
7-
{{- range .Attachments -}}
8-
<div class="twelve wide column" style="padding: 6px;">
5+
{{$hasThumbnails := false}}
6+
{{- range .Attachments -}}
7+
<div class="gt-df">
8+
<div class="gt-f1 gt-p-3">
99
<a target="_blank" rel="noopener noreferrer" href="{{.DownloadURL}}" title='{{$.ctxData.locale.Tr "repo.issues.attachment.open_tab" .Name}}'>
1010
{{if FilenameIsImage .Name}}
1111
{{if not (containGeneric $.Content .UUID)}}
@@ -18,20 +18,20 @@
1818
<span><strong>{{.Name}}</strong></span>
1919
</a>
2020
</div>
21-
<div class="four wide column" style="padding: 0px;">
21+
<div class="gt-p-3 gt-df gt-ac">
2222
<span class="ui text grey right">{{.Size | FileSize}}</span>
2323
</div>
24-
{{end -}}
25-
</div>
24+
</div>
25+
{{end -}}
2626

2727
{{if $hasThumbnails}}
28-
<div class="ui clearing divider"></div>
28+
<div class="ui divider"></div>
2929
<div class="ui small thumbnails">
3030
{{- range .Attachments -}}
3131
{{if FilenameIsImage .Name}}
3232
{{if not (containGeneric $.Content .UUID)}}
3333
<a target="_blank" rel="noopener noreferrer" href="{{.DownloadURL}}">
34-
<img src="{{.DownloadURL}}" title='{{$.ctxData.locale.Tr "repo.issues.attachment.open_tab" .Name}}'>
34+
<img alt="{{.Name}}" src="{{.DownloadURL}}" title='{{$.ctxData.locale.Tr "repo.issues.attachment.open_tab" .Name}}'>
3535
</a>
3636
{{end}}
3737
{{end}}

0 commit comments

Comments
 (0)