From 99cb09c6f75bfc0889ccac2e9ec09f6958e45b44 Mon Sep 17 00:00:00 2001
From: Rohan Mitchell <rohan@rohanmitchell.com>
Date: Wed, 20 May 2015 14:05:41 +1000
Subject: [PATCH] When loading products for shopfront, load all variants in one
 go

---
 app/controllers/shop_controller.rb          | 28 ++++++++++++++++++---
 app/serializers/api/product_serializer.rb   | 14 +++--------
 spec/controllers/shop_controller_spec.rb    | 14 +++++++++++
 spec/serializers/product_serializer_spec.rb | 14 -----------
 4 files changed, 42 insertions(+), 28 deletions(-)
 delete mode 100644 spec/serializers/product_serializer_spec.rb

diff --git a/app/controllers/shop_controller.rb b/app/controllers/shop_controller.rb
index 73861def5c..35aece5e1d 100644
--- a/app/controllers/shop_controller.rb
+++ b/app/controllers/shop_controller.rb
@@ -10,12 +10,15 @@ class ShopController < BaseController
   end
 
   def products
-    # Can we make this query less slow?
-    #
     if @products = products_for_shop
+
       render status: 200,
-        json: ActiveModel::ArraySerializer.new(@products, each_serializer: Api::ProductSerializer,
-        current_order_cycle: current_order_cycle, current_distributor: current_distributor).to_json
+             json: ActiveModel::ArraySerializer.new(@products,
+                                                    each_serializer: Api::ProductSerializer,
+                                                    current_order_cycle: current_order_cycle,
+                                                    current_distributor: current_distributor,
+                                                    variants: variants_for_shop_by_id).to_json
+
     else
       render json: "", status: 404
     end
@@ -46,6 +49,23 @@ class ShopController < BaseController
     end
   end
 
+  def variants_for_shop_by_id
+    # We use the in_stock? method here instead of the in_stock scope because we need to
+    # look up the stock as overridden by VariantOverrides, and the scope method is not affected
+    # by them.
+    variants = Spree::Variant.
+               where(is_master: false).
+               for_distribution(current_order_cycle, current_distributor).
+               each { |v| v.scope_to_hub current_distributor }.
+               select(&:in_stock?)
+
+    variants.inject({}) do |vs, v|
+      vs[v.product_id] ||= []
+      vs[v.product_id] << v
+      vs
+    end
+  end
+
   def taxon_order
     if current_distributor.preferred_shopfront_taxon_order.present?
       current_distributor
diff --git a/app/serializers/api/product_serializer.rb b/app/serializers/api/product_serializer.rb
index 0de794796b..b707aa281c 100644
--- a/app/serializers/api/product_serializer.rb
+++ b/app/serializers/api/product_serializer.rb
@@ -30,8 +30,9 @@ class Api::CachedProductSerializer < ActiveModel::Serializer
   #cached
   #delegate :cache_key, to: :object
 
-  attributes :id, :name, :permalink, :count_on_hand, :on_demand, :group_buy,
-    :notes, :description, :properties_with_values
+  attributes :id, :name, :permalink, :count_on_hand
+  attributes :on_demand, :group_buy, :notes, :description
+  attributes :properties_with_values
 
   has_many :variants, serializer: Api::VariantSerializer
   has_many :taxons, serializer: Api::IdSerializer
@@ -46,13 +47,6 @@ class Api::CachedProductSerializer < ActiveModel::Serializer
   end
 
   def variants
-    # We use the in_stock? method here instead of the in_stock scope because we need to
-    # look up the stock as overridden by VariantOverrides, and the scope method is not affected
-    # by them.
-
-    object.variants.
-      for_distribution(options[:current_order_cycle], options[:current_distributor]).
-      each { |v| v.scope_to_hub options[:current_distributor] }.
-      select(&:in_stock?)
+    options[:variants][object.id] || []
   end
 end
diff --git a/spec/controllers/shop_controller_spec.rb b/spec/controllers/shop_controller_spec.rb
index 4b7c53da19..10f0eed2cd 100644
--- a/spec/controllers/shop_controller_spec.rb
+++ b/spec/controllers/shop_controller_spec.rb
@@ -175,4 +175,18 @@ describe ShopController do
       end
     end
   end
+
+  describe "loading variants" do
+    let(:hub) { create(:distributor_enterprise) }
+    let(:oc) { create(:simple_order_cycle, distributors: [hub], variants: [v1]) }
+    let(:p) { create(:simple_product) }
+    let!(:v1) { create(:variant, product: p, unit_value: 3) }
+    let!(:v2) { create(:variant, product: p, unit_value: 5) }
+
+    it "scopes variants to distribution" do
+      controller.stub(:current_order_cycle) { oc }
+      controller.stub(:current_distributor) { hub }
+      controller.send(:variants_for_shop_by_id).should == {p.id => [v1]}
+    end
+  end
 end
diff --git a/spec/serializers/product_serializer_spec.rb b/spec/serializers/product_serializer_spec.rb
deleted file mode 100644
index 0091668dbb..0000000000
--- a/spec/serializers/product_serializer_spec.rb
+++ /dev/null
@@ -1,14 +0,0 @@
-describe Api::ProductSerializer do
-  let(:hub) { create(:distributor_enterprise) }
-  let(:oc) { create(:simple_order_cycle, distributors: [hub], variants: [v1]) }
-  let(:p) { create(:simple_product) }
-  let!(:v1) { create(:variant, product: p, unit_value: 3) }
-  let!(:v2) { create(:variant, product: p, unit_value: 5) }
-
-  it "scopes variants to distribution" do
-    s = Api::ProductSerializer.new p, current_distributor: hub, current_order_cycle: oc
-    json = s.to_json
-    json.should     include v1.options_text
-    json.should_not include v2.options_text
-  end
-end