Add test for collection *_ids= setter when association primary key set

Fixes casting of IDs to the data type of the association primary key,
rather than then the data type of the model's primary key. (Tests use a
string primary key on the association, rather than an int.)

Tests issue #20995

4 files changed, 23 insertions, 1 deletions

diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md
index f4218cc110..35f7c158a6 100644
--- a/activerecord/CHANGELOG.md
+++ b/activerecord/CHANGELOG.md
@@ -1,6 +1,10 @@
 *   Raise ActiveRecord::RecordNotFound from collection `*_ids` setters
     for unknown IDs with a better error message.
 
+    Changes the collection `*_ids` setters to cast provided IDs the data
+    type of the primary key set in the association, not the model
+    primary key.
+
     *Dominic Cleal*
 
 *   For PostgreSQL >= 9.4 use `pgcrypto`'s `gen_random_uuid()` instead of
diff --git a/activerecord/lib/active_record/associations/collection_association.rb b/activerecord/lib/active_record/associations/collection_association.rb
index 99e6696c66..3d23fa1e46 100644
--- a/activerecord/lib/active_record/associations/collection_association.rb
+++ b/activerecord/lib/active_record/associations/collection_association.rb
@@ -68,7 +68,7 @@ module ActiveRecord
 
       # Implements the ids writer method, e.g. foo.item_ids= for Foo.has_many :items
       def ids_writer(ids)
-        pk_type = reflection.primary_key_type
+        pk_type = reflection.association_primary_key_type
         ids = Array(ids).reject(&:blank?)
         ids.map! { |i| pk_type.cast(i) }
         records = klass.where(reflection.association_primary_key => ids).index_by do |r|
diff --git a/activerecord/lib/active_record/reflection.rb b/activerecord/lib/active_record/reflection.rb
index ef3c3bfae8..17751c9571 100644
--- a/activerecord/lib/active_record/reflection.rb
+++ b/activerecord/lib/active_record/reflection.rb
@@ -397,6 +397,10 @@ module ActiveRecord
         options[:primary_key] || primary_key(klass || self.klass)
       end
 
+      def association_primary_key_type
+        klass.type_for_attribute(association_primary_key)
+      end
+
       def active_record_primary_key
         @active_record_primary_key ||= options[:primary_key] || primary_key(active_record)
       end
diff --git a/activerecord/test/cases/associations/has_many_through_associations_test.rb b/activerecord/test/cases/associations/has_many_through_associations_test.rb
index 08a22a58b1..8defb09db7 100644
--- a/activerecord/test/cases/associations/has_many_through_associations_test.rb
+++ b/activerecord/test/cases/associations/has_many_through_associations_test.rb
@@ -883,6 +883,13 @@ class HasManyThroughAssociationsTest < ActiveRecord::TestCase
 
   end
 
+  def test_collection_singular_ids_setter_with_changed_primary_key
+    company = companies(:first_firm)
+    client = companies(:first_client)
+    company.clients_using_primary_key_ids = [client.name]
+    assert_equal [client], company.clients_using_primary_key
+  end
+
   def test_collection_singular_ids_setter_raises_exception_when_invalid_ids_set
     company = companies(:rails_core)
     ids = [Developer.first.id, -9999]
@@ -890,6 +897,13 @@ class HasManyThroughAssociationsTest < ActiveRecord::TestCase
     assert_match(/Couldn't find all Developers with 'id'/, e.message)
   end
 
+  def test_collection_singular_ids_setter_raises_exception_when_invalid_ids_set_with_changed_primary_key
+    company = companies(:first_firm)
+    ids = [Client.first.name, "unknown client"]
+    e = assert_raises(ActiveRecord::RecordNotFound) { company.clients_using_primary_key_ids = ids }
+    assert_match(/Couldn't find all Clients with 'name'/, e.message)
+  end
+
   def test_build_a_model_from_hm_through_association_with_where_clause
     assert_nothing_raised { books(:awdr).subscribers.where(nick: "marklazz").build }
   end