Rails Lighthouse ticket #1334

count_calculations_respect_scoped_selects.diff

From 949f3987ad337fb23d411310d39b5c43bee854e5 Mon Sep 17 00:00:00 2001
From: stopdropandrew <andrew@andrew.local>
Date: Wed, 5 Nov 2008 13:49:17 -0800
Subject: [PATCH] Count calculations respect explicit selects specified in scopes.

---
 activerecord/lib/active_record/calculations.rb |   20 ++++++++++++++------
 activerecord/test/cases/calculations_test.rb   |   13 ++++++++++++-
 2 files changed, 26 insertions(+), 7 deletions(-)

diff --git a/activerecord/lib/active_record/calculations.rb b/activerecord/lib/active_record/calculations.rb
index 5e33cf1..18b2bf1 100644
--- a/activerecord/lib/active_record/calculations.rb
+++ b/activerecord/lib/active_record/calculations.rb
@@ -132,23 +132,31 @@ module ActiveRecord
       protected
         def construct_count_options_from_args(*args)
           options     = {}
-          column_name = :all
-          
+          column_name = nil
+        
           # We need to handle
           #   count()
           #   count(:column_name=:all)
           #   count(options={})
           #   count(column_name=:all, options={})
+          #   selects specified by scopes
           case args.size
+          when 0
+            column_name = (scope(:find) || {})[:select]
           when 1
-            args[0].is_a?(Hash) ? options = args[0] : column_name = args[0]
+            if args[0].is_a?(Hash)
+              column_name = (scope(:find) || {})[:select]
+              options = args[0]
+            else
+              column_name = args[0]
+            end
           when 2
             column_name, options = args
           else
             raise ArgumentError, "Unexpected parameters passed to count(): #{args.inspect}"
-          end if args.size > 0
-          
-          [column_name, options]
+          end
+        
+          [column_name || :all, options]
         end
 
         def construct_calculation_sql(operation, column_name, options) #:nodoc:
diff --git a/activerecord/test/cases/calculations_test.rb b/activerecord/test/cases/calculations_test.rb
index 0fa6150..d2740c1 100644
--- a/activerecord/test/cases/calculations_test.rb
+++ b/activerecord/test/cases/calculations_test.rb
@@ -251,7 +251,18 @@ class CalculationsTest < ActiveRecord::TestCase
     assert_equal 6, Account.count(:distinct => true, :include => :firm)
     assert_equal 4, Account.count(:distinct => true, :include => :firm, :select => :credit_limit)
   end
-
+  
+  def test_should_count_scoped_select
+    Account.update_all("credit_limit = 50")
+    assert_equal 1, Account.scoped(:select => "DISTINCT credit_limit").count
+  end
+  
+  def test_should_count_scoped_select_with_options
+    Account.update_all("credit_limit = 50")
+    Account.first.update_attribute('credit_limit', 49)
+    assert_equal 1, Account.scoped(:select => "DISTINCT credit_limit").count(:conditions => [ 'credit_limit >= 50'] )
+  end
+  
   def test_should_count_manual_select_with_include
     assert_equal 6, Account.count(:select => "DISTINCT accounts.id", :include => :firm)
   end
-- 
1.5.5

failing_test_calculations_count_with_scoped_has_many_through.diff

From 8fa4c57d46b03a705c2c665dd2b58c0f4d172793 Mon Sep 17 00:00:00 2001
From: Ian Terrell <ian.terrell@gmail.com>
Date: Tue, 17 Mar 2009 12:08:47 -0400
Subject: [PATCH] added a failing test case for counting has_many :through associations with scopes

---
 activerecord/test/cases/calculations_test.rb |   11 +++++++++--
 activerecord/test/models/toy.rb              |    2 ++
 2 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/activerecord/test/cases/calculations_test.rb b/activerecord/test/cases/calculations_test.rb
index 56dcdea..6d3b8e6 100644
--- a/activerecord/test/cases/calculations_test.rb
+++ b/activerecord/test/cases/calculations_test.rb
@@ -2,6 +2,9 @@ require "cases/helper"
 require 'models/company'
 require 'models/topic'
 require 'models/edge'
+require 'models/owner'
+require 'models/pet'
+require 'models/toy'
 
 Company.has_many :accounts
 
@@ -10,7 +13,7 @@ class NumericData < ActiveRecord::Base
 end
 
 class CalculationsTest < ActiveRecord::TestCase
-  fixtures :companies, :accounts, :topics
+  fixtures :companies, :accounts, :topics, :owners, :pets, :toys
 
   def test_should_sum_field
     assert_equal 318, Account.sum(:credit_limit)
@@ -296,7 +299,11 @@ class CalculationsTest < ActiveRecord::TestCase
   def test_count_with_too_many_parameters_raises
     assert_raise(ArgumentError) { Account.count(1, 2, 3) }
   end
-
+  
+  def test_count_with_scoped_has_many_through_association
+    assert_equal 1, owners(:blackbeard).toys.with_name('bone').count
+  end
+  
   def test_should_sum_expression
     assert_equal '636', Account.sum("2 * credit_limit")
   end
diff --git a/activerecord/test/models/toy.rb b/activerecord/test/models/toy.rb
index 79a88db..9635e19 100644
--- a/activerecord/test/models/toy.rb
+++ b/activerecord/test/models/toy.rb
@@ -1,4 +1,6 @@
 class Toy < ActiveRecord::Base
   set_primary_key :toy_id
   belongs_to :pet
+  
+  named_scope :with_name, lambda { |name| {:conditions => {:name => name}} }
 end
-- 
1.6.0.2

fix_count_with_scoped_select.diff

From 7df10895cfd6a8b4af6ca8b4b88a7a8e30d9be14 Mon Sep 17 00:00:00 2001
From: David Cuddeback <david.cuddeback@gmail.com>
Date: Fri, 4 Feb 2011 12:04:43 -0800
Subject: [PATCH] fix count with scoped select

---
 activerecord/lib/active_record/calculations.rb |    2 +-
 activerecord/test/cases/calculations_test.rb   |   20 +++++++++++++++++++-
 2 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/activerecord/lib/active_record/calculations.rb b/activerecord/lib/active_record/calculations.rb
index f6249e5..8a9342c 100644
--- a/activerecord/lib/active_record/calculations.rb
+++ b/activerecord/lib/active_record/calculations.rb
@@ -140,7 +140,7 @@ module ActiveRecord
       protected
         def construct_count_options_from_args(*args)
           options     = {}
-          column_name = :all
+          column_name = scope(:find, :select) || :all
           
           # We need to handle
           #   count()
diff --git a/activerecord/test/cases/calculations_test.rb b/activerecord/test/cases/calculations_test.rb
index d5bb358..56a8fb8 100644
--- a/activerecord/test/cases/calculations_test.rb
+++ b/activerecord/test/cases/calculations_test.rb
@@ -1,4 +1,6 @@
 require "cases/helper"
+require 'models/author'
+require 'models/post'
 require 'models/company'
 require 'models/topic'
 require 'models/edge'
@@ -15,7 +17,7 @@ class NumericData < ActiveRecord::Base
 end
 
 class CalculationsTest < ActiveRecord::TestCase
-  fixtures :companies, :accounts, :topics, :owners, :pets, :toys
+  fixtures :companies, :accounts, :topics, :owners, :pets, :toys, :authors, :posts
 
   def test_should_sum_field
     assert_equal 318, Account.sum(:credit_limit)
@@ -285,6 +287,22 @@ class CalculationsTest < ActiveRecord::TestCase
     assert_equal 4, Account.count(:distinct => true, :include => :firm, :select => :credit_limit)
   end
 
+  def test_should_count_scoped_select
+    Account.update_all("credit_limit = 50")
+    assert_equal 1, Account.scoped(:select => "DISTINCT credit_limit").count
+  end
+
+  def test_should_count_scoped_select_with_options
+    Account.update_all("credit_limit = 50")
+    Account.first.update_attribute('credit_limit', 49)
+    assert_equal 1, Account.scoped(:select => "DISTINCT credit_limit").count(:conditions => [ 'credit_limit >= 50'] )
+  end
+
+  def test_should_count_scoped_select_on_association
+    author = Author.first
+    assert_equal author.posts.count, author.posts.scoped(:conditions => "1=1").count
+  end
+
   def test_should_count_manual_select_with_include
     assert_equal 6, Account.count(:select => "DISTINCT accounts.id", :include => :firm)
   end
-- 
1.7.3.5