BusinessTemplate: sort properties when saving them

The _getOrderedList approach was not ideal, because the properties were
saved when being displayed, so the typical workflow was:

 1. enter the property as non sorted
 2. click save (property is saved as non sorted)
 3. the page displays the property again as sorted
 4. click save again so that the property is saved as sorted

by sorting the properties at save time, step 1 is enough

This change back the accessors to be "standard" accessors, ie. returning
lists, like every other list accessors, so a few tests and a few scripts
had to be adjusted for the new API.

bt5/erp5_core_test/TestTemplateItem/portal_components/test.erp5.testBusinessTemplate.py

@@ -3295,6 +3295,12 @@ class TestBusinessTemplate(BusinessTemplateMixin):
     """Tests the Title of the Template Tool."""
     self.assertEqual('Business Templates', self.getTemplateTool().Title())
 
+  def test_business_template_properties_sorted(self):
+    bt = self.portal.portal_templates.newContent(
+        portal_type='Business Template')
+    bt.edit(template_path_list=['b', 'c', 'a'])
+    self.assertEqual(bt.getTemplatePathList(), ['a', 'b', 'c'])
+
   def test_01_checkNewSite(self):
     """Test Check New Site"""
     sequence_list = SequenceList()
@@ -6812,7 +6818,7 @@ class TestBusinessTemplate(BusinessTemplateMixin):
       portal_type='Business Template',
       title=self.id(),
       template_path_list=(
-        'portal_categories/test_category/**'
+        'portal_categories/test_category/**',
       ),
       template_base_category_list=['test_category'],
     )
@@ -6883,7 +6889,7 @@ class TestBusinessTemplate(BusinessTemplateMixin):
       portal_type='Business Template',
       title=self.id(),
       template_path_list=(
-        'portal_categories/test_category/**'
+        'portal_categories/test_category/**',
       ),
       template_base_category_list=['test_category'],
     )

bt5/erp5_forge/SkinTemplateItem/portal_skins/erp5_forge/BusinessTemplate_createSkinFolder.py

@@ -5,7 +5,8 @@ portal = context.getPortalObject()
 if skin_folder_name not in portal.portal_skins.objectIds():
   portal.portal_skins.manage_addFolder(skin_folder_name)
 if skin_folder_name not in (context.getTemplateSkinIdList() or []):
-  context.setTemplateSkinIdList(tuple(context.getTemplateSkinIdList() or []) + (skin_folder_name, ))
+  context.setTemplateSkinIdList(
+    sorted(tuple(context.getTemplateSkinIdList() or []) + (skin_folder_name, )))
 
 skin_folder = portal.portal_skins[skin_folder_name]
 
@@ -30,7 +31,8 @@ if skin_layer_list:
         registered_skin = '%s | %s' % (skin_folder_name, skin_name)
         registered_skin_selection_list = context.getTemplateRegisteredSkinSelectionList() or []
         if registered_skin not in registered_skin_selection_list:
-          context.setTemplateRegisteredSkinSelectionList(tuple(registered_skin_selection_list) + (registered_skin, ))
+          context.setTemplateRegisteredSkinSelectionList(
+            sorted(tuple(registered_skin_selection_list) + (registered_skin, )))
 
   if not all_skin_layers_selected:
     marker = []

bt5/erp5_forge/SkinTemplateItem/portal_skins/erp5_toolbox/BusinessTemplate_createReport.py

@@ -99,9 +99,11 @@ type_information.addAction(
 # Associate the dialog with type information in business template meta data
 if context.getPortalType() == 'Business Template' and \
      context.getInstallationState() != 'installed':
-  context.setTemplateActionPathList(context.getTemplateActionPathList() +
+  context.setTemplateActionPathList(
+    sorted(
+      tuple(context.getTemplateActionPathList()) +
       ('%s | %s' % (portal_type, action_id),
-       '%s | %s' % (portal_type, action_id.replace('_report', '_export')), ))
+       '%s | %s' % (portal_type, action_id.replace('_report', '_export')))))
 
 # Create the report
 skin_folder.manage_addProduct['ERP5Form'].addERP5Report(report_form_name, report_name)

bt5/erp5_forge/TestTemplateItem/portal_components/test.erp5.testERP5Toolbox.py

@@ -76,9 +76,10 @@ class TestBusinessTemplateScripts(ERP5TypeTestCase):
 
     # actions were added to business template
     self.assertEqual(
-        (
+        [
             'Foo Module | dummy_export_export',
-            'Foo Module | dummy_report_report'),
+            'Foo Module | dummy_report_report',
+        ],
         self.business_template.getTemplateActionPathList(),
     )
 
@@ -97,7 +98,7 @@ class TestBusinessTemplateScripts(ERP5TypeTestCase):
     self.assertIn('dummy_skin_folder', self.portal.portal_skins.objectIds())
     # skin is added to business template
     self.assertEqual(
-        ('dummy_skin_folder', 'existing'),
+        ['dummy_skin_folder', 'existing'],
         self.business_template.getTemplateSkinIdList())
 
   def test_BusinessTemplate_createSkinFolder_priority(self):
@@ -154,8 +155,8 @@ class TestBusinessTemplateScripts(ERP5TypeTestCase):
 
     # skin is added to business template
     self.assertEqual(
-        (
+        [
             'dummy_skin_folder | SelectedSkinSelection',
             'dummy_skin_folder | View',
             'existing | SelectedSkinSelection',
-        ), self.business_template.getTemplateRegisteredSkinSelectionList())
+        ], self.business_template.getTemplateRegisteredSkinSelectionList())

product/ERP5/Document/BusinessTemplate.py

@@ -45,6 +45,7 @@ from Products.CMFCore.utils import getToolByName
 from Products.PythonScripts.PythonScript import PythonScript
 from Products.ZSQLMethods.SQL import SQL
 from Products.ERP5Type.Accessor.Constant import PropertyGetter as ConstantGetter
+from Products.ERP5Type.Accessor.TypeDefinition import asList
 from Products.ERP5Type.Cache import transactional_cached
 from Products.ERP5Type.Message import translateString
 from Products.ERP5Type.UnrestrictedMethod import super_user
@@ -5578,185 +5579,36 @@ Business Template is a set of definitions, such as skins, portal types and categ
                                                download=1)
       return export_string
 
-    def _getOrderedList(self, id):
-      """
-        We have to set this method because we want an
-        ordered list
-      """
-      method_id = '_baseGet%sList' % convertToUpperCase(id)
-      result = getattr(self, method_id)(())
-      if result is None: result = ()
-      if result != ():
-        result = list(result)
-        result.sort()
-        # XXX Why do we need to return a tuple ?
-        result = tuple(result)
-      return result
-
-    security.declareProtected(Permissions.AccessContentsInformation, 'getTemplateCatalogMethodIdList')
-    def getTemplateCatalogMethodIdList(self):
-      """
-      We have to set this method because we want an
-      ordered list
-      """
-      return self._getOrderedList('template_catalog_method_id')
-
-    security.declareProtected(Permissions.AccessContentsInformation, 'getTemplateBaseCategoryList')
-    def getTemplateBaseCategoryList(self):
-      """
-      We have to set this method because we want an
-      ordered list
-      """
-      return self._getOrderedList('template_base_category')
-
-    security.declareProtected(Permissions.AccessContentsInformation, 'getTemplateWorkflowIdList')
-    def getTemplateWorkflowIdList(self):
-      """
-      We have to set this method because we want an
-      ordered list
-      """
-      return self._getOrderedList('template_workflow_id')
-
-    security.declareProtected(Permissions.AccessContentsInformation, 'getTemplatePortalTypeIdList')
-    def getTemplatePortalTypeIdList(self):
-      """
-      We have to set this method because we want an
-      ordered list
-      """
-      return self._getOrderedList('template_portal_type_id')
-
-    security.declareProtected(Permissions.AccessContentsInformation, 'getTemplatePortalTypeWorkflowChainList')
-    def getTemplatePortalTypeWorkflowChainList(self):
-      """
-      We have to set this method because we want an
-      ordered list
-      """
-      return self._getOrderedList('template_portal_type_workflow_chain')
-
-    security.declareProtected(Permissions.AccessContentsInformation, 'getTemplatePathList')
-    def getTemplatePathList(self):
-      """
-      We have to set this method because we want an
-      ordered list
-      """
-      return self._getOrderedList('template_path')
-
-    security.declareProtected(Permissions.AccessContentsInformation, 'getTemplatePreferenceList')
-    def getTemplatePreferenceList(self):
-      """
-      We have to set this method because we want an
-      ordered list
-      """
-      return self._getOrderedList('template_preference')
-
-    security.declareProtected(Permissions.AccessContentsInformation, 'getTemplatePortalTypeAllowedContentTypeList')
-    def getTemplatePortalTypeAllowedContentTypeList(self):
-      """
-      We have to set this method because we want an
-      ordered list
-      """
-      return self._getOrderedList('template_portal_type_allowed_content_type')
-
-    security.declareProtected(Permissions.AccessContentsInformation, 'getTemplatePortalTypeHiddenContentTypeList')
-    def getTemplatePortalTypeHiddenContentTypeList(self):
-      """
-      We have to set this method because we want an
-      ordered list
-      """
-      return self._getOrderedList('template_portal_type_hidden_content_type')
-
-    security.declareProtected(Permissions.AccessContentsInformation, 'getTemplatePortalTypePropertySheetList')
-    def getTemplatePortalTypePropertySheetList(self):
-      """
-      We have to set this method because we want an
-      ordered list
-      """
-      return self._getOrderedList('template_portal_type_property_sheet')
-
-    security.declareProtected(Permissions.AccessContentsInformation, 'getTemplatePortalTypeBaseCategoryList')
-    def getTemplatePortalTypeBaseCategoryList(self):
-      """
-      We have to set this method because we want an
-      ordered list
-      """
-      return self._getOrderedList('template_portal_type_base_category')
-
-    security.declareProtected(Permissions.AccessContentsInformation, 'getTemplateActionPathList')
-    def getTemplateActionPathList(self):
-      """
-      We have to set this method because we want an
-      ordered list
-      """
-      return self._getOrderedList('template_action_path')
-
-    security.declareProtected(Permissions.AccessContentsInformation, 'getTemplatePortalTypeRoleList')
-    def getTemplatePortalTypeRoleList(self):
-      """
-      We have to set this method because we want an
-      ordered list
-      """
-      return self._getOrderedList('template_portal_type_role')
-
-    security.declareProtected(Permissions.AccessContentsInformation, 'getTemplateLocalRoleList')
-    def getTemplateLocalRoleList(self):
-      """
-      We have to set this method because we want an
-      ordered list
-      """
-      return self._getOrderedList('template_local_role')
-
-    security.declareProtected(Permissions.AccessContentsInformation, 'getTemplateSkinIdList')
-    def getTemplateSkinIdList(self):
-      """
-      We have to set this method because we want an
-      ordered list
-      """
-      return self._getOrderedList('template_skin_id')
-
-    security.declareProtected(Permissions.AccessContentsInformation, 'getTemplateRegisteredSkinSelectionList')
-    def getTemplateRegisteredSkinSelectionList(self):
-      """
-      We have to set this method because we want an
-      ordered list
-      """
-      return self._getOrderedList('template_registered_skin_selection')
-
-    security.declareProtected(Permissions.AccessContentsInformation, 'getTemplateRegisteredVersionPrioritySelectionList')
-    def getTemplateRegisteredVersionPrioritySelectionList(self):
-      """
-      We have to set this method because we want an
-      ordered list
-      """
-      try:
-        return self._getOrderedList('template_registered_version_priority_selection')
-      # This property may not be defined if erp5_property_sheets has not been
-      # upgraded yet
-      except AttributeError:
-        return ()
-
-    security.declareProtected(Permissions.AccessContentsInformation, 'getTemplateModuleIdList')
-    def getTemplateModuleIdList(self):
-      """
-      We have to set this method because we want an
-      ordered list
-      """
-      return self._getOrderedList('template_module_id')
-
-    security.declareProtected(Permissions.AccessContentsInformation, 'getTemplateMessageTranslationList')
-    def getTemplateMessageTranslationList(self):
-      """
-      We have to set this method because we want an
-      ordered list
-      """
-      return self._getOrderedList('template_message_translation')
-
-    security.declareProtected(Permissions.AccessContentsInformation, 'getTemplateToolIdList')
-    def getTemplateToolIdList(self):
-      """
-      We have to set this method because we want an
-      ordered list
-      """
-      return self._getOrderedList('template_tool_id')
+    def _edit(self, *args, **kw):
+      """Make sure UI stores list properties as sorted.
+      """
+      edit_kw = {}
+      for k, v in six.iteritems(kw):
+        if v and k in (
+          'template_action_path_list',
+          'template_base_category_list',
+          'template_catalog_method_id_list',
+          'template_local_role_list',
+          'template_message_translation_list',
+          'template_module_id_list',
+          'template_path_list',
+          'template_portal_type_allowed_content_type_list',
+          'template_portal_type_base_category_list',
+          'template_portal_type_hidden_content_type_list',
+          'template_portal_type_id_list',
+          'template_portal_type_property_sheet_list',
+          'template_portal_type_role_list',
+          'template_portal_type_workflow_chain_list',
+          'template_preference_list',
+          'template_registered_skin_selection_list',
+          'template_registered_version_priority_selection_list',
+          'template_skin_id_list',
+          'template_tool_id_list',
+          'template_workflow_id_list',
+        ):
+          v = sorted(asList(v))
+        edit_kw[k] = v
+      return super(BusinessTemplate, self)._edit(*args, **edit_kw)
 
     def _isInKeepList(self, keep_list, path):
       for keep_path in keep_list: