WIP: Workflow are now "real" ERP5 objects (deprecating DCWorkflow)
@nexedi This migrates away from DCWorkflow to ERP5 "real" objects (called ERP5Workflow from here on), based on the work started by Wenjie/@romain and Isabelle/@seb a while ago. Everything can be found in product/ERP5Type/Core
. I have tried to deduplicate code between legacy implementation (product/ERP5Type/patches/
and product/ERP5/{InteractionWorkflow, Interaction}.py
) and new implementation in product/ERP5Type/Core/
as much as possible. As with everything else, Workflows are not defined on the Portal Type (rather than the ugly portal_workflow._chains_by_type
).
The basic idea is to migrate automatically at startup portal_workflow
itself from CMFCore.WorkflowTool to ERP5Type.Tool.WorkflowTool but not its Workflows. In other words, portal_workflow
can both contain DCWorkflow and ERP5Workflow workflows. product/ERP5Type/patches/DCWorkflow.py
monkey patch DCWorkflow classes to provide the same API as ERP5Workflow, allowing to use both at the same time. Also, Workflow Python Scripts should not require any change as ComputedAttribute
are provided for DCWorkflow attributes (such as scripts
, transitions
...).
Unit Tests are executed after all Workflows have been migrated to ERP5Workflows. Except Configurator-related Unit Tests (see below), most of them are passing. While this MR is under discussion, I will work on fixing the remaining tests failures.
I ran erp5_performance_test:testWorkflowPerformance to compare DCWorkflow and ERP5Workflow implementations and it seems to be about 4% slower with the new implementation: DCWorkflow: 7.547, 7.593, 7.618, 7.59, 7.514. ERP5Workflow: 7.842, 7.723, 7.902, 7.837, 7.875.
I have not ran any benchmark or performance tests: my idea being to first carefully discuss and define the API/PropertySheets with everybody and then merge it before looking into performances later on (otherwise it never gets merged). Here is a list of Documents and PropertySheets:
- portal_property_sheets/Guard
- portal_property_sheets/Interaction
- portal_property_sheets/State
- portal_property_sheets/Worklist
- portal_property_sheets/Transition
- portal_property_sheets/Workflow
- portal_property_sheets/WorkflowConfigurator
- product/ERP5Type/Core/Interaction.py
- product/ERP5Type/Core/InteractionWorkflow.py
- product/ERP5Type/Core/State.py
- product/ERP5Type/Core/Transition.py
- product/ERP5Type/Core/Workflow.py
- product/ERP5Type/Core/WorkflowScript.py
- product/ERP5Type/Core/WorkflowVariable.py
- product/ERP5Type/Core/Worklist.py
- product/ERP5Type/Core/WorklistVariable.py
- product/ERP5Type/Tool/WorkflowTool.py
- product/ERP5Type/mixin/expression.py
- product/ERP5Type/mixin/guardable.py
Migration of Configurator Workflows in workflow_module
With your current instance:
- Update erp5.git
- Start your instance.
- Upgrade
erp5_property_sheets
,erp5_core
anderp5_configurator
bt5s. - Add an External Method for the module
migrateWorkflowModuleToPortalWorkflow
and the function of the same name as the module. - Run the External Method to convert them from
workflow_module
toportal_workflow
. - Move all workflow_module from PathTemplateItem of your Business Template to Workflows and commit.
Questions/TODO:
-
Core/{State,Transition,Interaction}: Prefix with "Workflow"/"InteractionWorkflow" to avoid naming conflicts. -
erp5_configurator*
: ERP5Workflow was initially implemented for Configurator (workflow_module
). All of these Workflows have been migrated toportal_workflow
. Is there any project usingworkflow_module
or a need for backward compatibility here?-
ConfiguratorTransition.transition_form_id => action_name (Display in Action Box Name) or action (Display in Action Box URL).➔ Leave it as it is as Configurator are not working with the new UI and Configurator may be revamped entirely at that time (@romain). -
state_base_category? (see comment in BusinessConfiguration.initializeWorkflow())➔ state_base_category should be removed andcurrent_state
used by default as there seem to be no need for this to be configurable. -
Configurator-specific Workflow API (maybe more than these), not implemented because probably not needed for general-purpose Workflows (that's the reason why Configurator-related Unit Tests fail):workflow.initializeDocument()state.getAvailableTransitionList()state.executeTransition()state.getWorkflowHistory()-
state.undoTransition()➔ Moved to BusinessConfiguration Document.
-
-
get*ValueById()
: I didn't change the name from the initial work but I think it should rather be renamed toget*ValueByReference()
instead (sames goes forget*IdList()
) as they all take a reference. -
I have not fixed➔ Fixed (used in projects).dumpWorkflowChain()
for the new API as it is not used anywhere in erp5.git. Does any project actually use it? -
Migrate all Workflows in erp5.git to ERP5Workflow (currently they are only converted automatically before running Unit Tests but in the repository they are still DCWorkflow). -
Fix all TODO-BEFORE-MERGE and TODO-ERP5Workflow. -
Check that API has not changed before and after: method parameters names must not changed. -
With DCWorkflow, we havedefault_value
(static value) anddefault_expr
(TALES Expression) on a Workflow Variable,default_expr
taking precedence overdefault_value
, to get the default value of a Variable. With new Workflow implementation, we also have both (named respectivelyvariable_default_value
andvariable_default_expression
in Variable Property Sheet). It seems thatvariable_default_value
is not used (not even in projects as confirmed by @romain and @jerome) so it could be dropped in favor ofvariable_default_expression
. -
AddDeleteObject_resetDynamicClasses: Reset for Workflow and Interaction Workflow need to be done only when a Transition is added or deleted (only its ID matters, see
initializePortalTypeDynamicWorkflowMethods()
): No way to do that properly with current implementation?
container.newContent(id=XXX, **kw)
container._setObject(id, ob)
container._setOb(id, ob)
tree[id] = ob (Folder-implementation dependent)
ob._edit(**kw)
After the merge TODO list:
-
WorkflowTool.*{Of,For}()
methods: Move these methods to their appropriate classes (such asWorkflow
Document for those taking a Workflow ID). See !1378 (comment 129616). XXX Jérome: note sure was this mean. The linked discussion explain that the workflow id argument was generally not passed and workflow tool took care of finding the proper workflow. -
Configurator:-
Configurator Workflow uses {before,after}_script_id on Transitions to define before/after scripts to be called (in DCWorkflow, we had {before,after}_script_name and in new ERP5Workflow we have {before,after}_script which is a category pointing on Workflow Script). These scripts are currently in portal_skins and must be moved to➔ Leave it as it is as Configurator are not working with the new UI and Configurator may be revamped entirely at that time (@romain).Workflow Script
into their respective Workflows. See the commented code inmigrateToERP5Workflow()
. -
Configurator Workflow migrated from workflow_module to portal_workflow have very generic names, perhaps they should be renamed (for example: erp5_standard_workflow ➔ erp5_standard_configuration_workflow).
-
-
Consider removing
showAsXML()
if not really useful (AFAIK, it was implemented solely to compare old and new Workflows). -
Improve ERP5 Python/Workflow Scripts usability:
-
They are not checked in by coding style test
⚠ (seeBusinessTemplate_getPythonSourceCodeMessageList
) -
some roles are missing in items of
PythonScript_viewProxyRole/my_proxy_role_list
, we should define this with a TALES expression. - Syntax highlighting, diagnostics, etc does not work in workflow script editor (also check ERP5JS).
-
They are not checked in by coding style test
-
Deprecate
WorkflowTool._listTypeInfo()
? (This actually callsportal_types.listTypeInfo()
and only in legacy WorkflowTool). -
Related bug: "Update Security Roles" on workflow (
Workflow_updateSecurityRoles
) does not work. It is using this_listTypeInfo
from restricted python so it fails with unauthorized. This does not seem to be covered by test, there is still a TODO comment -
WorkflowTool
class inherits from CMF WorkflowTool (aka OriginalWorkflowTool). All methods of CMF WorkflowTool should be@deprecated
. XXX Jérome: why ? - Fix graph editor problems (see !1378 (comment 131221))
-
Is➔ Needed.Comment
field really useful considering that we already haveDescription
? -
bug: "Convert DC Workflow" on portal workflow (
WorkflowTool_viewWorkflowConversionDialog
) makes agetWorkflowTempObjectList() got an unexpected keyword argument 'ignore_layout'
error when being used from xhtml style UI and is not enabled in ERP5JS.