diff --git a/app/controllers/spree/admin/shipping_methods_controller_decorator.rb b/app/controllers/spree/admin/shipping_methods_controller_decorator.rb index 9b2b03cf5e..496612b287 100644 --- a/app/controllers/spree/admin/shipping_methods_controller_decorator.rb +++ b/app/controllers/spree/admin/shipping_methods_controller_decorator.rb @@ -5,22 +5,10 @@ module Spree before_filter :load_hubs, only: [:new, :edit, :create, :update] # Sort shipping methods by distributor name - # ! Code copied from Spree::Admin::ResourceController with two added lines def collection - return parent.send(controller_name) if parent_data.present? + collection = super + collection = collection.managed_by(spree_current_user).by_name - collection = if model_class.respond_to?(:accessible_by) && - !current_ability.has_block?(params[:action], model_class) - - model_class.accessible_by(current_ability, action) - - else - model_class.scoped - end - - collection = collection.managed_by(spree_current_user).by_name # This line added - - # This block added if params.key? :enterprise_id distributor = Enterprise.find params[:enterprise_id] collection = collection.for_distributor(distributor) @@ -29,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