From a341561446c0bc67ccfba599ded6bd51855b2c13 Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Tue, 18 Dec 2018 17:30:36 +1100 Subject: [PATCH] Move tax rate finding into service and spec it The `#tax_rates` method is actually not present in Spree. We encapsulate the logic in this commit and enable better unit testing. The new tests cover a current bug: https://github.com/openfoodfoundation/openfoodnetwork/issues/3127 --- app/models/spree/adjustment_decorator.rb | 24 +------- app/services/tax_rate_finder.rb | 33 +++++++++++ spec/models/spree/adjustment_spec.rb | 15 ----- spec/services/tax_rate_finder_spec.rb | 73 ++++++++++++++++++++++++ 4 files changed, 107 insertions(+), 38 deletions(-) create mode 100644 app/services/tax_rate_finder.rb create mode 100644 spec/services/tax_rate_finder_spec.rb diff --git a/app/models/spree/adjustment_decorator.rb b/app/models/spree/adjustment_decorator.rb index bca08752da..61e968d844 100644 --- a/app/models/spree/adjustment_decorator.rb +++ b/app/models/spree/adjustment_decorator.rb @@ -42,29 +42,7 @@ module Spree # @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] + TaxRateFinder.new.tax_rates(originator, source, amount, included_tax) end def self.without_callbacks diff --git a/app/services/tax_rate_finder.rb b/app/services/tax_rate_finder.rb new file mode 100644 index 0000000000..b0a5c65446 --- /dev/null +++ b/app/services/tax_rate_finder.rb @@ -0,0 +1,33 @@ +# 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 tax_rates(originator, source, amount, included_tax) + 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 + tax_category ? tax_category.tax_rates.match(source.order) : [] + when Spree::Order + originator.tax_category ? originator.tax_category.tax_rates.match(source) : [] + end + else + find_closest_tax_rates_from_included_tax(amount, included_tax) + end + end + + private + + # 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(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/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..7debe8d154 --- /dev/null +++ b/spec/services/tax_rate_finder_spec.rb @@ -0,0 +1,73 @@ +require 'spec_helper' + +describe TaxRateFinder do + describe "getting the corresponding tax rate" do + let(:amount) { BigDecimal(100) } + let(:included_tax) { BigDecimal(20) } + let(:tax_rate) { create_rate(0.2) } + let(:close_tax_rate) { create_rate(0.25) } + let(:other_tax_rate) { create_rate(0.3) } + 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 + close_tax_rate + other_tax_rate + + 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 + pending "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