From 9c0fe051e29364f88b5e18e44f8323fb7a87406a Mon Sep 17 00:00:00 2001 From: monishdeb Date: Tue, 27 Jan 2015 20:05:50 +0530 Subject: [PATCH] Optimization and test changes --- CRM/Core/BAO/ActionSchedule.php | 10 ++- CRM/Upgrade/Incremental/php/FourSix.php | 65 +++++-------------- .../CRM/Core/BAO/ActionScheduleTest.php | 56 ++++++---------- 3 files changed, 43 insertions(+), 88 deletions(-) diff --git a/CRM/Core/BAO/ActionSchedule.php b/CRM/Core/BAO/ActionSchedule.php index 5c8e4d3292..bd643bd1ac 100755 --- a/CRM/Core/BAO/ActionSchedule.php +++ b/CRM/Core/BAO/ActionSchedule.php @@ -1272,8 +1272,8 @@ LEFT JOIN {$reminderJoinClause} // value via UNION operation if (strpos($selectColumns, 'reference_date') !== FALSE) { $dateClause = str_replace('reminder.id IS NULL', 'reminder.id IS NOT NULL', $dateClause); - $query .= " -UNION + $referenceQuery = " +INSERT INTO civicrm_action_log ({$selectColumns}) {$selectClause} {$fromClause} {$joinClause} @@ -1281,6 +1281,7 @@ UNION {$whereClause} {$limitWhereClause} {$notINClause} AND {$dateClause} AND reminder.action_date_time IS NOT NULL AND (reminder.reference_date IS NOT NULL AND reminder.reference_date != {$dateField}) +LIMIT 0,1 "; // As per the usage of UNION clause above we always INSERT a new reminder if reference_date (RD) @@ -1290,6 +1291,10 @@ UNION $updateQuery = "UPDATE civicrm_action_log reminder INNER JOIN {$mapping->entity} e ON e.id = reminder.entity_id AND reminder.reference_date IS NOT NULL AND reminder.action_date_time IS NOT NULL + INNER JOIN civicrm_action_log new_reminder ON + new_reminder.action_schedule_id = reminder.action_schedule_id AND + new_reminder.reference_date = {$dateField} AND + new_reminder.action_date_time IS NULL SET reminder.reference_date = {$dateField} WHERE reminder.action_schedule_id = %1 AND reminder.reference_date IS NOT NULL AND reminder.reference_date != {$dateField} "; @@ -1298,6 +1303,7 @@ UNION CRM_Core_DAO::executeQuery($query, array(1 => array($actionSchedule->id, 'Integer'))); if (!empty($updateQuery)) { + $dao = CRM_Core_DAO::executeQuery($referenceQuery, array(1 => array($actionSchedule->id, 'Integer'))); CRM_Core_DAO::executeQuery($updateQuery, array(1 => array($actionSchedule->id, 'Integer'))); } diff --git a/CRM/Upgrade/Incremental/php/FourSix.php b/CRM/Upgrade/Incremental/php/FourSix.php index f4c240892a..238f78da28 100644 --- a/CRM/Upgrade/Incremental/php/FourSix.php +++ b/CRM/Upgrade/Incremental/php/FourSix.php @@ -125,23 +125,19 @@ class CRM_Upgrade_Incremental_php_FourSix { ADD COLUMN `reference_date` date COMMENT 'Stores the date from the entity which triggered this reminder action (e.g. membership.end_date for most membership renewal reminders)'"; CRM_Core_DAO::executeQuery($query); - //Retrieve schedule reminders for membership entity and is not repeatable + //Retrieve schedule reminders for membership entity and is not repeatable and no absolute date chosen $query = "SELECT schedule.* FROM civicrm_action_schedule schedule LEFT JOIN civicrm_action_mapping mapper ON mapper.id = schedule.mapping_id AND - mapper.entity = 'civicrm_membership' AND schedule.is_repeat = 0"; + mapper.entity = 'civicrm_membership' AND + schedule.is_repeat = 0 AND + schedule.start_action_date IS NOT NULL"; // construct basic where clauses $where = array( - 'reminder.reference_date IS NOT NULL', - '( m.is_override IS NULL OR m.is_override = 0 )', - 'reminder.action_date_time >= DATE_SUB(reminder.action_date_time, INTERVAL 9 MONTH)' - ); + 'reminder.action_date_time >= DATE_SUB(reminder.action_date_time, INTERVAL 9 MONTH)' + ); //choose reminder older then 9 months $dao = CRM_Core_DAO::executeQuery($query); while($dao->fetch()) { - //if absolute date is chosen then bypass - if (empty($dao->start_action_date)) { - continue; - } $referenceColumn = str_replace('membership_', "m.", $dao->start_action_date); $value = implode(', ', explode(CRM_Core_DAO::VALUE_SEPARATOR, trim($dao->entity_value, CRM_Core_DAO::VALUE_SEPARATOR))); @@ -152,47 +148,16 @@ class CRM_Upgrade_Incremental_php_FourSix { $where[] = "m.membership_type_id IS NULL"; } - // Update reference_date with action_start_date chosen, only to those which are not additional contacts - // (Additional contacts are include via Group/Smart Group or Manual Recipients) - $addtionalGroupJoin = NULL; - if (!is_null($dao->limit_to) && $dao->limit_to == 0) { - if ($dao->group_id) { - // CRM-13577 If smart group then use Cache table - $group = CRM_Contact_DAO_Group::getTableName(); - // Get the group information - $sql = " -SELECT $group.id, $group.cache_date, $group.saved_search_id, $group.children -FROM $group -WHERE $group.id = {$dao->group_id} -"; - $groupDAO = CRM_Core_DAO::executeQuery($sql); - - $isSmartGroup = FALSE; - if ($groupDAO->fetch() && !empty($groupDAO->saved_search_id)) { - // Check that the group is in place in the cache and up to date - CRM_Contact_BAO_GroupContactCache::check($dao->group_id); - // Set smart group flag - $isSmartGroup = TRUE; - } - if ($isSmartGroup) { - $addtionalGroupJoin = " INNER JOIN civicrm_group_contact_cache grp ON reminder.contact_id = grp.contact_id"; - $where[] = " grp.group_id NOT IN ({$dao->group_id})"; - } - else { - $addtionalGroupJoin = " INNER JOIN civicrm_group_contact grp ON - reminder.contact_id = grp.contact_id AND grp.status = 'Added'"; - $where[] = " grp.group_id NOT IN ({$dao->group_id})"; - } - } - if (!empty($dao->recipient_manual)) { - $rList = CRM_Utils_Type::escape($dao->recipient_manual, 'String'); - $where[] = "reminder.contact_id NOT IN ({$rList})"; - } - } - + //Create new action_log records where action_start_date changes and exclude reminders for additional contacts + //and select contacts are active $sql = "UPDATE civicrm_action_log reminder - LEFT JOIN civicrm_membership m ON reminder.entity_id = m.id - {$addtionalGroupJoin} + LEFT JOIN civicrm_membership m + ON reminder.entity_id = m.id AND + reminder.entity_table = 'civicrm_membership' AND + ( m.is_override IS NULL OR m.is_override = 0 ) + INNER JOIN civicrm_contact c + ON c.id = e.contact_id AND + c.is_deleted = 0 AND c.is_deceased = 0 SET reminder.reference_date = {$referenceColumn} WHERE " . implode(" AND ", $where); CRM_Core_DAO::executeQuery($sql); diff --git a/tests/phpunit/CRM/Core/BAO/ActionScheduleTest.php b/tests/phpunit/CRM/Core/BAO/ActionScheduleTest.php index c20699aae5..6cf39e5e67 100755 --- a/tests/phpunit/CRM/Core/BAO/ActionScheduleTest.php +++ b/tests/phpunit/CRM/Core/BAO/ActionScheduleTest.php @@ -921,7 +921,11 @@ class CRM_Core_BAO_ActionScheduleTest extends CiviUnitTestCase { array( // After the 2-week mark, send an email 'time' => '2012-03-29 01:00:00', 'recipients' => array(array('member@example.com')), - ) + ), + array( // After the 2-week 1day mark, don't send an email + 'time' => '2012-03-30 01:00:00', + 'recipients' => array(), + ), )); //check if reference date is set to membership's join date @@ -934,20 +938,14 @@ class CRM_Core_BAO_ActionScheduleTest extends CiviUnitTestCase { $membership->join_date = '2012-03-29'; $membership->save(); - //change the email id of chosen membership contact to assert - //recipient of not the previously sent mail but the new one - $result = $this->callAPISuccess('Email', 'create', array( - 'is_primary' => 1, - 'contact_id' => $membership->contact_id, - 'email' => 'member2@example.com' - )); - $this->assertAPISuccess($result); - $this->assertCronRuns(array( + array( // After the 13 days of the changed join date 2012-03-29, don't send an email + 'time' => '2012-04-11 01:00:00', + 'recipients' => array() + ), array( // After the 2-week of the changed join date 2012-03-29, send an email 'time' => '2012-04-12 01:00:00', - 'recipients' => array(array('member2@example.com') -), + 'recipients' => array(array('member@example.com')) ), )); @@ -959,15 +957,13 @@ class CRM_Core_BAO_ActionScheduleTest extends CiviUnitTestCase { public function testMembershipOnMultipleReminder() { $membership = $this->createTestObject('CRM_Member_DAO_Membership', array_merge($this->fixtures['rolling_membership'], array('status_id' => 2))); - print_r($membership ); $this->assertTrue(is_numeric($membership->id)); $result = $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))); + $result = $this->callAPISuccess('contact', 'create', array_merge($this->fixtures['contact'], array('contact_id' => $membership->contact_id))); $this->assertAPISuccess($result); $actionScheduleBefore = $this->fixtures['sched_membership_end_2week']; // Send email 2 weeks before end_date @@ -1018,48 +1014,36 @@ class CRM_Core_BAO_ActionScheduleTest extends CiviUnitTestCase { //extend MED to 2 weeks after the current MED (that may signifies as memberhip renewal activity) // and lets assert as of when the new set of reminders will be sent against their respective Schedule Reminders(SR) - $membership->end_date = '2012-06-29'; + $membership->end_date = '2012-06-20'; $membership->save(); - print_r($membership ); - - //change the email id of chosen membership contact to assert - //recipient of not the previously sent mail but the new one - $result = $this->callAPISuccess('Email', 'create', array( - 'is_primary' => 1, - 'contact_id' => $membership->contact_id, - 'email' => 'member2@example.com' - )); - $this->assertAPISuccess($result); + $result = $this->callAPISuccess('Contact', 'get', array('id' => $membership->contact_id)); $this->assertCronRuns( array( array( // 1day 2weeks before membership end date(MED), don't send mail - 'time' => '2012-06-14 01:00:00', + 'time' => '2012-06-05 01:00:00', 'recipients' => array(), ), - //TODO : Add asssertion for before, on and after SR impact on new MED - /* array( // 2 weeks before MED, send an email - 'time' => '2012-06-15 01:00:00', - 'recipients' => array(array('member2@example.com')), + 'time' => '2012-06-06 01:00:00', + 'recipients' => array(array('member@example.com')), ), array( // 1day before MED, don't send mail - 'time' => '2012-06-28 01:00:00', + 'time' => '2012-06-19 01:00:00', 'recipients' => array(), ), array( // On MED, send an email - 'time' => '2012-06-29 00:00:00', + 'time' => '2012-06-20 00:00:00', 'recipients' => array(array('member@example.com')), ), array( // After 1day of MED, send an email - 'time' => '2012-06-30 01:00:00', + 'time' => '2012-06-21 01:00:00', 'recipients' => array(array('member@example.com')), ), array( // After 1day 1min of MED, don't send an email - 'time' => '2012-07-01 00:01:00', + 'time' => '2012-07-21 00:01:00', 'recipients' => array(), ), - */ )); } -- 2.25.1