Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Eager loading has_many association when primary_key is a method #1696

Closed
wants to merge 2 commits into from

4 participants

@sigmike

In a has_many association, the primary key may be a method (the doc says: "Specify the method that returns the primary key used for the association. By default this is id."). It works fine except when the association is eager loaded. In this case it uses the nonexistent attribute and loads an empty list of records.

This fix is based on 3-0-stable.

@jake3030 jake3030 referenced this pull request from a commit in jake3030/rails
@h-lame h-lame Ensure NoMethodError isn't raised when some of the nested eager loade…
…d associations are empty [#1696 state:resolved]

Signed-off-by: Pratik Naik <pratiknaik@gmail.com>
db26ace
@spastorino
Owner

is this an issue in 3-1-stable?

@isaacsanders

@piglop Is this still an issue?

@rafaelfranca

This pull request is for a not supported Rails version. I'm closing it now. Please see if this one is still can be applied in the Rails >= 3.1. If so, please open another one against the master branch.

Thanks for your contribution.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Jun 14, 2011
  1. @sigmike
Commits on Jun 17, 2011
  1. @sigmike

    fixed broken test

    sigmike authored
This page is out of date. Refresh to see the latest.
View
2  activerecord/lib/active_record/association_preload.rb
@@ -176,7 +176,7 @@ def construct_id_map(records, primary_key=nil)
ids = []
records.each do |record|
primary_key ||= record.class.primary_key
- ids << record[primary_key]
+ ids << record.send(primary_key)
mapped_records = (id_to_record_map[ids.last.to_s] ||= [])
mapped_records << record
end
View
8 activerecord/test/cases/associations/eager_test.rb
@@ -905,6 +905,14 @@ def test_preload_has_many_using_primary_key
end
end
+ def test_preload_has_many_using_primary_key_method
+ expected = Firm.find(:first).clients_using_primary_key_method.to_a
+ firm = Firm.find :first, :include => :clients_using_primary_key_method
+ assert_no_queries do
+ assert_equal expected, firm.clients_using_primary_key_method
+ end
+ end
+
def test_include_has_many_using_primary_key
expected = Firm.find(1).clients_using_primary_key.sort_by(&:name)
# Oracle adapter truncates alias to 30 characters
View
4 activerecord/test/cases/reflection_test.rb
@@ -179,8 +179,8 @@ def test_association_reflection_in_modules
def test_reflection_of_all_associations
# FIXME these assertions bust a lot
- assert_equal 38, Firm.reflect_on_all_associations.size
- assert_equal 28, Firm.reflect_on_all_associations(:has_many).size
+ assert_equal 39, Firm.reflect_on_all_associations.size
+ assert_equal 29, Firm.reflect_on_all_associations(:has_many).size
assert_equal 10, Firm.reflect_on_all_associations(:has_one).size
assert_equal 0, Firm.reflect_on_all_associations(:belongs_to).size
end
View
6 activerecord/test/models/company.rb
@@ -69,6 +69,8 @@ class Firm < Company
has_many :readonly_clients, :class_name => 'Client', :readonly => true
has_many :clients_using_primary_key, :class_name => 'Client',
:primary_key => 'name', :foreign_key => 'firm_name'
+ has_many :clients_using_primary_key_method, :class_name => 'Client',
+ :primary_key => 'primary_key_method', :foreign_key => 'firm_name'
has_many :clients_using_primary_key_with_delete_all, :class_name => 'Client',
:primary_key => 'name', :foreign_key => 'firm_name', :dependent => :delete_all
has_many :clients_grouped_by_firm_id, :class_name => "Client", :group => "firm_id", :select => "firm_id"
@@ -89,6 +91,10 @@ class Firm < Company
has_one :unautosaved_account, :foreign_key => "firm_id", :class_name => 'Account', :autosave => false
has_many :accounts
has_many :unautosaved_accounts, :foreign_key => "firm_id", :class_name => 'Account', :autosave => false
+
+ def primary_key_method
+ name
+ end
end
class DependentFirm < Company
Something went wrong with that request. Please try again.