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 4d93f9ef11..ff7d05b3ba 100644 --- a/CRM/Contribute/BAO/Contribution.php +++ b/CRM/Contribute/BAO/Contribution.php @@ -97,7 +97,8 @@ class CRM_Contribute_BAO_Contribution extends CRM_Contribute_DAO_Contribution { * @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 @@ class CRM_Contribute_BAO_Contribution extends CRM_Contribute_DAO_Contribution { $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 @@ class CRM_Contribute_BAO_Contribution extends CRM_Contribute_DAO_Contribution { $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 d8e090517c..f91d894784 100644 --- a/CRM/Contribute/Form/Contribution/Confirm.php +++ b/CRM/Contribute/Form/Contribution/Confirm.php @@ -716,14 +716,15 @@ class CRM_Contribute_Form_Contribution_Confirm extends CRM_Contribute_Form_Contr */ 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 @@ class CRM_Contribute_Form_Contribution_Confirm extends CRM_Contribute_Form_Contr // @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 @@ class CRM_Contribute_Form_Contribution_Confirm extends CRM_Contribute_Form_Contr $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 @@ class CRM_Contribute_Form_Contribution_Confirm extends CRM_Contribute_Form_Contr //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 @@ class CRM_Contribute_Form_Contribution_Confirm extends CRM_Contribute_Form_Contr } } + /** + * 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 51f9126b9d..6a883262d8 100644 --- a/CRM/Event/Cart/Form/Checkout/Payment.php +++ b/CRM/Event/Cart/Form/Checkout/Payment.php @@ -670,10 +670,7 @@ class CRM_Event_Cart_Form_Checkout_Payment extends CRM_Event_Cart_Form_Cart { $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; -- 2.25.1