From 257b5a9eefa964be7b9fc48b28bee743c409e50e Mon Sep 17 00:00:00 2001
From: Rohan Mitchell <rohan@rohanmitchell.com>
Date: Thu, 7 Jan 2016 11:21:25 +1100
Subject: [PATCH] Move premature Spree::Order#update prevention up a level, for
 even greater efficiency gains

---
 .../spree/orders_controller_decorator.rb      | 24 +++++++++++-----
 app/models/spree/order_decorator.rb           | 28 +++++++------------
 spec/models/spree/order_spec.rb               |  5 ----
 3 files changed, 27 insertions(+), 30 deletions(-)

diff --git a/app/controllers/spree/orders_controller_decorator.rb b/app/controllers/spree/orders_controller_decorator.rb
index fd5d3b634e..c6d325d0f8 100644
--- a/app/controllers/spree/orders_controller_decorator.rb
+++ b/app/controllers/spree/orders_controller_decorator.rb
@@ -23,13 +23,23 @@ Spree::OrdersController.class_eval do
   end
 
   def populate
-    populator = Spree::OrderPopulator.new(current_order(true), current_currency)
-    if populator.populate(params.slice(:products, :variants, :quantity), true)
-      fire_event('spree.cart.add')
-      fire_event('spree.order.contents_changed')
-      render json: true, status: 200
-    else
-      render json: false, status: 402
+    # Without intervention, the Spree::Adjustment#update_adjustable callback is called many times
+    # during cart population, for both taxation and enterprise fees. This operation triggers a
+    # costly Spree::Order#update!, which only needs to be run once. We avoid this by disabling
+    # callbacks on Spree::Adjustment and then manually invoke Spree::Order#update! on success.
+
+    Spree::Adjustment.without_callbacks do
+      populator = Spree::OrderPopulator.new(current_order(true), current_currency)
+      if populator.populate(params.slice(:products, :variants, :quantity), true)
+        fire_event('spree.cart.add')
+        fire_event('spree.order.contents_changed')
+
+        current_order.update!
+
+        render json: true, status: 200
+      else
+        render json: false, status: 402
+      end
     end
   end
 
diff --git a/app/models/spree/order_decorator.rb b/app/models/spree/order_decorator.rb
index 53e80bbdef..8f1c0381b1 100644
--- a/app/models/spree/order_decorator.rb
+++ b/app/models/spree/order_decorator.rb
@@ -164,29 +164,21 @@ Spree::Order.class_eval do
 
   def update_distribution_charge!
     with_lock do
+      EnterpriseFee.clear_all_adjustments_on_order self
 
-      # Without intervention, the Spree::Adjustment#update_adjustable callback is called
-      # once for every (line item x fee), which triggers a costly Spree::Order#update!
-      Spree::Adjustment.without_callbacks do
-        EnterpriseFee.clear_all_adjustments_on_order self
+      line_items.each do |line_item|
+        if provided_by_order_cycle? line_item
+          OpenFoodNetwork::EnterpriseFeeCalculator.new.create_line_item_adjustments_for line_item
 
-        line_items.each do |line_item|
-          if provided_by_order_cycle? line_item
-            OpenFoodNetwork::EnterpriseFeeCalculator.new.create_line_item_adjustments_for line_item
-
-          else
-            pd = product_distribution_for line_item
-            pd.create_adjustment_for line_item if pd
-          end
-        end
-
-        if order_cycle
-          OpenFoodNetwork::EnterpriseFeeCalculator.new.create_order_adjustments_for self
+        else
+          pd = product_distribution_for line_item
+          pd.create_adjustment_for line_item if pd
         end
       end
 
-      # After fees are calculated, we update the order once
-      update!
+      if order_cycle
+        OpenFoodNetwork::EnterpriseFeeCalculator.new.create_order_adjustments_for self
+      end
     end
   end
 
diff --git a/spec/models/spree/order_spec.rb b/spec/models/spree/order_spec.rb
index 300cf26bac..4d681e01cc 100644
--- a/spec/models/spree/order_spec.rb
+++ b/spec/models/spree/order_spec.rb
@@ -54,7 +54,6 @@ describe Spree::Order do
       product_distribution.should_receive(:create_adjustment_for).with(line_item)
       subject.stub(:product_distribution_for) { product_distribution }
 
-      subject.should_receive(:update!)
 
       subject.update_distribution_charge!
     end
@@ -67,8 +66,6 @@ describe Spree::Order do
 
       subject.stub(:product_distribution_for) { nil }
 
-      subject.should_receive(:update!)
-
       subject.update_distribution_charge!
     end
 
@@ -94,8 +91,6 @@ describe Spree::Order do
       OpenFoodNetwork::EnterpriseFeeCalculator.any_instance.stub(:create_order_adjustments_for)
       subject.stub(:order_cycle) { order_cycle }
 
-      subject.should_receive(:update!)
-
       subject.update_distribution_charge!
     end