diff --git a/addons/web_editor/controllers/main.py b/addons/web_editor/controllers/main.py index 7dbc3415f21..02d46a7d7bd 100644 --- a/addons/web_editor/controllers/main.py +++ b/addons/web_editor/controllers/main.py @@ -7,6 +7,7 @@ import logging import os import re import time +import uuid import werkzeug.wrappers from PIL import Image, ImageFont, ImageDraw from lxml import etree, html @@ -412,6 +413,7 @@ class Web_Editor(http.Controller): view_to_xpath = IrUiView.get_related_views(bundle_xmlid, bundles=True).filtered(lambda v: v.arch.find(url) >= 0) IrUiView.create(dict( name = custom_url, + key='web_editor.scss_%s' % str(uuid.uuid4())[:6], mode = "extension", inherit_id = view_to_xpath.id, arch = """ diff --git a/addons/web_editor/models/ir_ui_view.py b/addons/web_editor/models/ir_ui_view.py index 612abc1596a..ae698ba1c23 100644 --- a/addons/web_editor/models/ir_ui_view.py +++ b/addons/web_editor/models/ir_ui_view.py @@ -105,6 +105,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')) diff --git a/addons/website/controllers/main.py b/addons/website/controllers/main.py index f9c7d700e15..84a880e3b94 100644 --- a/addons/website/controllers/main.py +++ b/addons/website/controllers/main.py @@ -343,7 +343,7 @@ class Website(Home): record_id = View.search([ ("website_id", "=", request.website.id), ("key", "=", xml_id), - ]).id or request.env.ref(xml_id).id + ], limit=1).id or request.env.ref(xml_id).id else: record_id = int(xml_id) ids.append(record_id) diff --git a/addons/website/models/ir_ui_view.py b/addons/website/models/ir_ui_view.py index 5e7ad9fb4ad..c5ebf48caad 100644 --- a/addons/website/models/ir_ui_view.py +++ b/addons/website/models/ir_ui_view.py @@ -2,6 +2,7 @@ # Part of Odoo. See LICENSE file for full copyright and licensing details. import logging +import uuid from itertools import groupby from odoo import api, fields, models, _ @@ -35,26 +36,35 @@ class View(models.Model): websites. Also this way newly created websites will only contain the default views. ''' - if not self._context.get('no_cow'): - current_website_id = self._context.get('website_id') + current_website_id = self._context.get('website_id') + if current_website_id: for view in self: - # if generic view in multi-website context - if current_website_id and not view.website_id: - new_website_specific_view = view.copy({'website_id': current_website_id}) - view._create_website_specific_pages_for_view(new_website_specific_view, - view.env['website'].browse(current_website_id)) + if not view.key and not vals.get('key'): + view.with_context(no_cow=True).key = 'website.key_%s' % str(uuid.uuid4())[:6] + if not view.website_id and current_website_id and not self._context.get('no_cow'): + # If already a specific view for this generic view, write on it + website_specific_view = self.env['ir.ui.view'].search([ + ('key', '=', view.key), + ('website_id', '=', current_website_id) + ], limit=1) + if not website_specific_view: + # Set key to avoid copy() to generate an unique key as we want the specific view to have the same key + website_specific_view = view.copy({'website_id': current_website_id, 'key': view.key}) + view._create_website_specific_pages_for_view(website_specific_view, + view.env['website'].browse(current_website_id)) - # trigger COW on inheriting views - for inherit_child in view.inherit_children_ids: - inherit_child.write({'inherit_id': new_website_specific_view.id}) + for inherit_child in view.inherit_children_ids: + # COW won't be triggered if there is already a website_id on the view, we should copy the view ourself + if inherit_child.website_id.id == current_website_id: + inherit_child.copy({'inherit_id': website_specific_view.id, 'key': inherit_child.key}) + # We should unlink website specific view from generic tree as it now copied on specific tree + inherit_child.unlink() + else: + # trigger COW on inheriting views + inherit_child.write({'inherit_id': website_specific_view.id}) - new_website_specific_view.write(vals) - else: - super(View, view).write(vals) - else: - super(View, self).write(vals) - - return True + return super(View, website_specific_view).write(vals) + return super(View, self).write(vals) @api.multi def unlink(self): @@ -121,10 +131,12 @@ class View(models.Model): def filter_duplicate(self): """ Filter current recordset only keeping the most suitable view per distinct key """ + view_with_key = self.filtered('key') + view_without_key = self - view_with_key filtered = self.env['ir.ui.view'] - for dummy, group in groupby(self.sorted('key'), key=lambda record: record.key): + for dummy, group in groupby(view_with_key.sorted('key'), key=lambda record: record.key): filtered += sorted(group, key=lambda record: record._sort_suitability_key())[0] - return filtered.sorted(key=lambda view: (view.priority, view.id)) + return (filtered + view_without_key).sorted(key=lambda view: (view.priority, view.id)) @api.model def _view_obj(self, view_id): @@ -275,3 +287,18 @@ class View(models.Model): def _read_template_keys(self): return super(View, self)._read_template_keys() + ['website_id'] + + @api.multi + def save(self, value, xpath=None): + self.ensure_one() + # The first time a generic view is edited, it will send multiple rpc to this method. + # If there is already a website specific view, we need to divert the super to it + current_website_id = self._context.get('website_id') + if self.key and current_website_id: + website_specific_view = self.env['ir.ui.view'].search([ + ('key', '=', self.key), + ('website_id', '=', current_website_id) + ], limit=1) + if website_specific_view: + self = website_specific_view + super(View, self).save(value, xpath=xpath) diff --git a/addons/website/tests/test_page.py b/addons/website/tests/test_page.py index 3f717740e79..1b82d92ad57 100644 --- a/addons/website/tests/test_page.py +++ b/addons/website/tests/test_page.py @@ -21,6 +21,7 @@ class TestPage(common.TransactionCase): 'mode': 'extension', 'inherit_id': self.base_view.id, 'arch': '
, extended content
', + 'key': 'test.extension_view', }) self.page_1 = Page.create({ diff --git a/addons/website/tests/test_views.py b/addons/website/tests/test_views.py index 10c2f3bcbf8..4b3759cdfb6 100644 --- a/addons/website/tests/test_views.py +++ b/addons/website/tests/test_views.py @@ -140,7 +140,6 @@ class TestViewSaving(common.TransactionCase): def test_save(self): Company = self.env['res.company'] - View = self.env['ir.ui.view'] # create an xmlid for the view imd = self.env['ir.model.data'].create({ @@ -225,19 +224,18 @@ class TestViewSaving(common.TransactionCase): node = html.tostring(h.SPAN( "Acme Corporation", attrs(model='res.company', id=company_id, field="name", expression='bob', type='char')), - encoding='unicode') + encoding='unicode') View = self.env['ir.ui.view'] View.browse(company_id).save(value=node) self.assertEqual(company.name, "Acme Corporation") def test_field_tail(self): - View = self.env['ir.ui.view'] replacement = ET.tostring( h.LI(h.SPAN("+12 3456789", attrs( model='res.company', id=1, type='char', field='phone', expression="edmund")), "whop whop" - ), encoding="utf-8") + ), encoding="utf-8") self.view_id.save(value=replacement, xpath='/div/div[2]/ul/li[3]') self.eq( @@ -259,80 +257,131 @@ class TestViewSaving(common.TransactionCase): ) ) - def test_cow_leaf(self): + +class TestCowViewSaving(common.TransactionCase): + def setUp(self): + super(TestCowViewSaving, self).setUp() View = self.env['ir.ui.view'] - base_view = View.create({ + self.base_view = View.create({ 'name': 'Base', 'type': 'qweb', 'arch': '
base content
', + 'key': 'website.base_view', }).with_context(load_all_views=True) - inherit_view = View.create({ + self.inherit_view = View.create({ 'name': 'Extension', 'mode': 'extension', - 'inherit_id': base_view.id, - 'arch': '
extended content
', + 'inherit_id': self.base_view.id, + 'arch': '
, extended content
', + 'key': 'website.extension_view', }) - # edit on backend, regular write - inherit_view.write({'arch': '
modified content
'}) - self.assertEqual(View.search_count([('name', '=', 'Base')]), 1) - self.assertEqual(View.search_count([('name', '=', 'Extension')]), 1) + def test_cow_on_base_after_extension(self): + View = self.env['ir.ui.view'] + self.inherit_view.with_context(website_id=1).write({'name': 'Extension Specific'}) + v1 = self.base_view + v2 = self.inherit_view + v3 = View.search([('website_id', '=', 1), ('name', '=', 'Extension Specific')]) + v4 = self.inherit_view.copy({'name': 'Second Extension'}) + v5 = self.inherit_view.copy({'name': 'Third Extension (Specific)'}) + v5.write({'website_id': 1}) - arch = base_view.read_combined(['arch'])['arch'] + # id | name | website_id | inherit | key + # ------------------------------------------------------------------------ + # 1 | Base | / | / | website.base_view + # 2 | Extension | / | 1 | website.extension_view + # 3 | Extension Specific | 1 | 1 | website.extension_view + # 4 | Second Extension | / | 1 | website.extension_view_a5f579d5 (generated hash) + # 5 | Third Extension (Specific) | 1 | 1 | website.extension_view_5gr87e6c (another generated hash) + + self.assertEqual(v2.key == v3.key, True, "Making specific a generic inherited view should copy it's key (just change the website_id)") + self.assertEqual(v3.key != v4.key != v5.key, True, "Copying a view should generate a new key for the new view (not the case when triggering COW)") + self.assertEqual('website.extension_view' in v3.key and 'website.extension_view' in v4.key and 'website.extension_view' in v5.key, True, "The copied views should have the key from the view it was copied from but with an unique suffix") + + total_views = View.search_count([]) + v1.with_context(website_id=1).write({'name': 'Base Specific'}) + + # id | name | website_id | inherit | key + # ------------------------------------------------------------------------ + # 1 | Base | / | / | website.base_view + # 2 | Extension | / | 1 | website.extension_view + # 3 - DELETED + # 4 | Second Extension | / | 1 | website.extension_view_a5f579d5 + # 5 - DELETED + # 6 | Base Specific | 1 | / | website.base_view + # 7 | Extension Specific | 1 | 6 | website.extension_view + # 8 | Second Extension | 1 | 6 | website.extension_view_a5f579d5 + # 9 | Third Extension (Specific) | 1 | 6 | website.extension_view_5gr87e6c + + v6 = View.search([('website_id', '=', 1), ('name', '=', 'Base Specific')]) + v7 = View.search([('website_id', '=', 1), ('name', '=', 'Extension Specific')]) + v8 = View.search([('website_id', '=', 1), ('name', '=', 'Second Extension')]) + v9 = View.search([('website_id', '=', 1), ('name', '=', 'Third Extension (Specific)')]) + + self.assertEqual(total_views + 4 - 2, View.search_count([]), "It should have duplicated the view tree with a website_id, taking only most specific (only specific `b` key), and removing website_specific from generic tree") + self.assertEqual(len((v3 + v5).exists()), 0, "v3 and v5 should have been deleted as they were already specific and copied to the new specific base") + # Check generic tree + self.assertEqual((v1 + v2 + v4).mapped('website_id').ids, []) + self.assertEqual((v2 + v4).mapped('inherit_id'), v1) + # Check specific tree + self.assertEqual((v6 + v7 + v8 + v9).mapped('website_id').ids, [1]) + self.assertEqual((v7 + v8 + v9).mapped('inherit_id'), v6) + # Check key + self.assertEqual(v6.key == v1.key, True) + self.assertEqual(v7.key == v2.key, True) + self.assertEqual(v4.key == v8.key, True) + self.assertEqual(View.search_count([('key', '=', v9.key)]), 1) + + def test_cow_leaf(self): + View = self.env['ir.ui.view'] + + # edit on backend, regular write + self.inherit_view.write({'arch': '
modified content
'}) + self.assertEqual(View.search_count([('key', '=', 'website.base_view')]), 1) + self.assertEqual(View.search_count([('key', '=', 'website.extension_view')]), 1) + + arch = self.base_view.read_combined(['arch'])['arch'] self.assertEqual(arch, '
modified content
') # edit on frontend, copy just the leaf - inherit_view.with_context(website_id=1).write({'arch': '
website 1 content
'}) - inherit_views = View.search([('name', '=', 'Extension')]) - self.assertEqual(View.search_count([('name', '=', 'Base')]), 1) + self.inherit_view.with_context(website_id=1).write({'arch': '
website 1 content
'}) + inherit_views = View.search([('key', '=', 'website.extension_view')]) + self.assertEqual(View.search_count([('key', '=', 'website.base_view')]), 1) self.assertEqual(len(inherit_views), 2) self.assertEqual(len(inherit_views.filtered(lambda v: v.website_id.id == 1)), 1) # read in backend should be unaffected - arch = base_view.read_combined(['arch'])['arch'] + arch = self.base_view.read_combined(['arch'])['arch'] self.assertEqual(arch, '
modified content
') # read on website should reflect change - arch = base_view.with_context(website_id=1).read_combined(['arch'])['arch'] + arch = self.base_view.with_context(website_id=1).read_combined(['arch'])['arch'] self.assertEqual(arch, '
website 1 content
') # website-specific inactive view should take preference over active generic one when viewing the website # this is necessary to make customize_show=True templates work correctly inherit_views.filtered(lambda v: v.website_id.id == 1).write({'active': False}) - arch = base_view.with_context(website_id=1).read_combined(['arch'])['arch'] + arch = self.base_view.with_context(website_id=1).read_combined(['arch'])['arch'] self.assertEqual(arch, '
base content
') def test_cow_root(self): View = self.env['ir.ui.view'] - base_view = View.create({ - 'name': 'Base', - 'type': 'qweb', - 'arch': '
content
', - }) - - View.create({ - 'name': 'Extension', - 'mode': 'extension', - 'inherit_id': base_view.id, - 'arch': '
, extended content
', - }) - # edit on backend, regular write - base_view.write({'arch': '
modified base content
'}) - self.assertEqual(View.search_count([('name', '=', 'Base')]), 1) - self.assertEqual(View.search_count([('name', '=', 'Extension')]), 1) + self.base_view.write({'arch': '
modified base content
'}) + self.assertEqual(View.search_count([('key', '=', 'website.base_view')]), 1) + self.assertEqual(View.search_count([('key', '=', 'website.extension_view')]), 1) # edit on frontend, copy the entire tree - base_view.with_context(website_id=1).write({'arch': '
website 1 content
'}) + self.base_view.with_context(website_id=1).write({'arch': '
website 1 content
'}) - generic_base_view = View.search([('name', '=', 'Base'), ('website_id', '=', False)]) - website_specific_base_view = View.search([('name', '=', 'Base'), ('website_id', '=', 1)]) + generic_base_view = View.search([('key', '=', 'website.base_view'), ('website_id', '=', False)]) + website_specific_base_view = View.search([('key', '=', 'website.base_view'), ('website_id', '=', 1)]) self.assertEqual(len(generic_base_view), 1) self.assertEqual(len(website_specific_base_view), 1) - inherit_views = View.search([('name', '=', 'Extension')]) + inherit_views = View.search([('key', '=', 'website.extension_view')]) self.assertEqual(len(inherit_views), 2) self.assertEqual(len(inherit_views.filtered(lambda v: v.website_id.id == 1)), 1) @@ -341,3 +390,133 @@ class TestViewSaving(common.TransactionCase): arch = website_specific_base_view.with_context(load_all_views=True, website_id=1).read_combined(['arch'])['arch'] self.assertEqual(arch, '
website 1 content, extended content
') + + # # As there is a new SQL constraint that prevent QWeb views to have an empty `key`, this test won't work + # def test_cow_view_without_key(self): + # # Remove key for this test + # self.base_view.key = False + # + # View = self.env['ir.ui.view'] + # + # # edit on backend, regular write + # self.base_view.write({'arch': '
modified base content
'}) + # self.assertEqual(self.base_view.key, False, "Writing on a keyless view should not set a key on it if there is no website in context") + # + # # edit on frontend, copy just the leaf + # self.base_view.with_context(website_id=1).write({'arch': '
website 1 content
'}) + # self.assertEqual('website.key_' in self.base_view.key, True, "Writing on a keyless view should set a key on it if there is a website in context") + # total_views_with_key = View.search_count([('key', '=', self.base_view.key)]) + # self.assertEqual(total_views_with_key, 2, "It should have set the key on generic view then copy to specific view (with they key)") + + 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) + """ + # 1. Test with calling write directly + View = self.env['ir.ui.view'] + + base_view = View.create({ + 'name': 'Base', + 'type': 'qweb', + 'arch': '
content
', + }) + + total_views = View.search_count([]) + base_view.with_context(website_id=1).write({'name': 'New Name'}) # This will not write on `base_view` but will copy it to a specific view on which the `name` change will be applied + base_view.with_context(website_id=1).write({'name': 'Another New Name'}) + self.assertEqual(total_views + 1, View.search_count([]), "Second write should have wrote on the view copied during first write") + + # 2. Test with calling save() from ir.ui.view + view_arch = ''' + +
+
+
+

Second View

+
+
+
+ + ''' + second_view = View.create({ + 'name': 'Base', + 'type': 'qweb', + 'arch': view_arch, + }) + + total_views = View.search_count([]) + second_view.with_context(website_id=1).save('
First oe_structure
' % second_view.id, "/t[1]/t[1]/div[1]/div[1]") + second_view.with_context(website_id=1).save('
Second oe_structure
' % 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") + + def test_cow_complete_flow(self): + View = self.env['ir.ui.view'] + total_views = View.search_count([]) + + self.base_view.write({'arch': '
Hi
'}) + self.inherit_view.write({'arch': '
World
'}) + + # id | name | content | website_id | inherit | key + # ------------------------------------------------------- + # 1 | Base | Hi | / | / | website.base_view + # 2 | Extension | World | / | 1 | website.extension_view + + arch = self.base_view.with_context(website_id=1).read_combined(['arch'])['arch'] + self.assertEqual('Hi World' in arch, True) + + self.base_view.write({'arch': '
Hello
'}) + + # id | name | content | website_id | inherit | key + # ------------------------------------------------------- + # 1 | Base | Hello | / | / | website.base_view + # 2 | Extension | World | / | 1 | website.extension_view + + arch = self.base_view.with_context(website_id=1).read_combined(['arch'])['arch'] + self.assertEqual('Hello World' in arch, True) + + self.base_view.with_context(website_id=1).write({'arch': '
Bye
'}) + + # id | name | content | website_id | inherit | key + # ------------------------------------------------------- + # 1 | Base | Hello | / | / | website.base_view + # 3 | Base | Bye | 1 | / | website.base_view + # 2 | Extension | World | / | 1 | website.extension_view + # 4 | Extension | World | 1 | 3 | website.extension_view + + base_specific = View.search([('key', '=', self.base_view.key), ('website_id', '=', 1)]).with_context(load_all_views=True) + extend_specific = View.search([('key', '=', self.inherit_view.key), ('website_id', '=', 1)]) + self.assertEqual(total_views + 2, View.search_count([]), "Should have copied Base & Extension with a website_id") + self.assertEqual(self.base_view.key, base_specific.key) + self.assertEqual(self.inherit_view.key, extend_specific.key) + + extend_specific.write({'arch': '
All
'}) + + # id | name | content | website_id | inherit | key + # ------------------------------------------------------- + # 1 | Base | Hello | / | / | website.base_view + # 3 | Base | Bye | 1 | / | website.base_view + # 2 | Extension | World | / | 1 | website.extension_view + # 4 | Extension | All | 1 | 3 | website.extension_view + + arch = base_specific.with_context(website_id=1).read_combined(['arch'])['arch'] + self.assertEqual('Bye All' in arch, True) + + self.inherit_view.with_context(website_id=1).write({'arch': '
Nobody
'}) + + # id | name | content | website_id | inherit | key + # ------------------------------------------------------- + # 1 | Base | Hello | / | / | website.base_view + # 3 | Base | Bye | 1 | / | website.base_view + # 2 | Extension | World | / | 1 | website.extension_view + # 4 | Extension | Nobody | 1 | 3 | website.extension_view + + arch = base_specific.with_context(website_id=1).read_combined(['arch'])['arch'] + self.assertEqual('Bye Nobody' in arch, True, "Write on generic `inherit_view` should have been diverted to already existing specific view") + + base_arch = self.base_view.read_combined(['arch'])['arch'] + base_arch_w1 = self.base_view.with_context(website_id=1).read_combined(['arch'])['arch'] + self.assertEqual('Hello World' in base_arch, True) + self.assertEqual(base_arch, base_arch_w1, "Reading a top level view with or without a website_id in the context should render that exact view..") # ..even if there is a specific view for that one, as read_combined is supposed to render specific inherited view over generic but not specific top level instead of generic top level diff --git a/addons/website/views/website_views.xml b/addons/website/views/website_views.xml index 40e86ec0b75..89aba718624 100644 --- a/addons/website/views/website_views.xml +++ b/addons/website/views/website_views.xml @@ -271,7 +271,7 @@ - + diff --git a/odoo/addons/base/models/ir_ui_view.py b/odoo/addons/base/models/ir_ui_view.py index f46dd3eff39..312d76f4d2d 100644 --- a/odoo/addons/base/models/ir_ui_view.py +++ b/odoo/addons/base/models/ir_ui_view.py @@ -9,6 +9,7 @@ import logging import os import re import time +import uuid import itertools from dateutil.relativedelta import relativedelta @@ -363,6 +364,9 @@ actual arch. "CHECK (mode != 'extension' OR inherit_id IS NOT NULL)", "Invalid inheritance mode: if the mode is 'extension', the view must" " extend an other view"), + ('qweb_required_key', + "CHECK (type != 'qweb' OR key IS NOT NULL)", + "Invalid key: QWeb view should have a key"), ] @api.model_cr_context @@ -393,6 +397,10 @@ actual arch. # don't raise here, the constraint that runs `self._check_xml` will # do the job properly. pass + if not values.get('key') and values.get('type') == 'qweb': + values['key'] = "gen_key.%s" % str(uuid.uuid4())[:6] + if values.get('model'): + values['key'] = "%s.gen_key_%s" % (values.get('model'), str(uuid.uuid4())[:6]) if not values.get('name'): values['name'] = "%s %s" % (values.get('model'), values['type']) values.update(self._compute_defaults(values)) @@ -422,6 +430,15 @@ actual arch. self.mapped('inherit_children_ids').unlink() super(View, self).unlink() + @api.multi + @api.returns('self', lambda value: value.id) + def copy(self, default=None): + self.ensure_one() + if self.key and default and 'key' not in default: + new_key = self.key + '_%s' % str(uuid.uuid4())[:6] + default = dict(default or {}, key=new_key) + return super(View, self).copy(default) + @api.multi def toggle(self): """ Switches between enabled and disabled statuses