Merge pull request #20701 from iamvery/date-helper-argument-error

Ensure input to distance_of_time_in_words is not nil

3 files changed, 41 insertions, 18 deletions

diff --git a/actionview/CHANGELOG.md b/actionview/CHANGELOG.md
index c514e757c8..1fc38e76b2 100644
--- a/actionview/CHANGELOG.md
+++ b/actionview/CHANGELOG.md
@@ -1 +1,6 @@
+*   Update distance_of_time_in_words helper to display better error messages
+    for bad input.
+
+    *Jay Hayes*
+
 Please check [5-1-stable](https://github.com/rails/rails/blob/5-1-stable/actionview/CHANGELOG.md) for previous changes.
diff --git a/actionview/lib/action_view/helpers/date_helper.rb b/actionview/lib/action_view/helpers/date_helper.rb
index 09dc6ef6bd..3f43465aa4 100644
--- a/actionview/lib/action_view/helpers/date_helper.rb
+++ b/actionview/lib/action_view/helpers/date_helper.rb
@@ -95,8 +95,8 @@ module ActionView
           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 @@ module ActionView
             # 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 @@ module ActionView
 
         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:
diff --git a/actionview/test/template/date_helper_test.rb b/actionview/test/template/date_helper_test.rb
index d257147e1f..a259752d6a 100644
--- a/actionview/test/template/date_helper_test.rb
+++ b/actionview/test/template/date_helper_test.rb
@@ -128,6 +128,16 @@ class DateHelperTest < ActionView::TestCase
     assert_distance_of_time_in_words(from)
   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_distance_in_words_with_mathn_required
     # test we avoid Integer#/ (redefined by mathn)
     silence_warnings { require "mathn" }