diff options
author | Ryuta Kamizono <kamipo@gmail.com> | 2017-01-09 01:43:08 +0900 |
---|---|---|
committer | Ryuta Kamizono <kamipo@gmail.com> | 2017-08-13 16:17:35 +0900 |
commit | b23e86967c4dfa5aa4f007291458c48dd39b179a (patch) | |
tree | bf35c7a8d0fc2eb722e67a9fb1291d81861f614c /activerecord | |
parent | e3962308e7947f3d99c70c192e2d1356d3420e1e (diff) | |
download | rails-b23e86967c4dfa5aa4f007291458c48dd39b179a.tar.gz rails-b23e86967c4dfa5aa4f007291458c48dd39b179a.tar.bz2 rails-b23e86967c4dfa5aa4f007291458c48dd39b179a.zip |
Fix `reflection.association_primary_key` for `has_many` associations
It is incorrect to treat `options[:primary_key]` as
`association_primary_key` if `has_many` associations because the
`:primary_key` means the column on the owner record, not on the
association record. It will break `ids_reader` and `ids_writer`.
```ruby
people(:david).essay_ids
# => ActiveRecord::StatementInvalid: Mysql2::Error: Unknown column 'essays.first_name' in 'field list': SELECT `essays`.first_name FROM `essays` WHERE `essays`.`writer_id` = 'David'
```
Fixes #14439.
Diffstat (limited to 'activerecord')
4 files changed, 25 insertions, 21 deletions
diff --git a/activerecord/lib/active_record/associations/collection_association.rb b/activerecord/lib/active_record/associations/collection_association.rb index 1a424896fe..ea9ab297d1 100644 --- a/activerecord/lib/active_record/associations/collection_association.rb +++ b/activerecord/lib/active_record/associations/collection_association.rb @@ -55,13 +55,16 @@ module ActiveRecord 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 diff --git a/activerecord/lib/active_record/reflection.rb b/activerecord/lib/active_record/reflection.rb index d2b85e168b..8e1ef840fa 100644 --- a/activerecord/lib/active_record/reflection.rb +++ b/activerecord/lib/active_record/reflection.rb @@ -715,6 +715,10 @@ module ActiveRecord Associations::HasManyAssociation end end + + def association_primary_key(klass = nil) + primary_key(klass || self.klass) + end end class HasOneReflection < AssociationReflection # :nodoc: diff --git a/activerecord/test/cases/associations/has_many_associations_test.rb b/activerecord/test/cases/associations/has_many_associations_test.rb index cedb621b4f..383c7314e1 100644 --- a/activerecord/test/cases/associations/has_many_associations_test.rb +++ b/activerecord/test/cases/associations/has_many_associations_test.rb @@ -63,7 +63,7 @@ class HasManyAssociationsTestPrimaryKeys < ActiveRecord::TestCase 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 @@ class HasManyAssociationsTestPrimaryKeys < ActiveRecord::TestCase 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 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 4c2723addc..f49383a1a8 100644 --- a/activerecord/test/cases/associations/has_many_through_associations_test.rb +++ b/activerecord/test/cases/associations/has_many_through_associations_test.rb @@ -880,13 +880,6 @@ class HasManyThroughAssociationsTest < ActiveRecord::TestCase 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 @@ class HasManyThroughAssociationsTest < ActiveRecord::TestCase 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"] |