diff options
author | Bogdan Gusiev <agresso@gmail.com> | 2019-02-13 12:25:43 +0200 |
---|---|---|
committer | Bogdan Gusiev <agresso@gmail.com> | 2019-02-20 13:48:51 +0200 |
commit | bd4eff2f99ffa5868574c497bc63f320cbda3083 (patch) | |
tree | b21d4bdea255230b35ebc142472aaf3120dfed33 /activerecord | |
parent | 4cb1438b57067d637c79d49d0662c43b5b4e64c2 (diff) | |
download | rails-bd4eff2f99ffa5868574c497bc63f320cbda3083.tar.gz rails-bd4eff2f99ffa5868574c497bc63f320cbda3083.tar.bz2 rails-bd4eff2f99ffa5868574c497bc63f320cbda3083.zip |
Fix reset of the source association when through association is loaded
The special case happens when through association has a custom scope
that is applied to the source association when loading.
In this case, the soucre association would need to be reset after
main association is loaded. See tests.
The special case exists when a through association has
Diffstat (limited to 'activerecord')
5 files changed, 22 insertions, 24 deletions
diff --git a/activerecord/lib/active_record/associations/preloader/association.rb b/activerecord/lib/active_record/associations/preloader/association.rb index d6f7359055..85a994b27a 100644 --- a/activerecord/lib/active_record/associations/preloader/association.rb +++ b/activerecord/lib/active_record/associations/preloader/association.rb @@ -116,7 +116,7 @@ module ActiveRecord def build_scope scope = klass.scope_for_association - if reflection.type + if reflection.type && !reflection.through_reflection? scope.where!(reflection.type => model.polymorphic_name) end diff --git a/activerecord/lib/active_record/associations/preloader/through_association.rb b/activerecord/lib/active_record/associations/preloader/through_association.rb index a6b7ab80a2..25254e652a 100644 --- a/activerecord/lib/active_record/associations/preloader/through_association.rb +++ b/activerecord/lib/active_record/associations/preloader/through_association.rb @@ -7,10 +7,9 @@ module ActiveRecord def run(preloader) already_loaded = owners.first.association(through_reflection.name).loaded? through_scope = through_scope() - reflection_scope = target_reflection_scope through_preloaders = preloader.preload(owners, through_reflection.name, through_scope) middle_records = through_preloaders.flat_map(&:preloaded_records) - preloaders = preloader.preload(middle_records, source_reflection.name, reflection_scope) + preloaders = preloader.preload(middle_records, source_reflection.name, scope) @preloaded_records = preloaders.flat_map(&:preloaded_records) owners.each do |owner| @@ -25,18 +24,18 @@ module ActiveRecord owner.association(through_reflection.name).reset if through_scope end result = through_records.flat_map do |record| - association = record.association(source_reflection.name) - target = association.target - association.reset if preload_scope - target + record.association(source_reflection.name).target end result.compact! - if reflection_scope - result.sort_by! { |rhs| preload_index[rhs] } if reflection_scope.order_values.any? - result.uniq! if reflection_scope.distinct_value - end + result.sort_by! { |rhs| preload_index[rhs] } if scope.order_values.any? + result.uniq! if scope.distinct_value associate_records_to_owner(owner, result) end + unless scope.empty_scope? + middle_records.each do |owner| + owner.association(source_reflection.name).reset + end + end end private @@ -91,16 +90,6 @@ module ActiveRecord scope unless scope.empty_scope? end - - def target_reflection_scope - if preload_scope - reflection_scope.merge(preload_scope) - elsif reflection.scope - reflection_scope - else - nil - end - end end end end diff --git a/activerecord/test/cases/associations/has_many_associations_test.rb b/activerecord/test/cases/associations/has_many_associations_test.rb index 5fdc5a92fc..38e25a9100 100644 --- a/activerecord/test/cases/associations/has_many_associations_test.rb +++ b/activerecord/test/cases/associations/has_many_associations_test.rb @@ -2080,10 +2080,12 @@ class HasManyAssociationsTest < ActiveRecord::TestCase end def test_associations_order_should_be_priority_over_throughs_order - david = authors(:david) + original = authors(:david) expected = [12, 10, 9, 8, 7, 6, 5, 3, 2, 1] - assert_equal expected, david.comments_desc.map(&:id) - assert_equal expected, Author.includes(:comments_desc).find(david.id).comments_desc.map(&:id) + assert_equal expected, original.comments_desc.map(&:id) + preloaded = Author.includes(:comments_desc).find(original.id) + assert_equal expected, preloaded.comments_desc.map(&:id) + assert_equal original.posts_sorted_by_id.first.comments.map(&:id), preloaded.posts_sorted_by_id.first.comments.map(&:id) end def test_dynamic_find_should_respect_association_order_for_through diff --git a/activerecord/test/cases/associations/nested_through_associations_test.rb b/activerecord/test/cases/associations/nested_through_associations_test.rb index 5821744530..0b83fd8421 100644 --- a/activerecord/test/cases/associations/nested_through_associations_test.rb +++ b/activerecord/test/cases/associations/nested_through_associations_test.rb @@ -610,6 +610,12 @@ class NestedThroughAssociationsTest < ActiveRecord::TestCase assert_equal hotel, Hotel.joins(:cake_designers, :drink_designers).take end + def test_has_many_through_reset_source_reflection_after_loading_is_complete + preloaded = Category.preload(:ordered_post_comments).find(1, 2).last + original = Category.find(2) + assert_equal original.ordered_post_comments.ids, preloaded.ordered_post_comments.ids + end + private def assert_includes_and_joins_equal(query, expected, association) diff --git a/activerecord/test/models/category.rb b/activerecord/test/models/category.rb index 2ccc00bed9..8c86879dc6 100644 --- a/activerecord/test/models/category.rb +++ b/activerecord/test/models/category.rb @@ -26,6 +26,7 @@ class Category < ActiveRecord::Base has_many :categorizations has_many :special_categorizations has_many :post_comments, through: :posts, source: :comments + has_many :ordered_post_comments, -> { order(id: :desc) }, through: :posts, source: :comments has_many :authors, through: :categorizations has_many :authors_with_select, -> { select "authors.*, categorizations.post_id" }, through: :categorizations, source: :author |