Skip to content

Commit 492e1c2

Browse files
Gustedwxiaoguanglunny
authored
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 <[email protected]> Co-authored-by: Lunny Xiao <[email protected]>
1 parent a4dc0c5 commit 492e1c2

File tree

5 files changed

+111
-74
lines changed

5 files changed

+111
-74
lines changed

‎models/issue.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ type Issue struct {
7171
IsLocked bool `xorm:"NOT NULL DEFAULT false"`
7272

7373
// For view issue page.
74-
ShowTag CommentTag `xorm:"-"`
74+
ShowRole RoleDescriptor `xorm:"-"`
7575
}
7676

7777
var (

‎models/issue_comment.go‎

Lines changed: 34 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -105,17 +105,43 @@ const (
105105
CommentTypeDismissReview
106106
)
107107

108-
// CommentTag defines comment tag type
109-
type CommentTag int
108+
// RoleDescriptor defines comment tag type
109+
type RoleDescriptor int
110110

111-
// Enumerate all the comment tag types
111+
// Enumerate all the role tags.
112112
const (
113-
CommentTagNone CommentTag = iota
114-
CommentTagPoster
115-
CommentTagWriter
116-
CommentTagOwner
113+
RoleDescriptorNone RoleDescriptor = iota
114+
RoleDescriptorPoster
115+
RoleDescriptorWriter
116+
RoleDescriptorOwner
117117
)
118118

119+
// WithRole enable a specific tag on the RoleDescriptor.
120+
func (rd RoleDescriptor) WithRole(role RoleDescriptor) RoleDescriptor {
121+
rd |= (1 << role)
122+
return rd
123+
}
124+
125+
func stringToRoleDescriptor(role string) RoleDescriptor {
126+
switch role {
127+
case "Poster":
128+
return RoleDescriptorPoster
129+
case "Writer":
130+
return RoleDescriptorWriter
131+
case "Owner":
132+
return RoleDescriptorOwner
133+
default:
134+
return RoleDescriptorNone
135+
}
136+
}
137+
138+
// HasRole returns if a certain role is enabled on the RoleDescriptor.
139+
func (rd RoleDescriptor) HasRole(role string) bool {
140+
roleDescriptor := stringToRoleDescriptor(role)
141+
bitValue := rd & (1 << roleDescriptor)
142+
return (bitValue > 0)
143+
}
144+
119145
// Comment represents a comment in commit and issue page.
120146
type Comment struct {
121147
ID int64 `xorm:"pk autoincr"`
@@ -174,7 +200,7 @@ type Comment struct {
174200
Reactions ReactionList `xorm:"-"`
175201

176202
// For view issue page.
177-
ShowTag CommentTag `xorm:"-"`
203+
ShowRole RoleDescriptor `xorm:"-"`
178204

179205
Review *Review `xorm:"-"`
180206
ReviewID int64 `xorm:"index"`

‎routers/web/repo/issue.go‎

Lines changed: 43 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -1015,38 +1015,46 @@ func NewIssuePost(ctx *context.Context) {
10151015
}
10161016
}
10171017

1018-
// commentTag returns the CommentTag for a comment in/with the given repo, poster and issue
1019-
func commentTag(repo *models.Repository, poster *models.User, issue *models.Issue) (models.CommentTag, error) {
1018+
// roleDescriptor returns the Role Decriptor for a comment in/with the given repo, poster and issue
1019+
func roleDescriptor(repo *models.Repository, poster *models.User, issue *models.Issue) (models.RoleDescriptor, error) {
10201020
perm, err := models.GetUserRepoPermission(repo, poster)
10211021
if err != nil {
1022-
return models.CommentTagNone, err
1022+
return models.RoleDescriptorNone, err
10231023
}
1024-
if perm.IsOwner() {
1025-
if !poster.IsAdmin {
1026-
return models.CommentTagOwner, nil
1027-
}
10281024

1029-
ok, err := models.IsUserRealRepoAdmin(repo, poster)
1030-
if err != nil {
1031-
return models.CommentTagNone, err
1032-
}
1025+
// By default the poster has no roles on the comment.
1026+
roleDescriptor := models.RoleDescriptorNone
10331027

1034-
if ok {
1035-
return models.CommentTagOwner, nil
1036-
}
1028+
// Check if the poster is owner of the repo.
1029+
if perm.IsOwner() {
1030+
// If the poster isn't a admin, enable the owner role.
1031+
if !poster.IsAdmin {
1032+
roleDescriptor = roleDescriptor.WithRole(models.RoleDescriptorOwner)
1033+
} else {
10371034

1038-
if ok, err = repo.IsCollaborator(poster.ID); ok && err == nil {
1039-
return models.CommentTagWriter, nil
1035+
// Otherwise check if poster is the real repo admin.
1036+
ok, err := models.IsUserRealRepoAdmin(repo, poster)
1037+
if err != nil {
1038+
return models.RoleDescriptorNone, err
1039+
}
1040+
if ok {
1041+
roleDescriptor = roleDescriptor.WithRole(models.RoleDescriptorOwner)
1042+
}
10401043
}
1044+
}
10411045

1042-
return models.CommentTagNone, err
1046+
// Is the poster can write issues or pulls to the repo, enable the Writer role.
1047+
// Only enable this if the poster doesn't have the owner role already.
1048+
if !roleDescriptor.HasRole("Owner") && perm.CanWriteIssuesOrPulls(issue.IsPull) {
1049+
roleDescriptor = roleDescriptor.WithRole(models.RoleDescriptorWriter)
10431050
}
10441051

1045-
if perm.CanWriteIssuesOrPulls(issue.IsPull) {
1046-
return models.CommentTagWriter, nil
1052+
// If the poster is the actual poster of the issue, enable Poster role.
1053+
if issue.IsPoster(poster.ID) {
1054+
roleDescriptor = roleDescriptor.WithRole(models.RoleDescriptorPoster)
10471055
}
10481056

1049-
return models.CommentTagNone, nil
1057+
return roleDescriptor, nil
10501058
}
10511059

10521060
func getBranchData(ctx *context.Context, issue *models.Issue) {
@@ -1249,9 +1257,9 @@ func ViewIssue(ctx *context.Context) {
12491257
}
12501258

12511259
var (
1252-
tag models.CommentTag
1260+
role models.RoleDescriptor
12531261
ok bool
1254-
marked = make(map[int64]models.CommentTag)
1262+
marked = make(map[int64]models.RoleDescriptor)
12551263
comment *models.Comment
12561264
participants = make([]*models.User, 1, 10)
12571265
)
@@ -1298,11 +1306,11 @@ func ViewIssue(ctx *context.Context) {
12981306
// check if dependencies can be created across repositories
12991307
ctx.Data["AllowCrossRepositoryDependencies"] = setting.Service.AllowCrossRepositoryDependencies
13001308

1301-
if issue.ShowTag, err = commentTag(repo, issue.Poster, issue); err != nil {
1302-
ctx.ServerError("commentTag", err)
1309+
if issue.ShowRole, err = roleDescriptor(repo, issue.Poster, issue); err != nil {
1310+
ctx.ServerError("roleDescriptor", err)
13031311
return
13041312
}
1305-
marked[issue.PosterID] = issue.ShowTag
1313+
marked[issue.PosterID] = issue.ShowRole
13061314

13071315
// Render comments and and fetch participants.
13081316
participants[0] = issue.Poster
@@ -1331,18 +1339,18 @@ func ViewIssue(ctx *context.Context) {
13311339
return
13321340
}
13331341
// Check tag.
1334-
tag, ok = marked[comment.PosterID]
1342+
role, ok = marked[comment.PosterID]
13351343
if ok {
1336-
comment.ShowTag = tag
1344+
comment.ShowRole = role
13371345
continue
13381346
}
13391347

1340-
comment.ShowTag, err = commentTag(repo, comment.Poster, issue)
1348+
comment.ShowRole, err = roleDescriptor(repo, comment.Poster, issue)
13411349
if err != nil {
1342-
ctx.ServerError("commentTag", err)
1350+
ctx.ServerError("roleDescriptor", err)
13431351
return
13441352
}
1345-
marked[comment.PosterID] = comment.ShowTag
1353+
marked[comment.PosterID] = comment.ShowRole
13461354
participants = addParticipant(comment.Poster, participants)
13471355
} else if comment.Type == models.CommentTypeLabel {
13481356
if err = comment.LoadLabel(); err != nil {
@@ -1430,18 +1438,18 @@ func ViewIssue(ctx *context.Context) {
14301438
for _, lineComments := range codeComments {
14311439
for _, c := range lineComments {
14321440
// Check tag.
1433-
tag, ok = marked[c.PosterID]
1441+
role, ok = marked[c.PosterID]
14341442
if ok {
1435-
c.ShowTag = tag
1443+
c.ShowRole = role
14361444
continue
14371445
}
14381446

1439-
c.ShowTag, err = commentTag(repo, c.Poster, issue)
1447+
c.ShowRole, err = roleDescriptor(repo, c.Poster, issue)
14401448
if err != nil {
1441-
ctx.ServerError("commentTag", err)
1449+
ctx.ServerError("roleDescriptor", err)
14421450
return
14431451
}
1444-
marked[c.PosterID] = c.ShowTag
1452+
marked[c.PosterID] = c.ShowRole
14451453
participants = addParticipant(c.Poster, participants)
14461454
}
14471455
}

‎templates/repo/issue/view_content.tmpl‎

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -49,14 +49,17 @@
4949
</div>
5050
<div class="comment-header-right actions df ac">
5151
{{if not $.Repository.IsArchived}}
52-
{{if gt .Issue.ShowTag 0}}
53-
<div class="ui basic label">
54-
{{if eq .Issue.ShowTag 2}}
52+
{{if gt .Issue.ShowRole 0}}
53+
{{if (.Issue.ShowRole.HasRole "Writer")}}
54+
<div class="ui basic label">
5555
{{$.i18n.Tr "repo.issues.collaborator"}}
56-
{{else if eq .Issue.ShowTag 3}}
56+
</div>
57+
{{end}}
58+
{{if (.Issue.ShowRole.HasRole "Owner")}}
59+
<div class="ui basic label">
5760
{{$.i18n.Tr "repo.issues.owner"}}
58-
{{end}}
59-
</div>
61+
</div>
62+
{{end}}
6063
{{end}}
6164
{{template "repo/issue/view_content/add_reaction" Dict "ctx" $ "ActionURL" (Printf "%s/issues/%d/reactions" $.RepoLink .Issue.Index)}}
6265
{{template "repo/issue/view_content/context_menu" Dict "ctx" $ "item" .Issue "delete" false "issue" true "diff" false "IsCommentPoster" $.IsIssuePoster}}

‎templates/repo/issue/view_content/comments.tmpl‎

Lines changed: 24 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -44,18 +44,19 @@
4444
</div>
4545
<div class="comment-header-right actions df ac">
4646
{{if not $.Repository.IsArchived}}
47-
{{if or (and (eq .PosterID .Issue.PosterID) (eq .Issue.OriginalAuthorID 0)) (and (eq .Issue.OriginalAuthorID .OriginalAuthorID) (not (eq .OriginalAuthorID 0))) }}
47+
{{if (.ShowRole.HasRole "Poster")}}
4848
<div class="ui basic label">
4949
{{$.i18n.Tr "repo.issues.poster"}}
5050
</div>
5151
{{end}}
52-
{{if gt .ShowTag 0}}
52+
{{if (.ShowRole.HasRole "Writer")}}
5353
<div class="ui basic label">
54-
{{if eq .ShowTag 2}}
55-
{{$.i18n.Tr "repo.issues.collaborator"}}
56-
{{else if eq .ShowTag 3}}
57-
{{$.i18n.Tr "repo.issues.owner"}}
58-
{{end}}
54+
{{$.i18n.Tr "repo.issues.collaborator"}}
55+
</div>
56+
{{end}}
57+
{{if (.ShowRole.HasRole "Owner")}}
58+
<div class="ui basic label">
59+
{{$.i18n.Tr "repo.issues.owner"}}
5960
</div>
6061
{{end}}
6162
{{template "repo/issue/view_content/add_reaction" Dict "ctx" $ "ActionURL" (Printf "%s/comments/%d/reactions" $.RepoLink .ID)}}
@@ -549,24 +550,23 @@
549550
</span>
550551
</div>
551552
<div class="comment-header-right actions df ac">
552-
{{if not $.Repository.IsArchived}}
553-
{{if or (and (eq .PosterID $.Issue.PosterID) (eq $.Issue.OriginalAuthorID 0)) (eq $.Issue.OriginalAuthorID .OriginalAuthorID) }}
554-
<div class="ui basic label">
555-
{{$.i18n.Tr "repo.issues.poster"}}
556-
</div>
557-
{{end}}
558-
{{if gt .ShowTag 0}}
559-
<div class="ui basic label">
560-
{{if eq .ShowTag 2}}
561-
{{$.i18n.Tr "repo.issues.collaborator"}}
562-
{{else if eq .ShowTag 3}}
563-
{{$.i18n.Tr "repo.issues.owner"}}
564-
{{end}}
565-
</div>
566-
{{end}}
567-
{{template "repo/issue/view_content/add_reaction" Dict "ctx" $ "ActionURL" (Printf "%s/comments/%d/reactions" $.RepoLink .ID)}}
568-
{{template "repo/issue/view_content/context_menu" Dict "ctx" $ "item" . "delete" true "issue" true "diff" true "IsCommentPoster" (and $.IsSigned (eq $.SignedUserID .PosterID))}}
553+
{{if (.ShowRole.HasRole "Poster")}}
554+
<div class="ui basic label">
555+
{{$.i18n.Tr "repo.issues.poster"}}
556+
</div>
557+
{{end}}
558+
{{if (.ShowRole.HasRole "Writer")}}
559+
<div class="ui basic label">
560+
{{$.i18n.Tr "repo.issues.collaborator"}}
561+
</div>
562+
{{end}}
563+
{{if (.ShowRole.HasRole "Owner")}}
564+
<div class="ui basic label">
565+
{{$.i18n.Tr "repo.issues.owner"}}
566+
</div>
569567
{{end}}
568+
{{template "repo/issue/view_content/add_reaction" Dict "ctx" $ "ActionURL" (Printf "%s/comments/%d/reactions" $.RepoLink .ID)}}
569+
{{template "repo/issue/view_content/context_menu" Dict "ctx" $ "item" . "delete" true "issue" true "diff" true "IsCommentPoster" (and $.IsSigned (eq $.SignedUserID .PosterID))}}
570570
</div>
571571
</div>
572572
<div class="text comment-content">

0 commit comments

Comments
 (0)