From 8427b3a38b72602623080bf345e925367b76c9bd Mon Sep 17 00:00:00 2001 From: Gabe da Silveira Date: Tue, 17 Nov 2009 09:36:23 -0800 Subject: [PATCH] Insert generated association members in the same order they are specified when assigning to a has_many :through using the generated *_ids method --- activerecord/lib/active_record/associations.rb | 4 ++- .../has_many_through_associations_test.rb | 22 ++++++++++++++++++++ 2 files changed, 25 insertions(+), 1 deletions(-) diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb index 7e997fa..8058af6 100755 --- a/activerecord/lib/active_record/associations.rb +++ b/activerecord/lib/active_record/associations.rb @@ -1328,7 +1328,9 @@ module ActiveRecord 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)) + hashed_objects = reflection.klass.find(ids).inject({}){ |h,obj| h[obj.id]=obj; h } + ordered_objects = ids.map{ |id| hashed_objects[id.to_i] } + send("#{reflection.name}=", ordered_objects) 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 a43f876..aae2047 100644 --- a/activerecord/test/cases/associations/has_many_through_associations_test.rb +++ b/activerecord/test/cases/associations/has_many_through_associations_test.rb @@ -132,6 +132,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) } -- 1.6.0.2