Skip to content

Commit b9dd5dd

Browse files
wxiaoguangwxiaoguang
authored
Fix template error when comment review doesn't exist (#29888) (#29889)
Backport #29888
1 parent 440be51 commit b9dd5dd

File tree

8 files changed

+255
-193
lines changed

8 files changed

+255
-193
lines changed

‎models/fixtures/comment.yml

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,3 +75,11 @@
7575
content: "comment in private pository"
7676
created_unix: 946684811
7777
updated_unix: 946684811
78+
79+
-
80+
id: 9
81+
type: 22 # review
82+
poster_id: 2
83+
issue_id: 2 # in repo_id 1
84+
review_id: 20
85+
created_unix: 946684810

‎models/fixtures/review.yml

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -170,3 +170,12 @@
170170
content: "review request for user15"
171171
updated_unix: 946684835
172172
created_unix: 946684835
173+
174+
-
175+
id: 20
176+
type: 22
177+
reviewer_id: 1
178+
issue_id: 2
179+
content: "Review Comment"
180+
updated_unix: 946684810
181+
created_unix: 946684810

‎routers/web/repo/pull_review_test.go

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
package repo
55

66
import (
7+
"net/http"
78
"net/http/httptest"
89
"testing"
910

@@ -73,4 +74,20 @@ func TestRenderConversation(t *testing.T) {
7374
renderConversation(ctx, preparedComment, "timeline")
7475
assert.Contains(t, resp.Body.String(), `<div id="code-comments-`)
7576
})
77+
run("diff non-existing review", func(t *testing.T, ctx *context.Context, resp *httptest.ResponseRecorder) {
78+
err := db.TruncateBeans(db.DefaultContext, &issues_model.Review{})
79+
assert.NoError(t, err)
80+
ctx.Data["ShowOutdatedComments"] = true
81+
renderConversation(ctx, preparedComment, "diff")
82+
assert.Equal(t, http.StatusOK, resp.Code)
83+
assert.NotContains(t, resp.Body.String(), `status-page-500`)
84+
})
85+
run("timeline non-existing review", func(t *testing.T, ctx *context.Context, resp *httptest.ResponseRecorder) {
86+
err := db.TruncateBeans(db.DefaultContext, &issues_model.Review{})
87+
assert.NoError(t, err)
88+
ctx.Data["ShowOutdatedComments"] = true
89+
renderConversation(ctx, preparedComment, "timeline")
90+
assert.Equal(t, http.StatusOK, resp.Code)
91+
assert.NotContains(t, resp.Body.String(), `status-page-500`)
92+
})
7693
}

‎templates/repo/diff/comments.tmpl

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@
3737
{{ctx.Locale.Tr "repo.issues.review.outdated"}}
3838
</a>
3939
{{end}}
40-
{{if and .Review}}
40+
{{if .Review}}
4141
{{if eq .Review.Type 0}}
4242
<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")}}">
4343
{{ctx.Locale.Tr "repo.issues.review.pending"}}

‎templates/repo/diff/conversation.tmpl

Lines changed: 67 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -1,66 +1,72 @@
1-
{{$resolved := (index .comments 0).IsResolved}}
2-
{{$invalid := (index .comments 0).Invalidated}}
3-
{{$resolveDoer := (index .comments 0).ResolveDoer}}
4-
{{$isNotPending := (not (eq (index .comments 0).Review.Type 0))}}
5-
{{$referenceUrl := printf "%s#%s" $.Issue.Link (index .comments 0).HashTag}}
6-
<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}}">
7-
{{if $resolved}}
8-
<div class="ui attached header resolved-placeholder gt-df gt-ac gt-sb">
9-
<div class="ui grey text gt-df gt-ac gt-fw gt-gap-2">
10-
{{svg "octicon-check" 16 "icon gt-mr-2"}}
11-
<b>{{$resolveDoer.Name}}</b> {{ctx.Locale.Tr "repo.issues.review.resolved_by"}}
12-
{{if $invalid}}
13-
<!--
14-
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).
15-
The case $resolved=false and $invalid=true is handled in repo/diff/comments.tmpl
16-
-->
17-
<a href="{{$referenceUrl}}" class="ui label basic small gt-ml-3" data-tooltip-content="{{ctx.Locale.Tr "repo.issues.review.outdated_description"}}">
18-
{{ctx.Locale.Tr "repo.issues.review.outdated"}}
19-
</a>
20-
{{end}}
1+
{{if len .comments}}
2+
{{$comment := index .comments 0}}
3+
{{$resolved := $comment.IsResolved}}
4+
{{$invalid := $comment.Invalidated}}
5+
{{$resolveDoer := $comment.ResolveDoer}}
6+
{{$hasReview := and $comment.Review}}
7+
{{$isReviewPending := and $hasReview (eq $comment.Review.Type 0)}}
8+
{{$referenceUrl := printf "%s#%s" $.Issue.Link $comment.HashTag}}
9+
<div class="conversation-holder" data-path="{{$comment.TreePath}}" data-side="{{if lt $comment.Line 0}}left{{else}}right{{end}}" data-idx="{{$comment.UnsignedLine}}">
10+
{{if $resolved}}
11+
<div class="ui attached header resolved-placeholder gt-df gt-ac gt-sb">
12+
<div class="ui grey text gt-df gt-ac gt-fw gt-gap-2">
13+
{{svg "octicon-check" 16 "icon gt-mr-2"}}
14+
<b>{{$resolveDoer.Name}}</b> {{ctx.Locale.Tr "repo.issues.review.resolved_by"}}
15+
{{if $invalid}}
16+
<!--
17+
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).
18+
The case $resolved=false and $invalid=true is handled in repo/diff/comments.tmpl
19+
-->
20+
<a href="{{$referenceUrl}}" class="ui label basic small gt-ml-3" data-tooltip-content="{{ctx.Locale.Tr "repo.issues.review.outdated_description"}}">
21+
{{ctx.Locale.Tr "repo.issues.review.outdated"}}
22+
</a>
23+
{{end}}
24+
</div>
25+
<div class="gt-df gt-ac gt-gap-3">
26+
<button id="show-outdated-{{$comment.ID}}" data-comment="{{$comment.ID}}" class="ui tiny labeled button show-outdated gt-df gt-ac">
27+
{{svg "octicon-unfold" 16 "gt-mr-3"}}
28+
{{ctx.Locale.Tr "repo.issues.review.show_resolved"}}
29+
</button>
30+
<button id="hide-outdated-{{$comment.ID}}" data-comment="{{$comment.ID}}" class="ui tiny labeled button hide-outdated gt-df gt-ac gt-hidden">
31+
{{svg "octicon-fold" 16 "gt-mr-3"}}
32+
{{ctx.Locale.Tr "repo.issues.review.hide_resolved"}}
33+
</button>
34+
</div>
2135
</div>
22-
<div class="gt-df gt-ac gt-gap-3">
23-
<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">
24-
{{svg "octicon-unfold" 16 "gt-mr-3"}}
25-
{{ctx.Locale.Tr "repo.issues.review.show_resolved"}}
26-
</button>
27-
<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">
28-
{{svg "octicon-fold" 16 "gt-mr-3"}}
29-
{{ctx.Locale.Tr "repo.issues.review.hide_resolved"}}
30-
</button>
36+
{{end}}
37+
<div id="code-comments-{{$comment.ID}}" class="field comment-code-cloud {{if $resolved}}gt-hidden{{end}}">
38+
<div class="comment-list">
39+
<ui class="ui comments">
40+
{{template "repo/diff/comments" dict "root" $ "comments" .comments}}
41+
</ui>
3142
</div>
32-
</div>
33-
{{end}}
34-
<div id="code-comments-{{(index .comments 0).ID}}" class="field comment-code-cloud {{if $resolved}}gt-hidden{{end}}">
35-
<div class="comment-list">
36-
<ui class="ui comments">
37-
{{template "repo/diff/comments" dict "root" $ "comments" .comments}}
38-
</ui>
39-
</div>
40-
<div class="gt-df gt-je gt-ac gt-fw gt-mt-3">
41-
<div class="ui buttons gt-mr-2">
42-
<button class="ui icon tiny basic button previous-conversation">
43-
{{svg "octicon-arrow-up" 12 "icon"}} {{ctx.Locale.Tr "repo.issues.previous"}}
44-
</button>
45-
<button class="ui icon tiny basic button next-conversation">
46-
{{svg "octicon-arrow-down" 12 "icon"}} {{ctx.Locale.Tr "repo.issues.next"}}
47-
</button>
43+
<div class="gt-df gt-je gt-ac gt-fw gt-mt-3">
44+
<div class="ui buttons gt-mr-2">
45+
<button class="ui icon tiny basic button previous-conversation">
46+
{{svg "octicon-arrow-up" 12 "icon"}} {{ctx.Locale.Tr "repo.issues.previous"}}
47+
</button>
48+
<button class="ui icon tiny basic button next-conversation">
49+
{{svg "octicon-arrow-down" 12 "icon"}} {{ctx.Locale.Tr "repo.issues.next"}}
50+
</button>
51+
</div>
52+
{{if and $.CanMarkConversation $hasReview (not $isReviewPending)}}
53+
<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">
54+
{{if $resolved}}
55+
{{ctx.Locale.Tr "repo.issues.review.un_resolve_conversation"}}
56+
{{else}}
57+
{{ctx.Locale.Tr "repo.issues.review.resolve_conversation"}}
58+
{{end}}
59+
</button>
60+
{{end}}
61+
{{if and $.SignedUserID (not $.Repository.IsArchived)}}
62+
<button class="comment-form-reply ui primary tiny labeled icon button gt-ml-2 gt-mr-0">
63+
{{svg "octicon-reply" 16 "reply icon gt-mr-2"}}{{ctx.Locale.Tr "repo.diff.comment.reply"}}
64+
</button>
65+
{{end}}
4866
</div>
49-
{{if and $.CanMarkConversation $isNotPending}}
50-
<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">
51-
{{if $resolved}}
52-
{{ctx.Locale.Tr "repo.issues.review.un_resolve_conversation"}}
53-
{{else}}
54-
{{ctx.Locale.Tr "repo.issues.review.resolve_conversation"}}
55-
{{end}}
56-
</button>
57-
{{end}}
58-
{{if and $.SignedUserID (not $.Repository.IsArchived)}}
59-
<button class="comment-form-reply ui primary tiny labeled icon button gt-ml-2 gt-mr-0">
60-
{{svg "octicon-reply" 16 "reply icon gt-mr-2"}}{{ctx.Locale.Tr "repo.diff.comment.reply"}}
61-
</button>
62-
{{end}}
67+
{{template "repo/diff/comment_form_datahandler" dict "hidden" true "reply" $comment.ReviewID "root" $ "comment" $comment}}
6368
</div>
64-
{{template "repo/diff/comment_form_datahandler" dict "hidden" true "reply" (index .comments 0).ReviewID "root" $ "comment" (index .comments 0)}}
6569
</div>
66-
</div>
70+
{{else}}
71+
{{template "repo/diff/conversation_outdated"}}
72+
{{end}}

‎templates/repo/issue/view_content/comments.tmpl

Lines changed: 17 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -365,16 +365,20 @@
365365
{{else if eq .Type 22}}
366366
<div class="timeline-item-group">
367367
<div class="timeline-item event">
368+
{{$reviewType := -1}}
369+
{{if .Review}}{{$reviewType = .Review.Type}}{{end}}
368370
{{if .OriginalAuthor}}
369371
{{else}}
370372
{{/* Some timeline avatars need a offset to correctly align with their speech
371373
bubble. The condition depends on review type and for positive reviews whether
372374
there is a comment element or not */}}
373-
<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}}>
375+
<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}}>
374376
{{ctx.AvatarUtils.Avatar .Poster 40}}
375377
</a>
376378
{{end}}
377-
<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>
379+
<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}}">
380+
{{if .Review}}{{svg (printf "octicon-%s" .Review.Type.Icon)}}{{end}}
381+
</span>
378382
<span class="text grey muted-links">
379383
{{if .OriginalAuthor}}
380384
<span class="text black">
@@ -387,16 +391,16 @@
387391
{{template "shared/user/authorlink" .Poster}}
388392
{{end}}
389393

390-
{{if eq .Review.Type 1}}
394+
{{if eq $reviewType 1}}
391395
{{ctx.Locale.Tr "repo.issues.review.approve" $createdStr | Safe}}
392-
{{else if eq .Review.Type 2}}
396+
{{else if eq $reviewType 2}}
393397
{{ctx.Locale.Tr "repo.issues.review.comment" $createdStr | Safe}}
394-
{{else if eq .Review.Type 3}}
398+
{{else if eq $reviewType 3}}
395399
{{ctx.Locale.Tr "repo.issues.review.reject" $createdStr | Safe}}
396400
{{else}}
397401
{{ctx.Locale.Tr "repo.issues.review.comment" $createdStr | Safe}}
398402
{{end}}
399-
{{if .Review.Dismissed}}
403+
{{if and .Review .Review.Dismissed}}
400404
<div class="ui small label">{{ctx.Locale.Tr "repo.issues.review.dismissed_label"}}</div>
401405
{{end}}
402406
</span>
@@ -456,7 +460,7 @@
456460
</div>
457461
{{end}}
458462

459-
{{if .Review.CodeComments}}
463+
{{if and .Review .Review.CodeComments}}
460464
<div class="timeline-item event">
461465
{{range $filename, $lines := .Review.CodeComments}}
462466
{{range $line, $comms := $lines}}
@@ -610,10 +614,12 @@
610614
<span class="text grey muted-links">
611615
{{template "shared/user/authorlink" .Poster}}
612616
{{$reviewerName := ""}}
613-
{{if eq .Review.OriginalAuthor ""}}
614-
{{$reviewerName = .Review.Reviewer.Name}}
615-
{{else}}
616-
{{$reviewerName = .Review.OriginalAuthor}}
617+
{{if .Review}}
618+
{{if eq .Review.OriginalAuthor ""}}
619+
{{$reviewerName = .Review.Reviewer.Name}}
620+
{{else}}
621+
{{$reviewerName = .Review.OriginalAuthor}}
622+
{{end}}
617623
{{end}}
618624
<span class="dismissed-message">{{ctx.Locale.Tr "repo.issues.review.dismissed" ($reviewerName | Escape) $createdStr | Safe}}</span>
619625
</span>

0 commit comments

Comments
 (0)