From e7450ec7b566a7d64c9799b09150f9a2f1564590 Mon Sep 17 00:00:00 2001 From: Jordan Brough Date: Thu, 3 Sep 2015 10:49:02 -0600 Subject: [PATCH] Use destroy_all instead of delete_all when deleting shipments In Order#ensure_available_shipping_rates. We should trigger ActiveRecord callbacks unless we have a good reason not to. In this case at least the `dependent: :delete_all` on inventory units was being skipped. This caused foreign key errors in our app since we have foreign keys set up. --- core/app/models/spree/order.rb | 2 +- core/spec/models/spree/order/checkout_spec.rb | 16 ++++++++++++++++ 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/core/app/models/spree/order.rb b/core/app/models/spree/order.rb index afdb2fe00dd..6802830f105 100644 --- a/core/app/models/spree/order.rb +++ b/core/app/models/spree/order.rb @@ -751,7 +751,7 @@ def ensure_available_shipping_rates if shipments.empty? || shipments.any? { |shipment| shipment.shipping_rates.blank? } # After this point, order redirects back to 'address' state and asks user to pick a proper address # Therefore, shipments are not necessary at this point. - shipments.delete_all + shipments.destroy_all errors.add(:base, Spree.t(:items_cannot_be_shipped)) and return false end end diff --git a/core/spec/models/spree/order/checkout_spec.rb b/core/spec/models/spree/order/checkout_spec.rb index 22c909614f5..89feb9c7989 100644 --- a/core/spec/models/spree/order/checkout_spec.rb +++ b/core/spec/models/spree/order/checkout_spec.rb @@ -597,6 +597,22 @@ def assert_state_changed(order, from, to) expect(order.reload.state).to eq 'payment' end end + + context 'a shipment has no shipping rates' do + let(:order) { create(:order_with_line_items, state: 'confirm') } + let(:shipment) { order.shipments.first } + + before do + shipment.shipping_rates.destroy_all + end + + it 'clears the shipments and fails the transition' do + expect(order.complete).to eq(false) + expect(order.errors[:base]).to include(Spree.t(:items_cannot_be_shipped)) + expect(order.shipments.count).to eq(0) + expect(Spree::InventoryUnit.where(shipment_id: shipment.id).count).to eq(0) + end + end end context "subclassed order" do