diff --git a/app/models/spree/order_decorator.rb b/app/models/spree/order_decorator.rb index f12cdb341f..7deccb9607 100644 --- a/app/models/spree/order_decorator.rb +++ b/app/models/spree/order_decorator.rb @@ -334,15 +334,22 @@ Spree::Order.class_eval do errors.add(:base, e.message) and return result end - # Override or Spree method. Used to prevent payments on subscriptions from being processed in the normal way. - # ie. they are 'hidden' from processing logic until after the order cycle has closed. - def pending_payments - return [] if subscription.present? && order_cycle.orders_close_at.andand > Time.zone.now - payments.select {|p| p.state == "checkout"} # Original definition + # Override Spree method to allow unpaid orders to be completed. + # Subscriptions place orders at the beginning of an order cycle. They need to + # be completed to draw from stock levels and trigger emails. + # Spree doesn't allow this. Other options would be to introduce an additional + # order state or implement a special proxy payment method. + # https://github.com/openfoodfoundation/openfoodnetwork/pull/3012#issuecomment-438146484 + def payment_required? + total.to_f > 0.0 && !skip_payment_for_subscription? end private + def skip_payment_for_subscription? + subscription.present? && order_cycle.orders_close_at.andand > Time.zone.now + end + def address_from_distributor address = distributor.address.clone if bill_address diff --git a/spec/jobs/subscription_confirm_job_spec.rb b/spec/jobs/subscription_confirm_job_spec.rb index 024a8bb0ec..3c433bf94d 100644 --- a/spec/jobs/subscription_confirm_job_spec.rb +++ b/spec/jobs/subscription_confirm_job_spec.rb @@ -141,6 +141,7 @@ describe SubscriptionConfirmJob do before do allow(order).to receive(:payment_total) { 0 } allow(order).to receive(:total) { 10 } + allow(order).to receive(:payment_required?) { true } allow(order).to receive(:pending_payments) { [payment] } end diff --git a/spec/models/spree/order_spec.rb b/spec/models/spree/order_spec.rb index ce3a4acea1..e84c225569 100644 --- a/spec/models/spree/order_spec.rb +++ b/spec/models/spree/order_spec.rb @@ -739,13 +739,24 @@ describe Spree::Order do end end - describe "finding pending_payments" do - let!(:order) { create(:order ) } - let!(:payment) { create(:payment, order: order, state: 'checkout') } + describe "payments" do + let(:payment_method) { create(:payment_method) } + let(:shipping_method) { create(:shipping_method) } + let(:order) { create(:order_with_totals) } + + before { order.update_totals } context "when the order is not a subscription" do - it "returns the payments on the order" do - expect(order.reload.pending_payments).to eq [payment] + it "it requires a payment" do + expect(order.payment_required?).to be true + end + + it "advances to payment state" do + advance_to_delivery_state(order) + + order.next! + + expect(order.state).to eq "payment" end end @@ -756,8 +767,8 @@ describe Spree::Order do context "and order_cycle has no order_close_at set" do before { order.order_cycle.update_attributes(orders_close_at: nil) } - it "returns the payments on the order" do - expect(order.reload.pending_payments).to eq [payment] + it "requires a payment" do + expect(order.payment_required?).to be true end end @@ -765,7 +776,7 @@ describe Spree::Order do before { order.order_cycle.update_attributes(orders_close_at: 5.minutes.ago) } it "returns the payments on the order" do - expect(order.reload.pending_payments).to eq [payment] + expect(order.payment_required?).to be true end end @@ -773,10 +784,30 @@ describe Spree::Order do before { order.order_cycle.update_attributes(orders_close_at: 5.minutes.from_now) } it "returns an empty array" do - expect(order.reload.pending_payments).to eq [] + expect(order.payment_required?).to be false + end + + it "skips the payment state" do + advance_to_delivery_state(order) + + order.next! + + expect(order.state).to eq "complete" end end end + + def advance_to_delivery_state(order) + # advance to address state + create(:shipment_with, :shipping_method, shipping_method: shipping_method, order: order) + order.reload + order.ship_address = create(:address) + order.next! + + # advance to delivery state + create(:payment, order: order, payment_method: payment_method) + order.next! + end end describe '#restart_checkout!' do