Merge pull request #22365 from phuibonhoa/phuibonhoa/polymorphic_where_multiple_types

Fixed `where` for polymorphic associations when passed an array containing different types.

7 files changed, 111 insertions, 6 deletions

diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md
index 70549243a8..18d0a12e49 100644
--- a/activerecord/CHANGELOG.md
+++ b/activerecord/CHANGELOG.md
@@ -1,3 +1,19 @@
+*   Fixed `where` for polymorphic associations when passed an array containing different types.
+
+    Fixes #17011.
+
+    Example:
+
+    PriceEstimate.where(estimate_of: [Treasure.find(1), Car.find(2)])
+    # => SELECT "price_estimates".* FROM "price_estimates"
+         WHERE (("price_estimates"."estimate_of_type" = 'Treasure' AND "price_estimates"."estimate_of_id" = 1)
+         OR ("price_estimates"."estimate_of_type" = 'Car' AND "price_estimates"."estimate_of_id" = 2))
+
+    *Philippe Huibonhoa*
+
+
+*   Rework `ActiveRecord::Relation#last` 
+    
 *   Fix a bug where using `t.foreign_key` twice with the same `to_table` within
     the same table definition would only create one foreign key.
 
diff --git a/activerecord/lib/active_record/relation/predicate_builder.rb b/activerecord/lib/active_record/relation/predicate_builder.rb
index 0f88791d92..953495a8b6 100644
--- a/activerecord/lib/active_record/relation/predicate_builder.rb
+++ b/activerecord/lib/active_record/relation/predicate_builder.rb
@@ -5,6 +5,7 @@ module ActiveRecord
     require 'active_record/relation/predicate_builder/base_handler'
     require 'active_record/relation/predicate_builder/basic_object_handler'
     require 'active_record/relation/predicate_builder/class_handler'
+    require 'active_record/relation/predicate_builder/polymorphic_array_handler'
     require 'active_record/relation/predicate_builder/range_handler'
     require 'active_record/relation/predicate_builder/relation_handler'
 
@@ -22,6 +23,7 @@ module ActiveRecord
       register_handler(Relation, RelationHandler.new)
       register_handler(Array, ArrayHandler.new(self))
       register_handler(AssociationQueryValue, AssociationQueryHandler.new(self))
+      register_handler(PolymorphicArrayValue, PolymorphicArrayHandler.new(self))
     end
 
     def build_from_hash(attributes)
@@ -40,10 +42,7 @@ module ActiveRecord
       #
       # For polymorphic relationships, find the foreign key and type:
       # PriceEstimate.where(estimate_of: treasure)
-      if table.associated_with?(column)
-        value = AssociationQueryValue.new(table.associated_table(column), value)
-      end
-
+      value = AssociationQueryHandler.value_for(table, column, value) if table.associated_with?(column)
       build(table.arel_attribute(column), value)
     end
 
diff --git a/activerecord/lib/active_record/relation/predicate_builder/association_query_handler.rb b/activerecord/lib/active_record/relation/predicate_builder/association_query_handler.rb
index e81be63cd3..d7fd878265 100644
--- a/activerecord/lib/active_record/relation/predicate_builder/association_query_handler.rb
+++ b/activerecord/lib/active_record/relation/predicate_builder/association_query_handler.rb
@@ -1,6 +1,16 @@
 module ActiveRecord
   class PredicateBuilder
     class AssociationQueryHandler # :nodoc:
+      def self.value_for(table, column, value)
+        klass = if table.associated_table(column).polymorphic_association? && ::Array === value && value.first.is_a?(Base)
+          PolymorphicArrayValue
+        else
+          AssociationQueryValue
+        end
+
+        klass.new(table.associated_table(column), value)
+      end
+
       def initialize(predicate_builder)
         @predicate_builder = predicate_builder
       end
diff --git a/activerecord/lib/active_record/relation/predicate_builder/polymorphic_array_handler.rb b/activerecord/lib/active_record/relation/predicate_builder/polymorphic_array_handler.rb
new file mode 100644
index 0000000000..b6c6240343
--- /dev/null
+++ b/activerecord/lib/active_record/relation/predicate_builder/polymorphic_array_handler.rb
@@ -0,0 +1,57 @@
+module ActiveRecord
+  class PredicateBuilder
+    class PolymorphicArrayHandler # :nodoc:
+      def initialize(predicate_builder)
+        @predicate_builder = predicate_builder
+      end
+
+      def call(attribute, value)
+        table = value.associated_table
+        queries = value.type_to_ids_mapping.map do |type, ids|
+          { table.association_foreign_type.to_s => type, table.association_foreign_key.to_s => ids }
+        end
+
+        predicates = queries.map { |query| predicate_builder.build_from_hash(query) }
+
+        if predicates.size > 1
+          type_and_ids_predicates = predicates.map { |type_predicate, id_predicate| Arel::Nodes::Grouping.new(type_predicate.and(id_predicate)) }
+          type_and_ids_predicates.inject(&:or)
+        else
+          predicates.first
+        end
+      end
+
+      protected
+
+      attr_reader :predicate_builder
+    end
+
+    class PolymorphicArrayValue # :nodoc:
+      attr_reader :associated_table, :values
+
+      def initialize(associated_table, values)
+        @associated_table = associated_table
+        @values = values
+      end
+
+      def type_to_ids_mapping
+        default_hash = Hash.new { |hsh, key| hsh[key] = [] }
+        values.each_with_object(default_hash) { |value, hash| hash[base_class(value).name] << convert_to_id(value) }
+      end
+
+      private
+
+      def primary_key(value)
+        associated_table.association_primary_key(base_class(value))
+      end
+
+      def base_class(value)
+        value.class.base_class
+      end
+
+      def convert_to_id(value)
+        value._read_attribute(primary_key(value))
+      end
+    end
+  end
+end
diff --git a/activerecord/test/cases/relation/where_test.rb b/activerecord/test/cases/relation/where_test.rb
index bc6378b90e..56a2b5b8c6 100644
--- a/activerecord/test/cases/relation/where_test.rb
+++ b/activerecord/test/cases/relation/where_test.rb
@@ -2,6 +2,7 @@ require "cases/helper"
 require "models/author"
 require "models/binary"
 require "models/cake_designer"
+require "models/car"
 require "models/chef"
 require "models/comment"
 require "models/edge"
@@ -14,7 +15,7 @@ require "models/vertex"
 
 module ActiveRecord
   class WhereTest < ActiveRecord::TestCase
-    fixtures :posts, :edges, :authors, :binaries, :essays
+    fixtures :posts, :edges, :authors, :binaries, :essays, :cars, :treasures, :price_estimates
 
     def test_where_copies_bind_params
       author = authors(:david)
@@ -114,6 +115,17 @@ module ActiveRecord
       assert_equal expected.to_sql, actual.to_sql
     end
 
+    def test_polymorphic_array_where_multiple_types
+      treasure_1 = treasures(:diamond)
+      treasure_2 = treasures(:sapphire)
+      car = cars(:honda)
+
+      expected = [price_estimates(:diamond), price_estimates(:sapphire_1), price_estimates(:sapphire_2), price_estimates(:honda)].sort
+      actual   = PriceEstimate.where(estimate_of: [treasure_1, treasure_2, car]).to_a.sort
+
+      assert_equal expected, actual
+    end
+
     def test_polymorphic_nested_relation_where
       expected = PriceEstimate.where(estimate_of_type: 'Treasure', estimate_of_id: Treasure.where(id: [1,2]))
       actual   = PriceEstimate.where(estimate_of: Treasure.where(id: [1,2]))
diff --git a/activerecord/test/fixtures/price_estimates.yml b/activerecord/test/fixtures/price_estimates.yml
index 1149ab17a2..406d65a142 100644
--- a/activerecord/test/fixtures/price_estimates.yml
+++ b/activerecord/test/fixtures/price_estimates.yml
@@ -1,7 +1,16 @@
-saphire_1:
+sapphire_1:
   price: 10
   estimate_of: sapphire (Treasure)
 
 sapphire_2:
   price: 20
   estimate_of: sapphire (Treasure)
+
+diamond:
+  price: 30
+  estimate_of: diamond (Treasure)
+
+honda:
+  price: 40
+  estimate_of_type: Car
+  estimate_of_id: 1
diff --git a/activerecord/test/models/car.rb b/activerecord/test/models/car.rb
index 778c22b1f6..0f37e9a289 100644
--- a/activerecord/test/models/car.rb
+++ b/activerecord/test/models/car.rb
@@ -12,6 +12,8 @@ class Car < ActiveRecord::Base
   has_many :engines, :dependent => :destroy, inverse_of: :my_car
   has_many :wheels, :as => :wheelable, :dependent => :destroy
 
+  has_many :price_estimates, :as => :estimate_of
+
   scope :incl_tyres, -> { includes(:tyres) }
   scope :incl_engines, -> { includes(:engines) }