Merge pull request #11719 from eileenmcnaughton/fatal

Throw exception instead of using fatal
civicrm · Feb 26, 2018 · 716dc78 · 716dc78
2 parents 9385aac + 77beddb
commit 716dc78
Show file tree

Hide file tree

Showing 3 changed files with 34 additions and 30 deletions.
diff --git 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
@@ -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
@@ -669,10 +669,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;