Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

Add Relation#construct_relation_for_association_calculations for calc…

…ulations with includes
  • Loading branch information...
commit b9599502c9e738a5a1513e75d08f8d40ed408265 1 parent 71d67fc
@lifo lifo authored
View
25 activerecord/lib/active_record/calculations.rb
@@ -2,8 +2,6 @@ module ActiveRecord
module Calculations #:nodoc:
extend ActiveSupport::Concern
- CALCULATIONS_OPTIONS = [:conditions, :joins, :order, :select, :group, :having, :distinct, :limit, :offset, :include, :from]
-
module ClassMethods
# Count operates using three different approaches.
#
@@ -147,26 +145,11 @@ def calculate(operation, column_name, options = {})
end
private
- def validate_calculation_options(options = {})
- options.assert_valid_keys(CALCULATIONS_OPTIONS)
- end
-
- def construct_calculation_arel(options = {})
- validate_calculation_options(options)
- options = options.except(:distinct)
-
- merge_with_includes = current_scoped_methods ? current_scoped_methods.includes_values : []
- includes = (merge_with_includes + Array.wrap(options[:include])).uniq
- if includes.any?
- merge_with_joins = current_scoped_methods ? current_scoped_methods.joins_values : []
- joins = (merge_with_joins + Array.wrap(options[:joins])).uniq
- join_dependency = ActiveRecord::Associations::ClassMethods::JoinDependency.new(self, includes, construct_join(joins))
- construct_finder_arel_with_included_associations(options, join_dependency)
- else
- scoped.apply_finder_options(options)
- end
- end
+ def construct_calculation_arel(options = {})
+ relation = scoped.apply_finder_options(options.except(:distinct))
+ (relation.eager_loading? || relation.includes_values.present?) ? relation.send(:construct_relation_for_association_calculations) : relation
+ end
end
end
View
11 activerecord/lib/active_record/relation.rb
@@ -45,12 +45,10 @@ def respond_to?(method, include_private = false)
def to_a
return @records if loaded?
- eager_loading = @eager_load_values.any? || (@includes_values.any? && references_eager_loaded_tables?)
-
- @records = eager_loading ? find_with_associations : @klass.find_by_sql(arel.to_sql)
+ @records = eager_loading? ? find_with_associations : @klass.find_by_sql(arel.to_sql)
preload = @preload_values
- preload += @includes_values unless eager_loading
+ preload += @includes_values unless eager_loading?
preload.each {|associations| @klass.send(:preload_associations, @records, associations) }
# @readonly_value is true only if set explicity. @implicit_readonly is true if there are JOINS and no explicit SELECT.
@@ -112,6 +110,7 @@ def reload
def reset
@first = @last = @to_sql = @order_clause = @scope_for_create = @arel = @loaded = nil
+ @should_eager_load = @join_dependency = nil
@records = []
self
end
@@ -133,6 +132,10 @@ def scope_for_create
end
end
+ def eager_loading?
+ @should_eager_load ||= (@eager_load_values.any? || (@includes_values.any? && references_eager_loaded_tables?))
+ end
+
protected
def method_missing(method, *args, &block)
View
6 activerecord/lib/active_record/relation/finder_methods.rb
@@ -53,6 +53,12 @@ def find_with_associations
[]
end
+ def construct_relation_for_association_calculations
+ including = (@eager_load_values + @includes_values).uniq
+ join_dependency = ActiveRecord::Associations::ClassMethods::JoinDependency.new(@klass, including, arel.joins(arel))
+ construct_relation_for_association_find(join_dependency)
+ end
+
def construct_relation_for_association_find(join_dependency)
relation = except(:includes, :eager_load, :preload, :select).select(@klass.send(:column_aliases, join_dependency))
View
17 activerecord/test/cases/calculations_test.rb
@@ -246,23 +246,6 @@ def test_should_group_by_summed_field_through_association_and_having
assert_equal 8, c['Jadedpixel']
end
- def test_should_reject_invalid_options
- assert_nothing_raised do
- # empty options are valid
- Company.send(:validate_calculation_options)
- # these options are valid for all calculations
- [:select, :conditions, :joins, :order, :group, :having, :distinct].each do |opt|
- Company.send(:validate_calculation_options, opt => true)
- end
-
- # :include is only valid on :count
- Company.send(:validate_calculation_options, :include => true)
- end
-
- assert_raise(ArgumentError) { Company.send(:validate_calculation_options, :sum, :foo => :bar) }
- assert_raise(ArgumentError) { Company.send(:validate_calculation_options, :count, :foo => :bar) }
- end
-
def test_should_count_selected_field_with_include
assert_equal 6, Account.count(:distinct => true, :include => :firm)
assert_equal 4, Account.count(:distinct => true, :include => :firm, :select => :credit_limit)
Please sign in to comment.
Something went wrong with that request. Please try again.