Merge pull request #27609 from kamipo/fix_association_primary_key

Fix `reflection.association_primary_key` for `has_many` association

activerecord/lib/active_record/associations/collection_association.rb

@@ -55,13 +55,16 @@ def ids_writer(ids)
         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|
-          r.send(reflection.association_primary_key)
+
+        primary_key = reflection.association_primary_key
+        records = klass.where(primary_key => ids).index_by do |r|
+          r.public_send(primary_key)
         end.values_at(*ids).compact
+
         if records.size != ids.size
-          found_ids = records.map { |record| record.send(reflection.association_primary_key) }
+          found_ids = records.map { |record| record.public_send(primary_key) }
           not_found_ids = ids - found_ids
-          klass.all.raise_record_not_found_exception!(ids, records.size, ids.size, reflection.association_primary_key, not_found_ids)
+          klass.all.raise_record_not_found_exception!(ids, records.size, ids.size, primary_key, not_found_ids)
         else
           replace(records)
         end

activerecord/lib/active_record/reflection.rb

@@ -715,6 +715,10 @@ def association_class
           Associations::HasManyAssociation
         end
       end
+
+      def association_primary_key(klass = nil)
+        primary_key(klass || self.klass)
+      end
     end
 
     class HasOneReflection < AssociationReflection # :nodoc:

activerecord/test/cases/associations/has_many_associations_test.rb

@@ -63,7 +63,7 @@ def test_custom_primary_key_on_new_record_should_fetch_with_query
       assert_equal 2, subscriber.subscriptions.size
     end
 
-    assert_equal subscriber.subscriptions, Subscription.where(subscriber_id: "webster132")
+    assert_equal Subscription.where(subscriber_id: "webster132"), subscriber.subscriptions
   end
 
   def test_association_primary_key_on_new_record_should_fetch_with_query
@@ -74,12 +74,23 @@ def test_association_primary_key_on_new_record_should_fetch_with_query
       assert_equal 1, author.essays.size
     end
 
-    assert_equal author.essays, Essay.where(writer_id: "David")
+    assert_equal Essay.where(writer_id: "David"), author.essays
   end
 
   def test_has_many_custom_primary_key
     david = authors(:david)
-    assert_equal david.essays, Essay.where(writer_id: "David")
+    assert_equal Essay.where(writer_id: "David"), david.essays
+  end
+
+  def test_ids_on_unloaded_association_with_custom_primary_key
+    david = people(:david)
+    assert_equal Essay.where(writer_id: "David").pluck(:id), david.essay_ids
+  end
+
+  def test_ids_on_loaded_association_with_custom_primary_key
+    david = people(:david)
+    david.essays.load
+    assert_equal Essay.where(writer_id: "David").pluck(:id), david.essay_ids
   end
 
   def test_has_many_assignment_with_custom_primary_key

activerecord/test/cases/associations/has_many_through_associations_test.rb

@@ -880,13 +880,6 @@ def test_collection_singular_ids_setter_with_string_primary_keys
     end
   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]
@@ -895,13 +888,6 @@ def test_collection_singular_ids_setter_raises_exception_when_invalid_ids_set
     assert_equal(msg, 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_collection_singular_ids_through_setter_raises_exception_when_invalid_ids_set
     author = authors(:david)
     ids = [categories(:general).name, "Unknown"]