Merge pull request rails#35868 from kamipo/association_isnt_to_be_affected_by_scoping_consistently

Association loading isn't to be affected by scoping consistently

Author: kamipo

activerecord/CHANGELOG.md

@@ -1,3 +1,30 @@
+*   Association loading isn't to be affected by scoping consistently
+    whether preloaded / eager loaded or not, with the exception of `unscoped`.
+
+    Before:
+
+    ```ruby
+    Post.where("1=0").scoping do
+      Comment.find(1).post                   # => nil
+      Comment.preload(:post).find(1).post    # => #<Post id: 1, ...>
+      Comment.eager_load(:post).find(1).post # => #<Post id: 1, ...>
+    end
+    ```
+
+    After:
+
+    ```ruby
+    Post.where("1=0").scoping do
+      Comment.find(1).post                   # => #<Post id: 1, ...>
+      Comment.preload(:post).find(1).post    # => #<Post id: 1, ...>
+      Comment.eager_load(:post).find(1).post # => #<Post id: 1, ...>
+    end
+    ```
+
+    Fixes #34638, #35398.
+
+    *Ryuta Kamizono*
+
 *   Add `rails db:prepare` to migrate or setup a database.
 
     Runs `db:migrate` if the database exists or `db:setup` if it doesn't.

activerecord/lib/active_record/associations/association.rb

@@ -225,7 +225,7 @@ def association_scope
         # Can be overridden (i.e. in ThroughAssociation) to merge in other scopes (i.e. the
         # through association's scope)
         def target_scope
-          AssociationRelation.create(klass, self).merge!(klass.all)
+          AssociationRelation.create(klass, self).merge!(klass.scope_for_association)
         end
 
         def scope_for_create

activerecord/test/cases/associations/eager_test.rb

@@ -1400,11 +1400,24 @@ def test_preloading_has_many_through_with_custom_scope
     assert_equal expected, FirstPost.unscoped.find(2)
   end
 
-  test "preload ignores the scoping" do
-    assert_equal(
-      Comment.find(1).post,
-      Post.where("1 = 0").scoping { Comment.preload(:post).find(1).post }
-    )
+  test "belongs_to association ignores the scoping" do
+    post = Comment.find(1).post
+
+    Post.where("1=0").scoping do
+      assert_equal post, Comment.find(1).post
+      assert_equal post, Comment.preload(:post).find(1).post
+      assert_equal post, Comment.eager_load(:post).find(1).post
+    end
+  end
+
+  test "has_many association ignores the scoping" do
+    comments = Post.find(1).comments.to_a
+
+    Comment.where("1=0").scoping do
+      assert_equal comments, Post.find(1).comments
+      assert_equal comments, Post.preload(:comments).find(1).comments
+      assert_equal comments, Post.eager_load(:comments).find(1).comments
+    end
   end
 
   test "deep preload" do

activerecord/test/cases/scoping/relation_scoping_test.rb

@@ -411,7 +411,7 @@ def test_forwarding_to_scoped
 
   def test_nested_scope_finder
     Comment.where("1=0").scoping do
-      assert_equal 0, @welcome.comments.count
+      assert_equal 2, @welcome.comments.count
       assert_equal "a comment...", @welcome.comments.what_are_you
     end
 
@@ -452,7 +452,7 @@ def test_forwarding_of_static_methods
 
   def test_nested_scope_finder
     Category.where("1=0").scoping do
-      assert_equal 0, @welcome.categories.count
+      assert_equal 2, @welcome.categories.count
       assert_equal "a category...", @welcome.categories.what_are_you
     end