From de2268ffab922de67f51e98541d0f9078795ac5d Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Thu, 20 Apr 2023 14:39:44 +0800 Subject: [PATCH] Fix issue attachment handling (#24202) Close #24195 Some of the changes are taken from my another fix https://github.com/go-gitea/gitea/pull/20147/commits/f07b0de997125c9b79cc5af27966a7cdd1803a4d 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 Co-authored-by: Giteabot --- models/issues/comment.go | 147 +++++++++--------- models/issues/comment_test.go | 5 +- models/issues/issue.go | 2 +- routers/api/v1/repo/issue_comment.go | 4 +- routers/web/repo/issue.go | 28 ++-- services/issue/comments.go | 3 +- templates/repo/diff/comments.tmpl | 2 +- templates/repo/issue/view_content.tmpl | 20 +-- .../repo/issue/view_content/attachments.tmpl | 20 +-- .../repo/issue/view_content/comments.tmpl | 6 +- web_src/css/features/dropzone.css | 10 -- web_src/js/features/repo-legacy.js | 54 ++++--- 12 files changed, 148 insertions(+), 153 deletions(-) diff --git a/models/issues/comment.go b/models/issues/comment.go index a47dc7c151..c7e815c8b6 100644 --- a/models/issues/comment.go +++ b/models/issues/comment.go @@ -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 + 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 - // 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 ) var commentStrings = []string{ @@ -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 @@ -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 { diff --git a/models/issues/comment_test.go b/models/issues/comment_test.go index f1232729f1..43bad1660f 100644 --- a/models/issues/comment_test.go +++ b/models/issues/comment_test.go @@ -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")) } diff --git a/models/issues/issue.go b/models/issues/issue.go index 5836030476..4f8e9e161d 100644 --- a/models/issues/issue.go +++ b/models/issues/issue.go @@ -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 diff --git a/routers/api/v1/repo/issue_comment.go b/routers/api/v1/repo/issue_comment.go index 3d14343d47..6ae6063303 100644 --- a/routers/api/v1/repo/issue_comment.go +++ b/routers/api/v1/repo/issue_comment.go @@ -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) @@ -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 } diff --git a/routers/web/repo/issue.go b/routers/web/repo/issue.go index fb61ec00d1..a9c67a9e34 100644 --- a/routers/web/repo/issue.go +++ b/routers/web/repo/issue.go @@ -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(), @@ -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 } @@ -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 } @@ -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 } @@ -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) } diff --git a/services/issue/comments.go b/services/issue/comments.go index 4fe07c17b9..4a181499bc 100644 --- a/services/issue/comments.go +++ b/services/issue/comments.go @@ -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 { diff --git a/templates/repo/diff/comments.tmpl b/templates/repo/diff/comments.tmpl index 5bcaeeb463..f301261533 100644 --- a/templates/repo/diff/comments.tmpl +++ b/templates/repo/diff/comments.tmpl @@ -55,7 +55,7 @@ {{end}}
{{.Content}}
-
+
{{$reactions := .Reactions.GroupByType}} {{if $reactions}} diff --git a/templates/repo/issue/view_content.tmpl b/templates/repo/issue/view_content.tmpl index a5d9b0bde0..081b7d08c6 100644 --- a/templates/repo/issue/view_content.tmpl +++ b/templates/repo/issue/view_content.tmpl @@ -78,7 +78,7 @@ {{end}}
{{.Issue.Content}}
-
+
{{if .Issue.Attachments}} {{template "repo/issue/view_content/attachments" dict "ctxData" $ "Attachments" .Issue.Attachments "Content" .Issue.RenderedContent}} {{end}} @@ -166,13 +166,15 @@