diff --git a/activesupport/CHANGELOG.md b/activesupport/CHANGELOG.md index 04e45a01e2..86e5492439 100644 --- a/activesupport/CHANGELOG.md +++ b/activesupport/CHANGELOG.md @@ -1,3 +1,10 @@ +* Fix `ActiveSupport::Duration.build` to support negative values. + The algorithm to collect the `parts` of the `ActiveSupport::Duration` + ignored the sign of the `value` and accumulated incorrect part values. This + impacted `ActiveSupport::Duration#sum` (which is dependent on `parts`) but + not `ActiveSupport::Duration#eql?` (which is dependent on `value`). + + *Caleb Buxton*, *Braden Staudacher* Please check [7-0-stable](https://github.com/rails/rails/blob/7-0-stable/activesupport/CHANGELOG.md) for previous changes. diff --git a/activesupport/lib/active_support/duration.rb b/activesupport/lib/active_support/duration.rb index a393c9a07a..19986d1d0c 100644 --- a/activesupport/lib/active_support/duration.rb +++ b/activesupport/lib/active_support/duration.rb @@ -191,13 +191,14 @@ module ActiveSupport end parts = {} - remainder = value.round(9) + remainder_sign = value <=> 0 + remainder = value.round(9).abs variable = false PARTS.each do |part| unless part == :seconds part_in_seconds = PARTS_IN_SECONDS[part] - parts[part] = remainder.div(part_in_seconds) + parts[part] = remainder.div(part_in_seconds) * remainder_sign remainder %= part_in_seconds unless parts[part].zero? @@ -206,7 +207,7 @@ module ActiveSupport end end unless value == 0 - parts[:seconds] = remainder + parts[:seconds] = remainder * remainder_sign new(value, parts, variable) end diff --git a/activesupport/test/core_ext/duration_test.rb b/activesupport/test/core_ext/duration_test.rb index 5f38ccf290..f957daa894 100644 --- a/activesupport/test/core_ext/duration_test.rb +++ b/activesupport/test/core_ext/duration_test.rb @@ -752,6 +752,17 @@ class DurationTest < ActiveSupport::TestCase assert (1.day + 12.hours).variable? end + def test_duration_symmetry + time = Time.parse("Dec 7, 2021") + expected_time = Time.parse("2021-12-06 23:59:59") + + assert_equal expected_time, time + -1.second + assert_equal expected_time, time + ActiveSupport::Duration.build(1) * -1 + assert_equal expected_time, time + -ActiveSupport::Duration.build(1) + assert_equal expected_time, time + ActiveSupport::Duration::Scalar.new(-1) + assert_equal expected_time, time + ActiveSupport::Duration.build(-1) + end + private def eastern_time_zone if Gem.win_platform?