WIP: Refactor, of standing order placement and confirmation jobs

ie. Transfering responsibility for tracking standing order placement and confirmation to proxy orders

app/jobs/order_cycle_open_close_job.rb

@@ -1,35 +1,46 @@
 class OrderCycleOpenCloseJob
   def perform
-    order_cycles = recently_opened_order_cycles.all
-    recently_opened_order_cycles.update_all(standing_orders_placed_at: Time.now)
-    order_cycles.each do |order_cycle|
-      Delayed::Job.enqueue(StandingOrderPlacementJob.new(order_cycle))
+    recently_opened_order_cycles.each do |order_cycle|
+      proxy_orders = placeable_proxy_orders_for(order_cycle).all
+      if proxy_orders.any?
+        placeable_proxy_orders_for(order_cycle).update_all(placed_at: Time.now)
+        Delayed::Job.enqueue(StandingOrderPlacementJob.new(proxy_orders))
+      end
     end
 
-    order_cycles = recently_closed_order_cycles.all
-    recently_closed_order_cycles.update_all(standing_orders_confirmed_at: Time.now)
-    order_cycles.each do |order_cycle|
-      Delayed::Job.enqueue(StandingOrderConfirmJob.new(order_cycle))
+    recently_closed_order_cycles.each do |order_cycle|
+      proxy_orders = confirmable_proxy_orders_for(order_cycle).all
+      if proxy_orders.any?
+        confirmable_proxy_orders_for(order_cycle).update_all(confirmed_at: Time.now)
+        Delayed::Job.enqueue(StandingOrderConfirmJob.new(proxy_orders))
+      end
     end
   end
 
   private
 
   def recently_opened_order_cycles
-    return @recently_opened_order_cycles unless @recently_opened_order_cycles.nil?
-    @recently_opened_order_cycles =
-      OrderCycle.active.where(
-        'standing_orders_placed_at IS NULL AND (orders_open_at BETWEEN (?) AND (?) OR updated_at BETWEEN (?) AND (?))',
-        10.minutes.ago, Time.now, 10.minutes.ago, Time.now
-      )
+    OrderCycle.active.where('(orders_open_at BETWEEN (?) AND (?) OR updated_at BETWEEN (?) AND (?))', 10.minutes.ago, Time.now, 10.minutes.ago, Time.now)
   end
 
   def recently_closed_order_cycles
-    return @recently_closed_order_cycles unless @recently_closed_order_cycles.nil?
-    @recently_closed_order_cycles =
-      OrderCycle.closed.where(
-        'standing_orders_confirmed_at IS NULL AND (orders_close_at BETWEEN (?) AND (?) OR updated_at BETWEEN (?) AND (?))',
-        10.minutes.ago, Time.now, 10.minutes.ago, Time.now
-      )
+    OrderCycle.closed.where('(orders_close_at BETWEEN (?) AND (?) OR updated_at BETWEEN (?) AND (?))', 10.minutes.ago, Time.now, 10.minutes.ago, Time.now)
+  end
+
+  def placeable_proxy_orders_for(order_cycle)
+    # Load proxy orders for standing orders whose begins at date may between now and the order cycle close date
+    # Does not load proxy orders for standing orders who ends_at date is before order_cycle close
+    ProxyOrder.not_canceled.where(order_cycle_id: order_cycle, placed_at: nil)
+    .where('begins_at < ? AND (ends_at IS NULL OR ends_at > ?)', order_cycle.orders_close_at, order_cycle.orders_close_at)
+    .joins(:standing_order).merge(StandingOrder.not_canceled.not_paused)
+    .readonly(false)
+  end
+
+  def confirmable_proxy_orders_for(order_cycle)
+    ProxyOrder.not_canceled.where(order_cycle_id: order_cycle, confirmed_at: nil).where('placed_at IS NOT NULL')
+    .where('begins_at < ? AND (ends_at IS NULL OR ends_at > ?)', order_cycle.orders_close_at, order_cycle.orders_close_at)
+    .joins(:standing_order).merge(StandingOrder.not_canceled.not_paused)
+    .joins(:order).merge(Spree::Order.complete)
+    .readonly(false)
   end
 end

app/jobs/standing_order_confirm_job.rb

@@ -1,23 +1,18 @@
 class StandingOrderConfirmJob
-  attr_accessor :order_cycle
+  attr_accessor :proxy_orders
 
-  def initialize(order_cycle)
-    @order_cycle = order_cycle
+  def initialize(proxy_orders)
+    @proxy_orders = proxy_orders
   end
 
   def perform
-    orders.each do |order|
-      process(order)
+    proxy_orders.each do |proxy_order|
+      process(proxy_order.order)
     end
   end
 
   private
 
-  def orders
-    Spree::Order.complete.where(order_cycle_id: order_cycle)
-    .merge(ProxyOrder.not_canceled).joins(:proxy_order).readonly(false)
-  end
-
   def process(order)
     send_confirm_email(order)
   end

app/jobs/standing_order_placement_job.rb

@@ -1,8 +1,8 @@
 class StandingOrderPlacementJob
-  attr_accessor :order_cycle
+  attr_accessor :proxy_orders
 
-  def initialize(order_cycle)
-    @order_cycle = order_cycle
+  def initialize(proxy_orders)
+    @proxy_orders = proxy_orders
   end
 
   def perform
@@ -14,14 +14,6 @@ class StandingOrderPlacementJob
 
   private
 
-  def proxy_orders
-    # Load proxy orders for standing orders whose begins at date may between now and the order cycle close date
-    # Does not load proxy orders for standing orders who ends_at date is before order_cycle close
-    ProxyOrder.not_canceled.where(order_cycle_id: order_cycle)
-    .where('begins_at < ? AND (ends_at IS NULL OR ends_at > ?)', order_cycle.orders_close_at, order_cycle.orders_close_at)
-    .merge(StandingOrder.not_canceled.not_paused).joins(:standing_order).readonly(false)
-  end
-
   def process(order)
     return if order.completed?
     changes = cap_quantity_and_store_changes(order) unless order.completed?
@@ -64,7 +56,7 @@ class StandingOrderPlacementJob
   end
 
   def available_variants_for(order)
-    DistributionChangeValidator.new(order).variants_available_for_distribution(order.distributor, order_cycle)
+    DistributionChangeValidator.new(order).variants_available_for_distribution(order.distributor, order.order_cycle)
   end
 
   def send_placement_email(order, changes)

spec/jobs/order_cycle_open_close_job_spec.rb

@@ -7,13 +7,12 @@ describe OrderCycleOpenCloseJob do
     let!(:order_cycle1) { create(:simple_order_cycle, orders_open_at: 11.minutes.ago, updated_at: 11.minutes.ago) }
     let!(:order_cycle2) { create(:simple_order_cycle, orders_open_at: 11.minutes.ago, updated_at: 9.minutes.ago) }
     let!(:order_cycle3) { create(:simple_order_cycle, orders_open_at: 9.minutes.ago, updated_at: 9.minutes.ago) }
-    let!(:order_cycle4) { create(:simple_order_cycle, orders_open_at: 2.minutes.ago, standing_orders_placed_at: 1.minute.ago ) }
-    let!(:order_cycle5) { create(:simple_order_cycle, orders_open_at: 1.minute.from_now) }
+    let!(:order_cycle4) { create(:simple_order_cycle, orders_open_at: 1.minute.from_now) }
 
     it "returns unprocessed order cycles whose orders_open_at or updated_at date is within the past 10 minutes" do
       order_cycles = job.send(:recently_opened_order_cycles)
       expect(order_cycles).to include order_cycle2, order_cycle3
-      expect(order_cycles).to_not include order_cycle1, order_cycle4, order_cycle5
+      expect(order_cycles).to_not include order_cycle1, order_cycle4
     end
   end
 
@@ -21,23 +20,83 @@ describe OrderCycleOpenCloseJob do
     let!(:order_cycle1) { create(:simple_order_cycle, orders_close_at: 11.minutes.ago, updated_at: 11.minutes.ago) }
     let!(:order_cycle2) { create(:simple_order_cycle, orders_close_at: 11.minutes.ago, updated_at: 9.minutes.ago) }
     let!(:order_cycle3) { create(:simple_order_cycle, orders_close_at: 9.minutes.ago, updated_at: 9.minutes.ago) }
-    let!(:order_cycle4) { create(:simple_order_cycle, orders_close_at: 2.minutes.ago, standing_orders_confirmed_at: 1.minute.ago ) }
-    let!(:order_cycle5) { create(:simple_order_cycle, orders_close_at: 1.minute.from_now) }
+    let!(:order_cycle4) { create(:simple_order_cycle, orders_close_at: 1.minute.from_now) }
 
     it "returns unprocessed order cycles whose orders_close_at or updated_at date is within the past 10 minutes" do
       order_cycles = job.send(:recently_closed_order_cycles)
       expect(order_cycles).to include order_cycle2, order_cycle3
-      expect(order_cycles).to_not include order_cycle1, order_cycle4, order_cycle5
+      expect(order_cycles).to_not include order_cycle1, order_cycle4
+    end
+  end
+
+  describe "finding placeable proxy_orders for a particular order cycle" do
+    let(:shop) { create(:distributor_enterprise) }
+    let(:order_cycle1) { create(:simple_order_cycle, coordinator: shop, orders_close_at: 10.minutes.from_now) }
+    let(:order_cycle2) { create(:simple_order_cycle, coordinator: shop) }
+    let(:schedule) { create(:schedule, order_cycles: [order_cycle1, order_cycle2]) }
+    let(:standing_order1) { create(:standing_order, shop: shop, schedule: schedule, begins_at: 9.minutes.from_now, ends_at: 11.minutes.from_now) }
+    let(:standing_order2) { create(:standing_order, shop: shop, schedule: schedule, begins_at: 9.minutes.from_now, ends_at: 11.minutes.from_now, paused_at: 1.minute.ago) }
+    let(:standing_order3) { create(:standing_order, shop: shop, schedule: schedule, begins_at: 9.minutes.from_now, ends_at: 11.minutes.from_now, canceled_at: 1.minute.ago) }
+    let(:standing_order4) { create(:standing_order, shop: shop, schedule: schedule, begins_at: 11.minutes.from_now, ends_at: 20.minutes.from_now) }
+    let(:standing_order5) { create(:standing_order, shop: shop, schedule: schedule, begins_at: 1.minute.ago, ends_at: 9.minutes.from_now) }
+    let!(:proxy_order1) { create(:proxy_order, standing_order: standing_order1, order_cycle: order_cycle2) } # OC Mismatch
+    let!(:proxy_order2) { create(:proxy_order, standing_order: standing_order2, order_cycle: order_cycle1) } # Standing Order Paused
+    let!(:proxy_order3) { create(:proxy_order, standing_order: standing_order3, order_cycle: order_cycle1) } # Standing Order Cancelled
+    let!(:proxy_order4) { create(:proxy_order, standing_order: standing_order4, order_cycle: order_cycle1) } # Standing Order Begins after OC close
+    let!(:proxy_order5) { create(:proxy_order, standing_order: standing_order5, order_cycle: order_cycle1) } # Standing Order Ends before OC close
+    let!(:proxy_order6) { create(:proxy_order, standing_order: standing_order1, order_cycle: order_cycle1, canceled_at: 5.minutes.ago) } # Cancelled
+    let!(:proxy_order7) { create(:proxy_order, standing_order: standing_order1, order_cycle: order_cycle1, placed_at: 5.minutes.ago) } # Already placed
+    let!(:proxy_order8) { create(:proxy_order, standing_order: standing_order1, order_cycle: order_cycle1) } # OK
+
+    it "only returns not_canceled proxy_orders for the relevant order cycle" do
+      proxy_orders = job.send(:placeable_proxy_orders_for, order_cycle1)
+      expect(proxy_orders).to include proxy_order8
+      expect(proxy_orders).to_not include proxy_order1, proxy_order2, proxy_order3, proxy_order4, proxy_order5, proxy_order6, proxy_order7
+    end
+  end
+
+  describe "finding confirmable proxy orders for a particular order cycle" do
+    let(:shop) { create(:distributor_enterprise) }
+    let(:order_cycle1) { create(:simple_order_cycle, coordinator: shop, orders_close_at: 10.minutes.from_now) }
+    let(:order_cycle2) { create(:simple_order_cycle, coordinator: shop) }
+    let(:schedule) { create(:schedule, order_cycles: [order_cycle1, order_cycle2]) }
+    let(:standing_order1) { create(:standing_order, shop: shop, schedule: schedule, begins_at: 9.minutes.from_now, ends_at: 11.minutes.from_now) }
+    let(:standing_order2) { create(:standing_order, shop: shop, schedule: schedule, begins_at: 9.minutes.from_now, ends_at: 11.minutes.from_now, paused_at: 1.minute.ago) }
+    let(:standing_order3) { create(:standing_order, shop: shop, schedule: schedule, begins_at: 9.minutes.from_now, ends_at: 11.minutes.from_now, canceled_at: 1.minute.ago) }
+    let(:standing_order4) { create(:standing_order, shop: shop, schedule: schedule, begins_at: 11.minutes.from_now, ends_at: 20.minutes.from_now) }
+    let(:standing_order5) { create(:standing_order, shop: shop, schedule: schedule, begins_at: 1.minute.ago, ends_at: 9.minutes.from_now) }
+    let!(:proxy_order1) { create(:proxy_order, standing_order: standing_order1, order_cycle: order_cycle2, placed_at: 5.minutes.ago, order: create(:order, completed_at: 1.minute.ago)) } # OC Mismatch
+    let!(:proxy_order2) { create(:proxy_order, standing_order: standing_order2, order_cycle: order_cycle1, placed_at: 5.minutes.ago, order: create(:order, completed_at: 1.minute.ago)) } # Standing Order Paused
+    let!(:proxy_order3) { create(:proxy_order, standing_order: standing_order3, order_cycle: order_cycle1, placed_at: 5.minutes.ago, order: create(:order, completed_at: 1.minute.ago)) } # Standing Order Cancelled
+    let!(:proxy_order4) { create(:proxy_order, standing_order: standing_order4, order_cycle: order_cycle1, placed_at: 5.minutes.ago, order: create(:order, completed_at: 1.minute.ago)) } # Standing Order Begins after OC close
+    let!(:proxy_order5) { create(:proxy_order, standing_order: standing_order5, order_cycle: order_cycle1, placed_at: 5.minutes.ago, order: create(:order, completed_at: 1.minute.ago)) } # Standing Order Ends before OC close
+    let!(:proxy_order6) { create(:proxy_order, standing_order: standing_order1, order_cycle: order_cycle1, placed_at: 5.minutes.ago, order: create(:order, completed_at: 1.minute.ago), canceled_at: 1.minute.ago) } # Cancelled
+    let!(:proxy_order7) { create(:proxy_order, standing_order: standing_order1, order_cycle: order_cycle1, placed_at: 5.minutes.ago, order: create(:order)) } # Order Incomplete
+    let!(:proxy_order8) { create(:proxy_order, standing_order: standing_order1, order_cycle: order_cycle1, placed_at: 5.minutes.ago, order: nil) } # No Order
+    let!(:proxy_order9) { create(:proxy_order, standing_order: standing_order1, order_cycle: order_cycle1, placed_at: nil, order: create(:order, completed_at: 1.minute.ago)) } # Not Placed
+    let!(:proxy_order10) { create(:proxy_order, standing_order: standing_order1, order_cycle: order_cycle1, placed_at: 5.minutes.ago, confirmed_at: 5.minutes.ago, order: create(:order, completed_at: 1.minute.ago)) } # Already Confirmed
+    let!(:proxy_order11) { create(:proxy_order, standing_order: standing_order1, order_cycle: order_cycle1, placed_at: 5.minutes.ago, order: create(:order, completed_at: 1.minute.ago)) } # OK
+
+    it "only returns incomplete orders in the relevant order cycle that are linked to a standing order" do
+      proxy_orders = job.send(:confirmable_proxy_orders_for, order_cycle1)
+      expect(proxy_orders).to include proxy_order11
+      expect(proxy_orders).to_not include proxy_order1, proxy_order2, proxy_order3, proxy_order4, proxy_order5
+      expect(proxy_orders).to_not include proxy_order6, proxy_order7, proxy_order8, proxy_order9, proxy_order10
     end
   end
 
   describe "running the job" do
     context "when an order cycle has just opened" do
       let!(:order_cycle) { create(:simple_order_cycle, orders_open_at: 5.minutes.ago) }
+      let!(:proxy_order) { create(:proxy_order, order_cycle: order_cycle) }
 
-      it "marks the order cycle as processed by setting standing_orders_placed_at" do
-        expect{job.perform}.to change{order_cycle.reload.standing_orders_placed_at}
-        expect(order_cycle.standing_orders_placed_at).to be_within(5.seconds).of Time.now
+      before do
+        allow(job).to receive(:placeable_proxy_orders_for) { ProxyOrder.where(id: proxy_order.id) }
+      end
+
+      it "marks placeable proxy_orders as processed by setting placed_at" do
+        expect{job.perform}.to change{proxy_order.reload.placed_at}
+        expect(proxy_order.placed_at).to be_within(5.seconds).of Time.now
       end
 
       it "enqueues a StandingOrderPlacementJob for each recently opened order_cycle" do
@@ -47,13 +106,18 @@ describe OrderCycleOpenCloseJob do
 
     context "when an order cycle has just closed" do
       let!(:order_cycle) { create(:simple_order_cycle, orders_close_at: 5.minutes.ago) }
+      let!(:proxy_order) { create(:proxy_order, order_cycle: order_cycle) }
 
-      it "marks the order cycle as processed by setting standing_orders_placed_at" do
-        expect{job.perform}.to change{order_cycle.reload.standing_orders_confirmed_at}
-        expect(order_cycle.standing_orders_confirmed_at).to be_within(5.seconds).of Time.now
+      before do
+        allow(job).to receive(:confirmable_proxy_orders_for) { ProxyOrder.where(id: proxy_order.id) }
       end
 
-      it "enqueues a StandingOrderPlacementJob for each recently opened order_cycle" do
+      it "marks confirmable proxy_orders as processed by setting confirmed_at" do
+        expect{job.perform}.to change{proxy_order.reload.confirmed_at}
+        expect(proxy_order.confirmed_at).to be_within(5.seconds).of Time.now
+      end
+
+      it "enqueues a StandingOrderPlacementJob for each recently closed order_cycle" do
         expect{job.perform}.to enqueue_job StandingOrderConfirmJob
       end
     end

spec/jobs/standing_order_confirm_job_spec.rb

@@ -6,26 +6,9 @@ describe StandingOrderConfirmJob do
   let(:order_cycle2) { create(:simple_order_cycle, coordinator: shop) }
   let(:schedule1) { create(:schedule, order_cycles: [order_cycle1, order_cycle2]) }
   let(:standing_order1) { create(:standing_order, shop: shop, schedule: schedule1, with_items: true) }
+
   let!(:job) { StandingOrderConfirmJob.new(order_cycle1) }
 
-  describe "finding standing_order orders for the specified order cycle" do
-    let(:order1) { create(:order) } # Incomplete + Linked + OC Matches
-    let(:order2) { create(:order, completed_at: 5.minutes.ago) } # Complete + Not-Linked + OC Matches
-    let(:order3) { create(:order, completed_at: 5.minutes.ago) } # Complete + Linked + OC Mismatch
-    let(:order4) { create(:order, completed_at: 5.minutes.ago) } # Complete + Linked + OC Matches + Cancelled
-    let(:order5) { create(:order, completed_at: 5.minutes.ago) } # Complete + Linked + OC Matches
-    let!(:proxy_order1) { create(:proxy_order, order_cycle: order_cycle1, standing_order: standing_order1, order: order1) }
-    let!(:proxy_order3) { create(:proxy_order, order_cycle: order_cycle2, standing_order: standing_order1, order: order3) }
-    let!(:proxy_order4) { create(:proxy_order, order_cycle: order_cycle1, standing_order: standing_order1, order: order4, canceled_at: 1.minute.ago) }
-    let!(:proxy_order5) { create(:proxy_order, order_cycle: order_cycle1, standing_order: standing_order1, order: order5) }
-
-    it "only returns incomplete orders in the relevant order cycle that are linked to a standing order" do
-      orders = job.send(:orders)
-      expect(orders).to include order5
-      expect(orders).to_not include order1, order2, order3, order4
-    end
-  end
-
   describe "processing an order" do
     let(:proxy_order) { create(:proxy_order, standing_order: standing_order1) }
     let(:order) { proxy_order.initialise_order! }

spec/jobs/standing_order_placement_job_spec.rb

@@ -1,33 +1,6 @@
 require 'spec_helper'
 
 describe StandingOrderPlacementJob do
-
-  describe "finding proxy_orders for the specified order cycle" do
-    let(:shop) { create(:distributor_enterprise) }
-    let(:order_cycle1) { create(:simple_order_cycle, coordinator: shop, orders_close_at: 10.minutes.from_now) }
-    let(:order_cycle2) { create(:simple_order_cycle, coordinator: shop) }
-    let(:schedule) { create(:schedule, order_cycles: [order_cycle1, order_cycle2]) }
-    let(:standing_order1) { create(:standing_order, shop: shop, schedule: schedule, begins_at: 9.minutes.from_now, ends_at: 11.minutes.from_now) }
-    let(:standing_order2) { create(:standing_order, shop: shop, schedule: schedule, begins_at: 9.minutes.from_now, ends_at: 11.minutes.from_now, paused_at: 1.minute.ago) }
-    let(:standing_order3) { create(:standing_order, shop: shop, schedule: schedule, begins_at: 9.minutes.from_now, ends_at: 11.minutes.from_now, canceled_at: 1.minute.ago) }
-    let(:standing_order4) { create(:standing_order, shop: shop, schedule: schedule, begins_at: 11.minutes.from_now, ends_at: 20.minutes.from_now) }
-    let(:standing_order5) { create(:standing_order, shop: shop, schedule: schedule, begins_at: 1.minute.ago, ends_at: 9.minutes.from_now) }
-    let!(:proxy_order1) { create(:proxy_order, standing_order: standing_order1, order_cycle: order_cycle2) } # OC Mismatch
-    let!(:proxy_order2) { create(:proxy_order, standing_order: standing_order2, order_cycle: order_cycle1) } # Paused
-    let!(:proxy_order3) { create(:proxy_order, standing_order: standing_order3, order_cycle: order_cycle1) } # Cancelled
-    let!(:proxy_order4) { create(:proxy_order, standing_order: standing_order4, order_cycle: order_cycle1) } # Begins after OC close
-    let!(:proxy_order5) { create(:proxy_order, standing_order: standing_order5, order_cycle: order_cycle1) } # Ends before OC close
-    let!(:proxy_order6) { create(:proxy_order, standing_order: standing_order1, order_cycle: order_cycle1) } # OK
-
-    let!(:job) { StandingOrderPlacementJob.new(order_cycle1) }
-
-    it "only returns not_canceled proxy_orders for the relevant order cycle" do
-      proxy_orders = job.send(:proxy_orders)
-      expect(proxy_orders).to include proxy_order6
-      expect(proxy_orders).to_not include proxy_order1, proxy_order2, proxy_order3, proxy_order4, proxy_order5
-    end
-  end
-
   describe "checking that line items are available to purchase" do
     let(:order_cycle) { create(:simple_order_cycle) }
     let(:shop) { order_cycle.coordinator }