From 259811617ba15c77ddd89360178a59251d611af2 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Sun, 3 Nov 2024 05:04:53 +0800 Subject: [PATCH] Replace DateTime with proper functions (#32402) Follow #32383 This PR cleans up the "Deadline" usages in templates, make them call `ParseLegacy` first to get a `Time` struct then display by `DateUtils`. Now it should be pretty clear how "deadline string" works, it makes it possible to do further refactoring and correcting. --- modules/templates/helper.go | 4 +-- modules/templates/util_date.go | 26 ++++++++++++++ .../util_date_test.go} | 36 ++++++++++--------- modules/timeutil/datetime.go | 12 ++----- templates/repo/issue/milestone_issues.tmpl | 2 +- templates/repo/issue/milestones.tmpl | 2 +- .../repo/issue/view_content/comments.tmpl | 10 +++--- templates/user/dashboard/milestones.tmpl | 2 +- 8 files changed, 59 insertions(+), 35 deletions(-) rename modules/{timeutil/datetime_test.go => templates/util_date_test.go} (61%) diff --git a/modules/templates/helper.go b/modules/templates/helper.go index 5038e8a1327..a5168541d8b 100644 --- a/modules/templates/helper.go +++ b/modules/templates/helper.go @@ -54,7 +54,7 @@ func NewFuncMap() template.FuncMap { "StringUtils": NewStringUtils, "SliceUtils": NewSliceUtils, "JsonUtils": NewJsonUtils, - "DateUtils": NewDateUtils, // TODO: to be replaced by DateUtils + "DateUtils": NewDateUtils, // ----------------------------------------------------------------- // svg / avatar / icon / color @@ -71,7 +71,7 @@ func NewFuncMap() template.FuncMap { "CountFmt": base.FormatNumberSI, "TimeSince": timeutil.TimeSince, "TimeSinceUnix": timeutil.TimeSinceUnix, - "DateTime": timeutil.DateTime, + "DateTime": dateTimeLegacy, // for backward compatibility only, do not use it anymore "Sec2Time": util.SecToTime, "LoadTimes": func(startTime time.Time) string { return fmt.Sprint(time.Since(startTime).Nanoseconds()/1e6) + "ms" diff --git a/modules/templates/util_date.go b/modules/templates/util_date.go index ec48a7e4beb..45dd8da02f1 100644 --- a/modules/templates/util_date.go +++ b/modules/templates/util_date.go @@ -6,7 +6,9 @@ package templates import ( "context" "html/template" + "time" + "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/timeutil" ) @@ -32,3 +34,27 @@ func (du *DateUtils) AbsoluteLong(time any) template.HTML { func (du *DateUtils) FullTime(time any) template.HTML { return timeutil.DateTime("full", time) } + +// ParseLegacy parses the datetime in legacy format, eg: "2016-01-02" in server's timezone. +// It shouldn't be used in new code. New code should use Time or TimeStamp as much as possible. +func (du *DateUtils) ParseLegacy(datetime string) time.Time { + return parseLegacy(datetime) +} + +func parseLegacy(datetime string) time.Time { + t, err := time.Parse(time.RFC3339, datetime) + if err != nil { + t, _ = time.ParseInLocation(time.DateOnly, datetime, setting.DefaultUILocation) + } + return t +} + +func dateTimeLegacy(format string, datetime any, _ ...string) template.HTML { + if !setting.IsProd || setting.IsInTesting { + panic("dateTimeLegacy is for backward compatibility only, do not use it in new code") + } + if s, ok := datetime.(string); ok { + datetime = parseLegacy(s) + } + return timeutil.DateTime(format, datetime) +} diff --git a/modules/timeutil/datetime_test.go b/modules/templates/util_date_test.go similarity index 61% rename from modules/timeutil/datetime_test.go rename to modules/templates/util_date_test.go index ac2ce35ba27..96c3776d398 100644 --- a/modules/timeutil/datetime_test.go +++ b/modules/templates/util_date_test.go @@ -1,7 +1,7 @@ // Copyright 2023 The Gitea Authors. All rights reserved. // SPDX-License-Identifier: MIT -package timeutil +package templates import ( "testing" @@ -9,6 +9,7 @@ import ( "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/test" + "code.gitea.io/gitea/modules/timeutil" "github.com/stretchr/testify/assert" ) @@ -16,32 +17,35 @@ import ( func TestDateTime(t *testing.T) { testTz, _ := time.LoadLocation("America/New_York") defer test.MockVariableValue(&setting.DefaultUILocation, testTz)() + defer test.MockVariableValue(&setting.IsInTesting, false)() + + du := NewDateUtils(nil) refTimeStr := "2018-01-01T00:00:00Z" refDateStr := "2018-01-01" refTime, _ := time.Parse(time.RFC3339, refTimeStr) - refTimeStamp := TimeStamp(refTime.Unix()) + refTimeStamp := timeutil.TimeStamp(refTime.Unix()) - assert.EqualValues(t, "-", DateTime("short", nil)) - assert.EqualValues(t, "-", DateTime("short", 0)) - assert.EqualValues(t, "-", DateTime("short", time.Time{})) - assert.EqualValues(t, "-", DateTime("short", TimeStamp(0))) + assert.EqualValues(t, "-", du.AbsoluteShort(nil)) + assert.EqualValues(t, "-", du.AbsoluteShort(0)) + assert.EqualValues(t, "-", du.AbsoluteShort(time.Time{})) + assert.EqualValues(t, "-", du.AbsoluteShort(timeutil.TimeStamp(0))) - actual := DateTime("short", "invalid") - assert.EqualValues(t, `invalid`, actual) + actual := dateTimeLegacy("short", "invalid") + assert.EqualValues(t, `-`, actual) - actual = DateTime("short", refTimeStr) - assert.EqualValues(t, `2018-01-01T00:00:00Z`, actual) - - actual = DateTime("short", refTime) + actual = dateTimeLegacy("short", refTimeStr) assert.EqualValues(t, `2018-01-01`, actual) - actual = DateTime("short", refDateStr) - assert.EqualValues(t, `2018-01-01`, actual) + actual = du.AbsoluteShort(refTime) + assert.EqualValues(t, `2018-01-01`, actual) - actual = DateTime("short", refTimeStamp) + actual = dateTimeLegacy("short", refDateStr) + assert.EqualValues(t, `2018-01-01`, actual) + + actual = du.AbsoluteShort(refTimeStamp) assert.EqualValues(t, `2017-12-31`, actual) - actual = DateTime("full", refTimeStamp) + actual = du.FullTime(refTimeStamp) assert.EqualValues(t, `2017-12-31 19:00:00 -05:00`, actual) } diff --git a/modules/timeutil/datetime.go b/modules/timeutil/datetime.go index c089173560b..664e0320b03 100644 --- a/modules/timeutil/datetime.go +++ b/modules/timeutil/datetime.go @@ -12,9 +12,7 @@ import ( ) // DateTime renders an absolute time HTML element by datetime. -func DateTime(format string, datetime any, extraAttrs ...string) template.HTML { - // TODO: remove the extraAttrs argument, it's not used in any call to DateTime - +func DateTime(format string, datetime any) template.HTML { if p, ok := datetime.(*time.Time); ok { datetime = *p } @@ -34,9 +32,6 @@ func DateTime(format string, datetime any, extraAttrs ...string) template.HTML { switch v := datetime.(type) { case nil: return "-" - case string: - datetimeEscaped = html.EscapeString(v) - textEscaped = datetimeEscaped case time.Time: if v.IsZero() || v.Unix() == 0 { return "-" @@ -51,10 +46,7 @@ func DateTime(format string, datetime any, extraAttrs ...string) template.HTML { panic(fmt.Sprintf("Unsupported time type %T", datetime)) } - attrs := make([]string, 0, 10+len(extraAttrs)) - attrs = append(attrs, extraAttrs...) - attrs = append(attrs, `weekday=""`, `year="numeric"`) - + attrs := []string{`weekday=""`, `year="numeric"`} switch format { case "short", "long": // date only attrs = append(attrs, `month="`+format+`"`, `day="numeric"`) diff --git a/templates/repo/issue/milestone_issues.tmpl b/templates/repo/issue/milestone_issues.tmpl index 5bae6fc6d58..01bd944f46b 100644 --- a/templates/repo/issue/milestone_issues.tmpl +++ b/templates/repo/issue/milestone_issues.tmpl @@ -38,7 +38,7 @@ {{if .Milestone.DeadlineString}} {{svg "octicon-calendar"}} - {{DateTime "short" .Milestone.DeadlineString}} + {{ctx.DateUtils.AbsoluteShort (.Milestone.DeadlineString|ctx.DateUtils.ParseLegacy)}} {{else}} {{svg "octicon-calendar"}} diff --git a/templates/repo/issue/milestones.tmpl b/templates/repo/issue/milestones.tmpl index bce7ad87171..8b84659d108 100644 --- a/templates/repo/issue/milestones.tmpl +++ b/templates/repo/issue/milestones.tmpl @@ -59,7 +59,7 @@ {{if .DeadlineString}} {{svg "octicon-calendar" 14}} - {{DateTime "short" .DeadlineString}} + {{ctx.DateUtils.AbsoluteShort (.DeadlineString|ctx.DateUtils.ParseLegacy)}} {{else}} {{svg "octicon-calendar" 14}} diff --git a/templates/repo/issue/view_content/comments.tmpl b/templates/repo/issue/view_content/comments.tmpl index 57abbeb8f79..9324959bedd 100644 --- a/templates/repo/issue/view_content/comments.tmpl +++ b/templates/repo/issue/view_content/comments.tmpl @@ -296,7 +296,8 @@ {{template "shared/user/avatarlink" dict "user" .Poster}} {{template "shared/user/authorlink" .Poster}} - {{ctx.Locale.Tr "repo.issues.due_date_added" (DateTime "long" .Content) $createdStr}} + {{$dueDate := ctx.DateUtils.AbsoluteLong (.Content|ctx.DateUtils.ParseLegacy)}} + {{ctx.Locale.Tr "repo.issues.due_date_added" $dueDate $createdStr}} {{else if eq .Type 17}} @@ -307,8 +308,8 @@ {{template "shared/user/authorlink" .Poster}} {{$parsedDeadline := StringUtils.Split .Content "|"}} {{if eq (len $parsedDeadline) 2}} - {{$from := DateTime "long" (index $parsedDeadline 1)}} - {{$to := DateTime "long" (index $parsedDeadline 0)}} + {{$to := ctx.DateUtils.AbsoluteLong ((index $parsedDeadline 0)|ctx.DateUtils.ParseLegacy)}} + {{$from := ctx.DateUtils.AbsoluteLong ((index $parsedDeadline 1)|ctx.DateUtils.ParseLegacy)}} {{ctx.Locale.Tr "repo.issues.due_date_modified" $to $from $createdStr}} {{end}} @@ -319,7 +320,8 @@ {{template "shared/user/avatarlink" dict "user" .Poster}} {{template "shared/user/authorlink" .Poster}} - {{ctx.Locale.Tr "repo.issues.due_date_remove" (DateTime "long" .Content) $createdStr}} + {{$dueDate := ctx.DateUtils.AbsoluteLong (.Content|ctx.DateUtils.ParseLegacy)}} + {{ctx.Locale.Tr "repo.issues.due_date_remove" $dueDate $createdStr}} {{else if eq .Type 19}} diff --git a/templates/user/dashboard/milestones.tmpl b/templates/user/dashboard/milestones.tmpl index 71ff8dba3f4..e01eca7c743 100644 --- a/templates/user/dashboard/milestones.tmpl +++ b/templates/user/dashboard/milestones.tmpl @@ -116,7 +116,7 @@ {{if .DeadlineString}} {{svg "octicon-calendar" 14}} - {{DateTime "short" .DeadlineString}} + {{ctx.DateUtils.AbsoluteShort (.DeadlineString|ctx.DateUtils.ParseLegacy)}} {{else}} {{svg "octicon-calendar" 14}}