From d7cfda83853325fb6bc38279708d87004a94942b Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Sun, 23 Feb 2020 17:53:13 +0000 Subject: [PATCH 1/4] Handle strong params in subscription_line_items controller --- .../admin/subscription_line_items_controller.rb | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/app/controllers/admin/subscription_line_items_controller.rb b/app/controllers/admin/subscription_line_items_controller.rb index a87ddebe45..27ec3611a1 100644 --- a/app/controllers/admin/subscription_line_items_controller.rb +++ b/app/controllers/admin/subscription_line_items_controller.rb @@ -11,7 +11,7 @@ module Admin respond_to :json def build - @subscription_line_item.assign_attributes(params[:subscription_line_item]) + @subscription_line_item.assign_attributes(subscription_line_item_params) @subscription_line_item.price_estimate = price_estimate render json: @subscription_line_item, serializer: Api::Admin::SubscriptionLineItemSerializer, shop: @shop, schedule: @schedule @@ -27,7 +27,7 @@ module Admin @shop = Enterprise.managed_by(spree_current_user).find_by(id: params[:shop_id]) @schedule = permissions.editable_schedules.find_by(id: params[:schedule_id]) @order_cycle = @schedule.andand.current_or_next_order_cycle - @variant = variant_if_eligible(params[:subscription_line_item][:variant_id]) if @shop.present? + @variant = variant_if_eligible(subscription_line_item_params[:variant_id]) if @shop.present? end def new_actions @@ -58,5 +58,9 @@ module Admin def variant_if_eligible(variant_id) SubscriptionVariantsService.eligible_variants(@shop).find_by(id: variant_id) end + + def subscription_line_item_params + params.require(:subscription_line_item).permit(:quantity, :variant_id) + end end end From c3897b2f1c0776ca4e16491900659fa55768cea0 Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Sun, 23 Feb 2020 22:03:04 +0000 Subject: [PATCH 2/4] Handle strong params in subscriptions controller --- .../admin/subscriptions_controller.rb | 18 ++++++++++++++++-- app/services/subscription_form.rb | 8 ++++---- 2 files changed, 20 insertions(+), 6 deletions(-) diff --git a/app/controllers/admin/subscriptions_controller.rb b/app/controllers/admin/subscriptions_controller.rb index da99409469..4e6c65018a 100644 --- a/app/controllers/admin/subscriptions_controller.rb +++ b/app/controllers/admin/subscriptions_controller.rb @@ -65,7 +65,7 @@ module Admin private def save_form_and_render(render_issues = true) - form = SubscriptionForm.new(@subscription, params[:subscription]) + form = SubscriptionForm.new(@subscription, subscription_params) unless form.save render json: { errors: form.json_errors }, status: :unprocessable_entity return @@ -149,11 +149,25 @@ module Admin # Overriding Spree method to load data from params here so that # we can authorise #create using an object with required attributes def build_resource - Subscription.new(params[:subscription]) + Subscription.new(subscription_params) end def ams_prefix_whitelist [:index] end + + def subscription_params + return params[:subscription] if params[:subscription].empty? + + params.require(:subscription).permit( + :shop_id, :schedule_id, :customer_id, + :payment_method_id, :shipping_method_id, + :begins_at, :ends_at, + :canceled_at, :paused_at, + :subscription_line_items_attributes => [:id, :quantity, :variant_id], + :bill_address_attributes => permitted_address_attributes, + :ship_address_attributes => permitted_address_attributes + ) + end end end diff --git a/app/services/subscription_form.rb b/app/services/subscription_form.rb index da82063b5c..0458467fb5 100644 --- a/app/services/subscription_form.rb +++ b/app/services/subscription_form.rb @@ -1,21 +1,21 @@ require 'open_food_network/proxy_order_syncer' class SubscriptionForm - attr_accessor :subscription, :params, :order_update_issues, :validator, :order_syncer, :estimator + attr_accessor :subscription, :subscription_params, :order_update_issues, :validator, :order_syncer, :estimator delegate :json_errors, :valid?, to: :validator delegate :order_update_issues, to: :order_syncer - def initialize(subscription, params = {}) + def initialize(subscription, subscription_params = {}) @subscription = subscription - @params = params + @subscription_params = subscription_params @estimator = SubscriptionEstimator.new(subscription) @validator = SubscriptionValidator.new(subscription) @order_syncer = OrderSyncer.new(subscription) end def save - subscription.assign_attributes(params) + subscription.assign_attributes(subscription_params) return false unless valid? subscription.transaction do From 58c83d056db630477e757fc7cf257a0abbbc0ff7 Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Tue, 10 Mar 2020 15:45:45 +0000 Subject: [PATCH 3/4] Add missing permitted attributes to subscriptions controller --- app/controllers/admin/subscriptions_controller.rb | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/app/controllers/admin/subscriptions_controller.rb b/app/controllers/admin/subscriptions_controller.rb index 4e6c65018a..6b65ddda86 100644 --- a/app/controllers/admin/subscriptions_controller.rb +++ b/app/controllers/admin/subscriptions_controller.rb @@ -160,11 +160,12 @@ module Admin return params[:subscription] if params[:subscription].empty? params.require(:subscription).permit( - :shop_id, :schedule_id, :customer_id, + :id, :shop_id, :schedule_id, :customer_id, :payment_method_id, :shipping_method_id, :begins_at, :ends_at, :canceled_at, :paused_at, - :subscription_line_items_attributes => [:id, :quantity, :variant_id], + :shipping_fee_estimate, :payment_fee_estimate, + :subscription_line_items_attributes => [:id, :quantity, :variant_id, :price_estimate, :_destroy], :bill_address_attributes => permitted_address_attributes, :ship_address_attributes => permitted_address_attributes ) From 79b08675070ce8693050f5abe61545f3a0c57c1e Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Sat, 21 Mar 2020 19:05:01 +0000 Subject: [PATCH 4/4] Extract permitted attributes to separate service --- .../admin/subscriptions_controller.rb | 13 +------ .../permitted_attributes/subscription.rb | 37 +++++++++++++++++++ 2 files changed, 38 insertions(+), 12 deletions(-) create mode 100644 app/services/permitted_attributes/subscription.rb diff --git a/app/controllers/admin/subscriptions_controller.rb b/app/controllers/admin/subscriptions_controller.rb index 6b65ddda86..4561ef3a79 100644 --- a/app/controllers/admin/subscriptions_controller.rb +++ b/app/controllers/admin/subscriptions_controller.rb @@ -157,18 +157,7 @@ module Admin end def subscription_params - return params[:subscription] if params[:subscription].empty? - - params.require(:subscription).permit( - :id, :shop_id, :schedule_id, :customer_id, - :payment_method_id, :shipping_method_id, - :begins_at, :ends_at, - :canceled_at, :paused_at, - :shipping_fee_estimate, :payment_fee_estimate, - :subscription_line_items_attributes => [:id, :quantity, :variant_id, :price_estimate, :_destroy], - :bill_address_attributes => permitted_address_attributes, - :ship_address_attributes => permitted_address_attributes - ) + PermittedAttributes::Subscription.new(params).call end end end diff --git a/app/services/permitted_attributes/subscription.rb b/app/services/permitted_attributes/subscription.rb new file mode 100644 index 0000000000..2ab3956fca --- /dev/null +++ b/app/services/permitted_attributes/subscription.rb @@ -0,0 +1,37 @@ +# frozen_string_literal: true + +module PermittedAttributes + class Subscription + def initialize(params) + @params = params + end + + def call + return @params[:subscription] if @params[:subscription].empty? + + @params.require(:subscription).permit(basic_permitted_attributes + other_permitted_attributes) + end + + private + + def basic_permitted_attributes + [ + :id, :shop_id, :schedule_id, :customer_id, + :payment_method_id, :shipping_method_id, + :begins_at, :ends_at, + :canceled_at, :paused_at, + :shipping_fee_estimate, :payment_fee_estimate, + ] + end + + def other_permitted_attributes + [ + subscription_line_items_attributes: [ + :id, :quantity, :variant_id, :price_estimate, :_destroy + ], + bill_address_attributes: PermittedAttributes::Address.attributes, + ship_address_attributes: PermittedAttributes::Address.attributes + ] + end + end +end