Refactor SecToTime() function (#18863)

- Add helper method to reduce redundancy
- Expand the scope from displaying days to years
- Reduce irrelevance by not displaying small units (hours, minutes, seconds) when bigger ones apply (years)

diff --git a/models/issue_tracked_time_test.go b/models/issue_tracked_time_test.go
index e6c9caf90013bdb7cf6e936029b9b9f4919a47c0..68e78c71c726cd019b424c04bfbe32ea8f3a1dbe 100644 (file)
--- a/models/issue_tracked_time_test.go
+++ b/models/issue_tracked_time_test.go
@@ -34,7 +34,7 @@ func TestAddTime(t *testing.T) {
        assert.Equal(t, int64(3661), tt.Time)
 
        comment := unittest.AssertExistsAndLoadBean(t, &Comment{Type: CommentTypeAddTimeManual, PosterID: 3, IssueID: 1}).(*Comment)
-       assert.Equal(t, comment.Content, "1h 1m 1s")
+       assert.Equal(t, comment.Content, "1 hour 1 minute")
 }
 
 func TestGetTrackedTimes(t *testing.T) {
@@ -86,7 +86,7 @@ func TestTotalTimes(t *testing.T) {
        assert.Len(t, total, 1)
        for user, time := range total {
                assert.Equal(t, int64(1), user.ID)
-               assert.Equal(t, "6m 40s", time)
+               assert.Equal(t, "6 minutes 40 seconds", time)
        }
 
        total, err = TotalTimes(&FindTrackedTimesOptions{IssueID: 2})
@@ -94,9 +94,9 @@ func TestTotalTimes(t *testing.T) {
        assert.Len(t, total, 2)
        for user, time := range total {
                if user.ID == 2 {
-                       assert.Equal(t, "1h 1m 2s", time)
+                       assert.Equal(t, "1 hour 1 minute", time)
                } else if user.ID == 1 {
-                       assert.Equal(t, "20s", time)
+                       assert.Equal(t, "20 seconds", time)
                } else {
                        assert.Error(t, assert.AnError)
                }
@@ -107,7 +107,7 @@ func TestTotalTimes(t *testing.T) {
        assert.Len(t, total, 1)
        for user, time := range total {
                assert.Equal(t, int64(2), user.ID)
-               assert.Equal(t, "1s", time)
+               assert.Equal(t, "1 second", time)
        }
 
        total, err = TotalTimes(&FindTrackedTimesOptions{IssueID: 4})

diff --git a/modules/util/sec_to_time.go b/modules/util/sec_to_time.go
index 657b30cddffc13c0860ed70281806e84279159f8..9ce6fe1a13eb5ea37ac8827a9b759b0f11b0f0a8 100644 (file)
--- a/modules/util/sec_to_time.go
+++ b/modules/util/sec_to_time.go
@@ -4,40 +4,64 @@
 
 package util
 
-import "fmt"
+import (
+       "fmt"
+       "strings"
+)
 
-// SecToTime converts an amount of seconds to a human-readable string (example: 66s -> 1min 6s)
+// SecToTime converts an amount of seconds to a human-readable string. E.g.
+// 66s                 -> 1 minute 6 seconds
+// 52410s              -> 14 hours 33 minutes
+// 563418              -> 6 days 12 hours
+// 1563418             -> 2 weeks 4 days
+// 3937125s     -> 1 month 2 weeks
+// 45677465s   -> 1 year 6 months
 func SecToTime(duration int64) string {
+       formattedTime := ""
+       years := duration / (3600 * 24 * 7 * 4 * 12)
+       months := (duration / (3600 * 24 * 30)) % 12
+       weeks := (duration / (3600 * 24 * 7)) % 4
+       days := (duration / (3600 * 24)) % 7
+       hours := (duration / 3600) % 24
+       minutes := (duration / 60) % 60
        seconds := duration % 60
-       minutes := (duration / (60)) % 60
-       hours := duration / (60 * 60) % 24
-       days := duration / (60 * 60) / 24
 
-       var formattedTime string
-
-       if days > 0 {
-               formattedTime = fmt.Sprintf("%dd", days)
-       }
-       if hours > 0 {
-               if formattedTime == "" {
-                       formattedTime = fmt.Sprintf("%dh", hours)
-               } else {
-                       formattedTime = fmt.Sprintf("%s %dh", formattedTime, hours)
-               }
-       }
-       if minutes > 0 {
-               if formattedTime == "" {
-                       formattedTime = fmt.Sprintf("%dm", minutes)
-               } else {
-                       formattedTime = fmt.Sprintf("%s %dm", formattedTime, minutes)
-               }
+       // Extract only the relevant information of the time
+       // If the time is greater than a year, it makes no sense to display seconds.
+       switch {
+       case years > 0:
+               formattedTime = formatTime(years, "year", formattedTime)
+               formattedTime = formatTime(months, "month", formattedTime)
+       case months > 0:
+               formattedTime = formatTime(months, "month", formattedTime)
+               formattedTime = formatTime(weeks, "week", formattedTime)
+       case weeks > 0:
+               formattedTime = formatTime(weeks, "week", formattedTime)
+               formattedTime = formatTime(days, "day", formattedTime)
+       case days > 0:
+               formattedTime = formatTime(days, "day", formattedTime)
+               formattedTime = formatTime(hours, "hour", formattedTime)
+       case hours > 0:
+               formattedTime = formatTime(hours, "hour", formattedTime)
+               formattedTime = formatTime(minutes, "minute", formattedTime)
+       default:
+               formattedTime = formatTime(minutes, "minute", formattedTime)
+               formattedTime = formatTime(seconds, "second", formattedTime)
        }
-       if seconds > 0 {
-               if formattedTime == "" {
-                       formattedTime = fmt.Sprintf("%ds", seconds)
-               } else {
-                       formattedTime = fmt.Sprintf("%s %ds", formattedTime, seconds)
-               }
+
+       // The formatTime() function always appends a space at the end. This will be trimmed
+       return strings.TrimRight(formattedTime, " ")
+}
+
+// formatTime appends the given value to the existing forammattedTime. E.g:
+// formattedTime = "1 year"
+// input: value = 3, name = "month"
+// output will be "1 year 3 months "
+func formatTime(value int64, name, formattedTime string) string {
+       if value == 1 {
+               formattedTime = fmt.Sprintf("%s1 %s ", formattedTime, name)
+       } else if value > 1 {
+               formattedTime = fmt.Sprintf("%s%d %ss ", formattedTime, value, name)
        }
 
        return formattedTime

diff --git a/modules/util/sec_to_time_test.go b/modules/util/sec_to_time_test.go
index 915dcbf7279045ba2f38f33af566519c4118d029..854190462bc372b6bf5429e6113072419dac986c 100644 (file)
--- a/modules/util/sec_to_time_test.go
+++ b/modules/util/sec_to_time_test.go
@@ -11,10 +11,10 @@ import (
 )
 
 func TestSecToTime(t *testing.T) {
-       assert.Equal(t, SecToTime(10), "10s")
-       assert.Equal(t, SecToTime(100), "1m 40s")
-       assert.Equal(t, SecToTime(1000), "16m 40s")
-       assert.Equal(t, SecToTime(10000), "2h 46m 40s")
-       assert.Equal(t, SecToTime(100000), "1d 3h 46m 40s")
-       assert.Equal(t, SecToTime(1000000), "11d 13h 46m 40s")
+       assert.Equal(t, SecToTime(66), "1 minute 6 seconds")
+       assert.Equal(t, SecToTime(52410), "14 hours 33 minutes")
+       assert.Equal(t, SecToTime(563418), "6 days 12 hours")
+       assert.Equal(t, SecToTime(1563418), "2 weeks 4 days")
+       assert.Equal(t, SecToTime(3937125), "1 month 2 weeks")
+       assert.Equal(t, SecToTime(45677465), "1 year 5 months")
 }