From 12eab1bfa9b8e6ece85c1e5dd6e3691beda1d687 Mon Sep 17 00:00:00 2001 From: luisramos0 Date: Mon, 18 Feb 2019 22:00:16 +0000 Subject: [PATCH] Merge variant_stock.count_on_hand into variant_stock.on_hand variant.on_hand will not return infinity any longer if variant.on_demand is true. Clients of these methods will have to handle the combinations between on_hand and on_demand values --- app/models/concerns/variant_stock.rb | 49 ++++---------------- spec/models/concerns/variant_stock_spec.rb | 54 +++++++--------------- 2 files changed, 25 insertions(+), 78 deletions(-) diff --git a/app/models/concerns/variant_stock.rb b/app/models/concerns/variant_stock.rb index f7553b107f..8eac3eb32e 100644 --- a/app/models/concerns/variant_stock.rb +++ b/app/models/concerns/variant_stock.rb @@ -20,61 +20,30 @@ module VariantStock after_update :save_stock end - # Returns the number of items of the variant available in the stock. When - # allowing on demand, it returns infinite. - # - # Spree computes it as the sum of the count_on_hand of all its stock_items. + # Returns the number of items of the variant available. + # Spree computes total_on_hand as the sum of the count_on_hand of all its stock_items. # # @return [Float|Integer] def on_hand warn_deprecation(__method__, '#total_on_hand') - if on_demand - Float::INFINITY - else - total_on_hand - end - end - - # Returns the number of items available in the stock for this variant - # - # @return [Float|Integer] - def count_on_hand - warn_deprecation(__method__, '#total_on_hand') total_on_hand end - # Sets the stock level when `track_inventory_levels` config is - # set. It raises otherwise. + # Sets the stock level of the variant. + # This will only work if `track_inventory_levels` config is set + # and if there is a stock item for the variant. # - # @raise [StandardError] when the track_inventory_levels config - # key is not set. + # @raise [StandardError] when the track_inventory_levels config key is not set + # and when the variant has no stock item def on_hand=(new_level) warn_deprecation(__method__, '#total_on_hand') error = 'Cannot set on_hand value when Spree::Config[:track_inventory_levels] is false' raise error unless Spree::Config.track_inventory_levels - self.count_on_hand = new_level - end - - # Sets the stock level. As opposed to #on_hand= it does not check - # `track_inventory_levels`'s value as it was previously an ActiveModel - # setter of the database column of the `spree_variants` table. That is why - # #on_hand= is more widely used in Spree's codebase using #count_on_hand= - # underneath. - # - # So, if #count_on_hand= is used, `track_inventory_levels` won't be taken - # into account thus dismissing instance's configuration. - # - # It does ensure there's a stock item for the variant however. See - # #raise_error_if_no_stock_item_available for details. - # - # @raise [StandardError] when the variant has no stock item yet - def count_on_hand=(new_level) - warn_deprecation(__method__, '#total_on_hand') - raise_error_if_no_stock_item_available + overwrite_stock_levels(new_level) end @@ -131,7 +100,7 @@ module VariantStock # Here we depend only on variant.total_on_hand and variant.on_demand. # This way, variant_overrides only need to override variant.total_on_hand and variant.on_demand. def fill_status(quantity) - if count_on_hand >= quantity + if on_hand >= quantity on_hand = quantity backordered = 0 else diff --git a/spec/models/concerns/variant_stock_spec.rb b/spec/models/concerns/variant_stock_spec.rb index 50c89671e4..0c521ac3e6 100644 --- a/spec/models/concerns/variant_stock_spec.rb +++ b/spec/models/concerns/variant_stock_spec.rb @@ -18,15 +18,15 @@ describe VariantStock do end describe '#on_hand' do - context 'when the variant is ordered on demand' do + context 'when the variant is on demand' do before do variant.stock_items.first.update_attribute( :backorderable, true ) end - it 'returns infinite' do - expect(variant.on_hand).to eq(Float::INFINITY) + it 'returns the total items in stock anyway' do + expect(variant.on_hand).to eq(variant.stock_items.sum(&:count_on_hand)) end end @@ -44,28 +44,27 @@ describe VariantStock do end end - describe '#count_on_hand' do - before { allow(variant).to receive(:total_on_hand) } - - it 'delegates to #total_on_hand' do - variant.count_on_hand - expect(variant).to have_received(:total_on_hand) - end - end - describe '#on_hand=' do context 'when track_inventory_levels is set' do before do - allow(variant).to receive(:count_on_hand=) allow(Spree::Config) .to receive(:track_inventory_levels) { true } end - it 'delegates to #count_on_hand=' do + it 'sets the new level as the stock item\'s count_on_hand' do variant.on_hand = 3 - expect(variant) - .to have_received(:count_on_hand=).with(3) + unique_stock_item = variant.stock_items.first + expect(unique_stock_item.count_on_hand).to eq(3) end + + context 'when the variant has no stock item' do + let(:variant) { build(:variant) } + + it 'raises' do + expect { variant.on_hand = 3 } + .to raise_error(StandardError) + end + end end context 'when track_inventory_levels is not set' do @@ -81,27 +80,6 @@ describe VariantStock do end end - describe '#count_on_hand=' do - context 'when the variant has a stock item' do - let(:variant) { create(:variant) } - - it 'sets the new level as the stock item\'s count_on_hand' do - variant.count_on_hand = 3 - unique_stock_item = variant.stock_items.first - expect(unique_stock_item.count_on_hand).to eq(3) - end - end - - context 'when the variant has no stock item' do - let(:variant) { build(:variant) } - - it 'raises' do - expect { variant.count_on_hand = 3 } - .to raise_error(StandardError) - end - end - end - describe '#on_demand' do context 'when the variant has a stock item' do let(:variant) { create(:variant) } @@ -187,7 +165,7 @@ describe VariantStock do end context 'when variant out of stock' do - before { variant.count_on_hand = 0 } + before { variant.on_hand = 0 } it "returns true for zero" do expect(variant.can_supply?(0)).to eq(true)