diff --git a/app/controllers/admin/bulk_line_items_controller.rb b/app/controllers/admin/bulk_line_items_controller.rb index fff22ea718..f8667f69ec 100644 --- a/app/controllers/admin/bulk_line_items_controller.rb +++ b/app/controllers/admin/bulk_line_items_controller.rb @@ -33,7 +33,9 @@ module Admin # and https://www.postgresql.org/docs/current/static/sql-select.html#SQL-FOR-UPDATE-SHARE order.with_lock do if @line_item.update(line_item_params) - order.update_distribution_charge! + order.update_line_item_fees! @line_item + order.update_order_fees! + order.update! render nothing: true, status: :no_content # No Content, does not trigger ng resource auto-update else render json: { errors: @line_item.errors }, status: :precondition_failed diff --git a/app/controllers/cart_controller.rb b/app/controllers/cart_controller.rb index 658ef2e2f5..8d11580aa6 100644 --- a/app/controllers/cart_controller.rb +++ b/app/controllers/cart_controller.rb @@ -13,7 +13,7 @@ class CartController < BaseController cart_service.populate(params.slice(:variants, :quantity), true) if cart_service.valid? - order.update_distribution_charge! + order.recreate_all_fees! order.cap_quantity_at_stock! order.update! diff --git a/app/controllers/line_items_controller.rb b/app/controllers/line_items_controller.rb index 28b8d4a324..b4d8624134 100644 --- a/app/controllers/line_items_controller.rb +++ b/app/controllers/line_items_controller.rb @@ -42,7 +42,8 @@ class LineItemsController < BaseController item.destroy order.update_shipping_fees! order.update_payment_fees! - order.update_distribution_charge! + order.update_order_fees! + order.update! order.create_tax_charge! end end diff --git a/app/controllers/spree/admin/orders_controller.rb b/app/controllers/spree/admin/orders_controller.rb index 38da5e812e..76daa85014 100644 --- a/app/controllers/spree/admin/orders_controller.rb +++ b/app/controllers/spree/admin/orders_controller.rb @@ -12,14 +12,6 @@ module Spree # Ensure that the distributor is set for an order when before_action :ensure_distribution, only: :new - - # After updating an order, the fees should be updated as well - # Currently, adding or deleting line items does not trigger updating the - # fees! This is a quick fix for that. - # TODO: update fees when adding/removing line items - # instead of the update_distribution_charge method. - after_action :update_distribution_charge, only: :update - before_action :require_distributor_abn, only: :invoice respond_to :html, :json @@ -43,6 +35,9 @@ module Spree end 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? @order.errors.add(:line_items, Spree.t('errors.messages.blank')) @@ -51,7 +46,6 @@ module Spree flash: { error: @order.errors.full_messages.join(', ') }) end - @order.update! if @order.complete? redirect_to spree.edit_admin_order_path(@order) else @@ -103,10 +97,6 @@ module Spree render template: "spree/admin/orders/ticket", layout: false end - def update_distribution_charge - @order.update_distribution_charge! - end - private def order_params diff --git a/app/controllers/spree/orders_controller.rb b/app/controllers/spree/orders_controller.rb index a25f43b1a5..7eb12a39c5 100644 --- a/app/controllers/spree/orders_controller.rb +++ b/app/controllers/spree/orders_controller.rb @@ -75,11 +75,18 @@ module Spree redirect_to(main_app.root_path) && return end + # This action is called either from the cart page when the order is not yet complete, or from + # the edit order page (frontoffice) if the hub allows users to update completed orders. + # This line item updating logic should probably be handled through CartService#populate. if @order.update(order_params) discard_empty_line_items - with_open_adjustments { update_totals_and_taxes } - @order.update_distribution_charge! + @order.recreate_all_fees! # Enterprise fees on line items and on the order itself + + if @order.complete? + @order.update_shipping_fees! + @order.update_payment_fees! + end respond_with(@order) do |format| format.html do @@ -148,30 +155,6 @@ module Spree private - # Updates the various denormalized total attributes of the order and - # recalculates the shipment taxes - def update_totals_and_taxes - @order.updater.update_totals - @order.shipment&.ensure_correct_adjustment - end - - # Sets the adjustments to open to perform the block's action and restores - # their state to whatever the they had. Note that it does not change any new - # adjustments that might get created in the yielded block. - def with_open_adjustments - previous_states = @order.adjustments.each_with_object({}) do |adjustment, hash| - hash[adjustment.id] = adjustment.state - end - @order.adjustments.each { |adjustment| adjustment.fire_events(:open) } - - yield - - @order.adjustments.each do |adjustment| - previous_state = previous_states[adjustment.id] - adjustment.update_attribute(:state, previous_state) if previous_state - end - end - def discard_empty_line_items @order.line_items = @order.line_items.select { |li| li.quantity > 0 } end diff --git a/app/models/enterprise_fee.rb b/app/models/enterprise_fee.rb index f1161bb8ab..b7a3eeb6e2 100644 --- a/app/models/enterprise_fee.rb +++ b/app/models/enterprise_fee.rb @@ -40,8 +40,8 @@ class EnterpriseFee < ActiveRecord::Base joins(:calculator).where('spree_calculators.type IN (?)', PER_ORDER_CALCULATORS) } - def self.clear_all_adjustments_on_order(order) - order.adjustments.where(originator_type: 'EnterpriseFee').destroy_all + def self.clear_all_adjustments(order) + order.adjustments.enterprise_fee.destroy_all end private diff --git a/app/models/spree/adjustment.rb b/app/models/spree/adjustment.rb index 3ffd9d8cac..eb970bc659 100644 --- a/app/models/spree/adjustment.rb +++ b/app/models/spree/adjustment.rb @@ -111,8 +111,8 @@ module Spree # more than on line items at once via accepted_nested_attributes the order # object on the association would be in a old state and therefore the # adjustment calculations would not performed on proper values - def update!(calculable = nil) - return if immutable? + def update!(calculable = nil, force: false) + return if immutable? && !force # Fix for Spree issue #3381 # If we attempt to call 'source' before the reload, then source is currently diff --git a/app/models/spree/order.rb b/app/models/spree/order.rb index 64278ed400..a8aa560875 100644 --- a/app/models/spree/order.rb +++ b/app/models/spree/order.rb @@ -68,6 +68,9 @@ module Spree accepts_nested_attributes_for :shipments delegate :admin_and_handling_total, :payment_fee, :ship_total, to: :adjustments_fetcher + delegate :update_totals, to: :updater + delegate :create_line_item_fees!, :create_order_fees!, :update_order_fees!, + :update_line_item_fees!, :recreate_all_fees!, to: :fee_handler # Needs to happen before save_permalink is called before_validation :set_currency @@ -268,8 +271,6 @@ module Spree updater.update end - delegate :update_totals, to: :updater - def clone_billing_address if bill_address && ship_address.nil? self.ship_address = bill_address.clone @@ -660,29 +661,6 @@ module Spree end end - def update_distribution_charge! - # `with_lock` acquires an exclusive row lock on order so no other - # requests can update it until the transaction is commited. - # See https://github.com/rails/rails/blob/3-2-stable/activerecord/lib/active_record/locking/pessimistic.rb#L69 - # and https://www.postgresql.org/docs/current/static/sql-select.html#SQL-FOR-UPDATE-SHARE - with_lock do - EnterpriseFee.clear_all_adjustments_on_order self - - loaded_line_items = - line_items.includes(variant: :product, order: [:distributor, :order_cycle]).all - - loaded_line_items.each do |line_item| - if provided_by_order_cycle? line_item - OpenFoodNetwork::EnterpriseFeeCalculator.new.create_line_item_adjustments_for line_item - end - end - - if order_cycle - OpenFoodNetwork::EnterpriseFeeCalculator.new.create_order_adjustments_for self - end - end - end - def set_order_cycle!(order_cycle) return if self.order_cycle == order_cycle @@ -753,6 +731,10 @@ module Spree private + def fee_handler + @fee_handler ||= OrderFeesHandler.new(self) + end + def process_each_payment raise Core::GatewayError, Spree.t(:no_pending_payments) if pending_payments.empty? @@ -829,11 +811,6 @@ module Spree subscription.present? && order_cycle.orders_close_at.andand > Time.zone.now end - def provided_by_order_cycle?(line_item) - order_cycle_variants = order_cycle.andand.variants || [] - order_cycle_variants.include? line_item.variant - end - def require_customer? return true unless new_record? || state == 'cart' end @@ -867,11 +844,8 @@ module Spree def update_adjustment!(adjustment) return if adjustment.finalized? - state = adjustment.state - adjustment.state = 'open' - adjustment.update! + adjustment.update!(force: true) update! - adjustment.state = state end # object_params sets the payment amount to the order total, but it does this diff --git a/app/services/order_factory.rb b/app/services/order_factory.rb index bb2a99d018..3d8510a0df 100644 --- a/app/services/order_factory.rb +++ b/app/services/order_factory.rb @@ -78,7 +78,7 @@ class OrderFactory end def create_payment - @order.update_distribution_charge! + @order.recreate_all_fees! @order.payments.create(payment_method_id: attrs[:payment_method_id], amount: @order.reload.total) end diff --git a/app/services/order_fees_handler.rb b/app/services/order_fees_handler.rb new file mode 100644 index 0000000000..3fd09bdb39 --- /dev/null +++ b/app/services/order_fees_handler.rb @@ -0,0 +1,61 @@ +# frozen_string_literal: true + +class OrderFeesHandler + attr_reader :order, :distributor, :order_cycle + + def initialize(order) + @order = order + @distributor = order.distributor + @order_cycle = order.order_cycle + end + + def recreate_all_fees! + # `with_lock` acquires an exclusive row lock on order so no other + # requests can update it until the transaction is commited. + # See https://github.com/rails/rails/blob/3-2-stable/activerecord/lib/active_record/locking/pessimistic.rb#L69 + # and https://www.postgresql.org/docs/current/static/sql-select.html#SQL-FOR-UPDATE-SHARE + order.with_lock do + EnterpriseFee.clear_all_adjustments order + + create_line_item_fees! + create_order_fees! + end + end + + def create_line_item_fees! + order.line_items.includes(variant: :product).each do |line_item| + if provided_by_order_cycle? line_item + calculator.create_line_item_adjustments_for line_item + end + end + end + + def create_order_fees! + return unless order_cycle + + calculator.create_order_adjustments_for order + end + + def update_line_item_fees!(line_item) + line_item.adjustments.enterprise_fee.each do |fee| + fee.update!(line_item, force: true) + end + end + + def update_order_fees! + order.adjustments.enterprise_fee.where(source_type: 'Spree::Order').each do |fee| + fee.update!(order, force: true) + end + end + + private + + def calculator + @calculator ||= OpenFoodNetwork::EnterpriseFeeCalculator.new(distributor, order_cycle) + end + + def provided_by_order_cycle?(line_item) + @order_cycle_variant_ids ||= order_cycle&.variants&.map(&:id) || [] + @order_cycle_variant_ids.include? line_item.variant_id + end +end diff --git a/lib/open_food_network/enterprise_fee_calculator.rb b/lib/open_food_network/enterprise_fee_calculator.rb index 08eec58899..47db702182 100644 --- a/lib/open_food_network/enterprise_fee_calculator.rb +++ b/lib/open_food_network/enterprise_fee_calculator.rb @@ -39,8 +39,6 @@ module OpenFoodNetwork def create_line_item_adjustments_for(line_item) variant = line_item.variant - @distributor = line_item.order.distributor - @order_cycle = line_item.order.order_cycle per_item_enterprise_fee_applicators_for(variant).each do |applicator| applicator.create_line_item_adjustment(line_item) @@ -48,9 +46,6 @@ module OpenFoodNetwork end def create_order_adjustments_for(order) - @distributor = order.distributor - @order_cycle = order.order_cycle - per_order_enterprise_fee_applicators_for(order).each do |applicator| applicator.create_order_adjustment(order) end diff --git a/spec/controllers/admin/bulk_line_items_controller_spec.rb b/spec/controllers/admin/bulk_line_items_controller_spec.rb index d40e0682d2..4d94591a0e 100644 --- a/spec/controllers/admin/bulk_line_items_controller_spec.rb +++ b/spec/controllers/admin/bulk_line_items_controller_spec.rb @@ -210,7 +210,9 @@ describe Admin::BulkLineItemsController, type: :controller do .to receive(:find).with(line_item1.id.to_s).and_return(line_item1) expect(line_item1.order).to receive(:reload).with(lock: true) - expect(line_item1.order).to receive(:update_distribution_charge!) + expect(line_item1.order).to receive(:update_line_item_fees!) + expect(line_item1.order).to receive(:update_order_fees!) + expect(line_item1.order).to receive(:update!).twice spree_put :update, params end diff --git a/spec/controllers/line_items_controller_spec.rb b/spec/controllers/line_items_controller_spec.rb index c2816fb8f3..473b261745 100644 --- a/spec/controllers/line_items_controller_spec.rb +++ b/spec/controllers/line_items_controller_spec.rb @@ -127,28 +127,31 @@ describe LineItemsController, type: :controller do context "on a completed order with enterprise fees" do let(:user) { create(:user) } - let(:variant) { create(:variant) } + let(:variant1) { create(:variant) } + let(:variant2) { 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: build(:calculator_per_item) ) } - let!(:exchange) { create(:exchange, incoming: true, sender: variant.product.supplier, receiver: order_cycle.coordinator, variants: [variant], enterprise_fees: [enterprise_fee]) } + let(:calculator) { Calculator::PriceSack.new(preferred_minimal_amount: 15, preferred_normal_amount: 22, preferred_discount_amount: 11) } + let(:enterprise_fee) { create(:enterprise_fee, calculator: calculator) } + let!(:exchange) { create(:exchange, incoming: true, sender: variant1.product.supplier, receiver: order_cycle.coordinator, variants: [variant1, variant2], enterprise_fees: [enterprise_fee]) } let!(:order) do - order = create(:completed_order_with_totals, user: user, distributor: distributor, order_cycle: order_cycle, line_items_count: 1) - order.reload.line_items.first.update(variant_id: variant.id) + order = create(:completed_order_with_totals, user: user, distributor: distributor, order_cycle: order_cycle, line_items_count: 2) + order.reload.line_items.first.update(variant_id: variant1.id) + order.line_items.last.update(variant_id: variant2.id) while !order.completed? do break unless order.next! end - order.update_distribution_charge! + order.recreate_all_fees! order end let(:params) { { format: :json, id: order.line_items.first } } it "updates the fees" do - expect(order.reload.adjustment_total).to eq enterprise_fee.calculator.preferred_amount + expect(order.reload.adjustment_total).to eq calculator.preferred_discount_amount allow(controller).to receive_messages spree_current_user: user delete :destroy, params expect(response.status).to eq 204 - expect(order.reload.adjustment_total).to eq 0 + expect(order.reload.adjustment_total).to eq calculator.preferred_normal_amount end end end diff --git a/spec/controllers/spree/admin/orders_controller_spec.rb b/spec/controllers/spree/admin/orders_controller_spec.rb index 817e1c3b3a..f9cae535e5 100644 --- a/spec/controllers/spree/admin/orders_controller_spec.rb +++ b/spec/controllers/spree/admin/orders_controller_spec.rb @@ -56,12 +56,48 @@ describe Spree::Admin::OrdersController, type: :controller do end it "updates distribution charges and redirects to order details page" do - expect_any_instance_of(Spree::Order).to receive(:update_distribution_charge!) + expect_any_instance_of(Spree::Order).to receive(:recreate_all_fees!) spree_put :update, params expect(response).to redirect_to spree.edit_admin_order_path(order) end + + context "recalculating enterprise fees" do + let(:user) { create(:admin_user) } + let(:variant1) { create(:variant) } + let(:variant2) { 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: build(:calculator_per_item) ) } + let!(:exchange) { create(:exchange, incoming: true, sender: variant1.product.supplier, receiver: order_cycle.coordinator, variants: [variant1, variant2], enterprise_fees: [enterprise_fee]) } + let!(:order) do + order = create(:completed_order_with_totals, line_items_count: 2, distributor: distributor, order_cycle: order_cycle) + order.reload.line_items.first.update(variant_id: variant1.id) + 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 + + before do + allow(controller).to receive(:spree_current_user) { user } + allow(controller).to receive(:order_to_update) { order } + end + + it "recalculates fees if the orders contents have changed" do + expect(order.total).to eq order.item_total + (enterprise_fee.calculator.preferred_amount * 2) + expect(order.adjustment_total).to eq enterprise_fee.calculator.preferred_amount * 2 + + order.contents.add(order.line_items.first.variant, 1) + + spree_put :update, { id: order.number } + + expect(order.reload.total).to eq order.item_total + (enterprise_fee.calculator.preferred_amount * 3) + expect(order.adjustment_total).to eq enterprise_fee.calculator.preferred_amount * 3 + end + end end context "incomplete order" do @@ -86,7 +122,7 @@ describe Spree::Admin::OrdersController, type: :controller do context "and no errors" do it "updates distribution charges and redirects to customer details page" do - expect_any_instance_of(Spree::Order).to receive(:update_distribution_charge!) + expect_any_instance_of(Spree::Order).to receive(:recreate_all_fees!) spree_put :update, params diff --git a/spec/controllers/spree/orders_controller_spec.rb b/spec/controllers/spree/orders_controller_spec.rb index dcc25c2a00..e84f747262 100644 --- a/spec/controllers/spree/orders_controller_spec.rb +++ b/spec/controllers/spree/orders_controller_spec.rb @@ -286,43 +286,34 @@ describe Spree::OrdersController, type: :controller do allow(subject).to receive(:order_to_update) { order } end - it "updates the fees" do + it "updates the shipping and payment fees" do spree_post :update, order: { line_items_attributes: { "0" => { id: line_item1.id, quantity: 1 }, "1" => { id: line_item2.id, quantity: 0 } } } - expect(order.line_items.count).to eq 1 - expect(order.adjustment_total).to eq((item_num - 1) * (shipping_fee + payment_fee)) + expect(order.reload.line_items.count).to eq 1 + expect(order.adjustment_total).to eq(1 * (shipping_fee + payment_fee)) expect(order.shipment.adjustment.included_tax).to eq 0.6 end - - it "keeps the adjustments' previous state" do - spree_post :update, - order: { line_items_attributes: { - "0" => { id: line_item1.id, quantity: 1 }, - "1" => { id: line_item2.id, quantity: 0 } - } } - - # The second adjustment (shipping adjustment) is open before the update - # so, restoring its state leaves it open. - expect(order.adjustments.map(&:state)).to eq(['closed', 'open']) - end end context "with enterprise fees" do let(:user) { create(:user) } - let(:variant) { create(:variant) } + let(:variant1) { create(:variant) } + let(:variant2) { 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: build(:calculator_per_item) ) } - let!(:exchange) { create(:exchange, incoming: true, sender: variant.product.supplier, receiver: order_cycle.coordinator, variants: [variant], enterprise_fees: [enterprise_fee]) } + let!(:exchange) { create(:exchange, incoming: true, sender: variant1.product.supplier, receiver: order_cycle.coordinator, variants: [variant1, variant2], enterprise_fees: [enterprise_fee]) } let!(:order) do - order = create(:completed_order_with_totals, line_items_count: 1, user: user, distributor: distributor, order_cycle: order_cycle) - order.reload.line_items.first.update(variant_id: variant.id) + order = create(:completed_order_with_totals, line_items_count: 2, user: user, distributor: distributor, order_cycle: order_cycle) + order.reload.line_items.first.update(variant_id: variant1.id) + order.reload.line_items.last.update(variant_id: variant2.id) while !order.completed? do break unless order.next! end - order.update_distribution_charge! + order.recreate_all_fees! + order.update! order end let(:params) { @@ -337,12 +328,34 @@ describe Spree::OrdersController, type: :controller do end it "updates the fees" do - expect(order.reload.adjustment_total).to eq enterprise_fee.calculator.preferred_amount + expect(order.total).to eq order.item_total + (enterprise_fee.calculator.preferred_amount * 2) + expect(order.adjustment_total).to eq enterprise_fee.calculator.preferred_amount * 2 allow(controller).to receive_messages spree_current_user: user spree_post :update, params - expect(order.reload.adjustment_total).to eq enterprise_fee.calculator.preferred_amount * 2 + expect(order.total).to eq order.item_total + (enterprise_fee.calculator.preferred_amount * 3) + expect(order.adjustment_total).to eq enterprise_fee.calculator.preferred_amount * 3 + end + + context "when a line item is removed" do + let(:params) { + { order: { line_items_attributes: { + "0" => { id: order.line_items.first.id, quantity: 0 }, + "1" => { id: order.line_items.last.id, quantity: 1 } + } } } + } + + it "updates the fees" do + expect(order.total).to eq order.item_total + (enterprise_fee.calculator.preferred_amount * 2) + expect(order.adjustment_total).to eq enterprise_fee.calculator.preferred_amount * 2 + + allow(controller).to receive_messages spree_current_user: user + spree_post :update, params + + expect(order.total).to eq order.item_total + (enterprise_fee.calculator.preferred_amount * 1) + expect(order.adjustment_total).to eq enterprise_fee.calculator.preferred_amount * 1 + end end end end diff --git a/spec/factories/order_factory.rb b/spec/factories/order_factory.rb index aec931b752..4f24d9b767 100644 --- a/spec/factories/order_factory.rb +++ b/spec/factories/order_factory.rb @@ -90,10 +90,11 @@ FactoryBot.define do after(:create) do |order, evaluator| create(:payment, state: "checkout", order: order, amount: order.total, payment_method: evaluator.payment_method) - order.update_distribution_charge! + order.recreate_all_fees! order.ship_address = evaluator.ship_address while !order.completed? do break unless a = order.next! end order.select_shipping_method(evaluator.shipping_method.id) + order.save end end end diff --git a/spec/features/admin/reports_spec.rb b/spec/features/admin/reports_spec.rb index fe5df92355..6123e14b24 100644 --- a/spec/features/admin/reports_spec.rb +++ b/spec/features/admin/reports_spec.rb @@ -185,7 +185,7 @@ feature ' 2.times { order1.next } order1.select_shipping_method shipping_method.id - order1.reload.update_distribution_charge! + order1.reload.recreate_all_fees! order1.finalize! login_as_admin_and_visit spree.admin_reports_path diff --git a/spec/features/consumer/shopping/cart_spec.rb b/spec/features/consumer/shopping/cart_spec.rb index d6c8a7909c..0994bd9dc6 100644 --- a/spec/features/consumer/shopping/cart_spec.rb +++ b/spec/features/consumer/shopping/cart_spec.rb @@ -132,7 +132,7 @@ feature "full-page cart", js: true do cart_service = CartService.new(order) cart_service.populate(variants: { product_with_fee.variants.first.id => 3, product_with_tax.variants.first.id => 3 }) - order.update_distribution_charge! + order.recreate_all_fees! visit main_app.cart_path end diff --git a/spec/lib/open_food_network/enterprise_fee_calculator_spec.rb b/spec/lib/open_food_network/enterprise_fee_calculator_spec.rb index 0dc9b121b4..97068516f9 100644 --- a/spec/lib/open_food_network/enterprise_fee_calculator_spec.rb +++ b/spec/lib/open_food_network/enterprise_fee_calculator_spec.rb @@ -141,7 +141,7 @@ module OpenFoodNetwork it "creates adjustments for a line item" do exchange.enterprise_fees << enterprise_fee_line_item - EnterpriseFeeCalculator.new.create_line_item_adjustments_for line_item + EnterpriseFeeCalculator.new(distributor, order_cycle).create_line_item_adjustments_for line_item a = Spree::Adjustment.last expect(a.metadata.fee_name).to eq(enterprise_fee_line_item.name) @@ -150,7 +150,7 @@ module OpenFoodNetwork it "creates adjustments for an order" do exchange.enterprise_fees << enterprise_fee_order - EnterpriseFeeCalculator.new.create_order_adjustments_for order + EnterpriseFeeCalculator.new(distributor, order_cycle).create_order_adjustments_for order a = Spree::Adjustment.last expect(a.metadata.fee_name).to eq(enterprise_fee_order.name) diff --git a/spec/models/enterprise_fee_spec.rb b/spec/models/enterprise_fee_spec.rb index b5065ecee7..9cf76958d5 100644 --- a/spec/models/enterprise_fee_spec.rb +++ b/spec/models/enterprise_fee_spec.rb @@ -110,7 +110,7 @@ describe EnterpriseFee do order_cycle.exchanges[0].enterprise_fees[0].create_adjustment('foo4', line_item2.order, line_item2, true) expect do - EnterpriseFee.clear_all_adjustments_on_order order + EnterpriseFee.clear_all_adjustments order end.to change(order.adjustments, :count).by(-4) end @@ -121,7 +121,7 @@ describe EnterpriseFee do enterprise_fee_aplicator.create_order_adjustment(order) expect do - EnterpriseFee.clear_all_adjustments_on_order order + EnterpriseFee.clear_all_adjustments order end.to change(order.adjustments, :count).by(-1) end @@ -135,7 +135,7 @@ describe EnterpriseFee do label: 'hello' }) expect do - EnterpriseFee.clear_all_adjustments_on_order order + EnterpriseFee.clear_all_adjustments order end.to change(order.adjustments, :count).by(0) end end diff --git a/spec/models/spree/adjustment_spec.rb b/spec/models/spree/adjustment_spec.rb index 0fab3d3cbd..58c4caa00d 100644 --- a/spec/models/spree/adjustment_spec.rb +++ b/spec/models/spree/adjustment_spec.rb @@ -45,6 +45,20 @@ module Spree expect(originator).to receive(:update_adjustment) adjustment.update! end + + context "using the :force argument" do + it "should update adjustments without changing their state" do + expect(originator).to receive(:update_adjustment) + adjustment.update!(force: true) + expect(adjustment.state).to eq "open" + end + + it "should update closed adjustments" do + adjustment.close + expect(originator).to receive(:update_adjustment) + adjustment.update!(force: true) + end + end end it "should do nothing when originator is nil" do @@ -307,7 +321,7 @@ module Spree context "when enterprise fees have a fixed tax_category" do before do - order.reload.update_distribution_charge! + order.reload.recreate_all_fees! end context "when enterprise fees are taxed per-order" do @@ -326,7 +340,7 @@ module Spree before do fee_tax_rate.update_attribute :included_in_price, false order.reload.create_tax_charge! # Updating line_item or order has the same effect - order.update_distribution_charge! + order.recreate_all_fees! end it "records the tax on TaxRate adjustment on the order" do @@ -339,7 +353,7 @@ module Spree before do enterprise_fee.tax_category = nil enterprise_fee.save! - order.update_distribution_charge! + order.recreate_all_fees! end it "records no tax as charged" do @@ -361,7 +375,7 @@ module Spree before do fee_tax_rate.update_attribute :included_in_price, false order.reload.create_tax_charge! # Updating line_item or order has the same effect - order.update_distribution_charge! + order.recreate_all_fees! end it "records the tax on TaxRate adjustment on the order" do @@ -382,7 +396,7 @@ module Spree variant.product.update_attribute(:tax_category_id, product_tax_category.id) order.create_tax_charge! # Updating line_item or order has the same effect - order.update_distribution_charge! + order.recreate_all_fees! end context "when enterprise fees are taxed per-order" do @@ -402,7 +416,7 @@ module Spree before do product_tax_rate.update_attribute :included_in_price, false order.reload.create_tax_charge! # Updating line_item or order has the same effect - order.reload.update_distribution_charge! + order.reload.recreate_all_fees! end it "records the no tax on TaxRate adjustment on the order" do @@ -432,7 +446,7 @@ module Spree before do product_tax_rate.update_attribute :included_in_price, false order.reload.create_tax_charge! # Updating line_item or order has the same effect - order.update_distribution_charge! + order.recreate_all_fees! end it "records the tax on TaxRate adjustment on the order" do diff --git a/spec/models/spree/order_spec.rb b/spec/models/spree/order_spec.rb index 73f2b228e1..c886d5677d 100644 --- a/spec/models/spree/order_spec.rb +++ b/spec/models/spree/order_spec.rb @@ -533,26 +533,36 @@ describe Spree::Order do end end - describe "updating the distribution charge" do - let(:order) { build(:order) } + describe "applying enterprise fees" do + let(:fee_handler) { ::OrderFeesHandler.new(subject) } + + before do + allow(subject).to receive(:fee_handler) { fee_handler } + end it "clears all enterprise fee adjustments on the order" do - expect(EnterpriseFee).to receive(:clear_all_adjustments_on_order).with(subject) - subject.update_distribution_charge! + expect(EnterpriseFee).to receive(:clear_all_adjustments).with(subject) + subject.recreate_all_fees! + end + + it "creates line item and order fee adjustments via OrderFeesHandler" do + expect(fee_handler).to receive(:create_line_item_fees!) + expect(fee_handler).to receive(:create_order_fees!) + subject.recreate_all_fees! end it "skips order cycle per-order adjustments for orders that don't have an order cycle" do - allow(EnterpriseFee).to receive(:clear_all_adjustments_on_order) + allow(EnterpriseFee).to receive(:clear_all_adjustments) allow(subject).to receive(:order_cycle) { nil } - subject.update_distribution_charge! + subject.recreate_all_fees! end it "ensures the correct adjustment(s) are created for order cycles" do - allow(EnterpriseFee).to receive(:clear_all_adjustments_on_order) + allow(EnterpriseFee).to receive(:clear_all_adjustments) line_item = create(:line_item, order: subject) - allow(subject).to receive(:provided_by_order_cycle?) { true } + allow(fee_handler).to receive(:provided_by_order_cycle?) { true } order_cycle = double(:order_cycle) expect_any_instance_of(OpenFoodNetwork::EnterpriseFeeCalculator). @@ -561,11 +571,11 @@ describe Spree::Order do allow_any_instance_of(OpenFoodNetwork::EnterpriseFeeCalculator).to receive(:create_order_adjustments_for) allow(subject).to receive(:order_cycle) { order_cycle } - subject.update_distribution_charge! + subject.recreate_all_fees! end it "ensures the correct per-order adjustment(s) are created for order cycles" do - allow(EnterpriseFee).to receive(:clear_all_adjustments_on_order) + allow(EnterpriseFee).to receive(:clear_all_adjustments) order_cycle = double(:order_cycle) expect_any_instance_of(OpenFoodNetwork::EnterpriseFeeCalculator). @@ -574,35 +584,7 @@ describe Spree::Order do allow(subject).to receive(:order_cycle) { order_cycle } - subject.update_distribution_charge! - end - end - - describe "looking up whether a line item can be provided by an order cycle" do - it "returns true when the variant is provided" do - v = double(:variant) - line_item = double(:line_item, variant: v) - order_cycle = double(:order_cycle, variants: [v]) - allow(subject).to receive(:order_cycle) { order_cycle } - - expect(subject.send(:provided_by_order_cycle?, line_item)).to be true - end - - it "returns false otherwise" do - v = double(:variant) - line_item = double(:line_item, variant: v) - order_cycle = double(:order_cycle, variants: []) - allow(subject).to receive(:order_cycle) { order_cycle } - - expect(subject.send(:provided_by_order_cycle?, line_item)).to be false - end - - it "returns false when there is no order cycle" do - v = double(:variant) - line_item = double(:line_item, variant: v) - allow(subject).to receive(:order_cycle) { nil } - - expect(subject.send(:provided_by_order_cycle?, line_item)).to be false + subject.recreate_all_fees! end end @@ -810,7 +792,7 @@ describe Spree::Order do order.add_variant v1 order.add_variant v2 - order.update_distribution_charge! + order.recreate_all_fees! end it "removes the variant's line item" do diff --git a/spec/models/tag_rule/discount_order_spec.rb b/spec/models/tag_rule/discount_order_spec.rb index 4ea426a278..1da5635c5e 100644 --- a/spec/models/tag_rule/discount_order_spec.rb +++ b/spec/models/tag_rule/discount_order_spec.rb @@ -56,7 +56,7 @@ describe TagRule::DiscountOrder, type: :model do let!(:order) { line_item.order } before do - order.update_distribution_charge! + order.recreate_all_fees! tag_rule.calculator.update_attribute(:preferred_flat_percent, -10.00) tag_rule.context = { subject: order } end diff --git a/spec/services/order_fees_handler_spec.rb b/spec/services/order_fees_handler_spec.rb new file mode 100644 index 0000000000..f0358a49c9 --- /dev/null +++ b/spec/services/order_fees_handler_spec.rb @@ -0,0 +1,61 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe OrderFeesHandler do + let(:order_cycle) { create(:order_cycle) } + let(:order) { create(:order_with_line_items, line_items_count: 1, order_cycle: order_cycle) } + let(:line_item) { order.line_items.first } + + let(:service) { OrderFeesHandler.new(order) } + let(:calculator) { + double(OpenFoodNetwork::EnterpriseFeeCalculator, create_order_adjustments_for: true) + } + + before do + allow(service).to receive(:calculator) { calculator } + end + + describe "#create_line_item_fees!" do + it "creates per-line-item fee adjustments for line items in the order cycle" do + allow(service).to receive(:provided_by_order_cycle?) { true } + expect(calculator).to receive(:create_line_item_adjustments_for).with(line_item) + + service.create_line_item_fees! + end + end + + describe "#create_order_fees!" do + it "creates per-order adjustment for the order cycle" do + expect(calculator).to receive(:create_order_adjustments_for).with(order) + service.create_order_fees! + end + + it "skips per-order fee adjustments for orders that don't have an order cycle" do + allow(service).to receive(:order_cycle) { nil } + expect(calculator).to_not receive(:create_order_adjustments_for) + + service.create_order_fees! + end + end + + context "checking if a line item can be provided by the order cycle" do + it "returns true when the variant is provided" do + allow(order_cycle).to receive(:variants) { [line_item.variant] } + + expect(service.__send__(:provided_by_order_cycle?, line_item)).to be true + end + + it "returns false otherwise" do + allow(order_cycle).to receive(:variants) { [] } + + expect(service.__send__(:provided_by_order_cycle?, line_item)).to be false + end + + it "returns false when there is no order cycle" do + allow(order).to receive(:order_cycle) { nil } + + expect(service.__send__(:provided_by_order_cycle?, line_item)).to be false + end + end +end diff --git a/spec/services/order_tax_adjustments_fetcher_spec.rb b/spec/services/order_tax_adjustments_fetcher_spec.rb index 4319f17b4d..ed86a1f15e 100644 --- a/spec/services/order_tax_adjustments_fetcher_spec.rb +++ b/spec/services/order_tax_adjustments_fetcher_spec.rb @@ -81,7 +81,7 @@ describe OrderTaxAdjustmentsFetcher do before do order.create_tax_charge! - order.update_distribution_charge! + order.recreate_all_fees! end subject { OrderTaxAdjustmentsFetcher.new(order).totals } diff --git a/spec/support/request/shop_workflow.rb b/spec/support/request/shop_workflow.rb index 738d9f7113..d6859c2f9f 100644 --- a/spec/support/request/shop_workflow.rb +++ b/spec/support/request/shop_workflow.rb @@ -42,7 +42,7 @@ module ShopWorkflow cart_service.populate(variants: { product.variants.first.id => quantity }) # Recalculate fee totals - order.update_distribution_charge! + order.recreate_all_fees! end # Add an item to the cart