diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 546320930f..742af5bd20 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -192,7 +192,6 @@ Metrics/ClassLength: - 'app/controllers/admin/order_cycles_controller.rb' - 'app/controllers/admin/product_import_controller.rb' - 'app/controllers/admin/resource_controller.rb' - - 'app/controllers/admin/schedules_controller.rb' - 'app/controllers/admin/subscriptions_controller.rb' - 'app/controllers/application_controller.rb' - 'app/controllers/payment_gateways/paypal_controller.rb' @@ -492,7 +491,7 @@ Rails/InverseOf: - 'app/models/spree/variant.rb' - 'app/models/subscription_line_item.rb' -# Offense count: 38 +# Offense count: 35 # Configuration parameters: Include. # Include: app/controllers/**/*.rb, app/mailers/**/*.rb Rails/LexicallyScopedActionFilter: diff --git a/app/controllers/admin/schedules_controller.rb b/app/controllers/admin/schedules_controller.rb index 57a3d463cc..3165667312 100644 --- a/app/controllers/admin/schedules_controller.rb +++ b/app/controllers/admin/schedules_controller.rb @@ -8,15 +8,13 @@ module Admin include PaperTrailLogging before_action :adapt_params, only: [:update] - 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] after_action :sync_subscriptions_for_update, only: :update respond_to :json - respond_override create: { json: { + OVERRIDE_RESPONSE = { json: { success: lambda { render_as_json @schedule, editable_schedule_ids: permissions.editable_schedules.pluck(:id) @@ -25,17 +23,10 @@ module Admin render json: { errors: @schedule.errors.full_messages }, status: :unprocessable_entity } - } } - respond_override update: { json: { - success: lambda { - render_as_json @schedule, - editable_schedule_ids: permissions.editable_schedules.pluck(:id) - }, - failure: lambda { - render json: { errors: @schedule.errors.full_messages }, - status: :unprocessable_entity - } - } } + } }.freeze + + respond_override create: OVERRIDE_RESPONSE + respond_override update: OVERRIDE_RESPONSE def index respond_to do |format| @@ -50,22 +41,23 @@ module Admin end def create - return respond_with(@schedule) if params[:order_cycle_ids].blank? - - @schedule.attributes = permitted_resource_params - - if @schedule.save - @schedule.order_cycle_ids = params[:order_cycle_ids] - @schedule.save! - - sync_subscriptions_for_create + @schedule_form = ScheduleForm.new(params, spree_current_user, @schedule) + if @schedule_form.save flash[:success] = flash_message_for(@schedule, :successfully_created) + @existing_order_cycle_ids = [] + sync_subscriptions_for_create end respond_with(@schedule) end + def update + @existing_order_cycle_ids = @schedule.order_cycle_ids + @object = ScheduleForm.new(params, spree_current_user, @schedule) + super + end + private def collection @@ -96,37 +88,6 @@ module Admin params[:schedule][:order_cycle_ids] = params[:order_cycle_ids] end - def editable_order_cycle_ids_for_create - return unless params[:order_cycle_ids] - - @existing_order_cycle_ids = [] - result = editable_order_cycles(params[:order_cycle_ids]) - - params[:order_cycle_ids] = result - end - - def editable_order_cycle_ids_for_update - return unless params[:schedule][:order_cycle_ids] - - @existing_order_cycle_ids = @schedule.order_cycle_ids - result = editable_order_cycles(params[:schedule][:order_cycle_ids]) - - params[:schedule][:order_cycle_ids] = result - @schedule.order_cycle_ids = result - end - - def editable_order_cycles(requested) - permitted = OrderCycle - .where(id: params[:order_cycle_ids] | @existing_order_cycle_ids) - .merge(OrderCycle.managed_by(spree_current_user)) - .pluck(:id) - result = @existing_order_cycle_ids - result |= (requested & permitted) # add any requested & permitted ids - # remove any existing and permitted ids that were not specifically requested - result -= ((result & permitted) - requested) - result - end - def check_dependent_subscriptions return if Subscription.where(schedule_id: @schedule).empty? @@ -140,14 +101,14 @@ module Admin @permissions = OpenFoodNetwork::Permissions.new(spree_current_user) end - def sync_subscriptions_for_update - return unless params[:schedule][:order_cycle_ids] && @object.errors.blank? + def sync_subscriptions_for_create + return unless params[:order_cycle_ids] sync_subscriptions end - def sync_subscriptions_for_create - return unless params[:order_cycle_ids] + def sync_subscriptions_for_update + return unless params[:schedule][:order_cycle_ids] && @schedule.errors.blank? sync_subscriptions end @@ -155,6 +116,7 @@ module Admin def sync_subscriptions removed_ids = @existing_order_cycle_ids - @schedule.order_cycle_ids new_ids = @schedule.order_cycle_ids - @existing_order_cycle_ids + return unless removed_ids.any? || new_ids.any? subscriptions = Subscription.where(schedule_id: @schedule) diff --git a/app/forms/schedule_form.rb b/app/forms/schedule_form.rb new file mode 100644 index 0000000000..e596afb4a0 --- /dev/null +++ b/app/forms/schedule_form.rb @@ -0,0 +1,78 @@ +# frozen_string_literal: true + +class ScheduleForm + include ActiveModel::Model + + attr_reader :errors, :flash_success + + def initialize(params, user, schedule = nil) + @errors = ActiveModel::Errors.new self + + # Not strong + @params = params + @current_user = user + @schedule = schedule + end + + def save + editable_order_cycle_ids_for_create + + return false if @params[:order_cycle_ids].blank? + + @schedule.attributes = permitted_resource_params + + if @schedule.save + @schedule.order_cycle_ids = @params[:order_cycle_ids] + @schedule.save! + + end + + true + end + + def update(_params) + editable_order_cycle_ids_for_update + + false unless @schedule.update(permitted_resource_params) + end + + def order_cycle_ids + @schedule.order_cycle_ids + end + + private + + def editable_order_cycle_ids_for_create + return unless @params[:order_cycle_ids] + + @existing_order_cycle_ids = [] + result = editable_order_cycles(@params[:order_cycle_ids]) + @params[:order_cycle_ids] = result + end + + def editable_order_cycle_ids_for_update + return unless @params[:schedule][:order_cycle_ids] + + @existing_order_cycle_ids = @schedule.order_cycle_ids + result = editable_order_cycles(@params[:schedule][:order_cycle_ids]) + + @params[:schedule][:order_cycle_ids] = result + @schedule.order_cycle_ids = result + end + + def editable_order_cycles(requested) + permitted = OrderCycle + .where(id: @params[:order_cycle_ids] | @existing_order_cycle_ids) + .merge(OrderCycle.managed_by(@current_user)) + .pluck(:id) + result = @existing_order_cycle_ids + result |= (requested & permitted) # add any requested & permitted ids + # remove any existing and permitted ids that were not specifically requested + result -= ((result & permitted) - requested) + result + end + + def permitted_resource_params + @params.require(:schedule).permit(:id, :name, order_cycle_ids: []) + end +end