From 7758bd2b8e0c470e365b1f0e205a80349d8fa1e4 Mon Sep 17 00:00:00 2001 From: Eileen McNaughton Date: Tue, 5 May 2015 17:47:11 +1200 Subject: [PATCH] CRM-16417, CRM-16357, CRM-14011 (Contribution Page) was working on keeping failed transactions but cross-il between tickets is high & the solution for one seems to be the solution for all --- CRM/Contribute/BAO/Contribution.php | 15 ++ CRM/Contribute/Form/Contribution.php | 176 ++++++++---------- CRM/Contribute/Form/Contribution/Confirm.php | 2 - CRM/Core/Payment.php | 31 ++- CRM/Core/Payment/Dummy.php | 7 + .../Exception/PaymentProcessorException.php | 10 + .../CRM/Contribute/Form/ContributionTest.php | 88 +++++---- 7 files changed, 189 insertions(+), 140 deletions(-) create mode 100644 Civi/Payment/Exception/PaymentProcessorException.php diff --git a/CRM/Contribute/BAO/Contribution.php b/CRM/Contribute/BAO/Contribution.php index 10a72adc4a..0c697d02a3 100644 --- a/CRM/Contribute/BAO/Contribution.php +++ b/CRM/Contribute/BAO/Contribution.php @@ -857,6 +857,21 @@ INNER JOIN civicrm_contact contact ON ( contact.id = civicrm_contribution.conta return $results; } + /** + * React to a financial transaction (payment) failure. + * + * Prior to CRM-16417 these were simply removed from the database but it has been agreed that seeing attempted + * payments is important for forensic and outreach reasons. + * + * This function updates the financial transaction records to failed. + * + * @todo in principle we also think it makes sense to add an activity - this part would be a second step as + * the first change is likely to go into the LTS. + */ + public static function failPayment($contributionID, $message) { + + } + /** * Check if there is a contribution with the same trxn_id or invoice_id. * diff --git a/CRM/Contribute/Form/Contribution.php b/CRM/Contribute/Form/Contribution.php index e72877b589..ceaccc3ca2 100644 --- a/CRM/Contribute/Form/Contribution.php +++ b/CRM/Contribute/Form/Contribution.php @@ -25,6 +25,8 @@ +--------------------------------------------------------------------+ */ +use Civi\Payment\Exception\PaymentProcessorException; + /** * This class generates form components for processing a contribution. */ @@ -990,10 +992,24 @@ class CRM_Contribute_Form_Contribution extends CRM_Contribute_Form_AbstractEditP )); return; } - - // Get the submitted form values. $submittedValues = $this->controller->exportValues($this->_name); - $contribution = $this->submit($submittedValues, $this->_action, $this->_ppID); + + try { + // Get the submitted form values. + $contribution = $this->submit($submittedValues, $this->_action, $this->_ppID); + } + catch (PaymentProcessorException $e) { + // Set the contribution mode. + $urlParams = "action=add&cid={$this->_contactID}"; + if ($this->_mode) { + $urlParams .= "&mode={$this->_mode}"; + } + if (!empty($this->_ppID)) { + $urlParams .= "&context=pledge&ppid={$this->_ppID}"; + } + + CRM_Core_Error::statusBounce($e->getMessage(), $urlParams, ts('Payment Processor Error')); + } $session = CRM_Core_Session::singleton(); $buttonName = $this->controller->getButtonName(); if ($this->_context == 'standalone') { @@ -1035,6 +1051,8 @@ class CRM_Contribute_Form_Contribution extends CRM_Contribute_Form_AbstractEditP * Contact ID * * @return bool|\CRM_Contribute_DAO_Contribution + * @throws \CiviCRM_API3_Exception + * @throws \Civi\Payment\Exception\PaymentProcessorException */ protected function processCreditCard($submittedValues, $lineItem, $contactID) { $contribution = FALSE; @@ -1051,6 +1069,15 @@ class CRM_Contribute_Form_Contribution extends CRM_Contribute_Form_AbstractEditP $this->_paymentObject = Civi\Payment\System::singleton()->getById($submittedValues['payment_processor_id']); $this->_paymentProcessor = $this->_paymentObject->getPaymentProcessor(); + // Set source if not set + if (empty($submittedValues['source'])) { + $userID = CRM_Core_Session::singleton()->get('userID'); + $userSortName = CRM_Core_DAO::getFieldValue('CRM_Contact_DAO_Contact', $userID, + 'sort_name' + ); + $submittedValues['source'] = ts('Submit Credit Card Payment by: %1', array(1 => $userSortName)); + } + $params = $this->_params = $submittedValues; // Mapping requiring documentation. @@ -1175,53 +1202,6 @@ class CRM_Contribute_Form_Contribution extends CRM_Contribute_Form_AbstractEditP $paymentParams['receive_date'] = $this->_params['receive_date']; } - // For recurring contribution, create Contribution Record first. - // Contribution ID, Recurring ID and Contact ID needed - // When we get a callback from the payment processor, CRM-7115 - - if (!empty($paymentParams['is_recur'])) { - $contribution = CRM_Contribute_Form_Contribution_Confirm::processContribution($this, - $this->_params, - NULL, - $contactID, - $financialType, - TRUE, - FALSE, - $isTest, - $this->_lineItem, - $this->_bltID - ); - $paymentParams['contributionID'] = $contribution->id; - $paymentParams['contributionTypeID'] = $contribution->financial_type_id; - $paymentParams['contributionPageID'] = $contribution->contribution_page_id; - $paymentParams['contributionRecurID'] = $contribution->contribution_recur_id; - } - $result = array(); - if ($paymentParams['amount'] > 0.0) { - // force a re-get of the payment processor in case the form changed it, CRM-7179 - // NOTE - I expect this is not obsolete. - $payment = CRM_Core_Payment::singleton($this->_mode, $this->_paymentProcessor, $this, TRUE); - try { - $result = $payment->doPayment($paymentParams, 'contribute'); - } - catch (CRM_Core_Exception $e) { - $message = ts("Payment Processor Error message") . $e->getMessage(); - $this->cleanupDBAfterPaymentFailure($paymentParams, $message); - // Set the contribution mode. - $urlParams = "action=add&cid={$contactID}"; - if ($this->_mode) { - $urlParams .= "&mode={$this->_mode}"; - } - if (!empty($this->_ppID)) { - $urlParams .= "&context=pledge&ppid={$this->_ppID}"; - } - - CRM_Core_Error::statusBounce($message, $urlParams, ts('Payment Processor Error')); - } - } - - $this->_params = array_merge($this->_params, $result); - $this->_params['receive_date'] = $now; if (!empty($this->_params['is_email_receipt'])) { @@ -1234,7 +1214,7 @@ class CRM_Contribute_Form_Contribution extends CRM_Contribute_Form_AbstractEditP } $this->set('params', $this->_params); - $this->assign('trxn_id', $result['trxn_id']); + $this->assign('receive_date', $this->_params['receive_date']); // Result has all the stuff we need @@ -1244,28 +1224,56 @@ class CRM_Contribute_Form_Contribution extends CRM_Contribute_Form_AbstractEditP $this->set('is_deductible', TRUE); } - // Set source if not set - if (empty($this->_params['source'])) { - $userID = CRM_Core_Session::singleton()->get('userID'); - $userSortName = CRM_Core_DAO::getFieldValue('CRM_Contact_DAO_Contact', $userID, - 'sort_name' - ); - $this->_params['source'] = ts('Submit Credit Card Payment by: %1', array(1 => $userSortName)); - } + $contribution = CRM_Contribute_Form_Contribution_Confirm::processContribution($this, + $this->_params, + NULL, + $contactID, + $financialType, + TRUE, + FALSE, + $isTest, + $lineItem, + $this->_bltID + ); - if (empty($paymentParams['is_recur'])) { - $contribution = CRM_Contribute_Form_Contribution_Confirm::processContribution($this, - $this->_params, - $result, - $contactID, - $financialType, - FALSE, FALSE, - $isTest, - $lineItem, - $this->_bltID - ); - } + $paymentParams['contributionID'] = $contribution->id; + $paymentParams['contributionTypeID'] = $contribution->financial_type_id; + $paymentParams['contributionPageID'] = $contribution->contribution_page_id; + $paymentParams['contributionRecurID'] = $contribution->contribution_recur_id; + if ($paymentParams['amount'] > 0.0) { + // force a re-get of the payment processor in case the form changed it, CRM-7179 + // NOTE - I expect this is not obsolete. + $payment = CRM_Core_Payment::singleton($this->_mode, $this->_paymentProcessor, $this, TRUE); + try { + $statuses = CRM_Contribute_BAO_Contribution::buildOptions('contribution_status_id'); + $result = $payment->doPayment($paymentParams, 'contribute'); + $this->assign('trxn_id', $result['trxn_id']); + $contribution->trxn_id = $result['trxn_id']; + /* Our scenarios here are + * 1) the payment failed & an Exception should have been thrown + * 2) the payment succeeded but the payment is not immediate (for example a recurring payment + * with a delayed start) + * 3) the payment succeeded with an immediate payment. + * + * The doPayment function ensures that contribution_status_id is always set + * as historically we have had to guess from the context - ie doDirectPayment + * = error or success, unless it is a recurring contribution in which case it is pending. + */ + if (!isset($result['contribution_status_id']) || $result['contribution_status_id'] == + array_search('Completed', $statuses)) { + civicrm_api3('contribution', 'completetransaction', array('id' => $contribution->id, 'trxn_id' => $result['trxn_id'])); + } + else { + // Save the trxn_id. + $contribution->save(); + } + } + catch (PaymentProcessorException $e) { + CRM_Contribute_BAO_Contribution::failPayment($contribution->id, $e->getMessage()); + throw new PaymentProcessorException($e->getMessage()); + } + } // Send receipt mail. if ($contribution->id && !empty($this->_params['is_email_receipt'])) { $this->_params['trxn_id'] = CRM_Utils_Array::value('trxn_id', $result); @@ -1279,32 +1287,6 @@ class CRM_Contribute_Form_Contribution extends CRM_Contribute_Form_AbstractEditP return $contribution; } - /** - * Clean up DB after payment fails. - * - * This function removes related DB entries. Note that it has been agreed in principle, - * but not implemented, that contributions should be retained as 'Failed' rather than - * deleted. - * - * @todo it doesn't clean up line items. - * - * @param array $paymentParams - * @param string $message - */ - public function cleanupDBAfterPaymentFailure($paymentParams, $message) { - // Make sure to cleanup db for recurring case. - if (!empty($paymentParams['contributionID'])) { - CRM_Core_Error::debug_log_message($message . - "contact id={$this->_contactID} (deleting contribution {$paymentParams['contributionID']}"); - CRM_Contribute_BAO_Contribution::deleteContribution($paymentParams['contributionID']); - } - if (!empty($paymentParams['contributionRecurID'])) { - CRM_Core_Error::debug_log_message($message . - "contact id={$this->_contactID} (deleting recurring contribution {$paymentParams['contributionRecurID']}"); - CRM_Contribute_BAO_ContributionRecur::deleteRecurContribution($paymentParams['contributionRecurID']); - } - } - /** * Generate the data to construct a snippet based pane. * diff --git a/CRM/Contribute/Form/Contribution/Confirm.php b/CRM/Contribute/Form/Contribution/Confirm.php index 32f76c753c..090a6fd6fd 100644 --- a/CRM/Contribute/Form/Contribution/Confirm.php +++ b/CRM/Contribute/Form/Contribution/Confirm.php @@ -29,8 +29,6 @@ * * @package CRM * @copyright CiviCRM LLC (c) 2004-2015 - * $Id$ - * */ /** diff --git a/CRM/Core/Payment.php b/CRM/Core/Payment.php index 63c2ce9033..992a8b1751 100644 --- a/CRM/Core/Payment.php +++ b/CRM/Core/Payment.php @@ -26,6 +26,7 @@ */ use Civi\Payment\System; +use Civi\Payment\Exception\PaymentProcessorException; /** * Class CRM_Core_Payment. @@ -481,25 +482,45 @@ abstract class CRM_Core_Payment { * The function ensures an exception is thrown & moves some of this logic out of the form layer and makes the forms * more agnostic. * + * Payment processors should set contribution_status_id. This function adds some historical defaults ie. the + * assumption that if a 'doDirectPayment' processors comes back it completed the transaction & in fact + * doTransferCheckout would not traditionally come back. + * + * doDirectPayment does not do an immediate payment for Authorize.net or Paypal so the default is assumed + * to be Pending. + * * @param array $params * - * @param $component + * @param string $component * * @return array - * (modified) - * @throws CRM_Core_Exception + * Result array + * + * @throws \Civi\Payment\Exception\PaymentProcessorException */ public function doPayment(&$params, $component = 'contribute') { + $statuses = CRM_Contribute_BAO_Contribution::buildOptions('contribution_status_id'); if ($this->_paymentProcessor['billing_mode'] == 4) { $result = $this->doTransferCheckout($params, $component); + if (is_array($result) && !isset($result['contribution_status_id'])) { + $result['contribution_status_id'] = array_search('Pending', $statuses); + } } else { $result = $this->doDirectPayment($params, $component); + if (is_array($result) && !isset($result['contribution_status_id'])) { + if ($params['is_recur']) { + // See comment block. + $paymentParams['contribution_status_id'] = array_search('Pending', $statuses); + } + else { + $result['contribution_status_id'] = array_search('Completed', $statuses); + } + } } if (is_a($result, 'CRM_Core_Error')) { - throw new CRM_Core_Exception(CRM_Core_Error::getMessages($result)); + throw new PaymentProcessorException(CRM_Core_Error::getMessages($result)); } - //CRM-15767 - Submit Credit Card Contribution not being saved return $result; } diff --git a/CRM/Core/Payment/Dummy.php b/CRM/Core/Payment/Dummy.php index 2ac03d16c0..9daaeba2cf 100644 --- a/CRM/Core/Payment/Dummy.php +++ b/CRM/Core/Payment/Dummy.php @@ -87,6 +87,13 @@ class CRM_Core_Payment_Dummy extends CRM_Core_Payment { $params, $cookedParams ); + // This means we can test failing transactions by setting a past year in expiry. A full expiry check would + // be more complete. + if (!empty($params['credit_card_exp_date']) && date('Y') > + CRM_Core_Payment_Form::getCreditCardExpirationYear($params)) { + $error = new CRM_Core_Error(ts('transaction failed')); + return $error; + } //end of hook invocation if (!empty($this->_doDirectPaymentResult)) { $result = $this->_doDirectPaymentResult; diff --git a/Civi/Payment/Exception/PaymentProcessorException.php b/Civi/Payment/Exception/PaymentProcessorException.php new file mode 100644 index 0000000000..40fc1085ea --- /dev/null +++ b/Civi/Payment/Exception/PaymentProcessorException.php @@ -0,0 +1,10 @@ +createLoggedInUser(); $this->_individualId = $this->individualCreate(); - $paymentProcessor = $this->processorCreate(); $this->_params = array( 'contact_id' => $this->_individualId, 'receive_date' => '20120511', @@ -116,17 +115,7 @@ class CRM_Contribute_Form_ContributionTest extends CiviUnitTestCase { 'url_recur' => 'http://dummy.com', 'billing_mode' => 1, ); - $this->_pageParams = array( - 'title' => 'Test Contribution Page', - 'financial_type_id' => 1, - 'currency' => 'USD', - 'financial_account_id' => 1, - 'payment_processor' => $paymentProcessor->id, - 'is_active' => 1, - 'is_allow_other_amount' => 1, - 'min_amount' => 10, - 'max_amount' => 1000, - ); + $instruments = $this->callAPISuccess('contribution', 'getoptions', array('field' => 'payment_instrument_id')); $this->paymentInstruments = $instruments['values']; $product1 = $this->callAPISuccess('product', 'create', array( @@ -179,32 +168,59 @@ class CRM_Contribute_Form_ContributionTest extends CiviUnitTestCase { 'payment_instrument_id' => array_search('Credit Card', $this->paymentInstruments), 'contribution_status_id' => 1, ), CRM_Core_Action::ADD); - $this->callAPISuccessGetCount('Contribution', array('contact_id' => $this->_individualId), 1); + $this->callAPISuccessGetCount('Contribution', array( + 'contact_id' => $this->_individualId, + 'contribution_status_id' => 'Completed', + ), + 1); } /** - * Test the submit function on the contribution page. + * Test the submit function with an invalid payment. + * + * We expect the contribution to be created but left pending. The payment has failed. + * + * Test covers CRM-16417 change to keep failed transactions. + * + * We are left with + * - 1 Contribution with status = Pending + * - 1 Line item + * - 1 civicrm_financial_item. This is linked to the line item and has a status of 3 */ - public function testSubmitCreditCardInvalidExpiry() { + public function testSubmitCreditCardInvalid() { $form = new CRM_Contribute_Form_Contribution(); - $form->testSubmit(array( - 'total_amount' => 50, - 'financial_type_id' => 1, - 'receive_date' => '04/21/2015', - 'receive_date_time' => '11:27PM', - 'contact_id' => $this->_individualId, - 'payment_instrument_id' => array_search('Credit Card', $this->paymentInstruments), - 'payment_processor_id' => $this->paymentProcessor->id, - 'credit_card_exp_date' => array('M' => 5, 'Y' => 2012), - 'credit_card_number' => '411111111111111', - ), CRM_Core_Action::ADD, - 'live'); - $this->callAPISuccessGetCount('Contribution', array('contact_id' => $this->_individualId), 1); - $lineItem = $this->callAPISuccessGetSingle('line_item', array()); - $this->assertEquals('50.00', $lineItem['unit_price']); - $this->assertEquals('50.00', $lineItem['line_total']); - $this->assertEquals(1, $lineItem['qty']); - $this->assertEquals(1, $lineItem['financial_type_id']); + $this->paymentProcessor->setDoDirectPaymentResult(array('is_error' => 1)); + try { + $form->testSubmit(array( + 'total_amount' => 50, + 'financial_type_id' => 1, + 'receive_date' => '04/21/2015', + 'receive_date_time' => '11:27PM', + 'contact_id' => $this->_individualId, + 'payment_instrument_id' => array_search('Credit Card', $this->paymentInstruments), + 'payment_processor_id' => $this->paymentProcessor->id, + 'credit_card_exp_date' => array('M' => 5, 'Y' => 2012), + 'credit_card_number' => '411111111111111', + ), CRM_Core_Action::ADD, + 'live'); + } + catch (\Civi\Payment\Exception\PaymentProcessorException $e) { + $this->callAPISuccessGetCount('Contribution', array('contact_id' => $this->_individualId, + 'contribution_status_id' => 'Pending'), 1); + $lineItem = $this->callAPISuccessGetSingle('line_item', array()); + $this->assertEquals('50.00', $lineItem['unit_price']); + $this->assertEquals('50.00', $lineItem['line_total']); + $this->assertEquals(1, $lineItem['qty']); + $this->assertEquals(1, $lineItem['financial_type_id']); + $financialItem = $this->callAPISuccessGetSingle('financial_item', array( + 'civicrm_line_item' => $lineItem['id'], + 'entity_id' => $lineItem['id'], + )); + $this->assertEquals('50.00', $financialItem['amount']); + $this->assertEquals(3, $financialItem['status_id']); + return; + } + $this->fail('An expected exception has not been raised.'); } /** @@ -320,7 +336,7 @@ class CRM_Contribute_Form_ContributionTest extends CiviUnitTestCase { 'is_email_receipt' => TRUE, 'from_email_address' => 'test@test.com', 'payment_processor_id' => $this->paymentProcessor->id, - 'credit_card_exp_date' => array('M' => 5, 'Y' => 2012), + 'credit_card_exp_date' => array('M' => 5, 'Y' => 2026), 'credit_card_number' => '411111111111111', ), CRM_Core_Action::ADD, 'live'); -- 2.25.1