Skip to content

Commit

Permalink
Support payment related fields on Payment.Create
Browse files Browse the repository at this point in the history 
On some digging I found that various payment related fields like pan_truncation & card_type_id were
being quietly ignored rather than saved by Payment.create

The additional payment form now uses this api so it is a regression on that form.

This fixes the metadata, tests & support for payment-related trxn fields
  • Loading branch information
@eileenmcnaughton
eileenmcnaughton committed Oct 20, 2019
1 parent 13cbd98 commit 3643945
Show file tree
Hide file tree
Showing 4 changed files with 156 additions and 19 deletions.
43 changes: 28 additions & 15 deletions CRM/Financial/BAO/Payment.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,27 +59,40 @@ public static function create($params) {

$isPaymentCompletesContribution = self::isPaymentCompletesContribution($params['contribution_id'], $params['total_amount']);

$whiteList = ['check_number', 'payment_processor_id', 'fee_amount', 'total_amount', 'contribution_id', 'net_amount', 'card_type_id', 'pan_truncation', 'trxn_result_code', 'payment_instrument_id', 'trxn_id'];
$paymentTrxnParams = array_intersect_key($params, array_fill_keys($whiteList, 1));
$paymentTrxnParams['is_payment'] = 1;
if (!empty($params['payment_processor'])) {
// I can't find evidence this is passed in - I was gonna just remove it but decided to deprecate as I see getToFinancialAccount
// also anticipates it.
CRM_Core_Error::deprecatedFunctionWarning('passing payment_processor is deprecated - use payment_processor_id');
$paymentTrxnParams['payment_processor_id'] = $params['payment_processor'];
}
if (isset($paymentTrxnParams['payment_processor_id']) && empty($paymentTrxnParams['payment_processor_id'])) {
// Don't pass 0 - ie the Pay Later processor as it is a pseudo-processor.
unset($paymentTrxnParams['payment_processor_id']);
}
if (empty($paymentTrxnParams['payment_instrument_id'])) {
if (!empty($params['payment_processor_id'])) {
$paymentTrxnParams['payment_instrument_id'] = civicrm_api3('PaymentProcessor', 'getvalue', ['return' => 'payment_instrument_id', 'id' => $paymentTrxnParams['payment_processor_id']]);
}
else {
// Fall back on the payment instrument already used - should we deprecate this?
$paymentTrxnParams['payment_instrument_id'] = $contribution['payment_instrument_id'];
}
}
if (empty($paymentTrxnParams['trxn_id']) && !empty($paymentTrxnParams['contribution_trxn_id'])) {
CRM_Core_Error::deprecatedFunctionWarning('contribution_trxn_id is deprecated - use trxn_id');
$paymentTrxnParams['trxn_id'] = $paymentTrxnParams['contribution_trxn_id'];
}

if ($params['total_amount'] > 0) {
$paymentTrxnParams['to_financial_account_id'] = CRM_Contribute_BAO_Contribution::getToFinancialAccount($contribution, $params);
$paymentTrxnParams['from_financial_account_id'] = CRM_Financial_BAO_FinancialAccount::getFinancialAccountForFinancialTypeByRelationship($contribution['financial_type_id'], 'Accounts Receivable Account is');
$paymentTrxnParams['total_amount'] = $params['total_amount'];
$paymentTrxnParams['contribution_id'] = $params['contribution_id'];
$paymentTrxnParams['trxn_date'] = CRM_Utils_Array::value('trxn_date', $params, CRM_Utils_Array::value('contribution_receive_date', $params, date('YmdHis')));
$paymentTrxnParams['fee_amount'] = CRM_Utils_Array::value('fee_amount', $params);
$paymentTrxnParams['net_amount'] = CRM_Utils_Array::value('total_amount', $params);
$paymentTrxnParams['currency'] = $contribution['currency'];
$paymentTrxnParams['trxn_id'] = CRM_Utils_Array::value('contribution_trxn_id', $params, NULL);
$paymentTrxnParams['status_id'] = CRM_Core_PseudoConstant::getKey('CRM_Core_BAO_FinancialTrxn', 'status_id', 'Completed');
$paymentTrxnParams['payment_instrument_id'] = CRM_Utils_Array::value('payment_instrument_id', $params, $contribution['payment_instrument_id']);
$paymentTrxnParams['check_number'] = CRM_Utils_Array::value('check_number', $params);
$paymentTrxnParams['is_payment'] = 1;

if (!empty($params['payment_processor'])) {
// I can't find evidence this is passed in - I was gonna just remove it but decided to deprecate as I see getToFinancialAccount
// also anticipates it.
CRM_Core_Error::deprecatedFunctionWarning('passing payment_processor is deprecated - use payment_processor_id');
$paymentTrxnParams['payment_processor_id'] = $params['payment_processor'];
}

$trxn = CRM_Core_BAO_FinancialTrxn::create($paymentTrxnParams);

// @todo - this is just weird & historical & inconsistent - why 2 tracks?
Expand Down
113 changes: 110 additions & 3 deletions api/v3/Payment.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -129,9 +129,12 @@ function civicrm_api3_payment_cancel($params) {
* @param array $params
* Input parameters.
*
* @throws API_Exception
* @return array
* Api result array
*
* @throws \API_Exception
* @throws \CRM_Core_Exception
* @throws \CiviCRM_API3_Exception
*/
function civicrm_api3_payment_create($params) {
// Check if it is an update
Expand Down Expand Up @@ -168,9 +171,16 @@ function _civicrm_api3_payment_create_spec(&$params) {
'type' => CRM_Utils_Type::T_FLOAT,
],
'payment_processor_id' => [
'title' => ts('Payment Processor ID'),
'name' => 'payment_processor_id',
'type' => CRM_Utils_Type::T_INT,
'description' => ts('Payment processor ID - required for payment processor payments'),
'title' => ts('Payment Processor'),
'description' => ts('Payment Processor for this payment'),
'where' => 'civicrm_financial_trxn.payment_processor_id',
'table_name' => 'civicrm_financial_trxn',
'entity' => 'FinancialTrxn',
'bao' => 'CRM_Financial_DAO_FinancialTrxn',
'localizable' => 0,
'FKClassName' => 'CRM_Financial_DAO_PaymentProcessor',
],
'id' => [
'title' => ts('Payment ID'),
Expand All @@ -187,6 +197,103 @@ function _civicrm_api3_payment_create_spec(&$params) {
'type' => CRM_Utils_Type::T_BOOLEAN,
'api.default' => TRUE,
],
'payment_instrument_id' => [
'name' => 'payment_instrument_id',
'type' => CRM_Utils_Type::T_INT,
'title' => ts('Payment Method'),
'description' => ts('FK to payment_instrument option group values'),
'where' => 'civicrm_financial_trxn.payment_instrument_id',
'table_name' => 'civicrm_financial_trxn',
'entity' => 'FinancialTrxn',
'bao' => 'CRM_Financial_DAO_FinancialTrxn',
'localizable' => 0,
'html' => [
'type' => 'Select',
],
'pseudoconstant' => [
'optionGroupName' => 'payment_instrument',
'optionEditPath' => 'civicrm/admin/options/payment_instrument',
],
],
'card_type_id' => [
'name' => 'card_type_id',
'type' => CRM_Utils_Type::T_INT,
'title' => ts('Card Type ID'),
'description' => ts('FK to accept_creditcard option group values'),
'where' => 'civicrm_financial_trxn.card_type_id',
'table_name' => 'civicrm_financial_trxn',
'entity' => 'FinancialTrxn',
'bao' => 'CRM_Financial_DAO_FinancialTrxn',
'localizable' => 0,
'html' => [
'type' => 'Select',
],
'pseudoconstant' => [
'optionGroupName' => 'accept_creditcard',
'optionEditPath' => 'civicrm/admin/options/accept_creditcard',
],
],
'trxn_result_code' => [
'name' => 'trxn_result_code',
'type' => CRM_Utils_Type::T_STRING,
'title' => ts('Transaction Result Code'),
'description' => ts('processor result code'),
'maxlength' => 255,
'size' => CRM_Utils_Type::HUGE,
'where' => 'civicrm_financial_trxn.trxn_result_code',
'table_name' => 'civicrm_financial_trxn',
'entity' => 'FinancialTrxn',
'bao' => 'CRM_Financial_DAO_FinancialTrxn',
'localizable' => 0,
],
'trxn_id' => [
'name' => 'trxn_id',
'type' => CRM_Utils_Type::T_STRING,
'title' => ts('Transaction ID'),
'description' => ts('Transaction id supplied by external processor. This may not be unique.'),
'maxlength' => 255,
'size' => 10,
'where' => 'civicrm_financial_trxn.trxn_id',
'table_name' => 'civicrm_financial_trxn',
'entity' => 'FinancialTrxn',
'bao' => 'CRM_Financial_DAO_FinancialTrxn',
'localizable' => 0,
'html' => [
'type' => 'Text',
],
],
'check_number' => [
'name' => 'check_number',
'type' => CRM_Utils_Type::T_STRING,
'title' => ts('Check Number'),
'description' => ts('Check number'),
'maxlength' => 255,
'size' => 6,
'where' => 'civicrm_financial_trxn.check_number',
'table_name' => 'civicrm_financial_trxn',
'entity' => 'FinancialTrxn',
'bao' => 'CRM_Financial_DAO_FinancialTrxn',
'localizable' => 0,
'html' => [
'type' => 'Text',
],
],
'pan_truncation' => [
'name' => 'pan_truncation',
'type' => CRM_Utils_Type::T_STRING,
'title' => ts('Pan Truncation'),
'description' => ts('Last 4 digits of credit card'),
'maxlength' => 4,
'size' => 4,
'where' => 'civicrm_financial_trxn.pan_truncation',
'table_name' => 'civicrm_financial_trxn',
'entity' => 'FinancialTrxn',
'bao' => 'CRM_Financial_DAO_FinancialTrxn',
'localizable' => 0,
'html' => [
'type' => 'Text',
],
],
];
}

Expand Down
3 changes: 3 additions & 0 deletions api/v3/utils.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -390,6 +390,9 @@ function _civicrm_api3_get_BAO($name) {
// not the other cache info like search results (which could in fact be in Redis or another cache engine)
$name = 'PrevNextCache';
}
if ($name === 'Payment') {
$name = 'FinancialTrxn';
}
$dao = _civicrm_api3_get_DAO($name);
if (!$dao) {
return NULL;
Expand Down
16 changes: 15 additions & 1 deletion tests/phpunit/api/v3/PaymentTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -371,10 +371,13 @@ public function testCreatePaymentNoLineItems() {

/**
* Function to assert db values
*
* @throws \CRM_Core_Exception
*/
public function checkPaymentResult($payment, $expectedResult) {
$refreshedPayment = $this->callAPISuccessGetSingle('Payment', ['financial_trxn_id' => $payment['id']]);
foreach ($expectedResult[$payment['id']] as $key => $value) {
$this->assertEquals($payment['values'][$payment['id']][$key], $value, 'mismatch on ' . $key);
$this->assertEquals($refreshedPayment[$key], $value, 'mismatch on ' . $key); $this->assertEquals($refreshedPayment[$key], $value, 'mismatch on ' . $key);
}
}

Expand Down Expand Up @@ -670,6 +673,7 @@ public function testUpdatePayment() {
*/
public function testCreatePaymentPayLater() {
$this->createLoggedInUser();
$processorID = $this->paymentProcessorCreate();
$contributionParams = [
'total_amount' => 100,
'currency' => 'USD',
Expand All @@ -683,6 +687,11 @@ public function testCreatePaymentPayLater() {
$params = [
'contribution_id' => $contribution['id'],
'total_amount' => 100,
'card_type_id' => 'Visa',
'pan_truncation' => '1234',
'trxn_result_code' => 'Startling success',
'payment_instrument_id' => $processorID,
'trxn_id' => 1234,
];
$payment = $this->callAPISuccess('Payment', 'create', $params);
$expectedResult = [
Expand All @@ -692,6 +701,11 @@ public function testCreatePaymentPayLater() {
'total_amount' => 100,
'status_id' => 1,
'is_payment' => 1,
'card_type_id' => 1,
'pan_truncation' => '1234',
'trxn_result_code' => 'Startling success',
'trxn_id' => 1234,
'payment_instrument_id' => 1,
],
];
$this->checkPaymentResult($payment, $expectedResult);
Expand Down

0 comments on commit 3643945

Please sign in to comment.