From 596cc3b2329a9cc4a30c95c157ce36b2d08975df Mon Sep 17 00:00:00 2001 From: Jon Leighton Date: Tue, 19 Oct 2010 12:47:19 +0100 Subject: Respect the :primary_key option on the through_reflection of (non-nested) through associations --- .../associations/has_many_association.rb | 6 +++--- .../associations/has_one_association.rb | 6 +++--- .../associations/through_association_scope.rb | 4 ++-- .../has_many_through_associations_test.rb | 19 ++++++++++++++++++- .../has_one_through_associations_test.rb | 20 +++++++++++++++++++- activerecord/test/fixtures/essays.yml | 6 ++++++ activerecord/test/models/author.rb | 12 +++++++++++- activerecord/test/models/essay.rb | 1 + activerecord/test/schema/schema.rb | 2 ++ 9 files changed, 65 insertions(+), 11 deletions(-) create mode 100644 activerecord/test/fixtures/essays.yml (limited to 'activerecord') diff --git a/activerecord/lib/active_record/associations/has_many_association.rb b/activerecord/lib/active_record/associations/has_many_association.rb index 830a82980d..7eaa05ee36 100644 --- a/activerecord/lib/active_record/associations/has_many_association.rb +++ b/activerecord/lib/active_record/associations/has_many_association.rb @@ -7,9 +7,9 @@ module ActiveRecord # is provided by its child HasManyThroughAssociation. class HasManyAssociation < AssociationCollection #:nodoc: protected - def owner_quoted_id - if @reflection.options[:primary_key] - @owner.class.quote_value(@owner.send(@reflection.options[:primary_key])) + def owner_quoted_id(reflection = @reflection) + if reflection.options[:primary_key] + @owner.class.quote_value(@owner.send(reflection.options[:primary_key])) else @owner.quoted_id end diff --git a/activerecord/lib/active_record/associations/has_one_association.rb b/activerecord/lib/active_record/associations/has_one_association.rb index 17901387e9..c6bcfec275 100644 --- a/activerecord/lib/active_record/associations/has_one_association.rb +++ b/activerecord/lib/active_record/associations/has_one_association.rb @@ -64,9 +64,9 @@ module ActiveRecord end protected - def owner_quoted_id - if @reflection.options[:primary_key] - @owner.class.quote_value(@owner.send(@reflection.options[:primary_key])) + def owner_quoted_id(reflection = @reflection) + if reflection.options[:primary_key] + @owner.class.quote_value(@owner.send(reflection.options[:primary_key])) else @owner.quoted_id end diff --git a/activerecord/lib/active_record/associations/through_association_scope.rb b/activerecord/lib/active_record/associations/through_association_scope.rb index feb0a93360..86ceb1a204 100644 --- a/activerecord/lib/active_record/associations/through_association_scope.rb +++ b/activerecord/lib/active_record/associations/through_association_scope.rb @@ -44,14 +44,14 @@ module ActiveRecord # Associate attributes pointing to owner, quoted. def construct_quoted_owner_attributes(reflection) if as = reflection.options[:as] - { "#{as}_id" => owner_quoted_id, + { "#{as}_id" => owner_quoted_id(reflection), "#{as}_type" => reflection.klass.quote_value( @owner.class.base_class.name.to_s, reflection.klass.columns_hash["#{as}_type"]) } elsif reflection.macro == :belongs_to { reflection.klass.primary_key => @owner.class.quote_value(@owner[reflection.primary_key_name]) } else - { reflection.primary_key_name => owner_quoted_id } + { reflection.primary_key_name => owner_quoted_id(reflection) } end end 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 4b9f49f1ec..5a2e6b26aa 100644 --- a/activerecord/test/cases/associations/has_many_through_associations_test.rb +++ b/activerecord/test/cases/associations/has_many_through_associations_test.rb @@ -17,11 +17,14 @@ require 'models/developer' require 'models/subscriber' require 'models/book' require 'models/subscription' +require 'models/essay' +require 'models/category' class HasManyThroughAssociationsTest < ActiveRecord::TestCase fixtures :posts, :readers, :people, :comments, :authors, :owners, :pets, :toys, :jobs, :references, :companies, - :subscribers, :books, :subscriptions, :developers + :subscribers, :books, :subscriptions, :developers, + :essays, :categories # Dummies to force column loads so query counts are clean. def setup @@ -449,4 +452,18 @@ class HasManyThroughAssociationsTest < ActiveRecord::TestCase comment = post.comments.build assert author.comments.include?(comment) end + + def test_has_many_through_polymorphic_with_primary_key_option_on_through_reflection + assert_equal [categories(:general)], authors(:david).essay_categories + + authors = Author.joins(:essay_categories).where('categories.id' => categories(:general).id) + assert_equal authors(:david), authors.first + end + + def test_has_many_through_with_primary_key_option_on_through_reflection + assert_equal [categories(:general)], authors(:david).essay_categories_2 + + authors = Author.joins(:essay_categories_2).where('categories.id' => categories(:general).id) + assert_equal authors(:david), authors.first + end end diff --git a/activerecord/test/cases/associations/has_one_through_associations_test.rb b/activerecord/test/cases/associations/has_one_through_associations_test.rb index 5d153147f5..8805968869 100644 --- a/activerecord/test/cases/associations/has_one_through_associations_test.rb +++ b/activerecord/test/cases/associations/has_one_through_associations_test.rb @@ -9,9 +9,13 @@ require 'models/member_detail' require 'models/minivan' require 'models/dashboard' require 'models/speedometer' +require 'models/category' +require 'models/author' +require 'models/essay' class HasOneThroughAssociationsTest < ActiveRecord::TestCase - fixtures :member_types, :members, :clubs, :memberships, :sponsors, :organizations, :minivans, :dashboards, :speedometers + fixtures :member_types, :members, :clubs, :memberships, :sponsors, :organizations, :minivans, + :dashboards, :speedometers, :categories, :authors, :essays def setup @member = members(:groucho) @@ -212,4 +216,18 @@ class HasOneThroughAssociationsTest < ActiveRecord::TestCase minivan.dashboard end end + + def test_has_one_through_polymorphic_with_primary_key_option_on_through_reflection + assert_equal categories(:general), authors(:david).essay_category + + authors = Author.joins(:essay_category).where('categories.id' => categories(:general).id) + assert_equal authors(:david), authors.first + end + + def test_has_one_through_with_primary_key_option_on_through_reflection + assert_equal categories(:general), authors(:david).essay_category_2 + + authors = Author.joins(:essay_category_2).where('categories.id' => categories(:general).id) + assert_equal authors(:david), authors.first + end end diff --git a/activerecord/test/fixtures/essays.yml b/activerecord/test/fixtures/essays.yml new file mode 100644 index 0000000000..8c96a469e6 --- /dev/null +++ b/activerecord/test/fixtures/essays.yml @@ -0,0 +1,6 @@ +david_modest_proposal: + name: A Modest Proposal + writer_type: Author + writer_id: David + category_id: 1 + author_id: David diff --git a/activerecord/test/models/author.rb b/activerecord/test/models/author.rb index c0e082836d..1ba01d6b6b 100644 --- a/activerecord/test/models/author.rb +++ b/activerecord/test/models/author.rb @@ -95,8 +95,18 @@ class Author < ActiveRecord::Base has_many :subscriptions, :through => :books has_many :subscribers, :through => :subscriptions, :order => "subscribers.nick" # through has_many :through (on through reflection) has_many :distinct_subscribers, :through => :subscriptions, :source => :subscriber, :select => "DISTINCT subscribers.*", :order => "subscribers.nick" - + has_one :essay, :primary_key => :name, :as => :writer + has_one :essay_category, :through => :essay, :source => :category + + has_one :essay_2, :primary_key => :name, :class_name => 'Essay', :foreign_key => :author_id + has_one :essay_category_2, :through => :essay_2, :source => :category + + has_many :essays, :primary_key => :name, :as => :writer + has_many :essay_categories, :through => :essays, :source => :category + + has_many :essays_2, :primary_key => :name, :class_name => 'Essay', :foreign_key => :author_id + has_many :essay_categories_2, :through => :essays_2, :source => :category belongs_to :author_address, :dependent => :destroy belongs_to :author_address_extra, :dependent => :delete, :class_name => "AuthorAddress" diff --git a/activerecord/test/models/essay.rb b/activerecord/test/models/essay.rb index 6c28f5e49b..6a62042863 100644 --- a/activerecord/test/models/essay.rb +++ b/activerecord/test/models/essay.rb @@ -1,3 +1,4 @@ class Essay < ActiveRecord::Base belongs_to :writer, :primary_key => :name, :polymorphic => true + belongs_to :category end diff --git a/activerecord/test/schema/schema.rb b/activerecord/test/schema/schema.rb index ee129162a6..b5bf9a7349 100644 --- a/activerecord/test/schema/schema.rb +++ b/activerecord/test/schema/schema.rb @@ -214,6 +214,8 @@ ActiveRecord::Schema.define do t.string :name t.string :writer_id t.string :writer_type + t.integer :category_id + t.integer :author_id end create_table :events, :force => true do |t| -- cgit v1.2.3