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

Tax Categories on Line Items respect updates to Variant and Product Tax Categories #6059

19 changes: 19 additions & 0 deletions core/app/models/spree/line_item.rb
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ class LineItem < Spree::Base
before_destroy :destroy_inventory_units

delegate :name, :description, :sku, :should_track_inventory?, to: :variant
delegate :tax_category, :tax_category_id, to: :variant, prefix: true
delegate :currency, to: :order, allow_nil: true

attr_accessor :target_shipment, :price_currency
Expand Down Expand Up @@ -128,6 +129,24 @@ def pricing_options
Spree::Config.pricing_options_class.from_line_item(self)
end

# @return [Spree::TaxCategory] the variant's tax category
#
# This returns the variant's tax category if the tax category ID on the line_item is nil. It looks
# like an association, but really is an override.
#
def tax_category
super || variant_tax_category
end

# @return [Integer] the variant's tax category ID
#
# This returns the variant's tax category ID if the tax category ID on the line_id is nil. It looks
# like an association, but really is an override.
#
def tax_category_id
super || variant_tax_category_id
end

private

# Sets the quantity to zero if it is nil or less than zero.
Expand Down
1 change: 1 addition & 0 deletions core/app/models/spree/order_taxation.rb
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ def apply(taxes)

@order.line_items.each do |item|
taxed_items = taxes.line_item_taxes.select { |element| element.item_id == item.id }
item.tax_category_id = item.variant_tax_category_id
update_adjustments(item, taxed_items)
end

Expand Down
13 changes: 12 additions & 1 deletion core/app/models/spree/tax/tax_helpers.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,22 @@ module Tax
module TaxHelpers
private

# Select active rates matching tax category and address
#
# @private
# @param [Spree::LineItem, Spree::Shipment, Spree::ShippingRate] item
# the line item, shipment, or shipping rate to select rates for
# @return [Array<Spree::TaxRate>] the active Tax Rates that match both
# Tax Category and the item's order's tax address
def rates_for_item(item)
@rates_for_item ||= Spree::TaxRate.item_level.for_address(item.order.tax_address)
# try is used here to ensure that a LineItem has rates selected for the
# currently configured Tax Category. Shipments and ShippingRates do not
# implement variant_tax_category_id, so try is necessary instead of .&
tax_category_id = item.try(:variant_tax_category_id) || item.tax_category_id

@rates_for_item.select do |rate|
rate.active? && rate.tax_categories.map(&:id).include?(item.tax_category_id)
rate.active? && rate.tax_categories.map(&:id).include?(tax_category_id)
end
end
end
Expand Down
2 changes: 1 addition & 1 deletion core/app/models/spree/tax_calculator/default.rb
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ def calculate_rates(item)
# @return [Array<Spree::TaxRate>] rates that apply to an order
def rates_for_order
tax_category_ids = Set[
*@order.line_items.map(&:tax_category_id),
*@order.line_items.map(&:variant_tax_category_id),
*@order.shipments.map(&:tax_category_id)
]
rates = Spree::TaxRate.active.order_level.for_address(@order.tax_address)
Expand Down
11 changes: 10 additions & 1 deletion core/app/models/spree/variant.rb
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ class Variant < Spree::Base
delegate :name, :description, :slug, :available_on, :discontinue_on, :discontinued?,
:meta_description, :meta_keywords,
to: :product
delegate :tax_category, to: :product, prefix: true
delegate :tax_category, :tax_category_id, to: :product, prefix: true
delegate :shipping_category, :shipping_category_id,
to: :product, prefix: true
delegate :tax_rates, to: :tax_category
Expand Down Expand Up @@ -150,6 +150,15 @@ def tax_category
super || product_tax_category
end

# @return [Integer] the variant's tax category ID
#
# This returns the product's tax category ID if the tax category ID on the variant is nil. It looks
# like an association, but really is an override.
#
def tax_category_id
super || product_tax_category_id
end

# @return [Spree::ShippingCategory] the variant's shipping category
#
# This returns the product's shipping category if the shipping category ID on the variant is nil. It looks
Expand Down
10 changes: 9 additions & 1 deletion core/lib/spree/testing_support/factories/order_factory.rb
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,15 @@
evaluator.stock_location # must evaluate before creating line items

evaluator.line_items_attributes.each do |attributes|
attributes = { order:, price: evaluator.line_items_price }.merge(attributes)
attributes = { order:, price: evaluator.line_items_price }.merge(attributes).tap do |attrs|
tax_category = attributes.delete(:tax_category)
if attrs[:variant] && tax_category
attrs[:variant].update(tax_category: )
elsif tax_category
attrs[:variant] = create(:variant, tax_category: )
end
end

create(:line_item, attributes)
end
order.line_items.reload
Expand Down
8 changes: 5 additions & 3 deletions core/spec/models/spree/line_item_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
RSpec.describe Spree::LineItem, type: :model do
let(:order) { create :order_with_line_items, line_items_count: 1 }
let(:line_item) { order.line_items.first }
let(:target_shipment) { Spree::Shipment.new }

context '#destroy' do
it "fetches soft-deleted products" do
Expand All @@ -18,7 +19,8 @@
end

it "returns inventory when a line item is destroyed" do
expect_any_instance_of(Spree::OrderInventory).to receive(:verify)
line_item.target_shipment = target_shipment
expect_any_instance_of(Spree::OrderInventory).to receive(:verify).with(target_shipment)
line_item.destroy
end

Expand All @@ -30,8 +32,8 @@
context "#save" do
context "target_shipment is provided" do
it "verifies inventory" do
line_item.target_shipment = Spree::Shipment.new
expect_any_instance_of(Spree::OrderInventory).to receive(:verify)
line_item.target_shipment = target_shipment
expect_any_instance_of(Spree::OrderInventory).to receive(:verify).with(target_shipment)
line_item.save
end
end
Expand Down
24 changes: 21 additions & 3 deletions core/spec/models/spree/order_taxation_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -26,14 +26,19 @@
:product,
price: 20,
name: "Book",
tax_category: books_category,
tax_category: create(:tax_category),
)
end

let(:taxation) { described_class.new(order) }

before do
order.contents.add(book.master)
book.update(tax_category: books_category)
end

describe "#apply" do
let(:line_item) { order.contents.add(book.master) }
let(:line_item) { order.line_items.first }

let(:line_item_tax) do
Spree::Tax::ItemTax.new(
Expand All @@ -53,9 +58,16 @@
)
end

before { taxation.apply(taxes) }
subject(:apply) { taxation.apply(taxes) }

it "updates the line_item's tax_category_id" do
expect { apply }.to change {
line_item[:tax_category_id]
}.to(books_category.id)
end

it "creates a new tax adjustment", aggregate_failures: true do
apply
expect(line_item.adjustments.count).to eq 1

tax_adjustment = line_item.adjustments.first
Expand All @@ -66,6 +78,8 @@
end

context "when new taxes are applied" do
before { apply }

let(:new_line_item_tax) do
Spree::Tax::ItemTax.new(
item_id: line_item.id,
Expand Down Expand Up @@ -108,6 +122,8 @@
end

context "when taxes are removed" do
before { apply }

let(:new_taxes) do
Spree::Tax::OrderTax.new(
order_id: order.id,
Expand All @@ -126,6 +142,8 @@
end

context "with order-level taxes" do
before { apply }

let(:delivery_fee) do
FactoryBot.create(
:tax_rate,
Expand Down
42 changes: 39 additions & 3 deletions core/spec/models/spree/tax/tax_helpers_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,17 +16,20 @@ def valid_rates(item)
let(:tax_category) { create(:tax_category) }
let(:irrelevant_tax_category) { create(:tax_category) }

let(:item) { create(:line_item, tax_category:) }
let(:item) { create(:line_item, variant:) }
let(:variant) { create(:variant, tax_category:) }
let(:tax_address) { item.order.tax_address }
let(:zone) { create(:zone, name: "Country Zone", countries: [tax_address.country]) }

let!(:tax_rate) do
create(:tax_rate, tax_categories: [tax_category], zone:)
end

subject { DummyClass.new.valid_rates(item) }

describe '#rates_for_item' do
it 'returns tax rates that match the tax category of the given item' do
expect(DummyClass.new.valid_rates(item)).to contain_exactly(tax_rate)
expect(subject).to contain_exactly(tax_rate)
end

context 'when multiple rates exist that are currently not valid' do
Expand All @@ -41,7 +44,40 @@ def valid_rates(item)
it 'returns only active rates that match the tax category of given item' do
expect(Spree::TaxRate.for_address(tax_address)).to contain_exactly(tax_rate, invalid_tax_rate)

expect(DummyClass.new.valid_rates(item)).to contain_exactly(tax_rate)
expect(subject).to contain_exactly(tax_rate)
end
end

context "when the line_item's variant's tax_category is changed" do
let(:new_tax_category) { create(:tax_category) }
before do
variant.update(tax_category: new_tax_category)
tax_rate.update(tax_categories: [new_tax_category])
end

it "returns the new tax rate for the variant's tax category" do
expect(subject).to contain_exactly(tax_rate)
end
end

context "when item is a shipping_rate" do
let(:item) { create(:shipping_rate, shipping_method:) }
let(:shipping_method) { create(:shipping_method, tax_category:) }

it "returns the tax rate for the shipping_method's tax category" do
expect(subject).to contain_exactly(tax_rate)
end

context "when the shipping_method's tax_category is changed" do
let(:new_tax_category) { create(:tax_category) }
before do
shipping_method.update(tax_category: new_tax_category)
tax_rate.update(tax_categories: [new_tax_category])
end

it "returns the new tax rate for the shipping_method's tax category" do
expect(subject).to contain_exactly(tax_rate)
end
end
end
end
Expand Down
Loading