Patch #36897

Wrong formatting of date in Time Entries

Added by Denis Yurashku about 1 month ago. Updated about 1 month ago.

Status:NewStart date:
Priority:NormalDue date:
Assignee:-% Done:

0%

Category:Time tracking
Target version:Candidate for next minor release

Description

Date is formatted wrong at "<PROJECT> – Time entries" ("7:60" instead of "8:00", for example). See screenshot.

This patch fixes it.

Format_hours.diff Magnifier (450 Bytes) Denis Yurashku, 2022-04-05 14:30

Снимок экрана от 2022-04-05 16-59-25_0.png - Screenshot of bug. (71.6 KB) Denis Yurashku, 2022-04-05 16:03

Format_hours_edit.diff Magnifier (443 Bytes) Denis Yurashku, 2022-04-28 10:14


Related issues

Related to Redmine - Feature #23996: Introduce a setting to change the display format of times... Closed

History

#1 Updated by Denis Yurashku about 1 month ago

I've encountered this in 4.0.6, but AFAIK this bugged part of code still not fixed in 5.0.0.

#2 Updated by Holger Just about 1 month ago

  • Related to Feature #23996: Introduce a setting to change the display format of timespans to HH:MM added

#3 Updated by Holger Just about 1 month ago

  • Target version set to Candidate for next minor release

This whole thing can probably be simplified to just

def format_hours(hours)
  return "" if hours.blank?

  if Setting.timespan_format == 'minutes'
    "%d:%02d" % (hours * 60).divmod(60)
  else
    l_number("%.2f", hours.to_f)
  end
end

#4 Updated by Denis Yurashku about 1 month ago

Probably yes.

But as far as I understood from how hours are displayed in Time entries, they need to be `.round`-ed (not just dropping the fractional part).
I.e. if you've spent 7:59:30 (7.991666666666667 hrs), for human it's 8:00 (we don't count seconds in Time entries). :)
Patch gives "8:00" too.

I think like this.

#5 Updated by Holger Just about 1 month ago

Hmmm, with my version, trailing "seconds" indeed would just be cut off instead of being rounded. Thus, semantically 7:59:01 and 7:59:59 would both be shown as "7:59" which is different from the current behavior.

If we round the minutes, we would again have to deal with the case of 7.99 hours being rounded up to "7:60" however.

Maybe this version could work then:

h, m = (hours * 60).divmod(60).map!(&:round)
if m == 60
  h += 1
  m = 0
end
"%d:%02d" % [h, m]

The if block could also be shortened to the equivalent one-liner:

h, m = h+1, 0 if m == 60

In any case, we can be sure that after the first line, m is an Integer between 0 and 60 (inclusive). If it was rounded up to 60, we set to to 0 and increase the hour by one in the if block.

This should then be equivalent to the proposed version in the original patch, but with a little less math involved and a little bit more straight-forward.

At Planio, we run this code for some time now which is a mixture of both approaches:

h = hours.floor
m = ((hours - h) * 60).round
if m == 60
  h += 1
  m = 0
end
"%d:%02d" % [h, m]

Here, the first two lines (which are the same as in Redmine core) return the exact same result as h, m = (hours * 60).divmod(60).map!(&:round).

#6 Updated by Denis Yurashku about 1 month ago

Holger Just wrote:

At Planio, we run this code for some time now which is a mixture of both approaches:

h = hours.floor
m = ((hours - h) * 60).round
if m == 60
  h += 1
  m = 0
end
"%d:%02d" % [h, m]

Here, the first two lines (which are the same as in Redmine core) return the exact same result as h, m = (hours * 60).divmod(60).map!(&:round).

Yes, I think we can go this way. :)

#7 Updated by Denis Yurashku about 1 month ago

So, the adjusted patch.

Also available in: Atom PDF