- Issue created by @sboden
Some more information, I'm still debugging.
I have a node bundle subdossier that has a workflow subdossier, and that workflow has some extra fields. These fields pop up on the subdossier node view form depending on the chosen "to workflow" with some manual conditional hook alter forms.
There's a field "field_amount_paid_back" that exists both on the workflow and on the subdossier node bundle. And in the presave hook of the WorkflowTransition the field is copied from Workflow to node bundle with following function:
public static function presave_transition_paid_back(EntityInterface $entity) { $subdossier = $entity->getTargetEntity(); \Drupal::logger('SBO_INFO')->info('Starting presave_transition_paid_back - Subdossier ID: @id, Label: @label', [ '@id' => $subdossier->id(), '@label' => $subdossier->label(), ]); $subdossier->field_amount_paid_back->value = $entity->field_amount_paid_back->value; \Drupal::logger('SBO_INFO')->info('Setting field_amount_paid_back to @value for subdossier @label', [ '@value' => $subdossier->field_amount_paid_back->value, '@label' => $subdossier->label(), ]); }
I added some extra logging... read from bottom to top (it's from "Recents logs")
7 24/Apr 12:57 SBO_INFO Info Setting field_amount_paid_back to 112 for subdossier 2020CCP003262-C 6 24/Apr 12:57 SBO_INFO Info Starting presave_transition_paid_back - Subdossier ID: 688488, Label: 2020CCP003262-C 5 24/Apr 12:57 SBO_INFO Info Workflow transition presave - Bundle: subdossier, From: subdossier_paid, To: subdossier_paid_back 4 24/Apr 12:57 SBO_INFO Info Entering entity_presave for 3 24/Apr 12:57 SBO_INFO Info Node presave - Bundle: subdossier, ID: 688488, Label: 2020CCP003262-C 2 24/Apr 12:57 SBO_INFO Info Entering entity_presave for 2020CCP003262-C 1 24/Apr 12:57 SBO_INFO Info Entering node_presave for 2020CCP003262-C
This happens when I press "Change workflow" on the node view form of subdossier 2020CCP003262-C. It enters the presave for the node, the entity presave is entered. Then the workflow transition presave is entered (the one without label above). And then finally that one field is copied from workflow to node bundle.
The results:
- When using workflow v1.7 the field_amount_paid_back is saved correctly on the node bundle.
- When using workflow v1.8 or higher the field_amount_paid_back is not saved on the node bundle.However in both cases the flow is the same according to my debug logging statements. In v1.8 and higher the field is copied from workflow to node bundle, but the node bundle doesn't seem to get saved anymore. In v1.7 for some reason the node bundle is still saved after the presave from workflow transition is executed.
The only thing different is the version of the workflow module.
Found it in v1.8.
When I add following to workflow.entity.inc:
/** * Implements hook_entity_presave(). * * @param \Drupal\Core\Entity\EntityInterface $entity * An EntityInterface object. */ function workflow_entity_presave(EntityInterface $entity) { if (!$entity->isNew()) { // Avoid a double call by hook_entity_presave and hook_entity_insert. _workflow_execute_transitions($entity); } }
Then the functionality reverts to the v1.7 behaviour.
Patches attached revert the behaviour back to the way v1.7 behaved for saving target entities (in a sneaky way). I have a patch for workflow 1.8 and workflow 2.0
If possible I would like to have them reviewed and published as workflow version 1.8.1 and 2.0.1. It is breaking behaviour, but it's actually reintroducing the way v1.7 worked. The main change is reintroducing the presave hook.
The way I see it, no project is yet using workflow v2.0 (it's too new). Most projects are on workflow v1.7, maybe tried to upgrade to v1.8 but reverted because of the changed behaviour, I did as well.
If you have a project using workflow in a simple way, workflow v1.8 would work as is, the new patch is probably not going to break anything. When you have a project using workflow in a complex way, these patches will revert saving behaviour of the target entity of a workflow transition as that was done in workflow v1.7
- 🇳🇱Netherlands johnv
this part is committed in the latest commit for another issue , by accident:
I think I remember removing it. It was a mistake.diff --git a/src/Form/WorkflowTransitionForm.php b/src/Form/WorkflowTransitionForm.php index 9ae3804..2d4f15d 100644 --- a/src/Form/WorkflowTransitionForm.php +++ b/src/Form/WorkflowTransitionForm.php @@ -154,6 +154,11 @@ class WorkflowTransitionForm extends ContentEntityForm { // since it will be done in many form_id's. // Keep aligned: workflow_form_alter(), WorkflowTransitionForm::actions(). // addActionButtons($form, $form_state); + + if (!empty($actions['submit']['#value'])) { + $actions['submit']['#value'] = $this->t('Update workflow'); + } + return $actions; }
- 🇳🇱Netherlands johnv
I do not understand the following:
diff --git a/workflow.theme.inc b/workflow.theme.inc index 6a7da29..e78d4dd 100644 --- a/workflow.theme.inc +++ b/workflow.theme.inc @@ -19,8 +19,17 @@ use Drupal\Core\Template\Attribute; * the details element. Properties used: #children. */ function template_preprocess_workflow_transition(array &$variables) { + $elements = $variables['elements'] ?? NULL; + if (empty($elements)) { + return; + } + /** @var \Drupal\workflow\Entity\WorkflowTransitionInterface $workflowTransition */ - $workflowTransition = $variables['elements']['#workflow_transition']; + $workflowTransition = $elements['#workflow_transition'] ?? NULL; + + if ($workflowTransition) { + return; + } $variables['from_state'] = $workflowTransition->getFromState() ?? ''; $variables['to_state'] = $workflowTransition->getToState() ?? '';
- In my D11 system, the hook is not even called.
- why+ if ($workflowTransition)
, should this not be :+ if (!$workflowTransition) {
to avoid an error in the following lines? - 🇳🇱Netherlands johnv
Also, in my test system, the extra fields are added normally to the saved transitions, and can be recalled in the Workflow History view.
The are not copied back to the TargetEntity, and IIRC that was never the design.
There is also an otion to add Workflow to Comments, and there, the comment field must be identical to the TargetEntity field. I need to test that scenario.
For template_preprocess_workflow_transition you're right, the patch I have running in production for that part is:
diff --git a/workflow.theme.inc b/workflow.theme.inc index 1f28e39..b46bebc 100644 --- a/workflow.theme.inc +++ b/workflow.theme.inc @@ -20,7 +20,11 @@ use Drupal\Core\Template\Attribute; */ function template_preprocess_workflow_transition(array &$variables) { /** @var \Drupal\workflow\Entity\WorkflowTransitionInterface $workflowTransition */ - $workflowTransition = $variables['elements']['#workflow_transition']; + $workflowTransition = $variables['elements']['#workflow_transition'] ?? NULL; + + if (!$workflowTransition) { + return; + } $variables['from_state'] = $workflowTransition->getFromState(); $variables['to_state'] = $workflowTransition->getToState();
I wanted to make it nicer, but I slipped up.
I'll rephrase the problem when going from workflow version v1.7 to workflow v1.8 or v2.0.
In workflow v1.7, the
target_entity
was saved when a workflow transition happened. This saving happened in a hidden way - probably not even on purpose - but it did happen in workflow v1.7.In some of our Drupal applications, we change data during a workflow transition: we copy fields from the workflow to its
target_entity
. After a transition, we saw that the value was also updated on thetarget_entity
.But in workflow v1.8 (and also in v2.0, which is based on v1.8), that saving no longer happens. The update to the
target_entity
still takes place (in our custom code), but because it is not saved, the update is lost. In v1.7, the saving was triggered - indirectly - by theworkflow_entity_presave
hook.If we want backwards compatibility with v1.7 in v1.8 or v2.0, then
workflow_entity_presave
is needed. This missing hook is probably also the reason I never updated to v1.8 earlier - because my applications started acting strangely after the upgrade.Choices:
- Includeworkflow_entity_presave
. In that case, most people can upgrade from v1.7 to v2.0 without many issues.
- Don’t includeworkflow_entity_presave
. Then you have two options:
1. Patch workflow yourself to addworkflow_entity_presave
. But this might break in the future if workflow changes again.
2. Change your application to explicitly save thetarget_entity
. I’ve tried this, but in some cases it’s difficult to get exactly the same behavior as before.Personally, I would prefer if
workflow_entity_presave
were part of the base version of workflow. But if it’s not, I can patch it myself. I don't know how many others are affected when going from v1.7 to v2.0. If you use workflow in a simple way, v2.0 works fine as is. But if your application depends heavily on workflow, then you will run into problems withoutworkflow_entity_presave
.I also noticed that the label "Update workflow" was removed. This caused my Behat tests to fail, because the label on the button was changed. That label was "reorganized away" in v2.0
- 🇳🇱Netherlands johnv
Sorry for the delay. I spent my time on other module, and more/other foundational errors.
I committed your function template_preprocess_workflow_transition patch, slightly modified. I guess you did this since it was activated without any transition present?
I still cannot reproduce calling this function. Also, '#workflow_transition' does not occur in another part of the code. Something must be lost.
The comments hint to 'comment', so I will now test the use case that Workflow is added to the Comments, not the Node. There, the value should be copied from the Comment to the Node.
Now I realize that that is the only use case for copying back values to the node. I never tested the option to copy the 'extra fields' back.So, from you initial patch, only hook_entity_presave() remains open.
- 🇳🇱Netherlands johnv
Please test/check latest dev version, no that 📌 Support core change record template_preprocess_HOOK Active is implemented.
It also fixes an error I introduced in the preprocess() function.The presave() hook is still not in. Instead of adding an extra save() of the Entity, I'm investigating the use of setEntityWorkflowField(), which is used already. We also update the ChangedTime field already.
// Update targetEntity's itemList with the workflow field in two formats. $this->setEntityWorkflowField(); $this->setEntityChangedTime();
I tried different workflow versions with the behat tests on my biggest Drupal project. The project has 10 node bundle types with each a workflow attached to it and the project does a lot of things upon workflow transitions: e.g. if you change a workflow in one type, it may hunt down nodes in other types and recursively change the workflow in those, which may trigger further other changes. It uses a lot of workflow functionality, except scheduled workflow transitions: we didn't find a use for those.
The behats test system functionality, the behats go through a lot of the functionality to check whether everything is still ok. And since a lot of our functionality is behind workflow transitions, these also get exercised a lot. It does not 100% proof everything works, but if some behats fail, it's a bad sign. 1 behat test can be upto 2 or 3 A4 pages long.
With
composer require drupal/workflow:=1.7
: all 225 behats runs successfully (as expected). This is my current baseline in production.With
composer require drupal/workflow:=2.0.0
: (without my patch above): 220 behats succeed, 5 fail. The 5 failures are about workflow transition history being different.With
composer require drupal/workflow:=2.0.0
: (with my patch above): 220 behats succeed, 5 fail. Not expected, but it is what it is.With
composer require drupal/workflow:=1.x-dev#430c793f889ece2a527db93033139eba65df8c1
This was the latest merge I found today (26may2025). All behats break. At first sight this version introduces a problem with saving nodes. Nodes which were saving with the tagged workflow 2.0.0 version now complain they can't be saved, as some fields need data.For the presave hook in my patch. Personally I would like to have it included in the base workflow version, but I may be biased ;). The presave hook is a little bit sneaky, but it is how workflow worked up until and including workflow v1.7. In workflow v1.8 that functionality broke. From personal experience and the comments in the workflow issues I deduce that most people are still on workflow v1.7 and skipped/delayed the update to workflow v1.8 since v1.8 broke things. To me it does make sense to reintroduce v1.7 behavior to make upgrades easier. E.g. for my projects it will take me about a week per project to find all places where I have to adapt code, if I don't reintroduce the presave hook (and hoping I find all places I need to adapt).
- 🇳🇱Netherlands johnv
Hi,
I am still working on your problem.
Unfortunately, I am each time distracted by other small bits&pieces that hurt my eyes.
This gave me the insight that the entity_save() (including $this->executeTransitionsOfEntity ) is done AFTER the node_save().
So, it must be possible to not ADD the entity_presave() - as in your patch, but to MOVE the same function from entity_insert() and entity_update() into presave().
I do recall having some issues with the correct behaviour of these hooks. But that in an old version.Let me do some tests.
- 🇳🇱Netherlands johnv
I am stuck, having an Error message "This value should not be null." upon node creation.
You have no such experience? - 🇳🇱Netherlands johnv
Never mind. I found the reason. Will be fixed in next commits.
- 🇳🇱Netherlands johnv
@sboden, the problem stems from attached issue 🐛 Revision id is NULL on node edit if revision enabled Fixed
- Status changed to Fixed
3 months ago 10:14am 3 July 2025 Automatically closed - issue fixed for 2 weeks with no activity.
TL;DR: Using workflow v2.1.3: 218 of the 225 behats that work using workflow 1.7 and 1.8 (with patches) work, 7 failed.
What I did is:
composer require drupal/workflow:^2.1.2
which imports workflow v2.1.3 (at this moment). Then:
drush cr drush updb ---------- ----------- --------------- ----------------------------------------------------------------------------- Module Update ID Type Description ---------- ----------- --------------- ----------------------------------------------------------------------------- workflow 8007 hook_update_n 8007 - This will fix the Error 'Mismatched entity and/or field definitions' on /admin/reports/status. workflow 8008 hook_update_n 8008 - Updates existing configuration to match configuration schema. ---------- ----------- --------------- -----------------------------------------------------------------------------
And then run my set of behat tests. 7 fail... 5 of them with following (and the other 2 are dependent I think):
Error: Call to a member function setTargetEntity() on null in Drupal\jobbonus\Data\ApplicationDAO->saveData() (line 183 of modules/custom/jobbonus/src/Data/ApplicationDAO.php).
And in my custom code this is what triggers it:
$transition = WorkflowTransition::create([ $current_sid, // phpcs:ignore 'field_name' => 'field_objection_status', ]); $transition->setTargetEntity($objection); // this is line 183.
Looking at WorkflowTransition::create() it doesn't do the same anymore. workflow v2 needs 'wid' in the input array, else no transaction is created.
- 🇳🇱Netherlands johnv
The create() function as in your code works as expected in my system. I released v2. 1.4, but that should not make a difference.
The WID should be deducted from the SID.
Does the entity have that field name?
Do you have multiple workflows per entity?
Can you debug the function? I will try again... can you give me the exact version of workflow you want me to test. Maybe with git hash?
- 🇳🇱Netherlands johnv
Please use https://www.drupal.org/project/workflow/releases/2.1.x-dev →
I am only working on Actions, now. Do you use them?
I retested my behats. What I did:
ddev composer require 'drupal/workflow:2.1.x-dev@dev' # Which got me "Cloning e21f8ee43f" ddev drush updb ---------- ----------- --------------- ----------------------------------------------------------------------------- Module Update ID Type Description ---------- ----------- --------------- ----------------------------------------------------------------------------- workflow 8007 hook_update_n 8007 - This will fix the Error 'Mismatched entity and/or field definitions' on /admin/reports/status. workflow 8009 hook_update_n 8009 - Updates existing configuration to match configuration schema. ---------- ----------- --------------- ----------------------------------------------------------------------------- ddev drush cr ddev robo behat ... --- Failed scenarios: features/backoffice/operator/operator.feature:3 features/frontoffice/frontoffice.feature:57 features/frontoffice/frontoffice.feature:124 features/frontoffice/frontoffice.feature:167 features/frontoffice/frontoffice.feature:200 features/frontoffice/frontoffice.feature:256 features/frontoffice/frontoffice.feature:344 225 scenarios (218 passed, 7 failed) 10429 steps (10344 passed, 7 failed, 78 skipped)
All behats worked successfully with workflow 1.7 and workflow 1.8 (with the hook patch) starting from the same database state.
This is a piece of my code that crashes. It crashes because WorkflowTransition::create() returns NULL.
... $objection->save(); // Always set the initial state. $current_sid = workflow_node_current_state($objection, 'field_objection_status'); $comment = $address_overridden ? t('Correspondence address was manually overridden by citizen') : ''; $transition = WorkflowTransition::create([ $current_sid, // phpcs:ignore 'field_name' => 'field_objection_status', ]); $transition->setTargetEntity($objection); $transition->setValues($status, \Drupal::currentUser()->id(), \Drupal::time()->getRequestTime(), $comment, TRUE); $transition->force(TRUE); workflow_execute_transition($transition, TRUE);
The problem seems to be that workflow_node_current_state() seems to doing something else than before. When you created a new bundle, workflow_node_current_state() in v1.7 would return the initial state (as string) attached to the workflow, now it seems to return "".
- 🇳🇱Netherlands johnv
Hmm, we do not get any further.
You save the entity, then set the field name and create a transition.
It seems the field is not defined yet in the entity. Can you find out in which lin of code the error occurs?
I could/should add exceptions? When I debug I find this message as exception "Drupal\workflow\Entity\Workflow::getFirstSid(): Argument #3 ($user) must be of type Drupal\Core\Session\AccountInterface, null given, called in /var/www/html/www/modules/contrib/workflow/src/Entity/WorkflowTransition.php on line 1053"
I'll debug further in the weekend, and hopefully find a fix.
I debugged and made a patch... actually 2 patches.
This patch is a manual patch. What changed:
- In workflow v1.7 when you requested the workflow status of a newly saved bundle, it would return the creation status. In v2 it would return ''. You could work around it since you know when you have a newly saved object and set the creation status yourself, but it's easier to be backwards compatible.
- In some cases owners could be empty, giving problems. Patch now works around it by filling current user when not filled in.
- getCreationStatus() was doing a lot of stuff it didn't need to do. We can exit with the first creation status you find (which is probably the first anyway), and no need to try and create a creation status every time.With this patch on worflow "dev-2.1.x bc9ef90" (I did a
composer require 'drupal/workflow:2.1.x-dev@dev
) all of my behats are green again. Not that 100% proof correctness, as I don't use comments and scheduled transactions e.g.This patch is also included in the next one. Please use the next one.
After debugging and patching, I asked an AI to have a look at the workflow module and suggest some easy very local optimizations. And it did. I reviewed the changes, they look very much ok.
I made a new patch of its changes + there's also a summary of the changes.
All my behats are still green with this patch (this patch also includes everything from #36).
So please evaluate and integrate.
I fixed a few other small things until all my projects using workflow had green behats.
This should be used instead of the previous ones in this thread. Base is still "dev-2.1.x bc9ef90".
So, for testing purposes:
composer require drupal/workflow:dev-2.1.x#bc9ef90
Let me know when you did all patches. Or you think you're finished, and I will let my behats run again.
- 🇳🇱Netherlands johnv
// Handle CLI contexts where current user ID is 0/null.
In CLI context, there is no user?, and userId is NULL (not 0) ?
- 🇳🇱Netherlands johnv
Now working on the $user/ $owner.
The $timestamp is left. There were no comments about that, only code. Was/is there an error in that? - 🇳🇱Netherlands johnv
I updated workflow_current_user() for anonymous users.
I do not know how to test CLI context, but I presume there is always a user, right? I may be a user with ID = 0.
I did not apply below chunks, since getOwner() should return an object for user 0, too.
And workflow_current_user() is now better.diff --git a/src/Entity/WorkflowTransition.php b/src/Entity/WorkflowTransition.php index 02b0bdd..83d78f3 100644 --- a/src/Entity/WorkflowTransition.php +++ b/src/Entity/WorkflowTransition.php @@ -18,6 +18,7 @@ use Drupal\Core\Session\AccountInterface; @@ -709,7 +712,8 @@ class WorkflowTransition extends ContentEntityBase implements WorkflowTransition @@ -1053,7 +1051,7 @@ class WorkflowTransition extends ContentEntityBase implements WorkflowTransition @@ -1447,4 +1445,21 @@ class WorkflowTransition extends ContentEntityBase implements WorkflowTransition
- 🇳🇱Netherlands johnv
Above commit handles the timestamp. I now understand the goal: simpler code.
I avoided the $formstate input vs. values discussion (which I always tend to struggle with), by re-using the result from valueCallback.I will create some follow-up on the timestamp, which is outside the scope of your patch.
- 🇳🇱Netherlands johnv
And the above concludes the processing of your input.
Please give me a day for some additional changes, then test again with 2.1.x dev version.
Thanks. Almost but not quite, 8 behats of the 225 fail:
there's 2 times "workflow_transion", which should be "workflow_transition". A typo.
2. there's a small bug in the key caching you transcribed.
Patch attached.
After this patch I can replace workflow v1.7 by the dev 2 version without application changes.
- 🇳🇱Netherlands johnv
Ow, 8 errors, that is not progress...
I reverted the code structure of WorkflowHistoryAccess::access(). The new code introduced a gap.
The misspelled 'workflow_transion' exists already for years, see #2927556: Correct typo on permission "access ... workflow_transion overview" (D9) → .
I think it exists since the start of the D8 version.
Are your tests failing on that?It is annoying, but changing that would need a hook_update_xxx().
Both are not typo's since the real typo is in file WorkflowPermissions.pgp:// D7->D8-Conversion of 'workflow history' permission on Workflow settings to "access $type_id overview" (@see NodePermissions::access content overview). "access own $type_id workflow_transion overview" => [ [...] ], "access any $type_id workflow_transion overview" => [ [...] ],
And that is being referred to.
I do not really know how to fix that without breaking existing installs. You're right, with either workflow_transion or workflow_transition my tests now pass (probably because those are running as administrator role).
I run custom tooling that tries to detect anomalies, and that came up as anomaly while fixing the array reference. The array reference is real.
Can we get a tagged version when it's +- stable. It's easier to explain to upgrade from v1.7 to v2.x.y, than to provide a commit tag to upgrade to.
Looking at https://git.drupalcode.org/project/workflow/-/commit/9dc7f7d12a1592a19ad7652123e6e072131c34c9 the last part of this one still needs to be included (it's partially in):
// Note: Field name may have been set, if empty initially. $field_name = $definition->getName(); - $cache_key = "{$uid}:{$entity_type}:{$entity_id}:{$field_name}"; - $access[$cache_key] = $access_result; + $new_cache_key = "{$uid}:{$entity_type}:{$entity_id}:{$field_name}"; + $access[$new_cache_key] = $access_result; } + + // Store the result for the original cache key as well + $access[$cache_key] = $access_result;
- 🇳🇱Netherlands johnv
I made the code more explicite to handle all cases:
- a field name is given yes or no
- an entity has 0, 1, multiple workflow fields.Please check.
- 🇳🇱Netherlands johnv
Wow, that was a long way!
Thanks a lot for your cooperation an patience! It's nice to be able to upgrade workflow from Drupal 10 to Drupal 11 without having to change application code.
I've always used workflow v1.7 on Drupal 10, since workflow v1.8 broke things for me.Now it looks like I can upgrade from v1.7 to the latest 2 version in dev without problems.
Looking at the changes that were made in v2, I was at first pessimistic a simple workflow upgrade would suffice.
Thank you.
- 🇳🇱Netherlands johnv
It all started with supporting 'baseFieldDefinitions' in the WorkflowTransition. But that infected the FormElement.
Like doing a simple change in your old house... - 🇳🇱Netherlands johnv
I committed the README file in 📌 Update README file Active .
Thanks.
I made a small change by referring to workflow.api.php file instead of copying the information.