From c932d20ef5f2b4f6200f3ab063e1b687206e724f Mon Sep 17 00:00:00 2001
From: Rohan Mitchell <rohan@rohanmitchell.com>
Date: Thu, 5 Nov 2015 12:01:52 +1100
Subject: [PATCH 01/13] Extract multi-sample benchmarking into a helper method

---
 spec/performance/shop_controller_spec.rb | 16 +++-------------
 spec/spec_helper.rb                      |  1 +
 spec/support/performance_helper.rb       | 20 ++++++++++++++++++++
 3 files changed, 24 insertions(+), 13 deletions(-)
 create mode 100644 spec/support/performance_helper.rb

diff --git a/spec/performance/shop_controller_spec.rb b/spec/performance/shop_controller_spec.rb
index 6794c70da5..bfe49cd178 100644
--- a/spec/performance/shop_controller_spec.rb
+++ b/spec/performance/shop_controller_spec.rb
@@ -28,20 +28,10 @@ describe ShopController, type: :controller, performance: true do
     end
 
     it "returns products via json" do
-      results = []
-      4.times do |i|
-        ActiveRecord::Base.connection.query_cache.clear
-        Rails.cache.clear
-        result = Benchmark.measure do
-          xhr :get, :products
-          response.should be_success
-        end
-
-        results << result.total if i > 0
-        puts result
+      results = multi_benchmark(3) do
+        xhr :get, :products
+        response.should be_success
       end
-
-      puts (results.sum / results.count * 1000).round 0
     end
   end
 end
diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb
index a309037ccb..155b617860 100644
--- a/spec/spec_helper.rb
+++ b/spec/spec_helper.rb
@@ -109,6 +109,7 @@ RSpec.configure do |config|
   config.include OpenFoodNetwork::HtmlHelper
   config.include ActionView::Helpers::DateHelper
   config.include OpenFoodNetwork::DelayedJobHelper
+  config.include OpenFoodNetwork::PerformanceHelper
 
   # FactoryGirl
   require 'factory_girl_rails'
diff --git a/spec/support/performance_helper.rb b/spec/support/performance_helper.rb
new file mode 100644
index 0000000000..a2d4fe3630
--- /dev/null
+++ b/spec/support/performance_helper.rb
@@ -0,0 +1,20 @@
+module OpenFoodNetwork
+  module PerformanceHelper
+    def multi_benchmark(num_samples)
+      results = (0..num_samples).map do |i|
+        ActiveRecord::Base.connection.query_cache.clear
+        Rails.cache.clear
+
+        result = Benchmark.measure { yield }
+
+        puts result
+
+        result.total
+      end.drop(1) # Do not return the first sample
+
+      puts (results.sum / results.count * 1000).round 0
+
+      results
+    end
+  end
+end

From 64e8927ae9a1d0c8daad02e56833ebd645fa4aaa Mon Sep 17 00:00:00 2001
From: Rohan Mitchell <rohan@rohanmitchell.com>
Date: Thu, 5 Nov 2015 12:33:49 +1100
Subject: [PATCH 02/13] Add benchmark: add to cart

---
 spec/performance/orders_controller_spec.rb | 41 ++++++++++++++++++++++
 1 file changed, 41 insertions(+)
 create mode 100644 spec/performance/orders_controller_spec.rb

diff --git a/spec/performance/orders_controller_spec.rb b/spec/performance/orders_controller_spec.rb
new file mode 100644
index 0000000000..f23b21d05e
--- /dev/null
+++ b/spec/performance/orders_controller_spec.rb
@@ -0,0 +1,41 @@
+require 'spec_helper'
+
+describe Spree::OrdersController, type: :controller, performance: true do
+  let(:distributor) { create(:distributor_enterprise) }
+  let(:order_cycle) { create(:simple_order_cycle, distributors: [distributor], variants: products.map { |p| p.variants.first }) }
+  let(:products) { (0..9).map { create(:product) } }
+  let(:order) { subject.current_order(true) }
+  let(:num_runs) { 2 }
+
+  before do
+    order.set_distribution! distributor, order_cycle
+    controller.stub(:current_order) { order }
+
+    Spree::Config.currency = 'AUD'
+  end
+
+  describe "adding products to cart" do
+    it "adds products to cart" do
+      puts "1 product, #{num_runs} times..."
+      multi_benchmark(3) do
+        num_runs.times do
+          expect do
+            spree_post :populate, variants: {products[0].variants.first.id => 1}
+          end.to change(Spree::LineItem, :count).by(1)
+          order.empty!
+        end
+      end
+
+      puts "10 products, #{num_runs} times..."
+      variants = Hash[ products.map { |p| [p.variants.first.id, 1] } ]
+      multi_benchmark(3) do
+        num_runs.times do
+          expect do
+            spree_post :populate, variants: variants
+          end.to change(Spree::LineItem, :count).by(10)
+          order.empty!
+        end
+      end
+    end
+  end
+end

From 2f4b5bad26fa6bab09a71c9336c81f79d1736e13 Mon Sep 17 00:00:00 2001
From: Rohan Mitchell <rohan@rohanmitchell.com>
Date: Thu, 5 Nov 2015 13:15:09 +1100
Subject: [PATCH 03/13] Make benchmark more realistic

---
 spec/performance/orders_controller_spec.rb | 30 ++++++++--------------
 1 file changed, 11 insertions(+), 19 deletions(-)

diff --git a/spec/performance/orders_controller_spec.rb b/spec/performance/orders_controller_spec.rb
index f23b21d05e..e151e4f506 100644
--- a/spec/performance/orders_controller_spec.rb
+++ b/spec/performance/orders_controller_spec.rb
@@ -3,9 +3,9 @@ require 'spec_helper'
 describe Spree::OrdersController, type: :controller, performance: true do
   let(:distributor) { create(:distributor_enterprise) }
   let(:order_cycle) { create(:simple_order_cycle, distributors: [distributor], variants: products.map { |p| p.variants.first }) }
-  let(:products) { (0..9).map { create(:product) } }
+  let(:products) { (0...num_products).map { create(:product) } }
   let(:order) { subject.current_order(true) }
-  let(:num_runs) { 2 }
+  let(:num_products) { 20 }
 
   before do
     order.set_distribution! distributor, order_cycle
@@ -16,26 +16,18 @@ describe Spree::OrdersController, type: :controller, performance: true do
 
   describe "adding products to cart" do
     it "adds products to cart" do
-      puts "1 product, #{num_runs} times..."
-      multi_benchmark(3) do
-        num_runs.times do
-          expect do
-            spree_post :populate, variants: {products[0].variants.first.id => 1}
-          end.to change(Spree::LineItem, :count).by(1)
-          order.empty!
+      puts "Pre-populating first product"
+      spree_post :populate, variants: {products[0].variants.first.id => 1}
+
+      result = Benchmark.measure do
+        (1..num_products).each do |num_products|
+          puts "Populating #{num_products} products"
+          variants = Hash[ products.map { |p| [p.variants.first.id, 1] }.first(num_products) ]
+          spree_post :populate, variants: variants
         end
       end
 
-      puts "10 products, #{num_runs} times..."
-      variants = Hash[ products.map { |p| [p.variants.first.id, 1] } ]
-      multi_benchmark(3) do
-        num_runs.times do
-          expect do
-            spree_post :populate, variants: variants
-          end.to change(Spree::LineItem, :count).by(10)
-          order.empty!
-        end
-      end
+      puts result
     end
   end
 end

From 1b5901317dd7ae3ca1e5a255e81153a5b00df310 Mon Sep 17 00:00:00 2001
From: Rohan Mitchell <rohan@rohanmitchell.com>
Date: Fri, 6 Nov 2015 08:27:58 +1100
Subject: [PATCH 04/13] Separate concerns: reading products hash and adding
 items to cart

---
 app/models/spree/order_populator_decorator.rb | 31 +++++++++++++------
 1 file changed, 21 insertions(+), 10 deletions(-)

diff --git a/app/models/spree/order_populator_decorator.rb b/app/models/spree/order_populator_decorator.rb
index cd80d17b97..8b0c11d9a1 100644
--- a/app/models/spree/order_populator_decorator.rb
+++ b/app/models/spree/order_populator_decorator.rb
@@ -13,23 +13,34 @@ Spree::OrderPopulator.class_eval do
       @order.with_lock do
         @order.empty! if overwrite
 
-        from_hash[:products].each do |product_id, variant_id|
-          attempt_cart_add(variant_id, from_hash[:quantity])
-        end if from_hash[:products]
+        variants = read_products_hash(from_hash) +
+                   read_variants_hash(from_hash)
 
-        from_hash[:variants].each do |variant_id, quantity|
-          if quantity.is_a?(Hash)
-            attempt_cart_add(variant_id, quantity[:quantity], quantity[:max_quantity])
-          else
-            attempt_cart_add(variant_id, quantity)
-          end
-        end if from_hash[:variants]
+        variants.each do |v|
+          attempt_cart_add(v[:variant_id], v[:quantity], v[:max_quantity])
+        end
       end
     end
 
     valid?
   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
     variant = Spree::Variant.find(variant_id)

From e175149e76df8678da3cf07a748293a56c053821 Mon Sep 17 00:00:00 2001
From: Rohan Mitchell <rohan@rohanmitchell.com>
Date: Fri, 6 Nov 2015 09:13:47 +1100
Subject: [PATCH 05/13] Add method to check whether a passed-in cart value
 varies from the cart

---
 app/models/spree/order_populator_decorator.rb | 14 ++++++
 spec/models/spree/order_populator_spec.rb     | 48 +++++++++++++++++++
 2 files changed, 62 insertions(+)

diff --git a/app/models/spree/order_populator_decorator.rb b/app/models/spree/order_populator_decorator.rb
index 8b0c11d9a1..d9c261593e 100644
--- a/app/models/spree/order_populator_decorator.rb
+++ b/app/models/spree/order_populator_decorator.rb
@@ -65,6 +65,16 @@ Spree::OrderPopulator.class_eval do
     DistributionChangeValidator.new(@order).can_change_to_distribution?(distributor, order_cycle)
   end
 
+  def varies_from_cart(variant_data)
+    li = line_item_for_variant_id variant_data[:variant_id]
+
+    li_added = li.nil? && (variant_data[:quantity].to_i > 0 || variant_data[:max_quantity].to_i > 0)
+    li_quantity_changed     = li.present? && li.quantity.to_i     != variant_data[:quantity].to_i
+    li_max_quantity_changed = li.present? && li.max_quantity.to_i != variant_data[:max_quantity].to_i
+
+    li_added || li_quantity_changed || li_max_quantity_changed
+  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
@@ -83,4 +93,8 @@ Spree::OrderPopulator.class_eval do
   def order_cycle_required_for(variant)
     variant.product.product_distributions.empty?
   end
+
+  def line_item_for_variant_id(variant_id)
+    order.find_line_item_by_variant Spree::Variant.find(variant_id)
+  end
 end
diff --git a/spec/models/spree/order_populator_spec.rb b/spec/models/spree/order_populator_spec.rb
index ef72106e16..99b980e20d 100644
--- a/spec/models/spree/order_populator_spec.rb
+++ b/spec/models/spree/order_populator_spec.rb
@@ -45,6 +45,54 @@ module Spree
       end
     end
 
+    describe "varies_from_cart" do
+      #let(:order) { create(:order) }
+      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 "attempt_cart_add" do
       it "performs additional validations" do
         variant = double(:variant)

From c432ed9e08b845dc39159a00126e481f4e651c53 Mon Sep 17 00:00:00 2001
From: Rohan Mitchell <rohan@rohanmitchell.com>
Date: Fri, 6 Nov 2015 10:19:25 +1100
Subject: [PATCH 06/13] Build list of variants removed from the cart when
 overwriting

---
 app/models/spree/order_populator_decorator.rb | 10 ++++++++
 spec/models/spree/order_populator_spec.rb     | 23 ++++++++++++++++++-
 2 files changed, 32 insertions(+), 1 deletion(-)

diff --git a/app/models/spree/order_populator_decorator.rb b/app/models/spree/order_populator_decorator.rb
index d9c261593e..32d6afe12d 100644
--- a/app/models/spree/order_populator_decorator.rb
+++ b/app/models/spree/order_populator_decorator.rb
@@ -75,6 +75,12 @@ Spree::OrderPopulator.class_eval do
     li_added || li_quantity_changed || li_max_quantity_changed
   end
 
+  def variants_removed(variants_data)
+    variant_ids_given = variants_data.map { |data| data[:variant_id] }
+
+    (variant_ids_in_cart - variant_ids_given).uniq
+  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
@@ -97,4 +103,8 @@ Spree::OrderPopulator.class_eval do
   def line_item_for_variant_id(variant_id)
     order.find_line_item_by_variant Spree::Variant.find(variant_id)
   end
+
+  def variant_ids_in_cart
+    @order.line_items.map &:variant_id
+  end
 end
diff --git a/spec/models/spree/order_populator_spec.rb b/spec/models/spree/order_populator_spec.rb
index 99b980e20d..1e6e06677d 100644
--- a/spec/models/spree/order_populator_spec.rb
+++ b/spec/models/spree/order_populator_spec.rb
@@ -46,7 +46,6 @@ module Spree
     end
 
     describe "varies_from_cart" do
-      #let(:order) { create(:order) }
       let(:variant) { double(:variant, id: 123) }
 
       it "returns true when item is not in cart and a quantity is specified" do
@@ -93,6 +92,28 @@ module Spree
       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
       it "performs additional validations" do
         variant = double(:variant)

From d3c7c46800207c0a4ca69d15a21d1175cf0b548e Mon Sep 17 00:00:00 2001
From: Rohan Mitchell <rohan@rohanmitchell.com>
Date: Tue, 10 Nov 2015 10:36:39 +1100
Subject: [PATCH 07/13] Add method to remove a variant from an order

---
 app/models/spree/order_decorator.rb |  7 +++++++
 spec/models/spree/order_spec.rb     | 16 ++++++++++++++++
 2 files changed, 23 insertions(+)

diff --git a/app/models/spree/order_decorator.rb b/app/models/spree/order_decorator.rb
index 4ca328a5b1..1aa21bf193 100644
--- a/app/models/spree/order_decorator.rb
+++ b/app/models/spree/order_decorator.rb
@@ -102,6 +102,13 @@ Spree::Order.class_eval do
     end
   end
 
+  def remove_variant(variant)
+    line_items(:reload)
+    current_item = find_line_item_by_variant(variant)
+    current_item.destroy
+  end
+
+
   # Overridden to support max_quantity
   def add_variant(variant, quantity = 1, max_quantity = nil, currency = nil)
     line_items(:reload)
diff --git a/spec/models/spree/order_spec.rb b/spec/models/spree/order_spec.rb
index d86916e24e..e50a64a4d9 100644
--- a/spec/models/spree/order_spec.rb
+++ b/spec/models/spree/order_spec.rb
@@ -329,6 +329,22 @@ describe Spree::Order do
     end
   end
 
+  describe "removing an item from the order" do
+    let(:order) { create(:order) }
+    let(:v1)    { create(:variant) }
+    let(:v2)    { create(:variant) }
+
+    before do
+      order.add_variant v1
+      order.add_variant v2
+    end
+
+    it "removes the variant's line item" do
+      order.remove_variant v1
+      order.line_items(:reload).map(&:variant).should == [v2]
+    end
+  end
+
   describe "emptying the order" do
     it "removes shipping method" do
       subject.shipping_method = create(:shipping_method)

From 888e4d80efe906fd60099b2a4ae6995923424b29 Mon Sep 17 00:00:00 2001
From: Rohan Mitchell <rohan@rohanmitchell.com>
Date: Tue, 10 Nov 2015 10:37:37 +1100
Subject: [PATCH 08/13] Add reliable way to check if cart is dirty. Previous
 way returned true when cart empty.

---
 app/views/shared/menu/_cart.html.haml       | 2 +-
 spec/support/request/ui_component_helper.rb | 6 +++---
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/app/views/shared/menu/_cart.html.haml b/app/views/shared/menu/_cart.html.haml
index f4d3e00c67..6fc538a752 100644
--- a/app/views/shared/menu/_cart.html.haml
+++ b/app/views/shared/menu/_cart.html.haml
@@ -1,4 +1,4 @@
-%span.cart-span{"ng-controller" => "CartCtrl", "ng-class" => "{ dirty: Cart.dirty || Cart.empty() }"}
+%span.cart-span{"ng-controller" => "CartCtrl", "ng-class" => "{ dirty: Cart.dirty || Cart.empty(), 'pure-dirty': Cart.dirty }"}
   %a#cart.icon{cart: true}
     %span.nav-branded
       %i.ofn-i_027-shopping-cart
diff --git a/spec/support/request/ui_component_helper.rb b/spec/support/request/ui_component_helper.rb
index d9f01b447b..35bf628bc0 100644
--- a/spec/support/request/ui_component_helper.rb
+++ b/spec/support/request/ui_component_helper.rb
@@ -36,7 +36,7 @@ module UIComponentHelper
   end
 
   def have_login_modal
-    have_selector ".login-modal" 
+    have_selector ".login-modal"
   end
 
   def open_product_modal(product)
@@ -69,7 +69,7 @@ module UIComponentHelper
   end
 
   def cart_dirty
-    page.find("span.cart-span")[:class].include? 'dirty'
+    page.find("span.cart-span")[:class].include? 'pure-dirty'
   end
 
   def wait_for_ajax
@@ -100,7 +100,7 @@ module UIComponentHelper
   def expand_active_table_node(name)
     find(".active_table_node", text: name).click
   end
-  
+
   def follow_active_table_node(name)
     expand_active_table_node(name)
     find(".active_table_node a", text: "#{name}").click

From 7fffa03d8d76c222b7a28313a3526fcd9289a905 Mon Sep 17 00:00:00 2001
From: Rohan Mitchell <rohan@rohanmitchell.com>
Date: Tue, 10 Nov 2015 11:18:31 +1100
Subject: [PATCH 09/13] Fix bug: set difference comparison of string with
 number

---
 app/models/spree/order_populator_decorator.rb | 2 +-
 spec/models/spree/order_populator_spec.rb     | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/app/models/spree/order_populator_decorator.rb b/app/models/spree/order_populator_decorator.rb
index 32d6afe12d..e9e3f0d0ed 100644
--- a/app/models/spree/order_populator_decorator.rb
+++ b/app/models/spree/order_populator_decorator.rb
@@ -76,7 +76,7 @@ Spree::OrderPopulator.class_eval do
   end
 
   def variants_removed(variants_data)
-    variant_ids_given = variants_data.map { |data| data[:variant_id] }
+    variant_ids_given = variants_data.map { |data| data[:variant_id].to_i }
 
     (variant_ids_in_cart - variant_ids_given).uniq
   end
diff --git a/spec/models/spree/order_populator_spec.rb b/spec/models/spree/order_populator_spec.rb
index 1e6e06677d..2ea77b5b5c 100644
--- a/spec/models/spree/order_populator_spec.rb
+++ b/spec/models/spree/order_populator_spec.rb
@@ -100,12 +100,12 @@ module Spree
 
       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 == []
+        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 == []
+        op.send(:variants_removed, [{variant_id: '123'}, {variant_id: '456'}]).should == []
       end
 
       it "does not return duplicates" do

From 178e5f59e604583f6af0aca9c8689095849a00f7 Mon Sep 17 00:00:00 2001
From: Rohan Mitchell <rohan@rohanmitchell.com>
Date: Tue, 10 Nov 2015 11:22:57 +1100
Subject: [PATCH 10/13] Update cart by applying differences rather than
 clear-and-readd every time

---
 app/models/spree/order_populator_decorator.rb | 17 +++++--
 .../consumer/shopping/shopping_spec.rb        | 17 +++++--
 spec/models/spree/order_populator_spec.rb     | 47 +++++++++++++++----
 3 files changed, 67 insertions(+), 14 deletions(-)

diff --git a/app/models/spree/order_populator_decorator.rb b/app/models/spree/order_populator_decorator.rb
index e9e3f0d0ed..5bf42b0112 100644
--- a/app/models/spree/order_populator_decorator.rb
+++ b/app/models/spree/order_populator_decorator.rb
@@ -11,13 +11,19 @@ Spree::OrderPopulator.class_eval do
 
     if valid?
       @order.with_lock do
-        @order.empty! if overwrite
-
         variants = read_products_hash(from_hash) +
                    read_variants_hash(from_hash)
 
         variants.each do |v|
-          attempt_cart_add(v[:variant_id], v[:quantity], v[:max_quantity])
+          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
       end
     end
@@ -54,6 +60,11 @@ Spree::OrderPopulator.class_eval do
     end
   end
 
+  def cart_remove(variant_id)
+    variant = Spree::Variant.find(variant_id)
+    @order.remove_variant(variant)
+  end
+
 
   private
 
diff --git a/spec/features/consumer/shopping/shopping_spec.rb b/spec/features/consumer/shopping/shopping_spec.rb
index f1bed3a276..12e9d8323b 100644
--- a/spec/features/consumer/shopping/shopping_spec.rb
+++ b/spec/features/consumer/shopping/shopping_spec.rb
@@ -206,18 +206,29 @@ feature "As a consumer I want to shop with a distributor", js: true do
       end
     end
 
-    describe "adding products to cart" do
+    describe "adding and removing products from cart" do
       let(:exchange) { Exchange.find(oc1.exchanges.to_enterprises(distributor).outgoing.first.id) }
       let(:product) { create(:simple_product) }
       let(:variant) { create(:variant, product: product) }
+
       before do
         add_product_and_variant_to_order_cycle(exchange, product, variant)
         set_order_cycle(order, oc1)
         visit shop_path
       end
-      it "should let us add products to our cart" do
-        fill_in "variants[#{variant.id}]", with: "1"
+
+      it "lets us add and remove products from our cart" do
+        fill_in "variants[#{variant.id}]", with: '1'
         page.should have_in_cart product.name
+        wait_until { !cart_dirty }
+        li = Spree::Order.order(:created_at).last.line_items.order(:created_at).last
+        li.quantity.should == 1
+
+        fill_in "variants[#{variant.id}]", with: '0'
+        within('li.cart') { page.should_not have_content product.name }
+        wait_until { !cart_dirty }
+
+        Spree::LineItem.where(id: li).should be_empty
       end
     end
 
diff --git a/spec/models/spree/order_populator_spec.rb b/spec/models/spree/order_populator_spec.rb
index 2ea77b5b5c..4fc3b57d8f 100644
--- a/spec/models/spree/order_populator_spec.rb
+++ b/spec/models/spree/order_populator_spec.rb
@@ -9,11 +9,49 @@ module Spree
     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
+        end
+
+        it "updates a variant's quantity and max quantity" 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
+        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)
@@ -22,13 +60,6 @@ module Spree
         op.errors.to_a.should == ["That distributor or order cycle can't supply all the products in your cart. Please choose another."]
       end
 
-      it "empties the order if override is true" do
-        op.stub(:distribution_can_supply_products_in_cart).and_return true
-        order.stub(:with_lock).and_yield
-        order.should_receive(:empty!)
-        op.populate(params, true)
-      end
-
       it "locks the order" do
         op.stub(:distribution_can_supply_products_in_cart).and_return(true)
         order.should_receive(:with_lock)
@@ -37,9 +68,9 @@ module Spree
 
       it "attempts cart add with max_quantity" do
         op.stub(:distribution_can_supply_products_in_cart).and_return true
-        order.should_receive(:empty!)
         params = {variants: {"1" => {quantity: 1, max_quantity: 2}}}
         order.stub(:with_lock).and_yield
+        op.stub(:varies_from_cart) { true }
         op.should_receive(:attempt_cart_add).with("1", 1, 2).and_return true
         op.populate(params, true)
       end

From a86cc9645297f5a1bda987c6a036168a091efb81 Mon Sep 17 00:00:00 2001
From: Rohan Mitchell <rohan@rohanmitchell.com>
Date: Tue, 10 Nov 2015 11:43:14 +1100
Subject: [PATCH 11/13] Use pluck instead of map

---
 app/models/spree/order_populator_decorator.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/models/spree/order_populator_decorator.rb b/app/models/spree/order_populator_decorator.rb
index 5bf42b0112..1ca0f9efc2 100644
--- a/app/models/spree/order_populator_decorator.rb
+++ b/app/models/spree/order_populator_decorator.rb
@@ -116,6 +116,6 @@ Spree::OrderPopulator.class_eval do
   end
 
   def variant_ids_in_cart
-    @order.line_items.map &:variant_id
+    @order.line_items.pluck :variant_id
   end
 end

From b6f3e6eca665c76cffee98d4e6cf549b9a247b0a Mon Sep 17 00:00:00 2001
From: Rohan Mitchell <rohan@rohanmitchell.com>
Date: Tue, 10 Nov 2015 11:49:31 +1100
Subject: [PATCH 12/13] Fix broken spec

---
 spec/models/spree/order_populator_spec.rb | 1 +
 1 file changed, 1 insertion(+)

diff --git a/spec/models/spree/order_populator_spec.rb b/spec/models/spree/order_populator_spec.rb
index 4fc3b57d8f..b32e87f7f9 100644
--- a/spec/models/spree/order_populator_spec.rb
+++ b/spec/models/spree/order_populator_spec.rb
@@ -71,6 +71,7 @@ module Spree
         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

From 6de44877c888a840aee6473c08f13c6065418974 Mon Sep 17 00:00:00 2001
From: Rohan Mitchell <rohan@rohanmitchell.com>
Date: Wed, 4 Nov 2015 15:43:13 +1100
Subject: [PATCH 13/13] Update link for uber-style sell food dropdown

---
 app/views/shared/menu/_alert.html.haml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/views/shared/menu/_alert.html.haml b/app/views/shared/menu/_alert.html.haml
index 3ce7da91f1..14e3ddef26 100644
--- a/app/views/shared/menu/_alert.html.haml
+++ b/app/views/shared/menu/_alert.html.haml
@@ -1,6 +1,6 @@
 .text-center.page-alert.fixed{ "ofn-page-alert" => true }
   .alert-box
-    %a.alert-cta{href: "http://www.openfoodnetwork.org", target: "_blank"}
+    %a.alert-cta{href: registration_path, target: "_blank"}
       %h6
         Interested in selling food on the Open Food Network?  &nbsp;
         %strong