From 73c59638549686fccc749ffd3ac53cb533c5fd61 Mon Sep 17 00:00:00 2001 From: Ryan Bates Date: Tue, 20 May 2008 12:11:25 +0100 Subject: Add first/last methods to associations/named_scope. [#226 state:resolved] Signed-off-by: Pratik Naik --- activerecord/CHANGELOG | 2 + .../associations/association_collection.rb | 25 +++++++- activerecord/lib/active_record/named_scope.rb | 24 +++++++- .../has_and_belongs_to_many_associations_test.rb | 2 + .../associations/has_many_associations_test.rb | 66 ++++++++++++++++++++++ .../test/cases/associations/join_model_test.rb | 2 + activerecord/test/cases/associations_test.rb | 4 +- activerecord/test/cases/named_scope_test.rb | 28 +++++++++ 8 files changed, 149 insertions(+), 4 deletions(-) (limited to 'activerecord') 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 -- cgit v1.2.3