From beb19cdc8a3097305205903f1183702f20055654 Mon Sep 17 00:00:00 2001 From: luisramos0 Date: Thu, 14 Mar 2019 17:37:59 +0000 Subject: [PATCH 1/4] Override default spree splitters config (was Shipping Category and Backordered) to use only Base splitter, this splitter does not split the orders into multiple shipments In OFN we cannot split the orders because one order can only have one shipment Additionally, add spec to validate that the order workflow now works with products with different shipping categories --- config/application.rb | 11 ++++++++ spec/config/application_spec.rb | 9 +++++++ spec/models/spree/order/checkout_spec.rb | 34 +++++++++++++++++++++--- 3 files changed, 50 insertions(+), 4 deletions(-) create mode 100644 spec/config/application_spec.rb diff --git a/config/application.rb b/config/application.rb index de267a8956..b86316c960 100644 --- a/config/application.rb +++ b/config/application.rb @@ -83,6 +83,17 @@ module Openfoodnetwork ] end + # Every splitter (except Base splitter) will split the order in multiple packages + # Each package will generate a separate shipment in the order + # Base splitter does not split the packages + # So, because in OFN we have locked orders to have only one shipment, + # we must use this splitter and no other + initializer "spree.register.stock_splitters" do |app| + app.config.spree.stock_splitters = [ + Spree::Stock::Splitter::Base + ] + end + # Register Spree payment methods initializer "spree.gateway.payment_methods", :after => "spree.register.payment_methods" do |app| app.config.spree.payment_methods << Spree::Gateway::Migs diff --git a/spec/config/application_spec.rb b/spec/config/application_spec.rb new file mode 100644 index 0000000000..126325a4f1 --- /dev/null +++ b/spec/config/application_spec.rb @@ -0,0 +1,9 @@ +require 'spec_helper' + +describe Openfoodnetwork::Application, 'configuration' do + let(:config) { described_class.config } + + it "sets Spree::Stock::Splitter::Base as the only stock splitter" do + expect(config.spree.stock_splitters).to eq [Spree::Stock::Splitter::Base] + end +end diff --git a/spec/models/spree/order/checkout_spec.rb b/spec/models/spree/order/checkout_spec.rb index a78e7bb3a5..c8a1a3b86a 100644 --- a/spec/models/spree/order/checkout_spec.rb +++ b/spec/models/spree/order/checkout_spec.rb @@ -2,9 +2,9 @@ require 'spec_helper' describe Spree::Order do describe 'event :restart_checkout' do - context 'when the order is not complete' do - let(:order) { create(:order) } + let(:order) { create(:order) } + context 'when the order is not complete' do before { allow(order).to receive(:completed?) { false } } it 'does transition to cart state' do @@ -13,8 +13,6 @@ describe Spree::Order do end context 'when the order is complete' do - let(:order) { create(:order) } - before { allow(order).to receive(:completed?) { true } } it 'raises' do @@ -26,4 +24,32 @@ describe Spree::Order do end end end + + describe "order with products with different shipping categories" do + let(:order) { create(:order_with_totals_and_distribution, ship_address: create(:address) ) } + let(:shipping_method) { create(:shipping_method, distributors: [order.distributor]) } + let(:other_shipping_category) { create(:shipping_category) } + let(:other_product) { create(:product, shipping_category: other_shipping_category ) } + let(:other_variant) { other_product.variants.first } + + before do + order.order_cycle = create(:simple_order_cycle, + distributors: [order.distributor], + variants: [order.line_items.first.variant, other_variant]) + order.line_items = [order.line_items.first, create(:line_item, + order: order, + variant: other_variant)] + end + + it "can progress to delivery" do + shipping_method.shipping_categories << other_shipping_category + + # If the shipping category package splitter is enabled, + # an order with products with two shipping categories will be split into two shipments + # and the spec will fail with a unique constraint error on index_spree_shipments_on_order_id + order.next + order.next + expect(order.state).to eq "delivery" + end + end end From 428656e92ecf61d2ce1e9a3e69c82d7be20372fa Mon Sep 17 00:00:00 2001 From: luisramos0 Date: Thu, 14 Mar 2019 21:57:02 +0000 Subject: [PATCH 2/4] Add distributor to shipping method in factory completed_order_with_fees so that shipping method is not discarded during order processing in package.shipping_methods --- spec/factories.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/factories.rb b/spec/factories.rb index f3f730c00c..5c178d0a78 100644 --- a/spec/factories.rb +++ b/spec/factories.rb @@ -427,7 +427,7 @@ FactoryBot.define do payment_method = create(:payment_method, calculator: payment_calculator) create(:payment, order: order, amount: order.total, payment_method: payment_method, state: 'checkout') - create(:shipping_method_with, :shipping_fee, shipping_fee: evaluator.shipping_fee) + create(:shipping_method_with, :shipping_fee, shipping_fee: evaluator.shipping_fee, distributors: [order.distributor]) order.reload while !order.completed? do break unless order.next! end From ae66e864a5e55108eafc6e41a55575a308f7f042 Mon Sep 17 00:00:00 2001 From: luisramos0 Date: Wed, 20 Mar 2019 12:18:51 +0000 Subject: [PATCH 3/4] Improve models/spree/order/checkout_spec: AR is here to help you :-D --- spec/models/spree/order/checkout_spec.rb | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/spec/models/spree/order/checkout_spec.rb b/spec/models/spree/order/checkout_spec.rb index c8a1a3b86a..e2a133ebbc 100644 --- a/spec/models/spree/order/checkout_spec.rb +++ b/spec/models/spree/order/checkout_spec.rb @@ -36,9 +36,7 @@ describe Spree::Order do order.order_cycle = create(:simple_order_cycle, distributors: [order.distributor], variants: [order.line_items.first.variant, other_variant]) - order.line_items = [order.line_items.first, create(:line_item, - order: order, - variant: other_variant)] + order.line_items << create(:line_item, order: order, variant: other_variant) end it "can progress to delivery" do From b780823970e2d4b53c8525d8f8ad78adea4c9bf4 Mon Sep 17 00:00:00 2001 From: luisramos0 Date: Wed, 27 Mar 2019 13:47:04 +0000 Subject: [PATCH 4/4] Fix specs to adapt to the fact that for the order workflow to get to delivery, the shipping method must be linked to the order distributor otherwise it's ignored when creating the order shipment --- spec/factories.rb | 1 + spec/models/proxy_order_spec.rb | 4 +++- spec/services/order_factory_spec.rb | 2 +- 3 files changed, 5 insertions(+), 2 deletions(-) diff --git a/spec/factories.rb b/spec/factories.rb index 5c178d0a78..5f2d2c809f 100644 --- a/spec/factories.rb +++ b/spec/factories.rb @@ -443,6 +443,7 @@ FactoryBot.define do shipment = order.reload.shipments.first if shipment.nil? shipping_method = create(:shipping_method_with, :shipping_fee, shipping_fee: shipping_fee) + shipping_method.distributors << order.distributor if order.distributor shipment = create(:shipment_with, :shipping_method, shipping_method: shipping_method, order: order) end shipment diff --git a/spec/models/proxy_order_spec.rb b/spec/models/proxy_order_spec.rb index d7c0a1ba8d..2e15c39cc6 100644 --- a/spec/models/proxy_order_spec.rb +++ b/spec/models/proxy_order_spec.rb @@ -78,7 +78,9 @@ describe ProxyOrder, type: :model do describe "resume" do let!(:payment_method) { create(:payment_method) } let!(:shipment) { create(:shipment) } - let(:order) { create(:order_with_totals, ship_address: create(:address), shipments: [shipment]) } + let(:order) { create(:order_with_totals, ship_address: create(:address), + shipments: [shipment], + distributor: shipment.shipping_method.distributors.first) } let(:proxy_order) { create(:proxy_order, order: order, canceled_at: Time.zone.now) } let(:order_cycle) { proxy_order.order_cycle } diff --git a/spec/services/order_factory_spec.rb b/spec/services/order_factory_spec.rb index a1b47c0282..7c9c0bd7d5 100644 --- a/spec/services/order_factory_spec.rb +++ b/spec/services/order_factory_spec.rb @@ -8,7 +8,7 @@ describe OrderFactory do let(:shop) { create(:distributor_enterprise) } let(:order_cycle) { create(:simple_order_cycle) } let!(:other_shipping_method_a) { create(:shipping_method) } - let!(:shipping_method) { create(:shipping_method) } + let!(:shipping_method) { create(:shipping_method, distributors: [shop]) } let!(:other_shipping_method_b) { create(:shipping_method) } let(:payment_method) { create(:payment_method) } let(:ship_address) { create(:address) }