From be12df5ab37429c6f227ead9297bcd551fbaa645 Mon Sep 17 00:00:00 2001 From: eileen Date: Fri, 27 Nov 2020 20:05:06 +1300 Subject: [PATCH] Remove use of nullArray in delete hooks Looking at #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 --- CRM/Batch/BAO/Batch.php | 2 +- CRM/Campaign/BAO/Campaign.php | 2 +- CRM/Case/BAO/Case.php | 2 +- CRM/Contact/BAO/Contact.php | 4 ++-- CRM/Contact/BAO/Group.php | 2 +- CRM/Contact/BAO/Relationship.php | 2 +- CRM/Contribute/BAO/Contribution.php | 2 +- CRM/Core/BAO/UFGroup.php | 4 ++++ CRM/Event/BAO/Event.php | 2 +- CRM/Event/BAO/Participant.php | 2 +- CRM/Grant/BAO/Grant.php | 2 +- CRM/Mailing/BAO/Mailing.php | 2 +- CRM/Mailing/BAO/MailingAB.php | 2 +- CRM/Mailing/BAO/MailingJob.php | 2 +- CRM/PCP/BAO/PCP.php | 2 +- CRM/Pledge/BAO/Pledge.php | 2 +- CRM/Pledge/BAO/PledgeBlock.php | 2 +- CRM/Utils/Hook.php | 2 +- Civi/Api4/Generic/Traits/CustomValueActionTrait.php | 2 +- 19 files changed, 23 insertions(+), 19 deletions(-) diff --git a/CRM/Batch/BAO/Batch.php b/CRM/Batch/BAO/Batch.php index 3bb93f140b..e9fea4c3fb 100644 --- a/CRM/Batch/BAO/Batch.php +++ b/CRM/Batch/BAO/Batch.php @@ -119,7 +119,7 @@ class CRM_Batch_BAO_Batch extends CRM_Batch_DAO_Batch { */ 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 index 7800cf925d..1aa324805e 100644 --- a/CRM/Campaign/BAO/Campaign.php +++ b/CRM/Campaign/BAO/Campaign.php @@ -84,7 +84,7 @@ class CRM_Campaign_BAO_Campaign extends CRM_Campaign_DAO_Campaign { 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 index 96a511e69b..9d7a008c89 100644 --- a/CRM/Case/BAO/Case.php +++ b/CRM/Case/BAO/Case.php @@ -187,7 +187,7 @@ WHERE civicrm_case.id = %1"; * 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 index 8d1095747e..fb643f47c3 100644 --- a/CRM/Contact/BAO/Contact.php +++ b/CRM/Contact/BAO/Contact.php @@ -3473,9 +3473,9 @@ LEFT JOIN civicrm_address ON ( civicrm_address.contact_id = civicrm_contact.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 index 5861b05757..138fd9e2d3 100644 --- a/CRM/Contact/BAO/Group.php +++ b/CRM/Contact/BAO/Group.php @@ -55,7 +55,7 @@ class CRM_Contact_BAO_Group extends CRM_Contact_DAO_Group { 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 index 85a8f53822..8df124f1c4 100644 --- a/CRM/Contact/BAO/Relationship.php +++ b/CRM/Contact/BAO/Relationship.php @@ -717,7 +717,7 @@ class CRM_Contact_BAO_Relationship extends CRM_Contact_DAO_Relationship { */ 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 index 5545936098..236c1c203e 100644 --- a/CRM/Contribute/BAO/Contribution.php +++ b/CRM/Contribute/BAO/Contribution.php @@ -1491,7 +1491,7 @@ INNER JOIN civicrm_contact contact ON ( contact.id = c.contact_id ) * $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 index 988be26421..20fd1ca049 100644 --- a/CRM/Core/BAO/UFGroup.php +++ b/CRM/Core/BAO/UFGroup.php @@ -1399,6 +1399,8 @@ class CRM_Core_BAO_UFGroup extends CRM_Core_DAO_UFGroup { * */ 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 @@ class CRM_Core_BAO_UFGroup extends CRM_Core_DAO_UFGroup { $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 index 26bb7f2765..6adb77093d 100644 --- a/CRM/Event/BAO/Event.php +++ b/CRM/Event/BAO/Event.php @@ -171,7 +171,7 @@ class CRM_Event_BAO_Event extends CRM_Event_DAO_Event { 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 index 9818d88898..9c726376c9 100644 --- a/CRM/Event/BAO/Participant.php +++ b/CRM/Event/BAO/Participant.php @@ -843,7 +843,7 @@ WHERE civicrm_participant.id = {$participantId} 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 index e203d546f5..bc33e04c80 100644 --- a/CRM/Grant/BAO/Grant.php +++ b/CRM/Grant/BAO/Grant.php @@ -252,7 +252,7 @@ class CRM_Grant_BAO_Grant extends CRM_Grant_DAO_Grant { * @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 index 8a1e952909..f28c1f17f5 100644 --- a/CRM/Mailing/BAO/Mailing.php +++ b/CRM/Mailing/BAO/Mailing.php @@ -2463,7 +2463,7 @@ LEFT JOIN civicrm_mailing_group g ON g.mailing_id = m.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 index 7d26a2c100..e035ec6962 100644 --- a/CRM/Mailing/BAO/MailingAB.php +++ b/CRM/Mailing/BAO/MailingAB.php @@ -61,7 +61,7 @@ class CRM_Mailing_BAO_MailingAB extends CRM_Mailing_DAO_MailingAB { 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 index 228406bb99..020c65b7e1 100644 --- a/CRM/Mailing/BAO/MailingJob.php +++ b/CRM/Mailing/BAO/MailingJob.php @@ -1122,7 +1122,7 @@ AND record_type_id = $targetRecordID * @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 index 6fdd36d027..9b6afa3dc5 100644 --- a/CRM/PCP/BAO/PCP.php +++ b/CRM/PCP/BAO/PCP.php @@ -340,7 +340,7 @@ WHERE pcp.id = %1 AND cc.contribution_status_id = %2 AND cc.is_test = 0"; * 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 index 9bc13cda31..299e3ffa56 100644 --- a/CRM/Pledge/BAO/Pledge.php +++ b/CRM/Pledge/BAO/Pledge.php @@ -285,7 +285,7 @@ class CRM_Pledge_BAO_Pledge extends CRM_Pledge_DAO_Pledge { * @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 index a3b80ff7d8..68304860c2 100644 --- a/CRM/Pledge/BAO/PledgeBlock.php +++ b/CRM/Pledge/BAO/PledgeBlock.php @@ -93,7 +93,7 @@ class CRM_Pledge_BAO_PledgeBlock extends CRM_Pledge_DAO_PledgeBlock { * @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 index 47233dd4bd..f173abac3b 100644 --- a/CRM/Utils/Hook.php +++ b/CRM/Utils/Hook.php @@ -339,7 +339,7 @@ abstract class CRM_Utils_Hook { * @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 index 2fed2b7f46..5a7a0e3312 100644 --- a/Civi/Api4/Generic/Traits/CustomValueActionTrait.php +++ b/Civi/Api4/Generic/Traits/CustomValueActionTrait.php @@ -83,7 +83,7 @@ trait CustomValueActionTrait { $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']) -- 2.25.1