diff options
| -rw-r--r-- | app/models/time_entry.rb | 9 | ||||
| -rw-r--r-- | lib/redmine/i18n.rb | 4 | ||||
| -rw-r--r-- | test/unit/time_entry_test.rb | 16 |
3 files changed, 25 insertions, 4 deletions
diff --git a/app/models/time_entry.rb b/app/models/time_entry.rb index b784c4f2d..abbbe4441 100644 --- a/app/models/time_entry.rb +++ b/app/models/time_entry.rb @@ -191,7 +191,14 @@ class TimeEntry < ApplicationRecord def hours h = read_attribute(:hours) if h.is_a?(Float) - h.round(2) + # Convert the float value to a rational with a denominator of 60 to + # avoid floating point errors. + # + # Examples: + # 0.38333333333333336 => (23/60) # 23m + # 0.9913888888888889 => (59/60) # 59m 29s is rounded to 59m + # 0.9919444444444444 => (1/1) # 59m 30s is rounded to 60m + (h * 60).round / 60r else h end diff --git a/lib/redmine/i18n.rb b/lib/redmine/i18n.rb index 0488fa289..a9cd1dd0d 100644 --- a/lib/redmine/i18n.rb +++ b/lib/redmine/i18n.rb @@ -50,12 +50,12 @@ module Redmine end def l_hours(hours) - hours = hours.to_f + hours = hours.to_f unless hours.is_a?(Numeric) l((hours < 2.0 ? :label_f_hour : :label_f_hour_plural), :value => format_hours(hours)) end def l_hours_short(hours) - l(:label_f_hour_short, :value => format_hours(hours.to_f)) + l(:label_f_hour_short, :value => format_hours(hours.is_a?(Numeric) ? hours : hours.to_f)) end def ll(lang, str, arg=nil) diff --git a/test/unit/time_entry_test.rb b/test/unit/time_entry_test.rb index 8722a86cd..358408c99 100644 --- a/test/unit/time_entry_test.rb +++ b/test/unit/time_entry_test.rb @@ -87,13 +87,27 @@ class TimeEntryTest < ActiveSupport::TestCase "3 hours" => 3.0, "12min" => 0.2, "12 Min" => 0.2, + "0:23" => Rational(23, 60), # 0.38333333333333336 + "0.9913888888888889" => Rational(59, 60), # 59m 29s is rounded to 59m + "0.9919444444444444" => 1 # 59m 30s is rounded to 60m } assertions.each do |k, v| t = TimeEntry.new(:hours => k) - assert_equal v, t.hours, "Converting #{k} failed:" + assert v == t.hours && t.hours.is_a?(Rational), "Converting #{k} failed:" end end + def test_hours_sum_precision + # The sum of 10, 10, and 40 minutes should be 1 hour, but in older + # versions of Redmine, the result was 1.01 hours. This was because + # TimeEntry#hours was a float value rounded to 2 decimal places. + # [0.17, 0.17, 0.67].sum => 1.01 + + hours = %w[10m 10m 40m].map {|m| TimeEntry.new(hours: m).hours} + assert_equal 1, hours.sum + hours.map {|h| assert h.is_a?(Rational)} + end + def test_hours_should_default_to_nil assert_nil TimeEntry.new.hours end |