From 756de667ae9a4b17a7ab6c7e4b17e73e67c27cba Mon Sep 17 00:00:00 2001 From: Jay Hayes Date: Thu, 25 Jun 2015 17:54:32 -0500 Subject: [PATCH] Ensure input to distance_of_time_in_words is not nil * Internally all input is converted to time so that it can be treated uniformly. Remove now-unneeded condition * Now that all input is treated is converted to time, we no longer need to type check it. Rename variables to clarify their purpose Extract private method to normalize distance_of_time args to time Update actionview changelog --- actionview/CHANGELOG.md | 5 +++ .../lib/action_view/helpers/date_helper.rb | 44 +++++++++++-------- actionview/test/template/date_helper_test.rb | 10 +++++ 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" }