Skip to content

Commit 15e2da6

Browse files
committed
Restore RecordNotFound when *_ids= can't find records by ID
9c9fb19 changed the behaviour of the _ids= setters for associations to raise an AssociationTypeMismatch when unknown IDs are given: Class: <ActiveRecord::AssociationTypeMismatch> Message: <"Developer(#43811860) expected, got NilClass(#16732720)"> This restores the original ActiveRecord::RecordNotFound exception with a much clearer error message: Class: <ActiveRecord::RecordNotFound> Message: <"Couldn't find all Developers with 'id': (1, -9999) [WHERE \"contracts\".\"company_id\" = ?] (found 1 results, but was looking for 2)"> Fixes rails#25719
1 parent 8556ab5 commit 15e2da6

File tree

4 files changed

+17
-7
lines changed

4 files changed

+17
-7
lines changed

activerecord/CHANGELOG.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,8 @@
1+
* Raise ActiveRecord::RecordNotFound from collection `*_ids` setters
2+
for unknown IDs with a better error message.
3+
4+
*Dominic Cleal*
5+
16
* For PostgreSQL >= 9.4 use `pgcrypto`'s `gen_random_uuid()` instead of
27
`uuid-ossp`'s UUID generation function.
38

activerecord/lib/active_record/associations/collection_association.rb

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -73,8 +73,12 @@ def ids_writer(ids)
7373
ids.map! { |i| pk_type.cast(i) }
7474
records = klass.where(reflection.association_primary_key => ids).index_by do |r|
7575
r.send(reflection.association_primary_key)
76-
end.values_at(*ids)
77-
replace(records)
76+
end.values_at(*ids).compact
77+
if records.size != ids.size
78+
scope.raise_record_not_found_exception!(ids, records.size, ids.size, reflection.association_primary_key)
79+
else
80+
replace(records)
81+
end
7882
end
7983

8084
def reset

activerecord/lib/active_record/relation/finder_methods.rb

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -345,7 +345,7 @@ def exists?(conditions = :none)
345345
# of results obtained should be provided in the +result_size+ argument and
346346
# the expected number of results should be provided in the +expected_size+
347347
# argument.
348-
def raise_record_not_found_exception!(ids = nil, result_size = nil, expected_size = nil) # :nodoc:
348+
def raise_record_not_found_exception!(ids = nil, result_size = nil, expected_size = nil, key = primary_key) # :nodoc:
349349
conditions = arel.where_sql(@klass.arel_engine)
350350
conditions = " [#{conditions}]" if conditions
351351
name = @klass.name
@@ -355,10 +355,10 @@ def raise_record_not_found_exception!(ids = nil, result_size = nil, expected_siz
355355
error << " with#{conditions}" if conditions
356356
raise RecordNotFound.new(error, name)
357357
elsif Array(ids).size == 1
358-
error = "Couldn't find #{name} with '#{primary_key}'=#{ids}#{conditions}"
359-
raise RecordNotFound.new(error, name, primary_key, ids)
358+
error = "Couldn't find #{name} with '#{key}'=#{ids}#{conditions}"
359+
raise RecordNotFound.new(error, name, key, ids)
360360
else
361-
error = "Couldn't find all #{name.pluralize} with '#{primary_key}': "
361+
error = "Couldn't find all #{name.pluralize} with '#{key}': "
362362
error << "(#{ids.join(", ")})#{conditions} (found #{result_size} results, but was looking for #{expected_size})"
363363

364364
raise RecordNotFound.new(error, name, primary_key, ids)

activerecord/test/cases/associations/has_many_through_associations_test.rb

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -886,7 +886,8 @@ def test_collection_singular_ids_setter_with_string_primary_keys
886886
def test_collection_singular_ids_setter_raises_exception_when_invalid_ids_set
887887
company = companies(:rails_core)
888888
ids = [Developer.first.id, -9999]
889-
assert_raises(ActiveRecord::AssociationTypeMismatch) { company.developer_ids = ids }
889+
e = assert_raises(ActiveRecord::RecordNotFound) { company.developer_ids = ids }
890+
assert_match(/Couldn't find all Developers with 'id'/, e.message)
890891
end
891892

892893
def test_build_a_model_from_hm_through_association_with_where_clause

0 commit comments

Comments
 (0)