RFC: ERP5Workflow: Existing Workflow PythonScript should not require any change.

* Add WORKFLOW.{scripts,transitions...} property which is just a ComputedAttribute
  returning a dict (TODO: Instead of a dict, this should be ContainerTab to have
  objectIds()...).

* Allow a Workflow Script to call another one by overriding portal_workflow.__getattr__.
  With DCWorkflow, `container` was bound to its parent (WORKFLOW.scripts which is a
  mapping), but now `container` is bound to the WORKFLOW itself as Transitions, Scripts
  and Variables are all at the same level. This is not very efficient but this is only
  in DCWorkflow compatibility mode after all.

  The initial implementation was creating a `ScriptContext`, a temporary object with
  all scripts of the current Workflow added. However, this required changing existing
  Workflow Script code and especially this did not work with the following use case:
  1. Script Context is created before calling a Workflow Script.
  2. That script calls a Workflow Script from another Workflow.
     => This will fail as ScriptContext is only created for the Workflow.

This commit is going to be squashed before merging to master.

bt5/erp5_accounting/WorkflowTemplateItem/portal_workflow/accounting_workflow/scripts/validateTransactionLines.py

@@ -13,8 +13,7 @@ section_portal_type_list = ['Person', 'Organisation']
 invalid_state_list = ['invalidated', 'deleted']
 
 # first of all, validate the transaction itself
-script_id = container.getScriptIdByReference('validateTransaction')
-container.getScriptValueById(script_id)(state_change)
+container.validateTransaction(state_change)
 
 
 # Check that all lines uses open accounts, and doesn't use invalid third

bt5/erp5_accounting/WorkflowTemplateItem/portal_workflow/internal_invoice_transaction_simulation_workflow/scripts/guessGroupingReference.py

-return sci.getPortal().portal_workflow.accounting_workflow.getScriptValueById(script.getId())(sci)
+return sci.getPortal().portal_workflow.accounting_workflow.scripts[script.getId()](sci)

bt5/erp5_accounting/WorkflowTemplateItem/portal_workflow/internal_invoice_transaction_simulation_workflow/scripts/resetGroupingReference.py

-return sci.getPortal().portal_workflow.accounting_workflow.getScriptValueById(script.getId())(sci)
+return sci.getPortal().portal_workflow.accounting_workflow.scripts[script.getId()](sci)

bt5/erp5_accounting/WorkflowTemplateItem/portal_workflow/internal_invoice_transaction_simulation_workflow/scripts/setReferences.py

-return state_change.getPortal().portal_workflow.accounting_workflow.getScriptValueById(script.getId())(state_change)
+return state_change.getPortal().portal_workflow.accounting_workflow.scripts[script.getId()](state_change)

bt5/erp5_accounting/WorkflowTemplateItem/portal_workflow/internal_invoice_transaction_simulation_workflow/scripts/validateTransaction.py

-return state_change.getPortal().portal_workflow.accounting_workflow.getScriptValueById(script.getId())(state_change)
+return state_change.getPortal().portal_workflow.accounting_workflow.scripts[script.getId()](state_change)

bt5/erp5_accounting/WorkflowTemplateItem/portal_workflow/internal_invoice_transaction_simulation_workflow/scripts/validateTransactionLines.py

@@ -7,5 +7,4 @@ if old_state.getId() == 'draft':
   if internal_invoice.InternalInvoiceTransaction_getAuthenticatedUserSection() == internal_invoice.getDestinationSection():
     raise ValidationFailed(translateString("Your entity should not be destination."))
 
-script = state_change.getPortal().portal_workflow.accounting_workflow.getScriptValueById(script.getId())
-return script(state_change)
+return state_change.getPortal().portal_workflow.accounting_workflow.scripts[script.getId()](state_change)

product/ERP5/bootstrap/erp5_core/SkinTemplateItem/portal_skins/erp5_core/Module_listWorkflowTransitionItemList.py

@@ -21,38 +21,37 @@ checked_workflow_id_dict = {}
 for document_portal_type_id in module_portal_type.getTypeAllowedContentTypeList():
   if (filter_portal_type_list is not None) and (document_portal_type_id not in filter_portal_type_list):
     continue
-  for workflow in workflow_tool.getWorkflowValueListFor(document_portal_type_id):
-    workflow_id = workflow.getId()
-    if workflow_id not in checked_workflow_id_dict:
+  for workflow in workflow_tool.getWorkflowsFor(document_portal_type_id):
+    if workflow.id not in checked_workflow_id_dict:
       # Do not check the same workflow twice
-      checked_workflow_id_dict[workflow_id] = None
+      checked_workflow_id_dict[workflow.id] = None
 
-      state_variable = workflow.getStateVariable()
+      state_variable = workflow.state_var
 
       allowed_state_dict = {}
 
-      state_value_list = workflow.getStateValueList()
-      if state_value_list:
-        for state in state_value_list:
-          state_reference = state.getReference()
-          for possible_transition in state.getDestinationValueList():
-            if possible_transition in allowed_state_dict:
-              allowed_state_dict[possible_transition].append(state_reference)
+      if getattr(workflow, 'states', None) is not None:
+        for state_id, state in workflow.states.items():
+          for possible_transition_id in state.transitions:
+            if possible_transition_id in allowed_state_dict:
+              allowed_state_dict[possible_transition_id].append(state_id)
             else:
-              allowed_state_dict[possible_transition] = [state_reference]
-        for transition in allowed_state_dict:
+              allowed_state_dict[possible_transition_id] = [state_id]
+        for transition_id in allowed_state_dict:
+          transition = workflow.transitions.get(transition_id, None)
+          if transition is None:
+            continue
           # Only display user action transition with a dialog to show to user
-          if transition.getTriggerType() == TRIGGER_USER_ACTION and transition.getAction() and transition.getActionName():
-            transition_reference = transition.getReference()
-            action_form_id = transition.getAction().rsplit('/', 1)[1].split('?')[0]
+          if (transition.trigger_type == TRIGGER_USER_ACTION) and (transition.actbox_url) and (transition.actbox_name):
+            action_form_id = transition.actbox_url.rsplit('/', 1)[1].split('?')[0]
 
-            result['transition_item_list'].append((translate(transition.getActionName()), transition_reference))
-            result['form_id_dict'][transition_reference] = action_form_id
+            result['transition_item_list'].append((translate(transition.actbox_name), transition_id))
+            result['form_id_dict'][transition_id] = action_form_id
             # XXX portal_type parameter must also probably be added too
             # This would required to detect identical transition id for different workflow
-            result['listbox_parameter_dict'][transition_reference] = [(state_variable, allowed_state_dict[transition])]
-          elif transition.getTriggerType() == TRIGGER_USER_ACTION and transition_reference == 'delete_action':
-            result['listbox_parameter_dict'][transition_reference] = [(state_variable, allowed_state_dict[transition])]
+            result['listbox_parameter_dict'][transition_id] = [(state_variable, allowed_state_dict[transition_id])]
+          elif (transition.trigger_type == TRIGGER_USER_ACTION) and (transition_id == 'delete_action'):
+            result['listbox_parameter_dict'][transition_id] = [(state_variable, allowed_state_dict[transition_id])]
 
 result['transition_item_list'].sort()
 result['transition_item_list'].insert(0, ('', ''))

product/ERP5Type/Core/InteractionWorkflow.py

@@ -221,10 +221,7 @@ class InteractionWorkflow(Workflow):
       former_status = self._getStatusOf(ob)
       # Pass lots of info to the script in a single parameter.
       sci = StateChangeInfo(ob, self, former_status, tdef, None, None, kwargs=kw)
-      script_value_list = tdef.getBeforeScriptValueList()
-      if script_value_list:
-        script_context = self._asScriptContext()
-      for script in script_value_list:
+      for script in tdef.getBeforeScriptValueList():
         script(sci)  # May throw an exception.
     return filtered_transition_list
 
@@ -288,10 +285,7 @@ class InteractionWorkflow(Workflow):
             ob, self, former_status, tdef, None, None, kwargs=kw)
 
       # Execute the "after" script.
-      after_script_value_list = tdef.getAfterScriptValueList()
-      if after_script_value_list:
-        script_context = self._asScriptContext()
-      for script in after_script_value_list:
+      for script in tdef.getAfterScriptValueList():
         script(sci) # May throw an exception.
 
       # Queue the "Before Commit" scripts
@@ -323,9 +317,7 @@ class InteractionWorkflow(Workflow):
         setSecurityManager(current_security_manager)
 
   security.declarePrivate('activeScript')
-  def activeScript(self, script_name, ob_url, former_status, tdef_id,
-                   script_context=None):
-    script_context = self._asScriptContext()
+  def activeScript(self, script_name, ob_url, former_status, tdef_id):
     ob = self.unrestrictedTraverse(ob_url)
     tdef = self.getTransitionValueById(tdef_id)
     sci = StateChangeInfo(
@@ -506,3 +498,16 @@ class InteractionWorkflow(Workflow):
       return root
     return etree.tostring(root, encoding='utf-8',
                           xml_declaration=True, pretty_print=True)
+
+from Products.ERP5Type import WITH_DC_WORKFLOW_BACKWARD_COMPATIBILITY
+if WITH_DC_WORKFLOW_BACKWARD_COMPATIBILITY:
+  from Products.ERP5Type.Utils import deprecated
+  from ComputedAttribute import ComputedAttribute
+
+  from Products.ERP5Type.Core.Workflow import _ContainerTab
+  InteractionWorkflow.interactions = ComputedAttribute(
+    deprecated('`interactions` is deprecated; use getTransitionValueList()')\
+              (lambda self: _ContainerTab({o.getReference(): o for o in self.getTransitionValueList()})),
+    1) # must be Acquisition-wrapped
+  InteractionWorkflow.security.declareProtected(Permissions.AccessContentsInformation,
+                                                'interactions')

product/ERP5Type/Core/State.py

@@ -127,10 +127,6 @@ class State(IdAsReferenceMixin("state_"),
     return [parent._getOb(destination_id) for destination_id in
             self.getDestinationIdList()]
 
-  security.declareProtected(Permissions.AccessContentsInformation,
-                            'getTransitions')
-  getTransitions = getDestinationIdList
-
   security.declareProtected(Permissions.AccessContentsInformation,
                             'setStatePermissionRolesDict')
   def setStatePermissionRolesDict(self, permission_roles):
@@ -186,4 +182,18 @@ class State(IdAsReferenceMixin("state_"),
     cell_role = cell._getRole()
     cell.selected = cell_role in self.getStatePermissionRolesDict()[cell_permission]
 
+from Products.ERP5Type import WITH_DC_WORKFLOW_BACKWARD_COMPATIBILITY
+if WITH_DC_WORKFLOW_BACKWARD_COMPATIBILITY:
+  from Products.ERP5Type.Utils import deprecated
+
+  State.getTransitions = deprecated(
+    'getTransitions() is deprecated; use getDestinationIdList()')\
+    (State.getDestinationIdList)
+  State.security.declareProtected(Permissions.AccessContentsInformation, 'getTransitions')
+
+  State.transitions = deprecated(
+    '`transitions` is deprecated; use getDestinationValueList()')\
+    (State.getDestinationIdList)
+  State.security.declareProtected(Permissions.AccessContentsInformation, 'transitions')
+
 InitializeClass(State)

product/ERP5Type/Core/Transition.py

@@ -156,4 +156,21 @@ class Transition(IdAsReferenceMixin("transition_"),
     """
     return objectValues(portal_type='Transition Variable')
 
+from Products.ERP5Type import WITH_DC_WORKFLOW_BACKWARD_COMPATIBILITY
+if WITH_DC_WORKFLOW_BACKWARD_COMPATIBILITY:
+  from Products.ERP5Type.Utils import deprecated
+  from ComputedAttribute import ComputedAttribute
+
+  Transition.actbox_url = ComputedAttribute(
+    deprecated('`actbox_url` is deprecated; use getAction()')\
+              (lambda self: self.getAction()))
+  Transition.security.declareProtected(Permissions.AccessContentsInformation,
+                                       'actbox_url')
+
+  Transition.actbox_name = ComputedAttribute(
+    deprecated('`actbox_name` is deprecated; use getActionName()')\
+              (lambda self: self.getActionName()))
+  Transition.security.declareProtected(Permissions.AccessContentsInformation,
+                                       'actbox_name')
+
 InitializeClass(Transition)

product/ERP5Type/Core/Workflow.py

@@ -59,6 +59,17 @@ def createExpressionContext(sci):
         }
     return getEngine().getContext(data)
 
+from Products.ERP5Type import WITH_DC_WORKFLOW_BACKWARD_COMPATIBILITY
+if WITH_DC_WORKFLOW_BACKWARD_COMPATIBILITY:
+  ## Patch for ERP5 Workflow: This must go before any Products.DCWorkflow
+  ## imports as createExprContext() is from-imported in several of its modules
+  import Products.DCWorkflow.Expression
+  Products.DCWorkflow.Expression.createExprContext = createExpressionContext
+  import inspect
+  for _, __m in inspect.getmembers(Products.DCWorkflow, inspect.ismodule):
+    if 'createExprContext' in __m.__dict__:
+      assert __m.__dict__['createExprContext'] is createExpressionContext
+
 import sys
 
 from collections import  defaultdict
@@ -839,7 +850,6 @@ class Workflow(XMLObject):
     econtext = None
     moved_exc = None
     validation_exc = None
-    script_context = None
     object_context = None
 
     # Figure out the old and new states.
@@ -873,14 +883,12 @@ class Workflow(XMLObject):
         if sci is None:
           sci = StateChangeInfo(document, self, former_status, tdef, old_state,
                                 new_state, form_kw)
-        if script_context is None:
-          script_context = self._asScriptContext()
         for script in script_value_list:
           # Pass lots of info to the script in a single parameter.
           if script.getPortalType() != 'Workflow Script':
             raise NotImplementedError ('Unsupported Script %s for state %s' %
                                        (script.getId(), old_state_reference))
-          script = getattr(script_context, script.getId())
+          script = getattr(self, script.getId())
           try:
             script(sci)  # May throw an exception.
           except ValidationFailed, validation_exc:
@@ -988,9 +996,7 @@ class Workflow(XMLObject):
           else:
             # Pass lots of info to the script in a single parameter.
             if script.getPortalType() == 'Workflow Script':
-              if script_context is None:
-                script_context = self._asScriptContext()
-              script = getattr(script_context, script.getId())
+              script = getattr(self, script.getId())
               script(sci)  # May throw an exception.
 
     # Return the new state object.
@@ -1428,31 +1434,6 @@ class Workflow(XMLObject):
           acquired_permission_set.remove(permission)
           state.setAcquirePermissionList(list(acquired_permission_set))
 
-  def _asScriptContext(self):
-    """
-      change the context given to the script by adding foo for script_foo to the
-      context dict in order to be able to call the script using its reference
-      (= not prefixed by script_) from another workflow script
-
-      historically, __getattr__ method of Workflow class was overriden for
-      the same purpose, but it was heavyweight: doing a lot of useless
-      operations (each time, it was checking for script_foo, even if foo was a
-      transition, state, ...)
-    """
-    script_context = self.asContext()
-    # asContext creates a temporary object and temporary object's "activate"
-    # method code is: "return self". This means that the script is not put in
-    # the activity queue as expected but it is instead directly executed. To fix
-    # this, we override the temporary object's "activate" method with the one of
-    # the original object.
-    script_context.activate = self.activate
-    script_prefix_len = len(SCRIPT_PREFIX)
-    for script_id in self.objectIds(meta_type="ERP5 Python Script"):
-      if script_id.startswith(SCRIPT_PREFIX):
-        script = getattr(script_context, script_id)
-        setattr(script_context, script_id[script_prefix_len:], script)
-    return script_context
-
   security.declareProtected(Permissions.AccessContentsInformation,
                             'getSourceValue')
   def getSourceValue(self):
@@ -1468,4 +1449,76 @@ class Workflow(XMLObject):
       return self._getOb(source_id)
     return None
 
+if WITH_DC_WORKFLOW_BACKWARD_COMPATIBILITY:
+  import warnings
+  def __getattr__(self, name):
+    """
+    Allow a Workflow Script to call another Workflow Script directly without
+    SCRIPT_PREFIX. This can be dropped as soon as DCWorkflow Compatibility is
+    not required anymore.
+    """
+    if not name or name[0] == '_': # Optimization (Folder.__getattr__)
+      raise AttributeError(name)
+    try:
+      return super(Workflow, self).__getattr__(name)
+    except AttributeError:
+      from Products.ERP5Type.Core.WorkflowScript import SCRIPT_PREFIX
+      if name.startswith(SCRIPT_PREFIX):
+        raise
+      prefixed_name = SCRIPT_PREFIX + name
+      if not hasattr(aq_base(self), prefixed_name):
+        raise
+      warnings.warn(
+        "%r: Script calling %s instead of %s" % (self, name, prefixed_name),
+        DeprecationWarning)
+      return self._getOb(prefixed_name)
+  Workflow.__getattr__ = __getattr__
+
+  Workflow._isAWorkflow = True # DCWorkflow Tool compatibility
+
+  from Products.ERP5Type.Utils import deprecated
+  from ComputedAttribute import ComputedAttribute
+
+  Workflow.state_var = ComputedAttribute(
+    deprecated('`state_var` attribute is deprecated; use getStateVariable()')\
+              (lambda self: self.getStateVariable()))
+  Workflow.security.declareProtected(Permissions.AccessContentsInformation, 'state_var')
+
+  class _ContainerTab(dict):
+    """
+    Backward compatibility for Products.DCWorkflow.ContainerTab
+    """
+    def objectIds(self):
+      return self.keys()
+
+    def objectValues(self):
+      return self.values()
+
+  Workflow.states = ComputedAttribute(
+    deprecated('`states` is deprecated; use getStateValueList()')\
+              (lambda self: _ContainerTab({o.getReference(): o for o in self.getStateValueList()})),
+    1) # must be Acquisition-wrapped
+  Workflow.security.declareProtected(Permissions.AccessContentsInformation, 'states')
+
+  Workflow.transitions = ComputedAttribute(
+    deprecated('`transitions` is deprecated; use getTransitionValueList()')\
+              (lambda self: _ContainerTab({o.getReference(): o for o in self.getTransitionValueList()})),
+    1) # must be Acquisition-wrapped
+  Workflow.security.declareProtected(Permissions.AccessContentsInformation, 'transitions')
+
+  @deprecated('`scripts` is deprecated; use getScriptValueList()')
+  def _scripts(self):
+    """
+    Backward compatibility to avoid modifying Workflow Scripts code
+    """
+    script_dict = _ContainerTab()
+    for script in self.getScriptValueList():
+      # wf.scripts['foobar']
+      script_dict[script.getReference()] = script
+      # another_workflow_with_the_same_script_id.scripts[script.getId()]
+      script_dict[script.getId()] = script
+    return script_dict
+  Workflow.scripts = ComputedAttribute(_scripts, 1)
+  Workflow.security.declareProtected(Permissions.AccessContentsInformation, 'scripts')
+
 InitializeClass(Workflow)

product/ERP5Type/__init__.py

@@ -35,6 +35,8 @@ from patches import python, pylint, globalrequest
 from zLOG import LOG, INFO
 DISPLAY_BOOT_PROCESS = False
 
+WITH_DC_WORKFLOW_BACKWARD_COMPATIBILITY = True
+
 # We have a name conflict with source_reference and destination_reference,
 # which are at the same time property accessors for 'source_reference'
 # property, and category accessors (similar to getSourceValue().getReference())