From 828701ff2de67e179e79c79e6b52e92e770df789 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Tue, 19 Mar 2024 12:19:48 +0800 Subject: [PATCH] Fix template error when comment review doesn't exist (#29888) Fix #29885 --- models/fixtures/comment.yml | 8 + models/fixtures/review.yml | 9 + routers/web/repo/pull_review_test.go | 17 ++ templates/repo/diff/comments.tmpl | 2 +- templates/repo/diff/conversation.tmpl | 130 ++++----- .../repo/issue/view_content/comments.tmpl | 28 +- .../repo/issue/view_content/conversation.tmpl | 258 +++++++++--------- tests/integration/pull_review_test.go | 14 +- 8 files changed, 264 insertions(+), 202 deletions(-) diff --git a/models/fixtures/comment.yml b/models/fixtures/comment.yml index 17586caa2..74fc71618 100644 --- a/models/fixtures/comment.yml +++ b/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 diff --git a/models/fixtures/review.yml b/models/fixtures/review.yml index 7a8808006..ac97e24c2 100644 --- a/models/fixtures/review.yml +++ b/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 diff --git a/routers/web/repo/pull_review_test.go b/routers/web/repo/pull_review_test.go index 5f035f1eb..8344ff409 100644 --- a/routers/web/repo/pull_review_test.go +++ b/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(), `
{{end}} - {{if and .Review}} + {{if .Review}} {{if eq .Review.Type 0}}
{{ctx.Locale.Tr "repo.issues.review.pending"}} diff --git a/templates/repo/diff/conversation.tmpl b/templates/repo/diff/conversation.tmpl index aaeac3c55..507f17fd9 100644 --- a/templates/repo/diff/conversation.tmpl +++ b/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}} -
- {{if $resolved}} -
-
- {{svg "octicon-check" 16 "icon gt-mr-2"}} - {{$resolveDoer.Name}} {{ctx.Locale.Tr "repo.issues.review.resolved_by"}} - {{if $invalid}} - - - {{ctx.Locale.Tr "repo.issues.review.outdated"}} - +{{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}} +
+ {{if $resolved}} +
+
+ {{svg "octicon-check" 16 "icon gt-mr-2"}} + {{$resolveDoer.Name}} {{ctx.Locale.Tr "repo.issues.review.resolved_by"}} + {{if $invalid}} + + + {{ctx.Locale.Tr "repo.issues.review.outdated"}} + + {{end}} +
+
+ + +
+
+ {{end}} +
+
+ + {{template "repo/diff/comments" dict "root" $ "comments" .comments}} + +
+
+
+ + +
+ {{if and $.CanMarkConversation $hasReview (not $isReviewPending)}} + + {{end}} + {{if and $.SignedUserID (not $.Repository.IsArchived)}} + {{end}}
-
- - -
+ {{template "repo/diff/comment_form_datahandler" dict "hidden" true "reply" $comment.ReviewID "root" $ "comment" $comment}}
- {{end}} -
-
- - {{template "repo/diff/comments" dict "root" $ "comments" .comments}} - -
-
-
- - -
- {{if and $.CanMarkConversation $isNotPending}} - - {{end}} - {{if and $.SignedUserID (not $.Repository.IsArchived)}} - - {{end}} -
- {{template "repo/diff/comment_form_datahandler" dict "hidden" true "reply" (index .comments 0).ReviewID "root" $ "comment" (index .comments 0)}}
-
+{{else}} + {{template "repo/diff/conversation_outdated"}} +{{end}} diff --git a/templates/repo/issue/view_content/comments.tmpl b/templates/repo/issue/view_content/comments.tmpl index a9c6bbe31..c9170d974 100644 --- a/templates/repo/issue/view_content/comments.tmpl +++ b/templates/repo/issue/view_content/comments.tmpl @@ -371,27 +371,31 @@ {{else if eq .Type 22}}
+ {{$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 */}} - + {{ctx.AvatarUtils.Avatar .Poster 40}} {{end}} - {{svg (printf "octicon-%s" .Review.Type.Icon)}} + + {{if .Review}}{{svg (printf "octicon-%s" .Review.Type.Icon)}}{{end}} + {{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}}
{{ctx.Locale.Tr "repo.issues.review.dismissed_label"}}
{{end}} @@ -451,7 +455,7 @@
{{end}} - {{if .Review.CodeComments}} + {{if and .Review .Review.CodeComments}}
{{range $filename, $lines := .Review.CodeComments}} {{range $line, $comms := $lines}} @@ -607,10 +611,12 @@ {{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}} diff --git a/templates/repo/issue/view_content/conversation.tmpl b/templates/repo/issue/view_content/conversation.tmpl index b6e075d0c..d93589539 100644 --- a/templates/repo/issue/view_content/conversation.tmpl +++ b/templates/repo/issue/view_content/conversation.tmpl @@ -1,137 +1,143 @@ -{{$invalid := (index .comments 0).Invalidated}} -{{$resolved := (index .comments 0).IsResolved}} -{{$resolveDoer := (index .comments 0).ResolveDoer}} -{{$isNotPending := (not (eq (index .comments 0).Review.Type 0))}} -
-
-
- {{(index .comments 0).TreePath}} - {{if $invalid}} - - {{ctx.Locale.Tr "repo.issues.review.outdated"}} - - {{end}} -
-
- {{if or $invalid $resolved}} - - - {{end}} -
-
- {{$diff := (CommentMustAsDiff ctx (index .comments 0))}} - {{if $diff}} - {{$file := (index $diff.Files 0)}} -
-
-
- - - {{template "repo/diff/section_unified" dict "file" $file "root" $}} - -
-
-
-
- {{end}} -
-
- {{range .comments}} - {{$createdSubStr:= TimeSinceUnix .CreatedUnix ctx.Locale}} -
-
-
-
- {{if not .OriginalAuthor}} - - {{ctx.AvatarUtils.Avatar .Poster 20}} - - {{end}} - - {{if .OriginalAuthor}} - - {{svg (MigrationIcon $.Repository.GetOriginalURLHostname)}} - {{.OriginalAuthor}} - - {{if $.Repository.OriginalURL}} - ({{ctx.Locale.Tr "repo.migrated_from" $.Repository.OriginalURL $.Repository.GetOriginalURLHostname}}) - {{end}} - {{else}} - {{template "shared/user/authorlink" .Poster}} - {{end}} - {{ctx.Locale.Tr "repo.issues.commented_at" .HashTag $createdSubStr}} - -
-
- {{template "repo/issue/view_content/show_role" dict "ShowRole" .ShowRole}} - {{if not $.Repository.IsArchived}} - {{template "repo/issue/view_content/add_reaction" dict "ctxData" $ "ActionURL" (printf "%s/comments/%d/reactions" $.RepoLink .ID)}} - {{template "repo/issue/view_content/context_menu" dict "ctxData" $ "item" . "delete" true "issue" true "diff" true "IsCommentPoster" (and $.IsSigned (eq $.SignedUserID .PosterID))}} - {{end}} -
-
-
-
- {{if .RenderedContent}} - {{.RenderedContent}} - {{else}} - {{ctx.Locale.Tr "repo.issues.no_content"}} - {{end}} -
-
{{.Content}}
-
- {{if .Attachments}} - {{template "repo/issue/view_content/attachments" dict "Attachments" .Attachments "RenderedContent" .RenderedContent}} - {{end}} -
- {{$reactions := .Reactions.GroupByType}} - {{if $reactions}} - {{template "repo/issue/view_content/reactions" dict "ctxData" $ "ActionURL" (printf "%s/comments/%d/reactions" $.RepoLink .ID) "Reactions" $reactions}} - {{end}} -
-
- {{end}} -
-
-
- {{if $resolved}} -
- {{svg "octicon-check" 16 "gt-mr-2"}} - {{$resolveDoer.Name}} {{ctx.Locale.Tr "repo.issues.review.resolved_by"}} -
+{{if len .comments}} + {{$comment := index .comments 0}} + {{$invalid := $comment.Invalidated}} + {{$resolved := $comment.IsResolved}} + {{$resolveDoer := $comment.ResolveDoer}} + {{$hasReview := and $comment.Review}} + {{$isReviewPending := and $hasReview (eq $comment.Review.Type 0)}} +
+
+
+ {{$comment.TreePath}} + {{if $invalid}} + + {{ctx.Locale.Tr "repo.issues.review.outdated"}} + {{end}}
-
- {{if and $.CanMarkConversation $isNotPending}} - - {{end}} - {{if and $.SignedUserID (not $.Repository.IsArchived)}} - {{end}}
- {{template "repo/diff/comment_form_datahandler" dict "hidden" true "reply" (index .comments 0).ReviewID "root" $ "comment" (index .comments 0)}} + {{$diff := (CommentMustAsDiff ctx $comment)}} + {{if $diff}} + {{$file := (index $diff.Files 0)}} +
+
+
+ + + {{template "repo/diff/section_unified" dict "file" $file "root" $}} + +
+
+
+
+ {{end}} +
+
+ {{range .comments}} + {{$createdSubStr:= TimeSinceUnix .CreatedUnix ctx.Locale}} +
+
+
+
+ {{if not .OriginalAuthor}} + + {{ctx.AvatarUtils.Avatar .Poster 20}} + + {{end}} + + {{if .OriginalAuthor}} + + {{svg (MigrationIcon $.Repository.GetOriginalURLHostname)}} + {{.OriginalAuthor}} + + {{if $.Repository.OriginalURL}} + ({{ctx.Locale.Tr "repo.migrated_from" $.Repository.OriginalURL $.Repository.GetOriginalURLHostname}}) + {{end}} + {{else}} + {{template "shared/user/authorlink" .Poster}} + {{end}} + {{ctx.Locale.Tr "repo.issues.commented_at" .HashTag $createdSubStr}} + +
+
+ {{template "repo/issue/view_content/show_role" dict "ShowRole" .ShowRole}} + {{if not $.Repository.IsArchived}} + {{template "repo/issue/view_content/add_reaction" dict "ctxData" $ "ActionURL" (printf "%s/comments/%d/reactions" $.RepoLink .ID)}} + {{template "repo/issue/view_content/context_menu" dict "ctxData" $ "item" . "delete" true "issue" true "diff" true "IsCommentPoster" (and $.IsSigned (eq $.SignedUserID .PosterID))}} + {{end}} +
+
+
+
+ {{if .RenderedContent}} + {{.RenderedContent}} + {{else}} + {{ctx.Locale.Tr "repo.issues.no_content"}} + {{end}} +
+
{{.Content}}
+
+ {{if .Attachments}} + {{template "repo/issue/view_content/attachments" dict "Attachments" .Attachments "RenderedContent" .RenderedContent}} + {{end}} +
+ {{$reactions := .Reactions.GroupByType}} + {{if $reactions}} + {{template "repo/issue/view_content/reactions" dict "ctxData" $ "ActionURL" (printf "%s/comments/%d/reactions" $.RepoLink .ID) "Reactions" $reactions}} + {{end}} +
+
+ {{end}} +
+
+
+ {{if $resolved}} +
+ {{svg "octicon-check" 16 "gt-mr-2"}} + {{$resolveDoer.Name}} {{ctx.Locale.Tr "repo.issues.review.resolved_by"}} +
+ {{end}} +
+
+ {{if and $.CanMarkConversation $hasReview (not $isReviewPending)}} + + {{end}} + {{if and $.SignedUserID (not $.Repository.IsArchived)}} + + {{end}} +
+
+ {{template "repo/diff/comment_form_datahandler" dict "hidden" true "reply" $comment.ReviewID "root" $ "comment" $comment}} +
-
+{{else}} + {{template "repo/diff/conversation_outdated"}} +{{end}} diff --git a/tests/integration/pull_review_test.go b/tests/integration/pull_review_test.go index 459aa5a58..bdfecb328 100644 --- a/tests/integration/pull_review_test.go +++ b/tests/integration/pull_review_test.go @@ -14,6 +14,7 @@ import ( "code.gitea.io/gitea/models/unittest" user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/git" + "code.gitea.io/gitea/modules/test" repo_service "code.gitea.io/gitea/services/repository" files_service "code.gitea.io/gitea/services/repository/files" "code.gitea.io/gitea/tests" @@ -26,10 +27,19 @@ func TestPullView_ReviewerMissed(t *testing.T) { session := loginUser(t, "user1") 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") - 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) {