Permalink
Browse files

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`.
  • Loading branch information...
Envek committed Dec 27, 2015
1 parent da2dc68 commit 434df0016e228a7d51f1ad0c3d1f89faeffbed9a
View
@@ -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
@@ -25,17 +25,17 @@ def seconds
# 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 @@ def days
# 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
@@ -120,7 +120,7 @@ def ago(time = ::Time.current)
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 @@ def sum(sign, time = ::Time.current) #:nodoc:
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
@@ -66,8 +66,9 @@ def test_inspect
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 @@ def test_plus_with_time
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('')

0 comments on commit 434df00

Please sign in to comment.