From 4b440c83a45ef5d6ff1cd18902ab5206c92b9b84 Mon Sep 17 00:00:00 2001 From: Rob Harrington Date: Mon, 12 Dec 2016 10:09:24 +1100 Subject: [PATCH] WIP: ProxyOrders don't require an order, StandingOrderForm changes --- app/forms/standing_order_form.rb | 6 +- app/models/proxy_order.rb | 5 +- spec/factories.rb | 13 ++-- spec/forms/standing_order_form_spec.rb | 93 ++++++++++++++------------ spec/models/proxy_order_spec.rb | 18 +++++ 5 files changed, 80 insertions(+), 55 deletions(-) diff --git a/app/forms/standing_order_form.rb b/app/forms/standing_order_form.rb index ac8657af4e..a28872aed5 100644 --- a/app/forms/standing_order_form.rb +++ b/app/forms/standing_order_form.rb @@ -47,7 +47,7 @@ class StandingOrderForm future_and_undated_orders.each(&:save) - standing_order.save + raise ActiveRecord::Rollback unless standing_order.save end end @@ -83,9 +83,7 @@ class StandingOrderForm def initialise_proxy_orders! uninitialised_order_cycle_ids.each do |order_cycle_id| - proxy_order = proxy_orders.build(standing_order: standing_order, order_cycle_id: order_cycle_id) - proxy_order.initialise_order! - proxy_order.save! unless standing_order.new_record? + proxy_orders << ProxyOrder.new(standing_order: standing_order, order_cycle_id: order_cycle_id) end end diff --git a/app/models/proxy_order.rb b/app/models/proxy_order.rb index c6aef6d50b..09132b8a38 100644 --- a/app/models/proxy_order.rb +++ b/app/models/proxy_order.rb @@ -3,7 +3,7 @@ class ProxyOrder < ActiveRecord::Base belongs_to :standing_order belongs_to :order_cycle - delegate :number, :completed_at, :total, to: :order + delegate :number, :completed_at, :total, to: :order, allow_nil: true scope :closed, -> { joins(order: :order_cycle).merge(OrderCycle.closed) } scope :not_closed, -> { joins(order: :order_cycle).merge(OrderCycle.not_closed) } @@ -51,6 +51,7 @@ class ProxyOrder < ActiveRecord::Base order.update_distribution_charge! order.payments.create(payment_method_id: standing_order.payment_method_id, amount: order.reload.total) - order.save! + save! + order end end diff --git a/spec/factories.rb b/spec/factories.rb index 20a36f5102..c5ad8a25e6 100644 --- a/spec/factories.rb +++ b/spec/factories.rb @@ -139,8 +139,8 @@ FactoryGirl.define do end factory :standing_order, :class => StandingOrder do - shop { FactoryGirl.create :enterprise } - schedule { FactoryGirl.create(:schedule, order_cycles: [create(:simple_order_cycle, coordinator: shop)]) } + shop { create :enterprise } + schedule { create(:schedule, order_cycles: [create(:simple_order_cycle, coordinator: shop)]) } customer { create(:customer, enterprise: shop) } bill_address { create(:address) } ship_address { create(:address) } @@ -150,7 +150,7 @@ FactoryGirl.define do ignore do with_items false - with_orders false + with_proxy_orders false end after(:create) do |standing_order, proxy| @@ -163,7 +163,7 @@ FactoryGirl.define do end end - if proxy.with_orders + if proxy.with_proxy_orders standing_order.order_cycles.each do |oc| standing_order.proxy_orders << create(:proxy_order, standing_order: standing_order, order_cycle: oc) end @@ -181,8 +181,9 @@ FactoryGirl.define do standing_order order_cycle { standing_order.order_cycles.first } before(:create) do |proxy_order, proxy| - proxy_order.initialise_order! unless proxy_order.order - proxy_order.order.update_attribute(:order_cycle_id, proxy_order.order_cycle_id) + if proxy_order.order + proxy_order.order.update_attribute(:order_cycle_id, proxy_order.order_cycle_id) + end end end diff --git a/spec/forms/standing_order_form_spec.rb b/spec/forms/standing_order_form_spec.rb index e1c3b3af26..1546e5da16 100644 --- a/spec/forms/standing_order_form_spec.rb +++ b/spec/forms/standing_order_form_spec.rb @@ -46,16 +46,16 @@ describe StandingOrderForm do Spree::Config.set allow_backorders: false form.save - expect(standing_order.orders.count).to be 2 + expect(standing_order.proxy_orders.count).to be 2 # This order cycle has already closed, so no order is initialized - order1 = standing_order.orders.find_by_order_cycle_id(order_cycle1.id) - expect(order1).to be nil + proxy_order1 = standing_order.proxy_orders.find_by_order_cycle_id(order_cycle1.id) + expect(proxy_order1).to be nil # Currently open order cycle, closing after begins_at and before ends_at - # Note: Quantity for variant3 is 3, despite available stock being 1 - order2 = standing_order.orders.find_by_order_cycle_id(order_cycle2.id) - expect(order2).to be_a Spree::Order + proxy_order2 = standing_order.proxy_orders.find_by_order_cycle_id(order_cycle2.id) + expect(proxy_order2).to be_a ProxyOrder + order2 = proxy_order2.initialise_order! expect(order2.line_items.count).to be 3 expect(order2.line_items.find_by_variant_id(variant3.id).quantity).to be 3 expect(order2.shipments.count).to be 1 @@ -68,8 +68,9 @@ describe StandingOrderForm do # Future order cycle, closing after begins_at and before ends_at # Adds line items for variants that aren't yet available from the order cycle - # Note: Quantity for variant3 is 3, despite available stock being 1 - order3 = standing_order.orders.find_by_order_cycle_id(order_cycle3.id) + proxy_order3 = standing_order.proxy_orders.find_by_order_cycle_id(order_cycle3.id) + expect(proxy_order3).to be_a ProxyOrder + order3 = proxy_order3.initialise_order! expect(order3).to be_a Spree::Order expect(order3.line_items.count).to be 3 expect(order2.line_items.find_by_variant_id(variant3.id).quantity).to be 3 @@ -82,35 +83,35 @@ describe StandingOrderForm do expect(order3.completed?).to be false # Future order cycle closing after ends_at - order4 = standing_order.orders.find_by_order_cycle_id(order_cycle4.id) - expect(order4).to be nil + proxy_order4 = standing_order.proxy_orders.find_by_order_cycle_id(order_cycle4.id) + expect(proxy_order4).to be nil end end describe "making a change that causes an error" do - let(:standing_order) { create(:standing_order, with_items: true, with_orders: true) } - let(:shipping_method) { standing_order.shipping_method } - let(:invalid_shipping_method) { create(:shipping_method, distributors: [create(:enterprise)]) } - let(:order) { standing_order.orders.first } - let(:params) { { shipping_method_id: invalid_shipping_method.id } } - let(:form) { StandingOrderForm.new(standing_order, params) } + let!(:standing_order) { create(:standing_order, with_items: true, with_proxy_orders: true) } + let!(:order) { standing_order.proxy_orders.first.initialise_order! } + let!(:shipping_method) { standing_order.shipping_method } + let!(:invalid_shipping_method) { create(:shipping_method, distributors: [create(:enterprise)]) } + let!(:params) { { shipping_method_id: invalid_shipping_method.id } } + let!(:form) { StandingOrderForm.new(standing_order, params) } before do form.save end it "does not update standing_order or associated orders" do - expect(order.shipping_method).to eq shipping_method + expect(order.reload.shipping_method).to eq shipping_method expect(order.shipments.first.shipping_method).to eq shipping_method expect(form.json_errors.keys).to eq [:shipping_method] end end describe "changing the shipping method" do - let(:standing_order) { create(:standing_order, with_items: true, with_orders: true) } + let(:standing_order) { create(:standing_order, with_items: true, with_proxy_orders: true) } + let(:order) { standing_order.proxy_orders.first.initialise_order! } let(:shipping_method) { standing_order.shipping_method } let(:new_shipping_method) { create(:shipping_method, distributors: [standing_order.shop]) } - let(:order) { standing_order.orders.first } let(:params) { { shipping_method_id: new_shipping_method.id } } let(:form) { StandingOrderForm.new(standing_order, params) } @@ -143,8 +144,8 @@ describe StandingOrderForm do end describe "changing the payment method" do - let(:standing_order) { create(:standing_order, with_items: true, with_orders: true) } - let(:order) { standing_order.orders.first } + let(:standing_order) { create(:standing_order, with_items: true, with_proxy_orders: true) } + let(:order) { standing_order.proxy_orders.first.initialise_order! } let(:payment_method) { standing_order.payment_method } let(:new_payment_method) { create(:payment_method, distributors: [standing_order.shop]) } let(:params) { { payment_method_id: new_payment_method.id } } @@ -180,22 +181,28 @@ describe StandingOrderForm do end describe "changing begins_at" do - let(:standing_order) { create(:standing_order, begins_at: Time.zone.now, with_items: true, with_orders: true) } + let(:standing_order) { create(:standing_order, begins_at: Time.zone.now, with_items: true, with_proxy_orders: true) } let(:params) { { begins_at: 1.year.from_now, ends_at: 2.years.from_now } } let(:form) { StandingOrderForm.new(standing_order, params) } - it "removes orders outside the newly specified date range" do + before { standing_order.proxy_orders.each(&:initialise_order!) } + + it "removes orders outside the newly specified date range, recreates proxy orders" do + expect(standing_order.reload.proxy_orders.count).to be 1 expect(standing_order.reload.orders.count).to be 1 form.save + expect(standing_order.reload.proxy_orders.count).to be 0 expect(standing_order.reload.orders.count).to be 0 form.params = { begins_at: 1.month.ago } form.save - expect(standing_order.reload.orders.count).to be 1 + expect(standing_order.reload.proxy_orders.count).to be 1 + expect(standing_order.reload.orders.count).to be 0 end end describe "changing the quantity of a line item" do - let(:standing_order) { create(:standing_order, with_items: true, with_orders: true) } + let(:standing_order) { create(:standing_order, with_items: true, with_proxy_orders: true) } + let(:order) { standing_order.proxy_orders.first.initialise_order! } let(:sli) { standing_order.standing_line_items.first } let(:variant) { sli.variant } @@ -206,11 +213,11 @@ describe StandingOrderForm do let(:form) { StandingOrderForm.new(standing_order, params) } it "updates the line_item quantities and totals on all orders" do - expect(standing_order.orders.first.reload.total.to_f).to eq 59.97 + expect(order.reload.total.to_f).to eq 59.97 form.save line_items = Spree::LineItem.where(order_id: standing_order.orders, variant_id: sli.variant_id) expect(line_items.map(&:quantity)).to eq [2] - expect(standing_order.orders.first.reload.total.to_f).to eq 79.96 + expect(order.reload.total.to_f).to eq 79.96 end end @@ -219,48 +226,48 @@ describe StandingOrderForm do let(:form) { StandingOrderForm.new(standing_order, params) } it "updates the line_item quantities and totals on all orders" do - expect(standing_order.orders.first.reload.total.to_f).to eq 59.97 + expect(order.reload.total.to_f).to eq 59.97 form.save line_items = Spree::LineItem.where(order_id: standing_order.orders, variant_id: sli.variant_id) expect(line_items.map(&:quantity)).to eq [3] - expect(standing_order.orders.first.reload.total.to_f).to eq 99.95 + expect(order.reload.total.to_f).to eq 99.95 end end end describe "adding a new line item" do - let!(:standing_order) { create(:standing_order, with_items: true, with_orders: true) } - let!(:variant) { create(:variant) } - let!(:order_cycle) { standing_order.schedule.order_cycles.first } - let!(:params) { { standing_line_items_attributes: [ { id: nil, variant_id: variant.id, quantity: 1} ] } } - let!(:form) { StandingOrderForm.new(standing_order, params) } - - before do - order_cycle.variants << variant - end + let(:variant) { create(:variant) } + let(:shop) { create(:enterprise) } + let(:order_cycle) { create(:simple_order_cycle, variants: [variant], coordinator: shop, distributors: [shop]) } + let(:schedule) { create(:schedule, order_cycles: [order_cycle] )} + let(:standing_order) { create(:standing_order, schedule: schedule, shop: shop, with_items: true, with_proxy_orders: true) } + let(:order) { standing_order.proxy_orders.first.initialise_order! } + let(:params) { { standing_line_items_attributes: [ { id: nil, variant_id: variant.id, quantity: 1} ] } } + let(:form) { StandingOrderForm.new(standing_order, params) } it "add the line item and updates the total on all orders" do - expect(standing_order.orders.first.reload.total.to_f).to eq 59.97 + expect(order.reload.total.to_f).to eq 59.97 form.save line_items = Spree::LineItem.where(order_id: standing_order.orders, variant_id: variant.id) expect(line_items.map(&:quantity)).to eq [1] - expect(standing_order.orders.first.reload.total.to_f).to eq 79.96 + expect(order.reload.total.to_f).to eq 79.96 end end describe "removing an existing line item" do - let(:standing_order) { create(:standing_order, with_items: true, with_orders: true) } + let(:standing_order) { create(:standing_order, with_items: true, with_proxy_orders: true) } + let(:order) { standing_order.proxy_orders.first.initialise_order! } let(:sli) { standing_order.standing_line_items.first } let(:variant) { sli.variant} let(:params) { { standing_line_items_attributes: [ { id: sli.id, _destroy: true } ] } } let(:form) { StandingOrderForm.new(standing_order, params) } it "removes the line item and updates totals on all orders" do - expect(standing_order.orders.first.reload.total.to_f).to eq 59.97 + expect(order.reload.total.to_f).to eq 59.97 form.save line_items = Spree::LineItem.where(order_id: standing_order.orders, variant_id: variant.id) expect(line_items.count).to be 0 - expect(standing_order.orders.first.reload.total.to_f).to eq 39.98 + expect(order.reload.total.to_f).to eq 39.98 end end diff --git a/spec/models/proxy_order_spec.rb b/spec/models/proxy_order_spec.rb index aaebaf981a..3eaf679753 100644 --- a/spec/models/proxy_order_spec.rb +++ b/spec/models/proxy_order_spec.rb @@ -109,6 +109,7 @@ describe ProxyOrder, type: :model do it "builds a new order based the standing order" do expect{ proxy_order.initialise_order! }.to change{Spree::Order.count}.by(1) + expect(proxy_order.reload.order).to be_a Spree::Order order = proxy_order.order expect(order.line_items.count).to eq standing_order.standing_line_items.count expect(order.distributor).to eq standing_order.shop @@ -120,5 +121,22 @@ describe ProxyOrder, type: :model do expect(order.ship_address).to eq standing_order.ship_address expect(order.complete?).to be false end + + context "when a requested quantity is greater than available stock" do + let(:sli) { standing_order.standing_line_items.first } + let(:variant) { sli.variant } + + before do + variant.update_attribute(:count_on_hand, 2) + sli.update_attribute(:quantity, 5) + end + + it "initialises the order with the requested quantity regardless" do + expect{ proxy_order.initialise_order! }.to change{Spree::Order.count}.by(1) + expect(proxy_order.reload.order).to be_a Spree::Order + order = proxy_order.order + expect(order.line_items.find_by_variant_id(variant.id).quantity).to eq 5 + end + end end end