From e08fae025bbf01a032fe8d35b5403133281a1881 Mon Sep 17 00:00:00 2001 From: Peter Haight Date: Tue, 23 Jan 2018 17:36:03 -0800 Subject: [PATCH] (dev/core#285) Fixed second membership repeating reminder This fixes the code for reminders that need to be sent again when the entity used for the date comparison changes. This is just membership right now. With this fix, when a membership is updated, the scheduled reminder conditions all act as if no reminders have been sent yet. This means that if you setup a schedule reminder that is checking against the membership end date, the schedule reminder will go out at the new membership end date even though one already went out with the previous end date. --- CRM/Contribute/ActionMapping/ByPage.php | 12 + CRM/Contribute/ActionMapping/ByType.php | 12 + CRM/Event/ActionMapping.php | 2 +- CRM/Member/ActionMapping.php | 22 +- .../Incremental/sql/5.14.alpha1.mysql.tpl | 1 + Civi/ActionSchedule/Mapping.php | 12 + Civi/ActionSchedule/MappingInterface.php | 10 + Civi/ActionSchedule/RecipientBuilder.php | 161 +++------ .../CRM/Core/BAO/ActionScheduleTest.php | 313 +++++++++++++++++- xml/schema/Core/ActionLog.xml | 3 +- 10 files changed, 419 insertions(+), 129 deletions(-) diff --git a/CRM/Contribute/ActionMapping/ByPage.php b/CRM/Contribute/ActionMapping/ByPage.php index 5e1d62448b..acfdea9d6a 100644 --- a/CRM/Contribute/ActionMapping/ByPage.php +++ b/CRM/Contribute/ActionMapping/ByPage.php @@ -220,4 +220,16 @@ class CRM_Contribute_ActionMapping_ByPage implements \Civi\ActionSchedule\Mappin return $query; } + /** + * Determine whether a schedule based on this mapping should + * reset the reminder state if the trigger date changes. + * + * @return bool + * + * @param \CRM_Core_DAO_ActionSchedule $schedule + */ + public function resetOnTriggerDateChange($schedule) { + return FALSE; + } + } diff --git a/CRM/Contribute/ActionMapping/ByType.php b/CRM/Contribute/ActionMapping/ByType.php index 3d4a85c8ae..e67b22956f 100644 --- a/CRM/Contribute/ActionMapping/ByType.php +++ b/CRM/Contribute/ActionMapping/ByType.php @@ -239,4 +239,16 @@ class CRM_Contribute_ActionMapping_ByType implements \Civi\ActionSchedule\Mappin return $query; } + /** + * Determine whether a schedule based on this mapping should + * reset the reminder state if the trigger date changes. + * + * @return bool + * + * @param \CRM_Core_DAO_ActionSchedule $schedule + */ + public function resetOnTriggerDateChange($schedule) { + return FALSE; + } + } diff --git a/CRM/Event/ActionMapping.php b/CRM/Event/ActionMapping.php index 3d34696b85..5a525d7a38 100644 --- a/CRM/Event/ActionMapping.php +++ b/CRM/Event/ActionMapping.php @@ -158,7 +158,7 @@ class CRM_Event_ActionMapping extends \Civi\ActionSchedule\Mapping { $query['casContactTableAlias'] = NULL; $query['casDateField'] = str_replace('event_', 'r.', $schedule->start_action_date); if (empty($query['casDateField']) && $schedule->absolute_date) { - $query['casDateField'] = $schedule->absolute_date; + $query['casDateField'] = "'" . CRM_Utils_Type::escape($schedule->absolute_date, 'String') . "'"; } $query->join('r', 'INNER JOIN civicrm_event r ON e.event_id = r.id'); diff --git a/CRM/Member/ActionMapping.php b/CRM/Member/ActionMapping.php index ee9c1b8170..13c5053fda 100644 --- a/CRM/Member/ActionMapping.php +++ b/CRM/Member/ActionMapping.php @@ -145,11 +145,6 @@ class CRM_Member_ActionMapping extends \Civi\ActionSchedule\Mapping { $query->where("e.status_id IN (#memberStatus)") ->param('memberStatus', \CRM_Member_PseudoConstant::membershipStatus(NULL, "(is_current_member = 1 OR name = 'Expired')", 'id')); - // Why is this only for civicrm_membership? - if ($schedule->start_action_date && $schedule->is_repeat == FALSE) { - $query['casUseReferenceDate'] = TRUE; - } - return $query; } @@ -172,4 +167,21 @@ class CRM_Member_ActionMapping extends \Civi\ActionSchedule\Mapping { ->where('!( e.owner_membership_id IS NOT NULL AND rela.id IS NULL and relb.id IS NULL )'); } + /** + * Determine whether a schedule based on this mapping should + * reset the reminder state if the trigger date changes. + * + * @return bool + * + * @param \CRM_Core_DAO_ActionSchedule $schedule + */ + public function resetOnTriggerDateChange($schedule) { + if ($schedule->absolute_date !== NULL) { + return FALSE; + } + else { + return TRUE; + } + } + } diff --git a/CRM/Upgrade/Incremental/sql/5.14.alpha1.mysql.tpl b/CRM/Upgrade/Incremental/sql/5.14.alpha1.mysql.tpl index 051f6eac08..741a5ccaf1 100644 --- a/CRM/Upgrade/Incremental/sql/5.14.alpha1.mysql.tpl +++ b/CRM/Upgrade/Incremental/sql/5.14.alpha1.mysql.tpl @@ -1 +1,2 @@ {* file to handle db changes in 5.14.alpha1 during upgrade *} +ALTER TABLE civicrm_action_log CHANGE COLUMN reference_date reference_date datetime; diff --git a/Civi/ActionSchedule/Mapping.php b/Civi/ActionSchedule/Mapping.php index 9e6dcbf919..85df528789 100644 --- a/Civi/ActionSchedule/Mapping.php +++ b/Civi/ActionSchedule/Mapping.php @@ -341,4 +341,16 @@ abstract class Mapping implements MappingInterface { */ abstract public function createQuery($schedule, $phase, $defaultParams); + /** + * Determine whether a schedule based on this mapping should + * reset the reminder state if the trigger date changes. + * + * @return bool + * + * @param \CRM_Core_DAO_ActionSchedule $schedule + */ + public function resetOnTriggerDateChange($schedule) { + return FALSE; + } + } diff --git a/Civi/ActionSchedule/MappingInterface.php b/Civi/ActionSchedule/MappingInterface.php index 18627968ae..137a25ef9b 100644 --- a/Civi/ActionSchedule/MappingInterface.php +++ b/Civi/ActionSchedule/MappingInterface.php @@ -145,4 +145,14 @@ interface MappingInterface { */ public function createQuery($schedule, $phase, $defaultParams); + /** + * Determine whether a schedule based on this mapping should + * reset the reminder state if the trigger date changes. + * + * @return bool + * + * @param \CRM_Core_DAO_ActionSchedule $schedule + */ + public function resetOnTriggerDateChange($schedule); + } diff --git a/Civi/ActionSchedule/RecipientBuilder.php b/Civi/ActionSchedule/RecipientBuilder.php index d979b94677..df03ef8c29 100644 --- a/Civi/ActionSchedule/RecipientBuilder.php +++ b/Civi/ActionSchedule/RecipientBuilder.php @@ -82,7 +82,6 @@ namespace Civi\ActionSchedule; * Some parameters are optional: * - casContactTableAlias: string, SQL table alias * - casAnniversaryMode: bool - * - casUseReferenceDate: bool * * Additionally, some parameters are automatically predefined: * - casNow @@ -169,7 +168,9 @@ class RecipientBuilder { } /** - * Generate action_log's for new, first-time alerts to related contacts. + * Generate action_log's for new, first-time alerts to related contacts, + * and contacts who are again eligible to receive the alert e.g. membership + * renewal reminders. * * @throws \Exception */ @@ -177,59 +178,15 @@ class RecipientBuilder { $query = $this->prepareQuery(self::PHASE_RELATION_FIRST); $startDateClauses = $this->prepareStartDateClauses(); - - // 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 - $referenceReminderIDs = []; - $referenceDate = NULL; - if (!empty($query['casUseReferenceDate'])) { - // First retrieve all the action log's ids which are outdated or in other words reference_date now don't match with entity date. - // And the retrieve the updated entity date which will later used below to update all other outdated action log records - $sql = $query->copy() - ->select('reminder.id as id') - ->select($query['casDateField'] . ' as reference_date') - ->merge($this->joinReminder('INNER JOIN', 'rel', $query)) - ->where("reminder.id IS NOT NULL AND reminder.reference_date IS NOT NULL AND reminder.reference_date <> !casDateField") - ->where($startDateClauses) - ->orderBy("reminder.id desc") - ->strict() - ->toSQL(); - $dao = \CRM_Core_DAO::executeQuery($sql); - - while ($dao->fetch()) { - $referenceReminderIDs[] = $dao->id; - $referenceDate = $dao->reference_date; - } - } - - if (empty($referenceReminderIDs)) { - $firstQuery = $query->copy() - ->merge($this->selectIntoActionLog(self::PHASE_RELATION_FIRST, $query)) - ->merge($this->joinReminder('LEFT JOIN', 'rel', $query)) - ->where("reminder.id IS NULL") - ->where($startDateClauses) - ->strict() - ->toSQL(); - \CRM_Core_DAO::executeQuery($firstQuery); - } - else { - // INSERT new log to send reminder as desired entity date got updated - $referenceQuery = $query->copy() - ->merge($this->selectIntoActionLog(self::PHASE_RELATION_FIRST, $query)) - ->merge($this->joinReminder('LEFT JOIN', 'rel', $query)) - ->where("reminder.id = !reminderID") - ->where($startDateClauses) - ->param('reminderID', $referenceReminderIDs[0]) - ->strict() - ->toSQL(); - \CRM_Core_DAO::executeQuery($referenceQuery); - - // Update all the previous outdated reference date valued, action_log rows to the latest changed entity date - $updateQuery = "UPDATE civicrm_action_log SET reference_date = '" . $referenceDate . "' WHERE id IN (" . implode(', ', $referenceReminderIDs) . ")"; - \CRM_Core_DAO::executeQuery($updateQuery); - } + // Send reminder to all contacts who have never received this scheduled reminder + $firstInstanceQuery = $query->copy() + ->merge($this->selectIntoActionLog(self::PHASE_RELATION_FIRST, $query)) + ->merge($this->joinReminder('LEFT JOIN', 'rel', $query)) + ->where("reminder.id IS NULL") + ->where($startDateClauses) + ->strict() + ->toSQL(); + \CRM_Core_DAO::executeQuery($firstInstanceQuery); } /** @@ -276,31 +233,18 @@ class RecipientBuilder { // @todo - this only handles events that get moved later. Potentially they might get moved earlier $repeatInsert = $query ->merge($this->joinReminder('INNER JOIN', 'rel', $query)) - ->merge($this->selectActionLogFields(self::PHASE_RELATION_REPEAT, $query)) - ->select("MAX(reminder.action_date_time) as latest_log_time") + ->merge($this->selectIntoActionLog(self::PHASE_RELATION_REPEAT, $query)) ->merge($this->prepareRepetitionEndFilter($query['casDateField'])) ->where($this->actionSchedule->start_action_date ? $startDateClauses[0] : []) ->groupBy("reminder.contact_id, reminder.entity_id, reminder.entity_table") - // @todo replace use of timestampdiff with a direct comparison as TIMESTAMPDIFF cannot use an index. - ->having("TIMESTAMPDIFF(HOUR, latest_log_time, CAST(!casNow AS datetime)) >= TIMESTAMPDIFF(HOUR, latest_log_time, DATE_ADD(latest_log_time, INTERVAL !casRepetitionInterval))") + ->having("TIMESTAMPDIFF(HOUR, MAX(reminder.action_date_time), CAST(!casNow AS datetime)) >= TIMESTAMPDIFF(HOUR, MAX(reminder.action_date_time), DATE_ADD(MAX(reminder.action_date_time), INTERVAL !casRepetitionInterval))") ->param([ 'casRepetitionInterval' => $this->parseRepetitionInterval(), ]) ->strict() ->toSQL(); - // For unknown reasons, we manually insert each row. Why not change - // selectActionLogFields() to selectIntoActionLog() above? - - $arrValues = \CRM_Core_DAO::executeQuery($repeatInsert)->fetchAll(); - if ($arrValues) { - \CRM_Core_DAO::executeQuery( - \CRM_Utils_SQL_Insert::into('civicrm_action_log') - ->columns(['contact_id', 'entity_id', 'entity_table', 'action_schedule_id']) - ->rows($arrValues) - ->toSQL() - ); - } + \CRM_Core_DAO::executeQuery($repeatInsert); } /** @@ -323,32 +267,19 @@ class RecipientBuilder { $daoCheck = \CRM_Core_DAO::executeQuery($addlCheck); if ($daoCheck->fetch()) { $repeatInsertAddl = \CRM_Utils_SQL_Select::from('civicrm_contact c') - ->merge($this->selectActionLogFields(self::PHASE_ADDITION_REPEAT, $query)) + ->merge($this->selectIntoActionLog(self::PHASE_ADDITION_REPEAT, $query)) ->merge($this->joinReminder('INNER JOIN', 'addl', $query)) - ->select("MAX(reminder.action_date_time) as latest_log_time") ->merge($this->prepareAddlFilter('c.id'), ['params']) ->where("c.is_deleted = 0 AND c.is_deceased = 0") ->groupBy("reminder.contact_id") - // @todo replace use of timestampdiff with a direct comparison as TIMESTAMPDIFF cannot use an index. - ->having("TIMESTAMPDIFF(HOUR, latest_log_time, CAST(!casNow AS datetime)) >= TIMESTAMPDIFF(HOUR, latest_log_time, DATE_ADD(latest_log_time, INTERVAL !casRepetitionInterval))") + ->having("TIMESTAMPDIFF(HOUR, MAX(reminder.action_date_time), CAST(!casNow AS datetime)) >= TIMESTAMPDIFF(HOUR, MAX(reminder.action_date_time), DATE_ADD(MAX(reminder.action_date_time), INTERVAL !casRepetitionInterval))") ->param([ 'casRepetitionInterval' => $this->parseRepetitionInterval(), ]) ->strict() ->toSQL(); - // For unknown reasons, we manually insert each row. Why not change - // selectActionLogFields() to selectIntoActionLog() above? - - $addValues = \CRM_Core_DAO::executeQuery($repeatInsertAddl)->fetchAll(); - if ($addValues) { - \CRM_Core_DAO::executeQuery( - \CRM_Utils_SQL_Insert::into('civicrm_action_log') - ->columns(['contact_id', 'entity_id', 'entity_table', 'action_schedule_id']) - ->rows($addValues) - ->toSQL() - ); - } + \CRM_Core_DAO::executeQuery($repeatInsertAddl); } } @@ -565,22 +496,20 @@ WHERE $group.id = {$groupId} * @throws \CRM_Core_Exception */ protected function selectActionLogFields($phase, $query) { + $selectArray = []; switch ($phase) { case self::PHASE_RELATION_FIRST: case self::PHASE_RELATION_REPEAT: $fragment = \CRM_Utils_SQL_Select::fragment(); - // CRM-15376: We are not tracking the reference date for 'repeated' schedule reminders. - if (!empty($query['casUseReferenceDate'])) { - $fragment->select($query['casDateField']); + $selectArray = [ + "!casContactIdField as contact_id", + "!casEntityIdField as entity_id", + "@casMappingEntity as entity_table", + "#casActionScheduleId as action_schedule_id", + ]; + if ($this->resetOnTriggerDateChange()) { + $selectArray[] = "!casDateField as reference_date"; } - $fragment->select( - [ - "!casContactIdField as contact_id", - "!casEntityIdField as entity_id", - "@casMappingEntity as entity_table", - "#casActionScheduleId as action_schedule_id", - ] - ); break; case self::PHASE_ADDITION_FIRST: @@ -591,19 +520,18 @@ WHERE $group.id = {$groupId} 'casNow' => $this->now, ]; $fragment = \CRM_Utils_SQL_Select::fragment()->param($params); - $fragment->select( - [ - "c.id as contact_id", - "c.id as entity_id", - "'civicrm_contact' as entity_table", - "#casActionScheduleId as action_schedule_id", - ] - ); + $selectArray = [ + "c.id as contact_id", + "c.id as entity_id", + "'civicrm_contact' as entity_table", + "#casActionScheduleId as action_schedule_id", + ]; break; default: throw new \CRM_Core_Exception("Unrecognized phase: $phase"); } + $fragment->select($selectArray); return $fragment; } @@ -625,10 +553,9 @@ WHERE $group.id = {$groupId} "entity_table", "action_schedule_id", ]; - if ($phase === self::PHASE_RELATION_FIRST || $phase === self::PHASE_RELATION_REPEAT) { - if (!empty($query['casUseReferenceDate'])) { - array_unshift($actionLogColumns, 'reference_date'); - } + + if ($this->resetOnTriggerDateChange() && ($phase == self::PHASE_RELATION_FIRST || $phase == self::PHASE_RELATION_REPEAT)) { + $actionLogColumns[] = "reference_date"; } return $this->selectActionLogFields($phase, $query) @@ -669,6 +596,10 @@ reminder.entity_id = {$entityIdField} AND reminder.entity_table = '{$entityName}' AND reminder.action_schedule_id = {$this->actionSchedule->id}"; + if ($for == 'rel' && $this->resetOnTriggerDateChange()) { + $joinClause .= " AND\nreminder.reference_date = !casDateField"; + } + // Why do we only include anniversary clause for 'rel' queries? if ($for === 'rel' && !empty($query['casAnniversaryMode'])) { // only consider reminders less than 11 months ago @@ -678,4 +609,14 @@ reminder.action_schedule_id = {$this->actionSchedule->id}"; return \CRM_Utils_SQL_Select::fragment()->join("reminder", "$joinType $joinClause"); } + /** + * Should we use the reference date when checking to see if we already + * sent reminders. + * + * @return bool + */ + protected function resetOnTriggerDateChange() { + return $this->mapping->resetOnTriggerDateChange($this->actionSchedule); + } + } diff --git a/tests/phpunit/CRM/Core/BAO/ActionScheduleTest.php b/tests/phpunit/CRM/Core/BAO/ActionScheduleTest.php index e552004e26..4365f21e4c 100644 --- a/tests/phpunit/CRM/Core/BAO/ActionScheduleTest.php +++ b/tests/phpunit/CRM/Core/BAO/ActionScheduleTest.php @@ -46,6 +46,15 @@ class CRM_Core_BAO_ActionScheduleTest extends CiviUnitTestCase { $this->mut = new CiviMailUtils($this, TRUE); + $this->fixtures['rolling_membership_type'] = array( + 'period_type' => 'rolling', + 'duration_unit' => 'month', + 'duration_interval' => '3', + 'is_active' => 1, + 'domain_id' => 1, + 'financial_type_id' => 2, + ); + $this->fixtures['rolling_membership'] = array( 'membership_type_id' => array( 'period_type' => 'rolling', @@ -100,6 +109,14 @@ class CRM_Core_BAO_ActionScheduleTest extends CiviUnitTestCase { 'first_name' => 'Churmondleia', 'last_name' => 'Ōtākou', ); + $this->fixtures['contact_2'] = array( + 'is_deceased' => 0, + 'contact_type' => 'Individual', + 'email' => 'test-contact-2@example.com', + 'gender_id' => 'Male', + 'first_name' => 'Fabble', + 'last_name' => 'Fi', + ); $this->fixtures['contact_birthdate'] = array( 'is_deceased' => 0, 'contact_type' => 'Individual', @@ -196,6 +213,36 @@ class CRM_Core_BAO_ActionScheduleTest extends CiviUnitTestCase { 'start_action_unit' => '', 'subject' => '1-Day (repeating) (about {activity.activity_type})', ); + $this->fixtures['sched_eventname_1day_on_abs_date'] = array( + 'name' => 'sched_eventname_1day_on_abs_date', + 'title' => 'sched_eventname_1day_on_abs_date', + 'limit_to' => 1, + 'absolute_date' => CRM_Utils_Date::processDate('20120614100000'), + 'body_html' => '

sched_eventname_1day_on_abs_date

', + 'body_text' => 'sched_eventname_1day_on_abs_date', + 'entity_status' => '1', + 'entity_value' => '2', + 'group_id' => NULL, + 'is_active' => '1', + 'is_repeat' => '0', + 'mapping_id' => '3', + 'msg_template_id' => NULL, + 'recipient' => '2', + 'recipient_listing' => NULL, + 'recipient_manual' => NULL, + 'record_activity' => NULL, + 'repetition_frequency_interval' => NULL, + 'repetition_frequency_unit' => NULL, + 'end_action' => NULL, + 'end_date' => NULL, + 'end_frequency_interval' => NULL, + 'end_frequency_unit' => NULL, + 'start_action_condition' => NULL, + 'start_action_date' => NULL, + 'start_action_offset' => NULL, + 'start_action_unit' => NULL, + 'subject' => 'sched_eventname_1day_on_abs_date', + ); $this->fixtures['sched_membership_join_2week'] = array( 'name' => 'sched_membership_join_2week', 'title' => 'sched_membership_join_2week', @@ -342,6 +389,36 @@ class CRM_Core_BAO_ActionScheduleTest extends CiviUnitTestCase { 'subject' => 'subject sched_membership_end_2month', ); + $this->fixtures['sched_membership_absolute_date'] = array( + 'name' => 'sched_membership_absolute_date', + 'title' => 'sched_membership_absolute_date', + 'absolute_date' => CRM_Utils_Date::processDate('20120614100000'), + 'body_html' => '

body sched_membership_absolute_date

', + 'body_text' => 'body sched_membership_absolute_date', + 'end_action' => '', + 'end_date' => '', + 'end_frequency_interval' => '', + 'end_frequency_unit' => '', + 'entity_status' => '', + 'entity_value' => '', + 'group_id' => '', + 'is_active' => 1, + 'is_repeat' => '0', + 'mapping_id' => 4, + 'msg_template_id' => '', + 'recipient' => '', + 'recipient_listing' => '', + 'recipient_manual' => '', + 'record_activity' => 1, + 'repetition_frequency_interval' => '', + 'repetition_frequency_unit' => '', + 'start_action_condition' => '', + 'start_action_date' => '', + 'start_action_offset' => '', + 'start_action_unit' => '', + 'subject' => 'subject sched_membership_absolute_date', + ); + $this->fixtures['sched_contact_bday_yesterday'] = array( 'name' => 'sched_contact_bday_yesterday', 'title' => 'sched_contact_bday_yesterday', @@ -1000,6 +1077,40 @@ class CRM_Core_BAO_ActionScheduleTest extends CiviUnitTestCase { )); } + public function testEventNameWithAbsoluteDateAndNothingElse() { + $participant = $this->createTestObject('CRM_Event_DAO_Participant', array_merge($this->fixtures['participant'], array('status_id' => 1))); + $this->callAPISuccess('Email', 'create', array( + 'contact_id' => $participant->contact_id, + 'email' => 'test-event@example.com', + )); + $this->callAPISuccess('contact', 'create', array_merge($this->fixtures['contact'], array('contact_id' => $participant->contact_id))); + + $actionSchedule = $this->fixtures['sched_eventname_1day_on_abs_date']; + $actionSchedule['entity_value'] = $participant->event_id; + $this->callAPISuccess('action_schedule', 'create', $actionSchedule); + + $this->assertCronRuns(array( + array( + // Before the 24-hour mark, no email + 'time' => '2012-06-13 04:00:00', + 'recipients' => array(), + 'subjects' => array(), + ), + array( + // On absolute date set on 2012-06-14 + 'time' => '2012-06-14 00:00:00', + 'recipients' => array(array('test-event@example.com')), + 'subjects' => array('sched_eventname_1day_on_abs_date'), + ), + array( + // Run cron 4 hours later; first message already sent + 'time' => '2012-06-14 04:00:00', + 'recipients' => array(), + 'subjects' => array(), + ), + )); + } + /** * For contacts/activities which don't match the schedule filter, * an email should *not* be sent. @@ -1203,15 +1314,43 @@ class CRM_Core_BAO_ActionScheduleTest extends CiviUnitTestCase { // end_date=2012-06-15 ; schedule is 2 weeks before end_date $this->assertCronRuns(array( array( - // After the 2-week mark, send an email. + // After the 1-month mark, no email + 'time' => '2012-07-15 01:00:00', + 'recipients' => array(), + ), + array( + // After the 2-month mark, send an email. 'time' => '2012-08-15 01:00:00', 'recipients' => array(array('test-member@example.com')), ), array( - // After the 2-week mark, send an email. + // 4 weeks after first email send first repeat 'time' => '2012-09-12 01:00:00', 'recipients' => array(array('test-member@example.com')), ), + array( + // 1 week after first repeat send nothing + // There was a bug where the first repeat went out and then + // it would keep going out every cron run. This is to check that's + // not happening. + 'time' => '2012-09-19 01:00:00', + 'recipients' => array(), + ), + array( + // 4 weeks after first repeat send second repeat + 'time' => '2012-10-10 01:00:00', + 'recipients' => array(array('test-member@example.com')), + ), + array( + // 4 months after membership end, send nothing + 'time' => '2012-10-15 01:00:00', + 'recipients' => array(), + ), + array( + // 5 months after membership end, send nothing + 'time' => '2012-11-15 01:00:00', + 'recipients' => array(), + ), )); } @@ -1280,8 +1419,6 @@ class CRM_Core_BAO_ActionScheduleTest extends CiviUnitTestCase { array( // Before the 2-week mark, no email. 'time' => '2012-05-31 01:00:00', - // 'time' => '2012-06-01 01:00:00', - // FIXME: Is this the right boundary? 'recipients' => array(), ), array( @@ -1289,6 +1426,11 @@ class CRM_Core_BAO_ActionScheduleTest extends CiviUnitTestCase { 'time' => '2012-06-01 01:00:00', 'recipients' => array(array('test-member@example.com')), ), + array( + // After the email is sent, another one is not sent + 'time' => '2012-06-01 02:00:00', + 'recipients' => array(), + ), )); // Now suppose user has renewed for rolling membership after 3 months, so upcoming assertion is written @@ -1312,10 +1454,121 @@ class CRM_Core_BAO_ActionScheduleTest extends CiviUnitTestCase { 'time' => '2012-08-31 01:00:00', 'recipients' => array(), ), - //array( // After the 2-week mark, send an email - //'time' => '2012-09-01 01:00:00', - //'recipients' => array(array('member2@example.com')), - //), + array( + // After the 2-week mark, send an email + 'time' => '2012-09-01 01:00:00', + 'recipients' => array(array('member2@example.com')), + ), + array( + // After the email is sent, another one is not sent + 'time' => '2012-09-01 02:00:00', + 'recipients' => array(), + ), + )); + + $membership->end_date = '2012-12-15'; + $membership->save(); + // end_date=2012-12-15 ; schedule is 2 weeks before end_date + $this->assertCronRuns(array( + array( + // Before the 2-week mark, no email + 'time' => '2012-11-30 01:00:00', + 'recipients' => array(), + ), + array( + // After the 2-week mark, send an email + 'time' => '2012-12-01 01:00:00', + 'recipients' => array(array('member2@example.com')), + ), + array( + // After the email is sent, another one is not sent + 'time' => '2012-12-01 02:00:00', + 'recipients' => array(), + ), + )); + + } + + public function createMembershipAndContact($contactFixture, $membershipTypeId) { + $result = $this->callAPISuccess('contact', 'create', $contactFixture); + $contact = $result['values'][$result['id']]; + $params = array( + 'status_id' => 2, + 'contact_id' => $contact['id'], + 'membership_type_id' => $membershipTypeId, + 'owner_membership_id' => 'NULL', + ); + $params = array_merge($this->fixtures['rolling_membership'], $params); + $membership = $this->createTestObject('CRM_Member_DAO_Membership', $params); + $this->assertTrue(is_numeric($membership->id)); + return $membership; + } + + /** + * This test is very similar to testMembershipEndDateMatch, but it adds + * another contact because there was a bug in + * RecipientBuilder::buildRelFirstPass where it was only sending the + * reminder for the first contact returned in a query for renewed + * memberships. Other contacts wouldn't get the mail. + */ + public function testMultipleMembershipEndDateMatch() { + $membershipTypeId = $this->membershipTypeCreate($this->fixtures['rolling_membership']['membership_type_id']); + $membershipOne = $this->createMembershipAndContact($this->fixtures['contact'], $membershipTypeId); + $membershipTwo = $this->createMembershipAndContact($this->fixtures['contact_2'], $membershipTypeId); + $actionSchedule = $this->fixtures['sched_membership_end_2week']; + $actionSchedule['entity_value'] = $membershipTypeId; + $actionScheduleDao = CRM_Core_BAO_ActionSchedule::add($actionSchedule); + $this->assertTrue(is_numeric($actionScheduleDao->id)); + + // end_date=2012-06-15 ; schedule is 2 weeks before end_date + $this->assertCronRuns(array( + array( + // Before the 2-week mark, no email. + 'time' => '2012-05-31 01:00:00', + 'recipients' => array(), + ), + array( + // After the 2-week mark, send emails. + 'time' => '2012-06-01 01:00:00', + 'recipients' => array( + array('test-member@example.com'), + array('test-contact-2@example.com'), + ), + ), + array( + // After the email is sent, another one is not sent + 'time' => '2012-06-01 02:00:00', + 'recipients' => array(), + ), + )); + + // Now suppose user has renewed for rolling membership after 3 months, so upcoming assertion is written + // to ensure that new reminder is sent 2 week before the new end_date i.e. '2012-09-15' + $membershipOne->end_date = '2012-09-15'; + $membershipOne->save(); + $membershipTwo->end_date = '2012-09-15'; + $membershipTwo->save(); + + // end_date=2012-09-15 ; schedule is 2 weeks before end_date + $this->assertCronRuns(array( + array( + // Before the 2-week mark, no email + 'time' => '2012-08-31 01:00:00', + 'recipients' => array(), + ), + array( + // After the 2-week mark, send an email + 'time' => '2012-09-01 01:00:00', + 'recipients' => array( + array('test-member@example.com'), + array('test-contact-2@example.com'), + ), + ), + array( + // After the email is sent, another one is not sent + 'time' => '2012-06-01 02:00:00', + 'recipients' => array(), + ), )); } @@ -1345,12 +1598,10 @@ class CRM_Core_BAO_ActionScheduleTest extends CiviUnitTestCase { array( // Before the 2-week mark, no email. 'time' => '2012-05-31 01:00:00', - // 'time' => '2012-06-01 01:00:00', - // FIXME: Is this the right boundary? 'recipients' => array(), ), array( - // After the 2-week mark, send an email. + // After the 2-week mark, no email 'time' => '2013-05-01 01:00:00', 'recipients' => array(), ), @@ -1544,7 +1795,7 @@ class CRM_Core_BAO_ActionScheduleTest extends CiviUnitTestCase { //check if reference date is set to membership's join date //as per the action_start_date chosen for current schedule reminder - $this->assertEquals('2012-03-15', + $this->assertEquals('2012-03-15 00:00:00', CRM_Core_DAO::getFieldValue('CRM_Core_DAO_ActionLog', $membership->contact_id, 'reference_date', 'contact_id') ); @@ -2192,4 +2443,42 @@ class CRM_Core_BAO_ActionScheduleTest extends CiviUnitTestCase { return $newDateTime; } + public function testMembershipScheduleWithAbsoluteDate() { + $membership = $this->createTestObject('CRM_Member_DAO_Membership', array_merge($this->fixtures['rolling_membership'], array('status_id' => 1))); + $this->assertTrue(is_numeric($membership->id)); + $result = $this->callAPISuccess('Email', 'create', array( + 'contact_id' => $membership->contact_id, + 'email' => 'test-member@example.com', + 'location_type_id' => 1, + 'is_primary' => 1, + )); + + $this->callAPISuccess('contact', 'create', array_merge($this->fixtures['contact'], array('contact_id' => $membership->contact_id))); + $actionSchedule = $this->fixtures['sched_membership_absolute_date']; + $actionSchedule['entity_value'] = $membership->membership_type_id; + $actionScheduleDao = CRM_Core_BAO_ActionSchedule::add($actionSchedule); + $this->assertTrue(is_numeric($actionScheduleDao->id)); + + $this->assertCronRuns(array( + array( + // Before the 24-hour mark, no email + 'time' => '2012-06-13 04:00:00', + 'recipients' => array(), + 'subjects' => array(), + ), + array( + // On absolute date set on 2012-06-14 + 'time' => '2012-06-14 00:00:00', + 'recipients' => array(array('test-member@example.com')), + 'subjects' => array('subject sched_membership_absolute_date'), + ), + array( + // Run cron 4 hours later; first message already sent + 'time' => '2012-06-14 04:00:00', + 'recipients' => array(), + 'subjects' => array(), + ), + )); + } + } diff --git a/xml/schema/Core/ActionLog.xml b/xml/schema/Core/ActionLog.xml index 9bfc8cbcd4..a707d70d6e 100644 --- a/xml/schema/Core/ActionLog.xml +++ b/xml/schema/Core/ActionLog.xml @@ -99,9 +99,10 @@ reference_date Reference Date - date + datetime NULL Stores the date from the entity which triggered this reminder action (e.g. membership.end_date for most membership renewal reminders) 4.6 + 5.14 -- 2.25.1