🇳🇱Netherlands @johnv

This is a very old issue.
In the mean time, it should have been working, with the attached issues.
At least, it is now recently tested with D11 in version 2.1.

OK, I will uninstall, donwgrade, then upgrade again,

Dou you have custom code?

I cannot reproduce.
Where do you have the Workflow Transition form (on node view, edit, block, history)
Did you run update.php?

You May need to rerun the most recent hook_update

I noticed the error in #7 already.
The field is now computed - it should not be saved.
Please try 2.1.1

Keeping this open, since not convinced this is solved.

Is your problem also/only in the Workflow History tab, as per 🐛 Workflow history has disappeared after upgrading from 8.x-1.7 to 2.0.0 Active , or in any Views view?

Hmm, I wonder why this is note reported more often.
Perhaps it is the same problem as 🐛 Upgrading from 1.7 to 1.8 Active .

At some point, the programmed History view was replaced by a View (with still a fallback to the programmed list.)
Do you have a customized History view?
Please check page admin/structure/views/view/workflow_entity_history

johnv → created an issue.

Let's link to this (because of its generic name): 🐛 Workflow ECA Compatibility Active

I am now working on 🐛 ECA error when importing model: There were errors validating the config synchronization Active

I did all that.
When trying to add a model, i get the following error:

There are no modellers available yet. Install at least one module that integrates a modeller. A list of available integrations can be found on the ECA project page.

What do I need to do?

I am trying to import a model, with problems for another config, to be tackled here: 🐛 ECA error when importing model: There were errors validating the config synchronization Active

This happens when the config was initially installed succesfully on the site, but was never removed. See related issue

There is a follow-up problem, when the config was installed successfully, initially.
It needs to be deleted explicitly (in the related issue)

johnv → created an issue.

In heed instructions how to "Try to add a model via BMPN and save it: nothing happens"

I need instructions how to create/import a process.

Same problem with
public function convert(ModelOwnerInterface $owner, ConfigEntityInterface $model, bool $readOnly = FALSE): array;

I updated the function declaration of both functions in bpmnio.php.
Update.php is now correct.

Also working with Workflow 2.1.0, but the mentioned classes were not touched, lately.

The $readOnly parameter is missing in the implemententation:

  public function edit(ModelOwnerInterface $owner, string $id, string $data, bool $isNew = FALSE, bool $readOnly = FALSE): array;

Downloading drupal/bpmn_io (3.0.0-alpha5)
Upgrading drupal/eca (dev-3.0.x b72d3ca => 2.1.8)
using modules
Machine name: bpmn_io Version: 3.0.0-alpha5
Machine name: eca (no version specified)
Machine name: eca_ui (no version specified)

Hmm,
when installing/updateing the modules, I get the following error:

Fatal error: Declaration of Drupal\bpmn_io\Plugin\modeler_api_modeler\BpmnIo::edit(Drupal\modeler_api\Plugin\modeler_api_model_owner\ModelOwnerInterface $owner, string $id, string $data, bool $isNew = false): array must be compatible with Drupal\modeler_api\Plugin\modeler_api_modeler\ModelerBase::edit(Drupal\modeler_api\Plugin\modeler_api_model_owner\ModelOwnerInterface $owner, string $id, string $data, bool $isNew = false, bool $readOnly = false): array in C:\damp\xampp\htdocs\drupal-11\drupal\modules\contrib\bpmn_io\src\Plugin\modeler_api_modeler\BpmnIo.php on line 109

Please try new version 2.1.0.

Please test new version 2.1.0
FTR, we had many constructive discussions in other, related, issues. Thanks for that.

Closing this as being outdated.
There are also some changed made to the 'Form display' page.

Please try the new 2.1.0 version and report back.

This is blocked by 📌 RenderElement and FormElement base plugin classes are deprecated in 10.3 and removed in 12.0 Postponed

Please test again with new version 2.1.0 and report your findings.

Please repeat with new version 2.1.0 and again give me the message including line numbers.

Please try new version 2.1.0

The problem introduced in v1.8 should now be fixed in the new version 2.1
The code is broken up in two parts, that are indeed called respectively in core's $entity->preSave() and $entity->postSave() functions, using the provided hook.

johnv → created an issue.

INdeed, I noticed. I reverted a change just now: #3529350-5: Modernize from Annotation to Attribute →

@sboden, the problem stems from attached issue 🐛 Revision id is NULL on node edit if revision enabled Fixed

johnv → created an issue.

This change has side effects, see related issues.

Indeed, the pre hook was moved/ removed in 🐛 Revision id is NULL on node edit if revision enabled Fixed

This is fixed in #3529071-2: Should WorkflowItem reference to ListItem, ListString or EntityReference? →

johnv → created an issue.

johnv → created an issue.

Hmm, not sure how to create a dev version for the 2.0 branch. It seems that this version is the one I am maintaining:
https://www.drupal.org/project/workflow/releases/8.x-1.x-dev →

Which version are you using? Please use dev version.

Never mind. I found the reason. Will be fixed in next commits.

I am stuck, having an Error message "This value should not be null." upon node creation.
You have no such experience?

I am working on D11.2 with latest Workflow version 2.0+dev, with WorkflowTransition::baseFieldDefinitions for from_sid and to_sid = 'list_string'.
(please run update. php)

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.

Closing this, as I guess this is fixed now.

johnv → created an issue.

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

johnv → created an issue.

johnv → created an issue.

The WorkflowOnComment still works. But it is possible to first create a WorkflowOnComment comment type, and then add this Comment type to a not-intended Content type. Therefore, '?->' was introduced in the code.
An error message is added for this case.

The patch is polluted with this usage of '?->' on many other places.

johnv → created an issue.

johnv → created an issue.

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.

The remaining topic is handled here: 🐛 Incorrect workflow_state_formatter['#label_display'] Active

The patch does not work for me.
At least the init() is needed to have a list of valid values.
There might be a dependency on 📌 WorkflowTransition references WorkflowStates as a reference_item, not a list_string Active . At the moment, the baseFieldDefinitions of states are defined as 'list_string', not 'entity_reference'.

Thisis fixed somewhere else.

This might still be an issue in 1.8, and perhaps in 2.0,
but it should be resolved in current dev version and upcoming v2.1.
The latest code has massaged away the difference between 'new entity' or 'existing entity with new field'.

Your analysis is correct, but the patch does not fit the analysis.
The committed change fixes the root cause - it sets a decent value for uid upon creating a new Transition.

Please test latest dev version and report back.

Attached patch defines the 'from_sid' and 'to_sid' state fields of WorkflowTransition as 'entity_reference', by subclassing that as 'workflow_state_reference'.

Conceptually, this seems better, but the work is still not done (Views display), and the impact is too big, adding many code, to implement this.

Above commit redefines the state fields in the WorkflowTransition back to 'list_string' .

johnv → created an issue.

This reverts the commit from #3432411-5: Add WorkflowTransition::uid as widget field using EntityOwnerTrait → .
Not sure why that commit would give another result for $user determination.

This should now be fixed. Please test latest dev and report back.

This is indeed a difficult problem.
The following commits are first cleaning the code, then a fix is added.
The result is that yesterday's season is still displayed. This is needed for e.g. nightclubs, where openingtimes are through midnight.
In the future, that season should not be displayed anymore of hours are not after midnight.
But for now, first remove the errors.

The second commit was needed because both 'short' and 'long' are valid options for 'time'/'weekday' and 'date'.
And Exceptions need both:
- weekday on widget,
- date on formatter.
So I needed to break this apart, and move the 'weekday' to Slot.php, since that class is bey definition in the widget, and the value is used per slot, not per day, as in the formatter.

Nice.
" It still shows midnight (even if there are hours for the exception) " Can you share a screenprint of this?

Indeed. Above patch should do the trick.

This should not be possible.
I do not understand how you have empty 'exceptions' settings.
But in any case, the 'defaultSettings' are there to prevent such cases.

Attached patch should fix the problem, not the symptom. The current code is too cautious.
Pleas test and report back.

johnv → created an issue.

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.

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?

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

johnv → created an issue.

Ah indeed, updating OP.

In my test case, both getName() and id() exist, but give differente results.
- edit node with office_hours.

What is your use case/ test? perhaps WebformOfficeHours?

Which drupal version are you using?
I am sure this is yested.

johnv → created an issue.

johnv → created an issue.

johnv → created an issue.

johnv → created an issue.

Hi rahulkhandelwal1990,
This issue is about anonymous users.
We still have an open issue for authenticated users 🐛 Status field caching still broken Active
Anw the two of us have an open discussionin ✨ Rework displaying dynamic information to JS for persistent caching Active
Let's discuss over there.