Skip to content

Commit a8b7826

Browse files
wxiaoguangwxiaoguang
committed
fix
1 parent f8b471a commit a8b7826

File tree

5 files changed

+91
-13
lines changed

5 files changed

+91
-13
lines changed

‎modules/contexttest/context_tests.go

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,16 +40,27 @@ func mockRequest(t *testing.T, reqPath string) *http.Request {
4040
return req
4141
}
4242

43+
type MockContextOption struct {
44+
Render context.Render
45+
}
46+
4347
// MockContext mock context for unit tests
44-
func MockContext(t *testing.T, reqPath string) (*context.Context, *httptest.ResponseRecorder) {
48+
func MockContext(t *testing.T, reqPath string, opts ...MockContextOption) (*context.Context, *httptest.ResponseRecorder) {
49+
var opt MockContextOption
50+
if len(opts) > 0 {
51+
opt = opts[0]
52+
}
53+
if opt.Render == nil {
54+
opt.Render = &MockRender{}
55+
}
4556
resp := httptest.NewRecorder()
4657
req := mockRequest(t, reqPath)
4758
base, baseCleanUp := context.NewBaseContext(resp, req)
4859
_ = baseCleanUp // during test, it doesn't need to do clean up. TODO: this can be improved later
4960
base.Data = middleware.GetContextData(req.Context())
5061
base.Locale = &translation.MockLocale{}
5162

52-
ctx := context.NewWebContext(base, &MockRender{}, nil)
63+
ctx := context.NewWebContext(base, opt.Render, nil)
5364

5465
chiCtx := chi.NewRouteContext()
5566
ctx.Base.AppendContextValue(chi.RouteCtxKey, chiCtx)

‎routers/web/repo/middlewares.go

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -98,23 +98,17 @@ func SetWhitespaceBehavior(ctx *context.Context) {
9898
// SetShowOutdatedComments set the show outdated comments option as context variable
9999
func SetShowOutdatedComments(ctx *context.Context) {
100100
showOutdatedCommentsValue := ctx.FormString("show-outdated")
101-
// var showOutdatedCommentsValue string
102-
103101
if showOutdatedCommentsValue != "true" && showOutdatedCommentsValue != "false" {
104102
// invalid or no value for this form string -> use default or stored user setting
103+
showOutdatedCommentsValue = "true"
105104
if ctx.IsSigned {
106-
showOutdatedCommentsValue, _ = user_model.GetUserSetting(ctx, ctx.Doer.ID, user_model.SettingsKeyShowOutdatedComments, "false")
107-
} else {
108-
// not logged in user -> use the default value
109-
showOutdatedCommentsValue = "false"
105+
showOutdatedCommentsValue, _ = user_model.GetUserSetting(ctx, ctx.Doer.ID, user_model.SettingsKeyShowOutdatedComments, showOutdatedCommentsValue)
110106
}
111107
} else {
112108
// valid value -> update user setting if user is logged in
113109
if ctx.IsSigned {
114110
_ = user_model.SetUserSetting(ctx, ctx.Doer.ID, user_model.SettingsKeyShowOutdatedComments, showOutdatedCommentsValue)
115111
}
116112
}
117-
118-
showOutdatedComments, _ := strconv.ParseBool(showOutdatedCommentsValue)
119-
ctx.Data["ShowOutdatedComments"] = showOutdatedComments
113+
ctx.Data["ShowOutdatedComments"], _ = strconv.ParseBool(showOutdatedCommentsValue)
120114
}

‎routers/web/repo/pull_review.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import (
2222

2323
const (
2424
tplDiffConversation base.TplName = "repo/diff/conversation"
25+
tplConversationOutdated base.TplName = "repo/diff/conversation_outdated"
2526
tplTimelineConversation base.TplName = "repo/issue/view_content/conversation"
2627
tplNewComment base.TplName = "repo/diff/new_comment"
2728
)
@@ -161,8 +162,8 @@ func renderConversation(ctx *context.Context, comment *issues_model.Comment, ori
161162
return
162163
}
163164
if len(comments) == 0 {
164-
// if the comments are empty (deleted, outdated, etc), it doesn't need to render anything, just return an empty body to replace "conversation-holder" on the page
165-
ctx.Resp.WriteHeader(http.StatusOK)
165+
// if the comments are empty (deleted, outdated, etc), it's better to tell the users that it is outdated
166+
ctx.HTML(http.StatusOK, tplConversationOutdated)
166167
return
167168
}
168169

‎routers/web/repo/pull_review_test.go

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
package repo
2+
3+
import (
4+
"net/http/httptest"
5+
"testing"
6+
7+
issues_model "code.gitea.io/gitea/models/issues"
8+
"code.gitea.io/gitea/models/unittest"
9+
"code.gitea.io/gitea/modules/context"
10+
"code.gitea.io/gitea/modules/contexttest"
11+
"code.gitea.io/gitea/modules/templates"
12+
"code.gitea.io/gitea/services/pull"
13+
14+
"github.com/stretchr/testify/assert"
15+
)
16+
17+
func TestRenderConversation(t *testing.T) {
18+
unittest.PrepareTestEnv(t)
19+
20+
run := func(name string, cb func(t *testing.T, ctx *context.Context, resp *httptest.ResponseRecorder, pr *issues_model.PullRequest)) {
21+
t.Run(name, func(t *testing.T) {
22+
ctx, resp := contexttest.MockContext(t, "/", contexttest.MockContextOption{Render: templates.HTMLRenderer()})
23+
pr, _ := issues_model.GetPullRequestByID(ctx, 2)
24+
_ = pr.LoadIssue(ctx)
25+
_ = pr.Issue.LoadPoster(ctx)
26+
_ = pr.Issue.LoadRepo(ctx)
27+
contexttest.LoadUser(t, ctx, pr.Issue.PosterID)
28+
contexttest.LoadRepo(t, ctx, pr.BaseRepoID)
29+
contexttest.LoadGitRepo(t, ctx)
30+
defer ctx.Repo.GitRepo.Close()
31+
cb(t, ctx, resp, pr)
32+
})
33+
}
34+
35+
var preparedComment *issues_model.Comment
36+
run("prepare", func(t *testing.T, ctx *context.Context, resp *httptest.ResponseRecorder, pr *issues_model.PullRequest) {
37+
comment, err := pull.CreateCodeComment(ctx, pr.Issue.Poster, ctx.Repo.GitRepo, pr.Issue, 1, "content", "", false, 0, pr.HeadCommitID)
38+
if !assert.NoError(t, err) {
39+
return
40+
}
41+
comment.Invalidated = true
42+
err = issues_model.UpdateCommentInvalidate(ctx, comment)
43+
if !assert.NoError(t, err) {
44+
return
45+
}
46+
preparedComment = comment
47+
})
48+
49+
run("diff with outdated", func(t *testing.T, ctx *context.Context, resp *httptest.ResponseRecorder, pr *issues_model.PullRequest) {
50+
ctx.Data["ShowOutdatedComments"] = true
51+
renderConversation(ctx, preparedComment, "diff")
52+
assert.Contains(t, resp.Body.String(), `<div class="content comment-container"`)
53+
})
54+
run("diff without outdated", func(t *testing.T, ctx *context.Context, resp *httptest.ResponseRecorder, pr *issues_model.PullRequest) {
55+
ctx.Data["ShowOutdatedComments"] = false
56+
renderConversation(ctx, preparedComment, "diff")
57+
assert.Contains(t, resp.Body.String(), `conversation-not-existing`)
58+
})
59+
run("timeline with outdated", func(t *testing.T, ctx *context.Context, resp *httptest.ResponseRecorder, pr *issues_model.PullRequest) {
60+
ctx.Data["ShowOutdatedComments"] = true
61+
renderConversation(ctx, preparedComment, "timeline")
62+
assert.Contains(t, resp.Body.String(), `<div id="code-comments-`)
63+
})
64+
run("timeline without outdated", func(t *testing.T, ctx *context.Context, resp *httptest.ResponseRecorder, pr *issues_model.PullRequest) {
65+
ctx.Data["ShowOutdatedComments"] = false
66+
renderConversation(ctx, preparedComment, "timeline")
67+
assert.Contains(t, resp.Body.String(), `conversation-not-existing`)
68+
})
69+
}

‎templates/repo/diff/conversation_outdated.tmpl

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
<div class="ui segment conversation-holder conversation-not-existing">
2+
{{ctx.Locale.Tr "repo.issues.review.outdated_description"}}
3+
</div>

0 commit comments

Comments
 (0)