[IMP] web_editor: use inheriting views for oe_structure content

Before this commit, when using the website editor, modifications were
saved in place in the original view. The view was also marked as non
updatable so that when an user migrates its database, he does not lose
its modifications.

The problem with this system is that users were able to keep their
modifications at the cost of having views which are not up-to-date with
the latest odoo updates.

Now, the views' content which is inside an oe_structure element is saved
in an inheriting view instead of the original view. The original
view is not marked as non updatable anymore.

Note 1: we only consider elements with the 'oe_structure' class which
also have an id containing the 'oe_structure' string.

Note 2: if a non oe_structure but editable content is edited, the
original view is still marked as non updatable (however, with the new
multi-website feature, this may not be necessary as the edited view will
be duplicated to be website-specific... we may want to disable the
behavior in a future update).

addons/web_editor/models/ir_ui_view.py

@@ -34,6 +34,10 @@ class IrUiView(models.Model):
     def extract_embedded_fields(self, arch):
         return arch.xpath('//*[@data-oe-model != "ir.ui.view"]')
 
+    @api.model
+    def extract_oe_structures(self, arch):
+        return arch.xpath('//*[hasclass("oe_structure")][contains(@id, "oe_structure")]')
+
     @api.model
     def get_default_lang_code(self):
         return False
@@ -54,6 +58,39 @@ class IrUiView(models.Model):
             else:
                 Model.browse(int(el.get('data-oe-id'))).write({field: value})
 
+    @api.multi
+    def save_oe_structure(self, el):
+        self.ensure_one()
+
+        if el.get('id') in self.key:
+            # Do not inherit if the oe_structure already has its own inheriting view
+            return False
+
+        arch = etree.Element('data')
+        xpath = etree.Element('xpath', expr="//*[hasclass('oe_structure')][@id='{}']".format(el.get('id')), position="replace")
+        arch.append(xpath)
+        structure = etree.Element(el.tag, attrib=el.attrib)
+        xpath.append(structure)
+        for child in el.iterchildren(tag=etree.Element):
+            structure.append(copy.deepcopy(child))
+
+        vals = {
+            'inherit_id': self.id,
+            'name': '%s (%s)' % (self.name, el.get('id')),
+            'arch': self._pretty_arch(arch),
+            'key': '%s_%s' % (self.key, el.get('id')),
+            'type': 'qweb',
+        }
+        vals.update(self._save_oe_structure_hook())
+        self.create(vals)
+
+        return True
+
+    @api.model
+    def _save_oe_structure_hook(self):
+        return {}
+
+    @api.model
     def _pretty_arch(self, arch):
         # remove_blank_string does not seem to work on HTMLParser, and
         # pretty-printing with lxml more or less requires stripping
@@ -66,8 +103,24 @@ class IrUiView(models.Model):
         return etree.tostring(
             arch_no_whitespace, encoding='unicode', pretty_print=True)
 
+    @api.model
+    def _are_archs_equal(self, arch1, arch2):
+        # Note that comparing the strings would not be ok as attributes order
+        # must not be relevant
+        if arch1.tag != arch2.tag:
+            return False
+        if arch1.text != arch2.text:
+            return False
+        if arch1.tail != arch2.tail:
+            return False
+        if arch1.attrib != arch2.attrib:
+            return False
+        if len(arch1) != len(arch2):
+            return False
+        return all(self._are_archs_equal(arch1, arch2) for arch1, arch2 in pycompat.izip(arch1, arch2))
+
     @api.multi
-    def replace_arch_section(self, section_xpath, replacement):
+    def replace_arch_section(self, section_xpath, replacement, replace_tail=False):
         # the root of the arch section shouldn't actually be replaced as it's
         # not really editable itself, only the content truly is editable.
         self.ensure_one()
@@ -80,7 +133,9 @@ class IrUiView(models.Model):
             [root] = arch.xpath(section_xpath)
 
         root.text = replacement.text
-        root.tail = replacement.tail
+        # Note: after a standard edition, the tail *must not* be replaced
+        if replace_tail:
+            root.tail = replacement.tail
         # replace all children
         del root[:]
         for child in replacement:
@@ -99,6 +154,12 @@ class IrUiView(models.Model):
         out.tail = el.tail
         return out
 
+    @api.model
+    def to_empty_oe_structure(self, el):
+        out = html.html_parser.makeelement(el.tag, attrib=el.attrib)
+        out.tail = el.tail
+        return out
+
     @api.multi
     def save(self, value, xpath=None):
         """ Update a view section. The view section may embed fields to write
@@ -106,6 +167,7 @@ class IrUiView(models.Model):
         :param str xpath: valid xpath to the tag to replace
         """
         self.ensure_one()
+
         arch_section = html.fromstring(
             value, parser=html.HTMLParser(encoding='utf-8'))
 
@@ -120,11 +182,20 @@ class IrUiView(models.Model):
             # transform embedded field back to t-field
             el.getparent().replace(el, self.to_field_ref(el))
 
-        for view in self:
-            arch = view.replace_arch_section(xpath, arch_section)
-            view.write({'arch': view._pretty_arch(arch)})
+        for el in self.extract_oe_structures(arch_section):
+            if self.save_oe_structure(el):
+                # empty oe_structure in parent view
+                empty = self.to_empty_oe_structure(el)
+                if el == arch_section:
+                    arch_section = empty
+                else:
+                    el.getparent().replace(el, empty)
 
-        self.sudo().mapped('model_data_id').write({'noupdate': True})
+        new_arch = self.replace_arch_section(xpath, arch_section)
+        old_arch = etree.fromstring(self.arch.encode('utf-8'))
+        if not self._are_archs_equal(old_arch, new_arch):
+            self.sudo().model_data_id.write({'noupdate': True}) # TODO check if we remove this
+            self.write({'arch': self._pretty_arch(new_arch)})
 
     @api.model
     def _view_obj(self, view_id):

addons/website/models/ir_ui_view.py

@@ -292,6 +292,12 @@ class View(models.Model):
     def _read_template_keys(self):
         return super(View, self)._read_template_keys() + ['website_id']
 
+    @api.model
+    def _save_oe_structure_hook(self):
+        res = super(View, self)._save_oe_structure_hook()
+        res['website_id'] = self.env['website'].get_current_website().id
+        return res
+
     @api.multi
     def save(self, value, xpath=None):
         self.ensure_one()

addons/website/tests/test_views.py

@@ -410,7 +410,7 @@ class TestCowViewSaving(common.TransactionCase):
 
     def test_cow_generic_view_with_already_existing_specific(self):
         """ Writing on a generic view should check if a website specific view already exists
-            (The flow of this test will happen when editing a generic view in the front end and changing more than one oe_structure)
+            (The flow of this test will happen when editing a generic view in the front end and changing more than one element)
         """
         # 1. Test with calling write directly
         View = self.env['ir.ui.view']
@@ -430,11 +430,11 @@ class TestCowViewSaving(common.TransactionCase):
         view_arch = '''<t name="Second View" t-name="website.second_view">
                           <t t-call="website.layout">
                             <div id="wrap">
-                              <div class="oe_structure"/>
+                              <div class="editable_part"/>
                               <div class="container">
                                   <h1>Second View</h1>
                               </div>
-                              <div class="oe_structure"/>
+                              <div class="editable_part"/>
                             </div>
                           </t>
                        </t>'''
@@ -445,12 +445,12 @@ class TestCowViewSaving(common.TransactionCase):
         })
 
         total_views = View.search_count([])
-        second_view.with_context(website_id=1).save('<div class="oe_structure" data-oe-id="%s" data-oe-xpath="/t[1]/t[1]/div[1]/div[1]" data-oe-field="arch" data-oe-model="ir.ui.view">First oe_structure</div>' % second_view.id, "/t[1]/t[1]/div[1]/div[1]")
-        second_view.with_context(website_id=1).save('<div class="oe_structure" data-oe-id="%s" data-oe-xpath="/t[1]/t[1]/div[1]/div[3]" data-oe-field="arch" data-oe-model="ir.ui.view">Second oe_structure</div>' % second_view.id, "/t[1]/t[1]/div[1]/div[3]")
+        second_view.with_context(website_id=1).save('<div class="editable_part" data-oe-id="%s" data-oe-xpath="/t[1]/t[1]/div[1]/div[1]" data-oe-field="arch" data-oe-model="ir.ui.view">First editable_part</div>' % second_view.id, "/t[1]/t[1]/div[1]/div[1]")
+        second_view.with_context(website_id=1).save('<div class="editable_part" data-oe-id="%s" data-oe-xpath="/t[1]/t[1]/div[1]/div[3]" data-oe-field="arch" data-oe-model="ir.ui.view">Second editable_part</div>' % second_view.id, "/t[1]/t[1]/div[1]/div[3]")
         self.assertEqual(total_views + 1, View.search_count([]), "Second save should have wrote on the view copied during first save")
 
-        total_specific_view = View.search_count([('arch_db', 'like', 'First oe_structure'), ('arch_db', 'like', 'Second oe_structure')])
-        self.assertEqual(total_specific_view, 1, "both oe_structure should have been replaced on a created specific view")
+        total_specific_view = View.search_count([('arch_db', 'like', 'First editable_part'), ('arch_db', 'like', 'Second editable_part')])
+        self.assertEqual(total_specific_view, 1, "both editable_part should have been replaced on a created specific view")
 
     def test_cow_complete_flow(self):
         View = self.env['ir.ui.view']