From 7a59253523453fd58b5b5ccb88a3d442232481f8 Mon Sep 17 00:00:00 2001 From: Coleman Watts Date: Sat, 21 Aug 2021 20:00:32 -0400 Subject: [PATCH] Move Email, Address, etc. is_primary handling on delete to a hook Previously this was all done in a delegated function. This moves that logic to a hook listener. --- CRM/Contact/BAO/Contact.php | 58 ++++++------------- CRM/Core/BAO/Address.php | 6 +- CRM/Core/BAO/Email.php | 24 +++++--- CRM/Core/BAO/IM.php | 6 +- CRM/Core/BAO/OpenID.php | 6 +- CRM/Core/BAO/Phone.php | 6 +- CRM/Core/DAO.php | 7 ++- tests/phpunit/api/v4/Entity/AddressTest.php | 62 +++++++++++++++++++++ 8 files changed, 118 insertions(+), 57 deletions(-) create mode 100644 tests/phpunit/api/v4/Entity/AddressTest.php diff --git a/CRM/Contact/BAO/Contact.php b/CRM/Contact/BAO/Contact.php index cb855c0b46..ab2850c07b 100644 --- a/CRM/Contact/BAO/Contact.php +++ b/CRM/Contact/BAO/Contact.php @@ -14,7 +14,7 @@ * @package CRM * @copyright CiviCRM LLC https://civicrm.org/licensing */ -class CRM_Contact_BAO_Contact extends CRM_Contact_DAO_Contact { +class CRM_Contact_BAO_Contact extends CRM_Contact_DAO_Contact implements Civi\Test\HookInterface { /** * SQL function used to format the phone_numeric field via trigger. @@ -3511,53 +3511,31 @@ LEFT JOIN civicrm_address ON ( civicrm_address.contact_id = civicrm_contact.id ) } /** - * Delete a contact-related object that has an 'is_primary' field. - * - * Ensures that is_primary gets assigned to another object if available - * Also calls pre/post hooks - * - * @param string $type - * @param int $id - * - * @return bool + * Event fired after modifying any entity. + * @param \Civi\Core\Event\PostEvent $event */ - public static function deleteObjectWithPrimary($type, $id) { - if (!$id || !is_numeric($id)) { - return FALSE; - } - $daoName = "CRM_Core_DAO_$type"; - $obj = new $daoName(); - $obj->id = $id; - $obj->find(); - - if ($obj->fetch()) { - CRM_Utils_Hook::pre('delete', $type, $id); - $contactId = $obj->contact_id; - $obj->delete(); - } - else { - return FALSE; - } - // is_primary is only relavent if this field belongs to a contact - if ($contactId) { - $dao = new $daoName(); - $dao->contact_id = $contactId; + public static function on_hook_civicrm_post(\Civi\Core\Event\PostEvent $event) { + // Handle deleting a related entity with is_primary + $hasPrimary = ['Address', 'Email', 'IM', 'OpenID', 'Phone']; + if ( + $event->action === 'delete' && $event->id && + in_array($event->entity, $hasPrimary) && + !empty($event->object->is_primary) && + !empty($event->object->contact_id) + ) { + $daoClass = CRM_Core_DAO_AllCoreTables::getFullName($event->entity); + $dao = new $daoClass(); + $dao->contact_id = $event->object->contact_id; $dao->is_primary = 1; // Pick another record to be primary (if one isn't already) if (!$dao->find(TRUE)) { $dao->is_primary = 0; - $dao->find(); - if ($dao->fetch()) { - $dao->is_primary = 1; - $dao->save(); - if ($type === 'Email') { - CRM_Core_BAO_UFMatch::updateUFName($dao->contact_id); - } + if ($dao->find(TRUE)) { + $baoClass = CRM_Core_DAO_AllCoreTables::getBAOClassName($daoClass); + $baoClass::writeRecord(['id' => $dao->id, 'is_primary' => 1]); } } } - CRM_Utils_Hook::post('delete', $type, $id, $obj); - return TRUE; } /** diff --git a/CRM/Core/BAO/Address.php b/CRM/Core/BAO/Address.php index 480501d815..f7c8ca8ecc 100644 --- a/CRM/Core/BAO/Address.php +++ b/CRM/Core/BAO/Address.php @@ -1210,12 +1210,14 @@ SELECT is_primary, /** * Call common delete function. * - * @param int $id + * @see \CRM_Contact_BAO_Contact::on_hook_civicrm_post * + * @param int $id + * @deprecated * @return bool */ public static function del($id) { - return CRM_Contact_BAO_Contact::deleteObjectWithPrimary('Address', $id); + return (bool) self::deleteRecord(['id' => $id]); } /** diff --git a/CRM/Core/BAO/Email.php b/CRM/Core/BAO/Email.php index 5341997ecb..1261608d23 100644 --- a/CRM/Core/BAO/Email.php +++ b/CRM/Core/BAO/Email.php @@ -20,7 +20,7 @@ use Civi\Api4\Email; /** * This class contains functions for email handling. */ -class CRM_Core_BAO_Email extends CRM_Core_DAO_Email { +class CRM_Core_BAO_Email extends CRM_Core_DAO_Email implements Civi\Test\HookInterface { use CRM_Contact_AccessTrait; /** @@ -80,15 +80,21 @@ WHERE contact_id = {$params['contact_id']} self::updateContactName($contactId, $address); } - if ($email->is_primary) { - // update the UF user email if that has changed - CRM_Core_BAO_UFMatch::updateUFName($email->contact_id); - } - CRM_Utils_Hook::post($hook, 'Email', $email->id, $email); return $email; } + /** + * Event fired after modifying an Email. + * @param \Civi\Core\Event\PostEvent $event + */ + public static function self_hook_civicrm_post(\Civi\Core\Event\PostEvent $event) { + if ($event->action !== 'delete' && !empty($event->object->is_primary) && !empty($event->object->contact_id)) { + // update the UF user email if that has changed + CRM_Core_BAO_UFMatch::updateUFName($event->object->contact_id); + } + } + /** * Takes an associative array and adds email. * @@ -342,12 +348,14 @@ AND reset_date IS NULL /** * Call common delete function. * - * @param int $id + * @see \CRM_Contact_BAO_Contact::on_hook_civicrm_post * + * @param int $id + * @deprecated * @return bool */ public static function del($id) { - return CRM_Contact_BAO_Contact::deleteObjectWithPrimary('Email', $id); + return (bool) self::deleteRecord(['id' => $id]); } /** diff --git a/CRM/Core/BAO/IM.php b/CRM/Core/BAO/IM.php index 12fc6238d4..a26e6c4ea3 100644 --- a/CRM/Core/BAO/IM.php +++ b/CRM/Core/BAO/IM.php @@ -158,12 +158,14 @@ ORDER BY cim.is_primary DESC, im_id ASC "; /** * Call common delete function. * - * @param int $id + * @see \CRM_Contact_BAO_Contact::on_hook_civicrm_post * + * @param int $id + * @deprecated * @return bool */ public static function del($id) { - return CRM_Contact_BAO_Contact::deleteObjectWithPrimary('IM', $id); + return (bool) self::deleteRecord(['id' => $id]); } } diff --git a/CRM/Core/BAO/OpenID.php b/CRM/Core/BAO/OpenID.php index 33a7c269fc..2b16d2c702 100644 --- a/CRM/Core/BAO/OpenID.php +++ b/CRM/Core/BAO/OpenID.php @@ -121,12 +121,14 @@ ORDER BY /** * Call common delete function. * - * @param int $id + * @see \CRM_Contact_BAO_Contact::on_hook_civicrm_post * + * @param int $id + * @deprecated * @return bool */ public static function del($id) { - return CRM_Contact_BAO_Contact::deleteObjectWithPrimary('OpenID', $id); + return (bool) self::deleteRecord(['id' => $id]); } } diff --git a/CRM/Core/BAO/Phone.php b/CRM/Core/BAO/Phone.php index 260b2e6973..6f9da9eccb 100644 --- a/CRM/Core/BAO/Phone.php +++ b/CRM/Core/BAO/Phone.php @@ -234,12 +234,14 @@ ORDER BY ph.is_primary DESC, phone_id ASC "; /** * Call common delete function. * - * @param int $id + * @see \CRM_Contact_BAO_Contact::on_hook_civicrm_post * + * @param int $id + * @deprecated * @return bool */ public static function del($id) { - return CRM_Contact_BAO_Contact::deleteObjectWithPrimary('Phone', $id); + return (bool) self::deleteRecord(['id' => $id]); } } diff --git a/CRM/Core/DAO.php b/CRM/Core/DAO.php index 04c63d5a63..774b3548b6 100644 --- a/CRM/Core/DAO.php +++ b/CRM/Core/DAO.php @@ -962,9 +962,14 @@ class CRM_Core_DAO extends DB_DataObject { CRM_Utils_Hook::pre('delete', $entityName, $record['id'], $record); $instance = new $className(); $instance->id = $record['id']; - if (!$instance->delete()) { + // Load complete object for the sake of hook_civicrm_post, below + $instance->find(TRUE); + if (!$instance || !$instance->delete()) { throw new CRM_Core_Exception("Could not delete {$entityName} id {$record['id']}"); } + // For other operations this hook is passed an incomplete object and hook listeners can load if needed. + // But that's not possible with delete because it's gone from the database by the time this hook is called. + // So in this case the object has been pre-loaded so hook listeners have access to the complete record. CRM_Utils_Hook::post('delete', $entityName, $record['id'], $instance); return $instance; diff --git a/tests/phpunit/api/v4/Entity/AddressTest.php b/tests/phpunit/api/v4/Entity/AddressTest.php new file mode 100644 index 0000000000..c3d8f83947 --- /dev/null +++ b/tests/phpunit/api/v4/Entity/AddressTest.php @@ -0,0 +1,62 @@ +addValue('first_name', uniqid())->execute()->single()['id']; + + $a1 = Address::create(FALSE) + ->addValue('is_primary', TRUE) + ->addValue('contact_id', $cid) + ->addValue('location_type_id', 1) + ->addValue('city', 'Somewhere') + ->execute(); + + $a2 = Address::create(FALSE) + ->addValue('is_primary', TRUE) + ->addValue('contact_id', $cid) + ->addValue('location_type_id', 2) + ->addValue('city', 'Elsewhere') + ->execute(); + + $addresses = Address::get(FALSE) + ->addWhere('contact_id', '=', $cid) + ->addOrderBy('id') + ->execute(); + + $this->assertFalse($addresses[0]['is_primary']); + $this->assertTrue($addresses[1]['is_primary']); + } + +} -- 2.25.1