From d840fa7b77459bcddb7df9de6a4b500848761017 Mon Sep 17 00:00:00 2001 From: luisramos0 Date: Fri, 1 Mar 2019 19:44:50 +0000 Subject: [PATCH] Improve order factories: - Make order_with_totals_and_distribution shorter by inheriting from order_with_distributor - Make completed_order_with_fees more correct by inheriting only from order_with_distributor: this removes the line_item_with_shipment of order_with_totals_and_distribution that was causing an extra shipping adjustment to be kept in the order. This adjustment was being tested in the order controller spec (fixed the explanation comment in the spec which was wrong, there was no enterprise fee in this order) --- spec/controllers/spree/orders_controller_spec.rb | 13 +++---------- spec/factories.rb | 15 +++++++++++---- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/spec/controllers/spree/orders_controller_spec.rb b/spec/controllers/spree/orders_controller_spec.rb index d77e493128..dfeb9933e3 100644 --- a/spec/controllers/spree/orders_controller_spec.rb +++ b/spec/controllers/spree/orders_controller_spec.rb @@ -224,16 +224,9 @@ describe Spree::OrdersController, type: :controller do } } } - # Before issuing the update, the second adjustment, which is associated - # to the shipment, is already open thus restoring its state leaves it - # also open. - # - # The third adjustment is originated from an EnterpriseFee and it gets - # created by #update_distribution_charge! in - # app/models/spree/order_decorator.rb:220, which is in turn triggered - # by the `contents_changed` notification event defined in - # app/models/spree/order_decorator.rb:7 - expect(order.adjustments.map(&:state)).to eq(['closed', 'open', 'closed']) + # The second adjustment (shipping adjustment) is open before the update + # so, restoring its state leaves it open. + expect(order.adjustments.map(&:state)).to eq(['closed', 'open']) end end diff --git a/spec/factories.rb b/spec/factories.rb index cbdbdab3a8..0b8f318201 100644 --- a/spec/factories.rb +++ b/spec/factories.rb @@ -291,17 +291,16 @@ FactoryBot.define do after(:create) { |c| c.set_preference(:per_kg, 0.5); c.save! } end - factory :order_with_totals_and_distribution, parent: :order do + factory :order_with_totals_and_distribution, parent: :order_with_distributor do transient do shipping_fee 3 end - distributor { create(:distributor_enterprise) } order_cycle { create(:simple_order_cycle) } after(:create) do |order, proxy| p = create(:simple_product, distributors: [order.distributor]) - FactoryBot.create(:line_item_with_shipment, shipping_fee: proxy.shipping_fee, order: order, product: p) + create(:line_item_with_shipment, shipping_fee: proxy.shipping_fee, order: order, product: p) order.reload end end @@ -416,19 +415,27 @@ FactoryBot.define do allow_order_changes { true } end - factory :completed_order_with_fees, parent: :order_with_totals_and_distribution do + factory :completed_order_with_fees, parent: :order_with_distributor do transient do payment_fee 5 + shipping_fee 3 end ship_address { create(:address) } + order_cycle { create(:simple_order_cycle) } after(:create) do |order, evaluator| create(:line_item, order: order) + product = create(:simple_product, distributors: [order.distributor]) + create(:line_item, order: order, product: product) + 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') + create(:shipping_method_with, :shipping_fee, shipping_fee: evaluator.shipping_fee) + + order.reload while !order.completed? do break unless order.next! end end end