From a42651d543e66d83c4144a98f8a3e107fdc4a513 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Sat, 20 Feb 2021 21:44:52 +0000 Subject: [PATCH 01/13] Update Payment fee adjustment --- app/models/spree/payment.rb | 10 +++++++--- spec/models/spree/order_spec.rb | 4 ++-- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/app/models/spree/payment.rb b/app/models/spree/payment.rb index e3a9aab7bb..1469eb0528 100644 --- a/app/models/spree/payment.rb +++ b/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_many :adjustments, as: :adjustable, dependent: :destroy validate :validate_source before_create :set_unique_identifier @@ -126,6 +126,10 @@ module Spree res || payment_method end + def adjustment + @adjustment ||= adjustments.first + end + def ensure_correct_adjustment revoke_adjustment_eligibility if ['failed', 'invalid'].include?(state) return if adjustment.try(:finalized?) @@ -135,8 +139,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 diff --git a/spec/models/spree/order_spec.rb b/spec/models/spree/order_spec.rb index a2c9ad50a7..dbd8f179a1 100644 --- a/spec/models/spree/order_spec.rb +++ b/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 From a0e6b64e981de4abab3f5fe0465e8d7cfb021e2a Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Sun, 21 Feb 2021 10:24:52 +0000 Subject: [PATCH 02/13] Update Order serializer --- app/serializers/api/order_detailed_serializer.rb | 2 +- spec/controllers/api/orders_controller_spec.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/serializers/api/order_detailed_serializer.rb b/app/serializers/api/order_detailed_serializer.rb index 5fa36dcddc..fd625b6936 100644 --- a/app/serializers/api/order_detailed_serializer.rb +++ b/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 diff --git a/spec/controllers/api/orders_controller_spec.rb b/spec/controllers/api/orders_controller_spec.rb index 2dfd2dd035..0bda9d464d 100644 --- a/spec/controllers/api/orders_controller_spec.rb +++ b/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", From cb179c794bdc0ab4db4bfb8af9640375a69626cd Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Sun, 21 Feb 2021 10:26:26 +0000 Subject: [PATCH 03/13] Update Paypal spec --- spec/requests/checkout/paypal_spec.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/spec/requests/checkout/paypal_spec.rb b/spec/requests/checkout/paypal_spec.rb index a6436d4347..d0017db605 100644 --- a/spec/requests/checkout/paypal_spec.rb +++ b/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 From 8abfd7c3f3b255d9fa22b37b43d370c3a91a6945 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Sun, 21 Feb 2021 10:29:01 +0000 Subject: [PATCH 04/13] Update Checkout spec --- spec/features/consumer/shopping/checkout_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/features/consumer/shopping/checkout_spec.rb b/spec/features/consumer/shopping/checkout_spec.rb index d9a9a855b2..a2a03a5979 100644 --- a/spec/features/consumer/shopping/checkout_spec.rb +++ b/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 From 242ccc4fb3e746aa3b4f7039cd6f59e2ac173d89 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Sun, 21 Feb 2021 10:33:35 +0000 Subject: [PATCH 05/13] Update OrderAdjustmentsFetcher --- app/services/order_adjustments_fetcher.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/services/order_adjustments_fetcher.rb b/app/services/order_adjustments_fetcher.rb index ea553766c8..d840505bfc 100644 --- a/app/services/order_adjustments_fetcher.rb +++ b/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? From 2ccaf800132ab14ea8b879f9e84ba44264f4643c Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Sun, 21 Feb 2021 10:39:33 +0000 Subject: [PATCH 06/13] Update EnterpriseFeeSummary report scopes --- .../order_management/reports/enterprise_fee_summary/scope.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/engines/order_management/app/services/order_management/reports/enterprise_fee_summary/scope.rb b/engines/order_management/app/services/order_management/reports/enterprise_fee_summary/scope.rb index d23668edf1..c5ec42190a 100644 --- a/engines/order_management/app/services/order_management/reports/enterprise_fee_summary/scope.rb +++ b/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 From 3294b33431620994c983875b11c3f93cb8cf760e Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Sun, 21 Feb 2021 10:44:16 +0000 Subject: [PATCH 07/13] Update Payment spec --- spec/models/spree/payment_spec.rb | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/spec/models/spree/payment_spec.rb b/spec/models/spree/payment_spec.rb index 64d9c04afe..d49898d83c 100644 --- a/spec/models/spree/payment_spec.rb +++ b/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 From 5a7792bebc8e99cd88289a9cecca3114846421b9 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Sun, 21 Feb 2021 10:45:43 +0000 Subject: [PATCH 08/13] Update Checkout Restart spec --- spec/services/order_checkout_restart_spec.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/spec/services/order_checkout_restart_spec.rb b/spec/services/order_checkout_restart_spec.rb index a5b7258e74..07c8c294b6 100644 --- a/spec/services/order_checkout_restart_spec.rb +++ b/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 From 21e0c36f4f733057cde3311ea71a1c43d803e42c Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Sun, 21 Feb 2021 11:57:55 +0000 Subject: [PATCH 09/13] Update order totals after deleting shipments and payments --- app/services/order_checkout_restart.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/services/order_checkout_restart.rb b/app/services/order_checkout_restart.rb index 5ca7d2bf78..58d7c29d76 100644 --- a/app/services/order_checkout_restart.rb +++ b/app/services/order_checkout_restart.rb @@ -13,7 +13,7 @@ class OrderCheckoutRestart clear_shipments clear_payments - order.reload + order.reload.update! end private From 5d1d72b36bff53524326d90dfed7b0a46e3b8a92 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Sat, 27 Feb 2021 12:40:01 +0000 Subject: [PATCH 10/13] Update Admin::OrdersHelper#order_adjustments_for_display --- app/helpers/admin/orders_helper.rb | 6 +++--- spec/helpers/admin/orders_helper_spec.rb | 5 +++-- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/app/helpers/admin/orders_helper.rb b/app/helpers/admin/orders_helper.rb index 9e16d27ed5..022394c624 100644 --- a/app/helpers/admin/orders_helper.rb +++ b/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 diff --git a/spec/helpers/admin/orders_helper_spec.rb b/spec/helpers/admin/orders_helper_spec.rb index 13079d4590..63357be422 100644 --- a/spec/helpers/admin/orders_helper_spec.rb +++ b/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 From e237727ba2e056026ba52b2928a0c4bb0c142f48 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Sat, 27 Feb 2021 14:52:34 +0000 Subject: [PATCH 11/13] Migrate payment fee adjustments to payment objects --- ...144926_migrate_payment_fees_to_payments.rb | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) create mode 100644 db/migrate/20210227144926_migrate_payment_fees_to_payments.rb diff --git a/db/migrate/20210227144926_migrate_payment_fees_to_payments.rb b/db/migrate/20210227144926_migrate_payment_fees_to_payments.rb new file mode 100644 index 0000000000..5cd9c24ace --- /dev/null +++ b/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 From 5840b0e33cdd43133fc420e3f0ea6f18479d2963 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Tue, 9 Mar 2021 16:14:25 +0000 Subject: [PATCH 12/13] Adapt adjustment interface for payment's adjustment being singular Payments only have one adjustment, all other adjustable objects have adjustments (plural). --- app/models/spree/payment.rb | 6 +----- lib/spree/core/calculated_adjustments.rb | 10 ++++++++-- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/app/models/spree/payment.rb b/app/models/spree/payment.rb index 1469eb0528..a1f635db45 100644 --- a/app/models/spree/payment.rb +++ b/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_many :adjustments, as: :adjustable, dependent: :destroy + has_one :adjustment, as: :adjustable, dependent: :destroy validate :validate_source before_create :set_unique_identifier @@ -126,10 +126,6 @@ module Spree res || payment_method end - def adjustment - @adjustment ||= adjustments.first - end - def ensure_correct_adjustment revoke_adjustment_eligibility if ['failed', 'invalid'].include?(state) return if adjustment.try(:finalized?) diff --git a/lib/spree/core/calculated_adjustments.rb b/lib/spree/core/calculated_adjustments.rb index 301d821642..944eff67b6 100644 --- a/lib/spree/core/calculated_adjustments.rb +++ b/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 From cbd7c9f4c06aa7e6ba081e2ea00d612e6a1c819e Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Thu, 25 Mar 2021 12:30:05 +0000 Subject: [PATCH 13/13] Update adjustments controller collection scope --- .../spree/admin/adjustments_controller.rb | 2 +- .../spree/admin/adjustments_controller_spec.rb | 18 ++++++++++++++++++ 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/app/controllers/spree/admin/adjustments_controller.rb b/app/controllers/spree/admin/adjustments_controller.rb index d596ed9b16..ed6b9d2cde 100644 --- a/app/controllers/spree/admin/adjustments_controller.rb +++ b/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 diff --git a/spec/controllers/spree/admin/adjustments_controller_spec.rb b/spec/controllers/spree/admin/adjustments_controller_spec.rb index 931112da4b..87b372c3b9 100644 --- a/spec/controllers/spree/admin/adjustments_controller_spec.rb +++ b/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) }