Merge pull request #38929 from kamipo/fix_unscoping_association_scope

Fix unscoping association scope on joins not to raise an error

activerecord/lib/active_record/reflection.rb

@@ -183,20 +183,22 @@ def join_scope(table, foreign_table, foreign_klass)
         scope_chain_items = join_scopes(table, predicate_builder)
         klass_scope       = klass_join_scope(table, predicate_builder)
 
+        if type
+          klass_scope.where!(type => foreign_klass.polymorphic_name)
+        end
+
+        scope_chain_items.inject(klass_scope, &:merge!)
+
         key         = join_keys.key
         foreign_key = join_keys.foreign_key
 
         klass_scope.where!(table[key].eq(foreign_table[foreign_key]))
 
-        if type
-          klass_scope.where!(type => foreign_klass.polymorphic_name)
-        end
-
         if klass.finder_needs_type_condition?
           klass_scope.where!(klass.send(:type_condition, table))
         end
 
-        scope_chain_items.inject(klass_scope, &:merge!)
+        klass_scope
       end
 
       def join_scopes(table, predicate_builder) # :nodoc:

activerecord/test/cases/associations/has_many_associations_test.rb

@@ -2658,10 +2658,11 @@ def test_association_with_rewhere_doesnt_set_inverse_instance_key
 
     assert_equal [bulb1], car.bulbs
     assert_equal [bulb1, bulb2], car.all_bulbs.sort_by(&:id)
+    assert_equal [bulb1, bulb2], Car.includes(:all_bulbs).find(car.id).all_bulbs.sort_by(&:id)
+    assert_equal [bulb1, bulb2], Car.eager_load(:all_bulbs).find(car.id).all_bulbs.sort_by(&:id)
   end
 
   test "can unscope and where the default scope of the associated model" do
-    Car.has_many :other_bulbs, -> { unscope(where: [:name]).where(name: "other") }, class_name: "Bulb"
     car = Car.create!
     bulb1 = Bulb.create! name: "defaulty", car: car
     bulb2 = Bulb.create! name: "other",    car: car
@@ -2671,7 +2672,6 @@ def test_association_with_rewhere_doesnt_set_inverse_instance_key
   end
 
   test "can rewhere the default scope of the associated model" do
-    Car.has_many :old_bulbs, -> { rewhere(name: "old") }, class_name: "Bulb"
     car = Car.create!
     bulb1 = Bulb.create! name: "defaulty", car: car
     bulb2 = Bulb.create! name: "old",      car: car
@@ -2682,11 +2682,11 @@ def test_association_with_rewhere_doesnt_set_inverse_instance_key
 
   test "unscopes the default scope of associated model when used with include" do
     car = Car.create!
-    bulb = Bulb.create! name: "other", car: car
+    bulb1 = Bulb.create! name: "defaulty", car: car
+    bulb2 = Bulb.create! name: "other",    car: car
 
-    assert_equal [bulb], Car.find(car.id).all_bulbs
-    assert_equal [bulb], Car.includes(:all_bulbs).find(car.id).all_bulbs
-    assert_equal [bulb], Car.eager_load(:all_bulbs).find(car.id).all_bulbs
+    assert_equal [bulb1, bulb2], Car.includes(:all_bulbs2).find(car.id).all_bulbs2.sort_by(&:id)
+    assert_equal [bulb1, bulb2], Car.eager_load(:all_bulbs2).find(car.id).all_bulbs2.sort_by(&:id)
   end
 
   test "raises RecordNotDestroyed when replaced child can't be destroyed" do

activerecord/test/models/car.rb

@@ -2,7 +2,10 @@
 
 class Car < ActiveRecord::Base
   has_many :bulbs
-  has_many :all_bulbs, -> { unscope where: :name }, class_name: "Bulb"
+  has_many :all_bulbs, -> { unscope(where: :name) }, class_name: "Bulb"
+  has_many :all_bulbs2, -> { unscope(:where) }, class_name: "Bulb"
+  has_many :other_bulbs, -> { unscope(where: :name).where(name: "other") }, class_name: "Bulb"
+  has_many :old_bulbs, -> { rewhere(name: "old") }, class_name: "Bulb"
   has_many :funky_bulbs, class_name: "FunkyBulb", dependent: :destroy
   has_many :failed_bulbs, class_name: "FailedBulb", dependent: :destroy
   has_many :foo_bulbs, -> { where(name: "foo") }, class_name: "Bulb"