diff --git a/modules/contexttest/context_tests.go b/modules/contexttest/context_tests.go index ea91bc5001..97d912ea57 100644 --- a/modules/contexttest/context_tests.go +++ b/modules/contexttest/context_tests.go @@ -40,8 +40,19 @@ func mockRequest(t *testing.T, reqPath string) *http.Request { return req } +type MockContextOption struct { + Render context.Render +} + // MockContext mock context for unit tests -func MockContext(t *testing.T, reqPath string) (*context.Context, *httptest.ResponseRecorder) { +func MockContext(t *testing.T, reqPath string, opts ...MockContextOption) (*context.Context, *httptest.ResponseRecorder) { + var opt MockContextOption + if len(opts) > 0 { + opt = opts[0] + } + if opt.Render == nil { + opt.Render = &MockRender{} + } resp := httptest.NewRecorder() req := mockRequest(t, reqPath) base, baseCleanUp := context.NewBaseContext(resp, req) @@ -49,7 +60,7 @@ func MockContext(t *testing.T, reqPath string) (*context.Context, *httptest.Resp base.Data = middleware.GetContextData(req.Context()) base.Locale = &translation.MockLocale{} - ctx := context.NewWebContext(base, &MockRender{}, nil) + ctx := context.NewWebContext(base, opt.Render, nil) chiCtx := chi.NewRouteContext() ctx.Base.AppendContextValue(chi.RouteCtxKey, chiCtx) diff --git a/routers/web/repo/middlewares.go b/routers/web/repo/middlewares.go index 5f4a219aa3..eecc845155 100644 --- a/routers/web/repo/middlewares.go +++ b/routers/web/repo/middlewares.go @@ -92,23 +92,15 @@ func SetWhitespaceBehavior(ctx *context.Context) { // SetShowOutdatedComments set the show outdated comments option as context variable func SetShowOutdatedComments(ctx *context.Context) { showOutdatedCommentsValue := ctx.FormString("show-outdated") - // var showOutdatedCommentsValue string - if showOutdatedCommentsValue != "true" && showOutdatedCommentsValue != "false" { // invalid or no value for this form string -> use default or stored user setting + showOutdatedCommentsValue = "true" if ctx.IsSigned { - showOutdatedCommentsValue, _ = user_model.GetUserSetting(ctx, ctx.Doer.ID, user_model.SettingsKeyShowOutdatedComments, "false") - } else { - // not logged in user -> use the default value - showOutdatedCommentsValue = "false" + showOutdatedCommentsValue, _ = user_model.GetUserSetting(ctx, ctx.Doer.ID, user_model.SettingsKeyShowOutdatedComments, showOutdatedCommentsValue) } - } else { + } else if ctx.IsSigned { // valid value -> update user setting if user is logged in - if ctx.IsSigned { - _ = user_model.SetUserSetting(ctx, ctx.Doer.ID, user_model.SettingsKeyShowOutdatedComments, showOutdatedCommentsValue) - } + _ = user_model.SetUserSetting(ctx, ctx.Doer.ID, user_model.SettingsKeyShowOutdatedComments, showOutdatedCommentsValue) } - - showOutdatedComments, _ := strconv.ParseBool(showOutdatedCommentsValue) - ctx.Data["ShowOutdatedComments"] = showOutdatedComments + ctx.Data["ShowOutdatedComments"], _ = strconv.ParseBool(showOutdatedCommentsValue) } diff --git a/routers/web/repo/pull_review.go b/routers/web/repo/pull_review.go index a6b3bd1c8d..b93460d169 100644 --- a/routers/web/repo/pull_review.go +++ b/routers/web/repo/pull_review.go @@ -22,6 +22,7 @@ import ( const ( tplDiffConversation base.TplName = "repo/diff/conversation" + tplConversationOutdated base.TplName = "repo/diff/conversation_outdated" tplTimelineConversation base.TplName = "repo/issue/view_content/conversation" tplNewComment base.TplName = "repo/diff/new_comment" ) @@ -161,8 +162,8 @@ func renderConversation(ctx *context.Context, comment *issues_model.Comment, ori return } if len(comments) == 0 { - // 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 - ctx.Resp.WriteHeader(http.StatusOK) + // if the comments are empty (deleted, outdated, etc), it's better to tell the users that it is outdated + ctx.HTML(http.StatusOK, tplConversationOutdated) return } diff --git a/routers/web/repo/pull_review_test.go b/routers/web/repo/pull_review_test.go new file mode 100644 index 0000000000..65019af40b --- /dev/null +++ b/routers/web/repo/pull_review_test.go @@ -0,0 +1,76 @@ +// Copyright 2024 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package repo + +import ( + "net/http/httptest" + "testing" + + "code.gitea.io/gitea/models/db" + issues_model "code.gitea.io/gitea/models/issues" + "code.gitea.io/gitea/models/unittest" + "code.gitea.io/gitea/modules/context" + "code.gitea.io/gitea/modules/contexttest" + "code.gitea.io/gitea/modules/templates" + "code.gitea.io/gitea/services/pull" + + "github.com/stretchr/testify/assert" +) + +func TestRenderConversation(t *testing.T) { + unittest.PrepareTestEnv(t) + + pr, _ := issues_model.GetPullRequestByID(db.DefaultContext, 2) + _ = pr.LoadIssue(db.DefaultContext) + _ = pr.Issue.LoadPoster(db.DefaultContext) + _ = pr.Issue.LoadRepo(db.DefaultContext) + + run := func(name string, cb func(t *testing.T, ctx *context.Context, resp *httptest.ResponseRecorder)) { + t.Run(name, func(t *testing.T) { + ctx, resp := contexttest.MockContext(t, "/", contexttest.MockContextOption{Render: templates.HTMLRenderer()}) + contexttest.LoadUser(t, ctx, pr.Issue.PosterID) + contexttest.LoadRepo(t, ctx, pr.BaseRepoID) + contexttest.LoadGitRepo(t, ctx) + defer ctx.Repo.GitRepo.Close() + cb(t, ctx, resp) + }) + } + + var preparedComment *issues_model.Comment + run("prepare", func(t *testing.T, ctx *context.Context, resp *httptest.ResponseRecorder) { + comment, err := pull.CreateCodeComment(ctx, pr.Issue.Poster, ctx.Repo.GitRepo, pr.Issue, 1, "content", "", false, 0, pr.HeadCommitID) + if !assert.NoError(t, err) { + return + } + comment.Invalidated = true + err = issues_model.UpdateCommentInvalidate(ctx, comment) + if !assert.NoError(t, err) { + return + } + preparedComment = comment + }) + if !assert.NotNil(t, preparedComment) { + return + } + run("diff with outdated", func(t *testing.T, ctx *context.Context, resp *httptest.ResponseRecorder) { + ctx.Data["ShowOutdatedComments"] = true + renderConversation(ctx, preparedComment, "diff") + assert.Contains(t, resp.Body.String(), `
+ {{ctx.Locale.Tr "repo.issues.review.outdated_description"}} +