Be sure to use the @finder_sql in the has_many association's #find method, even if explicit conditions have not been given.

git-svn-id: http://svn-commit.rubyonrails.org/rails/trunk@1412 5ecf4fe2-1ee6-0310-87b1-e25e094e27de

activerecord/lib/active_record/associations/has_many_association.rb

@@ -66,9 +66,11 @@ module ActiveRecord
             load_target.select { |record| ids.include?(record.id) }
           end
         else
-          if options[:conditions] = sanitize_sql(options[:conditions])
-            options[:conditions] = "#{@finder_sql} AND #{options[:conditions]}"
+          conditions = "#{@finder_sql}"
+          if sanitized_conditions = sanitize_sql(options[:conditions])
+            conditions << " AND #{sanitized_conditions}"
           end
+          options[:conditions] = conditions
 
           if options[:order] && @options[:order]
             options[:order] = "#{options[:order]}, #{@options[:order]}"

activerecord/test/associations_test.rb

@@ -540,9 +540,10 @@ class HasManyAssociationsTest < Test::Unit::TestCase
   end
 
   def test_dependence
-    assert_equal 2, Client.find_all.size
-    Firm.find_first.destroy
-    assert Client.find_all.empty?
+    firm = companies(:first_firm)
+    assert_equal 2, firm.clients.size
+    firm.destroy
+    assert Client.find(:all, :conditions => "firm_id=#{firm.id}").empty?
   end
 
   def test_destroy_dependent_when_deleted_from_association
@@ -567,14 +568,14 @@ class HasManyAssociationsTest < Test::Unit::TestCase
 
   uses_transaction :test_dependence_with_transaction_support_on_failure
   def test_dependence_with_transaction_support_on_failure
-    assert_equal 2, Client.find_all.length
-    firm = Firm.find_first
+    firm = companies(:first_firm)
     clients = firm.clients
+    assert_equal 2, clients.length
     clients.last.instance_eval { def before_destroy() raise "Trigger rollback" end }
 
     firm.destroy rescue "do nothing"
 
-    assert_equal 2, Client.find_all.length
+    assert_equal 2, Client.find(:all, :conditions => "firm_id=#{firm.id}").size
   end
 
   def test_dependence_on_account
@@ -590,6 +591,11 @@ class HasManyAssociationsTest < Test::Unit::TestCase
   def test_adding_array_and_collection
     assert_nothing_raised { Firm.find_first.clients + Firm.find_all.last.clients }
   end
+
+  def test_find_all_without_conditions
+    firm = companies(:first_firm)
+    assert_equal 2, firm.clients.find(:all).length
+  end
 end
 
 class BelongsToAssociationsTest < Test::Unit::TestCase

activerecord/test/base_test.rb

@@ -283,8 +283,9 @@ class BasicsTest < Test::Unit::TestCase
   end
 
   def test_destroy_many
+    assert_equal 3, Client.count
     Client.destroy([2, 3])
-    assert_equal 0, Client.count
+    assert_equal 1, Client.count
   end
 
   def test_delete_many

activerecord/test/deprecated_associations_test.rb

@@ -60,14 +60,14 @@ class DeprecatedAssociationsTest < Test::Unit::TestCase
   end
 
   def test_has_many_dependence
-    assert_equal 2, Client.find_all.length
+    assert_equal 3, Client.find_all.length
     Firm.find_first.destroy
-    assert_equal 0, Client.find_all.length
+    assert_equal 1, Client.find_all.length
   end
 
   uses_transaction :test_has_many_dependence_with_transaction_support_on_failure
   def test_has_many_dependence_with_transaction_support_on_failure
-    assert_equal 2, Client.find_all.length
+    assert_equal 3, Client.find_all.length
 
     firm = Firm.find_first
     clients = firm.clients
@@ -75,7 +75,7 @@ class DeprecatedAssociationsTest < Test::Unit::TestCase
 
     firm.destroy rescue "do nothing"
 
-    assert_equal 2, Client.find_all.length
+    assert_equal 3, Client.find_all.length
   end
 
   def test_has_one_dependence

activerecord/test/fixtures/companies.yml

@@ -19,3 +19,17 @@ second_client:
   client_of: 1
   name: Microsoft
   ruby_type: Client
+
+another_firm:
+  id: 4
+  type: Firm
+  name: Flamboyant Software
+  ruby_type: Firm
+
+another_client:
+  id: 5
+  type: Client
+  firm_id: 4
+  client_of: 4
+  name: Ex Nihilo
+  ruby_type: Client

activerecord/test/inheritance_test.rb

@@ -48,9 +48,9 @@ class InheritanceTest < Test::Unit::TestCase
   end
 
   def test_inheritance_condition
-    assert_equal 3, Company.find_all.length
-    assert_equal 1, Firm.find_all.length
-    assert_equal 2, Client.find_all.length
+    assert_equal 5, Company.find_all.length
+    assert_equal 2, Firm.find_all.length
+    assert_equal 3, Client.find_all.length
   end
   
   def test_alt_inheritance_condition
@@ -82,7 +82,7 @@ class InheritanceTest < Test::Unit::TestCase
   def test_destroy_all_within_inheritance
     Client.destroy_all
     assert_equal 0, Client.find_all.length
-    assert_equal 1, Firm.find_all.length
+    assert_equal 2, Firm.find_all.length
   end
   
   def test_alt_destroy_all_within_inheritance
@@ -132,4 +132,4 @@ class InheritanceTest < Test::Unit::TestCase
     
       def Company.inheritance_column() "ruby_type" end
     end
-end
+end