From cd033c300e472a2e4b6dbafb59de801c78446474 Mon Sep 17 00:00:00 2001 From: Will Marshall Date: Wed, 16 Jul 2014 16:00:28 +1000 Subject: [PATCH] Reworking order populator and controller to handle replacement of line items rather than addition --- .../spree/orders_controller_decorator.rb | 15 ++++--------- app/models/spree/order_populator_decorator.rb | 18 +++++++++++----- .../spree/orders_controller_spec.rb | 20 ++++++++++++++++++ spec/models/spree/order_populator_spec.rb | 21 +++++++++++++++---- 4 files changed, 54 insertions(+), 20 deletions(-) diff --git a/app/controllers/spree/orders_controller_decorator.rb b/app/controllers/spree/orders_controller_decorator.rb index 959e51204f..adfa4cd2ad 100644 --- a/app/controllers/spree/orders_controller_decorator.rb +++ b/app/controllers/spree/orders_controller_decorator.rb @@ -21,22 +21,15 @@ Spree::OrdersController.class_eval do end end - # Patch Orders#populate to populate multi_cart (if enabled) + # Patch populate to be Ajax def populate - if OpenFoodNetwork::FeatureToggle.enabled? :multi_cart - populate_cart params.slice(:products, :variants, :quantity, :distributor_id, :order_cycle_id) - end populator = Spree::OrderPopulator.new(current_order(true), current_currency) - if populator.populate(params.slice(:products, :variants, :quantity)) - + if populator.populate(params.slice(:products, :variants, :quantity), true) fire_event('spree.cart.add') fire_event('spree.order.contents_changed') - respond_with(@order) do |format| - format.html { redirect_to cart_path } - end + render json: true, status: 200 else - flash[:error] = populator.errors.full_messages.join(" ") - redirect_to :back + render json: false, status: 402 end end diff --git a/app/models/spree/order_populator_decorator.rb b/app/models/spree/order_populator_decorator.rb index aad019e06f..d7d4dc24bf 100644 --- a/app/models/spree/order_populator_decorator.rb +++ b/app/models/spree/order_populator_decorator.rb @@ -1,19 +1,27 @@ Spree::OrderPopulator.class_eval do - def populate_with_distribution_validation(from_hash) + 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 # this validation probably can't fail if !distribution_can_supply_products_in_cart(@distributor, @order_cycle) errors.add(:base, "That distributor or order cycle can't supply all the products in your cart. Please choose another.") end - populate_without_distribution_validation(from_hash) if valid? + if valid? + @order.with_lock do + @order.empty! if overwrite + from_hash[:products].each do |product_id,variant_id| + attempt_cart_add(variant_id, from_hash[:quantity]) + end if from_hash[:products] + + from_hash[:variants].each do |variant_id, quantity| + attempt_cart_add(variant_id, quantity) + end if from_hash[:variants] + end + end valid? end - alias_method_chain :populate, :distribution_validation - # Copied from Spree::OrderPopulator, with additional validations added def attempt_cart_add(variant_id, quantity) diff --git a/spec/controllers/spree/orders_controller_spec.rb b/spec/controllers/spree/orders_controller_spec.rb index e6c9b91894..354ebb8701 100644 --- a/spec/controllers/spree/orders_controller_spec.rb +++ b/spec/controllers/spree/orders_controller_spec.rb @@ -66,6 +66,26 @@ describe Spree::OrdersController do spree_post :populate, :variants => {p.master.id => 1}, :variant_attributes => {p.master.id => {:max_quantity => 3}} end.to change(Spree::LineItem, :count).by(1) end + + it "returns HTTP success when successful" do + Spree::OrderPopulator.stub(:new).and_return(populator = mock()) + populator.stub(:populate).and_return true + xhr :post, :populate, use_route: :spree, format: :json + response.status.should == 200 + end + + it "returns failure when unsuccessful" do + Spree::OrderPopulator.stub(:new).and_return(populator = mock()) + populator.stub(:populate).and_return false + xhr :post, :populate, use_route: :spree, format: :json + response.status.should == 402 + end + + it "tells populator to overwrite" do + Spree::OrderPopulator.stub(:new).and_return(populator = mock()) + populator.should_receive(:populate).with({}, true) + xhr :post, :populate, use_route: :spree, format: :json + end end context "removing line items from cart" do diff --git a/spec/models/spree/order_populator_spec.rb b/spec/models/spree/order_populator_spec.rb index 1e983872bb..a18fd952f0 100644 --- a/spec/models/spree/order_populator_spec.rb +++ b/spec/models/spree/order_populator_spec.rb @@ -4,23 +4,36 @@ module Spree describe OrderPopulator do let(:order) { double(:order, id: 123) } let(:currency) { double(:currency) } - let(:params) { double(:params) } + let(:params) { {} } let(:distributor) { double(:distributor) } let(:order_cycle) { double(:order_cycle) } let(:op) { OrderPopulator.new(order, currency) } describe "populate" do - - it "checks that distribution can supply all products in the cart" do + before do op.should_receive(:distributor_and_order_cycle). and_return([distributor, order_cycle]) + end + it "checks that distribution can supply all products in the cart" do op.should_receive(:distribution_can_supply_products_in_cart). with(distributor, order_cycle).and_return(false) - op.should_receive(:populate_without_distribution_validation).never op.populate(params).should be_false op.errors.to_a.should == ["That distributor or order cycle can't supply all the products in your cart. Please choose another."] end + + it "empties the order if override is true" do + op.stub(:distribution_can_supply_products_in_cart).and_return true + order.stub(:with_lock).and_yield + order.should_receive(:empty!) + op.populate(params, true) + end + + it "locks the order" do + op.stub(:distribution_can_supply_products_in_cart).and_return(true) + order.should_receive(:with_lock) + op.populate(params, true) + end end describe "attempt_cart_add" do