Created on 31 May 2023, almost 2 years ago

I see this integrates with Rules - just wondering if there was any work in progress or any discussion on integrating with the newer ECA module?

πŸ’¬ Support request
Status

Active

Version

2.0

Component

Code

Created by

πŸ‡ΊπŸ‡ΈUnited States w01f

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Merge Requests

Comments & Activities

  • Issue created by @w01f
  • πŸ‡¬πŸ‡§United Kingdom jonathan1055

    No discussion yet and no work in progress.

  • πŸ‡ΉπŸ‡·Turkey orkut murat yΔ±lmaz Istanbul

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

  • πŸ‡¬πŸ‡§United Kingdom jonathan1055

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

  • πŸ‡ΉπŸ‡·Turkey orkut murat yΔ±lmaz Istanbul

    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.

  • πŸ‡³πŸ‡ΏNew Zealand heiket

    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.)

  • πŸ‡±πŸ‡°Sri Lanka lakhithaD

    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.

  • πŸ‡©πŸ‡ͺGermany jurgenhaas Gottmadingen

    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?

  • πŸ‡¨πŸ‡¦Canada darkodev

    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 us vert helpful in that it exposes JΓΌrgen's thought process while creating a model of this nature: https://tube.tchncs.de/w/5cs5Du3579Nwau1m9Rv92G

  • πŸ‡©πŸ‡ͺGermany jurgenhaas Gottmadingen

    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?

  • πŸ‡¨πŸ‡¦Canada darkodev

    @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?

  • πŸ‡¨πŸ‡¦Canada bdunphy

    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.

  • πŸ‡©πŸ‡ͺGermany jurgenhaas Gottmadingen

    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.

  • πŸ‡©πŸ‡ͺGermany jurgenhaas Gottmadingen

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

    Please give it a try.

  • Pipeline finished with Failed
    14 days ago
    Total: 649s
    #436652
  • πŸ‡¨πŸ‡¦Canada bdunphy

    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.

  • πŸ‡¨πŸ‡¦Canada darkodev

    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).

  • πŸ‡¨πŸ‡¦Canada darkodev

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

  • Pipeline finished with Failed
    14 days ago
    Total: 637s
    #437187
  • πŸ‡¨πŸ‡¦Canada darkodev

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

  • πŸ‡¨πŸ‡¦Canada darkodev

    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 actually works in our favour, because this change is indicative of Scheduler acting on the entity.

    I've changed my model event to entity presave and changed the code to compare $entity->original->publish_on->value against $entity->publish_on->value, which works quite nicely.

    I'm also adding a check for the status changing from 0 to 1 to save an extra check in the model.

    Something like this ... could be more succinct, but I like stepping through my vars with breakpoints.

      public function evaluate(): bool {
        $entity = $this->getValueFromContext('entity');
        // Status is changing from unpublished to published
        // Scheduler is setting publish_on to null
        $publish_on_original = !empty($entity->original->publish_on->value);
        $publish_on = empty($entity->publish_on->value);
        $published_original = $entity->original->status->value;
        $published = $entity->status->value;
        $negation_check = $this->negationCheck($publish_on_original) && $this->negationCheck($publish_on) && !$published_original && $published;
        return $negation_check;
      }
    

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

  • πŸ‡ΊπŸ‡ΈUnited States SocialNicheGuru

    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.

  • πŸ‡©πŸ‡ͺGermany jurgenhaas Gottmadingen

    @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.

  • πŸ‡ΊπŸ‡ΈUnited States SocialNicheGuru

    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.

  • πŸ‡¨πŸ‡¦Canada bdunphy

    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.

  • Pipeline finished with Failed
    11 days ago
    Total: 490s
    #438438
  • πŸ‡©πŸ‡ͺGermany jurgenhaas Gottmadingen

    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.

  • Pipeline finished with Failed
    11 days ago
    Total: 777s
    #438444
  • πŸ‡¬πŸ‡§United Kingdom jonathan1055

    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 #24

    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.

  • πŸ‡¨πŸ‡¦Canada darkodev

    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

    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.

  • Pipeline finished with Failed
    11 days ago
    Total: 669s
    #439160
  • πŸ‡¬πŸ‡§United Kingdom jonathan1055

    The composer.json problem is entirely a Scheduler fault which I am fixing in πŸ“Œ Remove tempoary D11 alternative composer.json Active

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

  • πŸ‡¨πŸ‡¦Canada bdunphy

    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).

  • πŸ‡©πŸ‡ͺGermany jurgenhaas Gottmadingen

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

Production build 0.71.5 2024