From bbc3cad67d7add0818a2e31abf2337ccd4693390 Mon Sep 17 00:00:00 2001
From: Rob Harrington <oeoeaio@gmail.com>
Date: Wed, 31 Jan 2018 15:43:52 +1100
Subject: [PATCH] Refactor large StandingOrderUpdater#update! method

Note: extracted line items logic into separate class
---
 app/services/line_item_syncer.rb       | 55 ++++++++++++++++++++++++++
 app/services/standing_order_updater.rb | 44 ++++++---------------
 2 files changed, 66 insertions(+), 33 deletions(-)
 create mode 100644 app/services/line_item_syncer.rb

diff --git a/app/services/line_item_syncer.rb b/app/services/line_item_syncer.rb
new file mode 100644
index 0000000000..19281fcd0b
--- /dev/null
+++ b/app/services/line_item_syncer.rb
@@ -0,0 +1,55 @@
+# Responsible for keeping line items on initialised orders for a standing order in sync with
+# the standing line items on that standing order.
+
+class LineItemSyncer
+  def initialize(standing_order, order_update_issues)
+    @standing_order = standing_order
+    @order_update_issues = order_update_issues
+  end
+
+  def sync!(order)
+    update_item_quantities(order)
+    create_new_items(order)
+    destroy_obsolete_items(order)
+  end
+
+  private
+
+  delegate :standing_line_items, to: :standing_order
+
+  attr_reader :standing_order, :order_update_issues
+
+  def update_item_quantities(order)
+    changed_standing_line_items.each do |sli|
+      line_item = order.line_items.find_by_variant_id(sli.variant_id)
+
+      next update_quantity(line_item, sli.quantity) if line_item.quantity == sli.quantity_was
+      next if line_item.quantity == sli.quantity
+
+      product_name = "#{line_item.product.name} - #{line_item.full_name}"
+      order_update_issues.add(order, product_name)
+    end
+  end
+
+  def create_new_items(order)
+    new_standing_line_items.each do |sli|
+      order.line_items.create(variant_id: sli.variant_id, quantity: sli.quantity, skip_stock_check: true)
+    end
+  end
+
+  def destroy_obsolete_items(order)
+    order.line_items.where(variant_id: standing_line_items.select(&:marked_for_destruction?).map(&:variant_id)).destroy_all
+  end
+
+  def changed_standing_line_items
+    standing_line_items.select{ |sli| sli.changed? && sli.persisted? }
+  end
+
+  def new_standing_line_items
+    standing_line_items.select(&:new_record?)
+  end
+
+  def update_quantity(line_item, quantity)
+    line_item.update_attributes(quantity: quantity, skip_stock_check: true)
+  end
+end
diff --git a/app/services/standing_order_updater.rb b/app/services/standing_order_updater.rb
index aaa73e0730..77866d0a60 100644
--- a/app/services/standing_order_updater.rb
+++ b/app/services/standing_order_updater.rb
@@ -7,42 +7,21 @@ class StandingOrderUpdater
   def initialize(standing_order)
     @standing_order = standing_order
     @order_update_issues = OrderUpdateIssues.new
+    @line_item_syncer = LineItemSyncer.new(standing_order, order_update_issues)
   end
 
   def update!
     future_and_undated_orders.all? do |order|
       order.assign_attributes(customer_id: customer_id, email: customer.andand.email, distributor_id: shop_id)
-
-      update_bill_address_for(order) if (bill_address.changes.keys & relevant_address_attrs).any?
-      update_ship_address_for(order) if (ship_address.changes.keys & relevant_address_attrs).any?
-      update_shipment_for(order) if shipping_method_id_changed?
-      update_payment_for(order) if payment_method_id_changed?
-
-      changed_standing_line_items.each do |sli|
-        line_item = order.line_items.find_by_variant_id(sli.variant_id)
-        if line_item.quantity == sli.quantity_was
-          line_item.update_attributes(quantity: sli.quantity, skip_stock_check: true)
-        else
-          unless line_item.quantity == sli.quantity
-            product_name = "#{line_item.product.name} - #{line_item.full_name}"
-            order_update_issues.add(order, product_name)
-          end
-        end
-      end
-
-      new_standing_line_items.each do |sli|
-        order.line_items.create(variant_id: sli.variant_id, quantity: sli.quantity, skip_stock_check: true)
-      end
-
-      order.line_items.where(variant_id: standing_line_items.select(&:marked_for_destruction?).map(&:variant_id)).destroy_all
-
+      update_associations_for(order)
+      line_item_syncer.sync!(order)
       order.save
     end
   end
 
   private
 
-  attr_reader :standing_order
+  attr_reader :standing_order, :line_item_syncer
 
   delegate :orders, :bill_address, :ship_address, :standing_line_items, to: :standing_order
   delegate :shop_id, :customer, :customer_id, to: :standing_order
@@ -50,6 +29,13 @@ class StandingOrderUpdater
   delegate :shipping_method_id_changed?, :shipping_method_id_was, to: :standing_order
   delegate :payment_method_id_changed?, :payment_method_id_was, to: :standing_order
 
+  def update_associations_for(order)
+    update_bill_address_for(order) if (bill_address.changes.keys & relevant_address_attrs).any?
+    update_ship_address_for(order) if (ship_address.changes.keys & relevant_address_attrs).any?
+    update_shipment_for(order) if shipping_method_id_changed?
+    update_payment_for(order) if payment_method_id_changed?
+  end
+
   def future_and_undated_orders
     return @future_and_undated_orders unless @future_and_undated_orders.nil?
     @future_and_undated_orders = orders.joins(:order_cycle).merge(OrderCycle.not_closed).readonly(false)
@@ -95,14 +81,6 @@ class StandingOrderUpdater
     end
   end
 
-  def changed_standing_line_items
-    standing_line_items.select{ |sli| sli.changed? && sli.persisted? }
-  end
-
-  def new_standing_line_items
-    standing_line_items.select(&:new_record?)
-  end
-
   def relevant_address_attrs
     ["firstname", "lastname", "address1", "zipcode", "city", "state_id", "country_id", "phone"]
   end