The views filter needs to have cacheability metadata applied when using Views caching

I assume that even though the list is wrong, the edit links still don't work. Is that correct?

Confirming that this is still an issue.

1. Added the cache for post and pre render for views
2. Invalidator included for entity preserve.

Given that our main support cases are 'user' and 'node', I am not sure why 'taxonomy_term' is in this list.

And if 'taxonomy_term' is in the list, shouldn't any base entity that uses a WA field? That would suggest that we do a dynamic lookup of views tables potentially affected by this field.

[EDIT] Should we instead be checking for the presence of one of our controlled field or filter types in the view?

And a patch.

Interdiff for the last patch.

Try again to load the right file.

[Edit[ Here's a reference to the technique, which is used in the "Fields used in views" report.

https://api.drupal.org/api/drupal/core%21modules%21views_ui%21src%21Cont...

Comments in order:

1. Nice catch on the static array. Fixed.

2. ResultRow has no getter method. https://api.drupal.org/api/drupal/core%21modules%21views%21src%21ResultR...

3. I'm not sure how complex that part would be, so left it out. This should work.

As to tests...I am not quite sure how to test this. I think the answer is:

* Create two users
* Assign users to different sections in WA
* Create a cached WA view that shows something filtered by sections
* PRE-patch -- confirm view shows the same output for both users if caching enabled
* POST-patch -- confirm view does not show the same output for both users if caching enabled

I also wonder if we should just cache this per-user. More granular caching is good for performance, but these are administrative views where a cache would be less valuable.

I just reviewed the test tests/src/Functional/ViewsFieldTest.php which checks for this condition -- but it uses Exposed filters, which removes the issue.

The bug does not appear in cases where the user is not assigned to any sections.

I would note that I don't understand the use-case for not exposing the filter.

Note: Using an exposed filter fixes the cache *only* if an argument is present, which indicates a fail in our current test.

e.g. the page /cache-test will show the filters of the prior user, but https://workbench.ddev.site/cache-test?workbench_access_section__section... will not.

That suggests that we can just update the existing test in order to prove/fix this issue.

So we don't catch this in the existing test because it seems that the drupalLogin() process resets the cache. Otherwise we would catch this in tests/src/Functional/ViewsFieldTest.php.

In that test, User1 gets these cache tags (and all nodes)

  'X-Drupal-Cache-Tags' => 'config:views.view.editorial_section http_response node:1 node:2 node:3 node:4 node:5 node:6 node_list rendered',
  'X-Drupal-Cache-Contexts' => 'languages:language_content languages:language_interface theme url user.node_grants:view user.permissions',
  'X-Drupal-Cache-Max-Age' => '-1 (Permanent)',

User2 gets these (and only the proper nodes in their sections):

  'X-Drupal-Cache-Tags' => 'config:views.view.editorial_section http_response node:1 node:2 node_list rendered',
  'X-Drupal-Cache-Contexts' => 'languages:language_content languages:language_interface theme url user.node_grants:view user.permissions',
  'X-Drupal-Cache-Max-Age' => '-1 (Permanent)',

I am uncertain how we can write an automated test for this, given the behavior of the current tests which should fail according to this issue.

That suggests that we might need to commit this without a test. Or perhaps write a kernel test?

With manual testing, the patch in #16 does not work.

Nor does the patch in #8.

Research:

- https://drupal.stackexchange.com/questions/274714/adding-a-cache-tag-to-...

The addCacheTags() method may not be available, since it seems to be related to VIews UI -- https://api.drupal.org/api/drupal/core%21modules%21views_ui%21src%21View...

It is sufficient to add a user cache context to the View. Adding more complex rules seems self-defeating and over-optimization. If people wish, we can address that in a follow-up.

That decision radically simplifies the patch, which seems to test just fine.

* Install the default workbench access scheme (drush wa-test)
* Configure the field and access scheme rules
* Generate some content
* Create a view of content that filters by Workbench Access section
* Assign two users to different sections
* Login as both users in separate browsers
* Check the view render before and after clearing cache for both users
* Re-assign a user to new sections -- check render
* Remove a user from a section -- check render

Notes from previous comments:

* We do not need an update hook, since this is a runtime alter hook.
* No interdiff, since the new patch is radically different.

Now we just need to see if this is covered by the current set of tests.

It should be covered -- but I see no examples in core Views that actually test the rendered output for different users. Their tests just check that tags / context exist, so we could test that.

Note also that I wondered if we should not add this context to uncached Views, but this test suggests otherwise:

Views/tests/src/Functional/Plugin/CacheWebTest.php

  /**
   * Tests that a display without caching still contains the cache metadata.
   */
  public function testDisplayWithoutCacheStillBubblesMetadata() {
    $view = Views::getView('test_display');

    $uncached_block = $view->buildRenderable('block_1', [], FALSE);
    $cached_block = $view->buildRenderable('block_1', [], TRUE);
    $this->assertEquals($uncached_block['#cache']['contexts'], $cached_block['#cache']['contexts'], 'Cache contexts are the same when you render the view cached and uncached.');
  }

And a patch with proper testing...

Previous patch is malformed.

Here's a test-only version that shows the change.

So the patch in #27 looks good-to-go.

Committed.

Just a note for anyone who might be using this with an entity reference selection view, you'll probably hit a fatal Uncaught PHP Exception Error: "Call to a member function bundle() on null due to the way WBA no implements hook_views_post_render():

+++ b/workbench_access.module
@@ -246,3 +248,47 @@ function workbench_access_views_post_render(ViewExecutable $view, &$output, CachePluginBase $cache) {
+    $output['#cache']['tags'][] = 'workbench_access_view';

I can't see any issues here, it seems legit to add cache tags onto the output array. Instead it's a core issue, for a fix see #3341448: EntityReference ViewsSelection::stripAdminAndAnchorTagsFromResults() should call Element::children($results)