Skip to content
This repository
Browse code

Fix problem with loading polymorphic associations which have been def…

…ined in an abstract superclass. Fixes #552.
  • Loading branch information...
commit d7a910e31e839e5544aa9f83f9922ecfcee74ed5 1 parent 8c19ebe
Jon Leighton jonleighton authored
20 activerecord/lib/active_record/associations/association_scope.rb
@@ -75,10 +75,16 @@ def add_constraints(scope)
75 75 foreign_key = reflection.active_record_primary_key
76 76 end
77 77
  78 + conditions = self.conditions[i]
  79 +
78 80 if reflection == chain.last
79 81 scope = scope.where(table[key].eq(owner[foreign_key]))
80 82
81   - conditions[i].each do |condition|
  83 + if reflection.type
  84 + scope = scope.where(table[reflection.type].eq(owner.class.base_class.name))
  85 + end
  86 +
  87 + conditions.each do |condition|
82 88 if options[:through] && condition.is_a?(Hash)
83 89 condition = { table.name => condition }
84 90 end
@@ -87,12 +93,16 @@ def add_constraints(scope)
87 93 end
88 94 else
89 95 constraint = table[key].eq(foreign_table[foreign_key])
90   - join = join(foreign_table, constraint)
91 96
92   - scope = scope.joins(join)
  97 + if reflection.type
  98 + type = chain[i + 1].klass.base_class.name
  99 + constraint = constraint.and(table[reflection.type].eq(type))
  100 + end
  101 +
  102 + scope = scope.joins(join(foreign_table, constraint))
93 103
94   - unless conditions[i].empty?
95   - scope = scope.where(sanitize(conditions[i], table))
  104 + unless conditions.empty?
  105 + scope = scope.where(sanitize(conditions, table))
96 106 end
97 107 end
98 108 end
10 activerecord/lib/active_record/associations/join_dependency/join_association.rb
@@ -62,6 +62,7 @@ def find_parent_in(other_join_dependency)
62 62 def join_to(relation)
63 63 tables = @tables.dup
64 64 foreign_table = parent_table
  65 + foreign_klass = parent.active_record
65 66
66 67 # The chain starts with the target table, but we want to end with it here (makes
67 68 # more sense in this context), so we reverse
@@ -91,14 +92,17 @@ def join_to(relation)
91 92
92 93 constraint = build_constraint(reflection, table, key, foreign_table, foreign_key)
93 94
94   - unless conditions[i].empty?
95   - constraint = constraint.and(sanitize(conditions[i], table))
  95 + conditions = self.conditions[i].dup
  96 + conditions << { reflection.type => foreign_klass.base_class.name } if reflection.type
  97 +
  98 + unless conditions.empty?
  99 + constraint = constraint.and(sanitize(conditions, table))
96 100 end
97 101
98 102 relation.from(join(table, constraint))
99 103
100 104 # The current table in this iteration becomes the foreign table in the next
101   - foreign_table = table
  105 + foreign_table, foreign_klass = table, reflection.klass
102 106 end
103 107
104 108 relation
9 activerecord/lib/active_record/reflection.rb
@@ -212,7 +212,7 @@ def foreign_type
212 212 end
213 213
214 214 def type
215   - @type ||= "#{options[:as]}_type"
  215 + @type ||= options[:as] && "#{options[:as]}_type"
216 216 end
217 217
218 218 def primary_key_column
@@ -280,9 +280,7 @@ def chain
280 280 # in the #chain. The inside arrays are simply conditions (and each condition may itself be
281 281 # a hash, array, arel predicate, etc...)
282 282 def conditions
283   - conditions = [options[:conditions]].compact
284   - conditions << { type => active_record.base_class.name } if options[:as]
285   - [conditions]
  283 + [[options[:conditions]].compact]
286 284 end
287 285
288 286 alias :source_macro :macro
@@ -378,7 +376,8 @@ def derive_foreign_key
378 376 # Holds all the meta-data about a :through association as it was specified
379 377 # in the Active Record class.
380 378 class ThroughReflection < AssociationReflection #:nodoc:
381   - delegate :foreign_key, :foreign_type, :association_foreign_key, :active_record_primary_key, :to => :source_reflection
  379 + delegate :foreign_key, :foreign_type, :association_foreign_key,
  380 + :active_record_primary_key, :type, :to => :source_reflection
382 381
383 382 # Gets the source of the through reflection. It checks both a singularized
384 383 # and pluralized form for <tt>:belongs_to</tt> or <tt>:has_many</tt>.
7 activerecord/test/cases/associations/has_many_associations_test.rb
@@ -11,6 +11,7 @@
11 11 require 'models/person'
12 12 require 'models/reader'
13 13 require 'models/tagging'
  14 +require 'models/tag'
14 15 require 'models/invoice'
15 16 require 'models/line_item'
16 17 require 'models/car'
@@ -1468,4 +1469,10 @@ def test_new_is_called_with_attributes_and_options
1468 1469 bulb = car.bulbs.build({ :bulb_type => :custom }, :as => :admin)
1469 1470 assert_equal CustomBulb, bulb.class
1470 1471 end
  1472 +
  1473 + def test_abstract_class_with_polymorphic_has_many
  1474 + post = SubStiPost.create! :title => "fooo", :body => "baa"
  1475 + tagging = Tagging.create! :taggable => post
  1476 + assert_equal [tagging], post.taggings
  1477 + end
1471 1478 end
4 activerecord/test/cases/reflection_test.rb
@@ -216,7 +216,7 @@ def test_chain
216 216 def test_conditions
217 217 expected = [
218 218 [{ :tags => { :name => 'Blue' } }],
219   - [{ :taggings => { :comment => 'first' } }, { "taggable_type" => "Post" }],
  219 + [{ :taggings => { :comment => 'first' } }],
220 220 [{ :posts => { :title => ['misc post by bob', 'misc post by mary'] } }]
221 221 ]
222 222 actual = Author.reflect_on_association(:misc_post_first_blue_tags).conditions
@@ -224,7 +224,7 @@ def test_conditions
224 224
225 225 expected = [
226 226 [{ :tags => { :name => 'Blue' } }, { :taggings => { :comment => 'first' } }, { :posts => { :title => ['misc post by bob', 'misc post by mary'] } }],
227   - [{ "taggable_type" => "Post" }],
  227 + [],
228 228 []
229 229 ]
230 230 actual = Author.reflect_on_association(:misc_post_first_blue_tags_2).conditions

2 comments on commit d7a910e

Jon Leighton
Owner

@josevalim @tenderlove do you think it is okay to cherry-pick this into 3-1-stable? It fixes a regression from 3-0-stable.

Aaron Patterson
Owner

I'm ok with it.

Please sign in to comment.
Something went wrong with that request. Please try again.