From c0598e7ffdbbff7bbc4063a7497e6bf63cf4468d Mon Sep 17 00:00:00 2001 From: "Ronald A. Richardson" Date: Mon, 27 Jul 2026 13:54:27 +0800 Subject: [PATCH] Preserve sign and honor currency exponents in money formatting Utils::numbersOnly() stripped the leading minus sign via preg_replace('/[^0-9]/', ...), so negative monetary amounts lost their sign (e.g. moneyFormat(-500, 'USD') rendered as $5.00 instead of -$5.00). numbersOnly() now preserves a minus that appears before the first digit, while still ignoring hyphens that occur after digits (e.g. phone numbers like "276-7156"), and collapses empty/non-numeric input to 0. moneyFormat() treats its amount as an integer number of the currency's smallest (minor) unit; the money adapter derives decimal placement from each currency's ISO-4217 exponent, so zero-decimal (JPY/KRW) and three-decimal (BHD/KWD) currencies format correctly rather than assuming two places. Docblocks updated to document the contract. Tests now exercise the real Cknow\Money\Money adapter (added cknow/laravel-money to require-dev) instead of a stub, covering negative/zero/JPY/KRW/BHD/KWD/non-numeric/null cases, plus dedicated numbersOnly sign-preservation coverage. Co-Authored-By: Claude Opus 4.8 --- composer.json | 1 + src/Support/Utils.php | 39 +++++++- .../Reporting/ReportQueryExporterTest.php | 21 +---- tests/Unit/Support/UtilsMoneyFormatTest.php | 94 ++++++++++++------- tests/Unit/Support/UtilsTest.php | 21 +++++ 5 files changed, 118 insertions(+), 58 deletions(-) diff --git a/composer.json b/composer.json index f974f54e..40605b58 100644 --- a/composer.json +++ b/composer.json @@ -61,6 +61,7 @@ "rlanvin/php-rrule": "^2.4" }, "require-dev": { + "cknow/laravel-money": "^7.2", "friendsofphp/php-cs-fixer": "3.82.2", "nunomaduro/collision": "^7.0", "pestphp/pest": "^2.33.2", diff --git a/src/Support/Utils.php b/src/Support/Utils.php index 5b570d96..6fa6f070 100644 --- a/src/Support/Utils.php +++ b/src/Support/Utils.php @@ -730,14 +730,33 @@ public static function randomNumber($length = 4) /** * Converts the param to an integer with numbers only. * + * A leading minus sign is preserved so signed values (e.g. negative + * monetary amounts like "-5.00" or "-$5.00") keep their sign. Only a + * minus that appears *before* the first digit is treated as a sign; a + * hyphen occurring after digits (such as within a phone number like + * "276-7156") is ignored, so this remains safe for non-monetary input. + * * @return int */ public static function numbersOnly($value) { $string = strval($value); - $string = preg_replace('/[^0-9]/', '', $string); - return intval($string); + $isNegative = false; + if (preg_match('/\d/', $string, $matches, PREG_OFFSET_CAPTURE)) { + $firstDigitOffset = $matches[0][1]; + $minusOffset = strpos($string, '-'); + $isNegative = $minusOffset !== false && $minusOffset < $firstDigitOffset; + } + + $digits = preg_replace('/[^0-9]/', '', $string); + if ($digits === '') { + return 0; + } + + $number = intval($digits); + + return $isNegative ? -$number : $number; } /** @@ -764,10 +783,20 @@ public static function removeSpecialCharacters($string, $except = []) } /** - * Format number to a particular currency. + * Format an amount, expressed in a currency's smallest (minor) unit, into a + * localized currency string. + * + * The amount is interpreted as an integer number of minor units — cents for + * USD/EUR, whole yen for JPY/KRW (which have no minor unit), fils for + * BHD/KWD (three-decimal currencies), and so on. The underlying money + * adapter honors each currency's ISO-4217 minor-unit exponent, so the + * decimal placement is derived from the currency rather than assumed to be + * two places. Any surrounding formatting (currency symbols, grouping + * commas) is stripped and a leading minus sign is preserved, so negative + * amounts (refunds, adjustments) format correctly. * - * @param float $amount amount to format - * @param string $currency the currency to format into + * @param int|float|string $amount the amount in the currency's smallest unit + * @param string $currency the currency to format into * * @return string */ diff --git a/tests/Unit/Reporting/ReportQueryExporterTest.php b/tests/Unit/Reporting/ReportQueryExporterTest.php index ed4c2994..8d6fa41d 100644 --- a/tests/Unit/Reporting/ReportQueryExporterTest.php +++ b/tests/Unit/Reporting/ReportQueryExporterTest.php @@ -4,25 +4,6 @@ use Illuminate\Support\Carbon; use PhpOffice\PhpSpreadsheet\Spreadsheet; -class ReportQueryExporterMoneyFake -{ - public static array $constructed = []; - - public function __construct(private int $amount, private string $currency) - { - self::$constructed[] = compact('amount', 'currency'); - } - - public function format(): string - { - return $this->currency . ':' . $this->amount; - } -} - -if (!class_exists('Cknow\\Money\\Money')) { - class_alias(ReportQueryExporterMoneyFake::class, 'Cknow\\Money\\Money'); -} - beforeEach(function () { bind_test_container(); Carbon::setTestNow(Carbon::parse('2026-07-17 12:00:00')); @@ -159,7 +140,7 @@ public function ensureDirectoryForTest(): void ->and($exporter->formatValue('42', ['type' => 'number']))->toBe(42.0) ->and($exporter->formatValue('42.75', ['type' => 'decimal']))->toBe(42.75) ->and($exporter->formatValue('not numeric', ['type' => 'number']))->toBe('not numeric') - ->and($exporter->formatValue(1234.56, ['type' => 'currency']))->toBe('USD:123456') + ->and($exporter->formatValue(1234.56, ['type' => 'currency']))->toBe('$1,234.56') ->and($exporter->formatValue('', ['type' => 'currency']))->toBe('') ->and($exporter->formatValue('n/a', ['type' => 'percentage']))->toBe('n/a') ->and($exporter->formatValue(false, ['type' => 'boolean']))->toBe('No') diff --git a/tests/Unit/Support/UtilsMoneyFormatTest.php b/tests/Unit/Support/UtilsMoneyFormatTest.php index 49caa1c4..183ecc42 100644 --- a/tests/Unit/Support/UtilsMoneyFormatTest.php +++ b/tests/Unit/Support/UtilsMoneyFormatTest.php @@ -1,37 +1,65 @@ currency . ':' . $this->amount; - } - } - } -} +use Fleetbase\Support\Utils; + +/* + * These tests exercise Utils::moneyFormat() against the real Cknow\Money\Money + * adapter (backed by moneyphp/money + the intl extension) rather than a stub, + * so that per-currency minor-unit exponents and sign handling are verified end + * to end. + * + * The `$amount` argument is expected to be an integer number of the currency's + * smallest unit (cents for USD, whole yen for JPY, fils for BHD, ...). The + * adapter derives decimal placement from each currency's ISO-4217 exponent, so + * the assertions below focus on the numeric core of the formatted output. That + * keeps them stable across ICU versions, which vary the currency symbol and the + * (sometimes non-breaking) spacing around it. + */ +beforeEach(function () { + // A bound container with a config repository is required so the money + // adapter can resolve its locale/currencies via the config() helper. + bind_test_container(); +}); -namespace { - use Cknow\Money\Money; - use Fleetbase\Support\Utils; - - test('utils money format normalizes numeric input before formatting with the money adapter', function () { - Money::$constructed = []; - - expect(Utils::moneyFormat('USD 1,234.56', 'USD'))->toBe('USD:123456') - ->and(Money::$constructed)->toBe([ - [ - 'amount' => 123456, - 'currency' => 'USD', - ], - ]); - }); +/** + * Reduce a formatted currency string to just its signed numeric core so that + * assertions do not depend on the locale's currency symbol or spacing. + */ +function moneyNumericCore(string $formatted): string +{ + return preg_replace('/[^0-9.\-]/', '', $formatted); } + +test('utils money format renders positive, zero, and negative two-decimal amounts', function () { + expect(moneyNumericCore(Utils::moneyFormat(500, 'USD')))->toBe('5.00') + ->and(moneyNumericCore(Utils::moneyFormat(0, 'USD')))->toBe('0.00') + // Negative amounts (refunds, adjustments) keep their sign. + ->and(moneyNumericCore(Utils::moneyFormat(-500, 'USD')))->toBe('-5.00') + ->and(Utils::moneyFormat(-500, 'USD'))->toContain('-'); +}); + +test('utils money format normalizes formatted string input before formatting', function () { + expect(moneyNumericCore(Utils::moneyFormat('$1,234.56', 'USD')))->toBe('1234.56') + ->and(moneyNumericCore(Utils::moneyFormat('-$5.00', 'USD')))->toBe('-5.00'); +}); + +test('utils money format honors zero-decimal currencies', function () { + // JPY and KRW have no minor unit (exponent 0): 500 units renders with no + // decimal places rather than being treated as cents. + expect(moneyNumericCore(Utils::moneyFormat(500, 'JPY')))->toBe('500') + ->and(Utils::moneyFormat(500, 'JPY'))->not->toContain('.') + ->and(moneyNumericCore(Utils::moneyFormat(500, 'KRW')))->toBe('500') + ->and(Utils::moneyFormat(500, 'KRW'))->not->toContain('.'); +}); + +test('utils money format honors three-decimal currencies', function () { + // BHD and KWD have a three-digit minor unit (exponent 3): 500 fils is 0.500, + // not 5.00. + expect(moneyNumericCore(Utils::moneyFormat(500, 'BHD')))->toBe('0.500') + ->and(moneyNumericCore(Utils::moneyFormat(500, 'KWD')))->toBe('0.500'); +}); + +test('utils money format treats non-numeric and null input as zero', function () { + expect(moneyNumericCore(Utils::moneyFormat('abc', 'USD')))->toBe('0.00') + ->and(moneyNumericCore(Utils::moneyFormat(null, 'USD')))->toBe('0.00'); +}); diff --git a/tests/Unit/Support/UtilsTest.php b/tests/Unit/Support/UtilsTest.php index 315f365b..01d8da74 100644 --- a/tests/Unit/Support/UtilsTest.php +++ b/tests/Unit/Support/UtilsTest.php @@ -490,6 +490,27 @@ public function getBindings(): array ->and(Utils::calculatePercentage(12.5, 200))->toBe(25.0); }); +test('utils numbers only preserves a leading sign and normalizes non-numeric input', function () { + expect(Utils::numbersOnly(500))->toBe(500) + // Leading minus sign is preserved for negative amounts (refunds, adjustments). + ->and(Utils::numbersOnly(-500))->toBe(-500) + ->and(Utils::numbersOnly('-5.00'))->toBe(-500) + ->and(Utils::numbersOnly('-$5.00'))->toBe(-500) + ->and(Utils::numbersOnly('$-5.00'))->toBe(-500) + // Positive/zero/formatted values. + ->and(Utils::numbersOnly('$1,234.56'))->toBe(123456) + ->and(Utils::numbersOnly(0))->toBe(0) + ->and(Utils::numbersOnly('0.00'))->toBe(0) + // A hyphen occurring after digits (e.g. phone numbers) is not a sign. + ->and(Utils::numbersOnly('276-7156'))->toBe(2767156) + // A leading plus is not a negative sign. + ->and(Utils::numbersOnly('+5'))->toBe(5) + // Non-numeric and null collapse to zero. + ->and(Utils::numbersOnly('abc'))->toBe(0) + ->and(Utils::numbersOnly(''))->toBe(0) + ->and(Utils::numbersOnly(null))->toBe(0); +}); + test('utils resolves model class mutation and ember resource type contracts', function () { $user = new User(); $order = (object) [