Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 2 commits into from
Mar 19, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
add tests
  • Loading branch information
wxiaoguang committed Mar 18, 2024
commit 9ef1c7b8bfa12e85fae9fe8dc46f23a54faa2291
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`)
})
}
242 changes: 124 additions & 118 deletions templates/repo/issue/view_content/conversation.tmpl
Original file line number Diff line number Diff line change
@@ -1,133 +1,139 @@
{{$invalid := (index .comments 0).Invalidated}}
{{$resolved := (index .comments 0).IsResolved}}
{{$resolveDoer := (index .comments 0).ResolveDoer}}
{{$isNotPending := (not (eq (index .comments 0).Review.Type 0))}}
<div class="ui segments conversation-holder">
<div class="ui segment collapsible-comment-box gt-py-3 gt-df gt-ac gt-sb">
<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>
{{if $invalid}}
<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"}}
</span>
{{end}}
</div>
<div>
{{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">
{{svg "octicon-unfold" 16 "gt-mr-3"}}
{{if $resolved}}
{{ctx.Locale.Tr "repo.issues.review.show_resolved"}}
{{else}}
{{ctx.Locale.Tr "repo.issues.review.show_outdated"}}
{{end}}
</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">
{{svg "octicon-fold" 16 "gt-mr-3"}}
{{if $resolved}}
{{ctx.Locale.Tr "repo.issues.review.hide_resolved"}}
{{else}}
{{ctx.Locale.Tr "repo.issues.review.hide_outdated"}}
{{end}}
</button>
{{end}}
{{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)}}
<div class="ui segments conversation-holder">
<div class="ui segment collapsible-comment-box gt-py-3 gt-df gt-ac gt-sb">
<div class="gt-df gt-ac">
<a href="{{$comment.CodeCommentLink ctx}}" class="file-comment gt-ml-3 gt-word-break">{{$comment.TreePath}}</a>
{{if $invalid}}
<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"}}
</span>
{{end}}
</div>
<div>
{{if or $invalid $resolved}}
<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"}}
{{if $resolved}}
{{ctx.Locale.Tr "repo.issues.review.show_resolved"}}
{{else}}
{{ctx.Locale.Tr "repo.issues.review.show_outdated"}}
{{end}}
</button>
<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"}}
{{if $resolved}}
{{ctx.Locale.Tr "repo.issues.review.hide_resolved"}}
{{else}}
{{ctx.Locale.Tr "repo.issues.review.hide_outdated"}}
{{end}}
</button>
{{end}}
</div>
</div>
</div>
{{$diff := (CommentMustAsDiff (index .comments 0))}}
{{if $diff}}
{{$file := (index $diff.Files 0)}}
<div id="code-preview-{{(index .comments 0).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="file-body file-code code-view code-diff code-diff-unified unicode-escaped">
<table>
<tbody>
{{template "repo/diff/section_unified" dict "file" $file "root" $}}
</tbody>
</table>
{{$diff := (CommentMustAsDiff $comment)}}
{{if $diff}}
{{$file := (index $diff.Files 0)}}
<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="file-body file-code code-view code-diff code-diff-unified unicode-escaped">
<table>
<tbody>
{{template "repo/diff/section_unified" dict "file" $file "root" $}}
</tbody>
</table>
</div>
</div>
</div>
</div>
{{end}}
<div id="code-comments-{{(index .comments 0).ID}}" class="comment-code-cloud ui segment{{if $resolved}} gt-hidden{{end}}">
<div class="ui comments gt-mb-0">
{{range .comments}}
{{$createdSubStr:= TimeSinceUnix .CreatedUnix ctx.Locale}}
<div class="comment code-comment gt-pb-4" id="{{.HashTag}}">
<div class="content">
<div class="header comment-header">
<div class="comment-header-left gt-df gt-ac">
{{if not .OriginalAuthor}}
<a class="avatar">
{{ctx.AvatarUtils.Avatar .Poster 20}}
</a>
{{end}}
<span class="text grey muted-links">
{{if .OriginalAuthor}}
<span class="text black gt-font-semibold">
{{svg (MigrationIcon $.Repository.GetOriginalURLHostname)}}
{{.OriginalAuthor}}
</span>
<span class="text grey muted-links"> {{if $.Repository.OriginalURL}}</span>
<span class="text migrate">({{ctx.Locale.Tr "repo.migrated_from" ($.Repository.OriginalURL|Escape) ($.Repository.GetOriginalURLHostname|Escape) | Safe}}){{end}}</span>
{{else}}
{{template "shared/user/authorlink" .Poster}}
{{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">
{{range .comments}}
{{$createdSubStr:= TimeSinceUnix .CreatedUnix ctx.Locale}}
<div class="comment code-comment gt-pb-4" id="{{.HashTag}}">
<div class="content">
<div class="header comment-header">
<div class="comment-header-left gt-df gt-ac">
{{if not .OriginalAuthor}}
<a class="avatar">
{{ctx.AvatarUtils.Avatar .Poster 20}}
</a>
{{end}}
{{ctx.Locale.Tr "repo.issues.commented_at" (.HashTag|Escape) $createdSubStr | Safe}}
</span>
<span class="text grey muted-links">
{{if .OriginalAuthor}}
<span class="text black gt-font-semibold">
{{svg (MigrationIcon $.Repository.GetOriginalURLHostname)}}
{{.OriginalAuthor}}
</span>
<span class="text grey muted-links"> {{if $.Repository.OriginalURL}}</span>
<span class="text migrate">({{ctx.Locale.Tr "repo.migrated_from" ($.Repository.OriginalURL|Escape) ($.Repository.GetOriginalURLHostname|Escape) | Safe}}){{end}}</span>
{{else}}
{{template "shared/user/authorlink" .Poster}}
{{end}}
{{ctx.Locale.Tr "repo.issues.commented_at" (.HashTag|Escape) $createdSubStr | Safe}}
</span>
</div>
<div class="comment-header-right actions gt-df gt-ac">
{{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}}
</div>
</div>
<div class="comment-header-right actions gt-df gt-ac">
{{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))}}
<div class="text comment-content">
<div class="render-content markup" {{if or $.Permission.IsAdmin $.HasIssuesOrPullsWritePermission (and $.IsSigned (eq $.SignedUserID .PosterID))}}data-can-edit="true"{{end}}>
{{if .RenderedContent}}
{{.RenderedContent|Str2html}}
{{else}}
<span class="no-content">{{ctx.Locale.Tr "repo.issues.no_content"}}</span>
{{end}}
</div>
<div id="issuecomment-{{.ID}}-raw" class="raw-content gt-hidden">{{.Content}}</div>
<div class="edit-content-zone gt-hidden" data-update-url="{{$.RepoLink}}/comments/{{.ID}}" data-context="{{$.RepoLink}}" data-attachment-url="{{$.RepoLink}}/comments/{{.ID}}/attachments"></div>
</div>
</div>
<div class="text comment-content">
<div class="render-content markup" {{if or $.Permission.IsAdmin $.HasIssuesOrPullsWritePermission (and $.IsSigned (eq $.SignedUserID .PosterID))}}data-can-edit="true"{{end}}>
{{if .RenderedContent}}
{{.RenderedContent|Str2html}}
{{else}}
<span class="no-content">{{ctx.Locale.Tr "repo.issues.no_content"}}</span>
{{$reactions := .Reactions.GroupByType}}
{{if $reactions}}
{{template "repo/issue/view_content/reactions" dict "ctxData" $ "ActionURL" (printf "%s/comments/%d/reactions" $.RepoLink .ID) "Reactions" $reactions}}
{{end}}
</div>
<div id="issuecomment-{{.ID}}-raw" class="raw-content gt-hidden">{{.Content}}</div>
<div class="edit-content-zone gt-hidden" data-update-url="{{$.RepoLink}}/comments/{{.ID}}" data-context="{{$.RepoLink}}" data-attachment-url="{{$.RepoLink}}/comments/{{.ID}}/attachments"></div>
</div>
{{$reactions := .Reactions.GroupByType}}
{{if $reactions}}
{{template "repo/issue/view_content/reactions" dict "ctxData" $ "ActionURL" (printf "%s/comments/%d/reactions" $.RepoLink .ID) "Reactions" $reactions}}
{{end}}
</div>
</div>
{{end}}
</div>
<div class="code-comment-buttons gt-df gt-ac gt-fw gt-mt-3 gt-mb-2 gt-mx-3">
<div class="gt-f1">
{{if $resolved}}
<div class="ui grey text">
{{svg "octicon-check" 16 "gt-mr-2"}}
<b>{{$resolveDoer.Name}}</b> {{ctx.Locale.Tr "repo.issues.review.resolved_by"}}
</div>
{{end}}
</div>
<div class="code-comment-buttons-buttons">
{{if and $.CanMarkConversation $isNotPending}}
<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">
{{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 class="code-comment-buttons gt-df gt-ac gt-fw gt-mt-3 gt-mb-2 gt-mx-3">
<div class="gt-f1">
{{if $resolved}}
<div class="ui grey text">
{{svg "octicon-check" 16 "gt-mr-2"}}
<b>{{$resolveDoer.Name}}</b> {{ctx.Locale.Tr "repo.issues.review.resolved_by"}}
</div>
{{end}}
</div>
<div class="code-comment-buttons-buttons">
{{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="{{$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>
</div>
{{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}}
14 changes: 12 additions & 2 deletions tests/integration/pull_review_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
issues_model "code.gitea.io/gitea/models/issues"
"code.gitea.io/gitea/models/unittest"
user_model "code.gitea.io/gitea/models/user"
"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"
Expand All @@ -25,10 +26,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) {
Expand Down
Loading