From a2d239978776f3f9a2247ae3f330e182fa853b9f Mon Sep 17 00:00:00 2001
From: Seamus Lee <seamuslee001@gmail.com>
Date: Tue, 6 Sep 2016 00:00:32 +0000
Subject: [PATCH] CRM-18236 stop schedule reminders from sending multiple
 reminders for the same event

---
 CRM/Core/BAO/ActionSchedule.php               | 38 ++++++++++++-------
 .../CRM/Core/BAO/ActionScheduleTest.php       | 13 +++++--
 2 files changed, 34 insertions(+), 17 deletions(-)

diff --git a/CRM/Core/BAO/ActionSchedule.php b/CRM/Core/BAO/ActionSchedule.php
index c0d642ff45..55496cbc1a 100755
--- a/CRM/Core/BAO/ActionSchedule.php
+++ b/CRM/Core/BAO/ActionSchedule.php
@@ -29,8 +29,6 @@
  *
  * @package CRM
  * @copyright CiviCRM LLC (c) 2004-2015
- * $Id$
- *
  */
 
 /**
@@ -1220,10 +1218,10 @@ WHERE      $group.id = {$actionSchedule->group_id}
       $select[] = 'e.id as entity_id';
       $select[] = "'{$mapping->entity}' as entity_table";
       $select[] = "{$actionSchedule->id} as action_schedule_id";
-      $reminderJoinClause = "civicrm_action_log reminder ON reminder.contact_id = {$contactField} AND
-reminder.entity_id          = e.id AND
-reminder.entity_table       = '{$mapping->entity}' AND
-reminder.action_schedule_id = %1";
+      $reminderJoinClause = "civicrm_action_log reminder ON reminder.contact_id = {$contactField}
+      AND reminder.entity_id = e.id
+      AND reminder.entity_table  = '{$mapping->entity}'
+      AND reminder.action_schedule_id = %1";
 
       if ($anniversary) {
         // only consider reminders less than 11 months ago
@@ -1254,7 +1252,7 @@ reminder.action_schedule_id = %1";
       }
 
       // ( now >= date_built_from_start_time ) OR ( now = absolute_date )
-      $dateClause = "reminder.id IS NULL AND {$startDate}";
+      $dateClause = $startDate ? " AND {$startDate} " : '';
 
       // start composing query
       $selectClause = 'SELECT ' . implode(', ', $select);
@@ -1272,26 +1270,38 @@ INSERT INTO civicrm_action_log ({$selectColumns})
 {$fromClause}
 {$joinClause}
 LEFT JOIN {$reminderJoinClause}
-{$whereClause} {$limitWhereClause} AND {$dateClause} {$notINClause}
+{$whereClause} AND reminder.id IS NULL {$limitWhereClause} {$dateClause} {$notINClause}
 ";
 
       // In some cases reference_date got outdated due to many reason e.g. In Membership renewal end_date got extended
       // which means reference date mismatches with the end_date where end_date may be used as the start_action_date
       // criteria  for some schedule reminder so in order to send new reminder we INSERT new reminder with new reference_date
       // value via UNION operation
+      // We need to add in reminders that
+      // have not already had a reminder for the current end date and HAVE had a reminder for a different
+      // end date for the same reminder. These will have been excluded earlier, on the basis of a reminder having gone out
+      // so we want to selectively re-add them.
       if (strpos($selectColumns, 'reference_date') !== FALSE) {
-        $dateClause = str_replace('reminder.id IS NULL', 'reminder.id IS NOT NULL', $dateClause);
         $referenceQuery = "
 INSERT INTO civicrm_action_log ({$selectColumns})
 {$selectClause}
 {$fromClause}
 {$joinClause}
  LEFT JOIN {$reminderJoinClause}
-{$whereClause} {$limitWhereClause} {$notINClause} AND {$dateClause} AND
- reminder.action_date_time IS NOT NULL AND
- reminder.reference_date IS NOT NULL
-GROUP BY reminder.id, reminder.reference_date
-HAVING reminder.id = MAX(reminder.id) AND reminder.reference_date <> {$dateField}
+ LEFT JOIN (
+  SELECT entity_id, entity_table
+  {$fromClause}
+  {$joinClause}
+  LEFT JOIN {$reminderJoinClause}
+  {$whereClause} {$limitWhereClause} {$dateClause} {$notINClause}
+  AND reminder.reference_date = $dateField
+ ) as already_sent ON e.id = already_sent.entity_id AND already_sent.entity_table = '{$mapping->entity}'
+{$whereClause} {$limitWhereClause} {$notINClause} {$dateClause}
+ AND reminder.id IS NOT NULL
+ AND reminder.reference_date IS NOT NULL
+ AND reminder.reference_date <> $dateField
+ AND already_sent.entity_table IS NULL
+ GROUP BY e.id
 ";
       }
 
diff --git a/tests/phpunit/CRM/Core/BAO/ActionScheduleTest.php b/tests/phpunit/CRM/Core/BAO/ActionScheduleTest.php
index 8eaa903cdc..a79f3451be 100755
--- a/tests/phpunit/CRM/Core/BAO/ActionScheduleTest.php
+++ b/tests/phpunit/CRM/Core/BAO/ActionScheduleTest.php
@@ -1003,9 +1003,9 @@ class CRM_Core_BAO_ActionScheduleTest extends CiviUnitTestCase {
     $membership = $this->createTestObject('CRM_Member_DAO_Membership', array_merge($this->fixtures['rolling_membership'], array('status_id' => 2)));
 
     $this->assertTrue(is_numeric($membership->id));
-    $result = $this->callAPISuccess('Email', 'create', array(
-      'contact_id' => $membership->contact_id,
-      'email' => 'member@example.com',
+    $this->callAPISuccess('Email', 'create', array(
+        'contact_id' => $membership->contact_id,
+        'email' => 'member@example.com',
     ));
 
     $result = $this->callAPISuccess('contact', 'create', array_merge($this->fixtures['contact'], array('contact_id' => $membership->contact_id)));
@@ -1052,6 +1052,13 @@ class CRM_Core_BAO_ActionScheduleTest extends CiviUnitTestCase {
         'recipients' => array(array('member@example.com')),
       ),
     ));
+    $this->assertCronRuns(array(
+        array(
+          // It should not re-send on the same day
+          'time' => '2012-04-12 01:00:00',
+          'recipients' => array(),
+        ),
+    ));
   }
 
   public function testMembershipOnMultipleReminder() {
-- 
2.25.1