Fix precision issues in TimeEntry#hours calculation by returning Rational instead...

Patch by Go MAEDA (user:maeda).

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

diff --git a/app/models/time_entry.rb b/app/models/time_entry.rb
index b784c4f2d835b9cb0113c010a7d998dcb0df5e8f..abbbe44412df8532652c88ce659ecb234c11d360 100644 (file)
--- 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 0488fa2891d018a219db672b6a57d1d721f8d8b8..a9cd1dd0d8bcf60d5c6af0f43d67edf87759cd6e 100644 (file)
--- 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 8722a86cdd70135ed9e93ed69456e4d23943f3eb..358408c9953e47e989b3313454df4979810993e1 100644 (file)
--- 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