diff --git a/app/models/enterprise_fee.rb b/app/models/enterprise_fee.rb index 64f1c84bca..07eab69415 100644 --- a/app/models/enterprise_fee.rb +++ b/app/models/enterprise_fee.rb @@ -42,8 +42,8 @@ class EnterpriseFee < ApplicationRecord joins(:calculator).where(spree_calculators: { type: PER_ORDER_CALCULATORS }) } - def self.clear_order_adjustments(order) - order.all_adjustments.enterprise_fee.where.not(adjustable_type: "Spree::LineItem").destroy_all + def self.clear_all_adjustments(order) + order.all_adjustments.enterprise_fee.destroy_all end private diff --git a/app/models/spree/order.rb b/app/models/spree/order.rb index 9fd75f2a8f..66bfdb3db3 100644 --- a/app/models/spree/order.rb +++ b/app/models/spree/order.rb @@ -86,7 +86,7 @@ module Spree delegate :admin_and_handling_total, :payment_fee, :ship_total, to: :adjustments_fetcher delegate :update_totals, :update_totals_and_states, to: :updater - delegate :create_order_fees!, :update_order_fees!, + delegate :create_line_item_fees!, :create_order_fees!, :update_order_fees!, :update_line_item_fees!, :recreate_all_fees!, to: :fee_handler validates :customer, presence: true, if: :require_customer? diff --git a/app/services/orders/handle_fees_service.rb b/app/services/orders/handle_fees_service.rb index 46d3967566..09ee04f1a2 100644 --- a/app/services/orders/handle_fees_service.rb +++ b/app/services/orders/handle_fees_service.rb @@ -16,12 +16,9 @@ module Orders # 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_order_adjustments order + EnterpriseFee.clear_all_adjustments order - # To prevent issue with fee being removed when a product is not linked to the order cycle - # anymore, we now create or update line item fees. - # Previously fees were deleted and recreated, like we still do for order fees. - create_or_update_line_item_fees! + create_line_item_fees! create_order_fees! end @@ -29,17 +26,11 @@ module Orders order.update_order! end - def create_or_update_line_item_fees! - order.line_items.includes(:variant).each do |line_item| - # No fee associated with the line item so we just create them - if line_item.enterprise_fee_adjustments.blank? - create_line_item_fees!(line_item) - next + 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 - - create_or_update_line_item_fee!(line_item) - - delete_removed_fees!(line_item) end end @@ -75,34 +66,5 @@ module Orders @order_cycle_variant_ids ||= order_cycle&.variants&.map(&:id) || [] @order_cycle_variant_ids.include? line_item.variant_id end - - def create_line_item_fees!(line_item) - return unless provided_by_order_cycle? line_item - - calculator.create_line_item_adjustments_for(line_item) - end - - def create_or_update_line_item_fee!(line_item) - fee_applicators(line_item.variant).each do |fee_applicator| - fee_adjustment = line_item.adjustments.find_by(originator: fee_applicator.enterprise_fee) - - if fee_adjustment - fee_adjustment.update_adjustment!(line_item, force: true) - elsif provided_by_order_cycle? line_item - fee_applicator.create_line_item_adjustment(line_item) - end - end - end - - def delete_removed_fees!(line_item) - order_cycle_fees = fee_applicators(line_item.variant).map(&:enterprise_fee) - removed_fees = line_item.enterprise_fee_adjustments.where.not(originator: order_cycle_fees) - - removed_fees.each(&:destroy) - end - - def fee_applicators(variant) - calculator.order_cycle_per_item_enterprise_fee_applicators_for(variant) - end end end diff --git a/lib/open_food_network/enterprise_fee_calculator.rb b/lib/open_food_network/enterprise_fee_calculator.rb index 2412e82637..739725ce79 100644 --- a/lib/open_food_network/enterprise_fee_calculator.rb +++ b/lib/open_food_network/enterprise_fee_calculator.rb @@ -67,12 +67,13 @@ module OpenFoodNetwork @order_cycle.exchanges_carrying(variant, @distributor).each do |exchange| exchange.enterprise_fees.per_item.each do |enterprise_fee| - fees << EnterpriseFeeApplicator.new(enterprise_fee, variant, exchange.role) + fees << OpenFoodNetwork::EnterpriseFeeApplicator.new(enterprise_fee, variant, + exchange.role) end end @order_cycle.coordinator_fees.per_item.each do |enterprise_fee| - fees << EnterpriseFeeApplicator.new(enterprise_fee, variant, 'coordinator') + fees << OpenFoodNetwork::EnterpriseFeeApplicator.new(enterprise_fee, variant, 'coordinator') end fees @@ -85,30 +86,12 @@ module OpenFoodNetwork @order_cycle.exchanges_supplying(order).each do |exchange| exchange.enterprise_fees.per_order.each do |enterprise_fee| - fees << EnterpriseFeeApplicator.new(enterprise_fee, nil, exchange.role) + fees << OpenFoodNetwork::EnterpriseFeeApplicator.new(enterprise_fee, nil, exchange.role) end end @order_cycle.coordinator_fees.per_order.each do |enterprise_fee| - fees << EnterpriseFeeApplicator.new(enterprise_fee, nil, 'coordinator') - end - - fees - end - - def order_cycle_per_item_enterprise_fee_applicators_for(variant) - fees = [] - - return fees unless @order_cycle && @distributor - - @order_cycle.exchanges.supplying_to(@distributor).each do |exchange| - exchange.enterprise_fees.per_item.each do |enterprise_fee| - fees << EnterpriseFeeApplicator.new(enterprise_fee, variant, exchange.role) - end - end - - @order_cycle.coordinator_fees.per_item.each do |enterprise_fee| - fees << EnterpriseFeeApplicator.new(enterprise_fee, variant, 'coordinator') + fees << OpenFoodNetwork::EnterpriseFeeApplicator.new(enterprise_fee, nil, 'coordinator') end fees 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 2a011f1a66..58da6263ed 100644 --- a/spec/lib/open_food_network/enterprise_fee_calculator_spec.rb +++ b/spec/lib/open_food_network/enterprise_fee_calculator_spec.rb @@ -3,414 +3,376 @@ require 'spec_helper' require 'open_food_network/enterprise_fee_calculator' -RSpec.describe OpenFoodNetwork::EnterpriseFeeCalculator do - describe "integration" do - let(:supplier1) { create(:supplier_enterprise) } - let(:supplier2) { create(:supplier_enterprise) } - let(:coordinator) { create(:distributor_enterprise) } - let(:distributor) { create(:distributor_enterprise) } - let(:order_cycle) { create(:simple_order_cycle) } - let(:product1) { create(:simple_product, supplier_id: supplier1.id, price: 10.00) } - let(:product2) { create(:simple_product, supplier_id: supplier2.id, price: 20.00) } +module OpenFoodNetwork + RSpec.describe EnterpriseFeeCalculator do + describe "integration" do + let(:supplier1) { create(:supplier_enterprise) } + let(:supplier2) { create(:supplier_enterprise) } + let(:coordinator) { create(:distributor_enterprise) } + let(:distributor) { create(:distributor_enterprise) } + let(:order_cycle) { create(:simple_order_cycle) } + let(:product1) { create(:simple_product, supplier_id: supplier1.id, price: 10.00) } + let(:product2) { create(:simple_product, supplier_id: supplier2.id, price: 20.00) } - describe "calculating fees for a variant" do - describe "summing all the per-item fees for variant in the specified hub + order cycle" do - let(:enterprise_fee1) { create(:enterprise_fee, amount: 20) } - let(:enterprise_fee2) { create(:enterprise_fee, amount: 3) } - let(:enterprise_fee3) { - create(:enterprise_fee, calculator: Calculator::FlatRate.new(preferred_amount: 2)) - } - - describe "supplier fees" do - let!(:exchange1) { - create(:exchange, order_cycle:, sender: supplier1, receiver: coordinator, - incoming: true, enterprise_fees: [enterprise_fee1], - variants: [product1.variants.first]) - } - let!(:exchange2) { - create(:exchange, order_cycle:, sender: supplier2, receiver: coordinator, - incoming: true, enterprise_fees: [enterprise_fee2], - variants: [product2.variants.first]) + describe "calculating fees for a variant" do + describe "summing all the per-item fees for variant in the specified hub + order cycle" do + let(:enterprise_fee1) { create(:enterprise_fee, amount: 20) } + let(:enterprise_fee2) { create(:enterprise_fee, amount: 3) } + let(:enterprise_fee3) { + create(:enterprise_fee, calculator: Calculator::FlatRate.new(preferred_amount: 2)) } - it "calculates via regular computation" do - expect(OpenFoodNetwork::EnterpriseFeeCalculator.new(distributor, order_cycle) - .fees_for(product1.variants.first)).to eq(20) - expect(OpenFoodNetwork::EnterpriseFeeCalculator.new(distributor, order_cycle) - .fees_for(product2.variants.first)).to eq(3) + describe "supplier fees" do + let!(:exchange1) { + create(:exchange, order_cycle:, sender: supplier1, receiver: coordinator, + incoming: true, enterprise_fees: [enterprise_fee1], + variants: [product1.variants.first]) + } + let!(:exchange2) { + create(:exchange, order_cycle:, sender: supplier2, receiver: coordinator, + incoming: true, enterprise_fees: [enterprise_fee2], + variants: [product2.variants.first]) + } + + it "calculates via regular computation" do + expect(EnterpriseFeeCalculator.new(distributor, order_cycle) + .fees_for(product1.variants.first)).to eq(20) + expect(EnterpriseFeeCalculator.new(distributor, order_cycle) + .fees_for(product2.variants.first)).to eq(3) + end + + it "calculates via indexed computation" do + expect(EnterpriseFeeCalculator.new(distributor, order_cycle) + .indexed_fees_for(product1.variants.first)).to eq(20) + expect(EnterpriseFeeCalculator.new(distributor, order_cycle) + .indexed_fees_for(product2.variants.first)).to eq(3) + end end - it "calculates via indexed computation" do - expect(OpenFoodNetwork::EnterpriseFeeCalculator.new(distributor, order_cycle) - .indexed_fees_for(product1.variants.first)).to eq(20) - expect(OpenFoodNetwork::EnterpriseFeeCalculator.new(distributor, order_cycle) - .indexed_fees_for(product2.variants.first)).to eq(3) + describe "coordinator fees" do + let!(:exchange) { + create(:exchange, order_cycle:, sender: coordinator, + receiver: distributor, incoming: false, enterprise_fees: [], + variants: [product1.variants.first]) + } + + before do + order_cycle.coordinator_fees = [enterprise_fee1, enterprise_fee2, enterprise_fee3] + end + + it "sums via regular computation" do + expect(EnterpriseFeeCalculator.new(distributor, order_cycle) + .fees_for(product1.variants.first)).to eq(23) + end + + it "sums via indexed computation" do + expect(EnterpriseFeeCalculator.new(distributor, order_cycle) + .indexed_fees_for(product1.variants.first)).to eq(23) + end + end + + describe "distributor fees" do + let!(:exchange) { + create(:exchange, order_cycle:, sender: coordinator, + receiver: distributor, incoming: false, + enterprise_fees: [enterprise_fee1, + enterprise_fee2, + enterprise_fee3], + variants: [product1.variants.first]) + } + + it "sums via regular computation" do + expect(EnterpriseFeeCalculator.new(distributor, order_cycle) + .fees_for(product1.variants.first)).to eq(23) + end + + it "sums via indexed computation" do + expect(EnterpriseFeeCalculator.new(distributor, order_cycle) + .indexed_fees_for(product1.variants.first)).to eq(23) + end end end - describe "coordinator fees" do - let!(:exchange) { - create(:exchange, order_cycle:, sender: coordinator, - receiver: distributor, incoming: false, enterprise_fees: [], - variants: [product1.variants.first]) + describe "summing percentage fees for the variant" do + let!(:enterprise_fee1) { + create( + :enterprise_fee, + amount: 20, + fee_type: "admin", + calculator: ::Calculator::FlatPercentPerItem.new(preferred_flat_percent: 20) + ) } - - before do - order_cycle.coordinator_fees = [enterprise_fee1, enterprise_fee2, enterprise_fee3] - end - - it "sums via regular computation" do - expect(OpenFoodNetwork::EnterpriseFeeCalculator.new(distributor, order_cycle) - .fees_for(product1.variants.first)).to eq(23) - end - - it "sums via indexed computation" do - expect(OpenFoodNetwork::EnterpriseFeeCalculator.new(distributor, order_cycle) - .indexed_fees_for(product1.variants.first)).to eq(23) - end - end - - describe "distributor fees" do let!(:exchange) { - create(:exchange, order_cycle:, sender: coordinator, - receiver: distributor, incoming: false, - enterprise_fees: [enterprise_fee1, - enterprise_fee2, - enterprise_fee3], + create(:exchange, order_cycle:, sender: coordinator, receiver: distributor, + incoming: false, enterprise_fees: [enterprise_fee1], variants: [product1.variants.first]) } it "sums via regular computation" do - expect(OpenFoodNetwork::EnterpriseFeeCalculator.new(distributor, order_cycle) - .fees_for(product1.variants.first)).to eq(23) + expect(EnterpriseFeeCalculator.new(distributor, order_cycle) + .fees_for(product1.variants.first)).to eq(2.00) end it "sums via indexed computation" do - expect(OpenFoodNetwork::EnterpriseFeeCalculator.new(distributor, order_cycle) - .indexed_fees_for(product1.variants.first)).to eq(23) + expect(EnterpriseFeeCalculator.new(distributor, order_cycle) + .indexed_fees_for(product1.variants.first)).to eq(2.00) end end end - describe "summing percentage fees for the variant" do - let!(:enterprise_fee1) { - create( - :enterprise_fee, - amount: 20, - fee_type: "admin", - calculator: Calculator::FlatPercentPerItem.new(preferred_flat_percent: 20) - ) + describe "calculating fees by type" do + let!(:ef_admin) { create(:enterprise_fee, fee_type: 'admin', amount: 1.23, name: "Admin") } + let!(:ef_sales) { create(:enterprise_fee, fee_type: 'sales', amount: 4.56, name: "Sales") } + let!(:ef_packing) { + create(:enterprise_fee, fee_type: 'packing', amount: 7.89, name: "Packing") + } + let!(:ef_transport) { + create(:enterprise_fee, fee_type: 'transport', amount: 0.12, name: "Transport") + } + let!(:ef_fundraising) { + create(:enterprise_fee, fee_type: 'fundraising', amount: 3.45, name: "Fundraising") } let!(:exchange) { - create(:exchange, order_cycle:, sender: coordinator, receiver: distributor, - incoming: false, enterprise_fees: [enterprise_fee1], + create(:exchange, order_cycle:, + sender: coordinator, receiver: distributor, incoming: false, + enterprise_fees: [ + ef_admin, ef_sales, ef_packing, ef_transport, ef_fundraising + ], variants: [product1.variants.first]) } - it "sums via regular computation" do - expect(OpenFoodNetwork::EnterpriseFeeCalculator.new(distributor, order_cycle) - .fees_for(product1.variants.first)).to eq(2.00) + describe "regular computation" do + it "returns the fees names" do + expect(EnterpriseFeeCalculator + .new(distributor, order_cycle).fees_name_by_type_for(product1.variants.first)) + .to eq({ admin: "Admin", fundraising: "Fundraising", packing: "Packing", + sales: "Sales", transport: "Transport" }) + end + + it "returns a breakdown of fees" do + expect(EnterpriseFeeCalculator.new(distributor, order_cycle) + .fees_by_type_for(product1.variants.first)) + .to eq(admin: 1.23, sales: 4.56, packing: 7.89, transport: 0.12, fundraising: 3.45) + end + + it "filters out zero fees" do + ef_admin.calculator.update_attribute :preferred_amount, 0 + expect(EnterpriseFeeCalculator.new(distributor, order_cycle) + .fees_by_type_for(product1.variants.first)) + .to eq(sales: 4.56, packing: 7.89, transport: 0.12, fundraising: 3.45) + end end - it "sums via indexed computation" do - expect(OpenFoodNetwork::EnterpriseFeeCalculator.new(distributor, order_cycle) - .indexed_fees_for(product1.variants.first)).to eq(2.00) + describe "indexed computation" do + it "returns a breakdown of fees" do + expect(EnterpriseFeeCalculator.new(distributor, order_cycle) + .indexed_fees_by_type_for(product1.variants.first)) + .to eq(admin: 1.23, sales: 4.56, packing: 7.89, transport: 0.12, fundraising: 3.45) + end + + it "filters out zero fees" do + ef_admin.calculator.update_attribute :preferred_amount, 0 + expect(EnterpriseFeeCalculator.new(distributor, order_cycle) + .indexed_fees_by_type_for(product1.variants.first)) + .to eq(sales: 4.56, packing: 7.89, transport: 0.12, fundraising: 3.45) + end + end + end + + describe "creating adjustments" do + let(:order) { create(:order, distributor:, order_cycle:) } + let!(:line_item) { create(:line_item, order:, variant: product1.variants.first) } + let(:enterprise_fee_line_item) { create(:enterprise_fee) } + let(:enterprise_fee_order) { + create(:enterprise_fee, calculator: Calculator::FlatRate.new(preferred_amount: 2)) + } + let!(:exchange) { + create(:exchange, order_cycle:, sender: coordinator, receiver: distributor, + incoming: false, variants: [product1.variants.first]) + } + + before { order.reload } + + it "creates adjustments for a line item" do + exchange.enterprise_fees << enterprise_fee_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) + end + + it "creates adjustments for an order" do + exchange.enterprise_fees << enterprise_fee_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) end end end - describe "calculating fees by type" do - let!(:ef_admin) { create(:enterprise_fee, fee_type: 'admin', amount: 1.23, name: "Admin") } - let!(:ef_sales) { create(:enterprise_fee, fee_type: 'sales', amount: 4.56, name: "Sales") } - let!(:ef_packing) { - create(:enterprise_fee, fee_type: 'packing', amount: 7.89, name: "Packing") - } - let!(:ef_transport) { - create(:enterprise_fee, fee_type: 'transport', amount: 0.12, name: "Transport") - } - let!(:ef_fundraising) { - create(:enterprise_fee, fee_type: 'fundraising', amount: 3.45, name: "Fundraising") - } + describe "indexed fee retrieval" do + subject { EnterpriseFeeCalculator.new distributor, order_cycle } + let(:order_cycle) { create(:simple_order_cycle, coordinator_fees: [ef_coordinator]) } + let(:distributor) { create(:distributor_enterprise) } + let(:distributor_other) { create(:distributor_enterprise) } + let!(:ef_absent) { create(:enterprise_fee) } + let!(:ef_exchange) { create(:enterprise_fee) } + let!(:ef_coordinator) { create(:enterprise_fee) } + let!(:ef_other_distributor) { create(:enterprise_fee) } let!(:exchange) { - create(:exchange, order_cycle:, - sender: coordinator, receiver: distributor, incoming: false, - enterprise_fees: [ - ef_admin, ef_sales, ef_packing, ef_transport, ef_fundraising - ], - variants: [product1.variants.first]) + create(:exchange, sender: order_cycle.coordinator, receiver: distributor, + order_cycle:, enterprise_fees: [ef_exchange], variants: [v]) } + let(:v) { create(:variant) } + let(:indexed_variants) { { v.id => v } } + let(:indexed_enterprise_fees) { subject.instance_variable_get(:@indexed_enterprise_fees) } - describe "regular computation" do - it "returns the fees names" do - expect(OpenFoodNetwork::EnterpriseFeeCalculator.new(distributor, order_cycle) - .fees_name_by_type_for(product1.variants.first)) - .to eq({ admin: "Admin", fundraising: "Fundraising", packing: "Packing", - sales: "Sales", transport: "Transport" }) + before { subject.instance_variable_set(:@indexed_enterprise_fees, {}) } + + describe "fetching enterprise fees with pre-loaded exchange details" do + it "scopes enterprise fees to those on exchanges for the current order cycle" do + expect(subject.__send__(:per_item_enterprise_fees_with_exchange_details)).to eq( + [ef_exchange] + ) end - it "returns a breakdown of fees" do - expect(OpenFoodNetwork::EnterpriseFeeCalculator.new(distributor, order_cycle) - .fees_by_type_for(product1.variants.first)) - .to eq(admin: 1.23, sales: 4.56, packing: 7.89, transport: 0.12, fundraising: 3.45) + it "includes the exchange variant id" do + expect(subject.__send__(:per_item_enterprise_fees_with_exchange_details) + .first.variant_id.to_i).to eq(v.id) end - it "filters out zero fees" do - ef_admin.calculator.update_attribute :preferred_amount, 0 - expect(OpenFoodNetwork::EnterpriseFeeCalculator.new(distributor, order_cycle) - .fees_by_type_for(product1.variants.first)) - .to eq(sales: 4.56, packing: 7.89, transport: 0.12, fundraising: 3.45) + it "does not include outgoing exchanges to other distributors" do + create(:exchange, order_cycle:, sender: order_cycle.coordinator, + receiver: distributor_other, enterprise_fees: [ef_other_distributor], + variants: [v]) + + expect(subject.__send__(:per_item_enterprise_fees_with_exchange_details)).to eq( + [ef_exchange] + ) end end - describe "indexed computation" do - it "returns a breakdown of fees" do - expect(OpenFoodNetwork::EnterpriseFeeCalculator.new(distributor, order_cycle) - .indexed_fees_by_type_for(product1.variants.first)) - .to eq(admin: 1.23, sales: 4.56, packing: 7.89, transport: 0.12, fundraising: 3.45) - end + describe "loading exchange fees" do + let(:exchange_fees) { subject.__send__(:per_item_enterprise_fees_with_exchange_details) } - it "filters out zero fees" do - ef_admin.calculator.update_attribute :preferred_amount, 0 - expect(OpenFoodNetwork::EnterpriseFeeCalculator.new(distributor, order_cycle) - .indexed_fees_by_type_for(product1.variants.first)) - .to eq(sales: 4.56, packing: 7.89, transport: 0.12, fundraising: 3.45) + it "loads exchange fees" do + subject.__send__(:load_exchange_fees, exchange_fees) + expect(indexed_enterprise_fees).to eq(v.id => [ef_exchange]) + end + end + + describe "loading coordinator fees" do + it "loads coordinator fees" do + subject.__send__(:load_coordinator_fees) + expect(indexed_enterprise_fees).to eq(v.id => [ef_coordinator]) end end end describe "creating adjustments" do - let(:order) { create(:order, distributor:, order_cycle:) } - let!(:line_item) { create(:line_item, order:, variant: product1.variants.first) } - let(:enterprise_fee_line_item) { create(:enterprise_fee) } - let(:enterprise_fee_order) { - create(:enterprise_fee, calculator: Calculator::FlatRate.new(preferred_amount: 2)) - } - let!(:exchange) { - create(:exchange, order_cycle:, sender: coordinator, receiver: distributor, - incoming: false, variants: [product1.variants.first]) - } + let(:oc) { OrderCycle.new } + let(:distributor) { double(:distributor) } + let(:ef1) { double(:enterprise_fee) } + let(:ef2) { double(:enterprise_fee) } + let(:ef3) { double(:enterprise_fee) } + let(:incoming_exchange) { double(:exchange, role: 'supplier') } + let(:outgoing_exchange) { double(:exchange, role: 'distributor') } + let(:applicator) { double(:enterprise_fee_applicator) } - before { order.reload } + describe "for a line item" do + let(:variant) { double(:variant) } + let(:line_item) { double(:line_item, variant:, order:) } - it "creates adjustments for a line item" do - exchange.enterprise_fees << enterprise_fee_line_item - - OpenFoodNetwork::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) - end - - it "creates adjustments for an order" do - exchange.enterprise_fees << enterprise_fee_order - - OpenFoodNetwork::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) - end - end - end - - describe "indexed fee retrieval" do - subject { OpenFoodNetwork::EnterpriseFeeCalculator.new distributor, order_cycle } - let(:order_cycle) { create(:simple_order_cycle, coordinator_fees: [ef_coordinator]) } - let(:distributor) { create(:distributor_enterprise) } - let(:distributor_other) { create(:distributor_enterprise) } - let!(:ef_absent) { create(:enterprise_fee) } - let!(:ef_exchange) { create(:enterprise_fee) } - let!(:ef_coordinator) { create(:enterprise_fee) } - let!(:ef_other_distributor) { create(:enterprise_fee) } - let!(:exchange) { - create(:exchange, sender: order_cycle.coordinator, receiver: distributor, - order_cycle:, enterprise_fees: [ef_exchange], variants: [v]) - } - let(:v) { create(:variant) } - let(:indexed_variants) { { v.id => v } } - let(:indexed_enterprise_fees) { subject.instance_variable_get(:@indexed_enterprise_fees) } - - before { subject.instance_variable_set(:@indexed_enterprise_fees, {}) } - - describe "fetching enterprise fees with pre-loaded exchange details" do - it "scopes enterprise fees to those on exchanges for the current order cycle" do - expect(subject.__send__(:per_item_enterprise_fees_with_exchange_details)).to eq( - [ef_exchange] - ) - end - - it "includes the exchange variant id" do - expect(subject.__send__(:per_item_enterprise_fees_with_exchange_details) - .first.variant_id.to_i).to eq(v.id) - end - - it "does not include outgoing exchanges to other distributors" do - create(:exchange, order_cycle:, sender: order_cycle.coordinator, - receiver: distributor_other, enterprise_fees: [ef_other_distributor], - variants: [v]) - - expect(subject.__send__(:per_item_enterprise_fees_with_exchange_details)).to eq( - [ef_exchange] - ) - end - end - - describe "loading exchange fees" do - let(:exchange_fees) { subject.__send__(:per_item_enterprise_fees_with_exchange_details) } - - it "loads exchange fees" do - subject.__send__(:load_exchange_fees, exchange_fees) - expect(indexed_enterprise_fees).to eq(v.id => [ef_exchange]) - end - end - - describe "loading coordinator fees" do - it "loads coordinator fees" do - subject.__send__(:load_coordinator_fees) - expect(indexed_enterprise_fees).to eq(v.id => [ef_coordinator]) - end - end - end - - describe "creating adjustments" do - let(:oc) { OrderCycle.new } - let(:distributor) { double(:distributor) } - let(:ef1) { double(:enterprise_fee) } - let(:ef2) { double(:enterprise_fee) } - let(:ef3) { double(:enterprise_fee) } - let(:incoming_exchange) { double(:exchange, role: 'supplier') } - let(:outgoing_exchange) { double(:exchange, role: 'distributor') } - let(:applicator) { double(:enterprise_fee_applicator) } - - describe "for a line item" do - let(:variant) { double(:variant) } - let(:line_item) { double(:line_item, variant:, order:) } - - before do - allow(incoming_exchange).to receive(:enterprise_fees) { - double(:enterprise_fees, per_item: [ef1]) - } - allow(outgoing_exchange).to receive(:enterprise_fees) { - double(:enterprise_fees, per_item: [ef2]) - } - allow(oc).to receive(:exchanges_carrying) { [incoming_exchange, outgoing_exchange] } - allow(oc).to receive(:coordinator_fees) { double(:coodinator_fees, per_item: [ef3]) } - end - - context "with order_cycle and distributor set" do - let(:efc) { OpenFoodNetwork::EnterpriseFeeCalculator.new(distributor, oc) } - let(:order) { double(:order, distributor:, order_cycle: oc) } - - it "creates an adjustment for each fee" do - expect(efc).to receive(:per_item_enterprise_fee_applicators_for).with(variant) { - [applicator] - } - expect(applicator).to receive(:create_line_item_adjustment).with(line_item) - efc.create_line_item_adjustments_for line_item + before do + allow(incoming_exchange).to receive(:enterprise_fees) { + double(:enterprise_fees, per_item: [ef1]) + } + allow(outgoing_exchange).to receive(:enterprise_fees) { + double(:enterprise_fees, per_item: [ef2]) + } + allow(oc).to receive(:exchanges_carrying) { [incoming_exchange, outgoing_exchange] } + allow(oc).to receive(:coordinator_fees) { double(:coodinator_fees, per_item: [ef3]) } end - it "makes fee applicators for a line item" do - expect(efc.per_item_enterprise_fee_applicators_for(line_item.variant)) - .to eq [OpenFoodNetwork::EnterpriseFeeApplicator.new(ef1, line_item.variant, - 'supplier'), - OpenFoodNetwork::EnterpriseFeeApplicator.new(ef2, line_item.variant, - 'distributor'), - OpenFoodNetwork::EnterpriseFeeApplicator.new(ef3, line_item.variant, - 'coordinator')] + context "with order_cycle and distributor set" do + let(:efc) { EnterpriseFeeCalculator.new(distributor, oc) } + let(:order) { double(:order, distributor:, order_cycle: oc) } + + it "creates an adjustment for each fee" do + expect(efc).to receive(:per_item_enterprise_fee_applicators_for).with(variant) { + [applicator] + } + expect(applicator).to receive(:create_line_item_adjustment).with(line_item) + efc.create_line_item_adjustments_for line_item + end + + it "makes fee applicators for a line item" do + expect(efc.__send__(:per_item_enterprise_fee_applicators_for, line_item.variant)) + .to eq [OpenFoodNetwork::EnterpriseFeeApplicator.new(ef1, line_item.variant, + 'supplier'), + OpenFoodNetwork::EnterpriseFeeApplicator.new(ef2, line_item.variant, + 'distributor'), + OpenFoodNetwork::EnterpriseFeeApplicator.new(ef3, line_item.variant, + 'coordinator')] + end + end + + context "with no order_cycle or distributor set" do + let(:efc) { EnterpriseFeeCalculator.new } + let(:order) { double(:order, distributor: nil, order_cycle: nil) } + + it "does not make applicators for an order" do + expect(efc.__send__(:per_item_enterprise_fee_applicators_for, + line_item.variant)).to eq [] + end end end - context "with no order_cycle or distributor set" do - let(:efc) { OpenFoodNetwork::EnterpriseFeeCalculator.new } - let(:order) { double(:order, distributor: nil, order_cycle: nil) } - - it "does not make applicators for an order" do - expect(efc.per_item_enterprise_fee_applicators_for(line_item.variant)).to eq [] - end - end - end - - describe "for an order" do - before do - allow(incoming_exchange).to receive(:enterprise_fees) { - double(:enterprise_fees, per_order: [ef1]) - } - allow(outgoing_exchange).to receive(:enterprise_fees) { - double(:enterprise_fees, per_order: [ef2]) - } - allow(oc).to receive(:exchanges_supplying) { [incoming_exchange, outgoing_exchange] } - allow(oc).to receive(:coordinator_fees) { double(:coodinator_fees, per_order: [ef3]) } - end - - context "with order_cycle and distributor set" do - let(:efc) { OpenFoodNetwork::EnterpriseFeeCalculator.new(distributor, oc) } - let(:order) { double(:order, distributor:, order_cycle: oc) } - - it "creates an adjustment for each fee" do - expect(efc).to receive(:per_order_enterprise_fee_applicators_for).with(order) { - [applicator] - } - expect(applicator).to receive(:create_order_adjustment).with(order) - efc.create_order_adjustments_for order + describe "for an order" do + before do + allow(incoming_exchange).to receive(:enterprise_fees) { + double(:enterprise_fees, per_order: [ef1]) + } + allow(outgoing_exchange).to receive(:enterprise_fees) { + double(:enterprise_fees, per_order: [ef2]) + } + allow(oc).to receive(:exchanges_supplying) { [incoming_exchange, outgoing_exchange] } + allow(oc).to receive(:coordinator_fees) { double(:coodinator_fees, per_order: [ef3]) } end - it "makes fee applicators for an order" do - expect(efc.per_order_enterprise_fee_applicators_for(order)) - .to eq [OpenFoodNetwork::EnterpriseFeeApplicator.new(ef1, nil, 'supplier'), - OpenFoodNetwork::EnterpriseFeeApplicator.new(ef2, nil, 'distributor'), - OpenFoodNetwork::EnterpriseFeeApplicator.new(ef3, nil, 'coordinator')] + context "with order_cycle and distributor set" do + let(:efc) { EnterpriseFeeCalculator.new(distributor, oc) } + let(:order) { double(:order, distributor:, order_cycle: oc) } + + it "creates an adjustment for each fee" do + expect(efc).to receive(:per_order_enterprise_fee_applicators_for).with(order) { + [applicator] + } + expect(applicator).to receive(:create_order_adjustment).with(order) + efc.create_order_adjustments_for order + end + + it "makes fee applicators for an order" do + expect(efc.__send__(:per_order_enterprise_fee_applicators_for, order)) + .to eq [OpenFoodNetwork::EnterpriseFeeApplicator.new(ef1, nil, 'supplier'), + OpenFoodNetwork::EnterpriseFeeApplicator.new(ef2, nil, 'distributor'), + OpenFoodNetwork::EnterpriseFeeApplicator.new(ef3, nil, 'coordinator')] + end + end + + context "with no order_cycle or distributor set" do + let(:efc) { EnterpriseFeeCalculator.new } + let(:order) { double(:order, distributor: nil, order_cycle: nil) } + + it "does not make applicators for an order" do + expect(efc.__send__(:per_order_enterprise_fee_applicators_for, order)).to eq [] + end end end - - context "with no order_cycle or distributor set" do - let(:efc) { OpenFoodNetwork::EnterpriseFeeCalculator.new } - let(:order) { double(:order, distributor: nil, order_cycle: nil) } - - it "does not make applicators for an order" do - expect(efc.per_order_enterprise_fee_applicators_for(order)).to eq [] - end - end - end - end - - describe "#order_cycle_per_item_enterprise_fee_applicators_for" do - subject { calculator.order_cycle_per_item_enterprise_fee_applicators_for(variant) } - - let(:calculator) { described_class.new(distributor, order_cycle) } - let(:distributor) { create(:distributor_enterprise) } - let(:order_cycle) { create(:order_cycle, distributors: [distributor]) } - let(:variant) { instance_double(Spree::Variant) } - - context "with no distributor" do - let(:order_cycle) { create(:order_cycle) } - let(:distributor) { nil } - - it "return an empty array" do - expect(subject).to eq([]) - end - end - - context "with no order cycle" do - let(:order_cycle) { nil } - - it "return an empty array" do - expect(subject).to eq([]) - end - end - - it "returns enterprise fee applicators including per item exchange fees" do - applicators = subject - exchanges = order_cycle.exchanges.supplying_to(distributor) - fees = exchanges.map { |ex| ex.enterprise_fees.per_item }.flatten - - expect(applicators.map(&:enterprise_fee)).to include(*fees) - end - - it "returns enterprise fee applicators including per item coordinator fees" do - applicators = subject - - expect(applicators.map(&:enterprise_fee)).to include(*order_cycle.coordinator_fees.per_item) end end end diff --git a/spec/models/enterprise_fee_spec.rb b/spec/models/enterprise_fee_spec.rb index fac578eca2..769ab93fb0 100644 --- a/spec/models/enterprise_fee_spec.rb +++ b/spec/models/enterprise_fee_spec.rb @@ -142,22 +142,37 @@ RSpec.describe EnterpriseFee do end end - describe ".clear_order_adjustments" do - let(:order_cycle) { create(:order_cycle) } - let(:order) { create(:order, order_cycle:) } + describe "clearing all enterprise fee adjustments on an order" do + it "clears adjustments from many fees and on all line items" do + order_cycle = create(:order_cycle) + order = create(:order, order_cycle:) + line_item1 = create(:line_item, order:, variant: order_cycle.variants.first) + line_item2 = create(:line_item, order:, variant: order_cycle.variants.second) + + order_cycle.coordinator_fees[0].create_adjustment('foo1', line_item1.order, true) + order_cycle.coordinator_fees[0].create_adjustment('foo2', line_item2.order, true) + order_cycle.exchanges[0].enterprise_fees[0].create_adjustment('foo3', line_item1, true) + order_cycle.exchanges[0].enterprise_fees[0].create_adjustment('foo4', line_item2, true) + + expect do + EnterpriseFee.clear_all_adjustments order + end.to change { order.all_adjustments.count }.by(-4) + end it "clears adjustments from per-order fees" do + order = create(:order) enterprise_fee = create(:enterprise_fee) enterprise_fee_aplicator = OpenFoodNetwork::EnterpriseFeeApplicator.new(enterprise_fee, nil, 'coordinator') enterprise_fee_aplicator.create_order_adjustment(order) expect do - described_class.clear_order_adjustments order + EnterpriseFee.clear_all_adjustments order end.to change { order.adjustments.count }.by(-1) end it "does not clear adjustments from another originator" do + order = create(:order) tax_rate = create(:tax_rate, calculator: build(:calculator)) order.adjustments.create({ amount: 12.34, originator: tax_rate, @@ -165,28 +180,9 @@ RSpec.describe EnterpriseFee do label: 'hello' }) expect do - described_class.clear_order_adjustments order + EnterpriseFee.clear_all_adjustments order end.to change { order.adjustments.count }.by(0) end - - it "doesn't clear adjustments from many fees and on all line items" do - line_item1 = create(:line_item, order:, variant: order_cycle.variants.first) - line_item2 = create(:line_item, order:, variant: order_cycle.variants.second) - - # Order adjustment - fee1 = order_cycle.coordinator_fees[0].create_adjustment('foo1', line_item1.order, true) - fee2 = order_cycle.coordinator_fees[0].create_adjustment('foo2', line_item2.order, true) - # Line item adjustment - fee3 = order_cycle.exchanges[0].enterprise_fees[0].create_adjustment('foo3', line_item1, true) - fee4 = order_cycle.exchanges[0].enterprise_fees[0].create_adjustment('foo4', line_item2, true) - - described_class.clear_order_adjustments order - - adjustments = order.all_adjustments - # does not clear line item adjustments - expect(adjustments).not_to include(fee1, fee2) - expect(adjustments).to include(fee3, fee4) - end end describe "soft-deletion" do diff --git a/spec/models/spree/order_spec.rb b/spec/models/spree/order_spec.rb index b3f26520ef..72326e8dfe 100644 --- a/spec/models/spree/order_spec.rb +++ b/spec/models/spree/order_spec.rb @@ -614,6 +614,64 @@ RSpec.describe Spree::Order do end end + describe "applying enterprise fees" do + subject { create(:order) } + let(:fee_handler) { Orders::HandleFeesService.new(subject) } + + before do + allow(subject).to receive(:fee_handler) { fee_handler } + allow(subject).to receive(:update_order!) + end + + it "clears all enterprise fee adjustments on the order" do + expect(EnterpriseFee).to receive(:clear_all_adjustments).with(subject) + subject.recreate_all_fees! + end + + it "creates line item and order fee adjustments via Orders::HandleFeesService" 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) + + allow(subject).to receive(:order_cycle) { nil } + + subject.recreate_all_fees! + end + + it "ensures the correct adjustment(s) are created for order cycles" do + allow(EnterpriseFee).to receive(:clear_all_adjustments) + line_item = create(:line_item, order: subject) + allow(fee_handler).to receive(:provided_by_order_cycle?) { true } + + order_cycle = double(:order_cycle) + expect_any_instance_of(OpenFoodNetwork::EnterpriseFeeCalculator). + to receive(:create_line_item_adjustments_for). + with(line_item) + allow_any_instance_of(OpenFoodNetwork::EnterpriseFeeCalculator) + .to receive(:create_order_adjustments_for) + allow(subject).to receive(:order_cycle) { order_cycle } + + 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) + + order_cycle = double(:order_cycle) + expect_any_instance_of(OpenFoodNetwork::EnterpriseFeeCalculator). + to receive(:create_order_adjustments_for). + with(subject) + + allow(subject).to receive(:order_cycle) { order_cycle } + + subject.recreate_all_fees! + end + end + describe "getting the admin and handling charge" do let(:o) { create(:order) } let(:li) { create(:line_item, order: o) } diff --git a/spec/services/orders/handle_fees_service_spec.rb b/spec/services/orders/handle_fees_service_spec.rb index 3acbae250c..f31ce21ce2 100644 --- a/spec/services/orders/handle_fees_service_spec.rb +++ b/spec/services/orders/handle_fees_service_spec.rb @@ -9,177 +9,19 @@ RSpec.describe Orders::HandleFeesService do let(:service) { Orders::HandleFeesService.new(order) } let(:calculator) { - instance_double(OpenFoodNetwork::EnterpriseFeeCalculator, create_order_adjustments_for: true) + double(OpenFoodNetwork::EnterpriseFeeCalculator, create_order_adjustments_for: true) } before do allow(service).to receive(:calculator) { calculator } end - describe "#recreate_all_fees!" do - before do - allow(order).to receive(:update_order!) - 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) - it "clears order enterprise fee adjustments on the order" do - expect(EnterpriseFee).to receive(:clear_order_adjustments).with(order) - - service.recreate_all_fees! - end - - # both create_or_update_line_item_fees! and create_order_fees! are tested below, - # so it's enough to check they get called - it "creates line item and order fee adjustments" do - expect(service).to receive(:create_or_update_line_item_fees!) - expect(service).to receive(:create_order_fees!) - - service.recreate_all_fees! - end - - it "updates the order" do - expect(order).to receive(:update_order!) - - service.recreate_all_fees! - end - - it "doesn't create tax adjustment" do - expect(service).not_to receive(:tax_enterprise_fees!) - - service.recreate_all_fees! - end - - context "when after payment state" do - it "creates the tax adjustment for the fees" do - expect(service).to receive(:tax_enterprise_fees!) - - order.update(state: "confirmation") - service.recreate_all_fees! - end - end - end - - describe "#create_or_update_line_item_fees!" do - context "with no existing fee" do - it "creates per line item fee adjustments for line items in the order cylce" do - allow(service).to receive(:provided_by_order_cycle?) { true } - expect(calculator).to receive(:create_line_item_adjustments_for).with(line_item) - - service.create_or_update_line_item_fees! - end - - it "does create fee if variant not in Order Cyle" do - allow(service).to receive(:provided_by_order_cycle?) { false } - expect(calculator).not_to receive(:create_line_item_adjustments_for).with(line_item) - - service.create_or_update_line_item_fees! - end - end - - context "with existing line item fee" do - let(:fee) { order_cycle.exchanges[0].enterprise_fees.first } - let(:role) { order_cycle.exchanges[0].role } - let(:fee_applicator) { - OpenFoodNetwork::EnterpriseFeeApplicator.new(fee, line_item.variant, role) - } - - it "updates the line item fee" do - allow(calculator).to receive( - :order_cycle_per_item_enterprise_fee_applicators_for - ).and_return([fee_applicator]) - adjustment = fee.create_adjustment('foo', line_item, true) - - expect do - service.create_or_update_line_item_fees! - end.to change { adjustment.reload.updated_at } - end - - context "when enterprise fee is removed from the order cycle" do - it "removes the line item fee" do - adjustment = fee.create_adjustment('foo', line_item, true) - order_cycle.exchanges.first.enterprise_fees.destroy(fee) - allow(calculator).to receive( - :order_cycle_per_item_enterprise_fee_applicators_for - ).and_return([]) - - expect do - service.create_or_update_line_item_fees! - end.to change { line_item.adjustments.reload.enterprise_fee.count }.by(-1) - end - end - - context "when an enterprise fee is deleted" do - before do - fee.create_adjustment('foo', line_item, true) - allow(calculator).to receive( - :order_cycle_per_item_enterprise_fee_applicators_for - ).and_return([]) - end - - context "soft delete" do - it "deletes the line item fee" do - fee.destroy - - expect do - service.create_or_update_line_item_fees! - end.to change { line_item.adjustments.enterprise_fee.count }.by(-1) - end - end - - context "hard delete" do - it "deletes the line item fee" do - fee.really_destroy! - - expect do - service.create_or_update_line_item_fees! - end.to change { line_item.adjustments.enterprise_fee.count }.by(-1) - end - end - end - - context "with a new enterprise fee added to the order cylce" do - let(:new_fee) { create(:enterprise_fee, enterprise: fee.enterprise) } - let(:fee_applicator2) { - OpenFoodNetwork::EnterpriseFeeApplicator.new(new_fee, line_item.variant, role) - } - let!(:adjustment) { fee.create_adjustment('foo', line_item, true) } - - before do - allow(service).to receive(:provided_by_order_cycle?) { true } - end - - it "creates a line item fee for the new fee" do - allow(calculator).to receive( - :order_cycle_per_item_enterprise_fee_applicators_for - ).and_return([fee_applicator2]) - - expect(fee_applicator2).to receive(:create_line_item_adjustment).with(line_item) - - service.create_or_update_line_item_fees! - end - - it "updates existing line item fee" do - allow(calculator).to receive( - :order_cycle_per_item_enterprise_fee_applicators_for - ).and_return([fee_applicator, fee_applicator2]) - - expect do - service.create_or_update_line_item_fees! - end.to change { adjustment.reload.updated_at } - end - - context "with variant not included in the order cycle" do - it "doesn't create a new line item fee" do - allow(service).to receive(:provided_by_order_cycle?) { false } - allow(calculator).to receive( - :order_cycle_per_item_enterprise_fee_applicators_for - ).and_return([fee_applicator2]) - - expect(fee_applicator2).not_to receive(:create_line_item_adjustment).with(line_item) - - service.create_or_update_line_item_fees! - end - end - end + service.create_line_item_fees! end end