diff --git a/app/controllers/cart_controller.rb b/app/controllers/cart_controller.rb index 8d11580aa6..f4ab6abf29 100644 --- a/app/controllers/cart_controller.rb +++ b/app/controllers/cart_controller.rb @@ -4,29 +4,23 @@ class CartController < BaseController def populate order = current_order(true) - # Without intervention, the Spree::Adjustment#update_adjustable callback is called many times - # during cart population, for both taxation and enterprise fees. This operation triggers a - # costly Spree::Order#update!, which only needs to be run once. We avoid this by disabling - # callbacks on Spree::Adjustment and then manually invoke Spree::Order#update! on success. - Spree::Adjustment.without_callbacks do - cart_service = CartService.new(order) + cart_service = CartService.new(order) - cart_service.populate(params.slice(:variants, :quantity), true) - if cart_service.valid? - order.recreate_all_fees! - order.cap_quantity_at_stock! - order.update! + cart_service.populate(params.slice(:variants, :quantity), true) + if cart_service.valid? + order.cap_quantity_at_stock! + order.recreate_all_fees! - variant_ids = variant_ids_in(cart_service.variants_h) + variant_ids = variant_ids_in(cart_service.variants_h) - render json: { error: false, - stock_levels: VariantsStockLevels.new.call(order, variant_ids) }, - status: :ok - else - render json: { error: cart_service.errors.full_messages.join(",") }, - status: :precondition_failed - end + render json: { error: false, + stock_levels: VariantsStockLevels.new.call(order, variant_ids) }, + status: :ok + else + render json: { error: cart_service.errors.full_messages.join(",") }, + status: :precondition_failed end + populate_variant_attributes end diff --git a/app/controllers/checkout_controller.rb b/app/controllers/checkout_controller.rb index ce04528410..4e3a22415b 100644 --- a/app/controllers/checkout_controller.rb +++ b/app/controllers/checkout_controller.rb @@ -54,6 +54,8 @@ class CheckoutController < ::BaseController rescue StandardError => e flash[:error] = I18n.t("checkout.failed") action_failed(e) + ensure + @order.update! end # Clears the cached order. Required for #current_order to return a new order diff --git a/app/controllers/spree/admin/orders_controller.rb b/app/controllers/spree/admin/orders_controller.rb index 76daa85014..e5bc415bc5 100644 --- a/app/controllers/spree/admin/orders_controller.rb +++ b/app/controllers/spree/admin/orders_controller.rb @@ -36,7 +36,6 @@ module Spree def update @order.recreate_all_fees! - @order.update! unless order_params.present? && @order.update(order_params) && @order.line_items.present? if @order.line_items.empty? diff --git a/app/models/spree/adjustment.rb b/app/models/spree/adjustment.rb index 84a7f603de..ef34d0eb9f 100644 --- a/app/models/spree/adjustment.rb +++ b/app/models/spree/adjustment.rb @@ -47,9 +47,6 @@ module Spree validates :label, presence: true validates :amount, numericality: true - after_save :update_adjustable - after_destroy :update_adjustable - state_machine :state, initial: :open do event :close do transition from: :open, to: :closed @@ -144,29 +141,11 @@ module Spree included_tax.positive? end - def self.without_callbacks - skip_callback :save, :after, :update_adjustable - skip_callback :destroy, :after, :update_adjustable - - result = yield - ensure - set_callback :save, :after, :update_adjustable - set_callback :destroy, :after, :update_adjustable - - result - end - # Allow accessing soft-deleted originator objects def originator return if originator_type.blank? originator_type.constantize.unscoped { super } end - - private - - def update_adjustable - adjustable.update! if adjustable.is_a? Order - end end end diff --git a/app/models/spree/return_authorization.rb b/app/models/spree/return_authorization.rb index 8b762a19c4..fc5acb3f02 100644 --- a/app/models/spree/return_authorization.rb +++ b/app/models/spree/return_authorization.rb @@ -95,6 +95,7 @@ module Spree credit.save order.return if inventory_units.all?(&:returned?) + order.update! end def allow_receive? diff --git a/app/services/order_fees_handler.rb b/app/services/order_fees_handler.rb index 3fd09bdb39..55c2416136 100644 --- a/app/services/order_fees_handler.rb +++ b/app/services/order_fees_handler.rb @@ -20,6 +20,8 @@ class OrderFeesHandler create_line_item_fees! create_order_fees! end + + order.update! end def create_line_item_fees! diff --git a/spec/controllers/spree/admin/orders_controller_spec.rb b/spec/controllers/spree/admin/orders_controller_spec.rb index 5bfb20acfd..529743bd6c 100644 --- a/spec/controllers/spree/admin/orders_controller_spec.rb +++ b/spec/controllers/spree/admin/orders_controller_spec.rb @@ -78,7 +78,6 @@ describe Spree::Admin::OrdersController, type: :controller do order.line_items.last.update(variant_id: variant2.id) while !order.completed? do break unless order.next! end order.recreate_all_fees! - order.update! order end diff --git a/spec/controllers/spree/orders_controller_spec.rb b/spec/controllers/spree/orders_controller_spec.rb index 86283702a9..f136a51f64 100644 --- a/spec/controllers/spree/orders_controller_spec.rb +++ b/spec/controllers/spree/orders_controller_spec.rb @@ -313,7 +313,6 @@ describe Spree::OrdersController, type: :controller do order.reload.line_items.last.update(variant_id: variant2.id) while !order.completed? do break unless order.next! end order.recreate_all_fees! - order.update! order end let(:params) { diff --git a/spec/helpers/admin/orders_helper_spec.rb b/spec/helpers/admin/orders_helper_spec.rb index 4556763d22..13079d4590 100644 --- a/spec/helpers/admin/orders_helper_spec.rb +++ b/spec/helpers/admin/orders_helper_spec.rb @@ -7,13 +7,13 @@ describe Admin::OrdersHelper, type: :helper do let(:order) { create(:order) } it "selects eligible adjustments" do - adjustment = create(:adjustment, adjustable: order, amount: 1) + adjustment = create(:adjustment, order: order, adjustable: order, amount: 1) expect(helper.order_adjustments_for_display(order)).to eq [adjustment] end it "filters shipping method adjustments" do - create(:adjustment, 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 diff --git a/spec/helpers/checkout_helper_spec.rb b/spec/helpers/checkout_helper_spec.rb index 076f1f9fb5..966569459a 100644 --- a/spec/helpers/checkout_helper_spec.rb +++ b/spec/helpers/checkout_helper_spec.rb @@ -40,6 +40,7 @@ describe CheckoutHelper, type: :helper do } before do + order.update! # Sanity check initial adjustments state expect(order.shipment_adjustments.count).to eq 1 expect(order.adjustments.enterprise_fee.count).to eq 1 diff --git a/spec/models/spree/adjustment_spec.rb b/spec/models/spree/adjustment_spec.rb index 150e806c87..4c3cf7e687 100644 --- a/spec/models/spree/adjustment_spec.rb +++ b/spec/models/spree/adjustment_spec.rb @@ -63,18 +63,6 @@ module Spree end end - context "#save" do - it "should call order#update!" do - adjustment = Spree::Adjustment.new( - adjustable: order, - amount: 10, - label: "Foo" - ) - expect(order).to receive(:update!) - adjustment.save - end - end - context "adjustment state" do let(:adjustment) { create(:adjustment, state: 'open') } diff --git a/spec/models/spree/order_spec.rb b/spec/models/spree/order_spec.rb index afdbb112ed..4e789af564 100644 --- a/spec/models/spree/order_spec.rb +++ b/spec/models/spree/order_spec.rb @@ -525,6 +525,7 @@ describe Spree::Order do before do allow(subject).to receive(:fee_handler) { fee_handler } + allow(subject).to receive(:update!) end it "clears all enterprise fee adjustments on the order" do