Merge pull request #29594 from ahazem/fix-regression-with-date-helper

Fix regression with date helper in 5.1.2 

actionview/CHANGELOG.md

@@ -14,6 +14,11 @@
 
     *Yuji Yaginuma*
 
+*   Update distance_of_time_in_words helper to display better error messages
+    for bad input.
+
+    *Jay Hayes*
+
 
 ## Rails 5.1.1 (May 12, 2017) ##
 

actionview/lib/action_view/helpers/date_helper.rb

@@ -95,8 +95,8 @@ def distance_of_time_in_words(from_time, to_time = 0, options = {})
           scope: :'datetime.distance_in_words'
         }.merge!(options)
 
-        from_time = from_time.to_time if from_time.respond_to?(:to_time)
-        to_time = to_time.to_time if to_time.respond_to?(:to_time)
+        from_time = normalize_distance_of_time_argument_to_time(from_time)
+        to_time = normalize_distance_of_time_argument_to_time(to_time)
         from_time, to_time = to_time, from_time if from_time > to_time
         distance_in_minutes = ((to_time - from_time) / 60.0).round
         distance_in_seconds = (to_time - from_time).round
@@ -130,22 +130,18 @@ def distance_of_time_in_words(from_time, to_time = 0, options = {})
             # 60 days up to 365 days
           when 86400...525600   then locale.t :x_months,       count: (distance_in_minutes.to_f / 43200.0).round
             else
-            if from_time.acts_like?(:time) && to_time.acts_like?(:time)
-              fyear = from_time.year
-              fyear += 1 if from_time.month >= 3
-              tyear = to_time.year
-              tyear -= 1 if to_time.month < 3
-              leap_years = (fyear > tyear) ? 0 : (fyear..tyear).count { |x| Date.leap?(x) }
-              minute_offset_for_leap_year = leap_years * 1440
-              # Discount the leap year days when calculating year distance.
-              # e.g. if there are 20 leap year days between 2 dates having the same day
-              # and month then the based on 365 days calculation
-              # the distance in years will come out to over 80 years when in written
-              # English it would read better as about 80 years.
-              minutes_with_offset = distance_in_minutes - minute_offset_for_leap_year
-            else
-              minutes_with_offset = distance_in_minutes
-            end
+            from_year = from_time.year
+            from_year += 1 if from_time.month >= 3
+            to_year = to_time.year
+            to_year -= 1 if to_time.month < 3
+            leap_years = (from_year > to_year) ? 0 : (from_year..to_year).count { |x| Date.leap?(x) }
+            minute_offset_for_leap_year = leap_years * 1440
+            # Discount the leap year days when calculating year distance.
+            # e.g. if there are 20 leap year days between 2 dates having the same day
+            # and month then the based on 365 days calculation
+            # the distance in years will come out to over 80 years when in written
+            # English it would read better as about 80 years.
+            minutes_with_offset = distance_in_minutes - minute_offset_for_leap_year
             remainder                   = (minutes_with_offset % MINUTES_IN_YEAR)
             distance_in_years           = (minutes_with_offset.div MINUTES_IN_YEAR)
             if remainder < MINUTES_IN_QUARTER_YEAR
@@ -687,6 +683,18 @@ def time_tag(date_or_time, *args, &block)
 
         content_tag("time".freeze, content, options.reverse_merge(datetime: datetime), &block)
       end
+
+      private
+
+        def normalize_distance_of_time_argument_to_time(value)
+          if value.is_a?(Numeric)
+            Time.at(value)
+          elsif value.respond_to?(:to_time)
+            value.to_time
+          else
+            raise ArgumentError, "#{value.inspect} can't be converted to a Time value"
+          end
+        end
     end
 
     class DateTimeSelector #:nodoc:

actionview/test/template/date_helper_test.rb

@@ -143,6 +143,16 @@ def test_distance_in_words_doesnt_use_the_quotient_operator
     klass.send :public, :/
   end
 
+  def test_distance_in_words_with_nil_input
+    assert_raises(ArgumentError) { distance_of_time_in_words(nil) }
+    assert_raises(ArgumentError) { distance_of_time_in_words(0, nil) }
+  end
+
+  def test_distance_in_words_with_mixed_argument_types
+    assert_equal "1 minute", distance_of_time_in_words(0, Time.at(60))
+    assert_equal "10 minutes", distance_of_time_in_words(Time.at(600), 0)
+  end
+
   def test_time_ago_in_words_passes_include_seconds
     assert_equal "less than 20 seconds", time_ago_in_words(15.seconds.ago, include_seconds: true)
     assert_equal "less than a minute", time_ago_in_words(15.seconds.ago, include_seconds: false)