From c33ba950bcfdd2657f7572c6698e49b5097e24b2 Mon Sep 17 00:00:00 2001 From: Coleman Watts Date: Mon, 27 Mar 2023 17:41:58 -0400 Subject: [PATCH] Refactor CRM_Core_BAO_Email to use DAO::writeRecord --- CRM/Core/BAO/Email.php | 149 +++++++++++------------ tests/phpunit/CRM/Core/BAO/EmailTest.php | 12 +- 2 files changed, 77 insertions(+), 84 deletions(-) diff --git a/CRM/Core/BAO/Email.php b/CRM/Core/BAO/Email.php index e7e4eb9a1d..173b697cf9 100644 --- a/CRM/Core/BAO/Email.php +++ b/CRM/Core/BAO/Email.php @@ -24,64 +24,54 @@ class CRM_Core_BAO_Email extends CRM_Core_DAO_Email implements Civi\Core\HookInt use CRM_Contact_AccessTrait; /** - * Create email address. - * - * Note that the create function calls 'add' but has more business logic. - * + * @deprecated * @param array $params - * Input parameters. - * - * @return object + * @return CRM_Core_BAO_Email */ public static function create($params) { - CRM_Core_BAO_Block::handlePrimary($params, get_class()); + // FIXME: switch CRM_Core_BAO_Block::create to call writeRecord (once Address, IM, Phone create functions go through it) + // then this can be uncommented: + // CRM_Core_Error::deprecatedFunctionWarning('writeRecord'); + return self::writeRecord($params); + } - $hook = empty($params['id']) ? 'create' : 'edit'; - CRM_Utils_Hook::pre($hook, 'Email', CRM_Utils_Array::value('id', $params), $params); + /** + * Event fired before modifying an Email. + * @param \Civi\Core\Event\PreEvent $event + */ + public static function self_hook_civicrm_pre(\Civi\Core\Event\PreEvent $event) { + if (in_array($event->action, ['create', 'edit'])) { + CRM_Core_BAO_Block::handlePrimary($event->params, get_class()); - $email = new CRM_Core_DAO_Email(); - $email->copyValues($params); - if (!empty($email->email)) { - // lower case email field to optimize queries - $strtolower = function_exists('mb_strtolower') ? 'mb_strtolower' : 'strtolower'; - $email->email = $strtolower($email->email); - } + if (!empty($event->params['email'])) { + // lower case email field to optimize queries + $event->params['email'] = mb_strtolower($event->params['email']); + } - // - // Since we're setting bulkmail for 1 of this contact's emails, first reset - // all their other emails to is_bulkmail false. We shouldn't set the current - // email to false even though we are about to reset it to avoid - // contaminating the changelog if logging is enabled. (only 1 email - // address can have is_bulkmail = true) - // - // Note setting a the is_bulkmail to '' in $params results in $email->is_bulkmail === 'null'. - // @see https://lab.civicrm.org/dev/core/-/issues/2254 - // - if ($email->is_bulkmail == 1 && !empty($params['contact_id']) && !self::isMultipleBulkMail()) { - $sql = " + // Since we're setting bulkmail for 1 of this contact's emails, first reset + // all their other emails to is_bulkmail false. We shouldn't set the current + // email to false even though we are about to reset it to avoid + // contaminating the changelog if logging is enabled. (only 1 email + // address can have is_bulkmail = true) + // + // Note setting a the is_bulkmail to '' in $params results in $email->is_bulkmail === 'null'. + // @see https://lab.civicrm.org/dev/core/-/issues/2254 + // + if (!empty($event->params['is_bulkmail']) && !empty($event->params['contact_id']) && !self::isMultipleBulkMail()) { + $sql = " UPDATE civicrm_email SET is_bulkmail = 0 -WHERE contact_id = {$params['contact_id']} +WHERE contact_id = {$event->params['contact_id']} "; - if ($hook === 'edit') { - $sql .= " AND id <> {$params['id']}"; + if ($event->action === 'edit') { + $sql .= " AND id <> {$event->params['id']}"; + } + CRM_Core_DAO::executeQuery($sql); } - CRM_Core_DAO::executeQuery($sql); - } - - // handle if email is on hold - self::holdEmail($email); - - $email->save(); - $contactId = (int) ($email->contact_id ?? CRM_Core_DAO::getFieldValue(__CLASS__, $email->id, 'contact_id')); - if ($contactId && $email->is_primary) { - $address = $email->email ?? CRM_Core_DAO::getFieldValue(__CLASS__, $email->id, 'email'); - self::updateContactName($contactId, $address); + // handle if email is on hold + self::holdEmail($event->params); } - - CRM_Utils_Hook::post($hook, 'Email', $email->id, $email); - return $email; } /** @@ -89,24 +79,29 @@ WHERE contact_id = {$params['contact_id']} * @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)) { + $email = $event->object; + if ($event->action !== 'delete' && !empty($email->is_primary) && !empty($email->contact_id)) { // update the UF user email if that has changed - CRM_Core_BAO_UFMatch::updateUFName($event->object->contact_id); + CRM_Core_BAO_UFMatch::updateUFName($email->contact_id); + } + if (in_array($event->action, ['create', 'edit'])) { + $contactId = (int) ($email->contact_id ?? CRM_Core_DAO::getFieldValue(__CLASS__, $email->id, 'contact_id')); + $isPrimary = ($email->is_primary ?? CRM_Core_DAO::getFieldValue(__CLASS__, $email->id, 'is_primary')); + if ($contactId && $isPrimary) { + $address = $email->email ?? CRM_Core_DAO::getFieldValue(__CLASS__, $email->id, 'email'); + self::updateContactName($contactId, $address); + } } } /** - * Takes an associative array and adds email. - * + * @deprecated * @param array $params - * (reference ) an assoc array of name/value pairs. - * - * @return object - * CRM_Core_BAO_Email object on success, null otherwise + * @return CRM_Core_BAO_Email */ - public static function add(&$params) { - CRM_Core_Error::deprecatedFunctionWarning('apiv4 create'); - return self::create($params); + public static function add($params) { + CRM_Core_Error::deprecatedFunctionWarning('writeRecord'); + return self::writeRecord($params); } /** @@ -230,26 +225,26 @@ ORDER BY e.is_primary DESC, email_id ASC "; } /** - * Set / reset hold status for an email + * Set / reset hold status for an email prior to saving * - * @param object $email - * Email object. + * @param array $email + * Email params to be saved. */ - public static function holdEmail(&$email) { - if ($email->id && $email->on_hold === NULL) { + public static function holdEmail(array &$email) { + if (!empty($email['id']) && !isset($email['on_hold'])) { // email is being updated but no change to on_hold. return; } - if ($email->on_hold === 'null' || $email->on_hold === NULL) { + if (!isset($email['on_hold']) || $email['on_hold'] === 'null') { // legacy handling, deprecated. - $email->on_hold = 0; + $email['on_hold'] = 0; } - $email->on_hold = (int) $email->on_hold; + $email['on_hold'] = (int) $email['on_hold']; //check for update mode - if ($email->id) { - $params = [1 => [$email->id, 'Integer']]; - if ($email->on_hold) { + if (!empty($email['id'])) { + $params = [1 => [$email['id'], 'Integer']]; + if ($email['on_hold']) { $sql = " SELECT id FROM civicrm_email @@ -257,11 +252,11 @@ WHERE id = %1 AND hold_date IS NULL "; if (CRM_Core_DAO::singleValueQuery($sql, $params)) { - $email->hold_date = date('YmdHis'); - $email->reset_date = 'null'; + $email['hold_date'] = date('YmdHis'); + $email['reset_date'] = 'null'; } } - elseif ($email->on_hold === 0) { + elseif ($email['on_hold'] === 0) { // we do this lookup to see if reset_date should be changed. $sql = " SELECT id @@ -272,16 +267,14 @@ AND reset_date IS NULL "; if (CRM_Core_DAO::singleValueQuery($sql, $params)) { //set reset date only if it is not set and if hold date is set - $email->on_hold = FALSE; - $email->hold_date = 'null'; - $email->reset_date = date('YmdHis'); + $email['on_hold'] = FALSE; + $email['hold_date'] = 'null'; + $email['reset_date'] = date('YmdHis'); } } } - else { - if ($email->on_hold) { - $email->hold_date = date('YmdHis'); - } + elseif ($email['on_hold']) { + $email['hold_date'] = date('YmdHis'); } } diff --git a/tests/phpunit/CRM/Core/BAO/EmailTest.php b/tests/phpunit/CRM/Core/BAO/EmailTest.php index b9712df66f..8a0f96e13a 100644 --- a/tests/phpunit/CRM/Core/BAO/EmailTest.php +++ b/tests/phpunit/CRM/Core/BAO/EmailTest.php @@ -30,7 +30,7 @@ class CRM_Core_BAO_EmailTest extends CiviUnitTestCase { 'contact_id' => $contactId, ]; - CRM_Core_BAO_Email::create($params); + CRM_Core_BAO_Email::writeRecord($params); $emailId = $this->assertDBNotNull('CRM_Core_DAO_Email', 'jane.doe@example.com', 'id', 'email', 'Database check for created email address.' @@ -44,7 +44,7 @@ class CRM_Core_BAO_EmailTest extends CiviUnitTestCase { 'on_hold' => 1, ]; - CRM_Core_BAO_Email::create($params); + CRM_Core_BAO_Email::writeRecord($params); $isBulkMail = $this->assertDBNotNull('CRM_Core_DAO_Email', $emailId, 'is_bulkmail', 'id', 'Database check on updated email record.' @@ -69,7 +69,7 @@ class CRM_Core_BAO_EmailTest extends CiviUnitTestCase { 'contact_id' => $contactId, ]; - CRM_Core_BAO_Email::create($params); + CRM_Core_BAO_Email::writeRecord($params); $emailId = $this->assertDBNotNull('CRM_Core_DAO_Email', 'jane.doe@example.com', 'id', 'email', 'Database check for created email address.' @@ -82,7 +82,7 @@ class CRM_Core_BAO_EmailTest extends CiviUnitTestCase { 'on_hold' => 1, ]; - CRM_Core_BAO_Email::create($params); + CRM_Core_BAO_Email::writeRecord($params); // Use assertDBNotNull to get back value of hold_date and check if it's in the current year. // NOTE: The assertEquals will fail IF this test is run just as the year is changing (low likelihood). @@ -105,7 +105,7 @@ class CRM_Core_BAO_EmailTest extends CiviUnitTestCase { 'on_hold' => 2, ]; - CRM_Core_BAO_Email::create($params); + CRM_Core_BAO_Email::writeRecord($params); // Use assertDBNotNull to get back value of hold_date and check that it's in the current year. // NOTE: The assertEquals will fail IF this test is run just as the year is changing (low likelihood). @@ -128,7 +128,7 @@ class CRM_Core_BAO_EmailTest extends CiviUnitTestCase { 'on_hold' => 'null', ]; - CRM_Core_BAO_Email::create($params); + CRM_Core_BAO_Email::writeRecord($params); $this->assertDBCompareValue('CRM_Core_DAO_Email', $emailId, 'on_hold', 'id', 0, 'Check if on_hold=0 in updated email record.' ); -- 2.25.1