Skip to content

Commit

Permalink
dev/core#1603 fix tangental bug on form handling of long options
Browse files Browse the repository at this point in the history
This fixes the bug that derailed the fix for https://lab.civicrm.org/dev/core/-/issues/1603

On further testing I agree with Jitendra that the price field form was mis-saving the longer values (I only
tested with Euro style decimal separators but maybe on both) and it was bad data caused by this
that made the merged fix look like a regression.

I also observed that the Money functions intended to round only appeared to since it was rounding
just fine to 2 places, as tested, but it was still rounding to 2 places when we wanted more.

This fixes both the New Price field screen (when options are entered) and the edit options screen
  • Loading branch information
eileenmcnaughton committed Sep 8, 2020
1 parent 270761f commit f0718ab
Show file tree
Hide file tree
Showing 3 changed files with 34 additions and 1 deletion.
3 changes: 3 additions & 0 deletions CRM/Price/Form/Field.php
Original file line number Diff line number Diff line change
Expand Up @@ -642,6 +642,9 @@ public function postProcess() {
// store the submitted values in an array
$params = $this->controller->exportValues('Field');
$params['price'] = CRM_Utils_Rule::cleanMoney($params['price']);
foreach ($params['option_amount'] as $key => $amount) {
$params['option_amount'][$key] = CRM_Utils_Rule::cleanMoney($amount);
}

$params['is_display_amounts'] = CRM_Utils_Array::value('is_display_amounts', $params, FALSE);
$params['is_required'] = CRM_Utils_Array::value('is_required', $params, FALSE);
Expand Down
4 changes: 3 additions & 1 deletion CRM/Price/Form/Option.php
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,9 @@ public function setDefaultValues() {

// fix the display of the monetary value, CRM-4038
foreach ($this->_moneyFields as $field) {
// @todo use formatLocaleNumericRoundedByOptionalPrecision - but note that there is an issue where php doesn't handle
// money strings beyond a certain total length - per https://github.com/civicrm/civicrm-core/pull/18409
// & https://github.com/civicrm/civicrm-core/pull/18409
$defaults[$field] = CRM_Utils_Money::format(CRM_Utils_Array::value($field, $defaults), NULL, '%a');
}
}
Expand Down Expand Up @@ -326,7 +329,6 @@ public function postProcess() {
return NULL;
}
else {
$params = $ids = [];
$params = $this->controller->exportValues('Option');
$fieldLabel = CRM_Core_DAO::getFieldValue('CRM_Price_DAO_PriceField', $this->_fid, 'label');

Expand Down
28 changes: 28 additions & 0 deletions tests/phpunit/CRM/Utils/MoneyTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,34 @@ public function testFormatLocaleNumericRoundedByCurrencyEuroThousand() {
$this->setCurrencySeparators(',');
}

/**
* Test rounded by currency function with specified precision.
*
* @param string $thousandSeparator
*
* @dataProvider getThousandSeparators
*/
public function testFormatLocaleNumericRoundedByPrecision($thousandSeparator) {
$this->setCurrencySeparators($thousandSeparator);
$result = CRM_Utils_Money::formatLocaleNumericRoundedByPrecision(8950.3678, 3);
$expected = ($thousandSeparator === ',') ? '8,950.368' : '8.950,368';
$this->assertEquals($expected, $result);
}

/**
* Test rounded by currency function with specified precision but without padding to reach it.
*
* @param string $thousandSeparator
*
* @dataProvider getThousandSeparators
*/
public function testFormatLocaleNumericRoundedByOptionalPrecision($thousandSeparator) {
$this->setCurrencySeparators($thousandSeparator);
$result = CRM_Utils_Money::formatLocaleNumericRoundedByOptionalPrecision(8950.3678, 8);
$expected = ($thousandSeparator === ',') ? '8,950.3678' : '8.950,3678';
$this->assertEquals($expected, $result);
}

/**
* Test that using the space character as a currency works
*/
Expand Down

0 comments on commit f0718ab

Please sign in to comment.