Skip to content

Commit

Permalink
Pseudoconstants - Use id instead of label for name
Browse files Browse the repository at this point in the history
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`.
  • Loading branch information
colemanw committed Feb 8, 2022
1 parent 7b61c1d commit 7a57fa1
Show file tree
Hide file tree
Showing 4 changed files with 23 additions and 6 deletions.
12 changes: 8 additions & 4 deletions CRM/Core/PseudoConstant.php
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down Expand Up @@ -228,6 +227,11 @@ public static function get($daoName, $fieldName, $params = [], $context = NULL)
$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;
}

Expand Down Expand Up @@ -255,10 +259,10 @@ public static function get($daoName, $fieldName, $params = [], $context = NULL)
$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);
Expand Down
2 changes: 1 addition & 1 deletion Civi/Api4/Generic/BasicGetFieldsAction.php
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,7 @@ private function formatOptionList($options) {
if (!is_array($option)) {
$option = [
'id' => $id,
'name' => $option,
'name' => $id,
'label' => $option,
];
}
Expand Down
3 changes: 2 additions & 1 deletion tests/phpunit/api/v4/Action/BasicActionsTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,8 @@ public function testGetFields() {
// 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
Expand Down
12 changes: 12 additions & 0 deletions tests/phpunit/api/v4/Entity/MembershipTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -73,4 +73,16 @@ public function testUpdateWeights() {

}

/**
* 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']);
}

}

0 comments on commit 7a57fa1

Please sign in to comment.