Prevent passing object / relative_url to the wrong setter

product/CMFCategory/CategoryTool.py

@@ -638,6 +638,18 @@ class CategoryTool(BaseTool):
         category_list = (category_list, )
       elif category_list is None:
         category_list = ()
+      elif isinstance(category_list, (tuple, list, set, frozenset)):
+        if any([c is not None and not isinstance(c, str) for c in category_list]):
+          raise TypeError(
+            'This method only takes a string or an iterable of strings as parameter.',
+             base_category_list, category_list
+          )
+      else:
+        raise TypeError(
+          'This method only takes a string or an iterable of strings as parameter.',
+           base_category_list, category_list
+        )
+
 
       if isinstance(base_category_list, str):
         base_category_list = (base_category_list, )

product/ERP5Type/Base.py

@@ -1838,14 +1838,21 @@ class Base( CopyContainer,
     if target is None :
       path = target
     elif isinstance(target, str):
-      # We have been provided a string
       path = target
+      warnings.warn(
+        "Only objects should be passed to value accessors",
+        DeprecationWarning
+      )
     elif isinstance(target, (tuple, list, set, frozenset)):
       # We have been provided a list or tuple
       path_list = []
       for target_item in target:
         if isinstance(target_item, str):
           path = target_item
+          warnings.warn(
+            "Only objects should be passed to value accessors",
+            DeprecationWarning
+          )
         else:
           path = getRelativeUrl(target_item)
         path_list.append(cleanupCategory(path))

product/ERP5Type/tests/testDynamicClassGeneration.py

@@ -33,6 +33,7 @@ import os
 import shutil
 import tempfile
 import unittest
+import warnings
 
 import transaction
 from persistent import Persistent
@@ -378,6 +379,8 @@ class TestZodbPropertySheet(ERP5TypeTestCase):
   """
   XXX: WORK IN PROGRESS
   """
+
+
   def getBusinessTemplateList(self):
     return 'erp5_base',
 
@@ -1313,6 +1316,62 @@ class TestZodbPropertySheet(ERP5TypeTestCase):
       self.fail("Creating a Category Expression with syntax error raises "\
                 "an error")
 
+  def testCategoryValueRelationShowsWarningIfStringIsPassedAsParameter(self):
+    person_module = self.portal.person_module
+    person = person_module.newContent()
+
+    def _testDeprecationWarning(method, *args, **kw):
+      with warnings.catch_warnings(record=True) as warning_list:
+       warnings.simplefilter("always")
+       method(*args, **kw)
+      warning, = warning_list
+      self.assertTrue(issubclass(warning.category, DeprecationWarning))
+      self.assertEqual(
+        str(warning.message),
+        "Only objects should be passed to value accessors",
+      )
+
+    # Passing a string to a Value setter should raise
+    organisation = self.portal.organisation_module.newContent()
+    _testDeprecationWarning(
+      person.setSubordinationValue,
+      organisation.getRelativeUrl(),
+    )
+    _testDeprecationWarning(
+      person_module.newContent,
+      subordination_value=organisation.getRelativeUrl(),
+    )
+
+    # Same test but with a category instead of an object
+    social_title_value = self.portal.portal_categories.social_title.newContent(id='Mme')
+    _testDeprecationWarning(
+      person.setSocialTitleValue,
+      social_title_value.getRelativeUrl(),
+    )
+
+  def testCategoryRelationRaisesIfValueisPassedAsParameter(self):
+    person_module = self.portal.person_module
+    person = person_module.newContent()
+
+    # Passing an ERP5 object to a not-Value setter should raise
+    with self.assertRaises(TypeError):
+      organisation = self.portal.organisation_module.newContent()
+      person.setSubordination(organisation)
+      person_module.newContent(
+        subordination=organisation,
+      )
+
+    # Same test with a category instead of an object
+    social_title_value = self.portal.portal_categories.social_title.newContent(id='Mr')
+    with self.assertRaises(TypeError):
+      person.setSocialTitle(social_title_value)
+
+    # Passing a unicode object to a not-Value setter should raise
+    with self.assertRaises(TypeError):
+      organisation = self.portal.organisation_module.newContent()
+      person.setSubordination(unicode(organisation.getRelativeUrl()))
+
+
 from Products.ERP5Type.Tool.ComponentTool import ComponentTool
 ComponentTool._original_reset = ComponentTool.reset
 ComponentTool._reset_performed = False