tests for 22429 and apply same treatment to other functions

civicrm · Jan 14, 2022 · 32e1703 · 32e1703
1 parent ed5ce36
commit 32e1703
Show file tree

Hide file tree

Showing 2 changed files with 110 additions and 8 deletions.
diff --git a/Civi/Core/Format.php b/Civi/Core/Format.php
@@ -23,7 +23,7 @@ class Format {
   /**
    * Get formatted money
    *
-   * @param string|int|float $amount
+   * @param string|int|float|BigDecimal $amount
    * @param string|null $currency
    *   Currency, defaults to site currency if not provided.
    * @param string|null $locale
@@ -34,14 +34,9 @@ class Format {
    * @noinspection PhpUnhandledExceptionInspection
    */
   public function money($amount, ?string $currency = NULL, ?string $locale = NULL): string {
-    // Empty value => empty string
-    if (is_null($amount) || $amount === '' || $amount === FALSE) {
+    if (($amount = $this->checkAndConvertAmount($amount)) === '') {
       return '';
     }
-    // Verify the amount is a number or numeric string/object
-    elseif ($amount === TRUE || !is_numeric((string) $amount)) {
-      throw new \CRM_Core_Exception('Invalid value for type money');
-    }
     if (!$currency) {
       $currency = Civi::settings()->get('defaultCurrency');
     }
@@ -71,6 +66,9 @@ public function number($amount, ?string $locale = NULL, array $attributes = [
     NumberFormatter::MIN_FRACTION_DIGITS => 0,
     NumberFormatter::MAX_FRACTION_DIGITS => 8,
   ]): string {
+    if (($amount = $this->checkAndConvertAmount($amount)) === '') {
+      return '';
+    }
     $formatter = $this->getMoneyFormatter(NULL, $locale, NumberFormatter::DECIMAL, $attributes);
     return $formatter->format($amount);
   }
@@ -88,6 +86,9 @@ public function number($amount, ?string $locale = NULL, array $attributes = [
    * @noinspection PhpUnhandledExceptionInspection
    */
   public function moneyNumber($amount, string $currency, $locale): string {
+    if (($amount = $this->checkAndConvertAmount($amount)) === '') {
+      return '';
+    }
     $formatter = $this->getMoneyFormatter($currency, $locale, NumberFormatter::DECIMAL);
     $money = Money::of($amount, $currency, NULL, RoundingMode::HALF_UP);
     return $money->formatWith($formatter);
@@ -106,6 +107,9 @@ public function moneyNumber($amount, string $currency, $locale): string {
    * @noinspection PhpUnhandledExceptionInspection
    */
   public function moneyLong($amount, ?string $currency, ?string $locale): string {
+    if (($amount = $this->checkAndConvertAmount($amount)) === '') {
+      return '';
+    }
     $formatter = $this->getMoneyFormatter($currency, $locale, NumberFormatter::CURRENCY, [
       NumberFormatter::MAX_FRACTION_DIGITS => 9,
     ]);
@@ -126,6 +130,9 @@ public function moneyLong($amount, ?string $currency, ?string $locale): string {
    * @noinspection PhpUnhandledExceptionInspection
    */
   public function moneyNumberLong($amount, ?string $currency, ?string $locale): string {
+    if (($amount = $this->checkAndConvertAmount($amount)) === '') {
+      return '';
+    }
     $formatter = $this->getMoneyFormatter($currency, $locale, NumberFormatter::DECIMAL, [
       NumberFormatter::MAX_FRACTION_DIGITS => 9,
     ]);
@@ -189,4 +196,28 @@ public function getMoneyFormatter(?string $currency = NULL, ?string $locale = NU
     return \Civi::$statics[$cacheKey];
   }
 
+  /**
+   * Since the input can be various data types and values, we need to handle
+   * them before passing on to the Brick libraries which would throw exceptions
+   * for ones that we are ok just converting to the empty string.
+   *
+   * @param string|int|float|BigDecimal $amount
+   * @return string
+   *   Either the empty string if an empty-ish value, or the original amount as a string.
+   * @throws \CRM_Core_Exception
+   */
+  private function checkAndConvertAmount($amount): string {
+    // Empty value => empty string
+    // FALSE should be an error but some smarty variables are filled with FALSE to avoid ENOTICES.
+    if (is_null($amount) || $amount === '' || $amount === FALSE) {
+      return '';
+    }
+    // Verify the amount is a number or numeric string/object.
+    // We cast to string because it can be a BigDecimal object.
+    elseif ($amount === TRUE || !is_numeric((string) $amount)) {
+      throw new \CRM_Core_Exception('Invalid value for type money');
+    }
+    return (string) $amount;
+  }
+
 }
diff --git a/tests/phpunit/Civi/Core/FormatTest.php b/tests/phpunit/Civi/Core/FormatTest.php
@@ -208,6 +208,78 @@ public function localeMoneyTestCases(): array {
         'money_number_long' => '1 234,500',
       ],
     ];
+    $cases['en_US_USD_blank'] = [
+      [
+        'amount' => '',
+        'locale' => 'en_US',
+        'currency' => 'USD',
+        'money' => '',
+        'money_number' => '',
+        'money_number_long' => '',
+        'number' => '',
+        'money_long' => '',
+      ],
+    ];
+    $cases['en_US_USD_null'] = [
+      [
+        'amount' => NULL,
+        'locale' => 'en_US',
+        'currency' => 'USD',
+        'money' => '',
+        'money_number' => '',
+        'money_number_long' => '',
+        'number' => '',
+        'money_long' => '',
+      ],
+    ];
+    $cases['en_US_USD_0_int'] = [
+      [
+        'amount' => 0,
+        'locale' => 'en_US',
+        'currency' => 'USD',
+        'money' => '$0.00',
+        'money_number' => '0.00',
+        'money_number_long' => '0.00',
+        'number' => '0',
+        'money_long' => '$0.00',
+      ],
+    ];
+    $cases['en_US_USD_0_float'] = [
+      [
+        'amount' => 0.0,
+        'locale' => 'en_US',
+        'currency' => 'USD',
+        'money' => '$0.00',
+        'money_number' => '0.00',
+        'money_number_long' => '0.00',
+        'number' => '0',
+        'money_long' => '$0.00',
+      ],
+    ];
+    $cases['en_US_USD_0_string'] = [
+      [
+        'amount' => '0',
+        'locale' => 'en_US',
+        'currency' => 'USD',
+        'money' => '$0.00',
+        'money_number' => '0.00',
+        'money_number_long' => '0.00',
+        'number' => '0',
+        'money_long' => '$0.00',
+      ],
+    ];
+    $cases['en_US_USD_0_string2'] = [
+      [
+        'amount' => '0.00',
+        'locale' => 'en_US',
+        'currency' => 'USD',
+        'money' => '$0.00',
+        'money_number' => '0.00',
+        'money_number_long' => '0.00',
+        'number' => '0',
+        'money_long' => '$0.00',
+      ],
+    ];
     return $cases;
   }
 
@@ -222,7 +294,6 @@ public function testMoneyAndNumbers(array $testData): void {
     $this->assertEquals($testData['number'], Civi::format()->number($testData['amount'], $testData['locale']));
     $this->assertEquals($testData['money_long'], Civi::format()->moneyLong($testData['amount'], $testData['currency'], $testData['locale']));
     $this->assertEquals($testData['money_number_long'], Civi::format()->moneyNumberLong($testData['amount'], $testData['currency'], $testData['locale']));
-
   }
 
 }