From 8b42543ca34ef88ebd8aec8af834dfd224d427b3 Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Thu, 6 Aug 2020 11:39:09 +0100 Subject: [PATCH 01/11] Bring configuration and preferences code from spree_core --- app/models/spree/app_configuration.rb | 120 +++++++ app/models/spree/preference.rb | 35 ++ app/models/spree/preferences/configuration.rb | 71 ++++ app/models/spree/preferences/preferable.rb | 137 ++++++++ .../preferences/preferable_class_methods.rb | 86 +++++ app/models/spree/preferences/store.rb | 98 ++++++ spec/models/spree/app_configuration_spec.rb | 61 ++++ spec/models/spree/preference_spec.rb | 98 ++++++ .../spree/preferences/configuration_spec.rb | 30 ++ .../spree/preferences/preferable_spec.rb | 331 ++++++++++++++++++ spec/models/spree/preferences/store_spec.rb | 47 +++ 11 files changed, 1114 insertions(+) create mode 100644 app/models/spree/app_configuration.rb create mode 100644 app/models/spree/preference.rb create mode 100644 app/models/spree/preferences/configuration.rb create mode 100644 app/models/spree/preferences/preferable.rb create mode 100644 app/models/spree/preferences/preferable_class_methods.rb create mode 100644 app/models/spree/preferences/store.rb create mode 100644 spec/models/spree/app_configuration_spec.rb create mode 100644 spec/models/spree/preference_spec.rb create mode 100644 spec/models/spree/preferences/configuration_spec.rb create mode 100644 spec/models/spree/preferences/preferable_spec.rb create mode 100644 spec/models/spree/preferences/store_spec.rb diff --git a/app/models/spree/app_configuration.rb b/app/models/spree/app_configuration.rb new file mode 100644 index 0000000000..2442cb0790 --- /dev/null +++ b/app/models/spree/app_configuration.rb @@ -0,0 +1,120 @@ +# This is the primary location for defining spree preferences +# +# The expectation is that this is created once and stored in +# the spree environment +# +# setters: +# a.color = :blue +# a[:color] = :blue +# a.set :color = :blue +# a.preferred_color = :blue +# +# getters: +# a.color +# a[:color] +# a.get :color +# a.preferred_color +# +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 + 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 + 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 + 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) + 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 + preference :currency, :string, default: "USD" + preference :currency_decimal_mark, :string, default: "." + preference :currency_symbol_position, :string, default: "before" + preference :currency_thousands_separator, :string, default: "," + preference :display_currency, :boolean, default: false + preference :default_country_id, :integer + preference :default_meta_description, :string, default: 'Spree demo site' + preference :default_meta_keywords, :string, default: 'spree, demo' + preference :default_seo_title, :string, default: '' + preference :dismissed_spree_alerts, :string, default: '' + preference :hide_cents, :boolean, default: false + 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 + 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 + 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 + 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. + + # 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_style, :string, default: 'product' + preference :s3_access_key, :string + preference :s3_bucket, :string + preference :s3_secret, :string + preference :s3_headers, :string, default: "{\"Cache-Control\":\"max-age=31557600\"}" + preference :use_s3, :boolean, default: false # Use S3 for images rather than the file system + preference :s3_protocol, :string + preference :s3_host_alias, :string + + # Default mail headers settings + preference :enable_mail_delivery, :boolean, :default => false + preference :mails_from, :string, :default => 'spree@example.com' + preference :mail_bcc, :string, :default => 'spree@example.com' + preference :intercept_email, :string, :default => nil + + # Default smtp settings + preference :override_actionmailer_config, :boolean, :default => true + 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 :mail_auth_type, :string, :default => Core::MailSettings::MAIL_AUTH[0] + preference :smtp_username, :string + preference :smtp_password, :string + + # searcher_class allows spree extension writers to provide their own Search class + def searcher_class + @searcher_class ||= Spree::Core::Search::Base + end + + def searcher_class=(sclass) + @searcher_class = sclass + end + + attr_writer :package_factory, :order_updater_decorator + + def package_factory + @package_factory ||= Spree::Stock::Package + end + + def order_updater_decorator + @order_updater_decorator ||= NullDecorator + end + end +end diff --git a/app/models/spree/preference.rb b/app/models/spree/preference.rb new file mode 100644 index 0000000000..046ce1bcc0 --- /dev/null +++ b/app/models/spree/preference.rb @@ -0,0 +1,35 @@ +class Spree::Preference < ActiveRecord::Base + serialize :value + + 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)) } + + # The type conversions here should match + # the ones in spree::preferences::preferrable#convert_preference_value + def value + if self[:value_type].present? + case self[:value_type].to_sym + when :string, :text + self[:value].to_s + when :password + self[:value].to_s + when :decimal + BigDecimal.new(self[:value].to_s).round(2, BigDecimal::ROUND_HALF_UP) + when :integer + self[:value].to_i + when :boolean + (self[:value].to_s =~ /^[t|1]/i) != nil + else + self[:value].is_a?(String) ? YAML.load(self[:value]) : self[:value] + end + else + self[:value] + end + end + + def raw_value + self[:value] + end +end diff --git a/app/models/spree/preferences/configuration.rb b/app/models/spree/preferences/configuration.rb new file mode 100644 index 0000000000..8636c96ed4 --- /dev/null +++ b/app/models/spree/preferences/configuration.rb @@ -0,0 +1,71 @@ +# This takes the preferrable methods and adds some +# syntatic sugar to access the preferences +# +# class App < Configuration +# preference :color, :string +# end +# +# a = App.new +# +# setters: +# a.color = :blue +# a[:color] = :blue +# a.set :color = :blue +# a.preferred_color = :blue +# +# getters: +# a.color +# a[:color] +# a.get :color +# a.preferred_color +# +# +module Spree::Preferences + class Configuration + include Spree::Preferences::Preferable + + def configure + yield(self) if block_given? + end + + def preference_cache_key(name) + [ENV['RAILS_CACHE_ID'], self.class.name, name].flatten.join('::').underscore + end + + def reset + preferences.each do |name, value| + set_preference name, preference_default(name) + end + end + + alias :[] :get_preference + alias :[]= :set_preference + + alias :get :get_preference + + def set(*args) + options = args.extract_options! + options.each do |name, value| + set_preference name, value + end + + if args.size == 2 + set_preference args[0], args[1] + end + end + + def method_missing(method, *args) + name = method.to_s.gsub('=', '') + if has_preference? name + if method.to_s =~ /=$/ + set_preference(name, args.first) + else + get_preference name + end + else + super + end + end + + end +end diff --git a/app/models/spree/preferences/preferable.rb b/app/models/spree/preferences/preferable.rb new file mode 100644 index 0000000000..a02ab9265d --- /dev/null +++ b/app/models/spree/preferences/preferable.rb @@ -0,0 +1,137 @@ +# The preference_cache_key is used to determine if the preference +# can be set. The default behavior is to return nil if there is no +# id value. On ActiveRecords, new objects will have their preferences +# saved to a pending hash until it is persisted. +# +# class_attributes are inheritied unless you reassign them in +# the subclass, so when you inherit a Preferable class, the +# inherited hook will assign a new hash for the subclass definitions +# and copy all the definitions allowing the subclass to add +# additional defintions without affecting the base +module Spree::Preferences::Preferable + + def self.included(base) + base.class_eval do + extend Spree::Preferences::PreferableClassMethods + + if respond_to?(:after_create) + after_create do |obj| + obj.save_pending_preferences + end + end + + if respond_to?(:after_destroy) + after_destroy do |obj| + obj.clear_preferences + end + end + + end + end + + def get_preference(name) + has_preference! 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 + end + + def preference_type(name) + has_preference! 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) + end + + def preference_description(name) + has_preference! name + send self.class.preference_description_getter_method(name) + end + + def has_preference!(name) + raise NoMethodError.new "#{name} preference not defined" unless has_preference? name + end + + def has_preference?(name) + respond_to? self.class.preference_getter_method(name) + end + + def preferences + prefs = {} + methods.grep(/^prefers_.*\?$/).each do |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 + [ENV["RAILS_CACHE_ID"], self.class.name, name, id].join('::').underscore + end + + def save_pending_preferences + return unless @pending_preferences + @pending_preferences.each do |name, value| + set_preference(name, value) + end + end + + def clear_preferences + preferences.keys.each {|pref| preference_store.delete preference_cache_key(pref)} + end + + private + + def add_pending_preference(name, value) + @pending_preferences ||= {} + @pending_preferences[name] = value + end + + def get_pending_preference(name) + return unless @pending_preferences + @pending_preferences[name] + end + + def convert_preference_value(value, type) + case type + when :string, :text + value.to_s + when :password + value.to_s + when :decimal + BigDecimal.new(value.to_s).round(2, BigDecimal::ROUND_HALF_UP) + when :integer + value.to_i + when :boolean + if value.is_a?(FalseClass) || + value.nil? || + value == 0 || + value =~ /^(f|false|0)$/i || + (value.respond_to? :empty? and value.empty?) + false + else + true + end + else + value + end + end + + def preference_store + Spree::Preferences::Store.instance + end + +end + diff --git a/app/models/spree/preferences/preferable_class_methods.rb b/app/models/spree/preferences/preferable_class_methods.rb new file mode 100644 index 0000000000..180bc8c8f5 --- /dev/null +++ b/app/models/spree/preferences/preferable_class_methods.rb @@ -0,0 +1,86 @@ +module Spree::Preferences + module PreferableClassMethods + + def preference(name, type, *args) + options = args.extract_options! + options.assert_valid_keys(:default, :description) + default = options[:default] + description = options[:description] || name + + # cache_key will be nil for new objects, then if we check if there + # is a pending preference before going to default + define_method preference_getter_method(name) do + + # perference_cache_key will only be nil/false for new records + # + if preference_cache_key(name) + preference_store.get(preference_cache_key(name), default) + else + get_pending_preference(name) || default + end + end + alias_method prefers_getter_method(name), preference_getter_method(name) + + define_method preference_setter_method(name) do |value| + value = convert_preference_value(value, type) + if preference_cache_key(name) + preference_store.set preference_cache_key(name), value, type + else + add_pending_preference(name, value) + end + end + alias_method prefers_setter_method(name), preference_setter_method(name) + + define_method preference_default_getter_method(name) do + default + end + + define_method preference_type_getter_method(name) do + type + end + + define_method preference_description_getter_method(name) do + description + end + 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) + end + + def preference_getter_method(name) + "preferred_#{name}".to_sym + end + + def preference_setter_method(name) + "preferred_#{name}=".to_sym + end + + def prefers_getter_method(name) + "prefers_#{name}?".to_sym + end + + def prefers_setter_method(name) + "prefers_#{name}=".to_sym + end + + def preference_default_getter_method(name) + "preferred_#{name}_default".to_sym + end + + def preference_type_getter_method(name) + "preferred_#{name}_type".to_sym + end + + def preference_description_getter_method(name) + "preferred_#{name}_description".to_sym + end + + end +end diff --git a/app/models/spree/preferences/store.rb b/app/models/spree/preferences/store.rb new file mode 100644 index 0000000000..6c9c566fb7 --- /dev/null +++ b/app/models/spree/preferences/store.rb @@ -0,0 +1,98 @@ +# Use singleton class Spree::Preferences::Store.instance to access +# +# StoreInstance has a persistence flag that is on by default, +# but we disable database persistence in testing to speed up tests +# + +require 'singleton' + +module Spree::Preferences + + class StoreInstance + attr_accessor :persistence + + def initialize + @cache = Rails.cache + @persistence = true + end + + def set(key, value, type) + @cache.write(key, value) + persist(key, value, type) + end + + def exist?(key) + @cache.exist?(key) || + should_persist? && Spree::Preference.where(:key => key).exists? + end + + def get(key,fallback=nil) + # return the retrieved value, if it's in the cache + # use unless nil? incase the value is actually boolean false + # + unless (val = @cache.read(key)).nil? + return val + end + + if should_persist? + # If it's not in the cache, maybe it's in the database, but + # has been cleared from the cache + + # does it exist in the database? + if Spree::Preference.table_exists? && preference = Spree::Preference.find_by_key(key) + # it does exist, so let's put it back into the cache + @cache.write(preference.key, preference.value) + + # and return the value + return preference.value + end + end + + unless fallback.nil? + # cache fallback so we won't hit the db above on + # subsequent queries for the same key + # + @cache.write(key, fallback) + end + + return fallback + end + + def delete(key) + @cache.delete(key) + destroy(key) + end + + def clear_cache + @cache.clear + end + + private + + def persist(cache_key, value, type) + return unless should_persist? + + preference = Spree::Preference.where(:key => cache_key).first_or_initialize + preference.value = value + preference.value_type = type + preference.save + end + + def destroy(cache_key) + return unless should_persist? + + preference = Spree::Preference.find_by_key(cache_key) + preference.destroy if preference + end + + def should_persist? + @persistence && Spree::Preference.connected? && Spree::Preference.table_exists? + end + + end + + class Store < StoreInstance + include Singleton + end + +end diff --git a/spec/models/spree/app_configuration_spec.rb b/spec/models/spree/app_configuration_spec.rb new file mode 100644 index 0000000000..96503470c9 --- /dev/null +++ b/spec/models/spree/app_configuration_spec.rb @@ -0,0 +1,61 @@ +require 'spec_helper' + +describe Spree::AppConfiguration do + + let (:prefs) { Rails.application.config.spree.preferences } + + it "should be available from the environment" do + prefs.site_name = "TEST SITE NAME" + prefs.site_name.should eq "TEST SITE NAME" + end + + it "should be available as Spree::Config for legacy access" do + Spree::Config.site_name = "Spree::Config TEST SITE NAME" + Spree::Config.site_name.should eq "Spree::Config TEST SITE NAME" + end + + it "uses base searcher class by default" do + prefs.searcher_class = nil + prefs.searcher_class.should eq Spree::Core::Search::Base + end + + it 'uses Spree::Stock::Package by default' do + prefs.package_factory = nil + prefs.package_factory.should eq Spree::Stock::Package + end + + context 'when a package factory is specified' do + class TestPackageFactory; end + + around do |example| + default_factory = prefs.package_factory + example.run + prefs.package_factory = default_factory + end + + it 'uses the set package factory' do + prefs.package_factory = TestPackageFactory + prefs.package_factory.should eq TestPackageFactory + end + end + + it 'uses Spree::NullDecorator by default' do + prefs.order_updater_decorator = nil + prefs.order_updater_decorator.should eq Spree::NullDecorator + end + + context 'when an order_updater_decorator is specified' do + class FakeOrderUpdaterDecorator; end + + around do |example| + default_decorator = prefs.order_updater_decorator + example.run + prefs.order_updater_decorator = default_decorator + end + + it 'uses the set order_updater_decorator' do + prefs.order_updater_decorator = FakeOrderUpdaterDecorator + prefs.order_updater_decorator.should eq FakeOrderUpdaterDecorator + end + end +end diff --git a/spec/models/spree/preference_spec.rb b/spec/models/spree/preference_spec.rb new file mode 100644 index 0000000000..79cc9c2494 --- /dev/null +++ b/spec/models/spree/preference_spec.rb @@ -0,0 +1,98 @@ +require 'spec_helper' + +describe Spree::Preference do + + it "should require a key" do + @preference = Spree::Preference.new + @preference.key = :test + @preference.value_type = :boolean + @preference.value = true + @preference.should be_valid + end + + describe "type coversion for values" do + def round_trip_preference(key, value, value_type) + p = Spree::Preference.new + p.value = value + p.value_type = value_type + p.key = key + p.save + + Spree::Preference.find_by_key(key) + end + + it ":boolean" do + value_type = :boolean + value = true + key = "boolean_key" + pref = round_trip_preference(key, value, value_type) + pref.value.should eq value + pref.value_type.should == value_type.to_s + end + + it "false :boolean" do + value_type = :boolean + value = false + key = "boolean_key" + pref = round_trip_preference(key, value, value_type) + pref.value.should eq value + pref.value_type.should == value_type.to_s + end + + it ":integer" do + value_type = :integer + value = 10 + key = "integer_key" + pref = round_trip_preference(key, value, value_type) + pref.value.should eq value + pref.value_type.should == value_type.to_s + end + + it ":decimal" do + value_type = :decimal + value = 1.5 + key = "decimal_key" + pref = round_trip_preference(key, value, value_type) + pref.value.should eq value + pref.value_type.should == value_type.to_s + end + + it ":string" do + value_type = :string + value = "This is a string" + key = "string_key" + pref = round_trip_preference(key, value, value_type) + pref.value.should eq value + pref.value_type.should == value_type.to_s + end + + it ":text" do + value_type = :text + value = "This is a string stored as text" + key = "text_key" + pref = round_trip_preference(key, value, value_type) + pref.value.should eq value + pref.value_type.should == value_type.to_s + end + + it ":password" do + value_type = :password + value = "This is a password" + key = "password_key" + pref = round_trip_preference(key, value, value_type) + pref.value.should eq value + pref.value_type.should == value_type.to_s + end + + it ":any" do + value_type = :any + value = [1, 2] + key = "any_key" + pref = round_trip_preference(key, value, value_type) + pref.value.should eq value + pref.value_type.should == value_type.to_s + end + + end + +end diff --git a/spec/models/spree/preferences/configuration_spec.rb b/spec/models/spree/preferences/configuration_spec.rb new file mode 100644 index 0000000000..a1b45d0534 --- /dev/null +++ b/spec/models/spree/preferences/configuration_spec.rb @@ -0,0 +1,30 @@ +require 'spec_helper' + +describe Spree::Preferences::Configuration do + + before :all do + class AppConfig < Spree::Preferences::Configuration + preference :color, :string, :default => :blue + end + @config = AppConfig.new + end + + it "has named methods to access preferences" do + @config.color = 'orange' + @config.color.should eq 'orange' + end + + it "uses [ ] to access preferences" do + @config[:color] = 'red' + @config[:color].should eq 'red' + end + + it "uses set/get to access preferences" do + @config.set :color, 'green' + @config.get(:color).should eq 'green' + end + +end + + + diff --git a/spec/models/spree/preferences/preferable_spec.rb b/spec/models/spree/preferences/preferable_spec.rb new file mode 100644 index 0000000000..e01fcf8114 --- /dev/null +++ b/spec/models/spree/preferences/preferable_spec.rb @@ -0,0 +1,331 @@ +require 'spec_helper' + +describe Spree::Preferences::Preferable do + + before :all do + class A + include Spree::Preferences::Preferable + attr_reader :id + + def initialize + @id = rand(999) + end + + preference :color, :string, :default => 'green', :description => "My Favorite Color" + end + + class B < A + preference :flavor, :string + end + end + + before :each do + @a = A.new + @a.stub(:persisted? => true) + @b = B.new + @b.stub(:persisted? => true) + + # ensure we're persisting as that is the default + # + store = Spree::Preferences::Store.instance + store.persistence = true + end + + 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 + end + + it "child should have parent and own definitions" do + @b.has_preference?(:color).should be_true + @b.has_preference?(:flavor).should 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 + 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 + end + + it "can be asked and raises" do + expect { + @a.has_preference! :flavor + }.to raise_error(NoMethodError, "flavor preference not defined") + end + + it "has a type" do + @a.preferred_color_type.should eq :string + @a.preference_type(:color).should eq :string + end + + it "has a default" do + @a.preferred_color_default.should eq 'green' + @a.preference_default(:color).should 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" + end + + it "raises if not defined" do + expect { + @a.get_preference :flavor + }.to raise_error(NoMethodError, "flavor preference not defined") + end + + end + + describe "preference access" do + it "handles ghost methods for preferences" do + @a.preferred_color = 'blue' + @a.preferred_color.should eq 'blue' + + @a.prefers_color = 'green' + @a.prefers_color?.should eq 'green' + end + + it "has genric readers" do + @a.preferred_color = 'red' + @a.prefers?(:color).should eq 'red' + @a.preferred(:color).should 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' + end + + it "raises when preference not defined" do + expect { + @a.set_preference(:bad, :bone) + }.to raise_exception(NoMethodError, "bad preference not defined") + end + + 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 + end + + context "database fallback" do + before do + @a.instance_variable_set("@pending_preferences", {}) + end + + 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' + end + + it "defaults if no database key exists" do + Spree::Preference.should_receive(:find_by_key).and_return(nil) + @a.preferred_color.should == 'green' + end + end + + + context "converts integer preferences to integer values" do + before do + A.preference :is_integer, :integer + end + + it "with strings" do + @a.set_preference(:is_integer, '3') + @a.preferences[:is_integer].should == 3 + + @a.set_preference(:is_integer, '') + @a.preferences[:is_integer].should == 0 + end + + end + + context "converts decimal preferences to BigDecimal values" do + before do + A.preference :if_decimal, :decimal + end + + it "returns a BigDecimal" do + @a.set_preference(:if_decimal, 3.3) + @a.preferences[:if_decimal].class.should == BigDecimal + end + + it "with strings" do + @a.set_preference(:if_decimal, '3.3') + @a.preferences[:if_decimal].should == 3.3 + + @a.set_preference(:if_decimal, '') + @a.preferences[:if_decimal].should == 0.0 + end + end + + context "converts boolean preferences to boolean values" do + before do + A.preference :is_boolean, :boolean, :default => true + end + + it "with strings" do + @a.set_preference(:is_boolean, '0') + @a.preferences[:is_boolean].should be_false + @a.set_preference(:is_boolean, 'f') + @a.preferences[:is_boolean].should be_false + @a.set_preference(:is_boolean, 't') + @a.preferences[:is_boolean].should be_true + end + + it "with integers" do + @a.set_preference(:is_boolean, 0) + @a.preferences[:is_boolean].should be_false + @a.set_preference(:is_boolean, 1) + @a.preferences[:is_boolean].should be_true + end + + it "with an empty string" do + @a.set_preference(:is_boolean, '') + @a.preferences[:is_boolean].should be_false + end + + it "with an empty hash" do + @a.set_preference(:is_boolean, []) + @a.preferences[:is_boolean].should be_false + end + end + + context "converts any preferences to any values" do + before do + A.preference :product_ids, :any, :default => [] + A.preference :product_attributes, :any, :default => {} + end + + it "with array" do + @a.preferences[:product_ids].should == [] + @a.set_preference(:product_ids, [1, 2]) + @a.preferences[:product_ids].should == [1, 2] + end + + it "with hash" do + @a.preferences[:product_attributes].should == {} + @a.set_preference(:product_attributes, {:id => 1, :name => 2}) + @a.preferences[:product_attributes].should == {:id => 1, :name => 2} + end + end + + end + + describe "persisted preferables" do + before(:all) do + class CreatePrefTest < ActiveRecord::Migration + def self.up + create_table :pref_tests do |t| + t.string :col + end + end + + def self.down + drop_table :pref_tests + end + end + + @migration_verbosity = ActiveRecord::Migration.verbose + ActiveRecord::Migration.verbose = false + CreatePrefTest.migrate(:up) + + class PrefTest < ActiveRecord::Base + preference :pref_test_pref, :string, :default => 'abc' + preference :pref_test_any, :any, :default => [] + end + end + + after(:all) do + CreatePrefTest.migrate(:down) + ActiveRecord::Migration.verbose = @migration_verbosity + end + + before(:each) do + @pt = PrefTest.create + end + + describe "pending preferences for new activerecord objects" 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' + pr.save! + pr.get_preference(:pref_test_pref).should == '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] + pr.save! + pr.get_preference(:pref_test_any).should == [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 + + pref_test.save + pref_test.preference_cache_key(:pref_test_pref).should_not be_nil + end + + it "but returns default values" do + pref_test = PrefTest.new + pref_test.get_preference(:pref_test_pref).should == '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') + 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' + @pt.clear_preferences + @pt.preferred_pref_test_pref.should == 'abc' + end + + it "clear preferences when record is deleted" do + @pt.save! + @pt.preferred_pref_test_pref = 'lmn' + @pt.save! + @pt.destroy + @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' + end + end + + it "builds cache keys" do + @a.preference_cache_key(:color).should match /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 + A.remove_preference :test_temp + @a.has_preference?(:test_temp).should be_false + @a.respond_to?(:preferred_test_temp).should be_false + end + +end + + diff --git a/spec/models/spree/preferences/store_spec.rb b/spec/models/spree/preferences/store_spec.rb new file mode 100644 index 0000000000..f3e4ff4919 --- /dev/null +++ b/spec/models/spree/preferences/store_spec.rb @@ -0,0 +1,47 @@ +require 'spec_helper' + +describe Spree::Preferences::Store do + before :each do + @store = Spree::Preferences::StoreInstance.new + end + + it "sets and gets a key" do + @store.set :test, 1, :integer + @store.exist?(:test).should be_true + @store.get(:test).should eq 1 + end + + it "can set and get false values when cache return nil" do + @store.set :test, false, :boolean + @store.get(:test).should be_false + end + + it "will return db value when cache is emtpy and cache the db value" do + preference = Spree::Preference.where(:key => 'test').first_or_initialize + preference.value = '123' + preference.value_type = 'string' + preference.save + + Rails.cache.clear + @store.get(:test).should eq '123' + Rails.cache.read(:test).should eq '123' + end + + it "should return and cache fallback value when supplied" do + Rails.cache.clear + @store.get(:test, false).should be_false + Rails.cache.read(:test).should be_false + end + + it "should return and cache fallback value when persistence is disabled (i.e. during bootstrap)" do + Rails.cache.clear + @store.stub(:should_persist? => false) + @store.get(:test, true).should be_true + Rails.cache.read(:test).should be_true + end + + it "should return nil when key can't be found and fallback value is not supplied" do + @store.get(:random_key).should be_nil + end + +end From ed81ceaffed74b7b22791fd7548a22c322f541e4 Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Thu, 6 Aug 2020 11:41:31 +0100 Subject: [PATCH 02/11] Merge decorator with original file --- app/models/spree/app_configuration.rb | 42 ++++++++++++++++++ .../spree/app_configuration_decorator.rb | 44 ------------------- 2 files changed, 42 insertions(+), 44 deletions(-) delete mode 100644 app/models/spree/app_configuration_decorator.rb diff --git a/app/models/spree/app_configuration.rb b/app/models/spree/app_configuration.rb index 2442cb0790..e726b67327 100644 --- a/app/models/spree/app_configuration.rb +++ b/app/models/spree/app_configuration.rb @@ -1,5 +1,9 @@ # This is the primary location for defining spree preferences # +# This file allows us to add global configuration variables, which +# we can allow to be modified in the UI by adding appropriate form +# elements to existing or new configuration pages. +# # The expectation is that this is created once and stored in # the spree environment # @@ -98,6 +102,44 @@ module Spree preference :smtp_username, :string preference :smtp_password, :string + # Embedded Shopfronts + preference :enable_embedded_shopfronts, :boolean, default: false + preference :embedded_shopfronts_whitelist, :text, default: nil + + # Legal Preferences + preference :footer_tos_url, :string, default: "/Terms-of-service.pdf" + preference :enterprises_require_tos, :boolean, default: false + preference :privacy_policy_url, :string, default: nil + preference :cookies_consent_banner_toggle, :boolean, default: false + preference :cookies_policy_matomo_section, :boolean, default: false + + # Tax Preferences + preference :products_require_tax_category, :boolean, default: false + preference :shipping_tax_rate, :decimal, default: 0 + + # Monitoring + preference :last_job_queue_heartbeat_at, :string, default: nil + + # External services + preference :bugherd_api_key, :string, default: nil + preference :matomo_url, :string, default: nil + preference :matomo_site_id, :string, default: nil + preference :matomo_tag_manager_url, :string, default: nil + + # Invoices & Receipts + preference :enable_invoices?, :boolean, default: true + preference :invoice_style2?, :boolean, default: false + preference :enable_receipt_printing?, :boolean, default: false + + # Stripe Connect + preference :stripe_connect_enabled, :boolean, default: false + + # Number localization + preference :enable_localized_number?, :boolean, default: false + + # Enable cache + 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 @searcher_class ||= Spree::Core::Search::Base diff --git a/app/models/spree/app_configuration_decorator.rb b/app/models/spree/app_configuration_decorator.rb deleted file mode 100644 index a039ddbd45..0000000000 --- a/app/models/spree/app_configuration_decorator.rb +++ /dev/null @@ -1,44 +0,0 @@ -Spree::AppConfiguration.class_eval do - # This file decorates the existing preferences file defined by Spree. - # It allows us to add our own global configuration variables, which - # we can allow to be modified in the UI by adding appropriate form - # elements to existing or new configuration pages. - - # Embedded Shopfronts - preference :enable_embedded_shopfronts, :boolean, default: false - preference :embedded_shopfronts_whitelist, :text, default: nil - - # Legal Preferences - preference :footer_tos_url, :string, default: "/Terms-of-service.pdf" - preference :enterprises_require_tos, :boolean, default: false - preference :privacy_policy_url, :string, default: nil - preference :cookies_consent_banner_toggle, :boolean, default: false - preference :cookies_policy_matomo_section, :boolean, default: false - - # Tax Preferences - preference :products_require_tax_category, :boolean, default: false - preference :shipping_tax_rate, :decimal, default: 0 - - # Monitoring - preference :last_job_queue_heartbeat_at, :string, default: nil - - # External services - preference :bugherd_api_key, :string, default: nil - preference :matomo_url, :string, default: nil - preference :matomo_site_id, :string, default: nil - preference :matomo_tag_manager_url, :string, default: nil - - # Invoices & Receipts - preference :enable_invoices?, :boolean, default: true - preference :invoice_style2?, :boolean, default: false - preference :enable_receipt_printing?, :boolean, default: false - - # Stripe Connect - preference :stripe_connect_enabled, :boolean, default: false - - # Number localization - preference :enable_localized_number?, :boolean, default: false - - # Enable cache - preference :enable_products_cache?, :boolean, default: (Rails.env.production? || Rails.env.staging?) -end From 25e61897aa1c9d2977bc0baaf285ef511b4f3de9 Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Thu, 6 Aug 2020 11:44:20 +0100 Subject: [PATCH 03/11] Run rubocop autocorrect --- app/models/spree/app_configuration.rb | 33 ++++++++------- app/models/spree/preference.rb | 6 ++- app/models/spree/preferences/configuration.rb | 5 ++- app/models/spree/preferences/preferable.rb | 29 ++++++-------- .../preferences/preferable_class_methods.rb | 9 ++--- app/models/spree/preferences/store.rb | 19 +++++---- spec/models/spree/app_configuration_spec.rb | 3 +- spec/models/spree/preference_spec.rb | 7 ++-- .../spree/preferences/configuration_spec.rb | 9 ++--- .../spree/preferences/preferable_spec.rb | 40 ++++++++----------- spec/models/spree/preferences/store_spec.rb | 7 ++-- 11 files changed, 78 insertions(+), 89 deletions(-) diff --git a/app/models/spree/app_configuration.rb b/app/models/spree/app_configuration.rb index e726b67327..d1ad68e305 100644 --- a/app/models/spree/app_configuration.rb +++ b/app/models/spree/app_configuration.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + # This is the primary location for defining spree preferences # # This file allows us to add global configuration variables, which @@ -23,7 +25,6 @@ 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 preference :admin_interface_logo, :string, default: 'logo/spree_50.png' @@ -59,14 +60,14 @@ module Spree 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 :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 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 + 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 preference :show_products_without_price, :boolean, default: false - preference :show_raw_product_description, :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 @@ -87,18 +88,18 @@ module Spree preference :s3_host_alias, :string # Default mail headers settings - preference :enable_mail_delivery, :boolean, :default => false - preference :mails_from, :string, :default => 'spree@example.com' - preference :mail_bcc, :string, :default => 'spree@example.com' - preference :intercept_email, :string, :default => nil + preference :enable_mail_delivery, :boolean, default: false + preference :mails_from, :string, default: 'spree@example.com' + preference :mail_bcc, :string, default: 'spree@example.com' + preference :intercept_email, :string, default: nil # Default smtp settings - preference :override_actionmailer_config, :boolean, :default => true - 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 :mail_auth_type, :string, :default => Core::MailSettings::MAIL_AUTH[0] + preference :override_actionmailer_config, :boolean, default: true + 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 :mail_auth_type, :string, default: Core::MailSettings::MAIL_AUTH[0] preference :smtp_username, :string preference :smtp_password, :string @@ -145,9 +146,7 @@ module Spree @searcher_class ||= Spree::Core::Search::Base end - def searcher_class=(sclass) - @searcher_class = sclass - end + attr_writer :searcher_class attr_writer :package_factory, :order_updater_decorator diff --git a/app/models/spree/preference.rb b/app/models/spree/preference.rb index 046ce1bcc0..da6f5464f1 100644 --- a/app/models/spree/preference.rb +++ b/app/models/spree/preference.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class Spree::Preference < ActiveRecord::Base serialize :value @@ -16,13 +18,13 @@ class Spree::Preference < ActiveRecord::Base when :password self[:value].to_s when :decimal - BigDecimal.new(self[:value].to_s).round(2, BigDecimal::ROUND_HALF_UP) + BigDecimal(self[:value].to_s).round(2, BigDecimal::ROUND_HALF_UP) when :integer self[:value].to_i when :boolean (self[:value].to_s =~ /^[t|1]/i) != nil else - self[:value].is_a?(String) ? YAML.load(self[:value]) : self[:value] + self[:value].is_a?(String) ? YAML.safe_load(self[:value]) : self[:value] end else self[:value] diff --git a/app/models/spree/preferences/configuration.rb b/app/models/spree/preferences/configuration.rb index 8636c96ed4..25216fc899 100644 --- a/app/models/spree/preferences/configuration.rb +++ b/app/models/spree/preferences/configuration.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + # This takes the preferrable methods and adds some # syntatic sugar to access the preferences # @@ -33,7 +35,7 @@ module Spree::Preferences end def reset - preferences.each do |name, value| + preferences.each do |name, _value| set_preference name, preference_default(name) end end @@ -66,6 +68,5 @@ module Spree::Preferences super end end - end end diff --git a/app/models/spree/preferences/preferable.rb b/app/models/spree/preferences/preferable.rb index a02ab9265d..9242ed1d11 100644 --- a/app/models/spree/preferences/preferable.rb +++ b/app/models/spree/preferences/preferable.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + # The preference_cache_key is used to determine if the preference # can be set. The default behavior is to return nil if there is no # id value. On ActiveRecords, new objects will have their preferences @@ -9,23 +11,17 @@ # and copy all the definitions allowing the subclass to add # additional defintions without affecting the base module Spree::Preferences::Preferable - def self.included(base) base.class_eval do extend Spree::Preferences::PreferableClassMethods if respond_to?(:after_create) - after_create do |obj| - obj.save_pending_preferences - end + after_create(&:save_pending_preferences) end if respond_to?(:after_destroy) - after_destroy do |obj| - obj.clear_preferences - end + after_destroy(&:clear_preferences) end - end end @@ -57,7 +53,7 @@ module Spree::Preferences::Preferable end def has_preference!(name) - raise NoMethodError.new "#{name} preference not defined" unless has_preference? name + raise NoMethodError, "#{name} preference not defined" unless has_preference? name end def has_preference?(name) @@ -78,18 +74,20 @@ module Spree::Preferences::Preferable def preference_cache_key(name) return unless id + [ENV["RAILS_CACHE_ID"], self.class.name, name, id].join('::').underscore end def save_pending_preferences return unless @pending_preferences + @pending_preferences.each do |name, value| set_preference(name, value) end end def clear_preferences - preferences.keys.each {|pref| preference_store.delete preference_cache_key(pref)} + preferences.keys.each { |pref| preference_store.delete preference_cache_key(pref) } end private @@ -101,6 +99,7 @@ module Spree::Preferences::Preferable def get_pending_preference(name) return unless @pending_preferences + @pending_preferences[name] end @@ -111,7 +110,7 @@ module Spree::Preferences::Preferable when :password value.to_s when :decimal - BigDecimal.new(value.to_s).round(2, BigDecimal::ROUND_HALF_UP) + BigDecimal(value.to_s).round(2, BigDecimal::ROUND_HALF_UP) when :integer value.to_i when :boolean @@ -119,10 +118,10 @@ module Spree::Preferences::Preferable value.nil? || value == 0 || value =~ /^(f|false|0)$/i || - (value.respond_to? :empty? and value.empty?) - false + (value.respond_to?(:empty?) && value.empty?) + false else - true + true end else value @@ -132,6 +131,4 @@ module Spree::Preferences::Preferable def preference_store Spree::Preferences::Store.instance end - end - diff --git a/app/models/spree/preferences/preferable_class_methods.rb b/app/models/spree/preferences/preferable_class_methods.rb index 180bc8c8f5..b36255c8b5 100644 --- a/app/models/spree/preferences/preferable_class_methods.rb +++ b/app/models/spree/preferences/preferable_class_methods.rb @@ -1,6 +1,7 @@ +# frozen_string_literal: true + module Spree::Preferences module PreferableClassMethods - def preference(name, type, *args) options = args.extract_options! options.assert_valid_keys(:default, :description) @@ -10,7 +11,6 @@ module Spree::Preferences # cache_key will be nil for new objects, then if we check if there # is a pending preference before going to default define_method preference_getter_method(name) do - # perference_cache_key will only be nil/false for new records # if preference_cache_key(name) @@ -59,7 +59,7 @@ module Spree::Preferences end def preference_setter_method(name) - "preferred_#{name}=".to_sym + "preferred_#{name}=".to_sym end def prefers_getter_method(name) @@ -67,7 +67,7 @@ module Spree::Preferences end def prefers_setter_method(name) - "prefers_#{name}=".to_sym + "prefers_#{name}=".to_sym end def preference_default_getter_method(name) @@ -81,6 +81,5 @@ module Spree::Preferences def preference_description_getter_method(name) "preferred_#{name}_description".to_sym end - end end diff --git a/app/models/spree/preferences/store.rb b/app/models/spree/preferences/store.rb index 6c9c566fb7..28e9d8bfbd 100644 --- a/app/models/spree/preferences/store.rb +++ b/app/models/spree/preferences/store.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + # Use singleton class Spree::Preferences::Store.instance to access # # StoreInstance has a persistence flag that is on by default, @@ -7,7 +9,6 @@ require 'singleton' module Spree::Preferences - class StoreInstance attr_accessor :persistence @@ -23,10 +24,10 @@ module Spree::Preferences def exist?(key) @cache.exist?(key) || - should_persist? && Spree::Preference.where(:key => key).exists? + should_persist? && Spree::Preference.where(key: key).exists? end - def get(key,fallback=nil) + def get(key, fallback = nil) # return the retrieved value, if it's in the cache # use unless nil? incase the value is actually boolean false # @@ -39,7 +40,7 @@ module Spree::Preferences # has been cleared from the cache # does it exist in the database? - if Spree::Preference.table_exists? && preference = Spree::Preference.find_by_key(key) + if Spree::Preference.table_exists? && preference = Spree::Preference.find_by(key: key) # it does exist, so let's put it back into the cache @cache.write(preference.key, preference.value) @@ -55,7 +56,7 @@ module Spree::Preferences @cache.write(key, fallback) end - return fallback + fallback end def delete(key) @@ -72,7 +73,7 @@ module Spree::Preferences def persist(cache_key, value, type) return unless should_persist? - preference = Spree::Preference.where(:key => cache_key).first_or_initialize + preference = Spree::Preference.where(key: cache_key).first_or_initialize preference.value = value preference.value_type = type preference.save @@ -81,18 +82,16 @@ module Spree::Preferences def destroy(cache_key) return unless should_persist? - preference = Spree::Preference.find_by_key(cache_key) - preference.destroy if preference + preference = Spree::Preference.find_by(key: cache_key) + preference&.destroy end def should_persist? @persistence && Spree::Preference.connected? && Spree::Preference.table_exists? end - end class Store < StoreInstance include Singleton end - end diff --git a/spec/models/spree/app_configuration_spec.rb b/spec/models/spree/app_configuration_spec.rb index 96503470c9..40a886babd 100644 --- a/spec/models/spree/app_configuration_spec.rb +++ b/spec/models/spree/app_configuration_spec.rb @@ -1,7 +1,8 @@ +# frozen_string_literal: true + require 'spec_helper' describe Spree::AppConfiguration do - let (:prefs) { Rails.application.config.spree.preferences } it "should be available from the environment" do diff --git a/spec/models/spree/preference_spec.rb b/spec/models/spree/preference_spec.rb index 79cc9c2494..966c7c1bff 100644 --- a/spec/models/spree/preference_spec.rb +++ b/spec/models/spree/preference_spec.rb @@ -1,7 +1,8 @@ +# frozen_string_literal: true + require 'spec_helper' describe Spree::Preference do - it "should require a key" do @preference = Spree::Preference.new @preference.key = :test @@ -18,7 +19,7 @@ describe Spree::Preference do p.key = key p.save - Spree::Preference.find_by_key(key) + Spree::Preference.find_by(key: key) end it ":boolean" do @@ -92,7 +93,5 @@ describe Spree::Preference do pref.value.should eq value pref.value_type.should == value_type.to_s end - end - end diff --git a/spec/models/spree/preferences/configuration_spec.rb b/spec/models/spree/preferences/configuration_spec.rb index a1b45d0534..d51bad5860 100644 --- a/spec/models/spree/preferences/configuration_spec.rb +++ b/spec/models/spree/preferences/configuration_spec.rb @@ -1,10 +1,11 @@ +# frozen_string_literal: true + require 'spec_helper' describe Spree::Preferences::Configuration do - before :all do class AppConfig < Spree::Preferences::Configuration - preference :color, :string, :default => :blue + preference :color, :string, default: :blue end @config = AppConfig.new end @@ -23,8 +24,4 @@ describe Spree::Preferences::Configuration do @config.set :color, 'green' @config.get(:color).should eq 'green' end - end - - - diff --git a/spec/models/spree/preferences/preferable_spec.rb b/spec/models/spree/preferences/preferable_spec.rb index e01fcf8114..fb9aa74eb2 100644 --- a/spec/models/spree/preferences/preferable_spec.rb +++ b/spec/models/spree/preferences/preferable_spec.rb @@ -1,7 +1,8 @@ +# frozen_string_literal: true + require 'spec_helper' describe Spree::Preferences::Preferable do - before :all do class A include Spree::Preferences::Preferable @@ -11,7 +12,7 @@ describe Spree::Preferences::Preferable do @id = rand(999) end - preference :color, :string, :default => 'green', :description => "My Favorite Color" + preference :color, :string, default: 'green', description: "My Favorite Color" end class B < A @@ -21,9 +22,9 @@ describe Spree::Preferences::Preferable do before :each do @a = A.new - @a.stub(:persisted? => true) + @a.stub(persisted?: true) @b = B.new - @b.stub(:persisted? => true) + @b.stub(persisted?: true) # ensure we're persisting as that is the default # @@ -79,7 +80,6 @@ describe Spree::Preferences::Preferable do @a.get_preference :flavor }.to raise_error(NoMethodError, "flavor preference not defined") end - end describe "preference access" do @@ -114,7 +114,7 @@ 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 + @b.preferences[:color].should eq 'green' # default from A end context "database fallback" do @@ -123,7 +123,7 @@ describe Spree::Preferences::Preferable do end it "retrieves a preference from the database before falling back to default" do - preference = double(:value => "chatreuse", :key => 'a/color/123') + preference = double(value: "chatreuse", key: 'a/color/123') Spree::Preference.should_receive(:find_by_key).and_return(preference) @a.preferred_color.should == 'chatreuse' end @@ -134,7 +134,6 @@ describe Spree::Preferences::Preferable do end end - context "converts integer preferences to integer values" do before do A.preference :is_integer, :integer @@ -147,7 +146,6 @@ describe Spree::Preferences::Preferable do @a.set_preference(:is_integer, '') @a.preferences[:is_integer].should == 0 end - end context "converts decimal preferences to BigDecimal values" do @@ -171,7 +169,7 @@ describe Spree::Preferences::Preferable do context "converts boolean preferences to boolean values" do before do - A.preference :is_boolean, :boolean, :default => true + A.preference :is_boolean, :boolean, default: true end it "with strings" do @@ -203,8 +201,8 @@ describe Spree::Preferences::Preferable do context "converts any preferences to any values" do before do - A.preference :product_ids, :any, :default => [] - A.preference :product_attributes, :any, :default => {} + A.preference :product_ids, :any, default: [] + A.preference :product_attributes, :any, default: {} end it "with array" do @@ -215,11 +213,10 @@ describe Spree::Preferences::Preferable do it "with hash" do @a.preferences[:product_attributes].should == {} - @a.set_preference(:product_attributes, {:id => 1, :name => 2}) - @a.preferences[:product_attributes].should == {:id => 1, :name => 2} + @a.set_preference(:product_attributes, { id: 1, name: 2 }) + @a.preferences[:product_attributes].should == { id: 1, name: 2 } end end - end describe "persisted preferables" do @@ -241,8 +238,8 @@ describe Spree::Preferences::Preferable do CreatePrefTest.migrate(:up) class PrefTest < ActiveRecord::Base - preference :pref_test_pref, :string, :default => 'abc' - preference :pref_test_any, :any, :default => [] + preference :pref_test_pref, :string, default: 'abc' + preference :pref_test_any, :any, default: [] end end @@ -306,7 +303,7 @@ describe Spree::Preferences::Preferable do @pt.preferred_pref_test_pref = 'lmn' @pt.save! @pt.destroy - @pt1 = PrefTest.new(:col => 'aaaa') + @pt1 = PrefTest.new(col: 'aaaa') @pt1.id = @pt.id @pt1.save! @pt1.get_preference(:pref_test_pref).should_not == 'lmn' @@ -315,17 +312,14 @@ describe Spree::Preferences::Preferable do end it "builds cache keys" do - @a.preference_cache_key(:color).should match /a\/color\/\d+/ + @a.preference_cache_key(:color).should match %r{a/color/\d+} end it "can add and remove preferences" do - A.preference :test_temp, :boolean, :default => true + A.preference :test_temp, :boolean, default: true @a.preferred_test_temp.should be_true A.remove_preference :test_temp @a.has_preference?(:test_temp).should be_false @a.respond_to?(:preferred_test_temp).should be_false end - end - - diff --git a/spec/models/spree/preferences/store_spec.rb b/spec/models/spree/preferences/store_spec.rb index f3e4ff4919..8502b80ac3 100644 --- a/spec/models/spree/preferences/store_spec.rb +++ b/spec/models/spree/preferences/store_spec.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'spec_helper' describe Spree::Preferences::Store do @@ -17,7 +19,7 @@ describe Spree::Preferences::Store do end it "will return db value when cache is emtpy and cache the db value" do - preference = Spree::Preference.where(:key => 'test').first_or_initialize + preference = Spree::Preference.where(key: 'test').first_or_initialize preference.value = '123' preference.value_type = 'string' preference.save @@ -35,7 +37,7 @@ describe Spree::Preferences::Store do it "should return and cache fallback value when persistence is disabled (i.e. during bootstrap)" do Rails.cache.clear - @store.stub(:should_persist? => false) + @store.stub(should_persist?: false) @store.get(:test, true).should be_true Rails.cache.read(:test).should be_true end @@ -43,5 +45,4 @@ describe Spree::Preferences::Store do it "should return nil when key can't be found and fallback value is not supplied" do @store.get(:random_key).should be_nil end - end From 024a64b73ab2cbcee04d4f5507522994a80775b9 Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Thu, 6 Aug 2020 11:46:56 +0100 Subject: [PATCH 04/11] Fix single rubocop rule: Use nested module definitions instead of compact style --- app/models/spree/preference.rb | 54 ++-- app/models/spree/preferences/configuration.rb | 78 +++--- app/models/spree/preferences/preferable.rb | 234 +++++++++--------- .../preferences/preferable_class_methods.rb | 134 +++++----- app/models/spree/preferences/store.rb | 150 +++++------ 5 files changed, 331 insertions(+), 319 deletions(-) diff --git a/app/models/spree/preference.rb b/app/models/spree/preference.rb index da6f5464f1..145863dab3 100644 --- a/app/models/spree/preference.rb +++ b/app/models/spree/preference.rb @@ -1,37 +1,39 @@ # frozen_string_literal: true -class Spree::Preference < ActiveRecord::Base - serialize :value +module Spree + class Preference < ActiveRecord::Base + serialize :value - validates :key, presence: true - validates :value_type, presence: true + 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 - def value - if self[:value_type].present? - case self[:value_type].to_sym - when :string, :text - self[:value].to_s - when :password - self[:value].to_s - when :decimal - BigDecimal(self[:value].to_s).round(2, BigDecimal::ROUND_HALF_UP) - when :integer - self[:value].to_i - when :boolean - (self[:value].to_s =~ /^[t|1]/i) != nil + # The type conversions here should match + # the ones in spree::preferences::preferrable#convert_preference_value + def value + if self[:value_type].present? + case self[:value_type].to_sym + when :string, :text + self[:value].to_s + when :password + self[:value].to_s + when :decimal + BigDecimal(self[:value].to_s).round(2, BigDecimal::ROUND_HALF_UP) + when :integer + self[:value].to_i + when :boolean + (self[:value].to_s =~ /^[t|1]/i) != nil + else + self[:value].is_a?(String) ? YAML.safe_load(self[:value]) : self[:value] + end else - self[:value].is_a?(String) ? YAML.safe_load(self[:value]) : self[:value] + self[:value] end - else + end + + def raw_value self[:value] end end - - def raw_value - self[:value] - end end diff --git a/app/models/spree/preferences/configuration.rb b/app/models/spree/preferences/configuration.rb index 25216fc899..3c4c9a13b0 100644 --- a/app/models/spree/preferences/configuration.rb +++ b/app/models/spree/preferences/configuration.rb @@ -22,50 +22,52 @@ # a.preferred_color # # -module Spree::Preferences - class Configuration - include Spree::Preferences::Preferable +module Spree + module Preferences + class Configuration + include Spree::Preferences::Preferable - def configure - yield(self) if block_given? - end - - def preference_cache_key(name) - [ENV['RAILS_CACHE_ID'], self.class.name, name].flatten.join('::').underscore - end - - def reset - preferences.each do |name, _value| - set_preference name, preference_default(name) - end - end - - alias :[] :get_preference - alias :[]= :set_preference - - alias :get :get_preference - - def set(*args) - options = args.extract_options! - options.each do |name, value| - set_preference name, value + def configure + yield(self) if block_given? end - if args.size == 2 - set_preference args[0], args[1] + def preference_cache_key(name) + [ENV['RAILS_CACHE_ID'], self.class.name, name].flatten.join('::').underscore end - end - def method_missing(method, *args) - name = method.to_s.gsub('=', '') - if has_preference? name - if method.to_s =~ /=$/ - set_preference(name, args.first) - else - get_preference name + def reset + preferences.each do |name, _value| + set_preference name, preference_default(name) + end + end + + alias :[] :get_preference + alias :[]= :set_preference + + alias :get :get_preference + + def set(*args) + options = args.extract_options! + options.each do |name, value| + set_preference name, value + end + + if args.size == 2 + set_preference args[0], args[1] + end + end + + def method_missing(method, *args) + name = method.to_s.gsub('=', '') + if has_preference? name + if method.to_s =~ /=$/ + set_preference(name, args.first) + else + get_preference name + end + else + super end - else - super end end end diff --git a/app/models/spree/preferences/preferable.rb b/app/models/spree/preferences/preferable.rb index 9242ed1d11..603cec767d 100644 --- a/app/models/spree/preferences/preferable.rb +++ b/app/models/spree/preferences/preferable.rb @@ -10,125 +10,129 @@ # inherited hook will assign a new hash for the subclass definitions # and copy all the definitions allowing the subclass to add # additional defintions without affecting the base -module Spree::Preferences::Preferable - def self.included(base) - base.class_eval do - extend Spree::Preferences::PreferableClassMethods +module Spree + module Preferences + module Preferable + def self.included(base) + base.class_eval do + extend Spree::Preferences::PreferableClassMethods - if respond_to?(:after_create) - after_create(&:save_pending_preferences) + if respond_to?(:after_create) + after_create(&:save_pending_preferences) + end + + if respond_to?(:after_destroy) + after_destroy(&:clear_preferences) + end + end end - if respond_to?(:after_destroy) - after_destroy(&:clear_preferences) + def get_preference(name) + has_preference! 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 + end + + def preference_type(name) + has_preference! 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) + end + + def preference_description(name) + has_preference! name + send self.class.preference_description_getter_method(name) + end + + def has_preference!(name) + raise NoMethodError, "#{name} preference not defined" unless has_preference? name + end + + def has_preference?(name) + respond_to? self.class.preference_getter_method(name) + end + + def preferences + prefs = {} + methods.grep(/^prefers_.*\?$/).each do |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 + + [ENV["RAILS_CACHE_ID"], self.class.name, name, id].join('::').underscore + end + + def save_pending_preferences + return unless @pending_preferences + + @pending_preferences.each do |name, value| + set_preference(name, value) + end + end + + def clear_preferences + preferences.keys.each { |pref| preference_store.delete preference_cache_key(pref) } + end + + private + + def add_pending_preference(name, value) + @pending_preferences ||= {} + @pending_preferences[name] = value + end + + def get_pending_preference(name) + return unless @pending_preferences + + @pending_preferences[name] + end + + def convert_preference_value(value, type) + case type + when :string, :text + value.to_s + when :password + value.to_s + when :decimal + BigDecimal(value.to_s).round(2, BigDecimal::ROUND_HALF_UP) + when :integer + value.to_i + when :boolean + if value.is_a?(FalseClass) || + value.nil? || + value == 0 || + value =~ /^(f|false|0)$/i || + (value.respond_to?(:empty?) && value.empty?) + false + else + true + end + else + value + end + end + + def preference_store + Spree::Preferences::Store.instance end end end - - def get_preference(name) - has_preference! 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 - end - - def preference_type(name) - has_preference! 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) - end - - def preference_description(name) - has_preference! name - send self.class.preference_description_getter_method(name) - end - - def has_preference!(name) - raise NoMethodError, "#{name} preference not defined" unless has_preference? name - end - - def has_preference?(name) - respond_to? self.class.preference_getter_method(name) - end - - def preferences - prefs = {} - methods.grep(/^prefers_.*\?$/).each do |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 - - [ENV["RAILS_CACHE_ID"], self.class.name, name, id].join('::').underscore - end - - def save_pending_preferences - return unless @pending_preferences - - @pending_preferences.each do |name, value| - set_preference(name, value) - end - end - - def clear_preferences - preferences.keys.each { |pref| preference_store.delete preference_cache_key(pref) } - end - - private - - def add_pending_preference(name, value) - @pending_preferences ||= {} - @pending_preferences[name] = value - end - - def get_pending_preference(name) - return unless @pending_preferences - - @pending_preferences[name] - end - - def convert_preference_value(value, type) - case type - when :string, :text - value.to_s - when :password - value.to_s - when :decimal - BigDecimal(value.to_s).round(2, BigDecimal::ROUND_HALF_UP) - when :integer - value.to_i - when :boolean - if value.is_a?(FalseClass) || - value.nil? || - value == 0 || - value =~ /^(f|false|0)$/i || - (value.respond_to?(:empty?) && value.empty?) - false - else - true - end - else - value - end - end - - def preference_store - Spree::Preferences::Store.instance - end end diff --git a/app/models/spree/preferences/preferable_class_methods.rb b/app/models/spree/preferences/preferable_class_methods.rb index b36255c8b5..f6ac02e781 100644 --- a/app/models/spree/preferences/preferable_class_methods.rb +++ b/app/models/spree/preferences/preferable_class_methods.rb @@ -1,85 +1,87 @@ # frozen_string_literal: true -module Spree::Preferences - module PreferableClassMethods - def preference(name, type, *args) - options = args.extract_options! - options.assert_valid_keys(:default, :description) - default = options[:default] - description = options[:description] || name +module Spree + module Preferences + module PreferableClassMethods + def preference(name, type, *args) + options = args.extract_options! + options.assert_valid_keys(:default, :description) + default = options[:default] + description = options[:description] || name - # cache_key will be nil for new objects, then if we check if there - # is a pending preference before going to default - define_method preference_getter_method(name) do - # perference_cache_key will only be nil/false for new records - # - if preference_cache_key(name) - preference_store.get(preference_cache_key(name), default) - else - get_pending_preference(name) || default + # cache_key will be nil for new objects, then if we check if there + # is a pending preference before going to default + define_method preference_getter_method(name) do + # perference_cache_key will only be nil/false for new records + # + if preference_cache_key(name) + preference_store.get(preference_cache_key(name), default) + else + get_pending_preference(name) || default + end + end + alias_method prefers_getter_method(name), preference_getter_method(name) + + define_method preference_setter_method(name) do |value| + value = convert_preference_value(value, type) + if preference_cache_key(name) + preference_store.set preference_cache_key(name), value, type + else + add_pending_preference(name, value) + end + end + alias_method prefers_setter_method(name), preference_setter_method(name) + + define_method preference_default_getter_method(name) do + default + end + + define_method preference_type_getter_method(name) do + type + end + + define_method preference_description_getter_method(name) do + description end end - alias_method prefers_getter_method(name), preference_getter_method(name) - define_method preference_setter_method(name) do |value| - value = convert_preference_value(value, type) - if preference_cache_key(name) - preference_store.set preference_cache_key(name), value, type - else - add_pending_preference(name, value) - end - end - alias_method prefers_setter_method(name), preference_setter_method(name) - - define_method preference_default_getter_method(name) do - default + 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) end - define_method preference_type_getter_method(name) do - type + def preference_getter_method(name) + "preferred_#{name}".to_sym end - define_method preference_description_getter_method(name) do - description + def preference_setter_method(name) + "preferred_#{name}=".to_sym end - 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) - end + def prefers_getter_method(name) + "prefers_#{name}?".to_sym + end - def preference_getter_method(name) - "preferred_#{name}".to_sym - end + def prefers_setter_method(name) + "prefers_#{name}=".to_sym + end - def preference_setter_method(name) - "preferred_#{name}=".to_sym - end + def preference_default_getter_method(name) + "preferred_#{name}_default".to_sym + end - def prefers_getter_method(name) - "prefers_#{name}?".to_sym - end + def preference_type_getter_method(name) + "preferred_#{name}_type".to_sym + end - def prefers_setter_method(name) - "prefers_#{name}=".to_sym - end - - def preference_default_getter_method(name) - "preferred_#{name}_default".to_sym - end - - def preference_type_getter_method(name) - "preferred_#{name}_type".to_sym - end - - def preference_description_getter_method(name) - "preferred_#{name}_description".to_sym + def preference_description_getter_method(name) + "preferred_#{name}_description".to_sym + end end end end diff --git a/app/models/spree/preferences/store.rb b/app/models/spree/preferences/store.rb index 28e9d8bfbd..cb30d31eaa 100644 --- a/app/models/spree/preferences/store.rb +++ b/app/models/spree/preferences/store.rb @@ -8,90 +8,92 @@ require 'singleton' -module Spree::Preferences - class StoreInstance - attr_accessor :persistence +module Spree + module Preferences + class StoreInstance + attr_accessor :persistence - def initialize - @cache = Rails.cache - @persistence = true - end - - def set(key, value, type) - @cache.write(key, value) - persist(key, value, type) - end - - def exist?(key) - @cache.exist?(key) || - should_persist? && Spree::Preference.where(key: key).exists? - end - - def get(key, fallback = nil) - # return the retrieved value, if it's in the cache - # use unless nil? incase the value is actually boolean false - # - unless (val = @cache.read(key)).nil? - return val + def initialize + @cache = Rails.cache + @persistence = true end - if should_persist? - # If it's not in the cache, maybe it's in the database, but - # has been cleared from the cache - - # does it exist in the database? - if Spree::Preference.table_exists? && preference = Spree::Preference.find_by(key: key) - # it does exist, so let's put it back into the cache - @cache.write(preference.key, preference.value) - - # and return the value - return preference.value - end + def set(key, value, type) + @cache.write(key, value) + persist(key, value, type) end - unless fallback.nil? - # cache fallback so we won't hit the db above on - # subsequent queries for the same key + def exist?(key) + @cache.exist?(key) || + should_persist? && Spree::Preference.where(key: key).exists? + end + + def get(key, fallback = nil) + # return the retrieved value, if it's in the cache + # use unless nil? incase the value is actually boolean false # - @cache.write(key, fallback) + unless (val = @cache.read(key)).nil? + return val + end + + if should_persist? + # If it's not in the cache, maybe it's in the database, but + # has been cleared from the cache + + # does it exist in the database? + if Spree::Preference.table_exists? && preference = Spree::Preference.find_by(key: key) + # it does exist, so let's put it back into the cache + @cache.write(preference.key, preference.value) + + # and return the value + return preference.value + end + end + + unless fallback.nil? + # cache fallback so we won't hit the db above on + # subsequent queries for the same key + # + @cache.write(key, fallback) + end + + fallback end - fallback + def delete(key) + @cache.delete(key) + destroy(key) + end + + def clear_cache + @cache.clear + end + + private + + def persist(cache_key, value, type) + return unless should_persist? + + preference = Spree::Preference.where(key: cache_key).first_or_initialize + preference.value = value + preference.value_type = type + preference.save + end + + def destroy(cache_key) + return unless should_persist? + + preference = Spree::Preference.find_by(key: cache_key) + preference&.destroy + end + + def should_persist? + @persistence && Spree::Preference.connected? && Spree::Preference.table_exists? + end end - def delete(key) - @cache.delete(key) - destroy(key) + class Store < StoreInstance + include Singleton end - - def clear_cache - @cache.clear - end - - private - - def persist(cache_key, value, type) - return unless should_persist? - - preference = Spree::Preference.where(key: cache_key).first_or_initialize - preference.value = value - preference.value_type = type - preference.save - end - - def destroy(cache_key) - return unless should_persist? - - preference = Spree::Preference.find_by(key: cache_key) - preference&.destroy - end - - def should_persist? - @persistence && Spree::Preference.connected? && Spree::Preference.table_exists? - end - end - - class Store < StoreInstance - include Singleton end end From 9ebb689370ff372f676783bdf71b9fb716673306 Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Thu, 6 Aug 2020 12:02:09 +0100 Subject: [PATCH 05/11] Fix easy rubocop issues --- app/models/spree/app_configuration.rb | 52 +++++--- app/models/spree/preference.rb | 7 +- app/models/spree/preferences/configuration.rb | 6 +- app/models/spree/preferences/preferable.rb | 18 ++- .../preferences/preferable_class_methods.rb | 28 +++-- spec/models/spree/app_configuration_spec.rb | 2 +- .../spree/preferences/preferable_spec.rb | 119 +++++++++--------- spec/models/spree/preferences/store_spec.rb | 2 +- 8 files changed, 132 insertions(+), 102 deletions(-) diff --git a/app/models/spree/app_configuration.rb b/app/models/spree/app_configuration.rb index d1ad68e305..6d4cc793d5 100644 --- a/app/models/spree/app_configuration.rb +++ b/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 diff --git a/app/models/spree/preference.rb b/app/models/spree/preference.rb index 145863dab3..b877f460ce 100644 --- a/app/models/spree/preference.rb +++ b/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 diff --git a/app/models/spree/preferences/configuration.rb b/app/models/spree/preferences/configuration.rb index 3c4c9a13b0..d1a514484d 100644 --- a/app/models/spree/preferences/configuration.rb +++ b/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) diff --git a/app/models/spree/preferences/preferable.rb b/app/models/spree/preferences/preferable.rb index 603cec767d..a92fa51644 100644 --- a/app/models/spree/preferences/preferable.rb +++ b/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 diff --git a/app/models/spree/preferences/preferable_class_methods.rb b/app/models/spree/preferences/preferable_class_methods.rb index f6ac02e781..dd9369d3a0 100644 --- a/app/models/spree/preferences/preferable_class_methods.rb +++ b/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) diff --git a/spec/models/spree/app_configuration_spec.rb b/spec/models/spree/app_configuration_spec.rb index 40a886babd..c9db852427 100644 --- a/spec/models/spree/app_configuration_spec.rb +++ b/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" diff --git a/spec/models/spree/preferences/preferable_spec.rb b/spec/models/spree/preferences/preferable_spec.rb index fb9aa74eb2..c1ee73a140 100644 --- a/spec/models/spree/preferences/preferable_spec.rb +++ b/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 diff --git a/spec/models/spree/preferences/store_spec.rb b/spec/models/spree/preferences/store_spec.rb index 8502b80ac3..5ce0f6b60b 100644 --- a/spec/models/spree/preferences/store_spec.rb +++ b/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 From b9319239d5b957754958ed10846a58aa9b6dc8fd Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Thu, 6 Aug 2020 12:17:51 +0100 Subject: [PATCH 06/11] Remove configurations that are no longer used as we have moved the code to OFN and fix specs --- app/models/spree/app_configuration.rb | 10 ---- app/models/spree/preferences/preferable.rb | 2 +- spec/models/spree/app_configuration_spec.rb | 46 ++----------------- spec/models/spree/preference_spec.rb | 34 +++++++------- .../spree/preferences/configuration_spec.rb | 6 +-- .../spree/preferences/preferable_spec.rb | 38 +++++++-------- spec/models/spree/preferences/store_spec.rb | 20 ++++---- 7 files changed, 53 insertions(+), 103 deletions(-) diff --git a/app/models/spree/app_configuration.rb b/app/models/spree/app_configuration.rb index 6d4cc793d5..e644771616 100644 --- a/app/models/spree/app_configuration.rb +++ b/app/models/spree/app_configuration.rb @@ -163,15 +163,5 @@ module Spree end attr_writer :searcher_class - - attr_writer :package_factory, :order_updater_decorator - - def package_factory - @package_factory ||= Spree::Stock::Package - end - - def order_updater_decorator - @order_updater_decorator ||= NullDecorator - end end end diff --git a/app/models/spree/preferences/preferable.rb b/app/models/spree/preferences/preferable.rb index a92fa51644..191b6d1bb4 100644 --- a/app/models/spree/preferences/preferable.rb +++ b/app/models/spree/preferences/preferable.rb @@ -114,7 +114,7 @@ module Spree when :boolean if value.is_a?(FalseClass) || value.nil? || - value.zero? || + value == 0 || value =~ /^(f|false|0)$/i || (value.respond_to?(:empty?) && value.empty?) false diff --git a/spec/models/spree/app_configuration_spec.rb b/spec/models/spree/app_configuration_spec.rb index c9db852427..f40edfcabe 100644 --- a/spec/models/spree/app_configuration_spec.rb +++ b/spec/models/spree/app_configuration_spec.rb @@ -7,56 +7,16 @@ describe Spree::AppConfiguration do it "should be available from the environment" do prefs.site_name = "TEST SITE NAME" - prefs.site_name.should eq "TEST SITE NAME" + expect(prefs.site_name).to eq "TEST SITE NAME" end it "should be available as Spree::Config for legacy access" do Spree::Config.site_name = "Spree::Config TEST SITE NAME" - Spree::Config.site_name.should eq "Spree::Config TEST SITE NAME" + expect(Spree::Config.site_name).to eq "Spree::Config TEST SITE NAME" end it "uses base searcher class by default" do prefs.searcher_class = nil - prefs.searcher_class.should eq Spree::Core::Search::Base - end - - it 'uses Spree::Stock::Package by default' do - prefs.package_factory = nil - prefs.package_factory.should eq Spree::Stock::Package - end - - context 'when a package factory is specified' do - class TestPackageFactory; end - - around do |example| - default_factory = prefs.package_factory - example.run - prefs.package_factory = default_factory - end - - it 'uses the set package factory' do - prefs.package_factory = TestPackageFactory - prefs.package_factory.should eq TestPackageFactory - end - end - - it 'uses Spree::NullDecorator by default' do - prefs.order_updater_decorator = nil - prefs.order_updater_decorator.should eq Spree::NullDecorator - end - - context 'when an order_updater_decorator is specified' do - class FakeOrderUpdaterDecorator; end - - around do |example| - default_decorator = prefs.order_updater_decorator - example.run - prefs.order_updater_decorator = default_decorator - end - - it 'uses the set order_updater_decorator' do - prefs.order_updater_decorator = FakeOrderUpdaterDecorator - prefs.order_updater_decorator.should eq FakeOrderUpdaterDecorator - end + expect(prefs.searcher_class).to eq Spree::Core::Search::Base end end diff --git a/spec/models/spree/preference_spec.rb b/spec/models/spree/preference_spec.rb index 966c7c1bff..780434f768 100644 --- a/spec/models/spree/preference_spec.rb +++ b/spec/models/spree/preference_spec.rb @@ -8,7 +8,7 @@ describe Spree::Preference do @preference.key = :test @preference.value_type = :boolean @preference.value = true - @preference.should be_valid + expect(@preference).to be_valid end describe "type coversion for values" do @@ -27,8 +27,8 @@ describe Spree::Preference do value = true key = "boolean_key" pref = round_trip_preference(key, value, value_type) - pref.value.should eq value - pref.value_type.should == value_type.to_s + expect(pref.value).to eq value + expect(pref.value_type).to eq value_type.to_s end it "false :boolean" do @@ -36,8 +36,8 @@ describe Spree::Preference do value = false key = "boolean_key" pref = round_trip_preference(key, value, value_type) - pref.value.should eq value - pref.value_type.should == value_type.to_s + expect(pref.value).to eq value + expect(pref.value_type).to eq value_type.to_s end it ":integer" do @@ -45,8 +45,8 @@ describe Spree::Preference do value = 10 key = "integer_key" pref = round_trip_preference(key, value, value_type) - pref.value.should eq value - pref.value_type.should == value_type.to_s + expect(pref.value).to eq value + expect(pref.value_type).to eq value_type.to_s end it ":decimal" do @@ -54,8 +54,8 @@ describe Spree::Preference do value = 1.5 key = "decimal_key" pref = round_trip_preference(key, value, value_type) - pref.value.should eq value - pref.value_type.should == value_type.to_s + expect(pref.value).to eq value + expect(pref.value_type).to eq value_type.to_s end it ":string" do @@ -63,8 +63,8 @@ describe Spree::Preference do value = "This is a string" key = "string_key" pref = round_trip_preference(key, value, value_type) - pref.value.should eq value - pref.value_type.should == value_type.to_s + expect(pref.value).to eq value + expect(pref.value_type).to eq value_type.to_s end it ":text" do @@ -72,8 +72,8 @@ describe Spree::Preference do value = "This is a string stored as text" key = "text_key" pref = round_trip_preference(key, value, value_type) - pref.value.should eq value - pref.value_type.should == value_type.to_s + expect(pref.value).to eq value + expect(pref.value_type).to eq value_type.to_s end it ":password" do @@ -81,8 +81,8 @@ describe Spree::Preference do value = "This is a password" key = "password_key" pref = round_trip_preference(key, value, value_type) - pref.value.should eq value - pref.value_type.should == value_type.to_s + expect(pref.value).to eq value + expect(pref.value_type).to eq value_type.to_s end it ":any" do @@ -90,8 +90,8 @@ describe Spree::Preference do value = [1, 2] key = "any_key" pref = round_trip_preference(key, value, value_type) - pref.value.should eq value - pref.value_type.should == value_type.to_s + expect(pref.value).to eq value + expect(pref.value_type).to eq value_type.to_s end end end diff --git a/spec/models/spree/preferences/configuration_spec.rb b/spec/models/spree/preferences/configuration_spec.rb index d51bad5860..32883d3fb6 100644 --- a/spec/models/spree/preferences/configuration_spec.rb +++ b/spec/models/spree/preferences/configuration_spec.rb @@ -12,16 +12,16 @@ describe Spree::Preferences::Configuration do it "has named methods to access preferences" do @config.color = 'orange' - @config.color.should eq 'orange' + expect(@config.color).to eq 'orange' end it "uses [ ] to access preferences" do @config[:color] = 'red' - @config[:color].should eq 'red' + expect(@config[:color]).to eq 'red' end it "uses set/get to access preferences" do @config.set :color, 'green' - @config.get(:color).should eq 'green' + expect(@config.get(:color)).to eq 'green' end end diff --git a/spec/models/spree/preferences/preferable_spec.rb b/spec/models/spree/preferences/preferable_spec.rb index c1ee73a140..3e72812b39 100644 --- a/spec/models/spree/preferences/preferable_spec.rb +++ b/spec/models/spree/preferences/preferable_spec.rb @@ -34,13 +34,13 @@ describe Spree::Preferences::Preferable do describe "preference definitions" do it "parent should not see child definitions" do - expect(@a.has_preference?(:color)).to be_true - expect(@a.has_preference?(:flavor)).not_to be_true + expect(@a.has_preference?(:color)).to be_truthy + expect(@a.has_preference?(:flavor)).not_to be_truthy end it "child should have parent and own definitions" do - expect(@b.has_preference?(:color)).to be_true - expect(@b.has_preference?(:flavor)).to be_true + expect(@b.has_preference?(:color)).to be_truthy + expect(@b.has_preference?(:flavor)).to be_truthy end it "instances have defaults" do @@ -50,8 +50,8 @@ describe Spree::Preferences::Preferable do end it "can be asked if it has a preference definition" do - expect(@a.has_preference?(:color)).to be_true - expect(@a.has_preference?(:bad)).to be_false + expect(@a.has_preference?(:color)).to be_truthy + expect(@a.has_preference?(:bad)).to be_falsy end it "can be asked and raises" do @@ -124,12 +124,12 @@ 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') - expect(Spree::Preference).to receive(:find_by_key).and_return(preference) + expect(Spree::Preference).to receive(:find_by).and_return(preference) expect(@a.preferred_color).to eq 'chatreuse' end it "defaults if no database key exists" do - expect(Spree::Preference).to receive(:find_by_key).and_return(nil) + expect(Spree::Preference).to receive(:find_by).and_return(nil) expect(@a.preferred_color).to eq 'green' end end @@ -174,28 +174,28 @@ describe Spree::Preferences::Preferable do it "with strings" do @a.set_preference(:is_boolean, '0') - expect(@a.preferences[:is_boolean]).to be_false + expect(@a.preferences[:is_boolean]).to be_falsy @a.set_preference(:is_boolean, 'f') - expect(@a.preferences[:is_boolean]).to be_false + expect(@a.preferences[:is_boolean]).to be_falsy @a.set_preference(:is_boolean, 't') - expect(@a.preferences[:is_boolean]).to be_true + expect(@a.preferences[:is_boolean]).to be_truthy end it "with integers" do @a.set_preference(:is_boolean, 0) - expect(@a.preferences[:is_boolean]).to be_false + expect(@a.preferences[:is_boolean]).to be_falsy @a.set_preference(:is_boolean, 1) - expect(@a.preferences[:is_boolean]).to be_true + expect(@a.preferences[:is_boolean]).to be_truthy end it "with an empty string" do @a.set_preference(:is_boolean, '') - expect(@a.preferences[:is_boolean]).to be_false + expect(@a.preferences[:is_boolean]).to be_falsy end it "with an empty hash" do @a.set_preference(:is_boolean, []) - expect(@a.preferences[:is_boolean]).to be_false + expect(@a.preferences[:is_boolean]).to be_falsy end end @@ -212,7 +212,7 @@ describe Spree::Preferences::Preferable do end it "with hash" do - expect(@a.preferences[:product_attributes]).to eq {} + expect(@a.preferences[:product_attributes]).to eq({}) @a.set_preference(:product_attributes, { id: 1, name: 2 }) attributes_hash = { id: 1, name: 2 } expect(@a.preferences[:product_attributes]).to eq attributes_hash @@ -318,9 +318,9 @@ describe Spree::Preferences::Preferable do it "can add and remove preferences" do A.preference :test_temp, :boolean, default: true - expect(@a.preferred_test_temp).to be_true + expect(@a.preferred_test_temp).to be_truthy A.remove_preference :test_temp - expect(@a.has_preference?(:test_temp)).to be_false - expect(@a.respond_to?(:preferred_test_temp)).to be_false + expect(@a.has_preference?(:test_temp)).to be_falsy + expect(@a.respond_to?(:preferred_test_temp)).to be_falsy end end diff --git a/spec/models/spree/preferences/store_spec.rb b/spec/models/spree/preferences/store_spec.rb index 5ce0f6b60b..49c72b9274 100644 --- a/spec/models/spree/preferences/store_spec.rb +++ b/spec/models/spree/preferences/store_spec.rb @@ -9,13 +9,13 @@ describe Spree::Preferences::Store do it "sets and gets a key" do @store.set :test, 1, :integer - @store.exist?(:test).should be_true - @store.get(:test).should eq 1 + expect(@store.exist?(:test)).to be_truthy + expect(@store.get(:test)).to eq 1 end it "can set and get false values when cache return nil" do @store.set :test, false, :boolean - @store.get(:test).should be_false + expect(@store.get(:test)).to be_falsy end it "will return db value when cache is emtpy and cache the db value" do @@ -25,24 +25,24 @@ describe Spree::Preferences::Store do preference.save Rails.cache.clear - @store.get(:test).should eq '123' - Rails.cache.read(:test).should eq '123' + expect(@store.get(:test)).to eq '123' + expect(Rails.cache.read(:test)).to eq '123' end it "should return and cache fallback value when supplied" do Rails.cache.clear - @store.get(:test, false).should be_false - Rails.cache.read(:test).should be_false + expect(@store.get(:test, false)).to be_falsy + expect(Rails.cache.read(:test)).to be_falsy end 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 - Rails.cache.read(:test).should be_true + expect(@store.get(:test, true)).to be_truthy + expect(Rails.cache.read(:test)).to be_truthy end it "should return nil when key can't be found and fallback value is not supplied" do - @store.get(:random_key).should be_nil + expect(@store.get(:random_key)).to be_nil end end From d6862cdbce9987cc20038aa9fbfa10c5a6a90599 Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Thu, 6 Aug 2020 12:51:03 +0100 Subject: [PATCH 07/11] Revert rubocop autocorrect change, it breaks some of the tests --- app/models/spree/preferences/preferable.rb | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/app/models/spree/preferences/preferable.rb b/app/models/spree/preferences/preferable.rb index 191b6d1bb4..76f4e6aad7 100644 --- a/app/models/spree/preferences/preferable.rb +++ b/app/models/spree/preferences/preferable.rb @@ -18,11 +18,15 @@ module Spree extend Spree::Preferences::PreferableClassMethods if respond_to?(:after_create) - after_create(&:save_pending_preferences) + after_create do |obj| + obj.save_pending_preferences + end end if respond_to?(:after_destroy) - after_destroy(&:clear_preferences) + after_destroy do |obj| + obj.clear_preferences + end end end end From ce2f0a5b9ec6f90badfd153ada1b6448449f7aab Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Thu, 6 Aug 2020 12:52:30 +0100 Subject: [PATCH 08/11] Transpec specs --- spec/models/spree/preferences/preferable_spec.rb | 4 ++-- spec/models/spree/preferences/store_spec.rb | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/spec/models/spree/preferences/preferable_spec.rb b/spec/models/spree/preferences/preferable_spec.rb index 3e72812b39..3e156fce1f 100644 --- a/spec/models/spree/preferences/preferable_spec.rb +++ b/spec/models/spree/preferences/preferable_spec.rb @@ -22,9 +22,9 @@ describe Spree::Preferences::Preferable do before :each do @a = A.new - @a.stub(persisted?: true) + allow(@a).to receive_messages(persisted?: true) @b = B.new - @b.stub(persisted?: true) + allow(@b).to receive_messages(persisted?: true) # ensure we're persisting as that is the default # diff --git a/spec/models/spree/preferences/store_spec.rb b/spec/models/spree/preferences/store_spec.rb index 49c72b9274..7d5995e97a 100644 --- a/spec/models/spree/preferences/store_spec.rb +++ b/spec/models/spree/preferences/store_spec.rb @@ -37,7 +37,7 @@ describe Spree::Preferences::Store 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) + allow(@store).to receive_messages(should_persist?: false) expect(@store.get(:test, true)).to be_truthy expect(Rails.cache.read(:test)).to be_truthy end From 5bf91cb14019b18b06550e9d08c82d1c2cb49f54 Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Sat, 5 Sep 2020 15:03:03 +0100 Subject: [PATCH 09/11] Remove unused spree code --- app/models/spree/app_configuration.rb | 9 --------- spec/models/spree/app_configuration_spec.rb | 5 ----- 2 files changed, 14 deletions(-) diff --git a/app/models/spree/app_configuration.rb b/app/models/spree/app_configuration.rb index e644771616..66db0b26b3 100644 --- a/app/models/spree/app_configuration.rb +++ b/app/models/spree/app_configuration.rb @@ -21,8 +21,6 @@ # a.get :color # a.preferred_color # -require "spree/core/search/base" - module Spree class AppConfiguration < Preferences::Configuration # Should state/state_name be required @@ -156,12 +154,5 @@ module Spree # Enable cache 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 - @searcher_class ||= Spree::Core::Search::Base - end - - attr_writer :searcher_class end end diff --git a/spec/models/spree/app_configuration_spec.rb b/spec/models/spree/app_configuration_spec.rb index f40edfcabe..607dc8501b 100644 --- a/spec/models/spree/app_configuration_spec.rb +++ b/spec/models/spree/app_configuration_spec.rb @@ -14,9 +14,4 @@ describe Spree::AppConfiguration do Spree::Config.site_name = "Spree::Config TEST SITE NAME" expect(Spree::Config.site_name).to eq "Spree::Config TEST SITE NAME" end - - it "uses base searcher class by default" do - prefs.searcher_class = nil - expect(prefs.searcher_class).to eq Spree::Core::Search::Base - end end From 3074d3a17a738d4bfd21c207a80c49dd1cea1d9f Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Sat, 5 Sep 2020 15:13:20 +0100 Subject: [PATCH 10/11] Customize some defaults to OFN --- app/assets/images/ofn-logo.png | Bin 0 -> 4511 bytes app/models/spree/app_configuration.rb | 19 ++++++++++--------- 2 files changed, 10 insertions(+), 9 deletions(-) create mode 100644 app/assets/images/ofn-logo.png diff --git a/app/assets/images/ofn-logo.png b/app/assets/images/ofn-logo.png new file mode 100644 index 0000000000000000000000000000000000000000..6058b26a750d96d49f4fb99e73f1ae969e26abe7 GIT binary patch literal 4511 zcmV;Q5n%3#P)=!TpU>ZLkFT##U~rF6JdLw>ylFcO;B{Ut9+eNm zuuBYt)sFxG5QIrYK~#7F?VX2~-ZlU0lZhbqo zmBwrJw+jjtz!vc5fByQfzxsRl#UFn4t6%^27mOg@jz1i|@daraRP1dKMAdoY1lh4> zZ-5~DE*+g92Z`hU<9!pfj7=16h0$}O$f5lAoYX@_$Px9r5CHXDNQ%Z2(G3Z^xi2|-k;GN8g1z%x8xroRMHhsYx;whV!12(}1< zx_{xYNfJ{sX9yzLY8a(klZp`JU&mHbXQL#YX&^!Vg`BO%_m&w*kbh~#wxAp(9wUfJ zg3TjV=bVWUM7@310H=?PAikvv-w>@}V896C8~hahy~V(q5yWfC_hhUvFns-Bf(YQ> z)*LAnc7-~CVa2wmUop1b#PN>($56y^%wBOnSFmLe={q}W{?ezPPx;4#ebT5$BZcH# z;7l7mbNX@PHTvWtHTrSg&GhROJ)`&*w}+1hhjz!FX|6xc$&)(<34jDSRnZMLt|f<0!s4ORlsDifV*7(BXN0v2%0jL3WQ>lCJdlDuKh5SeQOT$Ll3HxGS~mvZLD{=+1OfX_sZ0=CNpyxN~^ zS|({dnxW-fC@KDOBe9uAfRy~!o9}F7WXJ0_o{#iPCfWNvp@kbaBqH7A;XxD6=9~|;`z=Y5V7oO zJmX{th~{xpKuNJcKm=g(+sWvBW6xj55Rl2f#P65_5jX{=5}~iGjCtrV(81x}y$cwb zW*7ppnF3zs3ka$7#~D8>mhGMeL@M(qWDE#7;S6dR4d*4Q&p$jAq0kOlBLg$?M?hk5XcfC@ z|6rhSXzzPh0b^$YTQ(&ih!+Z&Y$V`s81WpFDu>*`bE9DbY`Hr~=7+qw!)PIe{U1~4@Nw-o0Y>r_4*M-gvoHrBqWkoIS44<+vPI^+`TgiA+qTfxX1X4TL>&2 z+TePtVVDc%0a=G(VqHMl`2L3s!w}VU4(yL`)J^BiArJ1h8i)oC1JTjfi?YoJq7N$u zqNxkWr!NV%XwcE4?3cfaTPI+$iGaCMoUrs&906k-a$tYPYrSB)MGu!3BdilprXf^} z@SjwO;7wnWGAG;zV&;@%;aAtL7Eq=k>&mfiRF1W&fXC^6U8zsXRGA|*qU7>|)HMP^ zo{bAqQH^k#U64vmFrN-;y%0yw^V0DZ>HX|cqz>USh4T(2`AK1vV4t=V(%rE45V=pJOBH<#@dq>>93~mgz2F^ekXATfm8FnU_tbWfE}D z%f=1yj=>TOclKbmUO*Nt5U@Cy6Vut&6Vr(*DAVyBgnd^tuzYEbEd)Hz6L7)O9QlH! zIfwxh+}W(a8B1_(ds#|cu;>%|>zArNjT_PYWPR5FGICphho-M&`fcVZn&wDsm&0FQD_rh%)ytTP3 zPJfH%6G$rCpZH z69M;^EwlCc;~@HUw9*f3265c5{~UJl&fy%fV0UwnmK|TU(5j+$aP(|;2aF)t-^>u@ z3AndTKu~w3k>8YzAbvG!q({EV6eo|FR9!!6)Qn&>BMJ0D_3rALLG5k|94AP?tn|E0 z8F4A+WFR1$s;(JI5|Ds^({IsgHU|f{hUrDx2oey4Gtv@1=~m!j5gCF6-1se8@co<+ zUnt2i2}rO%jP9gewQhdZ~27i1h+yUMcPbow!qa=^_VT%rf@T^8+qRBb>E)nvwjkP9!h33wUDM zY_*%FjuiZA-+t!9zWr9D3V6mH3m|?yep1?UU7vSHc;hr&J=m7!sO+T#Q$A3SRy_ZaOlr@HAHj8} z3%HZV7DN2SeTR_O^WO$u=?l$pE{@iMz)VzMS9Q|@Ciwj>6Oiqd=Caj`SZ^{1iFApI z!`FlkGn$?>0gF9)sl=**z|v`R1<`*yTs>J2T@Fsm(LlgQa@sBv5Yk18jH)o#AEz30 z?(m3RlO+!p3qn?gMeJV`yPW!rY@yjHN$xb_>kecM!+5Px}Q4SlzXL4CnU-Ujkzu$ z#sw1y*dlZit9PnC3Z)n(^Sn}YywX=O_Y@`P2Ey80Zt&6;YC38odjf8>B&+7arORj=Je(B{ihaI zxd?cBe3k-k<{u$OC&KFW2Co(8;*K=+C_2vd%Y5f;Aq)W<*@i9^@TRU(UOaAYzdCeh zcLAlrIg^ccE;lA2R#Q6)c&sLNxaKn7FfS`Du~WDSh=CMUz(cICdm$i(qIdGlLH=9K zP9Pw#3-~oc9c{Y81`&>qq=El5Fc?^Rg@G9-xupfJ9R-9^5b%k{E+7U~E=n&!lDWLzw+H&`Vp=pNFe5e6bLDSE^5__#>6h#@C2B z0?NVY`2u1n`XF-<>Eji_?bW;BLKm?4z*SP-UA|c{7U6vrl0NWdSTtXO_(3|Z0-ibx zCDFoUHm`WY4SS7zgrs6Ij+Q|h1 zVji!|L1d?IcR~rw4p|5eTp^(3g3F|UWZ)Y^An-(Oj;xGb*UwNf>$}>>RNMuWzv_08 zfbs^JgUIJz4ode8b3ic6*FZq?vPj+f4fhgj=)d!)9L&5}u6jB}rp%n{aOcv&hS z-e?ps>CQn9dHYzYImjCbs9%O@nUej#%0Ma$%(`?mLm0Beq=f*ki`IAmU! zGH06P!W3>vl6+w_j(BCn8ImNsV&0A8#V_f*U0VW9OR6n0E+TuR&)f-APC-HEI3(uhnh9ClT^QJT8 zR1BfkZG?ZwlzqrIIT@WGjy3~*C}bz~Z3@?1^w~J443!`}(?I@ZQP`z+0goc!oY(Mg z%u>6kgcFpAz(4_iYV7Q~1W5e?g1{h37(AsPo{yc?$-lQlsqiSY?Bi%Ic~Abm6*92T zpif{n)a2iLAp=2L+b>9Qsf~>MTLWbx_Tjrg-SfQSow<%j=JlfERTq+f#CiiWuAM7w zUVa@+Ig?Z#0%*kuVh*L^U-jaxTwoch_WYlR1fJ0Qj3DL_z@Jb+Ac3RY5->|92{PK5 zpIAk$D3o|rASWqR{UGWB2*N*(AQY8$p!cmg$pgR3#toWZg5cSxpr*PVE-Lkd zLut%P+5&=tAjr`CR%%XSY++OSj38Eku@t*j0Y>`Fsp=kQ1hE$G*(RHJoL9{Mb~gLK zwuc}E-ADtr6$DY2<1Bj4wuB)3E*%}ox{7TMK~yddX2+Tl#GCMgGW$lpGN>3qyceH~ x(hJRHq6d=iUkLG+U)=fiZy*2uGb4zF{2va7Z9eRZZ;k)}002ovPDHLkV1kC;YSI7z literal 0 HcmV?d00001 diff --git a/app/models/spree/app_configuration.rb b/app/models/spree/app_configuration.rb index 66db0b26b3..c0c3d9f631 100644 --- a/app/models/spree/app_configuration.rb +++ b/app/models/spree/app_configuration.rb @@ -25,7 +25,7 @@ module Spree class AppConfiguration < Preferences::Configuration # 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_interface_logo, :string, default: 'ofn-logo.png' preference :admin_products_per_page, :integer, default: 10 # Should only be true if you don't need to track inventory preference :allow_backorder_shipping, :boolean, default: false @@ -52,14 +52,15 @@ module Spree preference :currency_thousands_separator, :string, default: "," preference :display_currency, :boolean, default: false preference :default_country_id, :integer - preference :default_meta_description, :string, default: 'Spree demo site' - preference :default_meta_keywords, :string, default: 'spree, demo' + preference :default_meta_description, :string, default: 'OFN demo site' + preference :default_meta_keywords, :string, default: 'ofn, demo' preference :default_seo_title, :string, default: '' preference :dismissed_spree_alerts, :string, default: '' preference :hide_cents, :boolean, default: false 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 :layout, :string, default: 'darkswarm' + preference :logo, :string, default: 'ofn-logo.png' + # Maximum nesting level in taxons menu preference :max_level_in_taxons_menu, :integer, default: 1 preference :orders_per_page, :integer, default: 15 @@ -75,8 +76,8 @@ module Spree 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 :site_name, :string, default: 'OFN Demo Site' + preference :site_url, :string, default: 'demo.openfoodnetwork.org' preference :tax_using_ship_address, :boolean, default: true # Determines whether to track on_hand values for variants / products. preference :track_inventory_levels, :boolean, default: true @@ -101,8 +102,8 @@ module Spree # Default mail headers settings preference :enable_mail_delivery, :boolean, default: false - preference :mails_from, :string, default: 'spree@example.com' - preference :mail_bcc, :string, default: 'spree@example.com' + preference :mails_from, :string, default: 'ofn@example.com' + preference :mail_bcc, :string, default: 'ofn@example.com' preference :intercept_email, :string, default: nil # Default smtp settings From 1b4e19b32de56258670a322258dbfe08780a3aff Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Sat, 5 Sep 2020 15:19:49 +0100 Subject: [PATCH 11/11] Remore preferences not used in OFN --- app/models/spree/app_configuration.rb | 11 ----------- .../orders/customer_details/_address_form.html.haml | 4 ---- lib/spree/core/controller_helpers/common.rb | 6 +----- .../admin/configuration/general_settings_spec.rb | 8 ++++---- 4 files changed, 5 insertions(+), 24 deletions(-) diff --git a/app/models/spree/app_configuration.rb b/app/models/spree/app_configuration.rb index c0c3d9f631..d56ca5a6ac 100644 --- a/app/models/spree/app_configuration.rb +++ b/app/models/spree/app_configuration.rb @@ -34,18 +34,10 @@ module Spree 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 - # 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 # 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 # 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" @@ -55,9 +47,7 @@ module Spree preference :default_meta_description, :string, default: 'OFN demo site' preference :default_meta_keywords, :string, default: 'ofn, demo' preference :default_seo_title, :string, default: '' - preference :dismissed_spree_alerts, :string, default: '' preference :hide_cents, :boolean, default: false - preference :last_check_for_spree_alerts, :string, default: nil preference :layout, :string, default: 'darkswarm' preference :logo, :string, default: 'ofn-logo.png' @@ -71,7 +61,6 @@ module Spree preference :shipment_inc_vat, :boolean, default: false # Request instructions/info for shipping preference :shipping_instructions, :boolean, default: false - preference :show_only_complete_orders_by_default, :boolean, default: true # 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 diff --git a/app/views/spree/admin/orders/customer_details/_address_form.html.haml b/app/views/spree/admin/orders/customer_details/_address_form.html.haml index 18bd0970b6..da8d943c18 100644 --- a/app/views/spree/admin/orders/customer_details/_address_form.html.haml +++ b/app/views/spree/admin/orders/customer_details/_address_form.html.haml @@ -16,10 +16,6 @@ %div{class: "field"} = f.label :lastname, Spree.t(:last_name) + ':' = f.text_field :lastname, class: 'fullwidth' - - if Spree::Config[:company] - %div{class: "field"} - = f.label :company, Spree.t(:company) + ':' - = f.text_field :company, class: 'fullwidth' %div{class: "field"} = f.label :address1, Spree.t(:street_address) + ':' = f.text_field :address1, class: 'fullwidth' diff --git a/lib/spree/core/controller_helpers/common.rb b/lib/spree/core/controller_helpers/common.rb index e88920eaf6..c1f12fdfc0 100644 --- a/lib/spree/core/controller_helpers/common.rb +++ b/lib/spree/core/controller_helpers/common.rb @@ -37,11 +37,7 @@ module Spree def title title_string = @title.presence || accurate_title if title_string.present? - if Spree::Config[:always_put_site_name_in_title] - [title_string, default_title].join(' - ') - else - title_string - end + [title_string, default_title].join(' - ') else default_title end diff --git a/spec/features/admin/configuration/general_settings_spec.rb b/spec/features/admin/configuration/general_settings_spec.rb index e676f32801..7a9299a634 100644 --- a/spec/features/admin/configuration/general_settings_spec.rb +++ b/spec/features/admin/configuration/general_settings_spec.rb @@ -12,19 +12,19 @@ describe "General Settings" do context "visiting general settings (admin)" do it "should have the right content" do expect(page).to have_content("General Settings") - expect(find("#site_name").value).to eq("Spree Demo Site") - expect(find("#site_url").value).to eq("demo.spreecommerce.com") + expect(find("#site_name").value).to eq("OFN Demo Site") + expect(find("#site_url").value).to eq("demo.openfoodnetwork.org") end end context "editing general settings (admin)" do it "should be able to update the site name" do - fill_in "site_name", with: "Spree Demo Site99" + fill_in "site_name", with: "OFN Demo Site99" click_button "Update" assert_successful_update_message(:general_settings) - expect(find("#site_name").value).to eq("Spree Demo Site99") + expect(find("#site_name").value).to eq("OFN Demo Site99") end def assert_successful_update_message(resource)