From 2859075e43590980f00b308e886ba67a0eaed917 Mon Sep 17 00:00:00 2001 From: Rob Harrington Date: Fri, 23 Dec 2016 08:21:43 +1100 Subject: [PATCH] WIP: StandingOrderConfirmJob no longer scoped to an order cycle --- app/jobs/order_cycle_open_close_job.rb | 29 +----- app/jobs/standing_order_confirm_job.rb | 21 ++-- spec/jobs/order_cycle_open_close_job_spec.rb | 65 +----------- spec/jobs/standing_order_confirm_job_spec.rb | 102 +++++++++++++++++-- 4 files changed, 115 insertions(+), 102 deletions(-) diff --git a/app/jobs/order_cycle_open_close_job.rb b/app/jobs/order_cycle_open_close_job.rb index 728b786771..2ef7736f23 100644 --- a/app/jobs/order_cycle_open_close_job.rb +++ b/app/jobs/order_cycle_open_close_job.rb @@ -1,33 +1,6 @@ class OrderCycleOpenCloseJob def perform Delayed::Job.enqueue(StandingOrderPlacementJob.new) - - 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 - 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 - 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 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) + Delayed::Job.enqueue(StandingOrderConfirmJob.new) end end diff --git a/app/jobs/standing_order_confirm_job.rb b/app/jobs/standing_order_confirm_job.rb index 182197a31c..b17dc01dc4 100644 --- a/app/jobs/standing_order_confirm_job.rb +++ b/app/jobs/standing_order_confirm_job.rb @@ -1,18 +1,21 @@ class StandingOrderConfirmJob - attr_accessor :proxy_orders - - def initialize(proxy_orders) - @proxy_orders = proxy_orders - end - def perform - proxy_orders.each do |proxy_order| + ids = proxy_orders.pluck(:id) + proxy_orders.update_all(confirmed_at: Time.now) + ProxyOrder.where(id: ids).each do |proxy_order| process(proxy_order.order) end end private + def proxy_orders + ProxyOrder.not_canceled.where('confirmed_at IS NULL AND placed_at IS NOT NULL') + .joins(:order_cycle).merge(recently_closed_order_cycles) + .joins(:standing_order).merge(StandingOrder.not_canceled.not_paused) + .joins(:order).merge(Spree::Order.complete) + end + def process(order) send_confirm_email(order) end @@ -20,4 +23,8 @@ class StandingOrderConfirmJob def send_confirm_email(order) Spree::OrderMailer.standing_order_email(order.id, 'confirmation', {}).deliver end + + def recently_closed_order_cycles + OrderCycle.closed.where('order_cycles.orders_close_at BETWEEN (?) AND (?) OR order_cycles.updated_at BETWEEN (?) AND (?)', 1.hour.ago, Time.now, 1.hour.ago, Time.now) + end end diff --git a/spec/jobs/order_cycle_open_close_job_spec.rb b/spec/jobs/order_cycle_open_close_job_spec.rb index a750ccf3b1..b3af22d881 100644 --- a/spec/jobs/order_cycle_open_close_job_spec.rb +++ b/spec/jobs/order_cycle_open_close_job_spec.rb @@ -1,72 +1,15 @@ require 'spec_helper' describe OrderCycleOpenCloseJob do - let!(:job) { OrderCycleOpenCloseJob.new } - - describe "finding recently closed order cycles" 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: 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 - 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 + let(:job) { OrderCycleOpenCloseJob.new } describe "running the job" do - it "enqueues a StandingOrderPlacementJob for each recently opened order_cycle" do + it "enqueues a StandingOrderPlacementJob" do expect{job.perform}.to enqueue_job StandingOrderPlacementJob end - 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) } - - before do - allow(job).to receive(:confirmable_proxy_orders_for) { ProxyOrder.where(id: proxy_order.id) } - end - - 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 + it "enqueues a StandingOrderConfirmJob" do + expect{job.perform}.to enqueue_job StandingOrderConfirmJob end end end diff --git a/spec/jobs/standing_order_confirm_job_spec.rb b/spec/jobs/standing_order_confirm_job_spec.rb index 9333d67d5e..f1d3894a53 100644 --- a/spec/jobs/standing_order_confirm_job_spec.rb +++ b/spec/jobs/standing_order_confirm_job_spec.rb @@ -1,15 +1,105 @@ require 'spec_helper' describe StandingOrderConfirmJob do - let(:shop) { create(:distributor_enterprise) } - let(:order_cycle1) { create(:simple_order_cycle, coordinator: shop) } - 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 } - let!(:job) { StandingOrderConfirmJob.new(order_cycle1) } + describe "finding proxy_orders that are ready to be confirmed" do + let(:shop) { create(:distributor_enterprise) } + let(:order_cycle1) { create(:simple_order_cycle, coordinator: shop, orders_close_at: 59.minutes.ago, updated_at: 1.day.ago) } + let(:order_cycle2) { create(:simple_order_cycle, coordinator: shop, orders_close_at: 61.minutes.ago, updated_at: 1.day.ago) } + let(:schedule) { create(:schedule, order_cycles: [order_cycle1, order_cycle2]) } + let(:standing_order) { create(:standing_order, shop: shop, schedule: schedule) } + let!(:proxy_order) { create(:proxy_order, standing_order: standing_order, order_cycle: order_cycle1, placed_at: 5.minutes.ago, order: create(:order, completed_at: 1.minute.ago)) } + let(:proxy_orders) { job.send(:proxy_orders) } + + it "returns proxy orders that meet all of the criteria" do + expect(proxy_orders).to include proxy_order + end + + it "ignores proxy orders where the OC closed more than 1 hour ago" do + proxy_order.update_attributes!(order_cycle_id: order_cycle2.id) + expect(proxy_orders).to_not include proxy_order + end + + it "ignores proxy orders for paused standing orders" do + standing_order.update_attributes!(paused_at: 1.minute.ago) + expect(proxy_orders).to_not include proxy_order + end + + it "ignores proxy orders for cancelled standing orders" do + standing_order.update_attributes!(canceled_at: 1.minute.ago) + expect(proxy_orders).to_not include proxy_order + end + + it "ignores cancelled proxy orders" do + proxy_order.update_attributes!(canceled_at: 5.minute.ago) + expect(proxy_orders).to_not include proxy_order + end + + it "ignores proxy orders without a completed order" do + proxy_order.order.completed_at = nil + proxy_order.order.save! + expect(proxy_orders).to_not include proxy_order + end + + it "ignores proxy orders without an associated order" do + proxy_order.update_attributes!(order_id: nil) + expect(proxy_orders).to_not include proxy_order + end + + it "ignores proxy orders that haven't been placed yet" do + proxy_order.update_attributes!(placed_at: nil) + expect(proxy_orders).to_not include proxy_order + end + + it "ignores proxy orders that have already been confirmed" do + proxy_order.update_attributes!(confirmed_at: 1.second.ago) + expect(proxy_orders).to_not include proxy_order + end + end + + describe "performing the job" do + context "when unconfirmed proxy_orders exist" do + let!(:proxy_order) { create(:proxy_order) } + + before do + proxy_order.initialise_order! + allow(job).to receive(:proxy_orders) { ProxyOrder.where(id: proxy_order.id) } + allow(job).to receive(:process) + end + + 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 "processes confirmable proxy_orders" do + job.perform + expect(job).to have_received(:process).with(proxy_order.reload.order) + end + end + end + + describe "finding recently closed order cycles" do + let!(:order_cycle1) { create(:simple_order_cycle, orders_close_at: 61.minutes.ago, updated_at: 61.minutes.ago) } + let!(:order_cycle2) { create(:simple_order_cycle, orders_close_at: nil, updated_at: 59.minutes.ago) } + let!(:order_cycle3) { create(:simple_order_cycle, orders_close_at: 61.minutes.ago, updated_at: 59.minutes.ago) } + let!(:order_cycle4) { create(:simple_order_cycle, orders_close_at: 59.minutes.ago, updated_at: 61.minutes.ago) } + let!(:order_cycle5) { create(:simple_order_cycle, orders_close_at: 1.minute.from_now) } + + it "returns closed order cycles whose orders_close_at or updated_at date is within the last hour" do + order_cycles = job.send(:recently_closed_order_cycles) + expect(order_cycles).to include order_cycle3, order_cycle4 + expect(order_cycles).to_not include order_cycle1, order_cycle2, order_cycle5 + end + end describe "processing an order" do + let(:shop) { create(:distributor_enterprise) } + let(:order_cycle1) { create(:simple_order_cycle, coordinator: shop) } + 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(:proxy_order) { create(:proxy_order, standing_order: standing_order1) } let(:order) { proxy_order.initialise_order! }