From 169502bf3df25bf60b87c8646eae6ff779b455b9 Mon Sep 17 00:00:00 2001 From: David Roetzel Date: Mon, 23 Oct 2023 16:32:55 +0200 Subject: [PATCH] Fix wrong table aliases in Rails 6.1 (#1447) 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. Co-authored-by: Sean --- .../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