diff --git a/app/models/concerns/variant_stock.rb b/app/models/concerns/variant_stock.rb new file mode 100644 index 0000000000..2e1495be1b --- /dev/null +++ b/app/models/concerns/variant_stock.rb @@ -0,0 +1,143 @@ +require 'active_support/concern' + +# These methods were available in Spree 1, but were removed in Spree 2. We +# would still like to use them so that we still give support to the consumers +# of this methods, making the upgrade backward compatible. +# +# Therefore we use only a single stock item per variant, which is associated to +# a single stock location per instance (default stock location) and use it to +# track the `count_on_hand` value that was previously a database column on +# variants. See +# https://github.com/openfoodfoundation/openfoodnetwork/wiki/Spree-Upgrade%3A-Stock-locations +# for details. +# +# These methods are or may become deprecated. +module VariantStock + extend ActiveSupport::Concern + + included do + 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. + # + # @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. + # + # @raise [StandardError] when the track_inventory_levels config + # key is not set. + 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 + + # Checks whether this variant is produced on demand. + # + # In Spree 2.0 this attribute is removed in favour of + # track_inventory_levels only. It was initially introduced in + # https://github.com/openfoodfoundation/spree/commit/20b5ad9835dca7f41a40ad16c7b45f987eea6dcc + def on_demand + warn_deprecation(__method__, 'StockItem#backorderable?') + stock_items.first.backorderable? + end + + # Sets whether the variant can be ordered on demand or not. Note that + # although this modifies the stock item, it is not persisted in DB. This + # may be done to fire a single UPDATE statement when changing various + # variant attributes, for performance reasons. + # + # @raise [StandardError] when the variant has no stock item yet + def on_demand=(new_value) + warn_deprecation(__method__, 'StockItem#backorderable=') + + raise_error_if_no_stock_item_available + + # There should be only one at the default stock location. + # + # This would be better off as `stock_items.first.save` but then, for + # unknown reasons, it does not pass the test. + stock_items.each do |item| + item.backorderable = new_value + end + end + + private + + # Persists the single stock item associated to this variant. As defined in + # the top-most comment, as there's a single stock location in the whole + # database, there can only be a stock item per variant. See StockItem's + # definition at + # https://github.com/openfoodfoundation/spree/blob/43950c3689a77a7f493cc6d805a0edccfe75ebc2/core/app/models/spree/stock_item.rb#L3-L4 + # for details. + def save_stock + stock_items.first.save + end + + def raise_error_if_no_stock_item_available + message = 'You need to save the variant to create a stock item before you can set stock levels.' + raise message if stock_items.empty? + end + + # Backwards compatible setting of stock levels in Spree 2.0. + # It would be better to use `Spree::StockItem.adjust_count_on_hand` which + # takes a value to add to the current stock level and uses proper locking. + # But this should work the same as in Spree 1.3. + def overwrite_stock_levels(new_level) + # There shouldn't be any other stock items, because we should + # have only one stock location. + stock_items.first.__send__(:count_on_hand=, new_level) + end + + def warn_deprecation(method_name, new_method_name) + ActiveSupport::Deprecation.warn( + "`##{method_name}` is deprecated and will be removed. " \ + "Please use `#{new_method_name}` instead." + ) + end +end diff --git a/app/models/spree/variant_decorator.rb b/app/models/spree/variant_decorator.rb index e53e91ca29..41529e3949 100644 --- a/app/models/spree/variant_decorator.rb +++ b/app/models/spree/variant_decorator.rb @@ -1,6 +1,6 @@ require 'open_food_network/enterprise_fee_calculator' require 'open_food_network/variant_and_line_item_naming' -require 'open_food_network/variant_stock' +require 'concerns/variant_stock' require 'open_food_network/products_cache' Spree::Variant.class_eval do @@ -10,7 +10,7 @@ Spree::Variant.class_eval do # removing the Spree method to prevent error. remove_method :options_text if instance_methods(false).include? :options_text include OpenFoodNetwork::VariantAndLineItemNaming - include OpenFoodNetwork::VariantStock + include VariantStock has_many :exchange_variants has_many :exchanges, through: :exchange_variants diff --git a/db/migrate/20180906094641_add_uniqueness_of_variant_id_to_spree_stock_items.rb b/db/migrate/20180906094641_add_uniqueness_of_variant_id_to_spree_stock_items.rb new file mode 100644 index 0000000000..bc7713f428 --- /dev/null +++ b/db/migrate/20180906094641_add_uniqueness_of_variant_id_to_spree_stock_items.rb @@ -0,0 +1,5 @@ +class AddUniquenessOfVariantIdToSpreeStockItems < ActiveRecord::Migration + def change + add_index :spree_stock_items, :variant_id, unique: true + end +end diff --git a/db/schema.rb b/db/schema.rb index 007691a87c..13f7aea145 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -961,6 +961,7 @@ ActiveRecord::Schema.define(:version => 20180910155506) do add_index "spree_stock_items", ["stock_location_id", "variant_id"], :name => "stock_item_by_loc_and_var_id" add_index "spree_stock_items", ["stock_location_id"], :name => "index_spree_stock_items_on_stock_location_id" + add_index "spree_stock_items", ["variant_id"], :name => "index_spree_stock_items_on_variant_id", :unique => true create_table "spree_stock_locations", :force => true do |t| t.string "name" diff --git a/lib/open_food_network/variant_stock.rb b/lib/open_food_network/variant_stock.rb deleted file mode 100644 index fba38ebd57..0000000000 --- a/lib/open_food_network/variant_stock.rb +++ /dev/null @@ -1,80 +0,0 @@ -require 'active_support/concern' - -# These methods were available in Spree 1, but were removed in Spree 2. -# We would still like to use them. Therefore we use only a single stock location -# (default stock location) and use it to track the `count_on_hand` value that -# was previously a database column on variants. -# -# We may decide to deprecate these methods after we designed the Network feature. -module OpenFoodNetwork - module VariantStock - extend ActiveSupport::Concern - - included do - after_save :save_stock - end - - def on_hand - if on_demand - Float::INFINITY - else - total_on_hand - end - end - - def count_on_hand - total_on_hand - end - - def on_hand=(new_level) - 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 - - def count_on_hand=(new_level) - raise_error_if_no_stock_item_available - overwrite_stock_levels new_level - end - - def on_demand - stock_items.any?(&:backorderable?) - end - - def on_demand=(new_value) - raise_error_if_no_stock_item_available - - # There should be only one at the default stock location. - stock_items.each do |item| - item.backorderable = new_value - end - end - - private - - def save_stock - stock_items.each(&:save) - end - - def raise_error_if_no_stock_item_available - message = 'You need to save the variant to create a stock item before you can set stock levels.' - raise message if stock_items.empty? - end - - # Backwards compatible setting of stock levels in Spree 2.0. - # It would be better to use `Spree::StockItem.adjust_count_on_hand` which - # takes a value to add to the current stock level and uses proper locking. - # But this should work the same as in Spree 1.3. - def overwrite_stock_levels(new_level) - stock_items.first.send :count_on_hand=, new_level - - # There shouldn't be any other stock items, because we should have only one - # stock location. But in case there are, the total should be new_level, - # so all others need to be zero. - stock_items[1..-1].each do |item| - item.send :count_on_hand=, 0 - end - end - end -end diff --git a/spec/models/concerns/variant_stock_spec.rb b/spec/models/concerns/variant_stock_spec.rb new file mode 100644 index 0000000000..ff95b84526 --- /dev/null +++ b/spec/models/concerns/variant_stock_spec.rb @@ -0,0 +1,165 @@ +require 'spec_helper' + +describe VariantStock do + let(:variant) { create(:variant) } + + describe '#after_save' do + context 'when updating a variant' do + let(:variant) { create(:variant) } + let(:stock_item) { variant.stock_items.first } + + before { allow(stock_item).to receive(:save) } + + it 'saves its stock item' do + variant.save + expect(stock_item).to have_received(:save) + end + end + end + + describe '#on_hand' do + context 'when the variant is ordered 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) + end + end + + context 'when the variant is not ordered on demand' do + before do + variant.stock_items.first.update_attribute( + :backorderable, false + ) + end + + it 'returns the total items in stock' do + expect(variant.on_hand) + .to eq(variant.stock_items.sum(&:count_on_hand)) + end + 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 + variant.on_hand = 3 + expect(variant) + .to have_received(:count_on_hand=).with(3) + end + end + + context 'when track_inventory_levels is not set' do + before do + allow(Spree::Config) + .to receive(:track_inventory_levels) { false } + end + + it 'raises' do + expect { variant.on_hand = 3 } + .to raise_error(StandardError) + end + 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 stock items is backorderable' do + before do + variant.stock_items.first.update_attribute( + :backorderable, true + ) + end + + it 'returns true' do + expect(variant.on_demand).to be_truthy + end + end + + context 'when the stock items is backorderable' do + before do + variant.stock_items.first.update_attribute( + :backorderable, false + ) + end + + it 'returns false' do + expect(variant.on_demand).to be_falsy + end + end + end + + describe '#on_demand=' do + context 'when the variant has multiple stock items' do + let(:variant) { create(:variant) } + + before do + # Spree creates a stock_item for each variant when creating a stock + # location by means of #propagate_variant + create(:stock_location, name: 'location') + create(:stock_location, name: 'another location') + end + + it 'raises' do + expect { variant.on_demand = true }.to raise_error(StandardError) + end + end + + context 'when the variant has a stock item' do + let(:variant) { create(:variant, on_demand: true) } + + it 'sets the value as the stock item\'s backorderable value' do + variant.on_demand = false + stock_item = variant.stock_items.first + expect(stock_item.backorderable).to eq(false) + end + end + + context 'when the variant has no stock item' do + let(:variant) { build(:variant) } + + it 'raises' do + expect { variant.on_demand = 3 }.to raise_error(StandardError) + end + end + end +end