Sanitize scoped conditions.

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

activerecord/CHANGELOG

@@ -1,5 +1,7 @@
 *SVN*
 
+* Sanitize scoped conditions. [Marcel Molina Jr.]
+
 * Added option to Base.reflection_of_all_associations to specify a specific association to scope the call. For example Base.reflection_of_all_associations(:has_many) [DHH]
 
 * Added ActiveRecord::SchemaDumper.ignore_tables which tells SchemaDumper which tables to ignore. Useful for tables with funky column like the ones required for tsearch2. [TobiasLuetke]

activerecord/lib/active_record/base.rb

@@ -944,7 +944,7 @@ module ActiveRecord #:nodoc:
 
         # Adds a sanitized version of +conditions+ to the +sql+ string. Note that the passed-in +sql+ string is changed.
         def add_conditions!(sql, conditions)          
-          segments = [scope(:find, :conditions)]
+          segments = [sanitize_sql(scope(:find, :conditions))]
           segments << sanitize_sql(conditions) unless conditions.nil?
           segments << type_condition unless descends_from_active_record?        
           segments.compact!

activerecord/test/base_test.rb

@@ -1082,27 +1082,24 @@ class BasicsTest < Test::Unit::TestCase
   end
 
   def test_scoped_find_conditions
-    developers = Developer.with_scope(:find => { :conditions => 'salary > 90000' }) do
+    scoped_developers = Developer.with_scope(:find => { :conditions => 'salary > 90000' }) do
       Developer.find(:all, :conditions => 'id < 5')
     end
-    david = Developer.find(1)
-    assert !developers.include?(david) # David's salary is less than 90,000
-    assert_equal 3, developers.size
+    assert !scoped_developers.include?(developers(:david)) # David's salary is less than 90,000
+    assert_equal 3, scoped_developers.size
   end
   
   def test_scoped_find_limit_offset
-    developers = Developer.with_scope(:find => { :limit => 3, :offset => 2 }) do
+    scoped_developers = Developer.with_scope(:find => { :limit => 3, :offset => 2 }) do
       Developer.find(:all, :order => 'id')
     end    
-    david = Developer.find(1)
-    jamis = Developer.find(1)
-    assert !developers.include?(david) # David has id 1
-    assert !developers.include?(jamis) # Jamis has id 2
-    assert_equal 3, developers.size
+    assert !scoped_developers.include?(developers(:david))
+    assert !scoped_developers.include?(developers(:jamis))
+    assert_equal 3, scoped_developers.size
     
     # Test without scoped find conditions to ensure we get the whole thing
     developers = Developer.find(:all, :order => 'id')
-    assert_equal 10, developers.size
+    assert_equal Developer.count, developers.size
   end
   
   # FIXME: this test ought to run, but it needs to run sandboxed so that it

activerecord/test/fixtures/developers.yml

@@ -13,4 +13,9 @@ dev_<%= digit %>:
   id: <%= digit %>
   name: fixture_<%= digit %>
   salary: 100000
-<% end %>
+<% end %>
+
+poor_jamis:
+  id: 11
+  name: Jamis
+  salary: 9000

activerecord/test/fixtures_test.rb

@@ -136,7 +136,7 @@ class FixturesTest < Test::Unit::TestCase
   end
 
   def test_erb_in_fixtures
-    assert_equal 10, @developers.size
+    assert_equal 11, @developers.size
     assert_equal "fixture_5", @dev_5.name
   end
 

activerecord/test/method_scoping_test.rb

@@ -25,9 +25,27 @@ class MethodScopingTest < Test::Unit::TestCase
     end
   end
   
+  def test_scoped_find_combines_conditions
+    Developer.with_scope(:find => { :conditions => "salary = 9000" }) do
+      assert_equal developers(:poor_jamis), Developer.find(:first, :conditions => "name = 'Jamis'")
+    end
+  end
+  
+  def test_scoped_find_sanitizes_conditions
+    Developer.with_scope(:find => { :conditions => ['salary = ?', 9000] }) do
+      assert_equal developers(:poor_jamis), Developer.find(:first)
+    end  
+  end
+  
+  def test_scoped_find_combines_and_sanitizes_conditions
+    Developer.with_scope(:find => { :conditions => ['salary = ?', 9000] }) do
+      assert_equal developers(:poor_jamis), Developer.find(:first, :conditions => ['name = ?', 'Jamis'])
+    end  
+  end
+  
   def test_scoped_find_all
     Developer.with_scope(:find => { :conditions => "name = 'David'" }) do
-      assert_equal [Developer.find(1)], Developer.find(:all)
+      assert_equal [developers(:david)], Developer.find(:all)
     end      
   end