Merge pull request #16392 from jaapjansma/dev_1522_tests

dev/core#1522 Handle space as thousand separator
civicrm · Jan 30, 2020 · f45f035 · f45f035
2 parents a5c7f85 + 80da79b
commit f45f035
Show file tree

Hide file tree

Showing 4 changed files with 202 additions and 29 deletions.
diff --git a/CRM/Utils/Rule.php b/CRM/Utils/Rule.php
@@ -596,19 +596,6 @@ public static function cleanMoney($value) {
    * @return bool
    */
   public static function money($value) {
-    $config = CRM_Core_Config::singleton();
-
-    // only edge case when we have a decimal point in the input money
-    // field and not defined in the decimal Point in config settings
-    if ($config->monetaryDecimalPoint &&
-      $config->monetaryDecimalPoint != '.' &&
-      // CRM-7122 also check for Thousands Separator in config settings
-      $config->monetaryThousandSeparator != '.' &&
-      substr_count($value, '.')
-    ) {
-      return FALSE;
-    }
-
     $value = self::cleanMoney($value);
 
     if (self::integer($value)) {

diff --git a/tests/phpunit/CRM/Utils/RuleTest.php b/tests/phpunit/CRM/Utils/RuleTest.php
@@ -82,9 +82,15 @@ public function numericDataProvider() {
   /**
    * @dataProvider moneyDataProvider
    * @param $inputData
+   * @param $decimalPoint
+   * @param $thousandSeparator
+   * @param $currency
    * @param $expectedResult
    */
-  public function testMoney($inputData, $expectedResult) {
+  public function testMoney($inputData, $decimalPoint, $thousandSeparator, $currency, $expectedResult) {
+    $this->setDefaultCurrency($currency);
+    $this->setMonetaryDecimalPoint($decimalPoint);
+    $this->setMonetaryThousandSeparator($thousandSeparator);
     $this->assertEquals($expectedResult, CRM_Utils_Rule::money($inputData));
   }
 
@@ -93,22 +99,66 @@ public function testMoney($inputData, $expectedResult) {
    */
   public function moneyDataProvider() {
     return [
-      [10, TRUE],
-      ['145.0E+3', FALSE],
-      ['10', TRUE],
-      [-10, TRUE],
-      ['-10', TRUE],
-      ['-10foo', FALSE],
-      ['-10.0345619', TRUE],
-      ['-10.010,4345619', TRUE],
-      ['10.0104345619', TRUE],
-      ['-0', TRUE],
-      ['-.1', TRUE],
-      ['.1', TRUE],
+      [10, '.', ',', 'USD', TRUE],
+      ['145.0E+3', '.', ',', 'USD', FALSE],
+      ['10', '.', ',', 'USD', TRUE],
+      [-10, '.', ',', 'USD', TRUE],
+      ['-10', '.', ',', 'USD', TRUE],
+      ['-10foo', '.', ',', 'USD', FALSE],
+      ['-10.0345619', '.', ',', 'USD', TRUE],
+      ['-10.010,4345619', '.', ',', 'USD', TRUE],
+      ['10.0104345619', '.', ',', 'USD', TRUE],
+      ['-0', '.', ',', 'USD', TRUE],
+      ['-.1', '.', ',', 'USD', TRUE],
+      ['.1', '.', ',', 'USD', TRUE],
       // Test currency symbols too, default locale uses $, so if we wanted to test others we'd need to reconfigure locale
-      ['$500.3333', TRUE],
-      ['-$500.3333', TRUE],
-      ['$-500.3333', TRUE],
+      ['$1,234,567.89', '.', ',', 'USD', TRUE],
+      ['-$1,234,567.89', '.', ',', 'USD', TRUE],
+      ['$-1,234,567.89', '.', ',', 'USD', TRUE],
+      // This is the float format. Encapsulated in strings
+      ['1234567.89', '.', ',', 'USD', TRUE],
+      // This is the float format.
+      [1234567.89, '.', ',', 'USD', TRUE],
+      // Test EURO currency
+      ['€1,234,567.89', '.', ',', 'EUR', TRUE],
+      ['-€1,234,567.89', '.', ',', 'EUR', TRUE],
+      ['€-1,234,567.89', '.', ',', 'EUR', TRUE],
+      // This is the float format. Encapsulated in strings
+      ['1234567.89', '.', ',', 'EUR', TRUE],
+      // This is the float format.
+      [1234567.89, '.', ',', 'EUR', TRUE],
+      // Test Norwegian KR currency
+      ['kr1,234,567.89', '.', ',', 'NOK', TRUE],
+      ['kr 1,234,567.89', '.', ',', 'NOK', TRUE],
+      ['-kr1,234,567.89', '.', ',', 'NOK', TRUE],
+      ['-kr 1,234,567.89', '.', ',', 'NOK', TRUE],
+      ['kr-1,234,567.89', '.', ',', 'NOK', TRUE],
+      ['kr -1,234,567.89', '.', ',', 'NOK', TRUE],
+      // This is the float format. Encapsulated in strings
+      ['1234567.89', '.', ',', 'NOK', TRUE],
+      // This is the float format.
+      [1234567.89, '.', ',', 'NOK', TRUE],
+      // Test different localization options: , as decimal separator and dot as thousand separator
+      ['$1.234.567,89', ',', '.', 'USD', TRUE],
+      ['-$1.234.567,89', ',', '.', 'USD', TRUE],
+      ['$-1.234.567,89', ',', '.', 'USD', TRUE],
+      ['1.234.567,89', ',', '.', 'USD', TRUE],
+      // This is the float format. Encapsulated in strings
+      ['1234567.89', ',', '.', 'USD', TRUE],
+      // This is the float format.
+      [1234567.89, ',', '.', 'USD', TRUE],
+      ['$1,234,567.89', ',', '.', 'USD', FALSE],
+      ['-$1,234,567.89', ',', '.', 'USD', FALSE],
+      ['$-1,234,567.89', ',', '.', 'USD', FALSE],
+      // Now with a space as thousand separator
+      ['$1 234 567,89', ',', ' ', 'USD', TRUE],
+      ['-$1 234 567,89', ',', ' ', 'USD', TRUE],
+      ['$-1 234 567,89', ',', ' ', 'USD', TRUE],
+      ['1 234 567,89', ',', ' ', 'USD', TRUE],
+      // This is the float format. Encapsulated in strings
+      ['1234567.89', ',', ' ', 'USD', TRUE],
+      // This is the float format.
+      [1234567.89, ',', ' ', 'USD', TRUE],
     ];
   }
 

diff --git a/tests/phpunit/CiviTest/CiviUnitTestCase.php b/tests/phpunit/CiviTest/CiviUnitTestCase.php
@@ -3262,14 +3262,52 @@ public function getBooleanDataProvider() {
   /**
    * Set the separators for thousands and decimal points.
    *
+   * Use setMonetaryDecimalPoint and setMonetaryThousandSeparator instead
+   *
    * @param string $thousandSeparator
+   * @deprecated
    */
   protected function setCurrencySeparators($thousandSeparator) {
+    // Jaap Jansma: I do think the code below is a bit useless. It does an assumption
+    // that when the thousand separator is a comma, the decimal is point. It
+    // does not cater for a situation where the thousand separator is a [space]
+    // Latter is the Norwegian localization.
     Civi::settings()->set('monetaryThousandSeparator', $thousandSeparator);
     Civi::settings()
       ->set('monetaryDecimalPoint', ($thousandSeparator === ',' ? '.' : ','));
   }
 
+  /**
+   * Sets the thousand separator.
+   *
+   * If you use this function also set the decimal separator: setMonetaryDecimalSeparator
+   *
+   * @param $thousandSeparator
+   */
+  protected function setMonetaryThousandSeparator($thousandSeparator) {
+    Civi::settings()->set('monetaryThousandSeparator', $thousandSeparator);
+  }
+
+  /**
+   * Sets the decimal separator.
+   *
+   * If you use this function also set the thousand separator setMonetaryDecimalPoint
+   *
+   * @param $decimalPoint
+   */
+  protected function setMonetaryDecimalPoint($decimalPoint) {
+    Civi::settings()->set('monetaryDecimalPoint', $decimalPoint);
+  }
+
+  /**
+   * Sets the default currency.
+   *
+   * @param $currency
+   */
+  protected function setDefaultCurrency($currency) {
+    Civi::settings()->set('defaultCurrency', $currency);
+  }
+
   /**
    * Format money as it would be input.
    *

diff --git a/tests/phpunit/api/v3/ContributionTest.php b/tests/phpunit/api/v3/ContributionTest.php
@@ -704,6 +704,104 @@ public function testGetContributionByTotalAmount() {
     $this->callAPISuccessGetCount('Contribution', [], 2);
   }
 
+  /**
+   * @dataProvider createLocalizedContributionDataProvider
+   * @param $totalAmount
+   * @param $decimalPoint
+   * @param $thousandSeparator
+   * @param $currency
+   * @param $expectedResult
+   * @throws \Exception
+   */
+  public function testCreateLocalizedContribution($totalAmount, $decimalPoint, $thousandSeparator, $currency, $expectedResult) {
+    $this->setDefaultCurrency($currency);
+    $this->setMonetaryDecimalPoint($decimalPoint);
+    $this->setMonetaryThousandSeparator($thousandSeparator);
+
+    $_params = [
+      'contact_id' => $this->_individualId,
+      'receive_date' => '20120511',
+      'total_amount' => $totalAmount,
+      'financial_type_id' => $this->_financialTypeId,
+      'contribution_status_id' => 1,
+    ];
+
+    if ($expectedResult) {
+      $this->callAPISuccess('Contribution', 'create', $_params);
+    }
+    else {
+      $this->callAPIFailure('Contribution', 'create', $_params);
+    }
+  }
+
+  /**
+   * @return array
+   */
+  public function createLocalizedContributionDataProvider() {
+    return [
+      [10, '.', ',', 'USD', TRUE],
+      ['145.0E+3', '.', ',', 'USD', FALSE],
+      ['10', '.', ',', 'USD', TRUE],
+      [-10, '.', ',', 'USD', TRUE],
+      ['-10', '.', ',', 'USD', TRUE],
+      ['-10foo', '.', ',', 'USD', FALSE],
+      ['-10.0345619', '.', ',', 'USD', TRUE],
+      ['-10.010,4345619', '.', ',', 'USD', TRUE],
+      ['10.0104345619', '.', ',', 'USD', TRUE],
+      ['-0', '.', ',', 'USD', TRUE],
+      ['-.1', '.', ',', 'USD', TRUE],
+      ['.1', '.', ',', 'USD', TRUE],
+      // Test currency symbols too, default locale uses $, so if we wanted to test others we'd need to reconfigure locale
+      ['$1,234,567.89', '.', ',', 'USD', TRUE],
+      ['-$1,234,567.89', '.', ',', 'USD', TRUE],
+      ['$-1,234,567.89', '.', ',', 'USD', TRUE],
+      // This is the float format. Encapsulated in strings
+      ['1234567.89', '.', ',', 'USD', TRUE],
+      // This is the float format.
+      [1234567.89, '.', ',', 'USD', TRUE],
+      // Test EURO currency
+      ['€1,234,567.89', '.', ',', 'EUR', TRUE],
+      ['-€1,234,567.89', '.', ',', 'EUR', TRUE],
+      ['€-1,234,567.89', '.', ',', 'EUR', TRUE],
+      // This is the float format. Encapsulated in strings
+      ['1234567.89', '.', ',', 'EUR', TRUE],
+      // This is the float format.
+      [1234567.89, '.', ',', 'EUR', TRUE],
+      // Test Norwegian KR currency
+      ['kr1,234,567.89', '.', ',', 'NOK', TRUE],
+      ['kr 1,234,567.89', '.', ',', 'NOK', TRUE],
+      ['-kr1,234,567.89', '.', ',', 'NOK', TRUE],
+      ['-kr 1,234,567.89', '.', ',', 'NOK', TRUE],
+      ['kr-1,234,567.89', '.', ',', 'NOK', TRUE],
+      ['kr -1,234,567.89', '.', ',', 'NOK', TRUE],
+      // This is the float format. Encapsulated in strings
+      ['1234567.89', '.', ',', 'NOK', TRUE],
+      // This is the float format.
+      [1234567.89, '.', ',', 'NOK', TRUE],
+      // Test different localization options: , as decimal separator and dot as thousand separator
+      ['$1.234.567,89', ',', '.', 'USD', TRUE],
+      ['-$1.234.567,89', ',', '.', 'USD', TRUE],
+      ['$-1.234.567,89', ',', '.', 'USD', TRUE],
+      ['1.234.567,89', ',', '.', 'USD', TRUE],
+      // This is the float format. Encapsulated in strings
+      ['1234567.89', ',', '.', 'USD', TRUE],
+      // This is the float format.
+      [1234567.89, ',', '.', 'USD', TRUE],
+      ['$1,234,567.89', ',', '.', 'USD', FALSE],
+      ['-$1,234,567.89', ',', '.', 'USD', FALSE],
+      ['$-1,234,567.89', ',', '.', 'USD', FALSE],
+      // Now with a space as thousand separator
+      ['$1 234 567,89', ',', ' ', 'USD', TRUE],
+      ['-$1 234 567,89', ',', ' ', 'USD', TRUE],
+      ['$-1 234 567,89', ',', ' ', 'USD', TRUE],
+      ['1 234 567,89', ',', ' ', 'USD', TRUE],
+      // This is the float format. Encapsulated in strings
+      ['1234567.89', ',', ' ', 'USD', TRUE],
+      // This is the float format.
+      [1234567.89, ',', ' ', 'USD', TRUE],
+    ];
+  }
+
   /**
    * Create test with unique field name on source.
    */