diff --git a/app/models/spree/option_value.rb b/app/models/spree/option_value.rb index cad9ba464a..b21eed19ba 100644 --- a/app/models/spree/option_value.rb +++ b/app/models/spree/option_value.rb @@ -4,7 +4,8 @@ module Spree class OptionValue < ActiveRecord::Base belongs_to :option_type acts_as_list scope: :option_type - has_and_belongs_to_many :variants, join_table: 'spree_option_values_variants', class_name: "Spree::Variant" + has_and_belongs_to_many :variants, join_table: 'spree_option_values_variants', + class_name: "Spree::Variant" validates :name, :presentation, presence: true end diff --git a/app/models/spree/price.rb b/app/models/spree/price.rb index 69670223dd..d0dab12359 100644 --- a/app/models/spree/price.rb +++ b/app/models/spree/price.rb @@ -34,19 +34,22 @@ module Spree private def check_price - if currency.nil? - self.currency = Spree::Config[:currency] - end + return unless currency.nil? + + self.currency = Spree::Config[:currency] end # strips all non-price-like characters from the price, taking into account locale settings def parse_price(price) return price unless price.is_a?(String) - separator, delimiter = I18n.t([:'number.currency.format.separator', :'number.currency.format.delimiter']) + separator, _delimiter = I18n.t([:'number.currency.format.separator', + :'number.currency.format.delimiter']) non_price_characters = /[^0-9\-#{separator}]/ - price.gsub!(non_price_characters, '') # strip everything else first - price.gsub!(separator, '.') unless separator == '.' # then replace the locale-specific decimal separator with the standard separator if necessary + # Strip everything else first + price.gsub!(non_price_characters, '') + # Then replace the locale-specific decimal separator with the standard separator if necessary + price.gsub!(separator, '.') unless separator == '.' price.to_d end diff --git a/app/models/spree/product.rb b/app/models/spree/product.rb index 7e5aaf8d95..76ca236351 100755 --- a/app/models/spree/product.rb +++ b/app/models/spree/product.rb @@ -52,21 +52,26 @@ module Spree class_name: 'Spree::Variant', dependent: :destroy - has_many :variants, - -> { where(is_master: false).order("#{::Spree::Variant.quoted_table_name}.position ASC") }, - class_name: 'Spree::Variant' + has_many :variants, -> { + where(is_master: false).order("#{::Spree::Variant.quoted_table_name}.position ASC") + }, class_name: 'Spree::Variant' has_many :variants_including_master, -> { order("#{::Spree::Variant.quoted_table_name}.position ASC") }, class_name: 'Spree::Variant', dependent: :destroy - has_many :prices, -> { order('spree_variants.position, spree_variants.id, currency') }, through: :variants + has_many :prices, -> { + order('spree_variants.position, spree_variants.id, currency') + }, through: :variants has_many :stock_items, through: :variants - delegate_belongs_to :master, :sku, :price, :currency, :display_amount, :display_price, :weight, :height, :width, :depth, :is_master, :has_default_price?, :cost_currency, :price_in, :amount_in, :unit_value, :unit_description - delegate_belongs_to :master, :cost_price if Variant.table_exists? && Variant.column_names.include?('cost_price') + delegate_belongs_to :master, :sku, :price, :currency, :display_amount, :display_price, :weight, + :height, :width, :depth, :is_master, :has_default_price?, :cost_currency, + :price_in, :amount_in, :unit_value, :unit_description + delegate_belongs_to :master, :cost_price if Variant.table_exists? && + Variant.column_names.include?('cost_price') delegate :images_attributes=, :display_as=, to: :master after_create :set_master_variant_defaults @@ -77,7 +82,8 @@ module Spree delegate :images, to: :master, prefix: true alias_method :images, :master_images - has_many :variant_images, -> { order(:position) }, source: :images, through: :variants_including_master + has_many :variant_images, -> { order(:position) }, source: :images, + through: :variants_including_master accepts_nested_attributes_for :variants, allow_destroy: true @@ -98,7 +104,9 @@ module Spree attr_accessor :option_values_hash - accepts_nested_attributes_for :product_properties, allow_destroy: true, reject_if: lambda { |pp| pp[:property_name].blank? } + accepts_nested_attributes_for :product_properties, + allow_destroy: true, + reject_if: lambda { |pp| pp[:property_name].blank? } make_permalink order: :name @@ -134,7 +142,8 @@ module Spree scope :imported_on, lambda { |import_date| import_date = Time.zone.parse import_date if import_date.is_a? String import_date = import_date.to_date - joins(:variants).merge(Spree::Variant.where(import_date: import_date.beginning_of_day..import_date.end_of_day)) + joins(:variants).merge(Spree::Variant. + where(import_date: import_date.beginning_of_day..import_date.end_of_day)) } scope :with_order_cycles_inner, -> { @@ -206,7 +215,9 @@ module Spree return where('1=0') if enterprise.blank? permitted_producer_ids = EnterpriseRelationship.joins(:parent).permitting(enterprise.id) - .with_permission(:add_to_order_cycle).where(enterprises: { is_primary_producer: true }).pluck(:parent_id) + .with_permission(:add_to_order_cycle) + .where(enterprises: { is_primary_producer: true }) + .pluck(:parent_id) return where('spree_products.supplier_id IN (?)', [enterprise.id] | permitted_producer_ids) } @@ -221,7 +232,7 @@ module Spree def tax_category if self[:tax_category_id].nil? - TaxCategory.where(is_default: true).first + TaxCategory.find_by(is_default: true) else TaxCategory.find(self[:tax_category_id]) end @@ -239,7 +250,9 @@ module Spree option_values_hash.keys.map(&:to_i).each do |id| option_type_ids << id unless option_type_ids.include?(id) - product_option_types.create(option_type_id: id) unless product_option_types.pluck(:option_type_id).include?(id) + unless product_option_types.pluck(:option_type_id).include?(id) + product_option_types.create(option_type_id: id) + end end end @@ -309,7 +322,8 @@ module Spree def set_property(property_name, property_value) ActiveRecord::Base.transaction do property = Property.where(name: property_name).first_or_create!(presentation: property_name) - product_property = ProductProperty.where(product: self, property: property).first_or_initialize + product_property = ProductProperty.where(product: self, + property: property).first_or_initialize product_property.value = property_value product_property.save! end @@ -332,7 +346,7 @@ module Spree # which would make AR's default finder return nil. # This is a stopgap for that little problem. def master - super || variants_including_master.with_deleted.where(is_master: true).first + super || variants_including_master.with_deleted.find_by(is_master: true) end def properties_including_inherited @@ -366,14 +380,18 @@ module Spree end def variant_unit_option_type - if variant_unit.present? - option_type_name = "unit_#{variant_unit}" - option_type_presentation = variant_unit.capitalize + return if variant_unit.blank? - Spree::OptionType.find_by(name: option_type_name) || - Spree::OptionType.create!(name: option_type_name, - presentation: option_type_presentation) - end + option_type_name = "unit_#{variant_unit}" + option_type_presentation = variant_unit.capitalize + + Spree::OptionType.find_by(name: option_type_name) || + Spree::OptionType.create!(name: option_type_name, + presentation: option_type_presentation) + end + + def self.all_variant_unit_option_types + Spree::OptionType.where('name LIKE ?', 'unit_%%') end def destroy_with_delete_from_order_cycles @@ -398,7 +416,7 @@ module Spree values = values.inject(values.shift) { |memo, value| memo.product(value).map(&:flatten) } values.each do |ids| - variant = variants.create( + variants.create( option_value_ids: ids, price: master.price ) @@ -407,12 +425,12 @@ module Spree end def add_properties_and_option_types_from_prototype - if prototype_id && prototype = Spree::Prototype.find_by(id: prototype_id) - prototype.properties.each do |property| - product_properties.create(property: property) - end - self.option_types = prototype.option_types + return unless prototype_id && prototype = Spree::Prototype.find_by(id: prototype_id) + + prototype.properties.each do |property| + product_properties.create(property: property) end + self.option_types = prototype.option_types end # ensures the master variant is flagged as such @@ -420,8 +438,8 @@ module Spree master.is_master = true end - # This fixes any problems arising from failing master saves, without the need for a validates_associated on - # master, while giving us more specific errors as to why saving failed + # Here we rescue errors when saving master variants (without the need for a + # validates_associated on master) and we get more specific data about the errors def save_master if master && ( master.changed? || master.new_record? || ( @@ -449,7 +467,8 @@ module Spree end def punch_permalink - update_attribute :permalink, "#{Time.now.to_i}_#{permalink}" # punch permalink with date prefix + # Punch permalink with date prefix + update_attribute :permalink, "#{Time.now.to_i}_#{permalink}" end def set_available_on_to_now @@ -457,11 +476,11 @@ module Spree end def update_units - if variant_unit_changed? - option_types.delete self.class.all_variant_unit_option_types - option_types << variant_unit_option_type if variant_unit.present? - variants_including_master.each(&:update_units) - end + return unless variant_unit_changed? + + option_types.delete self.class.all_variant_unit_option_types + option_types << variant_unit_option_type if variant_unit.present? + variants_including_master.each(&:update_units) end def touch_distributors @@ -478,25 +497,21 @@ module Spree taxons.destroy(primary_taxon_id_was) end - def self.all_variant_unit_option_types - Spree::OptionType.where('name LIKE ?', 'unit_%%') - end - def ensure_standard_variant - if master.valid? && variants.empty? - variant = master.dup - variant.product = self - variant.is_master = false - variants << variant - end + return unless master.valid? && variants.empty? + + variant = master.dup + variant.product = self + variant.is_master = false + variants << variant end # Spree creates a permalink already but our implementation fixes an edge case. def sanitize_permalink - if permalink.blank? || permalink_changed? - requested = permalink.presence || permalink_was.presence || name.presence || 'product' - self.permalink = create_unique_permalink(requested.parameterize) - end + return unless permalink.blank? || permalink_changed? + + requested = permalink.presence || permalink_was.presence || name.presence || 'product' + self.permalink = create_unique_permalink(requested.parameterize) end end end diff --git a/app/models/spree/product_property.rb b/app/models/spree/product_property.rb index 68e17bfa86..a86a12f1a9 100644 --- a/app/models/spree/product_property.rb +++ b/app/models/spree/product_property.rb @@ -16,12 +16,12 @@ module Spree end def property_name=(name) - if name.present? - unless property = Property.find_by(name: name) - property = Property.create(name: name, presentation: name) - end - self.property = property + return if name.blank? + + unless property = Property.find_by(name: name) + property = Property.create(name: name, presentation: name) end + self.property = property end end end diff --git a/app/models/spree/variant.rb b/app/models/spree/variant.rb index 13d7e769d6..1abf737d60 100644 --- a/app/models/spree/variant.rb +++ b/app/models/spree/variant.rb @@ -26,7 +26,9 @@ module Spree has_and_belongs_to_many :option_values, join_table: :spree_option_values_variants - has_many :images, -> { order(:position) }, as: :viewable, dependent: :destroy, class_name: "Spree::Image" + has_many :images, -> { order(:position) }, as: :viewable, + dependent: :destroy, + class_name: "Spree::Image" accepts_nested_attributes_for :images has_one :default_price, @@ -46,7 +48,9 @@ module Spree localize_number :price, :cost_price, :weight validate :check_price - validates :price, numericality: { greater_than_or_equal_to: 0 }, presence: true, if: proc { Spree::Config[:require_master_price] } + validates :price, numericality: { greater_than_or_equal_to: 0 }, + presence: true, + if: proc { Spree::Config[:require_master_price] } validates :cost_price, numericality: { greater_than_or_equal_to: 0, allow_nil: true } if table_exists? && column_names.include?('cost_price') validates :unit_value, presence: true, if: ->(variant) { @@ -89,7 +93,8 @@ module Spree } scope :for_distribution, lambda { |order_cycle, distributor| - where('spree_variants.id IN (?)', order_cycle.variants_distributed_by(distributor).select(&:id)) + where('spree_variants.id IN (?)', order_cycle.variants_distributed_by(distributor). + select(&:id)) } scope :visible_for, lambda { |enterprise| @@ -203,7 +208,8 @@ module Spree option_values.delete(current_value) end - option_value = Spree::OptionValue.where(option_type_id: option_type.id, name: opt_value).first_or_initialize do |o| + option_value = Spree::OptionValue.where(option_type_id: option_type.id, + name: opt_value).first_or_initialize do |o| o.presentation = opt_value o.save! end @@ -221,7 +227,8 @@ module Spree end def price_in(currency) - prices.select{ |price| price.currency == currency }.first || Spree::Price.new(variant_id: id, currency: currency) + prices.select{ |price| price.currency == currency }.first || + Spree::Price.new(variant_id: id, currency: currency) end def amount_in(currency) @@ -254,10 +261,13 @@ module Spree def parse_price(price) return price unless price.is_a?(String) - separator, delimiter = I18n.t([:'number.currency.format.separator', :'number.currency.format.delimiter']) + separator, _delimiter = I18n.t([:'number.currency.format.separator', + :'number.currency.format.delimiter']) non_price_characters = /[^0-9\-#{separator}]/ - price.gsub!(non_price_characters, '') # strip everything else first - price.gsub!(separator, '.') unless separator == '.' # then replace the locale-specific decimal separator with the standard separator if necessary + # Strip everything else first + price.gsub!(non_price_characters, '') + # Then replace the locale-specific decimal separator with the standard separator if necessary + price.gsub!(separator, '.') unless separator == '.' price.to_d end @@ -270,9 +280,10 @@ module Spree self.price = product.master.price end - if currency.nil? - self.currency = Spree::Config[:currency] - end + + return unless currency.nil? + + self.currency = Spree::Config[:currency] end def save_default_price @@ -294,7 +305,9 @@ module Spree end def update_weight_from_unit_value - self.weight = weight_from_unit_value if product.variant_unit == 'weight' && unit_value.present? + return unless product.variant_unit == 'weight' && unit_value.present? + + self.weight = weight_from_unit_value end def destruction