Merge pull request #6927 from Matt-Yorkley/adjustments-payment-fee

[Adjustments] Payment fee adjustment

app/controllers/spree/admin/adjustments_controller.rb

@@ -17,7 +17,7 @@ module Spree
       end
 
       def collection
-        parent.adjustments.eligible | parent.shipment_adjustments.shipping
+        parent.all_adjustments.eligible
       end
 
       def find_resource

app/helpers/admin/orders_helper.rb

@@ -5,9 +5,9 @@ module Admin
     # We exclude shipping method adjustments because they are displayed in a
     # separate table together with the order line items.
     def order_adjustments_for_display(order)
-      order.adjustments.eligible.reject do |adjustment|
-        adjustment.originator_type == "Spree::ShippingMethod"
-      end
+      order.all_adjustments.enterprise_fee +
+        order.all_adjustments.payment_fee.eligible +
+        order.adjustments.admin
     end
   end
 end

app/models/spree/payment.rb

@@ -20,7 +20,7 @@ module Spree
              class_name: "Spree::Payment", foreign_key: :source_id
     has_many :log_entries, as: :source, dependent: :destroy
 
-    has_one :adjustment, as: :source, dependent: :destroy
+    has_one :adjustment, as: :adjustable, dependent: :destroy
 
     validate :validate_source
     before_create :set_unique_identifier
@@ -135,8 +135,8 @@ module Spree
         adjustment.label = adjustment_label
         adjustment.save
       else
-        payment_method.create_adjustment(adjustment_label, order, self, true)
-        association(:adjustment).reload
+        payment_method.create_adjustment(adjustment_label, self, self, true)
+        adjustment.reload
       end
     end
 

app/serializers/api/order_detailed_serializer.rb

@@ -12,7 +12,7 @@ module Api
 
     def adjustments
       adjustments = object.all_adjustments.where(
-        adjustable_type: ["Spree::Order", "Spree::Shipment"]
+        adjustable_type: ["Spree::Order", "Spree::Shipment", "Spree::Payment"]
       ).order(label: :desc)
       ActiveModel::ArraySerializer.new(adjustments, each_serializer: Api::AdjustmentSerializer)
     end

app/services/order_adjustments_fetcher.rb

@@ -34,7 +34,7 @@ class OrderAdjustmentsFetcher
   attr_reader :order
 
   def adjustments
-    order.adjustments
+    order.all_adjustments
   end
 
   def adjustments_eager_loaded?

app/services/order_checkout_restart.rb

@@ -13,7 +13,7 @@ class OrderCheckoutRestart
     clear_shipments
     clear_payments
 
-    order.reload
+    order.reload.update!
   end
 
   private

db/migrate/20210227144926_migrate_payment_fees_to_payments.rb

@@ -0,0 +1,22 @@
+class MigratePaymentFeesToPayments < ActiveRecord::Migration
+  class Spree::Adjustment < ActiveRecord::Base
+    belongs_to :originator, polymorphic: true
+  end
+
+  def up
+    # Payment fee adjustments currently have the order as the `adjustable` and the payment as
+    # the `source`. Both `source` and `adjustable` will now be the payment. The `originator` is
+    # the payment method, and this is unchanged.
+    Spree::Adjustment.where(originator_type: 'Spree::PaymentMethod').update_all(
+      "adjustable_id = source_id, adjustable_type = 'Spree::Payment'"
+    )
+  end
+
+  def down
+    # Just in case: reversing this migration requires setting the `adjustable` back to the order.
+    # The type is 'Spree::Order', and the order's id is still available on the `order_id` field.
+    Spree::Adjustment.where(originator_type: 'Spree::PaymentMethod').update_all(
+      "adjustable_id = order_id, adjustable_type = 'Spree::Order'"
+    )
+  end
+end

engines/order_management/app/services/order_management/reports/enterprise_fee_summary/scope.rb

@@ -52,7 +52,9 @@ module OrderManagement
 
         def for_orders
           chain_to_scope do
-            where(adjustable_type: ["Spree::Order", "Spree::Shipment", "Spree::LineItem"])
+            where(
+              adjustable_type: ["Spree::Order", "Spree::Shipment", "Spree::LineItem", "Spree::Payment"]
+            )
           end
         end
 

lib/spree/core/calculated_adjustments.rb

@@ -36,7 +36,7 @@ module Spree
             amount = compute_amount(calculable)
             return if amount.zero? && !mandatory
 
-            target.adjustments.create(
+            adjustment_attributes = {
               amount: amount,
               source: old_calculable,
               originator: self,
@@ -45,7 +45,13 @@ module Spree
               mandatory: mandatory,
               state: state,
               included: tax_included?(self, target)
-            )
+            }
+
+            if target.respond_to?(:adjustments)
+              target.adjustments.create(adjustment_attributes)
+            else
+              target.create_adjustment(adjustment_attributes)
+            end
           end
 
           # Updates the amount of the adjustment using our Calculator and

spec/controllers/api/orders_controller_spec.rb

@@ -244,7 +244,7 @@ module Api
 
           expect(json_response[:adjustments].first).to include(
             'label' => "Transaction fee",
-            'amount' => order.adjustments.payment_fee.first.amount.to_s
+            'amount' => order.all_adjustments.payment_fee.first.amount.to_s
           )
           expect(json_response[:adjustments].second).to include(
             'label' => "Shipping",

spec/controllers/spree/admin/adjustments_controller_spec.rb

@@ -8,6 +8,24 @@ module Spree
 
     before { controller_login_as_admin }
 
+    describe "index" do
+      let!(:order) { create(:order) }
+      let!(:adjustment1) {
+        create(:adjustment, originator_type: "Spree::ShippingMethod", order: order)
+      }
+      let!(:adjustment2) {
+        create(:adjustment, originator_type: "Spree::PaymentMethod", eligible: false, order: order)
+      }
+      let!(:adjustment3) { create(:adjustment, originator_type: "EnterpriseFee", order: order) }
+
+      it "loads all eligible adjustments" do
+        spree_get :index, order_id: order.number
+
+        expect(assigns(:collection)).to include adjustment1, adjustment3
+        expect(assigns(:collection)).to_not include adjustment2
+      end
+    end
+
     describe "setting included tax" do
       let(:order) { create(:order) }
       let(:tax_rate) { create(:tax_rate, amount: 0.1, calculator: ::Calculator::DefaultTax.new) }

spec/features/consumer/shopping/checkout_spec.rb

@@ -462,7 +462,7 @@ feature "As a consumer I want to check out my cart", js: true do
 
             # There are two orders - our order and our new cart
             o = Spree::Order.complete.first
-            expect(o.adjustments.payment_fee.first.amount).to eq 5.67
+            expect(o.all_adjustments.payment_fee.first.amount).to eq 5.67
             expect(o.payments.first.amount).to eq(10 + 1.23 + 5.67) # items + fees + transaction
           end
         end

spec/helpers/admin/orders_helper_spec.rb

@@ -7,13 +7,14 @@ describe Admin::OrdersHelper, type: :helper do
     let(:order) { create(:order) }
 
     it "selects eligible adjustments" do
-      adjustment = create(:adjustment, order: order, adjustable: order, amount: 1)
+      adjustment = create(:adjustment, order: order, adjustable: order, amount: 1, source: nil)
 
       expect(helper.order_adjustments_for_display(order)).to eq [adjustment]
     end
 
     it "filters shipping method adjustments" do
-      create(:adjustment, order: order, adjustable: order, amount: 1, originator_type: "Spree::ShippingMethod")
+      create(:adjustment, order: order, adjustable: order, amount: 1,
+                          originator_type: "Spree::ShippingMethod")
 
       expect(helper.order_adjustments_for_display(order)).to eq []
     end

spec/models/spree/order_spec.rb

@@ -1053,7 +1053,7 @@ describe Spree::Order do
       Spree::Config.shipping_tax_rate = 0.25
 
       # Sanity check the fees
-      expect(order.adjustments.length).to eq 1
+      expect(order.all_adjustments.length).to eq 2
       expect(order.shipment_adjustments.length).to eq 1
       expect(item_num).to eq 2
       expect(order.adjustment_total).to eq expected_fees
@@ -1070,7 +1070,7 @@ describe Spree::Order do
       end
 
       context "when finalized fee adjustments exist on the order" do
-        let(:payment_fee_adjustment) { order.adjustments.payment_fee.first }
+        let(:payment_fee_adjustment) { order.all_adjustments.payment_fee.first }
         let(:shipping_fee_adjustment) { order.shipment_adjustments.first }
 
         before do

spec/models/spree/payment_spec.rb

@@ -856,8 +856,8 @@ describe Spree::Payment do
             expect(payment.state).to eq "failed"
             expect(payment.adjustment.eligible?).to be false
             expect(payment.adjustment.finalized?).to be true
-            expect(order.adjustments.payment_fee.count).to eq 1
-            expect(order.adjustments.payment_fee.eligible).to_not include payment.adjustment
+            expect(order.all_adjustments.payment_fee.count).to eq 1
+            expect(order.all_adjustments.payment_fee.eligible).to_not include payment.adjustment
           end
         end
 
@@ -875,8 +875,8 @@ describe Spree::Payment do
             expect(payment.state).to eq "invalid"
             expect(payment.adjustment.eligible?).to be false
             expect(payment.adjustment.finalized?).to be true
-            expect(order.adjustments.payment_fee.count).to eq 1
-            expect(order.adjustments.payment_fee.eligible).to_not include payment.adjustment
+            expect(order.all_adjustments.payment_fee.count).to eq 1
+            expect(order.all_adjustments.payment_fee.eligible).to_not include payment.adjustment
           end
         end
 
@@ -895,8 +895,8 @@ describe Spree::Payment do
             expect(order.payments).to include payment
             expect(payment.state).to eq "completed"
             expect(payment.adjustment.eligible?).to be true
-            expect(order.adjustments.payment_fee.count).to eq 1
-            expect(order.adjustments.payment_fee.eligible).to include payment.adjustment
+            expect(order.all_adjustments.payment_fee.count).to eq 1
+            expect(order.all_adjustments.payment_fee.eligible).to include payment.adjustment
             expect(payment.adjustment.amount).to eq 1.5
           end
         end

spec/requests/checkout/paypal_spec.rb

@@ -52,8 +52,8 @@ describe "checking out an order with a paypal express payment method", type: :re
       # Sanity check to condition of the order before we confirm the payment
       expect(order.payments.count).to eq 1
       expect(order.payments.first.state).to eq "checkout"
-      expect(order.adjustments.payment_fee.count).to eq 1
-      expect(order.adjustments.payment_fee.first.amount).to eq 1.5
+      expect(order.all_adjustments.payment_fee.count).to eq 1
+      expect(order.all_adjustments.payment_fee.first.amount).to eq 1.5
 
       get spree.confirm_paypal_path, params
 
@@ -64,8 +64,8 @@ describe "checking out an order with a paypal express payment method", type: :re
       # We have only one payment, and one transaction fee
       expect(order.payments.count).to eq 1
       expect(order.payments.first.state).to eq "completed"
-      expect(order.adjustments.payment_fee.count).to eq 1
-      expect(order.adjustments.payment_fee.first.amount).to eq 1.5
+      expect(order.all_adjustments.payment_fee.count).to eq 1
+      expect(order.all_adjustments.payment_fee.first.amount).to eq 1.5
     end
   end
 end

spec/services/order_checkout_restart_spec.rb

@@ -53,18 +53,18 @@ describe OrderCheckoutRestart do
 
           expect(order.state).to eq 'payment'
           expect(order.shipments.count).to eq 1
-          expect(order.adjustments.shipping.count).to eq 0
+          expect(order.all_adjustments.shipping.count).to eq 0
           expect(order.payments.count).to eq 2
-          expect(order.adjustments.payment_fee.count).to eq 2
+          expect(order.all_adjustments.payment_fee.count).to eq 2
         end
       end
 
       def expect_cart_state_and_reset_adjustments
         expect(order.state).to eq 'cart'
         expect(order.shipments.count).to eq 0
-        expect(order.adjustments.shipping.count).to eq 0
+        expect(order.all_adjustments.shipping.count).to eq 0
         expect(order.payments.count).to eq 1
-        expect(order.adjustments.payment_fee.count).to eq 1
+        expect(order.all_adjustments.payment_fee.count).to eq 1
       end
     end
   end