diff --git a/app/jobs/standing_order_placement_job.rb b/app/jobs/standing_order_placement_job.rb index 681d0d5bc8..00596bfdc0 100644 --- a/app/jobs/standing_order_placement_job.rb +++ b/app/jobs/standing_order_placement_job.rb @@ -6,32 +6,33 @@ class StandingOrderPlacementJob end def perform - orders.each do |order| - process(order) + proxy_orders.each do |proxy_order| + proxy_orders.initialise_order! + process(proxy_order.order) end end private - def orders - Spree::Order.incomplete.where(order_cycle_id: order_cycle) - .merge(StandingOrder.active).joins(:standing_order).readonly(false) + def proxy_orders + ProxyOrder.not_canceled.where(order_cycle_id: order_cycle) + .merge(StandingOrder.active).joins(:standing_order) end def process(order) + return if order.completed? changes = cap_quantity_and_store_changes(order) unless order.completed? until order.completed? - if order.errors.any? + unless order.next! Bugsnag.notify(RuntimeError.new("StandingOrderPlacementError"), { job: "StandingOrderPlacement", error: "Cannot process order due to errors", data: { + order_number: order.number, errors: order.errors.full_messages } }) break - else - order.next! end end send_placement_email(order, changes) @@ -47,6 +48,7 @@ class StandingOrderPlacementJob end def send_placement_email(order, changes) + return unless order.completed? Spree::OrderMailer.standing_order_email(order, 'placement', changes).deliver end end diff --git a/spec/jobs/standing_order_placement_job_spec.rb b/spec/jobs/standing_order_placement_job_spec.rb index 9d820a7edf..aac075f3fb 100644 --- a/spec/jobs/standing_order_placement_job_spec.rb +++ b/spec/jobs/standing_order_placement_job_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' describe StandingOrderPlacementJob do - describe "finding standing_order orders for the specified order cycle" 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) } let(:order_cycle2) { create(:simple_order_cycle, coordinator: shop) } @@ -12,28 +12,19 @@ describe StandingOrderPlacementJob do let(:standing_order3) { create(:standing_order, shop: shop, schedule: schedule, canceled_at: 1.minute.ago) } let(:standing_order4) { create(:standing_order, shop: shop, schedule: schedule, begins_at: 1.minute.from_now) } let(:standing_order5) { create(:standing_order, shop: shop, schedule: schedule, ends_at: 1.minute.ago) } - let!(:order1) { create(:order, completed_at: 5.minutes.ago) } # Complete + Linked + OC Matches - let!(:order2) { create(:order) } # Incomplete + Not-Linked + OC Matches - let!(:order3) { create(:order) } # Incomplete + Linked + OC Mismatch - let!(:order4) { create(:order) } # Incomplete + Linked + OC Matches + Paused - let!(:order5) { create(:order) } # Incomplete + Linked + OC Matches + Cancelled - let!(:order6) { create(:order) } # Incomplete + Linked + OC Matches + Yet To Begin - let!(:order7) { create(:order) } # Incomplete + Linked + OC Matches + Ended - let!(:order8) { create(:order) } # Incomplete + Linked + OC Matches - let!(:proxy_order1) { create(:proxy_order, standing_order: standing_order1, order: order1, order_cycle: order_cycle1) } - let!(:proxy_order3) { create(:proxy_order, standing_order: standing_order1, order: order3, order_cycle: order_cycle2) } - let!(:proxy_order4) { create(:proxy_order, standing_order: standing_order2, order: order4, order_cycle: order_cycle1) } - let!(:proxy_order5) { create(:proxy_order, standing_order: standing_order3, order: order5, order_cycle: order_cycle1) } - let!(:proxy_order6) { create(:proxy_order, standing_order: standing_order4, order: order6, order_cycle: order_cycle1) } - let!(:proxy_order7) { create(:proxy_order, standing_order: standing_order5, order: order7, order_cycle: order_cycle1) } - let!(:proxy_order8) { create(:proxy_order, standing_order: standing_order1, order: order8, order_cycle: order_cycle1) } + 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) } # Yet To Begin + let!(:proxy_order5) { create(:proxy_order, standing_order: standing_order5, order_cycle: order_cycle1) } # Ended + let!(:proxy_order6) { create(:proxy_order, standing_order: standing_order1, order_cycle: order_cycle1) } # OK let!(:job) { StandingOrderPlacementJob.new(order_cycle1) } - 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 order8 - expect(orders).to_not include order1, order2, order3, order4, order5, order6, order7 + 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 @@ -77,23 +68,66 @@ describe StandingOrderPlacementJob do before do expect_any_instance_of(Spree::Payment).to_not receive(:process!) allow(job).to receive(:cap_quantity_and_store_changes) { changes } - allow(job).to receive(:send_placement_email).and_call_original + allow(job).to receive(:send_placement_email) end - it "moves orders to completion, but does not process the payment" do - # If this spec starts complaining about no shipping methods being available - # on CI, there is probably another spec resetting the currency though Rails.cache.clear - ActionMailer::Base.deliveries.clear - expect{job.send(:process, order)}.to change{order.reload.completed_at}.from(nil) - expect(order.completed_at).to be_within(5.seconds).of Time.now - expect(order.payments.first.state).to eq "checkout" + context "when the order is already complete" do + before { while !order.completed? do break unless order.next! end } + + it "ignores it" do + ActionMailer::Base.deliveries.clear + expect{job.send(:process, order)}.to_not change{order.reload.state} + expect(order.payments.first.state).to eq "checkout" + expect(ActionMailer::Base.deliveries.count).to be 0 + end end - it "sends only a placement email, no confirmation emails" do - ActionMailer::Base.deliveries.clear - expect{job.send(:process, order)}.to_not enqueue_job ConfirmOrderJob - expect(job).to have_received(:send_placement_email).with(order, changes).once - expect(ActionMailer::Base.deliveries.count).to be 1 + context "when the order is not already complete" do + it "processes the order to completion, but does not process the payment" do + # If this spec starts complaining about no shipping methods being available + # on CI, there is probably another spec resetting the currency though Rails.cache.clear + ActionMailer::Base.deliveries.clear + expect{job.send(:process, order)}.to change{order.reload.completed_at}.from(nil) + expect(order.completed_at).to be_within(5.seconds).of Time.now + expect(order.payments.first.state).to eq "checkout" + end + + it "does not enqueue confirmation emails" do + expect{job.send(:process, order)}.to_not enqueue_job ConfirmOrderJob + expect(job).to have_received(:send_placement_email).with(order, changes).once + end + end + end + + describe "sending placement email" do + let(:standing_order) { create(:standing_order, with_items: true) } + let(:proxy_order) { create(:proxy_order, standing_order: standing_order) } + let!(:order) { proxy_order.initialise_order! } + let(:mail_mock) { double(:mailer_mock) } + + let!(:job) { StandingOrderPlacementJob.new(proxy_order.order_cycle) } + + before do + allow(Spree::OrderMailer).to receive(:standing_order_email) { mail_mock } + allow(mail_mock).to receive(:deliver) + end + + context "when the order is complete" do + before { while !order.completed? do break unless order.next! end } + + it "sends the email" do + job.send(:send_placement_email, order, {}) + expect(Spree::OrderMailer).to have_received(:standing_order_email) + expect(mail_mock).to have_received(:deliver) + end + end + + context "when the order is incomplete" do + it "does not send the email" do + job.send(:send_placement_email, order, {}) + expect(Spree::OrderMailer).to_not have_received(:standing_order_email) + expect(mail_mock).to_not have_received(:deliver) + end end end end