Merge pull request #3229 from mkllnk/3127-tax-rates-report

3127 More robust tax rates report

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
 

app/models/spree/adjustment_decorator.rb

@@ -40,33 +40,6 @@ module Spree
       included_tax > 0
     end
 
-    # @return [Array<Spree::TaxRate>]
-    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

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]

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<Spree::TaxRate>]
+  def self.tax_rates_of(adjustment)
+    new.tax_rates(
+      adjustment.originator,
+      adjustment.source,
+      adjustment.amount,
+      adjustment.included_tax
+    )
+  end
+
+  # @return [Array<Spree::TaxRate>]
+  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

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

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

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