Wes Foster
Wes Foster

Reputation: 8900

Rails select the object with the highest COUNT in a GROUP

The goal is to select the Store in which a Coupon is most used at.

Currently, I have this, and it works (broken down for explanation):

# coupon.rb
  has_many :redemptions
  has_and_belongs_to_many :stores

  def most_popular_store
    stores.find(        # Return a store object
      redemptions       # Start with all of the coupon's redemptions
      .group(:store_id) # Group them by the store_id
      .count            # Get a hash of { 'store_id' => 'count' } values
      .keys             # Create an array of keys
      .sort             # Sort the keys so highest is first
      .first            # Take the ID of the first key
    )
  end
###

And it's used like this:

describe 'most_popular_store' do
    it 'returns the most popular store' do
        # Create coupon
        coupon = FactoryGirl.create(:coupon)

        # Create two different stores
        most_popular_store = FactoryGirl.create(:store, coupons: [coupon])
        other_store        = FactoryGirl.create(:store, coupons: [coupon])

        # Add redemptions between those stores
        FactoryGirl.create_list(:redemption, 2, coupon: coupon, store: other_store)
        FactoryGirl.create_list(:redemption, 5, coupon: coupon, store: most_popular_store)

        # Verify
        expect(coupon.most_popular_store.title).to eq most_popular_store.title
    end
end

Like I said, the method works, but it looks monkeypatched. How can I refactor my most_popular_store method?

Upvotes: 5

Views: 2659

Answers (1)

AOG
AOG

Reputation: 541

I think your method actually doesn't work. count gives you a hash with keys as store_ids and values as counts, then you run keys on the hash, which gives you an array of store_ids. From then on, you have lost the counts, you are sorting by store_ids and grabbing the first one. The only reason your test passes is that you are creating the popular store before the other one, hence it gets a lower id (sort does the sort ascending by default). To get a correct result, make the following change:

redemptions       # Start with all of the coupon's redemptions
  .group(:store_id) # Group them by the store_id
  .count            # Get a hash of { 'store_id' => 'count' } values
  .max_by{|k,v| v}  # Get key, val pair with the highest value
                    # output => [key, value]
  .first            # Get the first item in array (the key)

Upvotes: 9

Related Questions