diff --git a/app/controllers/spree/admin/shipping_methods_controller_decorator.rb b/app/controllers/spree/admin/shipping_methods_controller_decorator.rb index a0f4292396..496612b287 100644 --- a/app/controllers/spree/admin/shipping_methods_controller_decorator.rb +++ b/app/controllers/spree/admin/shipping_methods_controller_decorator.rb @@ -17,14 +17,10 @@ module Spree collection end - # This method was originally written because ProductDistributions referenced shipping - # methods, and deleting a referenced shipping method would break all the reports that - # queried it. - # This has changed, and now all we're protecting is Orders, which is a spree resource. - # Do we really need to protect it ourselves? Does spree do this, or provide some means - # of preserving the shipping method information for past orders? + # Spree allows soft deletes of shipping_methods but our reports are not adapted to that. + # So, this method prevents the deletion (even soft) of shipping_methods that are referenced in orders. def do_not_destroy_referenced_shipping_methods - order = Order.where(:shipping_method_id => @object).first + order = Order.joins(shipments: :shipping_rates).where( spree_shipping_rates: { :shipping_method_id => @object } ).first if order flash[:error] = I18n.t(:shipping_method_destroy_error, number: order.number) redirect_to collection_url and return diff --git a/spec/controllers/spree/admin/shipping_methods_controller_spec.rb b/spec/controllers/spree/admin/shipping_methods_controller_spec.rb new file mode 100644 index 0000000000..c823d2cff6 --- /dev/null +++ b/spec/controllers/spree/admin/shipping_methods_controller_spec.rb @@ -0,0 +1,31 @@ +require 'spec_helper' + +describe Spree::Admin::ShippingMethodsController, type: :controller do + include AuthenticationWorkflow + + describe "shipping method not referenced by order" do + let(:shipping_method) { create(:shipping_method) } + + scenario "is soft deleted" do + login_as_admin + expect(shipping_method.deleted_at).to be_nil + + spree_delete :destroy, {"id" => shipping_method.id} + + expect(shipping_method.reload.deleted_at).not_to be_nil + end + end + + describe "shipping method referenced by order" do + let(:order) { create(:order_with_line_items) } + + scenario "is not soft deleted" do + login_as_admin + expect(order.shipping_method.deleted_at).to be_nil + + spree_delete :destroy, {"id" => order.shipping_method.id} + + expect(order.shipping_method.reload.deleted_at).to be_nil + end + end +end