From 59745fbc735450dccde7377b64385d4a3ccccbf8 Mon Sep 17 00:00:00 2001 From: Rob Harrington Date: Fri, 5 Feb 2016 09:26:27 +1100 Subject: [PATCH] EnterpriseFees can inherit tax_category from product --- app/models/enterprise_fee.rb | 15 ++ .../spree/calculator/default_tax_decorator.rb | 3 +- .../enterprise_fee_applicator.rb | 16 +- spec/models/enterprise_fee_spec.rb | 30 ++++ spec/models/spree/adjustment_spec.rb | 164 +++++++++++++----- 5 files changed, 179 insertions(+), 49 deletions(-) diff --git a/app/models/enterprise_fee.rb b/app/models/enterprise_fee.rb index cf88fdcf9d..59b2648d30 100644 --- a/app/models/enterprise_fee.rb +++ b/app/models/enterprise_fee.rb @@ -18,6 +18,7 @@ class EnterpriseFee < ActiveRecord::Base validates_inclusion_of :fee_type, :in => FEE_TYPES validates_presence_of :name + before_save :ensure_valid_tax_category_settings scope :for_enterprise, lambda { |enterprise| where(enterprise_id: enterprise) } scope :for_enterprises, lambda { |enterprises| where(enterprise_id: enterprises) } @@ -57,4 +58,18 @@ class EnterpriseFee < ActiveRecord::Base :mandatory => mandatory, :locked => true}, :without_protection => true) end + + private + + def ensure_valid_tax_category_settings + # Setting an explicit tax_category removes any inheritance behaviour + # In the absence of any current changes to tax_category, setting + # inherits_tax_category to true will clear the tax_category + if tax_category_id_changed? + self.inherits_tax_category = false if tax_category.present? + elsif inherits_tax_category_changed? + self.tax_category_id = nil if inherits_tax_category? + end + return true + end end diff --git a/app/models/spree/calculator/default_tax_decorator.rb b/app/models/spree/calculator/default_tax_decorator.rb index e55cfc5536..623f2df26d 100644 --- a/app/models/spree/calculator/default_tax_decorator.rb +++ b/app/models/spree/calculator/default_tax_decorator.rb @@ -19,7 +19,8 @@ Spree::Calculator::DefaultTax.class_eval do # Added this block, finds relevant fees for each line_item, calculates the tax on them, and returns the total tax per_item_fees_total = order.line_items.sum do |line_item| calculator.send(:per_item_enterprise_fee_applicators_for, line_item.variant) - .select { |applicator| applicator.enterprise_fee.tax_category == rate.tax_category } + .select { |applicator| (!applicator.enterprise_fee.inherits_tax_category && applicator.enterprise_fee.tax_category == rate.tax_category) || + (applicator.enterprise_fee.inherits_tax_category && line_item.product.tax_category == rate.tax_category) } .sum { |applicator| applicator.enterprise_fee.compute_amount(line_item) } end diff --git a/lib/open_food_network/enterprise_fee_applicator.rb b/lib/open_food_network/enterprise_fee_applicator.rb index 26b1397193..06e71dd21c 100644 --- a/lib/open_food_network/enterprise_fee_applicator.rb +++ b/lib/open_food_network/enterprise_fee_applicator.rb @@ -5,7 +5,7 @@ module OpenFoodNetwork AdjustmentMetadata.create! adjustment: a, enterprise: enterprise_fee.enterprise, fee_name: enterprise_fee.name, fee_type: enterprise_fee.fee_type, enterprise_role: role - a.set_absolute_included_tax! adjustment_tax(line_item.order, a) + a.set_absolute_included_tax! adjustment_tax(line_item, a) end def create_order_adjustment(order) @@ -31,12 +31,22 @@ module OpenFoodNetwork "#{enterprise_fee.fee_type} fee by #{role} #{enterprise_fee.enterprise.name}" end - def adjustment_tax(order, adjustment) - tax_rates = enterprise_fee.tax_category ? enterprise_fee.tax_category.tax_rates.match(order) : [] + def adjustment_tax(adjustable, adjustment) + tax_rates = rates_for(adjustable) tax_rates.select(&:included_in_price).sum do |rate| rate.compute_tax adjustment.amount end end + + def rates_for(adjustable) + case adjustable + when Spree::LineItem + tax_category = enterprise_fee.inherits_tax_category? ? adjustable.product.tax_category : enterprise_fee.tax_category + return tax_category ? tax_category.tax_rates.match(adjustable.order) : [] + when Spree::Order + return enterprise_fee.tax_category ? enterprise_fee.tax_category.tax_rates.match(adjustable) : [] + end + end end end diff --git a/spec/models/enterprise_fee_spec.rb b/spec/models/enterprise_fee_spec.rb index f0f9df1f0b..7cfc0a5783 100644 --- a/spec/models/enterprise_fee_spec.rb +++ b/spec/models/enterprise_fee_spec.rb @@ -26,6 +26,36 @@ describe EnterpriseFee do ef.destroy ex.reload.exchange_fee_ids.should be_empty end + + describe "for tax_category" do + let(:tax_category) { create(:tax_category) } + let(:enterprise_fee) { create(:enterprise_fee, tax_category_id: nil, inherits_tax_category: true) } + + + it "maintains valid tax_category settings" do + # Changing just tax_category, when inheriting + # tax_category is changed, inherits.. set to false + enterprise_fee.assign_attributes(tax_category_id: tax_category.id) + enterprise_fee.save! + expect(enterprise_fee.tax_category).to eq tax_category + expect(enterprise_fee.inherits_tax_category).to be false + + # Changing inherits_tax_category, when tax_category is set + # tax_category is dropped, inherits.. set to true + enterprise_fee.assign_attributes(inherits_tax_category: true) + enterprise_fee.save! + expect(enterprise_fee.tax_category).to be nil + expect(enterprise_fee.inherits_tax_category).to be true + + # Changing both tax_category and inherits_tax_category + # tax_category is changed, but inherits.. changes are dropped + enterprise_fee.assign_attributes(tax_category_id: tax_category.id) + enterprise_fee.assign_attributes(inherits_tax_category: true) + enterprise_fee.save! + expect(enterprise_fee.tax_category).to eq tax_category + expect(enterprise_fee.inherits_tax_category).to be false + end + end end describe "scopes" do diff --git a/spec/models/spree/adjustment_spec.rb b/spec/models/spree/adjustment_spec.rb index cd49e3893a..c3e2ae8304 100644 --- a/spec/models/spree/adjustment_spec.rb +++ b/spec/models/spree/adjustment_spec.rb @@ -120,78 +120,152 @@ module Spree describe "EnterpriseFee adjustments" do let!(:zone) { create(:zone_with_member) } - let(:tax_rate) { create(:tax_rate, included_in_price: true, calculator: Calculator::DefaultTax.new, zone: zone, amount: 0.1) } - let(:tax_category) { create(:tax_category, tax_rates: [tax_rate]) } + let(:fee_tax_rate) { create(:tax_rate, included_in_price: true, calculator: Calculator::DefaultTax.new, zone: zone, amount: 0.1) } + let(:fee_tax_category) { create(:tax_category, tax_rates: [fee_tax_rate]) } let(:coordinator) { create(:distributor_enterprise, charges_sales_tax: true) } - let(:variant) { create(:variant) } + let(:variant) { create(:variant, product: create(:product, tax_category: nil)) } let(:order_cycle) { create(:simple_order_cycle, coordinator: coordinator, coordinator_fees: [enterprise_fee], distributors: [coordinator], variants: [variant]) } let!(:order) { create(:order, order_cycle: order_cycle, distributor: coordinator) } let!(:line_item) { create(:line_item, order: order, variant: variant) } let(:adjustment) { order.adjustments(:reload).enterprise_fee.first } - before do - order.reload.update_distribution_charge! - end + context "when enterprise fees have a fixed tax_category" do + before do + order.reload.update_distribution_charge! + end - context "when enterprise fees are taxed per-order" do - let(:enterprise_fee) { create(:enterprise_fee, enterprise: coordinator, tax_category: tax_category, calculator: Calculator::FlatRate.new(preferred_amount: 50.0)) } + context "when enterprise fees are taxed per-order" do + let(:enterprise_fee) { create(:enterprise_fee, enterprise: coordinator, tax_category: fee_tax_category, calculator: Calculator::FlatRate.new(preferred_amount: 50.0)) } - describe "when the tax rate includes the tax in the price" do - it "records the tax on the enterprise fee adjustments" do - # The fee is $50, tax is 10%, and the fee is inclusive of tax - # Therefore, the included tax should be 0.1/1.1 * 50 = $4.55 + describe "when the tax rate includes the tax in the price" do + it "records the tax on the enterprise fee adjustments" do + # The fee is $50, tax is 10%, and the fee is inclusive of tax + # Therefore, the included tax should be 0.1/1.1 * 50 = $4.55 - adjustment.included_tax.should == 4.55 + adjustment.included_tax.should == 4.55 + end + end + + describe "when the tax rate does not include the tax in the price" do + 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! + end + + it "records the tax on TaxRate adjustment on the order" do + adjustment.included_tax.should == 0 + order.adjustments.tax.first.amount.should == 5.0 + end + end + + describe "when enterprise fees have no tax" do + before do + enterprise_fee.tax_category = nil + enterprise_fee.save! + order.update_distribution_charge! + end + + it "records no tax as charged" do + adjustment.included_tax.should == 0 + end end end - describe "when the tax rate does not include the tax in the price" do - before do - tax_rate.update_attribute :included_in_price, false - order.create_tax_charge! # Updating line_item or order has the same effect - order.update_distribution_charge! + + context "when enterprise fees are taxed per-item" do + let(:enterprise_fee) { create(:enterprise_fee, enterprise: coordinator, tax_category: fee_tax_category, calculator: Calculator::PerItem.new(preferred_amount: 50.0)) } + + describe "when the tax rate includes the tax in the price" do + it "records the tax on the enterprise fee adjustments" do + adjustment.included_tax.should == 4.55 + end end - it "records the tax on TaxRate adjustment on the order" do - adjustment.included_tax.should == 0 - order.adjustments.tax.first.amount.should == 5.0 - end - end + describe "when the tax rate does not include the tax in the price" do + 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! + end - describe "when enterprise fees have no tax" do - before do - enterprise_fee.tax_category = nil - enterprise_fee.save! - order.update_distribution_charge! - end - - it "records no tax as charged" do - adjustment.included_tax.should == 0 + it "records the tax on TaxRate adjustment on the order" do + adjustment.included_tax.should == 0 + order.adjustments.tax.first.amount.should == 5.0 + end end end end + context "when enterprise fees inherit their tax_category product they are applied to" do + let(:product_tax_rate) { create(:tax_rate, included_in_price: true, calculator: Calculator::DefaultTax.new, zone: zone, amount: 0.2) } + let(:product_tax_category) { create(:tax_category, tax_rates: [product_tax_rate]) } - context "when enterprise fees are taxed per-item" do - let(:enterprise_fee) { create(:enterprise_fee, enterprise: coordinator, tax_category: tax_category, calculator: Calculator::PerItem.new(preferred_amount: 50.0)) } + before do + variant.product.update_attribute(:tax_category_id, product_tax_category.id) + order.reload.create_tax_charge! # Updating line_item or order has the same effect + order.reload.update_distribution_charge! + end - describe "when the tax rate includes the tax in the price" do - it "records the tax on the enterprise fee adjustments" do - adjustment.included_tax.should == 4.55 + context "when enterprise fees are taxed per-order" do + let(:enterprise_fee) { create(:enterprise_fee, enterprise: coordinator, inherits_tax_category: true, calculator: Calculator::FlatRate.new(preferred_amount: 50.0)) } + + describe "when the tax rate includes the tax in the price" do + it "records no tax on the enterprise fee adjustments" do + # EnterpriseFee tax category is nil and inheritance only applies to per item fees + # so tax on the enterprise_fee adjustment will be 0 + # Tax on line item is: 0.2/1.2 x $10 = $1.67 + adjustment.included_tax.should == 0.0 + line_item.adjustments.first.included_tax.should == 1.67 + end + end + + describe "when the tax rate does not include the tax in the price" do + 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! + end + + it "records the no tax on TaxRate adjustment on the order" do + # EnterpriseFee tax category is nil and inheritance only applies to per item fees + # so total tax on the order is only that which applies to the line_item itself + # ie. $10 x 0.2 = $2.0 + adjustment.included_tax.should == 0 + order.adjustments.tax.first.amount.should == 2.0 + end end end - describe "when the tax rate does not include the tax in the price" do - before do - tax_rate.update_attribute :included_in_price, false - order.create_tax_charge! # Updating line_item or order has the same effect - order.update_distribution_charge! + + context "when enterprise fees are taxed per-item" do + let(:enterprise_fee) { create(:enterprise_fee, enterprise: coordinator, inherits_tax_category: true, calculator: Calculator::PerItem.new(preferred_amount: 50.0)) } + + describe "when the tax rate includes the tax in the price" do + it "records the tax on the enterprise fee adjustments" do + # Applying product tax rate of 0.2 to enterprise fee of $50 + # gives tax on fee of 0.2/1.2 x $50 = $8.33 + # Tax on line item is: 0.2/1.2 x $10 = $1.67 + adjustment.included_tax.should == 8.33 + line_item.adjustments.first.included_tax.should == 1.67 + end end - it "records the tax on TaxRate adjustment on the order" do - adjustment.included_tax.should == 0 - order.adjustments.tax.first.amount.should == 5.0 + describe "when the tax rate does not include the tax in the price" do + 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! + end + + it "records the tax on TaxRate adjustment on the order" do + # EnterpriseFee inherits tax_category from product so total tax on + # the order is that which applies to the line item itself, plus the + # same rate applied to the fee of $50. ie. ($10 + $50) x 0.2 = $12.0 + adjustment.included_tax.should == 0 + order.adjustments.tax.first.amount.should == 12.0 + end end end end