From c2377f72d2c7df1103eac73b0eb3781c02ef2e18 Mon Sep 17 00:00:00 2001 From: Jared Armstrong Date: Mon, 10 Jun 2013 15:45:58 +1200 Subject: Fixes CollectionAssociation#ids_reader returning incorrect ids for new records --- activerecord/CHANGELOG.md | 6 +++++ .../associations/collection_association.rb | 2 +- .../associations/has_many_associations_test.rb | 27 ++++++++++++++++++++++ 3 files changed, 34 insertions(+), 1 deletion(-) diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 29d218277f..cc1a66c098 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,5 +1,11 @@ ## unreleased ## +* Prevent query with NULL foreign key value on CollectionAssociation#ids_reader + for non-persisted records. Fixes bug where Company.new.contract_ids would + incorrectly load all non-associated Contracts. + + *Jared Armstrong* + * Fix the `:primary_key` option for `has_many` associations. Fixes #10693. diff --git a/activerecord/lib/active_record/associations/collection_association.rb b/activerecord/lib/active_record/associations/collection_association.rb index 65e882867e..baddb9852f 100644 --- a/activerecord/lib/active_record/associations/collection_association.rb +++ b/activerecord/lib/active_record/associations/collection_association.rb @@ -43,7 +43,7 @@ module ActiveRecord # Implements the ids reader method, e.g. foo.item_ids for Foo.has_many :items def ids_reader - if loaded? || options[:finder_sql] + if owner.new_record? || loaded? || options[:finder_sql] load_target.map do |record| record.send(reflection.association_primary_key) end diff --git a/activerecord/test/cases/associations/has_many_associations_test.rb b/activerecord/test/cases/associations/has_many_associations_test.rb index a75d064ac0..55b57bc2f2 100644 --- a/activerecord/test/cases/associations/has_many_associations_test.rb +++ b/activerecord/test/cases/associations/has_many_associations_test.rb @@ -1311,6 +1311,33 @@ class HasManyAssociationsTest < ActiveRecord::TestCase assert !company.clients.loaded? end + def test_get_ids_for_association_on_new_record_does_not_try_to_find_records + Company.columns # Load schema information so we don't query below + Contract.columns # if running just this test. + + company = Company.new + assert_queries(0) do + company.contract_ids + end + + assert_equal [], company.contract_ids + end + + def test_set_ids_for_association_on_new_record_applies_association_correctly + contract_a = Contract.create! + contract_b = Contract.create! + another_contract = Contract.create! + company = Company.new(:name => "Some Company") + + company.contract_ids = [contract_a.id, contract_b.id] + assert_equal [contract_a.id, contract_b.id], company.contract_ids + assert_equal [contract_a, contract_b], company.contracts + + company.save! + assert_equal company, contract_a.reload.company + assert_equal company, contract_b.reload.company + end + def test_get_ids_ignores_include_option assert_equal [readers(:michael_welcome).id], posts(:welcome).readers_with_person_ids end -- cgit v1.2.3