From 77beddbee3d88da18e367d6597f1596c1de33969 Mon Sep 17 00:00:00 2001 From: eileen Date: Mon, 26 Feb 2018 15:32:35 +1300 Subject: [PATCH] CRM-21804 improve error handling on duplicate contribution. Duplicate contributions are a really not handled well anywhere in the form layer. The CRM_Core_Error winds up using the fatal template, which has the same end result for form users as if an exception were thrown, let's switch. Note that this appears to be the only scenario when an error would be returned so we can simply drop that handling --- CRM/Contribute/BAO/Contribution.php | 22 +++++------- CRM/Contribute/Form/Contribution/Confirm.php | 37 +++++++++++++------- CRM/Event/Cart/Form/Checkout/Payment.php | 5 +-- 3 files changed, 34 insertions(+), 30 deletions(-) diff --git a/CRM/Contribute/BAO/Contribution.php b/CRM/Contribute/BAO/Contribution.php index 4d93f9ef1112..ff7d05b3baf2 100644 --- a/CRM/Contribute/BAO/Contribution.php +++ b/CRM/Contribute/BAO/Contribution.php @@ -97,7 +97,8 @@ public function __construct() { * @param array $ids * The array that holds all the db ids. * - * @return CRM_Contribute_BAO_Contribution|\CRM_Core_Error + * @return \CRM_Contribute_BAO_Contribution + * @throws \CRM_Core_Exception */ public static function add(&$params, $ids = array()) { if (empty($params)) { @@ -107,14 +108,8 @@ public static function add(&$params, $ids = array()) { $contributionID = CRM_Utils_Array::value('contribution', $ids, CRM_Utils_Array::value('id', $params)); $duplicates = array(); if (self::checkDuplicate($params, $duplicates, $contributionID)) { - $error = CRM_Core_Error::singleton(); - $d = implode(', ', $duplicates); - $error->push(CRM_Core_Error::DUPLICATE_CONTRIBUTION, - 'Fatal', - array($d), - "Duplicate error - existing contribution record(s) have a matching Transaction ID or Invoice ID. Contribution record ID(s) are: $d" - ); - return $error; + $message = ts("Duplicate error - existing contribution record(s) have a matching Transaction ID or Invoice ID. Contribution record ID(s) are: " . implode(', ', $duplicates)); + throw new CRM_Core_Exception($message); } // first clean up all the money fields @@ -515,11 +510,12 @@ public static function create(&$params, $ids = array()) { $transaction = new CRM_Core_Transaction(); - $contribution = self::add($params, $ids); - - if (is_a($contribution, 'CRM_Core_Error')) { + try { + $contribution = self::add($params, $ids); + } + catch (CRM_Core_Exception $e) { $transaction->rollback(); - return $contribution; + throw $e; } $params['contribution_id'] = $contribution->id; diff --git a/CRM/Contribute/Form/Contribution/Confirm.php b/CRM/Contribute/Form/Contribution/Confirm.php index d8e090517c8d..f91d8947845d 100644 --- a/CRM/Contribute/Form/Contribution/Confirm.php +++ b/CRM/Contribute/Form/Contribution/Confirm.php @@ -716,14 +716,15 @@ public function setDefaultValues() { */ public function postProcess() { $contactID = $this->getContactID(); - $result = $this->processFormSubmission($contactID); + try { + $result = $this->processFormSubmission($contactID); + } + catch (CRM_Core_Exception $e) { + $this->bounceOnError($e->getMessage()); + } + if (is_array($result) && !empty($result['is_payment_failure'])) { - // We will probably have the function that gets this error throw an exception on the next round of refactoring. - CRM_Core_Session::singleton()->setStatus(ts("Payment Processor Error message :") . - $result['error']->getMessage()); - CRM_Utils_System::redirect(CRM_Utils_System::url('civicrm/contribute/transact', - "_qf_Main_display=true&qfKey={$this->_params['qfKey']}" - )); + $this->bounceOnError($result['error']->getMessage()); } // Presumably this is for hooks to access? Not quite clear & perhaps not required. $this->set('params', $this->_params); @@ -966,6 +967,7 @@ public static function processFormContribution( // @todo this is the wrong place for this - it should be done as close to form submission // as possible $contributionParams['total_amount'] = $params['amount']; + $contribution = CRM_Contribute_BAO_Contribution::add($contributionParams); $invoiceSettings = Civi::settings()->get('contribution_invoice_settings'); @@ -990,10 +992,6 @@ public static function processFormContribution( $smarty->assign('dataArray', $dataArray); $smarty->assign('totalTaxAmount', $params['tax_amount']); } - if (is_a($contribution, 'CRM_Core_Error')) { - $message = CRM_Core_Error::getMessages($contribution); - CRM_Core_Error::fatal($message); - } // lets store it in the form variable so postProcess hook can get to this and use it $form->_contributionID = $contribution->id; @@ -1020,8 +1018,7 @@ public static function processFormContribution( //handle custom data. $params['contribution_id'] = $contribution->id; if (!empty($params['custom']) && - is_array($params['custom']) && - !is_a($contribution, 'CRM_Core_Error') + is_array($params['custom']) ) { CRM_Core_BAO_CustomValueTable::store($params['custom'], 'civicrm_contribution', $contribution->id); } @@ -2498,4 +2495,18 @@ protected function completeTransaction($result, $contributionID) { } } + /** + * Bounce the user back to retry when an error occurs. + * + * @param string $message + */ + protected function bounceOnError($message) { + CRM_Core_Session::singleton() + ->setStatus(ts("Payment Processor Error message :") . + $message); + CRM_Utils_System::redirect(CRM_Utils_System::url('civicrm/contribute/transact', + "_qf_Main_display=true&qfKey={$this->_params['qfKey']}" + )); + } + } diff --git a/CRM/Event/Cart/Form/Checkout/Payment.php b/CRM/Event/Cart/Form/Checkout/Payment.php index 51f9126b9da5..6a883262d8c0 100644 --- a/CRM/Event/Cart/Form/Checkout/Payment.php +++ b/CRM/Event/Cart/Form/Checkout/Payment.php @@ -670,10 +670,7 @@ public function record_contribution(&$mer_participant, &$params, $event) { $contribParams['payment_processor'] = $this->_paymentProcessor['id']; } - $contribution = &CRM_Contribute_BAO_Contribution::add($contribParams); - if (is_a($contribution, 'CRM_Core_Error')) { - CRM_Core_Error::fatal(ts("There was an error creating a contribution record for your event. Please report this error to the webmaster. Details: %1", array(1 => $contribution->getMessages($contribution)))); - } + $contribution = CRM_Contribute_BAO_Contribution::add($contribParams); $mer_participant->contribution_id = $contribution->id; $params['contributionID'] = $contribution->id;