Rubocop Corrections and Rubocop To Do Generation

.rubocop.yml

@@ -1,3 +1,5 @@
+inherit_from: .rubocop_todo.yml
+
 require:
   - solidus_dev_support/rubocop
 

.rubocop_todo.yml

@@ -1,49 +1,24 @@
 # This configuration was generated by
 # `rubocop --auto-gen-config`
-# on 2019-11-23 17:04:33 +0100 using RuboCop version 0.76.0.
+# on 2023-05-12 15:47:30 UTC using RuboCop version 1.50.2.
 # 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
 # versions of RuboCop, may require this file to be generated again.
 
-# Offense count: 1
-# Configuration parameters: Include.
-# Include: **/*.gemspec, 
-Gemspec/RequiredRubyVersion:
-  Exclude:
-    - 'solidus_volume_pricing.gemspec'
-
-# Offense count: 2
-Lint/InterpolationCheck:
-  Exclude:
-    - 'spec/lib/solidus_volume_pricing/range_from_string_spec.rb'
-
-# Offense count: 21
-# Cop supports --auto-correct.
-# Configuration parameters: AutoCorrect, AllowHeredoc, AllowURI, URISchemes, IgnoreCopDirectives, IgnoredPatterns.
-# URISchemes: http, https
-Metrics/LineLength:
-  Max: 160
-
-# Offense count: 43
-# Configuration parameters: Prefixes.
+# Offense count: 42
+# Configuration parameters: Prefixes, AllowedPatterns.
 # Prefixes: when, with, without
 RSpec/ContextWording:
   Exclude:
-    - 'spec/controllers/spree/admin/variants_controller_spec.rb'
     - 'spec/helpers/base_helper_spec.rb'
     - 'spec/models/solidus_volume_pricing/pricer_spec.rb'
     - 'spec/models/spree/order_spec.rb'
     - 'spec/models/spree/volume_price_spec.rb'
 
-# Offense count: 2
-RSpec/DescribeClass:
-  Exclude:
-    - 'spec/features/manage_volume_price_models_feature_spec.rb'
-    - 'spec/features/manage_volume_prices_feature_spec.rb'
-
 # Offense count: 1
-# Configuration parameters: CustomTransform, IgnoreMethods.
+# Configuration parameters: Include, CustomTransform, IgnoreMethods, SpecSuffixOnly.
+# Include: **/*_spec*rb*, **/spec/**/*
 RSpec/FilePath:
   Exclude:
     - 'spec/helpers/base_helper_spec.rb'
@@ -56,12 +31,17 @@ RSpec/InstanceVariable:
     - 'spec/models/spree/order_spec.rb'
 
 # Offense count: 5
-# Configuration parameters: AggregateFailuresByDefault.
 RSpec/MultipleExpectations:
   Max: 3
 
-# Offense count: 58
-# Configuration parameters: IgnoreSharedExamples.
+# Offense count: 82
+# Configuration parameters: AllowSubject.
+RSpec/MultipleMemoizedHelpers:
+  Max: 8
+
+# Offense count: 57
+# Configuration parameters: EnforcedStyle, IgnoreSharedExamples.
+# SupportedStyles: always, named_only
 RSpec/NamedSubject:
   Exclude:
     - 'spec/lib/solidus_volume_pricing/range_from_string_spec.rb'
@@ -70,57 +50,19 @@ RSpec/NamedSubject:
     - 'spec/models/spree/volume_price_spec.rb'
 
 # Offense count: 74
+# Configuration parameters: AllowedGroups.
 RSpec/NestedGroups:
   Max: 7
 
-# Offense count: 1
-# Configuration parameters: Include.
-# Include: db/migrate/*.rb
-Rails/CreateTableWithTimestamps:
-  Exclude:
-    - 'db/migrate/20150603143015_create_spree_volume_price_models.rb'
-
-# Offense count: 1
-# Configuration parameters: Include.
-# Include: app/models/**/*.rb
-Rails/HasManyOrHasOneDependent:
+# Offense count: 2
+RSpec/RepeatedExampleGroupDescription:
   Exclude:
-    - 'app/models/spree/volume_price_model.rb'
+    - 'spec/lib/solidus_volume_pricing/range_from_string_spec.rb'
 
 # Offense count: 2
-# Configuration parameters: Include.
+# Configuration parameters: IgnoreScopes, Include.
 # Include: app/models/**/*.rb
 Rails/InverseOf:
   Exclude:
     - 'app/models/spree/volume_price.rb'
     - 'app/models/spree/volume_price_model.rb'
-
-# Offense count: 1
-# Configuration parameters: Include.
-# Include: app/controllers/**/*.rb
-Rails/LexicallyScopedActionFilter:
-  Exclude:
-    - 'app/controllers/spree/admin/volume_price_models_controller.rb'
-
-# Offense count: 1
-# Configuration parameters: Include.
-# Include: app/**/*.rb, config/**/*.rb, db/**/*.rb, lib/**/*.rb
-Rails/Output:
-  Exclude:
-    - 'lib/generators/solidus_volume_pricing/install/install_generator.rb'
-
-# Offense count: 2
-# Configuration parameters: Blacklist, Whitelist.
-# Blacklist: decrement!, decrement_counter, increment!, increment_counter, toggle!, touch, update_all, update_attribute, update_column, update_columns, update_counters
-Rails/SkipsModelValidations:
-  Exclude:
-    - 'spec/models/spree/volume_price_spec.rb'
-
-# Offense count: 2
-# Cop supports --auto-correct.
-# Configuration parameters: AutoCorrect, EnforcedStyle.
-# SupportedStyles: nested, compact
-Style/ClassAndModuleChildren:
-  Exclude:
-    - 'app/models/spree/volume_price.rb'
-    - 'app/models/spree/volume_price_model.rb'

app/controllers/spree/admin/volume_price_models_controller.rb

@@ -3,7 +3,7 @@
 module Spree
   module Admin
     class VolumePriceModelsController < ResourceController
-      before_action :load_volume_prices, only: [:new, :edit]
+      before_action :load_volume_prices, only: [:new, :edit] # rubocop:disable Rails/LexicallyScopedActionFilter
 
       private
 

app/decorators/models/solidus_volume_pricing/spree/variant_decorator.rb

@@ -7,11 +7,13 @@ def self.prepended(base)
         base.class_eval do
           has_and_belongs_to_many :volume_price_models
           has_many :volume_prices, -> { order(position: :asc) }, dependent: :destroy
-          has_many :model_volume_prices, -> { order(position: :asc) }, class_name: '::Spree::VolumePrice', through: :volume_price_models, source: :volume_prices
+          has_many :model_volume_prices, -> {
+            order(position: :asc)
+          }, class_name: '::Spree::VolumePrice', through: :volume_price_models, source: :volume_prices
           accepts_nested_attributes_for :volume_prices, allow_destroy: true,
-                                                        reject_if: proc { |volume_price|
-                                                                     volume_price[:amount].blank? && volume_price[:range].blank?
-                                                                   }
+            reject_if: proc { |volume_price|
+                         volume_price[:amount].blank? && volume_price[:range].blank?
+                       }
         end
       end
 

app/models/spree/volume_price.rb

@@ -1,52 +1,52 @@
 # frozen_string_literal: true
 
-class Spree::VolumePrice < ApplicationRecord
-  belongs_to :variant, touch: true, optional: true
-  belongs_to :volume_price_model, touch: true, optional: true
-  belongs_to :spree_role, class_name: 'Spree::Role', foreign_key: 'role_id', optional: true
-  acts_as_list scope: [:variant_id, :volume_price_model_id]
-
-  validates :amount, presence: true
-  validates :discount_type,
-    presence: true,
-    inclusion: {
-      in: %w(price dollar percent)
-    }
-
-  validate :range_format
-
-  def self.for_variant(variant, user: nil)
-    roles = [nil]
-    if user
-      user.spree_roles.each {|r| roles << r.id}
+module Spree
+  class VolumePrice < ApplicationRecord
+    belongs_to :variant, touch: true, optional: true
+    belongs_to :volume_price_model, touch: true, optional: true
+    belongs_to :spree_role, class_name: 'Spree::Role', foreign_key: 'role_id', optional: true
+    acts_as_list scope: [:variant_id, :volume_price_model_id]
+
+    validates :amount, presence: true
+    validates :discount_type,
+      presence: true,
+      inclusion: {
+        in: %w(price dollar percent)
+      }
+
+    validate :range_format
+
+    def self.for_variant(variant, user: nil)
+      roles = [nil]
+      user&.spree_roles&.each { |r| roles << r.id }
+
+      where(
+        arel_table[:variant_id].eq(variant.id).
+        or(
+          arel_table[:volume_price_model_id].in(variant.volume_price_model_ids)
+        )
+      ).
+        where(role_id: roles).
+        order(position: :asc, amount: :asc)
     end
 
-    where(
-      arel_table[:variant_id].eq(variant.id).
-      or(
-        arel_table[:volume_price_model_id].in(variant.volume_price_model_ids)
-      )
-    ).
-      where(role_id: roles).
-      order(position: :asc, amount: :asc)
-  end
-
-  delegate :include?, to: :range_from_string
+    delegate :include?, to: :range_from_string
 
-  def display_range
-    range.gsub(/\.+/, "-").gsub(/\(|\)/, '')
-  end
+    def display_range
+      range.gsub(/\.+/, "-").gsub(/\(|\)/, '')
+    end
 
-  private
+    private
 
-  def range_format
-    if !(SolidusVolumePricing::RangeFromString::RANGE_FORMAT =~ range ||
-         SolidusVolumePricing::RangeFromString::OPEN_ENDED =~ range)
-      errors.add(:range, :must_be_in_format)
+    def range_format
+      if !(SolidusVolumePricing::RangeFromString::RANGE_FORMAT =~ range ||
+           SolidusVolumePricing::RangeFromString::OPEN_ENDED =~ range)
+        errors.add(:range, :must_be_in_format)
+      end
     end
-  end
 
-  def range_from_string
-    SolidusVolumePricing::RangeFromString.new(range).to_range
+    def range_from_string
+      SolidusVolumePricing::RangeFromString.new(range).to_range
+    end
   end
 end

app/models/spree/volume_price_model.rb

@@ -1,10 +1,12 @@
 # frozen_string_literal: true
 
-class Spree::VolumePriceModel < ApplicationRecord
-  has_many :variants
-  has_many :volume_prices, -> { order(position: :asc) }, dependent: :destroy
-  accepts_nested_attributes_for :volume_prices, allow_destroy: true,
-                                                reject_if: proc { |volume_price|
-                                                             volume_price[:amount].blank? && volume_price[:range].blank?
-                                                           }
+module Spree
+  class VolumePriceModel < ApplicationRecord
+    has_many :variants, dependent: :nullify
+    has_many :volume_prices, -> { order(position: :asc) }, dependent: :destroy
+    accepts_nested_attributes_for :volume_prices, allow_destroy: true,
+      reject_if: proc { |volume_price|
+                   volume_price[:amount].blank? && volume_price[:range].blank?
+                 }
+  end
 end

db/migrate/20150603143015_create_spree_volume_price_models.rb

@@ -10,6 +10,7 @@ def change
     create_table :spree_variants_volume_price_models do |t|
       t.belongs_to :volume_price_model
       t.belongs_to :variant
+      t.timestamps
     end
 
     add_reference :spree_volume_prices, :volume_price_model

lib/generators/solidus_volume_pricing/install/install_generator.rb

@@ -4,7 +4,7 @@ module SolidusVolumePricing
   module Generators
     class InstallGenerator < Rails::Generators::Base
       class_option :auto_run_migrations, type: :boolean, default: false
-      
+
       def self.exit_on_failure?
         true
       end

lib/solidus_volume_pricing.rb

@@ -8,4 +8,4 @@
 require 'solidus_volume_pricing/engine'
 require 'solidus_volume_pricing/version'
 require 'solidus_volume_pricing/range_from_string'
-require 'coffee_script'
+require 'coffee_script'

lib/solidus_volume_pricing/engine.rb

@@ -18,7 +18,7 @@ class Engine < Rails::Engine
     end
 
     def self.activate
-      Dir.glob(File.join(File.dirname(__FILE__), '../../app/**/*_decorator*.rb')) do |c|
+      Dir.glob(File.join(File.dirname(__FILE__), '../../app/**/*_decorator*.rb')).sort.each do |c|
         Rails.configuration.cache_classes ? require(c) : load(c)
         Rails.autoloaders.main.ignore(c) if Rails.autoloaders.zeitwerk_enabled?
       end

spec/controllers/spree/admin/variants_controller_spec.rb

@@ -3,7 +3,7 @@
 RSpec.describe Spree::Admin::VariantsController, type: :controller do
   stub_authorization!
 
-  context 'PUT #update' do
+  describe 'PUT #update' do
     it 'creates a volume price' do
       variant = create :variant
 

spec/features/manage_volume_price_models_feature_spec.rb

@@ -2,7 +2,7 @@
 
 require 'spec_helper'
 
-RSpec.describe 'Managing volume price models' do
+RSpec.describe 'Managing volume price models', type: :system do
   stub_authorization!
 
   it 'an admin can create and remove volume price models', :js do

spec/lib/solidus_volume_pricing/range_from_string_spec.rb

@@ -30,13 +30,13 @@
       it { is_expected.to eq(1...2) }
     end
 
-    context 'with an open-ended string like #{x}+' do
+    context 'with an open-ended string like x+' do
       let(:argument) { '10+' }
 
       it { is_expected.to eq(10..Float::INFINITY) }
     end
 
-    context 'with an open-ended string like #{x}+ and parens' do
+    context 'with an open-ended string like x+" and parens' do
       let(:argument) { '(10+)' }
 
       it { is_expected.to eq(10..Float::INFINITY) }