diff --git a/core/app/models/spree/tax_rate.rb b/core/app/models/spree/tax_rate.rb index 88118f0c9a7..1764f390cd3 100644 --- a/core/app/models/spree/tax_rate.rb +++ b/core/app/models/spree/tax_rate.rb @@ -1,13 +1,3 @@ -module Spree - class DefaultTaxZoneValidator < ActiveModel::Validator - def validate(record) - if record.included_in_price - record.errors.add(:included_in_price, Spree.t(:included_price_validation)) unless Zone.default_tax - end - end - end -end - module Spree class TaxRate < Spree::Base acts_as_paranoid @@ -25,38 +15,16 @@ class TaxRate < Spree::Base validates :amount, presence: true, numericality: true validates :tax_category_id, presence: true - validates_with DefaultTaxZoneValidator scope :by_zone, ->(zone) { where(zone_id: zone) } # Gets the array of TaxRates appropriate for the specified order - def self.match(order_tax_zone) - return [] unless order_tax_zone - rates = includes(zone: { zone_members: :zoneable }).load.select do |rate| - # Why "potentially"? - # Go see the documentation for that method. - rate.potentially_applicable?(order_tax_zone) - end - - # Imagine with me this scenario: - # You are living in Spain and you have a store which ships to France. - # Spain is therefore your default tax rate. - # When you ship to Spain, you want the Spanish rate to apply. - # When you ship to France, you want the French rate to apply. - # - # Normally, Spree would notice that you have two potentially applicable - # tax rates for one particular item. - # When you ship to Spain, only the Spanish one will apply. - # When you ship to France, you'll see a Spanish refund AND a French tax. - # This little bit of code at the end stops the Spanish refund from appearing. - # - # For further discussion, see #4397 and #4327. - rates.delete_if do |rate| - rate.included_in_price? && - (rates - [rate]).map(&:tax_category).include?(rate.tax_category) - end + scope :for_zone, -> (zone) do + where(zone_id: Spree::Zone.with_shared_members(zone).pluck(:id)) end + scope :included_in_price, -> { where(included_in_price: true) } + # Pre-tax amounts must be stored so that we can calculate # correct rate amounts in the future. For example: # https://github.com/spree/spree/issues/4318#issuecomment-34723428 @@ -74,35 +42,9 @@ def self.store_pre_tax_amount(item, rates) item.update_column(:pre_tax_amount, pre_tax_amount.round(2)) end - # This method is best described by the documentation on #potentially_applicable? - def self.adjust(order_tax_zone, items) - rates = self.match(order_tax_zone) - tax_categories = rates.map(&:tax_category) - relevant_items, non_relevant_items = items.partition { |item| tax_categories.include?(item.tax_category) } - unless relevant_items.empty? - Spree::Adjustment.where(adjustable: relevant_items).tax.destroy_all # using destroy_all to ensure adjustment destroy callback fires. - end - relevant_items.each do |item| - relevant_rates = rates.select { |rate| rate.tax_category == item.tax_category } - store_pre_tax_amount(item, relevant_rates) - relevant_rates.each do |rate| - rate.adjust(order_tax_zone, item) - end - end - non_relevant_items.each do |item| - if item.adjustments.tax.present? - item.adjustments.tax.destroy_all # using destroy_all to ensure adjustment destroy callback fires. - item.update_columns pre_tax_amount: 0 - end - end - end - # Tax rates can *potentially* be applicable to an order. - # We do not know if they are/aren't until we attempt to apply these rates to - # the items contained within the Order itself. - # For instance, if a rate passes the criteria outlined in this method, - # but then has a tax category that doesn't match against any of the line items - # inside of the order, then that tax rate will not be applicable to anything. + # We do not know if they are/aren't until we check their tax categories - if + # they match any of the line item's, they are applicable. # For instance: # # Zones: @@ -137,13 +79,56 @@ def self.adjust(order_tax_zone, items) # Under no circumstances should negative adjustments be applied for the Spanish tax rates. # # Those rates should never come into play at all and only the French rates should apply. - def potentially_applicable?(order_tax_zone) - # If the rate's zone matches the order's tax zone, then it's applicable. - self.zone == order_tax_zone || - # If the rate's zone *contains* the order's tax zone, then it's applicable. - self.zone.contains?(order_tax_zone) || - # 1) The rate's zone is the default zone, then it's always applicable. - (self.included_in_price? && self.zone.default_tax) + def self.adjust(order_tax_zone, items) + # Early return to make sure nothing happens if there's no tax zone on + # the order. + return unless order_tax_zone + + # Destroy all tax adjustments using destroy_all to ensure adjustment destroy callback fires. + Spree::Adjustment.where(adjustable: items).tax.destroy_all + # TODO: Make sure items is always an AR relation and use `update_columns` + # TODO: Also: The whole pre_tax_amount stuff is so unnecessary once prices are right. + # I think the main thing to be done here is adapting the tests, which use arrays. + items.each { |item| item.update_column(:pre_tax_amount, item.discounted_amount) } + + # Find tax rates matching the order's tax zone + rates = for_zone(order_tax_zone) + + # Imagine with me this scenario: + # You are living in Spain and you have a store which ships the US. + # Spain is therefore your default tax rate. + # When you ship to Spain, you want the Spanish rate to apply. + # When you ship to the US, you want your Spanish rate to be refunded. + # This little bit of code adds the default tax zone's VAT rates so #adjust + # knows what to refund. + # + # For further discussion, see #4397 and #4327. + if default_tax_zone && !default_tax_zone.contains?(order_tax_zone) && rates.included_in_price.empty? + rates += for_zone(default_tax_zone) + end + + # Get all tax categories for which we have tax rates + tax_categories = rates.map(&:tax_category) + # Identify which items have to have a tax rate applied + # I think this could be done with an `items.join(:tax_category)` How? + relevant_items = items.select do |item| + tax_categories.include?(item.tax_category) + end + + # For each item, + relevant_items.each do |item| + # Select the rates with the same tax category + relevant_rates = rates.select do |rate| + rate.tax_category == item.tax_category + end + # Store the pre_tax_amount on the item + # (incredibly inelegant, this line should go) + store_pre_tax_amount(item, relevant_rates) + # Have all the relevant rates adjust the item. + relevant_rates.each do |rate| + rate.adjust(order_tax_zone, item) + end + end end # Creates necessary tax adjustments for the order. @@ -187,6 +172,10 @@ def default_zone_or_zone_match?(order_tax_zone) private + def self.default_tax_zone + @_default_tax_zone = Spree::Zone.default_tax + end + def create_label label = "" label << (name.present? ? name : tax_category.name) + " " diff --git a/core/app/models/spree/zone.rb b/core/app/models/spree/zone.rb index 31e41e217d5..7cb5fce0290 100644 --- a/core/app/models/spree/zone.rb +++ b/core/app/models/spree/zone.rb @@ -3,6 +3,11 @@ class Zone < Spree::Base has_many :zone_members, dependent: :destroy, class_name: "Spree::ZoneMember", inverse_of: :zone has_many :tax_rates, dependent: :destroy, inverse_of: :zone + with_options through: :zone_members, source: :zoneable do + has_many :countries, source_type: "Spree::Country" + has_many :states, source_type: "Spree::State" + end + has_many :shipping_method_zones has_many :shipping_methods, through: :shipping_method_zones @@ -10,6 +15,18 @@ class Zone < Spree::Base after_save :remove_defunct_members after_save :remove_previous_default + scope :with_member_ids, + ->(state_ids, country_ids) do + joins(:zone_members).where( + "(spree_zone_members.zoneable_type = 'Spree::State' AND + spree_zone_members.zoneable_id IN (?)) + OR (spree_zone_members.zoneable_type = 'Spree::Country' AND + spree_zone_members.zoneable_id IN (?))", + state_ids, + country_ids + ).uniq + end + alias :members :zone_members accepts_nested_attributes_for :zone_members, allow_destroy: true, reject_if: proc { |a| a['zoneable_id'].blank? } @@ -19,12 +36,13 @@ def self.default_tax where(default_tax: true).first end - # Returns the matching zone with the highest priority zone type (State, Country, Zone.) - # Returns nil in the case of no matches. + # Returns the most specific matching zone for an address. Specific means: + # A State zone wins over a country zone, and a zone with few members wins + # over one with many members. If there is no match, returns nil. def self.match(address) - return unless address and matches = self.includes(:zone_members). + return unless address and matches = self. + with_member_ids(address.state_id, address.country_id). order(:zone_members_count, :created_at, :id). - where("(spree_zone_members.zoneable_type = 'Spree::Country' AND spree_zone_members.zoneable_id = ?) OR (spree_zone_members.zoneable_type = 'Spree::State' AND spree_zone_members.zoneable_id = ?)", address.country_id, address.state_id). references(:zones) ['state', 'country'].each do |zone_kind| @@ -35,6 +53,17 @@ def self.match(address) matches.first end + # Returns all zones that contain any of the zone members of the zone passed + # in. This also includes any country zones that contain the state of the + # current zone, if it's a state zone. If the passed-in zone has members, it + # will also be in the result set. + def self.with_shared_members(zone) + with_member_ids( + zone.states.pluck(:id), + zone.states.pluck(:country_id) + zone.countries.pluck(:id) + ).uniq + end + def kind if members.any? && !members.any? { |member| member.try(:zoneable_type).nil? } members.last.zoneable_type.demodulize.underscore diff --git a/core/lib/spree/testing_support/factories/zone_factory.rb b/core/lib/spree/testing_support/factories/zone_factory.rb index db3c3e583dd..6b06f283d7e 100644 --- a/core/lib/spree/testing_support/factories/zone_factory.rb +++ b/core/lib/spree/testing_support/factories/zone_factory.rb @@ -13,5 +13,9 @@ factory :zone, class: Spree::Zone do name { generate(:random_string) } description { generate(:random_string) } + + trait :with_country do + countries { [create(:country)] } + end end end diff --git a/core/spec/models/spree/reimbursement_spec.rb b/core/spec/models/spree/reimbursement_spec.rb index 60eebaa4410..5356fe8162d 100644 --- a/core/spec/models/spree/reimbursement_spec.rb +++ b/core/spec/models/spree/reimbursement_spec.rb @@ -50,7 +50,7 @@ let!(:adjustments) { [] } # placeholder to ensure it gets run prior the "before" at this level let!(:tax_rate) { nil } - let!(:tax_zone) { create :zone, default_tax: true } + let!(:tax_zone) { create :zone, :with_country, default_tax: true } let(:shipping_method) { create :shipping_method, zones: [tax_zone] } let(:variant) { create :variant } let(:order) { create(:order_with_line_items, state: 'payment', line_items_attributes: [{variant: variant, price: line_items_price}], shipment_cost: 0, shipping_method: shipping_method) } diff --git a/core/spec/models/spree/reimbursement_tax_calculator_spec.rb b/core/spec/models/spree/reimbursement_tax_calculator_spec.rb index 86f4a177c1a..2a050992774 100644 --- a/core/spec/models/spree/reimbursement_tax_calculator_spec.rb +++ b/core/spec/models/spree/reimbursement_tax_calculator_spec.rb @@ -25,7 +25,7 @@ context 'with additional tax' do let!(:tax_rate) { create(:tax_rate, name: "Sales Tax", amount: 0.10, included_in_price: false, zone: tax_zone) } - let(:tax_zone) { create(:zone, default_tax: true) } + let(:tax_zone) { create(:zone, :with_country, default_tax: true) } it 'sets additional_tax_total on the return items' do subject @@ -38,13 +38,13 @@ context 'with included tax' do let!(:tax_rate) { create(:tax_rate, name: "VAT Tax", amount: 0.1, included_in_price: true, zone: tax_zone) } - let(:tax_zone) { create(:zone, default_tax: true) } + let(:tax_zone) { create(:zone, :with_country, default_tax: true) } it 'sets included_tax_total on the return items' do subject return_item.reload - expect(return_item.included_tax_total).to be < 0 + expect(return_item.included_tax_total).to be > 0 expect(return_item.included_tax_total).to eq line_item.included_tax_total end end diff --git a/core/spec/models/spree/tax_rate_spec.rb b/core/spec/models/spree/tax_rate_spec.rb index d28fef36860..9fa464bb818 100644 --- a/core/spec/models/spree/tax_rate_spec.rb +++ b/core/spec/models/spree/tax_rate_spec.rb @@ -1,16 +1,13 @@ require 'spec_helper' describe Spree::TaxRate, :type => :model do - context "match" do + context ".for_zone" do let(:order) { create(:order) } let(:country) { create(:country) } let(:tax_category) { create(:tax_category) } let(:calculator) { Spree::Calculator::FlatRate.new } - it "should return an empty array when tax_zone is nil" do - allow(order).to receive_messages :tax_zone => nil - expect(Spree::TaxRate.match(order.tax_zone)).to eq([]) - end + subject(:tax_rates_for_order) { Spree::TaxRate.for_zone(order.tax_zone) } context "when no rate zones match the tax zone" do before do @@ -25,7 +22,7 @@ it "should return an empty array" do allow(order).to receive_messages :tax_zone => @zone - expect(Spree::TaxRate.match(order.tax_zone)).to eq([]) + expect(tax_rates_for_order).to eq([]) end it "should return the rate that matches the rate zone" do @@ -37,7 +34,7 @@ ) allow(order).to receive_messages :tax_zone => @zone - expect(Spree::TaxRate.match(order.tax_zone)).to eq([rate]) + expect(tax_rates_for_order).to eq([rate]) end it "should return all rates that match the rate zone" do @@ -56,7 +53,7 @@ ) allow(order).to receive_messages :tax_zone => @zone - expect(Spree::TaxRate.match(order.tax_zone)).to match_array([rate1, rate2]) + expect(tax_rates_for_order).to match_array([rate1, rate2]) end context "when the tax_zone is contained within a rate zone" do @@ -73,7 +70,7 @@ end it "should return the rate zone" do - expect(Spree::TaxRate.match(order.tax_zone)).to eq([@rate]) + expect(tax_rates_for_order).to eq([@rate]) end end end @@ -93,8 +90,6 @@ :included_in_price => included_in_price) end - subject { Spree::TaxRate.match(order.tax_zone) } - context "when the order has the same tax zone" do before do allow(order).to receive_messages :tax_zone => @zone @@ -122,13 +117,6 @@ context "when the order has a tax_address" do let(:tax_address) { stub_model(Spree::Address) } - context "when the tax is a VAT" do - let(:included_in_price) { true } - # The rate should match in this instance because: - # 1) It's the default rate (and as such, a negative adjustment should apply) - it { is_expected.to eq([rate]) } - end - context "when the tax is not VAT" do it "returns no tax rate" do expect(subject).to be_empty @@ -139,16 +127,6 @@ context "when the order does not have a tax_address" do let(:tax_address) { nil} - context "when the tax is a VAT" do - let(:included_in_price) { true } - # The rate should match in this instance because: - # 1) The order has no tax address by this stage - # 2) With no tax address, it has no tax zone - # 3) Therefore, we assume the default tax zone - # 4) This default zone has a default tax rate. - it { is_expected.to eq([rate]) } - end - context "when the tax is not a VAT" do it { is_expected.to be_empty } end @@ -159,217 +137,236 @@ end context ".adjust" do - let(:order) { stub_model(Spree::Order) } - let(:tax_category_1) { stub_model(Spree::TaxCategory) } - let(:tax_category_2) { stub_model(Spree::TaxCategory) } - let(:rate_1) { stub_model(Spree::TaxRate, :tax_category => tax_category_1) } - let(:rate_2) { stub_model(Spree::TaxRate, :tax_category => tax_category_2) } - - context "with line items" do - let(:line_item) do - stub_model(Spree::LineItem, - :price => 10.0, - :quantity => 1, - :tax_category => tax_category_1, - :variant => stub_model(Spree::Variant) - ) - end + let!(:country) { create(:country) } + let!(:taxables_category) { create(:tax_category, name: "Taxable Foo") } + let!(:non_taxables_category) { create(:tax_category, name: "Non Taxable") } + let!(:zone) { create(:zone, countries: [country]) } + let!(:rate1) do + create( + :tax_rate, + tax_category: taxables_category, + zone: zone, + amount: 0.1 + ) + end + let!(:rate2) do + create( + :tax_rate, + tax_category: taxables_category, + zone: zone, + amount: 0.05 + ) + end + let(:taxable) { create(:product, tax_category: taxables_category) } + let(:non_taxable) { create(:product, tax_category: non_taxables_category) } + let!(:order) { Spree::Order.create! } + let(:line_item) { order.line_items.last } - let(:line_items) { [line_item] } + subject(:adjust_order_items) { Spree::TaxRate.adjust(order.tax_zone, order.line_items) } - before do - allow(Spree::TaxRate).to receive_messages :match => [rate_1, rate_2] + context "with shipments" do + let(:shipments) do + [stub_model(Spree::Shipment, cost: 10.0, tax_category: taxables_category)] end - it "should apply adjustments for two tax rates to the order" do - expect(rate_1).to receive(:adjust) - expect(rate_2).not_to receive(:adjust) - Spree::TaxRate.adjust(order.tax_zone, line_items) + let!(:rate2) do + create( + :tax_rate, + tax_category: non_taxables_category, + zone: zone, + amount: 0.05 + ) end - end - - context "with shipments" do - let(:shipments) { [stub_model(Spree::Shipment, :cost => 10.0, :tax_category => tax_category_1)] } before do - allow(Spree::TaxRate).to receive_messages :match => [rate_1, rate_2] + allow(Spree::TaxRate).to receive(:for_zone).and_return([rate1, rate2]) end it "should apply adjustments for two tax rates to the order" do - expect(rate_1).to receive(:adjust) - expect(rate_2).not_to receive(:adjust) - Spree::TaxRate.adjust(order.tax_zone, shipments) + expect(rate1).to receive(:adjust) + expect(rate2).not_to receive(:adjust) + Spree::TaxRate.adjust(zone, shipments) end end - end - - context "#adjust" do - before do - @country = create(:country) - @zone = create(:zone, :name => "Country Zone", :default_tax => true, :zone_members => []) - @zone.zone_members.create(:zoneable => @country) - @category = Spree::TaxCategory.create :name => "Taxable Foo" - @category2 = Spree::TaxCategory.create(:name => "Non Taxable") - @rate1 = Spree::TaxRate.create( - :amount => 0.10, - :calculator => Spree::Calculator::DefaultTax.create, - :tax_category => @category, - :zone => @zone - ) - @rate2 = Spree::TaxRate.create( - :amount => 0.05, - :calculator => Spree::Calculator::DefaultTax.create, - :tax_category => @category, - :zone => @zone - ) - @order = Spree::Order.create! - @taxable = create(:product, :tax_category => @category) - @nontaxable = create(:product, :tax_category => @category2) - end context "not taxable line item " do - let!(:line_item) { @order.contents.add(@nontaxable.master, 1) } + before { order.contents.add(non_taxable.master, 1) } + + context "with order having no zone" do + before { allow(order).to receive(:tax_zone).and_return(nil) } - it "should not create a tax adjustment" do - Spree::TaxRate.adjust(@order.tax_zone, @order.line_items) - expect(line_item.adjustments.tax.charge.count).to eq(0) + it "should not create a tax adjustment" do + expect(line_item.adjustments.tax.charge.count).to eq(0) + expect { adjust_order_items }.not_to change { line_item.adjustments.tax.charge.count } + end + + it "should not create a refund" do + expect(line_item.adjustments.credit.count).to eq(0) + expect { adjust_order_items }.not_to change { line_item.adjustments.tax.charge.count } + end end - it "should not create a refund" do - Spree::TaxRate.adjust(@order.tax_zone, @order.line_items) - expect(line_item.adjustments.credit.count).to eq(0) + context "with order tax zone being the tax rate's zone" do + before { allow(order).to receive(:tax_zone).and_return(zone) } + + it "should not create a tax adjustment" do + expect(line_item.adjustments.tax.charge.count).to eq(0) + expect { adjust_order_items }.not_to change { line_item.adjustments.tax.charge.count } + end + + it "should not create a refund" do + expect(line_item.adjustments.credit.count).to eq(0) + expect { adjust_order_items }.not_to change { line_item.adjustments.tax.charge.count } + end end end context "taxable line item" do - let!(:line_item) { @order.contents.add(@taxable.master, 1) } + before do + order.contents.add(taxable.master, 1) + # Delete all adjustments created when adding an item so we can observe + # changes. + line_item.adjustments.destroy_all + end context "when price includes tax" do - before do - @rate1.update_column(:included_in_price, true) - @rate2.update_column(:included_in_price, true) - Spree::TaxRate.store_pre_tax_amount(line_item, [@rate1, @rate2]) + let!(:default_zone) { create(:zone, countries: [country], default_tax: true) } + + let!(:rate1) do + create( + :tax_rate, + tax_category: taxables_category, + zone: zone, + amount: 0.1, + included_in_price: true + ) + end + let!(:rate2) do + create( + :tax_rate, + tax_category: taxables_category, + zone: zone, + amount: 0.05, + included_in_price: true + ) end - context "when zone is contained by default tax zone" do + context "when order's zone is the default zone" do + let!(:zone) { default_zone } + it "should create two adjustments, one for each tax rate" do - Spree::TaxRate.adjust(@order.tax_zone, @order.line_items) - expect(line_item.adjustments.count).to eq(1) + expect(line_item.adjustments.credit.count).to eq(0) + expect { adjust_order_items }.to change { line_item.adjustments.tax.count }.by(2) end it "should not create a tax refund" do - Spree::TaxRate.adjust(@order.tax_zone, @order.line_items) expect(line_item.adjustments.credit.count).to eq(0) + expect { adjust_order_items }.not_to change { line_item.adjustments.tax.credit.count } + end + + # This does not work either. Both tax rates should be taken into account. + it "price adjustments should be accurate" do + expect { adjust_order_items }.to change { line_item.adjustments.tax.count } + included_tax = line_item.adjustments.sum(:amount) + expect(line_item.pre_tax_amount).to eq(17.38) + expect(line_item.pre_tax_amount + included_tax).to eq(line_item.price) end end - context "when order's zone is neither the default zone, or included in the default zone, but matches the rate's zone" do + context "when order's zone is nil" do before do - # With no zone members, this zone will not contain anything - # Previously: - # Zone.stub_chain :default_tax, :contains? => false - @zone.zone_members.delete_all - end - it "should create an adjustment" do - Spree::TaxRate.adjust(@order.tax_zone, @order.line_items) - expect(line_item.adjustments.charge.count).to eq(1) + allow(order).to receive(:tax_zone).and_return(nil) end - it "should not create a tax refund for each tax rate" do - Spree::TaxRate.adjust(@order.tax_zone, @order.line_items) + it "should create no adjustments" do expect(line_item.adjustments.credit.count).to eq(0) + expect { adjust_order_items }.not_to change { line_item.adjustments.tax.count } end end - context "when order's zone does not match default zone, is not included in the default zone, AND does not match the rate's zone" do - before do - @new_zone = create(:zone, :name => "New Zone", :default_tax => false) - @new_country = create(:country, :name => "New Country") - @new_zone.zone_members.create(:zoneable => @new_country) - @order.ship_address = create(:address, :country => @new_country) - @order.save - end + context "when order's zone is another zone" do + let(:foreign_country) { create(:country) } + let(:foreign_zone) { create(:zone, countries: [foreign_country]) } - it "should not create positive adjustments" do - Spree::TaxRate.adjust(@order.tax_zone, @order.line_items) - expect(line_item.adjustments.charge.count).to eq(0) - end - - it "should create a tax refund for each tax rate" do - Spree::TaxRate.adjust(@order.tax_zone, @order.line_items) - expect(line_item.adjustments.credit.count).to eq(1) + before do + allow(order).to receive(:tax_zone).and_return(foreign_zone) end - end - context "when price does not include tax" do - before do - allow(@order).to receive_messages :tax_zone => @zone - [@rate1, @rate2].each do |rate| - rate.included_in_price = false - rate.zone = @zone - rate.save + context "and the foreign zone has VAT rates" do + let!(:foreign_vat) do + create( + :tax_rate, + included_in_price: true, + tax_category: taxables_category, + zone: foreign_zone, + amount: 0.21 + ) end - Spree::TaxRate.adjust(@order.tax_zone, @order.line_items) - end - it "should delete adjustments for open order when taxrate is deleted" do - @rate1.destroy! - @rate2.destroy! - expect(line_item.adjustments.count).to eq(0) - end + it "applies the foreign VAT" do + expect(line_item.adjustments.credit.count).to eq(0) + expect { adjust_order_items }.to change { line_item.adjustments.tax.count }.by(1) + end - it "should not delete adjustments for complete order when taxrate is deleted" do - @order.update_column :completed_at, Time.now - @rate1.destroy! - @rate2.destroy! - expect(line_item.adjustments.count).to eq(2) + it "does not refund the default zone's VAT" do + expect(line_item.adjustments.credit.count).to eq(0) + expect { adjust_order_items }.not_to change { line_item.adjustments.credit.count } + end end - it "should create an adjustment" do - expect(line_item.adjustments.count).to eq(2) + context "and the foreign zone does not have VAT rates" do + # This does not work. The code removes a VAT rate if + # there's more than one VAT of the same category. + it "refunds both default zone's VATs" do + expect(line_item.adjustments.count).to eq(0) + expect { adjust_order_items }.to change { line_item.adjustments.tax.count }.by(2) + end end + end + end - it "should not create a tax refund" do - expect(line_item.adjustments.credit.count).to eq(0) - end + context "when price does not include tax" do + before do + allow(order).to receive(:tax_zone).and_return(zone) + adjust_order_items + end - describe 'tax adjustments' do - before { Spree::TaxRate.adjust(@order.tax_zone, @order.line_items) } + it "should delete adjustments for open order when taxrate is deleted" do + rate1.destroy! + rate2.destroy! + expect(line_item.adjustments.count).to eq(0) + end - it "should apply adjustments when a tax zone is present" do - expect(line_item.adjustments.count).to eq(2) - end + it "should not delete adjustments for complete order when taxrate is deleted" do + order.update_column :completed_at, Time.now + rate1.destroy! + rate2.destroy! + expect(line_item.adjustments.count).to eq(2) + end - describe 'when the tax zone is removed' do - before { allow(@order).to receive_messages :tax_zone => nil } + it "should create two adjustments" do + expect(line_item.adjustments.count).to eq(2) + end - it 'does not apply any adjustments' do - Spree::TaxRate.adjust(@order.tax_zone, @order.line_items) - expect(line_item.adjustments.count).to eq(0) - end - end - end + it "should not create a tax refund" do + expect(line_item.adjustments.credit.count).to eq(0) end - context "when two rates apply" do - before do - @price_before_taxes = line_item.price / (1 + @rate1.amount + @rate2.amount) - # Use the same rounding method as in DefaultTax calculator - @price_before_taxes = BigDecimal.new(@price_before_taxes).round(2, BigDecimal::ROUND_HALF_UP) - line_item.update_column(:pre_tax_amount, @price_before_taxes) - # Clear out any previously automatically-applied adjustments - @order.all_adjustments.delete_all - @rate1.adjust(@order.tax_zone, line_item) - @rate2.adjust(@order.tax_zone, line_item) + describe 'tax adjustments' do + it "should apply adjustments when a tax zone is present" do + expect(line_item.adjustments.count).to eq(2) end - it "should create two price adjustments" do - expect(@order.line_item_adjustments.count).to eq(2) - end + describe "when the order's tax zone is nil" do + before do + line_item.adjustments.tax.destroy_all + allow(order).to receive(:tax_zone).and_return(nil) + end - it "price adjustments should be accurate" do - included_tax = @order.line_item_adjustments.sum(:amount) - expect(@price_before_taxes + included_tax).to eq(line_item.price) + it 'does not apply any adjustments' do + Spree::TaxRate.adjust(order.tax_zone, order.line_items) + expect(line_item.adjustments.count).to eq(0) + end end end end diff --git a/core/spec/models/spree/zone_spec.rb b/core/spec/models/spree/zone_spec.rb index f9a76afcc81..dfbee1a8e1e 100644 --- a/core/spec/models/spree/zone_spec.rb +++ b/core/spec/models/spree/zone_spec.rb @@ -302,4 +302,98 @@ end end end + + context ".with_shared_members" do + let!(:country) { create(:country) } + let!(:country2) { create(:country, name: 'OtherCountry') } + let!(:country3) { create(:country, name: 'TaxCountry') } + + subject(:zones_with_shared_members) { Spree::Zone.with_shared_members(zone) } + + context "finding potential matches for a country zone" do + let!(:zone) do + create(:zone).tap do |z| + z.members.create(zoneable: country) + z.members.create(zoneable: country2) + z.save! + end + end + + let!(:zone2) do + create(:zone).tap { |z| z.members.create(zoneable: country) && z.save! } + end + + let!(:zone3) do + create(:zone).tap { |z| z.members.create(zoneable: country3) && z.save! } + end + + it "will find all zones with countries covered by the passed in zone" do + expect(zones_with_shared_members).to include(zone, zone2) + end + + it "will not return zones with countries not covered in the passed in zone" do + expect(zones_with_shared_members).not_to include(zone3) + end + + it "only returns each zone once" do + expect(zones_with_shared_members.select { |z| z == zone }.size).to be 1 + end + end + + context "finding potential matches for a state zone" do + let!(:state) { create(:state, country: country) } + let!(:state2) { create(:state, country: country2, name: 'OtherState') } + let!(:state3) { create(:state, country: country2, name: 'State') } + let!(:zone) do + create(:zone).tap do |z| + z.members.create(zoneable: state) + z.members.create(zoneable: state2) + z.save! + end + end + let!(:zone2) do + create(:zone).tap { |z| z.members.create(zoneable: state) && z.save! } + end + let!(:zone3) do + create(:zone).tap { |z| z.members.create(zoneable: state2) && z.save! } + end + + it "will find all zones which share states covered by passed in zone" do + expect(zones_with_shared_members).to include(zone, zone2) + end + + it "will find zones that share countries with any states of the passed in zone" do + expect(zones_with_shared_members).to include(zone3) + end + + it "only returns each zone once" do + expect(zones_with_shared_members.select { |z| z == zone }.size).to be 1 + end + end + + context "state and country associations" do + let!(:country) { create(:country) } + + context "has countries associated" do + let!(:zone) do + create(:zone, countries: [country]) + end + + it "can access associated countries" do + expect(zone.countries).to include(country) + end + end + + context "has states associated" do + let!(:state) { create(:state, country: country) } + let!(:zone) do + create(:zone, states: [state]) + end + + it "can access associated states" do + expect(zone.states).to include(state) + end + end + end + end end