Commit 61daf2ba authored by Kazuhiko Shiozaki's avatar Kazuhiko Shiozaki

fixup! ERP5Workflow: DC Workflows are now ERP5 objects (!1378).

parent a3e2d962
Pipeline #26650 failed with stage
......@@ -60,9 +60,10 @@ def createExpressionContext(sci):
'status': sci.status,
'kwargs': sci.kwargs,
'workflow': wf,
# Patch:
'scripts': {s.getReference(): s for s in wf.getScriptValueList()},
}
# Patch:
if WITH_LEGACY_WORKFLOW:
data['scripts'] = wf.scripts
return getEngine().getContext(data)
from Products.ERP5Type import WITH_LEGACY_WORKFLOW
......
  • @jerome @arnau This change caused one failure in testERP5Type.

    FAIL: test_products_document_legacy (testERP5Type.TestERP5Type)
    check document classes defined in Products/*/Document/*.py
    ----------------------------------------------------------------------
    Traceback (most recent call last):
      File "/SR/parts/erp5/Products/ERP5Type/tests/testERP5Type.py", line 237, in test_products_document_legacy
        DeprecationWarning, 2)
      File "/SR/eggs/mock-3.0.5-py2.7.egg/mock/mock.py", line 944, in assert_called_with
        six.raise_from(AssertionError(_error_message(cause)), cause)
      File "/SR/eggs/six-1.16.0-py2.7.egg/six.py", line 754, in raise_from
        raise value
    AssertionError: expected call not found.
    Expected: warn('newTemp*(self, ID) will be removed, use self.newContent(temp_object=True, id=ID, portal_type=...)', <type 'exceptions.DeprecationWarning'>, 2)
    Actual: warn('`scripts` is deprecated; use getScriptValueList()', <type 'exceptions.DeprecationWarning'>, 2)

    because now newTempAlarm(self.portal, '') got two deprecation warnings.

    Do you think we add scripts in expression context for legacy workflow only with the following patch ?

    --- product/ERP5Type/Core/Workflow.py
    +++ product/ERP5Type/Core/Workflow.py
    @@ -62,7 +62,7 @@ def createExpressionContext(sci):
             'workflow':     wf,
             }
         # Patch:
    -    if WITH_LEGACY_WORKFLOW:
    +    if WITH_LEGACY_WORKFLOW and wf.meta_type == 'Workflow':
             data['scripts'] = wf.scripts
         return getEngine().getContext(data)
    
  • yes, this looks good

  • The problem with the previous version is that it only allowed for getitem access ( scripts['another_script'](state_change) ) but not for getattr ( scripts.another_script(state_change) ) - maybe we could include this in commit message for postertiy

    Edited by Jérome Perrin
  • Thanks. I added more comment in the code. 55299097

  • ok too late but I just realized that it's not perfect if this triggers the warning and maybe we could write it with something like this

    
       class ScriptsCompat(dict):
         def __missing__(self, key):
           if key == 'scripts':
             return self['wf'].keys
           raise KeyError
    
        return getEngine().getContext(ScriptsCompat(data))
    

    but I did not try and it does not look so important

  • this idea was to trigger the warning only when scripts was actually used in TALES, but once again I did not try and maybe it does not work at all :) and this only affects non migrated workflows so it's OK

Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment