From 673c1c8124c6f7b6227c113e61d0ae13280e59fa Mon Sep 17 00:00:00 2001 From: Tim Otten Date: Tue, 4 Aug 2015 17:40:14 -0700 Subject: [PATCH] CRM-13244 - MappingInterface - Document/cleanup getRecipient* stuff --- CRM/Core/BAO/ActionSchedule.php | 17 +++++++++++++++-- CRM/Event/ActionMapping.php | 20 ++------------------ Civi/ActionSchedule/Mapping.php | 13 ++----------- Civi/ActionSchedule/MappingInterface.php | 19 +++++++++++++++++-- 4 files changed, 36 insertions(+), 33 deletions(-) diff --git a/CRM/Core/BAO/ActionSchedule.php b/CRM/Core/BAO/ActionSchedule.php index 736dc3bcc5..57edf36e30 100755 --- a/CRM/Core/BAO/ActionSchedule.php +++ b/CRM/Core/BAO/ActionSchedule.php @@ -99,7 +99,7 @@ class CRM_Core_BAO_ActionSchedule extends CRM_Core_DAO_ActionSchedule { if ($mapping->getId() == $id) { $dateFieldLabels = $mapping->getDateFields(); - $entityRecipientLabels = $mapping->getRecipientTypes(); + $entityRecipientLabels = $mapping->getRecipientTypes() + self::getAdditionalRecipients(); $entityRecipientNames = array_combine(array_keys($entityRecipientLabels), array_keys($entityRecipientLabels)); } @@ -143,7 +143,7 @@ class CRM_Core_BAO_ActionSchedule extends CRM_Core_DAO_ActionSchedule { foreach ($mappings as $mapping) { /** @var \Civi\ActionSchedule\Mapping $mapping */ $dateFieldLabels = $mapping->getDateFields(); - $entityRecipientLabels = $mapping->getRecipientTypes(!$isLimit); + $entityRecipientLabels = $mapping->getRecipientTypes(!$isLimit) + self::getAdditionalRecipients(); } return array( @@ -719,4 +719,17 @@ FROM civicrm_action_schedule cas return NULL; } + /** + * Get the list of generic recipient types supported by all entities/mappings. + * + * @return array + * array(mixed $value => string $label). + */ + protected static function getAdditionalRecipients() { + return array( + 'manual' => ts('Choose Recipient(s)'), + 'group' => ts('Select Group'), + ); + } + } diff --git a/CRM/Event/ActionMapping.php b/CRM/Event/ActionMapping.php index 75ba4661be..e04845709f 100644 --- a/CRM/Event/ActionMapping.php +++ b/CRM/Event/ActionMapping.php @@ -115,7 +115,7 @@ class CRM_Event_ActionMapping extends \Civi\ActionSchedule\Mapping { $query->join('r', 'INNER JOIN civicrm_event r ON e.event_id = r.id'); if ($schedule->recipient_listing && $schedule->limit_to) { - switch (\CRM_Utils_Array::value($schedule->recipient, $this->getRecipientOptions())) { + switch ($schedule->recipient) { case 'participant_role': $query->where("e.role_id IN (#recipList)") ->param('recipList', \CRM_Utils_Array::explodePadded($schedule->recipient_listing)); @@ -140,6 +140,7 @@ class CRM_Event_ActionMapping extends \Civi\ActionSchedule\Mapping { $query->where('r.is_template = 0'); // participant status criteria not to be implemented for additional recipients + // ... why not? if (!empty($selectedStatuses)) { switch ($phase) { case RecipientBuilder::PHASE_RELATION_FIRST: @@ -153,21 +154,4 @@ class CRM_Event_ActionMapping extends \Civi\ActionSchedule\Mapping { return $query; } - /** - * 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; - } - } diff --git a/Civi/ActionSchedule/Mapping.php b/Civi/ActionSchedule/Mapping.php index a22908de7a..ad1321528d 100644 --- a/Civi/ActionSchedule/Mapping.php +++ b/Civi/ActionSchedule/Mapping.php @@ -262,13 +262,8 @@ abstract class Mapping implements MappingInterface { } $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; + if ($this->entity === 'civicrm_participant' && $recipientType === 'participant_role') { + $options = \CRM_Event_PseudoConstant::participantRole(); } return $options; } @@ -298,10 +293,6 @@ abstract class Mapping implements MappingInterface { default: } - $entityRecipientLabels += array( - 'manual' => ts('Choose Recipient(s)'), - 'group' => ts('Select Group'), - ); return $entityRecipientLabels; } diff --git a/Civi/ActionSchedule/MappingInterface.php b/Civi/ActionSchedule/MappingInterface.php index b193e709d8..7abea4c582 100644 --- a/Civi/ActionSchedule/MappingInterface.php +++ b/Civi/ActionSchedule/MappingInterface.php @@ -90,8 +90,16 @@ interface MappingInterface { public function getDateFields(); /** - * FIXME: Unsure. Not sure how it differs from getRecipientTypes... but it does... + * Just return an empty array. * + * There's only one context where this returns actual data -- when using + * something like 'Limit To: Participant Role: Attendee or Speaker'. + * Unfortunately, that use-case has several other hacky, hard-coded bits + * which make it work. New entities can't take advantage of this because + * they don't have similar hacky bits. More generally, all the "Recipients"/ + * "Limit To"/"Also Include" stuff needs a rethink. + * + * @deprecated * @param string $recipientType * @return array * Array(mixed $name => string $label). @@ -100,7 +108,14 @@ interface MappingInterface { public function getRecipientListing($recipientType); /** - * FIXME: Unsure. Not sure how it differs from getRecipientListing... but it does... + * Just return an empty array. + * + * There are two contexts where this returns actual data -- when using + * Activities with the "Recipient" option, or whe using Events with the + * "Limit To:" option. However, the mechanisms around these do not + * work the same and rely on on hacky, hard-coded bits in the UI. + * More generally, all the "Recipients"/"Limit To"/"Also Include" stuff + * needs a rethink. * * @param bool|NULL $noThanksJustKidding * This is ridiculous and should not exist. -- 2.25.1