diff --git a/app/helpers/checkout_helper.rb b/app/helpers/checkout_helper.rb index bc885bd5dc..4d64735cfc 100644 --- a/app/helpers/checkout_helper.rb +++ b/app/helpers/checkout_helper.rb @@ -54,7 +54,7 @@ module CheckoutHelper end def display_adjustment_tax_rates(adjustment) - tax_rates = adjustment.tax_rates + tax_rates = TaxRateFinder.tax_rates_of(adjustment) tax_rates.map { |tr| number_to_percentage(tr.amount * 100, :precision => 1) }.join(", ") end diff --git a/app/models/spree/adjustment_decorator.rb b/app/models/spree/adjustment_decorator.rb index bca08752da..e7ffcf41f0 100644 --- a/app/models/spree/adjustment_decorator.rb +++ b/app/models/spree/adjustment_decorator.rb @@ -40,33 +40,6 @@ module Spree included_tax > 0 end - # @return [Array] - def tax_rates - case originator - when Spree::TaxRate - [originator] - when EnterpriseFee - case source - when Spree::LineItem - tax_category = originator.inherits_tax_category? ? source.product.tax_category : originator.tax_category - return tax_category ? tax_category.tax_rates.match(source.order) : [] - when Spree::Order - return originator.tax_category ? originator.tax_category.tax_rates.match(source) : [] - end - else - find_closest_tax_rates_from_included_tax - end - end - - # shipping fees and adjustments created from the admin panel have - # taxes set at creation in the included_tax field without relation - # to the corresponding TaxRate, so we look for the closest one - def find_closest_tax_rates_from_included_tax - approximation = (included_tax / (amount - included_tax)) - return [] if approximation.infinite? or approximation.zero? - [Spree::TaxRate.order("ABS(amount - #{approximation})").first] - end - def self.without_callbacks skip_callback :save, :after, :update_adjustable skip_callback :destroy, :after, :update_adjustable diff --git a/app/models/spree/order_decorator.rb b/app/models/spree/order_decorator.rb index 5baf95fa1c..be6fcd8fa1 100644 --- a/app/models/spree/order_decorator.rb +++ b/app/models/spree/order_decorator.rb @@ -293,7 +293,7 @@ Spree::Order.class_eval do def tax_adjustment_totals tax_adjustments.each_with_object(Hash.new) do |adjustment, hash| - tax_rates = adjustment.tax_rates + tax_rates = TaxRateFinder.tax_rates_of(adjustment) tax_rates_hash = Hash[tax_rates.collect do |tax_rate| tax_amount = tax_rates.one? ? adjustment.included_tax : tax_rate.compute_tax(adjustment.amount) [tax_rate, tax_amount] diff --git a/app/services/tax_rate_finder.rb b/app/services/tax_rate_finder.rb new file mode 100644 index 0000000000..3f7397af94 --- /dev/null +++ b/app/services/tax_rate_finder.rb @@ -0,0 +1,81 @@ +# Finds tax rates on which an adjustment is based on. +# For example a packaging fee may contain VAT. This service finds the VAT rate +# for the tax included in the packaging fee. +class TaxRateFinder + # @return [Array] + def self.tax_rates_of(adjustment) + new.tax_rates( + adjustment.originator, + adjustment.source, + adjustment.amount, + adjustment.included_tax + ) + end + + # @return [Array] + def tax_rates(originator, source, amount, included_tax) + find_associated_tax_rate(originator, source) || + find_closest_tax_rates_from_included_tax(amount, included_tax) + end + + private + + def find_associated_tax_rate(originator, source) + case originator + when Spree::TaxRate + [originator] + when EnterpriseFee + enterprise_fee_tax_rates(originator, source) + end + end + + def enterprise_fee_tax_rates(enterprise_fee, source) + case source + when Spree::LineItem + tax_category = line_item_tax_category(enterprise_fee, source) + tax_category ? tax_category.tax_rates.match(source.order) : [] + when Spree::Order + enterprise_fee.tax_category ? enterprise_fee.tax_category.tax_rates.match(source) : [] + end + end + + def line_item_tax_category(enterprise_fee, line_item) + if enterprise_fee.inherits_tax_category? + line_item.product.tax_category + else + enterprise_fee.tax_category + end + end + + # There are two cases in which a line item is not associated to a tax rate. + # + # 1. Shipping fees and adjustments created from the admin panel have taxes set + # at creation in the included_tax field without relation to the + # corresponding TaxRate. + # 2. Removing line items from an order doesn't always remove the associated + # enterprise fees. These orphaned fees don't have a line item any more to + # find the item's tax rate. + # + # In these cases we try to find the used tax rate based on the included tax. + # For example, if the included tax is 10% of the adjustment, we look for a tax + # rate of 10%. Due to rounding errors, the included tax may be 9.9% of the + # adjustment. That's why we call it an approximation of the tax rate and look + # for the closest and hopefully find the 10% tax rate. + # + # This attempt can fail. + # + # - If an admin created an adjustment with a miscalculated included tax then + # we don't know which tax rate the admin intended to use. + # - An admin may also enter included tax that doesn't correspond to any tax + # rate in the system. They may enter a fee of $1.2 with tax of $0.2, but + # that doesn't mean that there is a 20% tax rate in the database. + # - The used tax rate may also have been deleted. Maybe the tax law changed. + # + # In either of these cases, we will find a tax rate that doesn't correspond + # to the included tax. + def find_closest_tax_rates_from_included_tax(amount, included_tax) + approximation = (included_tax / (amount - included_tax)) + return [] if approximation.infinite? || approximation.zero? + [Spree::TaxRate.order("ABS(amount - #{approximation})").first] + end +end diff --git a/lib/open_food_network/enterprise_fee_applicator.rb b/lib/open_food_network/enterprise_fee_applicator.rb index e213204047..cafe67ea7b 100644 --- a/lib/open_food_network/enterprise_fee_applicator.rb +++ b/lib/open_food_network/enterprise_fee_applicator.rb @@ -32,7 +32,7 @@ module OpenFoodNetwork end def adjustment_tax(adjustable, adjustment) - tax_rates = adjustment.tax_rates + tax_rates = TaxRateFinder.tax_rates_of(adjustment) tax_rates.select(&:included_in_price).sum do |rate| rate.compute_tax adjustment.amount diff --git a/spec/models/spree/adjustment_spec.rb b/spec/models/spree/adjustment_spec.rb index fa24de3122..f6466ec0d1 100644 --- a/spec/models/spree/adjustment_spec.rb +++ b/spec/models/spree/adjustment_spec.rb @@ -281,21 +281,6 @@ module Spree adjustment.included_tax.should == 10.00 end end - - describe "getting the corresponding tax rate" do - let!(:adjustment_with_tax) { create(:adjustment, amount: 50, included_tax: 10) } - let!(:adjustment_without_tax) { create(:adjustment, amount: 50, included_tax: 0) } - let!(:tax_rate) { create(:tax_rate, calculator: Spree::Calculator::DefaultTax.new, amount: 0.25) } - let!(:other_tax_rate) { create(:tax_rate, calculator: Spree::Calculator::DefaultTax.new, amount: 0.3) } - - it "returns [] if there is no included tax" do - adjustment_without_tax.find_closest_tax_rates_from_included_tax.should == [] - end - - it "returns the most accurate tax rate" do - adjustment_with_tax.find_closest_tax_rates_from_included_tax.should == [tax_rate] - end - end end context "extends LocalizedNumber" do diff --git a/spec/services/tax_rate_finder_spec.rb b/spec/services/tax_rate_finder_spec.rb new file mode 100644 index 0000000000..3f5daadfcf --- /dev/null +++ b/spec/services/tax_rate_finder_spec.rb @@ -0,0 +1,74 @@ +require 'spec_helper' + +describe TaxRateFinder do + describe "getting the corresponding tax rate" do + let(:amount) { BigDecimal(120) } + let(:included_tax) { BigDecimal(20) } + let(:tax_rate) { create_rate(0.2) } + let(:tax_category) { create(:tax_category, tax_rates: [tax_rate]) } + # This zone is used by :order_with_taxes and needs to match it + let(:zone) { create(:zone, name: "GlobalZone") } + let(:shipment) { create(:shipment) } + let(:enterprise_fee) { create(:enterprise_fee, tax_category: tax_category) } + let(:order) { create(:order_with_taxes) } + + it "finds the tax rate of a shipping fee" do + rates = TaxRateFinder.new.tax_rates( + tax_rate, + shipment, + amount, + included_tax + ) + expect(rates).to eq [tax_rate] + end + + it "finds a close match" do + tax_rate.destroy + close_tax_rate = create_rate(tax_rate.amount + 0.05) + # other tax rates, not as close to the real one + create_rate(tax_rate.amount + 0.06) + create_rate(tax_rate.amount - 0.06) + + rates = TaxRateFinder.new.tax_rates( + nil, + shipment, + amount, + included_tax + ) + + expect(rates).to eq [close_tax_rate] + end + + it "finds the tax rate of an enterprise fee" do + rates = TaxRateFinder.new.tax_rates( + enterprise_fee, + order, + amount, + included_tax + ) + expect(rates).to eq [tax_rate] + end + + # There is a bug that leaves orphan adjustments on an order after + # associated line items have been removed. + # https://github.com/openfoodfoundation/openfoodnetwork/issues/3127 + it "deals with a missing line item" do + rates = TaxRateFinder.new.tax_rates( + enterprise_fee, + nil, + amount, + included_tax + ) + expect(rates).to eq [tax_rate] + end + + def create_rate(amount) + create( + :tax_rate, + amount: amount, + calculator: Spree::Calculator::DefaultTax.new, + zone: zone + ) + end + end +end