Allow find on a has_many association defined with :finder_sql to accept id arguments as strings like regular find does. Closes #9916 [krishna]

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

5 files changed, 22 insertions, 3 deletions

diff --git a/activerecord/CHANGELOG b/activerecord/CHANGELOG
index 77f882f143..61b060796b 100644
--- a/activerecord/CHANGELOG
+++ b/activerecord/CHANGELOG
@@ -1,5 +1,7 @@
 *SVN*
 
+* Allow find on a has_many association defined with :finder_sql to accept id arguments as strings like regular find does. Closes #9916 [krishna]
+
 * Use VALID_FIND_OPTIONS when resolving :find scoping rather than hard coding the list of valid find options. Closes #9443 [sur]
 
 * Limited eager loading no longer ignores scoped :order. Closes #9561 [danger, josh]
diff --git a/activerecord/lib/active_record/associations/has_many_association.rb b/activerecord/lib/active_record/associations/has_many_association.rb
index b1b9895832..2880bde3a4 100644
--- a/activerecord/lib/active_record/associations/has_many_association.rb
+++ b/activerecord/lib/active_record/associations/has_many_association.rb
@@ -37,7 +37,7 @@ module ActiveRecord
         # If using a custom finder_sql, scan the entire collection.
         if @reflection.options[:finder_sql]
           expects_array = args.first.kind_of?(Array)
-          ids = args.flatten.compact.uniq
+          ids           = args.flatten.compact.uniq.map(&:to_i)
 
           if ids.size == 1
             id = ids.first
diff --git a/activerecord/test/associations_test.rb b/activerecord/test/associations_test.rb
index 03431049ae..9f155dd4ab 100755
--- a/activerecord/test/associations_test.rb
+++ b/activerecord/test/associations_test.rb
@@ -535,6 +535,22 @@ class HasManyAssociationsTest < Test::Unit::TestCase
     assert_raises(ActiveRecord::RecordNotFound) { firm.clients.find(2, 99) }
   end
 
+  def test_find_string_ids_when_using_finder_sql
+    firm = Firm.find(:first)
+
+    client = firm.clients_using_finder_sql.find("2")
+    assert_kind_of Client, client
+
+    client_ary = firm.clients_using_finder_sql.find(["2"])
+    assert_kind_of Array, client_ary
+    assert_equal client, client_ary.first
+
+    client_ary = firm.clients_using_finder_sql.find("2", "3")
+    assert_kind_of Array, client_ary
+    assert_equal 2, client_ary.size
+    assert_equal client, client_ary.first
+  end
+
   def test_find_all
     firm = Firm.find(:first)
     assert_equal 2, firm.clients.find(:all, :conditions => "#{QUOTED_TYPE} = 'Client'").length
diff --git a/activerecord/test/fixtures/company.rb b/activerecord/test/fixtures/company.rb
index 0c93c80e03..1048d5a652 100755
--- a/activerecord/test/fixtures/company.rb
+++ b/activerecord/test/fixtures/company.rb
@@ -38,6 +38,7 @@ class Firm < Company
   has_many :no_clients_using_counter_sql, :class_name => "Client",
            :finder_sql  => 'SELECT * FROM companies WHERE client_of = 1000',
            :counter_sql => 'SELECT COUNT(*) FROM companies WHERE client_of = 1000'
+  has_many :clients_using_finder_sql, :class_name => "Client", :finder_sql => 'SELECT * FROM companies WHERE 1=1'
   has_many :plain_clients, :class_name => 'Client'
 
   has_one :account, :foreign_key => "firm_id", :dependent => :destroy
diff --git a/activerecord/test/reflection_test.rb b/activerecord/test/reflection_test.rb
index eca8b5e989..1ed5c2610c 100644
--- a/activerecord/test/reflection_test.rb
+++ b/activerecord/test/reflection_test.rb
@@ -159,8 +159,8 @@ class ReflectionTest < Test::Unit::TestCase
   end
   
   def test_reflection_of_all_associations
-    assert_equal 16, Firm.reflect_on_all_associations.size
-    assert_equal 14, Firm.reflect_on_all_associations(:has_many).size
+    assert_equal 17, Firm.reflect_on_all_associations.size
+    assert_equal 15, Firm.reflect_on_all_associations(:has_many).size
     assert_equal 2, Firm.reflect_on_all_associations(:has_one).size
     assert_equal 0, Firm.reflect_on_all_associations(:belongs_to).size
   end