Problem/Motivation
When creating list fields it is possible to use functions to define the allowed values for the list. This can either be done using a fixed list configured while creating the field in the UI or by setting the property allowed_values_function in the field storage configuration.
The property not only allows setting a procedural function but allows developers to use callbacks defined in classes (see #2982949):
This allows using something like
<?php
/**
* Implements hook_entity_field_storage_info_alter().
*/
#[Hook('entity_field_storage_info_alter')]
public function entityFieldStorageInfoAlter(&$fields, EntityTypeInterface $entity_type): void {
if (($entity_type->id() === 'node') && isset($fields['field_test_alllowed_values'])) {
$fields['field_test_allowed_values']->setSetting('allowed_values_function', [MyOptions::class, 'allowedValues']);
}
}Unfortunately this results in an error when trying to edit the field in the UI:
TypeError: Drupal\Component\Utility\Html::escape(): Argument #1 ($text) must be of type string, array given, called in /var/www/html/docroot/core/lib/Drupal/Component/Render/FormattableMarkup.php on line 238 in Drupal\Component\Utility\Html::escape() (line 433 of core/lib/Drupal/Component/Utility/Html.php).
When printing information about the allowed_values_function in ListItemBase::storageSettingsForm(), it is always assumed that the value for “allowed_values_function” is a string. Since it is allowed to pass the callable as array this assumption is wrong.
Steps to reproduce
* add a field of type "List (text)" to a content type
* alter the field storage info of this field in a custom module and set the allowed_values_function as shown above
* open the field settings in the admin UI
Proposed resolution
* update ListItemBase::storageSettingsForm() to handle cases where allowed_values_function is an array
Issue fork drupal-3579349
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #2
stborchertComment #4
stborchertI've added a test that sets the allowed_values_function to
[OptionsAllowedValues::class, 'simpleValues']. Without the second commit (the one, that fixes the issue) the test fails as expected.Comment #5
idebr commentedWould this be a good case for the callable resolver? https://www.drupal.org/node/3368504
Comment #6
stborchert@idebr The callable resolver already works for allowed_values_function. The problem here is the output in the field UI where the text always expects a string.
Comment #7
smustgrave commentedLeft a comment on the MR thanks!
Comment #8
stborchertThat should work, too. I'll try and update the MR.
Comment #9
smustgrave commentedping me and I'll happily review quickly
Comment #10
stborchertHm, I have some troubles accessing the field config page in
OptionsWidgetsTest. InOptionsFieldUITestwe've got$this->adminPathbut this does not work here ...Question: how do I access the field configuration page (something like admin/structure/[...]/fields/[...]) in
OptionsWidgetsTestfor the entity_test type?Comment #11
stborchertFinally managed to find the correct route for the field config. Phew :)
Comment #12
smustgrave commentedI couldn't find a better test to expand so believe it's fine as it's own. Manually testing using the test module and appears to be working. Believe all feedback has been addressed
Comment #13
godotislateNeeds a rebase for merge conflict. Also, seems like there are a lot of unrelated changes and files in the MR diff. Maybe that will be resolved by the rebase.
Comment #14
smustgrave commentedYea seems like a bad rebase maybe? But 100% remember not seeing that when I looked after I was pinged in slack. Weird
Comment #17
stborchertSeems I managed to mess the commits with the last rebase :/
Opened a new MR and closed https://git.drupalcode.org/project/drupal/-/merge_requests/15098.
Comment #18
smustgrave commentedSuper close! Just some small comments on the test. Rest looks good!
Comment #19
stborchertHope I could address the last comments ... :)
Comment #20
smustgrave commentedMade 1 change to avoid using code blocks inline, almost positive eventually that will be a phpcs rule to prevent them.
Rest of the feedback appears to be addressed.
Comment #21
stborchertI checked earlier, and
/** */isn't allowed. But your version works too :)Comment #22
amateescu commentedPosted a few comments on the MR.
Comment #23
stborchertComment #24
godotislateThe config schema for the allowed_values_function in options.schema.yml is this:
We'll likely need to update that somehow to allow both strings and arrays. But going further, do we need to prevent Closures from being set as the allowed values function in PHP?
Comment #25
smustgrave commentedSeems like there was additional feedback to be looked at in the previous comment
Thanks