Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Return Not found Ids in ActiveRecord::NotFound #29720

Merged
merged 1 commit into from
Aug 11, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,9 @@ def ids_writer(ids)
r.send(reflection.association_primary_key)
end.values_at(*ids).compact
if records.size != ids.size
klass.all.raise_record_not_found_exception!(ids, records.size, ids.size, reflection.association_primary_key)
found_ids = records.map { |record| record.send(reflection.association_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)
else
replace(records)
end
Expand Down
6 changes: 3 additions & 3 deletions activerecord/lib/active_record/relation/finder_methods.rb
Original file line number Diff line number Diff line change
Expand Up @@ -330,7 +330,7 @@ def exists?(conditions = :none)
# 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, key = primary_key) # :nodoc:
def raise_record_not_found_exception!(ids = nil, result_size = nil, expected_size = nil, key = primary_key, not_found_ids = nil) # :nodoc:
conditions = arel.where_sql(@klass)
conditions = " [#{conditions}]" if conditions
name = @klass.name
Expand All @@ -344,8 +344,8 @@ def raise_record_not_found_exception!(ids = nil, result_size = nil, expected_siz
raise RecordNotFound.new(error, name, key, ids)
else
error = "Couldn't find all #{name.pluralize} with '#{key}': ".dup
error << "(#{ids.join(", ")})#{conditions} (found #{result_size} results, but was looking for #{expected_size})"

error << "(#{ids.join(", ")})#{conditions} (found #{result_size} results, but was looking for #{expected_size})."
error << " Couldn't find #{name.pluralize(not_found_ids.size)} with #{key.to_s.pluralize(not_found_ids.size)} #{not_found_ids.join(', ')}." if not_found_ids
raise RecordNotFound.new(error, name, primary_key, ids)
end
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -891,7 +891,8 @@ def test_collection_singular_ids_setter_raises_exception_when_invalid_ids_set
company = companies(:rails_core)
ids = [Developer.first.id, -9999]
e = assert_raises(ActiveRecord::RecordNotFound) { company.developer_ids = ids }
assert_match(/Couldn't find all Developers with 'id'/, e.message)
msg = "Couldn't find all Developers with 'id': (1, -9999) (found 1 results, but was looking for 2). Couldn't find Developer with id -9999."
assert_equal(msg, e.message)
end

def test_collection_singular_ids_setter_raises_exception_when_invalid_ids_set_with_changed_primary_key
Expand All @@ -905,7 +906,8 @@ def test_collection_singular_ids_through_setter_raises_exception_when_invalid_id
author = authors(:david)
ids = [categories(:general).name, "Unknown"]
e = assert_raises(ActiveRecord::RecordNotFound) { author.essay_category_ids = ids }
assert_equal "Couldn't find all Categories with 'name': (General, Unknown) (found 1 results, but was looking for 2)", e.message
msg = "Couldn't find all Categories with 'name': (General, Unknown) (found 1 results, but was looking for 2). Couldn't find Category with name Unknown."
assert_equal msg, e.message
end

def test_build_a_model_from_hm_through_association_with_where_clause
Expand Down
2 changes: 1 addition & 1 deletion activerecord/test/cases/finder_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1153,7 +1153,7 @@ def test_find_some_message_with_custom_primary_key
e = assert_raises(ActiveRecord::RecordNotFound) do
model.find "Hello", "World!"
end
assert_equal "Couldn't find all MercedesCars with 'name': (Hello, World!) (found 0 results, but was looking for 2)", e.message
assert_equal "Couldn't find all MercedesCars with 'name': (Hello, World!) (found 0 results, but was looking for 2).", e.message
end
end

Expand Down