Fix template error when comment review doesn't exist (#29888)

models/fixtures/comment.yml

@@ -75,3 +75,11 @@
  content: "comment in private pository"
  created_unix: 946684811
  updated_unix: 946684811
+
+-
+ id: 9
+ type: 22 # review
+ poster_id: 2
+ issue_id: 2 # in repo_id 1
+ review_id: 20
+ created_unix: 946684810

models/fixtures/review.yml

@@ -170,3 +170,12 @@
  content: "review request for user15"
  updated_unix: 946684835
  created_unix: 946684835
+
+-
+ id: 20
+ type: 22
+ reviewer_id: 1
+ issue_id: 2
+ content: "Review Comment"
+ updated_unix: 946684810
+ created_unix: 946684810

routers/web/repo/pull_review_test.go

@@ -4,6 +4,7 @@
 package repo
 
 import (
+ "net/http"
  "net/http/httptest"
  "testing"
 
@@ -73,4 +74,20 @@ func TestRenderConversation(t *testing.T) {
  renderConversation(ctx, preparedComment, "timeline")
  assert.Contains(t, resp.Body.String(), `<div id="code-comments-`)
  })
+ run("diff non-existing review", func(t *testing.T, ctx *context.Context, resp *httptest.ResponseRecorder) {
+ err := db.TruncateBeans(db.DefaultContext, &issues_model.Review{})
+ assert.NoError(t, err)
+ ctx.Data["ShowOutdatedComments"] = true
+ renderConversation(ctx, preparedComment, "diff")
+ assert.Equal(t, http.StatusOK, resp.Code)
+ assert.NotContains(t, resp.Body.String(), `status-page-500`)
+ })
+ run("timeline non-existing review", func(t *testing.T, ctx *context.Context, resp *httptest.ResponseRecorder) {
+ err := db.TruncateBeans(db.DefaultContext, &issues_model.Review{})
+ assert.NoError(t, err)
+ ctx.Data["ShowOutdatedComments"] = true
+ renderConversation(ctx, preparedComment, "timeline")
+ assert.Equal(t, http.StatusOK, resp.Code)
+ assert.NotContains(t, resp.Body.String(), `status-page-500`)
+ })
 }

templates/repo/diff/comments.tmpl

@@ -37,7 +37,7 @@
  {{ctx.Locale.Tr "repo.issues.review.outdated"}}
  </a>
  {{end}}
- {{if and .Review}}
+ {{if .Review}}
  {{if eq .Review.Type 0}}
  <div class="ui label basic small yellow pending-label" data-tooltip-content="{{ctx.Locale.Tr "repo.issues.review.pending.tooltip" (ctx.Locale.Tr "repo.diff.review") (ctx.Locale.Tr "repo.diff.review.approve") (ctx.Locale.Tr "repo.diff.review.comment") (ctx.Locale.Tr "repo.diff.review.reject")}}">
  {{ctx.Locale.Tr "repo.issues.review.pending"}}

templates/repo/diff/conversation.tmpl

@@ -1,66 +1,72 @@
-{{$resolved := (index .comments 0).IsResolved}}
-{{$invalid := (index .comments 0).Invalidated}}
-{{$resolveDoer := (index .comments 0).ResolveDoer}}
-{{$isNotPending := (not (eq (index .comments 0).Review.Type 0))}}
-{{$referenceUrl := printf "%s#%s" $.Issue.Link (index .comments 0).HashTag}}
-<div class="conversation-holder" data-path="{{(index .comments 0).TreePath}}" data-side="{{if lt (index .comments 0).Line 0}}left{{else}}right{{end}}" data-idx="{{(index .comments 0).UnsignedLine}}">
- {{if $resolved}}
- <div class="ui attached header resolved-placeholder gt-df gt-ac gt-sb">
- <div class="ui grey text gt-df gt-ac gt-fw gt-gap-2">
- {{svg "octicon-check" 16 "icon gt-mr-2"}}
- <b>{{$resolveDoer.Name}}</b> {{ctx.Locale.Tr "repo.issues.review.resolved_by"}}
- {{if $invalid}}
- <!--
- We only handle the case $resolved=true and $invalid=true in this template because if the comment is not resolved it has the outdated label in the comments area (not the header above).
- The case $resolved=false and $invalid=true is handled in repo/diff/comments.tmpl
- -->
- <a href="{{$referenceUrl}}" class="ui label basic small gt-ml-3" data-tooltip-content="{{ctx.Locale.Tr "repo.issues.review.outdated_description"}}">
- {{ctx.Locale.Tr "repo.issues.review.outdated"}}
- </a>
- {{end}}
+{{if len .comments}}
+ {{$comment := index .comments 0}}
+ {{$resolved := $comment.IsResolved}}
+ {{$invalid := $comment.Invalidated}}
+ {{$resolveDoer := $comment.ResolveDoer}}
+ {{$hasReview := and $comment.Review}}
+ {{$isReviewPending := and $hasReview (eq $comment.Review.Type 0)}}
+ {{$referenceUrl := printf "%s#%s" $.Issue.Link $comment.HashTag}}
+ <div class="conversation-holder" data-path="{{$comment.TreePath}}" data-side="{{if lt $comment.Line 0}}left{{else}}right{{end}}" data-idx="{{$comment.UnsignedLine}}">
+ {{if $resolved}}
+ <div class="ui attached header resolved-placeholder gt-df gt-ac gt-sb">
+ <div class="ui grey text gt-df gt-ac gt-fw gt-gap-2">
+ {{svg "octicon-check" 16 "icon gt-mr-2"}}
+ <b>{{$resolveDoer.Name}}</b> {{ctx.Locale.Tr "repo.issues.review.resolved_by"}}
+ {{if $invalid}}
+ <!--
+ We only handle the case $resolved=true and $invalid=true in this template because if the comment is not resolved it has the outdated label in the comments area (not the header above).
+ The case $resolved=false and $invalid=true is handled in repo/diff/comments.tmpl
+ -->
+ <a href="{{$referenceUrl}}" class="ui label basic small gt-ml-3" data-tooltip-content="{{ctx.Locale.Tr "repo.issues.review.outdated_description"}}">
+ {{ctx.Locale.Tr "repo.issues.review.outdated"}}
+ </a>
+ {{end}}
+ </div>
+ <div class="gt-df gt-ac gt-gap-3">
+ <button id="show-outdated-{{$comment.ID}}" data-comment="{{$comment.ID}}" class="ui tiny labeled button show-outdated gt-df gt-ac">
+ {{svg "octicon-unfold" 16 "gt-mr-3"}}
+ {{ctx.Locale.Tr "repo.issues.review.show_resolved"}}
+ </button>
+ <button id="hide-outdated-{{$comment.ID}}" data-comment="{{$comment.ID}}" class="ui tiny labeled button hide-outdated gt-df gt-ac gt-hidden">
+ {{svg "octicon-fold" 16 "gt-mr-3"}}
+ {{ctx.Locale.Tr "repo.issues.review.hide_resolved"}}
+ </button>
+ </div>
  </div>
- <div class="gt-df gt-ac gt-gap-3">
- <button id="show-outdated-{{(index .comments 0).ID}}" data-comment="{{(index .comments 0).ID}}" class="ui tiny labeled button show-outdated gt-df gt-ac">
- {{svg "octicon-unfold" 16 "gt-mr-3"}}
- {{ctx.Locale.Tr "repo.issues.review.show_resolved"}}
- </button>
- <button id="hide-outdated-{{(index .comments 0).ID}}" data-comment="{{(index .comments 0).ID}}" class="ui tiny labeled button hide-outdated gt-df gt-ac gt-hidden">
- {{svg "octicon-fold" 16 "gt-mr-3"}}
- {{ctx.Locale.Tr "repo.issues.review.hide_resolved"}}
- </button>
+ {{end}}
+ <div id="code-comments-{{$comment.ID}}" class="field comment-code-cloud {{if $resolved}}gt-hidden{{end}}">
+ <div class="comment-list">
+ <ui class="ui comments">
+ {{template "repo/diff/comments" dict "root" $ "comments" .comments}}
+ </ui>
  </div>
- </div>
- {{end}}
- <div id="code-comments-{{(index .comments 0).ID}}" class="field comment-code-cloud {{if $resolved}}gt-hidden{{end}}">
- <div class="comment-list">
- <ui class="ui comments">
- {{template "repo/diff/comments" dict "root" $ "comments" .comments}}
- </ui>
- </div>
- <div class="gt-df gt-je gt-ac gt-fw gt-mt-3">
- <div class="ui buttons gt-mr-2">
- <button class="ui icon tiny basic button previous-conversation">
- {{svg "octicon-arrow-up" 12 "icon"}} {{ctx.Locale.Tr "repo.issues.previous"}}
- </button>
- <button class="ui icon tiny basic button next-conversation">
- {{svg "octicon-arrow-down" 12 "icon"}} {{ctx.Locale.Tr "repo.issues.next"}}
- </button>
+ <div class="gt-df gt-je gt-ac gt-fw gt-mt-3">
+ <div class="ui buttons gt-mr-2">
+ <button class="ui icon tiny basic button previous-conversation">
+ {{svg "octicon-arrow-up" 12 "icon"}} {{ctx.Locale.Tr "repo.issues.previous"}}
+ </button>
+ <button class="ui icon tiny basic button next-conversation">
+ {{svg "octicon-arrow-down" 12 "icon"}} {{ctx.Locale.Tr "repo.issues.next"}}
+ </button>
+ </div>
+ {{if and $.CanMarkConversation $hasReview (not $isReviewPending)}}
+ <button class="ui icon tiny basic button resolve-conversation" data-origin="diff" data-action="{{if not $resolved}}Resolve{{else}}UnResolve{{end}}" data-comment-id="{{$comment.ID}}" data-update-url="{{$.RepoLink}}/issues/resolve_conversation">
+ {{if $resolved}}
+ {{ctx.Locale.Tr "repo.issues.review.un_resolve_conversation"}}
+ {{else}}
+ {{ctx.Locale.Tr "repo.issues.review.resolve_conversation"}}
+ {{end}}
+ </button>
+ {{end}}
+ {{if and $.SignedUserID (not $.Repository.IsArchived)}}
+ <button class="comment-form-reply ui primary tiny labeled icon button gt-ml-2 gt-mr-0">
+ {{svg "octicon-reply" 16 "reply icon gt-mr-2"}}{{ctx.Locale.Tr "repo.diff.comment.reply"}}
+ </button>
+ {{end}}
  </div>
- {{if and $.CanMarkConversation $isNotPending}}
- <button class="ui icon tiny basic button resolve-conversation" data-origin="diff" data-action="{{if not $resolved}}Resolve{{else}}UnResolve{{end}}" data-comment-id="{{(index .comments 0).ID}}" data-update-url="{{$.RepoLink}}/issues/resolve_conversation">
- {{if $resolved}}
- {{ctx.Locale.Tr "repo.issues.review.un_resolve_conversation"}}
- {{else}}
- {{ctx.Locale.Tr "repo.issues.review.resolve_conversation"}}
- {{end}}
- </button>
- {{end}}
- {{if and $.SignedUserID (not $.Repository.IsArchived)}}
- <button class="comment-form-reply ui primary tiny labeled icon button gt-ml-2 gt-mr-0">
- {{svg "octicon-reply" 16 "reply icon gt-mr-2"}}{{ctx.Locale.Tr "repo.diff.comment.reply"}}
- </button>
- {{end}}
+ {{template "repo/diff/comment_form_datahandler" dict "hidden" true "reply" $comment.ReviewID "root" $ "comment" $comment}}
  </div>
- {{template "repo/diff/comment_form_datahandler" dict "hidden" true "reply" (index .comments 0).ReviewID "root" $ "comment" (index .comments 0)}}
  </div>
-</div>
+{{else}}
+ {{template "repo/diff/conversation_outdated"}}
+{{end}}

templates/repo/issue/view_content/comments.tmpl

@@ -365,16 +365,20 @@
  {{else if eq .Type 22}}
  <div class="timeline-item-group">
  <div class="timeline-item event">
+ {{$reviewType := -1}}
+ {{if .Review}}{{$reviewType = .Review.Type}}{{end}}
  {{if .OriginalAuthor}}
  {{else}}
  {{/* Some timeline avatars need a offset to correctly align with their speech
  bubble. The condition depends on review type and for positive reviews whether
  there is a comment element or not */}}
- <a class="timeline-avatar{{if or (and (eq .Review.Type 1) (or .Content .Attachments)) (and (eq .Review.Type 2) (or .Content .Attachments)) (eq .Review.Type 3)}} timeline-avatar-offset{{end}}"{{if gt .Poster.ID 0}} href="{{.Poster.HomeLink}}"{{end}}>
+ <a class="timeline-avatar{{if or (and (eq $reviewType 1) (or .Content .Attachments)) (and (eq $reviewType 2) (or .Content .Attachments)) (eq $reviewType 3)}} timeline-avatar-offset{{end}}"{{if gt .Poster.ID 0}} href="{{.Poster.HomeLink}}"{{end}}>
  {{ctx.AvatarUtils.Avatar .Poster 40}}
  </a>
  {{end}}
- <span class="badge{{if eq .Review.Type 1}} gt-bg-green gt-text-white{{else if eq .Review.Type 3}} gt-bg-red gt-text-white{{end}}">{{svg (printf "octicon-%s" .Review.Type.Icon)}}</span>
+ <span class="badge{{if eq $reviewType 1}} gt-bg-green gt-text-white{{else if eq $reviewType 3}} gt-bg-red gt-text-white{{end}}">
+ {{if .Review}}{{svg (printf "octicon-%s" .Review.Type.Icon)}}{{end}}
+ </span>
  <span class="text grey muted-links">
  {{if .OriginalAuthor}}
  <span class="text black">
@@ -387,16 +391,16 @@
  {{template "shared/user/authorlink" .Poster}}
  {{end}}
 
- {{if eq .Review.Type 1}}
+ {{if eq $reviewType 1}}
  {{ctx.Locale.Tr "repo.issues.review.approve" $createdStr | Safe}}
- {{else if eq .Review.Type 2}}
+ {{else if eq $reviewType 2}}
  {{ctx.Locale.Tr "repo.issues.review.comment" $createdStr | Safe}}
- {{else if eq .Review.Type 3}}
+ {{else if eq $reviewType 3}}
  {{ctx.Locale.Tr "repo.issues.review.reject" $createdStr | Safe}}
  {{else}}
  {{ctx.Locale.Tr "repo.issues.review.comment" $createdStr | Safe}}
  {{end}}
- {{if .Review.Dismissed}}
+ {{if and .Review .Review.Dismissed}}
  <div class="ui small label">{{ctx.Locale.Tr "repo.issues.review.dismissed_label"}}</div>
  {{end}}
  </span>
@@ -456,7 +460,7 @@
  </div>
  {{end}}
 
- {{if .Review.CodeComments}}
+ {{if and .Review .Review.CodeComments}}
  <div class="timeline-item event">
  {{range $filename, $lines := .Review.CodeComments}}
  {{range $line, $comms := $lines}}
@@ -610,10 +614,12 @@
  <span class="text grey muted-links">
  {{template "shared/user/authorlink" .Poster}}
  {{$reviewerName := ""}}
- {{if eq .Review.OriginalAuthor ""}}
- {{$reviewerName = .Review.Reviewer.Name}}
- {{else}}
- {{$reviewerName = .Review.OriginalAuthor}}
+ {{if .Review}}
+ {{if eq .Review.OriginalAuthor ""}}
+ {{$reviewerName = .Review.Reviewer.Name}}
+ {{else}}
+ {{$reviewerName = .Review.OriginalAuthor}}
+ {{end}}
  {{end}}
  <span class="dismissed-message">{{ctx.Locale.Tr "repo.issues.review.dismissed" ($reviewerName | Escape) $createdStr | Safe}}</span>
  </span>