diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index dc22595479..1d843fdf10 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -656,6 +656,7 @@ Metrics/BlockNesting: Metrics/ClassLength: Exclude: - 'app/components/products_table_component.rb' + - 'app/controllers/admin/enterprise_fees_controller.rb' - 'app/controllers/admin/enterprises_controller.rb' - 'app/controllers/admin/order_cycles_controller.rb' - 'app/controllers/admin/resource_controller.rb' diff --git a/app/controllers/admin/enterprise_fees_controller.rb b/app/controllers/admin/enterprise_fees_controller.rb index 8bd4793c5b..3281b67317 100644 --- a/app/controllers/admin/enterprise_fees_controller.rb +++ b/app/controllers/admin/enterprise_fees_controller.rb @@ -7,6 +7,7 @@ module Admin before_action :load_enterprise_fee_set, only: :index before_action :load_data before_action :check_enterprise_fee_input, only: [:bulk_update] + before_action :check_calculators_compatibility_with_taxes, only: [:bulk_update] def index @include_calculators = params[:include_calculators].present? @@ -119,5 +120,17 @@ module Admin end end end + + def check_calculators_compatibility_with_taxes + enterprise_fee_bulk_params['collection_attributes'].each do |_, enterprise_fee| + next unless enterprise_fee['inherits_tax_category'] == "true" + next unless EnterpriseFee::PER_ORDER_CALCULATORS.include?(enterprise_fee['calculator_type']) + + flash[:error] = I18n.t( + 'activerecord.errors.models.enterprise_fee.inherit_tax_requires_per_item_calculator' + ) + return redirect_to redirect_path + end + end end end diff --git a/app/models/enterprise_fee.rb b/app/models/enterprise_fee.rb index bac889327b..598035abed 100644 --- a/app/models/enterprise_fee.rb +++ b/app/models/enterprise_fee.rb @@ -58,6 +58,12 @@ class EnterpriseFee < ApplicationRecord elsif inherits_tax_category_changed? self.tax_category_id = nil if inherits_tax_category? end + + if inherits_tax_category? && PER_ORDER_CALCULATORS.include?(calculator_type) + errors.add(:base, :inherit_tax_requires_per_item_calculator) + throw :abort + end + true end end diff --git a/config/locales/en.yml b/config/locales/en.yml index 6701e895da..8e20e22529 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -92,6 +92,8 @@ en: preferred_per_unit: "Calculator Per Unit:" errors: models: + enterprise_fee: + inherit_tax_requires_per_item_calculator: "Inheriting the tax categeory requires a per-item calculator." spree/user: attributes: email: @@ -113,7 +115,6 @@ en: using_producer_stock_settings_but_count_on_hand_set: "must be blank because using producer stock settings" on_demand_but_count_on_hand_set: "must be blank if on demand" limited_stock_but_no_count_on_hand: "must be specified because forcing limited stock" - # Used by active_storage_validations errors: messages: diff --git a/db/migrate/20230316034707_disable_inherits_tax_category_from_per_order_enterprise_fees.rb b/db/migrate/20230316034707_disable_inherits_tax_category_from_per_order_enterprise_fees.rb new file mode 100644 index 0000000000..07b1e8bf21 --- /dev/null +++ b/db/migrate/20230316034707_disable_inherits_tax_category_from_per_order_enterprise_fees.rb @@ -0,0 +1,29 @@ +# frozen_string_literal: true + +class DisableInheritsTaxCategoryFromPerOrderEnterpriseFees < ActiveRecord::Migration[6.1] + class EnterpriseFee < ApplicationRecord + has_many :calculator, class_name: "Spree::Calculator", foreign_key: :calculable_id + end + + module Spree + class Calculator < ApplicationRecord + self.table_name = 'spree_calculators' + end + end + + def change + EnterpriseFee.joins(:calculator).merge(calculators) + .where(inherits_tax_category: true) + .update_all(inherits_tax_category: false) + end + + def calculators + Spree::Calculator.where(type: per_order_calculators, calculable_type: 'EnterpriseFee') + end + + def per_order_calculators + ['Calculator::FlatRate', + 'Calculator::FlexiRate', + 'Calculator::PriceSack'] + end +end diff --git a/spec/lib/reports/enterprise_fee_summary/enterprise_fee_summary_report_spec.rb b/spec/lib/reports/enterprise_fee_summary/enterprise_fee_summary_report_spec.rb index f36faea77e..58711f74ba 100644 --- a/spec/lib/reports/enterprise_fee_summary/enterprise_fee_summary_report_spec.rb +++ b/spec/lib/reports/enterprise_fee_summary/enterprise_fee_summary_report_spec.rb @@ -319,9 +319,9 @@ describe Reporting::Reports::EnterpriseFeeSummary::Base do amount: 15) end let!(:coordinator_fee_inheriting_product_tax_category) do - create(:enterprise_fee, :flat_rate, name: "Coordinator Fee B", enterprise: coordinator, - fee_type: "admin", inherits_tax_category: true, - amount: 20) + create(:enterprise_fee, :per_item, name: "Coordinator Fee B", enterprise: coordinator, + fee_type: "admin", inherits_tax_category: true, + amount: 20) end let!(:coordinator_fee_without_tax) do create(:enterprise_fee, :flat_rate, name: "Coordinator Fee C", enterprise: coordinator, diff --git a/spec/models/enterprise_fee_spec.rb b/spec/models/enterprise_fee_spec.rb index 2eeb2ae027..ab6f5eda53 100644 --- a/spec/models/enterprise_fee_spec.rb +++ b/spec/models/enterprise_fee_spec.rb @@ -9,6 +9,48 @@ describe EnterpriseFee do describe "validations" do it { is_expected.to validate_presence_of(:name) } + + describe "requires a per-item calculator to inherit tax" do + let(:per_order_calculators){ + [ + Calculator::FlatRate, + Calculator::FlexiRate, + Calculator::PriceSack, + ] + } + + let(:per_item_calculators){ + [ + Calculator::PerItem, + Calculator::FlatPercentPerItem + ] + } + + it "is valid when inheriting tax and using a per-item calculator" do + per_item_calculators.each do |calculator| + subject = build( + :enterprise_fee, + inherits_tax_category: true, + calculator: calculator.new + ) + expect(subject.save).to eq true + end + end + + it "is invalid when inheriting tax and using a per-order calculator" do + per_order_calculators.each do |calculator| + subject = build( + :enterprise_fee, + inherits_tax_category: true, + calculator: calculator.new + ) + expect(subject.save).to eq false + expect(subject.errors.full_messages.first).to eq( + "Inheriting the tax categeory requires a per-item calculator." + ) + end + end + end end describe "callbacks" do diff --git a/spec/models/spree/adjustment_spec.rb b/spec/models/spree/adjustment_spec.rb index 66d243fd59..9bb4baa06c 100644 --- a/spec/models/spree/adjustment_spec.rb +++ b/spec/models/spree/adjustment_spec.rb @@ -436,7 +436,7 @@ module Spree context "when enterprise fees are taxed per-order" do let(:enterprise_fee) { - create(:enterprise_fee, enterprise: coordinator, inherits_tax_category: true, + create(:enterprise_fee, enterprise: coordinator, inherits_tax_category: false, calculator: ::Calculator::FlatRate.new(preferred_amount: 50.0)) }