diff --git a/activerecord/lib/active_record/associations/association_scope.rb b/activerecord/lib/active_record/associations/association_scope.rb index 0e849c06efccd6babc6bd4c63f070a366b8a422d..4220a1499a8269d82204950f26e66c36b96af9d2 100644 --- a/activerecord/lib/active_record/associations/association_scope.rb +++ b/activerecord/lib/active_record/associations/association_scope.rb @@ -24,10 +24,10 @@ def scope(association) scope = klass.unscoped owner = association.owner alias_tracker = AliasTracker.create(klass.connection, klass.table_name) - chain_head, chain_tail = get_chain(reflection, association, alias_tracker) + chain = get_chain(reflection, association, alias_tracker) scope.extending! reflection.extensions - add_constraints(scope, owner, chain_head, chain_tail) + add_constraints(scope, owner, chain) end def join_type @@ -98,7 +98,6 @@ def next_chain_scope(scope, table, reflection, foreign_table, next_reflection) end class ReflectionProxy < SimpleDelegator # :nodoc: - attr_accessor :next attr_reader :alias_name def initialize(reflection, alias_name) @@ -111,36 +110,32 @@ def all_includes; nil; end def get_chain(reflection, association, tracker) name = reflection.name - runtime_reflection = Reflection::RuntimeReflection.new(reflection, association) - previous_reflection = runtime_reflection + chain = [Reflection::RuntimeReflection.new(reflection, association)] reflection.chain.drop(1).each do |refl| alias_name = tracker.aliased_table_for( refl.table_name, refl.alias_candidate(name), refl.klass.type_caster ) - proxy = ReflectionProxy.new(refl, alias_name) - previous_reflection.next = proxy - previous_reflection = proxy + chain << ReflectionProxy.new(refl, alias_name) end - [runtime_reflection, previous_reflection] + chain end - def add_constraints(scope, owner, chain_head, chain_tail) - owner_reflection = chain_tail + def add_constraints(scope, owner, chain) + owner_reflection = chain.last table = owner_reflection.alias_name scope = last_chain_scope(scope, table, owner_reflection, owner) - reflection = chain_head - while reflection + chain.each_cons(2) do |reflection, next_reflection| table = reflection.alias_name - next_reflection = reflection.next - - unless reflection == chain_tail - foreign_table = next_reflection.alias_name - scope = next_chain_scope(scope, table, reflection, foreign_table, next_reflection) - end + foreign_table = next_reflection.alias_name + scope = next_chain_scope(scope, table, reflection, foreign_table, next_reflection) + end + chain_head = chain.first + chain.reverse_each do |reflection| + table = reflection.alias_name # Exclude the scope of the association itself, because that # was already merged in the #scope method. reflection.constraints.each do |scope_chain_item| @@ -158,8 +153,6 @@ def add_constraints(scope, owner, chain_head, chain_tail) scope.where_clause += item.where_clause scope.order_values |= item.order_values end - - reflection = next_reflection end scope diff --git a/activerecord/lib/active_record/reflection.rb b/activerecord/lib/active_record/reflection.rb index 49ddcaecdf1f8a6226bf7e48167bc1dbfbce1d09..a01eb7b24206093b6f648555990bb118632e710e 100644 --- a/activerecord/lib/active_record/reflection.rb +++ b/activerecord/lib/active_record/reflection.rb @@ -1077,8 +1077,6 @@ def source_type_info end class RuntimeReflection < PolymorphicReflection # :nodoc: - attr_accessor :next - def initialize(reflection, association) @reflection = reflection @association = association 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 f8ea51225acefa19b17358df9333d02c9884d1c2..521b388ceef720d26d195ebbc7d7db4b79fa3101 100644 --- a/activerecord/test/cases/associations/has_many_through_associations_test.rb +++ b/activerecord/test/cases/associations/has_many_through_associations_test.rb @@ -1250,6 +1250,10 @@ def test_has_many_through_do_not_cache_association_reader_if_the_though_method_h TenantMembership.current_member = nil end + def test_has_many_through_with_unscope_should_affect_to_through_scope + assert_equal [comments(:eager_other_comment1)], authors(:mary).unordered_comments + end + def test_has_many_through_with_scope_should_respect_table_alias family = Family.create! users = 3.times.map { User.create! } diff --git a/activerecord/test/models/author.rb b/activerecord/test/models/author.rb index 0a52cc539017cbbea101c538cd2030df3128a56e..3371fcbfccccf349d71255f6613efc899496006b 100644 --- a/activerecord/test/models/author.rb +++ b/activerecord/test/models/author.rb @@ -40,6 +40,7 @@ def ratings class_name: "Post" has_many :comments_desc, -> { order("comments.id DESC") }, through: :posts, source: :comments + has_many :unordered_comments, -> { unscope(:order).distinct }, through: :posts_sorted_by_id_limited, source: :comments has_many :funky_comments, through: :posts, source: :comments has_many :ordered_uniq_comments, -> { distinct.order("comments.id") }, through: :posts, source: :comments has_many :ordered_uniq_comments_desc, -> { distinct.order("comments.id DESC") }, through: :posts, source: :comments