Merge pull request #2728 from luisramos0/2-0-ship-method-ctrl

[Spree Upgrade] Remove use of Order#shipping_method_id from ShippingMethodsControllerDecorator

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

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