From ab1552f9a6d3bee82671a98a255580852529eba4 Mon Sep 17 00:00:00 2001 From: eileen Date: Fri, 18 Oct 2019 17:20:40 +1300 Subject: [PATCH] Support payment related fields on Payment.Create 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 --- CRM/Financial/BAO/Payment.php | 39 ++++---- api/v3/Payment.php | 129 ++++++++++++++++++++++++++- api/v3/utils.php | 3 + tests/phpunit/api/v3/PaymentTest.php | 16 +++- 4 files changed, 168 insertions(+), 19 deletions(-) diff --git a/CRM/Financial/BAO/Payment.php b/CRM/Financial/BAO/Payment.php index 998f8af44c13..fc682d04cde5 100644 --- a/CRM/Financial/BAO/Payment.php +++ b/CRM/Financial/BAO/Payment.php @@ -59,27 +59,36 @@ 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', 'order_reference', '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_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 instrumend 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? diff --git a/api/v3/Payment.php b/api/v3/Payment.php index 4ec6647c22d3..ef71e08eb865 100644 --- a/api/v3/Payment.php +++ b/api/v3/Payment.php @@ -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 @@ -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 financial transaction'), + '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'), @@ -187,6 +197,119 @@ 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', + ], + ], + 'order_reference' => [ + 'name' => 'order_reference', + 'type' => CRM_Utils_Type::T_STRING, + 'title' => ts('Order Reference'), + 'description' => ts('Payment Processor external order reference'), + 'maxlength' => 255, + 'size' => 25, + 'where' => 'civicrm_financial_trxn.order_reference', + 'table_name' => 'civicrm_financial_trxn', + 'entity' => 'FinancialTrxn', + 'bao' => 'CRM_Financial_DAO_FinancialTrxn', + 'localizable' => 0, + 'html' => [ + 'type' => 'Text', + ], + ], ]; } diff --git a/api/v3/utils.php b/api/v3/utils.php index 5a8c0e54b75c..8d45e84ff8dd 100644 --- a/api/v3/utils.php +++ b/api/v3/utils.php @@ -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; diff --git a/tests/phpunit/api/v3/PaymentTest.php b/tests/phpunit/api/v3/PaymentTest.php index d1dfd7235710..19e78273ec08 100644 --- a/tests/phpunit/api/v3/PaymentTest.php +++ b/tests/phpunit/api/v3/PaymentTest.php @@ -366,10 +366,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); } } @@ -665,6 +668,7 @@ public function testUpdatePayment() { */ public function testCreatePaymentPayLater() { $this->createLoggedInUser(); + $processorID = $this->paymentProcessorCreate(); $contributionParams = [ 'total_amount' => 100, 'currency' => 'USD', @@ -678,6 +682,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 = [ @@ -687,6 +696,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);