Merge pull request rails#43386 from rdimartino/dirty_store

Fix tracking previous changes for ActiveRecord::Store accessors with underlying JSON data column

Author: eileencodes

activerecord/CHANGELOG.md

@@ -1,3 +1,9 @@
+*   Fix previous change tracking for `ActiveRecord::Store` when using a column with JSON structured database type
+
+    Before, the methods to access the changes made during the last save `#saved_change_to_key?`, `#saved_change_to_key`, and `#key_before_last_save` did not work if the store was defined as a `store_accessor` on a column with a JSON structured database type
+
+    *Robert DiMartino*
+
 *   Fully support `NULLS [NOT] DISTINCT` for PostgreSQL 15+ indexes.
 
     Previous work was done to allow the index to be created in a migration, but it was not

activerecord/lib/active_record/store.rb

@@ -163,19 +163,19 @@ def store_accessor(store_attribute, *keys, prefix: nil, suffix: nil)
 
             define_method("saved_change_to_#{accessor_key}?") do
               return false unless saved_change_to_attribute?(store_attribute)
-              prev_store, new_store = saved_change_to_attribute(store_attribute)
+              prev_store, new_store = saved_changes[store_attribute]
               prev_store&.dig(key) != new_store&.dig(key)
             end
 
             define_method("saved_change_to_#{accessor_key}") do
               return unless saved_change_to_attribute?(store_attribute)
-              prev_store, new_store = saved_change_to_attribute(store_attribute)
+              prev_store, new_store = saved_changes[store_attribute]
               [prev_store&.dig(key), new_store&.dig(key)]
             end
 
             define_method("#{accessor_key}_before_last_save") do
               return unless saved_change_to_attribute?(store_attribute)
-              prev_store, _new_store = saved_change_to_attribute(store_attribute)
+              prev_store, _new_store = saved_changes[store_attribute]
               prev_store&.dig(key)
             end
           end

activerecord/test/cases/store_test.rb

@@ -163,6 +163,23 @@ class StoreTest < ActiveRecord::TestCase
     assert_equal "Dallas", @john.partner_name_before_last_save
   end
 
+  test "saved changes tracking for accessors with json column" do
+    if current_adapter?(:Mysql2Adapter, :TrilogyAdapter) && ActiveRecord::Base.connection.mariadb?
+      skip "MariaDB doesn't support JSON store_accessor"
+    end
+    @john.enable_friend_requests = true
+    assert @john.enable_friend_requests_changed?
+
+    @john.save!
+    assert_not @john.enable_friend_requests_change
+    assert @john.saved_change_to_enable_friend_requests?
+    assert_equal [nil, true], @john.saved_change_to_enable_friend_requests
+    # Make a second change to test key_before_last_save
+    @john.enable_friend_requests = false
+    @john.save
+    assert_equal true, @john.enable_friend_requests_before_last_save
+  end
+
   test "object initialization with not nullable column" do
     assert_equal true, @john.remember_login
   end

activerecord/test/models/admin/user.rb

@@ -28,6 +28,7 @@ def load(s)
   store :preferences, accessors: [ :remember_login ]
   store :json_data, accessors: [ :height, :weight ], coder: Coder.new
   store :json_data_empty, accessors: [ :is_a_good_guy ], coder: Coder.new
+  store_accessor :json_options, :enable_friend_requests
 
   def phone_number
     read_store_attribute(:settings, :phone_number).gsub(/(\d{3})(\d{3})(\d{4})/, '(\1) \2-\3')

activerecord/test/schema/schema.rb

@@ -38,6 +38,7 @@
     t.string :json_data_empty, null: true, default: "", limit: 1024
     t.text :params
     t.references :account
+    t.json :json_options
   end
 
   create_table :admin_user_jsons, force: true do |t|