🇺🇸United States @luke.leber

Everyone: Thank you for manually testing the Drupal 11 upgrade.

I'd like to leave an additional comment regarding the explicit dropping of support for Drupal 9.

Given that no testing has been performed against Drupal 9, it's my opinion that it's unlikely that any future enhancements to this module will be tested against Drupal 9 or prior either. Given this, it's even more unlikely that support ranges excluding Drupal 9 would be considered in future merge requests, meaning that unfortunate site owners that are still trapped in Drupal 9 could otherwise be unwittingly walked into WSOD situations. The most responsible thing to do as a community is to drop support so that users stuck on out of support versions of Drupal will not be able to brick their site by an upgrade of this module that couldn't be effectively tested against end of life software.

I'll cut a new release for Drupal 11 support shortly. Thanks again.

We also need to test on the previous major given Drupal 10 is still supported, so the gitlab CI template needs to be updated.

The MR pipelines are presently failing.

A composer.json file is required for phpunit testing in Gitlab CI.

There needs to be a test for this to happen. Happy to accept any test coverage 😇.

I don't have any cycles to do much more than keep the lights on, as I no longer manage any live sites that use this module.

I'll test drive this over the weekend on 11.x. Given there were no manual tests against Drupal 9 and it's end of life, I think it's best to explicitly drop support, leaving ^10.3 || ^11 as the support range.

Claiming that Drupal 9 is still actively supported would be somewhat false advertising 😅.

luke.leber → changed the visibility of the branch 3506838-consider-adding-scheduling to active.

luke.leber → changed the visibility of the branch 3506838-consider-adding-scheduling to hidden.

I plan on opening a pull request this weekend at some point, so self-assigning.

luke.leber → created an issue.

The root cause here is that version 2.0.6 introduced a MAJOR backwards compatibility problem.

Prior to 2.0.6, library dependencies could be relied upon in order to prevent really gnarly race conditions from happening between the GTM JS and Drupal libraries.

This happened because https://www.drupal.org/project/google_tag/issues/3452712 🐛 Possibly script loading/placement issue Fixed significantly changed the loading order in a way that bypasses dependency resolution. As a result, in order to prevent race conditions, any drupal library that is depended on by GTM must also be loaded in the head, and the GTM library definition must be hooked into in order to declare the dependency..

This feature would allow the Inline All CSS → module to continue to live in Drupal 11+.

Feel free to review, but this approach seems very reasonable to me in terms of performance 💪. This operates at a very low level, so it's very likely that this first stab is not 100% perfect.

Business logic encapsulated in a helper service

<?php

namespace Drupal\psu_layout_builder;

use Drupal\Core\Database\Connection;
use Drupal\Core\StringTranslation\TranslatableMarkup;
use Drupal\layout_builder\Section;
use Drupal\layout_builder\SectionComponent;

/**
 * Helper to facilitate 'additional' to 'third_party_settings' migration.
 */
class SectionComponentTPS {

  /**
   * Service constructor.
   *
   * @param \Drupal\Core\Database\Connection $database
   *   The database connection service.
   */
  public function __construct(
    protected Connection $database
  ) { }

  /**
   * Migrates 'additional' to 'third_party_settings' for the provided table.
   *
   * @param string $table
   *   The database table to migrate.
   * @param array $sandbox
   *   The post-update sandbox.
   *
   * @throws \Exception
   *   😱 God help you if this happens.
   */
  public function migrate(string $table, array &$sandbox, $sections_per_batch = 10000) {
    if (!isset($sandbox['total'])) {
      $query = $this->database->select($table);
      $sandbox['total'] = $query->countQuery()->execute()->fetchObject()->expression;
      $sandbox['current'] = 0;
      if (empty($sandbox['total'])) {
        $sandbox['#finished'] = 1;
        return NULL;
      }
    }

    $query = $this->database
      ->select($table, 'layout')
      ->fields('layout', ['bundle', 'entity_id', 'revision_id', 'langcode', 'delta', 'layout_builder__layout_section'])
      ->range($sandbox['current'], $sections_per_batch);

    $rows = $query->execute()->fetchAll(\PDO::FETCH_ASSOC);
    if (empty($rows)) {
      $sandbox['#finished'] = 1;
      return;
    }
    else {

      // Combine all update queries into a single upsert for performance.
      $upsert = $this->database->upsert($table)
        ->fields(['bundle', 'entity_id', 'revision_id', 'langcode', 'delta', 'layout_builder__layout_section'])
        ->key('revision_id');

      foreach ($rows as $row) {
        /** @var \Drupal\layout_builder\Section $section */
        $section = unserialize($row['layout_builder__layout_section'], ['allowed_classes' => [Section::class, SectionComponent::class]]);
        foreach ($section->getComponents() as $component) {
          // Set might be deprecated, but it's hella efficient.
          // Don't run this code on Drupal 12, eh?
          $component->set('thirdPartySettings', $component->get('additional') ?: []);
          $component->set('additional', []);
        }
        $row['layout_builder__layout_section'] = serialize($section);
        $sandbox['current']++;
        $upsert->values($row);
      }

      $upsert->execute();
    }

    if ($sandbox['current'] >= $sandbox['total']) {
      $sandbox['#finished'] = 1;
    }
    else {
      $sandbox['#finished'] = ($sandbox['current'] / $sandbox['total']);
    }
    return new TranslatableMarkup('@count of @total' . ' layout sections processed.', [
      '@count' => $sandbox['current'],
      '@total' => $sandbox['total'],
    ]);
  }

}

Service configuration boilerplate

services:
  Drupal\psu_layout_builder\SectionComponentTPS:
    class: Drupal\psu_layout_builder\SectionComponentTPS
    autowire: true

Sample post update hooks for *one* entity type

/**
 * Migrate 'additional' to 'third_party_settings' for nodes.
 */
function psu_layout_builder_post_update_tps_nodes(array &$sandbox) {
  return \Drupal::service(SectionComponentTPS::class)->migrate('node__layout_builder__layout', $sandbox, 10000);
}

/**
 * Migrate 'additional' to 'third_party_settings' for node revisions.
 */
function psu_layout_builder_post_update_tps_node_revisions(array &$sandbox) {
  return \Drupal::service(SectionComponentTPS::class)->migrate('node_revision__layout_builder__layout', $sandbox, 10000);
}

Let's do this thing @ 10,000 records per batch.

www-data@9218f372c877:~/html$ time drush updb
 -------------------- -------------------- ------------- ---------------------- 
  Module               Update ID            Type          Description           
 -------------------- -------------------- ------------- ---------------------- 
  psu_layout_builder   tps_node_revisions   post-update   Migrate 'additional'  
                                                          to                    
                                                          'third_party_setting  
                                                          s' for node           
                                                          revisions.            
  psu_layout_builder   tps_nodes            post-update   Migrate 'additional'  
                                                          to                    
                                                          'third_party_setting  
                                                          s' for nodes.         
 -------------------- -------------------- ------------- ---------------------- 


 Do you wish to run the specified pending updates? (yes/no) [yes]:
 > 

>  [notice] Update started: psu_layout_builder_post_update_tps_node_revisions
>  [notice] 10000 of 208192 layout sections processed.
>  [notice] 20000 of 208192 layout sections processed.
>  [notice] 30000 of 208192 layout sections processed.
>  [notice] 40000 of 208192 layout sections processed.
>  [notice] 50000 of 208192 layout sections processed.
>  [notice] 60000 of 208192 layout sections processed.
>  [notice] 70000 of 208192 layout sections processed.
>  [notice] 80000 of 208192 layout sections processed.
>  [notice] 90000 of 208192 layout sections processed.
>  [notice] 100000 of 208192 layout sections processed.
>  [notice] 110000 of 208192 layout sections processed.
>  [notice] 120000 of 208192 layout sections processed.
>  [notice] 130000 of 208192 layout sections processed.
>  [notice] 140000 of 208192 layout sections processed.
>  [notice] 150000 of 208192 layout sections processed.
>  [notice] 160000 of 208192 layout sections processed.
>  [notice] 170000 of 208192 layout sections processed.
>  [notice] 180000 of 208192 layout sections processed.
>  [notice] 190000 of 208192 layout sections processed.
>  [notice] 200000 of 208192 layout sections processed.
>  [notice] 208192 of 208192 layout sections processed.
>  [notice] Update completed: psu_layout_builder_post_update_tps_node_revisions
>  [notice] Update started: psu_layout_builder_post_update_tps_nodes
>  [notice] 6082 of 6082 layout sections processed.
>  [notice] Update completed: psu_layout_builder_post_update_tps_nodes
 [success] Finished performing updates.

real    0m15.490s
user    0m7.965s
sys     0m0.749s

15 seconds to churn through over 210,000 records seems agreeable to me!

I wonder if the gist in #239 could be optimized by operating on the layout builder section storage tables and bypassing the Entity API? The SectionStorage can be loaded and saved independently of the content entity it's attached to I believe.

Generally, saving content entities is MAJORLY slow comparatively. I plan on running additional benchmarking on this at some point as we have over a quarter million sections to churn through when this finally lands. 🤞

Unassigned -- I have no idea where to even start to look now!

This is a pretty gnarly issue. From what I can gather, these are the enabling criteria for the mix-up in CSS order...

1. The SDC must be provided by a theme
2. The SDC must define a libraryOverrides that adds a dependency.

The result is that the dependency is loaded after the Experience Builder preview library 🤯. I'm not sure what the best way to share an example of this is.

Any suggestions?

This change doesn't seem to add much value. It simply restates the module's description.

I'll set it back to needs work for now, but unless there's anything helpful information for end-users to add via this hook, it won't be merged.

My recommendation is to gather feedback from users and see what they're having trouble with, if anything, before making an attempt at a hook_help implementation.

General +1 from me on this. In my evaluation of the CONTRIBUTING file, I didn't consider that setting up a working SSH key with Gitlab was a prerequisite. 😮

Might be even better to state the root cause and add a link to the doc page for setting up an SSH key?

You probably do not have a valid SSH key configured for accessing repositories on drupal.org. See <page> for setting one up or explore the following alternative options.

Until the README stops pointing to the CONTRIBUTING file, this seems safest. Ultimately there will probably have to be different instructions for people wanting to contribute/commit to the project and for those that are simply trying to casually use a pre-release to see what's coming.

Shouldn't a metatag integration of sorts be used for this? 😰

luke.leber → created an issue.

Update I.S. with more info.

luke.leber → created an issue.

luke.leber → created an issue.

smustgrave → credited luke.leber → .

Setting back to Needs Work. Here's a slack chat to document where things are.

balintbrews
23 minutes ago
@lleber
I might be doing something wrong, but I can’t reproduce the issue on 0.x anymore.

lleber
:male-scientist: 23 minutes ago
I'll double check, but I was just able to around 8:30 :sweat_smile:

lleber
:male-scientist: 16 minutes ago
Ah, so it may be a dependency based thing :open_mouth:.
I added an example-flexbox-break component and here's the CSS inheritance...

(XB styles override example-flexbox-break component styles here)

9:38
Then I added a header (custom) component and here's the inheritance!

(header component styles override XB styles here)

9:39
The [data-xb-slot-id] selector seems to have nondeterministic loading order, meaning the example in the I.S. seems to work, but another component does not!
9:40
I'll dig a little deeper to see if I can sleuth a better example.

balintbrews
13 minutes ago
I can see that you have CSS aggregation turned on. Maybe that comes into play as well?

lleber
:male-scientist: 13 minutes ago
I can try disabling it, but the only reason the example component in the I.S. seems to work is because its display: flex is being overridden.

balintbrews
13 minutes ago
Yes, I saw that happening on my end as well.

lleber
:male-scientist: 12 minutes ago
I do think that if the SDC had a dependency injected on the preview CSS, it'd straighten itself out, but that seems odd.

balintbrews
12 minutes ago
Yeah, we don’t want that.
9:44
@lleber
I’m leaning towards just committing the width: 100% you suggested, I can’t see how that would hurt us. And it may help you now. :slightly_smiling_face: If we see more of these styling issues, we can do a deeper investigation. What do you think?

lleber
:male-scientist: 8 minutes ago
I think it'd definitely help in the short-term, but it may be a harbinger of deeper issues. I think it has to do with the SMACSS layer the libraries are loaded at.
9:46
The component displaying problems is registered at the component layer. Trying to figure out which layer SDCs are registered at.
9:46
I wonder if the XB styles should load at the lowest layer so that everything can override them? Or will that cause issues (see: layout builder off-canvas :smile:)? (edited)
New

balintbrews
2 minutes ago
Our styles in preview.css are unlayered, so they will take precedence over other layers.

lleber
:male-scientist: 1 minute ago
Hm..then I wonder how our custom header is winning there. I'm out of time this morning to debug, but I'm unblocked to find a root cause now.

I think a deeper investigation is needed to see what exactly is going on.

I.S. update + NR

This is certainly still an issue, except now the .xb--sortable-slot-empty-placeholder class doesn't specify a minimum width, and will collapse down to 0px wide if the parent element has display: flex.

I think that the safest bet here is to give the .xb--sortable-slot-empty-placeholder a width of 100%. The rationality here being that there should only ever be one empty placeholder per slot, right? As soon as a component is added, the placeholder goes away. I can't think of any reason why we wouldn't want the placeholder's drop-zone to be smaller than it could be!

luke.leber → created an issue.

https://www.drupal.org/project/experience_builder/issues/3491021 ✨ Leverage HTML comment annotations, remove wrapping div elements Active has landed and this issue needs re-triaged. Self-assigning and will get around to this tomorrow between 8:30 and 5:00 ET.

Whenever I see a call to wait on a lock I get a chill up my spine.

From a cursory glance, this change could result on a ton of processes spinning/waiting to acquire a lock under a heavy load, couldn't it?

I think this one should be subject to some load testing on a larger site.

A first step is probably to get all of the changes needed in place that Gitlab needs to get all of the jobs green. That's a standardized set of tooling that everyone can share. I think https://www.drupal.org/project/juicebox/issues/3351683 📌 [PP-1] Resolve all coding standard violations Postponed is the place for that work I think. Any commit made to the issue fork over there will automatically run those tests.

Koustav Mondal self-assigned that issue to them self, but hasn't made any commits in a week, so IMO it's fair game for anyone to commit to.

Things like phpstorm configuration will probably vary from person to person, but the gitlab tests are a shared configuration between everyone. They should probably be resolved first before making any other changes so that everyone's on the same page with the standard checks.

After the gitlab checks are all passing, so long as further changes don't make the checks fail, they can probably be applied in a follow-up.

Here's something for us to run temporarily in the new year...

diff --git a/core/assets/scaffold/files/default.settings.php b/core/assets/scaffold/files/default.settings.php
index cd364bb00d..751a5de5ff 100644
--- a/core/assets/scaffold/files/default.settings.php
+++ b/core/assets/scaffold/files/default.settings.php
@@ -811,6 +811,14 @@
  */
 $settings['migrate_node_migrate_type_classic'] = FALSE;
 
+/**
+ * Aggregated asset error logging.
+ *
+ * This is used to inform site owners when CSS or JS assets fail to be included
+ * in an aggregated asset.
+ */
+$settings['log_aggregated_asset_errors'] = FALSE;
+
 /**
  * The default settings for migration sources.
  *
diff --git a/core/lib/Drupal/Core/Asset/CssOptimizer.php b/core/lib/Drupal/Core/Asset/CssOptimizer.php
index 8f2d910250..61d00ea129 100644
--- a/core/lib/Drupal/Core/Asset/CssOptimizer.php
+++ b/core/lib/Drupal/Core/Asset/CssOptimizer.php
@@ -5,6 +5,7 @@
 use Drupal\Component\Utility\Unicode;
 use Drupal\Core\StreamWrapper\StreamWrapperManager;
 use Drupal\Core\File\FileUrlGeneratorInterface;
+use Drupal\Core\Site\Settings;
 
 /**
  * Optimizes a CSS asset.
@@ -145,7 +146,11 @@ public function loadFile($file, $optimize = NULL, $reset_base_path = TRUE) {
     // stylesheets in their .info.yml file that don't exist in the theme's path,
     // but are merely there to disable certain module CSS files.
     $content = '';
-    if ($contents = @file_get_contents($file)) {
+
+    // Temporary hack for debugging impossible to diagnose aggregation issues...
+    $contents = Settings::get('log_aggregated_asset_errors') ? file_get_contents($file) : @file_get_contents($file);
+
+    if ($contents !== FALSE) {
       // If a BOM is found, convert the file to UTF-8, then use substr() to
       // remove the BOM from the result.
       if ($encoding = (Unicode::encodingFromBOM($contents))) {

Unrelated - there should probably be a strict comparison against the file_get_contents result here -- otherwise some false positives can slip through depending on the file contents 😱!

Ideally when we know that a corrupted asset is going to be generated, we'd want to issue an HTTP/500 to prevent the asset from becoming cached by downstream infrastructure, but that definitely can't happen without breaking huge swathes of sites.

Given the uncertainty about what's supported or not in #2, I think maybe we could add this behind an opt-in behavior to allow for a graceful path to deprecation?

I'm sure that a lot of sites out there have missing assets that's considered very, very normal.

I think this one might be dependent on how https://www.drupal.org/project/experience_builder/issues/3491021 ✨ Leverage HTML comment annotations, remove wrapping div elements Active pans out.

Hi all, given the relatively short remainder of life for Drupal 7, I'm going to close this out, but still assign issue credit where appropriate (hence the 'fixed' status) as it looks like genuine effort was put into this.

Cheers all.

luke.leber → changed the visibility of the branch 3351683-pp-1-resolve-all to hidden.

Hey, I'm closing this one as outdated, but if this is still an issue, please re-open it.

Thanks

I'm un-postponing this issue.

Given we're running on Gitlab now, let's get all of the jobs green. I'll open a fresh MR to start.

Hi Jane, that looks right for working with the Entity API in Drupal 8+. Thanks for sharing!

Hi, can you please provide the view configuration that you were seeing this on?

If we're not able to reproduce the issue with a carefully controlled set of steps to reproduce and write a new test for it, it's a lot harder to justify making any changes. I'm triaging this as PMNMI until we can hear more about the particulars of the site experiencing issues.

Thanks!

Good day, 4.0.0-alpha2 just landed. If you run into any issues don't hesitate to spin up an issue.

Thanks

Stand by for 4.0.0-alpha2 as soon as the main pipeline goes green.

I've manually tested the common use cases for this module against 11.1 and 10.4 including...

1. Media reference field configured to display juicebox formatters
2. Image field configured to display juicebox formatters
3. View configured to display content as juicebox galleries

Given we have green tests and a solid manual test, I'm going to merge this in and cut 4.0.0-alpha2 for folks waiting. If anything did manage to slip through, I'll be available to triage things throughout the week after the day job.

luke.leber → changed the visibility of the branch 4.0.x to hidden.

luke.leber → changed the visibility of the branch 3433346-automated-drupal-11 to hidden.

Green on 10.4.0, 11.0.8, and 11.1.0.

Testing on PREVIOUS_MAJOR, PREVIOUS_MINOR, and CURRENT seems very logical to me. Setting to Needs Review for manual testing.

This module does, indeed need work, as there are missing schema definitions.

Leaving NW so that someone can look into the test failures, or I can pick them back up when I'm able.

Outdated by https://www.drupal.org/project/juicebox/issues/3494657 📌 Get tests passing on Gitlab CI Active (oops, I forgot I already did this work last year :O ).

Now that I've merged this in,pulling in the 4.0.x branch on a dev site via composer require drupal/colorbox:dev-4.0.x will pull in the latest development build for the 4.0.x branch.

Subsequent patches can be applied on top of that. I'll see you over in the Drupal 11 issue.

The update bot has messed up the support ranges here.

The deprecation helper service was introduced in Drupal 10.1.3, but it has the module support range specified as >=10. That means that 10.0.0 through 10.1.2 users would have their sites killed by this.

The support range has to be at minimum ^10.1.3 || ^11 here. I strongly recommend using ^10.3 || ^11 since that's what we're able to test on though...

Setting back to needs work. If no one else picks this up before I'm able to, I'll make the commit myself, but in the meantime feel free.

Given these are test-only changes, I'll merge this in tomorrow evening if it hasn't been reviewed by then so we can move onto the Drupal 11 compatibility issue with confidence and get that one finished up by the weekend. 👍

FINALLY! A green run that actually runs the tests.

Setting to NR.

Hey, there are still some issues preventing this. Primarily not being able to run tests on Gitlab CI. Admittedly, I've slacked as a maintainer here, as I no longer manage any sites that actively use Juicebox, but I'll set aside time this through the weekend to hopefully grind through things and release something.

luke.leber → created an issue.

Isn't the required attribute the equivalent signal for non-visual users? It's announced by all major A/T tools.

Adding another signal is probably redundant IMO.

I gave !475 a test run, recompiled the UI, flushed some caches, and refreshed. Lo and behold, working required indicators.

This looks great to me, marking RTBC, as the accessibility concern is alleviated by this, even if a follow-up design task needs to happen.

luke.leber → created an issue.

This might be fine if we think SDC creators already know to output $attributes for other reasons, but #9 indicates that this isn't currently being consistently done in the wild.

We made the decision not to use $attributes in our twig, as it'd tightly couple things to Drupal. We may be the outlier, but I'm living proof of the concern :-)

Added copy/paste example to I.S. w/o any external dependencies :-).

It's actually pretty simple to resolve if the component knows if it's being observed in Experience Builder.

A custom twig extension...

class XBExtension extends AbstractExtension  {
  public function getFunctions() {
    return parent::getFunctions() + [
      new TwigFunction('is_xb_editing', [
        $this,
        'isXbEditing',
      ]),
    ];
  }

  public function isXbEditing() {
    return \Drupal::routeMatch()->getRouteName() === 'experience_builder.api.preview';
  }
}

...and using it in the SDC template:

{% set expanded = is_xb_editing() %} {# <-- "in_preview" variable-like-thing #}

{% include '@my-upstream-components/accordion.twig' %}

That said, I wouldn't recommend writing this off as a niche problem. I can't think of any interactive / progressive disclosure elements from *any* design systems that wouldn't need a mechanism like this. I think it's more along the lines of not having any example components available in XB to have stepped on this yet. It'd be really unfortunate for each design system to have to roll their own "in_preview"-like feature.

That's pretty neat, @jessebaker. It definitely *helps*, but the editing UX still seems off as the accordion seems to re-flip back to closed every time it's re-rendered.

Strangely enough, the newly added component seems to be selected by default even after re-paint and it's hidden.

luke.leber → created an issue.

Testing out the idea #32 is probably blocked by https://www.drupal.org/project/experience_builder/issues/3491021 ✨ Leverage HTML comment annotations, remove wrapping div elements Active .

I did play around with the latest work in #3491021, but it was too unstable confirm without any doubt that the approach is viable. From the limited experimentation I've performed, I think there's a good chance it'll be workable as #3491021 wraps up though.

Here's a gif of the behavior (described in previous comment).

Even when the accordion is collapsed, are you able to move a component into a hidden slot via the Layers panel (left sidebar)?

Yep. That works.

That should already work with #3486785: Initial support for allowing interaction with the preview. Not sure if state is retained when exiting preview though, or if the whole canvas gets re-rendered and therefore the accordion reverts to collapsed.

Yep, the preview mode works perfectly -- things are able to be interacted with just fine in there. It's just the non-preview mode where it's impossible to use the mouse and/or keyboard to expand/collapse interactive things.

While this can be worked around for accordions (through adding an "expand by default" prop and temporarily adjust that while adding content), I don't think it'll be quite as easy for other interactive UI patterns, like multiple tabs. For completeness, this is how we've set up our Tabs implementation in Layout Builder (dragging a block over a tab activator will actually activate the tab in the LB editor):

This all looks good to me! I re-ran

# Set a cache entry to persist for 5 minutes with a tag of "test_tag".
vendor/bin/drush eval "\Drupal::cache()->set('test', 'value', time() + 300, ['test_tag']);" && echo 'Set a cache entry'

# Confirm the cache entry is valid.
vendor/bin/drush eval "echo 'The cache entry is...' . (\Drupal::cache()->get('test') ? 'valid' : 'invalid');"

# Invalidate the tag via an ECA action.
vendor/bin/drush eca:trigger:custom_event test && echo 'ECA invalidated the tag'

# Confirm the cache entry is no longer valid.
vendor/bin/drush eval "echo 'The cache entry is...' . (\Drupal::cache()->get('test') ? 'valid' : 'invalid');"

with...

1. A single hard-coded tag
2. A single tag that was resolved by a token
3. No tags (invalidate everything)

I think this one's good to go! Great stuff!

Updated I.S. the xb link in toolbar works on HEAD now.

— Any chance you could add the missing test coverage? 🙏

All signs point to probably not 😭. NW and slowly backs away.

I suppose that an alternative approach might be to allow certain rendered elements receive input events, but that seems like it'd be quite the technical lift to accomplish.

Attached screenshot with more context.

However! when creating a brand new action (rather than starting with an old one, then patching), I get [warning] Undefined array key "backend" CacheActionBase.php:80 printed out.

I'm setting this back to NW. It looks like Drupal\eca_cache\Plugin\Action\CacheActionBase::getCacheBackend may need an update to cope with derivative classes that do not operate on a specific backend.

As a result, I'm pretty sure that Drupal\eca_cache\Plugin\Action::execute needs to be refactored to do a full cache flush and not exit early via

    if (!($cache = $this->getCacheBackend())) {
      return;
    }

That said, I'm not 100% sure what the right API calls should be in if tags are empty.

    if (empty($tags)) {
      $cache->invalidateAll(); <-- definitely not this, but I'm not sure what to replace it with!
    }

I was able to successfully test the invalidation action.

uuid: 46349853-db2b-4cfc-822c-9ac6680f2ff1
langcode: en
status: true
dependencies:
  module:
    - eca_base
    - eca_cache
id: process_jfh9njk
modeller: bpmn_io
label: noname
version: ''
weight: 0
events:
  Event_1bq8hvb:
    plugin: 'eca_base:eca_custom'
    label: Event_1bq8hvb
    configuration:
      event_id: test
    successors:
      -
        id: Activity_087tmd6
        condition: ''
conditions: {  }
gateways: {  }
actions:
  Activity_087tmd6:
    plugin: eca_raw_cache_invalidate
    label: Activity_087tmd6
    configuration:
      tags: test_tag
      backend: entity
      key: 'values:node:1'
    successors: {  }

# Set a cache entry to persist for 5 minutes with a tag of "test_tag".
vendor/bin/drush eval "\Drupal::cache()->set('test', 'value', time() + 300, ['test_tag']);" && echo 'Set a cache entry'

# Confirm the cache entry is valid.
vendor/bin/drush eval "echo 'The cache entry is...' . (\Drupal::cache()->get('test') ? 'valid' : 'invalid');"

# Invalidate the tag via an ECA action.
vendor/bin/drush eca:trigger:custom_event test && echo 'ECA invalidated the tag'

# Confirm the cache entry is no longer valid.
vendor/bin/drush eval "echo 'The cache entry is...' . (\Drupal::cache()->get('test') ? 'valid' : 'invalid');"

Output should be:

Set a cache entry
The cache entry is...valid
ECA invalidated the tag
The cache entry is...invalid

My only concern here is one of performance. If token substitution happens, but yields an empty string, does that mean that this kicks in?

When empty, then the whole cache is being invalidated.

For larger sites, full cache invalidations can lead to some pretty significant performance degradation, and in some cases denial of service if the ECA workflow runs often enough.

I'll put it on my list to do tomorrow.

luke.leber → created an issue.

luke.leber → created an issue.

luke.leber → created an issue.

luke.leber → created an issue.

luke.leber → created an issue.