Redmine

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

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.

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).

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. :)

Any movements on this?

Is it "Confirmed"?

Denis Yurashku wrote:

Any movements on this?

Is it "Confirmed"?

I still cannot reproduce the issue. Could you tell me detailed steps to output "7:60"? Without this, I cannot write test code.

You can reproduce the issue by creating a time entry with decimal logged hours slightly below 8 hours, e.g. 7.991666666666667 hours. Here, the fractional minutes will be rounded up to 60 minutes because 0.991666666666667 * 60 = 59.50000000000002. This value is always allowed as the time entry form also allows entering float values (which are then parsed as floats in lib/redmine/core_ext/string/conversions.rb. External time tracking apps might round durations by the second and then send those exact values which are accepted by Redmine.

Programmatically, you can also create this situation from "full" minutes by logging exactly 75 time entries of 44 minutes each (i.e. 0.7333333333333333 hours). The sum of those time entries then comes at 54.99999999999999. Here, the minutes part is again rounded up to 60 by the existing code.

Depending on the database and the rounding of the floats involved, this situation may also be created with other times as shown in the screenshot as we always round the raw values to two decimal places before summing them in TimeEntryQuery#total_for_hours which may introduce additional error margins.

This issue will be fixed if the patch proposed in #40914 is merged.