BusinessTemplate: fix removing and trashing a document and its container

Because we were backing up to trash documents in the wrong order (in the example
from the test, portal_categories/test_category/removed_container/removed_document
first and then portal_categories/test_category/removed_container), first the
document was backed-up and some intermediate trash folders were created to
keep the hierarchy. Then when backing up the container, there was an error like:

    BadRequest: The id "removed_container" is invalid - it is already in use.

Similar error happens when upgrading erp5_configurator_standard, it was failing with:

    BadRequest: The id "officejs_sdk_workflow" is invalid - it is already in use.

The fix is to backup in reverse order, to back up first the containers and then
the document in containers.

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

@@ -6886,6 +6886,77 @@ class TestBusinessTemplate(BusinessTemplateMixin):
       'This path should not be reinstalled during update',
       portal_categories.test_category.modified_category.container_in_which_child_is_added.getTitle())
 
+  def test_update_business_template_with_recursively_removed_documents(self):
+    """Non regression test for a case when a subtree is removed
+
+    Updating from:
+        portal_categories/test_category
+        portal_categories/test_category/removed_container
+        portal_categories/test_category/removed_container/removed_document
+
+    to:
+
+        portal_categories/test_category
+
+    was causing error when removed_container was backup by portal trash
+    """
+    portal_categories = self.portal.portal_categories
+    if 'test_category' in portal_categories.objectIds():
+      portal_categories.manage_delObjects(['test_category'])
+    base_category = portal_categories.newContent(portal_type='Base Category', id='test_category')
+
+    removed_container = base_category.newContent(portal_type='Category', id='removed_container')
+    removed_container.newContent(portal_type='Category', id='removed_document')
+
+    business_template = self.portal.portal_templates.newContent(
+      portal_type='Business Template',
+      title=self.id(),
+      template_path_list=(
+        'portal_categories/test_category/**'
+      ),
+      template_base_category_list=['test_category'],
+    )
+    self.tic()
+    business_template.build()
+    self.tic()
+    export_dir = tempfile.mkdtemp()
+    try:
+      business_template.export(path=export_dir, local=True)
+      self.tic()
+      new_business_template_version_1 = self.portal.portal_templates.download(
+         url='file://%s' % export_dir)
+    finally:
+      shutil.rmtree(export_dir)
+
+    # Apply the changes and build a second version of business template
+    base_category.manage_delObjects(['removed_container'])
+
+    business_template.build()
+    self.tic()
+    export_dir = tempfile.mkdtemp()
+    try:
+      business_template.export(path=export_dir, local=True)
+      self.tic()
+      self.portal.portal_categories.manage_delObjects(['test_category'])
+      self.tic()
+      new_business_template_version_1.install()
+      self.portal.portal_categories.test_category.removed_container.setTitle(
+        'modifications are saved in trash')
+      self.tic()
+      self.portal.portal_templates.updateBusinessTemplateFromUrl(
+        download_url='file://%s' % export_dir
+      )
+    finally:
+      shutil.rmtree(export_dir)
+    self.tic()
+    # documents are removed
+    self.assertEqual(list(self.portal.portal_categories.test_category.objectValues()), [])
+    # and they are saved in portal trash
+    trash_bin, = [x for x in self.portal.portal_trash.contentValues() if x.getId().startswith(self.id())]
+    self.assertEqual(
+        trash_bin.restrictedTraverse('portal_categories_items/test_category/removed_container').getTitle(),
+        'modifications are saved in trash')
+
   def test_update_business_template_with_template_keep_path_list_catalog_method(self):
     """Tests for `template_keep_path_list` feature for the special case of catalog methods
     """

product/ERP5/Document/BusinessTemplate.py

@@ -530,12 +530,10 @@ class BaseTemplateItem(Implicit, Persistent):
     """
     remove_dict = kw.get('remove_object_dict', {})
     keys = self._objects.keys()
-    keys.sort()
-    keys.reverse()
     # if you choose remove, the object and all its subobjects will be removed
     # even if you choose backup or keep for subobjects
     # it is same behaviour for backup_and_remove, all we be save
-    for path in keys:
+    for path in sorted(keys):
       if remove_dict.has_key(path):
         action = remove_dict[path]
         if action == 'save_and_remove':