Taxonomy views filters should not have user context

Well this was an interesting one to XDebug :D

I agree this should just be removed. The user.permissions context should cover variations here and it doesn't appear to provide any additional benefit that wouldn't be provided by #1947270: Add test coverage for the term_access query tag →

This is quite a large issue when a view is a page display or is embedded as part of the main content region and thus completely uncacheable for dynamic page cache.

Welp, it didn't break any tests, so I'd be in favor of getting this in and dealing with test coverage in the other ticket. Marking for review.

Views' default cache plugin uses the generated query as the cache key - see https://api.drupal.org/api/drupal/core%21modules%21views%21src%21Plugin%...

hook_query_alter() implementations are also able to add cache contexts, see for example: https://api.drupal.org/api/drupal/core%21modules%21node%21node.module/fu...

So we should definitely remove this, however we need a post update + presave here to resave all views so that the cache contexts get recalculated. Other view updates + ViewsConfigUpdater should have examples.

Agreed it doesn't need test coverage, we'd just be testing that the code doesn't do what it used to at a particular point in time, there wouldn't really be positive assertions to make.

Yup, makes sense. Normally just a view re-save would've done here, but it appears the re-calculating of cache meta is only done via UI. MR has been updated.

Was previously tagged for IS so moving to NW for that.

But with a post_update hook will this require test coverage?

Also new functions may need a change record, could be wrong there.

Updated issue summary and removed tag. I agree with @catch that there is no test coverage needed here as we're removing code, not adding. Tests for what this originally existed for should go into #1947270. I don't really see any reason for a change record here either, back to Needs Review.

I don't see the changes to the issue summary? Maybe they didn't save?

Not sure what happened there, it change the title but not summary.. trying again

Thanks much better!

Looks good to me too! this just bit me again so I'd be happy to get this committed

Committed/pushed to 11.x.

MR has conflicts with 10.1.x, and there's a post update in here, so leaving in that branch - it'll be released with 10.2.0

Automatically closed - issue fixed for 2 weeks with no activity.

For anyone looking for a fix, exporting the view to config/sync as yml, removing the cache_metadata.contexts.user and importing the config back to site fixed the caching issue for me.