Merge pull request rails#43148 from jbourassa/polymorphic-custom-name-fixes

Use `polymorphic_class_for` over `constantize`

Author: byroot

activerecord/CHANGELOG.md

@@ -1,3 +1,12 @@
+*   Fix 2 cases that inferred polymorphic class from the association's `foreign_type`
+    using `String#constantize` instead of the model's `polymorphic_class_for`.
+
+    When updating a polymorphic association, the old `foreign_type` was not inferred correctly when:
+    1. `touch`ing the previously associated record
+    2. updating the previously associated record's `counter_cache`
+
+    *Jimmy Bourassa*
+
 *   Add config option for ignoring tables when dumping the schema cache.
 
     Applications can now be configured to ignore certain tables when dumping the schema cache.

activerecord/lib/active_record/associations/belongs_to_association.rb

@@ -55,7 +55,8 @@ def increment_counters
 
       def decrement_counters_before_last_save
         if reflection.polymorphic?
-          model_was = owner.attribute_before_last_save(reflection.foreign_type)&.constantize
+          model_type_was = owner.attribute_before_last_save(reflection.foreign_type)
+          model_was = owner.class.polymorphic_class_for(model_type_was) if model_type_was
         else
           model_was = klass
         end

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

@@ -49,7 +49,7 @@ def self.touch_record(o, changes, foreign_key, name, touch, touch_method) # :nod
         if reflection.polymorphic?
           foreign_type = reflection.foreign_type
           klass = changes[foreign_type] && changes[foreign_type].first || o.public_send(foreign_type)
-          klass = klass.constantize
+          klass = o.class.polymorphic_class_for(klass)
         else
           klass = association.klass
         end

activerecord/test/cases/associations/belongs_to_associations_test.rb

@@ -1331,6 +1331,47 @@ def test_polymorphic_with_custom_foreign_type
     assert_equal groucho, sponsor.thing
   end
 
+  class WheelPolymorphicName < ActiveRecord::Base
+    self.table_name = "wheels"
+    belongs_to :wheelable, polymorphic: true, counter_cache: :wheels_count, touch: :wheels_owned_at
+
+    def self.polymorphic_class_for(name)
+      raise "Unexpected name: #{name}" unless name == "polymorphic_car"
+      CarPolymorphicName
+    end
+  end
+
+  class CarPolymorphicName < ActiveRecord::Base
+    self.table_name = "cars"
+    has_many :wheels, as: :wheelable
+
+    def self.polymorphic_name
+      "polymorphic_car"
+    end
+  end
+
+  def test_polymorphic_with_custom_name_counter_cache
+    car = CarPolymorphicName.create!
+    wheel = WheelPolymorphicName.create!(wheelable_type: "polymorphic_car", wheelable_id: car.id)
+    assert_equal 1, car.reload.wheels_count
+
+    wheel.update! wheelable: nil
+
+    assert_equal 0, car.reload.wheels_count
+  end
+
+  def test_polymorphic_with_custom_name_touch_old_belongs_to_model
+    car = CarPolymorphicName.create!
+    wheel = WheelPolymorphicName.create!(wheelable: car)
+
+    touch_time = 1.day.ago.round
+    travel_to(touch_time) do
+      wheel.update!(wheelable: nil)
+    end
+
+    assert_equal touch_time, car.reload.wheels_owned_at
+  end
+
   def test_build_with_conditions
     client = companies(:second_client)
     firm   = client.build_bob_firm