diff options
author | Aaron Patterson <tenderlove@github.com> | 2018-11-20 09:26:55 -0800 |
---|---|---|
committer | GitHub <noreply@github.com> | 2018-11-20 09:26:55 -0800 |
commit | 023a840f5f10c5a611a0618ff8ea9e16cd771f93 (patch) | |
tree | 11f22cbd809078856f9f99c196f3e1453252faef /activerecord | |
parent | dc28cf8b0420774ae8e0d4410cdef0098ba34337 (diff) | |
parent | f9157587c9d61a0e24a0dcba7b31aa698ddeb641 (diff) | |
download | rails-023a840f5f10c5a611a0618ff8ea9e16cd771f93.tar.gz rails-023a840f5f10c5a611a0618ff8ea9e16cd771f93.tar.bz2 rails-023a840f5f10c5a611a0618ff8ea9e16cd771f93.zip |
Merge pull request #33954 from febeling/inconsistent-assignment-has-many-through-33942
Fix handling of duplicates for `replace` on has_many-through
Diffstat (limited to 'activerecord')
4 files changed, 43 insertions, 3 deletions
diff --git a/activerecord/lib/active_record/associations/collection_association.rb b/activerecord/lib/active_record/associations/collection_association.rb index 840d900bbc..48bb9ab066 100644 --- a/activerecord/lib/active_record/associations/collection_association.rb +++ b/activerecord/lib/active_record/associations/collection_association.rb @@ -413,9 +413,9 @@ module ActiveRecord end def replace_records(new_target, original_target) - delete(target - new_target) + delete(difference(target, new_target)) - unless concat(new_target - target) + unless concat(difference(new_target, target)) @target = original_target raise RecordNotSaved, "Failed to replace #{reflection.name} because one or more of the " \ "new records could not be saved." @@ -425,7 +425,7 @@ module ActiveRecord end def replace_common_records_in_memory(new_target, original_target) - common_records = new_target & original_target + common_records = intersection(new_target, original_target) common_records.each do |record| skip_callbacks = true replace_on_target(record, @target.index(record), skip_callbacks) diff --git a/activerecord/lib/active_record/associations/has_many_association.rb b/activerecord/lib/active_record/associations/has_many_association.rb index cf85a87fa7..e224d3456a 100644 --- a/activerecord/lib/active_record/associations/has_many_association.rb +++ b/activerecord/lib/active_record/associations/has_many_association.rb @@ -130,6 +130,14 @@ module ActiveRecord end saved_successfully end + + def difference(a, b) + a - b + end + + def intersection(a, b) + a & b + end end end end diff --git a/activerecord/lib/active_record/associations/has_many_through_association.rb b/activerecord/lib/active_record/associations/has_many_through_association.rb index f84ac65fa2..2322a49931 100644 --- a/activerecord/lib/active_record/associations/has_many_through_association.rb +++ b/activerecord/lib/active_record/associations/has_many_through_association.rb @@ -163,6 +163,28 @@ module ActiveRecord end end + def difference(a, b) + distribution = distribution(b) + + a.reject { |record| mark_occurrence(distribution, record) } + end + + def intersection(a, b) + distribution = distribution(b) + + a.select { |record| mark_occurrence(distribution, record) } + end + + def mark_occurrence(distribution, record) + distribution[record] > 0 && distribution[record] -= 1 + end + + def distribution(array) + array.each_with_object(Hash.new(0)) do |record, distribution| + distribution[record] += 1 + end + end + def through_records_for(record) attributes = construct_join_attributes(record) candidates = Array.wrap(through_association.target) 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)] |