- 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