Skip to content

Commit

Permalink
Use destroy_all instead of delete_all when deleting shipments
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
jordan-brough committed Sep 3, 2015
1 parent 81cbc5c commit e7450ec
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 1 deletion.
2 changes: 1 addition & 1 deletion core/app/models/spree/order.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
16 changes: 16 additions & 0 deletions core/spec/models/spree/order/checkout_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit e7450ec

Please sign in to comment.