Merge pull request #39254 from gmcgibbon/through_cast_check_attribute_override

Refactor uncastable through reflection test to detect join key overrides

activerecord/lib/active_record/reflection.rb

@@ -767,15 +767,7 @@ module ActiveRecord
 
       def klass
         @klass ||= delegate_reflection.compute_class(class_name).tap do |klass|
-          if !parent_reflection.is_a?(HasAndBelongsToManyReflection) &&
-             !(klass.reflections.key?(options[:through].to_s) ||
-               klass.reflections.key?(options[:through].to_s.pluralize)) &&
-             active_record.type_for_attribute(active_record.primary_key).type != :integer
-            raise NotImplementedError, <<~MSG.squish
-              In order to correctly type cast #{active_record}.#{active_record.primary_key},
-              #{klass} needs to define a :#{options[:through]} association.
-            MSG
-          end
+          check_reflection_validity!(klass)
         end
       end
 
@@ -1007,6 +999,26 @@ module ActiveRecord
           options[:source_type] || source_reflection.class_name
         end
 
+        def custom_join_key_type?
+          source_reflection.active_record.attributes_to_define_after_schema_loads.key?(
+            delegate_reflection.foreign_key
+          )
+        end
+
+        def check_reflection_validity!(klass)
+          return unless custom_join_key_type?
+
+          through_reflection = options[:through].to_s
+
+          unless klass.reflections.key?(through_reflection) ||
+                 klass.reflections.key?(through_reflection.pluralize)
+            raise NotImplementedError, <<~MSG.squish
+              In order to correctly type cast #{active_record}.#{active_record.primary_key},
+              #{klass} needs to define a :#{through_reflection} association.
+            MSG
+          end
+        end
+
         delegate_methods = AssociationReflection.public_instance_methods -
           public_instance_methods
 

activerecord/test/cases/reflection_test.rb

@@ -519,17 +519,33 @@ class ReflectionTest < ActiveRecord::TestCase
     end
 end
 
-class UncastableReflectionTest < ActiveRecord::TestCase
+class UncastableOverriddenAttributeReflectionTest < ActiveRecord::TestCase
+  class NickType < ActiveRecord::Type::Binary
+    def serialize(value)
+      super("nickname-#{value}")
+    end
+
+    def deserialize(value)
+      super(value).to_s.delete_prefix("nickname-")
+    end
+
+    def cast_value(value)
+      value.to_s
+    end
+  end
+
   class Book < ActiveRecord::Base
   end
 
   class Subscription < ActiveRecord::Base
     belongs_to :subscriber
     belongs_to :book
+    attribute :subscriber_id, NickType.new
   end
 
   class Subscriber < ActiveRecord::Base
     self.primary_key = "nick"
+    attribute :nick, NickType.new
     has_many :subscriptions
     has_one :subscription
     has_many :books, through: :subscriptions
@@ -552,17 +568,16 @@ class UncastableReflectionTest < ActiveRecord::TestCase
   test "uncastable has_many through: reflection" do
     error = assert_raises(NotImplementedError) { @subscriber.books }
     assert_equal <<~MSG.squish, error.message
-      In order to correctly type cast UncastableReflectionTest::Subscriber.nick,
-      UncastableReflectionTest::Book needs to define a :subscriptions association.
+      In order to correctly type cast #{self.class}::Subscriber.nick,
+      #{self.class}::Book needs to define a :subscriptions association.
     MSG
   end
 
   test "uncastable has_one through: reflection" do
     error = assert_raises(NotImplementedError) { @subscriber.book }
-
     assert_equal <<~MSG.squish, error.message
-      In order to correctly type cast UncastableReflectionTest::Subscriber.nick,
-      UncastableReflectionTest::Book needs to define a :subscription association.
+      In order to correctly type cast #{self.class}::Subscriber.nick,
+      #{self.class}::Book needs to define a :subscription association.
     MSG
   end