Make this work with multilingual

Yikes. Well, this ain't great at all! Thanks for the detailed report.

I think, once #3003607: Fix broken tests is done, we should aim to write some tests of this before we actually fix the problem. I'll try to write a couple, based on your excellent issue summary.

A basic attempt to implement translations. Not sure if this will work, as it's completely untested...but I thought I'd throw it up here and see if it works for you. :)

This works when I've applied it manually to lightning_workflow 3.x.

Do we need/want a related issue in https://www.drupal.org/project/lightning_workflow issue queue?

No need, I think. Once we have a fix in this issue (and tests), I can port that into Lightning Workflow's package.

Right on. We needed to resolve this bug when using lightning_workflow 3.x, so I created the attached is a patch for lightning_workflow 3.x. Again this is for lightning_workflow 3.x, not lightning_scheduler.

Actually, I don't think this works as expected, well at least with lightning_workflow 3.x. I'm going to organize my observations and report them here > #2927363: Lightning Workflow should continue to allow editors to create forward revisions of multiple languages on a single entity initially.

Patch #5 works for me with lightning scheduler 3.x

But facing issues when scheduled multiple transitions like mentioned in this issue https://www.drupal.org/project/lightning_workflow/issues/2980968

#5 patch works for me too after applying it manually to lightning_workflow 3.x module, however when I edit scheduled multilingual node; transition date and state information is not shown.
Note: Transition state and date data is available in database tables. After running CRON, transition happens successfully with #5 patch.

Please help.

In patch #5 and #9 default value for translation was not rendered.

Please find updated patch file for lightning_workflow 3.x.

Updated patch file for lightning_scheduler.
PS: Not for lightning_workflow 3.x.

Update patch as it was not getting applied for lightning_scheduler module.

Sorry for spamming.

Thanks for the updated patches @n4r3n! The lightning_workflow 3.x patch works to allow scheduling of translation independently of each other.

However, during cron, both transitions are not processing. An initially look points to TransitionManager::getTransitionable() query logic. The use of latestRevision() only returns the most recent revision ID, not both for each language. As a result, only one translation's transition is processes while the other appears to removed...meaning the unprocessed scheduled transition does not still appear on the node edit form.

I'm not sure what the best path forward is.

I don't think using entity queries will work here...unless a core patch provides a new method to return latest revisions for all translations.

Using a select query on specific 'scheduled_transition_date tables to get most recent revision IDs for each language may work. However, I'm not sure how to write this abstractly. I was thinking we could use the entity type storage to get the revision table (using $storage->getRevisionTable()) to then concatenate 'scheduled_transition_date', but it appears this would not work for block_content. It's $storage->getRevisionTable() returns block_content_revision, not block_content, which is how the block_content__scheduled_transition_date table is named.

List of scheduled_transition_date tables after lightning install.

• block_content__scheduled_transition_date
• crop_revision__scheduled_transition_date
• media_revision__scheduled_transition_date
• node_revision__scheduled_transition_date

Thoughts?

Alright, I'm wrong here about the query. The query finds the correct entities, and TransitionManager::getTransitionable() returns both translations to TransitionManager::process() in the following snippet.

        foreach ($entity->getTranslationLanguages() as $language) {
          yield $entity->getTranslation($language->getId());
        }

However, when TransitionManager::process() sets the TransitionSet with the scheduled_transition_date or scheduled_transition_state values, the translation revision with the latest revision (largest revision ID) works but the other translation revision with the oldest revision (smallest revision ID) does not.

      $transition_set = new TransitionSet(
        $entity->get('scheduled_transition_date'),
        $entity->get('scheduled_transition_state')
      );

Alright, took another dive into this with the 3.x version. Attached patches are for this version.

1) The TransitionManager::getTransitionable() query logic does not work in all scenarios. Namely, when a pending draft translation has a revision ID that is less than a newer published source translation that does not have a scheduled transition. In this scenario, the query logic does not result with a revision ID associated with the entity that includes a scheduled transition for a translation revision.

To solve this, the query logic needs to be replaced. And, we need to use translations when instantiating the TransitionSet, otherwise the correct translation values are not retrieved. I've attached a working patch to address this. However, the query logic is a puzzle I haven't solved yet. Nor do I have tests yet.

2) For the same scenario, when a pending draft translation has a revision ID that is less than a newer published source translation that does not have a scheduled transition, the logic used in ModerationStateWidget does not work either. To reproduce this, create a pending draft revision of a translation with a scheduled transition. Then, update the source translation with a new revision. Then, edit the translation and see the scheduled transition no longer renders.

I've attached a separate patch for the widget. No tests yet.

See #3080995: EntityRepository::getActive does not return a translations most recent revision for related issues using Entity Repository.

I wanted to park these patches here to get some review until I can return to it.

FYI - I've added this support issue to Drupal core issue queue, #3088341: Add ability to query on most recent language specific revision

The widget patch in #18 wasn't working for me in lightning_workflow 3.14 so Ii rerolled it

Updating file with correct filename

Hi, re-rolled the last patch for lightning_workflow 3.15

None of the above patches worked for us so our team rolled this one up.

Ok, I can't get anything working on version 1.3

so I reverted to 8.x-1.2

I will re-roll the patch that works, have to upload it.

I also found a major performance big with the module and so I fixed it in the following patches, the 8.x-1.3 release executes a query that is processing everything back to 1970 when unix time began, this is really unnecessary, it only needs to go as far back as the last cron run, hopefully if someone is using this module they would be running cron more than once per month, so I think it would be reasonable to go back only 4 weeks (or 2 weeks, better yet, look up the last cron run time). This will vastly improve performance and scalability. I will have to roll this performance improvement right into the patch because it touches code near what the patch touches.

I'm also going to follow up in these two issues that seem to really make 1.3 unusable for us and impossible to reroll our patch for 1.3 and still have it work:

#3157558: Remove the transition manager from ModerationStateWidget

patch to follow in the next 24 hours or less

Friendly reminder that, while I'm happy to accept patches, I don't feel comfortable committing them without automated test coverage of some sort. Unit test, kernel test, or whatever else...but something like this cannot be without test coverage. :)

Also, please don't use the Critical priority for anything except data loss issues.

removing my previous patches on 1.3 that don't seem to work at all for me.

Here's the patch, and the interdiff explains the performance and scalability fix.

This patch is for 8.x-1.3. What I have done is revert the two conflicting commits in 8.x-1.3 and replaced the null fix with a simpler fix.

On top of this I've added the multilingual fix and also the performance and scalability fix.

Scenario: After installing and configuring ultimate cron, I noticed that lightning_scheduler was taking 4 minutes 45 seconds to run, yet there were NO scheduled records that were not already executed. I noticed that there is re-processing happening, loading all entities that had scheduled dates going back through to 1970, the beginning of unix time. So every cron run, all scheduled records were being re-processed.

I fixed this with a simple date limit, going back 8 weeks (completely arbitrary) , however this could be further optimized by only going back to the last successful cron run time (this is stored somewhere in Drupal , I could have added that but leaving that for a future improvement after maintainer feedback).

To do a performance test before and after patching:

• make sure to install ultimate_cron , observe the execution time before and after patching.
• In my system with many records, the cron job for lightning_scheduler took almost 5 minutes to execute. After patching this dropped down to about 2 seconds

This is a full and comprehensive patch for 8.x-1.3 tag resolving multilingual support and also the null uuid issue (related and linked to this issue) and also resolves the aforementioned performance issue.

the above patch is for 8.x-1.3

Nice patch, the performance improvement is quite welcome. However, I wouldn't feel comfortable committing this without automated test coverage of some sort. Is there any chance you could take a stab at that?

@phenaproxima, can you please update coverage so that Drupal 9 is included here: https://www.drupal.org/node/2997891/qa ?

Let's see if your HEAD tests even pass the current coverage on Drupal 9.2.x or 9.3.x? Please add and also set to php 7.4.x, php 8 and php 7.3 instead of php 7.1

as for automated tests, I'm not super hot with them but if you can get the automated jobs kicked in at least we'll have a baseline and I can maybe have a kick at it.

Hi @phenaproxima, it would only take you a moment to add Drupal 9.2.x to this list:
https://www.drupal.org/node/2997891/qa

or would you rather add me as a co-maintainer since this module is seeking co-maintainers?

Done, @joseph.olstad: https://www.drupal.org/pift-ci-job/2223076. Looks like there are a few failures, but they should be very easy to solve (I'll just put protected $defaultTheme = 'stark' into those tests, and should fix them up.)

Test failures are now fixed by #3246878: Fix tests in Drupal 9.

Thanks, great work.
you can also please remove postgres 9.1 coverage with D9.2 because postgres 9.1 isn't supported on D9
PHP 7.3 & PostgreSQL 9.1, D9.2 12 fail

It's gone; Postgres 10 with Drupal 9.2 is now the default. I can't remove the old test run from that page, but you can safely ignore it.

Patch isn't ready, but the interdiff I put up is helpful. Still working on this.

working on this now

patch #30 is not yet perfect , I hope to be able to spend more time on this module and test out our use cases with multilingual a bit more.

I'll be back at this hopefully soon.

I've been hammering on this for several days now, seeing a couple smoke signals

concerns for multiple schedules for more than one language at the same time or on the same cron run for the same entity:

1) The TransitionSet class has a keyed value pair for the entity but does not take into consideration language, it should have an entity key AND a language key but the current structure doesn't allow for that and it's pretty abstract design I'm still confused how the trim function actually ends up affecting the schema (tables).

2) revisions, making sure that the correct revision is used for the transition state, getLatestTranslationAffectedRevisionId works for one language but not the other. So I've come up with something that might work but still haven't nailed it because of perhaps the TransitionSet class structure which I still am having a hard time understanding how it affects the schema although yes I do understand the methods but not sure how this class ends up changing something in the database.

Wow this was tricky to figure out, here's the new patch and interdiff vs patch 30

See the changes we made to the TransitionSet 'trim' method. The while loop is gone, cannot do that logic because it'd end up in an infinite loop because the control condition isn't appropriate for the use case.

Instead I've done a for loop and note the decrementing when removing a list item, this is to not miss any indexes as items are removed the indexes change position (pretty sure on that), as far as my initial testing goes the logic seems to hold good.

This works best when publishers save drafts in desired translations and save those before scheduling. It's likely just how core works might not have to do with the patch.

If you try to transition something before the translation is made the translation schedule might not look right. Patch probably needs a bit of mileage yet but we're pleased enough with the progress today to share this.

Hi @phenaproxima, it would only take you a moment to add Drupal 9.3.x to this list and set php 7.4.x as the default version of php?:
https://www.drupal.org/node/2997891/qa

automated testing caught something unexpected. Need to sleep on it.

Triggering tests on this one.

still working on it

ok so to improve this, not sure if we'd need a core patch, but there's a stampede on updating the same node id.

What I've come up with is a workaround.

To improve this implementation we'd maybe need a core patch or possibly it might already be supplied, but an event subscriber for the 'update' entity but I'm not sure that would work either because at what point does the db get updated? still might be a stampede with an event subscriber.

I'll look at further improvements down the road, for now just wanting to make something that does the job.

I'll upload a new version with some relatively minor but important changes in the next 24 hrs

ok this patch seems to work quite well. not sure why the test is failing though.

Pretty sure I can fix the bug and get green tests. Just need to set up a barebones vanilla environment

this might fix the tests.

I couldn't get the tests running locally but did spend a few hours on that today, I can run core tests locally but for some reason contrib modules tests aren't working for me, save that for another day.

see interdiff vs patch 53, some cleanup and if there's only one language it will now use the current thread so hopefully this is what the tests want. New patch (56).

patch 56 was a mistake,

Try this one.

ok I know why this failed. A parameter I changed to ID due to an experiment with the batch api, should use the object as it was before.

w00t! passed tests!

next, make tests for multilingual (2 or more languages)

ANY VOLUNTEERS for writing the test?

ok a slight bit of cleanup

ok while that fixes the tests, it doesn't help the multilingual side of things according to my own extensive validation tests. Although I'm kicking things around while testing, and it's getting late too.

Loading by the id actually made things pretty reliable for transition states like published from not published and to archived however pending drafts are another story.

still working on this...

ok this is an untested unfinished patch, no interdiff.

ok making some progress here, had to refactor this query:

OLD CODE, doesn't do the job for us:

    $ids = $storage->getQuery()
       ->accessCheck(FALSE)
       ->condition('scheduled_transition_date.value', $now, '<=')
       ->condition('scheduled_transition_date.value', $time_ago_sql, '>=')
       ->execute();

Changed to something more useful:

    $database = \Drupal::database();
    $query = $database->query("SELECT entity_id, revision_id, langcode
     FROM {node__scheduled_transition_date}
     WHERE scheduled_transition_date_value <= :now_time
     AND scheduled_transition_date_value >= :time_ago", [
       'now_time' => $now,
       'time_ago' => $time_ago_sql,
     ]
    );
    $result = $query->fetchAll();

I don't yet have a patch ready though, still working on this.

Ok so major update, I knocked myself out on this one but it ends in a happy story.

There's other scheduling modules out there and I stumbled on one that works (when patched with a very small patch) with multilingual scheduling.

I'm still evaluating it but so far it looks very promising.

Here's the module I added to my build:
composer require 'drupal/scheduled_transitions:^2.2'

In the composer.json it's:
"drupal/scheduled_transitions": "^2.2",

Here's the patch I added to my composer.json :

So I'm uninstalling lightning_scheduler and going with scheduled_transitions and will report back to say how well it works out (or not).

I highly recommend to everyone here wishing to do multilingual transitions to check out the scheduled_transitions module .

So far the scheduled_transitions module seems to work for me without patches however there's a couple steps to follow for it to work regularly for pending drafts to other transition states simultaneously in more than one language.

• for two translations/languages of one entity when scheduling choose a specific revision instead of latest revision and uncheck the checkbox option for pending revision that comes up in the user interface when publishing
• also make sure that the default language code for new content is the same as the site language, then translations are always the 'other' language.

none of my multilingual support patches actually worked correctly and I've abandoned that effort since scheduled_transitions plus one tiny patch is all that is needed for multilingual scheduled transitions to work simultaneously in the same cron run.

For those not using multilingual, the lightning_scheduler has a beautiful UI interface so you'll perhaps want to continue using it however if you do use it you'll definately need/want this performance fix.

I did create a fix for the really nasty performance bug.

Here's that patch.

for the performance, scalability fix

use this patch instead:

#3102295-10: Transition does not happen accurately when lightning_scheduler_cron() runs each minute

Renaming this issue.
For those looking for multilingual, please abandon the lightning_scheduler module and go for scheduled_transitions module instead,

with that said, lightning scheduler works well for single language unilingual, has a nice interface.

but it desperately needs this change:

We need to limit the reprocessing time, I have cooked up something here, suggest as a starting point:

diff --git a/src/TransitionManager.php b/src/TransitionManager.php
index 2d0ac4d..6d283ac 100644
--- a/src/TransitionManager.php
+++ b/src/TransitionManager.php
@@ -203,19 +233,39 @@ final class TransitionManager {
    */
   private function getTransitionable($entity_type_id, DrupalDateTime $now) {
     $storage = $this->entityTypeManager->getStorage($entity_type_id);
-
-    $now = $now->format(DateTimeItemInterface::DATETIME_STORAGE_FORMAT);
-
+    $sql_timezone = DateTimeItemInterface::STORAGE_TIMEZONE;
+    $storage_format = DateTimeItemInterface::DATETIME_STORAGE_FORMAT;
+    $now = $now->format($storage_format, ['timezone' => $sql_timezone]);
+    $cron_last = \Drupal::state()->get('system.cron_last');
+    $time_ago = $cron_last - 259200; // Three days before cron last.
     // Entities are transitionable if its latest revision has any transitions
-    // scheduled now or in the past.
+    // scheduled now or going back to a little bit before the last cron run.
+    // We should not reprocess going back to 1970 beginning of unix time.
+    // Limit past re-processing for performance and scalability reasons.
+    // Only go back in time as far as the last successful cron.
     $IDs = $storage->getQuery()
       ->latestRevision()
       ->accessCheck(FALSE)
       ->condition('scheduled_transition_date.value', $now, '<=')
+      ->condition('scheduled_transition_date.value', $time_ago, '>=')
       ->execute();

Renaming back to original issue title.

For multilingual implementations please switch instead to scheduled_transitions version 3.

For those still getting use out of lightning_scheduler I am moving the issue I found into a new critical ticket.

#3379796: Performance - Prevent redundant processing back to January 1st 1970 on every cron run

thank you Joseph I'm going to try out the scheduled transitions module as an alternative to this one which is not being maintained. (also for a multiingual site)

Yes it's too bad that this module is abandonned, the user interface is excellent , it works well for mono-lingual content except for the critical performance bug which is easily resolved using the mentioned patch.

For multilingual workflows I highly recommend the scheduled_transitions module. It works extremely well.