From e9dc523058cf69d5b45b9ea3fa2c79204217aca7 Mon Sep 17 00:00:00 2001 From: Seamus Lee Date: Fri, 4 Dec 2020 07:46:59 +1100 Subject: [PATCH] [REF] Deprecate passing a blank currecny to CRM_Utils_Money::format and also update function call to remove at least one usage of money_format --- CRM/Core/Error.php | 10 +++++++++- CRM/Utils/Money.php | 21 +++++++++++---------- tests/phpunit/CRM/Utils/MoneyTest.php | 7 ------- 3 files changed, 20 insertions(+), 18 deletions(-) diff --git a/CRM/Core/Error.php b/CRM/Core/Error.php index 4011b3e0afa3..1aa9733b5d44 100644 --- a/CRM/Core/Error.php +++ b/CRM/Core/Error.php @@ -1039,7 +1039,15 @@ public static function deprecatedFunctionWarning($newMethod, $oldMethod = NULL) $callerClass = $dbt[1]['class'] ?? NULL; $oldMethod = "{$callerClass}::{$callerFunction}"; } - Civi::log()->warning("Deprecated function $oldMethod, use $newMethod.", ['civi.tag' => 'deprecated']); + self::deprecatedWarning("Deprecated function $oldMethod, use $newMethod."); + } + + /** + * Output a deprecated notice about a deprecated call path, rather than deprecating a whole function. + * @param string $message + */ + public static function deprecatedWarning($message) { + Civi::log()->warning($message, ['civi.tag' => 'deprecated']); } } diff --git a/CRM/Utils/Money.php b/CRM/Utils/Money.php index 274b96626a03..786fccd64c0b 100644 --- a/CRM/Utils/Money.php +++ b/CRM/Utils/Money.php @@ -67,14 +67,15 @@ public static function format($amount, $currency = NULL, $format = NULL, $onlyNu } if (!empty($valueFormat) && $valueFormat !== '%!i') { - CRM_Core_Error::deprecatedFunctionWarning('Having a Money Value format other than %!i is deprecated, please report this on the GitLab Issue https://lab.civicrm.org/dev/core/-/issues/1494 with the relevant moneyValueFormat you use.'); + CRM_Core_Error::deprecatedWarning('Having a Money Value format other than %!i is deprecated, please report this on the GitLab Issue https://lab.civicrm.org/dev/core/-/issues/1494 with the relevant moneyValueFormat you use.'); + } + + if (!$currency) { + $currency = $config->defaultCurrency; } if ($onlyNumber) { - // money_format() exists only in certain PHP install (CRM-650) - if (is_numeric($amount) and function_exists('money_format')) { - $amount = money_format($valueFormat, $amount); - } + $amount = self::formatLocaleNumericRoundedByCurrency($amount, $currency); return $amount; } @@ -85,16 +86,16 @@ public static function format($amount, $currency = NULL, $format = NULL, $onlyNu ]); } - if (!$currency) { - $currency = $config->defaultCurrency; - } - // ensure $currency is a valid currency code // for backwards-compatibility, also accept one space instead of a currency if ($currency != ' ' && !array_key_exists($currency, self::$_currencySymbols)) { throw new CRM_Core_Exception("Invalid currency \"{$currency}\""); } + if ($currency === ' ') { + CRM_Core_Error::deprecatedWarning('Passing empty currency to CRM_Utils_Money::format is deprecated if you need it for display without currency call CRM_Utils_Money::formatLocaleNumericRounded'); + } + $amount = self::formatNumericByFormat($amount, $valueFormat); // If it contains tags, means that HTML was passed and the // amount is already converted properly, @@ -181,7 +182,7 @@ public static function equals($value1, $value2, $currency) { */ protected static function formatLocaleNumeric($amount) { if (CRM_Core_Config::singleton()->moneyvalueformat !== '%!i') { - CRM_Core_Error::deprecatedFunctionWarning('Having a Money Value format other than !%i is deprecated, please report this on GitLab with the relevant moneyValueFormat you use.'); + CRM_Core_Error::deprecatedWarning('Having a Money Value format other than !%i is deprecated, please report this on GitLab with the relevant moneyValueFormat you use.'); } return self::formatNumericByFormat($amount, CRM_Core_Config::singleton()->moneyvalueformat); } diff --git a/tests/phpunit/CRM/Utils/MoneyTest.php b/tests/phpunit/CRM/Utils/MoneyTest.php index 82cca12ca821..8deb5818c3fe 100644 --- a/tests/phpunit/CRM/Utils/MoneyTest.php +++ b/tests/phpunit/CRM/Utils/MoneyTest.php @@ -104,13 +104,6 @@ public function testFormatLocaleNumericRoundedByOptionalPrecision($thousandSepar $this->assertEquals($result, $expected); } - /** - * Test that using the space character as a currency works - */ - public function testSpaceCurrency() { - $this->assertEquals(' 8,950.37', CRM_Utils_Money::format(8950.37, ' ')); - } - /** * Test that passing an invalid currency throws an error */