Skip to content

Commit

Permalink
CRM-17647 fix for submitting payment with thousand separator
Browse files Browse the repository at this point in the history
  • Loading branch information
eileenmcnaughton committed Jan 17, 2018
1 parent 543414a commit 3a360e4
Show file tree
Hide file tree
Showing 5 changed files with 43 additions and 6 deletions.
1 change: 1 addition & 0 deletions CRM/Contribute/BAO/Contribution.php
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,7 @@ public static function add(&$params, $ids = array()) {
else {
// @todo put a deprecated here - this should be done in the form layer.
$params['skipCleanMoney'] = FALSE;
Civi::log()->warning('Deprecated code path. Money should always be clean before it hits the BAO.', array('civi.tag' => 'deprecated'));
}

foreach ($moneyFields as $field) {
Expand Down
14 changes: 14 additions & 0 deletions CRM/Contribute/Form/AbstractEditPayment.php
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,15 @@ class CRM_Contribute_Form_AbstractEditPayment extends CRM_Contact_Form_Task {
*/
public $billingFieldSets = array();

/**
* Monetary fields that may be submitted.
*
* These should get a standardised format in the beginPostProcess function.
*
* @var array
*/
protected $submittableMoneyFields = [];

/**
* Pre process function with common actions.
*/
Expand Down Expand Up @@ -567,6 +576,11 @@ protected function beginPostProcess() {
$this->_params['ip_address'] = CRM_Utils_System::ipAddress();

self::formatCreditCardDetails($this->_params);
foreach ($this->submittableMoneyFields as $moneyField) {
if (isset($this->_params[$moneyField])) {
$this->_params[$moneyField] = CRM_Utils_Rule::cleanMoney($this->_params[$moneyField]);
}
}
}

/**
Expand Down
10 changes: 10 additions & 0 deletions CRM/Contribute/Form/Contribution.php
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,13 @@ class CRM_Contribute_Form_Contribution extends CRM_Contribute_Form_AbstractEditP
*/
protected $statusMessageTitle;

/**
* Monetary fields that may be submitted.
*
* These should get a standardised format in the beginPostProcess function.
*/
protected $submittableMoneyFields = ['total_amount', 'net_amount', 'non_deductible_amount', 'fee_amount'];

/**
* Explicitly declare the form context.
*/
Expand Down Expand Up @@ -1396,6 +1403,9 @@ protected function submit($submittedValues, $action, $pledgePaymentID) {
// would cause breakage for negative values in some cases.
$submittedValues['total_amount'] = CRM_Utils_Array::value('amount', $submittedValues);
}
else {
Civi::log()->warning('Deprecated code path. Price set id should always be set.', array('civi.tag' => 'deprecated'));
}
if ($this->_id) {
if ($this->_compId) {
if ($this->_context == 'participant') {
Expand Down
11 changes: 9 additions & 2 deletions tests/phpunit/CRM/Contribute/Form/ContributionTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -143,11 +143,16 @@ public function tearDown() {

/**
* Test the submit function on the contribution page.
*
* @param string $thousandSeparator
*
* @dataProvider getThousandSeparators
*/
public function testSubmit() {
public function testSubmit($thousandSeparator) {
$this->setCurrencySeparators($thousandSeparator);
$form = new CRM_Contribute_Form_Contribution();
$form->testSubmit(array(
'total_amount' => 50,
'total_amount' => $this->formatMoneyInput(1234),
'financial_type_id' => 1,
'contact_id' => $this->_individualId,
'payment_instrument_id' => array_search('Check', $this->paymentInstruments),
Expand All @@ -156,6 +161,8 @@ public function testSubmit() {
CRM_Core_Action::ADD);
$contribution = $this->callAPISuccessGetSingle('Contribution', array('contact_id' => $this->_individualId));
$this->assertEmpty($contribution['amount_level']);
$this->assertEquals(1234, $contribution['total_amount']);
$this->assertEquals(1234, $contribution['net_amount']);
}

/**
Expand Down
13 changes: 9 additions & 4 deletions tests/phpunit/CRM/Event/Form/ParticipantTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -119,9 +119,14 @@ public function testSubmitUpaidPriceChangeWhileStillPending() {
/**
* Initial test of submit function.
*
* @param string $thousandSeparator
*
* @dataProvider getThousandSeparators
*
* @throws \Exception
*/
public function testSubmitWithPayment() {
public function testSubmitWithPayment($thousandSeparator) {
$this->setCurrencySeparators($thousandSeparator);
$form = $this->getForm(array('is_monetary' => 1, 'financial_type_id' => 1));
$form->_mode = 'Live';
$form->_quickConfig = TRUE;
Expand Down Expand Up @@ -156,16 +161,16 @@ public function testSubmitWithPayment() {
13 => 1,
),
'amount_level' => 'Too much',
'fee_amount' => 55,
'total_amount' => 55,
'fee_amount' => $this->formatMoneyInput(1550.55),
'total_amount' => $this->formatMoneyInput(1550.55),
'from_email_address' => 'abc@gmail.com',
'send_receipt' => 1,
'receipt_text' => '',
));
$participants = $this->callAPISuccess('Participant', 'get', array());
$this->assertEquals(1, $participants['count']);
$contribution = $this->callAPISuccessGetSingle('Contribution', array());
$this->assertEquals(55, $contribution['total_amount']);
$this->assertEquals(1550.55, $contribution['total_amount']);
$this->assertEquals('Debit Card', $contribution['payment_instrument']);
}

Expand Down

0 comments on commit 3a360e4

Please sign in to comment.