diff --git a/app/controllers/spree/admin/orders_controller.rb b/app/controllers/spree/admin/orders_controller.rb index b1ee4f3dc6..51f4bbe84d 100644 --- a/app/controllers/spree/admin/orders_controller.rb +++ b/app/controllers/spree/admin/orders_controller.rb @@ -37,6 +37,11 @@ module Spree def update @order.recreate_all_fees! + unless @order.cart? + @order.create_tax_charge! + @order.update_order! + end + unless order_params.present? && @order.update(order_params) && @order.line_items.present? if @order.line_items.empty? && !params[:suppress_error_msg] @order.errors.add(:line_items, Spree.t('errors.messages.blank')) diff --git a/app/models/calculator/default_tax.rb b/app/models/calculator/default_tax.rb index fb764340fa..f9c118905a 100644 --- a/app/models/calculator/default_tax.rb +++ b/app/models/calculator/default_tax.rb @@ -12,10 +12,8 @@ module Calculator case computable when Spree::Order compute_order(computable) - when Spree::Shipment - compute_shipment(computable) - when Spree::LineItem - compute_line_item(computable) + when Spree::Shipment, Spree::LineItem, Spree::Adjustment + compute_item(computable) end end @@ -25,8 +23,13 @@ module Calculator calculable end - # Enable calculation of tax for enterprise fees with tax rates where included_in_price = false def compute_order(order) + # This legacy tax calculation applies to additional taxes only, and is no longer used. + # In theory it should never be called any more after this has been deployed. + # If the message below doesn't show up in Bugsnag, we can safely delete this method and all + # the related methods below it. + Bugsnag.notify("Calculator::DefaultTax was called with legacy tax calculations") + calculator = OpenFoodNetwork::EnterpriseFeeCalculator.new(order.distributor, order.order_cycle) @@ -78,19 +81,13 @@ module Calculator .sum { |applicator| applicator.enterprise_fee.compute_amount(order) } end - def compute_shipment_or_line_item(item) - if item.tax_category == rate.tax_category - if rate.included_in_price - deduced_total_by_rate(item.amount, rate) - else - round_to_two_places(item.amount * rate.amount) - end + def compute_item(item) + if rate.included_in_price + deduced_total_by_rate(item.amount, rate) else - 0 + round_to_two_places(item.amount * rate.amount) end end - alias_method :compute_shipment, :compute_shipment_or_line_item - alias_method :compute_line_item, :compute_shipment_or_line_item def round_to_two_places(amount) BigDecimal(amount.to_s).round(2, BigDecimal::ROUND_HALF_UP) diff --git a/app/models/spree/adjustment.rb b/app/models/spree/adjustment.rb index 973f7facee..20449e1213 100644 --- a/app/models/spree/adjustment.rb +++ b/app/models/spree/adjustment.rb @@ -35,10 +35,12 @@ module Spree # So we don't need the option `dependent: :destroy` as long as # AdjustmentMetadata has no destroy logic itself. has_one :metadata, class_name: 'AdjustmentMetadata' + has_many :adjustments, as: :adjustable, dependent: :destroy belongs_to :adjustable, polymorphic: true belongs_to :originator, -> { with_deleted }, polymorphic: true belongs_to :order, class_name: "Spree::Order" + belongs_to :tax_category, class_name: 'Spree::TaxCategory' belongs_to :tax_rate, -> { where spree_adjustments: { originator_type: 'Spree::TaxRate' } }, foreign_key: 'originator_id' @@ -70,6 +72,7 @@ module Spree scope :return_authorization, -> { where(originator_type: "Spree::ReturnAuthorization") } scope :inclusive, -> { where(included: true) } scope :additional, -> { where(included: false) } + scope :legacy_tax, -> { additional.tax.where(adjustable_type: "Spree::Order") } scope :enterprise_fee, -> { where(originator_type: 'EnterpriseFee') } scope :admin, -> { where(originator_type: nil) } diff --git a/app/models/spree/order.rb b/app/models/spree/order.rb index 8aa158f487..28ce6a2d69 100644 --- a/app/models/spree/order.rb +++ b/app/models/spree/order.rb @@ -293,7 +293,11 @@ module Spree # Creates new tax charges if there are any applicable rates. If prices already # include taxes then price adjustments are created instead. def create_tax_charge! - Spree::TaxRate.adjust(self) + clear_legacy_taxes! + + Spree::TaxRate.adjust(self, line_items) + Spree::TaxRate.adjust(self, shipments) if shipments.any? + fee_handler.tax_enterprise_fees! end def name @@ -564,7 +568,7 @@ module Spree end def enterprise_fee_tax - all_adjustments.reload.enterprise_fee.sum(:included_tax) + all_adjustments.tax.where(adjustable: all_adjustments.enterprise_fee).sum(:amount) end def total_tax @@ -591,6 +595,13 @@ module Spree @fee_handler ||= OrderFeesHandler.new(self) end + def clear_legacy_taxes! + # For instances that use additional taxes, old orders can have taxes recorded in + # lump-sum amounts per-order. We clear them here before re-applying the order's taxes, + # which will now be applied per-item. + adjustments.legacy_tax.delete_all + end + def process_each_payment raise Core::GatewayError, Spree.t(:no_pending_payments) if pending_payments.empty? diff --git a/app/models/spree/tax_rate.rb b/app/models/spree/tax_rate.rb index f3f28495e6..369aabb3bf 100644 --- a/app/models/spree/tax_rate.rb +++ b/app/models/spree/tax_rate.rb @@ -16,8 +16,10 @@ module Spree class TaxRate < ApplicationRecord acts_as_paranoid include Spree::Core::CalculatedAdjustments + belongs_to :zone, class_name: "Spree::Zone", inverse_of: :tax_rates belongs_to :tax_category, class_name: "Spree::TaxCategory", inverse_of: :tax_rates + has_many :adjustments, as: :originator validates :amount, presence: true, numericality: true validates :tax_category, presence: true @@ -30,16 +32,32 @@ module Spree return [] if order.distributor && !order.distributor.charges_sales_tax return [] unless order.tax_zone - all.select do |rate| - rate.zone == order.tax_zone || rate.zone.contains?(order.tax_zone) || rate.zone.default_tax + all.includes(zone: { zone_members: :zoneable }).load.select do |rate| + rate.potentially_applicable?(order.tax_zone) end end - def self.adjust(order) - order.all_adjustments.tax.destroy_all + def self.adjust(order, items) + applicable_rates = match(order) + applicable_tax_categories = applicable_rates.map(&:tax_category) - match(order).each do |rate| - rate.adjust(order) + relevant_items, non_relevant_items = items.partition do |item| + applicable_tax_categories.include?(item.tax_category) + end + + relevant_items.each do |item| + item.adjustments.tax.delete_all + relevant_rates = applicable_rates.select { |rate| rate.tax_category == item.tax_category } + relevant_rates.each do |rate| + rate.adjust(order, item) + end + end + + non_relevant_items.each do |item| + if item.adjustments.tax.present? + item.adjustments.tax.delete_all + Spree::ItemAdjustments.new(item).update + end end end @@ -56,31 +74,43 @@ module Spree rate || 0 end - # Creates necessary tax adjustments for the order. - def adjust(order) - label = create_label - if included_in_price - if default_zone_or_zone_match? order - order.line_items.each { |line_item| create_adjustment(label, line_item, false, "open") } - order.shipments.each { |shipment| create_adjustment(label, shipment, false, "open") } - else - amount = -1 * calculator.compute(order) - label = Spree.t(:refund) + label + def potentially_applicable?(order_tax_zone) + # If the rate's zone matches the order's tax zone, then it's applicable. + zone == order_tax_zone || + # If the rate's zone *contains* the order's tax zone, then it's applicable. + zone.contains?(order_tax_zone) || + # The rate's zone is the default zone, then it's always applicable. + (included_in_price? && zone.default_tax) + end - order.adjustments.create( - amount: amount, - originator: self, - order: order, - state: "closed", - label: label - ) + # Creates necessary tax adjustments for the item. + def adjust(order, item) + amount = compute_amount(item) + return if amount.zero? + + included = included_in_price && default_zone_or_zone_match?(order) + + self.adjustments.create!( + adjustable: item, + amount: amount, + order: order, + label: create_label(amount), + included: included + ) + end + + # This method is used by Adjustment#update to recalculate the cost. + def compute_amount(item) + if included_in_price + if default_zone_or_zone_match?(item.order) + calculator.compute(item) + else + # In this case, it's a refund. + calculator.compute(item) * - 1 end else - create_adjustment(label, order, false, "open") + calculator.compute(item) end - - order.adjustments.reload - order.line_items.reload end def default_zone_or_zone_match?(order) @@ -108,9 +138,10 @@ module Spree private - def create_label + def create_label(adjustment_amount) label = "" - label << (name.presence || tax_category.name) + " " + label << "#{Spree.t(:refund)} " if adjustment_amount.negative? + label << "#{(name.presence || tax_category.name)} " label << (show_rate_in_label? ? "#{amount * 100}%" : "") label << " (#{I18n.t('models.tax_rate.included_in_price')})" if included_in_price? label diff --git a/app/services/order_fees_handler.rb b/app/services/order_fees_handler.rb index 6aa71dd9b9..5f7862beb1 100644 --- a/app/services/order_fees_handler.rb +++ b/app/services/order_fees_handler.rb @@ -21,6 +21,7 @@ class OrderFeesHandler create_order_fees! end + tax_enterprise_fees! order.update_order! end @@ -38,6 +39,10 @@ class OrderFeesHandler calculator.create_order_adjustments_for order end + def tax_enterprise_fees! + Spree::TaxRate.adjust(order, order.all_adjustments.enterprise_fee) + end + def update_line_item_fees!(line_item) line_item.adjustments.enterprise_fee.each do |fee| fee.update_adjustment!(line_item, force: true) diff --git a/app/services/order_tax_adjustments_fetcher.rb b/app/services/order_tax_adjustments_fetcher.rb index 7c01a2b175..e39522c2ab 100644 --- a/app/services/order_tax_adjustments_fetcher.rb +++ b/app/services/order_tax_adjustments_fetcher.rb @@ -21,10 +21,9 @@ class OrderTaxAdjustmentsFetcher def all tax_adjustments = order.all_adjustments.tax - enterprise_fees_with_tax = order.all_adjustments.enterprise_fee.with_tax admin_adjustments_with_tax = order.all_adjustments.admin.with_tax - tax_adjustments.or(enterprise_fees_with_tax).or(admin_adjustments_with_tax) + tax_adjustments.or(admin_adjustments_with_tax) end def tax_rates_hash(adjustment) @@ -49,9 +48,8 @@ class OrderTaxAdjustmentsFetcher end def no_tax_adjustments?(adjustment) - # Enterprise Fees and Admin Adjustments currently do not have tax adjustments. + # Admin Adjustments currently do not have tax adjustments. # The tax amount is stored in the included_tax attribute. - adjustment.originator_type == "EnterpriseFee" || - adjustment.originator_type.nil? + adjustment.originator_type.nil? end end diff --git a/config/locales/en.yml b/config/locales/en.yml index a1934c45c0..23783b1d64 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -3150,6 +3150,7 @@ See the %{link} to find out more about %{sitename}'s features and to start using payment_method_not_supported: "Payment method not supported" resend_authorization_email: "Resend authorization email" rma_credit: "RMA credit" + refund: "Refund" server_error: "Server error" shipping_method_names: UPS Ground: "UPS Ground" diff --git a/db/migrate/20210323175627_add_tax_category_to_adjustments.rb b/db/migrate/20210323175627_add_tax_category_to_adjustments.rb new file mode 100644 index 0000000000..37cdb3f5a8 --- /dev/null +++ b/db/migrate/20210323175627_add_tax_category_to_adjustments.rb @@ -0,0 +1,6 @@ +class AddTaxCategoryToAdjustments < ActiveRecord::Migration[5.0] + def change + add_column :spree_adjustments, :tax_category_id, :integer + add_index :spree_adjustments, :tax_category_id + end +end diff --git a/db/migrate/20210406161242_migrate_enterprise_fee_tax_amounts.rb b/db/migrate/20210406161242_migrate_enterprise_fee_tax_amounts.rb new file mode 100644 index 0000000000..f0f99e2bc3 --- /dev/null +++ b/db/migrate/20210406161242_migrate_enterprise_fee_tax_amounts.rb @@ -0,0 +1,92 @@ +# It turns out the good_migrations gem doesn't play nicely with loading classes on polymorphic +# associations. The only workaround seems to be to load the class explicitly, which essentially +# skips the whole point of good_migrations... :/ +require 'enterprise_fee' +require 'concerns/balance' +require 'spree/order' + +class MigrateEnterpriseFeeTaxAmounts < ActiveRecord::Migration[5.0] + class Spree::Adjustment < ApplicationRecord + belongs_to :originator, -> { with_deleted }, polymorphic: true + belongs_to :adjustable, polymorphic: true + belongs_to :order, class_name: "Spree::Order" + belongs_to :tax_category, class_name: 'Spree::TaxCategory' + has_many :adjustments, as: :adjustable, dependent: :destroy + + scope :enterprise_fee, -> { where(originator_type: 'EnterpriseFee') } + end + class Spree::LineItem < ApplicationRecord + belongs_to :variant, class_name: "Spree::Variant" + has_one :product, through: :variant + end + class Spree::Variant < ApplicationRecord + belongs_to :product, class_name: 'Spree::Product' + has_many :line_items, inverse_of: :variant + end + class Spree::Product < ApplicationRecord + belongs_to :tax_category, class_name: 'Spree::TaxCategory' + has_many :variants, class_name: 'Spree::Variant' + end + class Spree::TaxCategory < ApplicationRecord + has_many :tax_rates, dependent: :destroy, inverse_of: :tax_category + end + class Spree::TaxRate < ApplicationRecord + belongs_to :zone, class_name: "Spree::Zone", inverse_of: :tax_rates + belongs_to :tax_category, class_name: "Spree::TaxCategory", inverse_of: :tax_rates + has_many :adjustments, as: :originator + end + + def up + migrate_enterprise_fee_taxes! + end + + def migrate_enterprise_fee_taxes! + Spree::Adjustment.enterprise_fee.where('included_tax <> 0'). + includes(:originator, :adjustable).find_each do |fee| + + tax_category = tax_category_for(fee) + tax_rate = tax_rate_for(tax_category) + + fee.update_columns(tax_category_id: tax_category.id) if tax_category.present? + + Spree::Adjustment.create!( + label: tax_adjustment_label(tax_rate), + amount: fee.included_tax, + order_id: fee.order_id, + adjustable: fee, + originator_type: "Spree::TaxRate", + originator_id: tax_rate&.id, + state: "closed", + included: true + ) + end + end + + def tax_adjustment_label(tax_rate) + if tax_rate.nil? + I18n.t('included_tax') + else + "#{tax_rate.name} #{tax_rate.amount * 100}% (#{I18n.t('models.tax_rate.included_in_price')})" + end + end + + def tax_category_for(fee) + enterprise_fee = fee.originator + + return if enterprise_fee.nil? + + if line_item_fee?(fee) && enterprise_fee.inherits_tax_category? + fee.adjustable&.product&.tax_category + else + enterprise_fee.tax_category + end + end + + def line_item_fee?(fee) + fee.adjustable_type == "Spree::LineItem" + end + + def tax_rate_for(tax_category) + tax_category&.tax_rates&.first + end +end diff --git a/db/schema.rb b/db/schema.rb index 664ad74c3f..710e91050d 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -388,9 +388,11 @@ ActiveRecord::Schema.define(version: 2021_04_15_052410) do t.string "state", limit: 255 t.integer "order_id" t.boolean "included", default: false + t.integer "tax_category_id" t.index ["adjustable_type", "adjustable_id"], name: "index_spree_adjustments_on_adjustable_type_and_adjustable_id" t.index ["order_id"], name: "index_spree_adjustments_on_order_id" t.index ["originator_type", "originator_id"], name: "index_spree_adjustments_on_originator_type_and_originator_id" + t.index ["tax_category_id"], name: "index_spree_adjustments_on_tax_category_id" end create_table "spree_assets", force: :cascade do |t| diff --git a/engines/order_management/app/services/order_management/order/updater.rb b/engines/order_management/app/services/order_management/order/updater.rb index fb1f7cda01..e700cf111f 100644 --- a/engines/order_management/app/services/order_management/order/updater.rb +++ b/engines/order_management/app/services/order_management/order/updater.rb @@ -24,6 +24,8 @@ module OrderManagement end def update_totals_and_states + handle_legacy_taxes + update_totals if order.completed? @@ -65,9 +67,7 @@ module OrderManagement def update_adjustment_total order.adjustment_total = all_adjustments.additional.eligible.sum(:amount) order.additional_tax_total = all_adjustments.tax.additional.sum(:amount) - order.included_tax_total = order.line_item_adjustments.tax.inclusive.sum(:amount) + - all_adjustments.enterprise_fee.sum(:included_tax) + - order.shipment_adjustments.tax.inclusive.sum(:amount) + + order.included_tax_total = all_adjustments.tax.inclusive.sum(:amount) + adjustments.admin.sum(:included_tax) end @@ -213,6 +213,13 @@ module OrderManagement def failed_payments? payments.present? && payments.valid.empty? end + + # Re-applies tax if any legacy taxes are present + def handle_legacy_taxes + return unless order.completed? && order.adjustments.legacy_tax.any? + + order.create_tax_charge! + end end end end diff --git a/engines/order_management/spec/services/order_management/order/updater_spec.rb b/engines/order_management/spec/services/order_management/order/updater_spec.rb index c2ddf1f9d7..7531ff7ec5 100644 --- a/engines/order_management/spec/services/order_management/order/updater_spec.rb +++ b/engines/order_management/spec/services/order_management/order/updater_spec.rb @@ -30,9 +30,7 @@ module OrderManagement it "updates adjustment totals" do allow(order).to receive_message_chain(:all_adjustments, :additional, :eligible, :sum).and_return(-5) allow(order).to receive_message_chain(:all_adjustments, :tax, :additional, :sum).and_return(20) - allow(order).to receive_message_chain(:all_adjustments, :enterprise_fee, :sum).and_return(10) - allow(order).to receive_message_chain(:all_adjustments, :shipping, :sum).and_return(5) - allow(order).to receive_message_chain(:shipment_adjustments, :tax, :inclusive, :sum).and_return(5) + allow(order).to receive_message_chain(:all_adjustments, :tax, :inclusive, :sum).and_return(15) allow(order).to receive_message_chain(:adjustments, :admin, :sum).and_return(2) updater.update_adjustment_total @@ -291,6 +289,52 @@ module OrderManagement end end end + + describe "updating order totals" do + describe "#update_totals_and_states" do + it "deals with legacy taxes" do + expect(updater).to receive(:handle_legacy_taxes) + + updater.update_totals_and_states + end + end + + describe "#handle_legacy_taxes" do + context "when the order is incomplete" do + it "doesn't touch taxes" do + allow(order).to receive(:completed?) { false } + + expect(order).to_not receive(:create_tax_charge!) + updater.__send__(:handle_legacy_taxes) + end + end + + context "when the order is complete" do + before { allow(order).to receive(:completed?) { true } } + + context "and the order has legacy taxes" do + let!(:legacy_tax_adjustment) { + create(:adjustment, order: order, adjustable: order, included: false, + originator_type: "Spree::TaxRate") + } + + it "re-applies order taxes" do + expect(order).to receive(:create_tax_charge!) + + updater.__send__(:handle_legacy_taxes) + end + end + + context "and the order has no legacy taxes" do + it "leaves taxes untouched" do + expect(order).to_not receive(:create_tax_charge!) + + updater.__send__(:handle_legacy_taxes) + end + end + end + end + end end end end diff --git a/lib/open_food_network/enterprise_fee_applicator.rb b/lib/open_food_network/enterprise_fee_applicator.rb index fe4544c604..8b12e17e76 100644 --- a/lib/open_food_network/enterprise_fee_applicator.rb +++ b/lib/open_food_network/enterprise_fee_applicator.rb @@ -11,11 +11,11 @@ module OpenFoodNetwork private def create_adjustment(label, adjustable) - adjustment = enterprise_fee.create_adjustment(label, adjustable, true) + adjustment = enterprise_fee.create_adjustment( + label, adjustable, true, "closed", tax_category(adjustable) + ) 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 line_item_adjustment_label @@ -30,11 +30,11 @@ 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) - tax_rates = TaxRateFinder.tax_rates_of(adjustment) - - tax_rates.select(&:included_in_price).sum do |rate| - rate.compute_tax adjustment.amount + def tax_category(target) + if target.is_a?(Spree::LineItem) && enterprise_fee.inherits_tax_category? + target.product.tax_category + else + enterprise_fee.tax_category end end end diff --git a/lib/open_food_network/xero_invoices_report.rb b/lib/open_food_network/xero_invoices_report.rb index 93b7dbb4e2..a92c133607 100644 --- a/lib/open_food_network/xero_invoices_report.rb +++ b/lib/open_food_network/xero_invoices_report.rb @@ -188,11 +188,11 @@ module OpenFoodNetwork end def total_untaxable_fees(order) - order.all_adjustments.enterprise_fee.without_tax.sum(:amount) + order.all_adjustments.enterprise_fee.where(tax_category: nil).sum(:amount) end def total_taxable_fees(order) - order.all_adjustments.enterprise_fee.with_tax.sum(:amount) + order.all_adjustments.enterprise_fee.where.not(tax_category: nil).sum(:amount) end def total_shipping(order) diff --git a/lib/spree/core/calculated_adjustments.rb b/lib/spree/core/calculated_adjustments.rb index 690dcb19a7..3e06a95179 100644 --- a/lib/spree/core/calculated_adjustments.rb +++ b/lib/spree/core/calculated_adjustments.rb @@ -26,7 +26,7 @@ module Spree # (which is any class that has_many :adjustments) and sets amount based on the # calculator as applied to the given calculable (Order, LineItems[], Shipment, etc.) # By default the adjustment will not be considered mandatory - def create_adjustment(label, adjustable, mandatory = false, state = "closed") + def create_adjustment(label, adjustable, mandatory = false, state = "closed", tax_category = nil) amount = compute_amount(adjustable) return if amount.zero? && !mandatory @@ -37,7 +37,7 @@ module Spree label: label, mandatory: mandatory, state: state, - included: tax_included?(self, adjustable) + tax_category: tax_category } if adjustable.respond_to?(:adjustments) @@ -65,15 +65,6 @@ module Spree private - # Used for setting the #included boolean on tax adjustments. This will be removed in a - # later step, as the responsibility for creating all adjustments related to tax will be - # moved into the Spree::TaxRate class. - def tax_included?(originator, target) - originator.is_a?(Spree::TaxRate) && - originator.included_in_price && - originator.default_zone_or_zone_match?(order_object_for(target)) - end - def order_object_for(target) # Temporary method for adjustments transition. if target.is_a? Spree::Order diff --git a/spec/controllers/admin/bulk_line_items_controller_spec.rb b/spec/controllers/admin/bulk_line_items_controller_spec.rb index ea6cf9da30..6d5fda5ce7 100644 --- a/spec/controllers/admin/bulk_line_items_controller_spec.rb +++ b/spec/controllers/admin/bulk_line_items_controller_spec.rb @@ -366,7 +366,7 @@ describe Admin::BulkLineItemsController, type: :controller do expect(order.shipment_adjustments.sum(:amount)).to eq 12.57 expect(order.item_total).to eq 40.0 expect(order.adjustment_total).to eq 27.0 - expect(order.included_tax_total).to eq 2.93 # Pending: taxes on enterprise fees unchanged + expect(order.included_tax_total).to eq 3.38 expect(order.payment_state).to eq "balance_due" end end diff --git a/spec/controllers/spree/admin/orders/payments/payments_controller_spec.rb b/spec/controllers/spree/admin/orders/payments/payments_controller_spec.rb index 8d64b3a7f2..3edf6a5fb6 100644 --- a/spec/controllers/spree/admin/orders/payments/payments_controller_spec.rb +++ b/spec/controllers/spree/admin/orders/payments/payments_controller_spec.rb @@ -311,7 +311,7 @@ describe Spree::Admin::PaymentsController, type: :controller do end context "the order contains an item that is out of stock" do - let!(:order) { create(:order, distributor: shop, state: 'payment') } + let!(:order) { create(:order_with_totals, distributor: shop, state: 'payment') } before do order.line_items.first.variant.update_attribute(:on_hand, 0) diff --git a/spec/controllers/spree/admin/orders_controller_spec.rb b/spec/controllers/spree/admin/orders_controller_spec.rb index 0d881bc82f..b4fdd52b9a 100644 --- a/spec/controllers/spree/admin/orders_controller_spec.rb +++ b/spec/controllers/spree/admin/orders_controller_spec.rb @@ -58,19 +58,28 @@ describe Spree::Admin::OrdersController, type: :controller do expect(response.status).to eq 302 end - it "updates distribution charges and redirects to order details page" do - expect_any_instance_of(Spree::Order).to receive(:recreate_all_fees!) + context "recalculating fees and taxes" do + before do + allow(Spree::Order).to receive_message_chain(:includes, :find_by!) { order } + end - spree_put :update, params + it "updates fees and taxes and redirects to order details page" do + expect(order).to receive(:recreate_all_fees!) + expect(order).to receive(:create_tax_charge!) - expect(response).to redirect_to spree.edit_admin_order_path(order) + spree_put :update, params + + expect(response).to redirect_to spree.edit_admin_order_path(order) + end end context "recalculating enterprise fees" do let(:user) { create(:admin_user) } let(:variant1) { create(:variant) } let(:variant2) { create(:variant) } - let(:distributor) { create(:distributor_enterprise, allow_order_changes: true) } + let(:distributor) { + create(:distributor_enterprise, allow_order_changes: true, charges_sales_tax: true) + } let(:order_cycle) { create(:simple_order_cycle, distributors: [distributor]) } let(:enterprise_fee) { create(:enterprise_fee, calculator: build(:calculator_per_item) ) } let!(:exchange) { create(:exchange, incoming: true, sender: variant1.product.supplier, receiver: order_cycle.coordinator, variants: [variant1, variant2], enterprise_fees: [enterprise_fee]) } @@ -133,6 +142,74 @@ describe Spree::Admin::OrdersController, type: :controller do expect(order.adjustment_total).to eq 0 end end + + context "with taxes on enterprise fees" do + let(:zone) { create(:zone_with_member) } + let(:tax_included) { true } + let(:tax_rate) { + create(:tax_rate, amount: 0.25, included_in_price: tax_included, zone: zone) + } + let!(:enterprise_fee) { + create(:enterprise_fee, tax_category: tax_rate.tax_category, amount: 1) + } + + before do + allow(order).to receive(:tax_zone) { zone } + end + + context "with included taxes" do + it "taxes fees correctly" do + spree_put :update, { id: order.number } + order.reload + + expect(order.all_adjustments.tax.count).to eq 2 + expect(order.enterprise_fee_tax).to eq 0.4 + + expect(order.included_tax_total).to eq 0.4 + expect(order.additional_tax_total).to eq 0 + end + end + + context "with added taxes" do + let(:tax_included) { false } + + it "taxes fees correctly" do + spree_put :update, { id: order.number } + order.reload + + expect(order.all_adjustments.tax.count).to eq 2 + expect(order.enterprise_fee_tax).to eq 0.5 + + expect(order.included_tax_total).to eq 0 + expect(order.additional_tax_total).to eq 0.5 + end + + context "when the order has legacy taxes" do + let(:legacy_tax_adjustment) { + create(:adjustment, amount: 0.5, included: false, originator: tax_rate, + order: order, adjustable: order, state: "closed") + } + + before do + order.all_adjustments.tax.delete_all + order.adjustments << legacy_tax_adjustment + end + + it "removes legacy tax adjustments before recalculating tax" do + expect(order.all_adjustments.tax.count).to eq 1 + expect(order.all_adjustments.tax).to include legacy_tax_adjustment + expect(order.additional_tax_total).to eq 0.5 + + spree_put :update, { id: order.number } + order.reload + + expect(order.all_adjustments.tax.count).to eq 2 + expect(order.all_adjustments.tax).to_not include legacy_tax_adjustment + expect(order.additional_tax_total).to eq 0.5 + end + end + end + end end end diff --git a/spec/features/admin/order_print_ticket_spec.rb b/spec/features/admin/order_print_ticket_spec.rb index 9bfddf205d..f5d9d30ab0 100644 --- a/spec/features/admin/order_print_ticket_spec.rb +++ b/spec/features/admin/order_print_ticket_spec.rb @@ -17,9 +17,9 @@ feature ' let!(:order) do create(:order_with_taxes, distributor: distributor, ship_address: create(:address), product_price: 110, tax_rate_amount: 0.1, - tax_rate_name: "Tax 1").tap do |record| - Spree::TaxRate.adjust(record) - record.update_shipping_fees! + tax_rate_name: "Tax 1").tap do |order| + order.create_tax_charge! + order.update_shipping_fees! end end diff --git a/spec/features/admin/order_spec.rb b/spec/features/admin/order_spec.rb index 34921c68d1..9ed8df28d0 100644 --- a/spec/features/admin/order_spec.rb +++ b/spec/features/admin/order_spec.rb @@ -308,9 +308,9 @@ feature ' let!(:order) do create(:order_with_taxes, distributor: distributor1, ship_address: create(:address), product_price: 110, tax_rate_amount: 0.1, - tax_rate_name: "Tax 1").tap do |record| - Spree::TaxRate.adjust(record) - record.update_shipping_fees! + tax_rate_name: "Tax 1").tap do |order| + order.create_tax_charge! + order.update_shipping_fees! end end diff --git a/spec/features/admin/reports_spec.rb b/spec/features/admin/reports_spec.rb index 7073da51be..460562e98f 100644 --- a/spec/features/admin/reports_spec.rb +++ b/spec/features/admin/reports_spec.rb @@ -186,7 +186,7 @@ feature ' let(:shipping_tax_category) { create(:tax_category, tax_rates: [shipping_tax_rate]) } let!(:shipping_method) { create(:shipping_method_with, :expensive_name, distributors: [distributor1], tax_category: shipping_tax_category) } let(:enterprise_fee) { create(:enterprise_fee, enterprise: user1.enterprises.first, tax_category: product2.tax_category, calculator: Calculator::FlatRate.new(preferred_amount: 120.0)) } - let(:order_cycle) { create(:simple_order_cycle, coordinator: distributor1, coordinator_fees: [enterprise_fee], distributors: [distributor1], variants: [product1.master]) } + let(:order_cycle) { create(:simple_order_cycle, coordinator: distributor1, coordinator_fees: [enterprise_fee], distributors: [distributor1], variants: [product1.variants.first, product2.variants.first]) } let!(:zone) { create(:zone_with_member) } let(:address) { create(:address) } @@ -194,20 +194,20 @@ feature ' let(:product1) { create(:taxed_product, zone: zone, price: 12.54, tax_rate_amount: 0) } let(:product2) { create(:taxed_product, zone: zone, price: 500.15, tax_rate_amount: 0.2) } - let!(:line_item1) { create(:line_item, variant: product1.master, price: 12.54, quantity: 1, order: order1) } - let!(:line_item2) { create(:line_item, variant: product2.master, price: 500.15, quantity: 3, order: order1) } + let!(:line_item1) { create(:line_item, variant: product1.variants.first, price: 12.54, quantity: 1, order: order1) } + let!(:line_item2) { create(:line_item, variant: product2.variants.first, price: 500.15, quantity: 3, order: order1) } before do order1.reload - 2.times { order1.next } - order1.select_shipping_method shipping_method.id - order1.reload.recreate_all_fees! - order1.create_tax_charge! - order1.update_order! - order1.finalize! + break unless order1.next! until order1.delivery? + order1.select_shipping_method(shipping_method.id) + order1.recreate_all_fees! + break unless order1.next! until order1.payment? + create(:payment, state: "checkout", order: order1, amount: order1.reload.total, + payment_method: create(:payment_method, distributors: [distributor1])) + break unless order1.next! until order1.complete? login_as_admin_and_visit spree.admin_reports_path - click_link "Sales Tax" select("Tax types", from: "report_type") end @@ -397,20 +397,25 @@ feature ' let(:order_cycle) { create(:simple_order_cycle, coordinator: distributor1, coordinator_fees: [enterprise_fee1, enterprise_fee2], distributors: [distributor1], variants: [product1.master]) } let!(:zone) { create(:zone_with_member) } - let(:country) { Spree::Country.find Spree::Config.default_country_id } - let(:bill_address) { create(:address, firstname: 'Customer', lastname: 'Name', address1: 'customer l1', address2: '', city: 'customer city', zipcode: 1234, country: country) } + let(:bill_address) { + create(:address, firstname: 'Customer', lastname: 'Name', address1: 'customer l1', + address2: '', city: 'customer city', zipcode: 1234) + } let(:order1) { create(:order, order_cycle: order_cycle, distributor: user1.enterprises.first, shipments: [shipment], bill_address: bill_address) } let(:product1) { create(:taxed_product, zone: zone, price: 12.54, tax_rate_amount: 0, sku: 'sku1') } let(:product2) { create(:taxed_product, zone: zone, price: 500.15, tax_rate_amount: 0.2, sku: 'sku2') } describe "with adjustments" do - let!(:line_item1) { create(:line_item, variant: product1.master, price: 12.54, quantity: 1, order: order1) } - let!(:line_item2) { create(:line_item, variant: product2.master, price: 500.15, quantity: 3, order: order1) } + let!(:line_item1) { create(:line_item, variant: product1.variants.first, price: 12.54, quantity: 1, order: order1) } + let!(:line_item2) { create(:line_item, variant: product2.variants.first, price: 500.15, quantity: 3, order: order1) } + let!(:tax_category) { create(:tax_category) } + let!(:tax_rate) { create(:tax_rate, tax_category: tax_category) } let!(:adj_shipping) { create(:adjustment, order: order1, adjustable: order1, label: "Shipping", originator: shipping_method, amount: 100.55) } - let!(:adj_fee1) { create(:adjustment, order: order1, adjustable: order1, originator: enterprise_fee1, label: "Enterprise fee untaxed", amount: 10, included_tax: 0) } - let!(:adj_fee2) { create(:adjustment, order: order1, adjustable: order1, originator: enterprise_fee2, label: "Enterprise fee taxed", amount: 20, included_tax: 2) } - let!(:adj_manual1) { create(:adjustment, order: order1, adjustable: order1, originator: nil, label: "Manual adjustment", amount: 30, included_tax: 0) } + let!(:adj_fee1) { create(:adjustment, order: order1, adjustable: order1, originator: enterprise_fee1, label: "Enterprise fee untaxed", amount: 10) } + let!(:adj_fee2) { create(:adjustment, order: order1, adjustable: order1, originator: enterprise_fee2, label: "Enterprise fee taxed", amount: 20, tax_category: tax_category) } + let!(:adj_fee2_tax) { create(:adjustment, order: order1, adjustable: adj_fee2, originator: tax_rate, amount: 3, state: "closed") } + let!(:adj_manual1) { create(:adjustment, order: order1, adjustable: order1, originator: nil, label: "Manual adjustment", amount: 30) } let!(:adj_manual2) { create(:adjustment, order: order1, adjustable: order1, originator: nil, label: "Manual adjustment", amount: 40, included_tax: 3) } before do @@ -418,8 +423,10 @@ feature ' order1.update_attribute :email, 'customer@email.com' order1.shipment.update_columns(included_tax_total: 10.06) Timecop.travel(Time.zone.local(2015, 4, 25, 14, 0, 0)) { order1.finalize! } - login_as_admin_and_visit spree.admin_reports_path + order1.reload + order1.create_tax_charge! + login_as_admin_and_visit spree.admin_reports_path click_link 'Xero Invoices' end @@ -508,7 +515,7 @@ feature ' end def xero_invoice_row(sku, description, amount, quantity, tax_type, opts = {}) - opts.reverse_merge!(customer_name: 'Customer Name', address1: 'customer l1', city: 'customer city', state: 'Victoria', zipcode: '1234', country: country.name, invoice_number: order1.number, order_number: order1.number, invoice_date: '2015-04-26', due_date: '2015-05-26', account_code: 'food sales') + opts.reverse_merge!(customer_name: 'Customer Name', address1: 'customer l1', city: 'customer city', state: 'Victoria', zipcode: '1234', country: 'Australia', invoice_number: order1.number, order_number: order1.number, invoice_date: '2015-04-26', due_date: '2015-05-26', account_code: 'food sales') [opts[:customer_name], 'customer@email.com', opts[:address1], '', '', '', opts[:city], opts[:state], opts[:zipcode], opts[:country], opts[:invoice_number], opts[:order_number], opts[:invoice_date], opts[:due_date], diff --git a/spec/features/consumer/shopping/checkout_spec.rb b/spec/features/consumer/shopping/checkout_spec.rb index 8d8be39ce0..581f006abf 100644 --- a/spec/features/consumer/shopping/checkout_spec.rb +++ b/spec/features/consumer/shopping/checkout_spec.rb @@ -13,7 +13,9 @@ feature "As a consumer I want to check out my cart", js: true do let(:distributor) { create(:distributor_enterprise, charges_sales_tax: true) } let(:supplier) { create(:supplier_enterprise) } let!(:order_cycle) { create(:simple_order_cycle, suppliers: [supplier], distributors: [distributor], coordinator: create(:distributor_enterprise), variants: [variant]) } - let(:enterprise_fee) { create(:enterprise_fee, amount: 1.23, tax_category: product.tax_category) } + let(:enterprise_fee) { create(:enterprise_fee, amount: 1.23, tax_category: fee_tax_category) } + let(:fee_tax_rate) { create(:tax_rate, amount: 0.10, zone: zone, included_in_price: true) } + let(:fee_tax_category) { create(:tax_category, tax_rates: [fee_tax_rate]) } let(:product) { create(:taxed_product, supplier: supplier, price: 10, zone: zone, tax_rate_amount: 0.1) } let(:variant) { product.variants.first } let(:order) { create(:order, order_cycle: order_cycle, distributor: distributor, bill_address_id: nil, ship_address_id: nil) } diff --git a/spec/lib/open_food_network/enterprise_fee_applicator_spec.rb b/spec/lib/open_food_network/enterprise_fee_applicator_spec.rb index 7ddab9d50d..be95930f95 100644 --- a/spec/lib/open_food_network/enterprise_fee_applicator_spec.rb +++ b/spec/lib/open_food_network/enterprise_fee_applicator_spec.rb @@ -5,65 +5,85 @@ require 'open_food_network/enterprise_fee_applicator' module OpenFoodNetwork describe EnterpriseFeeApplicator do - it "creates an adjustment for a line item" do - line_item = create(:line_item) - enterprise_fee = create(:enterprise_fee) - product = create(:simple_product) + let(:line_item) { create(:line_item) } + let(:inherits_tax) { true } + let(:enterprise_fee) { + create(:enterprise_fee, inherits_tax_category: inherits_tax, tax_category: fee_tax_category) + } + let(:fee_tax_category) { nil } + let(:tax_category) { create(:tax_category) } + let(:product) { create(:simple_product, tax_category: tax_category) } + let(:target_variant) { product.variants.first } + let(:applicator) { EnterpriseFeeApplicator.new(enterprise_fee, target_variant, 'role') } - efa = EnterpriseFeeApplicator.new enterprise_fee, product.master, 'role' - allow(efa).to receive(:line_item_adjustment_label) { 'label' } - efa.create_line_item_adjustment line_item + describe "#create_line_item_adjustment" do + it "creates an adjustment for a line item" do + allow(applicator).to receive(:line_item_adjustment_label) { 'label' } + applicator.create_line_item_adjustment line_item - adjustment = Spree::Adjustment.last - expect(adjustment.label).to eq('label') - expect(adjustment.adjustable).to eq(line_item) - expect(adjustment.originator).to eq(enterprise_fee) - expect(adjustment).to be_mandatory + adjustment = Spree::Adjustment.last + expect(adjustment.label).to eq('label') + expect(adjustment.adjustable).to eq(line_item) + expect(adjustment.originator).to eq(enterprise_fee) + expect(adjustment.tax_category).to eq(tax_category) + expect(adjustment).to be_mandatory - md = adjustment.metadata - expect(md.enterprise).to eq(enterprise_fee.enterprise) - expect(md.fee_name).to eq(enterprise_fee.name) - expect(md.fee_type).to eq(enterprise_fee.fee_type) - expect(md.enterprise_role).to eq('role') + metadata = adjustment.metadata + expect(metadata.enterprise).to eq(enterprise_fee.enterprise) + expect(metadata.fee_name).to eq(enterprise_fee.name) + expect(metadata.fee_type).to eq(enterprise_fee.fee_type) + expect(metadata.enterprise_role).to eq('role') + end end - it "creates an adjustment for an order" do - order = create(:order) - enterprise_fee = create(:enterprise_fee) - product = create(:simple_product) + describe "#create_order_adjustment" do + let(:target_variant) { nil } + let(:inherits_tax) { false } + let(:fee_tax_category) { tax_category } + let(:order) { line_item.order } - efa = EnterpriseFeeApplicator.new enterprise_fee, nil, 'role' - allow(efa).to receive(:order_adjustment_label) { 'label' } - efa.create_order_adjustment order + it "creates an adjustment for an order" do + allow(applicator).to receive(:order_adjustment_label) { 'label' } + applicator.create_order_adjustment order - adjustment = Spree::Adjustment.last - expect(adjustment.label).to eq('label') - expect(adjustment.adjustable).to eq(order) - expect(adjustment.originator).to eq(enterprise_fee) - expect(adjustment).to be_mandatory + adjustment = Spree::Adjustment.last + expect(adjustment.label).to eq('label') + expect(adjustment.adjustable).to eq(order) + expect(adjustment.originator).to eq(enterprise_fee) + expect(adjustment.tax_category).to eq(tax_category) + expect(adjustment).to be_mandatory - md = adjustment.metadata - expect(md.enterprise).to eq(enterprise_fee.enterprise) - expect(md.fee_name).to eq(enterprise_fee.name) - expect(md.fee_type).to eq(enterprise_fee.fee_type) - expect(md.enterprise_role).to eq('role') + metadata = adjustment.metadata + expect(metadata.enterprise).to eq(enterprise_fee.enterprise) + expect(metadata.fee_name).to eq(enterprise_fee.name) + expect(metadata.fee_type).to eq(enterprise_fee.fee_type) + expect(metadata.enterprise_role).to eq('role') + end end - it "makes an adjustment label for a line item" do - variant = double(:variant, product: double(:product, name: 'Bananas')) - enterprise_fee = double(:enterprise_fee, fee_type: 'packing', enterprise: double(:enterprise, name: 'Ballantyne')) + describe "making labels" do + let(:variant) { double(:variant, product: double(:product, name: 'Bananas')) } + let(:enterprise_fee) { + double(:enterprise_fee, fee_type: 'packing', + enterprise: double(:enterprise, name: 'Ballantyne')) + } + let(:applicator) { EnterpriseFeeApplicator.new enterprise_fee, variant, 'distributor' } - efa = EnterpriseFeeApplicator.new enterprise_fee, variant, 'distributor' + describe "#line_item_adjustment_label" do + it "makes an adjustment label for a line item" do + expect(applicator.send(:line_item_adjustment_label)). + to eq("Bananas - packing fee by distributor Ballantyne") + end + end - expect(efa.send(:line_item_adjustment_label)).to eq("Bananas - packing fee by distributor Ballantyne") - end + describe "#order_adjustment_label" do + let(:applicator) { EnterpriseFeeApplicator.new enterprise_fee, nil, 'distributor' } - it "makes an adjustment label for an order" do - enterprise_fee = double(:enterprise_fee, fee_type: 'packing', enterprise: double(:enterprise, name: 'Ballantyne')) - - efa = EnterpriseFeeApplicator.new enterprise_fee, nil, 'distributor' - - expect(efa.send(:order_adjustment_label)).to eq("Whole order - packing fee by distributor Ballantyne") + it "makes an adjustment label for an order" do + expect(applicator.send(:order_adjustment_label)). + to eq("Whole order - packing fee by distributor Ballantyne") + end + end end end end diff --git a/spec/migrations/migrate_enterprise_fee_tax_amounts_spec.rb b/spec/migrations/migrate_enterprise_fee_tax_amounts_spec.rb new file mode 100644 index 0000000000..62a6ddf16e --- /dev/null +++ b/spec/migrations/migrate_enterprise_fee_tax_amounts_spec.rb @@ -0,0 +1,177 @@ +# frozen_string_literal: true + +require 'spec_helper' +require_relative '../../db/migrate/20210406161242_migrate_enterprise_fee_tax_amounts' + +describe MigrateEnterpriseFeeTaxAmounts do + subject { MigrateEnterpriseFeeTaxAmounts.new } + + let(:tax_category_regular) { create(:tax_category) } + let(:tax_rate_regular) { create(:tax_rate, tax_category: tax_category_regular) } + let(:tax_category_inherited) { create(:tax_category) } + let(:tax_rate_inherited) { create(:tax_rate, tax_category: tax_category_inherited) } + let(:enterprise_fee_regular) { create(:enterprise_fee, inherits_tax_category: false, + tax_category: tax_category_regular) } + let(:enterprise_fee_inheriting) { create(:enterprise_fee, inherits_tax_category: true) } + let(:fee_without_tax) { create(:adjustment, originator: enterprise_fee_regular, included_tax: 0) } + let(:fee_regular) { create(:adjustment, originator: enterprise_fee_regular, included_tax: 1.23) } + let(:fee_inheriting) { create(:adjustment, originator: enterprise_fee_inheriting, + adjustable: line_item, included_tax: 4.56) } + let(:product) { create(:product, tax_category: tax_category_inherited) } + let!(:line_item) { create(:line_item, variant: product.variants.first) } + + describe '#migrate_enterprise_fee_taxes!' do + context "when the fee has no tax" do + before { fee_without_tax } + + it "doesn't move the tax to an adjustment" do + expect(Spree::Adjustment).to_not receive(:create!) + + subject.migrate_enterprise_fee_taxes! + end + end + + context "when the fee has (non-inheriting) tax" do + before { fee_regular; tax_rate_regular } + + it "moves the tax to an adjustment" do + expect(Spree::Adjustment).to receive(:create!).and_call_original + + subject.migrate_enterprise_fee_taxes! + + expect(fee_regular.reload.tax_category).to eq tax_category_regular + + tax_adjustment = Spree::Adjustment.tax.last + + expect(tax_adjustment.amount).to eq fee_regular.included_tax + expect(tax_adjustment.adjustable).to eq fee_regular + expect(tax_adjustment.originator).to eq tax_rate_regular + expect(tax_adjustment.state).to eq "closed" + expect(tax_adjustment.included).to eq true + end + end + + context "when the fee has tax and inherits tax category from product" do + before { fee_inheriting; tax_rate_inherited } + + it "moves the tax to an adjustment" do + expect(Spree::Adjustment).to receive(:create!).and_call_original + + subject.migrate_enterprise_fee_taxes! + + expect(fee_inheriting.reload.tax_category).to eq tax_category_inherited + + tax_adjustment = Spree::Adjustment.tax.last + + expect(tax_adjustment.amount).to eq fee_inheriting.included_tax + expect(tax_adjustment.adjustable).to eq fee_inheriting + expect(tax_adjustment.originator).to eq tax_rate_inherited + expect(tax_adjustment.state).to eq "closed" + expect(tax_adjustment.included).to eq true + end + end + + context "when the fee has a soft-deleted EnterpriseFee" do + before do + enterprise_fee_regular.update_columns(deleted_at: Time.zone.now) + fee_regular + tax_rate_regular + end + + it "moves the tax to an adjustment" do + expect(Spree::Adjustment).to receive(:create!).and_call_original + + subject.migrate_enterprise_fee_taxes! + + expect(fee_regular.reload.tax_category).to eq tax_category_regular + + tax_adjustment = Spree::Adjustment.tax.last + + expect(tax_adjustment.amount).to eq fee_regular.included_tax + expect(tax_adjustment.adjustable).to eq fee_regular + expect(tax_adjustment.originator).to eq tax_rate_regular + expect(tax_adjustment.state).to eq "closed" + expect(tax_adjustment.included).to eq true + end + end + + context "when the fee has a hard-deleted EnterpriseFee" do + before do + fee_regular + tax_rate_regular + EnterpriseFee.delete_all + expect(fee_regular.reload.originator).to eq nil + end + + it "moves the tax to an adjustment" do + expect(Spree::Adjustment).to receive(:create!).and_call_original + + subject.migrate_enterprise_fee_taxes! + + expect(fee_regular.reload.tax_category).to eq nil + + tax_adjustment = Spree::Adjustment.tax.last + + expect(tax_adjustment.amount).to eq fee_regular.included_tax + expect(tax_adjustment.adjustable).to eq fee_regular + expect(tax_adjustment.originator_id).to eq nil + expect(tax_adjustment.originator_type).to eq "Spree::TaxRate" + expect(tax_adjustment.state).to eq "closed" + expect(tax_adjustment.included).to eq true + end + end + end + + describe '#tax_category_for' do + it "returns the correct tax category when not inherited from line item" do + expect(subject.tax_category_for(fee_regular)).to eq tax_category_regular + end + + it "returns the correct tax category when inherited from line item" do + expect(subject.tax_category_for(fee_inheriting)).to eq tax_category_inherited + end + + it "returns nil if the associated EnterpriseFee was hard-deleted and can't be found" do + fee_regular + EnterpriseFee.delete_all + fee_regular.reload + + expect(subject.tax_category_for(fee_regular)).to eq nil + end + end + + describe '#tax_rate_for' do + let!(:tax_category) { create(:tax_category) } + let!(:tax_rate) { create(:tax_rate, tax_category: tax_category) } + + context "when a tax rate exists" do + it "returns a valid tax rate" do + expect(subject.tax_rate_for(tax_category)).to eq tax_rate + end + end + + context "when the tax category is nil" do + it "returns nil" do + expect(subject.tax_rate_for(nil)).to eq nil + end + end + end + + describe '#tax_adjustment_label' do + let(:tax_rate) { create(:tax_rate, name: "Test Rate", amount: 0.20) } + + context "when a tax rate is given" do + it "makes a detailed label" do + expect(subject.tax_adjustment_label(tax_rate)). + to eq("Test Rate 20.0% (Included in price)") + end + end + + context "when the tax rate is nil" do + it "makes a basic label" do + expect(subject.tax_adjustment_label(nil)). + to eq("Included tax") + end + end + end +end diff --git a/spec/models/spree/adjustment_spec.rb b/spec/models/spree/adjustment_spec.rb index 5d66493d64..76c63879dc 100644 --- a/spec/models/spree/adjustment_spec.rb +++ b/spec/models/spree/adjustment_spec.rb @@ -177,22 +177,38 @@ module Spree let!(:zone) { create(:zone_with_member) } let!(:order) { create(:order, bill_address: create(:address)) } let!(:line_item) { create(:line_item, order: order) } - let(:tax_rate) { create(:tax_rate, included_in_price: true, calculator: ::Calculator::FlatRate.new(preferred_amount: 0.1)) } + let(:tax_category) { create(:tax_category, tax_rates: [tax_rate]) } + let(:tax_rate) { create(:tax_rate, included_in_price: true, amount: 0.10) } let(:adjustment) { line_item.adjustments.reload.first } before do order.reload - tax_rate.adjust(order) + tax_rate.adjust(order, line_item) end - it "has tax included" do - expect(adjustment.amount).to be_positive - expect(adjustment.included).to be true + context "when the tax rate is inclusive" do + it "has 10% inclusive tax correctly recorded" do + amount = line_item.amount - (line_item.amount / (1 + tax_rate.amount)) + rounded_amount = tax_rate.calculator.__send__(:round_to_two_places, amount) + expect(adjustment.amount).to eq rounded_amount + expect(adjustment.amount).to eq 0.91 + expect(adjustment.included).to be true + end + + it "does not crash when order data has been updated previously" do + order.line_item_adjustments.first.destroy + tax_rate.adjust(order, line_item) + end end - it "does not crash when order data has been updated previously" do - order.line_item_adjustments.first.destroy - tax_rate.adjust(order) + context "when the tax rate is additional" do + let(:tax_rate) { create(:tax_rate, included_in_price: false, amount: 0.10) } + + it "has 10% added tax correctly recorded" do + expect(adjustment.amount).to eq line_item.amount * tax_rate.amount + expect(adjustment.amount).to eq 1.0 + expect(adjustment.included).to be false + end end end @@ -248,30 +264,7 @@ module Spree context "when the shipment has an added tax rate" do let(:inclusive_tax) { false } - # Current behaviour. Will be replaced by the pending test below - it "records the tax on the order's adjustments" do - order.shipments = [shipment] - order.create_tax_charge! - order.update_totals - - expect(order.shipment_adjustments.tax.count).to be_zero - - # Finding the added tax for an amount: - # total * rate - # 50 * 0.25 - # = 12.5 - expect(order.adjustments.tax.first.amount).to eq(12.50) - expect(order.adjustments.tax.first.included).to eq false - - expect(shipment.reload.cost).to eq(50) - expect(shipment.included_tax_total).to eq(0) - expect(shipment.additional_tax_total).to eq(0) - - expect(order.included_tax_total).to eq(0) - expect(order.additional_tax_total).to eq(12.50) - end - - xit "records the tax on the shipment's adjustments" do + it "records the tax on the shipment's adjustments" do order.shipments = [shipment] order.create_tax_charge! order.update_totals @@ -340,35 +333,34 @@ module Spree 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(:adjustment) { order.all_adjustments.reload.enterprise_fee.first } + let(:fee) { order.all_adjustments.reload.enterprise_fee.first } + let(:fee_tax) { fee.adjustments.tax.first } context "when enterprise fees have a fixed tax_category" do before do - order.reload.recreate_all_fees! + order.recreate_all_fees! end context "when enterprise fees are taxed per-order" do let(:enterprise_fee) { create(:enterprise_fee, enterprise: coordinator, tax_category: fee_tax_category, calculator: ::Calculator::FlatRate.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 + it "records the correct amount in a tax adjustment" do # 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) + expect(fee_tax.amount).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.recreate_all_fees! 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) + it "records the correct amount in a tax adjustment" do + expect(fee_tax.amount).to eq(5.0) end end @@ -380,7 +372,7 @@ module Spree end it "records no tax as charged" do - expect(adjustment.included_tax).to eq(0) + expect(fee_tax).to be_nil end end end @@ -389,21 +381,19 @@ module Spree 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) + it "records the correct amount in a tax adjustment" do + expect(fee_tax.amount).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.recreate_all_fees! 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) + it "records the correct amount in a tax adjustment" do + expect(fee_tax.amount).to eq(5.0) end end end @@ -417,8 +407,6 @@ module Spree 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.recreate_all_fees! end @@ -430,7 +418,7 @@ 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(fee_tax).to be_nil expect(line_item.adjustments.tax.first.amount).to eq(1.67) end end @@ -438,7 +426,6 @@ module Spree 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.recreate_all_fees! end @@ -446,8 +433,7 @@ 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) + expect(fee_tax).to be_nil end end end @@ -456,11 +442,11 @@ module Spree let(:enterprise_fee) { create(:enterprise_fee, enterprise: coordinator, inherits_tax_category: true, 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 + it "records the correct amount in a tax adjustment" do # 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(fee_tax.amount).to eq(8.33) expect(line_item.adjustments.tax.first.amount).to eq(1.67) end end @@ -468,16 +454,15 @@ module Spree 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.recreate_all_fees! end - it "records the tax on TaxRate adjustment on the order" do + it "records the correct amount in a tax adjustment" do # 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) + expect(fee_tax.amount).to eq(10.0) + expect(order.all_adjustments.tax.sum(:amount)).to eq(12.0) end end end diff --git a/spec/models/spree/order/state_machine_spec.rb b/spec/models/spree/order/state_machine_spec.rb index 0b37ee41fa..e61b84a9f4 100644 --- a/spec/models/spree/order/state_machine_spec.rb +++ b/spec/models/spree/order/state_machine_spec.rb @@ -55,7 +55,7 @@ describe Spree::Order do end it "adjusts tax rates when transitioning to payment" do - expect(Spree::TaxRate).to receive(:adjust) + expect(Spree::TaxRate).to receive(:adjust).at_least(:once) order.next! end end diff --git a/spec/models/spree/order/tax_spec.rb b/spec/models/spree/order/tax_spec.rb index dae075c9fa..072e8999cd 100644 --- a/spec/models/spree/order/tax_spec.rb +++ b/spec/models/spree/order/tax_spec.rb @@ -113,5 +113,60 @@ module Spree end end end + + describe "#create_tax_charge!" do + context "handling legacy taxes" do + let(:order) { create(:order) } + let(:zone) { create(:zone_with_member) } + let(:tax_rate20) { + create(:tax_rate, amount: 0.20, included_in_price: false, zone: zone) + } + let(:tax_rate30) { + create(:tax_rate, amount: 0.30, included_in_price: false, zone: zone) + } + let!(:variant) { + create(:variant, tax_category: tax_rate20.tax_category, price: 10) + } + let!(:line_item) { + create(:line_item, variant: variant, order: order, quantity: 2) + } + let!(:shipping_method) { + create(:shipping_method, tax_category: tax_rate30.tax_category) + } + let!(:shipment) { + create(:shipment_with, :shipping_method, order: order, cost: 50, + shipping_method: shipping_method) + } + + before do + shipment.update_columns(cost: 20.0) + order.reload + + allow(order).to receive(:completed_at) { Time.zone.now } + allow(order).to receive(:tax_zone) { zone } + end + + context "when the order has legacy taxes" do + let!(:legacy_tax_adjustment) { + create(:adjustment, order: order, adjustable: order, included: false, + label: "legacy", originator_type: "Spree::TaxRate") + } + + it "removes any legacy tax adjustments on order" do + order.create_tax_charge! + + expect(order.reload.adjustments).to_not include legacy_tax_adjustment + end + + it "re-applies taxes on individual items" do + order.create_tax_charge! + + expect(order.all_adjustments.tax.count).to eq 2 + expect(line_item.adjustments.tax.first.amount).to eq 4 + expect(shipment.adjustments.tax.first.amount).to eq 6 + end + end + end + end end end diff --git a/spec/models/spree/order_spec.rb b/spec/models/spree/order_spec.rb index 6b88c538b0..bc8fcb9510 100644 --- a/spec/models/spree/order_spec.rb +++ b/spec/models/spree/order_spec.rb @@ -630,38 +630,54 @@ describe Spree::Order do end end - describe "getting the enterprise fee tax" do + describe "#enterprise_fee_tax" do let!(:order) { create(:order) } - let(:enterprise_fee1) { create(:enterprise_fee) } - let(:enterprise_fee2) { create(:enterprise_fee) } - let!(:adjustment1) { - create(:adjustment, adjustable: order, originator: enterprise_fee1, label: "EF 1", - amount: 123, included_tax: 10.00, order: order) + let(:enterprise_fee) { create(:enterprise_fee) } + let!(:fee_adjustment) { + create(:adjustment, adjustable: order, originator: enterprise_fee, + amount: 100, order: order, state: "closed") } - let!(:adjustment2) { - create(:adjustment, adjustable: order, originator: enterprise_fee2, label: "EF 2", - amount: 123, included_tax: 2.00, order: order) + let!(:fee_tax1) { + create(:adjustment, adjustable: fee_adjustment, originator_type: "Spree::TaxRate", + amount: 12.3, order: order, state: "closed") + } + let!(:fee_tax2) { + create(:adjustment, adjustable: fee_adjustment, originator_type: "Spree::TaxRate", + amount: 4.5, order: order, state: "closed") + } + let!(:admin_adjustment) { + create(:adjustment, adjustable: order, originator: nil, + amount: 6.7, order: order, state: "closed") } - it "returns a sum of the tax included in all enterprise fees" do - expect(order.reload.enterprise_fee_tax).to eq(12) + it "returns a sum of all taxes on enterprise fees" do + expect(order.reload.enterprise_fee_tax).to eq(16.8) end end describe "getting the total tax" do let(:shipping_tax_rate) { create(:tax_rate, amount: 0.25) } + let(:fee_tax_rate) { create(:tax_rate, amount: 0.10) } let(:order) { create(:order) } let(:shipping_method) { create(:shipping_method_with, :flat_rate) } let!(:shipment) do create(:shipment_with, :shipping_method, shipping_method: shipping_method, order: order) end let(:enterprise_fee) { create(:enterprise_fee) } - - before do - create(:adjustment, adjustable: order, originator: enterprise_fee, label: "EF", amount: 123, - included_tax: 2, order: order) + let!(:fee) { + create(:adjustment, adjustable: order, originator: enterprise_fee, label: "EF", amount: 20, + order: order) + } + let!(:fee_tax) { + create(:adjustment, adjustable: fee, originator: fee_tax_rate, + amount: 2, order: order, state: "closed") + } + let!(:shipping_tax) { create(:adjustment, adjustable: shipment, originator: shipping_tax_rate, amount: 10, order: order, state: "closed") + } + + before do order.update_order! end diff --git a/spec/models/spree/tax_rate_spec.rb b/spec/models/spree/tax_rate_spec.rb index fa7c5c9f9a..e057373256 100644 --- a/spec/models/spree/tax_rate_spec.rb +++ b/spec/models/spree/tax_rate_spec.rb @@ -4,7 +4,7 @@ require 'spec_helper' module Spree describe TaxRate do - describe "selecting tax rates to apply to an order" do + describe "#match" do let!(:zone) { create(:zone_with_member) } let!(:order) { create(:order, distributor: hub, bill_address: create(:address)) } let!(:tax_rate) { create(:tax_rate, included_in_price: true, calculator: ::Calculator::FlatRate.new(preferred_amount: 0.1), zone: zone) } @@ -70,5 +70,383 @@ module Spree expect(tax_rate.calculator.calculable.included_in_price).to be false end end + + context "original Spree::TaxRate specs" do + context "match" do + let(:order) { create(:order) } + let(:country) { create(:country) } + let(:tax_category) { create(:tax_category) } + let(:calculator) { ::Calculator::FlatRate.new } + + it "should return an empty array when tax_zone is nil" do + allow(order).to receive(:tax_zone) { nil } + expect(Spree::TaxRate.match(order)).to eq [] + end + + context "when no rate zones match the tax zone" do + before do + Spree::TaxRate.create(amount: 1, zone: create(:zone)) + end + + context "when there is no default tax zone" do + before do + @zone = create(:zone, name: "Country Zone", default_tax: false, zone_members: []) + @zone.zone_members.create(zoneable: country) + end + + it "should return an empty array" do + order.stub tax_zone: @zone + expect(Spree::TaxRate.match(order)).to eq [] + end + + it "should return the rate that matches the rate zone" do + rate = Spree::TaxRate.create( + amount: 1, + zone: @zone, + tax_category: tax_category, + calculator: calculator + ) + + order.stub tax_zone: @zone + expect(Spree::TaxRate.match(order)).to eq [rate] + end + + it "should return all rates that match the rate zone" do + rate1 = Spree::TaxRate.create( + amount: 1, + zone: @zone, + tax_category: tax_category, + calculator: calculator + ) + + rate2 = Spree::TaxRate.create( + amount: 2, + zone: @zone, + tax_category: tax_category, + calculator: ::Calculator::FlatRate.new + ) + + order.stub tax_zone: @zone + expect(Spree::TaxRate.match(order)).to eq [rate1, rate2] + end + + context "when the tax_zone is contained within a rate zone" do + before do + sub_zone = create(:zone, name: "State Zone", zone_members: []) + sub_zone.zone_members.create(zoneable: create(:state, country: country)) + order.stub tax_zone: sub_zone + @rate = Spree::TaxRate.create( + amount: 1, + zone: @zone, + tax_category: tax_category, + calculator: calculator + ) + end + + it "should return the rate zone" do + expect(Spree::TaxRate.match(order)).to eq [@rate] + end + end + end + + context "when there is a default tax zone" do + before do + @zone = create(:zone, name: "Country Zone", default_tax: true, zone_members: []) + @zone.zone_members.create(zoneable: country) + end + + let(:included_in_price) { false } + let!(:rate) do + Spree::TaxRate.create(amount: 1, + zone: @zone, + tax_category: tax_category, + calculator: calculator, + included_in_price: included_in_price) + end + + subject { Spree::TaxRate.match(order) } + + context "when the order has the same tax zone" do + before do + order.stub tax_zone: @zone + order.stub billing_address: tax_address + end + + let(:tax_address) { build_stubbed(:address) } + + context "when the tax is not a VAT" do + it { is_expected.to eq [rate] } + end + + context "when the tax is a VAT" do + let(:included_in_price) { true } + it { is_expected.to eq [rate] } + end + end + + context "when the order has a different tax zone" do + let(:other_zone) { create(:zone, name: "Other Zone") } + + before do + allow(order).to receive(:tax_zone) { other_zone } + allow(order).to receive(:billing_address) { tax_address } + end + + context "when the order has a tax_address" do + let(:tax_address) { build_stubbed(: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 + end + end + end + + 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 + end + end + end + end + end + + context "adjust" do + let(:order) { create(:order) } + let(:tax_category_1) { build_stubbed(:tax_category) } + let(:tax_category_2) { build_stubbed(:tax_category) } + let(:rate_1) { build_stubbed(:tax_rate, tax_category: tax_category_1) } + let(:rate_2) { build_stubbed(:tax_rate, tax_category: tax_category_2) } + let(:line_items) { [build_stubbed(:line_item)] } + + context "with line items" do + let(:line_item) { build_stubbed(:line_item, tax_category: tax_category_1) } + let(:line_items) { [line_item] } + + before do + allow(Spree::TaxRate).to receive(:match) { [rate_1, rate_2] } + end + + it "should apply adjustments for two tax rates to the order" do + expect(rate_1).to receive(:adjust) + expect(rate_2).to_not receive(:adjust) + Spree::TaxRate.adjust(order, line_items) + end + end + + context "with shipments" do + let(:shipment) { build_stubbed(:shipment, order: order) } + let(:shipments) { [shipment] } + + before do + allow(shipment).to receive(:tax_category) { tax_category_1 } + allow(Spree::TaxRate).to receive(:match) { [rate_1, rate_2] } + end + + it "should apply adjustments for two tax rates to the order" do + expect(rate_1).to receive(:adjust) + expect(rate_2).to_not receive(:adjust) + Spree::TaxRate.adjust(order, shipments) + end + end + end + + context "default" do + let(:tax_category) { create(:tax_category) } + let(:country) { create(:country) } + let(:calculator) { ::Calculator::FlatRate.new } + + context "when there is no default tax_category" do + before { tax_category.is_default = false } + + it "should return 0" do + expect(Spree::TaxRate.default).to eq 0 + end + end + + context "when there is a default tax_category" do + before { tax_category.update_column :is_default, true } + + context "when the default category has tax rates in the default tax zone" do + before(:each) do + allow(DefaultCountry).to receive(:id) { country.id } + @zone = create(:zone, name: "Country Zone", default_tax: true) + @zone.zone_members.create(zoneable: country) + rate = Spree::TaxRate.create( + amount: 1, + zone: @zone, + tax_category: tax_category, + calculator: calculator + ) + end + + it "should return the correct tax_rate" do + expect(Spree::TaxRate.default.to_f).to eq 1.0 + end + end + + context "when the default category has no tax rates in the default tax zone" do + it "should return 0" do + expect(Spree::TaxRate.default).to eq 0 + end + 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: ::Calculator::DefaultTax.new, + tax_category: @category, + zone: @zone + ) + @rate2 = Spree::TaxRate.create( + amount: 0.05, + calculator: ::Calculator::DefaultTax.new, + 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.variants.first, 1) } + + it "should not create a tax adjustment" do + Spree::TaxRate.adjust(@order, @order.line_items) + expect(line_item.adjustments.tax.charge.count).to eq 0 + end + + it "should not create a refund" do + Spree::TaxRate.adjust(@order, @order.line_items) + expect(line_item.adjustments.credit.count).to eq 0 + end + end + + context "taxable line item" do + let!(:line_item) { @order.contents.add(@taxable.variants.first, 1) } + + before do + @rate1.update_column(:included_in_price, true) + @rate2.update_column(:included_in_price, true) + end + + context "when price includes tax" do + context "when zone is contained by default tax zone" do + it "should create two adjustments, one for each tax rate" do + Spree::TaxRate.adjust(@order, @order.line_items) + expect(line_item.adjustments.count).to eq 2 + end + + it "should not create a tax refund" do + Spree::TaxRate.adjust(@order, @order.line_items) + expect(line_item.adjustments.credit.count).to eq 0 + 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 + before do + # With no zone members, this zone will not contain anything + @zone.zone_members.delete_all + end + + it "should create an adjustment" do + Spree::TaxRate.adjust(@order, @order.line_items) + expect(line_item.adjustments.charge.count).to eq 2 + end + + it "should not create a tax refund for each tax rate" do + Spree::TaxRate.adjust(@order, @order.line_items) + expect(line_item.adjustments.credit.count).to eq 0 + end + 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) + @new_state = create(:state, country: @new_country) + @order.ship_address = create(:address, country: @new_country, state: @new_state) + @order.save + end + + it "should not create positive adjustments" do + Spree::TaxRate.adjust(@order, @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, @order.line_items) + expect(line_item.adjustments.credit.count).to eq 2 + end + end + + context "when price does not include tax" do + before do + allow(@order).to receive(:tax_zone) { @zone } + + [@rate1, @rate2].each do |rate| + rate.included_in_price = false + rate.zone = @zone + rate.save + end + 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 + + it "should create adjustments" do + expect(line_item.adjustments.count).to eq 2 + end + + it "should not create a tax refund" do + expect(line_item.adjustments.credit.count).to eq 0 + end + + it "should remove adjustments when tax_zone is removed" do + Spree::TaxRate.adjust(@order, @order.line_items) + expect(line_item.adjustments.count).to eq 2 + allow(@order).to receive(:tax_zone) { nil } + Spree::TaxRate.adjust(@order, @order.line_items) + expect(line_item.adjustments.count).to eq 0 + end + end + end + end + end end end diff --git a/spec/services/order_tax_adjustments_fetcher_spec.rb b/spec/services/order_tax_adjustments_fetcher_spec.rb index 866927d39f..0f0d1055b5 100644 --- a/spec/services/order_tax_adjustments_fetcher_spec.rb +++ b/spec/services/order_tax_adjustments_fetcher_spec.rb @@ -31,10 +31,17 @@ describe OrderTaxAdjustmentsFetcher do amount: 0.25, zone: zone) end + let(:tax_rate30) do + create(:tax_rate, included_in_price: false, + calculator: Calculator::DefaultTax.new, + amount: 0.30, + zone: zone) + end let(:tax_category10) { create(:tax_category, tax_rates: [tax_rate10]) } let(:tax_category15) { create(:tax_category, tax_rates: [tax_rate15]) } let(:tax_category20) { create(:tax_category, tax_rates: [tax_rate20]) } let(:tax_category25) { create(:tax_category, tax_rates: [tax_rate25]) } + let(:tax_category30) { create(:tax_category, tax_rates: [tax_rate30]) } let(:variant) do create(:variant, product: create(:product, tax_category: tax_category10)) @@ -73,18 +80,23 @@ describe OrderTaxAdjustmentsFetcher do let!(:shipment) do create(:shipment_with, :shipping_method, shipping_method: shipping_method, order: order) end + let(:legacy_tax_adjustment) do + create(:adjustment, order: order, adjustable: order, amount: 1.23, originator: tax_rate30, + label: "Additional Tax Adjustment", state: "closed") + end before do order.reload order.adjustments << admin_adjustment - order.create_tax_charge! order.recreate_all_fees! + order.create_tax_charge! + legacy_tax_adjustment end subject { OrderTaxAdjustmentsFetcher.new(order).totals } - it "returns a hash with all 4 taxes" do - expect(subject.size).to eq(4) + it "returns a hash with all 5 taxes" do + expect(subject.size).to eq(5) end it "contains tax on all line_items" do @@ -102,5 +114,9 @@ describe OrderTaxAdjustmentsFetcher do it "contains tax on admin adjustment" do expect(subject[tax_rate25]).to eq(10.0) end + + it "contains (legacy) additional taxes recorded on the order" do + expect(subject[tax_rate30]).to eq(1.23) + end end end diff --git a/spec/services/paypal_items_builder_spec.rb b/spec/services/paypal_items_builder_spec.rb index db0b0cef8b..767c1f460f 100644 --- a/spec/services/paypal_items_builder_spec.rb +++ b/spec/services/paypal_items_builder_spec.rb @@ -43,7 +43,7 @@ describe PaypalItemsBuilder do originator: included_tax_rate, included: true, state: "closed") } let!(:additional_tax_adjustment) { - create(:adjustment, label: "Additional Tax Adjustment", order: order, adjustable: order, + create(:adjustment, label: "Additional Tax Adjustment", order: order, adjustable: order.shipment, amount: 78, originator: additional_tax_rate, state: "closed") } let!(:enterprise_fee) { create(:enterprise_fee) }