diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 14906e441b..d389d7f6fe 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,10 @@ +* Fixed an error which would occur in dirty checking when calling + `update_attributes` from a getter. + + Fixes #20531. + + *Sean Griffin* + * Make `remove_foreign_key` reversible. Any foreign key options must be specified, similar to `remove_column`. diff --git a/activerecord/lib/active_record/attribute_methods/dirty.rb b/activerecord/lib/active_record/attribute_methods/dirty.rb index 7ba907f786..0171ef3bdf 100644 --- a/activerecord/lib/active_record/attribute_methods/dirty.rb +++ b/activerecord/lib/active_record/attribute_methods/dirty.rb @@ -108,6 +108,7 @@ module ActiveRecord end def save_changed_attribute(attr, old_value) + clear_changed_attributes_cache if attribute_changed_by_setter?(attr) clear_attribute_changes(attr) unless _field_changed?(attr, old_value) else @@ -176,7 +177,11 @@ module ActiveRecord @cached_changed_attributes = changed_attributes yield ensure - remove_instance_variable(:@cached_changed_attributes) + clear_changed_attributes_cache + end + + def clear_changed_attributes_cache + remove_instance_variable(:@cached_changed_attributes) if defined?(@cached_changed_attributes) end end end diff --git a/activerecord/test/cases/dirty_test.rb b/activerecord/test/cases/dirty_test.rb index 216f228142..f5aaf22e13 100644 --- a/activerecord/test/cases/dirty_test.rb +++ b/activerecord/test/cases/dirty_test.rb @@ -703,6 +703,22 @@ class DirtyTest < ActiveRecord::TestCase assert pirate.catchphrase_changed?(from: "arrrr", to: "arrrr matey!") end + test "getters with side effects are allowed" do + klass = Class.new(Pirate) do + def catchphrase + if super.blank? + update_attribute(:catchphrase, "arr") # what could possibly go wrong? + end + super + end + end + + pirate = klass.create!(catchphrase: "lol") + pirate.update_attribute(:catchphrase, nil) + + assert_equal "arr", pirate.catchphrase + end + private def with_partial_writes(klass, on = true) old = klass.partial_writes?