Insert generated association members in the same order they are specified when assigning to a has_many :through using the generated *_ids method

[#3491 state:committed]

Signed-off-by: Jeremy Kemper <jeremy@bitsweat.net>

2 files changed, 25 insertions, 2 deletions

diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb
index 6c5e25010f..fc6f15206a 100755
--- a/activerecord/lib/active_record/associations.rb
+++ b/activerecord/lib/active_record/associations.rb
@@ -1,4 +1,5 @@
 require 'active_support/core_ext/module/delegation'
+require 'active_support/core_ext/enumerable'
 
 module ActiveRecord
   class InverseOfAssociationNotFoundError < ActiveRecordError #:nodoc:
@@ -1396,8 +1397,8 @@ module ActiveRecord
             end
 
             define_method("#{reflection.name.to_s.singularize}_ids=") do |new_value|
-              ids = (new_value || []).reject { |nid| nid.blank? }
-              send("#{reflection.name}=", reflection.klass.find(ids))
+              ids = (new_value || []).reject { |nid| nid.blank? }.map(&:to_i)
+              send("#{reflection.name}=", reflection.klass.find(ids).index_by(&:id).values_at(*ids))
             end
           end
         end
diff --git a/activerecord/test/cases/associations/has_many_through_associations_test.rb b/activerecord/test/cases/associations/has_many_through_associations_test.rb
index 5f13b66d11..fe68d03de2 100644
--- a/activerecord/test/cases/associations/has_many_through_associations_test.rb
+++ b/activerecord/test/cases/associations/has_many_through_associations_test.rb
@@ -137,6 +137,28 @@ class HasManyThroughAssociationsTest < ActiveRecord::TestCase
     assert !posts(:welcome).reload.people(true).include?(people(:michael))
   end
 
+  def test_replace_order_is_preserved
+    posts(:welcome).people.clear
+    posts(:welcome).people = [people(:david), people(:michael)]
+    assert_equal [people(:david).id, people(:michael).id], posts(:welcome).readers.all(:order => 'id').map(&:person_id)
+
+    # Test the inverse order in case the first success was a coincidence
+    posts(:welcome).people.clear
+    posts(:welcome).people = [people(:michael), people(:david)]
+    assert_equal [people(:michael).id, people(:david).id], posts(:welcome).readers.all(:order => 'id').map(&:person_id)
+  end
+
+  def test_replace_by_id_order_is_preserved
+    posts(:welcome).people.clear
+    posts(:welcome).person_ids = [people(:david).id, people(:michael).id]
+    assert_equal [people(:david).id, people(:michael).id], posts(:welcome).readers.all(:order => 'id').map(&:person_id)
+
+    # Test the inverse order in case the first success was a coincidence
+    posts(:welcome).people.clear
+    posts(:welcome).person_ids = [people(:michael).id, people(:david).id]
+    assert_equal [people(:michael).id, people(:david).id], posts(:welcome).readers.all(:order => 'id').map(&:person_id)
+  end
+
   def test_associate_with_create
     assert_queries(1) { posts(:thinking) }