- Issue created by @sboden
I threw the problem an at A.I. and it wrote a patch for the "PHP 8.2 problem".
Patch attached, I'm still testing it but my behats stayed green.
- 🇳🇱Netherlands johnv
Hmm,
you are using an outdated version.
I am about to release v1.9.
v1.8 is crippled (as you can see from some issues).Please update to latest dev and report back. I will wait for your response, and then create a new -working- version
v1.8 was working with patches. I have a couple of projects that use workflow, and they have behats.
So what I did is:
composer require 'drupal/workflow:1.x-dev'
And then I reran the behats, and everything breaks. This is one of the errors I get
Error: Call to undefined method Drupal\workflow\Entity\WorkflowManager::getCurrentStateId() in Drupal\project_extras\Plugin\Field\FieldFormatter\WorkflowStateLabel-> viewElements() (line 132 of /var/www/html/www/modules/custom/project/src/Plugin/Field/FieldFormatter/WorkflowStateLabel.php).
I can try on some other projects as well.
The WorkflowStateLabel looks like this:
/** * Provides a default workflow formatter. * * @FieldFormatter( * id = "workflow_state_label", * module = "workflow", * label = @Translation("Workflow state label"), * field_types = { * "workflow" * }, * quickedit = { * "editor" = "disabled" * } * ) */ class WorkflowStateLabel extends FormatterBase implements ContainerFactoryPluginInterface { ... /** * {@inheritdoc} * * N.B. A large part of this function is taken from CommentDefaultFormatter. */ public function viewElements(FieldItemListInterface $items, $langcode) { $output = []; $field_name = $this->fieldDefinition->getName(); $workflow_field = $items->getName(); if ($entity = $items->getEntity()) { if ($entity->$workflow_field->value) { $current_sid = WorkflowManager::getCurrentStateId($entity, $field_name); /* @var $current_state WorkflowState */ $selected = workflow_state_formatter($entity, $field_name, $current_sid); return $selected; } } $selected = t('(Creation)'); $output[] = ['#markup' => $selected]; return $output; } ...
- 🇳🇱Netherlands johnv
Indeed,
several interfaces have changed in v1.8 (which should not be used) and upcoming v1.9.
This is due to the implemenation of the baseFieldDefinitions of Workflow(Config)Transition.
I did not anticipate uon usage of function like above by third party modules.(It seems you code can be replaced by workflow_state_allowed_values() )
But then it's not a v1.9 but v2.0. The previous example was from a huge Drupal app that built stuff on top of workflow.
I have about 500 behat tests in total that normally are green.Some other things that pop up:
Warning: Undefined property: Drupal\workflow\Entity\WorkflowState::$option in Drupal\views\Plugin\views\filter\InOperator->reduceValueOptions() (regel 276 van /var/www/html/www/core/modules/views/src/Plugin/views/filter/InOperator.php) #0 /var/www/html/www/core/includes/bootstrap.inc(166): _drupal_error_handler_real(2, 'Undefined prope...', '/var/www/html/w...', 276) #1 /var/www/html/www/core/modules/views/src/Plugin/views/filter/InOperator.php(276): _drupal_error_handler(2, 'Undefined prope...', '/var/www/html/w...', 276) #2 /var/www/html/www/core/modules/views/src/Plugin/views/filter/InOperator.php(203): Drupal\views\Plugin\views\filter\InOperator->reduceValueOptions() #3 /var/www/html/www/core/modules/views/src/Plugin/views/filter/ManyToOne.php(112): Drupal\views\Plugin\views\filter\InOperator->valueForm(Array, Object(Drupal\Core\Form\FormState)) #4 /var/www/html/www/core/modules/views/src/Plugin/views/filter/FilterPluginBase.php(1074): Drupal\views\Plugin\views\filter\ManyToOne->valueForm(Array, Object(Drupal\Core\Form\FormState)) #5 /var/www/html/www/core/modules/views/src/Form/ViewsExposedForm.php(105): Drupal\views\Plugin\views\filter\FilterPluginBase->buildExposedForm(Array, Object(Drupal\Core\Form\FormState)) #6 [internal function]: Drupal\views\Form\ViewsExposedForm->buildForm(Array, Object(Drupal\Core\Form\FormState)) #7 /var/www/html/www/core/lib/Drupal/Core/Form/FormBuilder.php(536): call_user_func_array(Array, Array) #8 /var/www/html/www/core/lib/Drupal/Core/Form/FormBuilder.php(284): Drupal\Core\Form\FormBuilder->retrieveForm('views_exposed_f...', Object(Drupal\Core\Form\FormState)) #9 /var/www/html/www/core/modules/views/src/Plugin/views/exposed_form/ExposedFormPluginBase.php(134): Drupal\Core\Form\FormBuilder->buildForm('\\Drupal\\views\\F...', Object(Drupal\Core\Form\FormState)) #10 /var/www/html/www/core/modules/views/src/ViewExecutable.php(1298): Drupal\views\Plugin\views\exposed_form\ExposedFormPluginBase->renderExposedForm() #11 /var/www/html/www/core/modules/views/src/Plugin/views/display/PathPluginBase.php(393): Drupal\views\ViewExecutable->build() #12 /var/www/html/www/core/modules/views/src/Plugin/views/display/Page.php(198): Drupal\views\Plugin\views\display\PathPluginBase->execute() #13 /var/www/html/www/core/modules/views/src/ViewExecutable.php(1690): Drupal\views\Plugin\views\display\Page->execute() #14 /var/www/html/www/core/modules/views/src/Element/View.php(81): Drupal\views\ViewExecutable->executeDisplay('page_1', Array) #15 [internal function]: Drupal\views\Element\View::preRenderViewElement(Array)
and
Error: Call to a member function getTransition() on null in Drupal\workflow\Hook\WorkflowEntityHooks->executeTransitionsOfEntity() (regel 367 van /var/www/html/www/modules/contrib/workflow/src/Hook/WorkflowEntityHooks.php).
These are from a smaller project that uses workflow, but doesn't do anything special with it.
I'm not fully convinced about the 1.9 development version yet.
I spent 3 hours upgrading one project from workflow v1.8 to v1.9, and I'm about 80% done.
Here are the extra issues I ran into:
- The function workflow_get_workflow_state_names() is missing in v1.9.
- The required parameters for WorkflowTransition::create() have changed a lot. e.g. without a wid value, no transition is created.
- In v1.8, you could use workflow on an entity before saving it. In v1.9, the entity must be saved first. If your code relies on the old behavior, you’ll need to adjust your logic.
- In v1.9, you must set an initial workflow status when creating a node. This wasn’t required in v1.8.
- Some interfaces are missing.
- For some reason, I now sometimes get the workflow status as a system name instead of a label/value. When I switch back to v1.8, this problem goes away.If you plan to release this version, I think it should be called v2.0 instead of v1.9. When moving from v1.8 to v1.9, I expect everything to keep working, with some extra features. That’s not the case here. In the test project alone, I'm already at 82 code changes, and I expect to pass 100.
Personally, I would roll back to v1.8 and improve it, but keep the existing interfaces the same. Else you're going to create a lot of work for a lot of people.
I did a complete debug session until the end of one project. I now get my behat tests to green status with both workflow v1.8 and the current dev version, with some minor changes in the workflow dev version. Patch attached, please review and add to the code.
I would still release it as v2.0.0, I did have to make some small changes in my project code: workflow version 1.8 had no problem with yet unsaved objects, but the new version apparently has. Saving objects upfront makes sense but does require a change. So it's not a minor workflow version: you can't just update workflow.
I can do the migration for another big Drupal project, but only beginning of next week.
- 🇳🇱Netherlands johnv
I have added your proposed changes from #8.
Indeed, there are multiple changes in the code and interfaces.
Version 1.8 was an attempt to change the code to use more core code, i.c. using baseField definitions.
There are reports that this is crippled, also when using hooks.
And some changes (e.g., WorkflowManager functions moved to WorkflowTargetEntity) are indeed just moving code, for better DX in the long term.
I will need to take a look at your 3rd paragraph in #8 separately.
When creating a WorkflowTransition, it should still not be needed to have a to-state. In the past, the to-state could not be defined, in the new version, the to-state is defined (either first state when entity->isNew(), or else same state). Then, It can be overwritten by the widget.
Your change in the$this->entity_id
might make a difference, since new Nodes have no ID, yet. - 🇳🇱Netherlands johnv
In #4, you should be able to use the standard 'Value' formatter, instead.
Indeed, I do see that the 'creation' state returns a machine_name, not a description. Not sure why. - 🇳🇱Netherlands johnv
IN #7:
- The function workflow_get_workflow_state_names() is missing in v1.9.
--> I can add this back
- The required parameters for WorkflowTransition::create() have changed a lot. e.g. without a wid value, no transition is created.
--> That should not be the case. I added the 'entity' as an input paramater, since that holds the $sid and $wid. Also $state holds $wid.
- In v1.8, you could use workflow on an entity before saving it. In v1.9, the entity must be saved first. If your code relies on the old behavior, you’ll need to adjust your logic.
--> Ofcourse, my test situation may be less complicated that yours.
- In v1.9, you must set an initial workflow status when creating a node. This wasn’t required in v1.8.
--> Before 1.8 upon node create, the widget has an initial value, being the 'first' state. it was removed shortly in dev, but not in 1.8. In current version, the 'first' state is set automatically. your contrib code should not be worrying about that.
- Some interfaces are missing.
--> Indeed, I introduced WorkflowTargetEntity and WorkflowRole, at some point, Roel and Entity should be subclassed/decorated.
- For some reason, I now sometimes get the workflow status as a system name instead of a label/value. When I switch back to v1.8, this problem goes away.
--> Is this only on the creation formatter? or also in other situations. I should have fixed this 2 weeks ago. - 🇳🇱Netherlands johnv
Regarding "- In v1.8, you could use workflow on an entity before saving it. In v1.9, the entity must be saved first."
I did notice this in the last week, and removed the code (thinking it was superfluous, and not the task of a transition to save the node), but did not realize it was a feature, too.
It could be reverted. For better backwards compatibility, it would be best to revert then.
I don't use it in the apps I wrote myself (same idea: it's not workflow responsibility to save entities), but some of the projects I inherited use that functionality.