Skip to content

Commit 1b86f17

Browse files
a1012112796lafriks6543guillep2k
authored
Add a way to mark Conversation (code comment) resolved (#11037)
* Add a way to mark Conversation (code comment) resolved mark Conversation is a way to mark a Conversation is stale or be solved. when it's marked as stale, will be hided like stale. all Pull Request writer , Offical Reviewers and poster can add or remove Conversation resolved mark. Signed-off-by: a1012112796 <[email protected]> * fix lint * Apply suggestions from code review * Add ResolveDoer * fix ui Co-Authored-By: Lauris BH <[email protected]> Co-Authored-By: 6543 <[email protected]> * change IsResolved to an function Add permission check in UpdateResolveConversation * Apply suggestions from code review * change return error for permisson check * add default message for deleted user * get issue message from comment * add migration for ``ResolveDoerID`` column another change: * block mark pending review as resolved because it's not necessary Co-authored-by: guillep2k <[email protected]> * change button color * resolve button size * fix code style * remove unusefull code Co-authored-by: guillep2k <[email protected]> Co-authored-by: Lauris BH <[email protected]> Co-authored-by: 6543 <[email protected]> Co-authored-by: guillep2k <[email protected]>
1 parent 38d5f88 commit 1b86f17

File tree

13 files changed

+301
-9
lines changed

13 files changed

+301
-9
lines changed

‎models/issue_comment.go

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,8 @@ type Comment struct {
122122
AssigneeID int64
123123
RemovedAssignee bool
124124
Assignee *User `xorm:"-"`
125+
ResolveDoerID int64
126+
ResolveDoer *User `xorm:"-"`
125127
OldTitle string
126128
NewTitle string
127129
OldRef string
@@ -420,6 +422,26 @@ func (c *Comment) LoadAssigneeUser() error {
420422
return nil
421423
}
422424

425+
// LoadResolveDoer if comment.Type is CommentTypeCode and ResolveDoerID not zero, then load resolveDoer
426+
func (c *Comment) LoadResolveDoer() (err error) {
427+
if c.ResolveDoerID == 0 || c.Type != CommentTypeCode {
428+
return nil
429+
}
430+
c.ResolveDoer, err = getUserByID(x, c.ResolveDoerID)
431+
if err != nil {
432+
if IsErrUserNotExist(err) {
433+
c.ResolveDoer = NewGhostUser()
434+
err = nil
435+
}
436+
}
437+
return
438+
}
439+
440+
// IsResolved check if an code comment is resolved
441+
func (c *Comment) IsResolved() bool {
442+
return c.ResolveDoerID != 0 && c.Type == CommentTypeCode
443+
}
444+
423445
// LoadDepIssueDetails loads Dependent Issue Details
424446
func (c *Comment) LoadDepIssueDetails() (err error) {
425447
if c.DependentIssueID <= 0 || c.DependentIssue != nil {
@@ -943,7 +965,12 @@ func fetchCodeCommentsByReview(e Engine, issue *Issue, currentUser *User, review
943965
if err := e.In("id", ids).Find(&reviews); err != nil {
944966
return nil, err
945967
}
968+
946969
for _, comment := range comments {
970+
if err := comment.LoadResolveDoer(); err != nil {
971+
return nil, err
972+
}
973+
947974
if re, ok := reviews[comment.ReviewID]; ok && re != nil {
948975
// If the review is pending only the author can see the comments (except the review is set)
949976
if review.ID == 0 {

‎models/migrations/migrations.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -208,6 +208,8 @@ var migrations = []Migration{
208208
NewMigration("Add CommitsAhead and CommitsBehind Column to PullRequest Table", addCommitDivergenceToPulls),
209209
// v137 -> v138
210210
NewMigration("Add Branch Protection Block Outdated Branch", addBlockOnOutdatedBranch),
211+
// v138 -> v139
212+
NewMigration("Add ResolveDoerID to Comment table", addResolveDoerIDCommentColumn),
211213
}
212214

213215
// GetCurrentDBVersion returns the current db version

‎models/migrations/v138.go

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
// Copyright 2020 The Gitea Authors. All rights reserved.
2+
// Use of this source code is governed by a MIT-style
3+
// license that can be found in the LICENSE file.
4+
5+
package migrations
6+
7+
import (
8+
"fmt"
9+
10+
"xorm.io/xorm"
11+
)
12+
13+
func addResolveDoerIDCommentColumn(x *xorm.Engine) error {
14+
type Comment struct {
15+
ResolveDoerID int64
16+
}
17+
18+
if err := x.Sync2(new(Comment)); err != nil {
19+
return fmt.Errorf("Sync2: %v", err)
20+
}
21+
return nil
22+
}

‎models/review.go

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
package models
66

77
import (
8+
"fmt"
89
"strings"
910

1011
"code.gitea.io/gitea/modules/timeutil"
@@ -594,3 +595,62 @@ func RemoveRewiewRequest(issue *Issue, reviewer *User, doer *User) (comment *Com
594595

595596
return comment, sess.Commit()
596597
}
598+
599+
// MarkConversation Add or remove Conversation mark for a code comment
600+
func MarkConversation(comment *Comment, doer *User, isResolve bool) (err error) {
601+
if comment.Type != CommentTypeCode {
602+
return nil
603+
}
604+
605+
if isResolve {
606+
if comment.ResolveDoerID != 0 {
607+
return nil
608+
}
609+
610+
if _, err = x.Exec("UPDATE `comment` SET resolve_doer_id=? WHERE id=?", doer.ID, comment.ID); err != nil {
611+
return err
612+
}
613+
} else {
614+
if comment.ResolveDoerID == 0 {
615+
return nil
616+
}
617+
618+
if _, err = x.Exec("UPDATE `comment` SET resolve_doer_id=? WHERE id=?", 0, comment.ID); err != nil {
619+
return err
620+
}
621+
}
622+
623+
return nil
624+
}
625+
626+
// CanMarkConversation Add or remove Conversation mark for a code comment permission check
627+
// the PR writer , offfcial reviewer and poster can do it
628+
func CanMarkConversation(issue *Issue, doer *User) (permResult bool, err error) {
629+
if doer == nil || issue == nil {
630+
return false, fmt.Errorf("issue or doer is nil")
631+
}
632+
633+
if doer.ID != issue.PosterID {
634+
if err = issue.LoadRepo(); err != nil {
635+
return false, err
636+
}
637+
638+
perm, err := GetUserRepoPermission(issue.Repo, doer)
639+
if err != nil {
640+
return false, err
641+
}
642+
643+
permResult = perm.CanAccess(AccessModeWrite, UnitTypePullRequests)
644+
if !permResult {
645+
if permResult, err = IsOfficialReviewer(issue, doer); err != nil {
646+
return false, err
647+
}
648+
}
649+
650+
if !permResult {
651+
return false, nil
652+
}
653+
}
654+
655+
return true, nil
656+
}

‎options/locale/locale_en-US.ini

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1067,6 +1067,11 @@ issues.review.review = Review
10671067
issues.review.reviewers = Reviewers
10681068
issues.review.show_outdated = Show outdated
10691069
issues.review.hide_outdated = Hide outdated
1070+
issues.review.show_resolved = Show resolved
1071+
issues.review.hide_resolved = Hide resolved
1072+
issues.review.resolve_conversation = Resolve conversation
1073+
issues.review.un_resolve_conversation = Unresolve conversation
1074+
issues.review.resolved_by = marked this conversation as resolved
10701075
issues.assignee.error = Not all assignees was added due to an unexpected error.
10711076

10721077
pulls.desc = Enable pull requests and code reviews.

‎routers/repo/issue.go

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -990,6 +990,11 @@ func ViewIssue(ctx *context.Context) {
990990
ctx.ServerError("Review.LoadCodeComments", err)
991991
return
992992
}
993+
994+
if err = comment.LoadResolveDoer(); err != nil {
995+
ctx.ServerError("LoadResolveDoer", err)
996+
return
997+
}
993998
}
994999
}
9951000

@@ -1033,6 +1038,11 @@ func ViewIssue(ctx *context.Context) {
10331038
ctx.ServerError("IsUserAllowedToMerge", err)
10341039
return
10351040
}
1041+
1042+
if ctx.Data["CanMarkConversation"], err = models.CanMarkConversation(issue, ctx.User); err != nil {
1043+
ctx.ServerError("CanMarkConversation", err)
1044+
return
1045+
}
10361046
}
10371047

10381048
prUnit, err := repo.GetUnit(models.UnitTypePullRequests)

‎routers/repo/pull.go

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -624,6 +624,13 @@ func ViewPullFiles(ctx *context.Context) {
624624
return
625625
}
626626

627+
if ctx.IsSigned && ctx.User != nil {
628+
if ctx.Data["CanMarkConversation"], err = models.CanMarkConversation(issue, ctx.User); err != nil {
629+
ctx.ServerError("CanMarkConversation", err)
630+
return
631+
}
632+
}
633+
627634
setImageCompareContext(ctx, baseCommit, commit)
628635
setPathsCompareContext(ctx, baseCommit, commit, headTarget)
629636

‎routers/repo/pull_review.go

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,53 @@ func CreateCodeComment(ctx *context.Context, form auth.CodeCommentForm) {
6161
ctx.Redirect(comment.HTMLURL())
6262
}
6363

64+
// UpdateResolveConversation add or remove an Conversation resolved mark
65+
func UpdateResolveConversation(ctx *context.Context) {
66+
action := ctx.Query("action")
67+
commentID := ctx.QueryInt64("comment_id")
68+
69+
comment, err := models.GetCommentByID(commentID)
70+
if err != nil {
71+
ctx.ServerError("GetIssueByID", err)
72+
return
73+
}
74+
75+
if err = comment.LoadIssue(); err != nil {
76+
ctx.ServerError("comment.LoadIssue", err)
77+
return
78+
}
79+
80+
var permResult bool
81+
if permResult, err = models.CanMarkConversation(comment.Issue, ctx.User); err != nil {
82+
ctx.ServerError("CanMarkConversation", err)
83+
return
84+
}
85+
if !permResult {
86+
ctx.Error(403)
87+
return
88+
}
89+
90+
if !comment.Issue.IsPull {
91+
ctx.Error(400)
92+
return
93+
}
94+
95+
if action == "Resolve" || action == "UnResolve" {
96+
err = models.MarkConversation(comment, ctx.User, action == "Resolve")
97+
if err != nil {
98+
ctx.ServerError("MarkConversation", err)
99+
return
100+
}
101+
} else {
102+
ctx.Error(400)
103+
return
104+
}
105+
106+
ctx.JSON(200, map[string]interface{}{
107+
"ok": true,
108+
})
109+
}
110+
64111
// SubmitReview creates a review out of the existing pending review or creates a new one if no pending review exist
65112
func SubmitReview(ctx *context.Context, form auth.SubmitReviewForm) {
66113
issue := GetActionIssue(ctx)

‎routers/routes/routes.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -739,6 +739,7 @@ func RegisterRoutes(m *macaron.Macaron) {
739739
m.Post("/assignee", reqRepoIssuesOrPullsWriter, repo.UpdateIssueAssignee)
740740
m.Post("/request_review", reqRepoIssuesOrPullsReader, repo.UpdatePullReviewRequest)
741741
m.Post("/status", reqRepoIssuesOrPullsWriter, repo.UpdateIssueStatus)
742+
m.Post("/resolve_conversation", reqRepoIssuesOrPullsReader, repo.UpdateResolveConversation)
742743
}, context.RepoMustNotBeArchived())
743744
m.Group("/comments/:id", func() {
744745
m.Post("", repo.UpdateCommentContent)

‎templates/repo/diff/box.tmpl

Lines changed: 49 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -147,32 +147,79 @@
147147
{{end}}
148148
</tr>
149149
{{if gt (len $line.Comments) 0}}
150+
{{$resolved := (index $line.Comments 0).IsResolved}}
151+
{{$resolveDoer := (index $line.Comments 0).ResolveDoer}}
152+
{{$isNotPending := (not (eq (index $line.Comments 0).Review.Type 0))}}
150153
<tr class="add-code-comment">
151154
<td class="lines-num"></td>
152155
<td class="lines-type-marker"></td>
153156
<td class="add-comment-left">
157+
{{if and $resolved (eq $line.GetCommentSide "previous")}}
158+
<div class="ui top attached header">
159+
<span class="ui grey text left"><b>{{$resolveDoer.Name}}</b> {{$.i18n.Tr "repo.issues.review.resolved_by"}}</span>
160+
<button id="show-outdated-{{(index $line.Comments 0).ID}}" data-comment="{{(index $line.Comments 0).ID}}" class="ui compact right labeled button show-outdated">
161+
{{svg "octicon-unfold" 16}}
162+
{{$.i18n.Tr "repo.issues.review.show_resolved"}}
163+
</button>
164+
<button id="hide-outdated-{{(index $line.Comments 0).ID}}" data-comment="{{(index $line.Comments 0).ID}}" class="hide ui compact right labeled button hide-outdated">
165+
{{svg "octicon-fold" 16}}
166+
{{$.i18n.Tr "repo.issues.review.hide_resolved"}}
167+
</button>
168+
</div>
169+
{{end}}
154170
{{if eq $line.GetCommentSide "previous"}}
155-
<div class="field comment-code-cloud">
171+
<div id="code-comments-{{(index $line.Comments 0).ID}}" class="field comment-code-cloud {{if $resolved}}hide{{end}}">
156172
<div class="comment-list">
157173
<ui class="ui comments">
158174
{{ template "repo/diff/comments" dict "root" $ "comments" $line.Comments}}
159175
</ui>
160176
</div>
161177
{{template "repo/diff/comment_form_datahandler" dict "reply" (index $line.Comments 0).ReviewID "hidden" true "root" $ "comment" (index $line.Comments 0)}}
178+
{{if and $.CanMarkConversation $isNotPending}}
179+
<button class="ui icon tiny button resolve-conversation" data-action="{{if not $resolved}}Resolve{{else}}UnResolve{{end}}" data-comment-id="{{(index $line.Comments 0).ID}}" data-update-url="{{$.RepoLink}}/issues/resolve_conversation" >
180+
{{if $resolved}}
181+
{{$.i18n.Tr "repo.issues.review.un_resolve_conversation"}}
182+
{{else}}
183+
{{$.i18n.Tr "repo.issues.review.resolve_conversation"}}
184+
{{end}}
185+
</button>
186+
{{end}}
162187
</div>
163188
{{end}}
164189
</td>
165190
<td class="lines-num"></td>
166191
<td class="lines-type-marker"></td>
167192
<td class="add-comment-right">
193+
{{if and $resolved (eq $line.GetCommentSide "proposed")}}
194+
<div class="ui top attached header">
195+
<span class="ui grey text left"><b>{{$resolveDoer.Name}}</b> {{$.i18n.Tr "repo.issues.review.resolved_by"}}</span>
196+
<button id="show-outdated-{{(index $line.Comments 0).ID}}" data-comment="{{(index $line.Comments 0).ID}}" class="ui compact right labeled button show-outdated">
197+
{{svg "octicon-unfold" 16}}
198+
{{$.i18n.Tr "repo.issues.review.show_resolved"}}
199+
</button>
200+
<button id="hide-outdated-{{(index $line.Comments 0).ID}}" data-comment="{{(index $line.Comments 0).ID}}" class="hide ui compact right labeled button hide-outdated">
201+
{{svg "octicon-fold" 16}}
202+
{{$.i18n.Tr "repo.issues.review.hide_resolved"}}
203+
</button>
204+
</div>
205+
{{end}}
168206
{{if eq $line.GetCommentSide "proposed"}}
169-
<div class="field comment-code-cloud">
207+
<div id="code-comments-{{(index $line.Comments 0).ID}}" class="field comment-code-cloud {{if $resolved}}hide{{end}}">
170208
<div class="comment-list">
171209
<ui class="ui comments">
172210
{{ template "repo/diff/comments" dict "root" $ "comments" $line.Comments}}
173211
</ui>
174212
</div>
175213
{{template "repo/diff/comment_form_datahandler" dict "reply" (index $line.Comments 0).ReviewID "hidden" true "root" $ "comment" (index $line.Comments 0)}}
214+
{{if and $.CanMarkConversation $isNotPending}}
215+
<button class="ui icon tiny button resolve-conversation" data-action="{{if not $resolved}}Resolve{{else}}UnResolve{{end}}" data-comment-id="{{(index $line.Comments 0).ID}}" data-update-url="{{$.RepoLink}}/issues/resolve_conversation" >
216+
{{if $resolved}}
217+
{{$.i18n.Tr "repo.issues.review.un_resolve_conversation"}}
218+
{{else}}
219+
{{$.i18n.Tr "repo.issues.review.resolve_conversation"}}
220+
{{end}}
221+
</button>
222+
{{end}}
176223
</div>
177224
{{end}}
178225
</td>

‎templates/repo/diff/section_unified.tmpl

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,17 +23,42 @@
2323
<td class="lines-code{{if (not $line.RightIdx)}} lines-code-old{{end}}">{{if and $.root.SignedUserID $line.CanComment $.root.PageIsPullFiles}}<a class="ui green button add-code-comment add-code-comment-{{if $line.RightIdx}}right{{else}}left{{end}}" data-path="{{$file.Name}}" data-side="{{if $line.RightIdx}}right{{else}}left{{end}}" data-idx="{{if $line.RightIdx}}{{$line.RightIdx}}{{else}}{{$line.LeftIdx}}{{end}}">+</a>{{end}}<span class="mono wrap{{if $highlightClass}} language-{{$highlightClass}}{{else}} nohighlight{{end}}">{{$section.GetComputedInlineDiffFor $line}}</span></td>
2424
</tr>
2525
{{if gt (len $line.Comments) 0}}
26+
{{$resolved := (index $line.Comments 0).IsResolved}}
27+
{{$resolveDoer := (index $line.Comments 0).ResolveDoer}}
28+
{{$isNotPending := (not (eq (index $line.Comments 0).Review.Type 0))}}
2629
<tr>
2730
<td colspan="2" class="lines-num"></td>
2831
<td class="lines-type-marker"></td>
2932
<td class="add-comment-left add-comment-right">
30-
<div class="field comment-code-cloud">
33+
{{if $resolved}}
34+
<div class = "ui attached header">
35+
<span class="ui grey text left"><b>{{$resolveDoer.Name}}</b> {{$.root.i18n.Tr "repo.issues.review.resolved_by"}}</span>
36+
<button id="show-outdated-{{(index $line.Comments 0).ID}}" data-comment="{{(index $line.Comments 0).ID}}" class="ui compact right labeled button show-outdated">
37+
{{svg "octicon-unfold" 16}}
38+
{{$.root.i18n.Tr "repo.issues.review.show_resolved"}}
39+
</button>
40+
<button id="hide-outdated-{{(index $line.Comments 0).ID}}" data-comment="{{(index $line.Comments 0).ID}}" class="hide ui compact right labeled button hide-outdated">
41+
{{svg "octicon-fold" 16}}
42+
{{$.root.i18n.Tr "repo.issues.review.hide_resolved"}}
43+
</button>
44+
</div>
45+
{{end}}
46+
<div id="code-comments-{{(index $line.Comments 0).ID}}" class="field comment-code-cloud {{if $resolved}}hide{{end}}">
3147
<div class="comment-list">
3248
<ui class="ui comments">
3349
{{ template "repo/diff/comments" dict "root" $.root "comments" $line.Comments}}
3450
</ui>
3551
</div>
3652
{{template "repo/diff/comment_form_datahandler" dict "hidden" true "reply" (index $line.Comments 0).ReviewID "root" $.root "comment" (index $line.Comments 0)}}
53+
{{if and $.root.CanMarkConversation $isNotPending}}
54+
<button class="ui icon tiny button resolve-conversation" data-action="{{if not $resolved}}Resolve{{else}}UnResolve{{end}}" data-comment-id="{{(index $line.Comments 0).ID}}" data-update-url="{{$.root.RepoLink}}/issues/resolve_conversation" >
55+
{{if $resolved}}
56+
{{$.root.i18n.Tr "repo.issues.review.un_resolve_conversation"}}
57+
{{else}}
58+
{{$.root.i18n.Tr "repo.issues.review.resolve_conversation"}}
59+
{{end}}
60+
</button>
61+
{{end}}
3762
</div>
3863
</td>
3964
</tr>

0 commit comments

Comments
 (0)