From 6d36c1dd05cc858dea92acbd7f3b476690878341 Mon Sep 17 00:00:00 2001 From: Kuldeep Aggarwal Date: Thu, 15 May 2014 00:49:29 +0530 Subject: [PATCH] Fixed a problem where `sum`, `size`, `average`, `minimum` and `maximum` used with a grouping was not returning a Hash. --- activerecord/CHANGELOG.md | 5 +++ .../lib/active_record/null_relation.rb | 20 +++++++-- activerecord/test/cases/relations_test.rb | 44 +++++++++++++++++++ 3 files changed, 66 insertions(+), 3 deletions(-) diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 6d4ea2b6829be..f5b8026c52f1d 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,8 @@ +* Fixed a problem where `sum`, `size`, `average`, `minimum` and `maximum` + used with a grouping was not returning a Hash. + + *Kuldeep Aggarwal* + * Fix `stored_attributes` to correctly merge the details of stored attributes defined in parent classes. diff --git a/activerecord/lib/active_record/null_relation.rb b/activerecord/lib/active_record/null_relation.rb index 05d0c41678a97..807c301596c89 100644 --- a/activerecord/lib/active_record/null_relation.rb +++ b/activerecord/lib/active_record/null_relation.rb @@ -23,7 +23,7 @@ def delete(_id_or_array) end def size - 0 + calculate :size, nil end def empty? @@ -47,14 +47,28 @@ def count(*) end def sum(*) - 0 + calculate :sum, nil + end + + def average(*) + calculate :average, nil + end + + def minimum(*) + calculate :minimum, nil + end + + def maximum(*) + calculate :maximum, nil end def calculate(operation, _column_name, _options = {}) # TODO: Remove _options argument as soon we remove support to # activerecord-deprecated_finders. - if operation == :count + if [:count, :sum, :size].include? operation group_values.any? ? Hash.new : 0 + elsif [:average, :minimum, :maximum].include?(operation) && group_values.any? + Hash.new else nil end diff --git a/activerecord/test/cases/relations_test.rb b/activerecord/test/cases/relations_test.rb index 6a880c66809c5..b6f25959b6076 100644 --- a/activerecord/test/cases/relations_test.rb +++ b/activerecord/test/cases/relations_test.rb @@ -366,6 +366,14 @@ def test_null_relation_where_values_hash assert_equal({ 'salary' => 100_000 }, Developer.none.where(salary: 100_000).where_values_hash) end + def test_null_relation_sum + ac = Aircraft.new + assert_equal Hash.new, ac.engines.group(:id).sum(:id) + assert_equal 0, ac.engines.count + ac.save + assert_equal Hash.new, ac.engines.group(:id).sum(:id) + assert_equal 0, ac.engines.count + end def test_null_relation_count ac = Aircraft.new @@ -376,6 +384,42 @@ def test_null_relation_count assert_equal 0, ac.engines.count end + def test_null_relation_size + ac = Aircraft.new + assert_equal Hash.new, ac.engines.group(:id).size + assert_equal 0, ac.engines.size + ac.save + assert_equal Hash.new, ac.engines.group(:id).size + assert_equal 0, ac.engines.size + end + + def test_null_relation_average + ac = Aircraft.new + assert_equal Hash.new, ac.engines.group(:car_id).average(:id) + assert_equal nil, ac.engines.average(:id) + ac.save + assert_equal Hash.new, ac.engines.group(:car_id).average(:id) + assert_equal nil, ac.engines.average(:id) + end + + def test_null_relation_minimum + ac = Aircraft.new + assert_equal Hash.new, ac.engines.group(:car_id).minimum(:id) + assert_equal nil, ac.engines.minimum(:id) + ac.save + assert_equal Hash.new, ac.engines.group(:car_id).minimum(:id) + assert_equal nil, ac.engines.minimum(:id) + end + + def test_null_relation_maximum + ac = Aircraft.new + assert_equal Hash.new, ac.engines.group(:car_id).maximum(:id) + assert_equal nil, ac.engines.maximum(:id) + ac.save + assert_equal Hash.new, ac.engines.group(:car_id).maximum(:id) + assert_equal nil, ac.engines.maximum(:id) + end + def test_joins_with_nil_argument assert_nothing_raised { DependentFirm.joins(nil).first } end