Add first/last methods to associations/named_scope. [#226 state:resolved]

Signed-off-by: Pratik Naik <pratiknaik@gmail.com>

8 files changed, 149 insertions, 4 deletions

diff --git a/activerecord/CHANGELOG b/activerecord/CHANGELOG
index cd526a52a7..78082665a4 100644
--- a/activerecord/CHANGELOG
+++ b/activerecord/CHANGELOG
@@ -1,3 +1,5 @@
+* Add first/last methods to associations/named_scope. Resolved #226. [Ryan Bates]
+
 *2.1.0 RC1 (May 11th, 2008)*
 
 * Ensure hm:t preloading honours reflection options. Resolves #137. [Frederick Cheung]
diff --git a/activerecord/lib/active_record/associations/association_collection.rb b/activerecord/lib/active_record/associations/association_collection.rb
index 7e47bf7bdf..2d3750e107 100644
--- a/activerecord/lib/active_record/associations/association_collection.rb
+++ b/activerecord/lib/active_record/associations/association_collection.rb
@@ -48,6 +48,26 @@ module ActiveRecord
         end
       end
       
+      # fetch first using SQL if possible
+      def first(*args)
+        if fetch_first_or_last_using_find? args
+          find(:first, *args)
+        else
+          load_target unless loaded?
+          @target.first(*args)
+        end
+      end
+
+      # fetch last using SQL if possible
+      def last(*args)
+        if fetch_first_or_last_using_find? args
+          find(:last, *args)
+        else
+          load_target unless loaded?
+          @target.last(*args)
+        end
+      end
+
       def to_ary
         load_target
         @target.to_ary
@@ -330,7 +350,10 @@ module ActiveRecord
             raise ActiveRecord::RecordNotSaved, "You cannot call create unless the parent is saved"
           end
         end
-               
+
+        def fetch_first_or_last_using_find?(args)
+          args.first.kind_of?(Hash) || !(loaded? || @owner.new_record? || @reflection.options[:finder_sql] || !@target.blank? || args.first.kind_of?(Integer))
+        end
     end
   end
 end
diff --git a/activerecord/lib/active_record/named_scope.rb b/activerecord/lib/active_record/named_scope.rb
index 06edaed3d5..c7cf1731c1 100644
--- a/activerecord/lib/active_record/named_scope.rb
+++ b/activerecord/lib/active_record/named_scope.rb
@@ -102,7 +102,13 @@ module ActiveRecord
     
     class Scope
       attr_reader :proxy_scope, :proxy_options
-      [].methods.each { |m| delegate m, :to => :proxy_found unless m =~ /(^__|^nil\?|^send|^object_id$|class|extend|find|count|sum|average|maximum|minimum|paginate)/ }
+
+      [].methods.each do |m|
+        unless m =~ /(^__|^nil\?|^send|^object_id$|class|extend|find|count|sum|average|maximum|minimum|paginate|first|last)/
+          delegate m, :to => :proxy_found
+        end
+      end
+
       delegate :scopes, :with_scope, :to => :proxy_scope
 
       def initialize(proxy_scope, options, &block)
@@ -115,6 +121,22 @@ module ActiveRecord
         load_found; self
       end
 
+      def first(*args)
+        if args.first.kind_of?(Integer) || (@found && !args.first.kind_of?(Hash))
+          proxy_found.first(*args)
+        else
+          find(:first, *args)
+        end
+      end
+
+      def last(*args)
+        if args.first.kind_of?(Integer) || (@found && !args.first.kind_of?(Hash))
+          proxy_found.last(*args)
+        else
+          find(:last, *args)
+        end
+      end
+
       protected
       def proxy_found
         @found || load_found
diff --git a/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb b/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb
index 64565141f9..294b993c55 100644
--- a/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb
+++ b/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb
@@ -401,6 +401,8 @@ class HasAndBelongsToManyAssociationsTest < ActiveRecord::TestCase
 
   def test_include_uses_array_include_after_loaded
     project = projects(:active_record)
+    project.developers.class # force load target
+
     developer = project.developers.first
     
     assert_no_queries do
diff --git a/activerecord/test/cases/associations/has_many_associations_test.rb b/activerecord/test/cases/associations/has_many_associations_test.rb
index 9e26e2ad58..53b55022eb 100644
--- a/activerecord/test/cases/associations/has_many_associations_test.rb
+++ b/activerecord/test/cases/associations/has_many_associations_test.rb
@@ -818,6 +818,8 @@ class HasManyAssociationsTest < ActiveRecord::TestCase
 
   def test_include_uses_array_include_after_loaded
     firm = companies(:first_firm)
+    firm.clients.class # force load target
+
     client = firm.clients.first
 
     assert_no_queries do
@@ -857,4 +859,68 @@ class HasManyAssociationsTest < ActiveRecord::TestCase
     assert ! firm.clients.include?(client)
   end
 
+  def test_calling_first_or_last_on_association_should_not_load_association
+    firm = companies(:first_firm)
+    firm.clients.first
+    firm.clients.last
+    assert !firm.clients.loaded?
+  end
+
+  def test_calling_first_or_last_on_loaded_association_should_not_fetch_with_query
+    firm = companies(:first_firm)
+    firm.clients.class # force load target
+    assert firm.clients.loaded?
+
+    assert_no_queries do
+      firm.clients.first
+      assert_equal 2, firm.clients.first(2).size
+      firm.clients.last
+      assert_equal 2, firm.clients.last(2).size
+    end
+  end
+
+  def test_calling_first_or_last_on_existing_record_with_build_should_load_association
+    firm = companies(:first_firm)
+    firm.clients.build(:name => 'Foo')
+    assert !firm.clients.loaded?
+
+    assert_queries 1 do
+      firm.clients.first
+      firm.clients.last
+    end
+
+    assert firm.clients.loaded?
+  end
+
+  def test_calling_first_or_last_on_new_record_should_not_run_queries
+    firm = Firm.new
+
+    assert_no_queries do
+      firm.clients.first
+      firm.clients.last
+    end
+  end
+
+  def test_calling_first_or_last_with_find_options_on_loaded_association_should_fetch_with_query
+    firm = companies(:first_firm)
+    firm.clients.class # force load target
+
+    assert_queries 2 do
+      assert firm.clients.loaded?
+      firm.clients.first(:order => 'name')
+      firm.clients.last(:order => 'name')
+    end
+  end
+
+  def test_calling_first_or_last_with_integer_on_association_should_load_association
+    firm = companies(:first_firm)
+
+    assert_queries 1 do
+      firm.clients.first(2)
+      firm.clients.last(2)
+    end
+
+    assert firm.clients.loaded?
+  end
+
 end
diff --git a/activerecord/test/cases/associations/join_model_test.rb b/activerecord/test/cases/associations/join_model_test.rb
index 952ea63706..9e79d9c8a1 100644
--- a/activerecord/test/cases/associations/join_model_test.rb
+++ b/activerecord/test/cases/associations/join_model_test.rb
@@ -664,6 +664,8 @@ class AssociationsJoinModelTest < ActiveRecord::TestCase
 
   def test_has_many_through_include_uses_array_include_after_loaded
     david = authors(:david)
+    david.categories.class # force load target
+
     category = david.categories.first
 
     assert_no_queries do
diff --git a/activerecord/test/cases/associations_test.rb b/activerecord/test/cases/associations_test.rb
index d8fe98bf57..3ad8c608e0 100755
--- a/activerecord/test/cases/associations_test.rb
+++ b/activerecord/test/cases/associations_test.rb
@@ -99,12 +99,12 @@ class AssociationProxyTest < ActiveRecord::TestCase
     david = authors(:david)
     assert_equal  david, david.posts.proxy_owner
     assert_equal  david.class.reflect_on_association(:posts), david.posts.proxy_reflection
-    david.posts.first   # force load target
+    david.posts.class   # force load target
     assert_equal  david.posts, david.posts.proxy_target
 
     assert_equal  david, david.posts_with_extension.testing_proxy_owner
     assert_equal  david.class.reflect_on_association(:posts_with_extension), david.posts_with_extension.testing_proxy_reflection
-    david.posts_with_extension.first   # force load target
+    david.posts_with_extension.class   # force load target
     assert_equal  david.posts_with_extension, david.posts_with_extension.testing_proxy_target
   end
 
diff --git a/activerecord/test/cases/named_scope_test.rb b/activerecord/test/cases/named_scope_test.rb
index 9730f93579..8f2fc53d67 100644
--- a/activerecord/test/cases/named_scope_test.rb
+++ b/activerecord/test/cases/named_scope_test.rb
@@ -118,4 +118,32 @@ class NamedScopeTest < ActiveRecord::TestCase
     assert_equal expected_proxy_options, Topic.approved.proxy_options
   end
 
+  def test_first_and_last_should_support_find_options
+    assert_equal Topic.base.first(:order => 'title'), Topic.base.find(:first, :order => 'title')
+    assert_equal Topic.base.last(:order => 'title'), Topic.base.find(:last, :order => 'title')
+  end
+
+  def test_first_and_last_should_allow_integers_for_limit
+    assert_equal Topic.base.first(2), Topic.base.to_a.first(2)
+    assert_equal Topic.base.last(2), Topic.base.to_a.last(2)
+  end
+
+  def test_first_and_last_should_not_use_query_when_results_are_loaded
+    topics = Topic.base
+    topics.reload # force load
+    assert_no_queries do
+      topics.first
+      topics.last
+    end
+  end
+
+  def test_first_and_last_find_options_should_use_query_when_results_are_loaded
+    topics = Topic.base
+    topics.reload # force load
+    assert_queries(2) do
+      topics.first(:order => 'title')
+      topics.last(:order => 'title')
+    end
+  end
+
 end