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

Make cart taxes configurable via Spree::Store object #933

Merged
merged 8 commits into from Mar 16, 2016
15 changes: 15 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,20 @@
## Solidus 1.3.0 (unreleased)

* Taxes for carts now configurable via the `Spree::Store` object

In VAT countries, carts (orders without addresses) have to be shown with
adjustments for the country whose taxes the cart's prices supposedly include.
This might differ from `Spree::Store` to `Spree::Store`. We're introducting
the `cart_tax_country_iso` setting on Spree::Store for this purpose.

Previously the setting for what country any prices include
Spree::Zone.default_tax. That, however, would *also* implicitly tag all
prices in Spree as including the taxes from that zone. Introducing the cart
tax setting on Spree::Store relieves that boolean of some of its
responsibilities.

https://github.com/solidusio/solidus/pull/933

* Make Spree::Product#prices association return all prices

Previously, only non-master variant prices would have been returned here.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ module Spree

context "order contents changed after shipments were created" do
let!(:store) { create(:store) }
let!(:order) { Order.create }
let!(:order) { Order.create(store: store) }
let!(:line_item) { order.contents.add(product.master) }

before { order.create_proposed_shipments }
Expand Down
19 changes: 19 additions & 0 deletions core/app/models/spree/app_configuration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,12 @@ class AppConfiguration < Preferences::Configuration
# @return [String] Two-letter ISO code of a {Spree::Country} to assumed as the country of an unidentified customer (default: "US")
preference :default_country_iso, :string, default: 'US'

# @!attribute [rw] admin_vat_country_iso
# Set this if you want to enter prices in the backend including value added tax.
# @return [String, nil] Two-letter ISO code of that {Spree::Country} for which
# prices are entered in the backend (default: nil)
preference :admin_vat_country_iso, :string, default: nil

# @!attribute [rw] expedited_exchanges
# Kicks off an exchange shipment upon return authorization save.
# charge customer if they do not return items within timely manner.
Expand Down Expand Up @@ -324,6 +330,19 @@ def stock
@stock_configuration ||= Spree::Core::StockConfiguration.new
end

# Default admin VAT location
#
# An object that responds to :state_id and :country_id so it can double as a Spree::Address in
# Spree::Zone.for_address. Takes the `admin_vat_country_iso` as input.
#
# @see admin_vat_country_iso The admin VAT country
# @return [Spree::Tax::TaxLocation] default tax location
def admin_vat_location
@default_tax_location ||= Spree::Tax::TaxLocation.new(
country: Spree::Country.find_by(iso: admin_vat_country_iso)
)
end

# all the following can be deprecated when store prefs are no longer supported
# @private
DEPRECATED_STORE_PREFERENCES = {
Expand Down
6 changes: 5 additions & 1 deletion core/app/models/spree/order.rb
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,11 @@ def tax_zone

# Returns the address for taxation based on configuration
def tax_address
Spree::Config[:tax_using_ship_address] ? ship_address : bill_address
if Spree::Config[:tax_using_ship_address]
ship_address
else
bill_address
end || store.default_cart_tax_location
end

def updater
Expand Down
5 changes: 5 additions & 0 deletions core/app/models/spree/store.rb
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,11 @@ def self.default
where(default: true).first || new
end

def default_cart_tax_location
@default_cart_tax_location ||=
Spree::Tax::TaxLocation.new(country: Spree::Country.find_by(iso: cart_tax_country_iso))
end

private

def ensure_default_exists_and_is_unique
Expand Down
33 changes: 33 additions & 0 deletions core/app/models/spree/tax/tax_location.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
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 [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, state: nil)
@country_id = country && country.id
@state_id = state && state.id
end

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

def empty?
country_id.nil? && state_id.nil?
end
end
end
end
3 changes: 3 additions & 0 deletions core/app/models/spree/tax_rate.rb
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,9 @@ class TaxRate < Spree::Base
validates :tax_category_id, presence: true
validates_with DefaultTaxZoneValidator

# Finds all tax rates whose zones match a given address
scope :for_address, ->(address) { joins(:zone).merge(Spree::Zone.for_address(address)) }

# Finds geographically matching tax rates for a tax zone.
# We do not know if they are/aren't applicable until we attempt to apply these rates to
# the items contained within the Order itself.
Expand Down
1 change: 1 addition & 0 deletions core/db/default/spree/stores.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,5 +5,6 @@
s.name = 'Spree Demo Site'
s.url = 'demo.spreecommerce.com'
s.mail_from_address = '[email protected]'
s.cart_tax_country_iso = Spree::Config.admin_vat_location
end.save!
end
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
class AddCartTaxCountryIsoToSpreeStore < ActiveRecord::Migration
def change
add_column :spree_stores, :cart_tax_country_iso, :string, null: true, default: nil
end
end
12 changes: 12 additions & 0 deletions core/spec/models/spree/app_configuration_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -30,4 +30,16 @@
expect(prefs[:default_country_iso]).to eq("US")
end
end

describe '@admin_vat_country_iso' do
it 'is `nil` by default' do
expect(prefs[:admin_vat_country_iso]).to eq(nil)
end
end

it 'has a default admin VAT location with nil values by default' do
expect(prefs.admin_vat_location).to eq(Spree::Tax::TaxLocation.new)
expect(prefs.admin_vat_location.state_id).to eq(nil)
expect(prefs.admin_vat_location.country_id).to eq(nil)
end
end
2 changes: 1 addition & 1 deletion core/spec/models/spree/order/totals_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

module Spree
describe Order, type: :model do
let(:order) { Order.create }
let(:order) { create(:order) }
let(:shirt) { create(:variant) }

context "adds item to cart and activates promo" do
Expand Down
1 change: 1 addition & 0 deletions core/spec/models/spree/order_merger_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
module Spree
describe OrderMerger, type: :model do
let(:variant) { create(:variant) }
let!(:store) { create(:store, default: true) }
let(:order_1) { Spree::Order.create }
let(:order_2) { Spree::Order.create }
let(:user) { stub_model(Spree::LegacyUser, email: "[email protected]") }
Expand Down
45 changes: 37 additions & 8 deletions core/spec/models/spree/order_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -395,22 +395,51 @@ def merge!(other_order, user = nil)
end

describe "#tax_address" do
let(:order) { build(:order, ship_address: ship_address, bill_address: bill_address, store: store) }
let(:store) { build(:store) }

before { Spree::Config[:tax_using_ship_address] = tax_using_ship_address }
subject { order.tax_address }

context "when tax_using_ship_address is true" do
let(:tax_using_ship_address) { true }
context "when the order has no addresses" do
let(:ship_address) { nil }
let(:bill_address) { nil }

context "when tax_using_ship_address is true" do
let(:tax_using_ship_address) { true }

it 'returns ship_address' do
expect(subject).to eq(order.ship_address)
it 'returns the stores default cart tax location' do
expect(subject).to eq(store.default_cart_tax_location)
end
end

context "when tax_using_ship_address is not true" do
let(:tax_using_ship_address) { false }

it 'returns the stores default cart tax location' do
expect(subject).to eq(store.default_cart_tax_location)
end
end
end

context "when tax_using_ship_address is not true" do
let(:tax_using_ship_address) { false }
context "when the order has addresses" do
let(:ship_address) { build(:address) }
let(:bill_address) { build(:address) }

it "returns bill_address" do
expect(subject).to eq(order.bill_address)
context "when tax_using_ship_address is true" do
let(:tax_using_ship_address) { true }

it 'returns ship_address' do
expect(subject).to eq(order.ship_address)
end
end

context "when tax_using_ship_address is not true" do
let(:tax_using_ship_address) { false }

it "returns bill_address" do
expect(subject).to eq(order.bill_address)
end
end
end
end
Expand Down
21 changes: 21 additions & 0 deletions core/spec/models/spree/store_spec.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
require 'spec_helper'

describe Spree::Store, type: :model do
it { is_expected.to respond_to(:cart_tax_country_iso) }

describe ".by_url" do
let!(:store) { create(:store, url: "website1.com\nwww.subdomain.com") }
let!(:store_2) { create(:store, url: 'freethewhales.com') }
Expand Down Expand Up @@ -50,4 +52,23 @@
expect(store.default).not_to be true
end
end

describe '#default_cart_tax_location' do
subject { described_class.new(cart_tax_country_iso: cart_tax_country_iso) }
context "when there is no cart_tax_country_iso set" do
let(:cart_tax_country_iso) { '' }
it "responds with an empty default_cart_tax_location" do
expect(subject.default_cart_tax_location).to be_empty
end
end

context "when there is a cart_tax_country_iso set" do
let(:country) { create(:country, iso: "DE") }
let(:cart_tax_country_iso) { country.iso }

it "responds with a default_cart_tax_location with that country" do
expect(subject.default_cart_tax_location).to eq(Spree::Tax::TaxLocation.new(country: country))
end
end
end
end
68 changes: 68 additions & 0 deletions core/spec/models/spree/tax/tax_location_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
require 'spec_helper'

RSpec.describe Spree::Tax::TaxLocation do
let(:country) { build_stubbed(:country) }
let(:state) { build_stubbed(:state) }

subject { described_class.new }

it { is_expected.to respond_to(:state_id) }
it { is_expected.to respond_to(:country_id) }

describe "default values" do
it "has a nil state and country id" do
expect(subject.state_id).to eq(nil)
expect(subject.country_id).to eq(nil)
end
end

describe '#==' do
let(:other) { described_class.new(state: nil, country: nil) }

it 'compares the values of state id and country id and does not care about object identity' do
expect(subject).to eq(other)
end
end

describe "initialization" do
subject { described_class.new(args) }

context 'with a country object' do
let(:args) { { country: country } }

it "will yield a location with that country's id" do
expect(subject.country_id).to eq(country.id)
end
end

context 'with a state object' do
let(:args) { { state: state } }

it "will yield a location with that state's id" do
expect(subject.state_id).to eq(state.id)
end
end
end

describe "#empty?" do
subject { described_class.new(args).empty? }

context 'with a country present' do
let(:args) { { country: country } }

it { is_expected.to be false }
end

context 'with a state present' do
let(:args) { { state: state } }

it { is_expected.to be false }
end

context 'with no region data present' do
let(:args) { {} }

it { is_expected.to be true }
end
end
end
47 changes: 47 additions & 0 deletions core/spec/models/spree/tax_rate_spec.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,53 @@
require 'spec_helper'

describe Spree::TaxRate, type: :model do
context '.for_address' do
let(:germany) { create(:country, iso: "DE") }
let(:germany_zone) { create(:zone, countries: [germany]) }
let!(:german_tax) { create(:tax_rate, zone: germany_zone) }
let(:france) { create(:country, iso: "FR") }
let(:france_zone) { create(:zone, countries: [france]) }
let!(:french_tax) { create(:tax_rate, zone: france_zone) }
let(:eu_zone) { create(:zone, countries: [germany, france]) }
let!(:eu_tax) { create(:tax_rate, zone: eu_zone) }
let(:usa) { create(:country, iso: "US") }
let(:us_zone) { create(:zone, countries: [usa]) }
let!(:us_tax) { create(:tax_rate, zone: us_zone) }
let(:new_york) { create(:state, country: usa, state_code: "NY") }
let(:new_york_zone) { create(:zone, states: [new_york]) }
let!(:new_york_tax) { create(:tax_rate, zone: new_york_zone) }
let(:alabama) { create(:state, country: usa, state_code: "AL") }
let(:alabama_zone) { create(:zone, states: [alabama]) }
let!(:alabama_tax) { create(:tax_rate, zone: alabama_zone) }

subject(:rates_for_address) { Spree::TaxRate.for_address(address) }

context 'when address is in germany' do
let(:address) { create(:address, country_iso_code: "DE") }
it { is_expected.to contain_exactly(german_tax, eu_tax) }
end

context 'when address is in france' do
let(:address) { create(:address, country_iso_code: "FR") }
it { is_expected.to contain_exactly(french_tax, eu_tax) }
end

context 'when address is in new york' do
let(:address) { create(:address, country_iso_code: "US", state_code: "NY") }
it { is_expected.to contain_exactly(new_york_tax, us_tax) }
end

context 'when address is in alabama' do
let(:address) { create(:address, country_iso_code: "US", state_code: "AL") }
it { is_expected.to contain_exactly(alabama_tax, us_tax) }
end

context 'when address is in alaska' do
let(:address) { create(:address, country_iso_code: "US", state_code: "AK") }
it { is_expected.to contain_exactly(us_tax) }
end
end

context ".for_zone" do
subject(:rates_for_zone) { Spree::TaxRate.for_zone(zone) }

Expand Down