Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Match tax zones by address instead of by zone #862

Closed
wants to merge 20 commits into from
Closed
Show file tree
Hide file tree
Changes from 17 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 0 additions & 5 deletions backend/app/views/spree/admin/zones/_form.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,6 @@
<%= zone_form.text_field :description, :class => 'fullwidth' %>
<% end %>

<div data-hook="default" class="field">
<%= zone_form.check_box :default_tax %>
<%= zone_form.label :default_tax %>
</div>

<div data-hook="type" class="field">
<%= label_tag Spree.t(:type) %>
<ul>
Expand Down
2 changes: 0 additions & 2 deletions backend/app/views/spree/admin/zones/index.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@
<th>
<%= sort_link @search,:description, Spree::Zone.human_attribute_name(:description), {}, {:title => 'zones_order_by_description_title'} %>
</th>
<th><%= Spree.t(:default_tax) %></th>
<th class="actions"></th>
</tr>
</thead>
Expand All @@ -37,7 +36,6 @@
<tr id="<%= spree_dom_id zone %>" data-hook="zones_row" class="<%= cycle('odd', 'even')%>">
<td class="align-center"><%= zone.name %></td>
<td><%= zone.description %></td>
<td class="align-center"><%= zone.default_tax? ? Spree.t(:say_yes) : Spree.t(:say_no) %></td>
<td class="actions">
<% if can?(:update, zone) %>
<%= link_to_edit zone, :no_text => true %>
Expand Down
13 changes: 13 additions & 0 deletions core/app/models/spree/app_configuration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -320,6 +320,19 @@ def stock
@stock_configuration ||= Spree::Core::StockConfiguration.new
end

# @!attribute [rw] default_tax_address
# Default Address used for taxation when no tax address present on an order
# Use this when you want the following behaviour: The order is tax-adjusted
# even if you don't yet know its tax_address (see also the setting `tax_using_ship_address`).
# If you do not wish your order to be taxed before knowing the customer's address,
# leave it at `nil`.
# @example Use the default country for taxing orders in cart state
# Spree.config do |config|
# config.default_tax_address = Spree::Address.build_default.freeze
# end
# @return [Spree::Address,nil] default tax address
attr_accessor :default_tax_address

# all the following can be deprecated when store prefs are no longer supported
# @private
DEPRECATED_STORE_PREFERENCES = {
Expand Down
2 changes: 1 addition & 1 deletion core/app/models/spree/calculator/default_tax.rb
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ def compute_shipment_or_line_item(item)
def compute_shipping_rate(shipping_rate)
if rate.included_in_price
pre_tax_amount = shipping_rate.cost / (1 + rate.amount)
if rate.zone == shipping_rate.shipment.order.tax_zone
if Spree::TaxRate.for_address(shipping_rate.order.tax_address).include?(rate)
deduced_total_by_rate(pre_tax_amount, rate)
else
deduced_total_by_rate(pre_tax_amount, rate) * - 1
Expand Down
5 changes: 3 additions & 2 deletions core/app/models/spree/order.rb
Original file line number Diff line number Diff line change
Expand Up @@ -203,12 +203,13 @@ def backordered?
# Returns the relevant zone (if any) to be used for taxation purposes.
# Uses default tax zone unless there is a specific match
def tax_zone
@tax_zone ||= Zone.match(tax_address) || Zone.default_tax
@tax_zone ||= Zone.match(tax_address)
end

# Returns the address for taxation based on configuration
def tax_address
Spree::Config[:tax_using_ship_address] ? ship_address : bill_address
(Spree::Config[:tax_using_ship_address] ? ship_address : bill_address) ||
Spree::Config.default_tax_address
end

def updater
Expand Down
10 changes: 4 additions & 6 deletions core/app/models/spree/shipment.rb
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@ class Shipment < Spree::Base
scope :reverse_chronological, -> { order('coalesce(spree_shipments.shipped_at, spree_shipments.created_at) desc', id: :desc) }
scope :by_store, ->(store) { joins(:order).merge(Spree::Order.by_store(store)) }

delegate :tax_category, to: :selected_shipping_rate

# shipment state machine (see http://github.com/pluginaweek/state_machine/tree/master for details)
state_machine initial: :pending, use_transactions: false do
event :ready do
Expand Down Expand Up @@ -88,8 +90,8 @@ def can_transition_from_canceled_to_ready?
end

extend DisplayMoney
money_methods :cost, :discounted_cost, :final_price, :item_cost
alias display_amount display_cost
money_methods :cost, :amount, :discounted_cost, :final_price, :item_cost
alias_attribute :amount, :cost

def add_shipping_method(shipping_method, selected = false)
shipping_rates.create(shipping_method: shipping_method, selected: selected, cost: cost)
Expand Down Expand Up @@ -244,10 +246,6 @@ def shipping_method
selected_shipping_rate.try(:shipping_method) || shipping_rates.first.try(:shipping_method)
end

def tax_category
selected_shipping_rate.try(:tax_rate).try(:tax_category)
end

# Only one of either included_tax_total or additional_tax_total is set
# This method returns the total of the two. Saves having to check if
# tax is included or additional.
Expand Down
59 changes: 39 additions & 20 deletions core/app/models/spree/shipping_rate.rb
Original file line number Diff line number Diff line change
@@ -1,12 +1,14 @@
module Spree
class ShippingRate < Spree::Base
include Spree::Tax::TaxHelpers

belongs_to :shipment, class_name: 'Spree::Shipment'
belongs_to :shipping_method, -> { with_deleted }, class_name: 'Spree::ShippingMethod', inverse_of: :shipping_rates
belongs_to :tax_rate, -> { with_deleted }, class_name: 'Spree::TaxRate'

delegate :order, :currency, to: :shipment
delegate :name, to: :shipping_method
delegate :name, :tax_category, to: :shipping_method
delegate :code, to: :shipping_method, prefix: true
alias_attribute :amount, :cost

def display_base_price
Spree::Money.new(cost, currency: currency)
Expand All @@ -16,31 +18,48 @@ def calculate_tax_amount
tax_rate.calculator.compute_shipping_rate(self)
end

def tax_rates
applicable_rates.select { |rate| rate.applicable_for?(self) }
end

def display_price
price = display_base_price.to_s
if tax_rate
tax_amount = calculate_tax_amount
if tax_amount != 0
if tax_rate.included_in_price?
if tax_amount > 0
amount = "#{display_tax_amount(tax_amount)} #{tax_rate.name}"
price += " (#{Spree.t(:incl)} #{amount})"
else
amount = "#{display_tax_amount(tax_amount * -1)} #{tax_rate.name}"
price += " (#{Spree.t(:excl)} #{amount})"
end
else
amount = "#{display_tax_amount(tax_amount)} #{tax_rate.name}"
price += " (+ #{amount})"
end
end
end
price

return price if tax_rates.empty? || amount == 0

tax_explanations = tax_rates.map { |rate| tax_explain(rate) }.join(", ")

Spree.t :display_price_with_explanations,
scope: 'shipping_rate.display_price',
price: price,
explanations: tax_explanations
end
alias_method :display_cost, :display_price

def display_tax_amount(tax_amount)
Spree::Money.new(tax_amount, currency: currency)
end

private

def tax_explain(rate)
amount = rate.calculator.compute_shipping_rate(self)
Spree.t translation_key(amount, rate),
scope: 'shipping_rate.display_price.tax_explanations',
tax_amount: display_tax_amount(amount.abs),
tax_rate_name: rate.name
end

def translation_key(amount, rate)
if rate.included_in_price?
if amount > 0
:vat
else
:vat_refund
end
else
:sales_tax
end
end
end
end
14 changes: 1 addition & 13 deletions core/app/models/spree/stock/estimator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -35,22 +35,10 @@ def choose_default_shipping_rate(shipping_rates)
def calculate_shipping_rates(package)
shipping_methods(package).map do |shipping_method|
cost = shipping_method.calculator.compute(package)
tax_category = shipping_method.tax_category
if tax_category
tax_rate = tax_category.tax_rates.detect do |rate|
# If the rate's zone matches the order's zone, a positive adjustment will be applied.
# If the rate is from the default tax zone, then a negative adjustment will be applied.
# See the tests in shipping_rate_spec.rb for an example of this.d
rate.zone == order.tax_zone || rate.zone.default_tax?
end
end

if cost
rate = shipping_method.shipping_rates.new(cost: cost)
rate.tax_rate = tax_rate if tax_rate
shipping_method.shipping_rates.new(cost: cost)
end

rate
end.compact
end

Expand Down
9 changes: 5 additions & 4 deletions core/app/models/spree/tax/item_adjuster.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,8 @@ def initialize(item, options = {})
@item = item
@order = @item.order
# set instance variable so `TaxRate.match` is only called when necessary
@rates_for_order_zone = options[:rates_for_order_zone]
@order_rates = options[:order_rates]
@default_vat_rates = options[:default_vat_rates]
end

# Deletes all existing tax adjustments and creates new adjustments for all
Expand All @@ -23,19 +24,19 @@ def initialize(item, options = {})
#
# @return [Array<Spree::Adjustment>] newly created adjustments
def adjust!
return unless order_tax_zone
return unless order.tax_address
# Using .destroy_all to make sure callbacks fire
item.adjustments.tax.destroy_all

TaxRate.store_pre_tax_amount(item, rates_for_item)

rates_for_item.map { |rate| rate.adjust(order_tax_zone, item) }
rates_for_item.map { |rate| rate.adjust(nil, item) }
end

private

def rates_for_item
@rates_for_item ||= applicable_rates.select { |rate| rate.tax_category == item.tax_category }
@rates_for_item ||= applicable_rates.select { |rate| rate.applicable_for?(item) }
end
end
end
Expand Down
13 changes: 11 additions & 2 deletions core/app/models/spree/tax/order_adjuster.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,21 @@ def initialize(order)
# Creates tax adjustments for all taxable items (shipments and line items)
# in the given order.
def adjust!
return unless order_tax_zone
return unless order.tax_address

(order.line_items + order.shipments).each do |item|
ItemAdjuster.new(item, rates_for_order_zone: applicable_rates).adjust!
ItemAdjuster.new(item, order_wide_options).adjust!
end
end

private

def order_wide_options
{
order_rates: order_rates,
default_vat_rates: default_vat_rates
}
end
end
end
end
20 changes: 9 additions & 11 deletions core/app/models/spree/tax/tax_helpers.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,24 +17,22 @@ module TaxHelpers
#
# For further discussion, see https://github.com/spree/spree/issues/4397 and https://github.com/spree/spree/issues/4327.
def applicable_rates
order_zone_tax_categories = rates_for_order_zone.map(&:tax_category)
default_rates_with_unmatched_tax_category = rates_for_default_zone.to_a.delete_if do |default_rate|
order_zone_tax_categories = order_rates.map(&:tax_category)
default_rates_with_unmatched_tax_category = default_vat_rates.to_a.delete_if do |default_rate|
order_zone_tax_categories.include?(default_rate.tax_category)
end

(rates_for_order_zone + default_rates_with_unmatched_tax_category).uniq
(order_rates + default_rates_with_unmatched_tax_category).uniq
end

def rates_for_order_zone
@rates_for_order_zone ||= Spree::TaxRate.for_zone(order_tax_zone)
def order_rates
@order_rates ||= Spree::TaxRate.for_address(order.tax_address)
end

def rates_for_default_zone
@rates_for_default_zone ||= Spree::TaxRate.for_zone(Spree::Zone.default_tax)
end

def order_tax_zone
@order_tax_zone ||= order.tax_zone
def default_vat_rates
@default_vat_rates ||= Spree::TaxRate
.included_in_price
.for_address(Spree::Config.default_tax_address)
end
end
end
Expand Down
31 changes: 31 additions & 0 deletions core/app/models/spree/tax/tax_location.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
module Spree
module Tax
# A class exclusively used as a drop-in replacement for a default tax address.
# It responds to `:country_id` and `:state_id`.
#
# @attr_reader [Integer] country_id the ID of a Spree::Country object
# @attr_reader [Integer] state_id the ID of a Spree::State object
class TaxLocation
attr_reader :country_id, :state_id

# Create a new TaxLocation object
#
# @see Spree::Zone.for_address
#
# @param [Integer] country_id the ID of a Spree::Country object, default: nil
# @param [Integer] state_id the ID of a Spree::State object, default: nil
# @param [Spree::Country] country a Spree::Country object, default: nil
# @param [Spree::State] state a Spree::State object, default: nil
#
# @return [Spree::Tax::TaxLocation] a Spree::Tax::TaxLocation object
def initialize(country: nil, country_id: nil, state: nil, state_id: nil)
@country_id = country_id || country && country.id
@state_id = state_id || state && state.id
end

def ==(other)
state_id == other.state_id && country_id == other.country_id
end
end
end
end
Loading