From 77e1639146fcc3f0d9e71a058f686ac67f07f206 Mon Sep 17 00:00:00 2001 From: Tim Otten Date: Tue, 28 Jul 2015 20:26:04 -0700 Subject: [PATCH] CRM-13244 - ActionSchedule\Mapping - Privatize various fields These fields are kind of stupid and are mostly only used within the Mapping class. The main external usage was for looking up a mapping record by `entity_value`. However, this can be made simpler now that mapping IDs are constant. --- CRM/Admin/Form/ScheduleReminders.php | 6 +- CRM/Core/ActionScheduleTmp.php | 19 +++-- CRM/Core/BAO/ActionSchedule.php | 69 +++++++------------ CRM/Event/BAO/Event.php | 6 +- .../Form/ManageEvent/ScheduleReminders.php | 9 ++- CRM/Event/Form/ManageEvent/TabHeader.php | 2 +- CRM/Event/Page/ManageEvent.php | 2 +- Civi/ActionSchedule/Mapping.php | 42 +++++++++-- tests/phpunit/api/v3/ActionScheduleTest.php | 10 +-- 9 files changed, 87 insertions(+), 78 deletions(-) diff --git a/CRM/Admin/Form/ScheduleReminders.php b/CRM/Admin/Form/ScheduleReminders.php index 8d59efb64c..39fb4108b2 100644 --- a/CRM/Admin/Form/ScheduleReminders.php +++ b/CRM/Admin/Form/ScheduleReminders.php @@ -82,13 +82,9 @@ class CRM_Admin_Form_ScheduleReminders extends CRM_Admin_Form { elseif (!empty($this->_context)) { if ($this->_context == 'event') { $this->_compId = CRM_Utils_Request::retrieve('compId', 'Integer', $this); - $field = 'civicrm_event'; $isTemplate = CRM_Core_DAO::getFieldValue('CRM_Event_DAO_Event', $this->_compId, 'is_template'); - if ($isTemplate) { - $field = 'event_template'; - } $mapping = CRM_Utils_Array::first(CRM_Core_BAO_ActionSchedule::getMappings(array( - 'entity_value' => $field, + 'id' => $isTemplate ? CRM_Core_ActionScheduleTmp::EVENT_TPL_MAPPING_ID : CRM_Core_ActionScheduleTmp::EVENT_NAME_MAPPING_ID, ))); if ($mapping) { $this->_mappingID = $mapping->id; diff --git a/CRM/Core/ActionScheduleTmp.php b/CRM/Core/ActionScheduleTmp.php index 146026632f..8c21a3988f 100644 --- a/CRM/Core/ActionScheduleTmp.php +++ b/CRM/Core/ActionScheduleTmp.php @@ -14,6 +14,13 @@ use Symfony\Component\EventDispatcher\EventSubscriberInterface; */ class CRM_Core_ActionScheduleTmp implements EventSubscriberInterface { + const ACTIVITY_MAPPING_ID = 1; + const EVENT_TYPE_MAPPING_ID = 2; + const EVENT_NAME_MAPPING_ID = 3; + const MEMBERSHIP_TYPE_MAPPING_ID = 4; + const EVENT_TPL_MAPPING_ID = 5; + const CONTACT_MAPPING_ID = 6; + /** * @inheritDoc */ @@ -27,7 +34,7 @@ class CRM_Core_ActionScheduleTmp implements EventSubscriberInterface { public function onRegisterMappings(\Civi\ActionSchedule\Event\MappingRegisterEvent $registrations) { $registrations->register(\Civi\ActionSchedule\Mapping::create(array( - 'id' => 1, + 'id' => CRM_Core_ActionScheduleTmp::ACTIVITY_MAPPING_ID, 'entity' => 'civicrm_activity', 'entity_label' => ts('Activity'), 'entity_value' => 'activity_type', @@ -38,7 +45,7 @@ class CRM_Core_ActionScheduleTmp implements EventSubscriberInterface { 'entity_recipient' => 'activity_contacts', ))); $registrations->register(\Civi\ActionSchedule\Mapping::create(array( - 'id' => 2, + 'id' => CRM_Core_ActionScheduleTmp::EVENT_TYPE_MAPPING_ID, 'entity' => 'civicrm_participant', 'entity_label' => ts('Event Type'), 'entity_value' => 'event_type', @@ -50,7 +57,7 @@ class CRM_Core_ActionScheduleTmp implements EventSubscriberInterface { 'entity_recipient' => 'event_contacts', ))); $registrations->register(\Civi\ActionSchedule\Mapping::create(array( - 'id' => 3, + 'id' => CRM_Core_ActionScheduleTmp::EVENT_NAME_MAPPING_ID, 'entity' => 'civicrm_participant', 'entity_label' => ts('Event Name'), 'entity_value' => 'civicrm_event', @@ -62,7 +69,7 @@ class CRM_Core_ActionScheduleTmp implements EventSubscriberInterface { 'entity_recipient' => 'event_contacts', ))); $registrations->register(\Civi\ActionSchedule\Mapping::create(array( - 'id' => 4, + 'id' => CRM_Core_ActionScheduleTmp::MEMBERSHIP_TYPE_MAPPING_ID, 'entity' => 'civicrm_membership', 'entity_label' => ts('Membership'), 'entity_value' => 'civicrm_membership_type', @@ -73,7 +80,7 @@ class CRM_Core_ActionScheduleTmp implements EventSubscriberInterface { 'entity_date_end' => 'membership_end_date', ))); $registrations->register(\Civi\ActionSchedule\Mapping::create(array( - 'id' => 5, + 'id' => CRM_Core_ActionScheduleTmp::EVENT_TPL_MAPPING_ID, 'entity' => 'civicrm_participant', 'entity_label' => ts('Event Template'), 'entity_value' => 'event_template', @@ -85,7 +92,7 @@ class CRM_Core_ActionScheduleTmp implements EventSubscriberInterface { 'entity_recipient' => 'event_contacts', ))); $registrations->register(\Civi\ActionSchedule\Mapping::create(array( - 'id' => 6, + 'id' => CRM_Core_ActionScheduleTmp::CONTACT_MAPPING_ID, 'entity' => 'civicrm_contact', 'entity_label' => ts('Contact'), 'entity_value' => 'civicrm_contact', diff --git a/CRM/Core/BAO/ActionSchedule.php b/CRM/Core/BAO/ActionSchedule.php index e0c6355961..283c620bf3 100755 --- a/CRM/Core/BAO/ActionSchedule.php +++ b/CRM/Core/BAO/ActionSchedule.php @@ -97,7 +97,7 @@ class CRM_Core_BAO_ActionSchedule extends CRM_Core_DAO_ActionSchedule { $entityValueLabels[$mapping->id] = $mapping->getValueLabels(); // Not sure why: everything *except* contact-dates have a $valueLabel. - if ($mapping->entity_value !== 'civicrm_contact') { + if ($mapping->id !== CRM_Core_ActionScheduleTmp::CONTACT_MAPPING_ID) { $valueLabel = array('- ' . strtolower($mapping->entity_value_label) . ' -'); $entityValueLabels[$mapping->id] = $valueLabel + $entityValueLabels[$mapping->id]; } @@ -157,13 +157,15 @@ class CRM_Core_BAO_ActionSchedule extends CRM_Core_DAO_ActionSchedule { * @param bool $namesOnly * Return simple list of names. * - * @param null $entityValue - * @param int $id + * @param \Civi\ActionSchedule\Mapping|NULL $filterMapping + * Filter by the schedule's mapping type. + * @param int $filterValue + * Filter by the schedule's entity_value. * * @return array * (reference) reminder list */ - public static function &getList($namesOnly = FALSE, $entityValue = NULL, $id = NULL) { + public static function &getList($namesOnly = FALSE, $filterMapping = NULL, $filterValue = NULL) { $query = " SELECT title, @@ -183,20 +185,17 @@ FROM civicrm_action_schedule cas "; $queryParams = array(); $where = " WHERE 1 "; - if ($entityValue and $id) { - $mappings = self::getMappings(array( - 'entity_value' => $entityValue, - )); - $mappingIds = implode(',', array_keys($mappings)); - $where .= " AND cas.entity_value = %1 AND cas.mapping_id IN ($mappingIds)"; - $queryParams[1] = array($id, 'Integer'); + if ($filterMapping and $filterValue) { + $where .= " AND cas.entity_value = %1 AND cas.mapping_id = %2"; + $queryParams[1] = array($filterValue, 'Integer'); + $queryParams[2] = array($filterMapping->id, 'Integer'); } $where .= " AND cas.used_for IS NULL"; $query .= $where; $dao = CRM_Core_DAO::executeQuery($query, $queryParams); while ($dao->fetch()) { - /** @var Civi\ActionSchedule\Mapping $mapping */ - $mapping = CRM_Utils_Array::first(self::getMappings(array( + /** @var Civi\ActionSchedule\Mapping $filterMapping */ + $filterMapping = CRM_Utils_Array::first(self::getMappings(array( 'id' => $dao->mapping_id, ))); $list[$dao->id]['id'] = $dao->id; @@ -206,13 +205,13 @@ FROM civicrm_action_schedule cas $list[$dao->id]['start_action_condition'] = $dao->start_action_condition; $list[$dao->id]['entityDate'] = ucwords(str_replace('_', ' ', $dao->entityDate)); $list[$dao->id]['absolute_date'] = $dao->absolute_date; - $list[$dao->id]['entity'] = $mapping->entity_label; + $list[$dao->id]['entity'] = $filterMapping->entity_label; $list[$dao->id]['value'] = implode(', ', CRM_Utils_Array::subset( - $mapping->getValueLabels(), + $filterMapping->getValueLabels(), explode(CRM_Core_DAO::VALUE_SEPARATOR, $dao->entityValueIds) )); $list[$dao->id]['status'] = implode(', ', CRM_Utils_Array::subset( - $mapping->getStatusLabels($dao->entityValueIds), + $filterMapping->getStatusLabels($dao->entityValueIds), explode(CRM_Core_DAO::VALUE_SEPARATOR, $dao->entityStatusIds) )); $list[$dao->id]['is_repeat'] = $dao->is_repeat; @@ -391,6 +390,7 @@ FROM civicrm_action_schedule cas $actionSchedule->find(); while ($actionSchedule->fetch()) { + /** @var \Civi\ActionSchedule\Mapping $mapping */ $mapping = CRM_Utils_Array::first(self::getMappings(array( 'id' => $mappingID, ))); @@ -414,14 +414,6 @@ FROM civicrm_action_schedule cas $anniversary = FALSE; - if (!CRM_Utils_System::isNull($mapping->entity_recipient)) { - if ($mapping->entity_recipient == 'event_contacts') { - $recipientOptions = CRM_Core_OptionGroup::values($mapping->entity_recipient, FALSE, FALSE, FALSE, NULL, 'name', TRUE, FALSE, 'name'); - } - else { - $recipientOptions = CRM_Core_OptionGroup::values($mapping->entity_recipient, FALSE, FALSE, FALSE, NULL, 'name'); - } - } $from = "{$mapping->entity} e"; if ($mapping->entity == 'civicrm_activity') { @@ -438,7 +430,7 @@ FROM civicrm_action_schedule cas $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)) { + switch (CRM_Utils_Array::value($actionSchedule->recipient, $mapping->getRecipientOptions())) { case 'Activity Assignees': $join[] = "INNER JOIN civicrm_activity_contact r ON r.activity_id = e.id AND record_type_id = {$assigneeID}"; break; @@ -480,7 +472,7 @@ FROM civicrm_action_schedule cas ); $rList = implode(',', $rList); - switch (CRM_Utils_Array::value($actionSchedule->recipient, $recipientOptions)) { + switch (CRM_Utils_Array::value($actionSchedule->recipient, $mapping->getRecipientOptions())) { case 'participant_role': $where[] = "e.role_id IN ({$rList})"; break; @@ -492,10 +484,10 @@ FROM civicrm_action_schedule cas // build where clause if (!empty($value)) { - $where[] = ($mapping->entity_value == 'event_type') ? "r.event_type_id IN ({$value})" : "r.id IN ({$value})"; + $where[] = ($mapping->id == CRM_Core_ActionScheduleTmp::EVENT_TYPE_MAPPING_ID) ? "r.event_type_id IN ({$value})" : "r.id IN ({$value})"; } else { - $where[] = ($mapping->entity_value == 'event_type') ? "r.event_type_id IS NULL" : "r.id IS NULL"; + $where[] = ($mapping->id == CRM_Core_ActionScheduleTmp::EVENT_TYPE_MAPPING_ID) ? "r.event_type_id IS NULL" : "r.id IS NULL"; } // participant status criteria not to be implemented @@ -959,28 +951,15 @@ WHERE m.owner_membership_id IS NOT NULL AND * @return array */ public static function getRecipientListing($mappingID, $recipientType) { - $options = array(); - if (!$mappingID || !$recipientType) { - return $options; + if (!$mappingID) { + return array(); } + /** @var \Civi\ActionSchedule\Mapping $mapping */ $mapping = CRM_Utils_Array::first(CRM_Core_BAO_ActionSchedule::getMappings(array( 'id' => $mappingID, ))); - - switch ($mapping->entity) { - case 'civicrm_participant': - $eventContacts = CRM_Core_OptionGroup::values('event_contacts', FALSE, FALSE, FALSE, NULL, 'name', TRUE, FALSE, 'name'); - if (empty($eventContacts[$recipientType])) { - return $options; - } - if ($eventContacts[$recipientType] == 'participant_role') { - $options = CRM_Event_PseudoConstant::participantRole(); - } - break; - } - - return $options; + return $mapping->getRecipientListing($recipientType); } /** diff --git a/CRM/Event/BAO/Event.php b/CRM/Event/BAO/Event.php index bf5bbd84af..f1f91e3bc1 100644 --- a/CRM/Event/BAO/Event.php +++ b/CRM/Event/BAO/Event.php @@ -441,7 +441,7 @@ $event_summary_limit $params = array(1 => array($optionGroupId, 'Integer')); $mapping = CRM_Utils_Array::first(CRM_Core_BAO_ActionSchedule::getMappings(array( - 'entity_value' => 'civicrm_event', + 'id' => CRM_Core_ActionScheduleTmp::EVENT_NAME_MAPPING_ID, ))); $dao = CRM_Core_DAO::executeQuery($query, $params); while ($dao->fetch()) { @@ -986,10 +986,10 @@ WHERE civicrm_event.is_active = 1 ); $oldMapping = CRM_Utils_Array::first(CRM_Core_BAO_ActionSchedule::getMappings(array( - 'entity_value' => ($eventValues['is_template'] ? 'event_template' : 'civicrm_event'), + 'id' => ($eventValues['is_template'] ? CRM_Core_ActionScheduleTmp::EVENT_TPL_MAPPING_ID : CRM_Core_ActionScheduleTmp::EVENT_NAME_MAPPING_ID), ))); $copyMapping = CRM_Utils_Array::first(CRM_Core_BAO_ActionSchedule::getMappings(array( - 'entity_value' => ($copyEvent->is_template == 1 ? 'event_template' : 'civicrm_event'), + 'id' => ($copyEvent->is_template == 1 ? CRM_Core_ActionScheduleTmp::EVENT_TPL_MAPPING_ID : CRM_Core_ActionScheduleTmp::EVENT_NAME_MAPPING_ID), ))); $copyReminder = &CRM_Core_DAO::copyGeneric('CRM_Core_DAO_ActionSchedule', array('entity_value' => $id, 'mapping_id' => $oldMapping->id), diff --git a/CRM/Event/Form/ManageEvent/ScheduleReminders.php b/CRM/Event/Form/ManageEvent/ScheduleReminders.php index 2d049286a6..273051fdf3 100644 --- a/CRM/Event/Form/ManageEvent/ScheduleReminders.php +++ b/CRM/Event/Form/ManageEvent/ScheduleReminders.php @@ -49,11 +49,10 @@ class CRM_Event_Form_ManageEvent_ScheduleReminders extends CRM_Event_Form_Manage parent::preProcess(); $setTab = CRM_Utils_Request::retrieve('setTab', 'Int', $this, FALSE, 0); - $field = 'civicrm_event'; - if ($this->_isTemplate) { - $field = 'event_template'; - } - $reminderList = CRM_Core_BAO_ActionSchedule::getList(FALSE, $field, $this->_id); + $mapping = CRM_Utils_Array::first(CRM_Core_BAO_ActionSchedule::getMappings(array( + 'id' => ($this->_isTemplate ? CRM_Core_ActionScheduleTmp::EVENT_TPL_MAPPING_ID : CRM_Core_ActionScheduleTmp::EVENT_NAME_MAPPING_ID), + ))); + $reminderList = CRM_Core_BAO_ActionSchedule::getList(FALSE, $mapping, $this->_id); if ($reminderList && is_array($reminderList)) { // Add action links to each of the reminders foreach ($reminderList as & $format) { diff --git a/CRM/Event/Form/ManageEvent/TabHeader.php b/CRM/Event/Form/ManageEvent/TabHeader.php index 7a32fd3bec..f96ab2ae1e 100644 --- a/CRM/Event/Form/ManageEvent/TabHeader.php +++ b/CRM/Event/Form/ManageEvent/TabHeader.php @@ -110,7 +110,7 @@ class CRM_Event_Form_ManageEvent_TabHeader { if ($eventID) { // disable tabs based on their configuration status $eventNameMapping = CRM_Utils_Array::first(CRM_Core_BAO_ActionSchedule::getMappings(array( - 'entity_value' => 'civicrm_event', + 'id' => CRM_Core_ActionScheduleTmp::EVENT_NAME_MAPPING_ID, ))); $sql = " SELECT e.loc_block_id as is_location, e.is_online_registration, e.is_monetary, taf.is_active, pcp.is_active as is_pcp, sch.id as is_reminder, re.id as is_repeating_event diff --git a/CRM/Event/Page/ManageEvent.php b/CRM/Event/Page/ManageEvent.php index 75b1d17184..8ddbc0288b 100644 --- a/CRM/Event/Page/ManageEvent.php +++ b/CRM/Event/Page/ManageEvent.php @@ -314,7 +314,7 @@ ORDER BY start_date desc ); $this->assign('eventCartEnabled', $enableCart); $mapping = CRM_Utils_Array::first(CRM_Core_BAO_ActionSchedule::getMappings(array( - 'entity_value' => 'civicrm_event', + 'id' => CRM_Core_ActionScheduleTmp::EVENT_NAME_MAPPING_ID, ))); $eventType = CRM_Core_OptionGroup::values('event_type'); while ($dao->fetch()) { diff --git a/Civi/ActionSchedule/Mapping.php b/Civi/ActionSchedule/Mapping.php index 894a562ac6..65c1358881 100644 --- a/Civi/ActionSchedule/Mapping.php +++ b/Civi/ActionSchedule/Mapping.php @@ -52,7 +52,7 @@ class Mapping { * @var string * Ex: 'activity_type', 'civicrm_event', 'event_template'. */ - public $entity_value; + private $entity_value; /** * Level 1 filter -- The field label. @@ -67,7 +67,7 @@ class Mapping { * @var string * Ex: 'activity_status, 'civicrm_participant_status_type', 'auto_renew_options'. */ - public $entity_status; + private $entity_status; /** * Level 2 filter -- the field label. @@ -81,14 +81,14 @@ class Mapping { * @var string|NULL * Ex: 'event_start_date' */ - public $entity_date_start; + private $entity_date_start; /** * Date filter -- the field name. * @var string|NULL * Ex: 'event_end_date'. */ - public $entity_date_end; + private $entity_date_end; /** * Contact selector -- The field/relationship/option-group name. @@ -145,6 +145,23 @@ class Mapping { return $dateFieldLabels; } + public function getRecipientListing($recipientType) { + if (!$recipientType) { + return array(); + } + + $options = array(); + switch ($this->entity) { + case 'civicrm_participant': + $eventContacts = \CRM_Core_OptionGroup::values('event_contacts', FALSE, FALSE, FALSE, NULL, 'name', TRUE, FALSE, 'name'); + if (!empty($eventContacts[$recipientType]) && $eventContacts[$recipientType] == 'participant_role') { + $options = \CRM_Event_PseudoConstant::participantRole(); + } + break; + } + return $options; + } + /** * @param bool|NULL $noThanksJustKidding * This is ridiculous and should not exist. @@ -175,6 +192,23 @@ class Mapping { return $entityRecipientLabels; } + /** + * FIXME: Seems to duplicate getRecipientTypes? + * @return array|null + */ + public function getRecipientOptions() { + $recipientOptions = NULL; + if (!\CRM_Utils_System::isNull($this->entity_recipient)) { + if ($this->entity_recipient == 'event_contacts') { + $recipientOptions = \CRM_Core_OptionGroup::values($this->entity_recipient, FALSE, FALSE, FALSE, NULL, 'name', TRUE, FALSE, 'name'); + } + else { + $recipientOptions = \CRM_Core_OptionGroup::values($this->entity_recipient, FALSE, FALSE, FALSE, NULL, 'name'); + } + } + return $recipientOptions; + } + protected static function getValueLabelMap($name) { static $valueLabelMap = NULL; if ($valueLabelMap === NULL) { diff --git a/tests/phpunit/api/v3/ActionScheduleTest.php b/tests/phpunit/api/v3/ActionScheduleTest.php index 0c7da114d6..8bf0b0c6ae 100644 --- a/tests/phpunit/api/v3/ActionScheduleTest.php +++ b/tests/phpunit/api/v3/ActionScheduleTest.php @@ -54,9 +54,6 @@ class api_v3_ActionScheduleTest extends CiviUnitTestCase { $activityContacts = CRM_Core_OptionGroup::values('activity_contacts', FALSE, FALSE, FALSE, NULL, 'name'); $assigneeID = CRM_Utils_Array::key('Activity Assignees', $activityContacts); $scheduledStatus = CRM_Core_OptionGroup::getValue('activity_status', 'Scheduled', 'name'); - $mapping = CRM_Utils_Array::first(CRM_Core_BAO_ActionSchedule::getMappings(array( - 'entity_value' => 'activity_type', - ))); $activityTypeId = CRM_Core_OptionGroup::getValue('activity_type', "Meeting", 'name'); $title = "simpleActionSchedule" . substr(sha1(rand()), 0, 7); $params = array( @@ -68,7 +65,7 @@ class api_v3_ActionScheduleTest extends CiviUnitTestCase { 'is_active' => 1, 'record_activity' => 1, 'start_action_date' => 'activity_date_time', - 'mapping_id' => $mapping->id, + 'mapping_id' => CRM_Core_ActionScheduleTmp::ACTIVITY_MAPPING_ID, ); $actionSchedule = $this->callAPISuccess('action_schedule', 'create', $params); $this->assertTrue(is_numeric($actionSchedule['id'])); @@ -96,9 +93,6 @@ class api_v3_ActionScheduleTest extends CiviUnitTestCase { $activityContacts = CRM_Core_OptionGroup::values('activity_contacts', FALSE, FALSE, FALSE, NULL, 'name'); $assigneeID = CRM_Utils_Array::key('Activity Assignees', $activityContacts); $scheduledStatus = CRM_Core_OptionGroup::getValue('activity_status', 'Scheduled', 'name'); - $mapping = CRM_Utils_Array::first(CRM_Core_BAO_ActionSchedule::getMappings(array( - 'entity_value' => 'activity_type', - ))); $activityTypeId = CRM_Core_OptionGroup::getValue('activity_type', "Meeting", 'name'); $title = "simpleActionSchedule" . substr(sha1(rand()), 0, 7); $params = array( @@ -109,7 +103,7 @@ class api_v3_ActionScheduleTest extends CiviUnitTestCase { 'entity_status' => $scheduledStatus, 'is_active' => 1, 'record_activity' => 1, - 'mapping_id' => $mapping->id, + 'mapping_id' => CRM_Core_ActionScheduleTmp::ACTIVITY_MAPPING_ID, 'start_action_offset' => 3, 'start_action_unit' => 'day', 'start_action_condition' => 'before', -- 2.25.1