Remove use of nullArray in delete hooks

Looking at civicrm#18995 I was struck by the delete hook being non-standard. In general we don't load from the DB just to pass to the hook as the hook can do that itself. However, when I looked at other hooks I found that they were passing out nullArray - this is a legacy method that precedes being on a php version that supported default params when passing by reference. It's further known to introduce intermittent hard to debug issues. This adds the new hook and also adds standardisation to the other hooks for pre+delete. I've left the create one for now but GroupContact is a good example of something close to what we want to standardise on there
eileenmcnaughton · Nov 27, 2020 · be12df5 · be12df5
1 parent 9219cf5
commit be12df5
Show file tree

Hide file tree

Showing 19 changed files with 23 additions and 19 deletions.
diff --git a/CRM/Batch/BAO/Batch.php b/CRM/Batch/BAO/Batch.php
@@ -119,7 +119,7 @@ public static function generateBatchName() {
    */
   public static function deleteBatch($batchId) {
     // delete entry from batch table
-    CRM_Utils_Hook::pre('delete', 'Batch', $batchId, CRM_Core_DAO::$_nullArray);
+    CRM_Utils_Hook::pre('delete', 'Batch', $batchId);
     $batch = new CRM_Batch_DAO_Batch();
     $batch->id = $batchId;
     $batch->delete();

diff --git a/CRM/Campaign/BAO/Campaign.php b/CRM/Campaign/BAO/Campaign.php
@@ -84,7 +84,7 @@ public static function del($id) {
       return FALSE;
     }
 
-    CRM_Utils_Hook::pre('delete', 'Campaign', $id, CRM_Core_DAO::$_nullArray);
+    CRM_Utils_Hook::pre('delete', 'Campaign', $id);
 
     $dao = new CRM_Campaign_DAO_Campaign();
     $dao->id = $id;

diff --git a/CRM/Case/BAO/Case.php b/CRM/Case/BAO/Case.php
@@ -187,7 +187,7 @@ public static function getCaseType($caseId, $colName = 'title') {
    *   is successful
    */
   public static function deleteCase($caseId, $moveToTrash = FALSE) {
-    CRM_Utils_Hook::pre('delete', 'Case', $caseId, CRM_Core_DAO::$_nullArray);
+    CRM_Utils_Hook::pre('delete', 'Case', $caseId);
 
     //delete activities
     $activities = self::getCaseActivityDates($caseId);

diff --git a/CRM/Contact/BAO/Contact.php b/CRM/Contact/BAO/Contact.php
@@ -3473,9 +3473,9 @@ public static function deleteObjectWithPrimary($type, $id) {
     $obj = new $daoName();
     $obj->id = $id;
     $obj->find();
-    $hookParams = [];
+
     if ($obj->fetch()) {
-      CRM_Utils_Hook::pre('delete', $type, $id, $hookParams);
+      CRM_Utils_Hook::pre('delete', $type, $id);
       $contactId = $obj->contact_id;
       $obj->delete();
     }

diff --git a/CRM/Contact/BAO/Group.php b/CRM/Contact/BAO/Group.php
@@ -55,7 +55,7 @@ public static function discard($id) {
     if (!$id || !is_numeric($id)) {
       throw new CRM_Core_Exception('Invalid group request attempted');
     }
-    CRM_Utils_Hook::pre('delete', 'Group', $id, CRM_Core_DAO::$_nullArray);
+    CRM_Utils_Hook::pre('delete', 'Group', $id);
 
     $transaction = new CRM_Core_Transaction();
 

diff --git a/CRM/Contact/BAO/Relationship.php b/CRM/Contact/BAO/Relationship.php
@@ -717,7 +717,7 @@ public static function clearCurrentEmployer($id, $action) {
    */
   public static function del($id) {
     // delete from relationship table
-    CRM_Utils_Hook::pre('delete', 'Relationship', $id, CRM_Core_DAO::$_nullArray);
+    CRM_Utils_Hook::pre('delete', 'Relationship', $id);
 
     $relationship = self::clearCurrentEmployer($id, CRM_Core_Action::DELETE);
     $relationship->delete();

diff --git a/CRM/Contribute/BAO/Contribution.php b/CRM/Contribute/BAO/Contribution.php
@@ -1491,7 +1491,7 @@ public static function getTotalAmountAndCount($status = NULL, $startDate = NULL,
    *   $results no of deleted Contribution on success, false otherwise
    */
   public static function deleteContribution($id) {
-    CRM_Utils_Hook::pre('delete', 'Contribution', $id, CRM_Core_DAO::$_nullArray);
+    CRM_Utils_Hook::pre('delete', 'Contribution', $id);
 
     $transaction = new CRM_Core_Transaction();
 

diff --git a/CRM/Core/BAO/UFGroup.php b/CRM/Core/BAO/UFGroup.php
@@ -1399,6 +1399,8 @@ public static function usedByModule($id) {
    *
    */
   public static function del($id) {
+    CRM_Utils_Hook::pre('delete', 'UFGroup', $id);
+
     //check whether this group contains  any profile fields
     $profileField = new CRM_Core_DAO_UFField();
     $profileField->uf_group_id = $id;
@@ -1416,6 +1418,8 @@ public static function del($id) {
     $group = new CRM_Core_DAO_UFGroup();
     $group->id = $id;
     $group->delete();
+
+    CRM_Utils_Hook::post('delete', 'UFGroup', $id, $group);
     return 1;
   }
 

diff --git a/CRM/Event/BAO/Event.php b/CRM/Event/BAO/Event.php
@@ -171,7 +171,7 @@ public static function del($id) {
       return NULL;
     }
 
-    CRM_Utils_Hook::pre('delete', 'Event', $id, CRM_Core_DAO::$_nullArray);
+    CRM_Utils_Hook::pre('delete', 'Event', $id);
 
     $extends = ['event'];
     $groupTree = CRM_Core_BAO_CustomGroup::getGroupDetail(NULL, NULL, $extends);

diff --git a/CRM/Event/BAO/Participant.php b/CRM/Event/BAO/Participant.php
@@ -843,7 +843,7 @@ public static function deleteParticipant($id) {
     if (!$participant->find()) {
       return FALSE;
     }
-    CRM_Utils_Hook::pre('delete', 'Participant', $id, CRM_Core_DAO::$_nullArray);
+    CRM_Utils_Hook::pre('delete', 'Participant', $id);
 
     $transaction = new CRM_Core_Transaction();
 

diff --git a/CRM/Grant/BAO/Grant.php b/CRM/Grant/BAO/Grant.php
@@ -252,7 +252,7 @@ public static function deleteContact($id) {
    * @return bool|mixed
    */
   public static function del($id) {
-    CRM_Utils_Hook::pre('delete', 'Grant', $id, CRM_Core_DAO::$_nullArray);
+    CRM_Utils_Hook::pre('delete', 'Grant', $id);
 
     $grant = new CRM_Grant_DAO_Grant();
     $grant->id = $id;

diff --git a/CRM/Mailing/BAO/Mailing.php b/CRM/Mailing/BAO/Mailing.php
@@ -2463,7 +2463,7 @@ public static function del($id) {
       throw new CRM_Core_Exception(ts('No id passed to mailing del function'));
     }
 
-    CRM_Utils_Hook::pre('delete', 'Mailing', $id, CRM_Core_DAO::$_nullArray);
+    CRM_Utils_Hook::pre('delete', 'Mailing', $id);
 
     // delete all file attachments
     CRM_Core_BAO_File::deleteEntityFile('civicrm_mailing',

diff --git a/CRM/Mailing/BAO/MailingAB.php b/CRM/Mailing/BAO/MailingAB.php
@@ -61,7 +61,7 @@ public static function del($id) {
       throw new CRM_Core_Exception(ts('No id passed to MailingAB del function'));
     }
     CRM_Core_Transaction::create()->run(function () use ($id) {
-      CRM_Utils_Hook::pre('delete', 'MailingAB', $id, CRM_Core_DAO::$_nullArray);
+      CRM_Utils_Hook::pre('delete', 'MailingAB', $id);
 
       $dao = new CRM_Mailing_DAO_MailingAB();
       $dao->id = $id;

diff --git a/CRM/Mailing/BAO/MailingJob.php b/CRM/Mailing/BAO/MailingJob.php
@@ -1122,7 +1122,7 @@ public static function findPendingTasks($jobId, $medium) {
    * @return mixed
    */
   public static function del($id) {
-    CRM_Utils_Hook::pre('delete', 'MailingJob', $id, CRM_Core_DAO::$_nullArray);
+    CRM_Utils_Hook::pre('delete', 'MailingJob', $id);
 
     $jobDAO = new CRM_Mailing_BAO_MailingJob();
     $jobDAO->id = $id;

diff --git a/CRM/PCP/BAO/PCP.php b/CRM/PCP/BAO/PCP.php
@@ -340,7 +340,7 @@ public static function &pcpLinks($pcpId = NULL) {
    *   Campaign page id.
    */
   public static function deleteById($id) {
-    CRM_Utils_Hook::pre('delete', 'Campaign', $id, CRM_Core_DAO::$_nullArray);
+    CRM_Utils_Hook::pre('delete', 'Campaign', $id);
 
     $transaction = new CRM_Core_Transaction();
 

diff --git a/CRM/Pledge/BAO/Pledge.php b/CRM/Pledge/BAO/Pledge.php
@@ -285,7 +285,7 @@ protected static function isPaymentsRequireRecalculation($params) {
    * @return mixed
    */
   public static function deletePledge($id) {
-    CRM_Utils_Hook::pre('delete', 'Pledge', $id, CRM_Core_DAO::$_nullArray);
+    CRM_Utils_Hook::pre('delete', 'Pledge', $id);
 
     $transaction = new CRM_Core_Transaction();
 

diff --git a/CRM/Pledge/BAO/PledgeBlock.php b/CRM/Pledge/BAO/PledgeBlock.php
@@ -93,7 +93,7 @@ public static function add($params) {
    * @return mixed|null
    */
   public static function deletePledgeBlock($id) {
-    CRM_Utils_Hook::pre('delete', 'PledgeBlock', $id, CRM_Core_DAO::$_nullArray);
+    CRM_Utils_Hook::pre('delete', 'PledgeBlock', $id);
 
     $transaction = new CRM_Core_Transaction();
 

diff --git a/CRM/Utils/Hook.php b/CRM/Utils/Hook.php
@@ -339,7 +339,7 @@ public function requireCiviModules(&$moduleList) {
    * @return null
    *   the return value is ignored
    */
-  public static function pre($op, $objectName, $id, &$params) {
+  public static function pre($op, $objectName, $id, &$params = []) {
     $event = new \Civi\Core\Event\PreEvent($op, $objectName, $id, $params);
     \Civi::dispatcher()->dispatch('hook_civicrm_pre', $event);
     return $event->getReturnValues();

diff --git a/Civi/Api4/Generic/Traits/CustomValueActionTrait.php b/Civi/Api4/Generic/Traits/CustomValueActionTrait.php
@@ -83,7 +83,7 @@ protected function deleteObjects($items) {
     $customTable = CoreUtil::getTableName($this->getEntityName());
     $ids = [];
     foreach ($items as $item) {
-      \CRM_Utils_Hook::pre('delete', $this->getEntityName(), $item['id'], \CRM_Core_DAO::$_nullArray);
+      \CRM_Utils_Hook::pre('delete', $this->getEntityName(), $item['id']);
       \CRM_Utils_SQL_Delete::from($customTable)
         ->where('id = #value')
         ->param('#value', $item['id'])