Fix support for default values in preferences when several preferences are active

Also fix testPreferences and set default value for
preferred_accounting_transaction_simulation_state property.

git-svn-id: https://svn.erp5.org/repos/public/erp5/trunk@29345 20353a03-c40f-0410-a6d1-a30d3c3de9de

product/ERP5/PropertySheet/AccountingPreference.py

@@ -48,6 +48,7 @@ class AccountingPreference:
       'description' : 'The simulation state for transactions',
       'type'        : 'tokens',
       'preference'  : 1,
+      'default'     : [],
       'mode'        : 'w'},
     { 'id'          : 'preferred_accounting_transaction_section_category',
       'description' : 'The section category for transactions; '\

product/ERP5Form/PreferenceTool.py

@@ -40,7 +40,7 @@ from Products.ERP5Type.Accessor.TypeDefinition import list_types
 from Products.ERP5Form import _dtmldir
 from Products.ERP5Form.Document.Preference import Priority
 
-_marker = []
+_marker = object()
 
 def updatePreferenceClassPropertySheetList():
   # The Preference class should be imported from the common location
@@ -104,7 +104,7 @@ def createPreferenceToolAccessorList(portal) :
       if prop['type'] in list_types :
         attr_list +=  ['get%sList' % convertToUpperCase(attribute), ]
       for attribute_name in attr_list:
-        method = PreferenceMethod(attribute_name)
+        method = PreferenceMethod(attribute_name, prop.get('default'))
         setattr(PreferenceTool, attribute_name, method)
 
 
@@ -118,46 +118,35 @@ class PreferenceMethod(Method):
   func_code.co_argcount = 1
   func_defaults = ()
 
-  def __init__(self, attribute):
-    self._preference_name = attribute
+  def __init__(self, attribute, default):
+    self._preference_getter = attribute
+    self._preference_default = default
     self._preference_cache_id = 'PreferenceTool.CachingMethod.%s' % attribute
-    self._null = (None, '', (), [])
 
-  def __call__(self, instance, *args, **kw):
+  def __call__(self, instance, default=_marker, *args, **kw):
     def _getPreference(*args, **kw):
-      value = None
       # XXX: sql_catalog_id is passed when calling getPreferredArchive
       # This is inconsistent with regular accessor API, and indicates that
       # there is a design problem in current archive API.
       sql_catalog_id = kw.pop('sql_catalog_id', None)
       for pref in instance._getSortedPreferenceList(sql_catalog_id=sql_catalog_id):
-        value = getattr(pref, self._preference_name, _marker)
-        # XXX-JPS Why don't we use accessors here such as:
-        # value = pref.getProperty(self._preference_name, _marker)
-        if value is not _marker:
-          # If callable, store the return value.
-          if callable(value):
-            value = value(*args, **kw)
-          if value not in self._null:
-            break
+        value = getattr(pref, self._preference_getter)(_marker, *args, **kw)
+        # XXX Due to UI limitation, null value is treated as if the property
+        #     was not defined. The drawback is that it is not possible for a
+        #     user to mask a non-null global value with a null value.
+        if value not in (_marker, None, '', (), []):
           return value
+      return _marker
     _getPreference = CachingMethod(_getPreference,
             id='%s.%s' % (self._preference_cache_id,
                           getSecurityManager().getUser().getId()),
             cache_factory='erp5_ui_short')
     value = _getPreference(*args, **kw)
-    # XXX Preference Tool has a strange assumption that, even if
-    # all values are null values, one of them must be returned.
-    # Therefore, return a default value, only if explicitly specified,
-    # instead of returning None.
-    default = _marker
-    if 'default' in kw:
-      default = kw['default']
-    elif args:
-      default = args[0]
-    if value in self._null and default is not _marker:
-      return default
+    if value is not _marker:
       return value
+    elif default is _marker:
+      return self._preference_default
+    return default
 
 class PreferenceTool(BaseTool):
   """
@@ -199,11 +188,7 @@ class PreferenceTool(BaseTool):
     """ get the preference on the most appopriate Preference object. """
     method = getattr(self, 'get%s' % convertToUpperCase(pref_name), None)
     if method is not None:
-      if default is not _marker:
-        kw = {'default': default}
-      else:
-        kw = {}
-      return method(**kw)
+      return method(default)
     return default
 
   security.declareProtected(Permissions.ModifyPortalContent, "setPreference")

product/ERP5Form/tests/testPreferences.py

@@ -39,6 +39,11 @@ from DateTime import DateTime
 
 from Products.ERP5Type.tests.ERP5TypeTestCase import ERP5TypeTestCase
 from Products.ERP5Form.Document.Preference import Priority
+from Products.ERP5.PropertySheet.HtmlStylePreference import HtmlStylePreference
+
+default_large_image_height, = [pref.get('default')
+                               for pref in HtmlStylePreference._properties
+                               if pref['id'] == 'preferred_large_image_height']
 
 
 class TestPreferences(ERP5TypeTestCase):
@@ -367,26 +372,27 @@ class TestPreferences(ERP5TypeTestCase):
 
     method = pref_tool.getPreferredAccountingTransactionSimulationState
     state = method()
-    self.assertEquals(state, None)
+    self.assertEquals(state, [])
     state = method('default')
     self.assertEquals(state, 'default')
 
     method = lambda *args: pref_tool.getPreference('preferred_accounting_transaction_simulation_state', *args)
     state = method()
-    self.assertEquals(state, None)
+    self.assertEquals(state, [])
     state = method('default')
     self.assertEquals(state, 'default')
 
     method = pref_tool.getPreferredAccountingTransactionSimulationStateList
     state_list = method()
-    self.assertEquals(state_list, None)
+    self.assertEquals(state_list, [])
     state_list = method(('default',))
-    self.assertEquals(state_list, ['default',]) # getPreferredAccountingTransactionSimulationStateList
-                                                # always tries to cast tuples to lists
+    # Initially, tuples were always casted to lists. This is not the case
+    # anymore when preference_tool.getXxxList returns the default value.
+    self.assertEquals(state_list, ('default',))
 
     method = lambda *args: pref_tool.getPreference('preferred_accounting_transaction_simulation_state_list', *args)
     state_list = method()
-    self.assertEquals(state_list, None)
+    self.assertEquals(state_list, [])
     state_list = method(('default',))
     self.assertEquals(state_list, ('default',))
 
@@ -424,21 +430,33 @@ class TestPreferences(ERP5TypeTestCase):
 
 
   def test_SystemPreference(self):
+    # We want to test a property with a default value defined
+    self.assertTrue(default_large_image_height > 0)
+    large_image_height = default_large_image_height + 1
+
     preference_tool = self.portal.portal_preferences
-    site_pref = preference_tool.newContent(
+    system_pref = preference_tool.newContent(
                           portal_type='System Preference',
-                          preferred_accounting_transaction_simulation_state_list="this_is_default",
+                          preferred_ooodoc_server_address='127.0.0.1',
                           priority=Priority.SITE)
     # check not taken into account if not enabled
-    self.assertEquals(None,
-                      preference_tool.getPreferredAccountingTransactionSimulationStateList())
+    self.assertEqual(None,
+                     preference_tool.getPreferredOoodocServerAddress())
+    self.assertEqual('localhost',
+                     preference_tool.getPreferredOoodocServerAddress('localhost'))
+    self.assertEqual(default_large_image_height,
+                     preference_tool.getPreferredLargeImageHeight())
     # enable it and check preference is returned
-    self.portal.portal_workflow.doActionFor(site_pref, 'enable_action')
-    self.assertEquals(site_pref.getPreferenceState(), 'global')
+    self.portal.portal_workflow.doActionFor(system_pref, 'enable_action')
+    self.assertEqual(system_pref.getPreferenceState(), 'global')
     transaction.commit()
     self.tic()
-    self.assertEquals(['this_is_default'],
-                      preference_tool.getPreferredAccountingTransactionSimulationStateList())
+    self.assertEqual('127.0.0.1',
+                     preference_tool.getPreferredOoodocServerAddress())
+    self.assertEqual('127.0.0.1',
+                     preference_tool.getPreferredOoodocServerAddress('localhost'))
+    self.assertEqual(default_large_image_height,
+                     preference_tool.getPreferredLargeImageHeight())
 
     # Members can't add new system preferences
     uf = self.getPortal().acl_users
@@ -447,40 +465,39 @@ class TestPreferences(ERP5TypeTestCase):
     newSecurityManager(None, member)
     self.assertRaises(Unauthorized, preference_tool.newContent, portal_type='System Preference')
     # But they can see others
-    site_pref.view()
+    system_pref.view()
     # check accessors works
-    site_pref.setPreferredAccountingTransactionSimulationStateList(
-      ['this_is_system'])
+    system_pref.setPreferredOoodocServerAddress('1.2.3.4')
     transaction.commit()
     self.tic()
-    self.assertEquals(['this_is_system'],
-                      preference_tool.getPreferredAccountingTransactionSimulationStateList())
+    self.assertEqual('1.2.3.4',
+                     preference_tool.getPreferredOoodocServerAddress())
+    self.assertEqual('1.2.3.4',
+                     preference_tool.getPreferredOoodocServerAddress('localhost'))
+    self.assertEqual(default_large_image_height,
+                     preference_tool.getPreferredLargeImageHeight())
 
     # create a user pref and check it doesn't outranks the system one if
     # they both defined same pref
     user_pref = preference_tool.newContent(
                           portal_type='Preference',
                           priority=Priority.USER)
-    user_pref.setPreferredAccountingTransactionSimulationStateList(
-      ['this_is_user'])
+    user_pref.setPreferredLargeImageHeight(large_image_height)
     self.portal.portal_workflow.doActionFor(user_pref, 'enable_action')
-    self.assertEquals(user_pref.getPreferenceState(), 'enabled')
+    self.assertEqual(user_pref.getPreferenceState(), 'enabled')
     transaction.commit()
     self.tic()
-    self.assertEquals(['this_is_user'],
-                      user_pref.getPreferredAccountingTransactionSimulationStateList())
-    self.assertEquals(['this_is_system'],
-                      preference_tool.getPreferredAccountingTransactionSimulationStateList())
-
-    # check a user can't edit preference which are marked for manager (only for zope2.8)
-    try:
-      from ZODB.Transaction import Transaction
-    except ImportError:
+    self.assertEqual('1.2.3.4',
+                     preference_tool.getPreferredOoodocServerAddress('localhost'))
+    self.assertEqual(large_image_height,
+                     preference_tool.getPreferredLargeImageHeight())
+
+    # check a user can't edit preference which are marked for manager
     self.assertRaises(Unauthorized, user_pref.edit, preferred_ooodoc_server_address="localhost")
     # even if there is System Preference enabled getActivePreference shall return
     # user preference
     self.assertEqual(user_pref, preference_tool.getActivePreference())
-    self.assertEqual(site_pref, preference_tool.getActiveSystemPreference())
+    self.assertEqual(system_pref, preference_tool.getActiveSystemPreference())
 
 def test_suite():
   suite = unittest.TestSuite()