Problem/Motivation

Drupal core ships with several list field storage, for example 'List (text)', 'List (integer)'. These fields allow the site administrator to enter allowed values from the UI. These fields also support an 'allowed_values_function' callback as an alternative to hardcoding these values.

When using an allowed_values_function, the Views filter differs from the allowed_values version as well as the Drupal core views filtering.

Allowed values:

Allowed values callback:

Proposed resolution

Use the Options views filter for fields with an allowed_values_function, so a filter can have multiple values.

Remaining tasks

  1. Write a patch
  2. Review
  3. Commit

User interface changes

The Views UI shows the Options filter for fields with an allowed_values_function.

API changes

None.

Data model changes

None.

CommentFileSizeAuthor
#21 search_api-views_filter_options_callback-2949022-21.patch953 bytesgueguerreiro
#19 views_filter_options_callback--2949022-18.patch1.09 KBpanda2ekino
#17 views_filter_options_callback--2949022-17.patch1.08 KBlinhnm
#13 2949022-13--views_filter_options_callback.patch1.07 KBsboden
#12 2949022-12--views_filter_options_callback.patch1.07 KBronaldtebrake
#7 2949022-7--views_filter_options_callback.patch1.07 KBallie micka
#3 2949022-3--views_filter_options_callback.patch947 bytesdrunken monkey
#3 2949022-3--views_filter_options_callback--interdiff.txt1.02 KBdrunken monkey
#2 search_api-views_options_filter_allowed_values_function-2949022-2.patch1.41 KBidebr
views-filter-allowed_values_callback.png37.91 KBidebr
views-filter-allowed_values.png48.64 KBidebr

Comments

idebr created an issue. See original summary.

idebr’s picture

idebr commented
Status: Active » Needs review
StatusFileSize
new search_api-views_options_filter_allowed_values_function-2949022-2.patch1.41 KB

Attached patch changes the views filter from Numeric to Options for option fields with an allowed_values_function.

Field storage configuration for testing purposes:
node.field_program_type_with_callback

langcode: en
status: true
dependencies:
  module:
    - node
    - options
id: node.field_program_type_with_callback
field_name: field_program_type_with_callback
entity_type: node
type: list_string
settings:
  allowed_values: {  }
  allowed_values_function: d8custom_allowed_values
module: options
locked: false
cardinality: 1
translatable: true
indexes: {  }
persist_with_no_fields: false
custom_storage: false

/**
 * Allowed values function for field_program_type_with_callback.
 */
function d8custom_allowed_values(FieldDefinitionInterface $field_definition) {
  return [
    'bachelor' => t('Bachelor'),
    'master' => t('Master'),
  ];
}
drunken monkey’s picture

drunken monkey
he/him
Primary language German
Location Vienna, Austria
commented
Issue tags: +Needs tests
StatusFileSize
new 2949022-3--views_filter_options_callback--interdiff.txt1.02 KB
new 2949022-3--views_filter_options_callback.patch947 bytes

Thanks a lot for reporting this! I wasn't aware of this method of specifying the available options, but of course we should support this, too. Your code also looks pretty good already, just made a few small tweaks – please see/test/review the attached patch.

We should also add test coverage for this, to make sure it keeps working. Would be great if you could add that, too!

idebr’s picture

idebr commented

Re #3

I would be happy to supply a test! It appears that currently there are not a whole lot of tests available for the Field/Views UI. Could you provide some pointers to what type of tests you would like to have?

drunken monkey’s picture

drunken monkey
he/him
Primary language German
Location Vienna, Austria
commented

I would be happy to supply a test! It appears that currently there are not a whole lot of tests available for the Field/Views UI. Could you provide some pointers to what type of tests you would like to have?

Great, thanks a lot!
I'd suggest a test like the following:

  1. Add a \Drupal\Tests\search_api\Functional\ViewsTest::regressionTest2949022() method.
  2. In there, change the field config for one of the indexed entities' fields (probably keywords would be easiest, since that's already added as a Views filter as well) to have such an options callback.
  3. The callback can live, e.g., in search_api_test_views.module.
  4. Clear the (Views) caches.
  5. Verify that the Views page now has an options filter for the field, with the correct options.

Please ask if you need any further pointers.

drunken monkey’s picture

drunken monkey
he/him
Primary language German
Location Vienna, Austria
commented
Status: Needs review » Needs work

Setting to NR for the tests.

allie micka’s picture

allie micka
she/her
commented
StatusFileSize
new 2949022-7--views_filter_options_callback.patch1.07 KB

The options_allowed_values might be a better fit for this, as it implements caching and whatever other magic might be useful to the output. Patch updated accordingly.

I'm not sure of how to handle an upgrade path for this. When I changed this code in my own environment, I wound up with a broken handler and I had to remove and re-add the affected field. Surely this has happened before?

drunken monkey’s picture

drunken monkey
he/him
Primary language German
Location Vienna, Austria
commented

Hm, not sure why the handler broke. Probably it didn't have the settings it expected to get? Because I don't think Views actually takes the plugin_id config into account at all (not sure why it's there), so I don't think it will care if that changes. Or does it?
In any case, since it's not a trivial change, I don't think we can provide an update path for this, since it wouldn't be possible for us to determine which filters need to be updated. (I don't think accessing the Field API is “allowed” in update hooks.)

Which does raise the question what to do about this, if this really does break handlers (and, thus, views). We definitely want to avoid that when someone just updates the module, so maybe we'll need to have a config setting for whether to use this new code? Pretty ugly and making things unnecessarily complicated, but might still be the best solution overall.

Any other opinions/input on this?

idebr’s picture

idebr commented
Issue tags: +Needs upgrade path

Locally I used the same method as #7. However, this can be updated programmatically using hook_post_update_NAME :

These updates are executed after all hook_update_N() implementations. At this stage Drupal is already fully repaired so you can use any API as you wish.

allie micka’s picture

allie micka
she/her
commented

Bummer that you can't go out and find where field handlers have been used during an update hook.

My recollection is the handler was indeed looking for settings that weren't there. Do you think there might be some way to keep the original handler if it was already established during _search_api_views_get_handlers()? Is that information available at that point? If so, nothing would change unless/until you removed and re-added the field and it wouldn't break in the interim.

drunken monkey’s picture

drunken monkey
he/him
Primary language German
Location Vienna, Austria
commented

My recollection is the handler was indeed looking for settings that weren't there. Do you think there might be some way to keep the original handler if it was already established during _search_api_views_get_handlers()?

I'm not sure I understand you correctly, but if you do, then no, that's not possible. Views uses a single set of "Views data" for the whole site to determine which tables and fields are available and which handlers are available for each of them. There is no way to have a view keep using an old plugin while updating the Views data to use a new plugin for the same field. We'd have to define a second, "fallback" version of the field with the old handler, and switch all existing definitions to that version. But that version would then also appear in the selection when adding a new filter.

ronaldtebrake’s picture

ronaldtebrake commented
StatusFileSize
new 2949022-12--views_filter_options_callback.patch1.07 KB

Not sure if I can fully weigh in on the upgrade path issue here, since I'm working on a Distro and had to use some ConfigOverrides that add filters instead of update / change existing. So the fact that I'm not seeing a broken handler might not be very useful information.

However, the issue itself is indeed solved for my by #7.
I have rerolled this for usage on 1.15 so I can add this patch to our composer.json and make sure the allowed values callback is taken in to account correctly. Would be happy to share any helpful information.

And thanks for all your input!

sboden’s picture

sboden commented
StatusFileSize
new 2949022-13--views_filter_options_callback.patch1.07 KB

Wrote/rerolled the patch for search_api 1.29.0

I wrote the patch before finding this issue, in hindsight in exactly the same way as the already existing patches. Could this patch be merged, so we don't have to keep rerolling it: the patch is a clean solution. I'm using it in production on a Drupal 9.5.7.

drunken monkey’s picture

drunken monkey
he/him
Primary language German
Location Vienna, Austria
commented

@ sboden: Same as for the old patch, this is missing tests and an upgrade path.

sboden’s picture

sboden commented

Tests are missing, ok.

For the upgrade path, that might be tricky. But, when you define a field with allowed_values_function, you can't use allowed_values in the Drupal GUI anymore - the options are mutually exclusive. So the patch is actually "new" functionality: without the patch search_api would only take allowed_values into account, after the patch allowed_values_function is also taken into account which people who defined a field as such would want to use. When allowed_values_function is filled in, allowed_values is empty anyway.

Are there people who defined a filter on a field using allowed_values_function but assume users will filter using its key as search criteria (without dropdown), I doubt it. That's e.g. why I ended up on this issue.

drunken monkey’s picture

drunken monkey
he/him
Primary language German
Location Vienna, Austria
commented

As described in #7, even if this is likely the expected behavior in general, specifically at an update this might break people’s views, which should really be avoided, if possible. Especially for a new feature, not a bug fix.

linhnm’s picture

linhnm commented
StatusFileSize
new views_filter_options_callback--2949022-17.patch1.08 KB

Re-rolled for 1.31

socialnicheguru’s picture

socialnicheguru commented

Does not work on the latest dev#34ccd9343

panda2ekino’s picture

panda2ekino commented
StatusFileSize
new views_filter_options_callback--2949022-18.patch1.09 KB

Re-rolled for 1.38

i just move file previous patch code used in search_api.views.inc to search_api.views_hooks

socialnicheguru’s picture

socialnicheguru commented

no longer works with dev version of module

gueguerreiro’s picture

gueguerreiro
Primary language Portuguese, Portugal
Location Lisboa
commented
StatusFileSize
new search_api-views_filter_options_callback-2949022-21.patch953 bytes

Re-rolled patch for 1.40.

The code now uses the new [#Hook] method classes, so I placed the code included in the patches there instead (SearchApiViewsHooks.php).