From a4b242ae7a3edc6302a2730a01cda5a2e76bd6b5 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Tue, 31 Oct 2023 22:11:48 +0800 Subject: [PATCH] Clean up template locale usage (#27856) After many refactoring PRs for the "locale" and "template context function", now the ".locale" is not needed for web templates any more. This PR does a clean up for: 1. Remove `ctx.Data["locale"]` for web context. 2. Use `ctx.Locale` in `500.tmpl`, for consistency. 3. Add a test check for `500 page` locale usage. 4. Remove the `Str2html` and `DotEscape` from mail template context data, they are copy&paste errors introduced by #19169 and #16200 . These functions are template functions (provided by the common renderer), but not template data variables. 5. Make email `SendAsync` function mockable (I was planning to add more tests but it would make this PR much too complex, so the tests could be done in another PR) --- modules/context/context.go | 1 - routers/common/errpage.go | 15 +++++++-------- routers/common/errpage_test.go | 1 + services/mailer/mail.go | 28 ++++++---------------------- services/mailer/mail_issue.go | 2 +- services/mailer/mail_release.go | 8 ++------ services/mailer/mail_repo.go | 6 +----- services/mailer/mail_team_invite.go | 6 +----- services/mailer/mailer.go | 11 ++++------- templates/status/500.tmpl | 20 ++++++++++---------- 10 files changed, 33 insertions(+), 65 deletions(-) diff --git a/modules/context/context.go b/modules/context/context.go index 47ad310b0..8a94e958b 100644 --- a/modules/context/context.go +++ b/modules/context/context.go @@ -157,7 +157,6 @@ func Contexter() func(next http.Handler) http.Handler { ctx.Data["Context"] = ctx // TODO: use "ctx" in template and remove this ctx.Data["CurrentURL"] = setting.AppSubURL + req.URL.RequestURI() ctx.Data["Link"] = ctx.Link - ctx.Data["locale"] = ctx.Locale // PageData is passed by reference, and it will be rendered to `window.config.pageData` in `head.tmpl` for JavaScript modules ctx.PageData = map[string]any{} diff --git a/routers/common/errpage.go b/routers/common/errpage.go index 9c8ccc338..923421a29 100644 --- a/routers/common/errpage.go +++ b/routers/common/errpage.go @@ -9,6 +9,7 @@ import ( user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/base" + "code.gitea.io/gitea/modules/context" "code.gitea.io/gitea/modules/httpcache" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/setting" @@ -35,20 +36,18 @@ func RenderPanicErrorPage(w http.ResponseWriter, req *http.Request, err any) { httpcache.SetCacheControlInHeader(w.Header(), 0, "no-transform") w.Header().Set(`X-Frame-Options`, setting.CORSConfig.XFrameOptions) - data := middleware.GetContextData(req.Context()) - if data["locale"] == nil { - data = middleware.CommonTemplateContextData() - data["locale"] = middleware.Locale(w, req) - } + tmplCtx := context.TemplateContext{} + tmplCtx["Locale"] = middleware.Locale(w, req) + ctxData := middleware.GetContextData(req.Context()) // This recovery handler could be called without Gitea's web context, so we shouldn't touch that context too much. // Otherwise, the 500-page may cause new panics, eg: cache.GetContextWithData, it makes the developer&users couldn't find the original panic. - user, _ := data[middleware.ContextDataKeySignedUser].(*user_model.User) + user, _ := ctxData[middleware.ContextDataKeySignedUser].(*user_model.User) if !setting.IsProd || (user != nil && user.IsAdmin) { - data["ErrorMsg"] = "PANIC: " + combinedErr + ctxData["ErrorMsg"] = "PANIC: " + combinedErr } - err = templates.HTMLRenderer().HTML(w, http.StatusInternalServerError, string(tplStatus500), data, nil) + err = templates.HTMLRenderer().HTML(w, http.StatusInternalServerError, string(tplStatus500), ctxData, tmplCtx) if err != nil { log.Error("Error occurs again when rendering error page: %v", err) w.WriteHeader(http.StatusInternalServerError) diff --git a/routers/common/errpage_test.go b/routers/common/errpage_test.go index 58a633b2a..4fd63ba49 100644 --- a/routers/common/errpage_test.go +++ b/routers/common/errpage_test.go @@ -26,6 +26,7 @@ func TestRenderPanicErrorPage(t *testing.T) { respContent := w.Body.String() assert.Contains(t, respContent, `class="page-content status-page-500"`) assert.Contains(t, respContent, ``) + assert.Contains(t, respContent, `lang="en-US"`) // make sure the locale work // the 500 page doesn't have normal pages footer, it makes it easier to distinguish a normal page and a failed page. // especially when a sub-template causes page error, the HTTP response code is still 200, diff --git a/services/mailer/mail.go b/services/mailer/mail.go index 021012804..1f139d233 100644 --- a/services/mailer/mail.go +++ b/services/mailer/mail.go @@ -26,7 +26,6 @@ import ( "code.gitea.io/gitea/modules/markup" "code.gitea.io/gitea/modules/markup/markdown" "code.gitea.io/gitea/modules/setting" - "code.gitea.io/gitea/modules/templates" "code.gitea.io/gitea/modules/timeutil" "code.gitea.io/gitea/modules/translation" incoming_payload "code.gitea.io/gitea/services/mailer/incoming/payload" @@ -68,15 +67,12 @@ func SendTestMail(email string) error { func sendUserMail(language string, u *user_model.User, tpl base.TplName, code, subject, info string) { locale := translation.NewLocale(language) data := map[string]any{ + "locale": locale, "DisplayName": u.DisplayName(), "ActiveCodeLives": timeutil.MinutesToFriendly(setting.Service.ActiveCodeLives, locale), "ResetPwdCodeLives": timeutil.MinutesToFriendly(setting.Service.ResetPwdCodeLives, locale), "Code": code, "Language": locale.Language(), - // helper - "locale": locale, - "Str2html": templates.Str2html, - "DotEscape": templates.DotEscape, } var content bytes.Buffer @@ -119,15 +115,12 @@ func SendActivateEmailMail(u *user_model.User, email *user_model.EmailAddress) { } locale := translation.NewLocale(u.Language) data := map[string]any{ + "locale": locale, "DisplayName": u.DisplayName(), "ActiveCodeLives": timeutil.MinutesToFriendly(setting.Service.ActiveCodeLives, locale), "Code": u.GenerateEmailActivateCode(email.Email), "Email": email.Email, "Language": locale.Language(), - // helper - "locale": locale, - "Str2html": templates.Str2html, - "DotEscape": templates.DotEscape, } var content bytes.Buffer @@ -152,13 +145,10 @@ func SendRegisterNotifyMail(u *user_model.User) { locale := translation.NewLocale(u.Language) data := map[string]any{ + "locale": locale, "DisplayName": u.DisplayName(), "Username": u.Name, "Language": locale.Language(), - // helper - "locale": locale, - "Str2html": templates.Str2html, - "DotEscape": templates.DotEscape, } var content bytes.Buffer @@ -185,14 +175,11 @@ func SendCollaboratorMail(u, doer *user_model.User, repo *repo_model.Repository) subject := locale.Tr("mail.repo.collaborator.added.subject", doer.DisplayName(), repoName) data := map[string]any{ + "locale": locale, "Subject": subject, "RepoName": repoName, "Link": repo.HTMLURL(), "Language": locale.Language(), - // helper - "locale": locale, - "Str2html": templates.Str2html, - "DotEscape": templates.DotEscape, } var content bytes.Buffer @@ -259,6 +246,7 @@ func composeIssueCommentMessages(ctx *mailCommentContext, lang string, recipient locale := translation.NewLocale(lang) mailMeta := map[string]any{ + "locale": locale, "FallbackSubject": fallback, "Body": body, "Link": link, @@ -275,10 +263,6 @@ func composeIssueCommentMessages(ctx *mailCommentContext, lang string, recipient "ReviewComments": reviewComments, "Language": locale.Language(), "CanReply": setting.IncomingEmail.Enabled && commentType != issues_model.CommentTypePullRequestPush, - // helper - "locale": locale, - "Str2html": templates.Str2html, - "DotEscape": templates.DotEscape, } var mailSubject bytes.Buffer @@ -469,7 +453,7 @@ func SendIssueAssignedMail(ctx context.Context, issue *issues_model.Issue, doer if err != nil { return err } - SendAsyncs(msgs) + SendAsync(msgs...) } return nil } diff --git a/services/mailer/mail_issue.go b/services/mailer/mail_issue.go index baa9f1d9a..fab3315be 100644 --- a/services/mailer/mail_issue.go +++ b/services/mailer/mail_issue.go @@ -162,7 +162,7 @@ func mailIssueCommentBatch(ctx *mailCommentContext, users []*user_model.User, vi if err != nil { return err } - SendAsyncs(msgs) + SendAsync(msgs...) receivers = receivers[:i] } } diff --git a/services/mailer/mail_release.go b/services/mailer/mail_release.go index 7bc3db3fe..88973a6be 100644 --- a/services/mailer/mail_release.go +++ b/services/mailer/mail_release.go @@ -14,7 +14,6 @@ import ( "code.gitea.io/gitea/modules/markup" "code.gitea.io/gitea/modules/markup/markdown" "code.gitea.io/gitea/modules/setting" - "code.gitea.io/gitea/modules/templates" "code.gitea.io/gitea/modules/translation" ) @@ -69,13 +68,10 @@ func mailNewRelease(ctx context.Context, lang string, tos []string, rel *repo_mo subject := locale.Tr("mail.release.new.subject", rel.TagName, rel.Repo.FullName()) mailMeta := map[string]any{ + "locale": locale, "Release": rel, "Subject": subject, "Language": locale.Language(), - // helper - "locale": locale, - "Str2html": templates.Str2html, - "DotEscape": templates.DotEscape, } var mailBody bytes.Buffer @@ -95,5 +91,5 @@ func mailNewRelease(ctx context.Context, lang string, tos []string, rel *repo_mo msgs = append(msgs, msg) } - SendAsyncs(msgs) + SendAsync(msgs...) } diff --git a/services/mailer/mail_repo.go b/services/mailer/mail_repo.go index e9c1991b5..b89dcd43b 100644 --- a/services/mailer/mail_repo.go +++ b/services/mailer/mail_repo.go @@ -12,7 +12,6 @@ import ( repo_model "code.gitea.io/gitea/models/repo" user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/setting" - "code.gitea.io/gitea/modules/templates" "code.gitea.io/gitea/modules/translation" ) @@ -65,6 +64,7 @@ func sendRepoTransferNotifyMailPerLang(lang string, newOwner, doer *user_model.U } data := map[string]any{ + "locale": locale, "Doer": doer, "User": repo.Owner, "Repo": repo.FullName(), @@ -72,10 +72,6 @@ func sendRepoTransferNotifyMailPerLang(lang string, newOwner, doer *user_model.U "Subject": subject, "Language": locale.Language(), "Destination": destination, - // helper - "locale": locale, - "Str2html": templates.Str2html, - "DotEscape": templates.DotEscape, } if err := bodyTemplates.ExecuteTemplate(&content, string(mailRepoTransferNotify), data); err != nil { diff --git a/services/mailer/mail_team_invite.go b/services/mailer/mail_team_invite.go index 88ad0c983..ab32beefa 100644 --- a/services/mailer/mail_team_invite.go +++ b/services/mailer/mail_team_invite.go @@ -14,7 +14,6 @@ import ( "code.gitea.io/gitea/modules/base" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/setting" - "code.gitea.io/gitea/modules/templates" "code.gitea.io/gitea/modules/translation" ) @@ -53,16 +52,13 @@ func MailTeamInvite(ctx context.Context, inviter *user_model.User, team *org_mod subject := locale.Tr("mail.team_invite.subject", inviter.DisplayName(), org.DisplayName()) mailMeta := map[string]any{ + "locale": locale, "Inviter": inviter, "Organization": org, "Team": team, "Invite": invite, "Subject": subject, "InviteURL": inviteURL, - // helper - "locale": locale, - "Str2html": templates.Str2html, - "DotEscape": templates.DotEscape, } var mailBody bytes.Buffer diff --git a/services/mailer/mailer.go b/services/mailer/mailer.go index 42d7d5ad5..5e8e3dbb3 100644 --- a/services/mailer/mailer.go +++ b/services/mailer/mailer.go @@ -425,15 +425,12 @@ func NewContext(ctx context.Context) { go graceful.GetManager().RunWithCancel(mailQueue) } -// SendAsync send mail asynchronously -func SendAsync(msg *Message) { - SendAsyncs([]*Message{msg}) -} +// SendAsync send emails asynchronously (make it mockable) +var SendAsync = sendAsync -// SendAsyncs send mails asynchronously -func SendAsyncs(msgs []*Message) { +func sendAsync(msgs ...*Message) { if setting.MailService == nil { - log.Error("Mailer: SendAsyncs is being invoked but mail service hasn't been initialized") + log.Error("Mailer: SendAsync is being invoked but mail service hasn't been initialized") return } diff --git a/templates/status/500.tmpl b/templates/status/500.tmpl index e5570bde7..edcb90f9a 100644 --- a/templates/status/500.tmpl +++ b/templates/status/500.tmpl @@ -1,12 +1,12 @@ {{/* This page should only depend the minimal template functions/variables, to avoid triggering new panics. * base template functions: AppName, AssetUrlPrefix, AssetVersion, AppSubUrl, ThemeName, Str2html -* locale -* Flash -* ErrorMsg -* SignedUser (optional) +* ctx.Locale +* .Flash +* .ErrorMsg +* .SignedUser (optional) */}} - + Internal Server Error - {{AppName}} @@ -19,8 +19,8 @@