aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
-rw-r--r--activerecord/CHANGELOG.md5
-rw-r--r--activerecord/lib/active_record/associations/collection_association.rb8
-rw-r--r--activerecord/lib/active_record/relation/finder_methods.rb8
-rw-r--r--activerecord/test/cases/associations/has_many_through_associations_test.rb3
4 files changed, 17 insertions, 7 deletions
diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md
index 0b9a2f9943..f4218cc110 100644
--- a/activerecord/CHANGELOG.md
+++ b/activerecord/CHANGELOG.md
@@ -1,3 +1,8 @@
+* Raise ActiveRecord::RecordNotFound from collection `*_ids` setters
+ for unknown IDs with a better error message.
+
+ *Dominic Cleal*
+
* For PostgreSQL >= 9.4 use `pgcrypto`'s `gen_random_uuid()` instead of
`uuid-ossp`'s UUID generation function.
diff --git a/activerecord/lib/active_record/associations/collection_association.rb b/activerecord/lib/active_record/associations/collection_association.rb
index b2cf4713bb..99e6696c66 100644
--- a/activerecord/lib/active_record/associations/collection_association.rb
+++ b/activerecord/lib/active_record/associations/collection_association.rb
@@ -73,8 +73,12 @@ module ActiveRecord
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)
- end.values_at(*ids)
- replace(records)
+ end.values_at(*ids).compact
+ if records.size != ids.size
+ scope.raise_record_not_found_exception!(ids, records.size, ids.size, reflection.association_primary_key)
+ else
+ replace(records)
+ end
end
def reset
diff --git a/activerecord/lib/active_record/relation/finder_methods.rb b/activerecord/lib/active_record/relation/finder_methods.rb
index 55ded4c6d0..93c8722aa3 100644
--- a/activerecord/lib/active_record/relation/finder_methods.rb
+++ b/activerecord/lib/active_record/relation/finder_methods.rb
@@ -345,7 +345,7 @@ module ActiveRecord
# of results obtained should be provided in the +result_size+ argument and
# the expected number of results should be provided in the +expected_size+
# argument.
- def raise_record_not_found_exception!(ids = nil, result_size = nil, expected_size = nil) # :nodoc:
+ def raise_record_not_found_exception!(ids = nil, result_size = nil, expected_size = nil, key = primary_key) # :nodoc:
conditions = arel.where_sql(@klass.arel_engine)
conditions = " [#{conditions}]" if conditions
name = @klass.name
@@ -355,10 +355,10 @@ module ActiveRecord
error << " with#{conditions}" if conditions
raise RecordNotFound.new(error, name)
elsif Array(ids).size == 1
- error = "Couldn't find #{name} with '#{primary_key}'=#{ids}#{conditions}"
- raise RecordNotFound.new(error, name, primary_key, ids)
+ error = "Couldn't find #{name} with '#{key}'=#{ids}#{conditions}"
+ raise RecordNotFound.new(error, name, key, ids)
else
- error = "Couldn't find all #{name.pluralize} with '#{primary_key}': "
+ error = "Couldn't find all #{name.pluralize} with '#{key}': "
error << "(#{ids.join(", ")})#{conditions} (found #{result_size} results, but was looking for #{expected_size})"
raise RecordNotFound.new(error, name, primary_key, ids)
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 c2239ac03a..08a22a58b1 100644
--- a/activerecord/test/cases/associations/has_many_through_associations_test.rb
+++ b/activerecord/test/cases/associations/has_many_through_associations_test.rb
@@ -886,7 +886,8 @@ class HasManyThroughAssociationsTest < ActiveRecord::TestCase
def test_collection_singular_ids_setter_raises_exception_when_invalid_ids_set
company = companies(:rails_core)
ids = [Developer.first.id, -9999]
- assert_raises(ActiveRecord::AssociationTypeMismatch) { company.developer_ids = ids }
+ e = assert_raises(ActiveRecord::RecordNotFound) { company.developer_ids = ids }
+ assert_match(/Couldn't find all Developers with 'id'/, e.message)
end
def test_build_a_model_from_hm_through_association_with_where_clause