core#1590: Don't send reminders to add'l recipients on deleted/inactive/template...

support reminders with event types/templates and multiple event/type/template IDs

fix

diff --git a/CRM/Activity/ActionMapping.php b/CRM/Activity/ActionMapping.php
index 05abcf9e24c99c846f76da938a81dae38c80016b..9f73592644b7378f9dcacf92da3eb4fa76cd88d1 100644 (file)
--- a/CRM/Activity/ActionMapping.php
+++ b/CRM/Activity/ActionMapping.php
@@ -118,4 +118,12 @@ class CRM_Activity_ActionMapping extends \Civi\ActionSchedule\Mapping {
     return $query;
   }
 
+  /**
+   * Determine whether a schedule based on this mapping should
+   * send to additional contacts.
+   */
+  public function sendToAdditional($entityId): bool {
+    return TRUE;
+  }
+
 }

diff --git a/CRM/Contact/ActionMapping.php b/CRM/Contact/ActionMapping.php
index f4aee154e83038bff31a4bd9381728d63b45bdf3..ea96b5da7863645e387c72106fbb3b7d4fa47fe4 100644 (file)
--- a/CRM/Contact/ActionMapping.php
+++ b/CRM/Contact/ActionMapping.php
@@ -139,4 +139,12 @@ class CRM_Contact_ActionMapping extends \Civi\ActionSchedule\Mapping {
     return $query;
   }
 
+  /**
+   * Determine whether a schedule based on this mapping should
+   * send to additional contacts.
+   */
+  public function sendToAdditional($entityId): bool {
+    return TRUE;
+  }
+
 }

diff --git a/CRM/Contribute/ActionMapping/ByPage.php b/CRM/Contribute/ActionMapping/ByPage.php
index 8c09c81cc274faecc894c526c9be23cf13041d74..6dcab6e2a7aeb77f9d88a784731f18b69160c3a9 100644 (file)
--- a/CRM/Contribute/ActionMapping/ByPage.php
+++ b/CRM/Contribute/ActionMapping/ByPage.php
@@ -216,4 +216,12 @@ class CRM_Contribute_ActionMapping_ByPage implements \Civi\ActionSchedule\Mappin
     return FALSE;
   }
 
+  /**
+   * Determine whether a schedule based on this mapping should
+   * send to additional contacts.
+   */
+  public function sendToAdditional($entityId): bool {
+    return TRUE;
+  }
+
 }

diff --git a/CRM/Contribute/ActionMapping/ByType.php b/CRM/Contribute/ActionMapping/ByType.php
index ea259901a569be5d106631db48688813c5a9dacf..88d60e49c767eb078edb1e8b9f2e70bf9120ccac 100644 (file)
--- a/CRM/Contribute/ActionMapping/ByType.php
+++ b/CRM/Contribute/ActionMapping/ByType.php
@@ -235,4 +235,12 @@ class CRM_Contribute_ActionMapping_ByType implements \Civi\ActionSchedule\Mappin
     return FALSE;
   }
 
+  /**
+   * Determine whether a schedule based on this mapping should
+   * send to additional contacts.
+   */
+  public function sendToAdditional($entityId): bool {
+    return TRUE;
+  }
+
 }

diff --git a/CRM/Event/ActionMapping.php b/CRM/Event/ActionMapping.php
index 77fa8d77edfb6fe72abc2b8920a6944a1cf2a694..9c8aaae7a5070c7b28ba118b9cb23cfe76f7819d 100644 (file)
--- a/CRM/Event/ActionMapping.php
+++ b/CRM/Event/ActionMapping.php
@@ -159,6 +159,7 @@ class CRM_Event_ActionMapping extends \Civi\ActionSchedule\Mapping {
     }
 
     // build where clause
+    // FIXME: This handles scheduled reminder of type "Event Name" and "Event Type", gives incorrect result on "Event Template".
     if (!empty($selectedValues)) {
       $valueField = ($this->id == \CRM_Event_ActionMapping::EVENT_TYPE_MAPPING_ID) ? 'event_type_id' : 'id';
       $query->where("r.{$valueField} IN (@selectedValues)")
@@ -186,4 +187,49 @@ class CRM_Event_ActionMapping extends \Civi\ActionSchedule\Mapping {
     return $query;
   }
 
+  /**
+   * Determine whether a schedule based on this mapping should
+   * send to additional contacts.
+   *
+   * @param string $entityId Either an event ID/event type ID, or a set of event IDs/types separated
+   *  by the separation character.
+   */
+  public function sendToAdditional($entityId): bool {
+    $selectedValues = (array) \CRM_Utils_Array::explodePadded($entityId);
+    switch ($this->id) {
+      case self::EVENT_TYPE_MAPPING_ID:
+        $valueTable = 'e';
+        $valueField = 'event_type_id';
+        $templateReminder = FALSE;
+        break;
+
+      case self::EVENT_NAME_MAPPING_ID:
+        $valueTable = 'e';
+        $valueField = 'id';
+        $templateReminder = FALSE;
+        break;
+
+      case self::EVENT_TPL_MAPPING_ID:
+        $valueTable = 't';
+        $valueField = 'id';
+        $templateReminder = TRUE;
+        break;
+    }
+    // Don't send to additional recipients if this event is deleted or a template.
+    $query = new \CRM_Utils_SQL_Select('civicrm_event e');
+    $query
+      ->select('e.id')
+      ->where("e.is_template = 0")
+      ->where("e.is_active = 1");
+    if ($templateReminder) {
+      $query->join('r', 'INNER JOIN civicrm_event t ON e.template_title = t.template_title AND t.is_template = 1');
+    }
+    $sql = $query
+      ->where("{$valueTable}.{$valueField} IN (@selectedValues)")
+      ->param('selectedValues', $selectedValues)
+      ->toSQL();
+    $dao = \CRM_Core_DAO::executeQuery($sql);
+    return (bool) $dao->N;
+  }
+
 }

diff --git a/CRM/Member/ActionMapping.php b/CRM/Member/ActionMapping.php
index 56db7eef4148ed53f2f3f0e26e217e282e97d04d..e42add868298b81d1dac16546f3f3bb566667469 100644 (file)
--- a/CRM/Member/ActionMapping.php
+++ b/CRM/Member/ActionMapping.php
@@ -168,4 +168,12 @@ class CRM_Member_ActionMapping extends \Civi\ActionSchedule\Mapping {
     }
   }
 
+  /**
+   * Determine whether a schedule based on this mapping should
+   * send to additional contacts.
+   */
+  public function sendToAdditional($entityId): bool {
+    return TRUE;
+  }
+
 }

diff --git a/Civi/ActionSchedule/Mapping.php b/Civi/ActionSchedule/Mapping.php
index f6c1450d5644dc197e38f860bac0c16f68645e64..fc0e667b4d85ebe92e3ff710c4e04dc934c95fd6 100644 (file)
--- a/Civi/ActionSchedule/Mapping.php
+++ b/Civi/ActionSchedule/Mapping.php
@@ -337,4 +337,10 @@ abstract class Mapping implements MappingInterface {
     return FALSE;
   }
 
+  /**
+   * Determine whether a schedule based on this mapping should
+   * send to additional contacts.
+   */
+  abstract public function sendToAdditional($entityId): bool;
+
 }

diff --git a/Civi/ActionSchedule/MappingInterface.php b/Civi/ActionSchedule/MappingInterface.php
index 3dcd04fea133f840c8ee3e7eb6fd38735d1f5dd5..f551fc11c1968a303968e18fd02cd35387252dcf 100644 (file)
--- a/Civi/ActionSchedule/MappingInterface.php
+++ b/Civi/ActionSchedule/MappingInterface.php
@@ -139,4 +139,10 @@ interface MappingInterface {
    */
   public function resetOnTriggerDateChange($schedule);
 
+  /**
+   * Determine whether a schedule based on this mapping should
+   * send to additional contacts.
+   */
+  public function sendToAdditional($entityId): bool;
+
 }

diff --git a/Civi/ActionSchedule/RecipientBuilder.php b/Civi/ActionSchedule/RecipientBuilder.php
index c8a32dd97e3756e8125fb2e31b881a12f0fe4b0d..c52f64adbb6549278a2c6a0dd83b8a945442243a 100644 (file)
--- a/Civi/ActionSchedule/RecipientBuilder.php
+++ b/Civi/ActionSchedule/RecipientBuilder.php
@@ -138,7 +138,7 @@ class RecipientBuilder {
   public function build() {
     $this->buildRelFirstPass();
 
-    if ($this->prepareAddlFilter('c.id') && $this->notTemplate()) {
+    if ($this->prepareAddlFilter('c.id') && $this->mapping->sendToAdditional($this->actionSchedule->entity_value)) {
       $this->buildAddlFirstPass();
     }
 
@@ -146,7 +146,7 @@ class RecipientBuilder {
       $this->buildRelRepeatPass();
     }
 
-    if ($this->actionSchedule->is_repeat && $this->prepareAddlFilter('c.id')) {
+    if ($this->actionSchedule->is_repeat && $this->prepareAddlFilter('c.id') && $this->mapping->sendToAdditional($this->actionSchedule->entity_value)) {
       $this->buildAddlRepeatPass();
     }
   }
@@ -603,25 +603,4 @@ reminder.action_schedule_id = {$this->actionSchedule->id}";
     return $this->mapping->resetOnTriggerDateChange($this->actionSchedule);
   }
 
-  /**
-   * Confirm this object isn't attached to a template.
-   * Returns TRUE if this action schedule isn't attached to a template.
-   * Templates are (currently) unique to events, so we only evaluate those.
-   *
-   * @return bool;
-   */
-  private function notTemplate() {
-    if ($this->mapping->getEntity() === 'civicrm_participant') {
-      $entityId = $this->actionSchedule->entity_value;
-      $query = new \CRM_Utils_SQL_Select('civicrm_event e');
-      $sql = $query
-        ->select('is_template')
-        ->where("e.id = {$entityId}")
-        ->toSQL();
-      $dao = \CRM_Core_DAO::executeQuery($sql);
-      return !(bool) $dao->fetchValue();
-    }
-    return TRUE;
-  }
-
 }

diff --git a/tests/phpunit/api/v3/JobTest.php b/tests/phpunit/api/v3/JobTest.php
index 444714204ab93936985eec1c5b06a698b38cefa6..08a0188f6ef67f76e67005411114e208c4cb2578 100644 (file)
--- a/tests/phpunit/api/v3/JobTest.php
+++ b/tests/phpunit/api/v3/JobTest.php
@@ -330,6 +330,41 @@ class api_v3_JobTest extends CiviUnitTestCase {
     $this->assertEquals(0, $successfulCronCount);
   }
 
+  /**
+   * Deleted events should not send reminders to additional contacts.
+   *
+   * @throws \CRM_Core_Exception
+   */
+  public function testDeletedEventRemindAddlContacts() {
+    $contactId = $this->individualCreate();
+    $groupId = $this->groupCreate(['name' => 'Additional Contacts', 'title' => 'Additional Contacts']);
+    $this->callAPISuccess('GroupContact', 'create', [
+      'contact_id' => $contactId,
+      'group_id' => $groupId,
+    ]);
+    $event = $this->eventCreate(['title' => 'delete this event']);
+    $eventId = $event['id'];
+
+    $this->callAPISuccess('action_schedule', 'create', [
+      'title' => 'Do not send me',
+      'subject' => 'I am a reminder attached to a (soon to be) deleted event.',
+      'entity_value' => $eventId,
+      'mapping_id' => CRM_Event_ActionMapping::EVENT_NAME_MAPPING_ID,
+      'start_action_date' => 'start_date',
+      'start_action_offset' => 1,
+      'start_action_condition' => 'before',
+      'start_action_unit' => 'day',
+      'group_id' => $groupId,
+      'limit_to' => FALSE,
+      'mode' => 'Email',
+    ]);
+    $this->callAPISuccess('event', 'delete', ['id' => $eventId]);
+
+    $this->callAPISuccess('job', 'send_reminder', []);
+    $successfulCronCount = CRM_Core_DAO::singleValueQuery('SELECT count(*) FROM civicrm_action_log');
+    $this->assertEquals(0, $successfulCronCount);
+  }
+
   /**
    * Test scheduled reminders respect limit to (since above identified addition_to handling issue).
    *