From 25b7f1749c567cc2948fefc1c3effa8a1d9fd332 Mon Sep 17 00:00:00 2001
From: Maikel Linke <maikel@email.org.au>
Date: Mon, 6 Mar 2023 14:37:25 +1100
Subject: [PATCH] Simplify error when tax rate can't be inherited

I changed the text to focus on the resolution: the user needs to choose
a tax category or a different calculator.

I associated the error to the model to prevent the attribute name from
being included in the error message. Alternatively, we could have
changed the name of the attribute to match the UI. But this error
affects the combination of two attributes, none of them is invalid on
its own.

I'm using Rails' default lazy lookup for error messages which results in
shorter code and a standard structure.

I also added a simple spec.
---
 app/models/enterprise_fee.rb       |  4 +---
 config/locales/en.yml              |  4 ++--
 spec/models/enterprise_fee_spec.rb | 13 +++++++++++++
 3 files changed, 16 insertions(+), 5 deletions(-)

diff --git a/app/models/enterprise_fee.rb b/app/models/enterprise_fee.rb
index 9acfc31134..598035abed 100644
--- a/app/models/enterprise_fee.rb
+++ b/app/models/enterprise_fee.rb
@@ -60,9 +60,7 @@ class EnterpriseFee < ApplicationRecord
     end
 
     if inherits_tax_category? && PER_ORDER_CALCULATORS.include?(calculator_type)
-      errors.add(:inherits_tax_category,
-                 I18n.t("activerecord.errors.models." \
-                  "enterpise_fee.cannot_inherit_from_product_when_per_order_calculator_selected"))
+      errors.add(:base, :inherit_tax_requires_per_item_calculator)
       throw :abort
     end
 
diff --git a/config/locales/en.yml b/config/locales/en.yml
index e991f46884..aaea8d1f1b 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,8 +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"
-        enterpise_fee:
-          cannot_inherit_from_product_when_per_order_calculator_selected: "You cannot select 'Inherit From Product' when a per order calculator is selected"
   # Used by active_storage_validations
   errors:
     messages:
diff --git a/spec/models/enterprise_fee_spec.rb b/spec/models/enterprise_fee_spec.rb
index 2eeb2ae027..6d579a42dd 100644
--- a/spec/models/enterprise_fee_spec.rb
+++ b/spec/models/enterprise_fee_spec.rb
@@ -9,6 +9,19 @@ describe EnterpriseFee do
 
   describe "validations" do
     it { is_expected.to validate_presence_of(:name) }
+
+    it "requires a per-item calculator to inherit tax" do
+      subject = build(
+        :enterprise_fee,
+        inherits_tax_category: true,
+        calculator: Calculator::FlatRate.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
 
   describe "callbacks" do