Merge pull request #7174 from Matt-Yorkley/dead-code-callbacks

Remove Spree ResourceController callbacks

.rubocop_todo.yml

@@ -1112,7 +1112,6 @@ Style/FrozenStringLiteralComment:
     - 'app/serializers/api/uncached_enterprise_serializer.rb'
     - 'app/serializers/api/user_serializer.rb'
     - 'app/serializers/api/variant_serializer.rb'
-    - 'app/services/action_callbacks.rb'
     - 'app/services/bulk_invoice_service.rb'
     - 'app/services/cart_service.rb'
     - 'app/services/create_order_cycle.rb'

app/controllers/admin/customers_controller.rb

@@ -53,15 +53,12 @@ def create
 
     # copy of Admin::ResourceController without flash notice
     def destroy
-      invoke_callbacks(:destroy, :before)
       if @object.destroy
-        invoke_callbacks(:destroy, :after)
         respond_with(@object) do |format|
           format.html { redirect_to location_after_destroy }
           format.js   { render partial: "spree/admin/shared/destroy" }
         end
       else
-        invoke_callbacks(:destroy, :fails)
         respond_with(@object) do |format|
           format.html { redirect_to location_after_destroy }
           format.json { render json: { errors: @object.errors.full_messages }, status: :conflict }

app/controllers/admin/enterprises_controller.rb

@@ -43,20 +43,18 @@ def welcome
     end
 
     def update
-      invoke_callbacks(:update, :before)
       tag_rules_attributes = params[object_name].delete :tag_rules_attributes
       update_tag_rules(tag_rules_attributes) if tag_rules_attributes.present?
       update_enterprise_notifications
+
       if @object.update(enterprise_params)
-        invoke_callbacks(:update, :after)
         flash[:success] = flash_message_for(@object, :successfully_updated)
         respond_with(@object) do |format|
           format.html { redirect_to location_after_save }
           format.js   { render layout: false }
           format.json { render_as_json @object, ams_prefix: 'index', spree_current_user: spree_current_user }
         end
       else
-        invoke_callbacks(:update, :fails)
         respond_with(@object) do |format|
           format.json { render json: { errors: @object.errors.messages }, status: :unprocessable_entity }
         end

app/controllers/admin/resource_controller.rb

@@ -1,5 +1,3 @@
-require 'action_callbacks'
-
 module Admin
   class ResourceController < Spree::Admin::BaseController
     helper_method :new_object_url, :edit_object_url, :object_url, :collection_url
@@ -11,7 +9,6 @@ class ResourceController < Spree::Admin::BaseController
     respond_to :js, except: [:show, :index]
 
     def new
-      invoke_callbacks(:new_action, :before)
       respond_with(@object) do |format|
         format.html { render layout: !request.xhr? }
         format.js   { render layout: false }
@@ -26,32 +23,26 @@ def edit
     end
 
     def update
-      invoke_callbacks(:update, :before)
       if @object.update(permitted_resource_params)
-        invoke_callbacks(:update, :after)
         flash[:success] = flash_message_for(@object, :successfully_updated)
         respond_with(@object) do |format|
           format.html { redirect_to location_after_save }
           format.js   { render layout: false }
         end
       else
-        invoke_callbacks(:update, :fails)
         respond_with(@object)
       end
     end
 
     def create
-      invoke_callbacks(:create, :before)
       @object.attributes = permitted_resource_params
       if @object.save
-        invoke_callbacks(:create, :after)
         flash[:success] = flash_message_for(@object, :successfully_created)
         respond_with(@object) do |format|
           format.html { redirect_to location_after_save }
           format.js   { render layout: false }
         end
       else
-        invoke_callbacks(:create, :fails)
         respond_with(@object)
       end
     end
@@ -67,16 +58,13 @@ def update_positions
     end
 
     def destroy
-      invoke_callbacks(:destroy, :before)
       if @object.destroy
-        invoke_callbacks(:destroy, :after)
         flash[:success] = flash_message_for(@object, :successfully_removed)
         respond_with(@object) do |format|
           format.html { redirect_to collection_url }
           format.js   { render partial: "spree/admin/shared/destroy" }
         end
       else
-        invoke_callbacks(:destroy, :fails)
         respond_with(@object) do |format|
           format.html { redirect_to collection_url }
         end
@@ -92,34 +80,13 @@ def resource_not_found
 
     class << self
       attr_accessor :parent_data
-      attr_accessor :callbacks
 
       def belongs_to(model_name, options = {})
         @parent_data ||= {}
         @parent_data[:model_name] = model_name
         @parent_data[:model_class] = model_name.to_s.classify.constantize
         @parent_data[:find_by] = options[:find_by] || :id
       end
-
-      def new_action
-        @callbacks ||= {}
-        @callbacks[:new_action] ||= ActionCallbacks.new
-      end
-
-      def create
-        @callbacks ||= {}
-        @callbacks[:create] ||= ActionCallbacks.new
-      end
-
-      def update
-        @callbacks ||= {}
-        @callbacks[:update] ||= ActionCallbacks.new
-      end
-
-      def destroy
-        @callbacks ||= {}
-        @callbacks[:destroy] ||= ActionCallbacks.new
-      end
     end
 
     def model_class
@@ -212,17 +179,6 @@ def location_after_save
       collection_url
     end
 
-    def invoke_callbacks(action, callback_type)
-      callbacks = self.class.callbacks || {}
-      return if callbacks[action].nil?
-
-      case callback_type.to_sym
-      when :before then callbacks[action].before_methods.each { |method| __send__ method }
-      when :after  then callbacks[action].after_methods.each  { |method| __send__ method }
-      when :fails  then callbacks[action].fails_methods.each  { |method| __send__ method }
-      end
-    end
-
     # URL helpers
     def new_object_url(options = {})
       if parent_data.present?

app/controllers/admin/schedules_controller.rb

@@ -9,7 +9,8 @@ class SchedulesController < Admin::ResourceController
     before_action :editable_order_cycle_ids_for_create, only: [:create]
     before_action :editable_order_cycle_ids_for_update, only: [:update]
     before_action :check_dependent_subscriptions, only: [:destroy]
-    update.after :sync_subscriptions_for_update
+
+    after_action :sync_subscriptions_for_update, only: :update
 
     respond_to :json
 
@@ -120,7 +121,7 @@ def permissions
     end
 
     def sync_subscriptions_for_update
-      return unless params[:schedule][:order_cycle_ids]
+      return unless params[:schedule][:order_cycle_ids] && @object.errors.blank?
 
       sync_subscriptions
     end

app/controllers/spree/admin/payment_methods_controller.rb

@@ -5,7 +5,6 @@ class PaymentMethodsController < ::Admin::ResourceController
       before_action :load_data
       before_action :validate_payment_method_provider, only: [:create]
       before_action :load_hubs, only: [:new, :edit, :update]
-      create.before :load_hubs
 
       respond_to :html
 
@@ -15,14 +14,12 @@ def create
         @payment_method = payment_method_class.constantize.new(base_params)
         @object = @payment_method
 
-        invoke_callbacks(:create, :before)
+        load_hubs
 
         if @payment_method.save
-          invoke_callbacks(:create, :after)
           flash[:success] = Spree.t(:successfully_created, resource: Spree.t(:payment_method))
           redirect_to spree.edit_admin_payment_method_path(@payment_method)
         else
-          invoke_callbacks(:create, :fails)
           respond_with(@payment_method)
         end
       end
@@ -31,8 +28,6 @@ def update
         restrict_stripe_account_change
         force_environment
 
-        invoke_callbacks(:update, :before)
-
         if @payment_method.type.to_s != payment_method_class
           @payment_method.update_columns(
             type: payment_method_class,
@@ -42,11 +37,9 @@ def update
         end
 
         if @payment_method.update(update_params)
-          invoke_callbacks(:update, :after)
           flash[:success] = Spree.t(:successfully_updated, resource: Spree.t(:payment_method))
           redirect_to spree.edit_admin_payment_method_path(@payment_method)
         else
-          invoke_callbacks(:update, :fails)
           respond_with(@payment_method)
         end
       end

app/controllers/spree/admin/products_controller.rb

@@ -38,7 +38,6 @@ def create
           end
         end
       rescue Paperclip::Errors::NotIdentifiedByImageMagickError
-        invoke_callbacks(:create, :fails)
         @object.errors.add(:base, t('spree.admin.products.image_upload_error'))
         respond_with(@object)
       end

app/controllers/spree/admin/return_authorizations_controller.rb

@@ -3,8 +3,7 @@ module Admin
     class ReturnAuthorizationsController < ::Admin::ResourceController
       belongs_to 'spree/order', find_by: :number
 
-      update.after :associate_inventory_units
-      create.after :associate_inventory_units
+      after_action :associate_inventory_units, only: [:create, :update]
 
       def fire
         @return_authorization.public_send("#{params[:e]}!")

app/controllers/spree/admin/users_controller.rb

@@ -102,7 +102,6 @@ def collection
 
       # handling raise from Admin::ResourceController#destroy
       def user_destroy_with_orders_error
-        invoke_callbacks(:destroy, :fails)
         render status: :forbidden, text: Spree.t(:error_user_destroy_with_orders)
       end
 

app/controllers/spree/admin/variants_controller.rb

@@ -4,9 +4,9 @@ module Spree
   module Admin
     class VariantsController < ::Admin::ResourceController
       helper 'spree/products'
-
       belongs_to 'spree/product', find_by: :permalink
-      new_action.before :new_before
+
+      before_action :assign_default_attributes, only: :new
 
       def index
         @url_filters = ::ProductFilters.new.extract(request.query_parameters)
@@ -84,7 +84,7 @@ def create_before
         @object.save
       end
 
-      def new_before
+      def assign_default_attributes
         @object.attributes = @object.product.master.
           attributes.except('id', 'created_at', 'deleted_at', 'sku', 'is_master')
         # Shallow Clone of the default price to populate the price field.