Simplify logic to sort business template properties

See merge request nexedi/erp5!1676

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

@@ -3295,6 +3295,16 @@ 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'])
+    bt.edit(template_keep_workflow_path_list=['b', 'c', 'a'])
+    self.assertEqual(bt.getTemplateKeepWorkflowPathList(), ['a', 'b', 'c'])
+    bt.edit(template_keep_last_workflow_history_only_path_list=['b', 'c', 'a'])
+    self.assertEqual(bt.getTemplateKeepLastWorkflowHistoryOnlyPathList(), ['a', 'b', 'c'])
+
   def test_01_checkNewSite(self):
     """Test Check New Site"""
     sequence_list = SequenceList()
@@ -6812,7 +6822,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 +6893,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,39 @@ Business Template is a set of definitions, such as skins, portal types and categ
                                                download=1)
       return export_string
 
-    def _getOrderedList(self, id):
+    def _edit(self, *args, **kw):
+      """Make sure UI stores list properties as sorted.
       """
-        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')
+      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_keep_path_list',
+          'template_keep_workflow_path_list',
+          'template_keep_last_workflow_history_only_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: