From 6caa36135484a97962358acabf4b14b2aaa14c8f Mon Sep 17 00:00:00 2001
From: luisramos0 <luisramos0@gmail.com>
Date: Sun, 12 Aug 2018 18:18:10 +0100
Subject: [PATCH 1/5] Moved Spree::OrderController.populate to new
 CartController. This was done to make order populate independent of
 Spree::OrdersController

---
 .../darkswarm/services/cart.js.coffee         |   2 +-
 app/controllers/cart_controller.rb            |  96 ++++++++++++++++
 .../spree/orders_controller_decorator.rb      |  80 --------------
 config/routes.rb                              |   8 ++
 spec/controllers/cart_controller_spec.rb      | 104 ++++++++++++++++++
 .../spree/orders_controller_spec.rb           |  98 -----------------
 .../darkswarm/services/cart_spec.js.coffee    |  12 +-
 7 files changed, 215 insertions(+), 185 deletions(-)
 create mode 100644 app/controllers/cart_controller.rb
 create mode 100644 spec/controllers/cart_controller_spec.rb

diff --git a/app/assets/javascripts/darkswarm/services/cart.js.coffee b/app/assets/javascripts/darkswarm/services/cart.js.coffee
index b797390e73..849af562b8 100644
--- a/app/assets/javascripts/darkswarm/services/cart.js.coffee
+++ b/app/assets/javascripts/darkswarm/services/cart.js.coffee
@@ -41,7 +41,7 @@ Darkswarm.factory 'Cart', (CurrentOrder, Variants, $timeout, $http, $modal, $roo
     update: =>
       @update_running = true
 
-      $http.post('/orders/populate', @data()).success (data, status)=>
+      $http.post('/cart/populate', @data()).success (data, status)=>
         @saved()
         @update_running = false
 
diff --git a/app/controllers/cart_controller.rb b/app/controllers/cart_controller.rb
new file mode 100644
index 0000000000..0aa1291eef
--- /dev/null
+++ b/app/controllers/cart_controller.rb
@@ -0,0 +1,96 @@
+require 'spree/core/controller_helpers/order_decorator'
+
+class CartController < BaseController
+  before_filter :check_authorization
+
+  def populate
+    # Without intervention, the Spree::Adjustment#update_adjustable callback is called many times
+    # during cart population, for both taxation and enterprise fees. This operation triggers a
+    # costly Spree::Order#update!, which only needs to be run once. We avoid this by disabling
+    # callbacks on Spree::Adjustment and then manually invoke Spree::Order#update! on success.
+    Spree::Adjustment.without_callbacks do
+      populator = Spree::OrderPopulator.new(current_order(true), current_currency)
+
+      if populator.populate(params.slice(:products, :variants, :quantity), true)
+        fire_event('spree.cart.add')
+        fire_event('spree.order.contents_changed')
+
+        current_order.cap_quantity_at_stock!
+        current_order.update!
+
+        variant_ids = variant_ids_in(populator.variants_h)
+
+        render json: { error: false, stock_levels: stock_levels(current_order, variant_ids) },
+               status: 200
+
+      else
+        render json: { error: true }, status: 412
+      end
+    end
+    populate_variant_attributes
+  end
+
+  # Report the stock levels in the order for all variant ids requested
+  def stock_levels(order, variant_ids)
+    stock_levels = li_stock_levels(order)
+
+    li_variant_ids = stock_levels.keys
+    (variant_ids - li_variant_ids).each do |variant_id|
+      stock_levels[variant_id] = { quantity: 0, max_quantity: 0, on_hand: Spree::Variant.find(variant_id).on_hand }
+    end
+
+    stock_levels
+  end
+
+  def variant_ids_in(variants_h)
+    variants_h.map { |v| v[:variant_id].to_i }
+  end
+
+  private
+
+  def check_authorization
+    session[:access_token] ||= params[:token]
+    order = Spree::Order.find_by_number(params[:id]) || current_order
+
+    if order
+      authorize! :edit, order, session[:access_token]
+    else
+      authorize! :create, Spree::Order
+    end
+  end
+
+  def populate_variant_attributes
+    order = current_order.reload
+
+    if params.key? :variant_attributes
+      params[:variant_attributes].each do |variant_id, attributes|
+        order.set_variant_attributes(Spree::Variant.find(variant_id), attributes)
+      end
+    end
+
+    if params.key? :quantity
+      params[:products].each do |_product_id, variant_id|
+        max_quantity = params[:max_quantity].to_i
+        order.set_variant_attributes(Spree::Variant.find(variant_id),
+                                     max_quantity: max_quantity)
+      end
+    end
+  end
+
+  def li_stock_levels(order)
+    Hash[
+      order.line_items.map do |li|
+        [li.variant.id,
+         { quantity: li.quantity,
+           max_quantity: li.max_quantity,
+           on_hand: wrap_json_infinity(li.variant.on_hand) }]
+      end
+    ]
+  end
+
+  # Rails to_json encodes Float::INFINITY as Infinity, which is not valid JSON
+  # Return it as a large integer (max 32 bit signed int)
+  def wrap_json_infinity(number)
+    number == Float::INFINITY ? 2147483647 : number
+  end
+end
diff --git a/app/controllers/spree/orders_controller_decorator.rb b/app/controllers/spree/orders_controller_decorator.rb
index e435b64522..a354852391 100644
--- a/app/controllers/spree/orders_controller_decorator.rb
+++ b/app/controllers/spree/orders_controller_decorator.rb
@@ -1,7 +1,6 @@
 require 'spree/core/controller_helpers/order_decorator'
 
 Spree::OrdersController.class_eval do
-  after_filter  :populate_variant_attributes, only: :populate
   before_filter :update_distribution, only: :update
   before_filter :filter_order_params, only: :update
   before_filter :enable_embedded_shopfront
@@ -71,61 +70,6 @@ Spree::OrdersController.class_eval do
     end
   end
 
-  def populate
-    # Without intervention, the Spree::Adjustment#update_adjustable callback is called many times
-    # during cart population, for both taxation and enterprise fees. This operation triggers a
-    # costly Spree::Order#update!, which only needs to be run once. We avoid this by disabling
-    # callbacks on Spree::Adjustment and then manually invoke Spree::Order#update! on success.
-
-    Spree::Adjustment.without_callbacks do
-      populator = Spree::OrderPopulator.new(current_order(true), current_currency)
-
-      if populator.populate(params.slice(:products, :variants, :quantity), true)
-        fire_event('spree.cart.add')
-        fire_event('spree.order.contents_changed')
-
-        current_order.cap_quantity_at_stock!
-        current_order.update!
-
-        variant_ids = variant_ids_in(populator.variants_h)
-
-        render json: {error: false, stock_levels: stock_levels(current_order, variant_ids)},
-               status: 200
-
-      else
-        render json: {error: true}, status: 412
-      end
-    end
-  end
-
-  # Report the stock levels in the order for all variant ids requested
-  def stock_levels(order, variant_ids)
-    stock_levels = li_stock_levels(order)
-
-    li_variant_ids = stock_levels.keys
-    (variant_ids - li_variant_ids).each do |variant_id|
-      stock_levels[variant_id] = {quantity: 0, max_quantity: 0,
-                                  on_hand: Spree::Variant.find(variant_id).on_hand}
-    end
-
-    stock_levels
-  end
-
-  def variant_ids_in(variants_h)
-    variants_h.map { |v| v[:variant_id].to_i }
-  end
-
-  def li_stock_levels(order)
-    Hash[
-      order.line_items.map do |li|
-        [li.variant.id,
-         {quantity: li.quantity,
-          max_quantity: li.max_quantity,
-          on_hand: wrap_json_infinity(li.variant.on_hand)}]
-      end
-    ]
-  end
-
   def update_distribution
     @order = current_order(true)
 
@@ -184,30 +128,6 @@ Spree::OrdersController.class_eval do
 
   private
 
-  def populate_variant_attributes
-    order = current_order.reload
-
-    if params.key? :variant_attributes
-      params[:variant_attributes].each do |variant_id, attributes|
-        order.set_variant_attributes(Spree::Variant.find(variant_id), attributes)
-      end
-    end
-
-    if params.key? :quantity
-      params[:products].each do |product_id, variant_id|
-        max_quantity = params[:max_quantity].to_i
-        order.set_variant_attributes(Spree::Variant.find(variant_id),
-                                             {:max_quantity => max_quantity})
-      end
-    end
-  end
-
-  # Rails to_json encodes Float::INFINITY as Infinity, which is not valid JSON
-  # Return it as a large integer (max 32 bit signed int)
-  def wrap_json_infinity(n)
-    n == Float::INFINITY ? 2147483647 : n
-  end
-
   def order_to_update
     return @order_to_update if defined? @order_to_update
     return @order_to_update = current_order unless params[:id]
diff --git a/config/routes.rb b/config/routes.rb
index 37dbfc1e51..574140bd5b 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -24,6 +24,14 @@ Openfoodnetwork::Application.routes.draw do
   get "/connect", to: redirect("https://openfoodnetwork.org/#{ENV['DEFAULT_COUNTRY_CODE'].andand.downcase}/connect/")
   get "/learn", to: redirect("https://openfoodnetwork.org/#{ENV['DEFAULT_COUNTRY_CODE'].andand.downcase}/learn/")
 
+  get "/cart", :to => "spree/orders#edit", :as => :cart
+  put "/cart", :to => "spree/orders#update", :as => :update_cart
+  put "/cart/empty", :to => 'spree/orders#empty', :as => :empty_cart
+
+  resource :cart, controller: "cart" do
+    post :populate
+  end
+
   resource :shop, controller: "shop" do
     get :products
     post :order_cycle
diff --git a/spec/controllers/cart_controller_spec.rb b/spec/controllers/cart_controller_spec.rb
new file mode 100644
index 0000000000..ff6e3bda9b
--- /dev/null
+++ b/spec/controllers/cart_controller_spec.rb
@@ -0,0 +1,104 @@
+require 'spec_helper'
+
+describe CartController, type: :controller do
+  let(:order) { create(:order) }
+
+  describe "returning stock levels in JSON on success" do
+    let(:product) { create(:simple_product) }
+
+    it "returns stock levels as JSON" do
+      controller.stub(:variant_ids_in) { [123] }
+      controller.stub(:stock_levels) { 'my_stock_levels' }
+      Spree::OrderPopulator.stub(:new).and_return(populator = double())
+      populator.stub(:populate) { true }
+      populator.stub(:variants_h) { {} }
+
+      xhr :post, :populate, use_route: :spree, format: :json
+
+      data = JSON.parse(response.body)
+      data['stock_levels'].should == 'my_stock_levels'
+    end
+
+    describe "generating stock levels" do
+      let!(:order) { create(:order) }
+      let!(:li) { create(:line_item, order: order, variant: v, quantity: 2, max_quantity: 3) }
+      let!(:v) { create(:variant, count_on_hand: 4) }
+      let!(:v2) { create(:variant, count_on_hand: 2) }
+
+      before do
+        order.reload
+        controller.stub(:current_order) { order }
+      end
+
+      it "returns a hash with variant id, quantity, max_quantity and stock on hand" do
+        controller.stock_levels(order, [v.id]).should ==
+          {v.id => {quantity: 2, max_quantity: 3, on_hand: 4}}
+      end
+
+      it "includes all line items, even when the variant_id is not specified" do
+        controller.stock_levels(order, []).should ==
+          {v.id => {quantity: 2, max_quantity: 3, on_hand: 4}}
+      end
+
+      it "includes an empty quantity entry for variants that aren't in the order" do
+        controller.stock_levels(order, [v.id, v2.id]).should ==
+          {v.id  => {quantity: 2, max_quantity: 3, on_hand: 4},
+           v2.id => {quantity: 0, max_quantity: 0, on_hand: 2}}
+      end
+
+      describe "encoding Infinity" do
+        let!(:v) { create(:variant, on_demand: true, count_on_hand: 0) }
+
+        it "encodes Infinity as a large, finite integer" do
+          controller.stock_levels(order, [v.id]).should ==
+            {v.id => {quantity: 2, max_quantity: 3, on_hand: 2147483647}}
+        end
+      end
+    end
+
+    it "extracts variant ids from the populator" do
+      variants_h = [{:variant_id=>"900", :quantity=>2, :max_quantity=>nil},
+       {:variant_id=>"940", :quantity=>3, :max_quantity=>3}]
+
+      controller.variant_ids_in(variants_h).should == [900, 940]
+    end
+  end
+
+  context "adding a group buy product to the cart" do
+    it "sets a variant attribute for the max quantity" do
+      distributor_product = create(:distributor_enterprise)
+      p = create(:product, :distributors => [distributor_product], :group_buy => true)
+
+      order = subject.current_order(true)
+      order.stub(:distributor) { distributor_product }
+      order.should_receive(:set_variant_attributes).with(p.master, {'max_quantity' => '3'})
+      controller.stub(:current_order).and_return(order)
+
+      expect do
+        spree_post :populate, :variants => {p.master.id => 1}, :variant_attributes => {p.master.id => {:max_quantity => 3}}
+      end.to change(Spree::LineItem, :count).by(1)
+    end
+
+    it "returns HTTP success when successful" do
+      Spree::OrderPopulator.stub(:new).and_return(populator = double())
+      populator.stub(:populate) { true }
+      populator.stub(:variants_h) { {} }
+      xhr :post, :populate, use_route: :spree, format: :json
+      response.status.should == 200
+    end
+
+    it "returns failure when unsuccessful" do
+      Spree::OrderPopulator.stub(:new).and_return(populator = double())
+      populator.stub(:populate).and_return false
+      xhr :post, :populate, use_route: :spree, format: :json
+      response.status.should == 412
+    end
+
+    it "tells populator to overwrite" do
+      Spree::OrderPopulator.stub(:new).and_return(populator = double())
+      populator.should_receive(:populate).with({}, true)
+      xhr :post, :populate, use_route: :spree, format: :json
+    end
+  end
+end
+
diff --git a/spec/controllers/spree/orders_controller_spec.rb b/spec/controllers/spree/orders_controller_spec.rb
index fbcd5ec107..86f232c48f 100644
--- a/spec/controllers/spree/orders_controller_spec.rb
+++ b/spec/controllers/spree/orders_controller_spec.rb
@@ -63,104 +63,6 @@ describe Spree::OrdersController, type: :controller do
     end
   end
 
-  describe "returning stock levels in JSON on success" do
-    let(:product) { create(:simple_product) }
-
-    it "returns stock levels as JSON" do
-      controller.stub(:variant_ids_in) { [123] }
-      controller.stub(:stock_levels) { 'my_stock_levels' }
-      Spree::OrderPopulator.stub(:new).and_return(populator = double())
-      populator.stub(:populate) { true }
-      populator.stub(:variants_h) { {} }
-
-      xhr :post, :populate, use_route: :spree, format: :json
-
-      data = JSON.parse(response.body)
-      data['stock_levels'].should == 'my_stock_levels'
-    end
-
-    describe "generating stock levels" do
-      let!(:order) { create(:order) }
-      let!(:li) { create(:line_item, order: order, variant: v, quantity: 2, max_quantity: 3) }
-      let!(:v) { create(:variant, count_on_hand: 4) }
-      let!(:v2) { create(:variant, count_on_hand: 2) }
-
-      before do
-        order.reload
-        controller.stub(:current_order) { order }
-      end
-
-      it "returns a hash with variant id, quantity, max_quantity and stock on hand" do
-        controller.stock_levels(order, [v.id]).should ==
-          {v.id => {quantity: 2, max_quantity: 3, on_hand: 4}}
-      end
-
-      it "includes all line items, even when the variant_id is not specified" do
-        controller.stock_levels(order, []).should ==
-          {v.id => {quantity: 2, max_quantity: 3, on_hand: 4}}
-      end
-
-      it "includes an empty quantity entry for variants that aren't in the order" do
-        controller.stock_levels(order, [v.id, v2.id]).should ==
-          {v.id  => {quantity: 2, max_quantity: 3, on_hand: 4},
-           v2.id => {quantity: 0, max_quantity: 0, on_hand: 2}}
-      end
-
-      describe "encoding Infinity" do
-        let!(:v) { create(:variant, on_demand: true, count_on_hand: 0) }
-
-        it "encodes Infinity as a large, finite integer" do
-          controller.stock_levels(order, [v.id]).should ==
-            {v.id => {quantity: 2, max_quantity: 3, on_hand: 2147483647}}
-        end
-      end
-    end
-
-    it "extracts variant ids from the populator" do
-      variants_h = [{:variant_id=>"900", :quantity=>2, :max_quantity=>nil},
-       {:variant_id=>"940", :quantity=>3, :max_quantity=>3}]
-
-      controller.variant_ids_in(variants_h).should == [900, 940]
-    end
-  end
-
-  context "adding a group buy product to the cart" do
-    it "sets a variant attribute for the max quantity" do
-      distributor_product = create(:distributor_enterprise)
-      p = create(:product, :distributors => [distributor_product], :group_buy => true)
-
-      order = subject.current_order(true)
-      order.stub(:distributor) { distributor_product }
-      order.should_receive(:set_variant_attributes).with(p.master, {'max_quantity' => '3'})
-      controller.stub(:current_order).and_return(order)
-
-      expect do
-        spree_post :populate, :variants => {p.master.id => 1}, :variant_attributes => {p.master.id => {:max_quantity => 3}}
-      end.to change(Spree::LineItem, :count).by(1)
-    end
-
-    it "returns HTTP success when successful" do
-      Spree::OrderPopulator.stub(:new).and_return(populator = double())
-      populator.stub(:populate) { true }
-      populator.stub(:variants_h) { {} }
-      xhr :post, :populate, use_route: :spree, format: :json
-      response.status.should == 200
-    end
-
-    it "returns failure when unsuccessful" do
-      Spree::OrderPopulator.stub(:new).and_return(populator = double())
-      populator.stub(:populate).and_return false
-      xhr :post, :populate, use_route: :spree, format: :json
-      response.status.should == 412
-    end
-
-    it "tells populator to overwrite" do
-      Spree::OrderPopulator.stub(:new).and_return(populator = double())
-      populator.should_receive(:populate).with({}, true)
-      xhr :post, :populate, use_route: :spree, format: :json
-    end
-  end
-
   describe "removing line items from cart" do
     describe "when I pass params that includes a line item no longer in our cart" do
       it "should silently ignore the missing line item" do
diff --git a/spec/javascripts/unit/darkswarm/services/cart_spec.js.coffee b/spec/javascripts/unit/darkswarm/services/cart_spec.js.coffee
index 53ae64f437..131880ddc7 100644
--- a/spec/javascripts/unit/darkswarm/services/cart_spec.js.coffee
+++ b/spec/javascripts/unit/darkswarm/services/cart_spec.js.coffee
@@ -80,7 +80,7 @@ describe 'Cart service', ->
     data = {variants: {}}
 
     it "sets update_running during the update, and clears it on success", ->
-      $httpBackend.expectPOST("/orders/populate", data).respond 200, {}
+      $httpBackend.expectPOST("/cart/populate", data).respond 200, {}
       expect(Cart.update_running).toBe(false)
       Cart.update()
       expect(Cart.update_running).toBe(true)
@@ -88,7 +88,7 @@ describe 'Cart service', ->
       expect(Cart.update_running).toBe(false)
 
     it "sets update_running during the update, and clears it on failure", ->
-      $httpBackend.expectPOST("/orders/populate", data).respond 404, {}
+      $httpBackend.expectPOST("/cart/populate", data).respond 404, {}
       expect(Cart.update_running).toBe(false)
       Cart.update()
       expect(Cart.update_running).toBe(true)
@@ -97,7 +97,7 @@ describe 'Cart service', ->
 
     it "marks the form as saved on success", ->
       spyOn(Cart, 'saved')
-      $httpBackend.expectPOST("/orders/populate", data).respond 200, {}
+      $httpBackend.expectPOST("/cart/populate", data).respond 200, {}
       Cart.update()
       $httpBackend.flush()
       expect(Cart.saved).toHaveBeenCalled()
@@ -106,7 +106,7 @@ describe 'Cart service', ->
       Cart.update_enqueued = true
       spyOn(Cart, 'saved')
       spyOn(Cart, 'popQueue')
-      $httpBackend.expectPOST("/orders/populate", data).respond 200, {}
+      $httpBackend.expectPOST("/cart/populate", data).respond 200, {}
       Cart.update()
       $httpBackend.flush()
       expect(Cart.popQueue).toHaveBeenCalled()
@@ -115,14 +115,14 @@ describe 'Cart service', ->
       Cart.update_enqueued = false
       spyOn(Cart, 'saved')
       spyOn(Cart, 'popQueue')
-      $httpBackend.expectPOST("/orders/populate", data).respond 200, {}
+      $httpBackend.expectPOST("/cart/populate", data).respond 200, {}
       Cart.update()
       $httpBackend.flush()
       expect(Cart.popQueue).not.toHaveBeenCalled()
 
     it "retries the update on failure", ->
       spyOn(Cart, 'scheduleRetry')
-      $httpBackend.expectPOST("/orders/populate", data).respond 404, {}
+      $httpBackend.expectPOST("/cart/populate", data).respond 404, {}
       Cart.update()
       $httpBackend.flush()
       expect(Cart.scheduleRetry).toHaveBeenCalled()

From 05bfc098ff7fc5b77c860a59a96f41a9e11acadd Mon Sep 17 00:00:00 2001
From: luisramos0 <luisramos0@gmail.com>
Date: Sun, 12 Aug 2018 20:56:44 +0100
Subject: [PATCH 2/5] /controllers/spree/order_populator_decorator (with a
 class_eval) is now /services/CartService with no dependency to
 Spree::OrderPopulator.

---
 app/controllers/cart_controller.rb            |   6 +-
 .../cart_service.rb}                          |  61 ++--
 spec/controllers/cart_controller_spec.rb      |  24 +-
 spec/models/product_distribution_spec.rb      |   4 +-
 spec/models/spree/order_populator_spec.rb     | 312 ------------------
 spec/services/cart_service_spec.rb            | 310 +++++++++++++++++
 spec/support/request/shop_workflow.rb         |   4 +-
 7 files changed, 365 insertions(+), 356 deletions(-)
 rename app/{models/spree/order_populator_decorator.rb => services/cart_service.rb} (94%)
 delete mode 100644 spec/models/spree/order_populator_spec.rb
 create mode 100644 spec/services/cart_service_spec.rb

diff --git a/app/controllers/cart_controller.rb b/app/controllers/cart_controller.rb
index 0aa1291eef..65bb2402c2 100644
--- a/app/controllers/cart_controller.rb
+++ b/app/controllers/cart_controller.rb
@@ -9,16 +9,16 @@ class CartController < BaseController
     # costly Spree::Order#update!, which only needs to be run once. We avoid this by disabling
     # callbacks on Spree::Adjustment and then manually invoke Spree::Order#update! on success.
     Spree::Adjustment.without_callbacks do
-      populator = Spree::OrderPopulator.new(current_order(true), current_currency)
+      cart_service = CartService.new(current_order(true), current_currency)
 
-      if populator.populate(params.slice(:products, :variants, :quantity), true)
+      if cart_service.populate(params.slice(:products, :variants, :quantity), true)
         fire_event('spree.cart.add')
         fire_event('spree.order.contents_changed')
 
         current_order.cap_quantity_at_stock!
         current_order.update!
 
-        variant_ids = variant_ids_in(populator.variants_h)
+        variant_ids = variant_ids_in(cart_service.variants_h)
 
         render json: { error: false, stock_levels: stock_levels(current_order, variant_ids) },
                status: 200
diff --git a/app/models/spree/order_populator_decorator.rb b/app/services/cart_service.rb
similarity index 94%
rename from app/models/spree/order_populator_decorator.rb
rename to app/services/cart_service.rb
index 314e786c89..6e5e823520 100644
--- a/app/models/spree/order_populator_decorator.rb
+++ b/app/services/cart_service.rb
@@ -1,7 +1,15 @@
 require 'open_food_network/scope_variant_to_hub'
 
-Spree::OrderPopulator.class_eval do
+class CartService
+  attr_accessor :order, :currency
   attr_reader :variants_h
+  attr_reader :errors
+
+  def initialize(order, currency)
+    @order = order
+    @currency = currency
+    @errors = ActiveModel::Errors.new(self)
+  end
 
   def populate(from_hash, overwrite = false)
     @distributor, @order_cycle = distributor_and_order_cycle
@@ -32,27 +40,6 @@ Spree::OrderPopulator.class_eval do
     valid?
   end
 
-  def read_variants(data)
-    @variants_h = read_products_hash(data) +
-                  read_variants_hash(data)
-  end
-
-  def read_products_hash(data)
-    (data[:products] || []).map do |product_id, variant_id|
-      {variant_id: variant_id, quantity: data[:quantity]}
-    end
-  end
-
-  def read_variants_hash(data)
-    (data[:variants] || []).map do |variant_id, quantity|
-      if quantity.is_a?(Hash)
-        {variant_id: variant_id, quantity: quantity[:quantity], max_quantity: quantity[:max_quantity]}
-      else
-        {variant_id: variant_id, quantity: quantity}
-      end
-    end
-  end
-
   def attempt_cart_add(variant_id, quantity, max_quantity = nil)
     quantity = quantity.to_i
     max_quantity = max_quantity.to_i if max_quantity
@@ -82,14 +69,38 @@ Spree::OrderPopulator.class_eval do
     [quantity_to_add, max_quantity_to_add]
   end
 
+  private
+
+  def read_variants(data)
+    @variants_h = read_products_hash(data) +
+                  read_variants_hash(data)
+  end
+
+  def read_products_hash(data)
+    (data[:products] || []).map do |product_id, variant_id|
+      {variant_id: variant_id, quantity: data[:quantity]}
+    end
+  end
+
+  def read_variants_hash(data)
+    (data[:variants] || []).map do |variant_id, quantity|
+      if quantity.is_a?(Hash)
+        {variant_id: variant_id, quantity: quantity[:quantity], max_quantity: quantity[:max_quantity]}
+      else
+        {variant_id: variant_id, quantity: quantity}
+      end
+    end
+  end
+
+  def valid?
+    errors.empty?
+  end
+
   def cart_remove(variant_id)
     variant = Spree::Variant.find(variant_id)
     @order.remove_variant(variant)
   end
 
-
-  private
-
   def distributor_and_order_cycle
     [@order.distributor, @order.order_cycle]
   end
diff --git a/spec/controllers/cart_controller_spec.rb b/spec/controllers/cart_controller_spec.rb
index ff6e3bda9b..4345b91a52 100644
--- a/spec/controllers/cart_controller_spec.rb
+++ b/spec/controllers/cart_controller_spec.rb
@@ -9,9 +9,9 @@ describe CartController, type: :controller do
     it "returns stock levels as JSON" do
       controller.stub(:variant_ids_in) { [123] }
       controller.stub(:stock_levels) { 'my_stock_levels' }
-      Spree::OrderPopulator.stub(:new).and_return(populator = double())
-      populator.stub(:populate) { true }
-      populator.stub(:variants_h) { {} }
+      CartService.stub(:new).and_return(cart_service = double())
+      cart_service.stub(:populate) { true }
+      cart_service.stub(:variants_h) { {} }
 
       xhr :post, :populate, use_route: :spree, format: :json
 
@@ -56,7 +56,7 @@ describe CartController, type: :controller do
       end
     end
 
-    it "extracts variant ids from the populator" do
+    it "extracts variant ids from the cart service" do
       variants_h = [{:variant_id=>"900", :quantity=>2, :max_quantity=>nil},
        {:variant_id=>"940", :quantity=>3, :max_quantity=>3}]
 
@@ -80,23 +80,23 @@ describe CartController, type: :controller do
     end
 
     it "returns HTTP success when successful" do
-      Spree::OrderPopulator.stub(:new).and_return(populator = double())
-      populator.stub(:populate) { true }
-      populator.stub(:variants_h) { {} }
+      CartService.stub(:new).and_return(cart_service = double())
+      cart_service.stub(:populate) { true }
+      cart_service.stub(:variants_h) { {} }
       xhr :post, :populate, use_route: :spree, format: :json
       response.status.should == 200
     end
 
     it "returns failure when unsuccessful" do
-      Spree::OrderPopulator.stub(:new).and_return(populator = double())
-      populator.stub(:populate).and_return false
+      CartService.stub(:new).and_return(cart_service = double())
+      cart_service.stub(:populate).and_return false
       xhr :post, :populate, use_route: :spree, format: :json
       response.status.should == 412
     end
 
-    it "tells populator to overwrite" do
-      Spree::OrderPopulator.stub(:new).and_return(populator = double())
-      populator.should_receive(:populate).with({}, true)
+    it "tells cart_service to overwrite" do
+      CartService.stub(:new).and_return(cart_service = double())
+      cart_service.should_receive(:populate).with({}, true)
       xhr :post, :populate, use_route: :spree, format: :json
     end
   end
diff --git a/spec/models/product_distribution_spec.rb b/spec/models/product_distribution_spec.rb
index 9577ea326f..bf53b0b721 100644
--- a/spec/models/product_distribution_spec.rb
+++ b/spec/models/product_distribution_spec.rb
@@ -36,8 +36,8 @@ describe ProductDistribution do
 
         # When I add the product to the order, an adjustment should be made
         expect do
-          op = Spree::OrderPopulator.new order, 'AU'
-          op.populate products: {product.id => product.master.id}, quantity: 1, distributor_id: distributor.id
+          cart_service = CartService.new order, 'AU'
+          cart_service.populate products: {product.id => product.master.id}, quantity: 1, distributor_id: distributor.id
 
           # Normally the controller would fire this event when the order's contents are changed
           fire_order_contents_changed_event(order.user, order)
diff --git a/spec/models/spree/order_populator_spec.rb b/spec/models/spree/order_populator_spec.rb
deleted file mode 100644
index c0430ba69a..0000000000
--- a/spec/models/spree/order_populator_spec.rb
+++ /dev/null
@@ -1,312 +0,0 @@
-require 'spec_helper'
-
-module Spree
-  describe OrderPopulator do
-    let(:order) { double(:order, id: 123) }
-    let(:currency) { double(:currency) }
-    let(:params) { {} }
-    let(:distributor) { double(:distributor) }
-    let(:order_cycle) { double(:order_cycle) }
-    let(:op) { OrderPopulator.new(order, currency) }
-
-    context "end-to-end" do
-      let(:order) { create(:order, distributor: distributor, order_cycle: order_cycle) }
-      let(:distributor) { create(:distributor_enterprise) }
-      let(:order_cycle) { create(:simple_order_cycle, distributors: [distributor], variants: [v]) }
-      let(:op) { OrderPopulator.new(order, nil) }
-      let(:v) { create(:variant) }
-
-      describe "populate" do
-        it "adds a variant" do
-          op.populate({variants: {v.id.to_s => {quantity: '1', max_quantity: '2'}}}, true)
-          li = order.find_line_item_by_variant(v)
-          li.should be
-          li.quantity.should == 1
-          li.max_quantity.should == 2
-          li.final_weight_volume.should == 1.0
-        end
-
-        it "updates a variant's quantity, max quantity and final_weight_volume" do
-          order.add_variant v, 1, 2
-
-          op.populate({variants: {v.id.to_s => {quantity: '2', max_quantity: '3'}}}, true)
-          li = order.find_line_item_by_variant(v)
-          li.should be
-          li.quantity.should == 2
-          li.max_quantity.should == 3
-          li.final_weight_volume.should == 2.0
-        end
-
-        it "removes a variant" do
-          order.add_variant v, 1, 2
-
-          op.populate({variants: {}}, true)
-          order.line_items(:reload)
-          li = order.find_line_item_by_variant(v)
-          li.should_not be
-        end
-      end
-    end
-
-    describe "populate" do
-      before do
-        op.should_receive(:distributor_and_order_cycle).
-          and_return([distributor, order_cycle])
-      end
-
-      it "checks that distribution can supply all products in the cart" do
-        op.should_receive(:distribution_can_supply_products_in_cart).
-          with(distributor, order_cycle).and_return(false)
-
-        op.populate(params).should be false
-        op.errors.to_a.should == ["That distributor or order cycle can't supply all the products in your cart. Please choose another."]
-      end
-
-      it "locks the order" do
-        op.stub(:distribution_can_supply_products_in_cart).and_return(true)
-        order.should_receive(:with_lock)
-        op.populate(params, true)
-      end
-
-      it "attempts cart add with max_quantity" do
-        op.stub(:distribution_can_supply_products_in_cart).and_return true
-        params = {variants: {"1" => {quantity: 1, max_quantity: 2}}}
-        order.stub(:with_lock).and_yield
-        op.stub(:varies_from_cart) { true }
-        op.stub(:variants_removed) { [] }
-        op.should_receive(:attempt_cart_add).with("1", 1, 2).and_return true
-        op.populate(params, true)
-      end
-    end
-
-    describe "varies_from_cart" do
-      let(:variant) { double(:variant, id: 123) }
-
-      it "returns true when item is not in cart and a quantity is specified" do
-        op.should_receive(:line_item_for_variant_id).with(variant.id).and_return(nil)
-        op.send(:varies_from_cart, {variant_id: variant.id, quantity: '2'}).should be true
-      end
-
-      it "returns true when item is not in cart and a max_quantity is specified" do
-        op.should_receive(:line_item_for_variant_id).with(variant.id).and_return(nil)
-        op.send(:varies_from_cart, {variant_id: variant.id, quantity: '0', max_quantity: '2'}).should be true
-      end
-
-      it "returns false when item is not in cart and no quantity or max_quantity are specified" do
-        op.should_receive(:line_item_for_variant_id).with(variant.id).and_return(nil)
-        op.send(:varies_from_cart, {variant_id: variant.id, quantity: '0'}).should be false
-      end
-
-      it "returns true when quantity varies" do
-        li = double(:line_item, quantity: 1, max_quantity: nil)
-        op.stub(:line_item_for_variant_id) { li }
-
-        op.send(:varies_from_cart, {variant_id: variant.id, quantity: '2'}).should be true
-      end
-
-      it "returns true when max_quantity varies" do
-        li = double(:line_item, quantity: 1, max_quantity: nil)
-        op.stub(:line_item_for_variant_id) { li }
-
-        op.send(:varies_from_cart, {variant_id: variant.id, quantity: '1', max_quantity: '3'}).should be true
-      end
-
-      it "returns false when max_quantity varies only in nil vs 0" do
-        li = double(:line_item, quantity: 1, max_quantity: nil)
-        op.stub(:line_item_for_variant_id) { li }
-
-        op.send(:varies_from_cart, {variant_id: variant.id, quantity: '1'}).should be false
-      end
-
-      it "returns false when both are specified and neither varies" do
-        li = double(:line_item, quantity: 1, max_quantity: 2)
-        op.stub(:line_item_for_variant_id) { li }
-
-        op.send(:varies_from_cart, {variant_id: variant.id, quantity: '1', max_quantity: '2'}).should be false
-      end
-    end
-
-    describe "variants_removed" do
-      it "returns the variant ids when one is in the cart but not in those given" do
-        op.stub(:variant_ids_in_cart) { [123] }
-        op.send(:variants_removed, []).should == [123]
-      end
-
-      it "returns nothing when all items in the cart are provided" do
-        op.stub(:variant_ids_in_cart) { [123] }
-        op.send(:variants_removed, [{variant_id: '123'}]).should == []
-      end
-
-      it "returns nothing when items are added to cart" do
-        op.stub(:variant_ids_in_cart) { [123] }
-        op.send(:variants_removed, [{variant_id: '123'}, {variant_id: '456'}]).should == []
-      end
-
-      it "does not return duplicates" do
-        op.stub(:variant_ids_in_cart) { [123, 123] }
-        op.send(:variants_removed, []).should == [123]
-      end
-    end
-
-    describe "attempt_cart_add" do
-      let(:variant) { double(:variant, on_hand: 250) }
-      let(:quantity) { 123 }
-
-      before do
-        Spree::Variant.stub(:find).and_return(variant)
-        VariantOverride.stub(:for).and_return(nil)
-      end
-
-      it "performs additional validations" do
-        op.should_receive(:check_order_cycle_provided_for).with(variant).and_return(true)
-        op.should_receive(:check_variant_available_under_distribution).with(variant).
-          and_return(true)
-        order.should_receive(:add_variant).with(variant, quantity, nil, currency)
-
-        op.attempt_cart_add(333, quantity.to_s)
-      end
-
-      it "filters quantities through #quantities_to_add" do
-        op.should_receive(:quantities_to_add).with(variant, 123, 123).
-          and_return([5, 5])
-
-        op.stub(:check_order_cycle_provided_for) { true }
-        op.stub(:check_variant_available_under_distribution) { true }
-
-        order.should_receive(:add_variant).with(variant, 5, 5, currency)
-
-        op.attempt_cart_add(333, quantity.to_s, quantity.to_s)
-      end
-
-      it "removes variants which have become out of stock" do
-        op.should_receive(:quantities_to_add).with(variant, 123, 123).
-          and_return([0, 0])
-
-        op.stub(:check_order_cycle_provided_for) { true }
-        op.stub(:check_variant_available_under_distribution) { true }
-
-        order.should_receive(:remove_variant).with(variant)
-        order.should_receive(:add_variant).never
-
-        op.attempt_cart_add(333, quantity.to_s, quantity.to_s)
-      end
-    end
-
-    describe "quantities_to_add" do
-      let(:v) { double(:variant, on_hand: 10) }
-
-      context "when backorders are not allowed" do
-        context "when max_quantity is not provided" do
-          it "returns full amount when available" do
-            op.quantities_to_add(v, 5, nil).should == [5, nil]
-          end
-
-          it "returns a limited amount when not entirely available" do
-            op.quantities_to_add(v, 15, nil).should == [10, nil]
-          end
-        end
-
-        context "when max_quantity is provided" do
-          it "returns full amount when available" do
-            op.quantities_to_add(v, 5, 6).should == [5, 6]
-          end
-
-          it "also returns the full amount when not entirely available" do
-            op.quantities_to_add(v, 15, 16).should == [10, 16]
-          end
-        end
-      end
-
-      context "when backorders are allowed" do
-        before do
-          Spree::Config.allow_backorders = true
-        end
-
-        it "does not limit quantity" do
-          op.quantities_to_add(v, 15, nil).should == [15, nil]
-        end
-
-        it "does not limit max_quantity" do
-          op.quantities_to_add(v, 15, 16).should == [15, 16]
-        end
-      end
-    end
-
-    describe "validations" do
-      describe "determining if distributor can supply products in cart" do
-        it "delegates to DistributionChangeValidator" do
-          dcv = double(:dcv)
-          dcv.should_receive(:can_change_to_distribution?).with(distributor, order_cycle).and_return(true)
-          DistributionChangeValidator.should_receive(:new).with(order).and_return(dcv)
-          op.send(:distribution_can_supply_products_in_cart, distributor, order_cycle).should be true
-        end
-      end
-
-      describe "checking order cycle is provided for a variant, OR is not needed" do
-        let(:variant) { double(:variant) }
-
-        it "returns false and errors when order cycle is not provided and is required" do
-          op.stub(:order_cycle_required_for).and_return true
-          op.send(:check_order_cycle_provided_for, variant).should be false
-          op.errors.to_a.should == ["Please choose an order cycle for this order."]
-        end
-        it "returns true when order cycle is provided" do
-          op.stub(:order_cycle_required_for).and_return true
-          op.instance_variable_set :@order_cycle,  double(:order_cycle)
-          op.send(:check_order_cycle_provided_for, variant).should be true
-        end
-        it "returns true when order cycle is not required" do
-          op.stub(:order_cycle_required_for).and_return false
-          op.send(:check_order_cycle_provided_for, variant).should be true
-        end
-      end
-
-      describe "checking variant is available under the distributor" do
-        let(:product) { double(:product) }
-        let(:variant) { double(:variant, product: product) }
-
-        it "delegates to DistributionChangeValidator, returning true when available" do
-          dcv = double(:dcv)
-          dcv.should_receive(:variants_available_for_distribution).with(123, 234).and_return([variant])
-          DistributionChangeValidator.should_receive(:new).with(order).and_return(dcv)
-          op.instance_eval { @distributor = 123; @order_cycle = 234 }
-          op.send(:check_variant_available_under_distribution, variant).should be true
-          op.errors.should be_empty
-        end
-
-        it "delegates to DistributionChangeValidator, returning false and erroring otherwise" do
-          dcv = double(:dcv)
-          dcv.should_receive(:variants_available_for_distribution).with(123, 234).and_return([])
-          DistributionChangeValidator.should_receive(:new).with(order).and_return(dcv)
-          op.instance_eval { @distributor = 123; @order_cycle = 234 }
-          op.send(:check_variant_available_under_distribution, variant).should be false
-          op.errors.to_a.should == ["That product is not available from the chosen distributor or order cycle."]
-        end
-      end
-    end
-
-
-    describe "support" do
-      describe "checking if order cycle is required for a variant" do
-        it "requires an order cycle when the product has no product distributions" do
-          product = double(:product, product_distributions: [])
-          variant = double(:variant, product: product)
-          op.send(:order_cycle_required_for, variant).should be true
-        end
-
-        it "does not require an order cycle when the product has product distributions" do
-          product = double(:product, product_distributions: [1])
-          variant = double(:variant, product: product)
-          op.send(:order_cycle_required_for, variant).should be false
-        end
-      end
-
-      it "provides the distributor and order cycle for the order" do
-        order.should_receive(:distributor).and_return(distributor)
-        order.should_receive(:order_cycle).and_return(order_cycle)
-        op.send(:distributor_and_order_cycle).should == [distributor,
-                                                         order_cycle]
-      end
-    end
-  end
-end
diff --git a/spec/services/cart_service_spec.rb b/spec/services/cart_service_spec.rb
new file mode 100644
index 0000000000..caa8afe2a5
--- /dev/null
+++ b/spec/services/cart_service_spec.rb
@@ -0,0 +1,310 @@
+require 'spec_helper'
+
+describe CartService do
+  let(:order) { double(:order, id: 123) }
+  let(:currency) { double(:currency) }
+  let(:params) { {} }
+  let(:distributor) { double(:distributor) }
+  let(:order_cycle) { double(:order_cycle) }
+  let(:cart_service) { CartService.new(order, currency) }
+
+  context "end-to-end" do
+    let(:order) { create(:order, distributor: distributor, order_cycle: order_cycle) }
+    let(:distributor) { create(:distributor_enterprise) }
+    let(:order_cycle) { create(:simple_order_cycle, distributors: [distributor], variants: [v]) }
+    let(:cart_service) { CartService.new(order, nil) }
+    let(:v) { create(:variant) }
+
+    describe "populate" do
+      it "adds a variant" do
+        cart_service.populate({variants: {v.id.to_s => {quantity: '1', max_quantity: '2'}}}, true)
+        li = order.find_line_item_by_variant(v)
+        li.should be
+        li.quantity.should == 1
+        li.max_quantity.should == 2
+        li.final_weight_volume.should == 1.0
+      end
+
+      it "updates a variant's quantity, max quantity and final_weight_volume" do
+        order.add_variant v, 1, 2
+
+        cart_service.populate({variants: {v.id.to_s => {quantity: '2', max_quantity: '3'}}}, true)
+        li = order.find_line_item_by_variant(v)
+        li.should be
+        li.quantity.should == 2
+        li.max_quantity.should == 3
+        li.final_weight_volume.should == 2.0
+      end
+
+      it "removes a variant" do
+        order.add_variant v, 1, 2
+
+        cart_service.populate({variants: {}}, true)
+        order.line_items(:reload)
+        li = order.find_line_item_by_variant(v)
+        li.should_not be
+      end
+    end
+  end
+
+  describe "populate" do
+    before do
+      cart_service.should_receive(:distributor_and_order_cycle).
+        and_return([distributor, order_cycle])
+    end
+
+    it "checks that distribution can supply all products in the cart" do
+      cart_service.should_receive(:distribution_can_supply_products_in_cart).
+        with(distributor, order_cycle).and_return(false)
+
+      cart_service.populate(params).should be false
+      cart_service.errors.to_a.should == ["That distributor or order cycle can't supply all the products in your cart. Please choose another."]
+    end
+
+    it "locks the order" do
+      cart_service.stub(:distribution_can_supply_products_in_cart).and_return(true)
+      order.should_receive(:with_lock)
+      cart_service.populate(params, true)
+    end
+
+    it "attempts cart add with max_quantity" do
+      cart_service.stub(:distribution_can_supply_products_in_cart).and_return true
+      params = {variants: {"1" => {quantity: 1, max_quantity: 2}}}
+      order.stub(:with_lock).and_yield
+      cart_service.stub(:varies_from_cart) { true }
+      cart_service.stub(:variants_removed) { [] }
+      cart_service.should_receive(:attempt_cart_add).with("1", 1, 2).and_return true
+      cart_service.populate(params, true)
+    end
+  end
+
+  describe "varies_from_cart" do
+    let(:variant) { double(:variant, id: 123) }
+
+    it "returns true when item is not in cart and a quantity is specified" do
+      cart_service.should_receive(:line_item_for_variant_id).with(variant.id).and_return(nil)
+      cart_service.send(:varies_from_cart, {variant_id: variant.id, quantity: '2'}).should be true
+    end
+
+    it "returns true when item is not in cart and a max_quantity is specified" do
+      cart_service.should_receive(:line_item_for_variant_id).with(variant.id).and_return(nil)
+      cart_service.send(:varies_from_cart, {variant_id: variant.id, quantity: '0', max_quantity: '2'}).should be true
+    end
+
+    it "returns false when item is not in cart and no quantity or max_quantity are specified" do
+      cart_service.should_receive(:line_item_for_variant_id).with(variant.id).and_return(nil)
+      cart_service.send(:varies_from_cart, {variant_id: variant.id, quantity: '0'}).should be false
+    end
+
+    it "returns true when quantity varies" do
+      li = double(:line_item, quantity: 1, max_quantity: nil)
+      cart_service.stub(:line_item_for_variant_id) { li }
+
+      cart_service.send(:varies_from_cart, {variant_id: variant.id, quantity: '2'}).should be true
+    end
+
+    it "returns true when max_quantity varies" do
+      li = double(:line_item, quantity: 1, max_quantity: nil)
+      cart_service.stub(:line_item_for_variant_id) { li }
+
+      cart_service.send(:varies_from_cart, {variant_id: variant.id, quantity: '1', max_quantity: '3'}).should be true
+    end
+
+    it "returns false when max_quantity varies only in nil vs 0" do
+      li = double(:line_item, quantity: 1, max_quantity: nil)
+      cart_service.stub(:line_item_for_variant_id) { li }
+
+      cart_service.send(:varies_from_cart, {variant_id: variant.id, quantity: '1'}).should be false
+    end
+
+    it "returns false when both are specified and neither varies" do
+      li = double(:line_item, quantity: 1, max_quantity: 2)
+      cart_service.stub(:line_item_for_variant_id) { li }
+
+      cart_service.send(:varies_from_cart, {variant_id: variant.id, quantity: '1', max_quantity: '2'}).should be false
+    end
+  end
+
+  describe "variants_removed" do
+    it "returns the variant ids when one is in the cart but not in those given" do
+      cart_service.stub(:variant_ids_in_cart) { [123] }
+      cart_service.send(:variants_removed, []).should == [123]
+    end
+
+    it "returns nothing when all items in the cart are provided" do
+      cart_service.stub(:variant_ids_in_cart) { [123] }
+      cart_service.send(:variants_removed, [{variant_id: '123'}]).should == []
+    end
+
+    it "returns nothing when items are added to cart" do
+      cart_service.stub(:variant_ids_in_cart) { [123] }
+      cart_service.send(:variants_removed, [{variant_id: '123'}, {variant_id: '456'}]).should == []
+    end
+
+    it "does not return duplicates" do
+      cart_service.stub(:variant_ids_in_cart) { [123, 123] }
+      cart_service.send(:variants_removed, []).should == [123]
+    end
+  end
+
+  describe "attempt_cart_add" do
+    let(:variant) { double(:variant, on_hand: 250) }
+    let(:quantity) { 123 }
+
+    before do
+      Spree::Variant.stub(:find).and_return(variant)
+      VariantOverride.stub(:for).and_return(nil)
+    end
+
+    it "performs additional validations" do
+      cart_service.should_receive(:check_order_cycle_provided_for).with(variant).and_return(true)
+      cart_service.should_receive(:check_variant_available_under_distribution).with(variant).
+        and_return(true)
+      order.should_receive(:add_variant).with(variant, quantity, nil, currency)
+
+      cart_service.attempt_cart_add(333, quantity.to_s)
+    end
+
+    it "filters quantities through #quantities_to_add" do
+      cart_service.should_receive(:quantities_to_add).with(variant, 123, 123).
+        and_return([5, 5])
+
+      cart_service.stub(:check_order_cycle_provided_for) { true }
+      cart_service.stub(:check_variant_available_under_distribution) { true }
+
+      order.should_receive(:add_variant).with(variant, 5, 5, currency)
+
+      cart_service.attempt_cart_add(333, quantity.to_s, quantity.to_s)
+    end
+
+    it "removes variants which have become out of stock" do
+      cart_service.should_receive(:quantities_to_add).with(variant, 123, 123).
+        and_return([0, 0])
+
+      cart_service.stub(:check_order_cycle_provided_for) { true }
+      cart_service.stub(:check_variant_available_under_distribution) { true }
+
+      order.should_receive(:remove_variant).with(variant)
+      order.should_receive(:add_variant).never
+
+      cart_service.attempt_cart_add(333, quantity.to_s, quantity.to_s)
+    end
+  end
+
+  describe "quantities_to_add" do
+    let(:v) { double(:variant, on_hand: 10) }
+
+    context "when backorders are not allowed" do
+      context "when max_quantity is not provided" do
+        it "returns full amount when available" do
+          cart_service.quantities_to_add(v, 5, nil).should == [5, nil]
+        end
+
+        it "returns a limited amount when not entirely available" do
+          cart_service.quantities_to_add(v, 15, nil).should == [10, nil]
+        end
+      end
+
+      context "when max_quantity is provided" do
+        it "returns full amount when available" do
+          cart_service.quantities_to_add(v, 5, 6).should == [5, 6]
+        end
+
+        it "also returns the full amount when not entirely available" do
+          cart_service.quantities_to_add(v, 15, 16).should == [10, 16]
+        end
+      end
+    end
+
+    context "when backorders are allowed" do
+      before do
+        Spree::Config.allow_backorders = true
+      end
+
+      it "does not limit quantity" do
+        cart_service.quantities_to_add(v, 15, nil).should == [15, nil]
+      end
+
+      it "does not limit max_quantity" do
+        cart_service.quantities_to_add(v, 15, 16).should == [15, 16]
+      end
+    end
+  end
+
+  describe "validations" do
+    describe "determining if distributor can supply products in cart" do
+      it "delegates to DistributionChangeValidator" do
+        dcv = double(:dcv)
+        dcv.should_receive(:can_change_to_distribution?).with(distributor, order_cycle).and_return(true)
+        DistributionChangeValidator.should_receive(:new).with(order).and_return(dcv)
+        cart_service.send(:distribution_can_supply_products_in_cart, distributor, order_cycle).should be true
+      end
+    end
+
+    describe "checking order cycle is provided for a variant, OR is not needed" do
+      let(:variant) { double(:variant) }
+
+      it "returns false and errors when order cycle is not provided and is required" do
+        cart_service.stub(:order_cycle_required_for).and_return true
+        cart_service.send(:check_order_cycle_provided_for, variant).should be false
+        cart_service.errors.to_a.should == ["Please choose an order cycle for this order."]
+      end
+      it "returns true when order cycle is provided" do
+        cart_service.stub(:order_cycle_required_for).and_return true
+        cart_service.instance_variable_set :@order_cycle,  double(:order_cycle)
+        cart_service.send(:check_order_cycle_provided_for, variant).should be true
+      end
+      it "returns true when order cycle is not required" do
+        cart_service.stub(:order_cycle_required_for).and_return false
+        cart_service.send(:check_order_cycle_provided_for, variant).should be true
+      end
+    end
+
+    describe "checking variant is available under the distributor" do
+      let(:product) { double(:product) }
+      let(:variant) { double(:variant, product: product) }
+
+      it "delegates to DistributionChangeValidator, returning true when available" do
+        dcv = double(:dcv)
+        dcv.should_receive(:variants_available_for_distribution).with(123, 234).and_return([variant])
+        DistributionChangeValidator.should_receive(:new).with(order).and_return(dcv)
+        cart_service.instance_eval { @distributor = 123; @order_cycle = 234 }
+        cart_service.send(:check_variant_available_under_distribution, variant).should be true
+        cart_service.errors.should be_empty
+      end
+
+      it "delegates to DistributionChangeValidator, returning false and erroring otherwise" do
+        dcv = double(:dcv)
+        dcv.should_receive(:variants_available_for_distribution).with(123, 234).and_return([])
+        DistributionChangeValidator.should_receive(:new).with(order).and_return(dcv)
+        cart_service.instance_eval { @distributor = 123; @order_cycle = 234 }
+        cart_service.send(:check_variant_available_under_distribution, variant).should be false
+        cart_service.errors.to_a.should == ["That product is not available from the chosen distributor or order cycle."]
+      end
+    end
+  end
+
+
+  describe "support" do
+    describe "checking if order cycle is required for a variant" do
+      it "requires an order cycle when the product has no product distributions" do
+        product = double(:product, product_distributions: [])
+        variant = double(:variant, product: product)
+        cart_service.send(:order_cycle_required_for, variant).should be true
+      end
+
+      it "does not require an order cycle when the product has product distributions" do
+        product = double(:product, product_distributions: [1])
+        variant = double(:variant, product: product)
+        cart_service.send(:order_cycle_required_for, variant).should be false
+      end
+    end
+
+    it "provides the distributor and order cycle for the order" do
+      order.should_receive(:distributor).and_return(distributor)
+      order.should_receive(:order_cycle).and_return(order_cycle)
+      cart_service.send(:distributor_and_order_cycle).should == [distributor,
+                                                       order_cycle]
+    end
+  end
+end
diff --git a/spec/support/request/shop_workflow.rb b/spec/support/request/shop_workflow.rb
index 3fc7ac53a6..5f3882da26 100644
--- a/spec/support/request/shop_workflow.rb
+++ b/spec/support/request/shop_workflow.rb
@@ -17,8 +17,8 @@ module ShopWorkflow
   end
 
   def add_product_to_cart(order, product, quantity: 1)
-    populator = Spree::OrderPopulator.new(order, order.currency)
-    populator.populate(variants: {product.variants.first.id => quantity})
+    cart_service = CartService.new(order, order.currency)
+    cart_service.populate(variants: {product.variants.first.id => quantity})
 
     # Recalculate fee totals
     order.update_distribution_charge!

From c7b202f932487da8541e5051c3af896e2c3cfd5e Mon Sep 17 00:00:00 2001
From: luisramos0 <luisramos0@gmail.com>
Date: Sun, 12 Aug 2018 23:40:24 +0100
Subject: [PATCH 3/5] Fixed CodeClimate warnings on services/cart_services.rb

---
 app/controllers/cart_controller.rb | 25 +++++-----
 app/services/cart_service.rb       | 75 ++++++++++++++++--------------
 2 files changed, 54 insertions(+), 46 deletions(-)

diff --git a/app/controllers/cart_controller.rb b/app/controllers/cart_controller.rb
index 65bb2402c2..d0efffe96c 100644
--- a/app/controllers/cart_controller.rb
+++ b/app/controllers/cart_controller.rb
@@ -62,18 +62,21 @@ class CartController < BaseController
   def populate_variant_attributes
     order = current_order.reload
 
-    if params.key? :variant_attributes
-      params[:variant_attributes].each do |variant_id, attributes|
-        order.set_variant_attributes(Spree::Variant.find(variant_id), attributes)
-      end
-    end
+    populate_variant_attributes_from_variant(order) if params.key? :variant_attributes
+    populate_variant_attributes_from_product(order) if params.key? :quantity
+  end
 
-    if params.key? :quantity
-      params[:products].each do |_product_id, variant_id|
-        max_quantity = params[:max_quantity].to_i
-        order.set_variant_attributes(Spree::Variant.find(variant_id),
-                                     max_quantity: max_quantity)
-      end
+  def populate_variant_attributes_from_variant(order)
+    params[:variant_attributes].each do |variant_id, attributes|
+      order.set_variant_attributes(Spree::Variant.find(variant_id), attributes)
+    end
+  end
+
+  def populate_variant_attributes_from_product(order)
+    params[:products].each do |_product_id, variant_id|
+      max_quantity = params[:max_quantity].to_i
+      order.set_variant_attributes(Spree::Variant.find(variant_id),
+                                   max_quantity: max_quantity)
     end
   end
 
diff --git a/app/services/cart_service.rb b/app/services/cart_service.rb
index 6e5e823520..a997804eeb 100644
--- a/app/services/cart_service.rb
+++ b/app/services/cart_service.rb
@@ -17,27 +17,23 @@ class CartService
     # this validation probably can't fail
     if !distribution_can_supply_products_in_cart(@distributor, @order_cycle)
       errors.add(:base, I18n.t(:spree_order_populator_error))
+      return false
     end
 
-    if valid?
-      @order.with_lock do
-        variants = read_variants from_hash
+    @order.with_lock do
+      variants = read_variants from_hash
+      attempt_cart_add_variants variants
+      overwrite_variants variants unless !overwrite
+    end
+    valid?
+  end
 
-        variants.each do |v|
-          if varies_from_cart(v)
-            attempt_cart_add(v[:variant_id], v[:quantity], v[:max_quantity])
-          end
-        end
-
-        if overwrite
-          variants_removed(variants).each do |id|
-            cart_remove(id)
-          end
-        end
+  def attempt_cart_add_variants(variants)
+    variants.each do |v|
+      if varies_from_cart(v)
+        attempt_cart_add(v[:variant_id], v[:quantity], v[:max_quantity])
       end
     end
-
-    valid?
   end
 
   def attempt_cart_add(variant_id, quantity, max_quantity = nil)
@@ -45,17 +41,18 @@ class CartService
     max_quantity = max_quantity.to_i if max_quantity
     variant = Spree::Variant.find(variant_id)
     OpenFoodNetwork::ScopeVariantToHub.new(@distributor).scope(variant)
-    if quantity > 0 &&
-       check_order_cycle_provided_for(variant) &&
-       check_variant_available_under_distribution(variant)
 
-      quantity_to_add, max_quantity_to_add = quantities_to_add(variant, quantity, max_quantity)
+    return unless quantity > 0 && valid_variant?(variant)
 
-      if quantity_to_add > 0
-        @order.add_variant(variant, quantity_to_add, max_quantity_to_add, currency)
-      else
-        @order.remove_variant variant
-      end
+    cart_add(variant, quantity, max_quantity)
+  end
+
+  def cart_add(variant, quantity, max_quantity)
+    quantity_to_add, max_quantity_to_add = quantities_to_add(variant, quantity, max_quantity)
+    if quantity_to_add > 0
+      @order.add_variant(variant, quantity_to_add, max_quantity_to_add, currency)
+    else
+      @order.remove_variant variant
     end
   end
 
@@ -69,6 +66,12 @@ class CartService
     [quantity_to_add, max_quantity_to_add]
   end
 
+  def overwrite_variants(variants)
+    variants_removed(variants).each do |id|
+      cart_remove(id)
+    end
+  end
+
   private
 
   def read_variants(data)
@@ -77,17 +80,17 @@ class CartService
   end
 
   def read_products_hash(data)
-    (data[:products] || []).map do |product_id, variant_id|
-      {variant_id: variant_id, quantity: data[:quantity]}
+    (data[:products] || []).map do |_product_id, variant_id|
+      { variant_id: variant_id, quantity: data[:quantity] }
     end
   end
 
   def read_variants_hash(data)
     (data[:variants] || []).map do |variant_id, quantity|
       if quantity.is_a?(Hash)
-        {variant_id: variant_id, quantity: quantity[:quantity], max_quantity: quantity[:max_quantity]}
+        { variant_id: variant_id, quantity: quantity[:quantity], max_quantity: quantity[:max_quantity] }
       else
-        {variant_id: variant_id, quantity: quantity}
+        { variant_id: variant_id, quantity: quantity }
       end
     end
   end
@@ -125,6 +128,10 @@ class CartService
     (variant_ids_in_cart - variant_ids_given).uniq
   end
 
+  def valid_variant?(variant)
+    check_order_cycle_provided_for(variant) && check_variant_available_under_distribution(variant)
+  end
+
   def check_order_cycle_provided_for(variant)
     order_cycle_provided = (!order_cycle_required_for(variant) || @order_cycle.present?)
     errors.add(:base, "Please choose an order cycle for this order.") unless order_cycle_provided
@@ -132,12 +139,10 @@ class CartService
   end
 
   def check_variant_available_under_distribution(variant)
-    if DistributionChangeValidator.new(@order).variants_available_for_distribution(@distributor, @order_cycle).include? variant
-      return true
-    else
-      errors.add(:base, I18n.t(:spree_order_populator_availability_error))
-      return false
-    end
+    return true if DistributionChangeValidator.new(@order).variants_available_for_distribution(@distributor, @order_cycle).include? variant
+
+    errors.add(:base, I18n.t(:spree_order_populator_availability_error))
+    false
   end
 
   def order_cycle_required_for(variant)

From ec069b1e3ecbe23ea5daf179ff8bb49be394b324 Mon Sep 17 00:00:00 2001
From: luisramos0 <luisramos0@gmail.com>
Date: Mon, 13 Aug 2018 16:13:02 +0100
Subject: [PATCH 4/5] Converted specs to latest rspec syntax

---
 spec/controllers/cart_controller_spec.rb      |  54 +++---
 .../spree/orders_controller_spec.rb           |  44 ++---
 spec/services/cart_service_spec.rb            | 172 +++++++++---------
 3 files changed, 137 insertions(+), 133 deletions(-)

diff --git a/spec/controllers/cart_controller_spec.rb b/spec/controllers/cart_controller_spec.rb
index 4345b91a52..4164cbddba 100644
--- a/spec/controllers/cart_controller_spec.rb
+++ b/spec/controllers/cart_controller_spec.rb
@@ -7,16 +7,16 @@ describe CartController, type: :controller do
     let(:product) { create(:simple_product) }
 
     it "returns stock levels as JSON" do
-      controller.stub(:variant_ids_in) { [123] }
-      controller.stub(:stock_levels) { 'my_stock_levels' }
-      CartService.stub(:new).and_return(cart_service = double())
-      cart_service.stub(:populate) { true }
-      cart_service.stub(:variants_h) { {} }
+      allow(controller).to receive(:variant_ids_in) { [123] }
+      allow(controller).to receive(:stock_levels) { 'my_stock_levels' }
+      allow(CartService).to receive(:new).and_return(cart_service = double())
+      allow(cart_service).to receive(:populate) { true }
+      allow(cart_service).to receive(:variants_h) { {} }
 
       xhr :post, :populate, use_route: :spree, format: :json
 
       data = JSON.parse(response.body)
-      data['stock_levels'].should == 'my_stock_levels'
+      expect(data['stock_levels']).to eq('my_stock_levels')
     end
 
     describe "generating stock levels" do
@@ -27,31 +27,35 @@ describe CartController, type: :controller do
 
       before do
         order.reload
-        controller.stub(:current_order) { order }
+        allow(controller).to receive(:current_order) { order }
       end
 
       it "returns a hash with variant id, quantity, max_quantity and stock on hand" do
-        controller.stock_levels(order, [v.id]).should ==
+        expect(controller.stock_levels(order, [v.id])).to eq(
           {v.id => {quantity: 2, max_quantity: 3, on_hand: 4}}
+        )
       end
 
       it "includes all line items, even when the variant_id is not specified" do
-        controller.stock_levels(order, []).should ==
+        expect(controller.stock_levels(order, [])).to eq(
           {v.id => {quantity: 2, max_quantity: 3, on_hand: 4}}
+        )
       end
 
       it "includes an empty quantity entry for variants that aren't in the order" do
-        controller.stock_levels(order, [v.id, v2.id]).should ==
+        expect(controller.stock_levels(order, [v.id, v2.id])).to eq(
           {v.id  => {quantity: 2, max_quantity: 3, on_hand: 4},
            v2.id => {quantity: 0, max_quantity: 0, on_hand: 2}}
+        )
       end
 
       describe "encoding Infinity" do
         let!(:v) { create(:variant, on_demand: true, count_on_hand: 0) }
 
         it "encodes Infinity as a large, finite integer" do
-          controller.stock_levels(order, [v.id]).should ==
+          expect(controller.stock_levels(order, [v.id])).to eq(
             {v.id => {quantity: 2, max_quantity: 3, on_hand: 2147483647}}
+          )
         end
       end
     end
@@ -60,7 +64,7 @@ describe CartController, type: :controller do
       variants_h = [{:variant_id=>"900", :quantity=>2, :max_quantity=>nil},
        {:variant_id=>"940", :quantity=>3, :max_quantity=>3}]
 
-      controller.variant_ids_in(variants_h).should == [900, 940]
+      expect(controller.variant_ids_in(variants_h)).to eq([900, 940])
     end
   end
 
@@ -70,33 +74,33 @@ describe CartController, type: :controller do
       p = create(:product, :distributors => [distributor_product], :group_buy => true)
 
       order = subject.current_order(true)
-      order.stub(:distributor) { distributor_product }
-      order.should_receive(:set_variant_attributes).with(p.master, {'max_quantity' => '3'})
-      controller.stub(:current_order).and_return(order)
+      allow(order).to receive(:distributor) { distributor_product }
+      expect(order).to receive(:set_variant_attributes).with(p.master, {'max_quantity' => '3'})
+      allow(controller).to receive(:current_order).and_return(order)
 
       expect do
-        spree_post :populate, :variants => {p.master.id => 1}, :variant_attributes => {p.master.id => {:max_quantity => 3}}
+        spree_post :populate, variants: { p.master.id => 1 }, variant_attributes: { p.master.id => {max_quantity: 3 } }
       end.to change(Spree::LineItem, :count).by(1)
     end
 
     it "returns HTTP success when successful" do
-      CartService.stub(:new).and_return(cart_service = double())
-      cart_service.stub(:populate) { true }
-      cart_service.stub(:variants_h) { {} }
+      allow(CartService).to receive(:new).and_return(cart_service = double())
+      allow(cart_service).to receive(:populate) { true }
+      allow(cart_service).to receive(:variants_h) { {} }
       xhr :post, :populate, use_route: :spree, format: :json
-      response.status.should == 200
+      expect(response.status).to eq(200)
     end
 
     it "returns failure when unsuccessful" do
-      CartService.stub(:new).and_return(cart_service = double())
-      cart_service.stub(:populate).and_return false
+      allow(CartService).to receive(:new).and_return(cart_service = double())
+      allow(cart_service).to receive(:populate).and_return false
       xhr :post, :populate, use_route: :spree, format: :json
-      response.status.should == 412
+      expect(response.status).to eq(412)
     end
 
     it "tells cart_service to overwrite" do
-      CartService.stub(:new).and_return(cart_service = double())
-      cart_service.should_receive(:populate).with({}, true)
+      allow(CartService).to receive(:new).and_return(cart_service = double())
+      expect(cart_service).to receive(:populate).with({}, true)
       xhr :post, :populate, use_route: :spree, format: :json
     end
   end
diff --git a/spec/controllers/spree/orders_controller_spec.rb b/spec/controllers/spree/orders_controller_spec.rb
index 86f232c48f..b9012ff890 100644
--- a/spec/controllers/spree/orders_controller_spec.rb
+++ b/spec/controllers/spree/orders_controller_spec.rb
@@ -7,40 +7,40 @@ describe Spree::OrdersController, type: :controller do
 
   it "redirects home when no distributor is selected" do
     spree_get :edit
-    response.should redirect_to root_path
+    expect(response).to redirect_to root_path
   end
 
   it "redirects to shop when order is empty" do
-    controller.stub(:current_distributor).and_return(distributor)
-    controller.stub(:current_order_cycle).and_return(order_cycle)
-    controller.stub(:current_order).and_return order
-    order.stub_chain(:line_items, :empty?).and_return true
-    order.stub(:insufficient_stock_lines).and_return []
+    allow(controller).to receive(:current_distributor).and_return(distributor)
+    allow(controller).to receive(:current_order_cycle).and_return(order_cycle)
+    allow(controller).to receive(:current_order).and_return order
+    allow(order).to receive_message_chain(:line_items, :empty?).and_return true
+    allow(order).to receive(:insufficient_stock_lines).and_return []
     session[:access_token] = order.token
     spree_get :edit
-    response.should redirect_to shop_path
+    expect(response).to redirect_to shop_path
   end
 
   it "redirects to the shop when no order cycle is selected" do
-    controller.stub(:current_distributor).and_return(distributor)
+    allow(controller).to receive(:current_distributor).and_return(distributor)
     spree_get :edit
-    response.should redirect_to shop_path
+    expect(response).to redirect_to shop_path
   end
 
   it "redirects home with message if hub is not ready for checkout" do
-    VariantOverride.stub(:indexed).and_return({})
+    allow(VariantOverride).to receive(:indexed).and_return({})
 
     order = subject.current_order(true)
-    distributor.stub(:ready_for_checkout?) { false }
-    order.stub(distributor: distributor, order_cycle: order_cycle)
+    allow(distributor).to receive(:ready_for_checkout?) { false }
+    allow(order).to receive_messages(distributor: distributor, order_cycle: order_cycle)
 
-    order.should_receive(:empty!)
-    order.should_receive(:set_distribution!).with(nil, nil)
+    expect(order).to receive(:empty!)
+    expect(order).to receive(:set_distribution!).with(nil, nil)
 
     spree_get :edit
 
-    response.should redirect_to root_url
-    flash[:info].should == "The hub you have selected is temporarily closed for orders. Please try again later."
+    expect(response).to redirect_to root_url
+    expect(flash[:info]).to eq("The hub you have selected is temporarily closed for orders. Please try again later.")
   end
 
   describe "when an item has insufficient stock" do
@@ -59,7 +59,7 @@ describe Spree::OrdersController, type: :controller do
     it "displays a flash message when we view the cart" do
       spree_get :edit
       expect(response.status).to eq 200
-      flash[:error].should == "An item in your cart has become unavailable."
+      expect(flash[:error]).to eq("An item in your cart has become unavailable.")
     end
   end
 
@@ -72,8 +72,8 @@ describe Spree::OrdersController, type: :controller do
           "0" => {quantity: "0", id: "9999"},
           "1" => {quantity: "99", id: li.id}
         }}
-        response.status.should == 302
-        li.reload.quantity.should == 99
+        expect(response.status).to eq(302)
+        expect(li.reload.quantity).to eq(99)
       end
     end
 
@@ -86,9 +86,9 @@ describe Spree::OrdersController, type: :controller do
         "1" => {quantity: "99", id: li.id}
       }
 
-      controller.remove_missing_line_items(attrs).should == {
+      expect(controller.remove_missing_line_items(attrs)).to eq({
         "1" => {quantity: "99", id: li.id}
-      }
+      })
     end
   end
 
@@ -159,7 +159,7 @@ describe Spree::OrdersController, type: :controller do
       it "updates the fees" do
         expect(order.reload.adjustment_total).to eq enterprise_fee.calculator.preferred_amount
 
-        controller.stub spree_current_user: user
+        allow(controller).to receive_messages spree_current_user: user
         spree_post :update, params
 
         expect(order.reload.adjustment_total).to eq enterprise_fee.calculator.preferred_amount * 2
diff --git a/spec/services/cart_service_spec.rb b/spec/services/cart_service_spec.rb
index caa8afe2a5..3f0f4e0597 100644
--- a/spec/services/cart_service_spec.rb
+++ b/spec/services/cart_service_spec.rb
@@ -19,10 +19,10 @@ describe CartService do
       it "adds a variant" do
         cart_service.populate({variants: {v.id.to_s => {quantity: '1', max_quantity: '2'}}}, true)
         li = order.find_line_item_by_variant(v)
-        li.should be
-        li.quantity.should == 1
-        li.max_quantity.should == 2
-        li.final_weight_volume.should == 1.0
+        expect(li).to be
+        expect(li.quantity).to eq(1)
+        expect(li.max_quantity).to eq(2)
+        expect(li.final_weight_volume).to eq(1.0)
       end
 
       it "updates a variant's quantity, max quantity and final_weight_volume" do
@@ -30,10 +30,10 @@ describe CartService do
 
         cart_service.populate({variants: {v.id.to_s => {quantity: '2', max_quantity: '3'}}}, true)
         li = order.find_line_item_by_variant(v)
-        li.should be
-        li.quantity.should == 2
-        li.max_quantity.should == 3
-        li.final_weight_volume.should == 2.0
+        expect(li).to be
+        expect(li.quantity).to eq(2)
+        expect(li.max_quantity).to eq(3)
+        expect(li.final_weight_volume).to eq(2.0)
       end
 
       it "removes a variant" do
@@ -42,38 +42,38 @@ describe CartService do
         cart_service.populate({variants: {}}, true)
         order.line_items(:reload)
         li = order.find_line_item_by_variant(v)
-        li.should_not be
+        expect(li).not_to be
       end
     end
   end
 
   describe "populate" do
     before do
-      cart_service.should_receive(:distributor_and_order_cycle).
+      expect(cart_service).to receive(:distributor_and_order_cycle).
         and_return([distributor, order_cycle])
     end
 
     it "checks that distribution can supply all products in the cart" do
-      cart_service.should_receive(:distribution_can_supply_products_in_cart).
+      expect(cart_service).to receive(:distribution_can_supply_products_in_cart).
         with(distributor, order_cycle).and_return(false)
 
-      cart_service.populate(params).should be false
-      cart_service.errors.to_a.should == ["That distributor or order cycle can't supply all the products in your cart. Please choose another."]
+      expect(cart_service.populate(params)).to be false
+      expect(cart_service.errors.to_a).to eq(["That distributor or order cycle can't supply all the products in your cart. Please choose another."])
     end
 
     it "locks the order" do
-      cart_service.stub(:distribution_can_supply_products_in_cart).and_return(true)
-      order.should_receive(:with_lock)
+      allow(cart_service).to receive(:distribution_can_supply_products_in_cart).and_return(true)
+      expect(order).to receive(:with_lock)
       cart_service.populate(params, true)
     end
 
     it "attempts cart add with max_quantity" do
-      cart_service.stub(:distribution_can_supply_products_in_cart).and_return true
+      allow(cart_service).to receive(:distribution_can_supply_products_in_cart).and_return true
       params = {variants: {"1" => {quantity: 1, max_quantity: 2}}}
-      order.stub(:with_lock).and_yield
-      cart_service.stub(:varies_from_cart) { true }
-      cart_service.stub(:variants_removed) { [] }
-      cart_service.should_receive(:attempt_cart_add).with("1", 1, 2).and_return true
+      allow(order).to receive(:with_lock).and_yield
+      allow(cart_service).to receive(:varies_from_cart) { true }
+      allow(cart_service).to receive(:variants_removed) { [] }
+      expect(cart_service).to receive(:attempt_cart_add).with("1", 1, 2).and_return true
       cart_service.populate(params, true)
     end
   end
@@ -82,68 +82,68 @@ describe CartService do
     let(:variant) { double(:variant, id: 123) }
 
     it "returns true when item is not in cart and a quantity is specified" do
-      cart_service.should_receive(:line_item_for_variant_id).with(variant.id).and_return(nil)
-      cart_service.send(:varies_from_cart, {variant_id: variant.id, quantity: '2'}).should be true
+      expect(cart_service).to receive(:line_item_for_variant_id).with(variant.id).and_return(nil)
+      expect(cart_service.send(:varies_from_cart, {variant_id: variant.id, quantity: '2'})).to be true
     end
 
     it "returns true when item is not in cart and a max_quantity is specified" do
-      cart_service.should_receive(:line_item_for_variant_id).with(variant.id).and_return(nil)
-      cart_service.send(:varies_from_cart, {variant_id: variant.id, quantity: '0', max_quantity: '2'}).should be true
+      expect(cart_service).to receive(:line_item_for_variant_id).with(variant.id).and_return(nil)
+      expect(cart_service.send(:varies_from_cart, {variant_id: variant.id, quantity: '0', max_quantity: '2'})).to be true
     end
 
     it "returns false when item is not in cart and no quantity or max_quantity are specified" do
-      cart_service.should_receive(:line_item_for_variant_id).with(variant.id).and_return(nil)
-      cart_service.send(:varies_from_cart, {variant_id: variant.id, quantity: '0'}).should be false
+      expect(cart_service).to receive(:line_item_for_variant_id).with(variant.id).and_return(nil)
+      expect(cart_service.send(:varies_from_cart, {variant_id: variant.id, quantity: '0'})).to be false
     end
 
     it "returns true when quantity varies" do
       li = double(:line_item, quantity: 1, max_quantity: nil)
-      cart_service.stub(:line_item_for_variant_id) { li }
+      allow(cart_service).to receive(:line_item_for_variant_id) { li }
 
-      cart_service.send(:varies_from_cart, {variant_id: variant.id, quantity: '2'}).should be true
+      expect(cart_service.send(:varies_from_cart, {variant_id: variant.id, quantity: '2'})).to be true
     end
 
     it "returns true when max_quantity varies" do
       li = double(:line_item, quantity: 1, max_quantity: nil)
-      cart_service.stub(:line_item_for_variant_id) { li }
+      allow(cart_service).to receive(:line_item_for_variant_id) { li }
 
-      cart_service.send(:varies_from_cart, {variant_id: variant.id, quantity: '1', max_quantity: '3'}).should be true
+      expect(cart_service.send(:varies_from_cart, {variant_id: variant.id, quantity: '1', max_quantity: '3'})).to be true
     end
 
     it "returns false when max_quantity varies only in nil vs 0" do
       li = double(:line_item, quantity: 1, max_quantity: nil)
-      cart_service.stub(:line_item_for_variant_id) { li }
+      allow(cart_service).to receive(:line_item_for_variant_id) { li }
 
-      cart_service.send(:varies_from_cart, {variant_id: variant.id, quantity: '1'}).should be false
+      expect(cart_service.send(:varies_from_cart, {variant_id: variant.id, quantity: '1'})).to be false
     end
 
     it "returns false when both are specified and neither varies" do
       li = double(:line_item, quantity: 1, max_quantity: 2)
-      cart_service.stub(:line_item_for_variant_id) { li }
+      allow(cart_service).to receive(:line_item_for_variant_id) { li }
 
-      cart_service.send(:varies_from_cart, {variant_id: variant.id, quantity: '1', max_quantity: '2'}).should be false
+      expect(cart_service.send(:varies_from_cart, {variant_id: variant.id, quantity: '1', max_quantity: '2'})).to be false
     end
   end
 
   describe "variants_removed" do
     it "returns the variant ids when one is in the cart but not in those given" do
-      cart_service.stub(:variant_ids_in_cart) { [123] }
-      cart_service.send(:variants_removed, []).should == [123]
+      allow(cart_service).to receive(:variant_ids_in_cart) { [123] }
+      expect(cart_service.send(:variants_removed, [])).to eq([123])
     end
 
     it "returns nothing when all items in the cart are provided" do
-      cart_service.stub(:variant_ids_in_cart) { [123] }
-      cart_service.send(:variants_removed, [{variant_id: '123'}]).should == []
+      allow(cart_service).to receive(:variant_ids_in_cart) { [123] }
+      expect(cart_service.send(:variants_removed, [{variant_id: '123'}])).to eq([])
     end
 
     it "returns nothing when items are added to cart" do
-      cart_service.stub(:variant_ids_in_cart) { [123] }
-      cart_service.send(:variants_removed, [{variant_id: '123'}, {variant_id: '456'}]).should == []
+      allow(cart_service).to receive(:variant_ids_in_cart) { [123] }
+      expect(cart_service.send(:variants_removed, [{variant_id: '123'}, {variant_id: '456'}])).to eq([])
     end
 
     it "does not return duplicates" do
-      cart_service.stub(:variant_ids_in_cart) { [123, 123] }
-      cart_service.send(:variants_removed, []).should == [123]
+      allow(cart_service).to receive(:variant_ids_in_cart) { [123, 123] }
+      expect(cart_service.send(:variants_removed, [])).to eq([123])
     end
   end
 
@@ -152,40 +152,40 @@ describe CartService do
     let(:quantity) { 123 }
 
     before do
-      Spree::Variant.stub(:find).and_return(variant)
-      VariantOverride.stub(:for).and_return(nil)
+      allow(Spree::Variant).to receive(:find).and_return(variant)
+      allow(VariantOverride).to receive(:for).and_return(nil)
     end
 
     it "performs additional validations" do
-      cart_service.should_receive(:check_order_cycle_provided_for).with(variant).and_return(true)
-      cart_service.should_receive(:check_variant_available_under_distribution).with(variant).
+      expect(cart_service).to receive(:check_order_cycle_provided_for).with(variant).and_return(true)
+      expect(cart_service).to receive(:check_variant_available_under_distribution).with(variant).
         and_return(true)
-      order.should_receive(:add_variant).with(variant, quantity, nil, currency)
+      expect(order).to receive(:add_variant).with(variant, quantity, nil, currency)
 
       cart_service.attempt_cart_add(333, quantity.to_s)
     end
 
     it "filters quantities through #quantities_to_add" do
-      cart_service.should_receive(:quantities_to_add).with(variant, 123, 123).
+      expect(cart_service).to receive(:quantities_to_add).with(variant, 123, 123).
         and_return([5, 5])
 
-      cart_service.stub(:check_order_cycle_provided_for) { true }
-      cart_service.stub(:check_variant_available_under_distribution) { true }
+      allow(cart_service).to receive(:check_order_cycle_provided_for) { true }
+      allow(cart_service).to receive(:check_variant_available_under_distribution) { true }
 
-      order.should_receive(:add_variant).with(variant, 5, 5, currency)
+      expect(order).to receive(:add_variant).with(variant, 5, 5, currency)
 
       cart_service.attempt_cart_add(333, quantity.to_s, quantity.to_s)
     end
 
     it "removes variants which have become out of stock" do
-      cart_service.should_receive(:quantities_to_add).with(variant, 123, 123).
+      expect(cart_service).to receive(:quantities_to_add).with(variant, 123, 123).
         and_return([0, 0])
 
-      cart_service.stub(:check_order_cycle_provided_for) { true }
-      cart_service.stub(:check_variant_available_under_distribution) { true }
+      allow(cart_service).to receive(:check_order_cycle_provided_for) { true }
+      allow(cart_service).to receive(:check_variant_available_under_distribution) { true }
 
-      order.should_receive(:remove_variant).with(variant)
-      order.should_receive(:add_variant).never
+      expect(order).to receive(:remove_variant).with(variant)
+      expect(order).to receive(:add_variant).never
 
       cart_service.attempt_cart_add(333, quantity.to_s, quantity.to_s)
     end
@@ -197,21 +197,21 @@ describe CartService do
     context "when backorders are not allowed" do
       context "when max_quantity is not provided" do
         it "returns full amount when available" do
-          cart_service.quantities_to_add(v, 5, nil).should == [5, nil]
+          expect(cart_service.quantities_to_add(v, 5, nil)).to eq([5, nil])
         end
 
         it "returns a limited amount when not entirely available" do
-          cart_service.quantities_to_add(v, 15, nil).should == [10, nil]
+          expect(cart_service.quantities_to_add(v, 15, nil)).to eq([10, nil])
         end
       end
 
       context "when max_quantity is provided" do
         it "returns full amount when available" do
-          cart_service.quantities_to_add(v, 5, 6).should == [5, 6]
+          expect(cart_service.quantities_to_add(v, 5, 6)).to eq([5, 6])
         end
 
         it "also returns the full amount when not entirely available" do
-          cart_service.quantities_to_add(v, 15, 16).should == [10, 16]
+          expect(cart_service.quantities_to_add(v, 15, 16)).to eq([10, 16])
         end
       end
     end
@@ -222,11 +222,11 @@ describe CartService do
       end
 
       it "does not limit quantity" do
-        cart_service.quantities_to_add(v, 15, nil).should == [15, nil]
+        expect(cart_service.quantities_to_add(v, 15, nil)).to eq([15, nil])
       end
 
       it "does not limit max_quantity" do
-        cart_service.quantities_to_add(v, 15, 16).should == [15, 16]
+        expect(cart_service.quantities_to_add(v, 15, 16)).to eq([15, 16])
       end
     end
   end
@@ -235,9 +235,9 @@ describe CartService do
     describe "determining if distributor can supply products in cart" do
       it "delegates to DistributionChangeValidator" do
         dcv = double(:dcv)
-        dcv.should_receive(:can_change_to_distribution?).with(distributor, order_cycle).and_return(true)
-        DistributionChangeValidator.should_receive(:new).with(order).and_return(dcv)
-        cart_service.send(:distribution_can_supply_products_in_cart, distributor, order_cycle).should be true
+        expect(dcv).to receive(:can_change_to_distribution?).with(distributor, order_cycle).and_return(true)
+        expect(DistributionChangeValidator).to receive(:new).with(order).and_return(dcv)
+        expect(cart_service.send(:distribution_can_supply_products_in_cart, distributor, order_cycle)).to be true
       end
     end
 
@@ -245,18 +245,18 @@ describe CartService do
       let(:variant) { double(:variant) }
 
       it "returns false and errors when order cycle is not provided and is required" do
-        cart_service.stub(:order_cycle_required_for).and_return true
-        cart_service.send(:check_order_cycle_provided_for, variant).should be false
-        cart_service.errors.to_a.should == ["Please choose an order cycle for this order."]
+        allow(cart_service).to receive(:order_cycle_required_for).and_return true
+        expect(cart_service.send(:check_order_cycle_provided_for, variant)).to be false
+        expect(cart_service.errors.to_a).to eq(["Please choose an order cycle for this order."])
       end
       it "returns true when order cycle is provided" do
-        cart_service.stub(:order_cycle_required_for).and_return true
+        allow(cart_service).to receive(:order_cycle_required_for).and_return true
         cart_service.instance_variable_set :@order_cycle,  double(:order_cycle)
-        cart_service.send(:check_order_cycle_provided_for, variant).should be true
+        expect(cart_service.send(:check_order_cycle_provided_for, variant)).to be true
       end
       it "returns true when order cycle is not required" do
-        cart_service.stub(:order_cycle_required_for).and_return false
-        cart_service.send(:check_order_cycle_provided_for, variant).should be true
+        allow(cart_service).to receive(:order_cycle_required_for).and_return false
+        expect(cart_service.send(:check_order_cycle_provided_for, variant)).to be true
       end
     end
 
@@ -266,20 +266,20 @@ describe CartService do
 
       it "delegates to DistributionChangeValidator, returning true when available" do
         dcv = double(:dcv)
-        dcv.should_receive(:variants_available_for_distribution).with(123, 234).and_return([variant])
-        DistributionChangeValidator.should_receive(:new).with(order).and_return(dcv)
+        expect(dcv).to receive(:variants_available_for_distribution).with(123, 234).and_return([variant])
+        expect(DistributionChangeValidator).to receive(:new).with(order).and_return(dcv)
         cart_service.instance_eval { @distributor = 123; @order_cycle = 234 }
-        cart_service.send(:check_variant_available_under_distribution, variant).should be true
-        cart_service.errors.should be_empty
+        expect(cart_service.send(:check_variant_available_under_distribution, variant)).to be true
+        expect(cart_service.errors).to be_empty
       end
 
       it "delegates to DistributionChangeValidator, returning false and erroring otherwise" do
         dcv = double(:dcv)
-        dcv.should_receive(:variants_available_for_distribution).with(123, 234).and_return([])
-        DistributionChangeValidator.should_receive(:new).with(order).and_return(dcv)
+        expect(dcv).to receive(:variants_available_for_distribution).with(123, 234).and_return([])
+        expect(DistributionChangeValidator).to receive(:new).with(order).and_return(dcv)
         cart_service.instance_eval { @distributor = 123; @order_cycle = 234 }
-        cart_service.send(:check_variant_available_under_distribution, variant).should be false
-        cart_service.errors.to_a.should == ["That product is not available from the chosen distributor or order cycle."]
+        expect(cart_service.send(:check_variant_available_under_distribution, variant)).to be false
+        expect(cart_service.errors.to_a).to eq(["That product is not available from the chosen distributor or order cycle."])
       end
     end
   end
@@ -290,21 +290,21 @@ describe CartService do
       it "requires an order cycle when the product has no product distributions" do
         product = double(:product, product_distributions: [])
         variant = double(:variant, product: product)
-        cart_service.send(:order_cycle_required_for, variant).should be true
+        expect(cart_service.send(:order_cycle_required_for, variant)).to be true
       end
 
       it "does not require an order cycle when the product has product distributions" do
         product = double(:product, product_distributions: [1])
         variant = double(:variant, product: product)
-        cart_service.send(:order_cycle_required_for, variant).should be false
+        expect(cart_service.send(:order_cycle_required_for, variant)).to be false
       end
     end
 
     it "provides the distributor and order cycle for the order" do
-      order.should_receive(:distributor).and_return(distributor)
-      order.should_receive(:order_cycle).and_return(order_cycle)
-      cart_service.send(:distributor_and_order_cycle).should == [distributor,
-                                                       order_cycle]
+      expect(order).to receive(:distributor).and_return(distributor)
+      expect(order).to receive(:order_cycle).and_return(order_cycle)
+      expect(cart_service.send(:distributor_and_order_cycle)).to eq([distributor,
+                                                       order_cycle])
     end
   end
 end

From 910297572e5a1409247a67d09ae03fc61991c4b9 Mon Sep 17 00:00:00 2001
From: luisramos0 <luisramos0@gmail.com>
Date: Thu, 16 Aug 2018 22:13:16 +0100
Subject: [PATCH 5/5] Removed unnecessary currency from CartService
 constructor, using order.currency instead

---
 app/controllers/cart_controller.rb       | 2 +-
 app/services/cart_service.rb             | 4 ++--
 spec/models/product_distribution_spec.rb | 2 +-
 spec/services/cart_service_spec.rb       | 8 ++++++--
 spec/support/request/shop_workflow.rb    | 2 +-
 5 files changed, 11 insertions(+), 7 deletions(-)

diff --git a/app/controllers/cart_controller.rb b/app/controllers/cart_controller.rb
index d0efffe96c..30ee434695 100644
--- a/app/controllers/cart_controller.rb
+++ b/app/controllers/cart_controller.rb
@@ -9,7 +9,7 @@ class CartController < BaseController
     # costly Spree::Order#update!, which only needs to be run once. We avoid this by disabling
     # callbacks on Spree::Adjustment and then manually invoke Spree::Order#update! on success.
     Spree::Adjustment.without_callbacks do
-      cart_service = CartService.new(current_order(true), current_currency)
+      cart_service = CartService.new(current_order(true))
 
       if cart_service.populate(params.slice(:products, :variants, :quantity), true)
         fire_event('spree.cart.add')
diff --git a/app/services/cart_service.rb b/app/services/cart_service.rb
index a997804eeb..a5e812c0d3 100644
--- a/app/services/cart_service.rb
+++ b/app/services/cart_service.rb
@@ -5,9 +5,9 @@ class CartService
   attr_reader :variants_h
   attr_reader :errors
 
-  def initialize(order, currency)
+  def initialize(order)
     @order = order
-    @currency = currency
+    @currency = order.currency
     @errors = ActiveModel::Errors.new(self)
   end
 
diff --git a/spec/models/product_distribution_spec.rb b/spec/models/product_distribution_spec.rb
index bf53b0b721..40ba536452 100644
--- a/spec/models/product_distribution_spec.rb
+++ b/spec/models/product_distribution_spec.rb
@@ -36,7 +36,7 @@ describe ProductDistribution do
 
         # When I add the product to the order, an adjustment should be made
         expect do
-          cart_service = CartService.new order, 'AU'
+          cart_service = CartService.new order
           cart_service.populate products: {product.id => product.master.id}, quantity: 1, distributor_id: distributor.id
 
           # Normally the controller would fire this event when the order's contents are changed
diff --git a/spec/services/cart_service_spec.rb b/spec/services/cart_service_spec.rb
index 3f0f4e0597..d3657bc809 100644
--- a/spec/services/cart_service_spec.rb
+++ b/spec/services/cart_service_spec.rb
@@ -6,13 +6,17 @@ describe CartService do
   let(:params) { {} }
   let(:distributor) { double(:distributor) }
   let(:order_cycle) { double(:order_cycle) }
-  let(:cart_service) { CartService.new(order, currency) }
+  let(:cart_service) { CartService.new(order) }
+
+  before do
+    allow(order).to receive(:currency).and_return( currency )
+  end
 
   context "end-to-end" do
     let(:order) { create(:order, distributor: distributor, order_cycle: order_cycle) }
     let(:distributor) { create(:distributor_enterprise) }
     let(:order_cycle) { create(:simple_order_cycle, distributors: [distributor], variants: [v]) }
-    let(:cart_service) { CartService.new(order, nil) }
+    let(:cart_service) { CartService.new(order) }
     let(:v) { create(:variant) }
 
     describe "populate" do
diff --git a/spec/support/request/shop_workflow.rb b/spec/support/request/shop_workflow.rb
index 5f3882da26..876f184644 100644
--- a/spec/support/request/shop_workflow.rb
+++ b/spec/support/request/shop_workflow.rb
@@ -17,7 +17,7 @@ module ShopWorkflow
   end
 
   def add_product_to_cart(order, product, quantity: 1)
-    cart_service = CartService.new(order, order.currency)
+    cart_service = CartService.new(order)
     cart_service.populate(variants: {product.variants.first.id => quantity})
 
     # Recalculate fee totals