Merge pull request #3904 from luisramos0/subs_stock

Fix ofn-on-hand directive and fix usage of variant.on_hand in subscriptions order factory

app/assets/javascripts/darkswarm/directives/on_hand.js.coffee

@@ -13,11 +13,16 @@ Darkswarm.directive "ofnOnHand", ->
       ngModel.$setDirty = setDirty
 
     ngModel.$parsers.push (viewValue) ->
-      on_hand = parseInt(attr.ofnOnHand)
-      if parseInt(viewValue) > on_hand
-        alert t("js.insufficient_stock", {on_hand: on_hand})
-        viewValue = on_hand
+      available_quantity = scope.available_quantity()
+      if parseInt(viewValue) > available_quantity
+        alert t("js.insufficient_stock", {on_hand: available_quantity})
+        viewValue = available_quantity
         ngModel.$setViewValue viewValue
         ngModel.$render()
 
       viewValue
+
+    scope.available_quantity = ->
+      on_hand = parseInt(attr.ofnOnHand)
+      finalized_quantity = parseInt(attr.finalizedquantity) || 0 # finalizedquantity is optional
+      on_hand + finalized_quantity

app/services/order_factory.rb

@@ -48,7 +48,7 @@ class OrderFactory
     attrs[:line_items].each do |li|
       next unless variant = Spree::Variant.find_by_id(li[:variant_id])
       scoper.scope(variant)
-      li[:quantity] = stock_limited_quantity(variant.on_hand, li[:quantity])
+      li[:quantity] = stock_limited_quantity(variant.on_demand, variant.on_hand, li[:quantity])
       li[:price] = variant.price
       build_item_from(li)
     end
@@ -81,9 +81,9 @@ class OrderFactory
     @order.payments.create(payment_method_id: attrs[:payment_method_id], amount: @order.reload.total)
   end
 
-  def stock_limited_quantity(stock, requested)
-    return requested if opts[:skip_stock_check]
-    [stock, requested].min
+  def stock_limited_quantity(variant_on_demand, variant_on_hand, requested)
+    return requested if opts[:skip_stock_check] || variant_on_demand
+    [variant_on_hand, requested].min
   end
 
   def scoper

app/views/spree/orders/_line_item.html.haml

@@ -22,7 +22,8 @@
   %td.text-right.cart-item-price{"data-hook" => "cart_item_price"}
     = line_item.single_display_amount_with_adjustments.to_html
   %td.text-center.cart-item-quantity{"data-hook" => "cart_item_quantity"}
-    = item_form.number_field :quantity, :min => 0, "ofn-on-hand" => "#{variant.on_demand && 9999 || variant.on_hand}", "ng-model" => "line_item_#{line_item.id}", :class => "line_item_quantity", :size => 5
+    - finalized_quantity = @order.completed? ? line_item.quantity : 0
+    = item_form.number_field :quantity, :min => 0, "ofn-on-hand" => "#{variant.on_demand && 9999 || variant.on_hand}", "finalizedquantity" => finalized_quantity, "ng-model" => "line_item_#{line_item.id}", :class => "line_item_quantity", :size => 5
   %td.cart-item-total.text-right{"data-hook" => "cart_item_total"}
     = line_item.display_amount_with_adjustments.to_html unless line_item.quantity.nil?
 

spec/features/consumer/shopping/orders_spec.rb

@@ -146,7 +146,10 @@ feature "Order Management", js: true do
         within "tr.variant-#{item1.variant.id}" do
           expect(page).to have_content item1.product.name
           expect(page).to have_field 'order_line_items_attributes_0_quantity'
-          fill_in 'order_line_items_attributes_0_quantity', with: 2
+          # The original item quantity is 1, there are 4 more items available in stock
+          # By changing quantity to 5 we validate the case where the original stock in the order
+          #   must be taken into account to fullfil the order (no insufficient stock error)
+          fill_in 'order_line_items_attributes_0_quantity', with: 5
         end
 
         expect(page).to have_button I18n.t(:save_changes)
@@ -158,15 +161,15 @@ feature "Order Management", js: true do
 
         click_button I18n.t(:save_changes)
 
-        expect(find(".order-total.grand-total")).to have_content "85.00"
-        expect(item1.reload.quantity).to eq 2
+        expect(find(".order-total.grand-total")).to have_content "115.00"
+        expect(item1.reload.quantity).to eq 5
 
         # Deleting an item
         within "tr.variant-#{item2.variant.id}" do
           click_link "delete_line_item_#{item2.id}"
         end
 
-        expect(find(".order-total.grand-total")).to have_content "75.00"
+        expect(find(".order-total.grand-total")).to have_content "105.00"
         expect(Spree::LineItem.find_by_id(item2.id)).to be nil
 
         # Cancelling the order

spec/services/order_factory_spec.rb

@@ -31,9 +31,8 @@ describe OrderFactory do
       attrs
     end
 
-    it "builds a new order based the provided attributes" do
-      expect{ order }.to change{ Spree::Order.count }.by(1)
-      expect(order).to be_a Spree::Order
+    it "builds a new order based on the provided attributes" do
+      expect_new_order
       expect(order.line_items.count).to eq 2
       expect(order.customer).to eq customer
       expect(order.user).to eq user
@@ -64,8 +63,7 @@ describe OrderFactory do
       before { customer.update_attribute(:user_id, nil) }
 
       it "initialises the order without a user_id" do
-        expect{ order }.to change{ Spree::Order.count }.by(1)
-        expect(order).to be_a Spree::Order
+        expect_new_order
         expect(order.user).to be nil
       end
     end
@@ -78,10 +76,18 @@ describe OrderFactory do
         end
 
         context "when skip_stock_check is not requested" do
-          it "initialised the order but limits stock to the available amount" do
-            expect{ order }.to change{ Spree::Order.count }.by(1)
-            expect(order).to be_a Spree::Order
-            expect(order.line_items.find_by_variant_id(variant1.id).quantity).to eq 2
+          it "initialises the order but limits stock to the available amount" do
+            expect_new_order
+            expect(variant1_line_item.quantity).to eq 2
+          end
+
+          context "when variant is on_demand" do
+            before { variant1.update_attribute(:on_demand, true) }
+
+            it "initialises the order with the requested quantity regardless of stock" do
+              expect_new_order
+              expect(variant1_line_item.quantity).to eq 5
+            end
           end
         end
 
@@ -89,9 +95,8 @@ describe OrderFactory do
           let(:opts) { { skip_stock_check: true } }
 
           it "initialises the order with the requested quantity regardless" do
-            expect{ order }.to change{ Spree::Order.count }.by(1)
-            expect(order).to be_a Spree::Order
-            expect(order.line_items.find_by_variant_id(variant1.id).quantity).to eq 5
+            expect_new_order
+            expect(variant1_line_item.quantity).to eq 5
           end
         end
       end
@@ -102,9 +107,8 @@ describe OrderFactory do
 
         context "when skip_stock_check is not requested" do
           it "initialised the order but limits stock to the available amount" do
-            expect{ order }.to change{ Spree::Order.count }.by(1)
-            expect(order).to be_a Spree::Order
-            expect(order.line_items.find_by_variant_id(variant1.id).quantity).to eq 3
+            expect_new_order
+            expect(variant1_line_item.quantity).to eq 3
           end
         end
 
@@ -112,9 +116,8 @@ describe OrderFactory do
           let(:opts) { { skip_stock_check: true } }
 
           it "initialises the order with the requested quantity regardless" do
-            expect{ order }.to change{ Spree::Order.count }.by(1)
-            expect(order).to be_a Spree::Order
-            expect(order.line_items.find_by_variant_id(variant1.id).quantity).to eq 6
+            expect_new_order
+            expect(variant1_line_item.quantity).to eq 6
           end
         end
       end
@@ -123,8 +126,8 @@ describe OrderFactory do
     describe "determining the price for line items" do
       context "when no override is present" do
         it "uses the price from the variant" do
-          expect{ order }.to change{ Spree::Order.count }.by(1)
-          expect(order.line_items.find_by_variant_id(variant1.id).price).to eq 5.0
+          expect_new_order
+          expect(variant1_line_item.price).to eq 5.0
           expect(order.total).to eq 38.0
         end
       end
@@ -133,11 +136,20 @@ describe OrderFactory do
         let!(:override) { create(:variant_override, hub_id: shop.id, variant_id: variant1.id, price: 3.0) }
 
         it "uses the price from the override" do
-          expect{ order }.to change{ Spree::Order.count }.by(1)
-          expect(order.line_items.find_by_variant_id(variant1.id).price).to eq 3.0
+          expect_new_order
+          expect(variant1_line_item.price).to eq 3.0
           expect(order.total).to eq 34.0
         end
       end
     end
+
+    def expect_new_order
+      expect{ order }.to change{ Spree::Order.count }.by(1)
+      expect(order).to be_a Spree::Order
+    end
+
+    def variant1_line_item
+      order.line_items.find_by_variant_id(variant1.id)
+    end
   end
 end