From b855a1c3087da615705a839b7f0369ef30195383 Mon Sep 17 00:00:00 2001 From: colemanw Date: Sat, 29 Jul 2023 21:07:43 -0400 Subject: [PATCH] ActionSchedule - Give every ActionMapping a name Going forward the name should always match the id in new implementations, but for legacy implementations the old numeric ids are supported. --- CRM/Activity/ActionMapping.php | 4 +++ CRM/Contact/ActionMapping.php | 4 +++ CRM/Contribute/ActionMapping/ByPage.php | 2 +- CRM/Contribute/ActionMapping/ByType.php | 2 +- CRM/Core/BAO/ActionSchedule.php | 25 +++++++++++++------ CRM/Event/ActionMapping/ByEvent.php | 4 +++ CRM/Event/ActionMapping/ByTemplate.php | 4 +++ CRM/Event/ActionMapping/ByType.php | 4 +++ CRM/Member/ActionMapping.php | 4 +++ Civi/ActionSchedule/MappingBase.php | 4 +++ Civi/ActionSchedule/MappingInterface.php | 11 +++++++- .../api/v4/Entity/ActionScheduleTest.php | 22 ++++++++-------- 12 files changed, 69 insertions(+), 21 deletions(-) diff --git a/CRM/Activity/ActionMapping.php b/CRM/Activity/ActionMapping.php index 9577578d63..1e5416bb5d 100644 --- a/CRM/Activity/ActionMapping.php +++ b/CRM/Activity/ActionMapping.php @@ -32,6 +32,10 @@ class CRM_Activity_ActionMapping extends \Civi\ActionSchedule\MappingBase { return self::ACTIVITY_MAPPING_ID; } + public function getName(): string { + return 'activity_type'; + } + public function getEntityName(): string { return 'Activity'; } diff --git a/CRM/Contact/ActionMapping.php b/CRM/Contact/ActionMapping.php index 281da12a2f..cbd92cb975 100644 --- a/CRM/Contact/ActionMapping.php +++ b/CRM/Contact/ActionMapping.php @@ -28,6 +28,10 @@ class CRM_Contact_ActionMapping extends \Civi\ActionSchedule\MappingBase { return self::CONTACT_MAPPING_ID; } + public function getName(): string { + return 'contact'; + } + public function getEntityName(): string { return 'Contact'; } diff --git a/CRM/Contribute/ActionMapping/ByPage.php b/CRM/Contribute/ActionMapping/ByPage.php index f332d6f1e9..11d11bde4b 100644 --- a/CRM/Contribute/ActionMapping/ByPage.php +++ b/CRM/Contribute/ActionMapping/ByPage.php @@ -23,7 +23,7 @@ class CRM_Contribute_ActionMapping_ByPage extends CRM_Contribute_ActionMapping { /** * @return string */ - public function getId() { + public function getName(): string { return 'contribpage'; } diff --git a/CRM/Contribute/ActionMapping/ByType.php b/CRM/Contribute/ActionMapping/ByType.php index a662ac2c59..42bf7e5c71 100644 --- a/CRM/Contribute/ActionMapping/ByType.php +++ b/CRM/Contribute/ActionMapping/ByType.php @@ -23,7 +23,7 @@ class CRM_Contribute_ActionMapping_ByType extends CRM_Contribute_ActionMapping { /** * @return string */ - public function getId() { + public function getName(): string { return 'contribtype'; } diff --git a/CRM/Core/BAO/ActionSchedule.php b/CRM/Core/BAO/ActionSchedule.php index 92a50e1626..4eeae87285 100644 --- a/CRM/Core/BAO/ActionSchedule.php +++ b/CRM/Core/BAO/ActionSchedule.php @@ -44,22 +44,31 @@ class CRM_Core_BAO_ActionSchedule extends CRM_Core_DAO_ActionSchedule implements } /** - * @param string|int $identifier - * Name of the mapping e.g. 'contribpage' or CRM_Contact_ActionMapping::CONTACT_MAPPING_ID + * @param string|int $mappingId + * Id of the mapping e.g. 'contribpage' or CRM_Contact_ActionMapping::CONTACT_MAPPING_ID * * @return \Civi\ActionSchedule\MappingInterface|NULL */ - public static function getMapping($identifier) { - return self::getMappings()[$identifier] ?? NULL; + public static function getMapping($mappingId) { + return self::getMappings()[$mappingId] ?? NULL; } /** * Provides the pseudoconstant list for `mapping_id` field. - * @return array + * @return array[] */ public static function getMappingOptions(): array { - $mappings = CRM_Utils_Array::collectMethod('getLabel', self::getMappings()); - natcasesort($mappings); + $mappings = []; + foreach (self::getMappings() as $mapping) { + $mappings[] = [ + 'id' => $mapping->getId(), + 'name' => $mapping->getName(), + 'label' => $mapping->getLabel(), + ]; + } + usort($mappings, function($m1, $m2) { + return strnatcasecmp($m1['label'], $m2['label']); + }); return $mappings; } @@ -203,7 +212,7 @@ class CRM_Core_BAO_ActionSchedule extends CRM_Core_DAO_ActionSchedule implements /** * Retrieve list of Scheduled Reminders. * - * @param \Civi\ActionSchedule\Mapping|null $filterMapping + * @param \Civi\ActionSchedule\MappingInterface|null $filterMapping * Filter by the schedule's mapping type. * @param int $filterValue * Filter by the schedule's entity_value. diff --git a/CRM/Event/ActionMapping/ByEvent.php b/CRM/Event/ActionMapping/ByEvent.php index ddb8107245..1143b23865 100644 --- a/CRM/Event/ActionMapping/ByEvent.php +++ b/CRM/Event/ActionMapping/ByEvent.php @@ -19,6 +19,10 @@ class CRM_Event_ActionMapping_ByEvent extends CRM_Event_ActionMapping { return self::EVENT_NAME_MAPPING_ID; } + public function getName(): string { + return 'event_id'; + } + public function getLabel(): string { return ts('Event Name'); } diff --git a/CRM/Event/ActionMapping/ByTemplate.php b/CRM/Event/ActionMapping/ByTemplate.php index 7ee4744d1c..229cbeeed6 100644 --- a/CRM/Event/ActionMapping/ByTemplate.php +++ b/CRM/Event/ActionMapping/ByTemplate.php @@ -19,6 +19,10 @@ class CRM_Event_ActionMapping_ByTemplate extends CRM_Event_ActionMapping { return self::EVENT_TPL_MAPPING_ID; } + public function getName(): string { + return 'event_template'; + } + public function getLabel(): string { return ts('Event Template'); } diff --git a/CRM/Event/ActionMapping/ByType.php b/CRM/Event/ActionMapping/ByType.php index e7a35070f3..78641ad8cf 100644 --- a/CRM/Event/ActionMapping/ByType.php +++ b/CRM/Event/ActionMapping/ByType.php @@ -19,6 +19,10 @@ class CRM_Event_ActionMapping_ByType extends CRM_Event_ActionMapping { return self::EVENT_TYPE_MAPPING_ID; } + public function getName(): string { + return 'event_type'; + } + public function getLabel(): string { return ts('Event Type'); } diff --git a/CRM/Member/ActionMapping.php b/CRM/Member/ActionMapping.php index e81824d490..99f2e7dfd7 100644 --- a/CRM/Member/ActionMapping.php +++ b/CRM/Member/ActionMapping.php @@ -27,6 +27,10 @@ class CRM_Member_ActionMapping extends \Civi\ActionSchedule\MappingBase { return self::MEMBERSHIP_TYPE_MAPPING_ID; } + public function getName(): string { + return 'membership_type'; + } + public function getEntityName(): string { return 'Membership'; } diff --git a/Civi/ActionSchedule/MappingBase.php b/Civi/ActionSchedule/MappingBase.php index 0c346178ef..4e7c7978da 100644 --- a/Civi/ActionSchedule/MappingBase.php +++ b/Civi/ActionSchedule/MappingBase.php @@ -22,6 +22,10 @@ use Civi\Core\Service\AutoSubscriber; */ abstract class MappingBase extends AutoSubscriber implements MappingInterface { + public function getId() { + return $this->getName(); + } + public static function getSubscribedEvents(): array { return [ 'civi.actionSchedule.getMappings' => 'onRegisterActionMappings', diff --git a/Civi/ActionSchedule/MappingInterface.php b/Civi/ActionSchedule/MappingInterface.php index f037fc4046..c2f0911ae2 100644 --- a/Civi/ActionSchedule/MappingInterface.php +++ b/Civi/ActionSchedule/MappingInterface.php @@ -20,11 +20,20 @@ interface MappingInterface { /** * Unique identifier of this mapping type. * - * Should return a "machine name" style string (older implementations return an int -- don't follow their example). + * Should return a "machine_name" style string (same output as `getName()`) + * Note: Some legacy implementations return an int. Don't follow those examples. * @return string|int */ public function getId(); + /** + * Unique name of this mapping type. + * + * Should return a "machine_name" style string (should be the same as `getId()`). + * @return string + */ + public function getName(): string; + /** * Name of the table belonging to the main entity e.g. `civicrm_activity` * @return string diff --git a/tests/phpunit/api/v4/Entity/ActionScheduleTest.php b/tests/phpunit/api/v4/Entity/ActionScheduleTest.php index 6d3e317b42..570f1f9302 100644 --- a/tests/phpunit/api/v4/Entity/ActionScheduleTest.php +++ b/tests/phpunit/api/v4/Entity/ActionScheduleTest.php @@ -24,22 +24,24 @@ class ActionScheduleTest extends Api4TestBase { public function testGetOptionsBasic() { $fields = ActionSchedule::getFields(FALSE) - ->setLoadOptions(TRUE) + ->setLoadOptions(['id', 'name', 'label']) ->execute() ->indexBy('name'); - $this->assertArrayHasKey('1', $fields['mapping_id']['options']); - $this->assertArrayHasKey('contribpage', $fields['mapping_id']['options']); + $this->assertContains(['id' => '1', 'name' => 'activity_type', 'label' => 'Activity'], $fields['mapping_id']['options']); + $this->assertContains(['id' => 'contribpage', 'name' => 'contribpage', 'label' => 'Contribution Page'], $fields['mapping_id']['options']); - $this->assertArrayHasKey('day', $fields['start_action_unit']['options']); - $this->assertArrayHasKey('week', $fields['repetition_frequency_unit']['options']); - $this->assertArrayHasKey('month', $fields['end_frequency_unit']['options']); + $this->assertContains(['id' => 'day', 'name' => 'day', 'label' => 'days'], $fields['start_action_unit']['options']); + $this->assertContains(['id' => 'week', 'name' => 'week', 'label' => 'weeks'], $fields['repetition_frequency_unit']['options']); + $this->assertContains(['id' => 'month', 'name' => 'month', 'label' => 'months'], $fields['end_frequency_unit']['options']); - $this->assertArrayHasKey('manual', $fields['recipient']['options']); - $this->assertArrayHasKey('group', $fields['recipient']['options']); + $this->assertEquals('manual', $fields['recipient']['options'][0]['name']); + $this->assertEquals('group', $fields['recipient']['options'][1]['name']); - $this->assertArrayHasKey('1', $fields['limit_to']['options']); - $this->assertArrayHasKey('2', $fields['limit_to']['options']); + $this->assertEquals('1', $fields['limit_to']['options'][0]['id']); + $this->assertEquals('limit', $fields['limit_to']['options'][0]['name']); + $this->assertEquals('2', $fields['limit_to']['options'][1]['id']); + $this->assertEquals('add', $fields['limit_to']['options'][1]['name']); } } -- 2.25.1