gitea icon indicating copy to clipboard operation
gitea copied to clipboard

Incorrect handling of durations in SecToTime()

Open edgar-bonet opened this issue 3 years ago • 3 comments

Description

Pull request #18863 introduced the following code

years := duration / (3600 * 24 * 7 * 4 * 12)
months := (duration / (3600 * 24 * 30)) % 12
weeks := (duration / (3600 * 24 * 7)) % 4
days := (duration / (3600 * 24)) % 7

This is wrong for a couple of reasons:

  1. the week count reverts to zero (because of the modulus operation) after 4 weeks, which is not quite the same as a month

  2. these “years” are 336 days long, which is about 11 months

The consequence is that this function displays:

  • a duration of 340 days (about 11.2 months) as “1 year 11 months”

  • a duration of 29 days as “1 day”

  • a duration of 28 days as the empty string.

See the attached screenshot for the effect of this bug on the display of tracked time.

I suggest to instead use something along these lines:

years := (duration / 86400) / 365
months := (duration / 86400 - years * 365)  / 30
weeks := (duration / 86400 - years * 365 - month * 30) / 7
days := duration / 86400 - years * 365 - month * 30 - weeks * 7

I am not submitting a pull request because I have zero experience with the Go programming language.

Gitea Version

1.18.0+dev-196-ge56005f90

Can you reproduce the bug on the Gitea demo site?

Yes

Log Gist

No response

Screenshots

time_tracker
Screenshot from this test issue.

Git Version

No response

Operating System

No response

How are you running Gitea?

using https://try.gitea.io/

Database

No response

edgar-bonet avatar Aug 01 '22 13:08 edgar-bonet

:eyes: I think we should just revert https://github.com/go-gitea/gitea/pull/18863 and use https://github.com/hako/durafmt instead, it also supports i18n.

edit: The formatted duration is written to the comments table in the DB, so it's not really feasible to translate that in the viewer's locale without reworking the whole thing. So maybe switching to durafmt is not worth it.

noerw avatar Aug 01 '22 14:08 noerw

I wonder whether it is really useful to format those times in years, months, weeks and days.

In the specific context of recording the time spent working on an issue, I would expect the word “day” to mean a “full work day”. Maybe something around 8 hours or so, but certainly not 24 hours. And a week would likely be 5 days. If someone spent 730 hours working on that issue, “730 hours” would be more suitable than “1 month”.

I suggest using hours and minutes as the only time units in this context.

Edit: Even when the numbers get large, hours seem to be the preferred unit. See for example the 10,000 hour rule: no one would call it the “13.7 months rule”.

edgar-bonet avatar Aug 01 '22 18:08 edgar-bonet

@edgar-bonet Thank you for your detailed issue, opened a PR for this at #20610 with the appropriate comments.

Gusted avatar Aug 02 '22 02:08 Gusted