Skip to content

Commit 2ea5ff9

Browse files
committed
Use polymorphic_class_for over constantize
This commit fixes 2 cases where `constantize` was used instead of of calling `polymorphic_class_for` to retrieve the class associated with a given string for polymorphic `belongs_to`.
1 parent e1f90a3 commit 2ea5ff9

File tree

4 files changed

+53
-2
lines changed

4 files changed

+53
-2
lines changed

activerecord/CHANGELOG.md

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,12 @@
1+
* Fix 2 cases that inferred polymorphic class from the association's `foreign_type`
2+
using `String#constantize` instead of the model's `polymorphic_class_for`.
3+
4+
When updating a polymorphic association, the old `foreign_type` was not inferred correctly when:
5+
1. `touch`ing the previously associated record
6+
2. updating the previously associated record's `counter_cache`
7+
8+
*Jimmy Bourassa*
9+
110
* Add config option for ignoring tables when dumping the schema cache.
211

312
Applications can now be configured to ignore certain tables when dumping the schema cache.

activerecord/lib/active_record/associations/belongs_to_association.rb

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,8 @@ def increment_counters
5555

5656
def decrement_counters_before_last_save
5757
if reflection.polymorphic?
58-
model_was = owner.attribute_before_last_save(reflection.foreign_type)&.constantize
58+
model_type_was = owner.attribute_before_last_save(reflection.foreign_type)
59+
model_was = owner.class.polymorphic_class_for(model_type_was) if model_type_was
5960
else
6061
model_was = klass
6162
end

activerecord/lib/active_record/associations/builder/belongs_to.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ def self.touch_record(o, changes, foreign_key, name, touch, touch_method) # :nod
4949
if reflection.polymorphic?
5050
foreign_type = reflection.foreign_type
5151
klass = changes[foreign_type] && changes[foreign_type].first || o.public_send(foreign_type)
52-
klass = klass.constantize
52+
klass = o.class.polymorphic_class_for(klass)
5353
else
5454
klass = association.klass
5555
end

activerecord/test/cases/associations/belongs_to_associations_test.rb

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1331,6 +1331,47 @@ def test_polymorphic_with_custom_foreign_type
13311331
assert_equal groucho, sponsor.thing
13321332
end
13331333

1334+
class WheelPolymorphicName < ActiveRecord::Base
1335+
self.table_name = "wheels"
1336+
belongs_to :wheelable, polymorphic: true, counter_cache: :wheels_count, touch: :wheels_owned_at
1337+
1338+
def self.polymorphic_class_for(name)
1339+
raise "Unexpected name: #{name}" unless name == "polymorphic_car"
1340+
CarPolymorphicName
1341+
end
1342+
end
1343+
1344+
class CarPolymorphicName < ActiveRecord::Base
1345+
self.table_name = "cars"
1346+
has_many :wheels, as: :wheelable
1347+
1348+
def self.polymorphic_name
1349+
"polymorphic_car"
1350+
end
1351+
end
1352+
1353+
def test_polymorphic_with_custom_name_counter_cache
1354+
car = CarPolymorphicName.create!
1355+
wheel = WheelPolymorphicName.create!(wheelable_type: "polymorphic_car", wheelable_id: car.id)
1356+
assert_equal 1, car.reload.wheels_count
1357+
1358+
wheel.update! wheelable: nil
1359+
1360+
assert_equal 0, car.reload.wheels_count
1361+
end
1362+
1363+
def test_polymorphic_with_custom_name_touch_old_belongs_to_model
1364+
car = CarPolymorphicName.create!
1365+
wheel = WheelPolymorphicName.create!(wheelable: car)
1366+
1367+
touch_time = 1.day.ago.round
1368+
travel_to(touch_time) do
1369+
wheel.update!(wheelable: nil)
1370+
end
1371+
1372+
assert_equal touch_time, car.reload.wheels_owned_at
1373+
end
1374+
13341375
def test_build_with_conditions
13351376
client = companies(:second_client)
13361377
firm = client.build_bob_firm

0 commit comments

Comments
 (0)