[views] Fix selection of afs rtag on creation of data-object for simplified archive units
authorSylvain Thénault <sylvain.thenault@logilab.fr>
Wed, 22 Feb 2017 14:36:38 +0100
changeset 2369 f410eba5a755
parent 2368 2880952c1aba
child 2370 36388c9744c8
[views] Fix selection of afs rtag on creation of data-object for simplified archive units There were concurrency between simplified_afs and do_ref_afs and the latter was unexpectedly selected, leading to erroneous interface proposing to link to an existing data objects, which makes no sense in the case of simplified profile or component archive units. This was because of an `is_typed_reference` unexpectedly return 1 in this case, while this is not possible because component archive unit are considered "simplified", and there may be no typed reference in a simplified profile. Fix it by extracting a `is_full_seda2_profile` function from the `full_seda2_profile` predicated. Along the way, renamed `is_typed_reference` into `typed_reference` for consistency with `full_seda2_profile` and `simplified_profile`.
entities/__init__.py
test/test_views.py
views/archiveunit.py
--- a/entities/__init__.py	Wed Feb 22 13:40:58 2017 +0100
+++ b/entities/__init__.py	Wed Feb 22 14:36:38 2017 +0100
@@ -105,9 +105,8 @@
     return 1 if entity.simplified_profile else 0
 
 
-@objectify_predicate
-def full_seda2_profile(cls, req, rset=None, entity=None, **kwargs):
-    """Predicate returning 1 score if context entity is within a full seda2 profile."""
+def is_full_seda2_profile(entity=None, rset=None):
+    """Return 1 if context entity is within a full seda2 profile, else 0."""
     entity = _seda_container_from_context(rset, entity)
     if entity is None:
         return 1
@@ -117,6 +116,12 @@
     return 0 if entity.simplified_profile else 1
 
 
+@objectify_predicate
+def full_seda2_profile(cls, req, rset=None, entity=None, **kwargs):
+    """Predicate returning 1 score if context entity is within a full seda2 profile."""
+    return is_full_seda2_profile(entity, rset)
+
+
 def rule_type_from_etype(etype):
     """Return the rule type (e.g. 'access') from an etype enclosing the information
     (e.g. 'SEDAAltAccessRulePreventInheritance', 'SEDASeqAaccessRuleRule' or 'SEDAAccessRule')
--- a/test/test_views.py	Wed Feb 22 13:40:58 2017 +0100
+++ b/test/test_views.py	Wed Feb 22 14:36:38 2017 +0100
@@ -15,6 +15,7 @@
 # with this program. If not, see <http://www.gnu.org/licenses/>.
 """cubicweb-seda unit tests for schema"""
 
+from json import dumps
 import unittest
 
 from six import text_type
@@ -28,6 +29,11 @@
 import testutils
 
 
+def entity_fields(fields):
+    return [text_type(field.name) for field in fields
+            if field.eidparam and not field.name.startswith('__')]
+
+
 class ManagementRulesTC(CubicWebTC):
     def test_rule_ref_vocabulary(self):
         with self.admin_access.client_cnx() as cnx:
@@ -300,14 +306,23 @@
             self.transfer_eid = transfer.eid
             self.archive_unit_eid = archive_unit.eid
 
-    def rule_form(self, req, etype, linkto=None):
-        # add minimal information in for to detect container even if edited entity isn't
-        # actually created
+    def create_and_link_form(self, req, etype, linkto=None, **kwargs):
+        # add minimal information in form to mimick __linkto behaviour
         if linkto is None:
             linkto = self.archive_unit_eid
         req.form['__linkto'] = 'x:{0}:y'.format(linkto)
-        rule = req.vreg['etypes'].etype_class(etype)(req)
-        return self.vreg['forms'].select('edition', req, entity=rule)
+        return self._create_etype_form(req, etype, **kwargs)
+
+    def create_inlined_form(self, req, etype, parent=None, **kwargs):
+        # add minimal information in form to mimick inlined form behaviour
+        if parent is None:
+            parent = self.archive_unit_eid
+        req.form['arg'] = [dumps(text_type(parent))]
+        return self._create_etype_form(req, etype, **kwargs)
+
+    def _create_etype_form(self, req, etype, **kwargs):
+        entity = req.vreg['etypes'].etype_class(etype)(req)
+        return self.vreg['forms'].select('edition', req, entity=entity, **kwargs)
 
     def assertInlinedFields(self, form, expected):
         """Assert the given inlined form as the expected set of inlined forms, each defined by its
@@ -336,14 +351,16 @@
                 if rule_type == 'classification':
                     continue
                 with self.subTest(rule_type=rule_type):
-                    form = self.rule_form(req, 'SEDA{0}Rule'.format(rule_type.capitalize()),
-                                          linkto=self.transfer_eid)
+                    form = self.create_and_link_form(
+                        req, 'SEDA{0}Rule'.format(rule_type.capitalize()),
+                        linkto=self.transfer_eid)
                     self.assertInlinedFields(form, [
                         ('seda_seq_{0}_rule_rule'.format(rule_type),
                          'InlineAddNewLinkView'),
                     ])
-            form = self.rule_form(req, 'SEDAClassificationRule'.format(rule_type.capitalize()),
-                                  linkto=self.transfer_eid)
+            form = self.create_and_link_form(
+                req, 'SEDAClassificationRule'.format(rule_type.capitalize()),
+                linkto=self.transfer_eid)
             self.assertInlinedFields(form, [
                 ('seda_seq_classification_rule_rule', 'InlineAddNewLinkView'),
                 ('seda_classification_reassessing_date', 'InlineAddNewLinkView'),
@@ -354,8 +371,9 @@
         with self.admin_access.web_request() as req:
             for rule_type in ('appraisal', 'access'):
                 with self.subTest(rule_type=rule_type):
-                    form = self.rule_form(req, 'SEDA{0}Rule'.format(rule_type.capitalize()),
-                                          linkto=self.transfer_eid)
+                    form = self.create_and_link_form(
+                        req, 'SEDA{0}Rule'.format(rule_type.capitalize()),
+                        linkto=self.transfer_eid)
                     self.assertInlinedFields(form, [
                         ('seda_seq_{0}_rule_rule'.format(rule_type),
                          'RuleRuleInlineEntityCreationFormView'),
@@ -364,7 +382,7 @@
     def test_rule_form(self):
         with self.admin_access.web_request() as req:
             for rule_type in ('access', 'appraisal'):
-                form = self.rule_form(req, 'SEDA{0}Rule'.format(rule_type.capitalize()))
+                form = self.create_and_link_form(req, 'SEDA{0}Rule'.format(rule_type.capitalize()))
                 self.assertInlinedFields(form, [
                     ('seda_seq_{0}_rule_rule'.format(rule_type),
                      'RuleRuleInlineEntityCreationFormView'),
@@ -376,7 +394,8 @@
     def test_rule_rule_form(self):
         with self.admin_access.web_request() as req:
             for rule_type in ('access', 'appraisal'):
-                form = self.rule_form(req, 'SEDASeq{0}RuleRule'.format(rule_type.capitalize()))
+                form = self.create_and_link_form(
+                    req, 'SEDASeq{0}RuleRule'.format(rule_type.capitalize()))
                 other_fields = [text_type(field.name)
                                 for field in form.fields if not hasattr(field, 'view')]
                 self.assertNotIn('user_cardinality', other_fields)
@@ -389,7 +408,7 @@
     def test_prevent_inheritance_form(self):
         with self.admin_access.web_request() as req:
             for rule_type in ('access', 'appraisal'):
-                form = self.rule_form(req, 'SEDAAlt{0}RulePreventInheritance'.format(
+                form = self.create_and_link_form(req, 'SEDAAlt{0}RulePreventInheritance'.format(
                     rule_type.capitalize()))
                 self.assertInlinedFields(form, [
                     ('seda_prevent_inheritance', 'PreventInheritanceInlineEntityCreationFormView'),
@@ -397,6 +416,29 @@
                 self.assertNoRemove(form, 'seda_prevent_inheritance', 'object')
                 self.assertEqual(form.form_renderer_id, 'not-an-alt')
 
+    def test_create_data_object_full(self):
+        with self.admin_access.web_request() as req:
+            req.entity_from_eid(self.transfer_eid).cw_set(simplified_profile=False)
+            req.cnx.commit()
+            form = self.create_inlined_form(req, 'SEDADataObjectReference', formtype='inlined')
+            self.assertEqual(entity_fields(form.fields),
+                             ['seda_data_object_reference_id'])
+
+    def test_create_data_object_simplified(self):
+        with self.admin_access.web_request() as req:
+            form = self.create_inlined_form(req, 'SEDADataObjectReference', formtype='inlined')
+            self.assertEqual(entity_fields(form.fields),
+                             [])
+
+    def test_create_data_object_from_archive_unit_component(self):
+        with self.admin_access.web_request() as req:
+            unit = testutils.create_archive_unit(None, cnx=req.cnx)[0]
+            req.cnx.commit()
+            form = self.create_inlined_form(req, 'SEDADataObjectReference', formtype='inlined',
+                                            parent=unit.eid)
+            self.assertEqual(entity_fields(form.fields),
+                             [])
+
 
 class CloneActionsTC(CubicWebTC):
 
--- a/views/archiveunit.py	Wed Feb 22 13:40:58 2017 +0100
+++ b/views/archiveunit.py	Wed Feb 22 14:36:38 2017 +0100
@@ -30,7 +30,8 @@
 from cubes.relationwidget import views as rwdg
 
 from ..xsd import un_camel_case
-from ..entities import full_seda2_profile, simplified_profile, parent_and_container
+from ..entities import (is_full_seda2_profile, full_seda2_profile,
+                        simplified_profile, parent_and_container)
 from ..entities.itree import parent_archive_unit
 from . import (CONTENT_ETYPE, add_subobject_link,
                rtags_from_xsd_element, rtags_from_rtype_role_targets, copy_rtag, has_rel_perm)
@@ -101,10 +102,15 @@
 
 
 @objectify_predicate
-def is_typed_reference(cls, req, entity=None, **kwargs):
-    """Return positive score for content's typed data object references (IsPartOf, VersionOf, etc.), not
-    those starting directly from archive unit.
+def typed_reference(cls, req, entity=None, **kwargs):
+    """Return positive score for content's typed data object references (IsPartOf, VersionOf, etc.),
+    not those starting directly from archive unit.
     """
+    # code below don't handle properly the case where entity isn't yet created while rtype isn't
+    # specified (e.g. because it's an inlined form). To avoid false positive, we've to also ensure
+    # we're within a full seda 2 profile (typed references aren't allowed in simplified profiles)
+    if not is_full_seda2_profile(entity, kwargs.get('rset')):
+        return 0
     if entity is None or not entity.has_eid():
         try:
             rtype, eid, role = req.form['__linkto'].split(':')
@@ -696,7 +702,7 @@
 
 
 do_ref_afs = copy_rtag(afs, __name__,
-                       is_instance('SEDADataObjectReference') & is_typed_reference())
+                       is_instance('SEDADataObjectReference') & typed_reference())
 do_ref_afs.tag_attribute(('SEDADataObjectReference', 'user_cardinality'), 'main', 'hidden')
 
 for etype in ('SEDAAltIsPartOfArchiveUnitRefId',