Extra Workflow fields don't work as of v1.8

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

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;
   }

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?

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 the target_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 the workflow_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:
- Include workflow_entity_presave. In that case, most people can upgrade from v1.7 to v2.0 without many issues.
- Don’t include workflow_entity_presave. Then you have two options:
1. Patch workflow yourself to add workflow_entity_presave. But this might break in the future if workflow changes again.
2. Change your application to explicitly save the target_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 without workflow_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

"Re-rolled" patches.

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.

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();