When creating a new node view does not display the new node

Hi Albert,

Thanks for using Pantheon! You are describing normal behavior of the module. We suggest using Views Custom Cache Tags module to set custom tags for listing.

Here is a blog post I wrote describing how that module can be used: https://pantheon.io/blog/pantheon-advanced-page-cache-drupal-cache-metad...

I also have a new Docs page coming out soon that will cover this scenario in greater detail. If you want, I can share you on the Google Doc where it is currently drafted.

The renaming of node_list is intentional. We specifically don't want every listing of nodes getting cleared any time any other piece of content is added or saved.

Shared!

This seems like an optimization that not everyone would expect or desire, so I would like to offer the suggestion of making this into a setting that's off by default. I agree that it'd be Better™ to do cache flushing in a more fine-grained manner, but this is not always straightforward. I personally expected parity with core caching (dynamic_page_cache) which does flush for entity_list tags as it does for any other tag.

I never noticed this about the D7 version of this module, but just had a user report an issue where they were not seeing newly-created events appearing on an event listing view. It looks like if I create a new event node, it does not clear this view listing, and the node:# is not in the Surrogate-Key-Raw header until the the page Age header exceeds max-age, or the page cache is manually flushed. If I edit and save an existing event node appearing in that view, the cached page is cleared out.

It seems like in this simple example, it should be a default behavior, not something that requires another module and configuration. The user expectation is that they have created a new event and expect to see it on the view listing page without having to manually clear the cache or know to configure another module. I'm in support of this being configurable as well, and we'd turn it on by default. Most of our end users think this is a bug, and being able to turn this behavior on would reduce confusion and our ticket load, outweighing the slight reduction in cache-hit ratio it would cause.

Thanks for the feedback! I think you're right here:

expected parity with core caching

being able to turn this behavior on would reduce confusion and our ticket load, outweighing the slight reduction in cache-hit ratio it would cause.

I think the default behavior needs to change. I'll try to work on a patch next week.

@stevector: great, thanks!

I second what decafdennis said. Many sites just have a single view for a given content type, and in such cases one would expect a newly added node to simply show up. The patch from #2 does just that, so thank you, albertski. I'd love to see a UI toggle added to the module to opt into or out of this behavior. Or, failing that, a line we can add into settings.php.

@stevector: what are your thoughts on the patch in #2? We got another ticket about this today so I'd like to backport this patch to D7 if you agree its current state makes sense. Thanks!

@stevector: I'm also looking forward to this patch being applied.

Hello, I'm sorry "next week" turned into "next year."

Here are PRs for D7 and D8 that change the default behavior for new installs of the module. Sites with the module already installed can change the behavior with a setting.

Can anyone here test before I merge?

@stevector: Thanks! I will test these on Monday.

Is node_list is only emitted for D8, and does that cover views in D8? I don't see views being cleared with this PR in D7. So maybe this patch doesn't have as large an impact in D7?

@vinmassaro, node_list is emitted in D7 also.

Here's a View in an environment created by CI for that PR. https://1297-d7papc.pantheonsite.io/frontpage

You can see with curl that node_list is present:

curl -I  https://1297-d7papc.pantheonsite.io/frontpage -H Pantheon-debug:1 | grep surrogate-key-raw

surrogate-key-raw: block:search.form block:system.help block:system.main block:system.navigation block:system.powered_by block:user.login block_list block_view node:1 node:2 node_article_list node_list node_view rendered user:1 views:frontpage views:frontpage.page

That page gets cleared when I add a new node.

Also, here's a small point, I just released 8.x-1.1 for Drupal 9 compatibility. That means the D8 release with this fix will be 8.x-1.2. Do you think I should skip 7.x-1.1 and go to 7.x-1.2? That way it might be easier to follow that the 1.2 release is the release with this fix.

Ok - I think I figured out what is going on in my case. Views is only outputting "node_list" if you are using the "Content" row plugin. Most views being built by site owners are using "Fields" and I don't get the node_list cache tag output. I'm assuming this behavior is in D8Cache.

@stevector, I've tested the PR for Drupal 8 and it works great! I have only tested it with one View, it's a View setup to show "Fields" instead of a "Content".

I decided to test the Drupal 8/9 PR one more time and I just dropped in the changed module on a site with the stable release already installed and I hit this error:

[12-Jun-2020 13:19:06 UTC] ArgumentCountError: Too few arguments to function Drupal\pantheon_advanced_page_cache\EventSubscriber\CacheableResponseSubscriber::__construct(), 1 passed in /srv/bindings/9876c88752d24017b21ec64f3514fc6c/code/web/core/lib/Drupal/Component/DependencyInjection/Container.php on line 259 and exactly 2 expected in /srv/bindings/9876c88752d24017b21ec64f3514fc6c/code/web/modules/contrib/pantheon_advanced_page_cache/src/EventSubscriber/CacheableResponseSubscriber.php on line 40 #0 /srv/bindings/9876c88752d24017b21ec64f3514fc6c/code/web/core/lib/Drupal/Component/DependencyInjection/Container.php(259):

A call to drush cr resolved the error. But I wonder if I need to change

  public function __construct(LoggerInterface $logger, ConfigFactory $configFactory) {

to

  public function __construct(LoggerInterface $logger, ConfigFactory $configFactory = NULL) {

and then make the code that calls $configFactory more complex to account for a NULL value.

Anyone know if there's a standard way to handle this?

I've added some feedback to your D8 pull request.

Just run into this issue on a D7 site. Any news for a D7 fix? Is the patch #2 the way to go?

I've merged the PRs. There are now dev releases of 7.x-1.x and 8.x-1.x.

Can anyone confirm that these are working for you before I tag 1.2?

@stevector

The changes in both 7.x-1.x-dev and 8.x-1.x-dev appear to be emitting the appropriate Surrogate-Key values according to D7 variable pantheon_advanced_page_cache_override_list_tags and D8 config key pantheon_advanced_page_cache.settings/override_list_tags.

I believe it is safe to tag the pending code changes for release at your convenience.

https://www.drupal.org/project/pantheon_advanced_page_cache/releases/8.x...
https://www.drupal.org/project/pantheon_advanced_page_cache/releases/7.x...

I ran into this same issue today. Updated the module to the new release. However you didn't include an update hook to add the new config, (Edited: I see the update hook, so not sure why it didn't run via drush updb) so the drush config-set command fails to run with the error:

> [error] Config pantheon_advanced_page_cache.settings does not exist

I had to uninstall and re-enable the module to get that setting.

(Also, Note to others reading this thread...the code committed to the release was not the same as the simple patch posted here. See commit https://git.drupalcode.org/project/pantheon_advanced_page_cache/commit/9...)

Edited: I see the line in the Readme:

> Sites that installed this module prior to 1.2 should uninstall and reinstall or run this command to update their settings.

But just running the command did not work.

The update hook doesn't set it to the new recommended value. It sets it to TRUE.

/**
 * Set override_list_tags to TRUE for backwards compatibility.
 *
 * We recommend manually changing to FALSE for more consistent clearing. See
 * README.
 */
function pantheon_advanced_page_cache_update_8001() {
  \Drupal::configFactory()
    ->getEditable('pantheon_advanced_page_cache.settings')
    ->set('override_list_tags', TRUE)
    ->save();
}

So running update.php isn't going to help. I see that the README was updated, but a note in the release notes would have been very helpful.