Permalink
Browse files

Insert generated association members in the same order they are speci…

…fied when assigning to a has_many :through using the generated *_ids method

[#3491 state:committed]

Signed-off-by: Jeremy Kemper <jeremy@bitsweat.net>
  • Loading branch information...
1 parent ed320cd commit df0720b8b74bd6d41d25390d332000b882b711d9 @gtd gtd committed with jeremy Nov 17, 2009
View
4 activerecord/lib/active_record/associations.rb
@@ -1327,8 +1327,8 @@ def collection_accessor_methods(reflection, association_proxy_class, writer = tr
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
View
22 activerecord/test/cases/associations/has_many_through_associations_test.rb
@@ -132,6 +132,28 @@ def test_replace_association
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 comment on commit df0720b

@blindninja

By mapping the ids to_i, it is causing problem with tables whose primary key is not an integer. Can the same be accomplished without to_i?

Please sign in to comment.