From a26266159cefcdc55c2e1bff7d5498a9d88c069d Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Fri, 22 Apr 2016 10:47:20 +1000 Subject: [PATCH] Fix timing issue: change in client-side value during server update --- .../darkswarm/services/cart.js.coffee | 20 ++++----- .../spree/orders_controller_decorator.rb | 26 ++++++++++-- app/models/spree/order_populator_decorator.rb | 6 ++- .../spree/orders_controller_spec.rb | 30 ++++++++++++-- .../darkswarm/services/cart_spec.js.coffee | 41 +++++++++++++++++-- 5 files changed, 100 insertions(+), 23 deletions(-) diff --git a/app/assets/javascripts/darkswarm/services/cart.js.coffee b/app/assets/javascripts/darkswarm/services/cart.js.coffee index 4f808e0710..d75ef7a628 100644 --- a/app/assets/javascripts/darkswarm/services/cart.js.coffee +++ b/app/assets/javascripts/darkswarm/services/cart.js.coffee @@ -28,6 +28,7 @@ Darkswarm.factory 'Cart', (CurrentOrder, Variants, $timeout, $http, $modal, $roo update: => @update_running = true + $http.post('/orders/populate', @data()).success (data, status)=> @saved() @update_running = false @@ -48,19 +49,14 @@ Darkswarm.factory 'Cart', (CurrentOrder, Variants, $timeout, $http, $modal, $roo # is unnecessary. for li in @line_items_present() - if !stockLevels[li.variant.id]? - li.quantity = 0 - li.max_quantity = 0 if li.max_quantity? - li.variant.count_on_hand = 0 - scope.variants.push li.variant - else - if stockLevels[li.variant.id].quantity < li.quantity - li.quantity = stockLevels[li.variant.id].quantity - scope.variants.push li.variant - if stockLevels[li.variant.id].max_quantity < li.max_quantity - li.max_quantity = stockLevels[li.variant.id].max_quantity - scope.variants.push li.variant + if stockLevels[li.variant.id]? li.variant.count_on_hand = stockLevels[li.variant.id].on_hand + if li.quantity > li.variant.count_on_hand + li.quantity = li.variant.count_on_hand + scope.variants.push li.variant + if li.max_quantity > li.variant.count_on_hand + li.max_quantity = li.variant.count_on_hand + scope.variants.push(li.variant) unless li.variant in scope.variants if scope.variants.length > 0 $modal.open(templateUrl: "out_of_stock.html", scope: scope, windowClass: 'out-of-stock-modal') diff --git a/app/controllers/spree/orders_controller_decorator.rb b/app/controllers/spree/orders_controller_decorator.rb index f9c7f53001..e675ce7e94 100644 --- a/app/controllers/spree/orders_controller_decorator.rb +++ b/app/controllers/spree/orders_controller_decorator.rb @@ -44,7 +44,10 @@ Spree::OrdersController.class_eval do current_order.cap_quantity_at_stock! current_order.update! - render json: {error: false, stock_levels: stock_levels}, status: 200 + variant_ids = variant_ids_in(populator.variants_h) + + render json: {error: false, stock_levels: stock_levels(current_order, variant_ids)}, + status: 200 else render json: {error: true}, status: 412 @@ -52,9 +55,26 @@ Spree::OrdersController.class_eval do end end - def stock_levels + # Report the stock levels in the order for all variant ids requested + def stock_levels(order, variant_ids) + stock_levels = li_stock_levels(order) + + li_variant_ids = stock_levels.keys + (variant_ids - li_variant_ids).each do |variant_id| + stock_levels[variant_id] = {quantity: 0, max_quantity: 0, + on_hand: Spree::Variant.find(variant_id).on_hand} + end + + stock_levels + end + + def variant_ids_in(variants_h) + variants_h.map { |v| v[:variant_id].to_i } + end + + def li_stock_levels(order) Hash[ - current_order.line_items.map do |li| + order.line_items.map do |li| [li.variant.id, {quantity: li.quantity, max_quantity: li.max_quantity, diff --git a/app/models/spree/order_populator_decorator.rb b/app/models/spree/order_populator_decorator.rb index 38c908a530..a106de6936 100644 --- a/app/models/spree/order_populator_decorator.rb +++ b/app/models/spree/order_populator_decorator.rb @@ -1,6 +1,8 @@ require 'open_food_network/scope_variant_to_hub' Spree::OrderPopulator.class_eval do + attr_reader :variants_h + def populate(from_hash, overwrite = false) @distributor, @order_cycle = distributor_and_order_cycle # Refactor: We may not need this validation - we can't change distribution here, so @@ -31,8 +33,8 @@ Spree::OrderPopulator.class_eval do end def read_variants(data) - read_products_hash(data) + - read_variants_hash(data) + @variants_h = read_products_hash(data) + + read_variants_hash(data) end def read_products_hash(data) diff --git a/spec/controllers/spree/orders_controller_spec.rb b/spec/controllers/spree/orders_controller_spec.rb index c0a844a4bc..286bb9217a 100644 --- a/spec/controllers/spree/orders_controller_spec.rb +++ b/spec/controllers/spree/orders_controller_spec.rb @@ -67,9 +67,11 @@ describe Spree::OrdersController do let(:product) { create(:simple_product) } it "returns stock levels as JSON" do + controller.stub(:variant_ids_in) { [123] } controller.stub(:stock_levels) { 'my_stock_levels' } Spree::OrderPopulator.stub(:new).and_return(populator = double()) populator.stub(:populate) { true } + populator.stub(:variants_h) { {} } xhr :post, :populate, use_route: :spree, format: :json @@ -81,6 +83,7 @@ describe Spree::OrdersController do let!(:order) { create(:order) } let!(:li) { create(:line_item, order: order, variant: v, quantity: 2, max_quantity: 3) } let!(:v) { create(:variant, count_on_hand: 4) } + let!(:v2) { create(:variant, count_on_hand: 2) } before do order.reload @@ -88,17 +91,37 @@ describe Spree::OrdersController do end it "returns a hash with variant id, quantity, max_quantity and stock on hand" do - controller.stock_levels.should == {v.id => {quantity: 2, max_quantity: 3, on_hand: 4}} + controller.stock_levels(order, [v.id]).should == + {v.id => {quantity: 2, max_quantity: 3, on_hand: 4}} + end + + it "includes all line items, even when the variant_id is not specified" do + controller.stock_levels(order, []).should == + {v.id => {quantity: 2, max_quantity: 3, on_hand: 4}} + end + + it "includes an empty quantity entry for variants that aren't in the order" do + controller.stock_levels(order, [v.id, v2.id]).should == + {v.id => {quantity: 2, max_quantity: 3, on_hand: 4}, + v2.id => {quantity: 0, max_quantity: 0, on_hand: 2}} end describe "encoding Infinity" do let!(:v) { create(:variant, on_demand: true, count_on_hand: 0) } it "encodes Infinity as a large, finite integer" do - controller.stock_levels.should == {v.id => {quantity: 2, max_quantity: 3, on_hand: 2147483647}} + controller.stock_levels(order, [v.id]).should == + {v.id => {quantity: 2, max_quantity: 3, on_hand: 2147483647}} end end end + + it "extracts variant ids from the populator" do + variants_h = [{:variant_id=>"900", :quantity=>2, :max_quantity=>nil}, + {:variant_id=>"940", :quantity=>3, :max_quantity=>3}] + + controller.variant_ids_in(variants_h).should == [900, 940] + end end context "adding a group buy product to the cart" do @@ -118,7 +141,8 @@ describe Spree::OrdersController do it "returns HTTP success when successful" do Spree::OrderPopulator.stub(:new).and_return(populator = double()) - populator.stub(:populate).and_return true + populator.stub(:populate) { true } + populator.stub(:variants_h) { {} } xhr :post, :populate, use_route: :spree, format: :json response.status.should == 200 end diff --git a/spec/javascripts/unit/darkswarm/services/cart_spec.js.coffee b/spec/javascripts/unit/darkswarm/services/cart_spec.js.coffee index 58b7795124..6df74e3ede 100644 --- a/spec/javascripts/unit/darkswarm/services/cart_spec.js.coffee +++ b/spec/javascripts/unit/darkswarm/services/cart_spec.js.coffee @@ -130,21 +130,24 @@ describe 'Cart service', -> describe "when an item is out of stock", -> it "reduces the quantity in the cart", -> li = {variant: {id: 1}, quantity: 5} + stockLevels = {1: {quantity: 0, max_quantity: 0, on_hand: 0}} spyOn(Cart, 'line_items_present').andReturn [li] - Cart.compareAndNotifyStockLevels {} + Cart.compareAndNotifyStockLevels stockLevels expect(li.quantity).toEqual 0 expect(li.max_quantity).toBeUndefined() it "reduces the max_quantity in the cart", -> li = {variant: {id: 1}, quantity: 5, max_quantity: 6} + stockLevels = {1: {quantity: 0, max_quantity: 0, on_hand: 0}} spyOn(Cart, 'line_items_present').andReturn [li] - Cart.compareAndNotifyStockLevels {} + Cart.compareAndNotifyStockLevels stockLevels expect(li.max_quantity).toEqual 0 it "resets the count on hand available", -> li = {variant: {id: 1, count_on_hand: 10}, quantity: 5} + stockLevels = {1: {quantity: 0, max_quantity: 0, on_hand: 0}} spyOn(Cart, 'line_items_present').andReturn [li] - Cart.compareAndNotifyStockLevels {} + Cart.compareAndNotifyStockLevels stockLevels expect(li.variant.count_on_hand).toEqual 0 describe "when the quantity available is less than that requested", -> @@ -170,6 +173,38 @@ describe 'Cart service', -> Cart.compareAndNotifyStockLevels stockLevels expect(li.variant.count_on_hand).toEqual 6 + describe "when the client-side quantity has been increased during the request", -> + it "does not reset the quantity", -> + li = {variant: {id: 1}, quantity: 6} + stockLevels = {1: {quantity: 5, on_hand: 6}} + spyOn(Cart, 'line_items_present').andReturn [li] + Cart.compareAndNotifyStockLevels stockLevels + expect(li.quantity).toEqual 6 + expect(li.max_quantity).toBeUndefined() + + it "does not reset the max_quantity", -> + li = {variant: {id: 1}, quantity: 5, max_quantity: 7} + stockLevels = {1: {quantity: 5, max_quantity: 6, on_hand: 7}} + spyOn(Cart, 'line_items_present').andReturn [li] + Cart.compareAndNotifyStockLevels stockLevels + expect(li.quantity).toEqual 5 + expect(li.max_quantity).toEqual 7 + + describe "when the client-side quantity has been changed from 0 to 1 during the request", -> + it "does not reset the quantity", -> + li = {variant: {id: 1}, quantity: 1} + spyOn(Cart, 'line_items_present').andReturn [li] + Cart.compareAndNotifyStockLevels {} + expect(li.quantity).toEqual 1 + expect(li.max_quantity).toBeUndefined() + + it "does not reset the max_quantity", -> + li = {variant: {id: 1}, quantity: 1, max_quantity: 1} + spyOn(Cart, 'line_items_present').andReturn [li] + Cart.compareAndNotifyStockLevels {} + expect(li.quantity).toEqual 1 + expect(li.max_quantity).toEqual 1 + it "pops the queue", -> Cart.update_enqueued = true spyOn(Cart, 'scheduleUpdate')