From a46eef291c0594bab9c10fb8fdfd72beaec3ce25 Mon Sep 17 00:00:00 2001 From: David Cook Date: Fri, 28 Apr 2023 11:59:47 +1000 Subject: [PATCH] Remove localized number logic from calculators The browser is now responsible for dealing with the decimal separator, for all numeric preferences (as input[type=number]; see Spree::Admin::BaseHelper::preference_field_tag). Calculators are the only place that numeric preferences are used. It will enforce that only one comma or dot (depending on user's locale) is entered, thus avoiding any ambiguity, and mis-interpretation (eg 100,001 could be interpreted as more than 100 thousand or 100 with a decimal place). --- app/models/calculator/flat_percent_item_total.rb | 9 +-------- app/models/calculator/flat_percent_per_item.rb | 9 +-------- app/models/calculator/flat_rate.rb | 9 +-------- app/models/calculator/flexi_rate.rb | 10 +--------- app/models/calculator/per_item.rb | 9 +-------- app/models/calculator/price_sack.rb | 11 +---------- .../models/calculator/flat_percent_item_total_spec.rb | 6 ------ spec/models/calculator/flat_percent_per_item_spec.rb | 6 ------ spec/models/calculator/flat_rate_spec.rb | 5 ----- spec/models/calculator/flexi_rate_spec.rb | 7 ------- spec/models/calculator/per_item_spec.rb | 5 ----- spec/models/calculator/price_sack_spec.rb | 9 --------- 12 files changed, 6 insertions(+), 89 deletions(-) diff --git a/app/models/calculator/flat_percent_item_total.rb b/app/models/calculator/flat_percent_item_total.rb index f43cae017d..83a5674833 100644 --- a/app/models/calculator/flat_percent_item_total.rb +++ b/app/models/calculator/flat_percent_item_total.rb @@ -1,18 +1,11 @@ # frozen_string_literal: false -require 'spree/localized_number' - module Calculator class FlatPercentItemTotal < Spree::Calculator - extend Spree::LocalizedNumber - preference :flat_percent, :decimal, default: 0 - localize_number :preferred_flat_percent - validates :preferred_flat_percent, - numericality: { message: :calculator_preferred_value_error }, - unless: -> { Spree::Config.enable_localized_number? } + numericality: { message: :calculator_preferred_value_error } def self.description Spree.t(:flat_percent) diff --git a/app/models/calculator/flat_percent_per_item.rb b/app/models/calculator/flat_percent_per_item.rb index c6a8693fd0..b1b6b2303b 100644 --- a/app/models/calculator/flat_percent_per_item.rb +++ b/app/models/calculator/flat_percent_per_item.rb @@ -1,22 +1,15 @@ # frozen_string_literal: true -require 'spree/localized_number' - class Calculator::FlatPercentPerItem < Spree::Calculator # Spree's FlatPercentItemTotal calculator sums all amounts, and then calculates a percentage # on them. # In the cart, we display line item individual amounts rounded, so to have consistent # calculations we do the same internally. Here, we round adjustments at the individual # item level first, then multiply by the item quantity. - extend Spree::LocalizedNumber - preference :flat_percent, :decimal, default: 0 - localize_number :preferred_flat_percent - validates :preferred_flat_percent, - numericality: { message: :calculator_preferred_value_error }, - unless: -> { Spree::Config.enable_localized_number? } + numericality: { message: :calculator_preferred_value_error } def self.description I18n.t(:flat_percent_per_item) diff --git a/app/models/calculator/flat_rate.rb b/app/models/calculator/flat_rate.rb index 0cdb2b6fa4..afcdc80b8d 100644 --- a/app/models/calculator/flat_rate.rb +++ b/app/models/calculator/flat_rate.rb @@ -1,18 +1,11 @@ # frozen_string_literal: false -require 'spree/localized_number' - module Calculator class FlatRate < Spree::Calculator - extend Spree::LocalizedNumber - preference :amount, :decimal, default: 0 - localize_number :preferred_amount - validates :preferred_amount, - numericality: { message: :calculator_preferred_value_error }, - unless: -> { Spree::Config.enable_localized_number? } + numericality: { message: :calculator_preferred_value_error } def self.description I18n.t(:flat_rate_per_order) diff --git a/app/models/calculator/flexi_rate.rb b/app/models/calculator/flexi_rate.rb index 8a400088b0..40556ef581 100644 --- a/app/models/calculator/flexi_rate.rb +++ b/app/models/calculator/flexi_rate.rb @@ -1,22 +1,14 @@ # frozen_string_literal: false -require 'spree/localized_number' - module Calculator class FlexiRate < Spree::Calculator - extend Spree::LocalizedNumber - preference :first_item, :decimal, default: 0.0 preference :additional_item, :decimal, default: 0.0 preference :max_items, :integer, default: 0 - localize_number :preferred_first_item, - :preferred_additional_item - validates :preferred_first_item, :preferred_additional_item, - numericality: { message: :calculator_preferred_value_error }, - unless: -> { Spree::Config.enable_localized_number? } + numericality: { message: :calculator_preferred_value_error } def self.description I18n.t(:flexible_rate) diff --git a/app/models/calculator/per_item.rb b/app/models/calculator/per_item.rb index 8458f4d432..c0525fd384 100644 --- a/app/models/calculator/per_item.rb +++ b/app/models/calculator/per_item.rb @@ -1,18 +1,11 @@ # frozen_string_literal: false -require 'spree/localized_number' - module Calculator class PerItem < Spree::Calculator - extend Spree::LocalizedNumber - preference :amount, :decimal, default: 0 - localize_number :preferred_amount - validates :preferred_amount, - numericality: { message: :calculator_preferred_value_error }, - unless: -> { Spree::Config.enable_localized_number? } + numericality: { message: :calculator_preferred_value_error } def self.description I18n.t(:flat_rate_per_item) diff --git a/app/models/calculator/price_sack.rb b/app/models/calculator/price_sack.rb index 30b02be331..86ada80dde 100644 --- a/app/models/calculator/price_sack.rb +++ b/app/models/calculator/price_sack.rb @@ -1,24 +1,15 @@ # frozen_string_literal: false -require 'spree/localized_number' - module Calculator class PriceSack < Spree::Calculator - extend Spree::LocalizedNumber - preference :minimal_amount, :decimal, default: 0 preference :normal_amount, :decimal, default: 0 preference :discount_amount, :decimal, default: 0 - localize_number :preferred_minimal_amount, - :preferred_normal_amount, - :preferred_discount_amount - validates :preferred_minimal_amount, :preferred_normal_amount, :preferred_discount_amount, - numericality: { message: :calculator_preferred_value_error }, - unless: -> { Spree::Config.enable_localized_number? } + numericality: { message: :calculator_preferred_value_error } def self.description I18n.t(:price_sack) diff --git a/spec/models/calculator/flat_percent_item_total_spec.rb b/spec/models/calculator/flat_percent_item_total_spec.rb index afcd2ea23a..c9ad3865c9 100644 --- a/spec/models/calculator/flat_percent_item_total_spec.rb +++ b/spec/models/calculator/flat_percent_item_total_spec.rb @@ -17,12 +17,6 @@ describe Calculator::FlatPercentItemTotal do expect(calculator.compute(line_item)).to eq(1.0) end - context "extends LocalizedNumber" do - it_behaves_like "a model using the LocalizedNumber module", [:preferred_flat_percent] - it_behaves_like "a Spree Calculator model using the LocalizedNumber module", - [:preferred_flat_percent] - end - it "computes amount correctly for a given OrderManagement::Stock::Package" do order = double(:order, line_items: [line_item] ) package = double(:package, order: order) diff --git a/spec/models/calculator/flat_percent_per_item_spec.rb b/spec/models/calculator/flat_percent_per_item_spec.rb index 8758cb7e95..b95e08d0ab 100644 --- a/spec/models/calculator/flat_percent_per_item_spec.rb +++ b/spec/models/calculator/flat_percent_per_item_spec.rb @@ -19,10 +19,4 @@ describe Calculator::FlatPercentPerItem do line_item = Spree::LineItem.new price: 0.86, quantity: 8 expect(calculator.compute(line_item)).to eq 1.36 end - - context "extends LocalizedNumber" do - it_behaves_like "a model using the LocalizedNumber module", [:preferred_flat_percent] - it_behaves_like "a Spree Calculator model using the LocalizedNumber module", - [:preferred_flat_percent] - end end diff --git a/spec/models/calculator/flat_rate_spec.rb b/spec/models/calculator/flat_rate_spec.rb index de8b999d9e..20ca13ec69 100644 --- a/spec/models/calculator/flat_rate_spec.rb +++ b/spec/models/calculator/flat_rate_spec.rb @@ -11,9 +11,4 @@ describe Calculator::FlatRate do should validate_numericality_of(:preferred_amount). with_message("Invalid input. Please use only numbers. For example: 10, 5.5, -20") end - - context "extends LocalizedNumber" do - it_behaves_like "a model using the LocalizedNumber module", [:preferred_amount] - it_behaves_like "a Spree Calculator model using the LocalizedNumber module", [:preferred_amount] - end end diff --git a/spec/models/calculator/flexi_rate_spec.rb b/spec/models/calculator/flexi_rate_spec.rb index f5a45f3b72..5224f5a408 100644 --- a/spec/models/calculator/flexi_rate_spec.rb +++ b/spec/models/calculator/flexi_rate_spec.rb @@ -42,11 +42,4 @@ describe Calculator::FlexiRate do Calculator::FlexiRate.new(preferred_first_item: 1, preferred_additional_item: 1, preferred_max_items: 1) end - - context "extends LocalizedNumber" do - it_behaves_like "a model using the LocalizedNumber module", - [:preferred_first_item, :preferred_additional_item] - it_behaves_like "a Spree Calculator model using the LocalizedNumber module", - [:preferred_first_item, :preferred_additional_item] - end end diff --git a/spec/models/calculator/per_item_spec.rb b/spec/models/calculator/per_item_spec.rb index c372c91f82..4b0f72ecd8 100644 --- a/spec/models/calculator/per_item_spec.rb +++ b/spec/models/calculator/per_item_spec.rb @@ -16,9 +16,4 @@ describe Calculator::PerItem do allow(calculator).to receive_messages(calculable: shipping_calculable) expect(calculator.compute(line_item).to_f).to eq(50) # 5 x 10 end - - context "extends LocalizedNumber" do - it_behaves_like "a model using the LocalizedNumber module", [:preferred_amount] - it_behaves_like "a Spree Calculator model using the LocalizedNumber module", [:preferred_amount] - end end diff --git a/spec/models/calculator/price_sack_spec.rb b/spec/models/calculator/price_sack_spec.rb index 966ee25248..0754848035 100644 --- a/spec/models/calculator/price_sack_spec.rb +++ b/spec/models/calculator/price_sack_spec.rb @@ -90,13 +90,4 @@ describe Calculator::PriceSack do end end end - - context "extends LocalizedNumber" do - it_behaves_like "a model using the LocalizedNumber module", - [:preferred_minimal_amount, :preferred_normal_amount, - :preferred_discount_amount] - it_behaves_like "a Spree Calculator model using the LocalizedNumber module", - [:preferred_minimal_amount, :preferred_normal_amount, - :preferred_discount_amount] - end end