From 217eda8362d30f64d5bd16e360a8361f3309be27 Mon Sep 17 00:00:00 2001 From: Rob Harrington Date: Thu, 4 May 2017 18:26:07 +1000 Subject: [PATCH] Shipping and payment fees are updated for completed orders when the order changes --- app/models/spree/order_decorator.rb | 13 ++- .../controllers/line_items_controller_spec.rb | 82 +++++++++++++------ spec/factories.rb | 2 +- spec/models/spree/order_spec.rb | 63 ++++++++------ 4 files changed, 104 insertions(+), 56 deletions(-) diff --git a/app/models/spree/order_decorator.rb b/app/models/spree/order_decorator.rb index a1b2ee0b56..2691b15798 100644 --- a/app/models/spree/order_decorator.rb +++ b/app/models/spree/order_decorator.rb @@ -21,6 +21,9 @@ Spree::Order.class_eval do before_validation :associate_customer, unless: :customer_id? before_validation :ensure_customer, unless: :customer_is_valid? + before_save :update_shipping_fees!, if: :complete? + before_save :update_payment_fees!, if: :complete? + checkout_flow do go_to_state :address go_to_state :delivery @@ -141,26 +144,20 @@ Spree::Order.class_eval do # After changing line items of a completed order def update_shipping_fees! - line_items(:reload) shipments.each do |shipment| next if shipment.shipped? update_adjustment! shipment.adjustment shipment.save # updates included tax end - update_totals - save end # After changing line items of a completed order def update_payment_fees! - payments(:reload) - line_items(:reload) payments.each do |payment| next if payment.completed? update_adjustment! payment.adjustment + payment.save end - update_totals - save end def cap_quantity_at_stock! @@ -350,7 +347,7 @@ Spree::Order.class_eval do def update_adjustment!(adjustment) locked = adjustment.locked adjustment.locked = false - adjustment.update! + adjustment.update!(self) adjustment.locked = locked end end diff --git a/spec/controllers/line_items_controller_spec.rb b/spec/controllers/line_items_controller_spec.rb index 2fcaf00d30..3ee277f4fe 100644 --- a/spec/controllers/line_items_controller_spec.rb +++ b/spec/controllers/line_items_controller_spec.rb @@ -60,31 +60,67 @@ describe LineItemsController do expect { item.reload }.to raise_error end - it "updates fees" do - Spree::Config.shipment_inc_vat = true - Spree::Config.shipping_tax_rate = 0.25 - distributor = create(:distributor_enterprise, charges_sales_tax: true, allow_order_changes: true) - shipping_fee = 3 - payment_fee = 5 - order = create(:completed_order_with_fees, distributor: distributor, shipping_fee: shipping_fee, payment_fee: payment_fee) + describe "destroying a line item" do + context "where shipping and payment fees apply" do + let(:distributor) { create(:distributor_enterprise, charges_sales_tax: true, allow_order_changes: true) } + let(:shipping_fee) { 3 } + let(:payment_fee) { 5 } + let(:order) { create(:completed_order_with_fees, distributor: distributor, shipping_fee: shipping_fee, payment_fee: payment_fee) } - # Sanity check fees - item_num = order.line_items.length - initial_fees = item_num * (shipping_fee + payment_fee) - expect(order.adjustment_total).to eq initial_fees - expect(order.shipment.adjustment.included_tax).to eq 1.2 + before do + Spree::Config.shipment_inc_vat = true + Spree::Config.shipping_tax_rate = 0.25 + end - # Delete the item - item = order.line_items.first - controller.stub spree_current_user: order.user - request = { format: :json, id: item } - delete :destroy, request - expect(response.status).to eq 204 + it "updates the fees" do + # Sanity check fees + item_num = order.line_items.length + initial_fees = item_num * (shipping_fee + payment_fee) + expect(order.adjustment_total).to eq initial_fees + expect(order.shipment.adjustment.included_tax).to eq 1.2 - # Check the fees again - order.reload - order.shipment.reload - expect(order.adjustment_total).to eq initial_fees - shipping_fee - payment_fee - expect(order.shipment.adjustment.included_tax).to eq 0.6 + # Delete the item + item = order.line_items.first + controller.stub spree_current_user: order.user + request = { format: :json, id: item } + delete :destroy, request + expect(response.status).to eq 204 + + # Check the fees again + order.reload + order.shipment.reload + expect(order.adjustment_total).to eq initial_fees - shipping_fee - payment_fee + expect(order.shipment.adjustment.amount).to eq shipping_fee + expect(order.payment.adjustment.amount).to eq payment_fee + expect(order.shipment.adjustment.included_tax).to eq 0.6 + end + end + + context "where enterprise fees apply" do + let(:user) { create(:user) } + let(:variant) { create(:variant) } + let(:distributor) { create(:distributor_enterprise, allow_order_changes: true) } + let(:order_cycle) { create(:simple_order_cycle, distributors: [distributor]) } + let(:enterprise_fee) { create(:enterprise_fee, calculator: Spree::Calculator::PerItem.new ) } + let!(:exchange) { create(:exchange, sender: variant.product.supplier, receiver: order_cycle.coordinator, variants: [variant], enterprise_fees: [enterprise_fee]) } + let!(:order) do + order = create(:order, distributor: distributor, order_cycle: order_cycle, user: user) + order.line_items << build(:line_item, variant: variant) + order.update_distribution_charge! + order.save! + order + end + let(:params) { { format: :json, id: order.line_items.first } } + + it "updates the fees" do + expect(order.adjustment_total).to eq enterprise_fee.calculator.preferred_amount + + controller.stub spree_current_user: user + delete :destroy, params + expect(response.status).to eq 204 + + expect(order.reload.adjustment_total).to eq 0 + end + end end end diff --git a/spec/factories.rb b/spec/factories.rb index 4c3cc67ebd..14f08ca619 100644 --- a/spec/factories.rb +++ b/spec/factories.rb @@ -272,7 +272,7 @@ FactoryGirl.define do 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') - order.finalize! + 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 763f7dd6c4..e7c46c27cb 100644 --- a/spec/models/spree/order_spec.rb +++ b/spec/models/spree/order_spec.rb @@ -656,44 +656,59 @@ describe Spree::Order do 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 } + let(:expected_fees) { item_num * (shipping_fee + payment_fee) } before do Spree::Config.shipment_inc_vat = true Spree::Config.shipping_tax_rate = 0.25 - end - it "updates shipping fees" do # Sanity check the fees expect(order.adjustments.length).to eq 2 - item_num = order.line_items.length expect(item_num).to eq 2 - expected_fees = item_num * (shipping_fee + payment_fee) expect(order.adjustment_total).to eq expected_fees expect(order.shipment.adjustment.included_tax).to eq 1.2 - - # Delete the item - order.line_items.first.destroy - order.update_shipping_fees! - - # Check if fees got updated - expect(order.adjustments.length).to eq 2 - expect(order.line_items.length).to eq item_num - 1 - expect(order.adjustment_total).to eq expected_fees - shipping_fee - expect(order.shipment.adjustment.included_tax).to eq 0.6 end - it "updates transaction fees" do - item_num = order.line_items.length - initial_fees = item_num * (shipping_fee + payment_fee) + context "removing line_items" do + it "updates shipping and transaction fees" do + # Setting quantity of an item to zero + order.update_attributes(line_items_attributes: [{id: order.line_items.first.id, quantity: 0}]) - # Delete the item - order.line_items.first.destroy - order.update_payment_fees! + # Check if fees got updated + order.reload + expect(order.adjustment_total).to eq expected_fees - shipping_fee - payment_fee + expect(order.shipment.adjustment.included_tax).to eq 0.6 + end + end - # Check if fees got updated - expect(order.adjustments.length).to eq 2 - expect(order.line_items.length).to eq item_num - 1 - expect(order.adjustment_total).to eq initial_fees - payment_fee + context "changing the shipping method to one without fees" do + let(:shipping_method) { create(:shipping_method, calculator: Spree::Calculator::FlatRate.new(preferred_amount: 0)) } + + it "updates shipping fees" do + # Change the shipping method + order.shipment.update_attributes(shipping_method_id: shipping_method.id) + order.save + + # Check if fees got updated + order.reload + expect(order.adjustment_total).to eq expected_fees - (item_num * shipping_fee) + expect(order.shipment.adjustment.included_tax).to eq 0 + end + end + + context "changing the payment method to one without fees" do + let(:payment_method) { create(:payment_method, calculator: Spree::Calculator::FlatRate.new(preferred_amount: 0)) } + + it "removes transaction fees" do + # Change the payment method + order.payment.update_attributes(payment_method_id: payment_method.id) + order.save + + # Check if fees got updated + order.reload + expect(order.adjustment_total).to eq expected_fees - (item_num * payment_fee) + end end end