Merge pull request #26162 from kamipo/take_respects_dirty_target

`CollectionProxy#take` should respect dirty target

3 files changed, 19 insertions, 19 deletions

diff --git a/activerecord/lib/active_record/associations/collection_association.rb b/activerecord/lib/active_record/associations/collection_association.rb
index ab267f6897..44404cc176 100644
--- a/activerecord/lib/active_record/associations/collection_association.rb
+++ b/activerecord/lib/active_record/associations/collection_association.rb
@@ -147,11 +147,11 @@ module ActiveRecord
         first_nth_or_last(:last, *args)
       end
 
-      def take(n = nil)
-        if loaded?
-          n ? target.take(n) : target.first
+      def take(limit = nil)
+        if find_from_target?
+          limit ? load_target.take(limit) : load_target.first
         else
-          scope.take(n)
+          scope.take(limit)
         end
       end
 
@@ -608,14 +608,10 @@ module ActiveRecord
         #   * target already loaded
         #   * owner is new record
         #   * target contains new or changed record(s)
-        def fetch_first_nth_or_last_using_find?(args)
-          if args.first.is_a?(Hash)
-            true
-          else
-            !(loaded? ||
-              owner.new_record? ||
-              target.any? { |record| record.new_record? || record.changed? })
-          end
+        def find_from_target?
+          loaded? ||
+            owner.new_record? ||
+            target.any? { |record| record.new_record? || record.changed? }
         end
 
         def include_in_memory?(record)
@@ -649,7 +645,7 @@ module ActiveRecord
         def first_nth_or_last(type, *args)
           args.shift if args.first.is_a?(Hash) && args.first.empty?
 
-          collection = fetch_first_nth_or_last_using_find?(args) ? scope : load_target
+          collection = find_from_target? ? load_target : scope
           collection.send(type, *args)
         end
     end
diff --git a/activerecord/lib/active_record/associations/collection_proxy.rb b/activerecord/lib/active_record/associations/collection_proxy.rb
index 806a905323..fb1b31429e 100644
--- a/activerecord/lib/active_record/associations/collection_proxy.rb
+++ b/activerecord/lib/active_record/associations/collection_proxy.rb
@@ -262,8 +262,8 @@ module ActiveRecord
       #   another_person_without.pets         # => []
       #   another_person_without.pets.take    # => nil
       #   another_person_without.pets.take(2) # => []
-      def take(n = nil)
-        @association.take(n)
+      def take(limit = nil)
+        @association.take(limit)
       end
 
       # Returns a new object of the collection type that has been instantiated
diff --git a/activerecord/test/cases/associations/has_many_associations_test.rb b/activerecord/test/cases/associations/has_many_associations_test.rb
index cc4419790c..dd3b7240b8 100644
--- a/activerecord/test/cases/associations/has_many_associations_test.rb
+++ b/activerecord/test/cases/associations/has_many_associations_test.rb
@@ -525,14 +525,18 @@ class HasManyAssociationsTest < ActiveRecord::TestCase
   def test_taking_with_a_number
     # taking from unloaded Relation
     bob = Author.find(authors(:bob).id)
+    new_post = bob.posts.build
+    assert_not bob.posts.loaded?
     assert_equal [posts(:misc_by_bob)], bob.posts.take(1)
-    bob = Author.find(authors(:bob).id)
     assert_equal [posts(:misc_by_bob), posts(:other_by_bob)], bob.posts.take(2)
+    assert_equal [posts(:misc_by_bob), posts(:other_by_bob), new_post], bob.posts.take(3)
 
     # taking from loaded Relation
-    bob.posts.to_a
-    assert_equal [posts(:misc_by_bob)], authors(:bob).posts.take(1)
-    assert_equal [posts(:misc_by_bob), posts(:other_by_bob)], authors(:bob).posts.take(2)
+    bob.posts.load
+    assert bob.posts.loaded?
+    assert_equal [posts(:misc_by_bob)], bob.posts.take(1)
+    assert_equal [posts(:misc_by_bob), posts(:other_by_bob)], bob.posts.take(2)
+    assert_equal [posts(:misc_by_bob), posts(:other_by_bob), new_post], bob.posts.take(3)
   end
 
   def test_taking_with_inverse_of