Fix PR comments UI (#18323)

Closes:
* Review comment cannot be edited #17768
* Changing PR Comment Resolved State Disables Further Changes #18315

models/issue_comment.go

@@ -519,7 +519,7 @@ func (c *Comment) LoadPoster() error {
 	return c.loadPoster(db.GetEngine(db.DefaultContext))
 }
 
-// LoadAttachments loads attachments
+// LoadAttachments loads attachments (it never returns error, the error during `GetAttachmentsByCommentIDCtx` is ignored)
 func (c *Comment) LoadAttachments() error {
 	if len(c.Attachments) > 0 {
 		return nil

routers/api/v1/repo/issue_comment.go

@@ -537,7 +537,9 @@ func editIssueComment(ctx *context.APIContext, form api.EditIssueCommentOption)
 	if !ctx.IsSigned || (ctx.User.ID != comment.PosterID && !ctx.Repo.IsAdmin()) {
 		ctx.Status(http.StatusForbidden)
 		return
-	} else if comment.Type != models.CommentTypeComment {
+	}
+
+	if comment.Type != models.CommentTypeComment && comment.Type != models.CommentTypeReview && comment.Type != models.CommentTypeCode {
 		ctx.Status(http.StatusNoContent)
 		return
 	}

routers/web/repo/issue.go

@@ -1333,7 +1333,7 @@ func ViewIssue(ctx *context.Context) {
 			return
 		}
 
-		if comment.Type == models.CommentTypeComment {
+		if comment.Type == models.CommentTypeComment || comment.Type == models.CommentTypeReview {
 			if err := comment.LoadAttachments(); err != nil {
 				ctx.ServerError("LoadAttachments", err)
 				return
@@ -2194,7 +2194,9 @@ func UpdateCommentContent(ctx *context.Context) {
 	if !ctx.IsSigned || (ctx.User.ID != comment.PosterID && !ctx.Repo.CanWriteIssuesOrPulls(comment.Issue.IsPull)) {
 		ctx.Error(http.StatusForbidden)
 		return
-	} else if comment.Type != models.CommentTypeComment && comment.Type != models.CommentTypeCode {
+	}
+
+	if comment.Type != models.CommentTypeComment && comment.Type != models.CommentTypeReview && comment.Type != models.CommentTypeCode {
 		ctx.Error(http.StatusNoContent)
 		return
 	}
@@ -2212,11 +2214,9 @@ func UpdateCommentContent(ctx *context.Context) {
 		return
 	}
 
-	if comment.Type == models.CommentTypeComment {
+	if err := comment.LoadAttachments(); err != nil {
-		if err := comment.LoadAttachments(); err != nil {
+		ctx.ServerError("LoadAttachments", err)
-			ctx.ServerError("LoadAttachments", err)
+		return
-			return
-		}
 	}
 
 	// when the update request doesn't intend to update attachments (eg: change checkbox state), ignore attachment updates
@@ -2404,7 +2404,9 @@ func ChangeCommentReaction(ctx *context.Context) {
 
 		ctx.Error(http.StatusForbidden)
 		return
-	} else if comment.Type != models.CommentTypeComment && comment.Type != models.CommentTypeCode {
+	}
+
+	if comment.Type != models.CommentTypeComment && comment.Type != models.CommentTypeCode && comment.Type != models.CommentTypeReview {
 		ctx.Error(http.StatusNoContent)
 		return
 	}

templates/repo/issue/view_content/comments.tmpl

@@ -399,7 +399,7 @@
 		</div>
 	{{else if eq .Type 22}}
 		<div class="timeline-item-group">
-			<div class="timeline-item event" id="{{.HashTag}}">
+			<div class="timeline-item event">
 				{{if .OriginalAuthor }}
 				{{else}}
 				<a class="timeline-avatar"{{if gt .Poster.ID 0}} href="{{.Poster.HomeLink}}"{{end}}>
@@ -434,23 +434,46 @@
 				</span>
 			</div>
 			{{if .Content}}
-			<div class="timeline-item comment">
+			<div class="timeline-item comment" id="{{.HashTag}}">
 				<div class="content comment-container">
 					<div class="ui top attached header comment-header df ac sb">
-						<span class="text grey">
+						<div class="comment-header-left df ac">
-							{{if .OriginalAuthor }}
+							<span class="text grey">
-								<span class="text black">
+								{{if .OriginalAuthor }}
-									{{svg (MigrationIcon $.Repository.GetOriginalURLHostname)}}
+									<span class="text black">
-									{{ .OriginalAuthor }}
+										{{svg (MigrationIcon $.Repository.GetOriginalURLHostname)}}
-								</span>
+										{{ .OriginalAuthor }}
-								<span class="text grey"> {{if $.Repository.OriginalURL}}</span>
+									</span>
-								<span class="text migrate">({{$.i18n.Tr "repo.migrated_from" ($.Repository.OriginalURL|Escape) ($.Repository.GetOriginalURLHostname|Escape) | Safe }}){{end}}</span>
+									<span class="text grey"> {{if $.Repository.OriginalURL}}</span>
-							{{else}}
+									<span class="text migrate">({{$.i18n.Tr "repo.migrated_from" ($.Repository.OriginalURL|Escape) ($.Repository.GetOriginalURLHostname|Escape) | Safe }}){{end}}</span>
-								<a class="author"{{if gt .Poster.ID 0}} href="{{.Poster.HomeLink}}"{{end}}>{{.Poster.GetDisplayName}}</a>
+								{{else}}
-							{{end}}
+									<a class="author"{{if gt .Poster.ID 0}} href="{{.Poster.HomeLink}}"{{end}}>{{.Poster.GetDisplayName}}</a>
+								{{end}}
 
-							{{$.i18n.Tr "repo.issues.review.left_comment" | Safe}}
+								{{$.i18n.Tr "repo.issues.review.left_comment" | Safe}}
-						</span>
+							</span>
+						</div>
+						<div class="comment-header-right actions df ac">
+								{{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}}
+								{{if not $.Repository.IsArchived}}
+										{{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" false "IsCommentPoster" (and $.IsSigned (eq $.SignedUserID .PosterID))}}
+								{{end}}
+						</div>
 					</div>
 					<div class="ui attached segment comment-body">
 						<div class="render-content markup">
@@ -460,10 +483,18 @@
 								<span class="no-content">{{$.i18n.Tr "repo.issues.no_content"}}</span>
 							{{end}}
 						</div>
+						<div id="comment-{{.ID}}" class="raw-content hide">{{.Content}}</div>
+						<div class="edit-content-zone hide" data-write="issuecomment-{{.ID}}-write" data-preview="issuecomment-{{.ID}}-preview" data-update-url="{{$.RepoLink}}/comments/{{.ID}}" data-context="{{$.RepoLink}}" data-attachment-url="{{$.RepoLink}}/comments/{{.ID}}/attachments"></div>
 						{{if .Attachments}}
 							{{template "repo/issue/view_content/attachments" Dict "ctx" $ "Attachments" .Attachments "Content" .RenderedContent}}
 						{{end}}
 					</div>
+					{{$reactions := .Reactions.GroupByType}}
+					{{if $reactions}}
+						<div class="ui attached segment reactions">
+								{{template "repo/issue/view_content/reactions" Dict "ctx" $ "ActionURL" (Printf "%s/comments/%d/reactions" $.RepoLink .ID) "Reactions" $reactions}}
+						</div>
+					{{end}}
 				</div>
 			</div>
 			{{end}}

web_src/js/features/repo-diff.js

@@ -45,7 +45,7 @@ export function initRepoDiffConversationForm() {
   });
 
 
-  $('.resolve-conversation').on('click', async function (e) {
+  $(document).on('click', '.resolve-conversation', async function (e) {
     e.preventDefault();
     const comment_id = $(this).data('comment-id');
     const origin = $(this).data('origin');

web_src/js/features/repo-issue-content.js

@@ -109,7 +109,7 @@ export function initRepoIssueContentHistory() {
   if (!issueIndex) return;
 
   const $itemIssue = $('.repository.issue .timeline-item.comment.first'); // issue(PR) main content
-  const $comments = $('.repository.issue .comment-list .comment'); // includes: issue(PR) comments, code rerview comments
+  const $comments = $('.repository.issue .comment-list .comment'); // includes: issue(PR) comments, review comments, code comments
   if (!$itemIssue.length && !$comments.length) return;
 
   const repoLink = $('#repolink').val();

web_src/less/_repository.less

@@ -2673,10 +2673,12 @@
 
   a {
     color: var(--color-text);
+    text-decoration: none;
   }
 
   a:hover {
     color: var(--color-primary);
+    text-decoration: none;
   }
 }