Skip to content

Commit ea290e7

Browse files
gtdjeremy
authored andcommitted
Insert generated association members in the same order they are specified when assigning to a has_many :through using the generated *_ids method
[rails#3491 state:committed] Signed-off-by: Jeremy Kemper <jeremy@bitsweat.net>
1 parent fb61fbd commit ea290e7

File tree

2 files changed

+25
-2
lines changed

2 files changed

+25
-2
lines changed

activerecord/lib/active_record/associations.rb

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
require 'active_support/core_ext/module/delegation'
2+
require 'active_support/core_ext/enumerable'
23

34
module ActiveRecord
45
class InverseOfAssociationNotFoundError < ActiveRecordError #:nodoc:
@@ -1396,8 +1397,8 @@ def collection_accessor_methods(reflection, association_proxy_class, writer = tr
13961397
end
13971398

13981399
define_method("#{reflection.name.to_s.singularize}_ids=") do |new_value|
1399-
ids = (new_value || []).reject { |nid| nid.blank? }
1400-
send("#{reflection.name}=", reflection.klass.find(ids))
1400+
ids = (new_value || []).reject { |nid| nid.blank? }.map(&:to_i)
1401+
send("#{reflection.name}=", reflection.klass.find(ids).index_by(&:id).values_at(*ids))
14011402
end
14021403
end
14031404
end

activerecord/test/cases/associations/has_many_through_associations_test.rb

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -137,6 +137,28 @@ def test_replace_association
137137
assert !posts(:welcome).reload.people(true).include?(people(:michael))
138138
end
139139

140+
def test_replace_order_is_preserved
141+
posts(:welcome).people.clear
142+
posts(:welcome).people = [people(:david), people(:michael)]
143+
assert_equal [people(:david).id, people(:michael).id], posts(:welcome).readers.all(:order => 'id').map(&:person_id)
144+
145+
# Test the inverse order in case the first success was a coincidence
146+
posts(:welcome).people.clear
147+
posts(:welcome).people = [people(:michael), people(:david)]
148+
assert_equal [people(:michael).id, people(:david).id], posts(:welcome).readers.all(:order => 'id').map(&:person_id)
149+
end
150+
151+
def test_replace_by_id_order_is_preserved
152+
posts(:welcome).people.clear
153+
posts(:welcome).person_ids = [people(:david).id, people(:michael).id]
154+
assert_equal [people(:david).id, people(:michael).id], posts(:welcome).readers.all(:order => 'id').map(&:person_id)
155+
156+
# Test the inverse order in case the first success was a coincidence
157+
posts(:welcome).people.clear
158+
posts(:welcome).person_ids = [people(:michael).id, people(:david).id]
159+
assert_equal [people(:michael).id, people(:david).id], posts(:welcome).readers.all(:order => 'id').map(&:person_id)
160+
end
161+
140162
def test_associate_with_create
141163
assert_queries(1) { posts(:thinking) }
142164

0 commit comments

Comments
 (0)