From dbdba5cda10d1f2a7a9dee6a4672b9e49109dc65 Mon Sep 17 00:00:00 2001 From: Jon Leighton Date: Tue, 21 Dec 2010 20:53:09 +0000 Subject: [PATCH] Fix problem where wrong keys are used in JoinAssociation when an association goes :through a belongs_to [#2801 state:resolved] --- activerecord/lib/active_record/associations.rb | 15 ++++++++++----- activerecord/lib/active_record/reflection.rb | 10 +++++++++- .../has_many_through_associations_test.rb | 10 +++++++++- activerecord/test/cases/reflection_test.rb | 10 ++++++++++ activerecord/test/models/post.rb | 1 + 5 files changed, 39 insertions(+), 7 deletions(-) diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb index 44e54c4..4c8fd77 100644 --- a/activerecord/lib/active_record/associations.rb +++ b/activerecord/lib/active_record/associations.rb @@ -2156,14 +2156,19 @@ module ActiveRecord when :has_many, :has_one if reflection.options[:through] join_table = Arel::Table.new(through_reflection.klass.table_name, :as => aliased_join_table_name, :engine => arel_engine) - jt_foreign_key = jt_as_extra = jt_source_extra = jt_sti_extra = nil + jt_as_extra = jt_source_extra = jt_sti_extra = nil first_key = second_key = as_extra = nil - if through_reflection.options[:as] # has_many :through against a polymorphic join - jt_foreign_key = through_reflection.options[:as].to_s + '_id' - jt_as_extra = join_table[through_reflection.options[:as].to_s + '_type'].eq(parent.active_record.base_class.name) + if through_reflection.macro == :belongs_to + jt_primary_key = through_reflection.primary_key_name + jt_foreign_key = through_reflection.association_primary_key else + jt_primary_key = through_reflection.active_record_primary_key jt_foreign_key = through_reflection.primary_key_name + + if through_reflection.options[:as] # has_many :through against a polymorphic join + jt_as_extra = join_table[through_reflection.options[:as].to_s + '_type'].eq(parent.active_record.base_class.name) + end end case source_reflection.macro @@ -2191,7 +2196,7 @@ module ActiveRecord end [ - [parent_table[parent.primary_key].eq(join_table[jt_foreign_key]), jt_as_extra, jt_source_extra, jt_sti_extra].reject{|x| x.blank? }, + [parent_table[jt_primary_key].eq(join_table[jt_foreign_key]), jt_as_extra, jt_source_extra, jt_sti_extra].reject{|x| x.blank? }, aliased_table[first_key].eq(join_table[second_key]) ] elsif reflection.options[:as] diff --git a/activerecord/lib/active_record/reflection.rb b/activerecord/lib/active_record/reflection.rb index db18fb7..6b28bcb 100644 --- a/activerecord/lib/active_record/reflection.rb +++ b/activerecord/lib/active_record/reflection.rb @@ -207,7 +207,15 @@ module ActiveRecord end def association_foreign_key - @association_foreign_key ||= @options[:association_foreign_key] || class_name.foreign_key + @association_foreign_key ||= options[:association_foreign_key] || class_name.foreign_key + end + + def association_primary_key + @association_primary_key ||= options[:primary_key] || klass.primary_key + end + + def active_record_primary_key + @active_record_primary_key ||= options[:primary_key] || active_record.primary_key end def counter_cache_column 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 94e1eb8..1d94601 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,12 @@ require 'models/developer' require 'models/subscriber' require 'models/book' require 'models/subscription' +require 'models/categorization' class HasManyThroughAssociationsTest < ActiveRecord::TestCase fixtures :posts, :readers, :people, :comments, :authors, :owners, :pets, :toys, :jobs, :references, :companies, - :subscribers, :books, :subscriptions, :developers + :subscribers, :books, :subscriptions, :developers, :categorizations # Dummies to force column loads so query counts are clean. def setup @@ -456,4 +457,11 @@ class HasManyThroughAssociationsTest < ActiveRecord::TestCase post.people << people(:michael) assert_equal readers + 1, post.readers.size end + + def test_joining_has_many_through_belongs_to + posts = Post.joins(:author_categorizations). + where('categorizations.id' => categorizations(:mary_thinking_sti).id) + + assert_equal [posts(:eager_other)], posts + end end diff --git a/activerecord/test/cases/reflection_test.rb b/activerecord/test/cases/reflection_test.rb index eeb619a..fe3f51b 100644 --- a/activerecord/test/cases/reflection_test.rb +++ b/activerecord/test/cases/reflection_test.rb @@ -191,6 +191,16 @@ class ReflectionTest < ActiveRecord::TestCase assert_kind_of ThroughReflection, Subscriber.reflect_on_association(:books) end + def test_association_primary_key + assert_equal "id", Author.reflect_on_association(:posts).association_primary_key.to_s + assert_equal "name", Author.reflect_on_association(:essay).association_primary_key.to_s + end + + def test_active_record_primary_key + assert_equal "nick", Subscriber.reflect_on_association(:subscriptions).active_record_primary_key.to_s + assert_equal "name", Author.reflect_on_association(:essay).active_record_primary_key.to_s + end + def test_collection_association assert Pirate.reflect_on_association(:birds).collection? assert Pirate.reflect_on_association(:parrots).collection? diff --git a/activerecord/test/models/post.rb b/activerecord/test/models/post.rb index a3cb9c7..61ce413 100644 --- a/activerecord/test/models/post.rb +++ b/activerecord/test/models/post.rb @@ -38,6 +38,7 @@ class Post < ActiveRecord::Base end has_many :author_favorites, :through => :author + has_many :author_categorizations, :through => :author, :source => :categorizations has_many :comments_with_interpolated_conditions, :class_name => 'Comment', :conditions => ['#{"#{aliased_table_name}." rescue ""}body = ?', 'Thank you for the welcome'] -- 1.7.1