[edit controller] fix handling of removal of subentities from an edit form
In CubicWeb 3.18 the semantics of dropping a composite relation were changed.
It had before the effect of deleting the subentities.
Now the edit controller must handle this by itself (it was not adapted then by
mistake).
So if it detects the removal of a composite relation (on the browser side, the
user clicked on the "remove" action), the subentity is now explicitly deleted.
It was chosen not to change the form generation but the EditController:
- to ensure pre-3.18 forms backward compat
- to avoid the introduction of a specific entity-deletion API (RQL is probably
our best API for complex DB operations)
Related to #8529868.
--- a/web/test/data/schema.py Wed Nov 25 16:28:41 2015 +0100
+++ b/web/test/data/schema.py Fri Dec 04 14:56:20 2015 +0100
@@ -93,5 +93,22 @@
title = String(maxsize=32, required=True, fulltextindexed=True)
concerns = SubjectRelation('Project', composite='object')
+class Filesystem(EntityType):
+ name = String()
+
+class parent_fs(RelationDefinition):
+ name = 'parent'
+ subject = 'Directory'
+ object = 'Filesystem'
+
+class Directory(EntityType):
+ name = String()
+
+class parent_directory(RelationDefinition):
+ name = 'parent'
+ subject = 'Directory'
+ object = 'Directory'
+ composite = 'object'
+
# used by windmill for `test_edit_relation`
from cubes.folder.schema import Folder
--- a/web/test/unittest_application.py Wed Nov 25 16:28:41 2015 +0100
+++ b/web/test/unittest_application.py Fri Dec 04 14:56:20 2015 +0100
@@ -257,6 +257,147 @@
{'login-subject': u'the value "admin" is already used, use another one'})
self.assertEqual(forminfo['values'], req.form)
+ def _edit_parent(self, dir_eid, parent_eid, role='subject',
+ etype='Directory'):
+ parent_eid = parent_eid or '__cubicweb_internal_field__'
+ with self.admin_access.web_request() as req:
+ req.form = {
+ 'eid': unicode(dir_eid),
+ '__maineid': unicode(dir_eid),
+ '__type:%s' % dir_eid: etype,
+ '_cw_entity_fields:%s' % dir_eid: 'parent-%s' % role,
+ 'parent-%s:%s' % (role, dir_eid): parent_eid,
+ }
+ self.expect_redirect_handle_request(req)
+
+ def test_subject_subentity_removal(self):
+ """Editcontroller: detaching a composite relation removes the subentity
+ (edit from the subject side)
+ """
+ with self.admin_access.repo_cnx() as cnx:
+ topd = cnx.create_entity('Directory', name=u'topd')
+ sub1 = cnx.create_entity('Directory', name=u'sub1', parent=topd)
+ sub2 = cnx.create_entity('Directory', name=u'sub2', parent=topd)
+ cnx.commit()
+
+ self._edit_parent(sub1.eid, parent_eid=None)
+
+ with self.admin_access.repo_cnx() as cnx:
+ self.assertTrue(cnx.find('Directory', eid=topd.eid))
+ self.assertFalse(cnx.find('Directory', eid=sub1.eid))
+ self.assertTrue(cnx.find('Directory', eid=sub2.eid))
+
+ def test_object_subentity_removal(self):
+ """Editcontroller: detaching a composite relation removes the subentity
+ (edit from the object side)
+ """
+ with self.admin_access.repo_cnx() as cnx:
+ topd = cnx.create_entity('Directory', name=u'topd')
+ sub1 = cnx.create_entity('Directory', name=u'sub1', parent=topd)
+ sub2 = cnx.create_entity('Directory', name=u'sub2', parent=topd)
+ cnx.commit()
+
+ self._edit_parent(topd.eid, parent_eid=sub1.eid, role='object')
+
+ with self.admin_access.repo_cnx() as cnx:
+ self.assertTrue(cnx.find('Directory', eid=topd.eid))
+ self.assertTrue(cnx.find('Directory', eid=sub1.eid))
+ self.assertFalse(cnx.find('Directory', eid=sub2.eid))
+
+ def test_reparent_subentity(self):
+ "Editcontroller: re-parenting a subentity does not remove it"
+ with self.admin_access.repo_cnx() as cnx:
+ top1 = cnx.create_entity('Directory', name=u'top1')
+ top2 = cnx.create_entity('Directory', name=u'top2')
+ subd = cnx.create_entity('Directory', name=u'subd', parent=top1)
+ cnx.commit()
+
+ self._edit_parent(subd.eid, parent_eid=top2.eid)
+
+ with self.admin_access.repo_cnx() as cnx:
+ self.assertTrue(cnx.find('Directory', eid=top1.eid))
+ self.assertTrue(cnx.find('Directory', eid=top2.eid))
+ self.assertTrue(cnx.find('Directory', eid=subd.eid))
+ self.assertEqual(
+ cnx.find('Directory', eid=subd.eid).one().parent[0], top2)
+
+ def test_subject_mixed_composite_subentity_removal_1(self):
+ """Editcontroller: detaching several subentities respects each rdef's
+ compositeness - Remove non composite
+ """
+ with self.admin_access.repo_cnx() as cnx:
+ topd = cnx.create_entity('Directory', name=u'topd')
+ fs = cnx.create_entity('Filesystem', name=u'/tmp')
+ subd = cnx.create_entity('Directory', name=u'subd',
+ parent=(topd, fs))
+ cnx.commit()
+
+ self._edit_parent(subd.eid, parent_eid=topd.eid)
+
+ with self.admin_access.repo_cnx() as cnx:
+ self.assertTrue(cnx.find('Directory', eid=topd.eid))
+ self.assertTrue(cnx.find('Directory', eid=subd.eid))
+ self.assertTrue(cnx.find('Filesystem', eid=fs.eid))
+ self.assertEqual(cnx.find('Directory', eid=subd.eid).one().parent,
+ [topd,])
+
+ def test_subject_mixed_composite_subentity_removal_2(self):
+ """Editcontroller: detaching several subentities respects each rdef's
+ compositeness - Remove composite
+ """
+ with self.admin_access.repo_cnx() as cnx:
+ topd = cnx.create_entity('Directory', name=u'topd')
+ fs = cnx.create_entity('Filesystem', name=u'/tmp')
+ subd = cnx.create_entity('Directory', name=u'subd',
+ parent=(topd, fs))
+ cnx.commit()
+
+ self._edit_parent(subd.eid, parent_eid=fs.eid)
+
+ with self.admin_access.repo_cnx() as cnx:
+ self.assertTrue(cnx.find('Directory', eid=topd.eid))
+ self.assertFalse(cnx.find('Directory', eid=subd.eid))
+ self.assertTrue(cnx.find('Filesystem', eid=fs.eid))
+
+ def test_object_mixed_composite_subentity_removal_1(self):
+ """Editcontroller: detaching several subentities respects each rdef's
+ compositeness - Remove non composite
+ """
+ with self.admin_access.repo_cnx() as cnx:
+ topd = cnx.create_entity('Directory', name=u'topd')
+ fs = cnx.create_entity('Filesystem', name=u'/tmp')
+ subd = cnx.create_entity('Directory', name=u'subd',
+ parent=(topd, fs))
+ cnx.commit()
+
+ self._edit_parent(fs.eid, parent_eid=None, role='object',
+ etype='Filesystem')
+
+ with self.admin_access.repo_cnx() as cnx:
+ self.assertTrue(cnx.find('Directory', eid=topd.eid))
+ self.assertTrue(cnx.find('Directory', eid=subd.eid))
+ self.assertTrue(cnx.find('Filesystem', eid=fs.eid))
+ self.assertEqual(cnx.find('Directory', eid=subd.eid).one().parent,
+ [topd,])
+
+ def test_object_mixed_composite_subentity_removal_2(self):
+ """Editcontroller: detaching several subentities respects each rdef's
+ compositeness - Remove composite
+ """
+ with self.admin_access.repo_cnx() as cnx:
+ topd = cnx.create_entity('Directory', name=u'topd')
+ fs = cnx.create_entity('Filesystem', name=u'/tmp')
+ subd = cnx.create_entity('Directory', name=u'subd',
+ parent=(topd, fs))
+ cnx.commit()
+
+ self._edit_parent(topd.eid, parent_eid=None, role='object')
+
+ with self.admin_access.repo_cnx() as cnx:
+ self.assertTrue(cnx.find('Directory', eid=topd.eid))
+ self.assertFalse(cnx.find('Directory', eid=subd.eid))
+ self.assertTrue(cnx.find('Filesystem', eid=fs.eid))
+
def test_ajax_view_raise_arbitrary_error(self):
class ErrorAjaxView(view.View):
__regid__ = 'test.ajax.error'
--- a/web/views/editcontroller.py Wed Nov 25 16:28:41 2015 +0100
+++ b/web/views/editcontroller.py Fri Dec 04 14:56:20 2015 +0100
@@ -27,7 +27,7 @@
from rql.utils import rqlvar_maker
-from cubicweb import Binary, ValidationError
+from cubicweb import Binary, ValidationError, neg_role
from cubicweb.view import EntityAdapter
from cubicweb.predicates import is_instance
from cubicweb.web import (INTERNAL_FIELD_VALUE, RequestError, NothingToEdit,
@@ -186,6 +186,7 @@
# deserves special treatment
req.data['pending_inlined'] = defaultdict(set)
req.data['pending_others'] = set()
+ req.data['pending_composite_delete'] = set()
try:
for formparams in self._ordered_formparams():
eid = self.edit_entity(formparams)
@@ -204,6 +205,13 @@
# then execute rql to set all relations
for querydef in self.relations_rql:
self._cw.execute(*querydef)
+ # delete pending composite
+ for entity, rtype, role, targettype in req.data['pending_composite_delete']:
+ # Check the composite relation was not re-set in the same form
+ # (see test_reparent_subentity in web/test/unittest_application.py)
+ entity.cw_clear_relation_cache(rtype, role)
+ if not entity.related(rtype, role=role, targettypes=(targettype,)):
+ entity.cw_delete()
# XXX this processes *all* pending operations of *all* entities
if '__delete' in req.form:
todelete = req.list_form_param('__delete', req.form, pop=True)
@@ -303,6 +311,12 @@
if value is None or value == origvalues:
continue # not edited / not modified / to do later
+ unlinked_eids = origvalues - value
+ if unlinked_eids:
+ # Special handling of composite relation removal
+ self.handle_composite_removal(
+ form, field, unlinked_eids)
+
if rschema.inlined and rqlquery is not None and field.role == 'subject':
self.handle_inlined_relation(form, field, value, origvalues, rqlquery)
elif form.edited_entity.has_eid():
@@ -313,6 +327,34 @@
except ProcessFormError as exc:
self.errors.append((field, exc))
+ def handle_composite_removal(self, form, field, removed_values):
+ """
+ In EditController-handled forms, when the user removes a composite
+ relation, it triggers the removal of the related entity in the
+ composite. This is where this happens.
+
+ See for instance test_subject_subentity_removal in
+ web/test/unittest_application.py.
+ """
+ rschema = self._cw.vreg.schema.rschema(field.name)
+ for unlinked_eid in removed_values:
+ unlinked_entity = self._cw.entity_from_eid(unlinked_eid)
+ rdef = rschema.role_rdef(form.edited_entity.cw_etype,
+ unlinked_entity.cw_etype,
+ field.role)
+ if rdef.composite is not None:
+ if rdef.composite == field.role:
+ targettype = form.edited_entity.e_schema
+ to_be_removed = unlinked_entity
+ else:
+ targettype = unlinked_entity.e_schema
+ to_be_removed = form.edited_entity
+ self.info('Scheduling removal of %s as composite relation '
+ '%s was removed', to_be_removed, rdef)
+ form._cw.data['pending_composite_delete'].add(
+ (to_be_removed, field.name,
+ neg_role(rdef.composite), targettype))
+
def handle_inlined_relation(self, form, field, values, origvalues, rqlquery):
"""handle edition for the (rschema, x) relation of the given entity
"""