Skip to content
This repository
Browse code

Fix for 3-0-stable - Conditions specified on through association shou…

…ldn't clobber asssociation join condition.

This fix refactors processing of association join conditions so that both the join condition and the custom condition will be used when called by query_methods.rb, which expects a 1 or 2-sized array (depending on the type of association). Previously, a custom condition specified would create a 2 or 3-sized array which will clobber the association join condition.
  • Loading branch information...
commit caec63929fabe0368da71128c96e74c174791eb9 1 parent 28f057c
Thong Kuah authored June 21, 2011
20  activerecord/lib/active_record/associations.rb
@@ -2145,6 +2145,7 @@ def association_join
2145 2145
               parent_table = Arel::Table.new(parent.table_name, :as      => parent.aliased_table_name,
2146 2146
                                                                 :engine  => arel_engine,
2147 2147
                                                                 :columns => parent.active_record.columns)
  2148
+              as_conditions = reflection.options[:conditions] && process_conditions(reflection.options[:conditions], aliased_table_name)
2148 2149
 
2149 2150
               @join = case reflection.macro
2150 2151
               when :has_and_belongs_to_many
@@ -2154,11 +2155,12 @@ def association_join
2154 2155
 
2155 2156
                 [
2156 2157
                   join_table[fk].eq(parent_table[reflection.active_record.primary_key]),
2157  
-                  aliased_table[klass.primary_key].eq(join_table[klass_fk])
  2158
+                  [aliased_table[klass.primary_key].eq(join_table[klass_fk]), as_conditions].reject{ |x| x.blank? }
2158 2159
                 ]
2159 2160
               when :has_many, :has_one
2160 2161
                 if reflection.options[:through]
2161 2162
                   join_table = Arel::Table.new(through_reflection.klass.table_name, :as => aliased_join_table_name, :engine => arel_engine)
  2163
+                  jt_as_conditions = through_reflection.options[:conditions] && process_conditions(through_reflection.options[:conditions], aliased_table_name)
2162 2164
                   jt_as_extra = jt_source_extra = jt_sti_extra = nil
2163 2165
                   first_key = second_key = as_extra = nil
2164 2166
 
@@ -2199,19 +2201,19 @@ def association_join
2199 2201
                   end
2200 2202
 
2201 2203
                   [
2202  
-                    [parent_table[jt_primary_key].eq(join_table[jt_foreign_key]), jt_as_extra, jt_source_extra, jt_sti_extra].reject{|x| x.blank? },
2203  
-                    [aliased_table[first_key].eq(join_table[second_key]), as_extra].reject{ |x| x.blank? }
  2204
+                    [parent_table[jt_primary_key].eq(join_table[jt_foreign_key]), jt_as_extra, jt_source_extra, jt_sti_extra, jt_as_conditions].reject{|x| x.blank? },
  2205
+                    [aliased_table[first_key].eq(join_table[second_key]), as_extra, as_conditions].reject{ |x| x.blank? }
2204 2206
                   ]
2205 2207
                 elsif reflection.options[:as]
2206 2208
                   id_rel = aliased_table["#{reflection.options[:as]}_id"].eq(parent_table[parent.primary_key])
2207 2209
                   type_rel = aliased_table["#{reflection.options[:as]}_type"].eq(parent.active_record.base_class.name)
2208  
-                  [id_rel, type_rel]
  2210
+                  [id_rel, type_rel, as_conditions].reject{ |x| x.blank?}
2209 2211
                 else
2210 2212
                   foreign_key = options[:foreign_key] || reflection.active_record.name.foreign_key
2211  
-                  [aliased_table[foreign_key].eq(parent_table[reflection.options[:primary_key] || parent.primary_key])]
  2213
+                  [aliased_table[foreign_key].eq(parent_table[reflection.options[:primary_key] || parent.primary_key]), as_conditions].reject{ |x| x.blank? }
2212 2214
                 end
2213 2215
               when :belongs_to
2214  
-                [aliased_table[options[:primary_key] || reflection.klass.primary_key].eq(parent_table[options[:foreign_key] || reflection.primary_key_name])]
  2216
+                [aliased_table[options[:primary_key] || reflection.klass.primary_key].eq(parent_table[options[:foreign_key] || reflection.primary_key_name]), as_conditions].reject{ |x| x.blank? }
2215 2217
               end
2216 2218
 
2217 2219
               unless klass.descends_from_active_record?
@@ -2222,12 +2224,6 @@ def association_join
2222 2224
                 @join << sti_condition
2223 2225
               end
2224 2226
 
2225  
-              [through_reflection, reflection].each do |ref|
2226  
-                if ref && ref.options[:conditions]
2227  
-                  @join << process_conditions(ref.options[:conditions], aliased_table_name)
2228  
-                end
2229  
-              end
2230  
-
2231 2227
               @join
2232 2228
             end
2233 2229
 
15  activerecord/test/cases/associations/has_many_through_associations_test.rb
@@ -482,6 +482,21 @@ def test_joining_has_many_through_belongs_to
482 482
     assert_equal [posts(:eager_other)], posts
483 483
   end
484 484
 
  485
+  def test_join_on_has_many_association_collection_with_conditions
  486
+    posts(:welcome).tags.create!(:name => 'Misc')
  487
+    invalid_posts = Post.joins(:misc_tags).where('posts.id' => posts(:welcome).id).where('taggings.tag_id != tags.id')
  488
+    assert_equal [], invalid_posts
  489
+
  490
+    posts = Post.joins(:misc_tags).where('posts.id' => posts(:welcome).id)
  491
+    assert_equal [posts(:welcome)], posts
  492
+
  493
+    invalid_posts = Post.all(:joins => :misc_tags, :conditions => ['posts.id =? and taggings.tag_id != tags.id', posts(:welcome).id])
  494
+    assert_equal [], invalid_posts
  495
+
  496
+    posts = Post.all(:joins => :misc_tags, :conditions => {:posts => {:id => posts(:welcome).id}})
  497
+    assert_equal [posts(:welcome)], posts
  498
+  end
  499
+
485 500
   def test_interpolated_conditions
486 501
     post = posts(:welcome)
487 502
     assert !post.tags.empty?
5  activerecord/test/cases/associations/join_model_test.rb
@@ -342,6 +342,11 @@ def test_has_many_polymorphic
342 342
     end
343 343
   end
344 344
 
  345
+  def test_polymorphic_join_with_conditions
  346
+    assert_equal [], Author.joins(:posts => :invalid_taggings).all
  347
+    assert_equal [], Post.joins(:invalid_taggings).all
  348
+  end
  349
+
345 350
   def test_has_many_polymorphic_with_source_type
346 351
     # added sort by ID as otherwise Oracle select sometimes returned rows in different order
347 352
     assert_equal posts(:welcome, :thinking).sort_by(&:id), tags(:general).tagged_posts.sort_by(&:id)

0 notes on commit caec639

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