diff options
author | Florian Ebeling <florian.ebeling@gmail.com> | 2018-09-23 15:18:57 +0200 |
---|---|---|
committer | Florian Ebeling <florian.ebeling@gmail.com> | 2018-11-06 17:56:58 +0100 |
commit | 8104589c0824c648a769be50e04fc8e7dbb26ba0 (patch) | |
tree | 99093e22d28fbc1c5fce77f43c2285466fc7f349 /activerecord/test | |
parent | 212c28ac86fec0f2baf57fbc21ceb8696092fe47 (diff) | |
download | rails-8104589c0824c648a769be50e04fc8e7dbb26ba0.tar.gz rails-8104589c0824c648a769be50e04fc8e7dbb26ba0.tar.bz2 rails-8104589c0824c648a769be50e04fc8e7dbb26ba0.zip |
Fix handling of duplicates for `replace` on has_many-through
There was a bug in the handling of duplicates when
assigning (replacing) associated records, which made the result
dependent on whether a given record was associated already before
being assigned anew. E.g.
post.people = [person, person]
post.people.count
# => 2
while
post.people = [person]
post.people = [person, person]
post.people.count
# => 1
This change adds a test to provoke the former incorrect behavior, and
fixes it.
Cause of the bug was the handling of record collections as sets, and
using `-` (difference) and `&` (union) operations on them
indiscriminately. This temporary conversion to sets would eliminate
duplicates.
The fix is to decorate record collections for these operations, and
only for the `has_many :through` case. It is done by counting
occurrences, and use the record together with the occurrence number as
element, in order to make them work well in sets. Given
a, b = *Person.all
then the collection used for finding the difference or union of
records would be internally changed from
[a, b, a]
to
[[a, 1], [b, 1], [a, 2]]
for these operations. So a first occurrence and a second occurrence
would be distinguishable, which is all that is necessary for this
task.
Fixes #33942.
Diffstat (limited to 'activerecord/test')
-rw-r--r-- | activerecord/test/cases/associations/has_many_through_associations_test.rb | 10 |
1 files changed, 10 insertions, 0 deletions
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 7b405c74c4..cf514957ca 100644 --- a/activerecord/test/cases/associations/has_many_through_associations_test.rb +++ b/activerecord/test/cases/associations/has_many_through_associations_test.rb @@ -586,6 +586,16 @@ class HasManyThroughAssociationsTest < ActiveRecord::TestCase assert_not_includes posts(:welcome).reload.people.reload, people(:michael) end + def test_replace_association_with_duplicates + post = posts(:thinking) + person = people(:david) + + assert_difference "post.people.count", 2 do + post.people = [person] + post.people = [person, person] + end + end + def test_replace_order_is_preserved posts(:welcome).people.clear posts(:welcome).people = [people(:david), people(:michael)] |