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

When calling association.find RecordNotFound is now raised with the s… #26097

Merged
merged 1 commit into from
Aug 16, 2016
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
5 changes: 5 additions & 0 deletions activerecord/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
* RecordNotFound raised by association.find exposes `id`, `primary_key` and
`model` methods to be consistent with RecordNotFound raised by Record.find.

*Michel Pigassou*

* Hashes can once again be passed to setters of `composed_of`, if all of the
mapping methods are methods implemented on `Hash`.

Expand Down
10 changes: 6 additions & 4 deletions activerecord/lib/active_record/relation/finder_methods.rb
Original file line number Diff line number Diff line change
Expand Up @@ -348,15 +348,17 @@ def exists?(conditions = :none)
def raise_record_not_found_exception!(ids, result_size, expected_size) #:nodoc:
conditions = arel.where_sql(@klass.arel_engine)
conditions = " [#{conditions}]" if conditions
name = @klass.name

if Array(ids).size == 1
error = "Couldn't find #{@klass.name} with '#{primary_key}'=#{ids}#{conditions}"
error = "Couldn't find #{name} with '#{primary_key}'=#{ids}#{conditions}"
raise RecordNotFound.new(error, name, primary_key, ids)
else
error = "Couldn't find all #{@klass.name.pluralize} with '#{primary_key}': "
error = "Couldn't find all #{name.pluralize} with '#{primary_key}': "
error << "(#{ids.join(", ")})#{conditions} (found #{result_size} results, but was looking for #{expected_size})"
end

raise RecordNotFound, error
raise RecordNotFound, error
end
end

private
Expand Down
12 changes: 12 additions & 0 deletions activerecord/test/cases/associations/has_many_associations_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -618,6 +618,18 @@ def test_find_ids
assert_raise(ActiveRecord::RecordNotFound) { firm.clients.find(2, 99) }
end

def test_find_one_message_on_primary_key
firm = Firm.all.merge!(order: "id").first

e = assert_raises(ActiveRecord::RecordNotFound) do
firm.clients.find(0)
end
assert_equal 0, e.id
assert_equal "id", e.primary_key
assert_equal "Client", e.model
assert_match (/\ACouldn't find Client with 'id'=0/), e.message
end

def test_find_ids_and_inverse_of
force_signal37_to_load_all_clients_of_firm

Expand Down
10 changes: 10 additions & 0 deletions activerecord/test/cases/finder_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1119,6 +1119,16 @@ def test_with_limiting_with_custom_select
assert_equal [0, 1, 1], posts.map(&:author_id).sort
end

def test_find_one_message_on_primary_key
e = assert_raises(ActiveRecord::RecordNotFound) do
Car.find(0)
end
assert_equal 0, e.id
assert_equal "id", e.primary_key
assert_equal "Car", e.model
assert_equal "Couldn't find Car with 'id'=0", e.message
end

def test_find_one_message_with_custom_primary_key
table_with_custom_primary_key do |model|
model.primary_key = :name
Expand Down