Cachetags improvements

Attaching path implementing suggested changes

Re-rolled patch against latest 2.0.x

The last submitted patch, 3: google_tag-3364758-3-cachetags-improvements.patch, failed testing. View results →
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

Added google_tag-config cachetag to expected tags in tests.

So I see where this patch is going, by including the cacheable metadata of conditions. But I'm concerned with the overall approach. The first bug is the fact we don't ensure the getListCacheTags are always applied, so that saving config entities invalidates tags.

The problem around cache contexts for conditions needs additional validation. I don't like adding the cacheable metadata onto the resolver service directly.

1. +++ b/google_tag.module
   @@ -98,11 +99,13 @@ function google_tag_page_attachments(array &$attachments) {
   -  $cacheable_metadata = new CacheableMetadata();
   -  $cacheable_metadata->addCacheTags($config->getEntityType()->getListCacheTags());
   

   The problem is that we don't apply the list cache tag. We should just always do this. That cache tag is invalidated whenever an entity of that type is saved.

Per mglaman's suggestions above, opened a MR to approach the issue a little bit differently. Also changed the tests to hardcode the result, since the insertions weren't strict. (Basically you could make changes to cache tags and the tests would keep passing.. oops.)

Found a big gap in test coverage with \Drupal\Tests\google_tag\Kernel\PageAttachmentsHookTest::testNoEmbedOnNoConfig. It wasn't verifying cacheable metadata when there were no configs. Addressed that in https://git.drupalcode.org/project/google_tag/-/merge_requests/50/diffs?...

Merged, we'll have a release this week.

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