From d5f98b8cbe8d2bc0cec2c04fd3a89138b924a14f Mon Sep 17 00:00:00 2001 From: Adam Mueller Date: Mon, 24 Apr 2017 15:01:13 -0700 Subject: [PATCH 1/9] Add Config#tax_calculator_class Will be used as a configuration point for hooking in different methods of calculating tax. For example, using an external service versus tax rates stored in the database. --- core/app/models/spree/app_configuration.rb | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/core/app/models/spree/app_configuration.rb b/core/app/models/spree/app_configuration.rb index 0bc754a18ef..d2233c2256a 100644 --- a/core/app/models/spree/app_configuration.rb +++ b/core/app/models/spree/app_configuration.rb @@ -365,6 +365,17 @@ def tax_adjuster_class @tax_adjuster_class ||= Spree::Tax::OrderAdjuster end + # Allows providing your own class for calculating taxes on an order. + # + # @!attribute [rw] tax_calculator_class + # @return [Class] a class with the same public interfaces as + # Spree::TaxCalculator::Default + # @api experimental + attr_writer :tax_calculator_class + def tax_calculator_class + @tax_calculator_class ||= Spree::TaxCalculator::Default + end + def static_model_preferences @static_model_preferences ||= Spree::Preferences::StaticModelPreferences.new end From d592df25b45f42222ecdca36ce9b82afdb267e42 Mon Sep 17 00:00:00 2001 From: Adam Mueller Date: Wed, 3 May 2017 10:16:19 -0700 Subject: [PATCH 2/9] Add simple output structure for tax calculation These very simple POROs will be usable by tax calculators to ensure they return a consistent result. --- core/app/models/spree/tax/taxed_item.rb | 20 ++++++++++++++++++++ core/app/models/spree/tax/taxed_order.rb | 18 ++++++++++++++++++ 2 files changed, 38 insertions(+) create mode 100644 core/app/models/spree/tax/taxed_item.rb create mode 100644 core/app/models/spree/tax/taxed_order.rb diff --git a/core/app/models/spree/tax/taxed_item.rb b/core/app/models/spree/tax/taxed_item.rb new file mode 100644 index 00000000000..5b9a4dfc171 --- /dev/null +++ b/core/app/models/spree/tax/taxed_item.rb @@ -0,0 +1,20 @@ +module Spree + module Tax + # Simple object used to hold tax data for an item. + # + # This generic object will hold the amount of tax that should be applied to + # an item. (Either a {Spree::LineItem} or a {Spree::Shipment}.) + # + # @attr_reader [Integer] id the {Spree::LineItem} or {Spree::Shipment} ID + # @attr_reader [String] label information about the taxes + # @attr_reader [Spree::TaxRate] tax_rate will be used as the source for tax + # adjustments + # @attr_reader [BigDecimal] amount the amount of tax applied to the item + # @attr_reader [Boolean] included_in_price whether the amount is included + # in the items price, or additional tax. + class TaxedItem + include ActiveModel::Model + attr_accessor :id, :label, :tax_rate, :amount, :included_in_price + end + end +end diff --git a/core/app/models/spree/tax/taxed_order.rb b/core/app/models/spree/tax/taxed_order.rb new file mode 100644 index 00000000000..51563aebab0 --- /dev/null +++ b/core/app/models/spree/tax/taxed_order.rb @@ -0,0 +1,18 @@ +module Spree + module Tax + # Simple object to pass back tax data from a calculator. + # + # Will be used by {Spree::OrderTaxation} to create or update tax + # adjustments on an order. + # + # @attr_reader [Integer] id the {Spree::Order} these taxes apply to + # @attr_reader [Array] line_item_taxes an array of + # tax data for order's line items + # @attr_reader [Array] shipment_taxes an array of + # tax data for the order's shipments + class TaxedOrder + include ActiveModel::Model + attr_accessor :id, :line_item_taxes, :shipment_taxes + end + end +end From ed908ae4e39534ff01a615073e43ed497fc6a414 Mon Sep 17 00:00:00 2001 From: Adam Mueller Date: Wed, 3 May 2017 16:50:35 -0700 Subject: [PATCH 3/9] Add default tax calculator Uses the built-in tax rates to calculate the taxes for an order. --- .../models/spree/tax_calculator/default.rb | 79 +++++++++++++++++++ .../spree/tax_calculator/default_spec.rb | 54 +++++++++++++ 2 files changed, 133 insertions(+) create mode 100644 core/app/models/spree/tax_calculator/default.rb create mode 100644 core/spec/models/spree/tax_calculator/default_spec.rb diff --git a/core/app/models/spree/tax_calculator/default.rb b/core/app/models/spree/tax_calculator/default.rb new file mode 100644 index 00000000000..e54b1a482bb --- /dev/null +++ b/core/app/models/spree/tax_calculator/default.rb @@ -0,0 +1,79 @@ +module Spree + module TaxCalculator + # Default implementation for tax calculations. Will go through all line + # items and shipments and calculate their tax based on tax rates in the DB. + # + # The class used for tax calculation is configurable, so that the + # calculation can easily be pushed to third-party services. Users looking + # to provide their own calculator should adhere to the API of this class. + class Default + include Spree::Tax::TaxHelpers + + # Create a new tax calculator. + # + # @param [Spree::Order] order the order to calculator taxes on + # @return [Spree::TaxCalculator::Default] a Spree::TaxCalculator::Default object + def initialize(order) + @order = order + end + + # Calculate taxes for an order. + # + # @return [Spree::Tax::TaxedOrder] the calculated taxes for the order + def calculate + Spree::Tax::TaxedOrder.new( + id: order.id, + line_item_taxes: line_item_rates, + shipment_taxes: shipment_rates + ) + end + + private + + attr_reader :order + + # Calculate the taxes for line items. + # + # @private + # @return [Array] calculated taxes for the line items + def line_item_rates + order.line_items.flat_map do |line_item| + calculate_rates(line_item) + end + end + + # Calculate the taxes for shipments. + # + # @private + # @return [Array] calculated taxes for the shipments + def shipment_rates + order.shipments.flat_map do |shipment| + calculate_rates(shipment) + end + end + + # Calculate the taxes for a single item. + # + # The item could be either a {Spree::LineItem} or a {Spree::Shipment}. + # + # Will go through all applicable rates for an item and create a new + # {Spree::Tax::TaxedItem} containing the calculated taxes for the item. + # + # @private + # @return [Array] calculated taxes for the item + def calculate_rates(item) + rates_for_item(item).map do |rate| + amount = rate.compute_amount(item) + + Spree::Tax::TaxedItem.new( + id: item.id, + label: rate.adjustment_label(amount), + tax_rate: rate, + amount: amount, + included_in_price: rate.included_in_price + ) + end + end + end + end +end diff --git a/core/spec/models/spree/tax_calculator/default_spec.rb b/core/spec/models/spree/tax_calculator/default_spec.rb new file mode 100644 index 00000000000..a02e523f0cc --- /dev/null +++ b/core/spec/models/spree/tax_calculator/default_spec.rb @@ -0,0 +1,54 @@ +require 'spec_helper' + +RSpec.describe Spree::TaxCalculator::Default do + let(:shipping_address) { FactoryGirl.create(:address, state: new_york) } + let(:order) { FactoryGirl.create(:order, ship_address: shipping_address, state: "delivery") } + + let(:new_york) { FactoryGirl.create(:state, state_code: "NY") } + let(:new_york_zone) { FactoryGirl.create(:zone, states: [new_york]) } + + let(:books_category) { FactoryGirl.create(:tax_category, name: "Books") } + let!(:book_tax_rate) do + FactoryGirl.create( + :tax_rate, + name: "New York Sales Tax", + tax_categories: [books_category], + zone: new_york_zone, + included_in_price: false, + amount: 0.05 + ) + end + + before do + book = FactoryGirl.create( + :product, + price: 20, + name: "Book", + tax_category: books_category, + ) + + order.contents.add(book.master) + end + + let(:calculator) { described_class.new(order) } + + describe '#calculate' do + subject(:calculated_taxes) { calculator.calculate } + + it { is_expected.to be_a Spree::Tax::TaxedOrder } + + it "has tax information for the line item", aggregate_failures: true do + expect(calculated_taxes.line_item_taxes.count).to eq 1 + + line_item_tax = calculated_taxes.line_item_taxes.first + expect(line_item_tax.amount).to eq 1 + expect(line_item_tax.included_in_price).to be false + expect(line_item_tax.tax_rate).to eq book_tax_rate + expect(line_item_tax.label).to eq "New York Sales Tax 5.000%" + end + + it "has tax information for the shipments" do + expect(calculated_taxes.shipment_taxes).to be_empty + end + end +end From f662f49171daa11885dfdee5dbf3c1a1d38bcb91 Mon Sep 17 00:00:00 2001 From: Adam Mueller Date: Fri, 5 May 2017 13:24:39 -0700 Subject: [PATCH 4/9] Add tax applicator model Will help consolidate the logic for applying taxes to line items and shipments. Making it easier to create third-party extensions. --- core/app/models/spree/order_taxation.rb | 79 +++++++++++ core/app/models/spree/tax_rate.rb | 4 +- core/spec/models/spree/order_taxation_spec.rb | 126 ++++++++++++++++++ 3 files changed, 207 insertions(+), 2 deletions(-) create mode 100644 core/app/models/spree/order_taxation.rb create mode 100644 core/spec/models/spree/order_taxation_spec.rb diff --git a/core/app/models/spree/order_taxation.rb b/core/app/models/spree/order_taxation.rb new file mode 100644 index 00000000000..222078df819 --- /dev/null +++ b/core/app/models/spree/order_taxation.rb @@ -0,0 +1,79 @@ +module Spree + # Relatively simple class used to apply a {Spree::Tax::TaxedOrder} to a + # {Spree::Order}. + # + # This class will create or update adjustments on the taxed items and remove + # any now inapplicable tax adjustments from the order. + class OrderTaxation + # Create a new order taxation. + # + # @param [Spree::Order] order the order to apply taxes to + # @return [Spree::OrderTaxation] a {Spree::OrderTaxation} object + def initialize(order) + @order = order + end + + # Apply taxes to the order. + # + # This method will create or update adjustments on all line items and + # shipments in the order to reflect the appropriate taxes passed in. It + # will also remove any now inapplicable tax adjustments. + # + # @param [Spree::Tax::TaxedOrder] taxes the taxes to apply to the order + # @return [void] + def apply(taxes) + @order.line_items.each do |item| + taxed_items = taxes.line_item_taxes.select { |i| i.id == item.id } + update_adjustments(item, taxed_items) + end + + @order.shipments.each do |item| + taxed_items = taxes.shipment_taxes.select { |i| i.id == item.id } + update_adjustments(item, taxed_items) + end + end + + private + + # Walk through the taxes for an item and update adjustments for it. Once + # all of the taxes have been added as adjustments, remove any old tax + # adjustments that weren't touched. + # + # @private + # @param [#adjustments] item a {Spree::LineItem} or {Spree::Shipment} + # @param [Array] taxed_items a list of calculated taxes for an item + # @return [void] + def update_adjustments(item, taxed_items) + tax_adjustments = item.adjustments.select(&:tax?) + + active_adjustments = taxed_items.map do |tax_item| + update_adjustment(item, tax_item) + end + + # Remove any tax adjustments tied to rates which no longer match. + unmatched_adjustments = tax_adjustments - active_adjustments + item.adjustments.destroy(unmatched_adjustments) + end + + # Update or create a new tax adjustment on an item. + # + # @private + # @param [#adjustments] item a {Spree::LineItem} or {Spree::Shipment} + # @param [Spree::Tax::TaxedItem] tax_item calculated taxes for an item + # @return [Spree::Adjustment] the created or updated tax adjustment + def update_adjustment(item, tax_item) + tax_adjustment = item.adjustments.detect do |adjustment| + adjustment.source == tax_item.tax_rate + end + + tax_adjustment ||= item.adjustments.new( + source: tax_item.tax_rate, + order_id: item.order_id, + label: tax_item.label, + included: tax_item.included_in_price + ) + tax_adjustment.update_attributes!(amount: tax_item.amount) + tax_adjustment + end + end +end diff --git a/core/app/models/spree/tax_rate.rb b/core/app/models/spree/tax_rate.rb index e6a88221452..924cb7b02a8 100644 --- a/core/app/models/spree/tax_rate.rb +++ b/core/app/models/spree/tax_rate.rb @@ -98,8 +98,6 @@ def compute_amount(item) calculator.compute(item) end - private - def adjustment_label(amount) Spree.t( translation_key(amount), @@ -109,6 +107,8 @@ def adjustment_label(amount) ) end + private + def amount_for_adjustment_label ActiveSupport::NumberHelper::NumberToPercentageConverter.convert( amount * 100, diff --git a/core/spec/models/spree/order_taxation_spec.rb b/core/spec/models/spree/order_taxation_spec.rb new file mode 100644 index 00000000000..053fb43c682 --- /dev/null +++ b/core/spec/models/spree/order_taxation_spec.rb @@ -0,0 +1,126 @@ +require 'spec_helper' + +RSpec.describe Spree::OrderTaxation do + let(:shipping_address) { FactoryGirl.create(:address, state: new_york) } + let(:order) { FactoryGirl.create(:order, ship_address: shipping_address, state: "delivery") } + + let(:new_york) { FactoryGirl.create(:state, state_code: "NY") } + let(:new_york_zone) { FactoryGirl.create(:zone, states: [new_york]) } + + let(:books_category) { FactoryGirl.create(:tax_category, name: "Books") } + let(:book_tax_rate) do + FactoryGirl.create( + :tax_rate, + name: "New York Sales Tax", + tax_categories: [books_category], + zone: new_york_zone, + included_in_price: false, + amount: 0.05 + ) + end + + let(:book) do + FactoryGirl.create( + :product, + price: 20, + name: "Book", + tax_category: books_category, + ) + end + + let(:taxation) { described_class.new(order) } + + describe "#apply" do + let(:line_item) { order.contents.add(book.master) } + + let(:line_item_tax) do + Spree::Tax::TaxedItem.new( + id: line_item.id, + label: "Tax!", + tax_rate: book_tax_rate, + amount: 5, + included_in_price: false + ) + end + + let(:taxes) do + Spree::Tax::TaxedOrder.new( + id: order.id, + line_item_taxes: [line_item_tax], + shipment_taxes: [] + ) + end + + before { taxation.apply(taxes) } + + it "creates a new tax adjustment", aggregate_failures: true do + expect(line_item.adjustments.count).to eq 1 + + tax_adjustment = line_item.adjustments.first + expect(tax_adjustment.label).to eq "Tax!" + expect(tax_adjustment.source).to eq book_tax_rate + expect(tax_adjustment.amount).to eq 5 + expect(tax_adjustment.included).to be false + end + + context "when new taxes are applied" do + let(:new_line_item_tax) do + Spree::Tax::TaxedItem.new( + id: line_item.id, + label: "Tax!", + tax_rate: book_tax_rate, + amount: 10, + included_in_price: false + ) + end + + let(:new_taxes) do + Spree::Tax::TaxedOrder.new( + id: order.id, + line_item_taxes: [new_line_item_tax], + shipment_taxes: [] + ) + end + + it "updates the existing tax amount", aggregate_failures: true do + expect { + taxation.apply(new_taxes) + }.to change { + line_item.adjustments.first.amount + }.from(5).to(10) + end + + context "and the adjustment is finalized" do + before do + line_item.adjustments.first.finalize! + end + + it "does not update the tax amount", aggregate_failures: true do + expect { + taxation.apply(new_taxes) + }.to change { + line_item.adjustments.first.amount + }.from(5).to(10) + end + end + end + + context "when taxes are removed" do + let(:new_taxes) do + Spree::Tax::TaxedOrder.new( + id: order.id, + line_item_taxes: [], + shipment_taxes: [] + ) + end + + it "removes the tax adjustment" do + expect { + taxation.apply(new_taxes) + }.to change { + line_item.adjustments.count + }.from(1).to(0) + end + end + end +end From c4f640d4ba48c11a70f4591971571c442a824b42 Mon Sep 17 00:00:00 2001 From: Adam Mueller Date: Fri, 5 May 2017 13:26:58 -0700 Subject: [PATCH 5/9] Use new tax calculator in order adjuster Means we can remove the item adjuster since it's no longer used. This provides us an easy upgrade path for users while also ensuring anyone who has extended taxation using an adjuster won't have anything stop working. --- core/app/models/spree/tax/item_adjuster.rb | 51 -------- core/app/models/spree/tax/order_adjuster.rb | 16 +-- .../models/spree/order/state_machine_spec.rb | 2 +- core/spec/models/spree/order_updater_spec.rb | 14 ++- .../models/spree/tax/item_adjuster_spec.rb | 109 ------------------ .../models/spree/tax/order_adjuster_spec.rb | 32 ++--- 6 files changed, 22 insertions(+), 202 deletions(-) delete mode 100644 core/app/models/spree/tax/item_adjuster.rb delete mode 100644 core/spec/models/spree/tax/item_adjuster_spec.rb diff --git a/core/app/models/spree/tax/item_adjuster.rb b/core/app/models/spree/tax/item_adjuster.rb deleted file mode 100644 index 6581dc306d3..00000000000 --- a/core/app/models/spree/tax/item_adjuster.rb +++ /dev/null @@ -1,51 +0,0 @@ -# @api private -# @note This is a helper class for Tax::OrderAdjuster. It is marked as api -# private because taxes should always be calculated on the entire order, so -# external code should call Tax::OrderAdjuster instead of Tax::ItemAdjuster. -module Spree - module Tax - # Adjust a single taxable item (line item or shipment) - class ItemAdjuster - attr_reader :item, :order - - include TaxHelpers - - # @param [Spree::LineItem,Spree::Shipment] item to adjust - # @param [Hash] options like already known tax rates for the order's zone - def initialize(item, options = {}) - @item = item - @order = @item.order - # set instance variable so `TaxRate.match` is only called when necessary - @rates_for_order = options[:rates_for_order] - @rates_for_default_zone = options[:rates_for_default_zone] - end - - # This updates the amounts for adjustments which already exist and - # creates and remove adjustments as needed to match the applicable - # (geographically and category-wise) tax rates. - def adjust! - rates = rates_for_item(item) - - tax_adjustments = item.adjustments.select(&:tax?) - active_adjustments = rates.map do |rate| - # Find an existing adjustment from the same source. - # All tax adjustments already have source_type == 'Spree::TaxRate' so - # we need only check source_id. - adjustment = tax_adjustments.detect { |a| a.source_id == rate.id } - if adjustment - adjustment.update! - adjustment - else - # Create a new adjustment - rate.adjust(nil, item) - end - end - - unmatched_adjustments = tax_adjustments - active_adjustments - - # Remove any tax adjustments tied to rates which no longer match - item.adjustments.destroy(unmatched_adjustments) - end - end - end -end diff --git a/core/app/models/spree/tax/order_adjuster.rb b/core/app/models/spree/tax/order_adjuster.rb index d5fd91bcfbe..fa2c1aea203 100644 --- a/core/app/models/spree/tax/order_adjuster.rb +++ b/core/app/models/spree/tax/order_adjuster.rb @@ -4,8 +4,6 @@ module Tax class OrderAdjuster attr_reader :order - include TaxHelpers - # @param [Spree::Order] order to be adjusted def initialize(order) @order = order @@ -14,18 +12,8 @@ def initialize(order) # Creates tax adjustments for all taxable items (shipments and line items) # in the given order. def adjust! - (order.line_items + order.shipments).each do |item| - ItemAdjuster.new(item, order_wide_options).adjust! - end - end - - private - - def order_wide_options - { - rates_for_order: rates_for_order(order), - rates_for_default_zone: rates_for_default_zone - } + taxes = Spree::Config.tax_calculator_class.new(order).calculate + Spree::OrderTaxation.new(order).apply(taxes) end end end diff --git a/core/spec/models/spree/order/state_machine_spec.rb b/core/spec/models/spree/order/state_machine_spec.rb index e7b7a9323fd..1c38bcde080 100644 --- a/core/spec/models/spree/order/state_machine_spec.rb +++ b/core/spec/models/spree/order/state_machine_spec.rb @@ -43,7 +43,7 @@ end it "adjusts tax rates when transitioning to delivery" do - expect(Spree::Tax::OrderAdjuster).to receive(:new).once.with(order).and_call_original + expect(Spree::TaxCalculator::Default).to receive(:new).once.with(order).and_call_original order.next! end end diff --git a/core/spec/models/spree/order_updater_spec.rb b/core/spec/models/spree/order_updater_spec.rb index 350955ff555..b9c4f24beb8 100644 --- a/core/spec/models/spree/order_updater_spec.rb +++ b/core/spec/models/spree/order_updater_spec.rb @@ -301,18 +301,20 @@ def create_adjustment(label, amount) end end - context 'with a custom tax_adjuster_class' do - let(:custom_adjuster_class) { double } - let(:custom_adjuster_instance) { double } + context 'with a custom tax_calculator_class' do + let(:custom_calculator_class) { double } + let(:custom_calculator_instance) { double } before do order # generate this first so we can expect it - Spree::Config.tax_adjuster_class = custom_adjuster_class + Spree::Config.tax_calculator_class = custom_calculator_class end it 'uses the configured class' do - expect(custom_adjuster_class).to receive(:new).with(order).at_least(:once).and_return(custom_adjuster_instance) - expect(custom_adjuster_instance).to receive(:adjust!).at_least(:once) + expect(custom_calculator_class).to receive(:new).with(order).at_least(:once).and_return(custom_calculator_instance) + expect(custom_calculator_instance).to receive(:calculate).at_least(:once).and_return( + Spree::Tax::TaxedOrder.new(id: order.id, line_item_taxes: [], shipment_taxes: []) + ) order.update! end diff --git a/core/spec/models/spree/tax/item_adjuster_spec.rb b/core/spec/models/spree/tax/item_adjuster_spec.rb deleted file mode 100644 index 6b3d7dc875b..00000000000 --- a/core/spec/models/spree/tax/item_adjuster_spec.rb +++ /dev/null @@ -1,109 +0,0 @@ -require 'spec_helper' - -RSpec.describe Spree::Tax::ItemAdjuster do - subject(:adjuster) { described_class.new(item) } - let(:order) { create(:order) } - let(:item) { create(:line_item, order: order) } - - def tax_adjustments - item.adjustments.select(&:tax?).to_a - end - - describe 'initialization' do - it 'sets order to item order' do - expect(adjuster.order).to eq(item.order) - end - - it 'sets adjustable' do - expect(adjuster.item).to eq(item) - end - end - - shared_examples_for 'untaxed item' do - it 'creates no adjustments' do - adjuster.adjust! - expect(tax_adjustments).to eq([]) - end - - context 'with an existing tax adjustment' do - let!(:existing_adjustment) { create(:tax_adjustment, adjustable: item) } - - it 'removes the existing adjustment' do - adjuster.adjust! - aggregate_failures do - expect(tax_adjustments).to eq([]) - expect(Spree::Adjustment).to_not be_exists(existing_adjustment.id) - end - end - end - end - - describe '#adjust!' do - before do - expect(order).to receive(:tax_address).at_least(:once).and_return(address) - end - - context 'when the order has no tax zone' do - let(:address) { Spree::Tax::TaxLocation.new } - - it_behaves_like 'untaxed item' - end - - context 'when the order has a taxable address' do - let(:item) { create :line_item, order: order } - let(:address) { order.tax_address } - - before do - expect(Spree::TaxRate).to receive(:for_address).with(order.tax_address).and_return(rates_for_order_zone) - end - - context 'when there are no matching rates' do - let(:rates_for_order_zone) { [] } - - it_behaves_like 'untaxed item' - end - - context 'when there are matching rates for the zone' do - context 'and all rates have the same tax category as the item' do - let(:item) { create :line_item, order: order, tax_category: item_tax_category } - let(:item_tax_category) { create(:tax_category) } - let(:rate_1) { create :tax_rate, tax_categories: [item_tax_category], amount: 0.1 } - let(:rate_2) { create :tax_rate } - let(:rate_3) { create :tax_rate, tax_categories: [item_tax_category, build(:tax_category)] } - let(:rates_for_order_zone) { [rate_1, rate_2, rate_3] } - - it 'creates an adjustment for every matching rate' do - adjuster.adjust! - expect(tax_adjustments.length).to eq(2) - end - - it 'creates adjustments only for matching rates' do - adjuster.adjust! - expect(tax_adjustments.map(&:source)).to match_array([rate_1, rate_3]) - end - - context 'when the adjustment exists' do - before do - adjuster.adjust! - end - - context 'when the existing adjustment is finalized' do - before do - tax_adjustments.first.finalize! - end - - it 'updates the adjustment' do - item.update_columns(price: item.price * 2) - adjuster.adjust! - tax_rate1_adjustment = tax_adjustments.detect do |adjustment| - adjustment.source == rate_1 - end - expect(tax_rate1_adjustment.amount).to eq(0.1 * item.price) - end - end - end - end - end - end - end -end diff --git a/core/spec/models/spree/tax/order_adjuster_spec.rb b/core/spec/models/spree/tax/order_adjuster_spec.rb index e9cd63b2f9a..2a7e59b200d 100644 --- a/core/spec/models/spree/tax/order_adjuster_spec.rb +++ b/core/spec/models/spree/tax/order_adjuster_spec.rb @@ -12,31 +12,21 @@ end describe '#adjust!' do - let(:line_items) { build_stubbed_list(:line_item, 2) } - let(:order) { build_stubbed(:order, line_items: line_items) } - let(:rates_for_order_zone) { [] } - let(:rates_for_default_zone) { [] } - let(:item_adjuster) { Spree::Tax::ItemAdjuster.new(line_items.first) } + let(:order) { Spree::Order.new } + + let(:custom_calculator_class) { double } + let(:custom_calculator_instance) { double } before do - expect(Spree::TaxRate).to receive(:for_address).with(order.tax_address).and_return(rates_for_order_zone) + Spree::Config.tax_calculator_class = custom_calculator_class end - it 'calls the item adjuster with all line items' do - expect(Spree::Tax::ItemAdjuster).to receive(:new). - with( - line_items.first, - rates_for_order: rates_for_order_zone, - rates_for_default_zone: rates_for_default_zone - ).and_return(item_adjuster) - expect(Spree::Tax::ItemAdjuster).to receive(:new). - with( - line_items.second, - rates_for_order: rates_for_order_zone, - rates_for_default_zone: rates_for_default_zone - ).and_return(item_adjuster) - - expect(item_adjuster).to receive(:adjust!).twice + it 'calls the configured tax calculator' do + expect(custom_calculator_class).to receive(:new).with(order).at_least(:once).and_return(custom_calculator_instance) + expect(custom_calculator_instance).to receive(:calculate).at_least(:once).and_return( + Spree::Tax::TaxedOrder.new(id: order.id, line_item_taxes: [], shipment_taxes: []) + ) + adjuster.adjust! end end From 4972cd9bb10eadc3806e0bbea349d03d2700aaf5 Mon Sep 17 00:00:00 2001 From: Adam Mueller Date: Thu, 11 May 2017 15:55:40 -0700 Subject: [PATCH 6/9] Add shipping rate tax calculator Adds another simple integration point to make configuring tax calculations to use third-party services easier. --- core/app/models/spree/app_configuration.rb | 10 +++++ .../models/spree/tax/shipping_rate_taxer.rb | 17 ++------ .../spree/tax_calculator/shipping_rate.rb | 43 +++++++++++++++++++ 3 files changed, 57 insertions(+), 13 deletions(-) create mode 100644 core/app/models/spree/tax_calculator/shipping_rate.rb diff --git a/core/app/models/spree/app_configuration.rb b/core/app/models/spree/app_configuration.rb index d2233c2256a..48ea18c94ba 100644 --- a/core/app/models/spree/app_configuration.rb +++ b/core/app/models/spree/app_configuration.rb @@ -309,6 +309,16 @@ def shipping_rate_taxer_class @shipping_rate_taxer_class ||= Spree::Tax::ShippingRateTaxer end + # Allows providing your own class for calculating taxes on a shipping rate. + # + # @!attribute [rw] shipping_rate_tax_calculator_class + # @return [Class] a class with the same public interfaces as + # Spree::TaxCalculator::ShippingRate + attr_writer :shipping_rate_tax_calculator_class + def shipping_rate_tax_calculator_class + @shipping_rate_tax_calculator_class ||= Spree::TaxCalculator::ShippingRate + end + # Allows providing your own Mailer for shipped cartons. # # @!attribute [rw] carton_shipped_email_class diff --git a/core/app/models/spree/tax/shipping_rate_taxer.rb b/core/app/models/spree/tax/shipping_rate_taxer.rb index 45568ca16be..94be4ae40c1 100644 --- a/core/app/models/spree/tax/shipping_rate_taxer.rb +++ b/core/app/models/spree/tax/shipping_rate_taxer.rb @@ -2,30 +2,21 @@ module Spree module Tax # Used to build shipping rate taxes class ShippingRateTaxer - include TaxHelpers - # Build shipping rate taxes for a shipping rate # Modifies the passed-in shipping rate with associated shipping rate taxes. # @param [Spree::ShippingRate] shipping_rate The shipping rate to add taxes to. # This parameter will be modified. # @return [Spree::ShippingRate] The shipping rate with associated tax objects def tax(shipping_rate) - tax_rates_for_shipping_rate(shipping_rate).each do |tax_rate| + taxes = Spree::Config.shipping_rate_tax_calculator_class.new(shipping_rate).calculate + taxes.each do |tax| shipping_rate.taxes.build( - amount: tax_rate.compute_amount(shipping_rate), - tax_rate: tax_rate + amount: tax.amount, + tax_rate: tax.tax_rate ) end shipping_rate end - - private - - def tax_rates_for_shipping_rate(shipping_rate) - applicable_rates(shipping_rate.order).select do |tax_rate| - tax_rate.tax_categories.include?(shipping_rate.tax_category) - end - end end end end diff --git a/core/app/models/spree/tax_calculator/shipping_rate.rb b/core/app/models/spree/tax_calculator/shipping_rate.rb new file mode 100644 index 00000000000..d6080550b61 --- /dev/null +++ b/core/app/models/spree/tax_calculator/shipping_rate.rb @@ -0,0 +1,43 @@ +module Spree + module TaxCalculator + # Default implementation for tax calculations on shipping rates. + # + # The class used for shipping rate tax calculation is configurable, so that + # the calculation can easily be pushed to third-party services. Users + # looking to provide their own calculator should adhere to the API of this + # class. + # + # @see Spree::Tax::ShippingRateTaxer + class ShippingRate + include Spree::Tax::TaxHelpers + + attr_reader :shipping_rate + + # Create a new tax calculator. + # + # @param [Spree::ShippingRate] shipping_rate the shipping rate to + # calculate taxes on + # @return [Spree::TaxCalculator::ShippingRate] + def initialize(shipping_rate) + @shipping_rate = shipping_rate + end + + # Calculate taxes for a shipping rate. + # + # @return [Array] the calculated taxes for the + # shipping rate + def calculate + rates_for_item(shipping_rate).map do |rate| + amount = rate.compute_amount(shipping_rate) + + Spree::Tax::TaxedItem.new( + id: shipping_rate.id, + label: rate.adjustment_label(amount), + tax_rate: rate, + amount: amount + ) + end + end + end + end +end From 9654cbaf19bbe8d4fc01e75d45221b4298e81ad8 Mon Sep 17 00:00:00 2001 From: Adam Mueller Date: Tue, 16 May 2017 14:44:12 -0700 Subject: [PATCH 7/9] Mark the new tax calculators as experimental We haven't finalized the API yet and want to ensure any early adopters know this interface is still under development and likely to change. --- core/app/models/spree/app_configuration.rb | 1 + core/app/models/spree/tax_calculator/default.rb | 4 ++++ core/app/models/spree/tax_calculator/shipping_rate.rb | 3 +++ 3 files changed, 8 insertions(+) diff --git a/core/app/models/spree/app_configuration.rb b/core/app/models/spree/app_configuration.rb index 48ea18c94ba..23efc8822e7 100644 --- a/core/app/models/spree/app_configuration.rb +++ b/core/app/models/spree/app_configuration.rb @@ -314,6 +314,7 @@ def shipping_rate_taxer_class # @!attribute [rw] shipping_rate_tax_calculator_class # @return [Class] a class with the same public interfaces as # Spree::TaxCalculator::ShippingRate + # @api experimental attr_writer :shipping_rate_tax_calculator_class def shipping_rate_tax_calculator_class @shipping_rate_tax_calculator_class ||= Spree::TaxCalculator::ShippingRate diff --git a/core/app/models/spree/tax_calculator/default.rb b/core/app/models/spree/tax_calculator/default.rb index e54b1a482bb..d4ef9855450 100644 --- a/core/app/models/spree/tax_calculator/default.rb +++ b/core/app/models/spree/tax_calculator/default.rb @@ -6,6 +6,10 @@ module TaxCalculator # The class used for tax calculation is configurable, so that the # calculation can easily be pushed to third-party services. Users looking # to provide their own calculator should adhere to the API of this class. + # + # @api experimental + # @note This API is currently in development and likely to change. + # Specifically, the input format is not yet finalized. class Default include Spree::Tax::TaxHelpers diff --git a/core/app/models/spree/tax_calculator/shipping_rate.rb b/core/app/models/spree/tax_calculator/shipping_rate.rb index d6080550b61..3b4893e00c1 100644 --- a/core/app/models/spree/tax_calculator/shipping_rate.rb +++ b/core/app/models/spree/tax_calculator/shipping_rate.rb @@ -8,6 +8,9 @@ module TaxCalculator # class. # # @see Spree::Tax::ShippingRateTaxer + # @api experimental + # @note This API is currently in development and likely to change. + # Specifically, the input format is not yet finalized. class ShippingRate include Spree::Tax::TaxHelpers From c3b8590c70e867e4b63f71ca3829998816c40e89 Mon Sep 17 00:00:00 2001 From: Adam Mueller Date: Mon, 22 May 2017 09:42:52 +0100 Subject: [PATCH 8/9] Rename TaxedItem as ItemTax --- core/app/models/spree/order_taxation.rb | 8 ++++---- .../models/spree/tax/{taxed_item.rb => item_tax.rb} | 6 +++--- core/app/models/spree/tax/taxed_order.rb | 4 ++-- core/app/models/spree/tax_calculator/default.rb | 12 ++++++------ .../app/models/spree/tax_calculator/shipping_rate.rb | 6 +++--- core/spec/models/spree/order_taxation_spec.rb | 8 ++++---- 6 files changed, 22 insertions(+), 22 deletions(-) rename core/app/models/spree/tax/{taxed_item.rb => item_tax.rb} (78%) diff --git a/core/app/models/spree/order_taxation.rb b/core/app/models/spree/order_taxation.rb index 222078df819..978f79356c4 100644 --- a/core/app/models/spree/order_taxation.rb +++ b/core/app/models/spree/order_taxation.rb @@ -23,12 +23,12 @@ def initialize(order) # @return [void] def apply(taxes) @order.line_items.each do |item| - taxed_items = taxes.line_item_taxes.select { |i| i.id == item.id } + taxed_items = taxes.line_item_taxes.select { |i| i.item_id == item.id } update_adjustments(item, taxed_items) end @order.shipments.each do |item| - taxed_items = taxes.shipment_taxes.select { |i| i.id == item.id } + taxed_items = taxes.shipment_taxes.select { |i| i.item_id == item.id } update_adjustments(item, taxed_items) end end @@ -41,7 +41,7 @@ def apply(taxes) # # @private # @param [#adjustments] item a {Spree::LineItem} or {Spree::Shipment} - # @param [Array] taxed_items a list of calculated taxes for an item + # @param [Array] taxed_items a list of calculated taxes for an item # @return [void] def update_adjustments(item, taxed_items) tax_adjustments = item.adjustments.select(&:tax?) @@ -59,7 +59,7 @@ def update_adjustments(item, taxed_items) # # @private # @param [#adjustments] item a {Spree::LineItem} or {Spree::Shipment} - # @param [Spree::Tax::TaxedItem] tax_item calculated taxes for an item + # @param [Spree::Tax::ItemTax] tax_item calculated taxes for an item # @return [Spree::Adjustment] the created or updated tax adjustment def update_adjustment(item, tax_item) tax_adjustment = item.adjustments.detect do |adjustment| diff --git a/core/app/models/spree/tax/taxed_item.rb b/core/app/models/spree/tax/item_tax.rb similarity index 78% rename from core/app/models/spree/tax/taxed_item.rb rename to core/app/models/spree/tax/item_tax.rb index 5b9a4dfc171..61ad2ee2550 100644 --- a/core/app/models/spree/tax/taxed_item.rb +++ b/core/app/models/spree/tax/item_tax.rb @@ -5,16 +5,16 @@ module Tax # This generic object will hold the amount of tax that should be applied to # an item. (Either a {Spree::LineItem} or a {Spree::Shipment}.) # - # @attr_reader [Integer] id the {Spree::LineItem} or {Spree::Shipment} ID + # @attr_reader [Integer] item_id the {Spree::LineItem} or {Spree::Shipment} ID # @attr_reader [String] label information about the taxes # @attr_reader [Spree::TaxRate] tax_rate will be used as the source for tax # adjustments # @attr_reader [BigDecimal] amount the amount of tax applied to the item # @attr_reader [Boolean] included_in_price whether the amount is included # in the items price, or additional tax. - class TaxedItem + class ItemTax include ActiveModel::Model - attr_accessor :id, :label, :tax_rate, :amount, :included_in_price + attr_accessor :item_id, :label, :tax_rate, :amount, :included_in_price end end end diff --git a/core/app/models/spree/tax/taxed_order.rb b/core/app/models/spree/tax/taxed_order.rb index 51563aebab0..62b72d92e83 100644 --- a/core/app/models/spree/tax/taxed_order.rb +++ b/core/app/models/spree/tax/taxed_order.rb @@ -6,9 +6,9 @@ module Tax # adjustments on an order. # # @attr_reader [Integer] id the {Spree::Order} these taxes apply to - # @attr_reader [Array] line_item_taxes an array of + # @attr_reader [Array] line_item_taxes an array of # tax data for order's line items - # @attr_reader [Array] shipment_taxes an array of + # @attr_reader [Array] shipment_taxes an array of # tax data for the order's shipments class TaxedOrder include ActiveModel::Model diff --git a/core/app/models/spree/tax_calculator/default.rb b/core/app/models/spree/tax_calculator/default.rb index d4ef9855450..bab6cc57147 100644 --- a/core/app/models/spree/tax_calculator/default.rb +++ b/core/app/models/spree/tax_calculator/default.rb @@ -39,7 +39,7 @@ def calculate # Calculate the taxes for line items. # # @private - # @return [Array] calculated taxes for the line items + # @return [Array] calculated taxes for the line items def line_item_rates order.line_items.flat_map do |line_item| calculate_rates(line_item) @@ -49,7 +49,7 @@ def line_item_rates # Calculate the taxes for shipments. # # @private - # @return [Array] calculated taxes for the shipments + # @return [Array] calculated taxes for the shipments def shipment_rates order.shipments.flat_map do |shipment| calculate_rates(shipment) @@ -61,16 +61,16 @@ def shipment_rates # The item could be either a {Spree::LineItem} or a {Spree::Shipment}. # # Will go through all applicable rates for an item and create a new - # {Spree::Tax::TaxedItem} containing the calculated taxes for the item. + # {Spree::Tax::ItemTax} containing the calculated taxes for the item. # # @private - # @return [Array] calculated taxes for the item + # @return [Array] calculated taxes for the item def calculate_rates(item) rates_for_item(item).map do |rate| amount = rate.compute_amount(item) - Spree::Tax::TaxedItem.new( - id: item.id, + Spree::Tax::ItemTax.new( + item_id: item.id, label: rate.adjustment_label(amount), tax_rate: rate, amount: amount, diff --git a/core/app/models/spree/tax_calculator/shipping_rate.rb b/core/app/models/spree/tax_calculator/shipping_rate.rb index 3b4893e00c1..6d8e857e4fd 100644 --- a/core/app/models/spree/tax_calculator/shipping_rate.rb +++ b/core/app/models/spree/tax_calculator/shipping_rate.rb @@ -27,14 +27,14 @@ def initialize(shipping_rate) # Calculate taxes for a shipping rate. # - # @return [Array] the calculated taxes for the + # @return [Array] the calculated taxes for the # shipping rate def calculate rates_for_item(shipping_rate).map do |rate| amount = rate.compute_amount(shipping_rate) - Spree::Tax::TaxedItem.new( - id: shipping_rate.id, + Spree::Tax::ItemTax.new( + item_id: shipping_rate.id, label: rate.adjustment_label(amount), tax_rate: rate, amount: amount diff --git a/core/spec/models/spree/order_taxation_spec.rb b/core/spec/models/spree/order_taxation_spec.rb index 053fb43c682..1cda240e3ee 100644 --- a/core/spec/models/spree/order_taxation_spec.rb +++ b/core/spec/models/spree/order_taxation_spec.rb @@ -34,8 +34,8 @@ let(:line_item) { order.contents.add(book.master) } let(:line_item_tax) do - Spree::Tax::TaxedItem.new( - id: line_item.id, + Spree::Tax::ItemTax.new( + item_id: line_item.id, label: "Tax!", tax_rate: book_tax_rate, amount: 5, @@ -65,8 +65,8 @@ context "when new taxes are applied" do let(:new_line_item_tax) do - Spree::Tax::TaxedItem.new( - id: line_item.id, + Spree::Tax::ItemTax.new( + item_id: line_item.id, label: "Tax!", tax_rate: book_tax_rate, amount: 10, From 38004cb52501d9937b413b01c7ced3073d4eaf5e Mon Sep 17 00:00:00 2001 From: Adam Mueller Date: Mon, 22 May 2017 09:44:29 +0100 Subject: [PATCH 9/9] Rename TaxedOrder as OrderTax --- core/app/models/spree/order_taxation.rb | 4 ++-- .../spree/tax/{taxed_order.rb => order_tax.rb} | 6 +++--- core/app/models/spree/tax_calculator/default.rb | 6 +++--- core/spec/models/spree/order_taxation_spec.rb | 12 ++++++------ core/spec/models/spree/order_updater_spec.rb | 2 +- core/spec/models/spree/tax/order_adjuster_spec.rb | 2 +- .../spec/models/spree/tax_calculator/default_spec.rb | 2 +- 7 files changed, 17 insertions(+), 17 deletions(-) rename core/app/models/spree/tax/{taxed_order.rb => order_tax.rb} (74%) diff --git a/core/app/models/spree/order_taxation.rb b/core/app/models/spree/order_taxation.rb index 978f79356c4..c1b533eb3ce 100644 --- a/core/app/models/spree/order_taxation.rb +++ b/core/app/models/spree/order_taxation.rb @@ -1,5 +1,5 @@ module Spree - # Relatively simple class used to apply a {Spree::Tax::TaxedOrder} to a + # Relatively simple class used to apply a {Spree::Tax::OrderTax} to a # {Spree::Order}. # # This class will create or update adjustments on the taxed items and remove @@ -19,7 +19,7 @@ def initialize(order) # shipments in the order to reflect the appropriate taxes passed in. It # will also remove any now inapplicable tax adjustments. # - # @param [Spree::Tax::TaxedOrder] taxes the taxes to apply to the order + # @param [Spree::Tax::OrderTax] taxes the taxes to apply to the order # @return [void] def apply(taxes) @order.line_items.each do |item| diff --git a/core/app/models/spree/tax/taxed_order.rb b/core/app/models/spree/tax/order_tax.rb similarity index 74% rename from core/app/models/spree/tax/taxed_order.rb rename to core/app/models/spree/tax/order_tax.rb index 62b72d92e83..a95544f347a 100644 --- a/core/app/models/spree/tax/taxed_order.rb +++ b/core/app/models/spree/tax/order_tax.rb @@ -5,14 +5,14 @@ module Tax # Will be used by {Spree::OrderTaxation} to create or update tax # adjustments on an order. # - # @attr_reader [Integer] id the {Spree::Order} these taxes apply to + # @attr_reader [Integer] order_id the {Spree::Order} these taxes apply to # @attr_reader [Array] line_item_taxes an array of # tax data for order's line items # @attr_reader [Array] shipment_taxes an array of # tax data for the order's shipments - class TaxedOrder + class OrderTax include ActiveModel::Model - attr_accessor :id, :line_item_taxes, :shipment_taxes + attr_accessor :order_id, :line_item_taxes, :shipment_taxes end end end diff --git a/core/app/models/spree/tax_calculator/default.rb b/core/app/models/spree/tax_calculator/default.rb index bab6cc57147..ddf7015d354 100644 --- a/core/app/models/spree/tax_calculator/default.rb +++ b/core/app/models/spree/tax_calculator/default.rb @@ -23,10 +23,10 @@ def initialize(order) # Calculate taxes for an order. # - # @return [Spree::Tax::TaxedOrder] the calculated taxes for the order + # @return [Spree::Tax::OrderTax] the calculated taxes for the order def calculate - Spree::Tax::TaxedOrder.new( - id: order.id, + Spree::Tax::OrderTax.new( + order_id: order.id, line_item_taxes: line_item_rates, shipment_taxes: shipment_rates ) diff --git a/core/spec/models/spree/order_taxation_spec.rb b/core/spec/models/spree/order_taxation_spec.rb index 1cda240e3ee..7fd4e7bec31 100644 --- a/core/spec/models/spree/order_taxation_spec.rb +++ b/core/spec/models/spree/order_taxation_spec.rb @@ -44,8 +44,8 @@ end let(:taxes) do - Spree::Tax::TaxedOrder.new( - id: order.id, + Spree::Tax::OrderTax.new( + order_id: order.id, line_item_taxes: [line_item_tax], shipment_taxes: [] ) @@ -75,8 +75,8 @@ end let(:new_taxes) do - Spree::Tax::TaxedOrder.new( - id: order.id, + Spree::Tax::OrderTax.new( + order_id: order.id, line_item_taxes: [new_line_item_tax], shipment_taxes: [] ) @@ -107,8 +107,8 @@ context "when taxes are removed" do let(:new_taxes) do - Spree::Tax::TaxedOrder.new( - id: order.id, + Spree::Tax::OrderTax.new( + order_id: order.id, line_item_taxes: [], shipment_taxes: [] ) diff --git a/core/spec/models/spree/order_updater_spec.rb b/core/spec/models/spree/order_updater_spec.rb index b9c4f24beb8..4016d9c2c47 100644 --- a/core/spec/models/spree/order_updater_spec.rb +++ b/core/spec/models/spree/order_updater_spec.rb @@ -313,7 +313,7 @@ def create_adjustment(label, amount) it 'uses the configured class' do expect(custom_calculator_class).to receive(:new).with(order).at_least(:once).and_return(custom_calculator_instance) expect(custom_calculator_instance).to receive(:calculate).at_least(:once).and_return( - Spree::Tax::TaxedOrder.new(id: order.id, line_item_taxes: [], shipment_taxes: []) + Spree::Tax::OrderTax.new(order_id: order.id, line_item_taxes: [], shipment_taxes: []) ) order.update! diff --git a/core/spec/models/spree/tax/order_adjuster_spec.rb b/core/spec/models/spree/tax/order_adjuster_spec.rb index 2a7e59b200d..0cff41f703d 100644 --- a/core/spec/models/spree/tax/order_adjuster_spec.rb +++ b/core/spec/models/spree/tax/order_adjuster_spec.rb @@ -24,7 +24,7 @@ it 'calls the configured tax calculator' do expect(custom_calculator_class).to receive(:new).with(order).at_least(:once).and_return(custom_calculator_instance) expect(custom_calculator_instance).to receive(:calculate).at_least(:once).and_return( - Spree::Tax::TaxedOrder.new(id: order.id, line_item_taxes: [], shipment_taxes: []) + Spree::Tax::OrderTax.new(order_id: order.id, line_item_taxes: [], shipment_taxes: []) ) adjuster.adjust! diff --git a/core/spec/models/spree/tax_calculator/default_spec.rb b/core/spec/models/spree/tax_calculator/default_spec.rb index a02e523f0cc..221dcf94972 100644 --- a/core/spec/models/spree/tax_calculator/default_spec.rb +++ b/core/spec/models/spree/tax_calculator/default_spec.rb @@ -35,7 +35,7 @@ describe '#calculate' do subject(:calculated_taxes) { calculator.calculate } - it { is_expected.to be_a Spree::Tax::TaxedOrder } + it { is_expected.to be_a Spree::Tax::OrderTax } it "has tax information for the line item", aggregate_failures: true do expect(calculated_taxes.line_item_taxes.count).to eq 1