From 797d4c524ff364c83c3ec80d6abbb9c85f27aa8b Mon Sep 17 00:00:00 2001 From: eileen Date: Sun, 27 Dec 2015 16:39:34 +1300 Subject: [PATCH] CRM-17751 add api support for setting the trxn_id for a refund The civicrm_financial_trxn.trxn_id is set to ['refund_trxn_id'] when it is passed in. See the tests for detail on expectations Change-Id: Id24a9af646dde23138ac7888314f7438c4065469 add missing commas Change-Id: I5ac41d28b17e7149b94742b26e3bc65479e1ce88 --- CRM/Contribute/BAO/Contribution.php | 19 ++- api/v3/Contribution.php | 13 +- tests/phpunit/CiviTest/CiviUnitTestCase.php | 2 +- tests/phpunit/api/v3/ContributionTest.php | 124 +++++++++++++++++++- 4 files changed, 145 insertions(+), 13 deletions(-) diff --git a/CRM/Contribute/BAO/Contribution.php b/CRM/Contribute/BAO/Contribution.php index 3582ef87a0..a42207dff1 100644 --- a/CRM/Contribute/BAO/Contribution.php +++ b/CRM/Contribute/BAO/Contribution.php @@ -3060,6 +3060,10 @@ INNER JOIN civicrm_activity ON civicrm_activity_contact.activity_id = civicrm_ac ); if ($contributionStatus == 'Refunded') { $trxnParams['trxn_date'] = !empty($params['contribution']->cancel_date) ? $params['contribution']->cancel_date : date('YmdHis'); + if (isset($params['refund_trxn_id'])) { + // CRM-17751 allow a separate trxn_id for the refund to be passed in via api & form. + $trxnParams['trxn_id'] = $params['refund_trxn_id']; + } } //CRM-16259, set is_payment flag for non pending status if (!in_array($contributionStatus, $pendingStatus)) { @@ -3084,7 +3088,13 @@ INNER JOIN civicrm_activity ON civicrm_activity_contact.activity_id = civicrm_ac $params['trxnParams']['total_amount'] = $trxnParams['total_amount'] = $params['total_amount'] = $params['prevContribution']->total_amount; $params['trxnParams']['fee_amount'] = $params['prevContribution']->fee_amount; $params['trxnParams']['net_amount'] = $params['prevContribution']->net_amount; - $params['trxnParams']['trxn_id'] = $params['prevContribution']->trxn_id; + if (!isset($params['trxnParams']['trxn_id'])) { + // Actually I have no idea why we are overwriting any values from the previous contribution. + // (filling makes sense to me). However, only protecting this value as I really really know we + // don't want this one overwritten. + // CRM-17751. + $params['trxnParams']['trxn_id'] = $params['prevContribution']->trxn_id; + } $params['trxnParams']['status_id'] = $params['prevContribution']->contribution_status_id; if (!(($params['prevContribution']->contribution_status_id == array_search('Pending', $contributionStatuses) @@ -3125,7 +3135,12 @@ INNER JOIN civicrm_activity ON civicrm_activity_contact.activity_id = civicrm_ac //Update contribution status $params['trxnParams']['status_id'] = $params['contribution']->contribution_status_id; - $params['trxnParams']['trxn_id'] = $params['contribution']->trxn_id; + if (!isset($params['refund_trxn_id'])) { + // CRM-17751 This has previously been deliberately set. No explanation as to why one variant + // gets preference over another so I am only 'protecting' a very specific tested flow + // and letting natural justice take care of the rest. + $params['trxnParams']['trxn_id'] = $params['contribution']->trxn_id; + } if (!empty($params['contribution_status_id']) && $params['prevContribution']->contribution_status_id != $params['contribution']->contribution_status_id ) { diff --git a/api/v3/Contribution.php b/api/v3/Contribution.php index dc1a9861ab..cb138a484e 100644 --- a/api/v3/Contribution.php +++ b/api/v3/Contribution.php @@ -123,21 +123,21 @@ function _civicrm_api3_contribution_create_spec(&$params) { $params['soft_credit_to'] = array( 'name' => 'soft_credit_to', 'title' => 'Soft Credit contact ID (legacy)', - 'type' => 1, + 'type' => CRM_Utils_Type::T_INT, 'description' => 'ID of Contact to be Soft credited to (deprecated - use contribution_soft api)', 'FKClassName' => 'CRM_Contact_DAO_Contact', ); $params['honor_contact_id'] = array( 'name' => 'honor_contact_id', 'title' => 'Honoree contact ID (legacy)', - 'type' => 1, + 'type' => CRM_Utils_Type::T_INT, 'description' => 'ID of honoree contact (deprecated - use contribution_soft api)', 'FKClassName' => 'CRM_Contact_DAO_Contact', ); $params['honor_type_id'] = array( 'name' => 'honor_type_id', 'title' => 'Honoree Type (legacy)', - 'type' => 1, + 'type' => CRM_Utils_Type::T_INT, 'description' => 'Type of honoree contact (deprecated - use contribution_soft api)', 'pseudoconstant' => TRUE, ); @@ -158,9 +158,14 @@ function _civicrm_api3_contribution_create_spec(&$params) { ); $params['batch_id'] = array( 'title' => 'Batch', - 'type' => 1, + 'type' => CRM_Utils_Type::T_INT, 'description' => 'Batch which relevant transactions should be added to', ); + $params['refund_trxn_id'] = array( + 'title' => 'Refund Transaction ID', + 'type' => CRM_Utils_Type::T_STRING, + 'description' => 'Transaction ID specific to the refund taking place', + ); } /** diff --git a/tests/phpunit/CiviTest/CiviUnitTestCase.php b/tests/phpunit/CiviTest/CiviUnitTestCase.php index ccdc8fafba..374a2512de 100755 --- a/tests/phpunit/CiviTest/CiviUnitTestCase.php +++ b/tests/phpunit/CiviTest/CiviUnitTestCase.php @@ -775,7 +775,7 @@ class CiviUnitTestCase extends PHPUnit_Extensions_Database_TestCase { $this->assertEquals($paramValue, $actualValues[$paramName], "Value Mismatch On $paramName - value 1 is " . print_r($paramValue, TRUE) . " value 2 is " . print_r($actualValues[$paramName], TRUE)); } else { - $this->fail("Attribute '$paramName' not present in actual array."); + $this->assertNull($expectedValues[$paramName], "Attribute '$paramName' not present in actual array and we expected it to be " . $expectedValues[$paramName]); } } } diff --git a/tests/phpunit/api/v3/ContributionTest.php b/tests/phpunit/api/v3/ContributionTest.php index 664f4f99d1..0784eae8fd 100644 --- a/tests/phpunit/api/v3/ContributionTest.php +++ b/tests/phpunit/api/v3/ContributionTest.php @@ -1037,26 +1037,137 @@ class api_v3_ContributionTest extends CiviUnitTestCase { * Function tests that financial records are added when Contribution is Refunded. */ public function testCreateUpdateContributionRefund() { - $contribParams = array( + $contributionParams = array( 'contact_id' => $this->_individualId, 'receive_date' => '2012-01-01', 'total_amount' => 100.00, 'financial_type_id' => $this->_financialTypeId, 'payment_instrument_id' => 4, 'contribution_status_id' => 1, + 'trxn_id' => 'original_payment', + ); + $contribution = $this->callAPISuccess('contribution', 'create', $contributionParams); + $newParams = array_merge($contributionParams, array( + 'id' => $contribution['id'], + 'contribution_status_id' => 'Refunded', + 'cancel_date' => '2015-01-01 09:00', + 'refund_trxn_id' => 'the refund', + ) + ); + + $contribution = $this->callAPISuccess('contribution', 'create', $newParams); + $this->_checkFinancialTrxn($contribution, 'refund'); + $this->_checkFinancialItem($contribution['id'], 'refund'); + $this->assertEquals('original_payment', $this->callAPISuccessGetValue('Contribution', array( + 'id' => $contribution['id'], + 'return' => 'trxn_id', + ))); + } + /** + * Function tests that trxn_id is set when passed in. + * + * Here we ensure that the civicrm_financial_trxn.trxn_id & the civicrm_contribution.trxn_id are set + * when trxn_id is passed in. + */ + public function testCreateUpdateContributionRefundTrxnIDPassedIn() { + $contributionParams = array( + 'contact_id' => $this->_individualId, + 'receive_date' => '2012-01-01', + 'total_amount' => 100.00, + 'financial_type_id' => $this->_financialTypeId, + 'payment_instrument_id' => 4, + 'contribution_status_id' => 1, + 'trxn_id' => 'original_payment', ); - $contribution = $this->callAPISuccess('contribution', 'create', $contribParams); - $newParams = array_merge($contribParams, array( + $contribution = $this->callAPISuccess('contribution', 'create', $contributionParams); + $newParams = array_merge($contributionParams, array( + 'id' => $contribution['id'], + 'contribution_status_id' => 'Refunded', + 'cancel_date' => '2015-01-01 09:00', + 'trxn_id' => 'the refund', + ) + ); + + $contribution = $this->callAPISuccess('contribution', 'create', $newParams); + $this->_checkFinancialTrxn($contribution, 'refund'); + $this->_checkFinancialItem($contribution['id'], 'refund'); + $this->assertEquals('the refund', $this->callAPISuccessGetValue('Contribution', array( + 'id' => $contribution['id'], + 'return' => 'trxn_id', + ))); + } + + /** + * Function tests that trxn_id is set when passed in. + * + * Here we ensure that the civicrm_contribution.trxn_id is set + * when trxn_id is passed in but if refund_trxn_id is different then that + * is kept for the refund transaction. + */ + public function testCreateUpdateContributionRefundRefundAndTrxnIDPassedIn() { + $contributionParams = array( + 'contact_id' => $this->_individualId, + 'receive_date' => '2012-01-01', + 'total_amount' => 100.00, + 'financial_type_id' => $this->_financialTypeId, + 'payment_instrument_id' => 4, + 'contribution_status_id' => 1, + 'trxn_id' => 'original_payment', + ); + $contribution = $this->callAPISuccess('contribution', 'create', $contributionParams); + $newParams = array_merge($contributionParams, array( 'id' => $contribution['id'], 'contribution_status_id' => 'Refunded', 'cancel_date' => '2015-01-01 09:00', + 'trxn_id' => 'cont id', + 'refund_trxn_id' => 'the refund', ) ); $contribution = $this->callAPISuccess('contribution', 'create', $newParams); $this->_checkFinancialTrxn($contribution, 'refund'); $this->_checkFinancialItem($contribution['id'], 'refund'); + $this->assertEquals('cont id', $this->callAPISuccessGetValue('Contribution', array( + 'id' => $contribution['id'], + 'return' => 'trxn_id', + ))); + } + + /** + * Function tests that refund_trxn_id is set when passed in empty. + * + * Here we ensure that the civicrm_contribution.trxn_id is set + * when trxn_id is passed in but if refund_trxn_id isset but empty then that + * is kept for the refund transaction. + */ + public function testCreateUpdateContributionRefundRefundNullTrxnIDPassedIn() { + $contributionParams = array( + 'contact_id' => $this->_individualId, + 'receive_date' => '2012-01-01', + 'total_amount' => 100.00, + 'financial_type_id' => $this->_financialTypeId, + 'payment_instrument_id' => 4, + 'contribution_status_id' => 1, + 'trxn_id' => 'original_payment', + ); + $contribution = $this->callAPISuccess('contribution', 'create', $contributionParams); + $newParams = array_merge($contributionParams, array( + 'id' => $contribution['id'], + 'contribution_status_id' => 'Refunded', + 'cancel_date' => '2015-01-01 09:00', + 'trxn_id' => 'cont id', + 'refund_trxn_id' => '', + ) + ); + + $contribution = $this->callAPISuccess('contribution', 'create', $newParams); + $this->_checkFinancialTrxn($contribution, 'refund', NULL, array('trxn_id' => NULL)); + $this->_checkFinancialItem($contribution['id'], 'refund'); + $this->assertEquals('cont id', $this->callAPISuccessGetValue('Contribution', array( + 'id' => $contribution['id'], + 'return' => 'trxn_id', + ))); } /** @@ -2002,7 +2113,7 @@ class api_v3_ContributionTest extends CiviUnitTestCase { * @param string $context * @param int $instrumentId */ - public function _checkFinancialTrxn($contribution, $context, $instrumentId = NULL) { + public function _checkFinancialTrxn($contribution, $context, $instrumentId = NULL, $extraParams = array()) { $trxnParams = array( 'entity_id' => $contribution['id'], 'entity_table' => 'civicrm_contribution', @@ -2024,6 +2135,7 @@ class api_v3_ContributionTest extends CiviUnitTestCase { 'total_amount' => -100, 'status_id' => 7, 'trxn_date' => '2015-01-01 09:00:00', + 'trxn_id' => 'the refund', ); } elseif ($context == 'cancelPending') { @@ -2056,7 +2168,7 @@ class api_v3_ContributionTest extends CiviUnitTestCase { else { $compareParams['to_financial_account_id'] = 12; } - $this->assertDBCompareValues('CRM_Financial_DAO_FinancialTrxn', $trxnParams1, $compareParams); + $this->assertDBCompareValues('CRM_Financial_DAO_FinancialTrxn', $trxnParams1, array_merge($compareParams, $extraParams)); $compareParams['total_amount'] = 100; if ($context == 'paymentInstrument') { $compareParams['to_financial_account_id'] = CRM_Financial_BAO_FinancialTypeAccount::getInstrumentFinancialAccount($instrumentId); @@ -2067,7 +2179,7 @@ class api_v3_ContributionTest extends CiviUnitTestCase { } } - $this->assertDBCompareValues('CRM_Financial_DAO_FinancialTrxn', $params, $compareParams); + $this->assertDBCompareValues('CRM_Financial_DAO_FinancialTrxn', $params, array_merge($compareParams, $extraParams)); } /** -- 2.25.1