CRM-15536 fix - Scheduled reminders: Clarify "Limit to" and "Also include" UI to...

https://issues.civicrm.org/jira/browse/CRM-15536

diff --git a/CRM/Admin/Form/ScheduleReminders.php b/CRM/Admin/Form/ScheduleReminders.php
index 39c55aaed15165a597ce08726504f191ade7f81c..947b18ce1b1a07327d97ead965c92cfd8bccdbcf 100644 (file)
--- a/CRM/Admin/Form/ScheduleReminders.php
+++ b/CRM/Admin/Form/ScheduleReminders.php
@@ -196,10 +196,14 @@ class CRM_Admin_Form_ScheduleReminders extends CRM_Admin_Form {
       );
     }
 
-    $limitOptions = array(1 => ts('Limit to'), 0 => ts('Also include'));
-    $this->add('select', 'limit_to', ts('Limit Options'), $limitOptions);
+    $limitOptions = array('' => '-neither-', 1 => ts('Limit to'), 0 => ts('Also include'));
 
-    $this->add('select', 'recipient', ts('Recipients'), $sel5[$recipient],
+    $recipientLabels = array('activity' => ts('Recipients'), 'other' => ts('Limit or Add Recipients'));
+    $this->assign('recipientLabels', $recipientLabels);
+
+    $this->add('select', 'limit_to', ts('Limit Options'), $limitOptions, FALSE, array('onChange' => "showHideByValue('limit_to','','recipient', 'select','select',true);"));
+
+    $this->add('select', 'recipient', $recipientLabels['other'], $sel5[$recipient],
       FALSE, array('onchange' => "showHideByValue('recipient','manual','recipientManual','table-row','select',false); showHideByValue('recipient','group','recipientGroup','table-row','select',false);")
     );
 
@@ -273,6 +277,20 @@ class CRM_Admin_Form_ScheduleReminders extends CRM_Admin_Form {
       }
     }
 
+    $recipientKind = array(
+      'participant_role' => array(
+        'name' => 'participant role',
+        'target_id' => 'recipient_listing'
+      ),
+      'manual' => array(
+        'name' => 'recipient',
+        'target_id' => 'recipient_manual_id'
+      )
+    );
+    if (!empty($fields['limit_to']) && array_key_exists($fields['recipient'], $recipientKind)) {
+      $errors[$recipientKind[$fields['recipient']]['target_id']] = ts('If "Also include" or "Limit to" are selected, you must specify atleast one %1', array(1 => $recipientKind[$fields['recipient']]['name']));
+    }
+
     if (!empty($errors)) {
       return $errors;
     }
@@ -420,6 +438,10 @@ class CRM_Admin_Form_ScheduleReminders extends CRM_Admin_Form {
     $entity_value = $values['entity'][1];
     $entity_status = $values['entity'][2];
 
+    if ($entity_value == 1) {
+      $params['limit_to'] = 1;
+    }
+
     foreach (array(
       'entity_value', 'entity_status') as $key) {
       $params[$key] = implode(CRM_Core_DAO::VALUE_SEPARATOR, $$key);
@@ -554,4 +576,3 @@ class CRM_Admin_Form_ScheduleReminders extends CRM_Admin_Form {
     CRM_Core_Session::setStatus($status, ts('Saved'), 'success');
   }
 }
-

diff --git a/CRM/Core/BAO/ActionSchedule.php b/CRM/Core/BAO/ActionSchedule.php
index fb916ae395cfa8f1c5ee20b28e7eea95cad7ec87..14270f6900fbde4ee3556a347ec9a8387ac86fe2 100755 (executable)
--- a/CRM/Core/BAO/ActionSchedule.php
+++ b/CRM/Core/BAO/ActionSchedule.php
@@ -192,9 +192,9 @@ class CRM_Core_BAO_ActionSchedule extends CRM_Core_DAO_ActionSchedule {
             break;
 
           case 'event_contacts':
-            $eventContacts = CRM_Core_OptionGroup::values('event_contacts');
+            $eventContacts = CRM_Core_OptionGroup::values('event_contacts', FALSE, FALSE, FALSE, NULL, 'label', TRUE, FALSE, 'name');
             $sel5[$entityRecipient] = $eventContacts + $options;
-            $recipientMapping += CRM_Core_OptionGroup::values('event_contacts', FALSE, FALSE, FALSE, NULL, 'name');
+            $recipientMapping += CRM_Core_OptionGroup::values('event_contacts', FALSE, FALSE, FALSE, NULL, 'name', TRUE, FALSE, 'name');
             break;
 
           case NULL:
@@ -295,9 +295,9 @@ class CRM_Core_BAO_ActionSchedule extends CRM_Core_DAO_ActionSchedule {
           break;
 
         case 'event_contacts':
-          $eventContacts = CRM_Core_OptionGroup::values('event_contacts');
+          $eventContacts = CRM_Core_OptionGroup::values('event_contacts', FALSE, FALSE, FALSE, NULL, 'label', TRUE, FALSE, 'name');
           $sel5[$id] = $eventContacts + $options;
-          $recipientMapping += CRM_Core_OptionGroup::values('event_contacts', FALSE, FALSE, FALSE, NULL, 'name');
+          $recipientMapping += CRM_Core_OptionGroup::values('event_contacts', FALSE, FALSE, FALSE, NULL, 'name', TRUE, FALSE, 'name');
           break;
 
         case NULL:
@@ -946,24 +946,26 @@ WHERE reminder.action_schedule_id = %1 AND reminder.action_date_time IS NULL
         $assigneeID = CRM_Utils_Array::key('Activity Assignees', $activityContacts);
         $targetID = CRM_Utils_Array::key('Activity Targets', $activityContacts);
 
-        if ($limitTo == 0) {
-          // including the activity target contacts if 'in addition' is defined
-          $join[] = "INNER JOIN civicrm_activity_contact r ON r.activity_id = e.id AND record_type_id = {$targetID}";
-        }
-        else {
-          switch (CRM_Utils_Array::value($actionSchedule->recipient, $recipientOptions)) {
-            case 'Activity Assignees':
-              $join[] = "INNER JOIN civicrm_activity_contact r ON r.activity_id = e.id AND record_type_id = {$assigneeID}";
-              break;
-
-            case 'Activity Source':
-              $join[] = "INNER JOIN civicrm_activity_contact r ON r.activity_id = e.id AND record_type_id = {$sourceID}";
-              break;
-
-            default:
-            case 'Activity Targets':
-              $join[] = "INNER JOIN civicrm_activity_contact r ON r.activity_id = e.id AND record_type_id = {$targetID}";
-              break;
+        if (!empty($limitTo)) {
+          if ($limitTo == 0) {
+            // including the activity target contacts if 'in addition' is defined
+            $join[] = "INNER JOIN civicrm_activity_contact r ON r.activity_id = e.id AND record_type_id = {$targetID}";
+          }
+          else {
+            switch (CRM_Utils_Array::value($actionSchedule->recipient, $recipientOptions)) {
+              case 'Activity Assignees':
+                $join[] = "INNER JOIN civicrm_activity_contact r ON r.activity_id = e.id AND record_type_id = {$assigneeID}";
+                break;
+
+              case 'Activity Source':
+                $join[] = "INNER JOIN civicrm_activity_contact r ON r.activity_id = e.id AND record_type_id = {$sourceID}";
+                break;
+
+              default:
+              case 'Activity Targets':
+                $join[] = "INNER JOIN civicrm_activity_contact r ON r.activity_id = e.id AND record_type_id = {$targetID}";
+                break;
+            }
           }
         }
         // build where clause
@@ -1131,7 +1133,7 @@ WHERE      $group.id = {$actionSchedule->group_id}
           $where[] = "{$contactField} IN ({$rList})";
         }
       }
-      else {
+      elseif (!empty($limitTo)) {
         $addGroup = $addWhere = '';
         if ($actionSchedule->group_id) {
           // CRM-13577 If smart group then use Cache table
@@ -1209,7 +1211,7 @@ LEFT JOIN {$reminderJoinClause}
 ";
       CRM_Core_DAO::executeQuery($query, array(1 => array($actionSchedule->id, 'Integer')));
 
-      if ($limitTo == 0 && (!empty($addGroup) || !empty($addWhere))) {
+      if (!empty($limitTo) && $limitTo == 0 && (!empty($addGroup) || !empty($addWhere))) {
         $additionWhere = ' WHERE ';
         if ($actionSchedule->start_action_date) {
           $additionWhere = $whereClause . ' AND ';
@@ -1297,7 +1299,7 @@ INNER JOIN {$reminderJoinClause}
           CRM_Core_DAO::executeQuery($query, array(1 => array($actionSchedule->id, 'Integer')));
         }
 
-        if ($limitTo == 0) {
+        if (!empty($limitTo) && $limitTo == 0) {
           $addSelect .= ', MAX(reminder.action_date_time) as latest_log_time';
           $sqlEndEventCheck = "
 SELECT * FROM {$table}

diff --git a/CRM/Upgrade/Incremental/sql/4.6.alpha1.mysql.tpl b/CRM/Upgrade/Incremental/sql/4.6.alpha1.mysql.tpl
index d8cada58e15e0c7c5352e2d0f92195ec22dea0f9..9fd13b68a1311058866bbf1576e2f74abc7625c6 100644 (file)
--- a/CRM/Upgrade/Incremental/sql/4.6.alpha1.mysql.tpl
+++ b/CRM/Upgrade/Incremental/sql/4.6.alpha1.mysql.tpl
@@ -78,3 +78,6 @@ UPDATE `civicrm_state_province` SET `name` = (N'Rīga') WHERE `id` = 3555;
 --CRM-15361: Allow selection of location type when sending bulk email
 ALTER TABLE civicrm_mailing ADD COLUMN location_type_id INT(10) unsigned DEFAULT 0 COMMENT 'With email_selection_method, determines which email address to use';
 ALTER TABLE civicrm_mailing ADD COLUMN email_selection_method varchar(20) DEFAULT 'automatic' COMMENT 'With location_type_id, determine how to choose the email address to use.';
+
+-- CRM-15500 fix
+ALTER TABLE `civicrm_action_schedule` CHANGE `limit_to` `limit_to` TINYINT( 4 ) NULL DEFAULT NULL;
\ No newline at end of file

diff --git a/templates/CRM/Admin/Form/ScheduleReminders.tpl b/templates/CRM/Admin/Form/ScheduleReminders.tpl
index 9ab8fbf2ab1fbb9ddaf2be11070f69be4bfa5f89..943b1afb7a4f14041d1680dbf23a3dfb17a0a7b2 100644 (file)
--- a/templates/CRM/Admin/Form/ScheduleReminders.tpl
+++ b/templates/CRM/Admin/Form/ScheduleReminders.tpl
@@ -82,15 +82,15 @@
     <tr class="crm-scheduleReminder-form-block-recipient">
       <td id="recipientLabel" class="right">{$form.recipient.label}</td><td colspan="3">{$form.limit_to.html}&nbsp;&nbsp;{$form.recipient.html}&nbsp;&nbsp;{help id="recipient" title=$form.recipient.label}</td>
     </tr>
-    <tr id="recipientList" class="crm-scheduleReminder-form-block-recipientListing">
+    <tr id="recipientList" class="crm-scheduleReminder-form-block-recipientListing recipient">
         <td class="right">{$form.recipient_listing.label}</td><td colspan="3">{$form.recipient_listing.html}</td>
     </tr>
-    <tr id="recipientManual" class="crm-scheduleReminder-form-block-recipient_manual_id">
+    <tr id="recipientManual" class="crm-scheduleReminder-form-block-recipient_manual_id recipient">
         <td class="label">{$form.recipient_manual_id.label}</td>
         <td>{$form.recipient_manual_id.html}{edit}<div class="description">{ts}You can manually send out the reminders to these recipients.{/ts}</div>{/edit}</td>
     </tr>
 
-    <tr id="recipientGroup" class="crm-scheduleReminder-form-block-recipient_group_id">
+    <tr id="recipientGroup" class="crm-scheduleReminder-form-block-recipient_group_id recipient">
         <td class="label">{$form.group_id.label}</td>
         <td>{$form.group_id.html}</td>
     </tr>
@@ -187,13 +187,18 @@
       $('#mode', $form).change(loadMsgBox);
 
       showHideLimitTo();
+      buildSelects();
 
       $('#recipient', $form).change(populateRecipient);
+      $('#limit_to', $form).change(populateRecipient).change(buildSelects);
 
       var entity = $('#entity_0', $form).val();
       if (!(entity === '2' || entity === '3') || $('#recipient', $form).val() !== '1') {
         $('#recipientList', $form).hide();
       }
+      else if (entity === '1') {
+        $('#recipient', $form).change(buildSelects);
+      }
 
       function buildSelects() {
         var mappingID = $('#entity_0', $form).val();
@@ -211,8 +216,7 @@
       }
       function populateRecipient() {
         var recipient = $("#recipient", $form).val();
-
-        if (recipientMapping[recipient] == 'Participant Status' || recipientMapping[recipient] == 'participant_role') {
+        if ((recipientMapping[recipient] == 'Participant Status' || recipientMapping[recipient] == 'participant_role') && $('#limit_to').val() != '') {
           CRM.api3('participant', 'getoptions', {field: recipientMapping[recipient] == 'participant_role' ? 'role_id' : 'status_id', sequential: 1})
             .done(function(result) {
               CRM.utils.setOptions($('#recipient_listing', $form), result.values);
@@ -222,11 +226,23 @@
         } else {
           $("#recipientList", $form).hide();
           $('#is_recipient_listing', $form).val('');
+          if ($('#entity_0', $form).val() !== '1' && $('#limit_to').val() == '') {
+            $('tr.recipient:visible').hide();
+          }
         }
       }
       // CRM-14070 Hide limit-to when entity is activity
       function showHideLimitTo() {
         $('#limit_to', $form).toggle(!($('#entity_0', $form).val() == '1'));
+        if ($('#entity_0', $form).val() != '1') {
+          if ($('#limit_to', $form).val() == '') {
+            $('#recipient').hide();
+          }
+          $("label[for='recipient']").text('{/literal}{$recipientLabels.other}{literal}');
+        }
+        else {
+          $("label[for='recipient']").text('{/literal}{$recipientLabels.activity}{literal}');
+        }
       }
     });
 

diff --git a/tests/phpunit/CRM/Core/BAO/ActionScheduleTest.php b/tests/phpunit/CRM/Core/BAO/ActionScheduleTest.php
index d9ea1cb18bc93de3ab0a65881480ce2faccf1af7..f205eef9e2d35f7b6d6354c330f12ee4f5afbbdc 100644 (file)
--- a/tests/phpunit/CRM/Core/BAO/ActionScheduleTest.php
+++ b/tests/phpunit/CRM/Core/BAO/ActionScheduleTest.php
@@ -103,6 +103,7 @@ class CRM_Core_BAO_ActionScheduleTest extends CiviUnitTestCase {
     $this->fixtures['sched_activity_1day'] = array( // create()
       'name' => 'One_Day_Phone_Call_Notice',
       'title' => 'One Day Phone Call Notice',
+      'limit_to' => '1',
       'absolute_date' => NULL,
       'body_html' => '<p>1-Day (non-repeating)</p>',
       'body_text' => '1-Day (non-repeating)',
@@ -132,6 +133,7 @@ class CRM_Core_BAO_ActionScheduleTest extends CiviUnitTestCase {
     $this->fixtures['sched_activity_1day_r'] = array(
       'name' => 'One_Day_Phone_Call_Notice_R',
       'title' => 'One Day Phone Call Notice R',
+      'limit_to' => 1,
       'absolute_date' => NULL,
       'body_html' => '<p>1-Day (repeating)</p>',
       'body_text' => '1-Day (repeating)',

diff --git a/xml/schema/Core/ActionSchedule.xml b/xml/schema/Core/ActionSchedule.xml
index 8253c06cd20d0feab35f9cc516513bbf0f6e9fa4..40f4bec4fbb8537a5fcf85ead5896e2863453b7d 100644 (file)
--- a/xml/schema/Core/ActionSchedule.xml
+++ b/xml/schema/Core/ActionSchedule.xml
@@ -43,7 +43,6 @@
   <field>
     <name>limit_to</name>
     <type>boolean</type>
-    <default>1</default>
     <comment>Is this the recipient criteria limited to OR in addition to?</comment>
     <add>4.4</add>
   </field>