Commit 0cacb444 authored by Jérome Perrin's avatar Jérome Perrin

BusinessTemplate: fix simulataneous update of categories and paths

This addresses the problem of
https://nexedijs.erp5.net/#/bug_module/20180719-135FAA8 a KeyError
raised when some categories in a subtree are modified and some are
removed and the corresponding base category is also installed as a base
category.

The problem was that both CategoryTemplateItem, which is in charge of
updating the base category and PathTemplateItem, which is in charge of
updating the categories listed as path both use the same
ObjectTemplateItem.install method, with the same object_to_update dict.
ObjectTemplateItem.install uninstall all objects that are listed in
object_to_update and not in self._objects so something like this
happened when business template from
test_update_business_template_with_category_having_subcategory_tree_modified
is updated:

  1. PathTemplateItem.install is called for the base category,
portal_categories/test_category/modified/removed looks removed, so it is
backed up. Because the the parent paths are not parts of self._objects,
trash tool will create simple trash folder for
portal_categories/test_category/modified

  2. PathTemplateItem.install is called for the paths,
portal_categories/test_category/modified is modified, so the previous
version will be backed up. At this point trash tool looks in the trash
bin and the path for portal_categories/test_category is already present,
so trash tool sees that path exists and does not return subobjects, so
after portal_categories/test_category/modified is modified, the subjects
such as
portal_categories/test_category/modified/container_in_which_child_is_added
are not restored and creating 'added' caused a
KeyError('container_in_which_child_is_added')

The approach is to make CategoryTemplateItem.install only consider base
categories - ie. objects where path is portal_categories/* and not the
subobjects, because they don't belong to CategoryTemplateItem but to
PathTemplateItem.
Co-authored-by: Georgios Dagkakis's avatarGeorgios Dagkakis <georgios.dagkakis@nexedi.com>
parent 30ef111a
......@@ -6854,6 +6854,86 @@ class TestBusinessTemplate(BusinessTemplateMixin):
{'test_document': ('Removed but should be kept', 'Path')},
new_bt.preinstall())
def test_update_business_template_with_category_having_subcategory_tree_modified(self):
"""Non regression test for a case where some categories in a subtrees are added and
some are removed.
Updating from:
portal_categories/test_category/modified
portal_categories/test_category/modified/container_in_which_child_is_added
portal_categories/test_category/modified/container_in_which_child_is_added/child_kept
portal_categories/test_category/modified/removed
to:
portal_categories/test_category/modified <-- this will be modified
portal_categories/test_category/modified/container_in_which_child_is_added
portal_categories/test_category/modified/container_in_which_child_is_added/child_kept
portal_categories/test_category/modified/container_in_which_child_is_added/added
was causing when test_category was added both as a base category and as paths.
This was the cause of KeyError when updating erp5_accounting_l10n_fr
"""
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')
parent_category = base_category.newContent(portal_type='Category', id='modified')
parent_category.newContent(portal_type='Category', id='container_in_which_child_is_added')
parent_category.newContent(portal_type='Category', id='removed')
parent_category.container_in_which_child_is_added.newContent(portal_type='Category', id='child_kept')
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
parent_category.setTitle('modified')
parent_category.container_in_which_child_is_added.newContent(portal_type='Category', id='added')
parent_category.manage_delObjects(['removed'])
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.tic()
portal_categories.test_category.modified.container_in_which_child_is_added.setTitle(
'This path should not be reinstalled during update'
)
self.tic()
self.portal.portal_templates.updateBusinessTemplateFromUrl(
download_url='file://%s' % export_dir
)
finally:
shutil.rmtree(export_dir)
self.tic()
self.assertEqual(
'This path should not be reinstalled during update',
portal_categories.test_category.modified.container_in_which_child_is_added.getTitle())
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
"""
......
......@@ -1950,6 +1950,17 @@ class CategoryTemplateItem(ObjectTemplateItem):
# reset accessors if we installed a new category
self.portal_types.resetDynamicDocumentsOnceAtTransactionBoundary()
def install(self, context, trashbin, **kw):
# Only update base categories, the category they contain will be installed/updated
# as PathTemplateItem.install
kw['object_to_update'] = {
path: action
for (path, action) in kw['object_to_update'].items()
if path.split('/')[:-1] == ['portal_categories'] or path in self._objects
}
return super(CategoryTemplateItem, self).install(context, trashbin, **kw)
class SkinTemplateItem(ObjectTemplateItem):
def __init__(self, id_list, tool_id='portal_skins', **kw):
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment