enhance: add permalinks to pull request reviews (#38849)

1. Make review threads linkable via `#pullrequestreview-<reviewID>`
2. Improve CSS so username and timestamp go colored on hover.
3. CSS cleanup, remove dead rules, nonexistant class name, make `.suppressed` actually do what it says in the doc above.

models/issues/review.go

@@ -1065,19 +1065,23 @@ func (r *Review) GetCodeCommentsCount(ctx context.Context) int {
 	return int(count)
 }
 
-// HTMLURL formats a URL-string to the related review issue-comment
+// HashTag returns unique hash tag for review.
+func (r *Review) HashTag() string {
+	return fmt.Sprintf("pullrequestreview-%d", r.ID)
+}
+
+// HTMLURL formats a URL-string to the review on the pull request page
 func (r *Review) HTMLURL(ctx context.Context) string {
-	opts := FindCommentsOptions{
+	if r.Type != ReviewTypeApprove && r.Type != ReviewTypeComment && r.Type != ReviewTypeReject {
-		Type:     CommentTypeReview,
+		return "" // only submitted reviews get a timeline block carrying the anchor
-		IssueID:  r.IssueID,
-		ReviewID: r.ID,
 	}
-	comment := new(Comment)
+	if err := r.LoadIssue(ctx); err != nil {
-	has, err := db.GetEngine(ctx).Where(opts.ToConds()).Get(comment)
-	if err != nil || !has {
 		return ""
 	}
-	return comment.HTMLURL(ctx)
+	if err := r.Issue.LoadRepo(ctx); err != nil {
+		return ""
+	}
+	return r.Issue.HTMLURL(ctx) + "#" + r.HashTag()
 }
 
 // RemapExternalUser ExternalUserRemappable interface

models/issues/review_test.go

@@ -12,6 +12,7 @@ import (
 	repo_model "gitea.dev/models/repo"
 	"gitea.dev/models/unittest"
 	user_model "gitea.dev/models/user"
+	"gitea.dev/modules/setting"
 
 	"github.com/stretchr/testify/assert"
 )
@@ -28,6 +29,15 @@ func TestGetReviewByID(t *testing.T) {
 	assert.True(t, issues_model.IsErrReviewNotExist(err), "IsErrReviewNotExist")
 }
 
+func TestReview_HTMLURL(t *testing.T) {
+	assert.NoError(t, unittest.PrepareTestDatabase())
+	review := unittest.AssertExistsAndLoadBean(t, &issues_model.Review{ID: 1})
+	assert.Equal(t, setting.AppURL+"user2/repo1/pulls/2#pullrequestreview-1", review.HTMLURL(t.Context()))
+
+	pendingReview := unittest.AssertExistsAndLoadBean(t, &issues_model.Review{ID: 4})
+	assert.Empty(t, pendingReview.HTMLURL(t.Context()))
+}
+
 func TestReview_LoadAttributes(t *testing.T) {
 	assert.NoError(t, unittest.PrepareTestDatabase())
 	review := unittest.AssertExistsAndLoadBean(t, &issues_model.Review{ID: 1})

templates/repo/issue/sidebar/stopwatch_timetracker.tmpl

@@ -72,7 +72,7 @@
 			{{range $user, $trackedtime := .WorkingUsers}}
 				<div class="item tw-gap-3">
 					{{template "shared/user/avatarlink" dict "user" $user}}
-					<div>
+					<div class="muted-links">
 						{{template "shared/user/authorlink" $user}}
 						<div class="text">{{$trackedtime|Sec2Hour}}</div>
 					</div>

templates/repo/issue/view_content/comments.tmpl

@@ -361,9 +361,10 @@
 			</div>
 		{{else if eq .Type 22}}
 			<div class="timeline-item-group" id="{{.HashTag}}">
-				<div class="timeline-item event">
+				{{/* default to the "comment" type and the comment anchor if the review record is missing */}}
-					{{$reviewType := 2}}{{/* default to "comment" type if the review record is missing */}}
+				{{$reviewType := 2}}{{$reviewHashTag := .HashTag}}
-					{{if .Review}}{{$reviewType = .Review.Type}}{{end}}
+				{{if .Review}}{{$reviewType = .Review.Type}}{{$reviewHashTag = .Review.HashTag}}{{end}}
+				<div class="timeline-item event"{{if .Review}} id="{{$reviewHashTag}}"{{end}}>
 					{{if not .OriginalAuthor}}
 					{{/* Some timeline avatars need a offset to correctly align with their speech bubble.
 						The condition depends on whether the comment has contents/attachments,
@@ -381,12 +382,13 @@
 					</span>
 					<span class="comment-text-line">
 						{{template "repo/issue/view_content/comments_authorlink" dict "comment" .}}
+						{{$createdLink := HTMLFormat `<a href="#%s">%s</a>` $reviewHashTag $createdStr}}
 						{{if eq $reviewType 1}}
-							{{ctx.Locale.Tr "repo.issues.review.approve" $createdStr}}
+							{{ctx.Locale.Tr "repo.issues.review.approve" $createdLink}}
 						{{else if eq $reviewType 3}}
-							{{ctx.Locale.Tr "repo.issues.review.reject" $createdStr}}
+							{{ctx.Locale.Tr "repo.issues.review.reject" $createdLink}}
 						{{else}}
-							{{ctx.Locale.Tr "repo.issues.review.comment" $createdStr}}
+							{{ctx.Locale.Tr "repo.issues.review.comment" $createdLink}}
 						{{end}}
 						{{if and .Review .Review.Dismissed}}
 							<div class="ui small label">{{ctx.Locale.Tr "repo.issues.review.dismissed_label"}}</div>

templates/shared/secrets/add_list.tmpl

@@ -34,7 +34,7 @@
 				</div>
 			</div>
 			<div class="item-trailing">
-				<span class="color-text-light-2">
+				<span>
 					{{ctx.Locale.Tr "settings.added_on" (DateUtils.AbsoluteShort .CreatedUnix)}}
 				</span>
 				<button class="btn interact-bg show-modal tw-p-2"

templates/shared/user/authorlink.tmpl

@@ -1 +1 @@
-<a class="muted tw-text-text tw-font-semibold"{{if gt .ID 0}} href="{{.HomeLink}}"{{end}}>{{.GetDisplayName}}</a>{{if .IsTypeBot}}&nbsp;<span class="ui basic label tw-p-1 tw-align-baseline">bot</span>{{end}}
+<a class="tw-font-semibold"{{if gt .ID 0}} href="{{.HomeLink}}"{{end}}>{{.GetDisplayName}}</a>{{if .IsTypeBot}}&nbsp;<span class="ui basic label tw-p-1 tw-align-baseline">bot</span>{{end}}

templates/shared/variables/variable_list.tmpl

@@ -33,7 +33,7 @@
 				</div>
 			</div>
 			<div class="item-trailing">
-				<span class="color-text-light-2">
+				<span>
 					{{ctx.Locale.Tr "settings.added_on" (DateUtils.AbsoluteShort .CreatedUnix)}}
 				</span>
 				<button class="btn interact-bg tw-p-2 show-modal"

web_src/css/base.css

@@ -301,9 +301,7 @@ a.silenced,
 }
 
 a:hover,
-a.suppressed:hover,
 a.muted:hover,
-a.muted:hover [class*="color-text"],
 .muted-links a:hover {
   color: var(--color-primary);
 }

web_src/css/repo.css

@@ -511,20 +511,24 @@ td .commit-summary {
   margin-left: -16px;
 }
 
-.repository.view.issue .comment-list .timeline-item .comment-text-line {
+.comment-text-line {
   color: var(--color-text-light);
   flex: 1;
   min-width: 0;
 }
 
-.repository.view.issue .comment-list .timeline-item .comment-text-line .ui.label {
+.comment-text-line .ui.label {
   line-height: 1.5; /* label has background, so it can't use parent's line-height */
 }
 
-.repository.view.issue .comment-list .timeline-item .comment-text-line a {
+.comment-text-line a {
   color: inherit;
 }
 
+.comment-text-line a:hover {
+  color: var(--color-primary);
+}
+
 .repository.view.issue .comment-list .timeline-item.commits-list {
   padding-left: 15px;
   padding-top: 0;

web_src/js/features/repo-issue.ts

@@ -197,8 +197,9 @@ export async function handleReply(el: HTMLElement) {
 }
 
 export function initRepoPullRequestReview() {
-  if (window.location.hash && window.location.hash.startsWith('#issuecomment-')) {
+  const currentHash = window.location.hash;
-    const commentDiv = document.querySelector(window.location.hash);
+  if (currentHash.startsWith('#issuecomment-') || currentHash.startsWith('#pullrequestreview-')) {
+    const commentDiv = document.querySelector(currentHash);
     if (commentDiv) {
       // get the name of the parent id
       const groupID = commentDiv.closest('div[id^="code-comments-"]')?.getAttribute('id');