Skip to content

Commit

Permalink
Fix template error when comment review doesn't exist (#29888)
Browse files Browse the repository at this point in the history
Fix #29885
  • Loading branch information
wxiaoguang authored Mar 19, 2024
1 parent 0e183d8 commit 828701f
Show file tree
Hide file tree
Showing 8 changed files with 258 additions and 196 deletions.
8 changes: 8 additions & 0 deletions models/fixtures/comment.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
9 changes: 9 additions & 0 deletions models/fixtures/review.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
17 changes: 17 additions & 0 deletions routers/web/repo/pull_review_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
package repo

import (
"net/http"
"net/http/httptest"
"testing"

Expand Down Expand Up @@ -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`)
})
}
2 changes: 1 addition & 1 deletion templates/repo/diff/comments.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -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"}}
Expand Down
128 changes: 67 additions & 61 deletions templates/repo/diff/conversation.tmpl
Original file line number Diff line number Diff line change
@@ -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}}
28 changes: 17 additions & 11 deletions templates/repo/issue/view_content/comments.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -371,27 +371,31 @@
{{else if eq .Type 22}}
<div class="timeline-item-group" id="{{.HashTag}}">
<div class="timeline-item event">
{{$reviewType := -1}}
{{if .Review}}{{$reviewType = .Review.Type}}{{end}}
{{if not .OriginalAuthor}}
{{/* 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}} tw-bg-green tw-text-white{{else if eq .Review.Type 3}} tw-bg-red tw-text-white{{end}}">{{svg (printf "octicon-%s" .Review.Type.Icon)}}</span>
<span class="badge{{if eq $reviewType 1}} tw-bg-green tw-text-white{{else if eq $reviewType 3}} tw-bg-red tw-text-white{{end}}">
{{if .Review}}{{svg (printf "octicon-%s" .Review.Type.Icon)}}{{end}}
</span>
<span class="text grey muted-links">
{{template "repo/issue/view_content/comments_authorlink" dict "ctxData" $ "comment" .}}
{{if eq .Review.Type 1}}
{{if eq $reviewType 1}}
{{ctx.Locale.Tr "repo.issues.review.approve" $createdStr}}
{{else if eq .Review.Type 2}}
{{else if eq $reviewType 2}}
{{ctx.Locale.Tr "repo.issues.review.comment" $createdStr}}
{{else if eq .Review.Type 3}}
{{else if eq $reviewType 3}}
{{ctx.Locale.Tr "repo.issues.review.reject" $createdStr}}
{{else}}
{{ctx.Locale.Tr "repo.issues.review.comment" $createdStr}}
{{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>
Expand Down Expand Up @@ -451,7 +455,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}}
Expand Down Expand Up @@ -607,10 +611,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}}
{{ctx.Locale.Tr "repo.issues.review.dismissed" $reviewerName $createdStr}}
</span>
Expand Down
Loading

0 comments on commit 828701f

Please sign in to comment.