Maintain current timezone when changing time during DST overlap

Currently if a time is changed during DST overlap in the autumn then the
method `period_for_local` will return the DST period. However if the
original time is not DST then this can be surprising and is not what is
generally wanted. This commit changes that behavior to maintain the current
period if it's in the list of periods returned by `periods_for_local`.

It is possible to alter the behavior of `period_for_local` by specifying a
second argument but since we may be change from another time that could be
either DST or not then this would give inconsistent results.

Fixes #12163.

4 files changed, 30 insertions, 3 deletions

diff --git a/activesupport/CHANGELOG.md b/activesupport/CHANGELOG.md
index adaeb99c87..ba450b85c4 100644
--- a/activesupport/CHANGELOG.md
+++ b/activesupport/CHANGELOG.md
@@ -1,3 +1,15 @@
+*   Maintain the current timezone when calling `change` during DST overlap
+
+    Currently if a time is changed during DST overlap in the autumn then the method
+    `period_for_local` will return the DST period. However if the original time is
+    not DST then this can be surprising and is not what is generally wanted. This
+    commit changes that behavior to maintain the current period if it's in the list
+    of periods returned by `periods_for_local`.
+
+    Fixes #12163.
+
+    *Andrew White*
+
 *   Added `Hash#compact` and `Hash#compact!` for removing items with nil value from hash.
 
     *Celestino Gomes*
diff --git a/activesupport/lib/active_support/time_with_zone.rb b/activesupport/lib/active_support/time_with_zone.rb
index 50db7da9d9..d47aee00f4 100644
--- a/activesupport/lib/active_support/time_with_zone.rb
+++ b/activesupport/lib/active_support/time_with_zone.rb
@@ -45,7 +45,7 @@ module ActiveSupport
 
     def initialize(utc_time, time_zone, local_time = nil, period = nil)
       @utc, @time_zone, @time = utc_time, time_zone, local_time
-      @period = @utc ? period : get_period_and_ensure_valid_local_time
+      @period = @utc ? period : get_period_and_ensure_valid_local_time(period)
     end
 
     # Returns a Time or DateTime instance that represents the time in +time_zone+.
@@ -292,6 +292,12 @@ module ActiveSupport
       end
     end
 
+    def change(options)
+      new_time = time.change(options)
+      periods = time_zone.periods_for_local(new_time)
+      self.class.new(nil, time_zone, new_time, periods.include?(period) ? period : nil)
+    end
+
     %w(year mon month day mday wday yday hour min sec usec nsec to_date).each do |method_name|
       class_eval <<-EOV, __FILE__, __LINE__ + 1
         def #{method_name}    # def month
@@ -367,12 +373,12 @@ module ActiveSupport
     end
 
     private
-      def get_period_and_ensure_valid_local_time
+      def get_period_and_ensure_valid_local_time(period)
         # we don't want a Time.local instance enforcing its own DST rules as well,
         # so transfer time values to a utc constructor if necessary
         @time = transfer_time_values_to_utc_constructor(@time) unless @time.utc?
         begin
-          @time_zone.period_for_local(@time)
+          period || @time_zone.period_for_local(@time)
         rescue ::TZInfo::PeriodNotFound
           # time is in the "spring forward" hour gap, so we're moving the time forward one hour and trying again
           @time += 1.hour
diff --git a/activesupport/lib/active_support/values/time_zone.rb b/activesupport/lib/active_support/values/time_zone.rb
index beaac42fa1..eb785d46ce 100644
--- a/activesupport/lib/active_support/values/time_zone.rb
+++ b/activesupport/lib/active_support/values/time_zone.rb
@@ -352,6 +352,10 @@ module ActiveSupport
       tzinfo.period_for_local(time, dst)
     end
 
+    def periods_for_local(time) #:nodoc:
+      tzinfo.periods_for_local(time)
+    end
+
     def self.find_tzinfo(name)
       TZInfo::TimezoneProxy.new(MAPPING[name] || name)
     end
diff --git a/activesupport/test/core_ext/time_with_zone_test.rb b/activesupport/test/core_ext/time_with_zone_test.rb
index 5494824a40..43fe572bfc 100644
--- a/activesupport/test/core_ext/time_with_zone_test.rb
+++ b/activesupport/test/core_ext/time_with_zone_test.rb
@@ -511,6 +511,11 @@ class TimeWithZoneTest < ActiveSupport::TestCase
     assert_equal "Fri, 31 Dec 1999 19:00:30 EST -05:00", @twz.change(:sec => 30).inspect
   end
 
+  def test_change_at_dst_boundary
+    twz = ActiveSupport::TimeWithZone.new(Time.at(1319936400).getutc, ActiveSupport::TimeZone['Madrid'])
+    assert_equal twz, twz.change(:min => 0)
+  end
+
   def test_advance
     assert_equal "Fri, 31 Dec 1999 19:00:00 EST -05:00", @twz.inspect
     assert_equal "Mon, 31 Dec 2001 19:00:00 EST -05:00", @twz.advance(:years => 2).inspect