Add `group_by` to `ActiveRecord::FinderMethods` #10447

Closed
wants to merge 1 commit into
from
Jump to file or symbol
Failed to load files and symbols.
+56 −0
Split
@@ -1,3 +1,12 @@
+* Add `ActiveRecord::FinderMethods#group_by` method, which collects records into sets, grouped by distinct values for the specified `field`. This patch leverages `ActiveRecord::Relation` to be far more efficient than `Enumerable#group_by` when selecting based on a column name.
+
+ Example:
+
+ User.group_by('role')
@egilburg

egilburg May 4, 2013

Contributor

Should probably use symbol rather than string here as an example here.

+ # => {"normal" => #<ActiveRecord::Relation [...]>, "admin" => #<ActiveRecord::Relation [...]>}
+
+ *Aidan Feldman*
+
* Handle aliased attributes in ActiveRecord::Relation.
When using symbol keys, ActiveRecord will now translate aliased attribute names to the actual column name used in the database:
@@ -176,6 +176,28 @@ def exists?(conditions = :none)
false
end
+ # Collect records into sets, grouped by distinct values for the specified +field+.
+ #
+ # User.select([:id, :name])
+ # => [#<User id: 1, name: "Oscar">, #<User id: 2, name: "Oscar">, #<User id: 3, name: "Foo">
+ #
+ # User.group_by(:name)
+ # => {"Foo" => #<ActiveRecord::Relation [...]>, "Oscar" => #<ActiveRecord::Relation [...]>}
+ def group_by(*args, &block)
@egilburg

egilburg May 4, 2013

Contributor

If you only expect one name, why do you accept varargrs? super (Enumerable's group_by) doesn't accept args at all (other than block), so there is currently no need for method signature to accept more than a single name.

Granted, it might be interesting for multi-arg group_by to work, returning nested structure such as:

array.group_by(:first_attr, :second_attr)

# =>
#  
#   { 
#    :first_value_1 => { :second_value_1 => subrelation_1, :second_value_2 => subrelation_2 }, 
#    :first_value_2 =>  { :second_value_1 => subrelation_3, :second_value_2 => subrelation_4 }
#  } 

But this topic can be explored in a different PR. For this one, I think its fine to only accept one name, defaulting to nil, and call super(&block) if its nil.

@afeld

afeld May 30, 2013

I thought I tried

def group_by(field = nil, &block)
  # ...
  super(&block)
  # ...
end

and had ArgumentErrors... must be going nuts.

+ field = args[0]
+ if field.nil? || block_given?
+ super(&block)
+ else
+ result = {}
+ self.select(field).distinct.each do |item|
@egilburg

egilburg May 4, 2013

Contributor

don't need to use self. prefix in this method for select or where

+ value = item[field]
+ result[value] = self.where(field => value)
+ end
+
+ result
+ end
+ end
+
# This method is called whenever no records are found with either a single
# id or multiple ids and raises a +ActiveRecord::RecordNotFound+ exception.
#
@@ -849,6 +849,31 @@ def test_finder_with_offset_string
assert_nothing_raised(ActiveRecord::StatementInvalid) { Topic.all.merge!(:offset => "3").to_a }
end
+ def test_group_by_with_symbol
+ groups = Company.all.group_by(:type)
+ assert_kind_of Hash, groups
+
+ keys = groups.keys
+ assert_equal 5, keys.size
+ assert_equal [nil, 'Client', 'DependentFirm', 'ExclusivelyDependentFirm', 'Firm'].to_set, keys.to_set
+
+ assert_equal [2, 3, 5, 10], groups['Client'].map(&:id).sort
+ assert_equal [1, 4], groups['Firm'].map(&:id).sort
+ assert_equal nil, groups['Nonexistent']
+ end
+
+ def test_group_by_with_block
+ groups = Company.all.group_by { |c| c.firm_id }
+
+ group = groups[4]
+ assert_kind_of Array, group
+ assert_equal 2, group.size
@egilburg

egilburg May 3, 2013

Contributor

perhaps have more detailed test here, to ensure both values are what you expect, e.g. by comparing ids or some other attributes

+ end
+
+ def test_group_by_with_no_args
+ assert_kind_of Enumerable, Company.all.group_by
+ end
+
protected
def bind(statement, *vars)
if vars.first.is_a?(Hash)