Merge pull request rails#47767 from joshuay03/fix-updated-at-not-updating-in-before-update

Fix rails#45389 | `updated_at` not updating in `before_update` callback

Author: byroot

activerecord/lib/active_record/callbacks.rb

@@ -446,7 +446,7 @@ def _create_record
     end
 
     def _update_record
-      _run_update_callbacks { super }
+      _run_update_callbacks { record_update_timestamps { super } }
     end
   end
 end

activerecord/lib/active_record/timestamp.rb

@@ -115,6 +115,17 @@ def _create_record
     end
 
     def _update_record
+      record_update_timestamps
+
+      super
+    end
+
+    def create_or_update(touch: true, **)
+      @_touch_record = touch
+      super
+    end
+
+    def record_update_timestamps
       if @_touch_record && should_record_timestamps?
         current_time = current_time_from_proper_timezone
 
@@ -124,12 +135,7 @@ def _update_record
         end
       end
 
-      super
-    end
-
-    def create_or_update(touch: true, **)
-      @_touch_record = touch
-      super
+      yield if block_given?
     end
 
     def should_record_timestamps?

activerecord/test/cases/timestamp_test.rb

@@ -253,6 +253,60 @@ def change_name
     assert_not_equal @previously_updated_at, @developer.updated_at
   end
 
+  def test_saving_an_unchanged_record_with_a_mutating_before_update_callback_updates_its_timestamp
+    klass = Class.new(ActiveRecord::Base) do
+      self.table_name = "developers"
+
+      include Developer::TimestampAliases
+
+      before_update :change_name
+
+      private
+        def change_name
+          return if new_record?
+
+          self.name = "Jack Bauer"
+        end
+    end
+
+    @developer = klass.create!
+    @previously_updated_at = @developer.updated_at
+    @previous_name = @developer.name
+
+    travel(1.second) do
+      @developer.save!
+    end
+
+    @developer.reload
+
+    assert_not_equal @previous_name, @developer.name
+    assert_not_equal @previously_updated_at, @developer.updated_at
+  end
+
+  def test_saving_an_unchanged_record_with_a_non_mutating_before_update_callback_does_not_update_its_timestamp
+    klass = Class.new(ActiveRecord::Base) do
+      self.table_name = "developers"
+
+      include Developer::TimestampAliases
+
+      before_update :change_name
+
+      private
+        def change_name; end
+    end
+
+    @developer = klass.create!
+    @previously_updated_at = @developer.updated_at
+
+    travel(1.second) do
+      @developer.save!
+    end
+
+    @developer.reload
+
+    assert_equal @previously_updated_at, @developer.updated_at
+  end
+
   def test_saving_a_record_with_a_belongs_to_that_specifies_touching_the_parent_should_update_the_parent_updated_at
     pet   = Pet.first
     owner = pet.owner
@@ -469,7 +523,34 @@ def self.name; "Toy"; end
     assert_not_equal time, pet.updated_at
   end
 
-  def test_timestamp_column_values_are_present_in_the_callbacks
+  def test_timestamp_column_values_are_present_in_create_callbacks
+    klass = Class.new(ActiveRecord::Base) do
+      self.table_name = "people"
+
+      before_create do
+        self.born_at = created_at
+      end
+    end
+
+    person = klass.create first_name: "David"
+    assert_not_nil person.born_at
+  end
+
+  def test_timestamp_column_values_are_present_in_update_callbacks
+    klass = Class.new(ActiveRecord::Base) do
+      self.table_name = "people"
+
+      before_update do
+        self.born_at = created_at
+      end
+    end
+
+    person = klass.create first_name: "David"
+    person.update first_name: "John"
+    assert_not_nil person.born_at
+  end
+
+  def test_timestamp_column_values_are_present_in_save_callbacks
     klass = Class.new(ActiveRecord::Base) do
       self.table_name = "people"
 
@@ -479,7 +560,7 @@ def test_timestamp_column_values_are_present_in_the_callbacks
     end
 
     person = klass.create first_name: "David"
-    assert_not_equal person.born_at, nil
+    assert_not_nil person.born_at
   end
 
   def test_timestamp_attributes_for_create_in_model