diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index eaff617a86..4c5f1cf9fc 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -241,7 +241,6 @@ 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' @@ -394,7 +393,6 @@ 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' @@ -1150,7 +1148,6 @@ 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 fd111084ce..a053394dbf 100644 --- a/app/models/spree/tax_rate_decorator.rb +++ b/app/models/spree/tax_rate_decorator.rb @@ -8,26 +8,24 @@ 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) - (order.adjustments.tax + order.price_adjustments).each do |a| - a.set_absolute_included_tax! a.amount + # 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 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.define_singleton_method(:product) { product } + line_item.tax_category = tax_category line_item.define_singleton_method(:price) { amount } # Tax on adjustments (represented by the included_tax field) is always inclusive of @@ -41,7 +39,6 @@ module Spree end end - private def with_tax_included_in_price diff --git a/config/initializers/spree.rb b/config/initializers/spree.rb index 83e854a08f..5c60a92778 100644 --- a/config/initializers/spree.rb +++ b/config/initializers/spree.rb @@ -7,7 +7,6 @@ # 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 e213204047..b8517d851d 100644 --- a/lib/open_food_network/enterprise_fee_applicator.rb +++ b/lib/open_food_network/enterprise_fee_applicator.rb @@ -1,24 +1,33 @@ module OpenFoodNetwork class EnterpriseFeeApplicator < Struct.new(:enterprise_fee, :variant, :role) def create_line_item_adjustment(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) + create_adjustment(line_item_adjustment_label, line_item.order, line_item) end def create_order_adjustment(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) + create_adjustment(order_adjustment_label, order, order) 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 @@ -31,7 +40,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(adjustable, adjustment) + def adjustment_tax(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 deleted file mode 100644 index 592bf59854..0000000000 --- a/lib/spree/core/calculated_adjustments_decorator.rb +++ /dev/null @@ -1,16 +0,0 @@ -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 2476a91be7..3db6cf46e1 100644 --- a/spec/factories.rb +++ b/spec/factories.rb @@ -374,6 +374,7 @@ 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 @@ -385,7 +386,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 # remove existing shipping_rates from shipment + shipment.shipping_rates.destroy_all 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 114c7e7cd3..6f23807851 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)) - adjustment.metadata.should be + expect(adjustment.metadata).to 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 - Adjustment.with_tax.should include adjustment_with_tax - Adjustment.with_tax.should_not include adjustment_without_tax + expect(Adjustment.with_tax).to include adjustment_with_tax + expect(Adjustment.with_tax).not_to include adjustment_without_tax end it "finds adjustments without tax" do - Adjustment.without_tax.should include adjustment_without_tax - Adjustment.without_tax.should_not include adjustment_with_tax + expect(Adjustment.without_tax).to include adjustment_without_tax + expect(Adjustment.without_tax).not_to include adjustment_with_tax end end describe "checking if an adjustment includes tax" do it "returns true when it has > 0 tax" do - adjustment_with_tax.should have_tax + expect(adjustment_with_tax).to have_tax end it "returns false when it has 0 tax" do - adjustment_without_tax.should_not have_tax + expect(adjustment_without_tax).not_to have_tax end end end @@ -48,8 +48,8 @@ module Spree end it "has 100% tax included" do - adjustment.amount.should be > 0 - adjustment.included_tax.should == adjustment.amount + expect(adjustment.amount).to be > 0 + expect(adjustment.included_tax).to eq(adjustment.amount) end it "does not crash when order data has been updated previously" do @@ -61,70 +61,76 @@ 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, shipments: [shipment]) } + let(:order) { create(:order, distributor: hub) } 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 } - it "has a shipping charge of $50" do - adjustment.amount.should == 50 + describe "the shipping charge" do + it "is the adjustment amount" do + order.shipments = [shipment] + + expect(adjustment.amount).to eq(50) + end 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] - adjustment.included_tax.should == 0 + expect(adjustment.included_tax).to eq(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] - adjustment.included_tax.should == 0 + expect(adjustment.included_tax).to eq(0) end end describe "when tax on shipping is enabled" do - before do - Config.shipment_inc_vat = true - Config.shipping_tax_rate = 0.25 - end + before { Config.shipment_inc_vat = true } 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 - adjustment.included_tax.should == 10.00 + expect(adjustment.included_tax).to eq(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] - adjustment.included_tax.should == 0 + expect(adjustment.included_tax).to eq(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] - adjustment.included_tax.should == 0 + expect(adjustment.included_tax).to eq(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!(:order) { create(:order, order_cycle: order_cycle, distributor: coordinator) } - let!(:line_item) { create(:line_item, order: order, variant: variant) } + let(:line_item) { create(:line_item, variant: variant) } + let(:order) { create(:order, line_items: [line_item], order_cycle: order_cycle, distributor: coordinator) } let(:adjustment) { order.adjustments(:reload).enterprise_fee.first } context "when enterprise fees have a fixed tax_category" do @@ -140,7 +146,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 - adjustment.included_tax.should == 4.55 + expect(adjustment.included_tax).to eq(4.55) end end @@ -152,8 +158,8 @@ module Spree end it "records the tax on TaxRate adjustment on the order" do - adjustment.included_tax.should == 0 - order.adjustments.tax.first.amount.should == 5.0 + expect(adjustment.included_tax).to eq(0) + expect(order.adjustments.tax.first.amount).to eq(5.0) end end @@ -165,44 +171,44 @@ module Spree end it "records no tax as charged" do - adjustment.included_tax.should == 0 + expect(adjustment.included_tax).to eq(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 - adjustment.included_tax.should == 4.55 + expect(adjustment.included_tax).to eq(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 - adjustment.included_tax.should == 0 - order.adjustments.tax.first.amount.should == 5.0 + expect(adjustment.included_tax).to eq(0) + expect(order.adjustments.tax.first.amount).to eq(5.0) end end end end - context "when enterprise fees inherit their tax_category product they are applied to" do + context "when enterprise fees inherit their tax_category from the 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.reload.create_tax_charge! # Updating line_item or order has the same effect - order.reload.update_distribution_charge! + + order.create_tax_charge! # Updating line_item or order has the same effect + order.update_distribution_charge! end context "when enterprise fees are taxed per-order" do @@ -213,8 +219,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 - adjustment.included_tax.should == 0.0 - line_item.adjustments.first.included_tax.should == 1.67 + expect(adjustment.included_tax).to eq(0.0) + expect(line_item.adjustments.first.included_tax).to eq(1.67) end end @@ -229,13 +235,12 @@ 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 - adjustment.included_tax.should == 0 - order.adjustments.tax.first.amount.should == 2.0 + expect(adjustment.included_tax).to eq(0) + expect(order.adjustments.tax.first.amount).to eq(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)) } @@ -244,15 +249,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 - adjustment.included_tax.should == 8.33 - line_item.adjustments.first.included_tax.should == 1.67 + expect(adjustment.included_tax).to eq(8.33) + expect(line_item.adjustments.first.included_tax).to eq(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 @@ -260,8 +265,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 - adjustment.included_tax.should == 0 - order.adjustments.tax.first.amount.should == 12.0 + expect(adjustment.included_tax).to eq(0) + expect(order.adjustments.tax.first.amount).to eq(12.0) end end end @@ -273,7 +278,7 @@ module Spree it "sets it, rounding to two decimal places" do adjustment.set_included_tax! 0.25 - adjustment.included_tax.should == 10.00 + expect(adjustment.included_tax).to eq(10.00) end end @@ -284,11 +289,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 - adjustment_without_tax.find_closest_tax_rates_from_included_tax.should == [] + expect(adjustment_without_tax.find_closest_tax_rates_from_included_tax).to eq([]) end it "returns the most accurate tax rate" do - adjustment_with_tax.find_closest_tax_rates_from_included_tax.should == [tax_rate] + expect(adjustment_with_tax.find_closest_tax_rates_from_included_tax).to eq([tax_rate]) end end end