Merge pull request #3197 from luisramos0/2-0-product-stock

[Spree Upgrade] Adapt Product level stock management to Spree 2

app/controllers/spree/admin/products_controller_decorator.rb

@@ -31,6 +31,18 @@ Spree::Admin::ProductsController.class_eval do
     @show_latest_import = params[:latest_import] || false
   end
 
+  def create
+    delete_stock_params_and_set_after do
+      super
+    end
+  end
+
+  def update
+    delete_stock_params_and_set_after do
+      super
+    end
+  end
+
   def bulk_update
     collection_hash = Hash[params[:products].each_with_index.map { |p,i| [i,p] }]
     product_set = Spree::ProductSet.new({:collection_attributes => collection_hash})
@@ -119,4 +131,22 @@ Spree::Admin::ProductsController.class_eval do
       end
     end
   end
+
+  def delete_stock_params_and_set_after
+    on_demand = params[:product].delete(:on_demand)
+    on_hand = params[:product].delete(:on_hand)
+
+    yield
+
+    set_stock_levels(@product, on_hand, on_demand) if @product.valid?
+  end
+
+  def set_stock_levels(product, on_hand, on_demand)
+    variant = product.master
+    if product.variants.any?
+      variant = product.variants.first
+    end
+    variant.on_demand = on_demand if on_demand.present?
+    variant.on_hand = on_hand.to_i if on_hand.present?
+  end
 end

app/models/concerns/product_on_demand.rb

@@ -1,10 +0,0 @@
-require 'active_support/concern'
-
-module ProductOnDemand
-  extend ActiveSupport::Concern
-
-  def on_demand=(value)
-    raise 'cannot set on_demand of product with variants' if variants.any?
-    master.on_demand = value
-  end
-end

app/models/concerns/product_stock.rb

@@ -0,0 +1,22 @@
+require 'active_support/concern'
+
+module ProductStock
+  extend ActiveSupport::Concern
+
+  def on_demand
+    if has_variants?
+      raise 'Cannot determine product on_demand value of product with multiple variants' if variants.size > 1
+      variants.first.on_demand
+    else
+      master.on_demand
+    end
+  end
+
+  def on_hand
+    if has_variants?
+      variants.map(&:on_hand).reduce(:+)
+    else
+      master.on_hand
+    end
+  end
+end

app/models/concerns/variant_stock.rb

@@ -85,6 +85,11 @@ module VariantStock
   # https://github.com/openfoodfoundation/spree/commit/20b5ad9835dca7f41a40ad16c7b45f987eea6dcc
   def on_demand
     warn_deprecation(__method__, 'StockItem#backorderable?')
+
+    # A variant that has not been saved yet, doesn't have a stock item
+    #   This provides a default value for variant.on_demand using Spree::StockLocation.backorderable_default
+    return Spree::StockLocation.first.backorderable_default if stock_items.empty?
+
     stock_item.backorderable?
   end
 

app/models/spree/product_decorator.rb

@@ -1,10 +1,10 @@
 require 'open_food_network/permalink_generator'
 require 'open_food_network/property_merge'
-require 'concerns/product_on_demand'
+require 'concerns/product_stock'
 
 Spree::Product.class_eval do
   include PermalinkGenerator
-  include ProductOnDemand
+  include ProductStock
 
   # We have an after_destroy callback on Spree::ProductOptionType. However, if we
   # don't specify dependent => destroy on this association, it is not called. See:
@@ -130,14 +130,6 @@ Spree::Product.class_eval do
 
   # -- Methods
 
-  def on_hand
-    if has_variants?
-      variants.map(&:on_hand).reduce(:+)
-    else
-      master.on_hand
-    end
-  end
-
   # Called by Spree::Product::duplicate before saving.
   def duplicate_extra(parent)
     # Spree sets the SKU to "COPY OF #{parent sku}".

app/models/spree/product_set.rb

@@ -63,16 +63,16 @@ class Spree::ProductSet < ModelSet
 
   def update_product_master(product, attributes)
     return true unless attributes[:master_attributes]
-    update_variant(product, attributes[:master_attributes])
+    create_or_update_variant(product, attributes[:master_attributes])
   end
 
   def update_variants_attributes(product, variants_attributes)
     variants_attributes.each do |attributes|
-      update_variant(product, attributes)
+      create_or_update_variant(product, attributes)
     end
   end
 
-  def update_variant(product, variant_attributes)
+  def create_or_update_variant(product, variant_attributes)
     found_variant = product.variants_including_master.find do |variant|
       variant.id.to_s == variant_attributes[:id].to_s && variant.persisted?
     end
@@ -80,10 +80,20 @@ class Spree::ProductSet < ModelSet
     if found_variant.present?
       found_variant.update_attributes(variant_attributes.except(:id))
     else
-      product.variants.create(variant_attributes)
+      create_variant(product, variant_attributes)
     end
   end
 
+  def create_variant(product, variant_attributes)
+      on_hand = variant_attributes.delete(:on_hand)
+      on_demand = variant_attributes.delete(:on_demand)
+
+      variant = product.variants.create(variant_attributes)
+
+      variant.on_demand = on_demand if on_demand.present?
+      variant.on_hand = on_hand.to_i if on_hand.present?
+  end
+
   def collection_attributes=(attributes)
     @collection = Spree::Product
       .where(id: attributes.each_value.map { |product| product[:id] })

knapsack_rspec_report.json

@@ -158,7 +158,7 @@
   "spec/requests/checkout/failed_checkout_spec.rb": 1.9082491397857666,
   "spec/requests/embedded_shopfronts_headers_spec.rb": 0.8479242324829102,
   "spec/models/spree/calculator/price_sack_spec.rb": 0.671210527420044,
-  "spec/models/concerns/product_on_demand_spec.rb": 0.43450260162353516,
+  "spec/models/concerns/product_stock_spec.rb": 0.43450260162353516,
   "spec/lib/open_food_network/order_and_distributor_report_spec.rb": 0.61724853515625,
   "spec/features/admin/caching_spec.rb": 0.7170243263244629,
   "spec/controllers/api/customers_controller_spec.rb": 0.49866557121276855,

spec/models/concerns/product_on_demand_spec.rb

@@ -1,28 +0,0 @@
-require 'spec_helper'
-
-describe ProductOnDemand do
-  describe '#on_demand=' do
-    context 'when the product has no variants' do
-      let(:product) { create(:simple_product) }
-
-      before do
-        product.variants.first.destroy
-        product.variants.reload
-      end
-
-      it 'sets the value on master.on_demand' do
-        product.on_demand = false
-        expect(product.master.on_demand).to eq(false)
-      end
-    end
-
-    context 'when the product has variants' do
-      let(:product) { create(:simple_product) }
-
-      it 'raises' do
-        expect { product.on_demand = true }
-          .to raise_error(StandardError, /cannot set on_demand/)
-      end
-    end
-  end
-end

spec/models/concerns/product_stock_spec.rb

@@ -0,0 +1,57 @@
+require "spec_helper"
+
+describe ProductStock do
+  let(:product) { create(:simple_product) }
+
+  context "when product has no variants" do
+    before do
+      product.variants.first.destroy
+      product.variants.reload
+    end
+
+    describe "product.on_demand" do
+      it "is master.on_demand" do
+        expect(product.on_demand).to eq(product.master.on_demand)
+      end
+    end
+
+    describe "product.on_hand" do
+      it "is master.on_hand" do
+        expect(product.on_hand).to eq(product.master.on_hand)
+      end
+    end
+  end
+
+  context "when product has one variant" do
+    describe "product.on_demand" do
+      it "is the products first variant on_demand" do
+        expect(product.on_demand).to eq(product.variants.first.on_demand)
+      end
+    end
+
+    describe "product.on_hand" do
+      it "is the products first variant on_hand" do
+        expect(product.on_hand).to eq(product.variants.first.on_hand)
+      end
+    end
+  end
+
+  context 'when product has more than one variant' do
+    before do
+      product.variants << create(:variant, product: product)
+    end
+
+    describe "product.on_demand" do
+      it "raises error" do
+        expect { product.on_demand }
+          .to raise_error(StandardError, /Cannot determine product on_demand value/)
+      end
+    end
+
+    describe "product.on_hand" do
+      it "is the sum of the products variants on_hand values" do
+        expect(product.on_hand).to eq(product.variants.first.on_hand + product.variants.second.on_hand)
+      end
+    end
+  end
+end

spec/models/concerns/variant_stock_spec.rb

@@ -103,27 +103,39 @@ describe VariantStock do
   end
 
   describe '#on_demand' do
-    context 'when the stock items is backorderable' do
-      before do
-        variant.stock_items.first.update_attribute(
-          :backorderable, true
-        )
+    context 'when the variant has a stock item' do
+      let(:variant) { create(:variant) }
+
+      context 'when the stock item 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
 
-      it 'returns true' do
-        expect(variant.on_demand).to be_truthy
+      context 'when the stock items is not 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
 
-    context 'when the stock items is backorderable' do
-      before do
-        variant.stock_items.first.update_attribute(
-          :backorderable, false
-        )
-      end
+    context 'when the variant has no stock item' do
+      let(:variant) { build(:variant) }
 
-      it 'returns false' do
-        expect(variant.on_demand).to be_falsy
+      it 'returns stock location default' do
+        expect(variant.on_demand).to be_truthy
       end
     end
   end