From 3a81cf88f85966696f97f4b3bd961a39e7baaf8d Mon Sep 17 00:00:00 2001
From: Olivier Dony <odo@openerp.com>
Date: Thu, 14 Jun 2012 16:46:33 +0200
Subject: [PATCH] [IMP] Improve view validation to based on fields_view_get
 rendering, not just raw arch

This will allow improved validation of inherited
views, which is not possible when only the raw
arch is validated on its own - without context
many things cannot be verified.
Calling fields_view_get() also catches early all
mistakes that require dynamic validation, like
wrong XPath expressions (parent view contains
no match).
In order to have current addons pass the improved
validation the RNG had to be fixed to support
the new @modifiers attribute added by fields_view_get()
itself on many view elements, and a few missing
valid attributes, like @invisible on <filter>
and <group>. The latter had never been used
as part of the view architecture but appear
as a result of the handling of @groups
restrictions on view elements, and must
be allowed by the RNG schema.

bzr revid: odo@openerp.com-20120614144633-31c642s7q7f28o6b
---
 openerp/addons/base/ir/ir_ui_view.py | 21 +++++++++++----------
 openerp/addons/base/rng/view.rng     | 24 ++++++++++++++++++++++++
 2 files changed, 35 insertions(+), 10 deletions(-)

diff --git a/openerp/addons/base/ir/ir_ui_view.py b/openerp/addons/base/ir/ir_ui_view.py
index 4d7b0f45d06..ac3644a28f7 100644
--- a/openerp/addons/base/ir/ir_ui_view.py
+++ b/openerp/addons/base/ir/ir_ui_view.py
@@ -94,21 +94,26 @@ class view(osv.osv):
            its inherited views, by rendering it using ``fields_view_get()``.
            
            @param browse_record view: view to validate
-           @return: True if the view hierarchy was rendered without any error, False if an error occurred.  
+           @return: the rendered definition (arch) of the view, always utf-8 bytestring (legacy convention)
+               if no error occurred, else False.  
         """
         try:
-            self.pool.get(view.model).fields_view_get(cr, uid, view_id=view.id, view_type=view.type, context=context)
-            return True
+            fvg = self.pool.get(view.model).fields_view_get(cr, uid, view_id=view.id, view_type=view.type, context=context)
+            return fvg['arch']
         except:
             _logger.exception("Can't render view %s for model: %s", view.xml_id, view.model)
             return False
 
     def _check_xml(self, cr, uid, ids, context=None):
         for view in self.browse(cr, uid, ids, context):
+            # Sanity check: the view should not break anything upon rendering!
+            view_arch_utf8 = self._check_render_view(cr, uid, view, context=context)
+            # always utf-8 bytestring - legacy convention
+            if not view_arch_utf8: return False
+
             # RNG-based validation is not possible anymore with 7.0 forms
-            # TODO 7.0: provide alternative assertion-based validation!
-            # TODO 7.0: and do the tests on the result of fields_view_get instead of each arch.
-            view_docs = [etree.fromstring(view.arch.encode('utf8'))]
+            # TODO 7.0: provide alternative assertion-based validation of view_arch_utf8
+            view_docs = [etree.fromstring(view_arch_utf8)]
             if view_docs[0].tag == 'data':
                 # A <data> element is a wrapper for multiple root nodes
                 view_docs = view_docs[0]
@@ -118,10 +123,6 @@ class view(osv.osv):
                     for error in validator.error_log:
                         _logger.error(tools.ustr(error))
                     return False
-
-            # Second sanity check: the view should not break anything upon rendering!
-            if not self._check_render_view(cr, uid, view, context=context):
-                return False
         return True
 
     _constraints = [
diff --git a/openerp/addons/base/rng/view.rng b/openerp/addons/base/rng/view.rng
index 54d08edc596..22d6ee12a88 100644
--- a/openerp/addons/base/rng/view.rng
+++ b/openerp/addons/base/rng/view.rng
@@ -39,6 +39,19 @@
         </rng:optional>
     </rng:define>
 
+    <rng:define name="modifiable">
+        <rng:optional>
+            <!-- @modifiers contains a JSON map unifying the various
+                 modifier attributes: @readonly, @required, @invisible.
+                 Each attribute is a key, mapped to a JSON list representing
+                 a condition expressed as an OpenERP `domain` filter
+                 Only some of the modifier keys make sense on some
+                 elements, for example <filter> and <group> only support
+                 `invisible`. -->
+            <rng:attribute name="modifiers"/>
+        </rng:optional>
+    </rng:define>
+
     <rng:define name="access_rights">
         <rng:optional>
             <rng:attribute name="groups"/>
@@ -294,6 +307,8 @@
         <rng:element name="label">
             <rng:ref name="overload"/>
             <rng:ref name="access_rights"/>
+            <rng:ref name="modifiable"/>
+            <rng:optional><rng:attribute name="invisible"/></rng:optional>
             <rng:optional><rng:attribute name="align"/></rng:optional>
             <rng:optional><rng:attribute name="nolabel"/></rng:optional>
             <rng:optional><rng:attribute name="colspan"/></rng:optional>
@@ -393,6 +408,7 @@
         <rng:element name="page">
             <rng:ref name="overload"/>
             <rng:ref name="access_rights"/>
+            <rng:ref name="modifiable"/>
             <rng:optional><rng:attribute name="string"/></rng:optional>
             <rng:optional><rng:attribute name="name"/></rng:optional>
             <rng:optional><rng:attribute name="attrs"/></rng:optional>
@@ -443,6 +459,8 @@
         <rng:element name="separator">
             <rng:ref name="overload"/>
             <rng:ref name="access_rights"/>
+            <rng:ref name="modifiable"/>
+            <rng:optional><rng:attribute name="invisible"/></rng:optional>
             <rng:optional><rng:attribute name="name"/></rng:optional>
             <rng:optional><rng:attribute name="colspan"/></rng:optional>
             <rng:optional><rng:attribute name="rowspan"/></rng:optional>
@@ -508,6 +526,7 @@
             <rng:attribute name="name" />
             <rng:ref name="overload"/>
             <rng:ref name="access_rights"/>
+            <rng:ref name="modifiable"/>
             <rng:optional><rng:attribute name="domain_filter"/></rng:optional>
             <rng:optional><rng:attribute name="attrs"/></rng:optional>
             <rng:optional><rng:attribute name="string"/></rng:optional>
@@ -587,6 +606,7 @@
         <rng:element name="group">
             <rng:ref name="overload"/>
             <rng:ref name="access_rights"/>
+            <rng:ref name="modifiable"/>
             <rng:optional><rng:attribute name="attrs"/></rng:optional>
             <rng:optional><rng:attribute name="colspan"/></rng:optional>
             <rng:optional><rng:attribute name="rowspan"/></rng:optional>
@@ -598,6 +618,7 @@
             <rng:optional><rng:attribute name="width"/></rng:optional>
             <rng:optional><rng:attribute name="name"/></rng:optional>
             <rng:optional><rng:attribute name="color" /></rng:optional>
+            <rng:optional><rng:attribute name="invisible"/></rng:optional>
             <rng:ref name="container"/>
         </rng:element>
     </rng:define>
@@ -640,6 +661,7 @@
         <rng:element name="button">
             <rng:ref name="overload"/>
             <rng:ref name="access_rights"/>
+            <rng:ref name="modifiable"/>
             <rng:optional><rng:attribute name="attrs"/></rng:optional>
             <rng:optional><rng:attribute name="invisible"/></rng:optional>
             <rng:optional><rng:attribute name="name" /></rng:optional>
@@ -684,7 +706,9 @@
         <rng:element name="filter">
             <rng:ref name="overload"/>
             <rng:ref name="access_rights"/>
+            <rng:ref name="modifiable"/>
             <rng:optional><rng:attribute name="attrs"/></rng:optional>
+            <rng:optional><rng:attribute name="invisible"/></rng:optional>
             <rng:optional><rng:attribute name="name" /></rng:optional>
             <rng:optional><rng:attribute name="separator" /></rng:optional>
             <rng:optional><rng:attribute name="icon" /></rng:optional>