Skip to content

Commit

Permalink
Speed up taxation of shipping rates
Browse files Browse the repository at this point in the history
Instead of loading the tax rates for the order for each rate which needs
taxing, we can perform the SQL queries only once.

This is done by building a single instance of the
shipping_rate_tax_calculator_class for a shipment, and calling
`calculate` on that object for each rate, allowing it to cache the order
rates.
  • Loading branch information
jhawthorn committed Sep 12, 2017
1 parent 332e216 commit 3639585
Show file tree
Hide file tree
Showing 5 changed files with 30 additions and 16 deletions.
10 changes: 9 additions & 1 deletion core/app/models/spree/stock/estimator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -31,14 +31,22 @@ def choose_default_shipping_rate(shipping_rates)
end

def calculate_shipping_rates(package)
tax_calculator_class = Spree::Config.shipping_rate_tax_calculator_class
tax_calculator = tax_calculator_class.new(package.shipment.order)
shipping_methods(package).map do |shipping_method|
cost = shipping_method.calculator.compute(package)
if cost
rate = shipping_method.shipping_rates.new(
cost: cost,
shipment: package.shipment
)
Spree::Config.shipping_rate_taxer_class.new.tax(rate)
tax_calculator.calculate(rate).each do |tax|
rate.taxes.new(
amount: tax.amount,
tax_rate: tax.tax_rate
)
end
rate
end
end.compact
end
Expand Down
2 changes: 1 addition & 1 deletion core/app/models/spree/tax/shipping_rate_taxer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ class ShippingRateTaxer
# This parameter will be modified.
# @return [Spree::ShippingRate] The shipping rate with associated tax objects
def tax(shipping_rate)
taxes = Spree::Config.shipping_rate_tax_calculator_class.new(shipping_rate).calculate
taxes = Spree::Config.shipping_rate_tax_calculator_class.new(shipping_rate.order).calculate(shipping_rate)
taxes.each do |tax|
shipping_rate.taxes.build(
amount: tax.amount,
Expand Down
15 changes: 12 additions & 3 deletions core/app/models/spree/tax_calculator/shipping_rate.rb
Original file line number Diff line number Diff line change
Expand Up @@ -21,15 +21,24 @@ class ShippingRate
# @param [Spree::ShippingRate] shipping_rate the shipping rate to
# calculate taxes on
# @return [Spree::TaxCalculator::ShippingRate]
def initialize(shipping_rate)
@shipping_rate = shipping_rate
def initialize(order)
if order.is_a?(::Spree::ShippingRate)
Spree::Deprecation.warn "passing a single shipping rate to Spree::TaxCalculator::ShippingRate is DEPRECATED. It now expects an order"
shipping_rate = order
@order = shipping_rate.order
@shipping_rate = shipping_rate
else
@order = order
@shipping_rate = nil
end
end

# Calculate taxes for a shipping rate.
#
# @return [Array<Spree::Tax::ItemTax>] the calculated taxes for the
# shipping rate
def calculate
def calculate(shipping_rate)
shipping_rate ||= @shipping_rate
rates_for_item(shipping_rate).map do |rate|
amount = rate.compute_amount(shipping_rate)

Expand Down
2 changes: 0 additions & 2 deletions core/lib/spree/app_configuration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -287,8 +287,6 @@ def default_pricing_options

class_name_attribute :shipping_rate_selector_class, default: 'Spree::Stock::ShippingRateSelector'

class_name_attribute :shipping_rate_taxer_class, default: 'Spree::Tax::ShippingRateTaxer'

# Allows providing your own class for calculating taxes on a shipping rate.
#
# @!attribute [rw] shipping_rate_tax_calculator_class
Expand Down
17 changes: 8 additions & 9 deletions core/spec/models/spree/stock/estimator_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -189,20 +189,19 @@ class Spree::Stock::TestSorter; end;
end

it 'uses the configured shipping rate taxer' do
class Spree::Tax::TestTaxer
def initialize
class Spree::Tax::TestTaxCalculator
def initialize(_order)
end

def tax(_)
Spree::ShippingRate.new
def calculate(_shipping_rate)
[
Spree::Tax::ItemTax.new(label: "TAX", amount: 5)
]
end
end
Spree::Config.shipping_rate_taxer_class = Spree::Tax::TestTaxer

shipping_rate = Spree::ShippingRate.new
allow(Spree::ShippingRate).to receive(:new).and_return(shipping_rate)
Spree::Config.shipping_rate_tax_calculator_class = Spree::Tax::TestTaxCalculator

expect(Spree::Tax::TestTaxer).to receive(:new).and_call_original
expect(Spree::Tax::TestTaxCalculator).to receive(:new).and_call_original
subject.shipping_rates(package)
end
end
Expand Down

0 comments on commit 3639585

Please sign in to comment.