diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 9527f9e0e2..73912a195d 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -809,33 +809,6 @@ Style/GlobalStdStream: - 'lib/tasks/subscriptions/debug.rake' - 'lib/tasks/subscriptions/test.rake' -# Offense count: 38 -# This cop supports safe autocorrection (--autocorrect). -# Configuration parameters: MinBodyLength, AllowConsecutiveConditionals. -Style/GuardClause: - Exclude: - - 'app/controllers/admin/enterprises_controller.rb' - - 'app/controllers/admin/order_cycles_controller.rb' - - 'app/controllers/admin/product_import_controller.rb' - - 'app/controllers/api/v0/shipments_controller.rb' - - 'app/controllers/application_controller.rb' - - 'app/controllers/home_controller.rb' - - 'app/controllers/spree/orders_controller.rb' - - 'app/models/enterprise.rb' - - 'app/models/enterprise_group.rb' - - 'app/models/producer_property.rb' - - 'app/models/product_import/entry_processor.rb' - - 'app/models/spree/order.rb' - - 'app/models/spree/preferences/preferable_class_methods.rb' - - 'app/services/order_syncer.rb' - - 'engines/order_management/app/services/order_management/order/updater.rb' - - 'lib/discourse/single_sign_on.rb' - - 'lib/open_food_network/order_cycle_form_applicator.rb' - - 'lib/spree/core/controller_helpers/respond_with.rb' - - 'spec/support/request/distribution_helper.rb' - - 'spec/support/request/shop_workflow.rb' - - 'spec/system/support/precompile_assets.rb' - # Offense count: 12 # This cop supports unsafe autocorrection (--autocorrect-all). # Configuration parameters: AllowSplatArgument. diff --git a/app/controllers/admin/enterprises_controller.rb b/app/controllers/admin/enterprises_controller.rb index 170530f144..dbdd701fb8 100644 --- a/app/controllers/admin/enterprises_controller.rb +++ b/app/controllers/admin/enterprises_controller.rb @@ -47,12 +47,12 @@ module Admin @object = Enterprise.where(permalink: params[:id]). includes(users: [:ship_address, :bill_address]).first @object.build_custom_tab if @object.custom_tab.nil? - if params[:stimulus] - @enterprise.is_primary_producer = params[:is_primary_producer] - @enterprise.sells = params[:enterprise_sells] - render cable_ready: cable_car.morph("#side_menu", partial("admin/shared/side_menu")) - .morph("#permalink", partial("admin/enterprises/form/permalink")) - end + return unless params[:stimulus] + + @enterprise.is_primary_producer = params[:is_primary_producer] + @enterprise.sells = params[:enterprise_sells] + render cable_ready: cable_car.morph("#side_menu", partial("admin/shared/side_menu")) + .morph("#permalink", partial("admin/enterprises/form/permalink")) end def welcome @@ -263,32 +263,32 @@ module Admin def update_enterprise_notifications user_id = params[:receives_notifications].to_i - if user_id.positive? && @enterprise.user_ids.include?(user_id) - @enterprise.update_contact(user_id) - end + return unless user_id.positive? && @enterprise.user_ids.include?(user_id) + + @enterprise.update_contact(user_id) end def create_calculator_for(rule, attrs) - if attrs[:calculator_type].present? && attrs[:calculator_attributes].present? - rule.update(calculator_type: attrs[:calculator_type]) - attrs[:calculator_attributes].merge!( id: rule.calculator.id ) - end + return unless attrs[:calculator_type].present? && attrs[:calculator_attributes].present? + + rule.update(calculator_type: attrs[:calculator_type]) + attrs[:calculator_attributes].merge!( id: rule.calculator.id ) end def check_can_change_bulk_sells - unless spree_current_user.admin? - params[:sets_enterprise_set][:collection_attributes].each do |_i, enterprise_params| - unless spree_current_user == Enterprise.find_by(id: enterprise_params[:id]).owner - enterprise_params.delete :sells - end + return if spree_current_user.admin? + + params[:sets_enterprise_set][:collection_attributes].each do |_i, enterprise_params| + unless spree_current_user == Enterprise.find_by(id: enterprise_params[:id]).owner + enterprise_params.delete :sells end end end def check_can_change_sells - unless spree_current_user.admin? || spree_current_user == @enterprise.owner - enterprise_params.delete :sells - end + return if spree_current_user.admin? || spree_current_user == @enterprise.owner + + enterprise_params.delete :sells end def override_owner @@ -296,31 +296,31 @@ module Admin end def override_sells - unless spree_current_user.admin? - has_hub = spree_current_user.owned_enterprises.is_hub.any? - new_enterprise_is_producer = Enterprise.new(enterprise_params).is_primary_producer - enterprise_params[:sells] = has_hub && !new_enterprise_is_producer ? 'any' : 'none' - end + return if spree_current_user.admin? + + has_hub = spree_current_user.owned_enterprises.is_hub.any? + new_enterprise_is_producer = Enterprise.new(enterprise_params).is_primary_producer + enterprise_params[:sells] = has_hub && !new_enterprise_is_producer ? 'any' : 'none' end def check_can_change_owner - unless ( spree_current_user == @enterprise.owner ) || spree_current_user.admin? + return if ( spree_current_user == @enterprise.owner ) || spree_current_user.admin? + + enterprise_params.delete :owner_id + end + + def check_can_change_bulk_owner + return if spree_current_user.admin? + + bulk_params[:collection_attributes].each do |_i, enterprise_params| enterprise_params.delete :owner_id end end - def check_can_change_bulk_owner - unless spree_current_user.admin? - bulk_params[:collection_attributes].each do |_i, enterprise_params| - enterprise_params.delete :owner_id - end - end - end - def check_can_change_managers - unless ( spree_current_user == @enterprise.owner ) || spree_current_user.admin? - enterprise_params.delete :user_ids - end + return if ( spree_current_user == @enterprise.owner ) || spree_current_user.admin? + + enterprise_params.delete :user_ids end def strip_new_properties diff --git a/app/controllers/admin/order_cycles_controller.rb b/app/controllers/admin/order_cycles_controller.rb index 1158ec9259..3df12684ff 100644 --- a/app/controllers/admin/order_cycles_controller.rb +++ b/app/controllers/admin/order_cycles_controller.rb @@ -182,17 +182,17 @@ module Admin end def load_data_for_index - if json_request? - # Split ransack params into all those that currently exist and new ones - # to limit returned ocs to recent or undated - orders_close_at_gt = raw_params[:q]&.delete(:orders_close_at_gt) || 31.days.ago - raw_params[:q] = { - g: [raw_params.delete(:q) || {}, { m: 'or', - orders_close_at_gt: orders_close_at_gt, - orders_close_at_null: true }] - } - @collection = collection - end + return unless json_request? + + # Split ransack params into all those that currently exist and new ones + # to limit returned ocs to recent or undated + orders_close_at_gt = raw_params[:q]&.delete(:orders_close_at_gt) || 31.days.ago + raw_params[:q] = { + g: [raw_params.delete(:q) || {}, { m: 'or', + orders_close_at_gt: orders_close_at_gt, + orders_close_at_null: true }] + } + @collection = collection end def redirect_to_after_update_path @@ -245,10 +245,10 @@ module Admin order_cycle_params.delete :coordinator_id - unless Enterprise.managed_by(spree_current_user).include?(@order_cycle.coordinator) - order_cycle_params.delete_if do |k, _v| - [:name, :orders_open_at, :orders_close_at].include? k.to_sym - end + return if Enterprise.managed_by(spree_current_user).include?(@order_cycle.coordinator) + + order_cycle_params.delete_if do |k, _v| + [:name, :orders_open_at, :orders_close_at].include? k.to_sym end end diff --git a/app/controllers/admin/product_import_controller.rb b/app/controllers/admin/product_import_controller.rb index 6d342b5cc6..181870f3b2 100644 --- a/app/controllers/admin/product_import_controller.rb +++ b/app/controllers/admin/product_import_controller.rb @@ -56,9 +56,9 @@ module Admin private def validate_upload_presence - unless params[:file] || (params[:filepath] && File.exist?(params[:filepath])) - redirect_to '/admin/product_import', notice: I18n.t(:product_import_file_not_found_notice) - end + return if params[:file] || (params[:filepath] && File.exist?(params[:filepath])) + + redirect_to '/admin/product_import', notice: I18n.t(:product_import_file_not_found_notice) end def process_data(method) @@ -90,11 +90,11 @@ module Admin end def check_spreadsheet_has_data(importer) - unless importer.item_count - redirect_to '/admin/product_import', - notice: I18n.t(:product_import_no_data_in_spreadsheet_notice) - true - end + return if importer.item_count + + redirect_to '/admin/product_import', + notice: I18n.t(:product_import_no_data_in_spreadsheet_notice) + true end def save_uploaded_file(upload) diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 3587072cc1..f0bc1c0255 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -115,25 +115,25 @@ class ApplicationController < ActionController::Base end def require_distributor_chosen - unless (@distributor = current_distributor) - redirect_to main_app.root_path - false - end + return if (@distributor = current_distributor) + + redirect_to main_app.root_path + false end def require_order_cycle - unless current_order_cycle - redirect_to main_app.shop_path - end + return if current_order_cycle + + redirect_to main_app.shop_path end def check_hub_ready_for_checkout - if current_distributor_closed? - current_order.empty! - current_order.set_distribution! nil, nil - flash[:info] = I18n.t('order_cycles_closed_for_hub') - redirect_to main_app.root_url - end + return unless current_distributor_closed? + + current_order.empty! + current_order.set_distribution! nil, nil + flash[:info] = I18n.t('order_cycles_closed_for_hub') + redirect_to main_app.root_url end def current_distributor_closed? diff --git a/app/controllers/home_controller.rb b/app/controllers/home_controller.rb index 2dd5455de1..34d9ad59d8 100644 --- a/app/controllers/home_controller.rb +++ b/app/controllers/home_controller.rb @@ -4,14 +4,14 @@ class HomeController < BaseController layout 'darkswarm' def index - if ContentConfig.home_show_stats - @num_distributors = cached_count('distributors', Enterprise.is_distributor.activated.visible) - @num_producers = cached_count('producers', Enterprise.is_primary_producer.activated.visible) - @num_orders = cached_count('orders', Spree::Order.complete) - @num_users = cached_count( - 'users', Spree::Order.complete.select('DISTINCT spree_orders.user_id') - ) - end + return unless ContentConfig.home_show_stats + + @num_distributors = cached_count('distributors', Enterprise.is_distributor.activated.visible) + @num_producers = cached_count('producers', Enterprise.is_primary_producer.activated.visible) + @num_orders = cached_count('orders', Spree::Order.complete) + @num_users = cached_count( + 'users', Spree::Order.complete.select('DISTINCT spree_orders.user_id') + ) end def sell; end diff --git a/app/controllers/spree/orders_controller.rb b/app/controllers/spree/orders_controller.rb index 501e6605dd..113f827628 100644 --- a/app/controllers/spree/orders_controller.rb +++ b/app/controllers/spree/orders_controller.rb @@ -133,10 +133,10 @@ module Spree end def filter_order_params - if params[:order] && params[:order][:line_items_attributes] - params[:order][:line_items_attributes] = - remove_missing_line_items(params[:order][:line_items_attributes]) - end + return unless params[:order] && params[:order][:line_items_attributes] + + params[:order][:line_items_attributes] = + remove_missing_line_items(params[:order][:line_items_attributes]) end def remove_missing_line_items(attrs) @@ -180,10 +180,10 @@ module Spree items = params[:order][:line_items_attributes] &.select{ |_k, attrs| attrs["quantity"].to_i > 0 } - if items.empty? - flash[:error] = I18n.t(:orders_cannot_remove_the_final_item) - redirect_to main_app.order_path(order_to_update) - end + return unless items.empty? + + flash[:error] = I18n.t(:orders_cannot_remove_the_final_item) + redirect_to main_app.order_path(order_to_update) end def order_params diff --git a/app/models/enterprise.rb b/app/models/enterprise.rb index f1efb62eb9..2553cfd176 100644 --- a/app/models/enterprise.rb +++ b/app/models/enterprise.rb @@ -515,10 +515,10 @@ class Enterprise < ApplicationRecord end def enforce_ownership_limit - unless owner.can_own_more_enterprises? - errors.add(:owner, I18n.t(:enterprise_owner_error, email: owner.email, - enterprise_limit: owner.enterprise_limit )) - end + return if owner.can_own_more_enterprises? + + errors.add(:owner, I18n.t(:enterprise_owner_error, email: owner.email, + enterprise_limit: owner.enterprise_limit )) end def set_default_contact @@ -543,26 +543,26 @@ class Enterprise < ApplicationRecord end # All pre-existing producers grant permission to new hubs - if is_hub - enterprises.is_primary_producer.each do |enterprise| - EnterpriseRelationship.create!(parent: enterprise, - child: self, - permissions_list: [:add_to_order_cycle, - :create_variant_overrides]) - end + return unless is_hub + + enterprises.is_primary_producer.each do |enterprise| + EnterpriseRelationship.create!(parent: enterprise, + child: self, + permissions_list: [:add_to_order_cycle, + :create_variant_overrides]) end end def shopfront_taxons - unless preferred_shopfront_taxon_order =~ /\A((\d+,)*\d+)?\z/ - errors.add(:shopfront_category_ordering, "must contain a list of taxons.") - end + return if preferred_shopfront_taxon_order =~ /\A((\d+,)*\d+)?\z/ + + errors.add(:shopfront_category_ordering, "must contain a list of taxons.") end def shopfront_producers - unless preferred_shopfront_producer_order =~ /\A((\d+,)*\d+)?\z/ - errors.add(:shopfront_category_ordering, "must contain a list of producers.") - end + return if preferred_shopfront_producer_order =~ /\A((\d+,)*\d+)?\z/ + + errors.add(:shopfront_category_ordering, "must contain a list of producers.") end def restore_permalink diff --git a/app/models/enterprise_group.rb b/app/models/enterprise_group.rb index deadc531e3..91be3db4fd 100644 --- a/app/models/enterprise_group.rb +++ b/app/models/enterprise_group.rb @@ -77,9 +77,9 @@ class EnterpriseGroup < ApplicationRecord private def sanitize_permalink - if permalink.blank? || permalink_changed? - requested = permalink.presence || permalink_was.presence || name.presence || 'group' - self.permalink = create_unique_permalink(requested.parameterize) - end + return unless permalink.blank? || permalink_changed? + + requested = permalink.presence || permalink_was.presence || name.presence || 'group' + self.permalink = create_unique_permalink(requested.parameterize) end end diff --git a/app/models/producer_property.rb b/app/models/producer_property.rb index 3cae30a59b..d636157ad5 100644 --- a/app/models/producer_property.rb +++ b/app/models/producer_property.rb @@ -13,9 +13,9 @@ class ProducerProperty < ApplicationRecord end def property_name=(name) - if name.present? - self.property = Spree::Property.find_by(name: name) || - Spree::Property.create(name: name, presentation: name) - end + return unless name.present? + + self.property = Spree::Property.find_by(name: name) || + Spree::Property.create(name: name, presentation: name) end end diff --git a/app/models/product_import/entry_processor.rb b/app/models/product_import/entry_processor.rb index 05b90ca856..d2603e2762 100644 --- a/app/models/product_import/entry_processor.rb +++ b/app/models/product_import/entry_processor.rb @@ -39,10 +39,10 @@ module ProductImport end end - if total_saved_count.zero? - @importer.errors.add(:importer, - I18n.t(:product_importer_products_save_error)) - end + return unless total_saved_count.zero? + + @importer.errors.add(:importer, + I18n.t(:product_importer_products_save_error)) end def count_existing_items diff --git a/app/models/spree/order.rb b/app/models/spree/order.rb index ea823f06b5..6798e9596b 100644 --- a/app/models/spree/order.rb +++ b/app/models/spree/order.rb @@ -505,10 +505,10 @@ module Spree # an order is part-way through checkout and the user changes items in the cart; in that case # we need to reset the checkout flow to ensure the order is processed correctly. def ensure_updated_shipments - if !completed? && shipments.any? - shipments.destroy_all - restart_checkout_flow - end + return unless !completed? && shipments.any? + + shipments.destroy_all + restart_checkout_flow end # After changing line items of a completed order diff --git a/app/models/spree/preferences/preferable_class_methods.rb b/app/models/spree/preferences/preferable_class_methods.rb index dd9369d3a0..4bff9406f6 100644 --- a/app/models/spree/preferences/preferable_class_methods.rb +++ b/app/models/spree/preferences/preferable_class_methods.rb @@ -64,9 +64,9 @@ module Spree if method_defined? preference_type_getter_method(name) remove_method preference_type_getter_method(name) end - if method_defined? preference_description_getter_method(name) - remove_method preference_description_getter_method(name) - end + return unless method_defined? preference_description_getter_method(name) + + remove_method preference_description_getter_method(name) end def preference_getter_method(name) diff --git a/app/services/order_syncer.rb b/app/services/order_syncer.rb index 71b515cef0..7ba47823e3 100644 --- a/app/services/order_syncer.rb +++ b/app/services/order_syncer.rb @@ -87,9 +87,9 @@ class OrderSyncer (ship_address.changes.keys & relevant_address_attrs).any? save_ship_address_in_order(order) end - if !pickup_to_delivery || order.shipment.blank? - order.updater.shipping_address_from_distributor - end + return unless !pickup_to_delivery || order.shipment.blank? + + order.updater.shipping_address_from_distributor end def relevant_address_attrs diff --git a/engines/order_management/app/services/order_management/order/updater.rb b/engines/order_management/app/services/order_management/order/updater.rb index 7646f07521..6c311dade7 100644 --- a/engines/order_management/app/services/order_management/order/updater.rb +++ b/engines/order_management/app/services/order_management/order/updater.rb @@ -163,9 +163,9 @@ module OrderManagement update_shipment_state end - if payment.completed? || order.completed? - persist_totals - end + return unless payment.completed? || order.completed? + + persist_totals end private diff --git a/lib/discourse/single_sign_on.rb b/lib/discourse/single_sign_on.rb index f5f2995b58..5e2db79461 100644 --- a/lib/discourse/single_sign_on.rb +++ b/lib/discourse/single_sign_on.rb @@ -30,13 +30,12 @@ module Discourse if sso.sign(parsed["sso"]) != parsed["sig"] diags = "\n\nsso: #{parsed['sso']}\n\nsig: #{parsed['sig']}\n\n" \ "expected sig: #{sso.sign(parsed['sso'])}" - if parsed["sso"] =~ %r{[^a-zA-Z0-9=\r\n/+]}m - raise "The SSO field should be Base64 encoded, using only A-Z, a-z, 0-9, +, /, " \ - "and = characters. Your input contains characters we don't understand as Base64, " \ - "see http://en.wikipedia.org/wiki/Base64 #{diags}" - else - raise "Bad signature for payload #{diags}" - end + raise "Bad signature for payload #{diags}" unless parsed["sso"] =~ %r{[^a-zA-Z0-9=\r\n/+]}m + + raise "The SSO field should be Base64 encoded, using only A-Z, a-z, 0-9, +, /, " \ + "and = characters. Your input contains characters we don't understand as Base64, " \ + "see http://en.wikipedia.org/wiki/Base64 #{diags}" + end decoded = Base64.decode64(parsed["sso"]) diff --git a/lib/open_food_network/order_cycle_form_applicator.rb b/lib/open_food_network/order_cycle_form_applicator.rb index cf96640c87..5397be7a31 100644 --- a/lib/open_food_network/order_cycle_form_applicator.rb +++ b/lib/open_food_network/order_cycle_form_applicator.rb @@ -73,12 +73,12 @@ module OpenFoodNetwork variant_ids = attrs.delete :variant_ids exchange = @order_cycle.exchanges.build attrs - if manages_coordinator? - exchange.save! - ExchangeVariantBulkUpdater.new(exchange).update!(variant_ids) unless variant_ids.nil? + return unless manages_coordinator? - @touched_exchanges << exchange - end + exchange.save! + ExchangeVariantBulkUpdater.new(exchange).update!(variant_ids) unless variant_ids.nil? + + @touched_exchanges << exchange end def update_exchange(sender_id, receiver_id, incoming, attrs = {}) @@ -104,9 +104,9 @@ module OpenFoodNetwork end def destroy_untouched_exchanges - if manages_coordinator? - untouched_exchanges.each(&:destroy) - end + return unless manages_coordinator? + + untouched_exchanges.each(&:destroy) end def untouched_exchanges diff --git a/lib/spree/core/controller_helpers/respond_with.rb b/lib/spree/core/controller_helpers/respond_with.rb index 3035cdea91..3e0f641297 100644 --- a/lib/spree/core/controller_helpers/respond_with.rb +++ b/lib/spree/core/controller_helpers/respond_with.rb @@ -39,12 +39,10 @@ module ActionController block.call(collector) if block_given? format = collector.negotiate_format(request) - if format - _process_format(format) - collector - else - raise ActionController::UnknownFormat - end + raise ActionController::UnknownFormat unless format + + _process_format(format) + collector end end end diff --git a/spec/support/request/distribution_helper.rb b/spec/support/request/distribution_helper.rb index 7cd550f164..d0b00ec18b 100644 --- a/spec/support/request/distribution_helper.rb +++ b/spec/support/request/distribution_helper.rb @@ -7,9 +7,9 @@ module OpenFoodNetwork visit root_path click_link distributor.name - if order_cycle && page.has_select?('order_order_cycle_id') - select_by_value order_cycle.id, from: 'order_order_cycle_id' - end + return unless order_cycle && page.has_select?('order_order_cycle_id') + + select_by_value order_cycle.id, from: 'order_order_cycle_id' end end end diff --git a/spec/support/request/shop_workflow.rb b/spec/support/request/shop_workflow.rb index 386fe4977f..81a3ebdab2 100644 --- a/spec/support/request/shop_workflow.rb +++ b/spec/support/request/shop_workflow.rb @@ -124,9 +124,9 @@ module ShopWorkflow # and not all needed enterprises are loaded into the shop page. def ensure_supplier_exchange(exchange, supplier) oc = exchange.order_cycle - if oc.exchanges.from_enterprise(supplier).incoming.empty? - create(:exchange, order_cycle: oc, incoming: true, - sender: supplier, receiver: oc.coordinator) - end + return unless oc.exchanges.from_enterprise(supplier).incoming.empty? + + create(:exchange, order_cycle: oc, incoming: true, + sender: supplier, receiver: oc.coordinator) end end diff --git a/spec/system/support/precompile_assets.rb b/spec/system/support/precompile_assets.rb index a06a0731f0..2cf819cbf1 100644 --- a/spec/system/support/precompile_assets.rb +++ b/spec/system/support/precompile_assets.rb @@ -15,12 +15,10 @@ RSpec.configure do |config| config.before(:suite) do # We can use webpack-dev-server for tests, too! # Useful if you working on a frontend code fixes and want to verify them via system tests. - if Webpacker.dev_server.running? - next - else - $stdout.puts "\n Precompiling assets.\n" + next if Webpacker.dev_server.running? - Webpacker.compile - end + $stdout.puts "\n Precompiling assets.\n" + + Webpacker.compile end end