diff options
| author | Romain Derie <rde@odoo.com> | 2018-12-07 10:38:37 +0000 |
|---|---|---|
| committer | Romain Derie <rde@odoo.com> | 2018-12-19 18:23:36 +0000 |
| commit | f7df35540b8404a14e8d29af80b8e8642a4c1bdb (patch) | |
| tree | 3863f5d566a5dbdcf01b742cf45b20242d52e1d9 | |
| parent | eb19016ba35d4dc4723edcc244fd412680b5ff2f (diff) | |
[FIX] web_editor, website(_theme_install): avoid theme.ir.ui.view crash
In a particular case, `_views_get` was mixing apples and oranges, ir.ui.view
and theme.ir.ui.view.
This was the case if a theme has a theme.ir.ui.view t-calling another
theme.ir.ui.view.
(Only theme's sale bridge cause this issue, eg theme_kea_sale.products)
Then, having that theme installed on a website would make all other website
without that theme to crash when opening the /shop customize menu.
Indeed, the theme.ir.ui.view would have the xml_id theme_kea_sale.search.
The generic view `website_sale.products` would have `theme_kea_sale.products`
as inherit child which is website specific and t-call `theme_kea_sale.search`.
Then, on a website witout theme_kea installed, opening the customize menu would
call `get_related_views` that would call `_views_get` that would loop through
inherit children calling `_view_obj` that would not find a view with that key
and would fallback on self.env.ref, finding the theme.ir.ui.view.
Note: `_views_get` might return specific view for another website but that will
be filtered on `get_related_views` override in website.
Step to reproduce:
- Have at least 2 websites and website_sale installed
- Install theme_kea on a website, it will also install theme_kea_sale
- Go on the website that has not theme_kea installed
- Go to /shop
- Open Customize menu, it won't load because of python crash
opw-1914576
Closes #29347
| -rw-r--r-- | addons/web_editor/models/ir_ui_view.py | 7 | ||||
| -rw-r--r-- | addons/website/models/ir_ui_view.py | 35 | ||||
| -rw-r--r-- | addons/website/models/website.py | 41 | ||||
| -rw-r--r-- | addons/website/tests/test_views.py | 31 | ||||
| -rw-r--r-- | addons/website_theme_install/tests/__init__.py | 4 | ||||
| -rw-r--r-- | addons/website_theme_install/tests/test_views.py | 151 |
6 files changed, 240 insertions, 29 deletions
diff --git a/addons/web_editor/models/ir_ui_view.py b/addons/web_editor/models/ir_ui_view.py index 45211502f11..908f58e6b8c 100644 --- a/addons/web_editor/models/ir_ui_view.py +++ b/addons/web_editor/models/ir_ui_view.py @@ -205,7 +205,8 @@ class IrUiView(models.Model): return self.search([('key', '=', view_id)], limit=1) or self.env.ref(view_id) elif isinstance(view_id, pycompat.integer_types): return self.browse(view_id) - # assume it's already a view object (WTF?) + # It can already be a view object when called by '_views_get()' that is calling '_view_obj' + # for it's inherit_children_ids, passing them directly as object record. return view_id # Returns all views (called and inherited) related to a view @@ -224,7 +225,7 @@ class IrUiView(models.Model): view = self._view_obj(view_id) except ValueError: _logger.warning("Could not find view object with view_id '%s'", view_id) - return [] + return self.env['ir.ui.view'] while root and view.inherit_id: view = view.inherit_id @@ -240,7 +241,7 @@ class IrUiView(models.Model): called_view = self._view_obj(child.get('t-call', child.get('t-call-assets'))) except ValueError: continue - if called_view not in views_to_return: + if called_view and called_view not in views_to_return: views_to_return += self._views_get(called_view, options=options, bundles=bundles) extensions = view.inherit_children_ids diff --git a/addons/website/models/ir_ui_view.py b/addons/website/models/ir_ui_view.py index b739c32c934..bead9cbb5ee 100644 --- a/addons/website/models/ir_ui_view.py +++ b/addons/website/models/ir_ui_view.py @@ -126,7 +126,9 @@ class View(models.Model): # get_related_views can be called through website=False routes # (e.g. /web_editor/get_assets_editor_resources), so website # dispatch_parameters may not be added. Manually set - # website_id. + # website_id. (It will then always fallback on a website, this + # method should never be called in a generic context, even for + # tests) self = self.with_context(website_id=self.env['website'].get_current_website().id) views = super(View, self).get_related_views(key, bundles=bundles) current_website_id = self._context.get('website_id') @@ -151,7 +153,7 @@ class View(models.Model): * then if the view has no set website """ self.ensure_one() - context_website_id = self.env.context.get('website_id', 1) + context_website_id = self.env.context.get('website_id', None) website_id = self.website_id.id or 0 different_website = context_website_id != website_id return (different_website, website_id) @@ -167,23 +169,18 @@ class View(models.Model): @api.model def _view_obj(self, view_id): - if isinstance(view_id, pycompat.string_types): - if 'website_id' in self._context: - domain = [('key', '=', view_id)] + self.env['website'].website_domain(self._context.get('website_id')) - order = 'website_id' - else: - domain = [('key', '=', view_id)] - order = self._order - views = self.search(domain, order=order) - if views: - return views.filter_duplicate() - else: - return self.env.ref(view_id) - elif isinstance(view_id, pycompat.integer_types): - return self.browse(view_id) - - # assume it's already a view object (WTF?) - return view_id + ''' Given an xml_id or a view_id, return the corresponding view record. + In case of website context, return the most specific one. + :param view_id: either a string xml_id or an integer view_id + :return: The view record or empty recordset + ''' + if isinstance(view_id, pycompat.string_types) or isinstance(view_id, pycompat.integer_types): + return self.env['website'].viewref(view_id) + else: + # It can already be a view object when called by '_views_get()' that is calling '_view_obj' + # for it's inherit_children_ids, passing them directly as object record. (Note that it might + # be a view_id from another website but it will be filtered in 'get_related_views()') + return view_id if view_id._name == 'ir.ui.view' else self.env['ir.ui.view'] @api.model def _get_inheriting_views_arch_website(self, view_id): diff --git a/addons/website/models/website.py b/addons/website/models/website.py index 968e01848e1..92c862bcdf8 100644 --- a/addons/website/models/website.py +++ b/addons/website/models/website.py @@ -282,11 +282,6 @@ class Website(models.Model): key_copy = string + (inc and "-%s" % inc or "") return key_copy - def key_to_view_id(self, view_id): - return self.env['ir.ui.view'].search([ - ('id', '=', view_id), ('type', '=', 'qweb') - ] + self.env['website'].website_domain(self._context.get('website_id'))) - @api.model def page_search_dependencies(self, page_id=False): """ Search dependencies just for information. It will not catch 100% @@ -507,6 +502,42 @@ class Website(models.Model): return request.env.user.id == request.website.user_id.id @api.model + def viewref(self, view_id, raise_if_not_found=True): + ''' Given an xml_id or a view_id, return the corresponding view record. + In case of website context, return the most specific one. + :param view_id: either a string xml_id or an integer view_id + :param raise_if_not_found: should the method raise an error if no view found + :return: The view record or empty recordset + ''' + View = self.env['ir.ui.view'] + view = None + if isinstance(view_id, pycompat.string_types): + if 'website_id' in self._context: + domain = [('key', '=', view_id)] + self.env['website'].website_domain(self._context.get('website_id')) + order = 'website_id' + else: + domain = [('key', '=', view_id)] + order = View._order + views = View.with_context(active_test=False).search(domain, order=order) + if views: + view = views.filter_duplicate() + else: + view = self.env.ref(view_id) + # self.env.ref might return something else than an ir.ui.view (eg: a theme.ir.ui.view) + if view._name != 'ir.ui.view': + view = None + elif isinstance(view_id, pycompat.integer_types): + view = View.browse(view_id) + else: + raise ValueError('Expecting a string or an integer, not a %s.' % (type(view_id))) + + if view: + return view + if raise_if_not_found: + raise ValueError('No record found for unique ID %s. It may have been deleted.' % (view_id)) + return None + + @api.model def get_template(self, template): View = self.env['ir.ui.view'] if isinstance(template, pycompat.integer_types): diff --git a/addons/website/tests/test_views.py b/addons/website/tests/test_views.py index 49a76cf78ab..99fc814a271 100644 --- a/addons/website/tests/test_views.py +++ b/addons/website/tests/test_views.py @@ -561,5 +561,32 @@ class TestCowViewSaving(common.TransactionCase): specific_view = View.with_context(website_id=1).get_view_id('website.main_view') generic_view_arch = View.browse(generic_view).with_context(load_all_views=True).read_combined(['arch'])['arch'] specific_view_arch = View.browse(specific_view).with_context(load_all_views=True, website_id=1).read_combined(['arch'])['arch'] - self.assertEqual(generic_view_arch == '<body>GENERIC<div>VIEW<span>C</span></div></body>', True) - self.assertEqual(specific_view_arch == '<body>SPECIFIC<div>VIEW<span>D</span></div></body>', True, "Writing on top level view hierarchy with a website in context should write on the view and clone it's inherited views") + self.assertEqual(generic_view_arch, '<body>GENERIC<div>VIEW<span>C</span></div></body>') + self.assertEqual(specific_view_arch, '<body>SPECIFIC<div>VIEW<span>D</span></div></body>', "Writing on top level view hierarchy with a website in context should write on the view and clone it's inherited views") + + def test_multi_website_view_obj_active(self): + ''' With the following structure: + * A generic active parent view + * A generic active child view, that is inactive on website 1 + The methods to retrieve views should return the specific inactive + child over the generic active one. + ''' + View = self.env['ir.ui.view'] + self.inherit_view.with_context(website_id=1).write({'active': False}) + + # Test _view_obj() return the inactive specific over active generic + inherit_view = View._view_obj(self.inherit_view.key) + self.assertEqual(inherit_view.active, True, "_view_obj should return the generic one") + inherit_view = View.with_context(website_id=1)._view_obj(self.inherit_view.key) + self.assertEqual(inherit_view.active, False, "_view_obj should return the specific one") + + # Test get_related_views() return the inactive specific over active generic + # Note that we cannot test get_related_views without a website in context as it will fallback on a website with get_current_website() + views = View.with_context(website_id=1).get_related_views(self.base_view.key) + self.assertEqual(views.mapped('active'), [True, False], "get_related_views should return the specific child") + + # Test filter_duplicate() return the inactive specific over active generic + view = View.with_context(active_test=False).search([('key', '=', self.inherit_view.key)]).filter_duplicate() + self.assertEqual(view.active, True, "filter_duplicate should return the generic one") + view = View.with_context(active_test=False, website_id=1).search([('key', '=', self.inherit_view.key)]).filter_duplicate() + self.assertEqual(view.active, False, "filter_duplicate should return the specific one") diff --git a/addons/website_theme_install/tests/__init__.py b/addons/website_theme_install/tests/__init__.py new file mode 100644 index 00000000000..28efb5bbfb5 --- /dev/null +++ b/addons/website_theme_install/tests/__init__.py @@ -0,0 +1,4 @@ +# -*- coding: utf-8 -*- +# Part of Odoo. See LICENSE file for full copyright and licensing details. + +from . import test_views diff --git a/addons/website_theme_install/tests/test_views.py b/addons/website_theme_install/tests/test_views.py new file mode 100644 index 00000000000..7657dcb19b8 --- /dev/null +++ b/addons/website_theme_install/tests/test_views.py @@ -0,0 +1,151 @@ +# -*- coding: utf-8 -*- +# Part of Odoo. See LICENSE file for full copyright and licensing details. + +from odoo.tests import HttpCase + + +class Crawler(HttpCase): + def test_multi_website_views_retrieving(self): + View = self.env['ir.ui.view'] + Website = self.env['website'] + + website_1 = Website.create({'name': 'Website 1'}) + website_2 = Website.create({'name': 'Website 2'}) + + main_view = View.create({ + 'name': 'Products', + 'type': 'qweb', + 'arch': '<body>Arch is not relevant for this test</body>', + 'key': '_website_sale.products', + }).with_context(load_all_views=True) + + View.with_context(load_all_views=True).create({ + 'name': 'Child View W1', + 'mode': 'extension', + 'inherit_id': main_view.id, + 'arch': '<xpath expr="//body" position="replace">It is really not relevant!</xpath>', + 'key': '_website_sale.child_view_w1', + 'website_id': website_1.id, + 'active': False, + 'customize_show': True, + }) + + # Simulate theme view instal + load on website + theme_view = self.env['theme.ir.ui.view'].with_context(install_filename='/testviews').create({ + 'name': 'Products Theme Kea', + 'mode': 'extension', + 'inherit_id': main_view.id, + 'arch': '<xpath expr="//p" position="replace"><span>C</span></xpath>', + 'key': '_theme_kea_sale.products', + }) + view_from_theme_view_on_w2 = View.with_context(load_all_views=True).create({ + 'name': 'Products Theme Kea', + 'mode': 'extension', + 'inherit_id': main_view.id, + 'arch': '<xpath expr="//body" position="replace">Really really not important for this test</xpath>', + 'key': '_theme_kea_sale.products', + 'website_id': website_2.id, + 'customize_show': True, + }) + self.env['ir.model.data'].create({ + 'module': '_theme_kea_sale', + 'name': 'products', + 'model': 'theme.ir.ui.view', + 'res_id': theme_view.id, + }) + + # ##################################################### ir.ui.view ############################################### + # id | name | website_id | inherit | key | xml_id | + # ---------------------------------------------------------------------------------------------------------------- + # 1 | Products | / | / | _website_sale.products | / | + # 2 | Child View W1 | 1 | 1 | _website_sale.child_view_w1 | / | + # 3 | Products Theme Kea | 2 | 1 | _theme_kea_sale.products | / | + + # ################################################# theme.ir.ui.view ############################################# + # id | name | inherit | key | xml_id | + # ---------------------------------------------------------------------------------------------------------------- + # 1 | Products Theme Kea | 1 | _theme_kea_sale.products | _theme_kea_sale.products | + + with self.assertRaises(ValueError): + # It should crash as it should not find a view on website 1 for '_theme_kea_sale.products', !!and certainly not a theme.ir.ui.view!!. + view = View.with_context(website_id=website_1.id)._view_obj('_theme_kea_sale.products') + view = View.with_context(website_id=website_2.id)._view_obj('_theme_kea_sale.products') + self.assertEquals(len(view), 1, "It should find the ir.ui.view with key '_theme_kea_sale.products' on website 2..") + self.assertEquals(view._name, 'ir.ui.view', "..and not a theme.ir.ui.view") + + views = View.with_context(website_id=website_1.id).get_related_views('_website_sale.products') + self.assertEquals(len(views), 2, "It should not mix apples and oranges, only ir.ui.view ['_website_sale.products', '_website_sale.child_view_w1'] should be returned") + views = View.with_context(website_id=website_2.id).get_related_views('_website_sale.products') + self.assertEquals(len(views), 2, "It should not mix apples and oranges, only ir.ui.view ['_website_sale.products', '_theme_kea_sale.products'] should be returned") + + # Part 2 of the test, it test the same stuff but from a higher level (get_related_views ends up calling _view_obj) + called_theme_view = self.env['theme.ir.ui.view'].with_context(install_filename='/testviews').create({ + 'name': 'Called View Kea', + 'arch': '<div></div>', + 'key': '_theme_kea_sale.t_called_view', + }) + View.create({ + 'name': 'Called View Kea', + 'type': 'qweb', + 'arch': '<div></div>', + 'key': '_theme_kea_sale.t_called_view', + 'website_id': website_2.id, + }).with_context(load_all_views=True) + self.env['ir.model.data'].create({ + 'module': '_theme_kea_sale', + 'name': 't_called_view', + 'model': 'theme.ir.ui.view', + 'res_id': called_theme_view.id, + }) + view_from_theme_view_on_w2.write({'arch': '<t t-call="_theme_kea_sale.t_called_view"/>'}) + + # ##################################################### ir.ui.view ############################################### + # id | name | website_id | inherit | key | xml_id | + # ---------------------------------------------------------------------------------------------------------------- + # 1 | Products | / | / | _website_sale.products | / | + # 2 | Child View W1 | 1 | 1 | _website_sale.child_view_w1 | / | + # 3 | Products Theme Kea | 2 | 1 | _theme_kea_sale.products | / | + # 4 | Called View Kea | 2 | / | _theme_kea_sale.t_called_view | / | + + # ################################################# theme.ir.ui.view ############################################# + # id | name | inherit | key | xml_id | + # ---------------------------------------------------------------------------------------------------------------- + # 1 | Products Theme Kea | 1 | _theme_kea_sale.products | _theme_kea_sale.products | + # 1 | Called View Kea | / | _theme_kea_sale.t_called_view | _theme_kea_sale.t_called_view | + + # Next line should not crash (was mixing apples and oranges - ir.ui.view and theme.ir.ui.view) + views = View.with_context(website_id=website_1.id).get_related_views('_website_sale.products') + self.assertEquals(len(views), 2, "It should not mix apples and oranges, only ir.ui.view ['_website_sale.products', '_website_sale.child_view_w1'] should be returned (2)") + views = View.with_context(website_id=website_2.id).get_related_views('_website_sale.products') + self.assertEquals(len(views), 3, "It should not mix apples and oranges, only ir.ui.view ['_website_sale.products', '_theme_kea_sale.products', '_theme_kea_sale.t_called_view'] should be returned") + + # ######################################################## + # Test the controller (which is calling get_related_views) + self.authenticate("admin", "admin") + base_url = self.env['ir.config_parameter'].sudo().get_param('web.base.url') + + # Simulate website 2 + url = base_url + '/website/force_website' + json = {'params': {'website_id': website_2.id}} + self.opener.post(url=url, json=json) + + # Test controller + url = base_url + '/website/get_switchable_related_views' + json = {'params': {'key': '_website_sale.products'}} + response = self.opener.post(url=url, json=json) + self.assertEqual(response.status_code, 200) + self.assertEqual(len(response.json()['result']), 1, "Only '_theme_kea_sale.products' should be returned as it is the only customize_show related view in website 2 context") + self.assertEqual(response.json()['result'][0]['key'], '_theme_kea_sale.products', "Only '_theme_kea_sale.products' should be returned") + + # Simulate website 1 + url = base_url + '/website/force_website' + json = {'params': {'website_id': website_1.id}} + self.opener.post(url=url, json=json) + + # Test controller + url = base_url + '/website/get_switchable_related_views' + json = {'params': {'key': '_website_sale.products'}} + response = self.opener.post(url=url, json=json) + self.assertEqual(response.status_code, 200) + self.assertEqual(len(response.json()['result']), 1, "Only '_website_sale.child_view_w1' should be returned as it is the only customize_show related view in website 1 context") + self.assertEqual(response.json()['result'][0]['key'], '_website_sale.child_view_w1', "Only '_website_sale.child_view_w1' should be returned") |
