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

Refactor commentTags functionality #17558

Merged
merged 12 commits into from
Nov 11, 2021
2 changes: 1 addition & 1 deletion models/issue.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ type Issue struct {
IsLocked bool `xorm:"NOT NULL DEFAULT false"`

// For view issue page.
ShowTag CommentTag `xorm:"-"`
ShowRole RoleDescriptor `xorm:"-"`
}

var (
Expand Down
42 changes: 34 additions & 8 deletions models/issue_comment.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"`
Expand Down Expand Up @@ -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"`
Expand Down
78 changes: 43 additions & 35 deletions routers/web/repo/issue.go
Original file line number Diff line number Diff line change
Expand Up @@ -1014,38 +1014,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
}
if perm.IsOwner() {
if !poster.IsAdmin {
return models.CommentTagOwner, nil
}

ok, err := models.IsUserRealRepoAdmin(repo, poster)
if err != nil {
return models.CommentTagNone, err
}
// By default the poster has no roles on the comment.
roleDescriptor := models.RoleDescriptorNone

if ok {
return models.CommentTagOwner, nil
}
// 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 {
roleDescriptor = roleDescriptor.WithRole(models.RoleDescriptorOwner)
} else {

if ok, err = repo.IsCollaborator(poster.ID); ok && err == nil {
return models.CommentTagWriter, nil
// 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)
}
}
}

return models.CommentTagNone, err
// 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)
}

if perm.CanWriteIssuesOrPulls(issue.IsPull) {
return models.CommentTagWriter, 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 models.CommentTagNone, nil
return roleDescriptor, nil
}

func getBranchData(ctx *context.Context, issue *models.Issue) {
Expand Down Expand Up @@ -1248,9 +1256,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)
)
Expand Down Expand Up @@ -1297,11 +1305,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
Expand Down Expand Up @@ -1330,18 +1338,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 {
Expand Down Expand Up @@ -1429,18 +1437,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)
}
}
Expand Down
15 changes: 9 additions & 6 deletions templates/repo/issue/view_content.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -49,14 +49,17 @@
</div>
<div class="comment-header-right actions df ac">
{{if not $.Repository.IsArchived}}
{{if gt .Issue.ShowTag 0}}
<div class="ui basic label">
{{if eq .Issue.ShowTag 2}}
{{if gt .Issue.ShowRole 0}}
{{if (.Issue.ShowRole.HasRole "Writer")}}
<div class="ui basic label">
{{$.i18n.Tr "repo.issues.collaborator"}}
{{else if eq .Issue.ShowTag 3}}
</div>
{{end}}
{{if (.Issue.ShowRole.HasRole "Owner")}}
<div class="ui basic label">
{{$.i18n.Tr "repo.issues.owner"}}
{{end}}
</div>
</div>
{{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}}
Expand Down
48 changes: 24 additions & 24 deletions templates/repo/issue/view_content/comments.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -44,18 +44,19 @@
</div>
<div class="comment-header-right actions df ac">
{{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")}}
<div class="ui basic label">
{{$.i18n.Tr "repo.issues.poster"}}
</div>
{{end}}
{{if gt .ShowTag 0}}
{{if (.ShowRole.HasRole "Writer")}}
<div class="ui basic label">
{{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"}}
</div>
{{end}}
{{if (.ShowRole.HasRole "Owner")}}
<div class="ui basic label">
{{$.i18n.Tr "repo.issues.owner"}}
</div>
{{end}}
{{template "repo/issue/view_content/add_reaction" Dict "ctx" $ "ActionURL" (Printf "%s/comments/%d/reactions" $.RepoLink .ID)}}
Expand Down Expand Up @@ -549,24 +550,23 @@
</span>
</div>
<div class="comment-header-right actions df ac">
{{if not $.Repository.IsArchived}}
{{if or (and (eq .PosterID $.Issue.PosterID) (eq $.Issue.OriginalAuthorID 0)) (eq $.Issue.OriginalAuthorID .OriginalAuthorID) }}
<div class="ui basic label">
{{$.i18n.Tr "repo.issues.poster"}}
</div>
{{end}}
{{if gt .ShowTag 0}}
<div class="ui basic label">
{{if eq .ShowTag 2}}
{{$.i18n.Tr "repo.issues.collaborator"}}
{{else if eq .ShowTag 3}}
{{$.i18n.Tr "repo.issues.owner"}}
{{end}}
</div>
{{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")}}
<div class="ui basic label">
{{$.i18n.Tr "repo.issues.poster"}}
</div>
{{end}}
{{if (.ShowRole.HasRole "Writer")}}
<div class="ui basic label">
{{$.i18n.Tr "repo.issues.collaborator"}}
</div>
{{end}}
{{if (.ShowRole.HasRole "Owner")}}
<div class="ui basic label">
{{$.i18n.Tr "repo.issues.owner"}}
</div>
{{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))}}
</div>
</div>
<div class="text comment-content">
Expand Down