From 492e1c2fbd1b646f4428207942a9f89b56f7b6a9 Mon Sep 17 00:00:00 2001 From: Gusted Date: Thu, 11 Nov 2021 07:29:30 +0100 Subject: [PATCH] Refactor commentTags functionality (#17558) * feat: Allow multiple tags on comments - Allow for multiples tags(Currently Poster + {Owner, Writer}). - Utilize the Poster tag within the commentTag function and remove the checking from templates. - Use bitwise on CommentTags to enable specific tags. - Don't show poster tag(view_content.tmpl) on the initial issue comment. * Change parameters naming * Change function name * refactor variable wording * Merge 'master' branch into 'tags-comments' branch * Change naming * `tag` -> `role` Co-authored-by: wxiaoguang Co-authored-by: Lunny Xiao --- models/issue.go | 2 +- models/issue_comment.go | 42 ++++++++-- routers/web/repo/issue.go | 80 ++++++++++--------- templates/repo/issue/view_content.tmpl | 15 ++-- .../repo/issue/view_content/comments.tmpl | 48 +++++------ 5 files changed, 112 insertions(+), 75 deletions(-) diff --git a/models/issue.go b/models/issue.go index f36267c94..0d34bdcaf 100644 --- a/models/issue.go +++ b/models/issue.go @@ -71,7 +71,7 @@ type Issue struct { IsLocked bool `xorm:"NOT NULL DEFAULT false"` // For view issue page. - ShowTag CommentTag `xorm:"-"` + ShowRole RoleDescriptor `xorm:"-"` } var ( diff --git a/models/issue_comment.go b/models/issue_comment.go index ad10b53b3..a41f4cb29 100644 --- a/models/issue_comment.go +++ b/models/issue_comment.go @@ -105,17 +105,43 @@ const ( CommentTypeDismissReview ) -// CommentTag defines comment tag type -type CommentTag int +// RoleDescriptor defines comment tag type +type RoleDescriptor int -// Enumerate all the comment tag types +// Enumerate all the role tags. const ( - CommentTagNone CommentTag = iota - CommentTagPoster - CommentTagWriter - CommentTagOwner + RoleDescriptorNone RoleDescriptor = iota + RoleDescriptorPoster + RoleDescriptorWriter + RoleDescriptorOwner ) +// WithRole enable a specific tag on the RoleDescriptor. +func (rd RoleDescriptor) WithRole(role RoleDescriptor) RoleDescriptor { + rd |= (1 << role) + return rd +} + +func stringToRoleDescriptor(role string) RoleDescriptor { + switch role { + case "Poster": + return RoleDescriptorPoster + case "Writer": + return RoleDescriptorWriter + case "Owner": + return RoleDescriptorOwner + default: + return RoleDescriptorNone + } +} + +// HasRole returns if a certain role is enabled on the RoleDescriptor. +func (rd RoleDescriptor) HasRole(role string) bool { + roleDescriptor := stringToRoleDescriptor(role) + bitValue := rd & (1 << roleDescriptor) + return (bitValue > 0) +} + // Comment represents a comment in commit and issue page. type Comment struct { ID int64 `xorm:"pk autoincr"` @@ -174,7 +200,7 @@ type Comment struct { Reactions ReactionList `xorm:"-"` // For view issue page. - ShowTag CommentTag `xorm:"-"` + ShowRole RoleDescriptor `xorm:"-"` Review *Review `xorm:"-"` ReviewID int64 `xorm:"index"` diff --git a/routers/web/repo/issue.go b/routers/web/repo/issue.go index 93b6b7a30..d9e15a784 100644 --- a/routers/web/repo/issue.go +++ b/routers/web/repo/issue.go @@ -1015,38 +1015,46 @@ func NewIssuePost(ctx *context.Context) { } } -// commentTag returns the CommentTag for a comment in/with the given repo, poster and issue -func commentTag(repo *models.Repository, poster *models.User, issue *models.Issue) (models.CommentTag, error) { +// roleDescriptor returns the Role Decriptor for a comment in/with the given repo, poster and issue +func roleDescriptor(repo *models.Repository, poster *models.User, issue *models.Issue) (models.RoleDescriptor, error) { perm, err := models.GetUserRepoPermission(repo, poster) if err != nil { - return models.CommentTagNone, err + return models.RoleDescriptorNone, err } + + // By default the poster has no roles on the comment. + roleDescriptor := models.RoleDescriptorNone + + // Check if the poster is owner of the repo. if perm.IsOwner() { + // If the poster isn't a admin, enable the owner role. if !poster.IsAdmin { - return models.CommentTagOwner, nil - } + roleDescriptor = roleDescriptor.WithRole(models.RoleDescriptorOwner) + } else { - ok, err := models.IsUserRealRepoAdmin(repo, poster) - if err != nil { - return models.CommentTagNone, err + // Otherwise check if poster is the real repo admin. + ok, err := models.IsUserRealRepoAdmin(repo, poster) + if err != nil { + return models.RoleDescriptorNone, err + } + if ok { + roleDescriptor = roleDescriptor.WithRole(models.RoleDescriptorOwner) + } } - - if ok { - return models.CommentTagOwner, nil - } - - if ok, err = repo.IsCollaborator(poster.ID); ok && err == nil { - return models.CommentTagWriter, nil - } - - return models.CommentTagNone, err } - if perm.CanWriteIssuesOrPulls(issue.IsPull) { - return models.CommentTagWriter, nil + // Is the poster can write issues or pulls to the repo, enable the Writer role. + // Only enable this if the poster doesn't have the owner role already. + if !roleDescriptor.HasRole("Owner") && perm.CanWriteIssuesOrPulls(issue.IsPull) { + roleDescriptor = roleDescriptor.WithRole(models.RoleDescriptorWriter) } - return models.CommentTagNone, nil + // If the poster is the actual poster of the issue, enable Poster role. + if issue.IsPoster(poster.ID) { + roleDescriptor = roleDescriptor.WithRole(models.RoleDescriptorPoster) + } + + return roleDescriptor, nil } func getBranchData(ctx *context.Context, issue *models.Issue) { @@ -1249,9 +1257,9 @@ func ViewIssue(ctx *context.Context) { } var ( - tag models.CommentTag + role models.RoleDescriptor ok bool - marked = make(map[int64]models.CommentTag) + marked = make(map[int64]models.RoleDescriptor) comment *models.Comment participants = make([]*models.User, 1, 10) ) @@ -1298,11 +1306,11 @@ func ViewIssue(ctx *context.Context) { // check if dependencies can be created across repositories ctx.Data["AllowCrossRepositoryDependencies"] = setting.Service.AllowCrossRepositoryDependencies - if issue.ShowTag, err = commentTag(repo, issue.Poster, issue); err != nil { - ctx.ServerError("commentTag", err) + if issue.ShowRole, err = roleDescriptor(repo, issue.Poster, issue); err != nil { + ctx.ServerError("roleDescriptor", err) return } - marked[issue.PosterID] = issue.ShowTag + marked[issue.PosterID] = issue.ShowRole // Render comments and and fetch participants. participants[0] = issue.Poster @@ -1331,18 +1339,18 @@ func ViewIssue(ctx *context.Context) { return } // Check tag. - tag, ok = marked[comment.PosterID] + role, ok = marked[comment.PosterID] if ok { - comment.ShowTag = tag + comment.ShowRole = role continue } - comment.ShowTag, err = commentTag(repo, comment.Poster, issue) + comment.ShowRole, err = roleDescriptor(repo, comment.Poster, issue) if err != nil { - ctx.ServerError("commentTag", err) + ctx.ServerError("roleDescriptor", err) return } - marked[comment.PosterID] = comment.ShowTag + marked[comment.PosterID] = comment.ShowRole participants = addParticipant(comment.Poster, participants) } else if comment.Type == models.CommentTypeLabel { if err = comment.LoadLabel(); err != nil { @@ -1430,18 +1438,18 @@ func ViewIssue(ctx *context.Context) { for _, lineComments := range codeComments { for _, c := range lineComments { // Check tag. - tag, ok = marked[c.PosterID] + role, ok = marked[c.PosterID] if ok { - c.ShowTag = tag + c.ShowRole = role continue } - c.ShowTag, err = commentTag(repo, c.Poster, issue) + c.ShowRole, err = roleDescriptor(repo, c.Poster, issue) if err != nil { - ctx.ServerError("commentTag", err) + ctx.ServerError("roleDescriptor", err) return } - marked[c.PosterID] = c.ShowTag + marked[c.PosterID] = c.ShowRole participants = addParticipant(c.Poster, participants) } } diff --git a/templates/repo/issue/view_content.tmpl b/templates/repo/issue/view_content.tmpl index 9ad6bee65..29c862659 100644 --- a/templates/repo/issue/view_content.tmpl +++ b/templates/repo/issue/view_content.tmpl @@ -49,14 +49,17 @@
{{if not $.Repository.IsArchived}} - {{if gt .Issue.ShowTag 0}} -
- {{if eq .Issue.ShowTag 2}} + {{if gt .Issue.ShowRole 0}} + {{if (.Issue.ShowRole.HasRole "Writer")}} +
{{$.i18n.Tr "repo.issues.collaborator"}} - {{else if eq .Issue.ShowTag 3}} +
+ {{end}} + {{if (.Issue.ShowRole.HasRole "Owner")}} +
{{$.i18n.Tr "repo.issues.owner"}} - {{end}} -
+
+ {{end}} {{end}} {{template "repo/issue/view_content/add_reaction" Dict "ctx" $ "ActionURL" (Printf "%s/issues/%d/reactions" $.RepoLink .Issue.Index)}} {{template "repo/issue/view_content/context_menu" Dict "ctx" $ "item" .Issue "delete" false "issue" true "diff" false "IsCommentPoster" $.IsIssuePoster}} diff --git a/templates/repo/issue/view_content/comments.tmpl b/templates/repo/issue/view_content/comments.tmpl index 51d1e093c..57ec007be 100644 --- a/templates/repo/issue/view_content/comments.tmpl +++ b/templates/repo/issue/view_content/comments.tmpl @@ -44,18 +44,19 @@
{{if not $.Repository.IsArchived}} - {{if or (and (eq .PosterID .Issue.PosterID) (eq .Issue.OriginalAuthorID 0)) (and (eq .Issue.OriginalAuthorID .OriginalAuthorID) (not (eq .OriginalAuthorID 0))) }} + {{if (.ShowRole.HasRole "Poster")}}
{{$.i18n.Tr "repo.issues.poster"}}
{{end}} - {{if gt .ShowTag 0}} + {{if (.ShowRole.HasRole "Writer")}}
- {{if eq .ShowTag 2}} - {{$.i18n.Tr "repo.issues.collaborator"}} - {{else if eq .ShowTag 3}} - {{$.i18n.Tr "repo.issues.owner"}} - {{end}} + {{$.i18n.Tr "repo.issues.collaborator"}} +
+ {{end}} + {{if (.ShowRole.HasRole "Owner")}} +
+ {{$.i18n.Tr "repo.issues.owner"}}
{{end}} {{template "repo/issue/view_content/add_reaction" Dict "ctx" $ "ActionURL" (Printf "%s/comments/%d/reactions" $.RepoLink .ID)}} @@ -549,24 +550,23 @@
- {{if not $.Repository.IsArchived}} - {{if or (and (eq .PosterID $.Issue.PosterID) (eq $.Issue.OriginalAuthorID 0)) (eq $.Issue.OriginalAuthorID .OriginalAuthorID) }} -
- {{$.i18n.Tr "repo.issues.poster"}} -
- {{end}} - {{if gt .ShowTag 0}} -
- {{if eq .ShowTag 2}} - {{$.i18n.Tr "repo.issues.collaborator"}} - {{else if eq .ShowTag 3}} - {{$.i18n.Tr "repo.issues.owner"}} - {{end}} -
- {{end}} - {{template "repo/issue/view_content/add_reaction" Dict "ctx" $ "ActionURL" (Printf "%s/comments/%d/reactions" $.RepoLink .ID)}} - {{template "repo/issue/view_content/context_menu" Dict "ctx" $ "item" . "delete" true "issue" true "diff" true "IsCommentPoster" (and $.IsSigned (eq $.SignedUserID .PosterID))}} + {{if (.ShowRole.HasRole "Poster")}} +
+ {{$.i18n.Tr "repo.issues.poster"}} +
{{end}} + {{if (.ShowRole.HasRole "Writer")}} +
+ {{$.i18n.Tr "repo.issues.collaborator"}} +
+ {{end}} + {{if (.ShowRole.HasRole "Owner")}} +
+ {{$.i18n.Tr "repo.issues.owner"}} +
+ {{end}} + {{template "repo/issue/view_content/add_reaction" Dict "ctx" $ "ActionURL" (Printf "%s/comments/%d/reactions" $.RepoLink .ID)}} + {{template "repo/issue/view_content/context_menu" Dict "ctx" $ "item" . "delete" true "issue" true "diff" true "IsCommentPoster" (and $.IsSigned (eq $.SignedUserID .PosterID))}}