From eca25a25649d7deff0ad03706dbc012f425f91e2 Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Fri, 31 Jul 2015 16:55:43 +1000 Subject: [PATCH] Make coordinator fees apply to all variants, not just those with exchange fees --- .../enterprise_fee_calculator.rb | 14 ++-- .../enterprise_fee_calculator_spec.rb | 83 ++++++++++++++----- 2 files changed, 67 insertions(+), 30 deletions(-) diff --git a/lib/open_food_network/enterprise_fee_calculator.rb b/lib/open_food_network/enterprise_fee_calculator.rb index eb875d2386..a8f56122f0 100644 --- a/lib/open_food_network/enterprise_fee_calculator.rb +++ b/lib/open_food_network/enterprise_fee_calculator.rb @@ -67,10 +67,8 @@ module OpenFoodNetwork @indexed_enterprise_fees = {} exchange_fees = per_item_enterprise_fees_with_exchange_details - indexed_variants = Spree::Variant.where(id: exchange_fees.pluck(:variant_id)).indexed - - load_exchange_fees indexed_variants, exchange_fees - load_coordinator_fees indexed_variants + load_exchange_fees exchange_fees + load_coordinator_fees end def per_item_enterprise_fees_with_exchange_details @@ -82,16 +80,16 @@ module OpenFoodNetwork select('enterprise_fees.*, exchange_variants.variant_id AS variant_id') end - def load_exchange_fees(indexed_variants, exchange_fees) + def load_exchange_fees(exchange_fees) exchange_fees.each do |enterprise_fee| @indexed_enterprise_fees[enterprise_fee.variant_id.to_i] ||= [] @indexed_enterprise_fees[enterprise_fee.variant_id.to_i] << enterprise_fee end end - def load_coordinator_fees(indexed_variants) - @order_cycle.coordinator_fees.each do |enterprise_fee| - indexed_variants.keys.each do |variant_id| + def load_coordinator_fees + @order_cycle.coordinator_fees.per_item.each do |enterprise_fee| + @order_cycle.variants.pluck(:id).each do |variant_id| @indexed_enterprise_fees[variant_id] ||= [] @indexed_enterprise_fees[variant_id] << enterprise_fee 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 6e6cdb90b1..d2d673e5ab 100644 --- a/spec/lib/open_food_network/enterprise_fee_calculator_spec.rb +++ b/spec/lib/open_food_network/enterprise_fee_calculator_spec.rb @@ -3,40 +3,79 @@ require 'open_food_network/enterprise_fee_calculator' module OpenFoodNetwork 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(:product) { create(:simple_product, price: 10.00) } + let(:product1) { create(:simple_product, supplier: supplier1, price: 10.00) } + let(:product2) { create(:simple_product, supplier: supplier2, price: 20.00) } describe "calculating fees for a variant" do describe "summing all the per-item fees for the 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: Spree::Calculator::FlatRate.new(preferred_amount: 2)) } + let(:enterprise_fee1) { create(:enterprise_fee, amount: 20) } + let(:enterprise_fee2) { create(:enterprise_fee, amount: 3) } + let(:enterprise_fee3) { create(:enterprise_fee, calculator: Spree::Calculator::FlatRate.new(preferred_amount: 2)) } - let!(:exchange) { create(:exchange, order_cycle: order_cycle, sender: coordinator, receiver: distributor, incoming: false, - enterprise_fees: [enterprise_fee1, enterprise_fee2, enterprise_fee3], variants: [product.master]) } + describe "supplier fees" do + let!(:exchange1) { create(:exchange, order_cycle: order_cycle, sender: supplier1, receiver: coordinator, incoming: true, + enterprise_fees: [enterprise_fee1], variants: [product1.master]) } + let!(:exchange2) { create(:exchange, order_cycle: order_cycle, sender: supplier2, receiver: coordinator, incoming: true, + enterprise_fees: [enterprise_fee2], variants: [product2.master]) } - it "sums via regular computation" do - EnterpriseFeeCalculator.new(distributor, order_cycle).fees_for(product.master).should == 23 + it "calculates via regular computation" do + EnterpriseFeeCalculator.new(distributor, order_cycle).fees_for(product1.master).should == 20 + EnterpriseFeeCalculator.new(distributor, order_cycle).fees_for(product2.master).should == 3 + end + + it "calculates via indexed computation" do + EnterpriseFeeCalculator.new(distributor, order_cycle).indexed_fees_for(product1.master).should == 20 + EnterpriseFeeCalculator.new(distributor, order_cycle).indexed_fees_for(product2.master).should == 3 + end end - it "sums via indexed computation" do - EnterpriseFeeCalculator.new(distributor, order_cycle).indexed_fees_for(product.master).should == 23 + describe "coordinator fees" do + let!(:exchange) { create(:exchange, order_cycle: order_cycle, sender: coordinator, receiver: distributor, incoming: false, + enterprise_fees: [], variants: [product1.master]) } + + before do + order_cycle.coordinator_fees = [enterprise_fee1, enterprise_fee2, enterprise_fee3] + end + + it "sums via regular computation" do + EnterpriseFeeCalculator.new(distributor, order_cycle).fees_for(product1.master).should == 23 + end + + it "sums via indexed computation" do + EnterpriseFeeCalculator.new(distributor, order_cycle).indexed_fees_for(product1.master).should == 23 + end + end + + describe "distributor fees" do + let!(:exchange) { create(:exchange, order_cycle: order_cycle, sender: coordinator, receiver: distributor, incoming: false, + enterprise_fees: [enterprise_fee1, enterprise_fee2, enterprise_fee3], variants: [product1.master]) } + + it "sums via regular computation" do + EnterpriseFeeCalculator.new(distributor, order_cycle).fees_for(product1.master).should == 23 + end + + it "sums via indexed computation" do + EnterpriseFeeCalculator.new(distributor, order_cycle).indexed_fees_for(product1.master).should == 23 + end end end describe "summing percentage fees for the variant" do let!(:enterprise_fee1) { create(:enterprise_fee, amount: 20, fee_type: "admin", calculator: Spree::Calculator::FlatPercentItemTotal.new(preferred_flat_percent: 20)) } let!(:exchange) { create(:exchange, order_cycle: order_cycle, sender: coordinator, receiver: distributor, incoming: false, - enterprise_fees: [enterprise_fee1], variants: [product.master]) } + enterprise_fees: [enterprise_fee1], variants: [product1.master]) } it "sums via regular computation" do - EnterpriseFeeCalculator.new(distributor, order_cycle).fees_for(product.master).should == 2.00 + EnterpriseFeeCalculator.new(distributor, order_cycle).fees_for(product1.master).should == 2.00 end it "sums via indexed computation" do - EnterpriseFeeCalculator.new(distributor, order_cycle).indexed_fees_for(product.master).should == 2.00 + EnterpriseFeeCalculator.new(distributor, order_cycle).indexed_fees_for(product1.master).should == 2.00 end end end @@ -50,37 +89,37 @@ module OpenFoodNetwork let!(:exchange) { create(:exchange, order_cycle: order_cycle, sender: coordinator, receiver: distributor, incoming: false, enterprise_fees: [ef_admin, ef_sales, ef_packing, ef_transport, ef_fundraising], - variants: [product.master]) } + variants: [product1.master]) } describe "regular computation" do it "returns a breakdown of fees" do - EnterpriseFeeCalculator.new(distributor, order_cycle).fees_by_type_for(product.master).should == {admin: 1.23, sales: 4.56, packing: 7.89, transport: 0.12, fundraising: 3.45} + EnterpriseFeeCalculator.new(distributor, order_cycle).fees_by_type_for(product1.master).should == {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 - EnterpriseFeeCalculator.new(distributor, order_cycle).fees_by_type_for(product.master).should == {sales: 4.56, packing: 7.89, transport: 0.12, fundraising: 3.45} + EnterpriseFeeCalculator.new(distributor, order_cycle).fees_by_type_for(product1.master).should == {sales: 4.56, packing: 7.89, transport: 0.12, fundraising: 3.45} end end describe "indexed computation" do it "returns a breakdown of fees" do - EnterpriseFeeCalculator.new(distributor, order_cycle).indexed_fees_by_type_for(product.master).should == {admin: 1.23, sales: 4.56, packing: 7.89, transport: 0.12, fundraising: 3.45} + EnterpriseFeeCalculator.new(distributor, order_cycle).indexed_fees_by_type_for(product1.master).should == {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 - EnterpriseFeeCalculator.new(distributor, order_cycle).indexed_fees_by_type_for(product.master).should == {sales: 4.56, packing: 7.89, transport: 0.12, fundraising: 3.45} + EnterpriseFeeCalculator.new(distributor, order_cycle).indexed_fees_by_type_for(product1.master).should == {sales: 4.56, packing: 7.89, transport: 0.12, fundraising: 3.45} end end end describe "creating adjustments" do let(:order) { create(:order, distributor: distributor, order_cycle: order_cycle) } - let!(:line_item) { create(:line_item, order: order, variant: product.master) } + let!(:line_item) { create(:line_item, order: order, variant: product1.master) } let(:enterprise_fee_line_item) { create(:enterprise_fee) } let(:enterprise_fee_order) { create(:enterprise_fee, calculator: Spree::Calculator::FlatRate.new(preferred_amount: 2)) } - let!(:exchange) { create(:exchange, order_cycle: order_cycle, sender: coordinator, receiver: distributor, incoming: false, variants: [product.master]) } + let!(:exchange) { create(:exchange, order_cycle: order_cycle, sender: coordinator, receiver: distributor, incoming: false, variants: [product1.master]) } before { order.reload } @@ -142,14 +181,14 @@ module OpenFoodNetwork let(:exchange_fees) { subject.send(:per_item_enterprise_fees_with_exchange_details) } it "loads exchange fees" do - subject.send(:load_exchange_fees, indexed_variants, exchange_fees) + subject.send(:load_exchange_fees, exchange_fees) indexed_enterprise_fees.should == {v.id => [ef_exchange]} end end describe "loading coordinator fees" do it "loads coordinator fees" do - subject.send(:load_coordinator_fees, indexed_variants) + subject.send(:load_coordinator_fees) indexed_enterprise_fees.should == {v.id => [ef_coordinator]} end end