test and fix collection_singular_ids= with string primary keys [rails#5125 state:resolved]

Signed-off-by: José Valim <[email protected]>

Author: paneq

activerecord/lib/active_record/associations.rb

@@ -1439,7 +1439,9 @@ def collection_accessor_methods(reflection, association_proxy_class, writer = tr
             end
 
             redefine_method("#{reflection.name.to_s.singularize}_ids=") do |new_value|
-              ids = (new_value || []).reject { |nid| nid.blank? }.map(&:to_i)
+              pk_column = reflection.klass.columns.find{|c| c.name == reflection.klass.primary_key }
+              ids = (new_value || []).reject { |nid| nid.blank? }
+              ids.map!{|i| pk_column.type_cast(i)}
               send("#{reflection.name}=", reflection.klass.find(ids).index_by(&:id).values_at(*ids))
             end
           end

activerecord/test/cases/associations/has_many_through_associations_test.rb

@@ -14,9 +14,14 @@
 require 'models/contract'
 require 'models/company'
 require 'models/developer'
+require 'models/subscriber'
+require 'models/book'
+require 'models/subscription'
 
 class HasManyThroughAssociationsTest < ActiveRecord::TestCase
-  fixtures :posts, :readers, :people, :comments, :authors, :owners, :pets, :toys, :jobs, :references, :companies
+  fixtures :posts, :readers, :people, :comments, :authors, 
+           :owners, :pets, :toys, :jobs, :references, :companies, 
+           :subscribers, :books, :subscriptions
 
   # Dummies to force column loads so query counts are clean.
   def setup
@@ -383,4 +388,37 @@ def test_modifying_has_many_through_has_one_reflection_should_raise
       lambda { authors(:david).very_special_comments.delete(authors(:david).very_special_comments.first) },
     ].each {|block| assert_raise(ActiveRecord::HasManyThroughCantAssociateThroughHasOneOrManyReflection, &block) }
   end
+
+  def test_collection_singular_ids_getter_with_string_primary_keys
+    book = books(:awdr)
+    assert_equal 2, book.subscriber_ids.size
+    assert_equal [subscribers(:first).nick, subscribers(:second).nick].sort, book.subscriber_ids.sort
+  end
+
+  def test_collection_singular_ids_setter
+    company = companies(:rails_core)
+    dev = Developer.find(:first)
+
+    company.developer_ids = [dev.id]
+    assert_equal [dev], company.developers
+  end
+
+  def test_collection_singular_ids_setter_with_string_primary_keys
+    assert_nothing_raised do
+      book = books(:awdr)
+      book.subscriber_ids = [subscribers(:second).nick]
+      assert_equal [subscribers(:second)], book.subscribers(true)
+
+      book.subscriber_ids = []
+      assert_equal [], book.subscribers(true)
+    end
+
+  end
+
+  def test_collection_singular_ids_setter_raises_exception_when_invalid_ids_set
+    company = companies(:rails_core)
+    ids =  [Developer.find(:first).id, -9999]
+    assert_raises(ActiveRecord::RecordNotFound) {company.developer_ids= ids}
+  end
+
 end

activerecord/test/fixtures/subscriptions.yml

@@ -9,4 +9,4 @@ webster_rfr:
 alterself_awdr:
   id: 3
   subscriber_id: alterself
-  book_id: 3
+  book_id: 1

activerecord/test/models/book.rb

@@ -1,4 +1,7 @@
 class Book < ActiveRecord::Base
   has_many :citations, :foreign_key => 'book1_id'
   has_many :references, :through => :citations, :source => :reference_of, :uniq => true
+
+  has_many :subscriptions
+  has_many :subscribers, :through => :subscriptions
 end