GitLab

@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:

1. Update erp5.git
2. Start your instance.
3. Upgrade erp5_property_sheets, erp5_core and erp5_configurator bt5s.
4. Add an External Method for the module migrateWorkflowModuleToPortalWorkflow and the function of the same name as the module.
5. Run the External Method to convert them from workflow_module to portal_workflow.
6. 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 to portal_workflow. Is there any project using workflow_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 and current_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 to get*ValueByReference() instead (sames goes for get*IdList()) as they all take a reference.
• I have not fixed dumpWorkflowChain() for the new API as it is not used anywhere in erp5.git. Does any project actually use it? ➔ Fixed (used in projects).
• 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 have default_value (static value) and default_expr (TALES Expression) on a Workflow Variable, default_expr taking precedence over default_value, to get the default value of a Variable. With new Workflow implementation, we also have both (named respectively variable_default_value and variable_default_expression in Variable Property Sheet). It seems that variable_default_value is not used (not even in projects as confirmed by @romain and @jerome) so it could be dropped in favor of variable_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 as Workflow 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 Workflow Script into their respective Workflows. See the commented code in migrateToERP5Workflow(). ➔ Leave it as it is as Configurator are not working with the new UI and Configurator may be revamped entirely at that time (@romain).
  • 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 ⚠ (see BusinessTemplate_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).
• Deprecate WorkflowTool._listTypeInfo()? (This actually calls portal_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 Comment field really useful considering that we already have Description? ➔　Needed.
• bug: "Convert DC Workflow" on portal workflow ( WorkflowTool_viewWorkflowConversionDialog ) makes a getWorkflowTempObjectList() got an unexpected keyword argument 'ignore_layout' error when being used from xhtml style UI and is not enabled in ERP5JS.