Merge pull request #11454 from macanudo527/fix_rubocop_8

Fix Rubocop Style Errors

.rubocop_todo.yml

@@ -1,6 +1,6 @@
 # This configuration was generated by
 # `rubocop --auto-gen-config --auto-gen-only-exclude --exclude-limit 1400 --no-auto-gen-timestamp`
-# using RuboCop version 1.56.0.
+# using RuboCop version 1.56.1.
 # The point is for the user to remove these configuration records
 # one by one as the offenses are removed from the code base.
 # Note that changes in the inspected code, or installation of new
@@ -98,12 +98,6 @@ Lint/NoReturnInBeginEndBlocks:
   Exclude:
     - 'app/controllers/payment_gateways/stripe_controller.rb'
 
-# Offense count: 2
-# This cop supports unsafe autocorrection (--autocorrect-all).
-Lint/NonAtomicFileOperation:
-  Exclude:
-    - 'app/services/bulk_invoice_service.rb'
-
 # Offense count: 4
 # This cop supports unsafe autocorrection (--autocorrect-all).
 Lint/RedundantDirGlobSort:
@@ -192,7 +186,7 @@ Metrics/BlockNesting:
   Exclude:
     - 'app/models/spree/payment/processing.rb'
 
-# Offense count: 45
+# Offense count: 43
 # Configuration parameters: CountComments, Max, CountAsOne.
 Metrics/ClassLength:
   Exclude:
@@ -789,13 +783,6 @@ Style/ClassVars:
   Exclude:
     - 'lib/spree/core/delegate_belongs_to.rb'
 
-# Offense count: 2
-# This cop supports safe autocorrection (--autocorrect).
-Style/FileRead:
-  Exclude:
-    - 'lib/tasks/karma.rake'
-    - 'spec/services/upload_sanitizer_spec.rb'
-
 # Offense count: 2
 # This cop supports safe autocorrection (--autocorrect).
 # Configuration parameters: MaxUnannotatedPlaceholdersAllowed, AllowedMethods, AllowedPatterns.
@@ -822,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.
@@ -871,7 +831,7 @@ Style/HashLikeCase:
   Exclude:
     - 'app/models/enterprise.rb'
 
-# Offense count: 1784
+# Offense count: 1767
 # This cop supports safe autocorrection (--autocorrect).
 # Configuration parameters: EnforcedStyle, EnforcedShorthandSyntax, UseHashRocketsWithSymbolValues, PreferHashRocketsForNonAlnumEndingSymbols.
 # SupportedStyles: ruby19, hash_rockets, no_mixed_keys, ruby19_no_mixed_keys
@@ -1055,7 +1015,6 @@ Style/HashSyntax:
     - 'spec/controllers/payment_gateways/stripe_controller_spec.rb'
     - 'spec/controllers/split_checkout_controller_spec.rb'
     - 'spec/controllers/spree/admin/adjustments_controller_spec.rb'
-    - 'spec/controllers/spree/admin/invoices_controller_spec.rb'
     - 'spec/controllers/spree/admin/orders/customer_details_controller_spec.rb'
     - 'spec/controllers/spree/admin/orders/payments/payments_controller_refunds_spec.rb'
     - 'spec/controllers/spree/admin/orders/payments/payments_controller_spec.rb'

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

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
 

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)

app/controllers/api/v0/shipments_controller.rb

@@ -44,14 +44,16 @@ module Api
 
       def ready
         authorize! :read, Spree::Shipment
-        unless @shipment.ready?
-          if @shipment.can_ready?
-            @shipment.ready!
-          else
-            render(json: { error: I18n.t(:cannot_ready, scope: "spree.api.shipment") },
-                   status: :unprocessable_entity) && return
-          end
+
+        unless @shipment.ready? || @shipment.can_ready?
+          return render(
+            json: { error: I18n.t(:cannot_ready, scope: "spree.api.shipment") },
+            status: :unprocessable_entity
+          )
         end
+
+        @shipment.ready! unless @shipment.ready?
+
         render json: @shipment, serializer: Api::ShipmentSerializer, status: :ok
       end
 

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?

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

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

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

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

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 if name.blank?
+
+    self.property = Spree::Property.find_by(name: name) ||
+                    Spree::Property.create(name: name, presentation: name)
   end
 end

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

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

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)

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

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

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"])

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

lib/reporting/reports/users_and_enterprises/base.rb

@@ -24,7 +24,7 @@ module Reporting
         def owners_and_enterprises
           query = Enterprise
             .joins("LEFT JOIN spree_users AS owner ON enterprises.owner_id = owner.id")
-            .where("enterprises.id IS NOT NULL")
+            .where.not(enterprises: { id: nil })
 
           query = filter_by_int_list_if_present(query, "enterprises.id", params[:enterprise_id_in])
           query = filter_by_int_list_if_present(query, "owner.id", params[:user_id_in])
@@ -36,8 +36,8 @@ module Reporting
           query = Enterprise
             .joins("LEFT JOIN enterprise_roles ON enterprises.id = enterprise_roles.enterprise_id")
             .joins("LEFT JOIN spree_users AS managers ON enterprise_roles.user_id = managers.id")
-            .where("enterprise_id IS NOT NULL")
-            .where("user_id IS NOT NULL")
+            .where.not('enterprise_roles.enterprise_id': nil)
+            .where.not('enterprise_roles.user_id': nil)
 
           query = filter_by_int_list_if_present(query, "enterprise_id", params[:enterprise_id_in])
           query = filter_by_int_list_if_present(query, "user_id", params[:user_id_in])

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

lib/tasks/karma.rake

@@ -30,7 +30,7 @@ namespace :karma do
 
   def unit_js(files)
     puts files
-    unit_js = File.open('config/ng-test.conf.js', 'r').read
+    unit_js = File.read('config/ng-test.conf.js')
     unit_js.gsub "APPLICATION_SPEC", "\"#{files.join("\",\n\"")}\""
   end
 

spec/controllers/api/v0/shipments_controller_spec.rb

@@ -102,6 +102,17 @@ describe Api::V0::ShipmentsController, type: :controller do
       expect(shipment.reload.state).to eq("ready")
     end
 
+    it "checks if shipment is ready already" do
+      allow_any_instance_of(Spree::Order).to receive_messages(paid?: true, complete?: true)
+      shipment.ready!
+
+      api_put :ready, order_id: shipment.order.to_param, id: shipment.to_param
+
+      expect(attributes.all?{ |attr| json_response.key? attr.to_s }).to be_truthy
+      expect(json_response["state"]).to eq("ready")
+      expect(shipment.reload.state).to eq("ready")
+    end
+
     it "cannot make a shipment ready if the order is unpaid" do
       allow_any_instance_of(Spree::Order).to receive_messages(paid?: false)
       api_put :ready, order_id: shipment.order.to_param, id: shipment.to_param

spec/services/upload_sanitizer_spec.rb

@@ -10,7 +10,7 @@ describe UploadSanitizer do
         f << "Test"
       end
     end
-    let(:service) { UploadSanitizer.new(File.open(upload).read) }
+    let(:service) { UploadSanitizer.new(File.read(upload)) }
 
     it "sanitizes the uploaded file" do
       sanitized_upload = service.call

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

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

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