From fb1f1dec8fce6661d669597dfa73733bd0f8a710 Mon Sep 17 00:00:00 2001 From: colemanw Date: Wed, 19 Jul 2023 17:10:32 -0400 Subject: [PATCH] ScheduledReminders - Add option list for limit_to column and fix type Before: limit_to column is 'boolean' in the metadata and has no option list, but effectively it has 3 options: NULL, '0' and '1'. After: Changed type to 'integer', added pseudoconstant, and renamed the '0' option to '2' so it isn't as easily confused with NULL. --- CRM/Activity/ActionMapping.php | 4 ++-- CRM/Admin/Form/ScheduleReminders.php | 6 +++--- CRM/Contribute/ActionMapping/ByType.php | 2 +- CRM/Core/BAO/ActionSchedule.php | 4 ++-- CRM/Core/BAO/RecurringEntity.php | 2 +- CRM/Core/DAO/ActionSchedule.php | 12 +++++++---- CRM/Core/Form/RecurringEntity.php | 2 +- CRM/Core/SelectValues.php | 20 +++++++++++++++++++ CRM/Event/ActionMapping.php | 2 +- CRM/Upgrade/Incremental/php/FiveSixtyFive.php | 1 + .../Incremental/sql/5.65.alpha1.mysql.tpl | 2 ++ Civi/ActionSchedule/RecipientBuilder.php | 5 ++--- .../CRM/Admin/Form/ScheduleReminders.tpl | 4 ++-- .../CRM/Core/BAO/ActionScheduleTest.php | 4 ++-- .../ActionSchedule/AbstractMappingTest.php | 2 +- tests/phpunit/api/v3/JobTest.php | 10 +++++----- xml/schema/Core/ActionSchedule.xml | 6 +++++- 17 files changed, 59 insertions(+), 29 deletions(-) diff --git a/CRM/Activity/ActionMapping.php b/CRM/Activity/ActionMapping.php index 91f46b5bb5..5d2f337d72 100644 --- a/CRM/Activity/ActionMapping.php +++ b/CRM/Activity/ActionMapping.php @@ -100,9 +100,9 @@ class CRM_Activity_ActionMapping extends \Civi\ActionSchedule\MappingBase { $query['casContactTableAlias'] = NULL; $query['casDateField'] = 'e.activity_date_time'; - if (!is_null($schedule->limit_to)) { + if ($schedule->limit_to) { $activityContacts = \CRM_Activity_BAO_ActivityContact::buildOptions('record_type_id', 'validate'); - if ($schedule->limit_to == 0 || !isset($activityContacts[$schedule->recipient])) { + if ($schedule->limit_to == 2 || !isset($activityContacts[$schedule->recipient])) { $recipientTypeId = \CRM_Utils_Array::key('Activity Targets', $activityContacts); } else { diff --git a/CRM/Admin/Form/ScheduleReminders.php b/CRM/Admin/Form/ScheduleReminders.php index b3f68fd7f3..d2c4dd580e 100644 --- a/CRM/Admin/Form/ScheduleReminders.php +++ b/CRM/Admin/Form/ScheduleReminders.php @@ -218,7 +218,7 @@ class CRM_Admin_Form_ScheduleReminders extends CRM_Admin_Form { $recipientListingOptions = []; - $limitOptions = ['' => '-neither-', 1 => ts('Limit to'), 0 => ts('Also include')]; + $limitOptions = ['' => ts('Neither')] + CRM_Core_BAO_ActionSchedule::buildOptions('limit_to'); $recipientLabels = ['activity' => ts('Recipients'), 'other' => ts('Limit or Add Recipients')]; $this->assign('recipientLabels', $recipientLabels); @@ -351,7 +351,7 @@ class CRM_Admin_Form_ScheduleReminders extends CRM_Admin_Form { 'target_id' => 'recipient_manual_id', ], ]; - if ($fields['limit_to'] != '' && array_key_exists($fields['recipient'], $recipientKind) && empty($fields[$recipientKind[$fields['recipient']]['target_id']])) { + if ($fields['limit_to'] && array_key_exists($fields['recipient'], $recipientKind) && empty($fields[$recipientKind[$fields['recipient']]['target_id']])) { $errors[$recipientKind[$fields['recipient']]['target_id']] = ts('If "Also include" or "Limit to" are selected, you must specify at least one %1', [1 => $recipientKind[$fields['recipient']]['name']]); } @@ -537,7 +537,7 @@ class CRM_Admin_Form_ScheduleReminders extends CRM_Admin_Form { $params['group_id'] = $values['group_id']; $params['recipient_manual'] = $params['recipient'] = $params['recipient_listing'] = 'null'; } - elseif (isset($values['recipient_listing']) && isset($values['limit_to']) && !CRM_Utils_System::isNull($values['recipient_listing']) && !CRM_Utils_System::isNull($values['limit_to'])) { + elseif (isset($values['recipient_listing']) && !empty($values['limit_to']) && !CRM_Utils_System::isNull($values['recipient_listing'])) { $params['recipient'] = $values['recipient'] ?? NULL; $params['recipient_listing'] = implode(CRM_Core_DAO::VALUE_SEPARATOR, CRM_Utils_Array::value('recipient_listing', $values) diff --git a/CRM/Contribute/ActionMapping/ByType.php b/CRM/Contribute/ActionMapping/ByType.php index 27f0c48184..2cab15f818 100644 --- a/CRM/Contribute/ActionMapping/ByType.php +++ b/CRM/Contribute/ActionMapping/ByType.php @@ -135,7 +135,7 @@ class CRM_Contribute_ActionMapping_ByType extends CRM_Contribute_ActionMapping { ->param('selectedStatuses', $selectedStatuses); } - if ($schedule->recipient_listing && $schedule->limit_to) { + if ($schedule->recipient_listing && $schedule->limit_to == 1) { switch ($schedule->recipient) { case 'soft_credit_type': $query['casContactIdField'] = 'soft.contact_id'; diff --git a/CRM/Core/BAO/ActionSchedule.php b/CRM/Core/BAO/ActionSchedule.php index 065c60464e..31c440ddac 100644 --- a/CRM/Core/BAO/ActionSchedule.php +++ b/CRM/Core/BAO/ActionSchedule.php @@ -460,13 +460,13 @@ FROM civicrm_action_schedule cas ->where("reminder.action_date_time IS NULL") ->param([ 'casActionScheduleId' => $actionSchedule->id, - 'casMailingJoinType' => ($actionSchedule->limit_to == 0) ? 'LEFT JOIN' : 'INNER JOIN', + 'casMailingJoinType' => ($actionSchedule->limit_to == 2) ? 'LEFT JOIN' : 'INNER JOIN', 'casMappingId' => $mapping->getId(), 'casMappingEntity' => $mapping->getEntityTable(), 'casEntityJoinExpr' => 'e.id = IF(reminder.entity_table = "civicrm_contact", reminder.contact_id, reminder.entity_id)', ]); - if ($actionSchedule->limit_to == 0) { + if ($actionSchedule->limit_to == 2) { $select->where("e.id = reminder.entity_id OR reminder.entity_table = 'civicrm_contact'"); } diff --git a/CRM/Core/BAO/RecurringEntity.php b/CRM/Core/BAO/RecurringEntity.php index 10ff3b0ff6..9a8aa74492 100644 --- a/CRM/Core/BAO/RecurringEntity.php +++ b/CRM/Core/BAO/RecurringEntity.php @@ -1014,7 +1014,7 @@ class CRM_Core_BAO_RecurringEntity extends CRM_Core_DAO_RecurringEntity implemen $concatStartActionDateBits = $startActionDate1 . strtoupper(substr($startActionDate[1], 0, 2)); $r->byday([$concatStartActionDateBits]); } - elseif ($scheduleReminderDetails['limit_to']) { + elseif ($scheduleReminderDetails['limit_to'] == 1) { $r->bymonthday([$scheduleReminderDetails['limit_to']]); } } diff --git a/CRM/Core/DAO/ActionSchedule.php b/CRM/Core/DAO/ActionSchedule.php index b7021c77f4..50f298b14d 100644 --- a/CRM/Core/DAO/ActionSchedule.php +++ b/CRM/Core/DAO/ActionSchedule.php @@ -6,7 +6,7 @@ * * Generated from xml/schema/CRM/Core/ActionSchedule.xml * DO NOT EDIT. Generated by CRM_Core_CodeGen - * (GenCodeChecksum:247a35dcd0734c0cd3d00c42910cf725) + * (GenCodeChecksum:0c18fcc2d83152d208ffb128cf3b0e32) */ /** @@ -43,6 +43,7 @@ class CRM_Core_DAO_ActionSchedule extends CRM_Core_DAO { * @var string[] */ protected static $_paths = [ + 'browse' => 'civicrm/admin/scheduleReminders', 'add' => 'civicrm/admin/scheduleReminders/edit?reset=1&action=add', 'update' => 'civicrm/admin/scheduleReminders/edit?reset=1&action=update&id=[id]', 'delete' => 'civicrm/admin/scheduleReminders/edit?reset=1&action=delete&id=[id]', @@ -85,8 +86,8 @@ class CRM_Core_DAO_ActionSchedule extends CRM_Core_DAO { /** * Is this the recipient criteria limited to OR in addition to? * - * @var bool|string|null - * (SQL type: tinyint) + * @var int|string|null + * (SQL type: int) * Note that values will be retrieved from the database as a string. */ public $limit_to; @@ -551,7 +552,7 @@ class CRM_Core_DAO_ActionSchedule extends CRM_Core_DAO { ], 'limit_to' => [ 'name' => 'limit_to', - 'type' => CRM_Utils_Type::T_BOOLEAN, + 'type' => CRM_Utils_Type::T_INT, 'title' => ts('Limit To'), 'description' => ts('Is this the recipient criteria limited to OR in addition to?'), 'usage' => [ @@ -568,6 +569,9 @@ class CRM_Core_DAO_ActionSchedule extends CRM_Core_DAO { 'html' => [ 'label' => ts("Limit To"), ], + 'pseudoconstant' => [ + 'callback' => 'CRM_Core_SelectValues::getLimitToValues', + ], 'add' => '4.4', ], 'entity_value' => [ diff --git a/CRM/Core/Form/RecurringEntity.php b/CRM/Core/Form/RecurringEntity.php index 67ce4f0052..181f7cc140 100644 --- a/CRM/Core/Form/RecurringEntity.php +++ b/CRM/Core/Form/RecurringEntity.php @@ -134,7 +134,7 @@ class CRM_Core_Form_RecurringEntity { $defaults['ends'] = 2; } $defaults['limit_to'] = self::$_scheduleReminderDetails->limit_to; - if (self::$_scheduleReminderDetails->limit_to) { + if (self::$_scheduleReminderDetails->limit_to == 1) { $defaults['repeats_by'] = 1; } if (self::$_scheduleReminderDetails->entity_status) { diff --git a/CRM/Core/SelectValues.php b/CRM/Core/SelectValues.php index 7f0905ac36..0ecdede8c0 100644 --- a/CRM/Core/SelectValues.php +++ b/CRM/Core/SelectValues.php @@ -1229,4 +1229,24 @@ class CRM_Core_SelectValues { return $options; } + /** + * Limit-to options for schedule reminders. + * + * @return array + */ + public static function getLimitToValues(): array { + return [ + [ + 'id' => 1, + 'name' => 'limit', + 'label' => ts('Limit to'), + ], + [ + 'id' => 2, + 'name' => 'add', + 'label' => ts('Also include'), + ], + ]; + } + } diff --git a/CRM/Event/ActionMapping.php b/CRM/Event/ActionMapping.php index 1aa9057f69..424090141c 100644 --- a/CRM/Event/ActionMapping.php +++ b/CRM/Event/ActionMapping.php @@ -116,7 +116,7 @@ abstract class CRM_Event_ActionMapping extends \Civi\ActionSchedule\MappingBase } $query->join('r', 'INNER JOIN civicrm_event r ON e.event_id = r.id'); - if ($schedule->recipient_listing && $schedule->limit_to) { + if ($schedule->recipient_listing && $schedule->limit_to == 1) { switch ($schedule->recipient) { case 'participant_role': $regex = "([[:cntrl:]]|^)" . implode('([[:cntrl:]]|$)|([[:cntrl:]]|^)', \CRM_Utils_Array::explodePadded($schedule->recipient_listing)) . "([[:cntrl:]]|$)"; diff --git a/CRM/Upgrade/Incremental/php/FiveSixtyFive.php b/CRM/Upgrade/Incremental/php/FiveSixtyFive.php index b2449b592b..0abb94df67 100644 --- a/CRM/Upgrade/Incremental/php/FiveSixtyFive.php +++ b/CRM/Upgrade/Incremental/php/FiveSixtyFive.php @@ -34,6 +34,7 @@ class CRM_Upgrade_Incremental_php_FiveSixtyFive extends CRM_Upgrade_Incremental_ $this->addTask('Make Group.title required', 'alterColumn', 'civicrm_group', 'title', "varchar(255) NOT NULL COMMENT 'Alternative public title for this Group.'", TRUE); $this->addTask('Make Group.frontend_title required', 'alterColumn', 'civicrm_group', 'frontend_title', "varchar(255) NOT NULL COMMENT 'Alternative public description of the group.'", TRUE); + $this->addTask('Update ActionSchedule.limit_to column', 'alterColumn', 'civicrm_action_schedule', 'limit_to', "int COMMENT 'Is this the recipient criteria limited to OR in addition to?'"); } } diff --git a/CRM/Upgrade/Incremental/sql/5.65.alpha1.mysql.tpl b/CRM/Upgrade/Incremental/sql/5.65.alpha1.mysql.tpl index ca22e0fdbc..b0bb6bd1f4 100644 --- a/CRM/Upgrade/Incremental/sql/5.65.alpha1.mysql.tpl +++ b/CRM/Upgrade/Incremental/sql/5.65.alpha1.mysql.tpl @@ -27,6 +27,8 @@ UPDATE `civicrm_job` SET `api_entity` = CONCAT(UPPER(SUBSTRING(`api_entity`, 1 , WHERE (`frontend_description` IS NULL OR `frontend_description` = '') AND description <> ''; {/if} +UPDATE `civicrm_action_schedule` SET `limit_to` = 2 WHERE `limit_to` = 0; + {literal} UPDATE civicrm_mailing_component SET body_html = REPLACE(body_html, '{welcome.group}', '{group.frontend_title}'), diff --git a/Civi/ActionSchedule/RecipientBuilder.php b/Civi/ActionSchedule/RecipientBuilder.php index c32820475f..959baf56f0 100644 --- a/Civi/ActionSchedule/RecipientBuilder.php +++ b/Civi/ActionSchedule/RecipientBuilder.php @@ -280,10 +280,9 @@ class RecipientBuilder { 'casNow' => $this->now, ]; - /** @var \CRM_Utils_SQL_Select $query */ $query = $this->mapping->createQuery($this->actionSchedule, $phase, $defaultParams); - if ($this->actionSchedule->limit_to /*1*/) { + if ($this->actionSchedule->limit_to == 1) { $query->merge($this->prepareContactFilter($query['casContactIdField'])); } @@ -468,7 +467,7 @@ WHERE $group.id = {$groupId} */ protected function prepareAddlFilter($contactIdField) { $contactAddlFilter = NULL; - if ($this->actionSchedule->limit_to !== NULL && !$this->actionSchedule->limit_to /*0*/) { + if ($this->actionSchedule->limit_to == 2) { $contactAddlFilter = $this->prepareContactFilter($contactIdField); } return $contactAddlFilter; diff --git a/templates/CRM/Admin/Form/ScheduleReminders.tpl b/templates/CRM/Admin/Form/ScheduleReminders.tpl index e58203ddbb..8873c50b29 100644 --- a/templates/CRM/Admin/Form/ScheduleReminders.tpl +++ b/templates/CRM/Admin/Form/ScheduleReminders.tpl @@ -226,10 +226,10 @@ if ($('#entity_0', $form).val() != '1' || !($('#entity_0').length)) { // Some Event entity is selected. if (['2', '3', '5'].includes($('#entity_0', $form).val())) { - $('#limit_to option[value="0"]', $form).attr('disabled','disabled').removeAttr('selected'); + $('#limit_to option[value="2"]', $form).attr('disabled','disabled').removeAttr('selected'); } else { - $('#limit_to option[value="0"]', $form).removeAttr('disabled'); + $('#limit_to option[value="2"]', $form).removeAttr('disabled'); } // Anything but Activity is selected. if ($('#limit_to', $form).val() == '') { diff --git a/tests/phpunit/CRM/Core/BAO/ActionScheduleTest.php b/tests/phpunit/CRM/Core/BAO/ActionScheduleTest.php index c65d292d61..1a6b64919a 100644 --- a/tests/phpunit/CRM/Core/BAO/ActionScheduleTest.php +++ b/tests/phpunit/CRM/Core/BAO/ActionScheduleTest.php @@ -750,7 +750,7 @@ class CRM_Core_BAO_ActionScheduleTest extends CiviUnitTestCase { 'end_frequency_unit' => 'month', 'entity_status' => '', 'entity_value' => '', - 'limit_to' => 0, + 'limit_to' => 2, 'group_id' => '', 'is_active' => 1, 'is_repeat' => '1', @@ -2319,7 +2319,7 @@ class CRM_Core_BAO_ActionScheduleTest extends CiviUnitTestCase { $this->callAPISuccess('Participant', 'create', $params); $actionSchedule = $this->fixtures['sched_event_type_start_1week_before']; - $actionSchedule['limit_to'] = TRUE; + $actionSchedule['limit_to'] = 1; $actionSchedule['recipient'] = 'participant_role'; $actionSchedule['recipient_listing'] = $testValue['recipient_listing']; $actionSchedule['entity_value'] = CRM_Core_DAO::getFieldValue('CRM_Event_DAO_Event', $params['event_id'], 'event_type_id'); diff --git a/tests/phpunit/Civi/ActionSchedule/AbstractMappingTest.php b/tests/phpunit/Civi/ActionSchedule/AbstractMappingTest.php index 3b803422a1..fb0317cdf5 100644 --- a/tests/phpunit/Civi/ActionSchedule/AbstractMappingTest.php +++ b/tests/phpunit/Civi/ActionSchedule/AbstractMappingTest.php @@ -173,7 +173,7 @@ abstract class AbstractMappingTest extends \CiviUnitTestCase { * Also include recipient Bob. */ public function alsoRecipientBob() { - $this->schedule->limit_to = 0; + $this->schedule->limit_to = 2; $this->schedule->recipient = NULL; $this->schedule->recipient_listing = NULL; $this->schedule->recipient_manual = $this->contacts['bob']['id']; diff --git a/tests/phpunit/api/v3/JobTest.php b/tests/phpunit/api/v3/JobTest.php index d1b1dee9ed..2685c0e10c 100644 --- a/tests/phpunit/api/v3/JobTest.php +++ b/tests/phpunit/api/v3/JobTest.php @@ -231,7 +231,7 @@ class api_v3_JobTest extends CiviUnitTestCase { 'start_action_condition' => 'before', 'start_action_unit' => 'hour', 'group_id' => $groupID, - 'limit_to' => FALSE, + 'limit_to' => 2, ]); $this->callAPISuccess('group_contact', 'create', [ 'contact_id' => $contactID, @@ -264,7 +264,7 @@ class api_v3_JobTest extends CiviUnitTestCase { 'start_action_condition' => 'before', 'start_action_unit' => 'day', 'group_id' => $groupID, - 'limit_to' => TRUE, + 'limit_to' => 1, 'sms_provider_id' => $provider['id'], 'mode' => 'User_Preference', ]); @@ -329,7 +329,7 @@ class api_v3_JobTest extends CiviUnitTestCase { 'start_action_condition' => 'before', 'start_action_unit' => 'day', 'group_id' => $groupId, - 'limit_to' => FALSE, + 'limit_to' => 2, 'mode' => 'Email', ]); @@ -361,7 +361,7 @@ class api_v3_JobTest extends CiviUnitTestCase { 'start_action_condition' => 'before', 'start_action_unit' => 'day', 'group_id' => $groupId, - 'limit_to' => FALSE, + 'limit_to' => 2, 'mode' => 'Email', ]); $this->callAPISuccess('event', 'delete', ['id' => $eventId]); @@ -393,7 +393,7 @@ class api_v3_JobTest extends CiviUnitTestCase { 'start_action_condition' => 'before', 'start_action_unit' => 'day', 'group_id' => $groupID, - 'limit_to' => TRUE, + 'limit_to' => 1, 'sms_provider_id' => $provider['id'], 'mode' => 'SMS', ]); diff --git a/xml/schema/Core/ActionSchedule.xml b/xml/schema/Core/ActionSchedule.xml index 9c2194faf2..7740f4be77 100644 --- a/xml/schema/Core/ActionSchedule.xml +++ b/xml/schema/Core/ActionSchedule.xml @@ -8,6 +8,7 @@ 3.4 title + civicrm/admin/scheduleReminders civicrm/admin/scheduleReminders/edit?reset=1&action=add civicrm/admin/scheduleReminders/edit?reset=1&action=update&id=[id] civicrm/admin/scheduleReminders/edit?reset=1&action=delete&id=[id] @@ -54,11 +55,14 @@ limit_to - boolean + int Is this the recipient criteria limited to OR in addition to? + + CRM_Core_SelectValues::getLimitToValues + 4.4 -- 2.25.1