From a82bae2a461a13afdfdaf727cd9ef9c9480c15ef Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Sun, 21 Mar 2021 13:17:26 +0000 Subject: [PATCH 1/6] Remove Spree resource controller custom callbacks --- .rubocop_todo.yml | 1 - app/controllers/admin/resource_controller.rb | 44 -------------------- app/services/action_callbacks.rb | 23 ---------- 3 files changed, 68 deletions(-) delete mode 100644 app/services/action_callbacks.rb diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index cd62dcf638b..b83b804206d 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -661,7 +661,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' diff --git a/app/controllers/admin/resource_controller.rb b/app/controllers/admin/resource_controller.rb index b316046b4be..3510093c0da 100644 --- a/app/controllers/admin/resource_controller.rb +++ b/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,7 +80,6 @@ def resource_not_found class << self attr_accessor :parent_data - attr_accessor :callbacks def belongs_to(model_name, options = {}) @parent_data ||= {} @@ -100,26 +87,6 @@ def belongs_to(model_name, options = {}) @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? diff --git a/app/services/action_callbacks.rb b/app/services/action_callbacks.rb deleted file mode 100644 index 2cfecc961b5..00000000000 --- a/app/services/action_callbacks.rb +++ /dev/null @@ -1,23 +0,0 @@ -class ActionCallbacks - attr_reader :before_methods - attr_reader :after_methods - attr_reader :fails_methods - - def initialize - @before_methods = [] - @after_methods = [] - @fails_methods = [] - end - - def before(method) - @before_methods << method - end - - def after(method) - @after_methods << method - end - - def fails(method) - @fails_methods << method - end -end From a70b2a12d1c034386e7855444ef7b63cc6e30fa0 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Sun, 21 Mar 2021 13:20:25 +0000 Subject: [PATCH 2/6] Update variants controller callback --- app/controllers/spree/admin/variants_controller.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/controllers/spree/admin/variants_controller.rb b/app/controllers/spree/admin/variants_controller.rb index 2177080a8f3..c11c89b5afc 100644 --- a/app/controllers/spree/admin/variants_controller.rb +++ b/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. From 365dc2a49e86a3f00a2375bb21fcc3a4106021e9 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Sun, 21 Mar 2021 13:22:07 +0000 Subject: [PATCH 3/6] Update payment methods controller callback --- app/controllers/spree/admin/payment_methods_controller.rb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/app/controllers/spree/admin/payment_methods_controller.rb b/app/controllers/spree/admin/payment_methods_controller.rb index 9cfd7c24925..c537201e223 100644 --- a/app/controllers/spree/admin/payment_methods_controller.rb +++ b/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,7 +14,7 @@ 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) From 6c17680423fe70d7b1d8a55ea6809ed9af42f642 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Sun, 21 Mar 2021 13:24:43 +0000 Subject: [PATCH 4/6] Update return authorizations controller callbacks --- .../spree/admin/return_authorizations_controller.rb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/app/controllers/spree/admin/return_authorizations_controller.rb b/app/controllers/spree/admin/return_authorizations_controller.rb index ac229acf83c..094dd37d4ac 100644 --- a/app/controllers/spree/admin/return_authorizations_controller.rb +++ b/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]}!") From 0da90ab834bc7f9a7be620bdf1a2d071f7b57d35 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Sun, 21 Mar 2021 13:27:02 +0000 Subject: [PATCH 5/6] Update schedules controller callbacks --- app/controllers/admin/schedules_controller.rb | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/app/controllers/admin/schedules_controller.rb b/app/controllers/admin/schedules_controller.rb index 8d375b39796..c88af92c32c 100644 --- a/app/controllers/admin/schedules_controller.rb +++ b/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 From 50e0d78c0c6a4b87110c69ccafcd8c0693af27c9 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Sun, 21 Mar 2021 13:45:37 +0000 Subject: [PATCH 6/6] Remove dead callback invocations --- app/controllers/admin/customers_controller.rb | 3 --- app/controllers/admin/enterprises_controller.rb | 4 +--- app/controllers/spree/admin/payment_methods_controller.rb | 6 ------ app/controllers/spree/admin/products_controller.rb | 1 - app/controllers/spree/admin/users_controller.rb | 1 - 5 files changed, 1 insertion(+), 14 deletions(-) diff --git a/app/controllers/admin/customers_controller.rb b/app/controllers/admin/customers_controller.rb index 4a2f4242e9f..9429a9161e9 100644 --- a/app/controllers/admin/customers_controller.rb +++ b/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 } diff --git a/app/controllers/admin/enterprises_controller.rb b/app/controllers/admin/enterprises_controller.rb index 9b42b391749..6dd9631ced1 100644 --- a/app/controllers/admin/enterprises_controller.rb +++ b/app/controllers/admin/enterprises_controller.rb @@ -43,12 +43,11 @@ 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 } @@ -56,7 +55,6 @@ def update 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 diff --git a/app/controllers/spree/admin/payment_methods_controller.rb b/app/controllers/spree/admin/payment_methods_controller.rb index c537201e223..0544f94faac 100644 --- a/app/controllers/spree/admin/payment_methods_controller.rb +++ b/app/controllers/spree/admin/payment_methods_controller.rb @@ -17,11 +17,9 @@ def create 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 @@ -30,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, @@ -41,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 diff --git a/app/controllers/spree/admin/products_controller.rb b/app/controllers/spree/admin/products_controller.rb index 0860d5a5a58..3d6e6be95b6 100644 --- a/app/controllers/spree/admin/products_controller.rb +++ b/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 diff --git a/app/controllers/spree/admin/users_controller.rb b/app/controllers/spree/admin/users_controller.rb index 30ebca12ca3..032e3aed56d 100644 --- a/app/controllers/spree/admin/users_controller.rb +++ b/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