Merge pull request #40043 from kamipo/fix_eager_loading_duplicated_association_part2

Fix incorrect result when eager loading with duplicated through association with join scope Part 2

activerecord/lib/active_record/associations/join_dependency.rb

@@ -174,24 +174,24 @@ def make_constraints(parent, child, join_type)
           foreign_table = parent.table
           foreign_klass = parent.base_klass
           child.join_constraints(foreign_table, foreign_klass, join_type, alias_tracker) do |reflection|
-            table = @joined_tables[reflection]
+            table, terminated = @joined_tables[reflection]
+            root = reflection == child.reflection
 
-            next table, true if table && reflection != child.reflection
+            if table && (!root || !terminated)
+              @joined_tables[reflection] = [table, root] if root
+              next table, true
+            end
 
             table = alias_tracker.aliased_table_for(reflection.klass.arel_table) do
-              table_alias_for(reflection, parent, reflection != child.reflection)
+              name = reflection.alias_candidate(parent.table_name)
+              root ? name : "#{name}_join"
             end
 
-            @joined_tables[reflection] ||= table if join_type == Arel::Nodes::OuterJoin
+            @joined_tables[reflection] ||= [table, root] if join_type == Arel::Nodes::OuterJoin
             table
           end.concat child.children.flat_map { |c| make_constraints(child, c, join_type) }
         end
 
-        def table_alias_for(reflection, parent, join)
-          name = reflection.alias_candidate(parent.table_name)
-          join ? "#{name}_join" : name
-        end
-
         def walk(left, right, join_type)
           intersection, missing = right.children.map { |node1|
             [left.children.find { |node2| node1.match? node2 }, node1]

activerecord/lib/active_record/associations/join_dependency/join_association.rb

@@ -21,19 +21,19 @@ def match?(other)
           super && reflection == other.reflection
         end
 
-        def join_constraints(foreign_table, foreign_klass, join_type, alias_tracker, &block)
+        def join_constraints(foreign_table, foreign_klass, join_type, alias_tracker)
           joins = []
           chain = []
 
           reflection.chain.each do |reflection|
             table, terminated = yield reflection
+            @table ||= table
 
             if terminated
               foreign_table, foreign_klass = table, reflection.klass
               break
             end
 
-            @table ||= table
             chain << [reflection, table]
           end
 

activerecord/test/cases/associations/has_many_through_associations_test.rb

@@ -1072,6 +1072,10 @@ def test_has_many_through_with_join_scope
   def test_duplicated_has_many_through_with_join_scope
     Categorization.create!(author: authors(:david), post: posts(:thinking), category: categories(:technology))
 
+    expected = [categorizations(:david_welcome_general)]
+    assert_equal expected, Author.preload(:general_posts, :general_categorizations).first.general_categorizations
+    assert_equal expected, Author.eager_load(:general_posts, :general_categorizations).first.general_categorizations
+
     expected = [posts(:welcome)]
     assert_equal expected, Author.preload(:general_categorizations, :general_posts).first.general_posts
     assert_equal expected, Author.eager_load(:general_categorizations, :general_posts).first.general_posts