"Limit list to selected items" on exposed filters does not filter

Hi @devad,

As the statement "Limit list to selected items" states that it should limit the items shown in the dropdown. It seems to be working fine as its properly filtering the dropdown. This checkbox "Limit list to selected items" only meant to limit the dropdown list not the result set as its an exposed filter and will act only when you select something. Hope this makes sense!

Thanks
Ashutosh

More accurate category and status.

Thanks,
-Derek

p.s. If you don't want the '- Any -' choice (which is what's giving you grief), you have to set that exposed filter to be 'Required'. That's really the problem you're hitting...

Understanding that this is working as intended but the InOperator filter contains code that should limit the "all" filter to the selected items:

if (empty($this->options['expose']['multiple']) && empty($this->options['expose']['required']) && !empty($this->options['expose']['limit'])) {
  $identifier = $this->options['expose']['identifier'];
  if ($input[$identifier] == 'All') {
    return TRUE;
  }
}

The related exposed filter form is assigning that value to reduce instead of limit:

$form['expose']['reduce'] = [
  '#type' => 'checkbox',
  '#title' => $this->t('Limit list to selected items'),
  '#description' => $this->t('If checked, the only items presented to the user will be the ones selected here.'),
  // Safety.
  '#default_value' => !empty($this->options['expose']['reduce']),
];

So, one possible work around would be to alter the value in a pre build hook:

/**
 * Implements hook_views_pre_build().
 */
function my_module_views_pre_build(ViewExecutable $view) {
  if ($view->id() === 'my_view_id') {
    $my_filter_id = 'my_filter_id';
    if (isset($view->filter[$my_filter_id]->options['expose']['reduce'])) {
      $view->filter[$my_filter_id]->options['expose']['limit'] = $view->filter[$my_filter_id]->options['expose']['reduce'];
    }
  }
}

Occasionally, I check the Drupal 8 issue queue and today was one of those days. Since Drupal 8 is no longer supported I am moving this to 11.x where it will be seen by the community. I am also removing the bug smash tag because that is added after the Bug Smash Initiative has triaged an issue.

Good morning everyone.

Comment #6 is right, I have tried what he says and he is right, thank you very much steyep.

I have created a fork branch and done mr to 10.2.x and 11.x.

Greetings to all.

Thanks for working on this old one.

Can the issue summary be updated using the standard issue template please

Changed to a bug as support requests typically don't have code fixes. But will need a test case showing the issue.

Hello, I have updated the summary as you suggested, I hope you like it, greetings.

Updated the issue summary to standard format, and added a better test script.

Not sure if you forgot to push but don’t see any test added to the MR

Wanted to comment that I got hit by this bug today and the MR does appear to work.

I had to rewrite the issue summary. The instructions were so vague it took 20-30 minutes of reading and testing to figure out what the problem is.

I left two comments on the MR. The change to the plugin is simple enough. The test looks good. But the documentation needs work.

My feedback was addressed. Looks good to me.

I tested this on main. The standard install profile no longer provides Article and Page, so I created three content types a, b, and c. Then I made a node of each type. I then followed the steps to reproduce from #3 onward. AT step 7, "Check the "Limit list to selected items" checkbox." there is no such checkbox. I continued, and when on the content page, the results showed all 3 nodes. I applied the diff, cleared cache, refreshed the view and only content of type A was displayed. And when two content types are selected to be exposed, then only nodes of that type were displayed.

It would be better to properly fix this and test for TRUE. But that could be done in a followup where the 'weird implementations' mentioned in this comment are considered.

I updated credit and didn't find any unanswered questions.

I also recently got nailed by this on a project. I tried the MR diff. Unfortunately, it had no effect in my case, since the filter that's giving me the grief allows multiple inputs. For reasons I don't fully understand, the first clause in the impacted if is this: empty($this->options['expose']['multiple']). So that specifically prevents this from working as expected, even with the patch applied. I tried removing that check, but it still doesn't work. 😅 So something is more fishy here. Seems like it should be possible for this to work as described, even with multiple inputs allowed. But that's not yet the case. So, another follow-up would be to either:

- Get this working, even for 'allow multiple' exposed filters.
OR
- Use #states to hide the Limit list to selected items checkbox if the Allow multiple selections box is checked.

All that said, once I re-configured the exposed filter to not accept multiple values, the 'Limit list...' checkbox is now working as expected, and rows that do not match the available choices in the filter are excluded.

So +1 to RTBC and committing this as-is, but yeah, definitely want to revisit this in a few follow-ups...

The failure in the last pipeline is due to #3588024: PHP 8.6 - Functions mb_regex_encoding() and mb_ereg() are deprecated. It would be all green if we rebase against latest main. Not sure that's needed, but it's an option...

Meanwhile, opened an MR thread. I'm glad to see any test coverage at all here, but the Unit test is so specific, it doesn't give me tons of confidence. The comment for the test is misleading. Either we should fix that comment, or make a real test that confirms what the comment says we're testing...

If I get time this week I’ll try and convert to another test. I didn’t see any existing view that had this setting and not sure it was overkill to add a new one

core/modules/views/tests/src/Kernel/Handler/FilterInOperatorTest.php is close. We don't necessarily need a whole new test view. The test can alter the configuration of the test view to enable this feature and see how that changes the behavior/output.

p.s. I don't mind leaving the Unit test as-is (except for fixing the comment). It's okay to have Unit tests. 😅 We just shouldn't put too much faith in them on their own.

Opened #3592464: Either make 'Limit list to selected items' work with multiple inputs, or use #states to hide the option for my concerns at #36...
And #3592465: [PP-1] Change clause in InOperator::acceptExposedInput() for 'reduce' to test for TRUE instead of !empty() for https://git.drupalcode.org/project/drupal/-/merge_requests/7121#note_816736

Converted to a kernel test! Thanks for the pointers @dww

Cool, that's much better. Due to #3576458: [regression] Subsystem and Topics maintainers require access to re-run, trigger, or view tests I can't trigger the test-only job. But running locally, if I revert the fix, I get this:

There was 1 failure:

1) Drupal\Tests\views\Kernel\Handler\FilterInOperatorTest::testFilterExposedReduce
Failed asserting that actual size 5 matches expected size 2.

/.../core/modules/views/tests/src/Kernel/Handler/FilterInOperatorTest.php:293

FAILURES!
Tests: 6, Assertions: 15, Failures: 1.

With the fix, it passes locally (just like the bot).

I wondered for a second if we should also assert that the options in the exposed filter match the configuration. But that would require either using the new drupalGet() in Kernel stuff, or converting this to Functional. No thanks. I think this test is enough to handle the actual bug here, and additional test coverage could happen later.

I don't see anything else to complain about. Back to RTBC.

Thanks!
-Derek

Committed and pushed c6be972 to main, 4f29e29 to 11.x, and f7c6fbe to 11.4.x. Thanks!

Wooo!!

I just opened a bug report for this exact issue #3608886: Limit list to selected items option applies exposed filter by default I think this behaviour is only going to be applicable for some sites, this has broken some of our views.

For example, we have a search_api view that shows multiple entity types. We have a block type filter and that is limited to just several block types that we are indexing.

Now with this change, every other entity type is filtered out by default and there is no way to get around that because those limited options are being applied automatically.

This feature should probably be put behind another config option?

I think there's a fairly good argument to revert this, nothing in the configuration screen mentions that using the "Limit list to selected items" option would automatically apply the filters. There's plenty of scenarios where you would want to limit the list but still allow the filter to be optional.

Re-opening, a revert sounds reasonable to me and probably adding back with an additional checkbox. Also I think I have the same understanding of this feature as @acbramley - e.g. it should limit what can be selected, not preselect?

I agree with @acbramley and @catch that I've always understood this to mean "limit the list of filters to the selected options."

+1 to revert.

So putting back a key that doesn’t exist?

New configuration sounds nice but think I’d be -1 for reverting a bug. Previously to get this to work you’d have to add a 2nd filter that actually does the filter which can have its own fun.

Also want to add that limiting a drop down seems like something better_exposed_filters would do. But adding a filter that doesn’t actually filter doesn’t seem right in the section it’s in.

But adding a filter that doesn’t actually filter doesn’t seem right in the section it’s in.

It does filter, you are just limiting what a user can apply. With this change in place, you are forcing an exposed filter on to the query before the user does any action, this is quite a big change in behaviour depending on how your view is set up.

So putting back a key that doesn’t exist?

A clean revert would be easiest, and then we work on the change again and agree how it should actually be implemented. IMO this could just be another toggle that is displayed after selecting the "Limit list" option that says something like "Automatically apply these filters" or whatever wording makes sense.

I don't think it's a question of what seems right or not, this is a pretty significant and unexpected change in behaviour with no warning. I agree reverting seems weird, but the new config has to provide the new behaviour otherwise it doesn't solve anything here.

You can argue the original intent but the wording is ambiguous and it's reasonable for people to assume the intent was what it actually did, which was to limit the filter list.

This is almost certainly a fix of a typo bug that dates to at least 2009: https://git.drupalcode.org/project/views/-/blob/31dd0540d4a893f03d0fead8...

There was never an options property called limit in the In operator, and it was always reduce, so it was broken code that never evaluated to TRUE.

Regardless, it was that way for so long, it's natural that people to expect or gotten used to the former functionality.

Regardless, it was that way for so long, it's natural that people to expect or gotten used to the former functionality.

I think that's the biggest argument here for revert, I have several views that are broken because of this change. The search api one (described in #54) especially I can't see a way around to have it work again without reverting this and then adding additional configuration to toggle it.

Went ahead and committed/pushed the revert to main, 11.x and 11.4.x

Let's try to add this back with the extra checkbox, can't really see another way around it.

I'm working on the new implementation

MR is up with the following:

1. New option limit (I'm not really sure what the key should be, but I thought it would be fun to reuse the one that went missing)
2. States API to only show when reduce is checked
3. Adjusts logic to only apply the filters when BOTH reduce and limit are TRUE in acceptExposedInput (tbh I don't really understand why this particular line of code does what it does but it was the original fix so I've kept it there)
4. Upgrade path with tests
5. Updates all shipped config in core
6. Expands test coverage with reduce = TRUE and limit = FALSE

I think the title and IS are going to need updates but wanted to send for review first to get opinions on the new setting key first.

The Needs Review Queue Bot tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

Thanks @acrambely for working on this. I tested following the summary

Created 3 content types and content for each
Created a view with an exposed filter limited to just 2 content types selected
See content for all 3 (as before)
With the MR the new configuration option appears and filters out the 3rd content type.

I re-ran a new pipeline and the nightwatch was random and the other failure seems unrelated.

LGTM!

The Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

The Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

bot lied, rebased cleanly.

Apologies, took a while for me to get back to this one.

We've started to avoid needing to populate new config properties, examples:
https://www.drupal.org/project/drupal/issues/3529464#comment-16149611
https://www.drupal.org/project/drupal/issues/256287#comment-16738735

I think we can do the same here, with this change to views.filter.schema.yml:

        limit:
          type: boolean
          label: 'Limit results by selected values'
          requiredKey: false

We should also make so that either the limit value is saved as true, or that it isn't saved in config at all, (i.e., there should be no limit: false entries). I think this can be done in InOperator by overriding FilterPluginBase::submitOptionsForm and unsetting the limit value from form state if it is FALSE or if reduce is FALSE.

Then we can remove all the changes to the views config files, the update hook, and update test.

Everything else looks pretty good to me.

@godotislate do we have any examples of this for views config? It seems to be much more complicated in this case.

I implemented submitExposeForm (this is called from submitOptionsForm) on InOperator and unset the key from form_state when either are false, however there is a lot of options merging/handling in Drupal\views_ui\Form\Ajax\ConfigHandler::submitForm.

At line 219 - $handler = $form_state->get('handler'); - at this stage $handler->options already contains the limit key. When it gets down to unpackOptions, it then merges the form state's options into the handler options. Since the key already exists at that point it doesn't get removed.

I then added unset($this->options['expose']['limit']); to the submitExposedForm but it still didn't work because the same ConfigHandler initialises a new handler and calls init() on it. init() populates options again based on options defined in defaults via defineOptions. You can't remove the key from defineOptions otherwise it won't save in the first place...

That means we have to unset it in submitFormCalculateOptions as well. It's pretty messy but it does work and has test coverage. Let me know what you think.

I used Claude to help me figure out this last bit, I've left its comment in the method override as I think it provides useful context.

The Needs Review Queue Bot tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

@godotislate do we have any examples of this for views config?

No, I don't think so. It's something fairly new we've only tried or done in a couple issues. Thanks for investigating. I'm on PTO this week, so I can't say for sure when I'll take a look, but it's on my list.