From 456f369b767c172bec6035be4d7d824c8c7d7c91 Mon Sep 17 00:00:00 2001 From: Pau Perez Date: Mon, 15 Jun 2020 10:09:43 +0200 Subject: [PATCH] Fix outstanding Rubocop violations --- .../admin/enterprise_groups_controller.rb | 4 +++- .../admin/enterprises_controller.rb | 7 +++++-- app/controllers/admin/schedules_controller.rb | 12 +++++------- .../api/enterprise_attachment_controller.rb | 2 ++ .../spree/admin/countries_controller.rb | 1 - .../spree/admin/products_controller.rb | 2 +- .../admin/shipping_categories_controller.rb | 1 - app/models/product_import/entry_processor.rb | 4 +++- app/models/product_import/entry_validator.rb | 4 +++- app/models/spree/adjustment_decorator.rb | 1 - app/models/spree/line_item_decorator.rb | 9 +++++---- app/models/spree/order_decorator.rb | 3 ++- app/models/spree/order_updater_decorator.rb | 4 +++- app/models/spree/payment_decorator.rb | 16 +++++++--------- app/models/spree/user.rb | 1 - app/services/permitted_attributes/checkout.rb | 2 ++ .../permitted_attributes/order_cycle.rb | 9 ++++++--- .../subscriptions/proxy_order_syncer_spec.rb | 18 ++++++++++++------ .../order_cycle_form_applicator.rb | 2 +- lib/open_food_network/permalink_generator.rb | 2 +- .../variant_and_line_item_naming.rb | 3 ++- 21 files changed, 63 insertions(+), 44 deletions(-) diff --git a/app/controllers/admin/enterprise_groups_controller.rb b/app/controllers/admin/enterprise_groups_controller.rb index 5483cdd45e..471d56d5ab 100644 --- a/app/controllers/admin/enterprise_groups_controller.rb +++ b/app/controllers/admin/enterprise_groups_controller.rb @@ -28,7 +28,9 @@ module Admin def build_resource_with_address enterprise_group = build_resource_without_address enterprise_group.address = Spree::Address.new - enterprise_group.address.country = Spree::Country.find_by(id: Spree::Config[:default_country_id]) + enterprise_group.address.country = Spree::Country.find_by( + id: Spree::Config[:default_country_id] + ) enterprise_group end alias_method_chain :build_resource, :address diff --git a/app/controllers/admin/enterprises_controller.rb b/app/controllers/admin/enterprises_controller.rb index 332d939f27..8b8901e211 100644 --- a/app/controllers/admin/enterprises_controller.rb +++ b/app/controllers/admin/enterprises_controller.rb @@ -211,7 +211,8 @@ module Admin # record is persisted. This problem is compounded by the use of calculators. @object.transaction do tag_rules_attributes.select{ |_i, attrs| attrs[:type].present? }.each do |_i, attrs| - rule = @object.tag_rules.find_by(id: attrs.delete(:id)) || attrs[:type].constantize.new(enterprise: @object) + rule = @object.tag_rules.find_by(id: attrs.delete(:id)) || + attrs[:type].constantize.new(enterprise: @object) create_calculator_for(rule, attrs) if rule.type == "TagRule::DiscountOrder" && rule.calculator.nil? rule.update_attributes(attrs) end @@ -234,7 +235,9 @@ module Admin def check_can_change_bulk_sells unless spree_current_user.admin? params[:enterprise_set][:collection_attributes].each do |_i, enterprise_params| - enterprise_params.delete :sells unless spree_current_user == Enterprise.find_by(id: enterprise_params[:id]).owner + unless spree_current_user == Enterprise.find_by(id: enterprise_params[:id]).owner + enterprise_params.delete :sells + end end end end diff --git a/app/controllers/admin/schedules_controller.rb b/app/controllers/admin/schedules_controller.rb index d0728ebc18..9532e4db9e 100644 --- a/app/controllers/admin/schedules_controller.rb +++ b/app/controllers/admin/schedules_controller.rb @@ -29,9 +29,7 @@ module Admin end def create - if params[:order_cycle_ids].blank? - return respond_with(@schedule) - end + return respond_with(@schedule) if params[:order_cycle_ids].blank? @schedule.attributes = permitted_resource_params @@ -42,10 +40,9 @@ module Admin sync_subscriptions_for_create flash[:success] = flash_message_for(@schedule, :successfully_created) - respond_with(@schedule) - else - respond_with(@schedule) end + + respond_with(@schedule) end private @@ -69,7 +66,8 @@ module Admin [:index] end - # In this controller, params like params[:name] are moved into params[:schedule] becoming params[:schedule][:name] + # In this controller, params like params[:name] are moved into + # params[:schedule] becoming params[:schedule][:name]. # For some reason in rails 4, this is not happening for params[:order_cycle_ids] # We do it manually in this filter def adapt_params diff --git a/app/controllers/api/enterprise_attachment_controller.rb b/app/controllers/api/enterprise_attachment_controller.rb index 645a5c092c..3f5f61e5dd 100644 --- a/app/controllers/api/enterprise_attachment_controller.rb +++ b/app/controllers/api/enterprise_attachment_controller.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'api/admin/enterprise_serializer' module Api diff --git a/app/controllers/spree/admin/countries_controller.rb b/app/controllers/spree/admin/countries_controller.rb index 9dc2cc00a3..dedd9fbdb4 100644 --- a/app/controllers/spree/admin/countries_controller.rb +++ b/app/controllers/spree/admin/countries_controller.rb @@ -1,7 +1,6 @@ module Spree module Admin class CountriesController < ResourceController - protected def permitted_resource_params diff --git a/app/controllers/spree/admin/products_controller.rb b/app/controllers/spree/admin/products_controller.rb index fe727ac77e..a6099cf021 100644 --- a/app/controllers/spree/admin/products_controller.rb +++ b/app/controllers/spree/admin/products_controller.rb @@ -159,7 +159,7 @@ module Spree private - def product_set_from_params(params) + def product_set_from_params(_params) collection_hash = Hash[products_params.each_with_index.map { |p, i| [i, p] }] Spree::ProductSet.new(collection_attributes: collection_hash) end diff --git a/app/controllers/spree/admin/shipping_categories_controller.rb b/app/controllers/spree/admin/shipping_categories_controller.rb index 91f476e12f..36c6e27db7 100644 --- a/app/controllers/spree/admin/shipping_categories_controller.rb +++ b/app/controllers/spree/admin/shipping_categories_controller.rb @@ -1,7 +1,6 @@ module Spree module Admin class ShippingCategoriesController < ResourceController - protected def permitted_resource_params diff --git a/app/models/product_import/entry_processor.rb b/app/models/product_import/entry_processor.rb index ec092a27bb..5d705bd4bb 100644 --- a/app/models/product_import/entry_processor.rb +++ b/app/models/product_import/entry_processor.rb @@ -160,7 +160,9 @@ module ProductImport end product = Spree::Product.new - product.assign_attributes(entry.assignable_attributes.except('id', 'on_hand', 'on_demand', 'display_name')) + product.assign_attributes( + entry.assignable_attributes.except('id', 'on_hand', 'on_demand', 'display_name') + ) product.supplier_id = entry.producer_id if product.save diff --git a/app/models/product_import/entry_validator.rb b/app/models/product_import/entry_validator.rb index 0dbbac1468..fa2380c07b 100644 --- a/app/models/product_import/entry_validator.rb +++ b/app/models/product_import/entry_validator.rb @@ -299,7 +299,9 @@ module ProductImport def mark_as_new_product(entry) new_product = Spree::Product.new - new_product.assign_attributes(entry.assignable_attributes.except('id', 'on_hand', 'on_demand', 'display_name')) + new_product.assign_attributes( + entry.assignable_attributes.except('id', 'on_hand', 'on_demand', 'display_name') + ) new_product.supplier_id = entry.producer_id entry.on_hand = 0 if entry.on_hand.nil? diff --git a/app/models/spree/adjustment_decorator.rb b/app/models/spree/adjustment_decorator.rb index ecb91ffcea..ece704bdd1 100644 --- a/app/models/spree/adjustment_decorator.rb +++ b/app/models/spree/adjustment_decorator.rb @@ -12,7 +12,6 @@ module Spree belongs_to :tax_rate, -> { where spree_adjustments: { originator_type: 'Spree::TaxRate' } }, foreign_key: 'originator_id' - scope :enterprise_fee, -> { where(originator_type: 'EnterpriseFee') } scope :admin, -> { where(source_type: nil, originator_type: nil) } scope :included_tax, -> { diff --git a/app/models/spree/line_item_decorator.rb b/app/models/spree/line_item_decorator.rb index 758cd2f89d..5d22c0fbc4 100644 --- a/app/models/spree/line_item_decorator.rb +++ b/app/models/spree/line_item_decorator.rb @@ -75,11 +75,12 @@ Spree::LineItem.class_eval do where('spree_adjustments.id IS NULL') } - # Overridden so that LineItems always have access to soft-deleted Variant attributes - # In some situations, unscoped super will be nil, in these cases we fetch the variant using the variant_id - # See isssue #4946 for more details + # Overridden so that LineItems always have access to soft-deleted Variant + # attributes. In some situations, unscoped super will be nil, in these cases + # we fetch the variant using the variant_id. See isssue #4946 for more + # details def variant - Spree::Variant.unscoped { super } || Spree::Variant.unscoped.find(self.variant_id) + Spree::Variant.unscoped { super } || Spree::Variant.unscoped.find(variant_id) end def cap_quantity_at_stock! diff --git a/app/models/spree/order_decorator.rb b/app/models/spree/order_decorator.rb index deecbc34e9..82609dccbc 100644 --- a/app/models/spree/order_decorator.rb +++ b/app/models/spree/order_decorator.rb @@ -36,7 +36,8 @@ Spree::Order.class_eval do callback.raw_filter.attributes.delete :email end end - validates :email, presence: true, format: /\A([\w\.%\+\-']+)@([\w\-]+\.)+([\w]{2,})\z/i, if: :require_email + validates :email, presence: true, format: /\A([\w\.%\+\-']+)@([\w\-]+\.)+([\w]{2,})\z/i, + if: :require_email before_validation :associate_customer, unless: :customer_id? before_validation :ensure_customer, unless: :customer_is_valid? diff --git a/app/models/spree/order_updater_decorator.rb b/app/models/spree/order_updater_decorator.rb index 029e39c74a..b227444c8b 100644 --- a/app/models/spree/order_updater_decorator.rb +++ b/app/models/spree/order_updater_decorator.rb @@ -1,6 +1,8 @@ +# frozen_string_literal: true + Spree::OrderUpdater.class_eval do # Override spree method to make it update all adjustments as in Spree v2.0.4 def update_shipping_adjustments - order.adjustments.reload.each { |adjustment| adjustment.update! } + order.adjustments.reload.each(&:update!) end end diff --git a/app/models/spree/payment_decorator.rb b/app/models/spree/payment_decorator.rb index 69f4170f19..ec84a83a83 100644 --- a/app/models/spree/payment_decorator.rb +++ b/app/models/spree/payment_decorator.rb @@ -15,9 +15,7 @@ module Spree # We bypass this after_rollback callback that is setup in Spree::Payment # The issues the callback fixes are not experienced in OFN: # if a payment fails on checkout the state "failed" is persisted correctly - def persist_invalid - return - end + def persist_invalid; end def ensure_correct_adjustment revoke_adjustment_eligibility if ['failed', 'invalid'].include?(state) @@ -64,12 +62,12 @@ module Spree record_response(response) if response.success? - self.class.create({ order: order, - source: self, - payment_method: payment_method, - amount: refund_amount.abs * -1, - response_code: response.authorization, - state: 'completed' }) + self.class.create(order: order, + source: self, + payment_method: payment_method, + amount: refund_amount.abs * -1, + response_code: response.authorization, + state: 'completed') else gateway_error(response) end diff --git a/app/models/spree/user.rb b/app/models/spree/user.rb index 80b3adc8b8..3df57a9cf8 100644 --- a/app/models/spree/user.rb +++ b/app/models/spree/user.rb @@ -10,7 +10,6 @@ module Spree before_validation :set_login before_destroy :check_completed_orders - users_table_name = User.table_name roles_table_name = Role.table_name scope :admin, lambda { includes(:spree_roles).where("#{roles_table_name}.name" => "admin") } diff --git a/app/services/permitted_attributes/checkout.rb b/app/services/permitted_attributes/checkout.rb index c5bda0739a..da698a02e5 100644 --- a/app/services/permitted_attributes/checkout.rb +++ b/app/services/permitted_attributes/checkout.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module PermittedAttributes class Checkout def initialize(params) diff --git a/app/services/permitted_attributes/order_cycle.rb b/app/services/permitted_attributes/order_cycle.rb index df15b47d58..6da6175a51 100644 --- a/app/services/permitted_attributes/order_cycle.rb +++ b/app/services/permitted_attributes/order_cycle.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module PermittedAttributes class OrderCycle def initialize(params) @@ -22,9 +24,10 @@ module PermittedAttributes :id, :sender_id, :receiver_id, :enterprise_id, :incoming, :active, :select_all_variants, :receival_instructions, :pickup_time, :pickup_instructions, - :tag_list, :tags => [:text], - :enterprise_fee_ids => [], - :variants => permitted_variant_ids + :tag_list, + tags: [:text], + enterprise_fee_ids: [], + variants: permitted_variant_ids ] end diff --git a/engines/order_management/spec/services/order_management/subscriptions/proxy_order_syncer_spec.rb b/engines/order_management/spec/services/order_management/subscriptions/proxy_order_syncer_spec.rb index 304eae01ef..8af99f27f5 100644 --- a/engines/order_management/spec/services/order_management/subscriptions/proxy_order_syncer_spec.rb +++ b/engines/order_management/spec/services/order_management/subscriptions/proxy_order_syncer_spec.rb @@ -24,22 +24,28 @@ module OrderManagement create(:simple_order_cycle, orders_open_at: now - 1.minute, orders_close_at: now) } let(:open_oc_closes_before_begins_at_oc) { # Open, but closes before begins at - create(:simple_order_cycle, orders_open_at: now - 1.minute, orders_close_at: now + 59.seconds) + create(:simple_order_cycle, + orders_open_at: now - 1.minute, orders_close_at: now + 59.seconds) } let(:open_oc) { # Open & closes between begins at and ends at - create(:simple_order_cycle, orders_open_at: now - 1.minute, orders_close_at: now + 90.seconds) + create(:simple_order_cycle, + orders_open_at: now - 1.minute, orders_close_at: now + 90.seconds) } let(:upcoming_closes_before_begins_at_oc) { # Upcoming, but closes before begins at - create(:simple_order_cycle, orders_open_at: now + 30.seconds, orders_close_at: now + 59.seconds) + create(:simple_order_cycle, + orders_open_at: now + 30.seconds, orders_close_at: now + 59.seconds) } let(:upcoming_closes_on_begins_at_oc) { # Upcoming & closes on begins at - create(:simple_order_cycle, orders_open_at: now + 30.seconds, orders_close_at: now + 1.minute) + create(:simple_order_cycle, + orders_open_at: now + 30.seconds, orders_close_at: now + 1.minute) } let(:upcoming_closes_on_ends_at_oc) { # Upcoming & closes on ends at - create(:simple_order_cycle, orders_open_at: now + 30.seconds, orders_close_at: now + 2.minutes) + create(:simple_order_cycle, + orders_open_at: now + 30.seconds, orders_close_at: now + 2.minutes) } let(:upcoming_closes_after_ends_at_oc) { # Upcoming & closes after ends at - create(:simple_order_cycle, orders_open_at: now + 30.seconds, orders_close_at: now + 121.seconds) + create(:simple_order_cycle, + orders_open_at: now + 30.seconds, orders_close_at: now + 121.seconds) } let(:subscription) { diff --git a/lib/open_food_network/order_cycle_form_applicator.rb b/lib/open_food_network/order_cycle_form_applicator.rb index 63af289913..f42fd427a9 100644 --- a/lib/open_food_network/order_cycle_form_applicator.rb +++ b/lib/open_food_network/order_cycle_form_applicator.rb @@ -142,7 +142,7 @@ module OpenFoodNetwork def find_exchange(sender_id, receiver_id, incoming) @order_cycle.exchanges. - find_by(sender_id:sender_id, receiver_id: receiver_id, incoming: incoming) + find_by(sender_id: sender_id, receiver_id: receiver_id, incoming: incoming) end def incoming_exchange_variant_ids(attrs) diff --git a/lib/open_food_network/permalink_generator.rb b/lib/open_food_network/permalink_generator.rb index eeee1bf18f..c86e6972de 100644 --- a/lib/open_food_network/permalink_generator.rb +++ b/lib/open_food_network/permalink_generator.rb @@ -41,6 +41,6 @@ module PermalinkGenerator self.class.with_deleted else self.class.where(nil) -end + end end end diff --git a/lib/open_food_network/variant_and_line_item_naming.rb b/lib/open_food_network/variant_and_line_item_naming.rb index fca7f25d0d..7eed59b892 100644 --- a/lib/open_food_network/variant_and_line_item_naming.rb +++ b/lib/open_food_network/variant_and_line_item_naming.rb @@ -59,7 +59,8 @@ module OpenFoodNetwork option_type = product.variant_unit_option_type if option_type name = option_value_name - ov = Spree::OptionValue.where(option_type_id: option_type, name: name, presentation: name).first || Spree::OptionValue.create!({ option_type: option_type, name: name, presentation: name }) + ov = Spree::OptionValue.where(option_type_id: option_type, name: name, presentation: name).first || + Spree::OptionValue.create!(option_type: option_type, name: name, presentation: name) option_values << ov end end