Fix easy rubocop issues

app/models/spree/app_configuration.rb

@@ -25,23 +25,29 @@ require "spree/core/search/base"
 
 module Spree
   class AppConfiguration < Preferences::Configuration
-    # Alphabetized to more easily lookup particular preferences
-    preference :address_requires_state, :boolean, default: true # should state/state_name be required
+    # Should state/state_name be required
+    preference :address_requires_state, :boolean, default: true
     preference :admin_interface_logo, :string, default: 'logo/spree_50.png'
     preference :admin_products_per_page, :integer, default: 10
-    preference :allow_backorder_shipping, :boolean, default: false # should only be true if you don't need to track inventory
+    # Should only be true if you don't need to track inventory
+    preference :allow_backorder_shipping, :boolean, default: false
     preference :allow_checkout_on_gateway_error, :boolean, default: false
     preference :allow_guest_checkout, :boolean, default: true
     preference :allow_ssl_in_development_and_test, :boolean, default: false
     preference :allow_ssl_in_production, :boolean, default: true
     preference :allow_ssl_in_staging, :boolean, default: true
-    preference :alternative_billing_phone, :boolean, default: false # Request extra phone for bill addr
-    preference :alternative_shipping_phone, :boolean, default: false # Request extra phone for ship addr
+    # Request extra phone for bill addr
+    preference :alternative_billing_phone, :boolean, default: false
+    # Request extra phone for ship addr
+    preference :alternative_shipping_phone, :boolean, default: false
     preference :always_put_site_name_in_title, :boolean, default: true
-    preference :auto_capture, :boolean, default: false # automatically capture the credit card (as opposed to just authorize and capture later)
+    # Automatically capture the credit card (as opposed to just authorize and capture later)
+    preference :auto_capture, :boolean, default: false
     preference :check_for_spree_alerts, :boolean, default: true
-    preference :checkout_zone, :string, default: nil # replace with the name of a zone if you would like to limit the countries
-    preference :company, :boolean, default: false # Request company field for billing and shipping addr
+    # Replace with the name of a zone if you would like to limit the countries
+    preference :checkout_zone, :string, default: nil
+    # Request company field for billing and shipping addr
+    preference :company, :boolean, default: false
     preference :currency, :string, default: "USD"
     preference :currency_decimal_mark, :string, default: "."
     preference :currency_symbol_position, :string, default: "before"
@@ -56,28 +62,36 @@ module Spree
     preference :last_check_for_spree_alerts, :string, default: nil
     preference :layout, :string, default: 'spree/layouts/spree_application'
     preference :logo, :string, default: 'logo/spree_50.png'
-    preference :max_level_in_taxons_menu, :integer, default: 1 # maximum nesting level in taxons menu
+    # Maximum nesting level in taxons menu
+    preference :max_level_in_taxons_menu, :integer, default: 1
     preference :orders_per_page, :integer, default: 15
     preference :prices_inc_tax, :boolean, default: false
     preference :products_per_page, :integer, default: 12
     preference :redirect_https_to_http, :boolean, default: false
     preference :require_master_price, :boolean, default: true
     preference :shipment_inc_vat, :boolean, default: false
-    preference :shipping_instructions, :boolean, default: false # Request instructions/info for shipping
+    # Request instructions/info for shipping
+    preference :shipping_instructions, :boolean, default: false
     preference :show_only_complete_orders_by_default, :boolean, default: true
-    preference :show_variant_full_price, :boolean, default: false # Displays variant full price or difference with product price. Default false to be compatible with older behavior
+    # Displays variant full price or difference with product price.
+    preference :show_variant_full_price, :boolean, default: false
     preference :show_products_without_price, :boolean, default: false
     preference :show_raw_product_description, :boolean, default: false
     preference :site_name, :string, default: 'Spree Demo Site'
     preference :site_url, :string, default: 'demo.spreecommerce.com'
     preference :tax_using_ship_address, :boolean, default: true
-    preference :track_inventory_levels, :boolean, default: true # Determines whether to track on_hand values for variants / products.
+    # Determines whether to track on_hand values for variants / products.
+    preference :track_inventory_levels, :boolean, default: true
 
     # Preferences related to image settings
-    preference :attachment_default_url, :string, default: '/spree/products/:id/:style/:basename.:extension'
-    preference :attachment_path, :string, default: ':rails_root/public/spree/products/:id/:style/:basename.:extension'
-    preference :attachment_url, :string, default: '/spree/products/:id/:style/:basename.:extension'
-    preference :attachment_styles, :string, default: "{\"mini\":\"48x48>\",\"small\":\"100x100>\",\"product\":\"240x240>\",\"large\":\"600x600>\"}"
+    preference :attachment_default_url, :string,
+               default: '/spree/products/:id/:style/:basename.:extension'
+    preference :attachment_path, :string,
+               default: ':rails_root/public/spree/products/:id/:style/:basename.:extension'
+    preference :attachment_url, :string,
+               default: '/spree/products/:id/:style/:basename.:extension'
+    preference :attachment_styles, :string,
+               default: "{\"mini\":\"48x48>\",\"small\":\"100x100>\",\"product\":\"240x240>\",\"large\":\"600x600>\"}"
     preference :attachment_default_style, :string, default: 'product'
     preference :s3_access_key, :string
     preference :s3_bucket, :string
@@ -98,7 +112,8 @@ module Spree
     preference :mail_host, :string, default: 'localhost'
     preference :mail_domain, :string, default: 'localhost'
     preference :mail_port, :integer, default: 25
-    preference :secure_connection_type, :string, default: Core::MailSettings::SECURE_CONNECTION_TYPES[0]
+    preference :secure_connection_type, :string,
+               default: Core::MailSettings::SECURE_CONNECTION_TYPES[0]
     preference :mail_auth_type, :string, default: Core::MailSettings::MAIL_AUTH[0]
     preference :smtp_username, :string
     preference :smtp_password, :string
@@ -139,7 +154,8 @@ module Spree
     preference :enable_localized_number?, :boolean, default: false
 
     # Enable cache
-    preference :enable_products_cache?, :boolean, default: (Rails.env.production? || Rails.env.staging?)
+    preference :enable_products_cache?, :boolean,
+               default: (Rails.env.production? || Rails.env.staging?)
 
     # searcher_class allows spree extension writers to provide their own Search class
     def searcher_class

app/models/spree/preference.rb

@@ -7,7 +7,10 @@ module Spree
     validates :key, presence: true
     validates :value_type, presence: true
 
-    scope :valid, -> { where(Spree::Preference.arel_table[:key].not_eq(nil)).where(Spree::Preference.arel_table[:value_type].not_eq(nil)) }
+    scope :valid, -> {
+      where(Spree::Preference.arel_table[:key].not_eq(nil)).
+        where(Spree::Preference.arel_table[:value_type].not_eq(nil))
+    }
 
     # The type conversions here should match
     # the ones in spree::preferences::preferrable#convert_preference_value
@@ -23,7 +26,7 @@ module Spree
         when :integer
           self[:value].to_i
         when :boolean
-          (self[:value].to_s =~ /^[t|1]/i) != nil
+          !(self[:value].to_s =~ /^[t|1]/i).nil?
         else
           self[:value].is_a?(String) ? YAML.safe_load(self[:value]) : self[:value]
         end

app/models/spree/preferences/configuration.rb

@@ -52,9 +52,9 @@ module Spree
           set_preference name, value
         end
 
-        if args.size == 2
-          set_preference args[0], args[1]
-        end
+        return unless args.size == 2
+
+        set_preference args[0], args[1]
       end
 
       def method_missing(method, *args)

app/models/spree/preferences/preferable.rb

@@ -29,29 +29,29 @@ module Spree
 
       def get_preference(name)
         has_preference! name
-        send self.class.preference_getter_method(name)
+        __send__ self.class.preference_getter_method(name)
       end
       alias :preferred :get_preference
       alias :prefers? :get_preference
 
       def set_preference(name, value)
         has_preference! name
-        send self.class.preference_setter_method(name), value
+        __send__ self.class.preference_setter_method(name), value
       end
 
       def preference_type(name)
         has_preference! name
-        send self.class.preference_type_getter_method(name)
+        __send__ self.class.preference_type_getter_method(name)
       end
 
       def preference_default(name)
         has_preference! name
-        send self.class.preference_default_getter_method(name)
+        __send__ self.class.preference_default_getter_method(name)
       end
 
       def preference_description(name)
         has_preference! name
-        send self.class.preference_description_getter_method(name)
+        __send__ self.class.preference_description_getter_method(name)
       end
 
       def has_preference!(name)
@@ -65,15 +65,11 @@ module Spree
       def preferences
         prefs = {}
         methods.grep(/^prefers_.*\?$/).each do |pref_method|
-          prefs[pref_method.to_s.gsub(/prefers_|\?/, '').to_sym] = send(pref_method)
+          prefs[pref_method.to_s.gsub(/prefers_|\?/, '').to_sym] = __send__(pref_method)
         end
         prefs
       end
 
-      def prefers?(name)
-        get_preference(name)
-      end
-
       def preference_cache_key(name)
         return unless id
 
@@ -118,7 +114,7 @@ module Spree
         when :boolean
           if value.is_a?(FalseClass) ||
              value.nil? ||
-             value == 0 ||
+             value.zero? ||
              value =~ /^(f|false|0)$/i ||
              (value.respond_to?(:empty?) && value.empty?)
             false

app/models/spree/preferences/preferable_class_methods.rb

@@ -46,13 +46,27 @@ module Spree
       end
 
       def remove_preference(name)
-        remove_method preference_getter_method(name) if method_defined? preference_getter_method(name)
-        remove_method preference_setter_method(name) if method_defined? preference_setter_method(name)
-        remove_method prefers_getter_method(name) if method_defined? prefers_getter_method(name)
-        remove_method prefers_setter_method(name) if method_defined? prefers_setter_method(name)
-        remove_method preference_default_getter_method(name) if method_defined? preference_default_getter_method(name)
-        remove_method preference_type_getter_method(name) if method_defined? preference_type_getter_method(name)
-        remove_method preference_description_getter_method(name) if method_defined? preference_description_getter_method(name)
+        if method_defined? preference_getter_method(name)
+          remove_method preference_getter_method(name)
+        end
+        if method_defined? preference_setter_method(name)
+          remove_method preference_setter_method(name)
+        end
+        if method_defined? prefers_getter_method(name)
+          remove_method prefers_getter_method(name)
+        end
+        if method_defined? prefers_setter_method(name)
+          remove_method prefers_setter_method(name)
+        end
+        if method_defined? preference_default_getter_method(name)
+          remove_method preference_default_getter_method(name)
+        end
+        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
       end
 
       def preference_getter_method(name)

spec/models/spree/app_configuration_spec.rb

@@ -3,7 +3,7 @@
 require 'spec_helper'
 
 describe Spree::AppConfiguration do
-  let (:prefs) { Rails.application.config.spree.preferences }
+  let(:prefs) { Rails.application.config.spree.preferences }
 
   it "should be available from the environment" do
     prefs.site_name = "TEST SITE NAME"

spec/models/spree/preferences/preferable_spec.rb

@@ -34,24 +34,24 @@ describe Spree::Preferences::Preferable do
 
   describe "preference definitions" do
     it "parent should not see child definitions" do
-      @a.has_preference?(:color).should be_true
-      @a.has_preference?(:flavor).should_not be_true
+      expect(@a.has_preference?(:color)).to be_true
+      expect(@a.has_preference?(:flavor)).not_to be_true
     end
 
     it "child should have parent and own definitions" do
-      @b.has_preference?(:color).should be_true
-      @b.has_preference?(:flavor).should be_true
+      expect(@b.has_preference?(:color)).to be_true
+      expect(@b.has_preference?(:flavor)).to be_true
     end
 
     it "instances have defaults" do
-      @a.preferred_color.should eq 'green'
-      @b.preferred_color.should eq 'green'
-      @b.preferred_flavor.should be_nil
+      expect(@a.preferred_color).to eq 'green'
+      expect(@b.preferred_color).to eq 'green'
+      expect(@b.preferred_flavor).to be_nil
     end
 
     it "can be asked if it has a preference definition" do
-      @a.has_preference?(:color).should be_true
-      @a.has_preference?(:bad).should be_false
+      expect(@a.has_preference?(:color)).to be_true
+      expect(@a.has_preference?(:bad)).to be_false
     end
 
     it "can be asked and raises" do
@@ -61,18 +61,18 @@ describe Spree::Preferences::Preferable do
     end
 
     it "has a type" do
-      @a.preferred_color_type.should eq :string
-      @a.preference_type(:color).should eq :string
+      expect(@a.preferred_color_type).to eq :string
+      expect(@a.preference_type(:color)).to eq :string
     end
 
     it "has a default" do
-      @a.preferred_color_default.should eq 'green'
-      @a.preference_default(:color).should eq 'green'
+      expect(@a.preferred_color_default).to eq 'green'
+      expect(@a.preference_default(:color)).to eq 'green'
     end
 
     it "has a description" do
-      @a.preferred_color_description.should eq "My Favorite Color"
-      @a.preference_description(:color).should eq "My Favorite Color"
+      expect(@a.preferred_color_description).to eq "My Favorite Color"
+      expect(@a.preference_description(:color)).to eq "My Favorite Color"
     end
 
     it "raises if not defined" do
@@ -85,24 +85,24 @@ describe Spree::Preferences::Preferable do
   describe "preference access" do
     it "handles ghost methods for preferences" do
       @a.preferred_color = 'blue'
-      @a.preferred_color.should eq 'blue'
+      expect(@a.preferred_color).to eq 'blue'
 
       @a.prefers_color = 'green'
-      @a.prefers_color?.should eq 'green'
+      expect(@a.prefers_color?).to eq 'green'
     end
 
     it "has genric readers" do
       @a.preferred_color = 'red'
-      @a.prefers?(:color).should eq 'red'
-      @a.preferred(:color).should eq 'red'
+      expect(@a.prefers?(:color)).to eq 'red'
+      expect(@a.preferred(:color)).to eq 'red'
     end
 
     it "parent and child instances have their own prefs" do
       @a.preferred_color = 'red'
       @b.preferred_color = 'blue'
 
-      @a.preferred_color.should eq 'red'
-      @b.preferred_color.should eq 'blue'
+      expect(@a.preferred_color).to eq 'red'
+      expect(@b.preferred_color).to eq 'blue'
     end
 
     it "raises when preference not defined" do
@@ -113,8 +113,8 @@ describe Spree::Preferences::Preferable do
 
     it "builds a hash of preferences" do
       @b.preferred_flavor = :strawberry
-      @b.preferences[:flavor].should eq 'strawberry'
-      @b.preferences[:color].should eq 'green' # default from A
+      expect(@b.preferences[:flavor]).to eq 'strawberry'
+      expect(@b.preferences[:color]).to eq 'green' # default from A
     end
 
     context "database fallback" do
@@ -124,13 +124,13 @@ describe Spree::Preferences::Preferable do
 
       it "retrieves a preference from the database before falling back to default" do
         preference = double(value: "chatreuse", key: 'a/color/123')
-        Spree::Preference.should_receive(:find_by_key).and_return(preference)
-        @a.preferred_color.should == 'chatreuse'
+        expect(Spree::Preference).to receive(:find_by_key).and_return(preference)
+        expect(@a.preferred_color).to eq 'chatreuse'
       end
 
       it "defaults if no database key exists" do
-        Spree::Preference.should_receive(:find_by_key).and_return(nil)
-        @a.preferred_color.should == 'green'
+        expect(Spree::Preference).to receive(:find_by_key).and_return(nil)
+        expect(@a.preferred_color).to eq 'green'
       end
     end
 
@@ -141,10 +141,10 @@ describe Spree::Preferences::Preferable do
 
       it "with strings" do
         @a.set_preference(:is_integer, '3')
-        @a.preferences[:is_integer].should == 3
+        expect(@a.preferences[:is_integer]).to eq 3
 
         @a.set_preference(:is_integer, '')
-        @a.preferences[:is_integer].should == 0
+        expect(@a.preferences[:is_integer]).to eq 0
       end
     end
 
@@ -155,15 +155,15 @@ describe Spree::Preferences::Preferable do
 
       it "returns a BigDecimal" do
         @a.set_preference(:if_decimal, 3.3)
-        @a.preferences[:if_decimal].class.should == BigDecimal
+        expect(@a.preferences[:if_decimal].class).to eq BigDecimal
       end
 
       it "with strings" do
         @a.set_preference(:if_decimal, '3.3')
-        @a.preferences[:if_decimal].should == 3.3
+        expect(@a.preferences[:if_decimal]).to eq 3.3
 
         @a.set_preference(:if_decimal, '')
-        @a.preferences[:if_decimal].should == 0.0
+        expect(@a.preferences[:if_decimal]).to eq 0.0
       end
     end
 
@@ -174,28 +174,28 @@ describe Spree::Preferences::Preferable do
 
       it "with strings" do
         @a.set_preference(:is_boolean, '0')
-        @a.preferences[:is_boolean].should be_false
+        expect(@a.preferences[:is_boolean]).to be_false
         @a.set_preference(:is_boolean, 'f')
-        @a.preferences[:is_boolean].should be_false
+        expect(@a.preferences[:is_boolean]).to be_false
         @a.set_preference(:is_boolean, 't')
-        @a.preferences[:is_boolean].should be_true
+        expect(@a.preferences[:is_boolean]).to be_true
       end
 
       it "with integers" do
         @a.set_preference(:is_boolean, 0)
-        @a.preferences[:is_boolean].should be_false
+        expect(@a.preferences[:is_boolean]).to be_false
         @a.set_preference(:is_boolean, 1)
-        @a.preferences[:is_boolean].should be_true
+        expect(@a.preferences[:is_boolean]).to be_true
       end
 
       it "with an empty string" do
         @a.set_preference(:is_boolean, '')
-        @a.preferences[:is_boolean].should be_false
+        expect(@a.preferences[:is_boolean]).to be_false
       end
 
       it "with an empty hash" do
         @a.set_preference(:is_boolean, [])
-        @a.preferences[:is_boolean].should be_false
+        expect(@a.preferences[:is_boolean]).to be_false
       end
     end
 
@@ -206,15 +206,16 @@ describe Spree::Preferences::Preferable do
       end
 
       it "with array" do
-        @a.preferences[:product_ids].should == []
+        expect(@a.preferences[:product_ids]).to eq []
         @a.set_preference(:product_ids, [1, 2])
-        @a.preferences[:product_ids].should == [1, 2]
+        expect(@a.preferences[:product_ids]).to eq [1, 2]
       end
 
       it "with hash" do
-        @a.preferences[:product_attributes].should == {}
+        expect(@a.preferences[:product_attributes]).to eq {}
         @a.set_preference(:product_attributes, { id: 1, name: 2 })
-        @a.preferences[:product_attributes].should == { id: 1, name: 2 }
+        attributes_hash = { id: 1, name: 2 }
+        expect(@a.preferences[:product_attributes]).to eq attributes_hash
       end
     end
   end
@@ -256,46 +257,46 @@ describe Spree::Preferences::Preferable do
       it "saves preferences after record is saved" do
         pr = PrefTest.new
         pr.set_preference(:pref_test_pref, 'XXX')
-        pr.get_preference(:pref_test_pref).should == 'XXX'
+        expect(pr.get_preference(:pref_test_pref)).to eq 'XXX'
         pr.save!
-        pr.get_preference(:pref_test_pref).should == 'XXX'
+        expect(pr.get_preference(:pref_test_pref)).to eq 'XXX'
       end
 
       it "saves preferences for serialized object" do
         pr = PrefTest.new
         pr.set_preference(:pref_test_any, [1, 2])
-        pr.get_preference(:pref_test_any).should == [1, 2]
+        expect(pr.get_preference(:pref_test_any)).to eq [1, 2]
         pr.save!
-        pr.get_preference(:pref_test_any).should == [1, 2]
+        expect(pr.get_preference(:pref_test_any)).to eq [1, 2]
       end
     end
 
     describe "requires a valid id" do
       it "for cache_key" do
         pref_test = PrefTest.new
-        pref_test.preference_cache_key(:pref_test_pref).should be_nil
+        expect(pref_test.preference_cache_key(:pref_test_pref)).to be_nil
 
         pref_test.save
-        pref_test.preference_cache_key(:pref_test_pref).should_not be_nil
+        expect(pref_test.preference_cache_key(:pref_test_pref)).to_not be_nil
       end
 
       it "but returns default values" do
         pref_test = PrefTest.new
-        pref_test.get_preference(:pref_test_pref).should == 'abc'
+        expect(pref_test.get_preference(:pref_test_pref)).to eq 'abc'
       end
 
       it "adds prefs in a pending hash until after_create" do
         pref_test = PrefTest.new
-        pref_test.should_receive(:add_pending_preference).with(:pref_test_pref, 'XXX')
+        expect(pref_test).to receive(:add_pending_preference).with(:pref_test_pref, 'XXX')
         pref_test.set_preference(:pref_test_pref, 'XXX')
       end
     end
 
     it "clear preferences" do
       @pt.set_preference(:pref_test_pref, 'xyz')
-      @pt.preferred_pref_test_pref.should == 'xyz'
+      expect(@pt.preferred_pref_test_pref).to eq 'xyz'
       @pt.clear_preferences
-      @pt.preferred_pref_test_pref.should == 'abc'
+      expect(@pt.preferred_pref_test_pref).to eq 'abc'
     end
 
     it "clear preferences when record is deleted" do
@@ -306,20 +307,20 @@ describe Spree::Preferences::Preferable do
       @pt1 = PrefTest.new(col: 'aaaa')
       @pt1.id = @pt.id
       @pt1.save!
-      @pt1.get_preference(:pref_test_pref).should_not == 'lmn'
-      @pt1.get_preference(:pref_test_pref).should == 'abc'
+      expect(@pt1.get_preference(:pref_test_pref)).to_not eq 'lmn'
+      expect(@pt1.get_preference(:pref_test_pref)).to eq 'abc'
     end
   end
 
   it "builds cache keys" do
-    @a.preference_cache_key(:color).should match %r{a/color/\d+}
+    expect(@a.preference_cache_key(:color)).to match %r{a/color/\d+}
   end
 
   it "can add and remove preferences" do
     A.preference :test_temp, :boolean, default: true
-    @a.preferred_test_temp.should be_true
+    expect(@a.preferred_test_temp).to be_true
     A.remove_preference :test_temp
-    @a.has_preference?(:test_temp).should be_false
-    @a.respond_to?(:preferred_test_temp).should be_false
+    expect(@a.has_preference?(:test_temp)).to be_false
+    expect(@a.respond_to?(:preferred_test_temp)).to be_false
   end
 end

spec/models/spree/preferences/store_spec.rb

@@ -35,7 +35,7 @@ describe Spree::Preferences::Store do
     Rails.cache.read(:test).should be_false
   end
 
-  it "should return and cache fallback value when persistence is disabled (i.e. during bootstrap)" do
+  it "should return and cache fallback value when persistence is disabled (i.e. on bootstrap)" do
     Rails.cache.clear
     @store.stub(should_persist?: false)
     @store.get(:test, true).should be_true