[FIX] rewrite ir_ui_view.save with new semantics, xpath fixes

since lxml provides built-in tools to generate the path for a node in a tree, don't reimplement it manually

bzr revid: xmo@openerp.com-20130812074509-yopeb4pxtsads4d9

openerp/addons/base/ir/ir_ui_view.py

@@ -26,8 +26,7 @@ import sys
 import re
 import time
 
-import lxml.html
+from lxml import etree, html
-from lxml import etree
 from functools import partial
 
 from openerp import tools
@@ -161,24 +160,58 @@ class view(osv.osv):
 
         return super(view, self).write(cr, uid, ids, vals, context)
 
-    def save(self, cr, uid, model, res_id, field, value, xpath=None, context=None):
+    def extract_embedded_fields(self, cr, uid, arch, context=None):
-        """ Update the content of a field
+        return arch.xpath('//*[@data-oe-model]')
+
+    def save_embedded_field(self, cr, uid, el, context=None):
+        embedded_id = int(el.get('data-oe-id'))
+        # FIXME: type conversions
+        self.pool[el.get('data-oe-model')].write(cr, uid, embedded_id, {
+            el.get('data-oe-field'): el.text
+        }, context=context)
+
+    def to_field_ref(self, cr, uid, el, context=None):
+        # FIXME: better ref?
+        return html.html_parser.makeelement(el.tag, attrib={
+            't-field': 'registry[%(model)r].browse(cr, uid, %(id)r).%(field)s' % {
+                'model': el.get('data-oe-model'),
+                'id': int(el.get('data-oe-id')),
+                'field': el.get('data-oe-field'),
+            }
+        })
+
+    def replace_arch_section(self, cr, uid, view_id, section_xpath, replacement, context=None):
+        arch = replacement
+        if section_xpath:
+            previous_arch = etree.fromstring(self.browse(cr, uid, view_id, context=context).arch.encode('utf-8'))
+            # ensure there's only one match
+            [previous_section] = previous_arch.xpath(section_xpath)
+            previous_section.getparent().replace(previous_section, replacement)
+            arch = previous_arch
+        return arch
+
+    def save(self, cr, uid, res_id, value, xpath=None, context=None):
+        """ Update a view section. The view section may embed fields to write
 
         :param str model:
         :param int res_id:
         :param str xpath: valid xpath to the tag to replace
         """
-        model_obj = self.pool.get(model)
+        res_id = int(res_id)
-        if xpath:
-            origin = model_obj.read(cr, uid, [res_id], [field], context=context)[0][field]
-            origin_tree = etree.fromstring(origin.encode('utf-8'))
-            zone = origin_tree.xpath(xpath)[0]
-            zone.getparent().replace(zone, lxml.html.fromstring(value))
-            value = etree.tostring(origin_tree, encoding='utf-8')
 
-        model_obj.write(cr, uid, res_id, {field: value}, context=context)
+        arch_section = etree.fromstring(value)
+
+        for el in self.extract_embedded_fields(cr, uid, arch_section, context=context):
+            self.save_embedded_field(cr, uid, el, context=context)
+
+            # transform embedded field back to t-field
+            el.getparent().replace(el, self.to_field_ref(cr, uid, el, context=context))
+
+        arch = self.replace_arch_section(cr, uid, res_id, xpath, arch_section, context=context)
+        self.write(cr, uid, res_id, {
+            'arch': etree.tostring(arch, encoding='utf-8').decode('utf-8')
+        }, context=context)
 
-    # default view selection
 
     def default_view(self, cr, uid, model, view_type, context=None):
         """ Fetches the default view for the provided (model, view_type) pair:
@@ -272,24 +305,14 @@ class view(osv.osv):
                 return node
         return None
 
-    def inherit_branding(self, specs_tree, view_id, base_xpath=None, count=None):
+    def inherit_branding(self, specs_tree, view_id):
-        if not count:
-            count = {}
         for node in specs_tree:
-            try:
+            xpath = node.getroottree().getpath(node)
-                count[node.tag] = count.get(node.tag, 0) + 1
+            if node.tag == 'data' or node.tag == 'xpath':
-                xpath = "%s/%s[%s]" % (base_xpath or '', node.tag, count.get(node.tag))
+                self.inherit_branding(node, view_id)
-                if node.tag == 'data' or node.tag == 'xpath':
+            else:
-                    node = self.inherit_branding(node, view_id, xpath, count)
+                node.set('data-oe-id', str(view_id))
-                else:
+                node.set('data-oe-xpath', xpath)
-                    node.attrib.update({
-                        'data-oe-model': 'ir.ui.view',
-                        'data-oe-id': str(view_id),
-                        'data-oe-field': 'arch',
-                        'data-oe-xpath': xpath
-                    })
-            except Exception,e:
-                print "inherit branding error",e,xpath,node.tag
 
         return specs_tree
 
@@ -713,14 +736,13 @@ class view(osv.osv):
         r = self.read_combined(cr, uid, id_, fields=['arch'], context=context)
         return r['arch']
 
-    def distribute_branding(self, e, branding=None, xpath=None, count=None):
+    def distribute_branding(self, e, branding=None):
         if e.attrib.get('t-ignore') or e.tag == 'head':
             # TODO: find a better name and check if we have a string to boolean helper
             return
-        xpath = "%s/%s[%s]" % (xpath or '', e.tag, count[e.tag] if count else 1)
         if branding and not (e.attrib.get('data-oe-model') or e.attrib.get('t-field')):
             e.attrib.update(branding)
-            e.attrib['data-oe-xpath'] = xpath
+            e.attrib['data-oe-xpath'] = e.getroottree().getpath(e)
         if not e.attrib.get('data-oe-model'): return
 
         # if a branded element contains branded elements distribute own
@@ -735,11 +757,8 @@ class view(osv.osv):
                 if e.attrib.get(attribute))
 
             if 't-raw' not in e.attrib:
-                # running index by tag type, for XPath query generation
-                count = {}
                 for child in e:
-                    count[child.tag] = count.get(child.tag, 0) + 1
+                    self.distribute_branding(child, distributed_branding)
-                    self.distribute_branding(child, distributed_branding, xpath, count)
 
     def render(self, cr, uid, id_or_xml_id, values, context=None):
         def loader(name):

openerp/addons/base/tests/test_views.py

@@ -1,6 +1,8 @@
 # -*- encoding: utf-8 -*-
-from lxml import etree as ET
+import itertools
+from lxml import etree as ET, html
 from lxml.builder import E
+from lxml.html import builder as h
 
 from openerp.tests import common
 import unittest2
@@ -424,3 +426,151 @@ class TestNoModel(common.TransactionCase):
                          'Copyrighter, tous droits réservés'))
         self.assertEqual(fields, {})
 
+def attrs(**kwargs):
+    return dict(('data-oe-%s' % key, str(value)) for key, value in kwargs.iteritems())
+class TestViewSaving(common.TransactionCase):
+    def eq(self, a, b):
+        self.assertEqual(a.tag, b.tag)
+        self.assertEqual(a.attrib, b.attrib)
+        self.assertEqual(a.text, b.text)
+        self.assertEqual(a.tail, b.tail)
+        for ca, cb in itertools.izip_longest(a, b):
+            self.eq(ca, cb)
+
+    def setUp(self):
+        super(TestViewSaving, self).setUp()
+        self.arch = h.DIV(
+            h.DIV(
+                h.H3("Column 1"),
+                h.UL(
+                    h.LI("Item 1"),
+                    h.LI("Item 2"),
+                    h.LI("Item 3"))),
+            h.DIV(
+                h.H3("Column 2"),
+                h.UL(
+                    h.LI("Item 1"),
+                    h.LI(h.SPAN("My Company", attrs(model='res.company', id=1, field='name'))),
+                    h.LI(h.SPAN("+00 00 000 00 0 000", attrs(model='res.company', id=1, field='phone')))
+                ))
+        )
+        self.view_id = self.registry('ir.ui.view').create(self.cr, self.uid, {
+            'name': "Test View",
+            'type': 'qweb',
+            'arch': ET.tostring(self.arch, encoding='utf-8').decode('utf-8')
+        })
+
+    def test_embedded_extraction(self):
+        fields = self.registry('ir.ui.view').extract_embedded_fields(
+            self.cr, self.uid, self.arch, context=None)
+
+        expect = [
+            h.SPAN("My Company", attrs(model='res.company', id=1, field='name')),
+            h.SPAN("+00 00 000 00 0 000", attrs(model='res.company', id=1, field='phone')),
+        ]
+        for actual, expected in itertools.izip_longest(fields, expect):
+            self.eq(actual, expected)
+
+    def test_embedded_save(self):
+        embedded = h.SPAN("+00 00 000 00 0 000", attrs(model='res.company', id=1, field='phone'))
+
+        self.registry('ir.ui.view').save_embedded_field(self.cr, self.uid, embedded)
+
+        company = self.registry('res.company').browse(self.cr, self.uid, 1)
+        self.assertEqual(company.phone, "+00 00 000 00 0 000")
+
+    @unittest2.skip("save conflict for embedded (saved by third party or previous version in page) not implemented")
+    def test_embedded_conflict(self):
+        e1 = h.SPAN("My Company", attrs(model='res.company', id=1, field='name'))
+        e2 = h.SPAN("Leeroy Jenkins", attrs(model='res.company', id=1, field='name'))
+
+        View = self.registry('ir.ui.view')
+
+        View.save_embedded_field(self.cr, self.uid, e1)
+        # FIXME: more precise exception
+        with self.assertRaises(Exception):
+            View.save_embedded_field(self.cr, self.uid, e2)
+
+    def test_embedded_to_field_ref(self):
+        View = self.registry('ir.ui.view')
+        embedded = h.SPAN("My Company", attrs(model='res.company', id=1, field='name'))
+        self.eq(
+            View.to_field_ref(self.cr, self.uid, embedded, context=None),
+            h.SPAN({'t-field': 'registry[%r].browse(cr, uid, %r).%s' % (
+                'res.company', 1, 'name'
+            )})
+        )
+
+    def test_replace_arch(self):
+        replacement = h.P("Wheee")
+
+        result = self.registry('ir.ui.view').replace_arch_section(
+            self.cr, self.uid, self.view_id, None, replacement)
+
+        self.eq(result, replacement)
+
+    def test_fixup_arch(self):
+        replacement = h.H1("I am the greatest title alive!")
+
+        result = self.registry('ir.ui.view').replace_arch_section(
+            self.cr, self.uid, self.view_id, '/div/div[1]/h3',
+            replacement)
+
+        self.eq(result, h.DIV(
+            h.DIV(
+                h.H1("I am the greatest title alive!"),
+                h.UL(
+                    h.LI("Item 1"),
+                    h.LI("Item 2"),
+                    h.LI("Item 3"))),
+            h.DIV(
+                h.H3("Column 2"),
+                h.UL(
+                    h.LI("Item 1"),
+                    h.LI(h.SPAN("My Company", attrs(model='res.company', id=1, field='name'))),
+                    h.LI(h.SPAN("+00 00 000 00 0 000", attrs(model='res.company', id=1, field='phone')))
+                ))
+        ))
+
+    def test_multiple_xpath_matches(self):
+        with self.assertRaises(ValueError):
+            self.registry('ir.ui.view').replace_arch_section(
+                self.cr, self.uid, self.view_id, '/div/div/h3',
+                h.H6("Lol nope"))
+
+    def test_save(self):
+        Company = self.registry('res.company')
+        View = self.registry('ir.ui.view')
+
+        replacement = ET.tostring(h.DIV(
+            h.H3("Column 2"),
+            h.UL(
+                h.LI("wob wob wob"),
+                h.LI(h.SPAN("Acme Corporation", attrs(model='res.company', id=1, field='name'))),
+                h.LI(h.SPAN("+12 3456789", attrs(model='res.company', id=1, field='phone'))),
+            )
+        ), encoding='utf-8')
+        View.save(self.cr, self.uid, res_id=self.view_id, value=replacement,
+                  xpath='/div/div[2]')
+
+        company = Company.browse(self.cr, self.uid, 1)
+        self.assertEqual(company.name, "Acme Corporation")
+        self.assertEqual(company.phone, "+12 3456789")
+        self.eq(
+            ET.fromstring(View.browse(self.cr, self.uid, self.view_id).arch.encode('utf-8')),
+            h.DIV(
+                h.DIV(
+                    h.H3("Column 1"),
+                    h.UL(
+                        h.LI("Item 1"),
+                        h.LI("Item 2"),
+                        h.LI("Item 3"))),
+                h.DIV(
+                    h.H3("Column 2"),
+                    h.UL(
+                        h.LI("wob wob wob"),
+                        h.LI(h.SPAN({'t-field': "registry['res.company'].browse(cr, uid, 1).name"})),
+                        h.LI(h.SPAN({'t-field': "registry['res.company'].browse(cr, uid, 1).phone"}))
+                    ))
+            )
+        )