From 434df0016e228a7d51f1ad0c3d1f89faeffbed9a Mon Sep 17 00:00:00 2001 From: Andrey Novikov Date: Sun, 27 Dec 2015 19:32:31 +0300 Subject: [PATCH] Change 1.week to create 1 week durations instead of 7 days durations. This is just to remove astonishment from getting `3600 seconds` from typing `1.hour`. --- activesupport/CHANGELOG.md | 22 +++++++++++++++++++ .../active_support/core_ext/numeric/time.rb | 16 +++++++------- activesupport/lib/active_support/duration.rb | 6 ++++- activesupport/test/core_ext/duration_test.rb | 14 ++++++++++-- 4 files changed, 47 insertions(+), 11 deletions(-) diff --git a/activesupport/CHANGELOG.md b/activesupport/CHANGELOG.md index d1b3147cef..cc27b6a0fc 100644 --- a/activesupport/CHANGELOG.md +++ b/activesupport/CHANGELOG.md @@ -1,3 +1,25 @@ +* Change `ActiveSupport::Duration` creation with numeric methods like `1.week` + to create durations with more predictable and ISO8601-conformant parts. + + This is to remove astonishment from getting `3600 seconds` from `1.hour`. + + It should not affect current apps as duration's `value` (number of seconds) remains the same, + only hash of `parts` (and `inspect` value) is changed and only when it's constructed by calling + methods on integers. Manual construction of Durations isn't affected. + Inside the ActiveSupport::Duration itself most operations rely only on number of seconds. + + Before: + + [1.hour.inspect, 1.hour.value, 1.hour.parts] # => ["3600 seconds", 3600, [[:seconds, 3600]]] + [1.week.inspect, 1.week.value, 1.week.parts] # => ["7 days", 604800, [[:days, 7]]] + + After: + + [1.hour.inspect, 1.hour.value, 1.hour.parts] # => ["1 hour", 3600, [[:hours, 1]]] + [1.week.inspect, 1.week.value, 1.week.parts] # => ["1 week", 604800, [[:weeks, 1]]] + + *Andrey Novikov* + ## Rails 5.0.0.beta4 (April 27, 2016) ## * Time zones: Ensure that the UTC offset reflects DST changes that occurred diff --git a/activesupport/lib/active_support/core_ext/numeric/time.rb b/activesupport/lib/active_support/core_ext/numeric/time.rb index 6c4a975495..c6ece22f8d 100644 --- a/activesupport/lib/active_support/core_ext/numeric/time.rb +++ b/activesupport/lib/active_support/core_ext/numeric/time.rb @@ -25,17 +25,17 @@ class Numeric # Returns a Duration instance matching the number of minutes provided. # - # 2.minutes # => 120 seconds + # 2.minutes # => 2 minutes def minutes - ActiveSupport::Duration.new(self * 60, [[:seconds, self * 60]]) + ActiveSupport::Duration.new(self * 60, [[:minutes, self]]) end alias :minute :minutes # Returns a Duration instance matching the number of hours provided. # - # 2.hours # => 7_200 seconds + # 2.hours # => 2 hours def hours - ActiveSupport::Duration.new(self * 3600, [[:seconds, self * 3600]]) + ActiveSupport::Duration.new(self * 3600, [[:hours, self]]) end alias :hour :hours @@ -49,17 +49,17 @@ class Numeric # Returns a Duration instance matching the number of weeks provided. # - # 2.weeks # => 14 days + # 2.weeks # => 2 weeks def weeks - ActiveSupport::Duration.new(self * 7.days, [[:days, self * 7]]) + ActiveSupport::Duration.new(self * 7.days, [[:weeks, self]]) end alias :week :weeks # Returns a Duration instance matching the number of fortnights provided. # - # 2.fortnights # => 28 days + # 2.fortnights # => 4 weeks def fortnights - ActiveSupport::Duration.new(self * 2.weeks, [[:days, self * 14]]) + ActiveSupport::Duration.new(self * 2.weeks, [[:weeks, self * 2]]) end alias :fortnight :fortnights diff --git a/activesupport/lib/active_support/duration.rb b/activesupport/lib/active_support/duration.rb index c717e7f357..47d09f4f5a 100644 --- a/activesupport/lib/active_support/duration.rb +++ b/activesupport/lib/active_support/duration.rb @@ -120,7 +120,7 @@ module ActiveSupport def inspect #:nodoc: parts. reduce(::Hash.new(0)) { |h,(l,r)| h[l] += r; h }. - sort_by {|unit, _ | [:years, :months, :days, :minutes, :seconds].index(unit)}. + sort_by {|unit, _ | [:years, :months, :weeks, :days, :hours, :minutes, :seconds].index(unit)}. map {|unit, val| "#{val} #{val == 1 ? unit.to_s.chop : unit.to_s}"}. to_sentence(locale: ::I18n.default_locale) end @@ -159,6 +159,10 @@ module ActiveSupport if t.acts_like?(:time) || t.acts_like?(:date) if type == :seconds t.since(sign * number) + elsif type == :minutes + t.since(sign * number * 60) + elsif type == :hours + t.since(sign * number * 3600) else t.advance(type => sign * number) end diff --git a/activesupport/test/core_ext/duration_test.rb b/activesupport/test/core_ext/duration_test.rb index 69c2c0d936..ce69364c68 100644 --- a/activesupport/test/core_ext/duration_test.rb +++ b/activesupport/test/core_ext/duration_test.rb @@ -66,8 +66,9 @@ class DurationTest < ActiveSupport::TestCase assert_equal '10 years, 2 months, and 1 day', (10.years + 2.months + 1.day).inspect assert_equal '10 years, 2 months, and 1 day', (10.years + 1.month + 1.day + 1.month).inspect assert_equal '10 years, 2 months, and 1 day', (1.day + 10.years + 2.months).inspect - assert_equal '7 days', 1.week.inspect - assert_equal '14 days', 1.fortnight.inspect + assert_equal '7 days', 7.days.inspect + assert_equal '1 week', 1.week.inspect + assert_equal '2 weeks', 1.fortnight.inspect end def test_inspect_locale @@ -87,6 +88,15 @@ class DurationTest < ActiveSupport::TestCase assert_equal 1 + 1.second, 1.second + 1, "Duration + Numeric should == Numeric + Duration" end + def test_time_plus_duration_returns_same_time_datatype + twz = ActiveSupport::TimeWithZone.new(nil, ActiveSupport::TimeZone['Moscow'] , Time.utc(2016,4,28,00,45)) + now = Time.now.utc + %w( second minute hour day week month year ).each do |unit| + assert_equal((now + 1.send(unit)).class, Time, "Time + 1.#{unit} must be Time") + assert_equal((twz + 1.send(unit)).class, ActiveSupport::TimeWithZone, "TimeWithZone + 1.#{unit} must be TimeWithZone") + end + end + def test_argument_error e = assert_raise ArgumentError do 1.second.ago('')