Fix dirty tracking for `touch`

Before this fix, `touch` only clears dirty tracking for touched
attributes, doesn't track saved (touched) changes.

This fixes that tracks saved changes and carry over remaining changes.

Fixes #33429.
Closes #34306.

activerecord/lib/active_record/attribute_methods/dirty.rb

@@ -166,6 +166,30 @@ module ActiveRecord
           result
         end
 
+        def _touch_row(attribute_names, time)
+          @_touch_attr_names = Set.new(attribute_names)
+
+          affected_rows = super
+
+          changes = {}
+          @attributes.keys.each do |attr_name|
+            next if @_touch_attr_names.include?(attr_name)
+
+            if attribute_changed?(attr_name)
+              changes[attr_name] = _read_attribute(attr_name)
+              _write_attribute(attr_name, attribute_was(attr_name))
+              clear_attribute_change(attr_name)
+            end
+          end
+
+          changes_applied
+          changes.each { |attr_name, value| _write_attribute(attr_name, value) }
+
+          affected_rows
+        ensure
+          @_touch_attr_names = nil
+        end
+
         def _update_record(attribute_names = attribute_names_for_partial_writes)
           affected_rows = super
           changes_applied

activerecord/lib/active_record/locking/optimistic.rb

@@ -71,9 +71,8 @@ module ActiveRecord
         end
 
         def _touch_row(attribute_names, time)
+          @_touch_attr_names << self.class.locking_column if locking_enabled?
           super
-        ensure
-          clear_attribute_change(self.class.locking_column) if locking_enabled?
         end
 
         def _update_row(attribute_names, attempted_action = "update")

activerecord/lib/active_record/persistence.rb

@@ -851,7 +851,9 @@ module ActiveRecord
       end
 
       attribute_names = timestamp_attributes_for_update_in_model
-      attribute_names |= names.map(&:to_s)
+      attribute_names |= names.map!(&:to_s).map! { |name|
+        self.class.attribute_alias?(name) ? self.class.attribute_alias(name) : name
+      }
 
       unless attribute_names.empty?
         affected_rows = _touch_row(attribute_names, time)
@@ -879,8 +881,7 @@ module ActiveRecord
       time ||= current_time_from_proper_timezone
 
       attribute_names.each do |attr_name|
-        write_attribute(attr_name, time)
-        clear_attribute_change(attr_name)
+        _write_attribute(attr_name, time)
       end
 
       _update_row(attribute_names, "touch")

activerecord/test/cases/locking_test.rb

@@ -182,7 +182,9 @@ class OptimisticLockingTest < ActiveRecord::TestCase
 
     p1.touch
     assert_equal 1, p1.lock_version
-    assert_not p1.changed?, "Changes should have been cleared"
+    assert_not_predicate p1, :changed?, "Changes should have been cleared"
+    assert_predicate p1, :saved_changes?
+    assert_equal ["lock_version", "updated_at"], p1.saved_changes.keys.sort
   end
 
   def test_touch_stale_object
@@ -193,6 +195,8 @@ class OptimisticLockingTest < ActiveRecord::TestCase
     assert_raises(ActiveRecord::StaleObjectError) do
       stale_person.touch
     end
+
+    assert_not_predicate stale_person, :saved_changes?
   end
 
   def test_update_with_dirty_primary_key
@@ -296,6 +300,9 @@ class OptimisticLockingTest < ActiveRecord::TestCase
     t1.touch
 
     assert_equal 1, t1.lock_version
+    assert_not_predicate t1, :changed?
+    assert_predicate t1, :saved_changes?
+    assert_equal ["lock_version", "updated_at"], t1.saved_changes.keys.sort
   end
 
   def test_touch_stale_object_with_lock_without_default
@@ -307,6 +314,8 @@ class OptimisticLockingTest < ActiveRecord::TestCase
     assert_raises(ActiveRecord::StaleObjectError) do
       stale_object.touch
     end
+
+    assert_not_predicate stale_object, :saved_changes?
   end
 
   def test_lock_without_default_should_work_with_null_in_the_database

activerecord/test/cases/timestamp_test.rb

@@ -40,17 +40,25 @@ class TimestampTest < ActiveRecord::TestCase
 
     assert_not_equal @previously_updated_at, @developer.updated_at
     assert_equal previous_salary + 10000, @developer.salary
-    assert @developer.salary_changed?, "developer salary should have changed"
-    assert @developer.changed?, "developer should be marked as changed"
+    assert_predicate @developer, :salary_changed?, "developer salary should have changed"
+    assert_predicate @developer, :changed?, "developer should be marked as changed"
+    assert_equal ["salary"], @developer.changed
+    assert_predicate @developer, :saved_changes?
+    assert_equal ["updated_at", "updated_on"], @developer.saved_changes.keys.sort
+
     @developer.reload
     assert_equal previous_salary, @developer.salary
   end
 
   def test_touching_a_record_with_default_scope_that_excludes_it_updates_its_timestamp
     developer = @developer.becomes(DeveloperCalledJamis)
-
     developer.touch
+
     assert_not_equal @previously_updated_at, developer.updated_at
+    assert_not_predicate developer, :changed?
+    assert_predicate developer, :saved_changes?
+    assert_equal ["updated_at", "updated_on"], developer.saved_changes.keys.sort
+
     developer.reload
     assert_not_equal @previously_updated_at, developer.updated_at
   end