From 79e6ec68bf36402e59147e0a62618d79e45c48e0 Mon Sep 17 00:00:00 2001 From: Coleman Watts Date: Tue, 8 Feb 2022 11:55:32 -0500 Subject: [PATCH] Pseudoconstants - Use id instead of label for name Each item in a field option list contains keys like id, name, label, color, icon, description, abbr. But some option lists only consist of simple key/value pairs. To convert those simple associative arrays into a full option list, the name should be derived from the id, not the label, because machine names are expected to be stable, and labels can be translated. Before: `name` derived from `label` when converting simple associative to an option list After: `name` derived from `id`. --- CRM/Core/PseudoConstant.php | 12 ++++++++---- Civi/Api4/Generic/BasicGetFieldsAction.php | 2 +- .../phpunit/api/v4/SearchDisplay/SearchRunTest.php | 4 ++-- tests/phpunit/api/v4/Action/BasicActionsTest.php | 7 ++++--- tests/phpunit/api/v4/Entity/MembershipTest.php | 12 ++++++++++++ 5 files changed, 27 insertions(+), 10 deletions(-) diff --git a/CRM/Core/PseudoConstant.php b/CRM/Core/PseudoConstant.php index 494201eaa6..17d614da87 100644 --- a/CRM/Core/PseudoConstant.php +++ b/CRM/Core/PseudoConstant.php @@ -162,8 +162,7 @@ class CRM_Core_PseudoConstant { * @param string $fieldName * @param array $params * - name string name of the option group - * - flip boolean results are return in id => label format if false - * if true, the results are reversed + * - flip DEPRECATED * - grouping boolean if true, return the value in 'grouping' column (currently unsupported for tables other than option_value) * - localize boolean if true, localize the results before returning * - condition string|array add condition(s) to the sql query - will be concatenated using 'AND' @@ -228,6 +227,11 @@ class CRM_Core_PseudoConstant { $fieldOptions = call_user_func(Civi\Core\Resolver::singleton()->get($pseudoconstant['callback']), $context, $params); //CRM-18223: Allow additions to field options via hook. CRM_Utils_Hook::fieldOptions($entity, $fieldName, $fieldOptions, $params); + if ($context === 'validate') { + // This mode requires machine names and key/value pairs don't have a name, so + // use key for name. (labels are translatable so don't make suitable machine names) + return array_combine(array_keys($fieldOptions), array_keys($fieldOptions)); + } return $fieldOptions; } @@ -255,10 +259,10 @@ class CRM_Core_PseudoConstant { $params['grouping'], $params['localize'], $params['condition'] ? ' AND ' . implode(' AND ', (array) $params['condition']) : NULL, - $params['labelColumn'] ? $params['labelColumn'] : 'label', + $params['labelColumn'] ?: 'label', $params['onlyActive'], $params['fresh'], - $params['keyColumn'] ? $params['keyColumn'] : 'value', + $params['keyColumn'] ?: 'value', !empty($params['orderColumn']) ? $params['orderColumn'] : 'weight' ); CRM_Utils_Hook::fieldOptions($entity, $fieldName, $options, $params); diff --git a/Civi/Api4/Generic/BasicGetFieldsAction.php b/Civi/Api4/Generic/BasicGetFieldsAction.php index 483cf7353e..afaee7fa07 100644 --- a/Civi/Api4/Generic/BasicGetFieldsAction.php +++ b/Civi/Api4/Generic/BasicGetFieldsAction.php @@ -177,7 +177,7 @@ class BasicGetFieldsAction extends BasicGetAction { if (!is_array($option)) { $option = [ 'id' => $id, - 'name' => $option, + 'name' => $id, 'label' => $option, ]; } diff --git a/ext/search_kit/tests/phpunit/api/v4/SearchDisplay/SearchRunTest.php b/ext/search_kit/tests/phpunit/api/v4/SearchDisplay/SearchRunTest.php index 2712da75be..574fe7049f 100644 --- a/ext/search_kit/tests/phpunit/api/v4/SearchDisplay/SearchRunTest.php +++ b/ext/search_kit/tests/phpunit/api/v4/SearchDisplay/SearchRunTest.php @@ -753,13 +753,13 @@ class SearchRunTest extends \PHPUnit\Framework\TestCase implements HeadlessInter 'cssRules' => [ [ 'bg-danger', - 'Contact_Email_contact_id_01.on_hold:name', + 'Contact_Email_contact_id_01.on_hold:label', '=', 'On Hold Bounce', ], [ 'bg-warning', - 'Contact_Email_contact_id_01.on_hold:name', + 'Contact_Email_contact_id_01.on_hold:label', '=', 'On Hold Opt Out', ], diff --git a/tests/phpunit/api/v4/Action/BasicActionsTest.php b/tests/phpunit/api/v4/Action/BasicActionsTest.php index d67a61598c..13390bee45 100644 --- a/tests/phpunit/api/v4/Action/BasicActionsTest.php +++ b/tests/phpunit/api/v4/Action/BasicActionsTest.php @@ -212,7 +212,8 @@ class BasicActionsTest extends UnitTestCase implements HookInterface { // Simple options should be expanded to non-assoc array $this->assertCount(2, $getFields); $this->assertEquals('one', $getFields['group']['options'][0]['id']); - $this->assertEquals('First', $getFields['group']['options'][0]['name']); + // Name is interchangeable with id + $this->assertEquals('one', $getFields['group']['options'][0]['name']); $this->assertEquals('First', $getFields['group']['options'][0]['label']); $this->assertFalse(isset($getFields['group']['options'][0]['color'])); // Complex options should give all requested properties @@ -400,7 +401,7 @@ class BasicActionsTest extends UnitTestCase implements HookInterface { public function testPseudoconstantMatch() { $records = [ ['group:label' => 'First', 'shape' => 'round', 'fruit:name' => 'banana'], - ['group:name' => 'Second', 'shape' => 'square', 'fruit:label' => 'Pear'], + ['group:name' => 'two', 'shape' => 'square', 'fruit:label' => 'Pear'], ]; $this->replaceRecords($records); @@ -412,7 +413,7 @@ class BasicActionsTest extends UnitTestCase implements HookInterface { $this->assertEquals('round', $results[0]['shape']); $this->assertEquals('one', $results[0]['group']); $this->assertEquals('First', $results[0]['group:label']); - $this->assertEquals('First', $results[0]['group:name']); + $this->assertEquals('one', $results[0]['group:name']); $this->assertEquals(3, $results[0]['fruit']); $this->assertEquals('Banana', $results[0]['fruit:label']); $this->assertEquals('banana', $results[0]['fruit:name']); diff --git a/tests/phpunit/api/v4/Entity/MembershipTest.php b/tests/phpunit/api/v4/Entity/MembershipTest.php index 5f553e3904..5355d55ad5 100644 --- a/tests/phpunit/api/v4/Entity/MembershipTest.php +++ b/tests/phpunit/api/v4/Entity/MembershipTest.php @@ -73,4 +73,16 @@ class MembershipTest extends UnitTestCase implements TransactionalInterface { } + /** + * Test getting options + */ + public function testGetOptions(): void { + $fields = MembershipType::getFields(FALSE) + ->setLoadOptions(['name', 'id', 'label']) + ->execute()->indexBy('name'); + $this->assertEquals('rolling', $fields['period_type']['options'][0]['name']); + $this->assertEquals('rolling', $fields['period_type']['options'][0]['id']); + $this->assertEquals('Rolling', $fields['period_type']['options'][0]['label']); + } + } -- 2.25.1