Tidy up on_hold handling in holdEmail function.

Follow up on https://github.com/civicrm/civicrm-core/pull/11807

diff --git a/CRM/Core/BAO/Email.php b/CRM/Core/BAO/Email.php
index e1370ca43317d8e24835d191cc8000cdeef3c7af..0718811723db62ea5db09968eb44b3d75ac74fba 100644 (file)
--- a/CRM/Core/BAO/Email.php
+++ b/CRM/Core/BAO/Email.php
@@ -237,13 +237,20 @@ ORDER BY e.is_primary DESC, email_id ASC ";
    *   Email object.
    */
   public static function holdEmail(&$email) {
-    if (!($email->on_hold === 'null' || $email->on_hold === NULL)) {
-      $email->on_hold = intval($email->on_hold);
+    if ($email->id && $email->on_hold === NULL) {
+      // email is being updated but no change to on_hold.
+      return;
     }
+    if ($email->on_hold === 'null' || $email->on_hold === NULL) {
+      // legacy handling, deprecated.
+      $email->on_hold = 0;
+    }
+    $email->on_hold = (int) $email->on_hold;
+
     //check for update mode
     if ($email->id) {
       $params = array(1 => array($email->id, 'Integer'));
-      if ($email->on_hold && $email->on_hold != 'null') {
+      if ($email->on_hold) {
         $sql = "
 SELECT id
 FROM   civicrm_email
@@ -255,7 +262,8 @@ AND    hold_date IS NULL
           $email->reset_date = 'null';
         }
       }
-      elseif ($email->on_hold == 'null') {
+      elseif ($email->on_hold === 0) {
+        // we do this lookup to see if reset_date should be changed.
         $sql = "
 SELECT id
 FROM   civicrm_email
@@ -272,7 +280,7 @@ AND    reset_date IS NULL
       }
     }
     else {
-      if (($email->on_hold != 'null') && $email->on_hold) {
+      if ($email->on_hold) {
         $email->hold_date = date('YmdHis');
       }
     }