From 32e1703c174cb8d98bea477ca11d9a855808ffbf Mon Sep 17 00:00:00 2001 From: demeritcowboy Date: Sun, 9 Jan 2022 19:25:07 -0500 Subject: [PATCH] tests for 22429 and apply same treatment to other functions --- Civi/Core/Format.php | 45 +++++++++++++--- tests/phpunit/Civi/Core/FormatTest.php | 73 +++++++++++++++++++++++++- 2 files changed, 110 insertions(+), 8 deletions(-) diff --git a/Civi/Core/Format.php b/Civi/Core/Format.php index 2ea33946ad2b..9d05b46798d7 100644 --- 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 index 826f4e1174c4..c61f70b2cd5f 100644 --- 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'])); - } }