From: monishdeb Date: Tue, 4 Nov 2014 14:31:50 +0000 (+0530) Subject: CRM-15536 fix - Scheduled reminders: Clarify "Limit to" and "Also include" UI to... X-Git-Url: https://vcs.fsf.org/?a=commitdiff_plain;h=84a3e359f3930a81a9183b1c4fe8bba3e96ae98b;p=civicrm-core.git CRM-15536 fix - Scheduled reminders: Clarify "Limit to" and "Also include" UI to prevent user errors https://issues.civicrm.org/jira/browse/CRM-15536 --- diff --git a/CRM/Admin/Form/ScheduleReminders.php b/CRM/Admin/Form/ScheduleReminders.php index 39c55aaed1..947b18ce1b 100644 --- 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 fb916ae395..14270f6900 100755 --- 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 d8cada58e1..9fd13b68a1 100644 --- 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 9ab8fbf2ab..943b1afb7a 100644 --- a/templates/CRM/Admin/Form/ScheduleReminders.tpl +++ b/templates/CRM/Admin/Form/ScheduleReminders.tpl @@ -82,15 +82,15 @@ {$form.recipient.label}{$form.limit_to.html}  {$form.recipient.html}  {help id="recipient" title=$form.recipient.label} - + {$form.recipient_listing.label}{$form.recipient_listing.html} - + {$form.recipient_manual_id.label} {$form.recipient_manual_id.html}{edit}
{ts}You can manually send out the reminders to these recipients.{/ts}
{/edit} - + {$form.group_id.label} {$form.group_id.html} @@ -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 d9ea1cb18b..f205eef9e2 100644 --- 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' => '

1-Day (non-repeating)

', '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' => '

1-Day (repeating)

', 'body_text' => '1-Day (repeating)', diff --git a/xml/schema/Core/ActionSchedule.xml b/xml/schema/Core/ActionSchedule.xml index 8253c06cd2..40f4bec4fb 100644 --- a/xml/schema/Core/ActionSchedule.xml +++ b/xml/schema/Core/ActionSchedule.xml @@ -43,7 +43,6 @@ limit_to boolean - 1 Is this the recipient criteria limited to OR in addition to? 4.4