From ba968e38bdaccf8b9a2780e9c555f8dff7e48bcb Mon Sep 17 00:00:00 2001 From: eileen Date: Sun, 7 Jun 2020 16:59:24 +1200 Subject: [PATCH] [REF] Replace some instances of fatal with thrown exceptions. This is ongoing cleanup to consisently throw execptions. The only thing notable is in the OG class where it turned out the exception was thrown if a never-passed-in-parameter was passed in, so I removed the construct --- CRM/Bridge/OG/CiviCRM.php | 6 +++--- CRM/Bridge/OG/Drupal.php | 2 +- CRM/Bridge/OG/Utils.php | 12 ++++-------- CRM/Extension/Manager/Payment.php | 4 ++-- CRM/Extension/Manager/Search.php | 4 ++-- CRM/Financial/BAO/FinancialType.php | 2 +- CRM/PCP/BAO/PCP.php | 2 +- CRM/Pledge/BAO/Pledge.php | 2 +- CRM/Pledge/BAO/PledgeBlock.php | 2 +- CRM/Price/BAO/PriceSet.php | 4 ++-- .../phpunit/CRM/Financial/BAO/FinancialTypeTest.php | 4 ++-- 11 files changed, 20 insertions(+), 24 deletions(-) diff --git a/CRM/Bridge/OG/CiviCRM.php b/CRM/Bridge/OG/CiviCRM.php index a9272d43ec..9c24642a24 100644 --- a/CRM/Bridge/OG/CiviCRM.php +++ b/CRM/Bridge/OG/CiviCRM.php @@ -35,7 +35,7 @@ class CRM_Bridge_OG_CiviCRM { * @param $group */ public static function groupAdd($groupID, $group) { - $ogID = CRM_Bridge_OG_Utils::ogID($groupID, FALSE); + $ogID = CRM_Bridge_OG_Utils::ogID($groupID); $node = new StdClass(); if ($ogID) { @@ -69,7 +69,7 @@ class CRM_Bridge_OG_CiviCRM { * @param $group */ public static function groupDelete($groupID, $group) { - $ogID = CRM_Bridge_OG_Utils::ogID($groupID, FALSE); + $ogID = CRM_Bridge_OG_Utils::ogID($groupID); if (!$ogID) { return; } @@ -84,7 +84,7 @@ class CRM_Bridge_OG_CiviCRM { */ public static function groupContact($groupID, $contactIDs, $op) { $config = CRM_Core_Config::singleton(); - $ogID = CRM_Bridge_OG_Utils::ogID($groupID, FALSE); + $ogID = CRM_Bridge_OG_Utils::ogID($groupID); if (!$ogID) { return; diff --git a/CRM/Bridge/OG/Drupal.php b/CRM/Bridge/OG/Drupal.php index 60e560ab1f..7a8b0be8b5 100644 --- a/CRM/Bridge/OG/Drupal.php +++ b/CRM/Bridge/OG/Drupal.php @@ -204,7 +204,7 @@ SELECT v.id $contactID = CRM_Bridge_OG_Utils::contactID($params['uf_id']); if (!$contactID) { - CRM_Core_Error::fatal(); + throw new CRM_Core_Exception(' no contact found'); } // get the group id of this OG diff --git a/CRM/Bridge/OG/Utils.php b/CRM/Bridge/OG/Utils.php index 0e5dc67e48..5fb44c7d81 100644 --- a/CRM/Bridge/OG/Utils.php +++ b/CRM/Bridge/OG/Utils.php @@ -56,12 +56,11 @@ class CRM_Bridge_OG_Utils { /** * @param int $groupID - * @param bool $abort * * @return int|null|string * @throws Exception */ - public static function ogID($groupID, $abort = TRUE) { + public static function ogID($groupID) { $source = CRM_Core_DAO::getFieldValue('CRM_Contact_DAO_Group', $groupID, 'source' @@ -73,9 +72,6 @@ class CRM_Bridge_OG_Utils { return $matches[1]; } } - if ($abort) { - CRM_Core_Error::fatal(); - } return NULL; } @@ -97,7 +93,7 @@ class CRM_Bridge_OG_Utils { CRM_Core_BAO_UFMatch::synchronizeUFMatch($account, $ufID, $account->mail, 'Drupal'); $contactID = CRM_Core_BAO_UFMatch::getContactId($ufID); if (!$contactID) { - CRM_Core_Error::fatal(); + throw new CRM_Core_Exception('no contact found'); } return $contactID; } @@ -108,7 +104,7 @@ class CRM_Bridge_OG_Utils { * @param bool $abort * * @return null|string - * @throws Exception + * @throws \CRM_Core_Exception */ public static function groupID($source, $title = NULL, $abort = FALSE) { $query = " @@ -126,7 +122,7 @@ SELECT id if ($abort && !$groupID ) { - CRM_Core_Error::fatal(); + throw new CRM_Core_Exception('no group found'); } return $groupID; diff --git a/CRM/Extension/Manager/Payment.php b/CRM/Extension/Manager/Payment.php index 9e90ed8e6e..0919690a6a 100644 --- a/CRM/Extension/Manager/Payment.php +++ b/CRM/Extension/Manager/Payment.php @@ -109,7 +109,7 @@ class CRM_Extension_Manager_Payment extends CRM_Extension_Manager_Base { public function onPreUninstall(CRM_Extension_Info $info) { $paymentProcessorTypes = $this->_getAllPaymentProcessorTypes('class_name'); if (!array_key_exists($info->key, $paymentProcessorTypes)) { - CRM_Core_Error::fatal(ts('This payment processor type is not registered.')); + throw new CRM_Core_Exception(ts('This payment processor type is not registered.')); } $dao = new CRM_Financial_DAO_PaymentProcessor(); @@ -229,7 +229,7 @@ class CRM_Extension_Manager_Payment extends CRM_Extension_Manager_Base { } if (empty($class_name)) { - CRM_Core_Error::fatal("Unable to find payment processor in " . __CLASS__ . '::' . __METHOD__); + throw new CRM_Core_Exception('Unable to find payment processor in ' . __CLASS__ . '::' . __METHOD__); } // In the case of uninstall, check for instances of PP first. diff --git a/CRM/Extension/Manager/Search.php b/CRM/Extension/Manager/Search.php index d79e2472ae..d498711f52 100644 --- a/CRM/Extension/Manager/Search.php +++ b/CRM/Extension/Manager/Search.php @@ -69,12 +69,12 @@ class CRM_Extension_Manager_Search extends CRM_Extension_Manager_Base { public function onPreUninstall(CRM_Extension_Info $info) { $customSearchesByName = $this->getCustomSearchesByName(); if (!array_key_exists($info->key, $customSearchesByName)) { - CRM_Core_Error::fatal('This custom search is not registered.'); + throw new CRM_Core_Exception('This custom search is not registered.'); } $cs = $this->getCustomSearchesById(); $id = $cs[$customSearchesByName[$info->key]]; - $optionValue = CRM_Core_BAO_OptionValue::del($id); + CRM_Core_BAO_OptionValue::del($id); return TRUE; } diff --git a/CRM/Financial/BAO/FinancialType.php b/CRM/Financial/BAO/FinancialType.php index 65c20c0221..dd4d85dbe6 100644 --- a/CRM/Financial/BAO/FinancialType.php +++ b/CRM/Financial/BAO/FinancialType.php @@ -422,7 +422,7 @@ class CRM_Financial_BAO_FinancialType extends CRM_Financial_DAO_FinancialType { foreach ($lineItems as $items) { if (!CRM_Core_Permission::check($op . ' contributions of type ' . CRM_Contribute_PseudoConstant::financialType($items['financial_type_id']))) { if ($force) { - CRM_Core_Error::fatal(ts('You do not have permission to access this page.')); + throw new CRM_Core_Exception(ts('You do not have permission to access this page.')); break; } $flag = FALSE; diff --git a/CRM/PCP/BAO/PCP.php b/CRM/PCP/BAO/PCP.php index 95af552f52..a871f13b50 100644 --- a/CRM/PCP/BAO/PCP.php +++ b/CRM/PCP/BAO/PCP.php @@ -936,7 +936,7 @@ INNER JOIN civicrm_uf_group ufgroup WHERE pb.entity_id = %1 AND pb.entity_table = %2"; $params = [1 => [$component_id, 'Integer'], 2 => [$entity_table, 'String']]; if (!$ownerNotificationId = CRM_Core_DAO::singleValueQuery($query, $params)) { - CRM_Core_Error::fatal(ts('Owner Notification is not set for this Personal Campaign Page. Please contact the site administrator if you need assistance.')); + throw new CRM_Core_Exception(ts('Owner Notification is not set for this Personal Campaign Page. Please contact the site administrator if you need assistance.')); } else { return $ownerNotificationId; diff --git a/CRM/Pledge/BAO/Pledge.php b/CRM/Pledge/BAO/Pledge.php index 1390af09f9..31f8fbc3c4 100644 --- a/CRM/Pledge/BAO/Pledge.php +++ b/CRM/Pledge/BAO/Pledge.php @@ -692,7 +692,7 @@ GROUP BY currency } if (is_a(CRM_Activity_BAO_Activity::create($activityParams), 'CRM_Core_Error')) { - CRM_Core_Error::fatal("Failed creating Activity for acknowledgment"); + throw new CRM_Core_Exception('Failed creating Activity for acknowledgment'); } } } diff --git a/CRM/Pledge/BAO/PledgeBlock.php b/CRM/Pledge/BAO/PledgeBlock.php index 2672e631cf..a3b80ff7d8 100644 --- a/CRM/Pledge/BAO/PledgeBlock.php +++ b/CRM/Pledge/BAO/PledgeBlock.php @@ -206,7 +206,7 @@ class CRM_Pledge_BAO_PledgeBlock extends CRM_Pledge_DAO_PledgeBlock { } // give error if empty or build form for payment. if (empty($payments)) { - CRM_Core_Error::fatal(ts("Oops. It looks like there is no valid payment status for online payment.")); + throw new CRM_Core_Exception(ts('Oops. It looks like there is no valid payment status for online payment.')); } else { $form->assign('is_pledge_payment', TRUE); diff --git a/CRM/Price/BAO/PriceSet.php b/CRM/Price/BAO/PriceSet.php index dcb362b67b..2ed80ff8eb 100644 --- a/CRM/Price/BAO/PriceSet.php +++ b/CRM/Price/BAO/PriceSet.php @@ -1670,8 +1670,8 @@ WHERE ct.id = cp.financial_type_id AND break; default: - CRM_Core_Error::fatal("$table is not supported in PriceSet::usedBy()"); - break; + throw new CRM_Core_Exception("$table is not supported in PriceSet::usedBy()"); + } } return $usedBy; diff --git a/tests/phpunit/CRM/Financial/BAO/FinancialTypeTest.php b/tests/phpunit/CRM/Financial/BAO/FinancialTypeTest.php index c468d61527..0709998e7c 100644 --- a/tests/phpunit/CRM/Financial/BAO/FinancialTypeTest.php +++ b/tests/phpunit/CRM/Financial/BAO/FinancialTypeTest.php @@ -320,8 +320,8 @@ class CRM_Financial_BAO_FinancialTypeTest extends CiviUnitTestCase { CRM_Financial_BAO_FinancialType::checkPermissionedLineItems($contributions->id, 'view'); $this->fail('Missed expected exception'); } - catch (Exception $e) { - $this->assertEquals('A fatal error was triggered: You do not have permission to access this page.', $e->getMessage()); + catch (CRM_Core_Exception $e) { + $this->assertEquals('You do not have permission to access this page.', $e->getMessage()); } $this->setPermissions([ -- 2.25.1