Add a way to mark Conversation (code comment) resolved (go-gitea#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]>
ydelafollye · Jul 31, 2020 · 5de7179 · 5de7179
1 parent 74c3ce2
commit 5de7179
Show file tree

Hide file tree

Showing 13 changed files with 301 additions and 9 deletions.
diff --git a/models/issue_comment.go b/models/issue_comment.go
@@ -122,6 +122,8 @@ type Comment struct {
 	AssigneeID       int64
 	RemovedAssignee  bool
 	Assignee         *User `xorm:"-"`
+	ResolveDoerID    int64
+	ResolveDoer      *User `xorm:"-"`
 	OldTitle         string
 	NewTitle         string
 	OldRef           string
@@ -420,6 +422,26 @@ func (c *Comment) LoadAssigneeUser() error {
 	return nil
 }
 
+// LoadResolveDoer if comment.Type is CommentTypeCode and ResolveDoerID not zero, then load resolveDoer
+func (c *Comment) LoadResolveDoer() (err error) {
+	if c.ResolveDoerID == 0 || c.Type != CommentTypeCode {
+		return nil
+	}
+	c.ResolveDoer, err = getUserByID(x, c.ResolveDoerID)
+	if err != nil {
+		if IsErrUserNotExist(err) {
+			c.ResolveDoer = NewGhostUser()
+			err = nil
+		}
+	}
+	return
+}
+
+// IsResolved check if an code comment is resolved
+func (c *Comment) IsResolved() bool {
+	return c.ResolveDoerID != 0 && c.Type == CommentTypeCode
+}
+
 // LoadDepIssueDetails loads Dependent Issue Details
 func (c *Comment) LoadDepIssueDetails() (err error) {
 	if c.DependentIssueID <= 0 || c.DependentIssue != nil {
@@ -943,7 +965,12 @@ func fetchCodeCommentsByReview(e Engine, issue *Issue, currentUser *User, review
 	if err := e.In("id", ids).Find(&reviews); err != nil {
 		return nil, err
 	}
+
 	for _, comment := range comments {
+		if err := comment.LoadResolveDoer(); err != nil {
+			return nil, err
+		}
+
 		if re, ok := reviews[comment.ReviewID]; ok && re != nil {
 			// If the review is pending only the author can see the comments (except the review is set)
 			if review.ID == 0 {

diff --git a/models/migrations/migrations.go b/models/migrations/migrations.go
@@ -208,6 +208,8 @@ var migrations = []Migration{
 	NewMigration("Add CommitsAhead and CommitsBehind Column to PullRequest Table", addCommitDivergenceToPulls),
 	// v137 -> v138
 	NewMigration("Add Branch Protection Block Outdated Branch", addBlockOnOutdatedBranch),
+	// v138 -> v139
+	NewMigration("Add ResolveDoerID to Comment table", addResolveDoerIDCommentColumn),
 }
 
 // GetCurrentDBVersion returns the current db version

diff --git a/models/migrations/v138.go b/models/migrations/v138.go
@@ -0,0 +1,22 @@
+// Copyright 2020 The Gitea Authors. All rights reserved.
+// Use of this source code is governed by a MIT-style
+// license that can be found in the LICENSE file.
+
+package migrations
+
+import (
+	"fmt"
+
+	"xorm.io/xorm"
+)
+
+func addResolveDoerIDCommentColumn(x *xorm.Engine) error {
+	type Comment struct {
+		ResolveDoerID int64
+	}
+
+	if err := x.Sync2(new(Comment)); err != nil {
+		return fmt.Errorf("Sync2: %v", err)
+	}
+	return nil
+}
diff --git a/models/review.go b/models/review.go
@@ -5,6 +5,7 @@
 package models
 
 import (
+	"fmt"
 	"strings"
 
 	"code.gitea.io/gitea/modules/timeutil"
@@ -594,3 +595,62 @@ func RemoveRewiewRequest(issue *Issue, reviewer *User, doer *User) (comment *Com
 
 	return comment, sess.Commit()
 }
+
+// MarkConversation Add or remove Conversation mark for a code comment
+func MarkConversation(comment *Comment, doer *User, isResolve bool) (err error) {
+	if comment.Type != CommentTypeCode {
+		return nil
+	}
+
+	if isResolve {
+		if comment.ResolveDoerID != 0 {
+			return nil
+		}
+
+		if _, err = x.Exec("UPDATE `comment` SET resolve_doer_id=? WHERE id=?", doer.ID, comment.ID); err != nil {
+			return err
+		}
+	} else {
+		if comment.ResolveDoerID == 0 {
+			return nil
+		}
+
+		if _, err = x.Exec("UPDATE `comment` SET resolve_doer_id=? WHERE id=?", 0, comment.ID); err != nil {
+			return err
+		}
+	}
+
+	return nil
+}
+
+// CanMarkConversation  Add or remove Conversation mark for a code comment permission check
+// the PR writer , offfcial reviewer and poster can do it
+func CanMarkConversation(issue *Issue, doer *User) (permResult bool, err error) {
+	if doer == nil || issue == nil {
+		return false, fmt.Errorf("issue or doer is nil")
+	}
+
+	if doer.ID != issue.PosterID {
+		if err = issue.LoadRepo(); err != nil {
+			return false, err
+		}
+
+		perm, err := GetUserRepoPermission(issue.Repo, doer)
+		if err != nil {
+			return false, err
+		}
+
+		permResult = perm.CanAccess(AccessModeWrite, UnitTypePullRequests)
+		if !permResult {
+			if permResult, err = IsOfficialReviewer(issue, doer); err != nil {
+				return false, err
+			}
+		}
+
+		if !permResult {
+			return false, nil
+		}
+	}
+
+	return true, nil
+}
diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini
@@ -1067,6 +1067,11 @@ issues.review.review = Review
 issues.review.reviewers = Reviewers
 issues.review.show_outdated = Show outdated
 issues.review.hide_outdated = Hide outdated
+issues.review.show_resolved = Show resolved
+issues.review.hide_resolved = Hide resolved
+issues.review.resolve_conversation = Resolve conversation
+issues.review.un_resolve_conversation = Unresolve conversation
+issues.review.resolved_by = marked this conversation as resolved
 issues.assignee.error = Not all assignees was added due to an unexpected error.
 
 pulls.desc = Enable pull requests and code reviews.

diff --git a/routers/repo/issue.go b/routers/repo/issue.go
@@ -990,6 +990,11 @@ func ViewIssue(ctx *context.Context) {
 				ctx.ServerError("Review.LoadCodeComments", err)
 				return
 			}
+
+			if err = comment.LoadResolveDoer(); err != nil {
+				ctx.ServerError("LoadResolveDoer", err)
+				return
+			}
 		}
 	}
 
@@ -1033,6 +1038,11 @@ func ViewIssue(ctx *context.Context) {
 				ctx.ServerError("IsUserAllowedToMerge", err)
 				return
 			}
+
+			if ctx.Data["CanMarkConversation"], err = models.CanMarkConversation(issue, ctx.User); err != nil {
+				ctx.ServerError("CanMarkConversation", err)
+				return
+			}
 		}
 
 		prUnit, err := repo.GetUnit(models.UnitTypePullRequests)

diff --git a/routers/repo/pull.go b/routers/repo/pull.go
@@ -624,6 +624,13 @@ func ViewPullFiles(ctx *context.Context) {
 		return
 	}
 
+	if ctx.IsSigned && ctx.User != nil {
+		if ctx.Data["CanMarkConversation"], err = models.CanMarkConversation(issue, ctx.User); err != nil {
+			ctx.ServerError("CanMarkConversation", err)
+			return
+		}
+	}
+
 	setImageCompareContext(ctx, baseCommit, commit)
 	setPathsCompareContext(ctx, baseCommit, commit, headTarget)
 

diff --git a/routers/repo/pull_review.go b/routers/repo/pull_review.go
@@ -61,6 +61,53 @@ func CreateCodeComment(ctx *context.Context, form auth.CodeCommentForm) {
 	ctx.Redirect(comment.HTMLURL())
 }
 
+// UpdateResolveConversation add or remove an Conversation resolved mark
+func UpdateResolveConversation(ctx *context.Context) {
+	action := ctx.Query("action")
+	commentID := ctx.QueryInt64("comment_id")
+
+	comment, err := models.GetCommentByID(commentID)
+	if err != nil {
+		ctx.ServerError("GetIssueByID", err)
+		return
+	}
+
+	if err = comment.LoadIssue(); err != nil {
+		ctx.ServerError("comment.LoadIssue", err)
+		return
+	}
+
+	var permResult bool
+	if permResult, err = models.CanMarkConversation(comment.Issue, ctx.User); err != nil {
+		ctx.ServerError("CanMarkConversation", err)
+		return
+	}
+	if !permResult {
+		ctx.Error(403)
+		return
+	}
+
+	if !comment.Issue.IsPull {
+		ctx.Error(400)
+		return
+	}
+
+	if action == "Resolve" || action == "UnResolve" {
+		err = models.MarkConversation(comment, ctx.User, action == "Resolve")
+		if err != nil {
+			ctx.ServerError("MarkConversation", err)
+			return
+		}
+	} else {
+		ctx.Error(400)
+		return
+	}
+
+	ctx.JSON(200, map[string]interface{}{
+		"ok": true,
+	})
+}
+
 // SubmitReview creates a review out of the existing pending review or creates a new one if no pending review exist
 func SubmitReview(ctx *context.Context, form auth.SubmitReviewForm) {
 	issue := GetActionIssue(ctx)

diff --git a/routers/routes/routes.go b/routers/routes/routes.go
@@ -739,6 +739,7 @@ func RegisterRoutes(m *macaron.Macaron) {
 			m.Post("/assignee", reqRepoIssuesOrPullsWriter, repo.UpdateIssueAssignee)
 			m.Post("/request_review", reqRepoIssuesOrPullsReader, repo.UpdatePullReviewRequest)
 			m.Post("/status", reqRepoIssuesOrPullsWriter, repo.UpdateIssueStatus)
+			m.Post("/resolve_conversation", reqRepoIssuesOrPullsReader, repo.UpdateResolveConversation)
 		}, context.RepoMustNotBeArchived())
 		m.Group("/comments/:id", func() {
 			m.Post("", repo.UpdateCommentContent)

diff --git a/templates/repo/diff/box.tmpl b/templates/repo/diff/box.tmpl
@@ -147,32 +147,79 @@
 															{{end}}
 														</tr>
 														{{if gt (len $line.Comments) 0}}
+															{{$resolved := (index $line.Comments 0).IsResolved}}
+															{{$resolveDoer := (index $line.Comments 0).ResolveDoer}}
+															{{$isNotPending := (not (eq (index $line.Comments 0).Review.Type 0))}}
 															<tr class="add-code-comment">
 																<td class="lines-num"></td>
 																<td class="lines-type-marker"></td>
 																<td class="add-comment-left">
+																	{{if and $resolved  (eq $line.GetCommentSide "previous")}}
+																		<div class="ui top attached header">
+																			<span class="ui grey text left"><b>{{$resolveDoer.Name}}</b> {{$.i18n.Tr "repo.issues.review.resolved_by"}}</span>
+																			<button id="show-outdated-{{(index $line.Comments 0).ID}}" data-comment="{{(index $line.Comments 0).ID}}" class="ui compact right labeled button show-outdated">
+																				{{svg "octicon-unfold" 16}}
+																				{{$.i18n.Tr "repo.issues.review.show_resolved"}}
+																			</button>
+																			<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">
+																				{{svg "octicon-fold" 16}}
+																				{{$.i18n.Tr "repo.issues.review.hide_resolved"}}
+																			</button>
+																		</div>
+																	{{end}}
 																	{{if eq $line.GetCommentSide "previous"}}
-																		<div class="field comment-code-cloud">
+																		<div id="code-comments-{{(index  $line.Comments 0).ID}}" class="field comment-code-cloud {{if $resolved}}hide{{end}}">
 																			<div class="comment-list">
 																				<ui class="ui comments">
 																				{{ template "repo/diff/comments" dict "root" $ "comments" $line.Comments}}
 																				</ui>
 																			</div>
 																		{{template "repo/diff/comment_form_datahandler" dict "reply" (index $line.Comments 0).ReviewID "hidden" true "root" $ "comment" (index $line.Comments 0)}}
+																			{{if and $.CanMarkConversation $isNotPending}}
+																				<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" >
+																					{{if $resolved}}
+																						{{$.i18n.Tr "repo.issues.review.un_resolve_conversation"}}
+																					{{else}}
+																						{{$.i18n.Tr "repo.issues.review.resolve_conversation"}}
+																					{{end}}
+																				</button>
+																			{{end}}
 																		</div>
 																	{{end}}
 																</td>
 																<td class="lines-num"></td>
 																<td class="lines-type-marker"></td>
 																<td class="add-comment-right">
+																	{{if and $resolved (eq $line.GetCommentSide "proposed")}}
+																		<div class="ui top attached header">
+																			<span class="ui grey text left"><b>{{$resolveDoer.Name}}</b> {{$.i18n.Tr "repo.issues.review.resolved_by"}}</span>	
+																			<button id="show-outdated-{{(index $line.Comments 0).ID}}" data-comment="{{(index $line.Comments 0).ID}}" class="ui compact right labeled button show-outdated">
+																				{{svg "octicon-unfold" 16}}
+																				{{$.i18n.Tr "repo.issues.review.show_resolved"}}
+																			</button>
+																			<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">
+																				{{svg "octicon-fold" 16}}
+																				{{$.i18n.Tr "repo.issues.review.hide_resolved"}}
+																			</button>	
+																		</div>
+																	{{end}}
 																	{{if eq $line.GetCommentSide "proposed"}}
-																		<div class="field comment-code-cloud">
+																		<div id="code-comments-{{(index  $line.Comments 0).ID}}" class="field comment-code-cloud {{if $resolved}}hide{{end}}">
 																			<div class="comment-list">
 																				<ui class="ui comments">
 																				{{ template "repo/diff/comments" dict "root" $ "comments" $line.Comments}}
 																				</ui>
 																			</div>
 																			{{template "repo/diff/comment_form_datahandler" dict "reply" (index $line.Comments 0).ReviewID "hidden" true "root" $ "comment" (index $line.Comments 0)}}
+																			{{if and $.CanMarkConversation $isNotPending}}
+																				<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" >
+																					{{if $resolved}}
+																						{{$.i18n.Tr "repo.issues.review.un_resolve_conversation"}}
+																					{{else}}
+																						{{$.i18n.Tr "repo.issues.review.resolve_conversation"}}
+																					{{end}}
+																				</button>
+																			{{end}}
 																		</div>
 																	{{end}}
 																</td>

diff --git a/templates/repo/diff/section_unified.tmpl b/templates/repo/diff/section_unified.tmpl
@@ -23,17 +23,42 @@
 			<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>
 		</tr>
 		{{if gt (len $line.Comments) 0}}
+			{{$resolved := (index $line.Comments 0).IsResolved}}
+			{{$resolveDoer := (index $line.Comments 0).ResolveDoer}}
+			{{$isNotPending := (not (eq (index $line.Comments 0).Review.Type 0))}}
 		<tr>
 			<td colspan="2" class="lines-num"></td>
 			<td class="lines-type-marker"></td>
 			<td class="add-comment-left add-comment-right">
-				<div class="field comment-code-cloud">
+				{{if $resolved}}
+					<div class = "ui attached header">
+						<span class="ui grey text left"><b>{{$resolveDoer.Name}}</b> {{$.root.i18n.Tr "repo.issues.review.resolved_by"}}</span>
+						<button id="show-outdated-{{(index $line.Comments 0).ID}}" data-comment="{{(index $line.Comments 0).ID}}" class="ui compact right labeled button show-outdated">
+							{{svg "octicon-unfold" 16}}
+								{{$.root.i18n.Tr "repo.issues.review.show_resolved"}}
+						</button>
+						<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">
+							{{svg "octicon-fold" 16}}
+							{{$.root.i18n.Tr "repo.issues.review.hide_resolved"}}
+						</button>
+					</div>
+				{{end}}
+				<div id="code-comments-{{(index  $line.Comments 0).ID}}" class="field comment-code-cloud {{if $resolved}}hide{{end}}">
 					<div class="comment-list">
 						<ui class="ui comments">
 						{{ template "repo/diff/comments" dict "root" $.root "comments" $line.Comments}}
 						</ui>
 					</div>
 					{{template "repo/diff/comment_form_datahandler" dict "hidden" true "reply" (index $line.Comments 0).ReviewID "root" $.root "comment" (index $line.Comments 0)}}
+					{{if and $.root.CanMarkConversation $isNotPending}}
+						<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" >
+							{{if $resolved}}
+								{{$.root.i18n.Tr "repo.issues.review.un_resolve_conversation"}}
+							{{else}}
+								{{$.root.i18n.Tr "repo.issues.review.resolve_conversation"}}
+							{{end}}
+						</button>
+					{{end}}
 				</div>
 			</td>
 		</tr>