Add conditions for ECA module

No discussion yet and no work in progress.

It'd be great, if scheduler can provide integration for ECA.

Hi,
Can you give a link to ECA project, and suggest what type of integration would be useful?

It would be so nice to define some event and action plugins of scheduler, for ECA, as seen on this source.

To schedule of publishing an entity, would be better, if we can add some conditions for publishing too.

Yes :) that would be amazing! It is very useful to send notifications to editorial team via ECA when content is scheduled for publication or 'unpublication'. (similar to the rules integration.)

It would be really nice to see an scheduler integration to ECA (https://www.drupal.org/project/eca) as many projects such as webform, commerce, flag, etc... supports integration to ECA. ECA is the future of Rules.

I was actually looking into the rules integration of scheduler and I can't find anything that can't already be done with ECA. I can't see what extra features would be required to make scheduler and ECA work together.

Can anyone tell me that I'm wrong? I'd be interested in getting this sorted, as many ECA users have asked for it.

In other words, what feature are you expecting from an ECA/Scheduler integration?

Thanks, Jürgen, for your thoughts on this, and for all the work your work on ECA and integrating submodules!

Scheduler module comes with the following Rules events (it also comes with a few actions, but I'm only interested in the events since ECA can do those actions):

• After a node has been published by Scheduler
• After a node has been unpublished by Scheduler
• After saving new content that is scheduled for publishing
• After saving new content that is scheduled for unpublishing
• After updating existing content that is scheduled for publishing
• After updating existing content that is scheduled for unpublishing

To paraphrase Jürgen from Drupal Slack #eca channel, Scheduler's Rules integration doesn't actually do what it says and simply acts on a generic entity updates. I think there is more to it, otherwise the Scheduler Rules integration's events seem to be bogus (and yet they work somehow to trigger Rules). I have looked at the code and see that events are being dispatched for the above list. Can ECA be made aware of these events?

I tried and failed miserably to emulate the "After a node has been published by Scheduler" event in ECA, since there is no way to evaluate when Scheduler is involved in cron publishing the scheduled node (no event is dispatched that ECA can react to, which is what I hoped a Scheduler ECA integration would expose).

Jürgen created an ECA publish/unpublish/scheduling model which could replace Scheduler module: https://ecaguide.org/library/simple/scheduled_publishing. The linked video is very helpful in that it exposes Jürgen's thought process while creating a model of this nature: https://tube.tchncs.de/w/5cs5Du3579Nwau1m9Rv92G

I've had another thought on this: all the events would be a duplication of what ECA already does, i.e. subscribing to entity update events.

A much simpler approach would be to provide a condition plugin in scheduler that tells us if the last update to the given entity was performed by scheduler or not. With that in place, we should be able to do everything I can find above.

Any thoughts?

@jurgenhaas Thanks very much for your time in thinking about this further. Yes, the only missing piece is knowing that Scheduler was responsible for the event. Would it be simpler to add a condition to Scheduler, or to create an ECA/Scheduler integration module?

I would be very interested in better Scheduler integration. For the purposes of the ECA I've created, I've resorted to a series of essentially if-then-else type conditions.

On entity save and validation that it's the proper node/entity that I want. Is Scheduled Open empty?

• If yes, check if published. If yes, check if Scheduled Close exists. If yes - do a bunch of work. The was a scheduled publish.
• If yes, check if published. If no, check if Scheduled Close empty. If yes - do a bunch of work. The was a scheduled unpublish.
• If no, do some other work (housekeeping)

This is a simplified version but you get the idea. Would be nice to have a definitive "Scheduler published/unpublished" type event.

the only missing piece is knowing that Scheduler was responsible for the event. Would it be simpler to add a condition to Scheduler, or to create an ECA/Scheduler integration module?

This could be a very simple ECA condition plugin which wouldn't do anything if ECA wasn't around. So, if the maintainers of the scheduler module would accept that, it would be best placed in this module. I'll provide an MR to show how simple that could be. That may help in making that decision.

Would be nice to have a definitive "Scheduler published/unpublished" type event.

Just for clarity, what I'm proposing is not a new event, it's just a condition that will tell the ECA model, if the latest update on an entity had been triggered by scheduler or not. All required events are already available in ECA and would be redundant if we added them here again.

This was probably the simplest condition plugin that I've ever committed ;-)

Please give it a try.

Just for clarity, what I'm proposing is not a new event, it's just a condition that will tell the ECA model, if the latest update on an entity had been triggered by scheduler or not. All required events are already available in ECA and would be redundant if we added them here again.

Works for me. As I review what I wrote and thinking about the ECA I've built, a condition makes sense. As you pointed out, required events already exist. I'll be giving your condition plugin a try today and report back.

Scheduler allows setting "publish" and/or "unpublish" seperately, so I believe we need separate conditions.

I would suggest changing UpdateTriggeredByScheduler to PublishTriggeredByScheduler and adding an UnpublishTriggeredByScheduler condition plugin that checks !empty($entity->unpublish_on->value).

Oops, that last commit throws a fatal
ArgumentCountError: Too few arguments to function Drupal\scheduler\SchedulerManager::__construct()

EDIT: this could have been caused by other factors in my local environment. Interested to see if my commit works for others.

Actually, it looks like Jurgen's original MR fails phpstan.

I'm testing the MR and have found an issue with checking $entity->publish_on->value since Scheduler sets this to null when it's done publishing.

This is in a new condition PublishTriggeredByScheduler, not Jurgen's original UpdateTriggeredByScheduler.

I'm also adding a check for the status changing from unpublished to published to save an extra condition in the model.

Something like this ...

  public function evaluate(): bool {
    $entity = $this->getValueFromContext('entity');

    if (!isset($entity->original)) {
      return FALSE;
    }

    $publish_on_original = !empty($entity->original->publish_on->value ?? NULL);
    $publish_on = empty($entity->publish_on->value ?? NULL);
    $published_original = $entity->original->status->value ?? FALSE;
    $published = $entity->status->value ?? FALSE;

    return $this->negationCheck($publish_on_original)
      && $this->negationCheck($publish_on)
      && !$published_original
      && $published;
  }

More testing, then I'll add to the MR for possible inclusion.

Could this be a submodule that depends on eca? Making the main scheduler module depend on eca seems more than is needed for a simple installation.

@socialnicheguru by adding a condition plugin the scheduler won't depend on ECA. If ECA is not available, the plugin just doesn't do anything. Only once ECA would be enabled, the plugin becomes active. An extra module (or sub-module) just for a single plugin sounds overkill.

I just took a quick peak at the MR and saw:

 diff --git a/composer.json b/composer.json
index f5f380a65bff59d87a62ee7ffb9928d1b317dd23..9ab54755ad21f72d612afe5a8dadcf28fb13277f 100644
--- a/composer.json
+++ b/composer.json
@@ -10,7 +10,8 @@
         "drupal/devel_generate": ">=4",
         "drupal/workbench_moderation": "*",
         "drupal/workbench_moderation_actions": "*",
-        "drupal/commerce": "^2 || ^3"
+        "drupal/commerce": "^2 || ^3",
+        "drupal/eca": ">=2"
     },
 

Now I see that ECA is being added to require-dev not require.

I agree with @darkodev that we need two conditions for publish/unpublish.

I have a very simplistic ECA model at the moment to test out the various scenarios that would mimic how we use scheduler. The code as found in the MRs doesn't behave as expected. I think part of this has to do with what @darkodev is working on in #20. Looking forward to testing more extensively once there is an update.

I've also added a verification that the entity type is supported by scheduler.

The failing phpstan is an odd behaviour from GitLab MR pipelines that uses the composer.json of the target branch instead of the one from the MR, that's why ECA isn't installed for testing. When merging or when adding the require-dev to the target branch, the test will be OK.

This looks interesting, thank you for working on it. If the presence of the plugin has no effect when ECA is not installed then I'm ok with adding it here in the main module, no need for a sub-module. I've not used ECA but it seems to have support and as it is newer it does not have the backwards compatibility problems and long-standing core issues that Rules faces. The Scheduler Rules Integration sub-module was written before Scheduler 2.0 and hence also before Scheduler dispatched its own events. Therefore supporting ECA is a good addition here.

1. From #25

The failing phpstan is an odd behaviour from GitLab MR pipelines that uses the composer.json of the target branch instead of the one from the MR

That's an interesting observation. Can you raise an issue in Gitlab Templates so we can investigate further, as that sounds like a bug which could/should be solved.

2. It would be good to have a phpunit test to cover the new ECA conditions

Setting to NW so we can discuss these two points.

I updated the MR with my proposed extra conditions that I think more accurately reflect what Scheduler does when it acts on a content entity, which is the following:

1. Content entity is changing from unpublished to published (or vice versa)
2. Entity's publish_on/unpublish_on setting is being removed after Scheduler processes it

This is by no means perfect, as the conditions above could be triggered by an author node save (ie, not a Scheduler cron). For this reason, I can only say that these conditions only reflect that Scheduler is involved in the change, not how that change was initiated (by cron, VBO, etc).

I am uploading two models to demonstrate the conditions.

To test:

Check out the latest MR version
Run db updates if you had a previous version installed
Import the two models attached
Create some content with Scheduled publish and /or unpublish dates
Run the Scheduler cron

The result should be that Scheduler publishes/unpublishes the entity and that a log message is created by the appropriate model.

Please test and provide feedback.

A test is a great idea, so perhaps someone can contribute to that task when we agree on the approach.

From #25 and #26 I have raised #3510904: New require-dev dependencies in composer.json are not loaded in composer pipeline job

The composer.json problem is entirely a Scheduler fault which I am fixing in #3510926: Remove tempoary D11 alternative composer.json

For the Scheduler content moderation integration sub-module we need a way to schedule the transitions.

I've had a chance to test @darkodev code in #27. The conditions work well for all my test scenarios. The only issue I found was that condition negation does not appear to ever fire. To be honest, I don't know if that is necessary.

I've uploaded my very simple ECA that covers all conditions (publish, unpublish, negation on both).

I have reviewed the code and left some comments as that needs some more work.

Thanks for the feedback, @jurgenhaas. My bad for misunderstanding negationCheck. I'll try to find some time to integrate your comments.

As per @jurgenhaas' feedback, I refactored for proper use of negationCheck() and the unpublished plugin now extends the published plugin.

My example models still work here locally.

Please review and provide any further feedback.

@darkodev - have tested your latest and found the update and negation works as expected.

I looked again as well. The code as such looks good to me. What I can't tell, if the 4 conditions that are chained together is really what tells us if the given entity got published or unpublished by scheduler. If it does, then the rest is OK, except for the failing PHPCS test.

@jurgenhaas - my tests included publish/unpublish not by scheduler and of course publish / unpublish by scheduler. These worked flawlessly. In order to determine if scheduler actually performed the publish, one has to look at the previous (original) values of both publish_on and status and compare to the current values. The logic looks correct to me. The same is true for unpublishing.

@bdunphy that's OK, and I'm sure the logic of that part can be reviewed by maintainers of the scheduler module who know exactly how that works.

I just wonder about 2 things:

$entity->original->publish_on->value ?? NULL

Wouldn't that throw a NULL pointer exception if publish_on didn't exist?

$entity->status->value

Similarly, the status field doesn't need to be called status. I guess the safe way of doing this is to get the property key for status from the entity annotation and then ask for the respective value of that field.

As for an ECA point of view, the plugins looks good to me, I think I mentioned that before.

The MR no longer applies to dev version of scheduler:

drupal/scheduler
https://git.drupalcode.org/project/scheduler/-/merge_requests/230.diff (Support for ECA - https://www.drupal.org/project/scheduler/issues/3363972)
Could not apply patch! Skipping. The error was: Cannot apply patch https://git.drupalcode.org/project/scheduler/-/merge_requests/230.diff

Yeah, the MR needs to be rebased to cover the latest changes of the dev branch.

Added patch. remove .gitlib and composer.json changes. seemed not germain to this issue.