From 9d97a64834a5ca9f0de3f3f0a1154fc1bb095f47 Mon Sep 17 00:00:00 2001 From: Tim Otten Date: Sat, 29 Aug 2015 15:24:19 -0700 Subject: [PATCH] CRM-13422 - MappingInterface::getRecipientTypes - Remove $noThanksJustKidding This is a weird option that owes to a quirk in the UX -- where activity roles are presented differently than other roles. That UX quirk probably shouldn't exist, but given that it does... we'll push the quirk up into the form layer (rather than in our API). --- CRM/Admin/Page/AJAX.php | 12 +++++++++++- CRM/Contribute/ActionMapping/ByPage.php | 5 +---- CRM/Contribute/ActionMapping/ByType.php | 5 +---- Civi/ActionSchedule/Mapping.php | 19 ++++++++++--------- Civi/ActionSchedule/MappingInterface.php | 5 +---- 5 files changed, 24 insertions(+), 22 deletions(-) diff --git a/CRM/Admin/Page/AJAX.php b/CRM/Admin/Page/AJAX.php index 2fe81d04e8..2b42cc238b 100644 --- a/CRM/Admin/Page/AJAX.php +++ b/CRM/Admin/Page/AJAX.php @@ -305,7 +305,17 @@ LIMIT $limit"; $mapping = CRM_Core_BAO_ActionSchedule::getMapping($_GET['mappingID']); $dateFieldLabels = $mapping ? $mapping->getDateFields() : array(); - $entityRecipientLabels = $mapping ? ($mapping->getRecipientTypes(!$_GET['isLimit']) + CRM_Core_BAO_ActionSchedule::getAdditionalRecipients()) : array(); + + // The UX here is quirky -- for "Activity" types, there's a simple drop "Recipients" + // dropdown which is always displayed. For other types, the "Recipients" drop down is + // conditional upon the weird isLimit ('Limit To / Also Include / Neither') dropdown. + $noThanksJustKidding = !$_GET['isLimit']; + if ($mapping instanceof CRM_Activity_ActionMapping || !$noThanksJustKidding) { + $entityRecipientLabels = $mapping ? ($mapping->getRecipientTypes() + CRM_Core_BAO_ActionSchedule::getAdditionalRecipients()) : array(); + } + else { + $entityRecipientLabels = CRM_Core_BAO_ActionSchedule::getAdditionalRecipients(); + } $recipientMapping = array_combine(array_keys($entityRecipientLabels), array_keys($entityRecipientLabels)); $output = array( diff --git a/CRM/Contribute/ActionMapping/ByPage.php b/CRM/Contribute/ActionMapping/ByPage.php index aa10c4ed06..22080b2f0b 100644 --- a/CRM/Contribute/ActionMapping/ByPage.php +++ b/CRM/Contribute/ActionMapping/ByPage.php @@ -150,14 +150,11 @@ class CRM_Contribute_ActionMapping_ByPage implements \Civi\ActionSchedule\Mappin /** * FIXME: Unsure. Not sure how it differs from getRecipientListing... but it does... * - * @param bool|NULL $noThanksJustKidding - * This is ridiculous and should not exist. - * If true, don't do our main job. * @return array * array(mixed $value => string $label). * Ex: array('assignee' => 'Activity Assignee'). */ - public function getRecipientTypes($noThanksJustKidding = FALSE) { + public function getRecipientTypes() { return array(); } diff --git a/CRM/Contribute/ActionMapping/ByType.php b/CRM/Contribute/ActionMapping/ByType.php index 1cd68ef0c3..23e694003e 100644 --- a/CRM/Contribute/ActionMapping/ByType.php +++ b/CRM/Contribute/ActionMapping/ByType.php @@ -150,14 +150,11 @@ class CRM_Contribute_ActionMapping_ByType implements \Civi\ActionSchedule\Mappin /** * FIXME: Unsure. Not sure how it differs from getRecipientListing... but it does... * - * @param bool|NULL $noThanksJustKidding - * This is ridiculous and should not exist. - * If true, don't do our main job. * @return array * array(mixed $value => string $label). * Ex: array('assignee' => 'Activity Assignee'). */ - public function getRecipientTypes($noThanksJustKidding = FALSE) { + public function getRecipientTypes() { return array(); } diff --git a/Civi/ActionSchedule/Mapping.php b/Civi/ActionSchedule/Mapping.php index 13f4888e67..71715a6955 100644 --- a/Civi/ActionSchedule/Mapping.php +++ b/Civi/ActionSchedule/Mapping.php @@ -241,12 +241,16 @@ abstract class Mapping implements MappingInterface { } /** - * Unsure. Not sure how it differs from getRecipientTypes... but it does... + * Get a list of recipients which match the given type. + * + * Note: A single schedule may target *multiple* recipients. * * @param string $recipientType + * Ex: 'participant_role'. * @return array * Array(mixed $name => string $label). * Ex: array(1 => 'Attendee', 2 => 'Volunteer'). + * @see getRecipientTypes */ public function getRecipientListing($recipientType) { if (!$recipientType) { @@ -261,16 +265,15 @@ abstract class Mapping implements MappingInterface { } /** - * Unsure. Not sure how it differs from getRecipientListing... but it does... + * Get a list of recipient types. + * + * Note: A single schedule may target at most *one* recipient type. * - * @param bool|NULL $noThanksJustKidding - * This is ridiculous and should not exist. - * If true, don't do our main job. * @return array * array(string $value => string $label). * Ex: array('assignee' => 'Activity Assignee'). */ - public function getRecipientTypes($noThanksJustKidding = FALSE) { + public function getRecipientTypes() { $entityRecipientLabels = array(); switch ($this->entity) { case 'civicrm_activity': @@ -278,9 +281,7 @@ abstract class Mapping implements MappingInterface { break; case 'civicrm_participant': - if (!$noThanksJustKidding) { - $entityRecipientLabels = \CRM_Core_OptionGroup::values('event_contacts', FALSE, FALSE, FALSE, NULL, 'label', TRUE, FALSE, 'name'); - } + $entityRecipientLabels = \CRM_Core_OptionGroup::values('event_contacts', FALSE, FALSE, FALSE, NULL, 'label', TRUE, FALSE, 'name'); break; default: diff --git a/Civi/ActionSchedule/MappingInterface.php b/Civi/ActionSchedule/MappingInterface.php index b3a1703066..68f350ea42 100644 --- a/Civi/ActionSchedule/MappingInterface.php +++ b/Civi/ActionSchedule/MappingInterface.php @@ -117,14 +117,11 @@ interface MappingInterface { * More generally, all the "Recipients"/"Limit To"/"Also Include" stuff * needs a rethink. * - * @param bool|NULL $noThanksJustKidding - * This is ridiculous and should not exist. - * If true, don't do our main job. * @return array * array(mixed $value => string $label). * Ex: array('assignee' => 'Activity Assignee'). */ - public function getRecipientTypes($noThanksJustKidding = FALSE); + public function getRecipientTypes(); /** * Determine whether a schedule based on this mapping is sufficiently -- 2.25.1