spree/spree — pull request #718 by tomash (2011-10-26 18:20:59+00:00)
Title: best_selling_variants and top_grossing_variants are now calculated properly

Admin::OverviewController, best_selling_variants and top_grossing_variants functions.

In best_selling_variants. Previous code that selects line_items (and sums by quantity) for building up a list of top-selling products:

li = LineItem.includes(:order).where("orders.state = 'complete'").sum(:quantity, :group => :variant_id, :limit => 5)

...doesn't work as expected: yes it does return "sum of quantity" of 5 lineitems grouped by variant_id. But they are not the top-selling products. These can be achieved by adding a proper ORDER clause:

li = LineItem.includes(:order).where("orders.state = 'complete'").order("SUM(line_items.quantity) DESC").sum(:quantity, :group => :variant_id, :limit => 5)

It's similar in top_grossing_variants, with a slight twist: we can (we have to) get away with just a single query, although a pretty complex one (counting sums of price-by-quantity multiplies):

def top_grossing_variants
total_sold_prices = LineItem.includes(:order).where("orders.state = 'complete'").order("SUM(line_items.quantity * line_items.price) DESC").sum("price * quantity", :group => :variant_id, :limit => 5)
variants = total_sold_prices.map do |v|
variant = Variant.find(v[0])
[variant.name, v[1]]
end

variants.sort { |x,y| y[1] <=> x[1] }
end

This pull request fixes both methods.

The good thing: this issue, like the whole controller, seems to be pretty old code (dating from 2009), so I think you can cherry-pick this commit to older versions of spree (I think up to 0.30.x).

The bad thing: ordering by SUM will, sooner or later, become a performance hog. It's not currenly in my store (over 1k of Orders, around 3k of LineItems, almost 5k Products), but it will for amounts a few orders of magnitude bigger. So, for high-volume stores, this should be cached somewhere and recalculated on a regular basis (but not with every request to dashboard).

I haven't included any tests: it's a pretty simple and quick fix, and as far as I know the Dashboard is going to experience quite a makeover in terms of available analytics.