diff --git a/app/models/spree/product.rb b/app/models/spree/product.rb index f4aa0bd8eb..612ed9c57f 100755 --- a/app/models/spree/product.rb +++ b/app/models/spree/product.rb @@ -316,11 +316,7 @@ module Spree end def total_on_hand - if Spree::Config.track_inventory_levels - stock_items.sum(&:count_on_hand) - else - Float::INFINITY - end + stock_items.sum(&:count_on_hand) end # Master variant may be deleted (i.e. when the product is deleted) diff --git a/app/models/spree/variant.rb b/app/models/spree/variant.rb index 1abf737d60..26a3d0327b 100644 --- a/app/models/spree/variant.rb +++ b/app/models/spree/variant.rb @@ -3,6 +3,7 @@ require 'open_food_network/enterprise_fee_calculator' require 'variant_units/variant_and_line_item_naming' require 'concerns/variant_stock' +require 'spree/localized_number' module Spree class Variant < ActiveRecord::Base @@ -148,10 +149,6 @@ module Spree select("spree_variants.id")) end - def cost_price=(price) - self[:cost_price] = parse_price(price) if price.present? - end - # Allow variant to access associated soft-deleted prices. def default_price Spree::Price.unscoped { super } @@ -257,21 +254,6 @@ module Spree private - # 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']) - non_price_characters = /[^0-9\-#{separator}]/ - # 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 - # Ensures a new variant takes the product master price when price is not supplied def check_price if price.nil? && Spree::Config[:require_master_price] @@ -296,7 +278,7 @@ module Spree def create_stock_items StockLocation.all.find_each do |stock_location| - stock_location.propagate_variant(self) if stock_location.propagate_all_variants? + stock_location.propagate_variant(self) end end diff --git a/spec/models/spree/product_spec.rb b/spec/models/spree/product_spec.rb index 8c468e8456..76e89b01a0 100644 --- a/spec/models/spree/product_spec.rb +++ b/spec/models/spree/product_spec.rb @@ -236,12 +236,7 @@ module Spree end describe '#total_on_hand' do - it 'should be infinite if track_inventory_levels is false' do - Spree::Config[:track_inventory_levels] = false - expect(build(:product).total_on_hand).to eql(Float::INFINITY) - end - - it 'should return sum of stock items count_on_hand' do + it 'returns sum of stock items count_on_hand' do product = build(:product) product.stub stock_items: [double(Spree::StockItem, count_on_hand: 5)] expect(product.total_on_hand).to eql(5) diff --git a/spec/models/spree/variant_spec.rb b/spec/models/spree/variant_spec.rb index 7a661c256b..2e7ae925ee 100644 --- a/spec/models/spree/variant_spec.rb +++ b/spec/models/spree/variant_spec.rb @@ -1,5 +1,6 @@ require 'spec_helper' require 'variant_units/option_value_namer' +require 'spree/localized_number' module Spree describe Variant do @@ -17,24 +18,6 @@ module Spree end end - context "after create" do - let!(:product) { create(:product) } - - it "propagate to stock items" do - Spree::StockLocation.any_instance.should_receive(:propagate_variant) - product.variants.create(name: "Foobar") - end - - context "stock location has disable propagate all variants" do - before { Spree::StockLocation.any_instance.stub(propagate_all_variants?: false) } - - it "propagate to stock items" do - Spree::StockLocation.any_instance.should_not_receive(:propagate_variant) - product.variants.create(name: "Foobar") - end - end - end - context "product has other variants" do describe "option value accessors" do before { @@ -48,10 +31,10 @@ module Spree expect(multi_variant.option_value('media_type')).to be_nil multi_variant.set_option_value('media_type', 'DVD') - expect(multi_variant.option_value('media_type')).to == 'DVD' + expect(multi_variant.option_value('media_type')).to eq 'DVD' multi_variant.set_option_value('media_type', 'CD') - expect(multi_variant.option_value('media_type')).to == 'CD' + expect(multi_variant.option_value('media_type')).to eq 'CD' end it "should not duplicate associated option values when set multiple times" do @@ -80,10 +63,10 @@ module Spree expect(multi_variant.option_value('media_type')).to be_nil multi_variant.set_option_value('media_type', 'DVD') - expect(multi_variant.option_value('media_type')).to == 'DVD' + expect(multi_variant.option_value('media_type')).to eq 'DVD' multi_variant.set_option_value('media_type', 'CD') - expect(multi_variant.option_value('media_type')).to == 'CD' + expect(multi_variant.option_value('media_type')).to eq 'CD' end it "should not duplicate associated option values when set multiple times" do @@ -115,48 +98,23 @@ module Spree context "with decimal point" do it "captures the proper amount for a formatted price" do variant.price = '1,599.99' - expect(variant.price).to == 1599.99 + expect(variant.price).to eq 1599.99 end end context "with decimal comma" do it "captures the proper amount for a formatted price" do - I18n.locale = :de + I18n.locale = :es variant.price = '1.599,99' - expect(variant.price).to == 1599.99 + expect(variant.price).to eq 1599.99 end end context "with a numeric price" do it "uses the price as is" do - I18n.locale = :de + I18n.locale = :es variant.price = 1599.99 - expect(variant.price).to == 1599.99 - end - end - end - - context "cost_price=" do - context "with decimal point" do - it "captures the proper amount for a formatted price" do - variant.cost_price = '1,599.99' - expect(variant.cost_price).to == 1599.99 - end - end - - context "with decimal comma" do - it "captures the proper amount for a formatted price" do - I18n.locale = :de - variant.cost_price = '1.599,99' - expect(variant.cost_price).to == 1599.99 - end - end - - context "with a numeric price" do - it "uses the price as is" do - I18n.locale = :de - variant.cost_price = 1599.99 - expect(variant.cost_price).to == 1599.99 + expect(variant.price).to eq 1599.99 end end end @@ -164,23 +122,24 @@ module Spree context "#currency" do it "returns the globally configured currency" do - expect(variant.currency).to == "USD" + expect(variant.currency).to eq Spree::Config[:currency] end end context "#display_amount" do it "returns a Spree::Money" do variant.price = 21.22 - expect(variant.display_amount.to_s).to == "$21.22" + expect(variant.display_amount.to_s).to eq "$21.22" end end context "#cost_currency" do context "when cost currency is nil" do before { variant.cost_currency = nil } + it "populates cost currency with the default value on save" do variant.save! - expect(variant.cost_currency).to == "USD" + expect(variant.cost_currency).to eq Spree::Config[:currency] end end end @@ -195,23 +154,23 @@ module Spree let(:currency) { nil } it "returns 0" do - expect(subject.to_s).to == "$0.00" + expect(subject.to_s).to eq "$0.00" end end context "when currency is EUR" do let(:currency) { 'EUR' } - it "returns the value in the EUR" do - expect(subject.to_s).to == "€33.33" + it "returns the value in EUR" do + expect(subject.to_s).to eq "€33.33" end end - context "when currency is USD" do - let(:currency) { 'USD' } + context "when currency is AUD" do + let(:currency) { 'AUD' } - it "returns the value in the USD" do - expect(subject.to_s).to == "$19.99" + it "returns the value in AUD" do + expect(subject.to_s).to eq "$19.99" end end end @@ -234,35 +193,20 @@ module Spree context "when currency is EUR" do let(:currency) { 'EUR' } - it "returns the value in the EUR" do - expect(subject).to == 33.33 + it "returns the value in EUR" do + expect(subject).to eq 33.33 end end - context "when currency is USD" do - let(:currency) { 'USD' } + context "when currency is AUD" do + let(:currency) { 'AUD' } - it "returns the value in the USD" do - expect(subject).to == 19.99 + it "returns the value in AUD" do + expect(subject).to eq 19.99 end end end - # Regression test for #2432 - describe 'options_text' do - before do - option_type = double("OptionType", presentation: "Foo") - option_values = [double("OptionValue", option_type: option_type, presentation: "bar")] - variant.stub(:option_values).and_return(option_values) - end - - it "orders options correctly" do - variant.option_values.should_receive(:joins).with(:option_type).and_return(scope = double) - scope.should_receive(:order).with('spree_option_types.position asc').and_return(variant.option_values) - variant.options_text - end - end - # Regression test for #2744 describe "set_position" do it "sets variant position after creation" do @@ -272,10 +216,6 @@ module Spree end describe '#in_stock?' do - before do - Spree::Config.track_inventory_levels = true - end - context 'when stock_items are not backorderable' do before do Spree::StockItem.any_instance.stub(backorderable: false) @@ -307,7 +247,7 @@ module Spree variant.stock_items.first.update_attribute(:count_on_hand, 10) end - it 'returns correctt value' do + it 'returns correct value' do expect(variant.in_stock?).to be_truthy expect(variant.in_stock?(2)).to be_truthy expect(variant.in_stock?(10)).to be_truthy @@ -318,7 +258,7 @@ module Spree context 'when stock_items are backorderable' do before do - Spree::StockItem.any_instance.stub(backorderable: true) + Spree::StockItem.any_instance.stub(backorderable?: true) end context 'when stock_items out of stock' do @@ -334,25 +274,12 @@ module Spree end describe '#total_on_hand' do - it 'should be infinite if track_inventory_levels is false' do - Spree::Config[:track_inventory_levels] = false - expect(build(:variant).total_on_hand).to eql(Float::INFINITY) - end - - it 'should match quantifier total_on_hand' do + it 'matches quantifier total_on_hand' do variant = build(:variant) expect(variant.total_on_hand).to eq(Spree::Stock::Quantifier.new(variant).total_on_hand) end end - describe "double loading" do - # app/models/spree/variant_decorator.rb may be double-loaded in delayed job environment, - # so we need to be able to do so without error. - it "succeeds without error" do - load "#{Rails.root}/app/models/spree/variant_decorator.rb" - end - end - describe "scopes" do describe "finding variants in a distributor" do let!(:d1) { create(:distributor_enterprise) }