From d5d76d9b9a8ddf3b442b5e78aa0dfac4c86d81a9 Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Fri, 7 May 2021 14:33:26 +0100
Subject: [PATCH 01/59] Move private controller methods to private

---
 app/controllers/cart_controller.rb            |  4 +-
 app/controllers/spree/orders_controller.rb    | 56 +++++++++----------
 spec/controllers/cart_controller_spec.rb      |  2 +-
 .../spree/orders_controller_spec.rb           |  2 +-
 4 files changed, 32 insertions(+), 32 deletions(-)

diff --git a/app/controllers/cart_controller.rb b/app/controllers/cart_controller.rb
index f3a0de59ea..6e6fb22775 100644
--- a/app/controllers/cart_controller.rb
+++ b/app/controllers/cart_controller.rb
@@ -24,12 +24,12 @@ class CartController < BaseController
     populate_variant_attributes
   end
 
+  private
+
   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
diff --git a/app/controllers/spree/orders_controller.rb b/app/controllers/spree/orders_controller.rb
index b519f3043f..4343ff87cb 100644
--- a/app/controllers/spree/orders_controller.rb
+++ b/app/controllers/spree/orders_controller.rb
@@ -38,17 +38,6 @@ module Spree
       redirect_to main_app.cart_path
     end
 
-    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
-
     # Patching to redirect to shop if order is empty
     def edit
       @order = current_order(true)
@@ -109,23 +98,6 @@ module Spree
       end
     end
 
-    def set_current_order
-      @order = current_order(true)
-    end
-
-    def filter_order_params
-      if params[:order] && params[:order][:line_items_attributes]
-        params[:order][:line_items_attributes] =
-          remove_missing_line_items(params[:order][:line_items_attributes])
-      end
-    end
-
-    def remove_missing_line_items(attrs)
-      attrs.select do |_i, line_item|
-        Spree::LineItem.find_by(id: line_item[:id])
-      end
-    end
-
     def cancel
       @order = Spree::Order.find_by!(number: params[:id])
       authorize! :cancel, @order
@@ -140,6 +112,21 @@ module Spree
 
     private
 
+    def set_current_order
+      @order = current_order(true)
+    end
+
+    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
+
     # Stripe can redirect here after a payment is processed in the backoffice.
     # We verify if it was successful here and persist the changes.
     def handle_stripe_response
@@ -153,6 +140,19 @@ module Spree
       @order.reload
     end
 
+    def filter_order_params
+      if params[:order] && params[:order][:line_items_attributes]
+        params[:order][:line_items_attributes] =
+          remove_missing_line_items(params[:order][:line_items_attributes])
+      end
+    end
+
+    def remove_missing_line_items(attrs)
+      attrs.select do |_i, line_item|
+        Spree::LineItem.find_by(id: line_item[:id])
+      end
+    end
+
     def discard_empty_line_items
       @order.line_items = @order.line_items.select { |li| li.quantity > 0 }
     end
diff --git a/spec/controllers/cart_controller_spec.rb b/spec/controllers/cart_controller_spec.rb
index 585c96a37e..5f830c583b 100644
--- a/spec/controllers/cart_controller_spec.rb
+++ b/spec/controllers/cart_controller_spec.rb
@@ -54,7 +54,7 @@ describe CartController, type: :controller do
       variants_h = [{ variant_id: "900", quantity: 2, max_quantity: nil },
                     { variant_id: "940", quantity: 3, max_quantity: 3 }]
 
-      expect(controller.variant_ids_in(variants_h)).to eq([900, 940])
+      expect(controller.__send__(:variant_ids_in, variants_h)).to eq([900, 940])
     end
   end
 
diff --git a/spec/controllers/spree/orders_controller_spec.rb b/spec/controllers/spree/orders_controller_spec.rb
index 78f0459110..34c8b31a52 100644
--- a/spec/controllers/spree/orders_controller_spec.rb
+++ b/spec/controllers/spree/orders_controller_spec.rb
@@ -315,7 +315,7 @@ describe Spree::OrdersController, type: :controller do
         "1" => { quantity: "99", id: li.id }
       }
 
-      expect(controller.remove_missing_line_items(attrs)).to eq(
+      expect(controller.__send__(:remove_missing_line_items, attrs)).to eq(
         "1" => { quantity: "99", id: li.id }
       )
     end

From eb59b691d8d527e20f086b94d55f564a0edf3f8d Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Mon, 10 May 2021 18:38:15 +0100
Subject: [PATCH 02/59] Move hash parsing outside of order lock

---
 app/services/cart_service.rb | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/app/services/cart_service.rb b/app/services/cart_service.rb
index c4f3b5a5c0..c69189e91e 100644
--- a/app/services/cart_service.rb
+++ b/app/services/cart_service.rb
@@ -16,8 +16,9 @@ class CartService
   def populate(from_hash, overwrite = false)
     @distributor, @order_cycle = distributor_and_order_cycle
 
+    variants_data = read_variants from_hash
+
     @order.with_lock do
-      variants_data = read_variants from_hash
       attempt_cart_add_variants variants_data
       overwrite_variants variants_data if overwrite
     end

From d99e598e7a7e344280656d27f79ed95766a2fb73 Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Mon, 10 May 2021 18:37:01 +0100
Subject: [PATCH 03/59] Delete dead code in CartController

The two conditionals in #populate_variant_attributes here are never actually true, so the subsequent code paths are never reached.
---
 app/controllers/cart_controller.rb       | 24 ------------------------
 spec/controllers/cart_controller_spec.rb |  1 -
 spec/models/spree/order_spec.rb          |  5 -----
 3 files changed, 30 deletions(-)

diff --git a/app/controllers/cart_controller.rb b/app/controllers/cart_controller.rb
index 6e6fb22775..2555ed6f34 100644
--- a/app/controllers/cart_controller.rb
+++ b/app/controllers/cart_controller.rb
@@ -20,8 +20,6 @@ class CartController < BaseController
       render json: { error: cart_service.errors.full_messages.join(",") },
              status: :precondition_failed
     end
-
-    populate_variant_attributes
   end
 
   private
@@ -40,26 +38,4 @@ class CartController < BaseController
       authorize! :create, Spree::Order
     end
   end
-
-  def populate_variant_attributes
-    order = current_order.reload
-
-    populate_variant_attributes_from_variant(order) if params.key? :variant_attributes
-    populate_variant_attributes_from_product(order) if params.key? :quantity
-  end
-
-  def populate_variant_attributes_from_variant(order)
-    params[:variant_attributes].each do |variant_id, attributes|
-      permitted = attributes.permit(:quantity, :max_quantity).to_h.with_indifferent_access
-      order.set_variant_attributes(Spree::Variant.find(variant_id), permitted)
-    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
 end
diff --git a/spec/controllers/cart_controller_spec.rb b/spec/controllers/cart_controller_spec.rb
index 5f830c583b..95bbd23927 100644
--- a/spec/controllers/cart_controller_spec.rb
+++ b/spec/controllers/cart_controller_spec.rb
@@ -110,7 +110,6 @@ describe CartController, type: :controller do
       order = subject.current_order(true)
       allow(order).to receive(:distributor) { distributor }
       allow(order).to receive(:order_cycle) { order_cycle }
-      expect(order).to receive(:set_variant_attributes).with(variant, max_quantity: "3")
       allow(controller).to receive(:current_order).and_return(order)
 
       expect do
diff --git a/spec/models/spree/order_spec.rb b/spec/models/spree/order_spec.rb
index e97ac29062..8bfd3c97ec 100644
--- a/spec/models/spree/order_spec.rb
+++ b/spec/models/spree/order_spec.rb
@@ -468,11 +468,6 @@ describe Spree::Order do
       li = Spree::LineItem.last
       expect(li.max_quantity).to eq(3)
     end
-
-    it "does nothing when the line item is not found" do
-      p = build_stubbed(:simple_product)
-      subject.set_variant_attributes(p.master, { 'max_quantity' => '3' }.with_indifferent_access)
-    end
   end
 
   describe "applying enterprise fees" do

From f550d1e9f37a2bf398b3426d258bb03504fda2ce Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Mon, 10 May 2021 15:39:35 +0100
Subject: [PATCH 04/59] Bring Order#ensure_updated_shipments from Spree 2.2

---
 app/models/spree/order.rb | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/app/models/spree/order.rb b/app/models/spree/order.rb
index 0a3042c418..75cdf9a2c4 100644
--- a/app/models/spree/order.rb
+++ b/app/models/spree/order.rb
@@ -517,6 +517,21 @@ module Spree
       shipments
     end
 
+    # Clear shipments and move order back to address state
+    def ensure_updated_shipments
+      if !self.completed? && shipments.any?
+        shipments.destroy_all
+        restart_checkout_flow
+      end
+    end
+
+    def restart_checkout_flow
+      self.update_columns(
+        state: checkout_steps.first,
+        updated_at: Time.zone.now,
+      )
+    end
+
     def refresh_shipment_rates
       shipments.map(&:refresh_rates)
     end

From 34482f671071dabde611c1f3188394fdb38d27f1 Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Mon, 10 May 2021 15:40:26 +0100
Subject: [PATCH 05/59] Bring OrderContents#update_cart from Spree 2.2

---
 app/models/spree/order_contents.rb | 19 +++++++++++++++++--
 1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/app/models/spree/order_contents.rb b/app/models/spree/order_contents.rb
index fb745ef891..d7c52e097e 100644
--- a/app/models/spree/order_contents.rb
+++ b/app/models/spree/order_contents.rb
@@ -27,6 +27,17 @@ module Spree
       remove_from_line_item(line_item, variant, quantity, shipment)
     end
 
+    def update_cart(params)
+      if order.update_attributes(params)
+        order.line_items = order.line_items.select {|li| li.quantity > 0 }
+        order.ensure_updated_shipments
+        update_order
+        true
+      else
+        false
+      end
+    end
+
     private
 
     def add_to_line_item(line_item, variant, quantity, shipment = nil)
@@ -40,7 +51,7 @@ module Spree
       end
 
       line_item.save
-      order.reload
+      update_order
       line_item
     end
 
@@ -55,8 +66,12 @@ module Spree
         line_item.save!
       end
 
-      order.reload
+      update_order
       line_item
     end
+
+    def update_order
+      order.reload
+    end
   end
 end

From 66a3246a23ab2c40663e5409638b32155880adb6 Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Mon, 10 May 2021 22:23:14 +0100
Subject: [PATCH 06/59] Move order updating

---
 app/models/spree/order_contents.rb | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/app/models/spree/order_contents.rb b/app/models/spree/order_contents.rb
index d7c52e097e..d0478256e6 100644
--- a/app/models/spree/order_contents.rb
+++ b/app/models/spree/order_contents.rb
@@ -13,6 +13,7 @@ module Spree
     def add(variant, quantity = 1, shipment = nil)
       line_item = order.find_line_item_by_variant(variant)
       add_to_line_item(line_item, variant, quantity, shipment)
+      update_order
     end
 
     # Get current line item for variant
@@ -25,6 +26,7 @@ module Spree
       end
 
       remove_from_line_item(line_item, variant, quantity, shipment)
+      update_order
     end
 
     def update_cart(params)
@@ -51,7 +53,6 @@ module Spree
       end
 
       line_item.save
-      update_order
       line_item
     end
 
@@ -66,7 +67,6 @@ module Spree
         line_item.save!
       end
 
-      update_order
       line_item
     end
 

From 8405a7672dab5b1dcf7de34e8b7072ab4e85cb67 Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Mon, 10 May 2021 22:23:43 +0100
Subject: [PATCH 07/59] Bring in shipment updating

---
 app/models/spree/order_contents.rb | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/app/models/spree/order_contents.rb b/app/models/spree/order_contents.rb
index d0478256e6..024038ffec 100644
--- a/app/models/spree/order_contents.rb
+++ b/app/models/spree/order_contents.rb
@@ -13,6 +13,7 @@ module Spree
     def add(variant, quantity = 1, shipment = nil)
       line_item = order.find_line_item_by_variant(variant)
       add_to_line_item(line_item, variant, quantity, shipment)
+      shipment.present? ? shipment.update_amounts : order.ensure_updated_shipments
       update_order
     end
 
@@ -26,6 +27,7 @@ module Spree
       end
 
       remove_from_line_item(line_item, variant, quantity, shipment)
+      shipment.present? ? shipment.update_amounts : order.ensure_updated_shipments
       update_order
     end
 

From 34eaf6f0f21b08cc63d3b9c2e5f44ea995114934 Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Mon, 10 May 2021 22:25:03 +0100
Subject: [PATCH 08/59] DRY shipment updating

---
 app/models/spree/order_contents.rb | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/app/models/spree/order_contents.rb b/app/models/spree/order_contents.rb
index 024038ffec..adcc411fdb 100644
--- a/app/models/spree/order_contents.rb
+++ b/app/models/spree/order_contents.rb
@@ -13,7 +13,7 @@ module Spree
     def add(variant, quantity = 1, shipment = nil)
       line_item = order.find_line_item_by_variant(variant)
       add_to_line_item(line_item, variant, quantity, shipment)
-      shipment.present? ? shipment.update_amounts : order.ensure_updated_shipments
+      update_shipment(shipment)
       update_order
     end
 
@@ -27,7 +27,7 @@ module Spree
       end
 
       remove_from_line_item(line_item, variant, quantity, shipment)
-      shipment.present? ? shipment.update_amounts : order.ensure_updated_shipments
+      update_shipment(shipment)
       update_order
     end
 
@@ -44,6 +44,10 @@ module Spree
 
     private
 
+    def update_shipment(shipment)
+      shipment.present? ? shipment.update_amounts : order.ensure_updated_shipments
+    end
+
     def add_to_line_item(line_item, variant, quantity, shipment = nil)
       if line_item
         line_item.target_shipment = shipment

From 67264c047f4c8ab87b2169047d00fb8f33bc08cc Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Mon, 10 May 2021 23:27:39 +0100
Subject: [PATCH 09/59] Delete dead code #editable_by?

This is junk Spree code from 2010. 11 years old! The method that previous called this was removed long ago...
---
 app/models/spree/shipment.rb | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/app/models/spree/shipment.rb b/app/models/spree/shipment.rb
index dd4cb87fe6..ac88f43974 100644
--- a/app/models/spree/shipment.rb
+++ b/app/models/spree/shipment.rb
@@ -160,10 +160,6 @@ module Spree
       Spree::Money.new(item_cost, currency: currency)
     end
 
-    def editable_by?(_user)
-      !shipped?
-    end
-
     def update_amounts
       return unless fee_adjustment&.amount != cost
 

From 1fff81c214b83f495298bab89da12d128341184a Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Tue, 11 May 2021 00:03:45 +0100
Subject: [PATCH 10/59] Remove call to OrderInventory#verify

This already gets called in after_save and after_destroy in Spree::LineItem.
---
 app/models/spree/order_contents.rb | 1 -
 1 file changed, 1 deletion(-)

diff --git a/app/models/spree/order_contents.rb b/app/models/spree/order_contents.rb
index adcc411fdb..4ee92ca906 100644
--- a/app/models/spree/order_contents.rb
+++ b/app/models/spree/order_contents.rb
@@ -67,7 +67,6 @@ module Spree
       line_item.target_shipment = shipment
 
       if line_item.quantity == 0
-        Spree::OrderInventory.new(order).verify(line_item, shipment)
         line_item.destroy
       else
         line_item.save!

From 0fabb5bc2fb57bf1d26e2714f13a8f8e7d67ebed Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Tue, 11 May 2021 01:45:54 +0100
Subject: [PATCH 11/59] Don't pluck variant_ids multiple times (once for each
 line item)

---
 app/models/spree/shipment.rb | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/app/models/spree/shipment.rb b/app/models/spree/shipment.rb
index ac88f43974..1a750dc207 100644
--- a/app/models/spree/shipment.rb
+++ b/app/models/spree/shipment.rb
@@ -185,7 +185,8 @@ module Spree
 
     def line_items
       if order.complete?
-        order.line_items.select { |li| inventory_units.pluck(:variant_id).include?(li.variant_id) }
+        inventory_unit_ids = inventory_units.pluck(:variant_id)
+        order.line_items.select { |li| inventory_unit_ids.include?(li.variant_id) }
       else
         order.line_items
       end

From a1241415378ad1a85fa609fc98b5d96d7305d39d Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Tue, 11 May 2021 01:46:23 +0100
Subject: [PATCH 12/59] Move Shipment#line_items to private

---
 app/models/spree/shipment.rb | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/app/models/spree/shipment.rb b/app/models/spree/shipment.rb
index 1a750dc207..447363cc7f 100644
--- a/app/models/spree/shipment.rb
+++ b/app/models/spree/shipment.rb
@@ -183,15 +183,6 @@ module Spree
       @scoper ||= OpenFoodNetwork::ScopeVariantToHub.new(order.distributor)
     end
 
-    def line_items
-      if order.complete?
-        inventory_unit_ids = inventory_units.pluck(:variant_id)
-        order.line_items.select { |li| inventory_unit_ids.include?(li.variant_id) }
-      else
-        order.line_items
-      end
-    end
-
     def finalize!
       InventoryUnit.finalize_units!(inventory_units)
       manifest.each { |item| manifest_unstock(item) }
@@ -292,6 +283,15 @@ module Spree
 
     private
 
+    def line_items
+      if order.complete?
+        inventory_unit_ids = inventory_units.pluck(:variant_id)
+        order.line_items.select { |li| inventory_unit_ids.include?(li.variant_id) }
+      else
+        order.line_items
+      end
+    end
+
     def manifest_unstock(item)
       stock_location.unstock item.variant, item.quantity, self
     end

From 03fc63ad14969869df6c0011d717143723ad442b Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Tue, 11 May 2021 02:52:04 +0100
Subject: [PATCH 13/59] Remove #order_update! from line item after_save
 callback

---
 app/models/spree/line_item.rb                          |  1 -
 app/models/spree/order_contents.rb                     |  1 +
 .../admin/bulk_line_items_controller_spec.rb           |  2 +-
 spec/models/spree/line_item_spec.rb                    |  1 -
 spec/services/order_syncer_spec.rb                     | 10 ++++++++--
 5 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/app/models/spree/line_item.rb b/app/models/spree/line_item.rb
index 25636291ed..38e92f0ccd 100644
--- a/app/models/spree/line_item.rb
+++ b/app/models/spree/line_item.rb
@@ -230,7 +230,6 @@ module Spree
 
       # update the order totals, etc.
       order.create_tax_charge!
-      order.update_order!
     end
 
     def update_inventory_before_destroy
diff --git a/app/models/spree/order_contents.rb b/app/models/spree/order_contents.rb
index 4ee92ca906..dc28749e14 100644
--- a/app/models/spree/order_contents.rb
+++ b/app/models/spree/order_contents.rb
@@ -76,6 +76,7 @@ module Spree
     end
 
     def update_order
+      order.update_order!
       order.reload
     end
   end
diff --git a/spec/controllers/admin/bulk_line_items_controller_spec.rb b/spec/controllers/admin/bulk_line_items_controller_spec.rb
index 966722bdc7..2351175485 100644
--- a/spec/controllers/admin/bulk_line_items_controller_spec.rb
+++ b/spec/controllers/admin/bulk_line_items_controller_spec.rb
@@ -212,7 +212,7 @@ describe Admin::BulkLineItemsController, type: :controller do
             expect(line_item1.order).to receive(:reload).with(lock: true)
             expect(line_item1.order).to receive(:update_line_item_fees!)
             expect(line_item1.order).to receive(:update_order_fees!)
-            expect(line_item1.order).to receive(:update_order!).twice
+            expect(line_item1.order).to receive(:update_order!)
 
             spree_put :update, params
           end
diff --git a/spec/models/spree/line_item_spec.rb b/spec/models/spree/line_item_spec.rb
index a98685bab4..c740d4b018 100644
--- a/spec/models/spree/line_item_spec.rb
+++ b/spec/models/spree/line_item_spec.rb
@@ -11,7 +11,6 @@ module Spree
       it 'should update inventory, totals, and tax' do
         # Regression check for Spree #1481
         expect(line_item.order).to receive(:create_tax_charge!)
-        expect(line_item.order).to receive(:update_order!)
         line_item.quantity = 2
         line_item.save
       end
diff --git a/spec/services/order_syncer_spec.rb b/spec/services/order_syncer_spec.rb
index 93c91448e4..f2158aaff0 100644
--- a/spec/services/order_syncer_spec.rb
+++ b/spec/services/order_syncer_spec.rb
@@ -443,7 +443,10 @@ describe OrderSyncer do
       before { variant.update_attribute(:on_hand, 3) }
 
       context "when the changed line_item quantity matches the new quantity on the subscription line item" do
-        before { changed_line_item.update(quantity: 3) }
+        before do
+          changed_line_item.update(quantity: 3)
+          order.update_order!
+        end
 
         it "does not change the quantity, and doesn't add the order to order_update_issues" do
           expect(order.reload.total.to_f).to eq 99.95
@@ -456,7 +459,10 @@ describe OrderSyncer do
       end
 
       context "when the changed line_item quantity doesn't match the new quantity on the subscription line item" do
-        before { changed_line_item.update(quantity: 2) }
+        before do
+          changed_line_item.update(quantity: 2)
+          order.update_order!
+        end
 
         it "does not change the quantity, and adds the order to order_update_issues" do
           expect(order.reload.total.to_f).to eq 79.96

From c55e47f6afc926641aa8cc18d3d8c49ec3d74cc4 Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Tue, 11 May 2021 03:02:00 +0100
Subject: [PATCH 14/59] Remove superfluous updates in OrderFeesHandler

---
 app/services/order_fees_handler.rb | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/app/services/order_fees_handler.rb b/app/services/order_fees_handler.rb
index 14e7300a0b..6aa71dd9b9 100644
--- a/app/services/order_fees_handler.rb
+++ b/app/services/order_fees_handler.rb
@@ -19,9 +19,6 @@ class OrderFeesHandler
 
       create_line_item_fees!
       create_order_fees!
-
-      order.updater.update_totals
-      order.updater.persist_totals
     end
 
     order.update_order!

From 1268cb565c252dd68575546ba576a5ccbbfd18b4 Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Tue, 11 May 2021 10:34:06 +0100
Subject: [PATCH 15/59] Extract method

---
 app/models/spree/order_contents.rb | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/app/models/spree/order_contents.rb b/app/models/spree/order_contents.rb
index dc28749e14..2e88c2efe9 100644
--- a/app/models/spree/order_contents.rb
+++ b/app/models/spree/order_contents.rb
@@ -33,7 +33,7 @@ module Spree
 
     def update_cart(params)
       if order.update_attributes(params)
-        order.line_items = order.line_items.select {|li| li.quantity > 0 }
+        discard_empty_line_items
         order.ensure_updated_shipments
         update_order
         true
@@ -44,6 +44,10 @@ module Spree
 
     private
 
+    def discard_empty_line_items
+      order.line_items = order.line_items.select {|li| li.quantity.positive? }
+    end
+
     def update_shipment(shipment)
       shipment.present? ? shipment.update_amounts : order.ensure_updated_shipments
     end

From 632f4228f0f97ecf32dc88dd78078284ab04fff2 Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Tue, 11 May 2021 11:02:22 +0100
Subject: [PATCH 16/59] Bring in line item fetching refactor

---
 app/models/spree/order_contents.rb | 31 +++++++++++++++++++-----------
 1 file changed, 20 insertions(+), 11 deletions(-)

diff --git a/app/models/spree/order_contents.rb b/app/models/spree/order_contents.rb
index 2e88c2efe9..1bafe0a82d 100644
--- a/app/models/spree/order_contents.rb
+++ b/app/models/spree/order_contents.rb
@@ -11,24 +11,19 @@ module Spree
     # Get current line item for variant if exists
     # Add variant qty to line_item
     def add(variant, quantity = 1, shipment = nil)
-      line_item = order.find_line_item_by_variant(variant)
-      add_to_line_item(line_item, variant, quantity, shipment)
+      line_item = add_to_line_item(variant, quantity, shipment)
       update_shipment(shipment)
       update_order
+      line_item
     end
 
     # Get current line item for variant
     # Remove variant qty from line_item
     def remove(variant, quantity = 1, shipment = nil)
-      line_item = order.find_line_item_by_variant(variant)
-
-      unless line_item
-        raise ActiveRecord::RecordNotFound, "Line item not found for variant #{variant.sku}"
-      end
-
-      remove_from_line_item(line_item, variant, quantity, shipment)
+      line_item = remove_from_line_item(variant, quantity, shipment)
       update_shipment(shipment)
       update_order
+      line_item
     end
 
     def update_cart(params)
@@ -52,7 +47,9 @@ module Spree
       shipment.present? ? shipment.update_amounts : order.ensure_updated_shipments
     end
 
-    def add_to_line_item(line_item, variant, quantity, shipment = nil)
+    def add_to_line_item(variant, quantity, shipment = nil)
+      line_item = find_line_item_by_variant(variant)
+
       if line_item
         line_item.target_shipment = shipment
         line_item.quantity += quantity.to_i
@@ -66,7 +63,9 @@ module Spree
       line_item
     end
 
-    def remove_from_line_item(line_item, _variant, quantity, shipment = nil)
+    def remove_from_line_item(variant, quantity, shipment = nil)
+      line_item = find_line_item_by_variant(variant, true)
+
       line_item.quantity += -quantity
       line_item.target_shipment = shipment
 
@@ -79,6 +78,16 @@ module Spree
       line_item
     end
 
+    def find_line_item_by_variant(variant, raise_error = false)
+      line_item = order.find_line_item_by_variant(variant)
+
+      if !line_item.present? && raise_error
+        raise ActiveRecord::RecordNotFound, "Line item not found for variant #{variant.sku}"
+      end
+
+      line_item
+    end
+
     def update_order
       order.update_order!
       order.reload

From 23910dbab418f9a8ea53ea2ad2baef679e6b6d19 Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Tue, 11 May 2021 11:28:55 +0100
Subject: [PATCH 17/59] Use OrderContents in BulkLineItemsController

We should move towards *all* operations on an order's line items being done exclusively through this service.
---
 app/controllers/admin/bulk_line_items_controller.rb       | 2 +-
 spec/controllers/admin/bulk_line_items_controller_spec.rb | 3 ---
 2 files changed, 1 insertion(+), 4 deletions(-)

diff --git a/app/controllers/admin/bulk_line_items_controller.rb b/app/controllers/admin/bulk_line_items_controller.rb
index b9e95c6e49..526c521f99 100644
--- a/app/controllers/admin/bulk_line_items_controller.rb
+++ b/app/controllers/admin/bulk_line_items_controller.rb
@@ -49,7 +49,7 @@ module Admin
       load_line_item
       authorize! :update, order
 
-      @line_item.destroy
+      order.contents.remove(@line_item.variant, @line_item.quantity)
       render body: nil, status: :no_content # No Content, does not trigger ng resource auto-update
     end
 
diff --git a/spec/controllers/admin/bulk_line_items_controller_spec.rb b/spec/controllers/admin/bulk_line_items_controller_spec.rb
index 2351175485..f376650554 100644
--- a/spec/controllers/admin/bulk_line_items_controller_spec.rb
+++ b/spec/controllers/admin/bulk_line_items_controller_spec.rb
@@ -378,9 +378,6 @@ describe Admin::BulkLineItemsController, type: :controller do
         expect(order.included_tax_total).to eq 1.22
         expect(order.payment_state).to eq "paid"
 
-        expect(order).to receive(:update_order!).at_least(:once).and_call_original
-        expect(order).to receive(:create_tax_charge!).at_least(:once).and_call_original
-
         spree_delete :destroy, params
         order.reload
 

From 305ac037fe35b3de9afc8009c35fcfe342e793d8 Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Tue, 11 May 2021 11:17:26 +0100
Subject: [PATCH 18/59] Use OrderContents in customers controller spec

---
 spec/controllers/admin/customers_controller_spec.rb | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/spec/controllers/admin/customers_controller_spec.rb b/spec/controllers/admin/customers_controller_spec.rb
index a63c023651..5afdcd6d50 100644
--- a/spec/controllers/admin/customers_controller_spec.rb
+++ b/spec/controllers/admin/customers_controller_spec.rb
@@ -70,6 +70,7 @@ module Admin
               let!(:line_item) { create(:line_item, order: order, price: 10.0) }
 
               it 'includes the customer balance in the response' do
+                order.update_order!
                 get :index, params: params
                 expect(json_response.first["balance"]).to eq("$-10.00")
               end
@@ -77,13 +78,16 @@ module Admin
 
             context 'when the customer has canceled orders' do
               let(:order) { create(:order, customer: customer) }
-              let!(:line_item) { create(:line_item, order: order, price: 10.0) }
-              let!(:payment) { create(:payment, order: order, amount: order.total) }
+              let!(:variant) { create(:variant, price: 10.0) }
 
               before do
                 allow_any_instance_of(Spree::Payment).to receive(:completed?).and_return(true)
-                order.process_payments!
 
+                order.contents.add(variant)
+                order.payments << create(:payment, order: order, amount: order.total)
+                order.reload
+
+                order.process_payments!
                 order.update_attribute(:state, 'canceled')
               end
 

From d5edf03042f24fbf928c9f719ba7fb64964e554d Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Tue, 11 May 2021 11:54:58 +0100
Subject: [PATCH 19/59] Update reports_spec test setup

---
 spec/features/admin/reports_spec.rb | 1 +
 1 file changed, 1 insertion(+)

diff --git a/spec/features/admin/reports_spec.rb b/spec/features/admin/reports_spec.rb
index 758d7e6d68..7073da51be 100644
--- a/spec/features/admin/reports_spec.rb
+++ b/spec/features/admin/reports_spec.rb
@@ -414,6 +414,7 @@ feature '
       let!(:adj_manual2) { create(:adjustment, order: order1, adjustable: order1, originator: nil, label: "Manual adjustment", amount: 40, included_tax: 3) }
 
       before do
+        order1.update_order!
         order1.update_attribute :email, 'customer@email.com'
         order1.shipment.update_columns(included_tax_total: 10.06)
         Timecop.travel(Time.zone.local(2015, 4, 25, 14, 0, 0)) { order1.finalize! }

From c1c69b00b438d6db3bd05de4130c312bdfbd7ed0 Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Tue, 11 May 2021 11:58:44 +0100
Subject: [PATCH 20/59] Update order explicitly in OrderSyncer

---
 app/services/order_syncer.rb | 1 +
 1 file changed, 1 insertion(+)

diff --git a/app/services/order_syncer.rb b/app/services/order_syncer.rb
index 98e4fc9a53..343e73c665 100644
--- a/app/services/order_syncer.rb
+++ b/app/services/order_syncer.rb
@@ -15,6 +15,7 @@ class OrderSyncer
                               distributor_id: shop_id)
       update_associations_for(order)
       line_item_syncer.sync!(order)
+      order.update_order!
       order.save
     end
   end

From 000e2c07f8c5ebff720d521630afd4eb3c4a9335 Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Tue, 11 May 2021 12:17:06 +0100
Subject: [PATCH 21/59] Use OrderContents in OrdersController

---
 app/controllers/spree/orders_controller.rb | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/app/controllers/spree/orders_controller.rb b/app/controllers/spree/orders_controller.rb
index 4343ff87cb..c380d016c5 100644
--- a/app/controllers/spree/orders_controller.rb
+++ b/app/controllers/spree/orders_controller.rb
@@ -66,10 +66,7 @@ module Spree
 
       # This action is called either from the cart page when the order is not yet complete, or from
       # the edit order page (frontoffice) if the hub allows users to update completed orders.
-      # This line item updating logic should probably be handled through CartService#populate.
-      if @order.update(order_params)
-        discard_empty_line_items
-
+      if @order.contents.update_cart(order_params)
         @order.recreate_all_fees! # Enterprise fees on line items and on the order itself
 
         if @order.complete?

From e8ef183623c9c43d83a42d2528ac510272a66d71 Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Tue, 11 May 2021 12:34:49 +0100
Subject: [PATCH 22/59] Delete dead code Order#set_variant_attributes

---
 app/models/spree/order.rb | 13 -------------
 1 file changed, 13 deletions(-)

diff --git a/app/models/spree/order.rb b/app/models/spree/order.rb
index 75cdf9a2c4..b643c22b6f 100644
--- a/app/models/spree/order.rb
+++ b/app/models/spree/order.rb
@@ -287,19 +287,6 @@ module Spree
       current_item
     end
 
-    def set_variant_attributes(variant, attributes)
-      line_item = find_line_item_by_variant(variant)
-
-      return unless line_item
-
-      if attributes.key?(:max_quantity) && attributes[:max_quantity].to_i < line_item.quantity
-        attributes[:max_quantity] = line_item.quantity
-      end
-
-      line_item.assign_attributes(attributes)
-      line_item.save!
-    end
-
     # Associates the specified user with the order.
     def associate_user!(user)
       self.user = user

From 423c14067064d01c4cd2b4d6a71e1711d1d78318 Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Tue, 11 May 2021 12:41:53 +0100
Subject: [PATCH 23/59] Remove pointless currency code in Order and CartService

The currency will always be the currency of the variant, there's no need to pass this value around.
---
 app/models/spree/order.rb    | 11 ++---------
 app/services/cart_service.rb |  5 ++---
 2 files changed, 4 insertions(+), 12 deletions(-)

diff --git a/app/models/spree/order.rb b/app/models/spree/order.rb
index b643c22b6f..890af7a28a 100644
--- a/app/models/spree/order.rb
+++ b/app/models/spree/order.rb
@@ -253,7 +253,7 @@ module Spree
     # This add_variant is equivalent but slightly different from Spree::OrderContents#add above.
     # Spree::OrderContents#add is the more modern version in Spree history
     #   but this add_variant has been customized for OFN FrontOffice.
-    def add_variant(variant, quantity = 1, max_quantity = nil, currency = nil)
+    def add_variant(variant, quantity = 1, max_quantity = nil)
       line_items.reload
       current_item = find_line_item_by_variant(variant)
 
@@ -268,18 +268,11 @@ module Spree
       if current_item
         current_item.quantity = quantity
         current_item.max_quantity = max_quantity
-
-        current_item.currency = currency unless currency.nil?
         current_item.save
       else
         current_item = Spree::LineItem.new(quantity: quantity, max_quantity: max_quantity)
         current_item.variant = variant
-        if currency
-          current_item.currency = currency unless currency.nil?
-          current_item.price    = variant.price_in(currency).amount
-        else
-          current_item.price    = variant.price
-        end
+        current_item.price = variant.price
         line_items << current_item
       end
 
diff --git a/app/services/cart_service.rb b/app/services/cart_service.rb
index c69189e91e..d1a07ba695 100644
--- a/app/services/cart_service.rb
+++ b/app/services/cart_service.rb
@@ -3,13 +3,12 @@ require 'open_food_network/scope_variant_to_hub'
 # Previously Spree::OrderPopulator. Modified to work with max_quantity and variant overrides.
 
 class CartService
-  attr_accessor :order, :currency
+  attr_accessor :order
   attr_reader :variants_h
   attr_reader :errors
 
   def initialize(order)
     @order = order
-    @currency = order.currency
     @errors = ActiveModel::Errors.new(self)
   end
 
@@ -76,7 +75,7 @@ class CartService
   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)
+      @order.add_variant(variant, quantity_to_add, max_quantity_to_add)
     else
       @order.remove_variant variant
     end

From a5ccaf9595627e650efa557cdf7b9b092f63aef2 Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Tue, 11 May 2021 12:59:08 +0100
Subject: [PATCH 24/59] Remove unnecessary code in BulkLineItemsController

This method isn't called from anywhere else
---
 app/controllers/admin/bulk_line_items_controller.rb | 9 +--------
 1 file changed, 1 insertion(+), 8 deletions(-)

diff --git a/app/controllers/admin/bulk_line_items_controller.rb b/app/controllers/admin/bulk_line_items_controller.rb
index 526c521f99..feedc4aa60 100644
--- a/app/controllers/admin/bulk_line_items_controller.rb
+++ b/app/controllers/admin/bulk_line_items_controller.rb
@@ -63,16 +63,9 @@ module Admin
       Spree::LineItem
     end
 
-    # Returns the appropriate serializer for this controller
-    #
-    # @return [Api::Admin::LineItemSerializer]
-    def serializer(_ams_prefix)
-      Api::Admin::LineItemSerializer
-    end
-
     def serialized_line_items
       ActiveModel::ArraySerializer.new(
-        @line_items, each_serializer: serializer(nil)
+        @line_items, each_serializer: Api::Admin::LineItemSerializer
       )
     end
 

From 5ff8436c1a46ee48a0b852dd038eba7ad02552f0 Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Tue, 11 May 2021 13:29:49 +0100
Subject: [PATCH 25/59] Adapt OrderContents#remove to allow deleting line item
 without passing a quantity

---
 app/controllers/admin/bulk_line_items_controller.rb | 2 +-
 app/models/spree/order_contents.rb                  | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/app/controllers/admin/bulk_line_items_controller.rb b/app/controllers/admin/bulk_line_items_controller.rb
index feedc4aa60..837ad47535 100644
--- a/app/controllers/admin/bulk_line_items_controller.rb
+++ b/app/controllers/admin/bulk_line_items_controller.rb
@@ -49,7 +49,7 @@ module Admin
       load_line_item
       authorize! :update, order
 
-      order.contents.remove(@line_item.variant, @line_item.quantity)
+      order.contents.remove(@line_item.variant)
       render body: nil, status: :no_content # No Content, does not trigger ng resource auto-update
     end
 
diff --git a/app/models/spree/order_contents.rb b/app/models/spree/order_contents.rb
index 1bafe0a82d..53ad53fde6 100644
--- a/app/models/spree/order_contents.rb
+++ b/app/models/spree/order_contents.rb
@@ -19,7 +19,7 @@ module Spree
 
     # Get current line item for variant
     # Remove variant qty from line_item
-    def remove(variant, quantity = 1, shipment = nil)
+    def remove(variant, quantity = nil, shipment = nil)
       line_item = remove_from_line_item(variant, quantity, shipment)
       update_shipment(shipment)
       update_order
@@ -66,7 +66,7 @@ module Spree
     def remove_from_line_item(variant, quantity, shipment = nil)
       line_item = find_line_item_by_variant(variant, true)
 
-      line_item.quantity += -quantity
+      quantity.present? ? line_item.quantity += -quantity : line_item.quantity = 0
       line_item.target_shipment = shipment
 
       if line_item.quantity == 0

From 9045cc7d65696a80778aae5a496a9447080590de Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Tue, 11 May 2021 13:32:43 +0100
Subject: [PATCH 26/59] Use OrderContents in CartService for removing line
 items from cart

---
 app/models/spree/order.rb    | 6 ------
 app/services/cart_service.rb | 4 ++--
 2 files changed, 2 insertions(+), 8 deletions(-)

diff --git a/app/models/spree/order.rb b/app/models/spree/order.rb
index 890af7a28a..f9f14842c3 100644
--- a/app/models/spree/order.rb
+++ b/app/models/spree/order.rb
@@ -575,12 +575,6 @@ module Spree
       save!
     end
 
-    def remove_variant(variant)
-      line_items.reload
-      current_item = find_line_item_by_variant(variant)
-      current_item.andand.destroy
-    end
-
     def cap_quantity_at_stock!
       line_items.includes(variant: :stock_items).find_each(&:cap_quantity_at_stock!)
     end
diff --git a/app/services/cart_service.rb b/app/services/cart_service.rb
index d1a07ba695..029541f740 100644
--- a/app/services/cart_service.rb
+++ b/app/services/cart_service.rb
@@ -77,7 +77,7 @@ class CartService
     if quantity_to_add > 0
       @order.add_variant(variant, quantity_to_add, max_quantity_to_add)
     else
-      @order.remove_variant variant
+      @order.contents.remove(variant)
     end
   end
 
@@ -119,7 +119,7 @@ class CartService
 
   def cart_remove(variant_id)
     variant = Spree::Variant.find(variant_id)
-    @order.remove_variant(variant)
+    @order.contents.remove(variant)
   end
 
   def distributor_and_order_cycle

From 7af5e8931f28a8161089f74ec1b96864259a119b Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Tue, 11 May 2021 13:37:26 +0100
Subject: [PATCH 27/59] Adapt OrderContents for use with
 BulkLineItemsController#update

---
 app/controllers/admin/bulk_line_items_controller.rb |  2 +-
 app/models/spree/order_contents.rb                  | 11 +++++++++++
 2 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/app/controllers/admin/bulk_line_items_controller.rb b/app/controllers/admin/bulk_line_items_controller.rb
index 837ad47535..a4ff5b1676 100644
--- a/app/controllers/admin/bulk_line_items_controller.rb
+++ b/app/controllers/admin/bulk_line_items_controller.rb
@@ -32,7 +32,7 @@ module Admin
       # See https://github.com/rails/rails/blob/3-2-stable/activerecord/lib/active_record/locking/pessimistic.rb#L69
       # and https://www.postgresql.org/docs/current/static/sql-select.html#SQL-FOR-UPDATE-SHARE
       order.with_lock do
-        if @line_item.update(line_item_params)
+        if order.contents.update_item(@line_item, line_item_params)
           order.update_line_item_fees! @line_item
           order.update_order_fees!
           order.update_order!
diff --git a/app/models/spree/order_contents.rb b/app/models/spree/order_contents.rb
index 53ad53fde6..c50ac0cd2e 100644
--- a/app/models/spree/order_contents.rb
+++ b/app/models/spree/order_contents.rb
@@ -37,6 +37,17 @@ module Spree
       end
     end
 
+    def update_item(line_item, params)
+      if line_item.update_attributes(params)
+        discard_empty_line_items
+        order.ensure_updated_shipments
+        update_order
+        true
+      else
+        false
+      end
+    end
+
     private
 
     def discard_empty_line_items

From 6451ef173c5b9e9ca73f6d57090bd784e0c6b09c Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Tue, 11 May 2021 13:39:53 +0100
Subject: [PATCH 28/59] Use OrderContents when removing line items with
 soft-deleted variants in CartService

---
 app/services/cart_service.rb | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/app/services/cart_service.rb b/app/services/cart_service.rb
index 029541f740..cc752542df 100644
--- a/app/services/cart_service.rb
+++ b/app/services/cart_service.rb
@@ -37,7 +37,7 @@ class CartService
       loaded_variant = loaded_variants[variant_data[:variant_id].to_i]
 
       if loaded_variant.deleted?
-        remove_deleted_variant(loaded_variant)
+        order.contents.remove(loaded_variant)
         next
       end
 
@@ -57,10 +57,6 @@ class CartService
     end
   end
 
-  def remove_deleted_variant(variant)
-    line_item_for_variant(variant).andand.destroy
-  end
-
   def attempt_cart_add(variant, quantity, max_quantity = nil)
     quantity = quantity.to_i
     max_quantity = max_quantity.to_i if max_quantity

From 6e8fd3c3f851834ac3b8867af9ac5e32487c5cd7 Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Tue, 11 May 2021 13:43:33 +0100
Subject: [PATCH 29/59] Simplify CartService interface

There's only one place this method is called, and the "overwrite" argument is always explicitly true. It doesn't need to be there if it's mandatory.
---
 app/controllers/cart_controller.rb       |  2 +-
 app/services/cart_service.rb             |  4 ++--
 spec/controllers/cart_controller_spec.rb |  7 ------
 spec/services/cart_service_spec.rb       | 27 ++++++++++++++++--------
 4 files changed, 21 insertions(+), 19 deletions(-)

diff --git a/app/controllers/cart_controller.rb b/app/controllers/cart_controller.rb
index 2555ed6f34..7afe8228f4 100644
--- a/app/controllers/cart_controller.rb
+++ b/app/controllers/cart_controller.rb
@@ -6,7 +6,7 @@ class CartController < BaseController
 
     cart_service = CartService.new(order)
 
-    cart_service.populate(params.slice(:variants, :quantity), true)
+    cart_service.populate(params.slice(:variants, :quantity))
     if cart_service.valid?
       order.cap_quantity_at_stock!
       order.recreate_all_fees!
diff --git a/app/services/cart_service.rb b/app/services/cart_service.rb
index cc752542df..cd859fbe62 100644
--- a/app/services/cart_service.rb
+++ b/app/services/cart_service.rb
@@ -12,14 +12,14 @@ class CartService
     @errors = ActiveModel::Errors.new(self)
   end
 
-  def populate(from_hash, overwrite = false)
+  def populate(from_hash)
     @distributor, @order_cycle = distributor_and_order_cycle
 
     variants_data = read_variants from_hash
 
     @order.with_lock do
       attempt_cart_add_variants variants_data
-      overwrite_variants variants_data if overwrite
+      overwrite_variants variants_data
     end
     valid?
   end
diff --git a/spec/controllers/cart_controller_spec.rb b/spec/controllers/cart_controller_spec.rb
index 95bbd23927..0285d99f5b 100644
--- a/spec/controllers/cart_controller_spec.rb
+++ b/spec/controllers/cart_controller_spec.rb
@@ -30,13 +30,6 @@ describe CartController, type: :controller do
       expect(response.status).to eq(412)
     end
 
-    it "tells cart_service to overwrite" do
-      allow(cart_service).to receive(:variants_h) { {} }
-      allow(cart_service).to receive(:valid?) { true }
-      expect(cart_service).to receive(:populate).with({}, true)
-      post :populate, xhr: true, params: { use_route: :spree }, as: :json
-    end
-
     it "returns stock levels as JSON on success" do
       allow(controller).to receive(:variant_ids_in) { [123] }
       allow_any_instance_of(VariantsStockLevels).to receive(:call).and_return("my_stock_levels")
diff --git a/spec/services/cart_service_spec.rb b/spec/services/cart_service_spec.rb
index 51e8f2b882..9a4c900e68 100644
--- a/spec/services/cart_service_spec.rb
+++ b/spec/services/cart_service_spec.rb
@@ -27,9 +27,9 @@ describe CartService do
     describe "#populate" do
       it "adds a variant" do
         cart_service.populate(
-          ActionController::Parameters.new({ variants: { variant.id.to_s => { quantity: '1',
-                                                                              max_quantity: '2' } } }),
-          true
+          ActionController::Parameters.new(
+            { variants: { variant.id.to_s => { quantity: '1', max_quantity: '2' } } }
+          )
         )
         li = order.find_line_item_by_variant(variant)
         expect(li).to be
@@ -42,10 +42,11 @@ describe CartService do
         order.add_variant variant, 1, 2
 
         cart_service.populate(
-          ActionController::Parameters.new({ variants: { variant.id.to_s => { quantity: '2',
-                                                                              max_quantity: '3' } } }),
-          true
+          ActionController::Parameters.new(
+            { variants: { variant.id.to_s => { quantity: '2', max_quantity: '3' } } }
+          )
         )
+
         li = order.find_line_item_by_variant(variant)
         expect(li).to be
         expect(li.quantity).to eq(2)
@@ -56,7 +57,7 @@ describe CartService do
       it "removes a variant" do
         order.add_variant variant, 1, 2
 
-        cart_service.populate(ActionController::Parameters.new({ variants: {} }), true)
+        cart_service.populate(ActionController::Parameters.new({ variants: {} }))
         order.line_items.reload
         li = order.find_line_item_by_variant(variant)
         expect(li).not_to be
@@ -69,7 +70,11 @@ describe CartService do
           it "does not add the deleted variant to the cart" do
             variant.delete
 
-            cart_service.populate(ActionController::Parameters.new({ variants: { variant.id.to_s => { quantity: '2' } } }), true)
+            cart_service.populate(
+              ActionController::Parameters.new(
+                { variants: { variant.id.to_s => { quantity: '2' } } }
+              )
+            )
 
             expect(relevant_line_item).to be_nil
             expect(cart_service.errors.count).to be 0
@@ -84,7 +89,11 @@ describe CartService do
           it "removes the line_item from the cart" do
             variant.delete
 
-            cart_service.populate(ActionController::Parameters.new({ variants: { variant.id.to_s => { quantity: '3' } } }), true)
+            cart_service.populate(
+              ActionController::Parameters.new(
+                { variants: { variant.id.to_s => { quantity: '3' } } }
+              )
+            )
 
             expect(Spree::LineItem.where(id: relevant_line_item).first).to be_nil
             expect(cart_service.errors.count).to be 0

From cb922318353b0380650deae2712ddab3c79af6a1 Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Tue, 11 May 2021 13:50:10 +0100
Subject: [PATCH 30/59] Clarify behavior via variable names

This is a small semantic thing to clarify that we're not adding to / incrementing the quantity here, we're setting it to the given value. In other places where we change line items on an order we are actually adding the given value to the existing quantity.
---
 app/services/cart_service.rb | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/app/services/cart_service.rb b/app/services/cart_service.rb
index cd859fbe62..77d3bea20e 100644
--- a/app/services/cart_service.rb
+++ b/app/services/cart_service.rb
@@ -69,22 +69,22 @@ class CartService
   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)
+    final_quantity, final_max_quantity = final_quantities(variant, quantity, max_quantity)
+    if final_quantity > 0
+      @order.add_variant(variant, final_quantity, final_max_quantity)
     else
       @order.contents.remove(variant)
     end
   end
 
-  def quantities_to_add(variant, quantity, max_quantity)
+  def final_quantities(variant, quantity, max_quantity)
     # If not enough stock is available, add as much as we can to the cart
     on_hand = variant.on_hand
     on_hand = [quantity, max_quantity].compact.max if variant.on_demand
-    quantity_to_add = [quantity, on_hand].min
-    max_quantity_to_add = max_quantity # max_quantity is not capped
+    final_quantity = [quantity, on_hand].min
+    final_max_quantity = max_quantity # max_quantity is not capped
 
-    [quantity_to_add, max_quantity_to_add]
+    [final_quantity, final_max_quantity]
   end
 
   def overwrite_variants(variants)

From 0178cd753035bf35fa02d40820fc982fb02b5c70 Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Tue, 11 May 2021 14:51:33 +0100
Subject: [PATCH 31/59] Adapt OrderContents to allow use with frontend cart
 service and updating :max_quantity

The cart service and it's surrounding logic is a bit messy, as it jumps through hoops to add extra logic for the :max_quantity attribute used with the "group buy" feature.

This is the last bit of code where an order's line items could be changed where we were not using OrderContents.
---
 app/models/spree/order.rb          | 34 +-----------------------------
 app/models/spree/order_contents.rb | 16 ++++++++++++++
 app/services/cart_service.rb       |  8 +++----
 3 files changed, 21 insertions(+), 37 deletions(-)

diff --git a/app/models/spree/order.rb b/app/models/spree/order.rb
index f9f14842c3..94c950abca 100644
--- a/app/models/spree/order.rb
+++ b/app/models/spree/order.rb
@@ -243,43 +243,11 @@ module Spree
       return_authorizations.any?(&:authorized?)
     end
 
-    # This is currently used when adding a variant to an order in the BackOffice.
-    # Spree::OrderContents#add is equivalent but slightly different from add_variant below.
+    # OrderContents should always be used when modifying an order's line items
     def contents
       @contents ||= Spree::OrderContents.new(self)
     end
 
-    # This is currently used when adding a variant to an order in the FrontOffice.
-    # This add_variant is equivalent but slightly different from Spree::OrderContents#add above.
-    # Spree::OrderContents#add is the more modern version in Spree history
-    #   but this add_variant has been customized for OFN FrontOffice.
-    def add_variant(variant, quantity = 1, max_quantity = nil)
-      line_items.reload
-      current_item = find_line_item_by_variant(variant)
-
-      # Notify bugsnag if we get line items with a quantity of zero
-      if quantity == 0
-        Bugsnag.notify(RuntimeError.new("Zero Quantity Line Item"),
-                       current_item: current_item.as_json,
-                       line_items: line_items.map(&:id),
-                       variant: variant.as_json)
-      end
-
-      if current_item
-        current_item.quantity = quantity
-        current_item.max_quantity = max_quantity
-        current_item.save
-      else
-        current_item = Spree::LineItem.new(quantity: quantity, max_quantity: max_quantity)
-        current_item.variant = variant
-        current_item.price = variant.price
-        line_items << current_item
-      end
-
-      reload
-      current_item
-    end
-
     # Associates the specified user with the order.
     def associate_user!(user)
       self.user = user
diff --git a/app/models/spree/order_contents.rb b/app/models/spree/order_contents.rb
index c50ac0cd2e..832c1bafd3 100644
--- a/app/models/spree/order_contents.rb
+++ b/app/models/spree/order_contents.rb
@@ -26,6 +26,22 @@ module Spree
       line_item
     end
 
+    def update_or_create(variant, attributes)
+      line_item = find_line_item_by_variant(variant)
+
+      if line_item
+        line_item.update(attributes)
+      else
+        line_item = Spree::LineItem.new(attributes)
+        line_item.variant = variant
+        line_item.price = variant.price
+        order.line_items << line_item
+      end
+
+      order.reload
+      line_item
+    end
+
     def update_cart(params)
       if order.update_attributes(params)
         discard_empty_line_items
diff --git a/app/services/cart_service.rb b/app/services/cart_service.rb
index 77d3bea20e..589a57f22b 100644
--- a/app/services/cart_service.rb
+++ b/app/services/cart_service.rb
@@ -69,9 +69,9 @@ class CartService
   end
 
   def cart_add(variant, quantity, max_quantity)
-    final_quantity, final_max_quantity = final_quantities(variant, quantity, max_quantity)
-    if final_quantity > 0
-      @order.add_variant(variant, final_quantity, final_max_quantity)
+    attributes = final_quantities(variant, quantity, max_quantity)
+    if attributes[:quantity] > 0
+      @order.contents.update_or_create(variant, attributes)
     else
       @order.contents.remove(variant)
     end
@@ -84,7 +84,7 @@ class CartService
     final_quantity = [quantity, on_hand].min
     final_max_quantity = max_quantity # max_quantity is not capped
 
-    [final_quantity, final_max_quantity]
+    { quantity: final_quantity, max_quantity: final_max_quantity }
   end
 
   def overwrite_variants(variants)

From 67df6728f6c1053ebe1f47532005e9f7c74a65b5 Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Tue, 11 May 2021 15:07:28 +0100
Subject: [PATCH 32/59] Fix Order line items association definition

Populating the cart was throwing an "Association Mismatch" error D:
---
 app/models/spree/order.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/models/spree/order.rb b/app/models/spree/order.rb
index 94c950abca..c6a6ee80d7 100644
--- a/app/models/spree/order.rb
+++ b/app/models/spree/order.rb
@@ -35,7 +35,7 @@ module Spree
     alias_attribute :shipping_address, :ship_address
 
     has_many :state_changes, as: :stateful
-    has_many :line_items, -> { order('created_at ASC') }, dependent: :destroy
+    has_many :line_items, -> { order('created_at ASC') }, class_name: "Spree::LineItem", dependent: :destroy
     has_many :payments, dependent: :destroy
     has_many :return_authorizations, dependent: :destroy, inverse_of: :order
     has_many :adjustments, -> { order "#{Spree::Adjustment.table_name}.created_at ASC" },

From 4a7c0e829720abf292126d95464b75d2ddfbe7f9 Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Tue, 11 May 2021 16:49:51 +0100
Subject: [PATCH 33/59] Remove variant early if we're deleting them and they're
 already loaded

---
 app/services/cart_service.rb | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/app/services/cart_service.rb b/app/services/cart_service.rb
index 589a57f22b..d8a123e545 100644
--- a/app/services/cart_service.rb
+++ b/app/services/cart_service.rb
@@ -36,7 +36,7 @@ class CartService
     variants_data.each do |variant_data|
       loaded_variant = loaded_variants[variant_data[:variant_id].to_i]
 
-      if loaded_variant.deleted?
+      if loaded_variant.deleted? || !variant_data[:quantity].to_i.positive?
         order.contents.remove(loaded_variant)
         next
       end
@@ -60,7 +60,6 @@ class CartService
   def attempt_cart_add(variant, quantity, max_quantity = nil)
     quantity = quantity.to_i
     max_quantity = max_quantity.to_i if max_quantity
-    return unless quantity > 0
 
     scoper.scope(variant)
     return unless valid_variant?(variant)
@@ -70,7 +69,8 @@ class CartService
 
   def cart_add(variant, quantity, max_quantity)
     attributes = final_quantities(variant, quantity, max_quantity)
-    if attributes[:quantity] > 0
+
+    if attributes[:quantity].positive?
       @order.contents.update_or_create(variant, attributes)
     else
       @order.contents.remove(variant)
@@ -88,8 +88,9 @@ class CartService
   end
 
   def overwrite_variants(variants)
-    variants_removed(variants).each do |id|
-      cart_remove(id)
+    variants_removed(variants).each do |variant_id|
+      variant = Spree::Variant.with_deleted.find(variant_id)
+      cart_remove(variant)
     end
   end
 

From 398d4b2612247217c6b66ec6b9cfe2d9dd79a38a Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Tue, 11 May 2021 16:52:15 +0100
Subject: [PATCH 34/59] Tidy up public interface of CartService

We don't need this extra hash to be passed around
---
 app/controllers/cart_controller.rb       | 6 +-----
 app/services/cart_service.rb             | 7 +------
 spec/controllers/cart_controller_spec.rb | 9 ---------
 3 files changed, 2 insertions(+), 20 deletions(-)

diff --git a/app/controllers/cart_controller.rb b/app/controllers/cart_controller.rb
index 7afe8228f4..edd9e69b6d 100644
--- a/app/controllers/cart_controller.rb
+++ b/app/controllers/cart_controller.rb
@@ -11,7 +11,7 @@ class CartController < BaseController
       order.cap_quantity_at_stock!
       order.recreate_all_fees!
 
-      variant_ids = variant_ids_in(cart_service.variants_h)
+      variant_ids = order.line_items.pluck(:variant_id)
 
       render json: { error: false,
                      stock_levels: VariantsStockLevels.new.call(order, variant_ids) },
@@ -24,10 +24,6 @@ class CartController < BaseController
 
   private
 
-  def variant_ids_in(variants_h)
-    variants_h.map { |v| v[:variant_id].to_i }
-  end
-
   def check_authorization
     session[:access_token] ||= params[:token]
     order = Spree::Order.find_by(number: params[:id]) || current_order
diff --git a/app/services/cart_service.rb b/app/services/cart_service.rb
index d8a123e545..4a6cdd92c6 100644
--- a/app/services/cart_service.rb
+++ b/app/services/cart_service.rb
@@ -4,7 +4,6 @@ require 'open_food_network/scope_variant_to_hub'
 
 class CartService
   attr_accessor :order
-  attr_reader :variants_h
   attr_reader :errors
 
   def initialize(order)
@@ -15,7 +14,7 @@ class CartService
   def populate(from_hash)
     @distributor, @order_cycle = distributor_and_order_cycle
 
-    variants_data = read_variants from_hash
+    variants_data = read_variants_hash(from_hash)
 
     @order.with_lock do
       attempt_cart_add_variants variants_data
@@ -98,10 +97,6 @@ class CartService
     @scoper ||= OpenFoodNetwork::ScopeVariantToHub.new(@distributor)
   end
 
-  def read_variants(data)
-    @variants_h = read_variants_hash(data)
-  end
-
   def read_variants_hash(data)
     variants_array = []
     (data[:variants] || []).each do |variant_id, quantity|
diff --git a/spec/controllers/cart_controller_spec.rb b/spec/controllers/cart_controller_spec.rb
index 0285d99f5b..86c78eacd5 100644
--- a/spec/controllers/cart_controller_spec.rb
+++ b/spec/controllers/cart_controller_spec.rb
@@ -16,7 +16,6 @@ describe CartController, type: :controller do
     it "returns HTTP success when successful" do
       allow(cart_service).to receive(:populate) { true }
       allow(cart_service).to receive(:valid?) { true }
-      allow(cart_service).to receive(:variants_h) { {} }
       post :populate, xhr: true, params: { use_route: :spree }, as: :json
       expect(response.status).to eq(200)
     end
@@ -35,20 +34,12 @@ describe CartController, type: :controller do
       allow_any_instance_of(VariantsStockLevels).to receive(:call).and_return("my_stock_levels")
       allow(cart_service).to receive(:populate) { true }
       allow(cart_service).to receive(:valid?) { true }
-      allow(cart_service).to receive(:variants_h) { {} }
 
       post :populate, xhr: true, params: { use_route: :spree }, as: :json
 
       data = JSON.parse(response.body)
       expect(data['stock_levels']).to eq('my_stock_levels')
     end
-
-    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 }]
-
-      expect(controller.__send__(:variant_ids_in, variants_h)).to eq([900, 940])
-    end
   end
 
   context "handling variant overrides correctly" do

From a883b2cf637ebf97a04342a1bd7ad66aea3dc42f Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Tue, 11 May 2021 18:08:59 +0100
Subject: [PATCH 35/59] Remove dead code CartService#cart_remove

---
 app/services/cart_service.rb | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/app/services/cart_service.rb b/app/services/cart_service.rb
index 4a6cdd92c6..30562c778a 100644
--- a/app/services/cart_service.rb
+++ b/app/services/cart_service.rb
@@ -109,11 +109,6 @@ class CartService
     variants_array
   end
 
-  def cart_remove(variant_id)
-    variant = Spree::Variant.find(variant_id)
-    @order.contents.remove(variant)
-  end
-
   def distributor_and_order_cycle
     [@order.distributor, @order.order_cycle]
   end

From f5c08baabbedfed1bb29ac6927e95fb66c9f4be9 Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Tue, 11 May 2021 18:15:08 +0100
Subject: [PATCH 36/59] Use OrderContents in LineItemsController and move
 enterprise fee updating logic

---
 app/controllers/api/v0/shipments_controller.rb | 1 -
 app/controllers/line_items_controller.rb       | 5 +----
 app/models/spree/order_contents.rb             | 1 +
 3 files changed, 2 insertions(+), 5 deletions(-)

diff --git a/app/controllers/api/v0/shipments_controller.rb b/app/controllers/api/v0/shipments_controller.rb
index c4ed320e17..cd0f31dcd3 100644
--- a/app/controllers/api/v0/shipments_controller.rb
+++ b/app/controllers/api/v0/shipments_controller.rb
@@ -81,7 +81,6 @@ module Api
         quantity = params[:quantity].to_i
 
         @order.contents.remove(variant, quantity, @shipment)
-        @order.recreate_all_fees!
         @shipment.reload if @shipment.persisted?
 
         render json: @shipment, serializer: Api::ShipmentSerializer, status: :ok
diff --git a/app/controllers/line_items_controller.rb b/app/controllers/line_items_controller.rb
index 4459249ba9..123d03510f 100644
--- a/app/controllers/line_items_controller.rb
+++ b/app/controllers/line_items_controller.rb
@@ -39,12 +39,9 @@ class LineItemsController < BaseController
   def destroy_with_lock(item)
     order = item.order
     order.with_lock do
-      item.destroy
+      order.contents.remove(item.variant)
       order.update_shipping_fees!
       order.update_payment_fees!
-      order.update_order_fees!
-      order.update_order!
-      order.create_tax_charge!
     end
   end
 end
diff --git a/app/models/spree/order_contents.rb b/app/models/spree/order_contents.rb
index 832c1bafd3..15a9030706 100644
--- a/app/models/spree/order_contents.rb
+++ b/app/models/spree/order_contents.rb
@@ -22,6 +22,7 @@ module Spree
     def remove(variant, quantity = nil, shipment = nil)
       line_item = remove_from_line_item(variant, quantity, shipment)
       update_shipment(shipment)
+      order.update_order_fees! if order.completed?
       update_order
       line_item
     end

From 79aebed40e2bc3a179b503a8f16acbf6f815f800 Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Wed, 12 May 2021 10:23:22 +0100
Subject: [PATCH 37/59] Bring OrderContents#update_cart tests from upstream and
 tidy up

---
 spec/models/spree/order_contents_spec.rb | 56 ++++++++++++++++++++----
 1 file changed, 47 insertions(+), 9 deletions(-)

diff --git a/spec/models/spree/order_contents_spec.rb b/spec/models/spree/order_contents_spec.rb
index 7a333521ee..e63318cf5b 100644
--- a/spec/models/spree/order_contents_spec.rb
+++ b/spec/models/spree/order_contents_spec.rb
@@ -3,12 +3,11 @@
 require 'spec_helper'
 
 describe Spree::OrderContents do
-  let(:order) { Spree::Order.create }
+  let!(:order) { create(:order) }
+  let!(:variant) { create(:variant) }
   subject { described_class.new(order) }
 
   context "#add" do
-    let(:variant) { create(:variant) }
-
     context 'given quantity is not explicitly provided' do
       it 'should add one line item' do
         line_item = subject.add(variant)
@@ -42,8 +41,6 @@ describe Spree::OrderContents do
   end
 
   context "#remove" do
-    let(:variant) { create(:variant) }
-
     context "given an invalid variant" do
       it "raises an exception" do
         expect {
@@ -53,11 +50,12 @@ describe Spree::OrderContents do
     end
 
     context 'given quantity is not explicitly provided' do
-      it 'should remove one line item' do
-        line_item = subject.add(variant, 3)
-        subject.remove(variant)
+      it 'should remove line item' do
+        subject.add(variant, 3)
 
-        expect(line_item.reload.quantity).to eq 2
+        expect{
+          subject.remove(variant)
+        }.to change(Spree::LineItem, :count).by -1
       end
     end
 
@@ -89,4 +87,44 @@ describe Spree::OrderContents do
       expect(order.total.to_f).to eq 19.99
     end
   end
+
+  context "#update_cart" do
+    let!(:line_item) { subject.add variant, 1 }
+
+    let(:params) do
+      { line_items_attributes: {
+        "0" => { id: line_item.id, quantity: 3 }
+      } }
+    end
+
+    it "changes item quantity" do
+      subject.update_cart params
+      expect(line_item.reload.quantity).to eq 3
+    end
+
+    it "updates order totals" do
+      expect {
+        subject.update_cart params
+      }.to change { subject.order.total }
+    end
+
+    context "submits item quantity 0" do
+      let(:params) do
+        { line_items_attributes: {
+          "0" => { id: line_item.id, quantity: 0 }
+        } }
+      end
+
+      it "removes item from order" do
+        expect {
+          subject.update_cart params
+        }.to change { order.line_items.count }
+      end
+    end
+
+    it "ensures updated shipments" do
+      expect(subject.order).to receive(:ensure_updated_shipments)
+      subject.update_cart params
+    end
+  end
 end

From b25e7fcb89b73014e536e7cfc62ed50abd33c207 Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Wed, 12 May 2021 10:40:56 +0100
Subject: [PATCH 38/59] Delete tests for removed Order #add_variant and
 #remove_variant methods

---
 spec/models/spree/order_spec.rb | 68 ---------------------------------
 1 file changed, 68 deletions(-)

diff --git a/spec/models/spree/order_spec.rb b/spec/models/spree/order_spec.rb
index 8bfd3c97ec..eda0c491c5 100644
--- a/spec/models/spree/order_spec.rb
+++ b/spec/models/spree/order_spec.rb
@@ -455,21 +455,6 @@ describe Spree::Order do
     end
   end
 
-  describe "setting variant attributes" do
-    it "sets attributes on line items for variants" do
-      d = create(:distributor_enterprise)
-      p = create(:product)
-
-      subject.distributor = d
-      subject.save!
-
-      subject.add_variant(p.master, 1, 3)
-
-      li = Spree::LineItem.last
-      expect(li.max_quantity).to eq(3)
-    end
-  end
-
   describe "applying enterprise fees" do
     subject { create(:order) }
     let(:fee_handler) { ::OrderFeesHandler.new(subject) }
@@ -722,59 +707,6 @@ 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) }
-    let(:v3)    { create(:variant) }
-
-    before do
-      order.add_variant v1
-      order.add_variant v2
-
-      order.recreate_all_fees!
-    end
-
-    it "removes the variant's line item" do
-      order.remove_variant v1
-      expect(order.line_items.reload.map(&:variant)).to eq([v2])
-    end
-
-    it "does nothing when there is no matching line item" do
-      expect do
-        order.remove_variant v3
-      end.to change(order.line_items.reload, :count).by(0)
-    end
-
-    context "when the item has an associated adjustment" do
-      let(:distributor) { create(:distributor_enterprise) }
-
-      let(:order_cycle) do
-        create(:order_cycle).tap do
-          create(:exchange, variants: [v1], incoming: true)
-          create(:exchange, variants: [v1], incoming: false, receiver: distributor)
-        end
-      end
-
-      let(:order) { create(:order, distributor: distributor, order_cycle: order_cycle) }
-
-      it "removes the variant's line item" do
-        order.remove_variant v1
-        expect(order.line_items.reload.map(&:variant)).to eq([v2])
-      end
-
-      it "removes the variant's adjustment" do
-        line_item = order.line_items.where(variant_id: v1.id).first
-        adjustment_scope = Spree::Adjustment.where(adjustable_type: "Spree::LineItem",
-                                                   adjustable_id: line_item.id)
-        expect(adjustment_scope.count).to eq(1)
-        adjustment = adjustment_scope.first
-        order.remove_variant v1
-        expect { adjustment.reload }.to raise_error
-      end
-    end
-  end
-
   describe "emptying the order" do
     it "removes shipments" do
       subject.shipments << create(:shipment)

From 42ff2307fa734e8c34a19563188228871e58fea7 Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Wed, 12 May 2021 10:41:36 +0100
Subject: [PATCH 39/59] Update specs that use removed #add_variant and
 #remove_variant methods in test setup

---
 spec/controllers/spree/orders_controller_spec.rb |  8 ++++----
 spec/models/spree/order_spec.rb                  | 12 +++++++++---
 spec/models/spree/payment_method_spec.rb         |  2 +-
 3 files changed, 14 insertions(+), 8 deletions(-)

diff --git a/spec/controllers/spree/orders_controller_spec.rb b/spec/controllers/spree/orders_controller_spec.rb
index 34c8b31a52..7934617294 100644
--- a/spec/controllers/spree/orders_controller_spec.rb
+++ b/spec/controllers/spree/orders_controller_spec.rb
@@ -250,7 +250,7 @@ describe Spree::OrdersController, type: :controller do
 
       before do
         order.set_distribution! d, oc
-        order.add_variant variant, 5
+        order.contents.add(variant, 5)
       end
 
       describe "the page" do
@@ -296,7 +296,7 @@ describe Spree::OrdersController, type: :controller 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
         order = subject.current_order(true)
-        li = order.add_variant(create(:simple_product, on_hand: 110).variants.first)
+        li = order.contents.add(create(:simple_product, on_hand: 110).variants.first)
         get :update, params: { order: { line_items_attributes: {
           "0" => { quantity: "0", id: "9999" },
           "1" => { quantity: "99", id: li.id }
@@ -308,7 +308,7 @@ describe Spree::OrdersController, type: :controller do
 
     it "filters line items that are missing from params" do
       order = subject.current_order(true)
-      li = order.add_variant(create(:simple_product).master)
+      li = order.contents.add(create(:simple_product).variants.first)
 
       attrs = {
         "0" => { quantity: "0", id: "9999" },
@@ -322,7 +322,7 @@ describe Spree::OrdersController, type: :controller do
 
     it "keeps the adjustments' previous state" do
       order = subject.current_order(true)
-      line_item = order.add_variant(create(:simple_product, on_hand: 110).variants.first)
+      line_item = order.contents.add(create(:simple_product, on_hand: 110).variants.first)
       adjustment = create(:adjustment, adjustable: order)
 
       get :update, params: { order: { line_items_attributes: {
diff --git a/spec/models/spree/order_spec.rb b/spec/models/spree/order_spec.rb
index eda0c491c5..842e7a8340 100644
--- a/spec/models/spree/order_spec.rb
+++ b/spec/models/spree/order_spec.rb
@@ -1126,8 +1126,11 @@ describe Spree::Order do
     context "when no order has been finalised in this order cycle" do
       let(:product) { create(:product) }
 
+      before do
+        order.contents.update_or_create(product.variants.first, { quantity: 1, max_quantity: 3 })
+      end
+
       it "returns no items even though the cart contains items" do
-        order.add_variant(product.master, 1, 3)
         expect(order.finalised_line_items).to eq []
       end
     end
@@ -1137,9 +1140,12 @@ describe Spree::Order do
       let!(:prev_order2) { create(:completed_order_with_totals, distributor: distributor, order_cycle: order_cycle, user: order.user) }
       let(:product) { create(:product) }
 
-      it "returns previous items" do
-        prev_order.add_variant(product.master, 1, 3)
+      before do
+        prev_order.contents.update_or_create(product.variants.first, { quantity: 1, max_quantity: 3 })
         prev_order2.reload # to get the right response from line_items
+      end
+
+      it "returns previous items" do
         expect(order.finalised_line_items.length).to eq 11
         expect(order.finalised_line_items).to match_array(prev_order.line_items + prev_order2.line_items)
       end
diff --git a/spec/models/spree/payment_method_spec.rb b/spec/models/spree/payment_method_spec.rb
index bddd520118..812b74b513 100644
--- a/spec/models/spree/payment_method_spec.rb
+++ b/spec/models/spree/payment_method_spec.rb
@@ -79,7 +79,7 @@ module Spree
       expect(flat_percent_payment_method.compute_amount(order)).to eq 0
 
       product = create(:product)
-      order.add_variant(product.master)
+      order.contents.add(product.variants.first)
       expect(flat_percent_payment_method.compute_amount(order)).to eq 2.0
     end
 

From 0cfe7fdc45eccd70453ae65e9cbb170d5f4b3f7b Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Wed, 12 May 2021 11:40:56 +0100
Subject: [PATCH 40/59] Move required logic into OrderContents and improve spec

---
 app/controllers/admin/bulk_line_items_controller.rb       | 3 ---
 app/models/spree/order_contents.rb                        | 2 ++
 spec/controllers/admin/bulk_line_items_controller_spec.rb | 4 ++--
 3 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/app/controllers/admin/bulk_line_items_controller.rb b/app/controllers/admin/bulk_line_items_controller.rb
index a4ff5b1676..91973448e9 100644
--- a/app/controllers/admin/bulk_line_items_controller.rb
+++ b/app/controllers/admin/bulk_line_items_controller.rb
@@ -33,9 +33,6 @@ module Admin
       # and https://www.postgresql.org/docs/current/static/sql-select.html#SQL-FOR-UPDATE-SHARE
       order.with_lock do
         if order.contents.update_item(@line_item, line_item_params)
-          order.update_line_item_fees! @line_item
-          order.update_order_fees!
-          order.update_order!
           render body: nil, status: :no_content # No Content, does not trigger ng resource auto-update
         else
           render json: { errors: @line_item.errors }, status: :precondition_failed
diff --git a/app/models/spree/order_contents.rb b/app/models/spree/order_contents.rb
index 15a9030706..5a14d41642 100644
--- a/app/models/spree/order_contents.rb
+++ b/app/models/spree/order_contents.rb
@@ -56,6 +56,8 @@ module Spree
 
     def update_item(line_item, params)
       if line_item.update_attributes(params)
+        order.update_line_item_fees! line_item
+        order.update_order_fees! if order.completed?
         discard_empty_line_items
         order.ensure_updated_shipments
         update_order
diff --git a/spec/controllers/admin/bulk_line_items_controller_spec.rb b/spec/controllers/admin/bulk_line_items_controller_spec.rb
index f376650554..853050b48f 100644
--- a/spec/controllers/admin/bulk_line_items_controller_spec.rb
+++ b/spec/controllers/admin/bulk_line_items_controller_spec.rb
@@ -209,10 +209,10 @@ describe Admin::BulkLineItemsController, type: :controller do
             allow(Spree::LineItem)
               .to receive(:find).with(line_item1.id.to_s).and_return(line_item1)
 
-            expect(line_item1.order).to receive(:reload).with(lock: true)
+            expect(line_item1.order).to receive(:with_lock).and_call_original
             expect(line_item1.order).to receive(:update_line_item_fees!)
             expect(line_item1.order).to receive(:update_order_fees!)
-            expect(line_item1.order).to receive(:update_order!)
+            expect(line_item1.order).to receive(:update_order!).once
 
             spree_put :update, params
           end

From b8ccee5f30fb6be4f205181cc7d5f6d497290f7e Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Wed, 12 May 2021 12:38:33 +0100
Subject: [PATCH 41/59] Simplify use of #to_i in variant data hash

---
 app/services/cart_service.rb | 21 ++++++++++++++-------
 1 file changed, 14 insertions(+), 7 deletions(-)

diff --git a/app/services/cart_service.rb b/app/services/cart_service.rb
index 30562c778a..da9fc4b269 100644
--- a/app/services/cart_service.rb
+++ b/app/services/cart_service.rb
@@ -33,9 +33,9 @@ class CartService
     loaded_variants = indexed_variants(variants_data)
 
     variants_data.each do |variant_data|
-      loaded_variant = loaded_variants[variant_data[:variant_id].to_i]
+      loaded_variant = loaded_variants[variant_data[:variant_id]]
 
-      if loaded_variant.deleted? || !variant_data[:quantity].to_i.positive?
+      if loaded_variant.deleted? || !variant_data[:quantity].positive?
         order.contents.remove(loaded_variant)
         next
       end
@@ -57,8 +57,8 @@ class CartService
   end
 
   def attempt_cart_add(variant, quantity, max_quantity = nil)
-    quantity = quantity.to_i
-    max_quantity = max_quantity.to_i if max_quantity
+    quantity = quantity
+    max_quantity = max_quantity if max_quantity
 
     scoper.scope(variant)
     return unless valid_variant?(variant)
@@ -101,9 +101,16 @@ class CartService
     variants_array = []
     (data[:variants] || []).each do |variant_id, quantity|
       if quantity.is_a?(ActionController::Parameters)
-        variants_array.push({ variant_id: variant_id, quantity: quantity[:quantity], max_quantity: quantity[:max_quantity] })
+        variants_array.push({
+          variant_id: variant_id.to_i,
+          quantity: quantity[:quantity].to_i,
+          max_quantity: quantity[:max_quantity].to_i
+        })
       else
-        variants_array.push({ variant_id: variant_id, quantity: quantity })
+        variants_array.push({
+          variant_id: variant_id.to_i,
+          quantity: quantity.to_i
+        })
       end
     end
     variants_array
@@ -118,7 +125,7 @@ class CartService
     li = line_item_for_variant loaded_variant
 
     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_quantity_changed = li.present? && li.quantity != 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

From 3b52b173e8af82b30ba469a31f0fc6a34e2fc391 Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Wed, 12 May 2021 13:06:31 +0100
Subject: [PATCH 42/59] Delete more dead code in CartService

---
 app/services/cart_service.rb | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/app/services/cart_service.rb b/app/services/cart_service.rb
index da9fc4b269..943baa5787 100644
--- a/app/services/cart_service.rb
+++ b/app/services/cart_service.rb
@@ -57,9 +57,6 @@ class CartService
   end
 
   def attempt_cart_add(variant, quantity, max_quantity = nil)
-    quantity = quantity
-    max_quantity = max_quantity if max_quantity
-
     scoper.scope(variant)
     return unless valid_variant?(variant)
 

From 38ce9eb0c42cc78501e63b73d8dee7dd91f24285 Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Wed, 12 May 2021 13:38:38 +0100
Subject: [PATCH 43/59] Update line item removal in CartService when the line
 item doesn't exist

---
 app/services/cart_service.rb | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/app/services/cart_service.rb b/app/services/cart_service.rb
index 943baa5787..5587438996 100644
--- a/app/services/cart_service.rb
+++ b/app/services/cart_service.rb
@@ -36,7 +36,7 @@ class CartService
       loaded_variant = loaded_variants[variant_data[:variant_id]]
 
       if loaded_variant.deleted? || !variant_data[:quantity].positive?
-        order.contents.remove(loaded_variant)
+        cart_remove(loaded_variant)
         next
       end
 
@@ -69,7 +69,15 @@ class CartService
     if attributes[:quantity].positive?
       @order.contents.update_or_create(variant, attributes)
     else
-      @order.contents.remove(variant)
+      cart_remove(variant)
+    end
+  end
+
+  def cart_remove(variant)
+    begin
+      order.contents.remove(variant)
+    rescue ActiveRecord::RecordNotFound
+      # Nothing to remove; no line items for this variant were found.
     end
   end
 

From 47e2976fd1fe2e4f9bd56de81ab090a9ed9a0e47 Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Wed, 12 May 2021 13:55:09 +0100
Subject: [PATCH 44/59] Update CartService specs

---
 spec/services/cart_service_spec.rb | 130 +++++++++++++----------------
 1 file changed, 60 insertions(+), 70 deletions(-)

diff --git a/spec/services/cart_service_spec.rb b/spec/services/cart_service_spec.rb
index 9a4c900e68..b92e03355a 100644
--- a/spec/services/cart_service_spec.rb
+++ b/spec/services/cart_service_spec.rb
@@ -38,29 +38,35 @@ describe CartService do
         expect(li.final_weight_volume).to eq(1.0)
       end
 
-      it "updates a variant's quantity, max quantity and final_weight_volume" do
-        order.add_variant variant, 1, 2
+      context "updating an existing variant" do
+        before do
+          order.contents.update_or_create(variant, { quantity: 1, max_quantity: 2 })
+        end
 
-        cart_service.populate(
-          ActionController::Parameters.new(
-            { variants: { variant.id.to_s => { quantity: '2', max_quantity: '3' } } }
+        it "updates a variant's quantity, max quantity and final_weight_volume" do
+          cart_service.populate(
+            ActionController::Parameters.new(
+              { variants: { variant.id.to_s => { quantity: '2', max_quantity: '3' } } }
+            )
           )
-        )
 
-        li = order.find_line_item_by_variant(variant)
-        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
+          li = order.find_line_item_by_variant(variant)
+          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
-        order.add_variant variant, 1, 2
-
-        cart_service.populate(ActionController::Parameters.new({ variants: {} }))
-        order.line_items.reload
-        li = order.find_line_item_by_variant(variant)
-        expect(li).not_to be
+        it "removes a variant" do
+          cart_service.populate(
+            ActionController::Parameters.new(
+              { variants: { variant.id.to_s => { quantity: '0' } } }
+            )
+          )
+          order.line_items.reload
+          li = order.find_line_item_by_variant(variant)
+          expect(li).not_to be
+        end
       end
 
       context "when a variant has been soft-deleted" do
@@ -107,28 +113,28 @@ describe CartService do
     let!(:variant) { create(:variant) }
 
     it "returns true when item is not in cart and a quantity is specified" do
-      variant_data = { variant_id: variant.id, quantity: '2' }
+      variant_data = { variant_id: variant.id, quantity: 2 }
 
       expect(cart_service).to receive(:line_item_for_variant).with(variant).and_return(nil)
       expect(cart_service.send(:varies_from_cart, variant_data, variant )).to be true
     end
 
     it "returns true when item is not in cart and a max_quantity is specified" do
-      variant_data = { variant_id: variant.id, quantity: '0', max_quantity: '2' }
+      variant_data = { variant_id: variant.id, quantity: 0, max_quantity: 2 }
 
       expect(cart_service).to receive(:line_item_for_variant).with(variant).and_return(nil)
       expect(cart_service.send(:varies_from_cart, variant_data, variant)).to be true
     end
 
     it "returns false when item is not in cart and no quantity or max_quantity are specified" do
-      variant_data = { variant_id: variant.id, quantity: '0' }
+      variant_data = { variant_id: variant.id, quantity: 0 }
 
       expect(cart_service).to receive(:line_item_for_variant).with(variant).and_return(nil)
       expect(cart_service.send(:varies_from_cart, variant_data, variant)).to be false
     end
 
     it "returns true when quantity varies" do
-      variant_data = { variant_id: variant.id, quantity: '2' }
+      variant_data = { variant_id: variant.id, quantity: 2 }
       line_item = double(:line_item, quantity: 1, max_quantity: nil)
       allow(cart_service).to receive(:line_item_for_variant) { line_item }
 
@@ -136,7 +142,7 @@ describe CartService do
     end
 
     it "returns true when max_quantity varies" do
-      variant_data = { variant_id: variant.id, quantity: '1', max_quantity: '3' }
+      variant_data = { variant_id: variant.id, quantity: 1, max_quantity: 3 }
       line_item = double(:line_item, quantity: 1, max_quantity: nil)
       allow(cart_service).to receive(:line_item_for_variant) { line_item }
 
@@ -144,7 +150,7 @@ describe CartService do
     end
 
     it "returns false when max_quantity varies only in nil vs 0" do
-      variant_data = { variant_id: variant.id, quantity: '1' }
+      variant_data = { variant_id: variant.id, quantity: 1 }
       line_item = double(:line_item, quantity: 1, max_quantity: nil)
       allow(cart_service).to receive(:line_item_for_variant) { line_item }
 
@@ -152,7 +158,7 @@ describe CartService do
     end
 
     it "returns false when both are specified and neither varies" do
-      variant_data = { variant_id: variant.id, quantity: '1', max_quantity: '2' }
+      variant_data = { variant_id: variant.id, quantity: 1, max_quantity: 2 }
       line_item = double(:line_item, quantity: 1, max_quantity: 2)
       allow(cart_service).to receive(:line_item_for_variant) { line_item }
 
@@ -160,30 +166,6 @@ describe CartService do
     end
   end
 
-  describe "variants_removed" do
-    it "returns the variant ids when one is in the cart but not in those given" do
-      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
-      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
-      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
-      allow(cart_service).to receive(:variant_ids_in_cart) { [123, 123] }
-      expect(cart_service.send(:variants_removed, [])).to eq([123])
-    end
-  end
-
   describe "attempt_cart_add" do
     let!(:variant) { create(:variant, on_hand: 250) }
     let(:quantity) { 123 }
@@ -198,38 +180,40 @@ describe CartService do
       expect(cart_service).to receive(:check_variant_available_under_distribution).with(variant).
         and_return(true)
       expect(variant).to receive(:on_demand).and_return(false)
-      expect(order).to receive(:add_variant).with(variant, quantity, nil, currency)
+      expect(order).to receive_message_chain(:contents, :update_or_create).
+        with(variant, { quantity: quantity, max_quantity: nil })
 
-      cart_service.send(:attempt_cart_add, variant, quantity.to_s)
+      cart_service.send(:attempt_cart_add, variant, quantity)
     end
 
-    it "filters quantities through #quantities_to_add" do
-      expect(cart_service).to receive(:quantities_to_add).with(variant, 123, 123).
-        and_return([5, 5])
+    it "filters quantities through #final_quantities" do
+      expect(cart_service).to receive(:final_quantities).with(variant, 123, 123).
+        and_return({ quantity: 5, max_quantity: 5 })
 
       allow(cart_service).to receive(:check_order_cycle_provided) { true }
       allow(cart_service).to receive(:check_variant_available_under_distribution) { true }
 
-      expect(order).to receive(:add_variant).with(variant, 5, 5, currency)
+      expect(order).to receive_message_chain(:contents, :update_or_create).
+        with(variant, { quantity: 5, max_quantity: 5 })
 
-      cart_service.send(:attempt_cart_add, variant, quantity.to_s, quantity.to_s)
+      cart_service.send(:attempt_cart_add, variant, quantity, quantity)
     end
 
     it "removes variants which have become out of stock" do
-      expect(cart_service).to receive(:quantities_to_add).with(variant, 123, 123).
-        and_return([0, 0])
+      expect(cart_service).to receive(:final_quantities).with(variant, 123, 123).
+        and_return({ quantity: 0, max_quantity: 0 })
 
       allow(cart_service).to receive(:check_order_cycle_provided) { true }
       allow(cart_service).to receive(:check_variant_available_under_distribution) { true }
 
-      expect(order).to receive(:remove_variant).with(variant)
-      expect(order).to receive(:add_variant).never
+      expect(cart_service).to receive(:cart_add).with(variant, 123, 123).and_call_original
+      expect(order).to receive_message_chain(:contents, :remove).with(variant)
 
-      cart_service.send(:attempt_cart_add, variant, quantity.to_s, quantity.to_s)
+      cart_service.send(:attempt_cart_add, variant, quantity, quantity)
     end
   end
 
-  describe "quantities_to_add" do
+  describe "#final_quantities" do
     let(:v) { double(:variant, on_hand: 10) }
 
     context "when backorders are not allowed" do
@@ -237,23 +221,27 @@ describe CartService do
         expect(v).to receive(:on_demand).and_return(false)
       end
 
-      context "when max_quantity is not provided" do
+      context "getting quantity and max_quantity" do
         it "returns full amount when available" do
-          expect(cart_service.send(:quantities_to_add, v, 5, nil)).to eq([5, nil])
+          expect(cart_service.send(:final_quantities, v, 5, nil)).
+            to eq({ quantity: 5, max_quantity: nil })
         end
 
         it "returns a limited amount when not entirely available" do
-          expect(cart_service.send(:quantities_to_add, v, 15, nil)).to eq([10, nil])
+          expect(cart_service.send(:final_quantities, v, 15, nil)).
+            to eq({ quantity: 10, max_quantity: nil })
         end
       end
 
       context "when max_quantity is provided" do
         it "returns full amount when available" do
-          expect(cart_service.send(:quantities_to_add, v, 5, 6)).to eq([5, 6])
+          expect(cart_service.send(:final_quantities, v, 5, 6)).
+            to eq({ quantity: 5, max_quantity: 6 })
         end
 
         it "also returns the full amount when not entirely available" do
-          expect(cart_service.send(:quantities_to_add, v, 15, 16)).to eq([10, 16])
+          expect(cart_service.send(:final_quantities, v, 15, 16)).
+            to eq({ quantity: 10, max_quantity: 16 })
         end
       end
     end
@@ -264,11 +252,13 @@ describe CartService do
       end
 
       it "does not limit quantity" do
-        expect(cart_service.send(:quantities_to_add, v, 15, nil)).to eq([15, nil])
+        expect(cart_service.send(:final_quantities, v, 15, nil)).
+          to eq({ quantity: 15, max_quantity: nil })
       end
 
       it "does not limit max_quantity" do
-        expect(cart_service.send(:quantities_to_add, v, 15, 16)).to eq([15, 16])
+        expect(cart_service.send(:final_quantities, v, 15, 16)).
+          to eq({ quantity: 15, max_quantity: 16 })
       end
     end
   end

From dc6be6f06bfafd4e35d4645a4a797e9319187278 Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Wed, 12 May 2021 15:41:32 +0100
Subject: [PATCH 45/59] Don't resubmit the whole cart contents for no reason.

There's a couple of places where this was causing a cart update submission where it wasn't needed, eg the items had not actually changed. The conditional here was designed to stop that from happening, but it was actually passing every time (the conditional logic was not actually catching the case it was supposed to).

This is really expensive!
---
 .../controllers/shop_variant_controller.js.coffee         | 8 +++-----
 .../checkout/shop_variant_controller_spec.js.coffee       | 6 +++++-
 2 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/app/assets/javascripts/darkswarm/controllers/shop_variant_controller.js.coffee b/app/assets/javascripts/darkswarm/controllers/shop_variant_controller.js.coffee
index 825bc9f39f..e71a7a0a29 100644
--- a/app/assets/javascripts/darkswarm/controllers/shop_variant_controller.js.coffee
+++ b/app/assets/javascripts/darkswarm/controllers/shop_variant_controller.js.coffee
@@ -1,9 +1,5 @@
 Darkswarm.controller "ShopVariantCtrl", ($scope, $modal, Cart) ->
-  $scope.$watchGroup [
-    'variant.line_item.quantity',
-    'variant.line_item.max_quantity'
-  ], (new_value, old_value) ->
-    return if old_value[0] == null && new_value[0] == null
+  $scope.updateCart = (line_item) ->
     Cart.adjust($scope.variant.line_item)
 
   $scope.variant.line_item.quantity ||= 0
@@ -44,10 +40,12 @@ Darkswarm.controller "ShopVariantCtrl", ($scope, $modal, Cart) ->
   $scope.add = (quantity) ->
     item = $scope.variant.line_item
     item.quantity = $scope.sanitizedQuantity() + quantity
+    $scope.updateCart(item)
 
   $scope.addMax = (quantity) ->
     item = $scope.variant.line_item
     item.max_quantity = $scope.sanitizedMaxQuantity() + quantity
+    $scope.updateCart(item)
 
   $scope.canAdd = (quantity) ->
     wantedQuantity = $scope.sanitizedQuantity() + quantity
diff --git a/spec/javascripts/unit/darkswarm/controllers/checkout/shop_variant_controller_spec.js.coffee b/spec/javascripts/unit/darkswarm/controllers/checkout/shop_variant_controller_spec.js.coffee
index 4576e376c7..c560a287c4 100644
--- a/spec/javascripts/unit/darkswarm/controllers/checkout/shop_variant_controller_spec.js.coffee
+++ b/spec/javascripts/unit/darkswarm/controllers/checkout/shop_variant_controller_spec.js.coffee
@@ -1,6 +1,7 @@
 describe "ShopVariantCtrl", ->
   ctrl = null
   scope = null
+  CartMock = null
 
   beforeEach ->
     module 'Darkswarm'
@@ -16,7 +17,10 @@ describe "ShopVariantCtrl", ->
           max_quantity: undefined
         }
       }
-      ctrl = $controller 'ShopVariantCtrl', {$scope: scope, $modal: $modal, Cart: null}
+      CartMock =
+        adjust: ->
+          true
+      ctrl = $controller 'ShopVariantCtrl', {$scope: scope, $modal: $modal, Cart: CartMock}
 
   it "initializes the quantity for shop display", ->
     expect(scope.variant.line_item.quantity).toEqual 0

From 6abe0b375cb4f1b1c0d2986d4fd2ba6329a3c011 Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Wed, 12 May 2021 16:31:49 +0100
Subject: [PATCH 46/59] Refactor stock levels check in CartController

---
 app/controllers/cart_controller.rb | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/app/controllers/cart_controller.rb b/app/controllers/cart_controller.rb
index edd9e69b6d..e35aedad28 100644
--- a/app/controllers/cart_controller.rb
+++ b/app/controllers/cart_controller.rb
@@ -11,11 +11,7 @@ class CartController < BaseController
       order.cap_quantity_at_stock!
       order.recreate_all_fees!
 
-      variant_ids = order.line_items.pluck(:variant_id)
-
-      render json: { error: false,
-                     stock_levels: VariantsStockLevels.new.call(order, variant_ids) },
-             status: :ok
+      render json: { error: false, stock_levels: stock_levels(order) }, status: :ok
     else
       render json: { error: cart_service.errors.full_messages.join(",") },
              status: :precondition_failed
@@ -24,6 +20,13 @@ class CartController < BaseController
 
   private
 
+  def stock_levels(order)
+    variants_in_cart = order.line_items.pluck(:variant_id)
+    variants_in_request = raw_params[:variants]&.map(&:first) || []
+
+    VariantsStockLevels.new.call(order, (variants_in_cart + variants_in_request).uniq)
+  end
+
   def check_authorization
     session[:access_token] ||= params[:token]
     order = Spree::Order.find_by(number: params[:id]) || current_order

From 2e96982e609ecf249ce331020e8bb065d7db8dd3 Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Wed, 12 May 2021 16:34:00 +0100
Subject: [PATCH 47/59] Refactor conditional

CartService#populate already returns the value of the #valid? method by default.
---
 app/controllers/cart_controller.rb | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/app/controllers/cart_controller.rb b/app/controllers/cart_controller.rb
index e35aedad28..8ea8d31ab0 100644
--- a/app/controllers/cart_controller.rb
+++ b/app/controllers/cart_controller.rb
@@ -3,11 +3,9 @@ class CartController < BaseController
 
   def populate
     order = current_order(true)
-
     cart_service = CartService.new(order)
 
-    cart_service.populate(params.slice(:variants, :quantity))
-    if cart_service.valid?
+    if cart_service.populate(params.slice(:variants, :quantity))
       order.cap_quantity_at_stock!
       order.recreate_all_fees!
 

From 011da05712fa0e836ec6f15b6fb5af311a612d9c Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Wed, 12 May 2021 17:48:47 +0100
Subject: [PATCH 48/59] Add more test coverage to OrderContents

---
 spec/models/spree/order_contents_spec.rb | 61 ++++++++++++++++++++++++
 1 file changed, 61 insertions(+)

diff --git a/spec/models/spree/order_contents_spec.rb b/spec/models/spree/order_contents_spec.rb
index e63318cf5b..88f6ff48fd 100644
--- a/spec/models/spree/order_contents_spec.rb
+++ b/spec/models/spree/order_contents_spec.rb
@@ -127,4 +127,65 @@ describe Spree::OrderContents do
       subject.update_cart params
     end
   end
+
+  describe "#update_item" do
+    let!(:line_item) { subject.add variant, 1 }
+
+    context "updating enterprise fees" do
+      it "updates the line item's enterprise fees" do
+        expect(order).to receive(:update_line_item_fees!).with(line_item)
+
+        subject.update_item(line_item, { quantity: 3 })
+      end
+
+      it "updates the order's enterprise fees if completed" do
+        allow(order).to receive(:completed?) { true }
+        expect(order).to receive(:update_order_fees!)
+
+        subject.update_item(line_item, { quantity: 3 })
+      end
+
+      it "does not update the order's enterprise fees if not complete" do
+        expect(order).to_not receive(:update_order_fees!)
+
+        subject.update_item(line_item, { quantity: 3 })
+      end
+    end
+
+    it "ensures updated shipments" do
+      expect(order).to receive(:ensure_updated_shipments)
+
+      subject.update_item(line_item, { quantity: 3 })
+    end
+
+    it "updates the order" do
+      expect(order).to receive(:update_order!)
+
+      subject.update_item(line_item, { quantity: 3 })
+    end
+  end
+
+  describe "#update_or_create" do
+    describe "creating" do
+      it "creates a new line item with given attributes" do
+        subject.update_or_create(variant, { quantity: 2, max_quantity: 3 })
+
+        line_item = order.line_items.reload.first
+        expect(line_item.quantity).to eq 2
+        expect(line_item.max_quantity).to eq 3
+        expect(line_item.price).to eq variant.price
+      end
+    end
+
+    describe "updating" do
+      let!(:line_item) { subject.add variant, 2 }
+
+      it "updates existing line item with given attributes" do
+        subject.update_or_create(variant, { quantity: 3, max_quantity: 4 })
+
+        expect(line_item.reload.quantity).to eq 3
+        expect(line_item.max_quantity).to eq 4
+      end
+    end
+  end
 end

From 05c001807e18cec3d326d9666ad916ad9ea7b1d1 Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Sat, 15 May 2021 21:40:29 +0100
Subject: [PATCH 49/59] Add variant override specs for
 Stock::AvailabilityValidator

---
 .../stock/availability_validator_spec.rb      | 27 +++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/spec/models/spree/stock/availability_validator_spec.rb b/spec/models/spree/stock/availability_validator_spec.rb
index 44125cedab..385ffd46e2 100644
--- a/spec/models/spree/stock/availability_validator_spec.rb
+++ b/spec/models/spree/stock/availability_validator_spec.rb
@@ -52,6 +52,33 @@ module Spree
             expect(line_item).not_to be_valid
           end
         end
+
+        context "when the line item's variant has an override" do
+          let(:hub) { order.distributor }
+          let(:variant) { line_item.variant }
+          let(:vo_stock) { 999 }
+          let!(:variant_override) {
+            create(:variant_override, variant: variant, hub: hub, count_on_hand: vo_stock)
+          }
+
+          context "when the override has stock" do
+            it "is valid" do
+              line_item.quantity = 999
+              validator.validate(line_item)
+              expect(line_item).to be_valid
+            end
+          end
+
+          context "when the override is out of stock" do
+            let(:vo_stock) { 1 }
+
+            it "is not valid" do
+              line_item.quantity = 999
+              validator.validate(line_item)
+              expect(line_item).to_not be_valid
+            end
+          end
+        end
       end
     end
   end

From d5b20d5446a8b391e95197671b1be0cc9b7e82cb Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Sun, 16 May 2021 10:06:29 +0100
Subject: [PATCH 50/59] Add specs for Order#ensure_updated_shipments

---
 spec/models/spree/order_spec.rb | 32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)

diff --git a/spec/models/spree/order_spec.rb b/spec/models/spree/order_spec.rb
index 842e7a8340..67c6306680 100644
--- a/spec/models/spree/order_spec.rb
+++ b/spec/models/spree/order_spec.rb
@@ -1313,4 +1313,36 @@ describe Spree::Order do
       end
     end
   end
+
+  describe "#ensure_updated_shipments" do
+    before { Spree::Shipment.create!(order: order) }
+
+    context "when the order is not completed" do
+      it "destroys current shipments" do
+        order.ensure_updated_shipments
+        expect(order.shipments).to be_empty
+      end
+
+      it "puts order back in address state" do
+        order.ensure_updated_shipments
+        expect(order.state).to eq "address"
+      end
+    end
+
+    context "when the order is completed" do
+      before do
+        allow(order).to receive(:completed?) { true }
+      end
+
+      it "does not change the shipments" do
+        expect {
+          order.ensure_updated_shipments
+        }.not_to change { order.shipments }
+
+        expect {
+          order.ensure_updated_shipments
+        }.not_to change { order.state }
+      end
+    end
+  end
 end

From 6b364dc4207708afc8a0f27eae76f1c6ca94fc56 Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Mon, 17 May 2021 13:04:42 +0100
Subject: [PATCH 51/59] Update specs on shipping fee changes and mark as
 pending

The shipping fee and it's tax actually don't get updated correctly here (in master).
---
 .../admin/bulk_line_items_controller_spec.rb  | 23 ++++++++++++-------
 1 file changed, 15 insertions(+), 8 deletions(-)

diff --git a/spec/controllers/admin/bulk_line_items_controller_spec.rb b/spec/controllers/admin/bulk_line_items_controller_spec.rb
index 853050b48f..1e0cc40170 100644
--- a/spec/controllers/admin/bulk_line_items_controller_spec.rb
+++ b/spec/controllers/admin/bulk_line_items_controller_spec.rb
@@ -347,8 +347,10 @@ describe Admin::BulkLineItemsController, type: :controller do
       let(:line_item_params) { { quantity: 3 } }
       let(:params) { { id: line_item1.id, order_id: order.number, line_item: line_item_params } }
 
-      it "correctly updates order totals and states" do
+      xit "correctly updates order totals and states" do
         expect(order.total).to eq 35.0
+        expect(order.shipment_adjustments.shipping.sum(:amount)).to eq 6.0
+        expect(order.shipment_adjustments.tax.sum(:amount)).to eq 0.29
         expect(order.item_total).to eq 20.0
         expect(order.adjustment_total).to eq 15.0
         expect(order.included_tax_total).to eq 1.22
@@ -360,10 +362,11 @@ describe Admin::BulkLineItemsController, type: :controller do
         spree_put :update, params
         order.reload
 
-        expect(order.total).to eq 61.0
+        expect(order.total).to eq 67.0
+        expect(order.shipment_adjustments.sum(:amount)).to eq 12.57
         expect(order.item_total).to eq 40.0
-        expect(order.adjustment_total).to eq 21.0
-        expect(order.included_tax_total).to eq 2.65 # Pending: this should be 3.10!
+        expect(order.adjustment_total).to eq 27.0
+        expect(order.included_tax_total).to eq 2.93 # Pending: taxes on enterprise fees unchanged
         expect(order.payment_state).to eq "balance_due"
       end
     end
@@ -371,8 +374,10 @@ describe Admin::BulkLineItemsController, type: :controller do
     describe "deleting a line item" do
       let(:params) { { id: line_item1.id, order_id: order.number } }
 
-      it "correctly updates order totals and states" do
+      xit "correctly updates order totals and states" do
         expect(order.total).to eq 35.0
+        expect(order.shipment_adjustments.shipping.sum(:amount)).to eq 6.0
+        expect(order.shipment_adjustments.tax.sum(:amount)).to eq 0.29
         expect(order.item_total).to eq 20.0
         expect(order.adjustment_total).to eq 15.0
         expect(order.included_tax_total).to eq 1.22
@@ -381,10 +386,12 @@ describe Admin::BulkLineItemsController, type: :controller do
         spree_delete :destroy, params
         order.reload
 
-        expect(order.total).to eq 22.0
+        expect(order.total).to eq 19.0
+        expect(order.shipment_adjustments.shipping.sum(:amount)).to eq 3.0
+        expect(order.shipment_adjustments.tax.sum(:amount)).to eq 0.14
         expect(order.item_total).to eq 10.0
-        expect(order.adjustment_total).to eq 12.0
-        expect(order.included_tax_total).to eq 0.99
+        expect(order.adjustment_total).to eq 9.0
+        expect(order.included_tax_total).to eq 0.84
         expect(order.payment_state).to eq "credit_owed"
       end
     end

From 2d76c2730a1f7533d48bc56fee0517db0ecfe30c Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Sun, 16 May 2021 11:21:02 +0100
Subject: [PATCH 52/59] Update shipment updating

---
 app/controllers/line_items_controller.rb           |  1 -
 app/controllers/spree/orders_controller.rb         |  1 -
 app/models/spree/order_contents.rb                 | 14 +++++++++-----
 .../admin/bulk_line_items_controller_spec.rb       |  4 ++--
 4 files changed, 11 insertions(+), 9 deletions(-)

diff --git a/app/controllers/line_items_controller.rb b/app/controllers/line_items_controller.rb
index 123d03510f..b61d476d2e 100644
--- a/app/controllers/line_items_controller.rb
+++ b/app/controllers/line_items_controller.rb
@@ -40,7 +40,6 @@ class LineItemsController < BaseController
     order = item.order
     order.with_lock do
       order.contents.remove(item.variant)
-      order.update_shipping_fees!
       order.update_payment_fees!
     end
   end
diff --git a/app/controllers/spree/orders_controller.rb b/app/controllers/spree/orders_controller.rb
index c380d016c5..015ad64936 100644
--- a/app/controllers/spree/orders_controller.rb
+++ b/app/controllers/spree/orders_controller.rb
@@ -70,7 +70,6 @@ module Spree
         @order.recreate_all_fees! # Enterprise fees on line items and on the order itself
 
         if @order.complete?
-          @order.update_shipping_fees!
           @order.update_payment_fees!
           @order.create_tax_charge!
         end
diff --git a/app/models/spree/order_contents.rb b/app/models/spree/order_contents.rb
index 5a14d41642..7914256e7a 100644
--- a/app/models/spree/order_contents.rb
+++ b/app/models/spree/order_contents.rb
@@ -46,7 +46,7 @@ module Spree
     def update_cart(params)
       if order.update_attributes(params)
         discard_empty_line_items
-        order.ensure_updated_shipments
+        update_shipment
         update_order
         true
       else
@@ -56,10 +56,10 @@ module Spree
 
     def update_item(line_item, params)
       if line_item.update_attributes(params)
+        discard_empty_line_items
         order.update_line_item_fees! line_item
         order.update_order_fees! if order.completed?
-        discard_empty_line_items
-        order.ensure_updated_shipments
+        update_shipment
         update_order
         true
       else
@@ -73,8 +73,12 @@ module Spree
       order.line_items = order.line_items.select {|li| li.quantity.positive? }
     end
 
-    def update_shipment(shipment)
-      shipment.present? ? shipment.update_amounts : order.ensure_updated_shipments
+    def update_shipment(target_shipment = nil)
+      if order.completed? || target_shipment.present?
+        order.update_shipping_fees!
+      else
+        order.ensure_updated_shipments
+      end
     end
 
     def add_to_line_item(variant, quantity, shipment = nil)
diff --git a/spec/controllers/admin/bulk_line_items_controller_spec.rb b/spec/controllers/admin/bulk_line_items_controller_spec.rb
index 1e0cc40170..ea6cf9da30 100644
--- a/spec/controllers/admin/bulk_line_items_controller_spec.rb
+++ b/spec/controllers/admin/bulk_line_items_controller_spec.rb
@@ -347,7 +347,7 @@ describe Admin::BulkLineItemsController, type: :controller do
       let(:line_item_params) { { quantity: 3 } }
       let(:params) { { id: line_item1.id, order_id: order.number, line_item: line_item_params } }
 
-      xit "correctly updates order totals and states" do
+      it "correctly updates order totals and states" do
         expect(order.total).to eq 35.0
         expect(order.shipment_adjustments.shipping.sum(:amount)).to eq 6.0
         expect(order.shipment_adjustments.tax.sum(:amount)).to eq 0.29
@@ -374,7 +374,7 @@ describe Admin::BulkLineItemsController, type: :controller do
     describe "deleting a line item" do
       let(:params) { { id: line_item1.id, order_id: order.number } }
 
-      xit "correctly updates order totals and states" do
+      it "correctly updates order totals and states" do
         expect(order.total).to eq 35.0
         expect(order.shipment_adjustments.shipping.sum(:amount)).to eq 6.0
         expect(order.shipment_adjustments.tax.sum(:amount)).to eq 0.29

From b69d61dd77a7ee3cd9468fa09894e852951d3df7 Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Mon, 17 May 2021 13:11:11 +0100
Subject: [PATCH 53/59] Remove #order_update! call from method used in order
 after_save callbacks

This method gets called twice every time we save a completed order, calling order_update! twice...
---
 app/models/spree/order.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/models/spree/order.rb b/app/models/spree/order.rb
index c6a6ee80d7..dbad51f2dc 100644
--- a/app/models/spree/order.rb
+++ b/app/models/spree/order.rb
@@ -706,7 +706,7 @@ module Spree
       return if adjustment.finalized?
 
       adjustment.update_adjustment!(force: true)
-      update_order!
+      updater.update_totals_and_states
     end
 
     # object_params sets the payment amount to the order total, but it does this

From e246eed99d54bb471018dd0d95e7ba68bc017c9c Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Tue, 18 May 2021 09:22:23 +0100
Subject: [PATCH 54/59] Move #restart_chceckout_flow out of Order class

---
 app/models/spree/order.rb          | 7 -------
 app/models/spree/order/checkout.rb | 7 +++++++
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/app/models/spree/order.rb b/app/models/spree/order.rb
index dbad51f2dc..7ef3b7dea7 100644
--- a/app/models/spree/order.rb
+++ b/app/models/spree/order.rb
@@ -473,13 +473,6 @@ module Spree
       end
     end
 
-    def restart_checkout_flow
-      self.update_columns(
-        state: checkout_steps.first,
-        updated_at: Time.zone.now,
-      )
-    end
-
     def refresh_shipment_rates
       shipments.map(&:refresh_rates)
     end
diff --git a/app/models/spree/order/checkout.rb b/app/models/spree/order/checkout.rb
index 4d34f091ca..9d50612390 100644
--- a/app/models/spree/order/checkout.rb
+++ b/app/models/spree/order/checkout.rb
@@ -130,6 +130,13 @@ module Spree
             steps << "complete" unless steps.include?("complete")
             steps
           end
+
+          def restart_checkout_flow
+            update_columns(
+              state: checkout_steps.first,
+              updated_at: Time.zone.now,
+            )
+          end
         end
       end
     end

From 3222d4980d54d7751b15e86667edafebac936bd1 Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Tue, 18 May 2021 10:03:42 +0100
Subject: [PATCH 55/59] Improve comment on #ensure_updated_shipments

---
 app/models/spree/order.rb | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/app/models/spree/order.rb b/app/models/spree/order.rb
index 7ef3b7dea7..c9bbec9989 100644
--- a/app/models/spree/order.rb
+++ b/app/models/spree/order.rb
@@ -465,7 +465,9 @@ module Spree
       shipments
     end
 
-    # Clear shipments and move order back to address state
+    # Clear shipments and move order back to address state unless compete. This is relevant where
+    # an order is part-way through checkout and the user changes items in the cart; in that case
+    # we need to reset the checkout flow to ensure the order is processed correctly.
     def ensure_updated_shipments
       if !self.completed? && shipments.any?
         shipments.destroy_all

From 443b180f32052654b5cfdecd6f395c1e762119a6 Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Thu, 20 May 2021 13:55:55 +0100
Subject: [PATCH 56/59] Update order totals in :order_with_totals factory

---
 spec/factories/order_factory.rb | 1 +
 1 file changed, 1 insertion(+)

diff --git a/spec/factories/order_factory.rb b/spec/factories/order_factory.rb
index 926020546d..1fc3156897 100644
--- a/spec/factories/order_factory.rb
+++ b/spec/factories/order_factory.rb
@@ -15,6 +15,7 @@ FactoryBot.define do
       after(:create) do |order|
         create(:line_item, order: order)
         order.line_items.reload # to ensure order.line_items is accessible after
+        order.updater.update_totals_and_states
       end
     end
 

From acf7c0c4d8e3075b830d2581b617d393c2250bfb Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Thu, 20 May 2021 13:58:46 +0100
Subject: [PATCH 57/59] Use :order_with_totals factory in CustomersController
 spec

---
 spec/controllers/admin/customers_controller_spec.rb | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/spec/controllers/admin/customers_controller_spec.rb b/spec/controllers/admin/customers_controller_spec.rb
index 5afdcd6d50..b8271ff27c 100644
--- a/spec/controllers/admin/customers_controller_spec.rb
+++ b/spec/controllers/admin/customers_controller_spec.rb
@@ -108,8 +108,7 @@ module Admin
             end
 
             context 'when the customer has an order with a void payment' do
-              let(:order) { create(:order, customer: customer, state: 'complete') }
-              let!(:line_item) { create(:line_item, order: order, price: 10.0) }
+              let(:order) { create(:order_with_totals, customer: customer, state: 'complete') }
               let!(:payment) { create(:payment, order: order, amount: order.total) }
 
               before do

From efcf71fd2b845d277955d274f6151605e7c14161 Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Fri, 4 Jun 2021 17:26:49 +0100
Subject: [PATCH 58/59] Use OrderContents in cart spec test setups

---
 spec/features/consumer/shopping/cart_spec.rb | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/spec/features/consumer/shopping/cart_spec.rb b/spec/features/consumer/shopping/cart_spec.rb
index 9af20fa926..b34c45ac7f 100644
--- a/spec/features/consumer/shopping/cart_spec.rb
+++ b/spec/features/consumer/shopping/cart_spec.rb
@@ -164,7 +164,7 @@ feature "full-page cart", js: true do
       let(:variant2) { product_with_fee.variants.first }
 
       before do
-        add_product_to_cart order, product_with_tax
+        order.contents.add(product_with_tax.variants.first)
       end
 
       describe "when on_hand is zero but variant is on demand" do
@@ -179,7 +179,7 @@ feature "full-page cart", js: true do
 
       describe "with insufficient stock available" do
         it "prevents user from entering invalid values" do
-          add_product_to_cart order, product_with_fee
+          order.contents.add(product_with_fee.variants.first)
 
           variant.update!(on_hand: 2, on_demand: false)
           variant2.update!(on_hand: 3, on_demand: false)

From 099ef5d35881cf4512dbe2c5a2e60467d05b6614 Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Fri, 4 Jun 2021 18:19:27 +0100
Subject: [PATCH 59/59] Add more explicit tests on updating shipping fees in
 Api::ShipmentsController

---
 .../api/v0/shipments_controller_spec.rb       | 36 +++++++++++++++++++
 1 file changed, 36 insertions(+)

diff --git a/spec/controllers/api/v0/shipments_controller_spec.rb b/spec/controllers/api/v0/shipments_controller_spec.rb
index dd1be3c0fc..35aebc1b88 100644
--- a/spec/controllers/api/v0/shipments_controller_spec.rb
+++ b/spec/controllers/api/v0/shipments_controller_spec.rb
@@ -174,6 +174,42 @@ describe Api::V0::ShipmentsController, type: :controller do
           }.to_not change { existing_variant.reload.on_hand }
         end
       end
+
+      context "with shipping fees" do
+        let!(:distributor) { create(:distributor_enterprise) }
+        let(:fee_amount) { 10 }
+        let!(:shipping_method_with_fee) {
+          create(:shipping_method_with, :shipping_fee, distributors: [distributor],
+                                                       shipping_fee: fee_amount)
+        }
+        let!(:order_cycle) { create(:order_cycle, distributors: [distributor]) }
+        let!(:order) {
+          create(:completed_order_with_totals, order_cycle: order_cycle, distributor: distributor)
+        }
+        let(:shipping_fee) { order.reload.shipment.adjustments.first }
+
+        before do
+          order.shipments.first.shipping_methods = [shipping_method_with_fee]
+          order.select_shipping_method(shipping_method_with_fee.id)
+          order.update_order!
+        end
+
+        context "adding item to a shipment" do
+          it "updates the shipping fee" do
+            expect {
+              api_put :add, params.merge(variant_id: new_variant.to_param)
+            }.to change { order.reload.shipment.adjustments.first.amount }.by(20)
+          end
+        end
+
+        context "removing item from a shipment" do
+          it "updates the shipping fee" do
+            expect {
+              api_put :remove, params.merge(variant_id: existing_variant.to_param)
+            }.to change { order.reload.shipment.adjustments.first.amount }.by(-20)
+          end
+        end
+      end
     end
 
     describe "#update" do