Merge pull request #2638 from coopdevs/unit-test-variant-stock

Unit test variant stock

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

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

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

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"

lib/open_food_network/variant_stock.rb

@@ -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

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