Fix precision issues in TimeEntry#hours calculation by returning Rational instead of Float (#40914).

Patch by Go MAEDA (user:maeda).


git-svn-id: https://svn.redmine.org/redmine/trunk@23000 e93f8b46-1217-0410-a6f0-8f06a7374b81

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

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)

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