Exposed term filter is not updated when terms are added, deleted, or rearranged (caching issue?)

@mikeker nice find.

This adds the right cachetag when I export the View, but the tag never gets hit when I add a tag...it doesn't even exist in the cachetags table. But this works if I manually invalidate the tag in the database.

Cache tag:

page_1:
display_plugin: page
id: page_1
display_title: Page
position: 1
display_options:
display_extenders: { }
path: test-tags
cache_metadata:
max-age: -1
contexts:
- 'languages:language_content'
- 'languages:language_interface'
- url
- url.query_args
- user
- 'user.node_grants:view'
tags:
- 'config:taxonomy.vocabulary.tags'

So I think there might be an issue in Taxonomy too that needs to get fixed before this patch has any effect.

Did some more digging and that tag only get invalidated when you change the actual config of the vocabulary. Apparently adding or changing terms doesn't fall under that.

There doesn't seem to be a vocabulary specific version of the 'taxonomy_term_list' tag, so the only option here would be to add that. Seems like overkill but better then looking at old content I'd guess.

Now with tests and a check if the vocab exists.

Forgot to tag VDC

Now with proper Cache::mergeTags use.

1. +++ b/core/modules/taxonomy/src/Plugin/views/filter/TaxonomyIndexTid.php
   @@ -385,6 +386,21 @@ public function getCacheContexts() {
   +    // There is no vocabulary specific tag for 'taxonomy_term_list' so we add
   +    // the generic tag here to catch any updates to terms.
   +    $tags = Cache::mergeTags($tags, ['taxonomy_term_list']);
   +    return $tags;
   

   I think this generic list cache tag is totally fine, especially because terms don't change constantly.

2. +++ b/core/modules/taxonomy/tests/src/Functional/Views/TaxonomyIndexTidFilterTest.php
   @@ -153,4 +153,21 @@ public function testPostUpdateFunction() {
    
   +  /**
   +   * Tests that the cache tags for the chosen vocabulary are added.
   +   */
   +  public function testGetCacheTags() {
   +    /** @var \Drupal\views\Entity\View $view */
   +    $view = View::load('test_filter_taxonomy_index_tid');
   +    $view_executable = $view->getExecutable();
   +    $view_executable->initDisplay();
   +    $cache_metadata = $view_executable->getDisplay()->calculateCacheMetadata();
   +
   +    $expected_cache_tags = [
   +      'config:taxonomy.vocabulary.tags',
   +      'taxonomy_term_list',
   +    ];
   +    $this->assertEquals($expected_cache_tags, $cache_metadata->getCacheTags());
   +  }
   +
   

   I think we should have a test which fails without the fix but more from a user point of view, aka. ensure that the terms are always up to date. Do you think this makes sense?

Rerolled patch #9

#16 worked fine for me on 8.7
submitting same file to test against 8.7.x-dev

I think that #3067937: Views Exposed Filter Block not inheriting the display handlers cache tags, causing filter options not to appear may fix this too.

Hi, when I added this patch I got watchdog flood problem "More than one Taxonomy terms on node" as relationship for "Term" exposed filter results in Trying to get property of non-object in Drupal\views\ManyToOneHelper->addTable() (issue #2882076). Also I try issue 2882076 patch but that didn't fix watchdog flood problem. After I take this (patch 16) patch off that watchdog flood problem when gone so for me I can't use this patch

So I got this issue and https://www.drupal.org/project/drupal/issues/2882076

We can now add a vocabulary specific 'taxonomy_term_list:BUNDLE' tag. See Added an ENTITY_TYPE_list:BUNDLE cache tag.

Limitation: A new ENTITY_TYPE_list:BUNDLE cache tag has been added to 8.9.x-dev.

Rerolled patch for 9.1.x, please review.

+1 to this. I removed that comment that is no longer relevant. Patch is against 9.1.x.

BTW, Wim said in #18 he suspects #3067937: Views Exposed Filter Block not inheriting the display handlers cache tags, causing filter options not to appear may fix this. It doesn't I tried it. That issue is separate and relates just to exposed filters in a separate block.

My patch didn't change any code, just removed a comment that was irrelevant, so I feel comfortable RTBCing this.

This change results in views configuration. This means we need an update path for existing views. For example if you save a view which uses the TaxonomyIndexTid after applying this patch you get the following changes:

diff --git a/views.view.frontpage.yml b/views.view.frontpage.yml
index 732ecd6..1f37621 100644
--- a/views.view.frontpage.yml
+++ b/views.view.frontpage.yml
@@ -307,7 +307,9 @@ display:
         - 'user.node_grants:view'
         - user.permissions
       max-age: -1
-      tags: {  }
+      tags:
+        - 'config:taxonomy.vocabulary.tags'
+        - 'taxonomy_term_list:tags'
   feed_1:
     display_plugin: feed
     id: feed_1
@@ -344,7 +346,9 @@ display:
         - 'user.node_grants:view'
         - user.permissions
       max-age: -1
-      tags: {  }
+      tags:
+        - 'config:taxonomy.vocabulary.tags'
+        - 'taxonomy_term_list:tags'
   page_1:
     display_options:
       path: node
@@ -362,4 +366,6 @@ display:
         - 'user.node_grants:view'
         - user.permissions
       max-age: -1
-      tags: {  }
+      tags:
+        - 'config:taxonomy.vocabulary.tags'
+        - 'taxonomy_term_list:tags'

Fortunately since this is calculated on save I don't think we don't have to do anything with views provided modules - although it might be nice to somehow tell them we have to export. We have ViewsConfigEntityUpdater for this sort of thing. Not sure. Anyhow we can start by adding a post update to find views that use this filter and re-save them.

Ah yes, that makes sense. Here's the update hook. Maybe it would be better to only find views that are using the tid filter and save those only? Dunno if that's over optimizing.

Last patch failed to apply re-rolling for 9.1.x , kindly review.

Re-rolled for 9.2.x
Also fixed the CS issues and a typo.

I think we still need to address @dawehner's point in #10, we should add a functional test for this, to see that this actually leads to the wanted behaviour.

Not sure if the update path needs testing, but I'm guessing it does even though it's pretty trivial

@Lendude
- I've added a simple test case to validate the options in the views exposed form.
Test only patch is failing on local and it is passing with patch.

Looks good to me.

1. +++ b/core/modules/taxonomy/src/Plugin/views/filter/TaxonomyIndexTid.php
   @@ -407,6 +408,19 @@ public function getCacheContexts() {
   +    $tags = Cache::mergeTags($tags, ['taxonomy_term_list:' . $this->options['vid']]);
   

   I think this might be one of the first usages of #2145751: Introduce ENTITY_TYPE_list:BUNDLE cache tag and add it to single bundle listing in core - nice!

2. +++ b/core/modules/taxonomy/tests/src/Functional/Views/TaxonomyIndexTidUiTest.php
   @@ -258,4 +258,20 @@ public function testExposedUnpublishedFilterOptions() {
   +    $this->drupalGet('test-taxonomy-exposed-filter');
   +    $this->assertSession()->optionExists('edit-tid', 'Term 1.0');
   +    $term = Term::create([
   +      'vid' => 'tags',
   +      'name' => "Test Term",
   +      'parent' => 0,
   +    ]);
   +    $term->save();
   +    $this->drupalGet('test-taxonomy-exposed-filter');
   +    $this->assertSession()->optionExists('edit-tid', 'Test Term');
   

   One thing I'm wondering is should we only be adding the new cache tags if the filter is exposed. If the filter not exposed I don't think this new cache tag gives us anything apart from more invalidations that are unnecessary - since without the filter being exposed there is no list of taxonomy on the resulting view.

3. +++ b/core/modules/views/views.post_update.php
   @@ -45,6 +45,18 @@ function views_post_update_field_names_for_multivalue_fields(&$sandbox = NULL) {
   +  // Re-save all existing views to force cache tags to be recalculated. This will
   +  // add the missing taxonomy cache tag for views using the taxonomy filter.
   +  $views = \Drupal::entityTypeManager()->getStorage('view')->loadMultiple();
   +  foreach ($views as $view) {
   +    $view->save();
   +  }
   

   This needs to use the \Drupal\Core\Config\Entity\ConfigEntityUpdater to do this as a batch and this should only be re-saving views that use the filter (or a filter that extends it).

updating tests for point 3 , raised by @alexpott in #43.

Kindly review.

@vakulrai thanks for looking at this

• The test view seems to be missing from your patch, so we need to add that back in
• The update now only checks if there is an exposed filter before running the update, it would be better if it checked if the view is using the taxonomy filter or a plugin extending the taxonomy filter before flagging it for an update

Thanks @Lendude for pointing out the missed test file, I mistakenly uploaded the wrong patch.
As suggested I have added one more condition to the post_update, to check if the expose filter is using "taxonomy_index_tid" plugin.

Thanks.

Added a condition to prevent unwanted warnings in the loop. Please consider this patch.

Thanks.

@vakulrai thank you, nice progress, couple of things that I see still:

#43.2 still needs to be addressed, we only want to add this tag if the filter is exposed

#43.3 is partially addressed now, but checking for the plugin ID means we miss all plugins that extend the taxonomy filter, but we also need to update those.

I came across this issue when searching for a resolution of exposed filters being cached although caching is diabled in their underlying views. In my case, I use exposed filters for building search pages (search_api in combination with solr_api_solr). I place these exposed filters as blocks on certain pages.

Sometimes it happens that after resetting all caches at night, these exposed filters are not rendered. Whatever the reason (perhaps system overload). They are then missing on all pages until the next "hard reset" of caches: drush cr. I have actually checked several times that the caching of the underlying views is deactivated.

I am not quite sure about this, but maybe the cause of the problem described here is deeper?

I rerolled the patch for Drupal 9.4.

Rerolled #54 so that it applies cleanly to 9.5.x

Fixed CCF for #56.

The Needs Review Queue Bot tested this issue. It either no longer applies to Drupal core, or fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

Apart from a re-roll or rebase, this issue may need more work to address feedback in the issue or MR comments. To progress an issue, incorporate this feedback as part of the process of updating the issue. This helps other contributors to know what is outstanding.

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

Re-rolled #57

Taking a look at the patch I see the upgrade path but no tests for it.

I have a view using database search api that has this issue. Adding/removing terms require a cache flush for them to apply within the exposed filter. This patch doesn't help as it doesn't add cache tags based on taxonomy within the DB search. May be a new issue, but the result is the same.

- Adding a test case for validating config save of views.
- Resolving merge conflicts as patch was not getting applied on 10.1.x head.

Upgrade tests look good, and fail locally.

Committed d7bd8ac and pushed to 10.1.x. Thanks!

@luenemann pointed out that the patch didn't address #50 and the update may not catch all cases. I think rather than copying more logic to detect when the view needs to be updated we could instead just load and save every view in the update. Reverted so we can re-commit with that change.

Made the update hook save all Views and added the 'only when exposed' option to the adding of tags and added test coverage for that.

Let's see what this does.

Local update testing seems to be broken for me, so let's see what the bot thinks....

Saw this in Slack, tagging to be smashed. 😉 I hope to look more closely soon...

I am on 10.1.2 and I have this same issue. I have some exposed filters that are taxonomies. If I add or edit a content type that has an entity reference to one of these taxonomies, the taxonomy does not show up in my exposed filter as a choice unless I clear the cache. I just applied the patch in #70, but it did not fix the issue. Do I need to apply the patch from #70 AND #63?

I haven't been able to work around this issue yet, but I'm still on D9. I ended up installing https://www.drupal.org/project/rebuild_cache_access and instructing my authors of the one and only time to use it, and to schedule that during non peak hours.

I will be on D10 within the next month or so, and can certainly test this again at that point. I'd love to be able to remove that button.