Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.Problem/Motivation
When utilizing an entity type with fields defined via FieldableEntityInterface::baseFieldDefinitions, the following error is displayed:
Recoverable fatal error: Argument 1 passed to Drupal\Core\Form\OptGroup::flattenOptions() must be of the type array, null given, called in core/modules/views/src/Plugin/views/filter/InOperator.php on line 330 and defined in Drupal\Core\Form\OptGroup::flattenOptions() (line 23 of core/lib/Drupal/Core/Form/OptGroup.php).
This occurs because in _search_api_views_get_allowed_values field configuration is loaded from FieldConfig::loadByName which only loads configuration entities for fields.
Proposed resolution
Utilize the entity_field.manager service, which properly analyzes fields defined in the entity class as well as fields provided via configuration.
Remaining tasks
- Decide if there is a more-graceful way of handling scenarios when the allowed values are not available. Possibly returning an empty array of options instead of
NULL. - Investigate if list fields with an "allowed values callback" work--indirectly related to this issue.
User interface changes
None.
API changes
None.
Data model changes
None.
| Comment | File | Size | Author |
|---|---|---|---|
| #3 | 2896312-3.patch | 1.8 KB | drunken monkey |
| |||
Comments
Comment #2
kerasai CreditAttribution: kerasai at Palantir.net commentedComment #3
drunken monkeyThanks a lot for reporting this issue, and providing a patch! (The fail is caused by #2896878: Change use of "cache.render" to "cache.page" in ViewsTest, so unrelated.)
You're right, your way of getting the field definition seems much cleaner. And due to caching, it might even be more performant. So yes, let's definitely go with that. (Don't actually know why that's a function, not just a public static method on
\Drupal\search_api\Plugin\views\filter\SearchApiOptions, but no point in changing that now, I'd say.)Just a bit of reformatting, see the attached revision.
Also, it's true, a
NULLreturn value is apparently problematic. The blame, as so often, lies with Views here: while\Drupal\views\Plugin\views\filter\InOperator::getValueOptions()clearly definesNULLas a possible return value, the rest of the code completely ignores that fact.Therefore, you're right, we should probably use an empty array instead. Instead of making the function less useful with that, though, I think it might be better to override the
getValueOptions()instead to add that logic – see attached patch. Due you agree with that, or what's your opinion.For the second "remaining task": why do you suppose this might not work? I'd say, as long as no-one complains, we don't need to check that.
Comment #4
kerasai CreditAttribution: kerasai at Palantir.net commentedOh, that stinks with the Views API not accepting the
NULL. It breaks the AJAX interaction in the UI when trying to add the filter, then the entire page to configure the View doesn't load. An empty list of options isn't the greatest, but seems better than completely broken.Regarding the "allowed values callback", it's a case of missing functionality. It probably doesn't need to be addressed as a part of this work but is worth noting. Someday someone will be looking for it, hopefully their favorite search engine will bring them here. :)
Comment #5
drunken monkeySo, does the patch in #3 work (and look good) for you?
If so, please set to RTBC and I'll commit it.
Thanks in any case for the feedback!
Comment #6
borisson_I haven't tested this, but it looks good.
Comment #8
drunken monkeyWould have been great to get definitive feedback, but I'm just gonna assume it works, since kerasai didn't complain in #5.
So: committed. Thanks again, everyone!