From f73e5c74fbc3fa54b5eb8877f237afa1f0da4d60 Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Bellet Date: Tue, 1 Jun 2021 11:40:02 +0200 Subject: [PATCH 1/7] Handle null/undefined cases for price --- .../admin/utils/filters/unlocalize_currency.js.coffee | 2 ++ .../unit/admin/filters/unlocalize_currency_spec.js.coffee | 7 +++++++ 2 files changed, 9 insertions(+) diff --git a/app/assets/javascripts/admin/utils/filters/unlocalize_currency.js.coffee b/app/assets/javascripts/admin/utils/filters/unlocalize_currency.js.coffee index 1d176db9ac..c6eca566ef 100644 --- a/app/assets/javascripts/admin/utils/filters/unlocalize_currency.js.coffee +++ b/app/assets/javascripts/admin/utils/filters/unlocalize_currency.js.coffee @@ -1,6 +1,8 @@ angular.module("admin.utils").filter "unlocalizeCurrency", ()-> # Convert string to number using injected currency configuration. (price) -> + if (!price) + return null # used decimal and thousands separators from currency configuration decimal_separator = I18n.toCurrency(.1, {precision: 1, unit: ''}).substring(1,2) thousands_separator = I18n.toCurrency(1000, {precision: 1, unit: ''}).substring(1,2) diff --git a/spec/javascripts/unit/admin/filters/unlocalize_currency_spec.js.coffee b/spec/javascripts/unit/admin/filters/unlocalize_currency_spec.js.coffee index a21db8d05f..7bba962c66 100644 --- a/spec/javascripts/unit/admin/filters/unlocalize_currency_spec.js.coffee +++ b/spec/javascripts/unit/admin/filters/unlocalize_currency_spec.js.coffee @@ -89,3 +89,10 @@ describe 'convert string to number with configurated currency', -> it "handle integer number with no thousands separator", -> expect(filter("1000")).toEqual 1000 + + describe "handle null/undefined case", -> + it "null case", -> + expect(filter(null)).toEqual null + + it "undefined case ", -> + expect(filter(undefined)).toEqual null From 3ebba9502ae16daf40c10eda31fddc3244f457f5 Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Bellet Date: Mon, 7 Jun 2021 16:53:20 +0200 Subject: [PATCH 2/7] Handle more cases with decimal/thousands separator - ',' or '.' can be used as decimal separator (defined in the application configuration) - Remove thousands separator if it's detected as so (use regexp to match) --- .../filters/unlocalize_currency.js.coffee | 10 ++++-- .../unlocalize_currency_spec.js.coffee | 33 +++++++++++++++++++ 2 files changed, 40 insertions(+), 3 deletions(-) diff --git a/app/assets/javascripts/admin/utils/filters/unlocalize_currency.js.coffee b/app/assets/javascripts/admin/utils/filters/unlocalize_currency.js.coffee index c6eca566ef..c8fe429bc3 100644 --- a/app/assets/javascripts/admin/utils/filters/unlocalize_currency.js.coffee +++ b/app/assets/javascripts/admin/utils/filters/unlocalize_currency.js.coffee @@ -6,11 +6,15 @@ angular.module("admin.utils").filter "unlocalizeCurrency", ()-> # used decimal and thousands separators from currency configuration decimal_separator = I18n.toCurrency(.1, {precision: 1, unit: ''}).substring(1,2) thousands_separator = I18n.toCurrency(1000, {precision: 1, unit: ''}).substring(1,2) + + # Replace comma used as a decimal separator and remplace by "." + if (price.match(/^[0-9]*(,{1})[0-9]{1,2}$/g)) + price = price.replace(",", ".") - if (price.length > 4) - # remove configured thousands separator if price is greater than 999 + # Remove configured thousands separator if it is actually a thousands separator + if (new RegExp("^([0-9]*(" + thousands_separator + "{1})[0-9]{3}[0-9\.,]*)*$", "g").test(price)) price = price.replaceAll(thousands_separator, '') - + if (decimal_separator == ",") price = price.replace(",", ".") diff --git a/spec/javascripts/unit/admin/filters/unlocalize_currency_spec.js.coffee b/spec/javascripts/unit/admin/filters/unlocalize_currency_spec.js.coffee index 7bba962c66..4b60c20de3 100644 --- a/spec/javascripts/unit/admin/filters/unlocalize_currency_spec.js.coffee +++ b/spec/javascripts/unit/admin/filters/unlocalize_currency_spec.js.coffee @@ -21,9 +21,18 @@ describe 'convert string to number with configurated currency', -> it "handle point as decimal separator", -> expect(filter("1.000")).toEqual 1.0 + it "also handle comma as decimal separator", -> + expect(filter("1,0")).toEqual 1.0 + it "also handle comma as decimal separator", -> expect(filter("1,00")).toEqual 1.0 + it "also handle comma as decimal separator", -> + expect(filter("11,00")).toEqual 11.0 + + it "handle comma as decimal separator but not confusing with thousands separator", -> + expect(filter("11,000")).toEqual 11000 + it "handle point as decimal separator and comma as thousands separator", -> expect(filter("1,000,000.00")).toEqual 1000000 @@ -47,10 +56,22 @@ describe 'convert string to number with configurated currency', -> it "handle comma as decimal separator", -> expect(filter("1,00")).toEqual 1.0 + + it "handle comma as decimal separator with one digit after the comma", -> + expect(filter("11,0")).toEqual 11.0 + + it "handle comma as decimal separator with two digit after the comma", -> + expect(filter("11,00")).toEqual 11.0 + + it "handle comma as decimal separator with three digit after the comma", -> + expect(filter("11,000")).toEqual 11.0 it "also handle point as decimal separator", -> expect(filter("1.00")).toEqual 1.0 + it "also handle point as decimal separator with integer part with two digits", -> + expect(filter("11.00")).toEqual 11.0 + it "handle point as decimal separator and final point as thousands separator", -> expect(filter("1.000.000,00")).toEqual 1000000 @@ -75,9 +96,21 @@ describe 'convert string to number with configurated currency', -> it "handle comma as decimal separator", -> expect(filter("1,00")).toEqual 1.0 + it "handle comma as decimal separator with one digit after the comma", -> + expect(filter("11,0")).toEqual 11.0 + + it "handle comma as decimal separator with two digit after the comma", -> + expect(filter("11,00")).toEqual 11.0 + + it "handle comma as decimal separator with three digit after the comma", -> + expect(filter("11,000")).toEqual 11.0 + it "also handle final point as decimal separator", -> expect(filter("1.00")).toEqual 1.0 + it "also handle final point as decimal separator with integer part with two digits", -> + expect(filter("11.00")).toEqual 11.0 + it "handle point as decimal separator and space as thousands separator", -> expect(filter("1 000 000,00")).toEqual 1000000 From 0cb2739139c0ec6f7b203b20cd10ff5d11488896 Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Bellet Date: Tue, 8 Jun 2021 09:46:24 +0200 Subject: [PATCH 3/7] Handle case when price is not a number And return a `null` if so. --- .../admin/utils/filters/unlocalize_currency.js.coffee | 7 ++++++- .../unit/admin/filters/unlocalize_currency_spec.js.coffee | 3 +++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/app/assets/javascripts/admin/utils/filters/unlocalize_currency.js.coffee b/app/assets/javascripts/admin/utils/filters/unlocalize_currency.js.coffee index c8fe429bc3..59f0513193 100644 --- a/app/assets/javascripts/admin/utils/filters/unlocalize_currency.js.coffee +++ b/app/assets/javascripts/admin/utils/filters/unlocalize_currency.js.coffee @@ -18,4 +18,9 @@ angular.module("admin.utils").filter "unlocalizeCurrency", ()-> if (decimal_separator == ",") price = price.replace(",", ".") - return parseFloat(price) + price = parseFloat(price) + + if (isNaN(price)) + return null + + return price diff --git a/spec/javascripts/unit/admin/filters/unlocalize_currency_spec.js.coffee b/spec/javascripts/unit/admin/filters/unlocalize_currency_spec.js.coffee index 4b60c20de3..8a92d99b5e 100644 --- a/spec/javascripts/unit/admin/filters/unlocalize_currency_spec.js.coffee +++ b/spec/javascripts/unit/admin/filters/unlocalize_currency_spec.js.coffee @@ -129,3 +129,6 @@ describe 'convert string to number with configurated currency', -> it "undefined case ", -> expect(filter(undefined)).toEqual null + + it "wtf case", -> + expect(filter("wtf")).toEqual null From d3c215812132cc6936c96ebd68abb760024b3137 Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Bellet Date: Tue, 8 Jun 2021 09:51:30 +0200 Subject: [PATCH 4/7] Handle comma as decimal separator in the unit value field - Add comma as a decimal separator in the regexp - Do not use parseFloat but our `unlocalizeCurrencyFilter` which is more tolerant --- .../controllers/units_controller.js.coffee | 6 +++--- .../products/units_controller_spec.js.coffee | 18 ++++++++++++++++++ 2 files changed, 21 insertions(+), 3 deletions(-) diff --git a/app/assets/javascripts/admin/products/controllers/units_controller.js.coffee b/app/assets/javascripts/admin/products/controllers/units_controller.js.coffee index df4cb42df4..77960b295c 100644 --- a/app/assets/javascripts/admin/products/controllers/units_controller.js.coffee +++ b/app/assets/javascripts/admin/products/controllers/units_controller.js.coffee @@ -1,5 +1,5 @@ angular.module("admin.products") - .controller "unitsCtrl", ($scope, VariantUnitManager, OptionValueNamer, UnitPrices) -> + .controller "unitsCtrl", ($scope, VariantUnitManager, OptionValueNamer, UnitPrices, unlocalizeCurrencyFilter) -> $scope.product = { master: {} } $scope.product.master.product = $scope.product $scope.placeholder_text = "" @@ -26,9 +26,9 @@ angular.module("admin.products") $scope.processUnitValueWithDescription = -> if $scope.product.master.hasOwnProperty("unit_value_with_description") - match = $scope.product.master.unit_value_with_description.match(/^([\d\.]+(?= *|$)|)( *)(.*)$/) + match = $scope.product.master.unit_value_with_description.match(/^([\d\.,]+(?= *|$)|)( *)(.*)$/) if match - $scope.product.master.unit_value = parseFloat(match[1]) + $scope.product.master.unit_value = unlocalizeCurrencyFilter(match[1]) $scope.product.master.unit_value = null if isNaN($scope.product.master.unit_value) $scope.product.master.unit_value *= $scope.product.variant_unit_scale if $scope.product.master.unit_value && $scope.product.variant_unit_scale $scope.product.master.unit_description = match[3] diff --git a/spec/javascripts/unit/admin/products/units_controller_spec.js.coffee b/spec/javascripts/unit/admin/products/units_controller_spec.js.coffee index 7e7ea03888..72046716cc 100644 --- a/spec/javascripts/unit/admin/products/units_controller_spec.js.coffee +++ b/spec/javascripts/unit/admin/products/units_controller_spec.js.coffee @@ -75,3 +75,21 @@ describe "unitsCtrl", -> scope.processUnitValueWithDescription() expect(scope.product.master.unit_value).toEqual 123 expect(scope.product.master.unit_description).toEqual "54 boxes" + + it "handle final point as decimal separator", -> + scope.product.master.unit_value_with_description = "22.22" + scope.processUnitValueWithDescription() + expect(scope.product.master.unit_value).toEqual 22.22 + expect(scope.product.master.unit_description).toEqual "" + + it "handle comma as decimal separator", -> + scope.product.master.unit_value_with_description = "22,22" + scope.processUnitValueWithDescription() + expect(scope.product.master.unit_value).toEqual 22.22 + expect(scope.product.master.unit_description).toEqual "" + + it "handle comma as decimal separator with description", -> + scope.product.master.unit_value_with_description = "22,22 things" + scope.processUnitValueWithDescription() + expect(scope.product.master.unit_value).toEqual 22.22 + expect(scope.product.master.unit_description).toEqual "things" From 6cf0c73453dcf11d4719730a8361c2403103f543 Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Bellet Date: Tue, 8 Jun 2021 10:55:43 +0200 Subject: [PATCH 5/7] Use our unlocalizeCurrencyFilter to parse unit value field - More tolerant (can handle `,` or `.` as decimal separator, remove thousands separator) to return a `number` --- .../products/controllers/variant_units_controller.js.coffee | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/assets/javascripts/admin/products/controllers/variant_units_controller.js.coffee b/app/assets/javascripts/admin/products/controllers/variant_units_controller.js.coffee index 33c01542ff..41327872dc 100644 --- a/app/assets/javascripts/admin/products/controllers/variant_units_controller.js.coffee +++ b/app/assets/javascripts/admin/products/controllers/variant_units_controller.js.coffee @@ -1,4 +1,4 @@ -angular.module("admin.products").controller "variantUnitsCtrl", ($scope, VariantUnitManager, $timeout, UnitPrices) -> +angular.module("admin.products").controller "variantUnitsCtrl", ($scope, VariantUnitManager, $timeout, UnitPrices, unlocalizeCurrencyFilter) -> $scope.unitName = (scale, type) -> VariantUnitManager.getUnitName(scale, type) @@ -23,7 +23,7 @@ angular.module("admin.products").controller "variantUnitsCtrl", ($scope, Variant $scope.updateValue = -> unit_value_human = angular.element('#unit_value_human').val() - $scope.unit_value = unit_value_human * $scope.scale + $scope.unit_value = unlocalizeCurrencyFilter(unit_value_human) * $scope.scale variant_unit_value = angular.element('#variant_unit_value').val() $scope.unit_value_human = variant_unit_value / $scope.scale From 178c0a441bba113dfdac83ad60e53f6ddfa2eba1 Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Bellet Date: Fri, 11 Jun 2021 10:01:19 +0200 Subject: [PATCH 6/7] Move unlocalize_currency filter to a PriceParser service - It's no longer a filter but more a service: it's therefore more logic. --- .../controllers/units_controller.js.coffee | 4 +- .../variant_units_controller.js.coffee | 4 +- .../products/services/unit_prices.js.coffee | 4 +- .../filters/unlocalize_currency.js.coffee | 26 ------- .../utils/services/price_parser.js.coffee | 24 ++++++ .../services/price_parser_spec.js.coffee} | 77 ++++++++++--------- 6 files changed, 69 insertions(+), 70 deletions(-) delete mode 100644 app/assets/javascripts/admin/utils/filters/unlocalize_currency.js.coffee create mode 100644 app/assets/javascripts/admin/utils/services/price_parser.js.coffee rename spec/javascripts/unit/admin/{filters/unlocalize_currency_spec.js.coffee => utils/services/price_parser_spec.js.coffee} (60%) diff --git a/app/assets/javascripts/admin/products/controllers/units_controller.js.coffee b/app/assets/javascripts/admin/products/controllers/units_controller.js.coffee index 77960b295c..89a7027e07 100644 --- a/app/assets/javascripts/admin/products/controllers/units_controller.js.coffee +++ b/app/assets/javascripts/admin/products/controllers/units_controller.js.coffee @@ -1,5 +1,5 @@ angular.module("admin.products") - .controller "unitsCtrl", ($scope, VariantUnitManager, OptionValueNamer, UnitPrices, unlocalizeCurrencyFilter) -> + .controller "unitsCtrl", ($scope, VariantUnitManager, OptionValueNamer, UnitPrices, PriceParser) -> $scope.product = { master: {} } $scope.product.master.product = $scope.product $scope.placeholder_text = "" @@ -28,7 +28,7 @@ angular.module("admin.products") if $scope.product.master.hasOwnProperty("unit_value_with_description") match = $scope.product.master.unit_value_with_description.match(/^([\d\.,]+(?= *|$)|)( *)(.*)$/) if match - $scope.product.master.unit_value = unlocalizeCurrencyFilter(match[1]) + $scope.product.master.unit_value = PriceParser.parse(match[1]) $scope.product.master.unit_value = null if isNaN($scope.product.master.unit_value) $scope.product.master.unit_value *= $scope.product.variant_unit_scale if $scope.product.master.unit_value && $scope.product.variant_unit_scale $scope.product.master.unit_description = match[3] diff --git a/app/assets/javascripts/admin/products/controllers/variant_units_controller.js.coffee b/app/assets/javascripts/admin/products/controllers/variant_units_controller.js.coffee index 41327872dc..39400c036e 100644 --- a/app/assets/javascripts/admin/products/controllers/variant_units_controller.js.coffee +++ b/app/assets/javascripts/admin/products/controllers/variant_units_controller.js.coffee @@ -1,4 +1,4 @@ -angular.module("admin.products").controller "variantUnitsCtrl", ($scope, VariantUnitManager, $timeout, UnitPrices, unlocalizeCurrencyFilter) -> +angular.module("admin.products").controller "variantUnitsCtrl", ($scope, VariantUnitManager, $timeout, UnitPrices, PriceParser) -> $scope.unitName = (scale, type) -> VariantUnitManager.getUnitName(scale, type) @@ -23,7 +23,7 @@ angular.module("admin.products").controller "variantUnitsCtrl", ($scope, Variant $scope.updateValue = -> unit_value_human = angular.element('#unit_value_human').val() - $scope.unit_value = unlocalizeCurrencyFilter(unit_value_human) * $scope.scale + $scope.unit_value = PriceParser.parse(unit_value_human) * $scope.scale variant_unit_value = angular.element('#variant_unit_value').val() $scope.unit_value_human = variant_unit_value / $scope.scale diff --git a/app/assets/javascripts/admin/products/services/unit_prices.js.coffee b/app/assets/javascripts/admin/products/services/unit_prices.js.coffee index a5cb4d67de..f5da0569f9 100644 --- a/app/assets/javascripts/admin/products/services/unit_prices.js.coffee +++ b/app/assets/javascripts/admin/products/services/unit_prices.js.coffee @@ -1,7 +1,7 @@ -angular.module("admin.products").factory "UnitPrices", (VariantUnitManager, localizeCurrencyFilter, unlocalizeCurrencyFilter) -> +angular.module("admin.products").factory "UnitPrices", (VariantUnitManager, localizeCurrencyFilter, PriceParser) -> class UnitPrices @displayableUnitPrice: (price, scale, unit_type, unit_value, variant_unit_name) -> - price = unlocalizeCurrencyFilter(price) + price = PriceParser.parse(price) if price && !isNaN(price) && unit_type && unit_value value = localizeCurrencyFilter(UnitPrices.price(price, scale, unit_type, unit_value, variant_unit_name)) unit = UnitPrices.unit(scale, unit_type, variant_unit_name) diff --git a/app/assets/javascripts/admin/utils/filters/unlocalize_currency.js.coffee b/app/assets/javascripts/admin/utils/filters/unlocalize_currency.js.coffee deleted file mode 100644 index 59f0513193..0000000000 --- a/app/assets/javascripts/admin/utils/filters/unlocalize_currency.js.coffee +++ /dev/null @@ -1,26 +0,0 @@ -angular.module("admin.utils").filter "unlocalizeCurrency", ()-> - # Convert string to number using injected currency configuration. - (price) -> - if (!price) - return null - # used decimal and thousands separators from currency configuration - decimal_separator = I18n.toCurrency(.1, {precision: 1, unit: ''}).substring(1,2) - thousands_separator = I18n.toCurrency(1000, {precision: 1, unit: ''}).substring(1,2) - - # Replace comma used as a decimal separator and remplace by "." - if (price.match(/^[0-9]*(,{1})[0-9]{1,2}$/g)) - price = price.replace(",", ".") - - # Remove configured thousands separator if it is actually a thousands separator - if (new RegExp("^([0-9]*(" + thousands_separator + "{1})[0-9]{3}[0-9\.,]*)*$", "g").test(price)) - price = price.replaceAll(thousands_separator, '') - - if (decimal_separator == ",") - price = price.replace(",", ".") - - price = parseFloat(price) - - if (isNaN(price)) - return null - - return price diff --git a/app/assets/javascripts/admin/utils/services/price_parser.js.coffee b/app/assets/javascripts/admin/utils/services/price_parser.js.coffee new file mode 100644 index 0000000000..492fbe48a0 --- /dev/null +++ b/app/assets/javascripts/admin/utils/services/price_parser.js.coffee @@ -0,0 +1,24 @@ +angular.module("admin.utils").factory "PriceParser", -> + new class PriceParser + parse: (price) => + return null unless price + # used decimal and thousands separators from currency configuration + decimal_separator = I18n.toCurrency(.1, {precision: 1, unit: ''}).substring(1,2) + thousands_separator = I18n.toCurrency(1000, {precision: 1, unit: ''}).substring(1,2) + + # Replace comma used as a decimal separator and remplace by "." + if (price.match(/^[0-9]*(,{1})[0-9]{1,2}$/g)) + price = price.replace(",", ".") + + # Remove configured thousands separator if it is actually a thousands separator + if (new RegExp("^([0-9]*(" + thousands_separator + "{1})[0-9]{3}[0-9\.,]*)*$", "g").test(price)) + price = price.replaceAll(thousands_separator, '') + + if (decimal_separator == ",") + price = price.replace(",", ".") + + price = parseFloat(price) + + return null if isNaN(price) + + return price diff --git a/spec/javascripts/unit/admin/filters/unlocalize_currency_spec.js.coffee b/spec/javascripts/unit/admin/utils/services/price_parser_spec.js.coffee similarity index 60% rename from spec/javascripts/unit/admin/filters/unlocalize_currency_spec.js.coffee rename to spec/javascripts/unit/admin/utils/services/price_parser_spec.js.coffee index 8a92d99b5e..3d3dffd1ae 100644 --- a/spec/javascripts/unit/admin/filters/unlocalize_currency_spec.js.coffee +++ b/spec/javascripts/unit/admin/utils/services/price_parser_spec.js.coffee @@ -1,10 +1,10 @@ -describe 'convert string to number with configurated currency', -> - filter = null +describe "PriceParser service", -> + priceParser = null beforeEach -> - module 'ofn.admin' - inject ($filter) -> - filter = $filter('unlocalizeCurrency') + module('admin.utils') + inject (PriceParser) -> + priceParser = PriceParser describe "with point as decimal separator and comma as thousands separator for I18n service", -> @@ -16,34 +16,34 @@ describe 'convert string to number with configurated currency', -> return "1,000" it "handle point as decimal separator", -> - expect(filter("1.00")).toEqual 1.0 + expect(priceParser.parse("1.00")).toEqual 1.0 it "handle point as decimal separator", -> - expect(filter("1.000")).toEqual 1.0 + expect(priceParser.parse("1.000")).toEqual 1.0 it "also handle comma as decimal separator", -> - expect(filter("1,0")).toEqual 1.0 + expect(priceParser.parse("1,0")).toEqual 1.0 it "also handle comma as decimal separator", -> - expect(filter("1,00")).toEqual 1.0 + expect(priceParser.parse("1,00")).toEqual 1.0 it "also handle comma as decimal separator", -> - expect(filter("11,00")).toEqual 11.0 + expect(priceParser.parse("11,00")).toEqual 11.0 it "handle comma as decimal separator but not confusing with thousands separator", -> - expect(filter("11,000")).toEqual 11000 + expect(priceParser.parse("11,000")).toEqual 11000 it "handle point as decimal separator and comma as thousands separator", -> - expect(filter("1,000,000.00")).toEqual 1000000 + expect(priceParser.parse("1,000,000.00")).toEqual 1000000 it "handle integer number", -> - expect(filter("10")).toEqual 10 + expect(priceParser.parse("10")).toEqual 10 it "handle integer number with comma as thousands separator", -> - expect(filter("1,000")).toEqual 1000 + expect(priceParser.parse("1,000")).toEqual 1000 it "handle integer number with no thousands separator", -> - expect(filter("1000")).toEqual 1000 + expect(priceParser.parse("1000")).toEqual 1000 describe "with comma as decimal separator and final point as thousands separator for I18n service", -> @@ -55,34 +55,34 @@ describe 'convert string to number with configurated currency', -> return "1.000" it "handle comma as decimal separator", -> - expect(filter("1,00")).toEqual 1.0 + expect(priceParser.parse("1,00")).toEqual 1.0 it "handle comma as decimal separator with one digit after the comma", -> - expect(filter("11,0")).toEqual 11.0 + expect(priceParser.parse("11,0")).toEqual 11.0 it "handle comma as decimal separator with two digit after the comma", -> - expect(filter("11,00")).toEqual 11.0 + expect(priceParser.parse("11,00")).toEqual 11.0 it "handle comma as decimal separator with three digit after the comma", -> - expect(filter("11,000")).toEqual 11.0 + expect(priceParser.parse("11,000")).toEqual 11.0 it "also handle point as decimal separator", -> - expect(filter("1.00")).toEqual 1.0 + expect(priceParser.parse("1.00")).toEqual 1.0 it "also handle point as decimal separator with integer part with two digits", -> - expect(filter("11.00")).toEqual 11.0 + expect(priceParser.parse("11.00")).toEqual 11.0 it "handle point as decimal separator and final point as thousands separator", -> - expect(filter("1.000.000,00")).toEqual 1000000 + expect(priceParser.parse("1.000.000,00")).toEqual 1000000 it "handle integer number", -> - expect(filter("10")).toEqual 10 + expect(priceParser.parse("10")).toEqual 10 it "handle integer number with final point as thousands separator", -> - expect(filter("1.000")).toEqual 1000 + expect(priceParser.parse("1.000")).toEqual 1000 it "handle integer number with no thousands separator", -> - expect(filter("1000")).toEqual 1000 + expect(priceParser.parse("1000")).toEqual 1000 describe "with comma as decimal separator and space as thousands separator for I18n service", -> @@ -94,41 +94,42 @@ describe 'convert string to number with configurated currency', -> return "1 000" it "handle comma as decimal separator", -> - expect(filter("1,00")).toEqual 1.0 + expect(priceParser.parse("1,00")).toEqual 1.0 it "handle comma as decimal separator with one digit after the comma", -> - expect(filter("11,0")).toEqual 11.0 + expect(priceParser.parse("11,0")).toEqual 11.0 it "handle comma as decimal separator with two digit after the comma", -> - expect(filter("11,00")).toEqual 11.0 + expect(priceParser.parse("11,00")).toEqual 11.0 it "handle comma as decimal separator with three digit after the comma", -> - expect(filter("11,000")).toEqual 11.0 + expect(priceParser.parse("11,000")).toEqual 11.0 it "also handle final point as decimal separator", -> - expect(filter("1.00")).toEqual 1.0 + expect(priceParser.parse("1.00")).toEqual 1.0 it "also handle final point as decimal separator with integer part with two digits", -> - expect(filter("11.00")).toEqual 11.0 + expect(priceParser.parse("11.00")).toEqual 11.0 it "handle point as decimal separator and space as thousands separator", -> - expect(filter("1 000 000,00")).toEqual 1000000 + expect(priceParser.parse("1 000 000,00")).toEqual 1000000 it "handle integer number", -> - expect(filter("10")).toEqual 10 + expect(priceParser.parse("10")).toEqual 10 it "handle integer number with space as thousands separator", -> - expect(filter("1 000")).toEqual 1000 + expect(priceParser.parse("1 000")).toEqual 1000 it "handle integer number with no thousands separator", -> - expect(filter("1000")).toEqual 1000 + expect(priceParser.parse("1000")).toEqual 1000 describe "handle null/undefined case", -> it "null case", -> - expect(filter(null)).toEqual null + expect(priceParser.parse(null)).toEqual null it "undefined case ", -> - expect(filter(undefined)).toEqual null + expect(priceParser.parse(undefined)).toEqual null it "wtf case", -> - expect(filter("wtf")).toEqual null + expect(priceParser.parse("wtf")).toEqual null + From db8f8a2675323a3249057d1f482f9ee07cbac58c Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Bellet Date: Fri, 11 Jun 2021 10:40:02 +0200 Subject: [PATCH 7/7] Create internal methods with regexp test - Make it more easily readable and add unit tests --- .../admin/utils/services/price_parser.js.coffee | 15 +++++++++++---- .../utils/services/price_parser_spec.js.coffee | 17 +++++++++++++++++ 2 files changed, 28 insertions(+), 4 deletions(-) diff --git a/app/assets/javascripts/admin/utils/services/price_parser.js.coffee b/app/assets/javascripts/admin/utils/services/price_parser.js.coffee index 492fbe48a0..ce7cd3b874 100644 --- a/app/assets/javascripts/admin/utils/services/price_parser.js.coffee +++ b/app/assets/javascripts/admin/utils/services/price_parser.js.coffee @@ -7,12 +7,10 @@ angular.module("admin.utils").factory "PriceParser", -> thousands_separator = I18n.toCurrency(1000, {precision: 1, unit: ''}).substring(1,2) # Replace comma used as a decimal separator and remplace by "." - if (price.match(/^[0-9]*(,{1})[0-9]{1,2}$/g)) - price = price.replace(",", ".") + price = this.replaceCommaByFinalPoint(price) # Remove configured thousands separator if it is actually a thousands separator - if (new RegExp("^([0-9]*(" + thousands_separator + "{1})[0-9]{3}[0-9\.,]*)*$", "g").test(price)) - price = price.replaceAll(thousands_separator, '') + price = this.removeThousandsSeparator(price, thousands_separator) if (decimal_separator == ",") price = price.replace(",", ".") @@ -22,3 +20,12 @@ angular.module("admin.utils").factory "PriceParser", -> return null if isNaN(price) return price + + replaceCommaByFinalPoint : (price) => + if price.match(/^[0-9]*(,{1})[0-9]{1,2}$/g) then price.replace(",", ".") else price + + removeThousandsSeparator : (price, thousands_separator) => + if (new RegExp("^([0-9]*(" + thousands_separator + "{1})[0-9]{3}[0-9\.,]*)*$", "g").test(price)) + price.replaceAll(thousands_separator, '') + else + price diff --git a/spec/javascripts/unit/admin/utils/services/price_parser_spec.js.coffee b/spec/javascripts/unit/admin/utils/services/price_parser_spec.js.coffee index 3d3dffd1ae..fb97815741 100644 --- a/spec/javascripts/unit/admin/utils/services/price_parser_spec.js.coffee +++ b/spec/javascripts/unit/admin/utils/services/price_parser_spec.js.coffee @@ -6,6 +6,23 @@ describe "PriceParser service", -> inject (PriceParser) -> priceParser = PriceParser + describe "test internal method with Regexp", -> + describe "test replaceCommaByFinalPoint() method", -> + it "handle the default case (with two numbers after comma)", -> + expect(priceParser.replaceCommaByFinalPoint("1,00")).toEqual "1.00" + it "doesn't confuse with thousands separator", -> + expect(priceParser.replaceCommaByFinalPoint("1,000")).toEqual "1,000" + it "handle also when there is only one number after the decimal separator", -> + expect(priceParser.replaceCommaByFinalPoint("1,0")).toEqual "1.0" + describe "test removeThousandsSeparator() method", -> + it "handle the default case", -> + expect(priceParser.removeThousandsSeparator("1,000", ",")).toEqual "1000" + expect(priceParser.removeThousandsSeparator("1,000,000", ",")).toEqual "1000000" + it "handle the case with decimal separator", -> + expect(priceParser.removeThousandsSeparator("1,000,000.00", ",")).toEqual "1000000.00" + it "handle the case when it is actually a decimal separator (and not a thousands one)", -> + expect(priceParser.removeThousandsSeparator("1,00", ",")).toEqual "1,00" + describe "with point as decimal separator and comma as thousands separator for I18n service", -> beforeEach ->