Commit 54929d3b authored by Jérome Perrin's avatar Jérome Perrin

Add missing error_message variable in workflows

With the new workflow implementation, error_message variable is required, because
we intentionally clone previous workflow history entries when passing transition,
as this leads to smaller pickle size

See merge request nexedi/erp5!1560
parents ed0c1297 c1f288b3
Pipeline #20674 failed with stage
in 0 seconds
...@@ -225,5 +225,5 @@ class TestDedup(ERP5TypeTestCase): ...@@ -225,5 +225,5 @@ class TestDedup(ERP5TypeTestCase):
new_obj_length, = deduped # pylint: disable=unbalanced-tuple-unpacking new_obj_length, = deduped # pylint: disable=unbalanced-tuple-unpacking
# The exact boundary does not matter much, but it should be greater than # The exact boundary does not matter much, but it should be greater than
# some arbitrary value considered satisfying. # some arbitrary value considered satisfying.
self.assertGreaterEqual(new_obj_length, 24) self.assertGreaterEqual(new_obj_length, 30)
self.assertEqual(len(list(whl)), new_obj_length + 1) self.assertEqual(len(list(whl)), new_obj_length + 1)
<?xml version="1.0"?>
<ZopeData>
<record id="1" aka="AAAAAAAAAAE=">
<pickle>
<global name="Workflow Variable" module="erp5.portal_type"/>
</pickle>
<pickle>
<dictionary>
<item>
<key> <string>automatic_update</string> </key>
<value> <int>1</int> </value>
</item>
<item>
<key> <string>description</string> </key>
<value>
<none/>
</value>
</item>
<item>
<key> <string>for_catalog</string> </key>
<value> <int>0</int> </value>
</item>
<item>
<key> <string>id</string> </key>
<value> <string>variable_error_message</string> </value>
</item>
<item>
<key> <string>portal_type</string> </key>
<value> <string>Workflow Variable</string> </value>
</item>
<item>
<key> <string>status_included</string> </key>
<value> <int>1</int> </value>
</item>
<item>
<key> <string>title</string> </key>
<value>
<none/>
</value>
</item>
</dictionary>
</pickle>
</record>
</ZopeData>
<?xml version="1.0"?>
<ZopeData>
<record id="1" aka="AAAAAAAAAAE=">
<pickle>
<global name="Workflow Variable" module="erp5.portal_type"/>
</pickle>
<pickle>
<dictionary>
<item>
<key> <string>automatic_update</string> </key>
<value> <int>1</int> </value>
</item>
<item>
<key> <string>description</string> </key>
<value>
<none/>
</value>
</item>
<item>
<key> <string>for_catalog</string> </key>
<value> <int>0</int> </value>
</item>
<item>
<key> <string>id</string> </key>
<value> <string>variable_error_message</string> </value>
</item>
<item>
<key> <string>portal_type</string> </key>
<value> <string>Workflow Variable</string> </value>
</item>
<item>
<key> <string>status_included</string> </key>
<value> <int>1</int> </value>
</item>
<item>
<key> <string>title</string> </key>
<value>
<none/>
</value>
</item>
</dictionary>
</pickle>
</record>
</ZopeData>
<?xml version="1.0"?>
<ZopeData>
<record id="1" aka="AAAAAAAAAAE=">
<pickle>
<global name="Workflow Variable" module="erp5.portal_type"/>
</pickle>
<pickle>
<dictionary>
<item>
<key> <string>automatic_update</string> </key>
<value> <int>1</int> </value>
</item>
<item>
<key> <string>description</string> </key>
<value>
<none/>
</value>
</item>
<item>
<key> <string>for_catalog</string> </key>
<value> <int>0</int> </value>
</item>
<item>
<key> <string>id</string> </key>
<value> <string>variable_error_message</string> </value>
</item>
<item>
<key> <string>portal_type</string> </key>
<value> <string>Workflow Variable</string> </value>
</item>
<item>
<key> <string>status_included</string> </key>
<value> <int>1</int> </value>
</item>
<item>
<key> <string>title</string> </key>
<value>
<none/>
</value>
</item>
</dictionary>
</pickle>
</record>
</ZopeData>
<?xml version="1.0"?>
<ZopeData>
<record id="1" aka="AAAAAAAAAAE=">
<pickle>
<global name="Workflow Variable" module="erp5.portal_type"/>
</pickle>
<pickle>
<dictionary>
<item>
<key> <string>automatic_update</string> </key>
<value> <int>1</int> </value>
</item>
<item>
<key> <string>description</string> </key>
<value>
<none/>
</value>
</item>
<item>
<key> <string>for_catalog</string> </key>
<value> <int>0</int> </value>
</item>
<item>
<key> <string>id</string> </key>
<value> <string>variable_error_message</string> </value>
</item>
<item>
<key> <string>portal_type</string> </key>
<value> <string>Workflow Variable</string> </value>
</item>
<item>
<key> <string>status_included</string> </key>
<value> <int>1</int> </value>
</item>
<item>
<key> <string>title</string> </key>
<value>
<none/>
</value>
</item>
</dictionary>
</pickle>
</record>
</ZopeData>
...@@ -4253,10 +4253,11 @@ class _ZodbComponentTemplateItem(ObjectTemplateItem): ...@@ -4253,10 +4253,11 @@ class _ZodbComponentTemplateItem(ObjectTemplateItem):
continue continue
wf_history = obj.workflow_history[wf_id][-1] wf_history = obj.workflow_history[wf_id][-1]
# Remove useless modifcation 'time' and 'actor' (conflicts with VCSs) # Remove useless workflow entries that are always different and cause conflicts with VCS
wf_history.pop('time', None)
wf_history.pop('actor', None) wf_history.pop('actor', None)
wf_history.pop('comment', None) wf_history.pop('comment', None)
wf_history.pop('error_message', None)
wf_history.pop('time', None)
obj.workflow_history[wf_id] = WorkflowHistoryList([wf_history]) obj.workflow_history[wf_id] = WorkflowHistoryList([wf_history])
......
<?xml version="1.0"?>
<ZopeData>
<record id="1" aka="AAAAAAAAAAE=">
<pickle>
<global name="Workflow Variable" module="erp5.portal_type"/>
</pickle>
<pickle>
<dictionary>
<item>
<key> <string>automatic_update</string> </key>
<value> <int>1</int> </value>
</item>
<item>
<key> <string>description</string> </key>
<value>
<none/>
</value>
</item>
<item>
<key> <string>for_catalog</string> </key>
<value> <int>0</int> </value>
</item>
<item>
<key> <string>id</string> </key>
<value> <string>variable_error_message</string> </value>
</item>
<item>
<key> <string>portal_type</string> </key>
<value> <string>Workflow Variable</string> </value>
</item>
<item>
<key> <string>status_included</string> </key>
<value> <int>1</int> </value>
</item>
<item>
<key> <string>title</string> </key>
<value>
<none/>
</value>
</item>
</dictionary>
</pickle>
</record>
</ZopeData>
<?xml version="1.0"?>
<ZopeData>
<record id="1" aka="AAAAAAAAAAE=">
<pickle>
<global name="Workflow Variable" module="erp5.portal_type"/>
</pickle>
<pickle>
<dictionary>
<item>
<key> <string>automatic_update</string> </key>
<value> <int>1</int> </value>
</item>
<item>
<key> <string>description</string> </key>
<value>
<none/>
</value>
</item>
<item>
<key> <string>for_catalog</string> </key>
<value> <int>0</int> </value>
</item>
<item>
<key> <string>id</string> </key>
<value> <string>variable_error_message</string> </value>
</item>
<item>
<key> <string>portal_type</string> </key>
<value> <string>Workflow Variable</string> </value>
</item>
<item>
<key> <string>status_included</string> </key>
<value> <int>1</int> </value>
</item>
<item>
<key> <string>title</string> </key>
<value>
<none/>
</value>
</item>
</dictionary>
</pickle>
</record>
</ZopeData>
<?xml version="1.0"?>
<ZopeData>
<record id="1" aka="AAAAAAAAAAE=">
<pickle>
<global name="Workflow Variable" module="erp5.portal_type"/>
</pickle>
<pickle>
<dictionary>
<item>
<key> <string>automatic_update</string> </key>
<value> <int>1</int> </value>
</item>
<item>
<key> <string>description</string> </key>
<value>
<none/>
</value>
</item>
<item>
<key> <string>for_catalog</string> </key>
<value> <int>0</int> </value>
</item>
<item>
<key> <string>id</string> </key>
<value> <string>variable_error_message</string> </value>
</item>
<item>
<key> <string>portal_type</string> </key>
<value> <string>Workflow Variable</string> </value>
</item>
<item>
<key> <string>status_included</string> </key>
<value> <int>1</int> </value>
</item>
<item>
<key> <string>title</string> </key>
<value>
<none/>
</value>
</item>
</dictionary>
</pickle>
</record>
</ZopeData>
...@@ -358,6 +358,9 @@ class InteractionWorkflow(Workflow): ...@@ -358,6 +358,9 @@ class InteractionWorkflow(Workflow):
def getStateValueList(self): def getStateValueList(self):
return [] return []
def _checkConsistency(self, fixit=False):
return []
security.declareProtected(Permissions.AccessContentsInformation, 'showAsXML') security.declareProtected(Permissions.AccessContentsInformation, 'showAsXML')
def showAsXML(self, root=None): def showAsXML(self, root=None):
from lxml import etree from lxml import etree
......
...@@ -30,6 +30,7 @@ ...@@ -30,6 +30,7 @@
## Used in Products.ERP5Type.patches.DCWorkflow so this needs to go first... ## Used in Products.ERP5Type.patches.DCWorkflow so this needs to go first...
from Acquisition import aq_parent, aq_inner from Acquisition import aq_parent, aq_inner
from Products.PageTemplates.Expressions import SecureModuleImporter from Products.PageTemplates.Expressions import SecureModuleImporter
from Products.ERP5Type.ConsistencyMessage import ConsistencyMessage
from AccessControl import getSecurityManager from AccessControl import getSecurityManager
from Products.PageTemplates.Expressions import getEngine from Products.PageTemplates.Expressions import getEngine
from six import reraise from six import reraise
...@@ -785,6 +786,17 @@ class Workflow(XMLObject): ...@@ -785,6 +786,17 @@ class Workflow(XMLObject):
tool = self.getParentValue() tool = self.getParentValue()
state_var = self.getStateVariable() state_var = self.getStateVariable()
# `status_dict` will hold the new status.
# Unlike DCWorkflow implementation, we don't start with an empty dict, but start
# by making a copy of the current status dict, this way the string used as keys
# will be the same string instances and this will reduce the pickle size:
# Copying existing dict saves space: when __setitem__(key, value) points at an
# existing key, python will just keep the existing string as key, which then
# means if both history entries are pickled together, the keys will be stored
# just once instead of once per dict.
# This is especially important with ERP5's WorkflowVariable implemented with
# IdAsReferenceMixin, because every call to getReference return a different string.
status_dict = self.getCurrentStatusDict(ob) status_dict = self.getCurrentStatusDict(ob)
if tdef is None: if tdef is None:
...@@ -947,6 +959,26 @@ class Workflow(XMLObject): ...@@ -947,6 +959,26 @@ class Workflow(XMLObject):
raise ObjectMoved(ex.getNewObject(), res) raise ObjectMoved(ex.getNewObject(), res)
return res return res
def _checkConsistency(self, fixit=False):
"""Checks the workflow definition.
"""
consistency_message_list = []
# make sure we have necessary variables
variable_reference_set = {
v.getReference()
for v in self.contentValues(portal_type='Workflow Variable')
}
for variable_reference in 'error_message', :
if variable_reference not in variable_reference_set:
consistency_message_list.append(
ConsistencyMessage(
self,
object_relative_url=self.getRelativeUrl(),
message=
'Required variable {variable_reference} missing in workflow.'.
format(variable_reference=variable_reference)))
return consistency_message_list
security.declareProtected(Permissions.AccessContentsInformation, 'showAsXML') security.declareProtected(Permissions.AccessContentsInformation, 'showAsXML')
def showAsXML(self, root=None): def showAsXML(self, root=None):
from lxml import etree from lxml import etree
......
...@@ -195,3 +195,16 @@ class CodingStyleTestCase(ERP5TypeTestCase): ...@@ -195,3 +195,16 @@ class CodingStyleTestCase(ERP5TypeTestCase):
'action_name': action_name, 'action_name': action_name,
}) })
self.assertEqual(duplicate_action_list, []) self.assertEqual(duplicate_action_list, [])
def test_workflow_consistency(self):
self.maxDiff = None
workflow_id_set = set()
for business_template in self._getTestedBusinessTemplateValueList():
workflow_id_set.update(business_template.getTemplateWorkflowIdList())
message_list = []
for workflow_id in workflow_id_set:
message_list.extend(
self.portal.portal_workflow[workflow_id].checkConsistency())
self.assertEqual(message_list, [])
...@@ -1659,6 +1659,38 @@ class TestZodbModuleComponent(SecurityTestCase): ...@@ -1659,6 +1659,38 @@ class TestZodbModuleComponent(SecurityTestCase):
self.tic() self.tic()
self.assertEqual(component.checkConsistency(), []) self.assertEqual(component.checkConsistency(), [])
def testWorkflowErrorMessage(self):
"""Check that validation error messages are stored in workflow
"""
component = self._newComponent(self._generateReference('WorkflowErrorMessage'))
valid_id = component.getId()
self.tic()
component.setId('wrong')
from Products.ERP5Type.Core.Workflow import ValidationFailed
with self.assertRaises(ValidationFailed):
self.portal.portal_workflow.doActionFor(component, 'validate_action')
last_error_message = str(
self.portal.portal_workflow.getInfoFor(
component, 'history',
wf_id='component_validation_workflow')[-1]['error_message'][0])
self.assertEqual(
last_error_message,
self.portal.Base_translateString(
ComponentMixin._message_invalid_id,
mapping={'id_prefix': self._document_class.getIdPrefix()}))
self.tic()
# non-regression test: when there is no error the error is no longer
# in workflow history
component.setId(valid_id)
component.validate()
self.tic()
last_error_message = self.portal.portal_workflow.getInfoFor(
component, 'history',
wf_id='component_validation_workflow')[-1]['error_message']
self.assertEqual(last_error_message, '')
def testReferenceWithReservedKeywords(self): def testReferenceWithReservedKeywords(self):
""" """
Check whether checkConsistency has been properly implemented for checking Check whether checkConsistency has been properly implemented for checking
......
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