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

Backport #29888
This commit is contained in:
wxiaoguang 2024-03-19 15:00:01 +08:00 committed by GitHub
parent 440be51a45
commit b9dd5dd471
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
8 changed files with 260 additions and 198 deletions

View File

@ -75,3 +75,11 @@
content: "comment in private pository" content: "comment in private pository"
created_unix: 946684811 created_unix: 946684811
updated_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

View File

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

View File

@ -4,6 +4,7 @@
package repo package repo
import ( import (
"net/http"
"net/http/httptest" "net/http/httptest"
"testing" "testing"
@ -73,4 +74,20 @@ func TestRenderConversation(t *testing.T) {
renderConversation(ctx, preparedComment, "timeline") renderConversation(ctx, preparedComment, "timeline")
assert.Contains(t, resp.Body.String(), `<div id="code-comments-`) 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`)
})
} }

View File

@ -37,7 +37,7 @@
{{ctx.Locale.Tr "repo.issues.review.outdated"}} {{ctx.Locale.Tr "repo.issues.review.outdated"}}
</a> </a>
{{end}} {{end}}
{{if and .Review}} {{if .Review}}
{{if eq .Review.Type 0}} {{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")}}"> <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"}} {{ctx.Locale.Tr "repo.issues.review.pending"}}

View File

@ -1,9 +1,12 @@
{{$resolved := (index .comments 0).IsResolved}} {{if len .comments}}
{{$invalid := (index .comments 0).Invalidated}} {{$comment := index .comments 0}}
{{$resolveDoer := (index .comments 0).ResolveDoer}} {{$resolved := $comment.IsResolved}}
{{$isNotPending := (not (eq (index .comments 0).Review.Type 0))}} {{$invalid := $comment.Invalidated}}
{{$referenceUrl := printf "%s#%s" $.Issue.Link (index .comments 0).HashTag}} {{$resolveDoer := $comment.ResolveDoer}}
<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}}"> {{$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}} {{if $resolved}}
<div class="ui attached header resolved-placeholder gt-df gt-ac gt-sb"> <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"> <div class="ui grey text gt-df gt-ac gt-fw gt-gap-2">
@ -20,18 +23,18 @@
{{end}} {{end}}
</div> </div>
<div class="gt-df gt-ac gt-gap-3"> <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"> <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"}} {{svg "octicon-unfold" 16 "gt-mr-3"}}
{{ctx.Locale.Tr "repo.issues.review.show_resolved"}} {{ctx.Locale.Tr "repo.issues.review.show_resolved"}}
</button> </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"> <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"}} {{svg "octicon-fold" 16 "gt-mr-3"}}
{{ctx.Locale.Tr "repo.issues.review.hide_resolved"}} {{ctx.Locale.Tr "repo.issues.review.hide_resolved"}}
</button> </button>
</div> </div>
</div> </div>
{{end}} {{end}}
<div id="code-comments-{{(index .comments 0).ID}}" class="field comment-code-cloud {{if $resolved}}gt-hidden{{end}}"> <div id="code-comments-{{$comment.ID}}" class="field comment-code-cloud {{if $resolved}}gt-hidden{{end}}">
<div class="comment-list"> <div class="comment-list">
<ui class="ui comments"> <ui class="ui comments">
{{template "repo/diff/comments" dict "root" $ "comments" .comments}} {{template "repo/diff/comments" dict "root" $ "comments" .comments}}
@ -46,8 +49,8 @@
{{svg "octicon-arrow-down" 12 "icon"}} {{ctx.Locale.Tr "repo.issues.next"}} {{svg "octicon-arrow-down" 12 "icon"}} {{ctx.Locale.Tr "repo.issues.next"}}
</button> </button>
</div> </div>
{{if and $.CanMarkConversation $isNotPending}} {{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="{{(index .comments 0).ID}}" data-update-url="{{$.RepoLink}}/issues/resolve_conversation"> <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}} {{if $resolved}}
{{ctx.Locale.Tr "repo.issues.review.un_resolve_conversation"}} {{ctx.Locale.Tr "repo.issues.review.un_resolve_conversation"}}
{{else}} {{else}}
@ -61,6 +64,9 @@
</button> </button>
{{end}} {{end}}
</div> </div>
{{template "repo/diff/comment_form_datahandler" dict "hidden" true "reply" (index .comments 0).ReviewID "root" $ "comment" (index .comments 0)}} {{template "repo/diff/comment_form_datahandler" dict "hidden" true "reply" $comment.ReviewID "root" $ "comment" $comment}}
</div> </div>
</div> </div>
{{else}}
{{template "repo/diff/conversation_outdated"}}
{{end}}

View File

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

View File

@ -1,11 +1,14 @@
{{$invalid := (index .comments 0).Invalidated}} {{if len .comments}}
{{$resolved := (index .comments 0).IsResolved}} {{$comment := index .comments 0}}
{{$resolveDoer := (index .comments 0).ResolveDoer}} {{$invalid := $comment.Invalidated}}
{{$isNotPending := (not (eq (index .comments 0).Review.Type 0))}} {{$resolved := $comment.IsResolved}}
{{$resolveDoer := $comment.ResolveDoer}}
{{$hasReview := and $comment.Review}}
{{$isReviewPending := and $hasReview (eq $comment.Review.Type 0)}}
<div class="ui segments conversation-holder"> <div class="ui segments conversation-holder">
<div class="ui segment collapsible-comment-box gt-py-3 gt-df gt-ac gt-sb"> <div class="ui segment collapsible-comment-box gt-py-3 gt-df gt-ac gt-sb">
<div class="gt-df gt-ac"> <div class="gt-df gt-ac">
<a href="{{(index .comments 0).CodeCommentLink ctx}}" class="file-comment gt-ml-3 gt-word-break">{{(index .comments 0).TreePath}}</a> <a href="{{$comment.CodeCommentLink ctx}}" class="file-comment gt-ml-3 gt-word-break">{{$comment.TreePath}}</a>
{{if $invalid}} {{if $invalid}}
<span class="ui label basic small gt-ml-3" data-tooltip-content="{{ctx.Locale.Tr "repo.issues.review.outdated_description"}}"> <span 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"}} {{ctx.Locale.Tr "repo.issues.review.outdated"}}
@ -14,7 +17,7 @@
</div> </div>
<div> <div>
{{if or $invalid $resolved}} {{if or $invalid $resolved}}
<button id="show-outdated-{{(index .comments 0).ID}}" data-comment="{{(index .comments 0).ID}}" class="{{if not $resolved}}gt-hidden {{end}}ui compact labeled button show-outdated gt-df gt-ac"> <button id="show-outdated-{{$comment.ID}}" data-comment="{{$comment.ID}}" class="{{if not $resolved}}gt-hidden {{end}}ui compact labeled button show-outdated gt-df gt-ac">
{{svg "octicon-unfold" 16 "gt-mr-3"}} {{svg "octicon-unfold" 16 "gt-mr-3"}}
{{if $resolved}} {{if $resolved}}
{{ctx.Locale.Tr "repo.issues.review.show_resolved"}} {{ctx.Locale.Tr "repo.issues.review.show_resolved"}}
@ -22,7 +25,7 @@
{{ctx.Locale.Tr "repo.issues.review.show_outdated"}} {{ctx.Locale.Tr "repo.issues.review.show_outdated"}}
{{end}} {{end}}
</button> </button>
<button id="hide-outdated-{{(index .comments 0).ID}}" data-comment="{{(index .comments 0).ID}}" class="{{if $resolved}}gt-hidden {{end}}ui compact labeled button hide-outdated gt-df gt-ac"> <button id="hide-outdated-{{$comment.ID}}" data-comment="{{$comment.ID}}" class="{{if $resolved}}gt-hidden {{end}}ui compact labeled button hide-outdated gt-df gt-ac">
{{svg "octicon-fold" 16 "gt-mr-3"}} {{svg "octicon-fold" 16 "gt-mr-3"}}
{{if $resolved}} {{if $resolved}}
{{ctx.Locale.Tr "repo.issues.review.hide_resolved"}} {{ctx.Locale.Tr "repo.issues.review.hide_resolved"}}
@ -33,10 +36,10 @@
{{end}} {{end}}
</div> </div>
</div> </div>
{{$diff := (CommentMustAsDiff (index .comments 0))}} {{$diff := (CommentMustAsDiff $comment)}}
{{if $diff}} {{if $diff}}
{{$file := (index $diff.Files 0)}} {{$file := (index $diff.Files 0)}}
<div id="code-preview-{{(index .comments 0).ID}}" class="ui table segment{{if $resolved}} gt-hidden{{end}}"> <div id="code-preview-{{$comment.ID}}" class="ui table segment{{if $resolved}} gt-hidden{{end}}">
<div class="diff-file-box diff-box file-content {{TabSizeClass $.Editorconfig $file.Name}}"> <div class="diff-file-box diff-box file-content {{TabSizeClass $.Editorconfig $file.Name}}">
<div class="file-body file-code code-view code-diff code-diff-unified unicode-escaped"> <div class="file-body file-code code-view code-diff code-diff-unified unicode-escaped">
<table> <table>
@ -48,7 +51,7 @@
</div> </div>
</div> </div>
{{end}} {{end}}
<div id="code-comments-{{(index .comments 0).ID}}" class="comment-code-cloud ui segment{{if $resolved}} gt-hidden{{end}}"> <div id="code-comments-{{$comment.ID}}" class="comment-code-cloud ui segment{{if $resolved}} gt-hidden{{end}}">
<div class="ui comments gt-mb-0"> <div class="ui comments gt-mb-0">
{{range .comments}} {{range .comments}}
{{$createdSubStr:= TimeSinceUnix .CreatedUnix ctx.Locale}} {{$createdSubStr:= TimeSinceUnix .CreatedUnix ctx.Locale}}
@ -112,8 +115,8 @@
{{end}} {{end}}
</div> </div>
<div class="code-comment-buttons-buttons"> <div class="code-comment-buttons-buttons">
{{if and $.CanMarkConversation $isNotPending}} {{if and $.CanMarkConversation $hasReview (not $isReviewPending)}}
<button class="ui tiny basic button resolve-conversation" data-origin="timeline" data-action="{{if not $resolved}}Resolve{{else}}UnResolve{{end}}" data-comment-id="{{(index .comments 0).ID}}" data-update-url="{{$.RepoLink}}/issues/resolve_conversation"> <button class="ui tiny basic button resolve-conversation" data-origin="timeline" data-action="{{if not $resolved}}Resolve{{else}}UnResolve{{end}}" data-comment-id="{{$comment.ID}}" data-update-url="{{$.RepoLink}}/issues/resolve_conversation">
{{if $resolved}} {{if $resolved}}
{{ctx.Locale.Tr "repo.issues.review.un_resolve_conversation"}} {{ctx.Locale.Tr "repo.issues.review.un_resolve_conversation"}}
{{else}} {{else}}
@ -128,6 +131,9 @@
{{end}} {{end}}
</div> </div>
</div> </div>
{{template "repo/diff/comment_form_datahandler" dict "hidden" true "reply" (index .comments 0).ReviewID "root" $ "comment" (index .comments 0)}} {{template "repo/diff/comment_form_datahandler" dict "hidden" true "reply" $comment.ReviewID "root" $ "comment" $comment}}
</div> </div>
</div> </div>
{{else}}
{{template "repo/diff/conversation_outdated"}}
{{end}}

View File

@ -13,6 +13,7 @@ import (
issues_model "code.gitea.io/gitea/models/issues" issues_model "code.gitea.io/gitea/models/issues"
"code.gitea.io/gitea/models/unittest" "code.gitea.io/gitea/models/unittest"
user_model "code.gitea.io/gitea/models/user" user_model "code.gitea.io/gitea/models/user"
"code.gitea.io/gitea/modules/test"
repo_service "code.gitea.io/gitea/services/repository" repo_service "code.gitea.io/gitea/services/repository"
files_service "code.gitea.io/gitea/services/repository/files" files_service "code.gitea.io/gitea/services/repository/files"
"code.gitea.io/gitea/tests" "code.gitea.io/gitea/tests"
@ -25,10 +26,19 @@ func TestPullView_ReviewerMissed(t *testing.T) {
session := loginUser(t, "user1") session := loginUser(t, "user1")
req := NewRequest(t, "GET", "/pulls") req := NewRequest(t, "GET", "/pulls")
session.MakeRequest(t, req, http.StatusOK) resp := session.MakeRequest(t, req, http.StatusOK)
assert.True(t, test.IsNormalPageCompleted(resp.Body.String()))
req = NewRequest(t, "GET", "/user2/repo1/pulls/3") req = NewRequest(t, "GET", "/user2/repo1/pulls/3")
session.MakeRequest(t, req, http.StatusOK) resp = session.MakeRequest(t, req, http.StatusOK)
assert.True(t, test.IsNormalPageCompleted(resp.Body.String()))
// if some reviews are missing, the page shouldn't fail
err := db.TruncateBeans(db.DefaultContext, &issues_model.Review{})
assert.NoError(t, err)
req = NewRequest(t, "GET", "/user2/repo1/pulls/2")
resp = session.MakeRequest(t, req, http.StatusOK)
assert.True(t, test.IsNormalPageCompleted(resp.Body.String()))
} }
func TestPullView_CodeOwner(t *testing.T) { func TestPullView_CodeOwner(t *testing.T) {