Shipping and payment fees are updated for completed orders when the order changes

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

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

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
 

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