[edit controller] Fix composite entity reparenting when inlined
authorFlorent Cayré <florent.cayre@logilab.fr>
Tue, 26 Jan 2016 14:04:45 +0100
changeset 11066 dcbb64d3a1d9
parent 11065 c7dbd10648e6
child 11098 7eb9e3e254bc
[edit controller] Fix composite entity reparenting when inlined This implies handling the reparenting case sooner, otherwise the child entity modifications are cancelled (see 113e9da47afc). Add a new test to ensure non-regression in all previous tests. Closes #10291977.
web/test/data/schema.py
web/test/unittest_application.py
web/views/editcontroller.py
--- a/web/test/data/schema.py	Mon Dec 14 14:29:22 2015 +0100
+++ b/web/test/data/schema.py	Tue Jan 26 14:04:45 2016 +0100
@@ -92,6 +92,11 @@
 class Ticket(EntityType):
     title = String(maxsize=32, required=True, fulltextindexed=True)
     concerns = SubjectRelation('Project', composite='object')
+    in_version = SubjectRelation('Version', composite='object',
+                                 cardinality='?*', inlined=True)
+
+class Version(EntityType):
+    name = String(required=True)
 
 class Filesystem(EntityType):
     name = String()
--- a/web/test/unittest_application.py	Mon Dec 14 14:29:22 2015 +0100
+++ b/web/test/unittest_application.py	Tue Jan 26 14:04:45 2016 +0100
@@ -275,6 +275,23 @@
                  if not key.startswith('_')])
             self.expect_redirect_handle_request(req)
 
+    def _edit_in_version(self, ticket_eid, version_eid, **kwargs):
+        version_eid = version_eid or '__cubicweb_internal_field__'
+        with self.admin_access.web_request() as req:
+            req.form = {
+                'eid': unicode(ticket_eid),
+                '__maineid': unicode(ticket_eid),
+                '__type:%s' % ticket_eid: 'Ticket',
+                'in_version-subject:%s' % ticket_eid: version_eid,
+            }
+            req.form.update(kwargs)
+            req.form['_cw_entity_fields:%s' % ticket_eid] = ','.join(
+                ['in_version-subject'] +
+                [key.split(':')[0]
+                 for key in kwargs.keys()
+                 if not key.startswith('_')])
+            self.expect_redirect_handle_request(req)
+
     def test_create_and_link_directories(self):
         with self.admin_access.web_request() as req:
             req.form = {
@@ -374,6 +391,25 @@
             self.assertEqual(
                 cnx.find('Directory', eid=subd.eid).one().parent[0], top2)
 
+    def test_reparent_subentity_inlined(self):
+        """Editcontroller: re-parenting a subentity does not remove it
+        (inlined case)"""
+        with self.admin_access.repo_cnx() as cnx:
+            version1 = cnx.create_entity('Version', name=u'version1')
+            version2 = cnx.create_entity('Version', name=u'version2')
+            ticket = cnx.create_entity('Ticket', title=u'ticket',
+                                       in_version=version1)
+            cnx.commit()
+
+        self._edit_in_version(ticket.eid, version_eid=version2.eid)
+
+        with self.admin_access.repo_cnx() as cnx:
+            self.assertTrue(cnx.find('Version', eid=version1.eid))
+            self.assertTrue(cnx.find('Version', eid=version2.eid))
+            self.assertTrue(cnx.find('Ticket', eid=ticket.eid))
+            self.assertEqual(
+                cnx.find('Ticket', eid=ticket.eid).one().in_version[0], version2)
+
     def test_subject_mixed_composite_subentity_removal_1(self):
         """Editcontroller: detaching several subentities respects each rdef's
         compositeness - Remove non composite
--- a/web/views/editcontroller.py	Mon Dec 14 14:29:22 2015 +0100
+++ b/web/views/editcontroller.py	Tue Jan 26 14:04:45 2016 +0100
@@ -27,7 +27,7 @@
 
 from rql.utils import rqlvar_maker
 
-from cubicweb import Binary, ValidationError, neg_role
+from cubicweb import Binary, ValidationError
 from cubicweb.view import EntityAdapter
 from cubicweb.predicates import is_instance
 from cubicweb.web import (INTERNAL_FIELD_VALUE, RequestError, NothingToEdit,
@@ -209,12 +209,8 @@
         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()
+        for entity in req.data['pending_composite_delete']:
+            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)
@@ -313,15 +309,15 @@
                     origvalues = set(data[0] for data in entity.related(field.name, field.role).rows)
                 else:
                     origvalues = set()
-
                 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, rqlquery)
+                        form, field, unlinked_eids, value, rqlquery)
 
                 if rschema.inlined and rqlquery is not None and field.role == 'subject':
                     self.handle_inlined_relation(form, field, value, origvalues, rqlquery)
@@ -333,7 +329,8 @@
         except ProcessFormError as exc:
             self.errors.append((field, exc))
 
-    def handle_composite_removal(self, form, field, removed_values, rqlquery):
+    def handle_composite_removal(self, form, field,
+                                 removed_values, new_values, rqlquery):
         """
         In EditController-handled forms, when the user removes a composite
         relation, it triggers the removal of the related entity in the
@@ -343,6 +340,8 @@
         web/test/unittest_application.py.
         """
         rschema = self._cw.vreg.schema.rschema(field.name)
+        new_value_etypes = set(self._cw.entity_from_eid(eid).cw_etype
+                               for eid in new_values)
         for unlinked_eid in removed_values:
             unlinked_entity = self._cw.entity_from_eid(unlinked_eid)
             rdef = rschema.role_rdef(form.edited_entity.cw_etype,
@@ -350,19 +349,18 @@
                                      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
+                    if unlinked_entity.cw_etype in new_value_etypes:
+                        # This is a same-rdef re-parenting: do not remove the entity
+                        continue
                     to_be_removed = form.edited_entity
                     self.info('Edition of %s is cancelled (deletion requested)',
                               to_be_removed)
                     rqlquery.canceled = True
                 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))
+                form._cw.data['pending_composite_delete'].add(to_be_removed)
 
     def handle_inlined_relation(self, form, field, values, origvalues, rqlquery):
         """handle edition for the (rschema, x) relation of the given entity