diff --git a/CHANGELOG.md b/CHANGELOG.md index 5e4f4931cd1..f8d17fe4345 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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. diff --git a/api/spec/controllers/spree/api/line_items_controller_spec.rb b/api/spec/controllers/spree/api/line_items_controller_spec.rb index 5c8782608a2..1b5e15a6cb6 100644 --- a/api/spec/controllers/spree/api/line_items_controller_spec.rb +++ b/api/spec/controllers/spree/api/line_items_controller_spec.rb @@ -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 } diff --git a/core/app/models/spree/app_configuration.rb b/core/app/models/spree/app_configuration.rb index 74da03e699a..96963e23f6a 100644 --- a/core/app/models/spree/app_configuration.rb +++ b/core/app/models/spree/app_configuration.rb @@ -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. @@ -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 = { diff --git a/core/app/models/spree/order.rb b/core/app/models/spree/order.rb index 7bac0d67893..e73d70a7695 100644 --- a/core/app/models/spree/order.rb +++ b/core/app/models/spree/order.rb @@ -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 diff --git a/core/app/models/spree/store.rb b/core/app/models/spree/store.rb index 055b1f504bb..c263884b596 100644 --- a/core/app/models/spree/store.rb +++ b/core/app/models/spree/store.rb @@ -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 diff --git a/core/app/models/spree/tax/tax_location.rb b/core/app/models/spree/tax/tax_location.rb new file mode 100644 index 00000000000..43f292b64e0 --- /dev/null +++ b/core/app/models/spree/tax/tax_location.rb @@ -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 diff --git a/core/app/models/spree/tax_rate.rb b/core/app/models/spree/tax_rate.rb index 92dfbeaa63f..b6ddf574816 100644 --- a/core/app/models/spree/tax_rate.rb +++ b/core/app/models/spree/tax_rate.rb @@ -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. diff --git a/core/db/default/spree/stores.rb b/core/db/default/spree/stores.rb index 43e39b10a77..ecccbcd467c 100644 --- a/core/db/default/spree/stores.rb +++ b/core/db/default/spree/stores.rb @@ -5,5 +5,6 @@ s.name = 'Spree Demo Site' s.url = 'demo.spreecommerce.com' s.mail_from_address = 'spree@example.com' + s.cart_tax_country_iso = Spree::Config.admin_vat_location end.save! end diff --git a/core/db/migrate/20160229133259_add_cart_tax_country_iso_to_spree_store.rb b/core/db/migrate/20160229133259_add_cart_tax_country_iso_to_spree_store.rb new file mode 100644 index 00000000000..68fb7ab9987 --- /dev/null +++ b/core/db/migrate/20160229133259_add_cart_tax_country_iso_to_spree_store.rb @@ -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 diff --git a/core/spec/models/spree/app_configuration_spec.rb b/core/spec/models/spree/app_configuration_spec.rb index 41960ccfc3c..78f0c600175 100644 --- a/core/spec/models/spree/app_configuration_spec.rb +++ b/core/spec/models/spree/app_configuration_spec.rb @@ -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 diff --git a/core/spec/models/spree/order/totals_spec.rb b/core/spec/models/spree/order/totals_spec.rb index 409bb6f7add..69e4e715f87 100644 --- a/core/spec/models/spree/order/totals_spec.rb +++ b/core/spec/models/spree/order/totals_spec.rb @@ -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 diff --git a/core/spec/models/spree/order_merger_spec.rb b/core/spec/models/spree/order_merger_spec.rb index 07aa268ce1c..1a38564fd24 100644 --- a/core/spec/models/spree/order_merger_spec.rb +++ b/core/spec/models/spree/order_merger_spec.rb @@ -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: "spree@example.com") } diff --git a/core/spec/models/spree/order_spec.rb b/core/spec/models/spree/order_spec.rb index 9452c65f9c1..24664a52421 100644 --- a/core/spec/models/spree/order_spec.rb +++ b/core/spec/models/spree/order_spec.rb @@ -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 diff --git a/core/spec/models/spree/store_spec.rb b/core/spec/models/spree/store_spec.rb index 4162970129e..f226e50d805 100644 --- a/core/spec/models/spree/store_spec.rb +++ b/core/spec/models/spree/store_spec.rb @@ -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') } @@ -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 diff --git a/core/spec/models/spree/tax/tax_location_spec.rb b/core/spec/models/spree/tax/tax_location_spec.rb new file mode 100644 index 00000000000..1f1eb46fef8 --- /dev/null +++ b/core/spec/models/spree/tax/tax_location_spec.rb @@ -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 diff --git a/core/spec/models/spree/tax_rate_spec.rb b/core/spec/models/spree/tax_rate_spec.rb index 8352cab1208..e7d53bdc97c 100644 --- a/core/spec/models/spree/tax_rate_spec.rb +++ b/core/spec/models/spree/tax_rate_spec.rb @@ -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) }