diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 4c5f1cf9fc..eaff617a86 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -241,6 +241,7 @@ Layout/EmptyLines: - 'spec/models/enterprise_spec.rb' - 'spec/models/model_set_spec.rb' - 'spec/models/product_distribution_spec.rb' + - 'spec/models/spree/adjustment_spec.rb' - 'spec/models/spree/line_item_spec.rb' - 'spec/models/spree/order_populator_spec.rb' - 'spec/models/spree/order_spec.rb' @@ -393,6 +394,7 @@ Layout/ExtraSpacing: - 'spec/models/enterprise_fee_spec.rb' - 'spec/models/enterprise_spec.rb' - 'spec/models/order_cycle_spec.rb' + - 'spec/models/spree/adjustment_spec.rb' - 'spec/models/spree/gateway/stripe_connect_spec.rb' - 'spec/models/spree/order_spec.rb' - 'spec/models/variant_override_spec.rb' @@ -1148,6 +1150,7 @@ Lint/Void: - 'spec/models/enterprise_spec.rb' - 'spec/models/exchange_spec.rb' - 'spec/models/order_cycle_spec.rb' + - 'spec/models/spree/adjustment_spec.rb' - 'spec/models/spree/line_item_spec.rb' - 'spec/models/spree/order_populator_spec.rb' - 'spec/models/spree/order_spec.rb' diff --git a/app/models/spree/tax_rate_decorator.rb b/app/models/spree/tax_rate_decorator.rb index a053394dbf..fd111084ce 100644 --- a/app/models/spree/tax_rate_decorator.rb +++ b/app/models/spree/tax_rate_decorator.rb @@ -8,24 +8,26 @@ module Spree alias_method_chain :match, :sales_tax_registration end + def adjust_with_included_tax(order) adjust_without_included_tax(order) order.adjustments(:reload) order.line_items(:reload) - # TaxRate adjustments (order.adjustments.tax) and price adjustments (tax included on line items) consist of 100% tax - (order.adjustments.tax + order.price_adjustments).each do |adjustment| - adjustment.set_absolute_included_tax! adjustment.amount + (order.adjustments.tax + order.price_adjustments).each do |a| + a.set_absolute_included_tax! a.amount end end alias_method_chain :adjust, :included_tax + # Manually apply a TaxRate to a particular amount. TaxRates normally compute against # LineItems or Orders, so we mock out a line item here to fit the interface # that our calculator (usually DefaultTax) expects. def compute_tax(amount) + product = OpenStruct.new tax_category: tax_category line_item = LineItem.new quantity: 1 - line_item.tax_category = tax_category + line_item.define_singleton_method(:product) { product } line_item.define_singleton_method(:price) { amount } # Tax on adjustments (represented by the included_tax field) is always inclusive of @@ -39,6 +41,7 @@ module Spree end end + private def with_tax_included_in_price diff --git a/config/initializers/spree.rb b/config/initializers/spree.rb index 5c60a92778..83e854a08f 100644 --- a/config/initializers/spree.rb +++ b/config/initializers/spree.rb @@ -7,6 +7,7 @@ # config.setting_name = 'new value' require 'spree/product_filters' +require 'spree/core/calculated_adjustments_decorator' require "#{Rails.root}/app/models/spree/payment_method_decorator" require "#{Rails.root}/app/models/spree/gateway_decorator" diff --git a/lib/open_food_network/enterprise_fee_applicator.rb b/lib/open_food_network/enterprise_fee_applicator.rb index b8517d851d..e213204047 100644 --- a/lib/open_food_network/enterprise_fee_applicator.rb +++ b/lib/open_food_network/enterprise_fee_applicator.rb @@ -1,33 +1,24 @@ module OpenFoodNetwork class EnterpriseFeeApplicator < Struct.new(:enterprise_fee, :variant, :role) def create_line_item_adjustment(line_item) - create_adjustment(line_item_adjustment_label, line_item.order, line_item) + a = enterprise_fee.create_adjustment(line_item_adjustment_label, line_item.order, line_item, true) + + AdjustmentMetadata.create! adjustment: a, enterprise: enterprise_fee.enterprise, fee_name: enterprise_fee.name, fee_type: enterprise_fee.fee_type, enterprise_role: role + + a.set_absolute_included_tax! adjustment_tax(line_item, a) end def create_order_adjustment(order) - create_adjustment(order_adjustment_label, order, order) + a = enterprise_fee.create_adjustment(order_adjustment_label, order, order, true) + + AdjustmentMetadata.create! adjustment: a, enterprise: enterprise_fee.enterprise, fee_name: enterprise_fee.name, fee_type: enterprise_fee.fee_type, enterprise_role: role + + a.set_absolute_included_tax! adjustment_tax(order, a) end + private - def create_adjustment(label, target, calculable) - adjustment = create_enterprise_fee_adjustment(label, target, calculable) - - AdjustmentMetadata.create! adjustment: adjustment, enterprise: enterprise_fee.enterprise, fee_name: enterprise_fee.name, fee_type: enterprise_fee.fee_type, enterprise_role: role - - adjustment.set_absolute_included_tax! adjustment_tax(adjustment) - end - - def create_enterprise_fee_adjustment(label, target, calculable) - adjustment = enterprise_fee.create_adjustment(label, target, calculable, true) - - # This is necessary when source is a line_item - # probably because the association order.adjustments contains "inverse_of :source" - # which overrides the value (the line item) set in calculated_adjustment.create_adjustment - adjustment.source = calculable - adjustment - end - def line_item_adjustment_label "#{variant.product.name} - #{base_adjustment_label}" end @@ -40,7 +31,7 @@ module OpenFoodNetwork I18n.t(:enterprise_fee_by, type: enterprise_fee.fee_type, role: role, enterprise_name: enterprise_fee.enterprise.name) end - def adjustment_tax(adjustment) + def adjustment_tax(adjustable, adjustment) tax_rates = adjustment.tax_rates tax_rates.select(&:included_in_price).sum do |rate| diff --git a/lib/spree/core/calculated_adjustments_decorator.rb b/lib/spree/core/calculated_adjustments_decorator.rb new file mode 100644 index 0000000000..592bf59854 --- /dev/null +++ b/lib/spree/core/calculated_adjustments_decorator.rb @@ -0,0 +1,16 @@ +module Spree + module Core + module CalculatedAdjustments + class << self + def included_with_explicit_class_name(klass) + included_without_explicit_class_name(klass) + + klass.class_eval do + has_one :calculator, as: :calculable, dependent: :destroy, class_name: 'Spree::Calculator' + end + end + alias_method_chain :included, :explicit_class_name + end + end + end +end diff --git a/spec/factories.rb b/spec/factories.rb index 3db6cf46e1..2476a91be7 100644 --- a/spec/factories.rb +++ b/spec/factories.rb @@ -374,7 +374,6 @@ FactoryBot.define do shipping_method { create :shipping_method } end after(:create) do |shipment, evaluator| - shipment.shipping_rates.destroy_all shipment.add_shipping_method(evaluator.shipping_method, true) end end @@ -386,7 +385,7 @@ FactoryBot.define do after(:create) do |shipment, evaluator| shipping_method = create(:shipping_method_with, :shipping_fee, shipping_fee: evaluator.shipping_fee) - shipment.shipping_rates.destroy_all + shipment.shipping_rates.destroy_all # remove existing shipping_rates from shipment shipment.add_shipping_method(shipping_method, true) end end diff --git a/spec/models/spree/adjustment_spec.rb b/spec/models/spree/adjustment_spec.rb index 6f23807851..114c7e7cd3 100644 --- a/spec/models/spree/adjustment_spec.rb +++ b/spec/models/spree/adjustment_spec.rb @@ -4,7 +4,7 @@ module Spree describe Adjustment do it "has metadata" do adjustment = create(:adjustment, metadata: create(:adjustment_metadata)) - expect(adjustment.metadata).to be + adjustment.metadata.should be end describe "querying included tax" do @@ -13,23 +13,23 @@ module Spree describe "finding adjustments with and without tax included" do it "finds adjustments with tax" do - expect(Adjustment.with_tax).to include adjustment_with_tax - expect(Adjustment.with_tax).not_to include adjustment_without_tax + Adjustment.with_tax.should include adjustment_with_tax + Adjustment.with_tax.should_not include adjustment_without_tax end it "finds adjustments without tax" do - expect(Adjustment.without_tax).to include adjustment_without_tax - expect(Adjustment.without_tax).not_to include adjustment_with_tax + Adjustment.without_tax.should include adjustment_without_tax + Adjustment.without_tax.should_not include adjustment_with_tax end end describe "checking if an adjustment includes tax" do it "returns true when it has > 0 tax" do - expect(adjustment_with_tax).to have_tax + adjustment_with_tax.should have_tax end it "returns false when it has 0 tax" do - expect(adjustment_without_tax).not_to have_tax + adjustment_without_tax.should_not have_tax end end end @@ -48,8 +48,8 @@ module Spree end it "has 100% tax included" do - expect(adjustment.amount).to be > 0 - expect(adjustment.included_tax).to eq(adjustment.amount) + adjustment.amount.should be > 0 + adjustment.included_tax.should == adjustment.amount end it "does not crash when order data has been updated previously" do @@ -61,76 +61,70 @@ module Spree describe "Shipment adjustments" do let(:shipping_method) { create(:shipping_method_with, :flat_rate) } let(:shipment) { create(:shipment_with, :shipping_method, shipping_method: shipping_method) } - let(:order) { create(:order, distributor: hub) } + let!(:order) { create(:order, distributor: hub, shipments: [shipment]) } let(:hub) { create(:distributor_enterprise, charges_sales_tax: true) } - let(:line_item) { create(:line_item, order: order) } + let!(:line_item) { create(:line_item, order: order) } let(:adjustment) { order.adjustments(:reload).shipping.first } - describe "the shipping charge" do - it "is the adjustment amount" do - order.shipments = [shipment] - - expect(adjustment.amount).to eq(50) - end + it "has a shipping charge of $50" do + adjustment.amount.should == 50 end describe "when tax on shipping is disabled" do - before { Config.shipment_inc_vat = false } it "records 0% tax on shipment adjustments" do + Config.shipment_inc_vat = false Config.shipping_tax_rate = 0 - order.shipments = [shipment] - expect(adjustment.included_tax).to eq(0) + adjustment.included_tax.should == 0 end it "records 0% tax on shipments when a rate is set but shipment_inc_vat is false" do + Config.shipment_inc_vat = false Config.shipping_tax_rate = 0.25 - order.shipments = [shipment] - expect(adjustment.included_tax).to eq(0) + adjustment.included_tax.should == 0 end end describe "when tax on shipping is enabled" do - before { Config.shipment_inc_vat = true } + before do + Config.shipment_inc_vat = true + Config.shipping_tax_rate = 0.25 + end it "takes the shipment adjustment tax included from the system setting" do - Config.shipping_tax_rate = 0.25 - order.shipments = [shipment] - # Finding the tax included in an amount that's already inclusive of tax: # total - ( total / (1 + rate) ) # 50 - ( 50 / (1 + 0.25) ) # = 10 - expect(adjustment.included_tax).to eq(10.00) + adjustment.included_tax.should == 10.00 end it "records 0% tax on shipments when shipping_tax_rate is not set" do + Config.shipment_inc_vat = true Config.shipping_tax_rate = nil - order.shipments = [shipment] - expect(adjustment.included_tax).to eq(0) + adjustment.included_tax.should == 0 end it "records 0% tax on shipments when the distributor does not charge sales tax" do order.distributor.update_attributes! charges_sales_tax: false - order.shipments = [shipment] - expect(adjustment.included_tax).to eq(0) + adjustment.included_tax.should == 0 end end end describe "EnterpriseFee adjustments" do - let(:zone) { create(:zone_with_member) } - let(:fee_tax_rate) { create(:tax_rate, included_in_price: true, calculator: Calculator::DefaultTax.new, zone: zone, amount: 0.1) } - let(:fee_tax_category) { create(:tax_category, tax_rates: [fee_tax_rate]) } + let!(:zone) { create(:zone_with_member) } + let(:fee_tax_rate) { create(:tax_rate, included_in_price: true, calculator: Calculator::DefaultTax.new, zone: zone, amount: 0.1) } + let(:fee_tax_category) { create(:tax_category, tax_rates: [fee_tax_rate]) } let(:coordinator) { create(:distributor_enterprise, charges_sales_tax: true) } let(:variant) { create(:variant, product: create(:product, tax_category: nil)) } let(:order_cycle) { create(:simple_order_cycle, coordinator: coordinator, coordinator_fees: [enterprise_fee], distributors: [coordinator], variants: [variant]) } - let(:line_item) { create(:line_item, variant: variant) } - let(:order) { create(:order, line_items: [line_item], order_cycle: order_cycle, distributor: coordinator) } + let!(:order) { create(:order, order_cycle: order_cycle, distributor: coordinator) } + let!(:line_item) { create(:line_item, order: order, variant: variant) } let(:adjustment) { order.adjustments(:reload).enterprise_fee.first } context "when enterprise fees have a fixed tax_category" do @@ -146,7 +140,7 @@ module Spree # The fee is $50, tax is 10%, and the fee is inclusive of tax # Therefore, the included tax should be 0.1/1.1 * 50 = $4.55 - expect(adjustment.included_tax).to eq(4.55) + adjustment.included_tax.should == 4.55 end end @@ -158,8 +152,8 @@ module Spree end it "records the tax on TaxRate adjustment on the order" do - expect(adjustment.included_tax).to eq(0) - expect(order.adjustments.tax.first.amount).to eq(5.0) + adjustment.included_tax.should == 0 + order.adjustments.tax.first.amount.should == 5.0 end end @@ -171,44 +165,44 @@ module Spree end it "records no tax as charged" do - expect(adjustment.included_tax).to eq(0) + adjustment.included_tax.should == 0 end end end + context "when enterprise fees are taxed per-item" do let(:enterprise_fee) { create(:enterprise_fee, enterprise: coordinator, tax_category: fee_tax_category, calculator: Calculator::PerItem.new(preferred_amount: 50.0)) } describe "when the tax rate includes the tax in the price" do it "records the tax on the enterprise fee adjustments" do - expect(adjustment.included_tax).to eq(4.55) + adjustment.included_tax.should == 4.55 end end describe "when the tax rate does not include the tax in the price" do before do fee_tax_rate.update_attribute :included_in_price, false - order.reload.create_tax_charge! # Updating line_item or order has the same effect + order.reload.create_tax_charge! # Updating line_item or order has the same effect order.update_distribution_charge! end it "records the tax on TaxRate adjustment on the order" do - expect(adjustment.included_tax).to eq(0) - expect(order.adjustments.tax.first.amount).to eq(5.0) + adjustment.included_tax.should == 0 + order.adjustments.tax.first.amount.should == 5.0 end end end end - context "when enterprise fees inherit their tax_category from the product they are applied to" do + context "when enterprise fees inherit their tax_category product they are applied to" do let(:product_tax_rate) { create(:tax_rate, included_in_price: true, calculator: Calculator::DefaultTax.new, zone: zone, amount: 0.2) } let(:product_tax_category) { create(:tax_category, tax_rates: [product_tax_rate]) } before do variant.product.update_attribute(:tax_category_id, product_tax_category.id) - - order.create_tax_charge! # Updating line_item or order has the same effect - order.update_distribution_charge! + order.reload.create_tax_charge! # Updating line_item or order has the same effect + order.reload.update_distribution_charge! end context "when enterprise fees are taxed per-order" do @@ -219,8 +213,8 @@ module Spree # EnterpriseFee tax category is nil and inheritance only applies to per item fees # so tax on the enterprise_fee adjustment will be 0 # Tax on line item is: 0.2/1.2 x $10 = $1.67 - expect(adjustment.included_tax).to eq(0.0) - expect(line_item.adjustments.first.included_tax).to eq(1.67) + adjustment.included_tax.should == 0.0 + line_item.adjustments.first.included_tax.should == 1.67 end end @@ -235,12 +229,13 @@ module Spree # EnterpriseFee tax category is nil and inheritance only applies to per item fees # so total tax on the order is only that which applies to the line_item itself # ie. $10 x 0.2 = $2.0 - expect(adjustment.included_tax).to eq(0) - expect(order.adjustments.tax.first.amount).to eq(2.0) + adjustment.included_tax.should == 0 + order.adjustments.tax.first.amount.should == 2.0 end end end + context "when enterprise fees are taxed per-item" do let(:enterprise_fee) { create(:enterprise_fee, enterprise: coordinator, inherits_tax_category: true, calculator: Calculator::PerItem.new(preferred_amount: 50.0)) } @@ -249,15 +244,15 @@ module Spree # Applying product tax rate of 0.2 to enterprise fee of $50 # gives tax on fee of 0.2/1.2 x $50 = $8.33 # Tax on line item is: 0.2/1.2 x $10 = $1.67 - expect(adjustment.included_tax).to eq(8.33) - expect(line_item.adjustments.first.included_tax).to eq(1.67) + adjustment.included_tax.should == 8.33 + line_item.adjustments.first.included_tax.should == 1.67 end end describe "when the tax rate does not include the tax in the price" do before do product_tax_rate.update_attribute :included_in_price, false - order.reload.create_tax_charge! # Updating line_item or order has the same effect + order.reload.create_tax_charge! # Updating line_item or order has the same effect order.update_distribution_charge! end @@ -265,8 +260,8 @@ module Spree # EnterpriseFee inherits tax_category from product so total tax on # the order is that which applies to the line item itself, plus the # same rate applied to the fee of $50. ie. ($10 + $50) x 0.2 = $12.0 - expect(adjustment.included_tax).to eq(0) - expect(order.adjustments.tax.first.amount).to eq(12.0) + adjustment.included_tax.should == 0 + order.adjustments.tax.first.amount.should == 12.0 end end end @@ -278,7 +273,7 @@ module Spree it "sets it, rounding to two decimal places" do adjustment.set_included_tax! 0.25 - expect(adjustment.included_tax).to eq(10.00) + adjustment.included_tax.should == 10.00 end end @@ -289,11 +284,11 @@ module Spree let!(:other_tax_rate) { create(:tax_rate, calculator: Spree::Calculator::DefaultTax.new, amount: 0.3) } it "returns [] if there is no included tax" do - expect(adjustment_without_tax.find_closest_tax_rates_from_included_tax).to eq([]) + adjustment_without_tax.find_closest_tax_rates_from_included_tax.should == [] end it "returns the most accurate tax rate" do - expect(adjustment_with_tax.find_closest_tax_rates_from_included_tax).to eq([tax_rate]) + adjustment_with_tax.find_closest_tax_rates_from_included_tax.should == [tax_rate] end end end