Add unit tests to VariantStock

This also replaces the after_save callback to after_update.

We enforce a stock_item exists when modifying the variant and the
stock_item gets created on variant's after_create. This means it's not
possible to use any of the VariantStock's setters before the variant is
persisted so executing the callback on creation is pointless.

lib/open_food_network/variant_stock.rb

@@ -1,19 +1,31 @@
 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.
+# 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.
 #
-# We may decide to deprecate these methods after we designed the Network feature.
+# 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 OpenFoodNetwork
   module VariantStock
     extend ActiveSupport::Concern
 
     included do
-      after_save :save_stock
+      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
       if on_demand
         Float::INFINITY
@@ -22,10 +34,18 @@ module OpenFoodNetwork
       end
     end
 
+    # Returns the number of items available in the stock for this variant
+    #
+    # @return [Float|Integer]
     def count_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)
       error = 'Cannot set on_hand value when Spree::Config[:track_inventory_levels] is false'
       raise error unless Spree::Config.track_inventory_levels
@@ -33,19 +53,47 @@ module OpenFoodNetwork
       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 te 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 tacken
+    # 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)
       raise_error_if_no_stock_item_available
-      overwrite_stock_levels new_level
+      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
-      stock_items.any?(&:backorderable?)
+      warn_deprecation(__method__)
+      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)
       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
@@ -53,8 +101,14 @@ module OpenFoodNetwork
 
     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.each(&:save)
+      stock_items.first.save
     end
 
     def raise_error_if_no_stock_item_available
@@ -67,14 +121,9 @@ module OpenFoodNetwork
     # 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
+      # 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
   end
 end

spec/lib/open_food_network/variant_stock_spec.rb

@@ -0,0 +1,150 @@
+require 'spec_helper'
+
+describe OpenFoodNetwork::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 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