From c9f234499c56a13a838f82cc077d35dfaca1c214 Mon Sep 17 00:00:00 2001
From: David Roetzel <david@roetzel.de>
Date: Wed, 18 Oct 2023 12:35:24 +0200
Subject: [PATCH] Fix wrong table aliases (#1217)

This simply mirrors ActiveRecord's "deduplication" of joined
tables, to not use table aliases that are not actually in the
final query's JOIN clause.
---
 .../join_dependency.rb                        | 18 +++++++++---
 .../adapters/active_record/base_spec.rb       | 16 +++++++++++
 spec/support/schema.rb                        | 28 +++++++++++++++++++
 3 files changed, 58 insertions(+), 4 deletions(-)

diff --git a/lib/polyamorous/activerecord_6.1_ruby_2/join_dependency.rb b/lib/polyamorous/activerecord_6.1_ruby_2/join_dependency.rb
index f9cd1d8c..0bcee6f7 100644
--- a/lib/polyamorous/activerecord_6.1_ruby_2/join_dependency.rb
+++ b/lib/polyamorous/activerecord_6.1_ruby_2/join_dependency.rb
@@ -56,11 +56,21 @@ def construct_tables_for_association!(join_root, association)
     private
 
     def table_aliases_for(parent, node)
+      @joined_tables ||= {}
       node.reflection.chain.map { |reflection|
-        alias_tracker.aliased_table_for(reflection.klass.arel_table) do
-          root = reflection == node.reflection
-          name = reflection.alias_candidate(parent.table_name)
-          root ? name : "#{name}_join"
+        table, terminated = @joined_tables[reflection]
+        root = reflection == node.reflection
+
+        if table && (!root || !terminated)
+          @joined_tables[reflection] = [table, true] if root
+          table
+        else
+          table = alias_tracker.aliased_table_for(reflection.klass.arel_table) do
+            name = reflection.alias_candidate(parent.table_name)
+            root ? name : "#{name}_join"
+          end
+          @joined_tables[reflection] ||= [table, root] if join_type == Arel::Nodes::OuterJoin
+          table
         end
       }
     end
diff --git a/spec/ransack/adapters/active_record/base_spec.rb b/spec/ransack/adapters/active_record/base_spec.rb
index 10e3640d..d41cd6d0 100644
--- a/spec/ransack/adapters/active_record/base_spec.rb
+++ b/spec/ransack/adapters/active_record/base_spec.rb
@@ -141,6 +141,22 @@ module ActiveRecord
           end
         end
 
+        context 'has_one through associations' do
+          let(:address)  { Address.create!(city: 'Boston') }
+          let(:org) { Organization.create!(name: 'Testorg', address: address) }
+          let!(:employee) { Employee.create!(name: 'Ernie', organization: org) }
+
+          it 'works when has_one through association is first' do
+            s = Employee.ransack(address_city_eq: 'Boston', organization_name_eq: 'Testorg')
+            expect(s.result.to_a).to include(employee)
+          end
+
+          it 'works when has_one through association is last' do
+            s = Employee.ransack(organization_name_eq: 'Testorg', address_city_eq: 'Boston')
+            expect(s.result.to_a).to include(employee)
+          end
+        end
+
         context 'negative conditions on HABTM associations' do
           let(:medieval) { Tag.create!(name: 'Medieval') }
           let(:fantasy)  { Tag.create!(name: 'Fantasy') }
diff --git a/spec/support/schema.rb b/spec/support/schema.rb
index 53af0f38..555299c4 100644
--- a/spec/support/schema.rb
+++ b/spec/support/schema.rb
@@ -237,6 +237,20 @@ class Account < ApplicationRecord
   belongs_to :trade_account, class_name: "Account"
 end
 
+class Address < ApplicationRecord
+  has_one :organization
+end
+
+class Organization < ApplicationRecord
+  belongs_to :address
+  has_many :employees
+end
+
+class Employee < ApplicationRecord
+  belongs_to :organization
+  has_one :address, through: :organization
+end
+
 module Schema
   def self.create
     ActiveRecord::Migration.verbose = false
@@ -300,6 +314,20 @@ def self.create
         t.belongs_to :agent_account
         t.belongs_to :trade_account
       end
+
+      create_table :addresses, force: true do |t|
+        t.string :city
+      end
+
+      create_table :organizations, force: true do |t|
+        t.string :name
+        t.integer :address_id
+      end
+
+      create_table :employees, force: true do |t|
+        t.string :name
+        t.integer :organization_id
+      end
     end
 
     10.times do