- Issue created by @spiderman
- 🇳🇱Netherlands johnv
Thanks you for your detailed report.
As youmay have noticed, 2.8 is not without error.
These days i Will try to fix the minimal use case, without access. - 🇳🇱Netherlands johnv
I see you have created some add-ons. Nice! Did not check them out, but I added them on the Workflow module project page → .
- 🇳🇱Netherlands johnv
Regarding Workflow Required by State → :
Since the Workflow field is a 'normal' as of D8 - only with a fancy widget and history -, it seems to ma that the same requirement can be fulfilled by the long-existing Conditional Fields module → - 🇳🇱Netherlands johnv
I guess there is a type in your test scritp:
- 4. Login with a Role1 user and create a node, immediately moving it to State2 ...
- 5. Login with a Role2 user and edit the node, attempt to move it to State2.
- 6. Save the node with State2 selected, ... but the State Transition hasn't happened (the Workflow tab on the node doesn't show any transition, and now entry is written to the database workflow_transition_history table).
- 7. Edit the node again, and observe that the Workflow field widget is still in State1.
- Move the Workflow to State2 again.
In above steps 5 and 6, you set the state from State2 to State2, without changing state nor adding a comment. In that case, indeed NO (now?) transition is written to the database, since there is no effective change. Adding a comment would force an entry in the history table.
In Step 7, you state that the entity is 'still in State1',Is there a typo in the text? Please correct in the summary.
Note: No transition is written to the database when changing to the same state, and no comment is given.
Note2: the latest version has some changes in defining the user. User must be current user, not the owner/author of the entity.But please observe 🐛 Version 1.8 is broken Active and rebase on today's new dev-version, that should have fixed the behaviour.
- 🇨🇦Canada spiderman Halifax, NS
@johnv Surprisingly, Workflow 1.8 seems relatively stable for us in the context of the project I'm working on. We are using the patch from 🐛 hook_update_8003 to 8.x-1.8 installs optional config and breaks my site Active , and haven't fully upgraded Drupal core (still on 10.3.2 atm), but aside from this odd behaviour it's done what we need from it.
That said, I spent a few hours yesterday trying to recreate the minimal use case in an isolated DDEV environment. I installed a fresh (10.4.3) Drupal site, with just Workflow module itself, and configured a node type, workflow, states, transitions, and roles as described in the issue summary. Unfortunately I can't seem to recreate the exact behaviour I'm seeing in our project, because I can't even submit the node on the second transition. It goes like this:
- Role1 user creates node, and puts it directly into State2
- Role2 edits node, tries to put it into State3
- On submitting the form, it POSTs, but the form validation fails and comes back with an error "This value should not be null."
As far as I can tell, this is due to the Workflow field's value not being picked up properly somewhere along the way. I haven't found the exact spot, but tracing it in my debugger it appears that by the time it gets to the core ValidReferenceConstraintValidator for the workflow field, it doesn't seem to have any value (or entity?) and thus concludes it's invalid and throws "This value should not be null."
I'm going to take another look and see if I can unlock this to get closer to the original behaviour, as there's a lot of config settings involved. I'm also going to try downgrading to Drupal 10.3.2 in my test environment, to see if that makes any difference. However, it seems increasingly possible this bug is actually within Workflow itself, so I'll continue to detail my efforts to get to the root of it, and maybe that'll dovetail with your efforts to clean things up on the dev branch.
I will also note that I tried applying the patch from 🐛 Warning : Undefined array key "to_sid" in WorkflowTransitionElement::copyFormValuesToTransition() line 356 when using Ajax button from other field Fixed and couldn't get one to apply, and when I updated to the -dev release, the site broke entirely. The fact that we don't seem to need this patch in our project gives me pause, but it looked potentially related so figured it was worth trying, esp since I'm working on slightly different core versions atm. I also notice the follow-up 🐛 TypeError: 'WidgetBase::errorElement(): Argument #1 ($element) must be of type array, null given, called in WidgetBase.php' Active but haven't tried applying that patch as well. Any guidance on which combination of things is likely to work best is greatly appreciated :)
- 🇨🇦Canada spiderman Halifax, NS
@johnv I missed your intervening comments earlier, as I hadn't fully reloaded this page nor seen them in my email yet! Thanks for reading my steps carefully, you are correct there was a typo, which I've fixed in the IS now. There is definitely a state change, so a transition should be created. That said, I'll try pulling the dev branch into my test environment again today, and see where that leaves me.
Also, thanks for adding our add-on Workflow modules to the project page! These both came out of the current project I'm working on, to meet requirements we couldn't satisfy otherwise. Indeed, we are already using Conditional Fields, but couldn't get it to work exactly as desired, in part because our node forms use IEF to include nested entities, and CF can't target fields across entities. We also didn't want the JS behaviour of making the field required immediately upon selecting the new state, but only when the node had been submitted in the new state. Your question makes me think we should explicitly distinguish Workflow Required by State from CF on it's project page, though :)
- 🇳🇱Netherlands johnv
Aside from your initial problems, 'Workflow Required by State' does not seem to be related to Workflow. It could work alone, too.
Indeed, if you do not want to act on the field changes on the edit page, CF does not serve your purpose. It might be a featur on CF, (you do know the undocumented $entity->original variable? ) - 🇨🇦Canada spiderman Halifax, NS
Hi @johnv- I'm guessing you meant you think this problem is solved on the latest dev- branch, but unfortunately I'm not seeing any improvement. As of this morning, with a fresh install and very minimal configuration (my test environment described above), I get the following error when simply going to the `/node/add` page for a node with a Workflow attached:
The website encountered an unexpected error. Try again later. Error: Call to a member function getTransition() on null in Drupal\workflow\Entity\WorkflowManager::executeTransitionsOfEntity() (line 159 of modules/contrib/workflow/src/Entity/WorkflowManager.php). _workflow_execute_transitions() (Line: 22) workflow_entity_insert() call_user_func_array() (Line: 416) Drupal\Core\Extension\ModuleHandler->Drupal\Core\Extension\{closure}() (Line: 395) Drupal\Core\Extension\ModuleHandler->invokeAllWith() (Line: 415) Drupal\Core\Extension\ModuleHandler->invokeAll() (Line: 217) Drupal\Core\Entity\EntityStorageBase->invokeHook() (Line: 900) Drupal\Core\Entity\ContentEntityStorageBase->invokeHook() (Line: 564) Drupal\Core\Entity\EntityStorageBase->doPostSave() (Line: 781) Drupal\Core\Entity\ContentEntityStorageBase->doPostSave() (Line: 489) Drupal\Core\Entity\EntityStorageBase->save() (Line: 806) Drupal\Core\Entity\Sql\SqlContentEntityStorage->save() (Line: 354) Drupal\Core\Entity\EntityBase->save() (Line: 277) Drupal\node\NodeForm->save() call_user_func_array() (Line: 129) Drupal\Core\Form\FormSubmitter->executeSubmitHandlers() (Line: 67) Drupal\Core\Form\FormSubmitter->doSubmitForm() (Line: 597) Drupal\Core\Form\FormBuilder->processForm() (Line: 326) Drupal\Core\Form\FormBuilder->buildForm() (Line: 73) Drupal\Core\Controller\FormController->getContentResult() call_user_func_array() (Line: 123) Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}() (Line: 638) Drupal\Core\Render\Renderer->executeInRenderContext() (Line: 121) Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->wrapControllerExecutionInRenderContext() (Line: 97) Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}() (Line: 181) Symfony\Component\HttpKernel\HttpKernel->handleRaw() (Line: 76) Symfony\Component\HttpKernel\HttpKernel->handle() (Line: 53) Drupal\Core\StackMiddleware\Session->handle() (Line: 48) Drupal\Core\StackMiddleware\KernelPreHandle->handle() (Line: 28) Drupal\Core\StackMiddleware\ContentLength->handle() (Line: 32) Drupal\big_pipe\StackMiddleware\ContentLength->handle() (Line: 116) Drupal\page_cache\StackMiddleware\PageCache->pass() (Line: 90) Drupal\page_cache\StackMiddleware\PageCache->handle() (Line: 48) Drupal\Core\StackMiddleware\ReverseProxyMiddleware->handle() (Line: 51) Drupal\Core\StackMiddleware\NegotiationMiddleware->handle() (Line: 36) Drupal\Core\StackMiddleware\AjaxPageState->handle() (Line: 51) Drupal\Core\StackMiddleware\StackedHttpKernel->handle() (Line: 741) Drupal\Core\DrupalKernel->handle() (Line: 19)
- 🇳🇱Netherlands johnv
Please download latest dev. There must be no errors in Workflow now. A lot has been done in the last 2 weeks.
If the error still persists, I will check the problem again. - 🇨🇦Canada spiderman Halifax, NS
@johnv Thanks for all your efforts on this module lately, I see a ton of commits going on. I've just run through my test environment setup again, using the latest dev branch. I'm tentatively going to report that the key problem of the transition not being created is no longer happening (and the other blocking issues I was seeing even creating a node with workflow have gone away). I plan to investigate a little more thoroughly and bring this code back into my main project for further testing, but this definitely looks like progress!
With that being said, I do notice some issues when first creating a node now:
- The radio button options list has a spurious "Select an option" radio that shouldn't be there (and if I do select it, I get WSOD when saving)
- Selecting any actual state and saving results in the error "You tried to set a Workflow State, but the entity is not relevant. Please contact your system administrator. ", except that everything appears to have happened correctly.
- The Workflow tab doesn't show the (creation) -> State1 transition anymore (this may be related to the second one, and perhaps the first workflow transition isn't being created?)
- First commit to issue fork.
- 🇨🇦Canada spiderman Halifax, NS
@johnv: I am still trying to narrow this down, and actively tracking the dev- branch and all the various issues you are working on (again, thanks!)
I was able to successfully bring the latest code into my main project, but it did require a number of small fixes in a number of places. Most of them were external, and I'm tracking them as patches to other modules to adapt to the Workflow updates toward a 1.9 release. The others I will report in this queue, just trying to see if they already exist or if they are new.
I will note here that the biggest problem I've seen so far with the newest code is this erroneous "You tried to set a Workflow State, but the entity is not relevant. Please contact your system administrator. " error message that shows up when creating a node, and my impression is that it may be the result of your efforts over on 🐛 New transition is not properly created in 8.x-1.8 Active recently. I'm not certain, but connecting these dots in your mind may be helpful? :)
Final note: I'm *also* seeing the extraneous "select a value" option in my *radiobuttons* rendering of the widget, as mentioned over in this comment 🐛 TypeError : count(): Argument #1 ($value) must be of type Countable|array, null given dans _workflow_transition_form_get_action_buttons() Active on 🐛 TypeError : count(): Argument #1 ($value) must be of type Countable|array, null given dans _workflow_transition_form_get_action_buttons() Active . My cursory search to find where that option was coming from turned up nothing, but I can see a lot of TODOs in the code I did find that relates to building these options, so possibly this is just WIP :)
- 🇳🇱Netherlands johnv
I have now finished my work on Workflow module.
Please rebase and run update.php. That will remove the 'Select a value' problem, since the field will be set 'required', leveraging 'BasefieldDefinition'.
The original problem should not happen as well. - 🇳🇱Netherlands johnv
I am now looking at "You tried to set a Workflow State, but the entity is not relevant. Please contact your system administrator. ".
When you save the form a second time, the correct value is saved. - 🇳🇱Netherlands johnv
This is now solved, after adding a test for isNew().
Thanks for your patience. - 🇨🇦Canada spiderman Halifax, NS
@johnv: Amazing! Thanks for the update, I'll look at this again today or tomorrow, and report back.
I have a question for you as well. It looks like you have been working toward a 1.9 release with this recent spike of effort. Since you indicated you are finished your work on this module, do you expect to cut a new release soon, or is there more to be done? Ideally I'd incorporate a new release into my client project, rather than tracking the dev- branch, but mostly it would just be helpful to know what you're planning :)
- 🇳🇱Netherlands johnv
Normally, i wait 10 days after finishing a job. Mostly, some early adopters have comments.
So, yes, as zoon as you give me ok, i Will create a new release.I am very happy with your patience, feedback and above all the coincidence that we work in the same weeks.
- 🇨🇦Canada spiderman Halifax, NS
@johnv: Please don't misunderstand, I am not trying to rush the process, I think giving a bit of space for recent commits to land makes good sense! I was mainly trying to understand your thinking and validate my sense that you were moving toward a new release soon-ish :)
I am also very happy to be a bit of a squeaky wheel at just the moment when you had attention to give. Thanks again for your efforts on this!
I have been able to *mostly* validate that the latest dev-branch code seems to resolve the core problem I was seeing, where a state transition wouldn't be recognized the first time. I have been working on adapting our custom code and related modules (see 📌 Adapt to upcoming 1.9 release of Workflow Active , 📌 Adapt to upcoming 1.9 release of Workflow Active , and 📌 Adapt to upcoming 1.9 release of Workflow Active ) to integrate into my client project, and have *almost* got our tests passing once again.
Unfortunately, there is one key problem remaining. I'm not certain it's exactly related to this issue, so let me know if you want me to create a separate ticket for it. What I'm seeing (this is back in my "clean" environment now), is a change in the behaviour when creating a new node. Previously the radiobuttons on the Workflow "change state" widget would have the "first state" pre-selected. This meant that our test would simply hit "Save" without explicitly choosing an option, and everything was fine.
As of today, what I see instead is a) the radiobuttons have no option selected initially, and if I simply hit Save without selecting one, I get a 500 error, with the following message:
The website encountered an unexpected error. Try again later. TypeError: Drupal\Core\Field\WidgetBase::errorElement(): Argument #1 ($element) must be of type array, null given, called in /var/www/html/web/core/lib/Drupal/Core/Field/WidgetBase.php on line 580 in Drupal\Core\Field\WidgetBase->errorElement() (line 646 of core/lib/Drupal/Core/Field/WidgetBase.php). Drupal\Core\Field\WidgetBase->flagErrors() (Line: 277) Drupal\Core\Entity\Entity\EntityFormDisplay->flagWidgetsErrorsFromViolations() (Line: 268) Drupal\Core\Entity\ContentEntityForm->flagViolations() (Line: 214) Drupal\Core\Entity\ContentEntityForm->validateForm() call_user_func_array() (Line: 82) Drupal\Core\Form\FormValidator->executeValidateHandlers() (Line: 274) Drupal\Core\Form\FormValidator->doValidateForm() (Line: 118) Drupal\Core\Form\FormValidator->validateForm() (Line: 593) Drupal\Core\Form\FormBuilder->processForm() (Line: 326) Drupal\Core\Form\FormBuilder->buildForm() (Line: 73) Drupal\Core\Controller\FormController->getContentResult() call_user_func_array() (Line: 123) ... snip ...
So far it seems that if I select one of the options, everything proceeds normally. I would prefer the original behaviour be restored here, as I notice that it also seems fragile in conjunction with our Workflow Save as Draft module, because the form_state doesn't seem to retain the value when the form page reloads due to validation errors. Let me know what you think!
- 🇳🇱Netherlands johnv
Thanks for your answer.
Regarding the error, it seems it is another use case of the issue that was created just yesterday: 🐛 Drupal\Core\Field\WidgetBase::errorElement() Active
Please add your comments over there.
- 🇨🇦Canada spiderman Halifax, NS
Coming back to this today after seeing 🐛 Drupal\Core\Field\WidgetBase::errorElement() Active appears to be resolved, I can confirm that testing in my clean environment, the behaviour I described originally doesn't happen anymore. I can create a node with one user, put it directly into the second state (see that transition get created correctly!), and then login with a second user, and push it forward to the second state (without saving twice!)
I'm also testing this in the context of the larger project I came from, and so far everything looks good. I'll report back here if anything goes awry, but for now I'm going to set this RTBC, and look forward to the upcoming 1.9 release!