From 2ccbb87adcb3174bb5a64b57b6fc9dd4ab2b3f02 Mon Sep 17 00:00:00 2001 From: luisramos0 Date: Thu, 11 Oct 2018 17:03:19 +0100 Subject: [PATCH 1/5] Fix payment decorator bug Payment line items must come from the order, payment does not have inventory_units --- app/models/spree/payment_decorator.rb | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/app/models/spree/payment_decorator.rb b/app/models/spree/payment_decorator.rb index 8aab22a7bb..cd78e2b155 100644 --- a/app/models/spree/payment_decorator.rb +++ b/app/models/spree/payment_decorator.rb @@ -32,11 +32,7 @@ module Spree # This is called by the calculator of a payment method def line_items - if order.complete? && Spree::Config[:track_inventory_levels] - order.line_items.select { |li| inventory_units.pluck(:variant_id).include?(li.variant_id) } - else - order.line_items - end + order.line_items end # Pin payments lacks void and credit methods, but it does have refund From 91f52d80c89f1d82a2fae9fae87468d1e6c8cfc1 Mon Sep 17 00:00:00 2001 From: luisramos0 Date: Thu, 11 Oct 2018 17:06:10 +0100 Subject: [PATCH 2/5] Fix problem in order and adjustment model specs by fixing the order.adjustment association This fix was taken from spree 2.1 here https://github.com/spree/spree/commit/3fa44165c7825f79a2fa4eb79b99dc29944c5d55 --- app/models/spree/order_decorator.rb | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/app/models/spree/order_decorator.rb b/app/models/spree/order_decorator.rb index 948cfc494a..b09408ba67 100644 --- a/app/models/spree/order_decorator.rb +++ b/app/models/spree/order_decorator.rb @@ -17,6 +17,14 @@ Spree::Order.class_eval do has_one :proxy_order has_one :subscription, through: :proxy_order + # This removes "inverse_of: source" which breaks shipment adjustment calculations + # This change is done in Spree 2.1 (see https://github.com/spree/spree/commit/3fa44165c7825f79a2fa4eb79b99dc29944c5d55) + # When OFN gets to Spree 2.1, this can be removed + has_many :adjustments, + as: :adjustable, + dependent: :destroy, + order: "#{Spree::Adjustment.table_name}.created_at ASC" + validates :customer, presence: true, if: :require_customer? validate :products_available_from_new_distribution, :if => lambda { distributor_id_changed? || order_cycle_id_changed? } validate :disallow_guest_order From 74eff8730b578ba5dec4b6d7a064722a58cb45f7 Mon Sep 17 00:00:00 2001 From: luisramos0 Date: Thu, 11 Oct 2018 17:10:01 +0100 Subject: [PATCH 3/5] Fix tests of fees in a completed order in models/spree/order_spec by: - set distributor with taxes on the order - simplify the factory completed_order_with_fees by replacing shipment with shipping_method and ship_address and letting the order workflow handle shipments and inventory units --- spec/factories.rb | 18 +++--------------- spec/models/spree/order_spec.rb | 3 ++- 2 files changed, 5 insertions(+), 16 deletions(-) diff --git a/spec/factories.rb b/spec/factories.rb index e5f785b439..b7129ee5f6 100644 --- a/spec/factories.rb +++ b/spec/factories.rb @@ -382,18 +382,6 @@ FactoryBot.define do shipment.add_shipping_method(evaluator.shipping_method, true) end end - - trait :shipping_fee do - transient do - shipping_fee 3 - end - - after(:create) do |shipment, evaluator| - shipping_method = create(:shipping_method_with, :shipping_fee, shipping_fee: evaluator.shipping_fee) - shipment.shipping_rates.destroy_all - shipment.add_shipping_method(shipping_method, true) - end - end end factory :distributor_enterprise_with_tax, parent: :distributor_enterprise do @@ -407,16 +395,16 @@ FactoryBot.define do payment_fee 5 end - shipments { [ create(:shipment_with, :shipping_fee, shipping_fee: shipping_fee) ] } + ship_address { create(:address) } after(:create) do |order, evaluator| + create(:shipping_method_with, :shipping_fee, shipping_fee: evaluator.shipping_fee) + create(:line_item, order: order) payment_calculator = build(:calculator_per_item, preferred_amount: evaluator.payment_fee) payment_method = create(:payment_method, calculator: payment_calculator) create(:payment, order: order, amount: order.total, payment_method: payment_method, state: 'checkout') - # skip the rebuilding of order.shipments from line_items and stock locations (this is enforced in checkout step :address to :delivery) - order.stub(:create_proposed_shipments) while !order.completed? do break unless order.next! end end end diff --git a/spec/models/spree/order_spec.rb b/spec/models/spree/order_spec.rb index e545127570..441e30828c 100644 --- a/spec/models/spree/order_spec.rb +++ b/spec/models/spree/order_spec.rb @@ -655,7 +655,8 @@ describe Spree::Order do end describe "a completed order with shipping and transaction fees" do - let(:order) { create(:completed_order_with_fees, shipping_fee: shipping_fee, payment_fee: payment_fee) } + let(:distributor) { create(:distributor_enterprise_with_tax) } + let(:order) { create(:completed_order_with_fees, distributor: distributor, shipping_fee: shipping_fee, payment_fee: payment_fee) } let(:shipping_fee) { 3 } let(:payment_fee) { 5 } let(:item_num) { order.line_items.length } From a83af367b4ae49c0c1cff286b296dead2e5e039f Mon Sep 17 00:00:00 2001 From: luisramos0 Date: Fri, 12 Oct 2018 11:35:40 +0100 Subject: [PATCH 4/5] Fix order_spec related to retrieving previously ordered items In spree 2, each completed_order_with_totals comes with 5 line items (see order_factory in spree), so instead of 1+1+1=3 the calculation becomes 5+5+1=11 --- spec/models/spree/order_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/models/spree/order_spec.rb b/spec/models/spree/order_spec.rb index 441e30828c..04303a8963 100644 --- a/spec/models/spree/order_spec.rb +++ b/spec/models/spree/order_spec.rb @@ -761,7 +761,7 @@ describe Spree::Order do it "returns previous items" do prev_order.add_variant(product.master, 1, 3) prev_order2.reload # to get the right response from line_items - expect(order.finalised_line_items.length).to eq 3 + expect(order.finalised_line_items.length).to eq 11 expect(order.finalised_line_items).to match_array(prev_order.line_items + prev_order2.line_items) end end From a92e8c992309b957f80f1def0ef78d377f371acd Mon Sep 17 00:00:00 2001 From: luisramos0 Date: Fri, 12 Oct 2018 17:53:06 +0100 Subject: [PATCH 5/5] Improve readability of shipment decorator --- app/models/spree/shipment_decorator.rb | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/app/models/spree/shipment_decorator.rb b/app/models/spree/shipment_decorator.rb index 625cf06dec..e9cdc1ed31 100644 --- a/app/models/spree/shipment_decorator.rb +++ b/app/models/spree/shipment_decorator.rb @@ -2,8 +2,12 @@ module Spree Shipment.class_eval do def ensure_correct_adjustment_with_included_tax ensure_correct_adjustment_without_included_tax - return unless adjustment + update_adjustment_included_tax if adjustment + end + alias_method_chain :ensure_correct_adjustment, :included_tax + + def update_adjustment_included_tax if Config.shipment_inc_vat && (order.distributor.nil? || order.distributor.charges_sales_tax) adjustment.set_included_tax! Config.shipping_tax_rate else @@ -11,8 +15,6 @@ module Spree end end - alias_method_chain :ensure_correct_adjustment, :included_tax - private # NOTE: This is an override of spree's method, needed to allow orders