Skip to content

Commit

Permalink
Merge pull request #15640 from eileenmcnaughton/pp_pay
Browse files Browse the repository at this point in the history
Fix Payment.create bug whereby payment_processor_id is not being used for the to_account_id
  • Loading branch information
eileenmcnaughton authored Nov 5, 2019
2 parents 302c4f9 + 6cc6cb8 commit fa4dd07
Show file tree
Hide file tree
Showing 6 changed files with 70 additions and 9 deletions.
4 changes: 2 additions & 2 deletions CRM/Contribute/BAO/Contribution.php
Original file line number Diff line number Diff line change
Expand Up @@ -952,8 +952,8 @@ public static function recordPaymentActivity($contributionId, $participantId, $t
* @return int
*/
public static function getToFinancialAccount($contribution, $params) {
if (!empty($params['payment_processor'])) {
return CRM_Contribute_PseudoConstant::getRelationalFinancialAccount($contribution['payment_processor'], NULL, 'civicrm_payment_processor');
if (!empty($params['payment_processor_id'])) {
return CRM_Contribute_PseudoConstant::getRelationalFinancialAccount($params['payment_processor_id'], NULL, 'civicrm_payment_processor');
}
if (!empty($params['payment_instrument_id'])) {
return CRM_Financial_BAO_FinancialTypeAccount::getInstrumentFinancialAccount($contribution['payment_instrument_id']);
Expand Down
7 changes: 1 addition & 6 deletions CRM/Financial/BAO/Payment.php
Original file line number Diff line number Diff line change
Expand Up @@ -62,12 +62,7 @@ public static function create($params) {
$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', 'trxn_date'];
$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']);
Expand Down
20 changes: 20 additions & 0 deletions CRM/Financial/BAO/PaymentProcessor.php
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,26 @@ public static function getCreditCards($paymentProcessorID = NULL) {
return [];
}

/**
* Get options for a given contribution field.
*
* @param string $fieldName
* @param string $context see CRM_Core_DAO::buildOptionsContext.
* @param array $props whatever is known about this dao object.
*
* @return array|bool
* @see CRM_Core_DAO::buildOptions
*
*/
public static function buildOptions($fieldName, $context = NULL, $props = []) {
$params = [];
if ($fieldName === 'financial_account_id') {
// Pseudo-field - let's help out.
return CRM_Core_BAO_FinancialTrxn::buildOptions('to_financial_account_id', $context, $props);
}
return CRM_Core_PseudoConstant::get(__CLASS__, $fieldName, $params, $context);
}

/**
* Retrieve DB object based on input parameters.
*
Expand Down
6 changes: 6 additions & 0 deletions api/v3/Payment.php
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,12 @@ function civicrm_api3_payment_create($params) {
}
}
}
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');
$params['payment_processor_id'] = $params['payment_processor'];
}
// Check if it is an update
if (!empty($params['id'])) {
$amount = $params['total_amount'];
Expand Down
6 changes: 6 additions & 0 deletions api/v3/PaymentProcessor.php
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,13 @@ function _civicrm_api3_payment_processor_create_spec(&$params) {
$params['domain_id']['api.default'] = CRM_Core_Config::domainID();
$params['financial_account_id']['api.default'] = CRM_Financial_BAO_PaymentProcessor::getDefaultFinancialAccountID();
$params['financial_account_id']['api.required'] = TRUE;
$params['financial_account_id']['type'] = CRM_Utils_Type::T_INT;
$params['financial_account_id']['title'] = ts('Financial Account for Processor');
$params['financial_account_id']['pseudoconstant'] = [
'table' => 'civicrm_financial_account',
'keyColumn' => 'id',
'labelColumn' => 'name',
];
}

/**
Expand Down
36 changes: 35 additions & 1 deletion tests/phpunit/api/v3/PaymentTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -753,6 +753,8 @@ public function testCreatePaymentIncompletePaymentPartialPayment() {

/**
* Test create payment api for pay later contribution with partial payment.
*
* @throws \CRM_Core_Exception
*/
public function testCreatePaymentPayLaterPartialPayment() {
$this->createLoggedInUser();
Expand All @@ -764,7 +766,7 @@ public function testCreatePaymentPayLaterPartialPayment() {
'contribution_status_id' => 2,
'is_pay_later' => 1,
];
$contribution = $this->callAPISuccess('Contribution', 'create', $contributionParams);
$contribution = $this->callAPISuccess('Order', 'create', $contributionParams);
//Create partial payment
$params = [
'contribution_id' => $contribution['id'],
Expand Down Expand Up @@ -839,10 +841,42 @@ public function testCreatePaymentPayLaterPartialPayment() {
$this->callAPISuccessGetCount('Activity', ['target_contact_id' => $this->_individualId, 'activity_type_id' => 'Payment'], 2);
}

/**
* Test that Payment.create uses the to_account of the payment processor.
*
* @throws \CiviCRM_API3_Exception
* @throws \CRM_Core_Exception
*/
public function testPaymentWithProcessorWithOddFinancialAccount() {
$processor = $this->dummyProcessorCreate(['financial_account_id' => 'Deposit Bank Account', 'payment_instrument_id' => 'Cash']);
$processor2 = $this->dummyProcessorCreate(['financial_account_id' => 'Payment Processor Account', 'name' => 'p2', 'payment_instrument_id' => 'EFT']);
$contributionParams = [
'total_amount' => 100,
'currency' => 'USD',
'contact_id' => $this->_individualId,
'financial_type_id' => 1,
'contribution_status_id' => 'Pending',
];
$order = $this->callAPISuccess('Order', 'create', $contributionParams);
$this->callAPISuccess('Payment', 'create', ['payment_processor_id' => $processor->getID(), 'total_amount' => 6, 'contribution_id' => $order['id']]);
$this->callAPISuccess('Payment', 'create', ['payment_processor_id' => $processor2->getID(), 'total_amount' => 15, 'contribution_id' => $order['id']]);
$payments = $this->callAPISuccess('Payment', 'get', ['sequential' => 1, 'contribution_id' => $order['id']])['values'];
$this->assertEquals('Deposit Bank Account', CRM_Core_PseudoConstant::getName('CRM_Core_BAO_FinancialTrxn', 'to_financial_account_id', $payments[0]['to_financial_account_id']));
$this->assertEquals('Payment Processor Account', CRM_Core_PseudoConstant::getName('CRM_Core_BAO_FinancialTrxn', 'to_financial_account_id', $payments[1]['to_financial_account_id']));
$this->assertEquals('Accounts Receivable', CRM_Core_PseudoConstant::getName('CRM_Core_BAO_FinancialTrxn', 'from_financial_account_id', $payments[0]['from_financial_account_id']));
$this->assertEquals('Accounts Receivable', CRM_Core_PseudoConstant::getName('CRM_Core_BAO_FinancialTrxn', 'from_financial_account_id', $payments[1]['from_financial_account_id']));
$this->assertEquals('Cash', CRM_Core_PseudoConstant::getName('CRM_Core_BAO_FinancialTrxn', 'payment_instrument_id', $payments[0]['payment_instrument_id']));
$this->assertEquals('EFT', CRM_Core_PseudoConstant::getName('CRM_Core_BAO_FinancialTrxn', 'payment_instrument_id', $payments[1]['payment_instrument_id']));
// $order = $this->callAPISuccessGetSingle('Order', ['id' => $processor->getID()]);
// $this->assertEquals('Cash', CRM_Core_PseudoConstant::getName('CRM_Core_BAO_FinancialTrxn', 'payment_instrument_id', $order['payment_instrument_id']));
}

/**
* Add a location to our event.
*
* @param int $eventID
*
* @throws \CRM_Core_Exception
*/
protected function addLocationToEvent($eventID) {
$addressParams = [
Expand Down

0 comments on commit fa4dd07

Please sign in to comment.