summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorRomain Derie <rde@odoo.com>2018-12-07 10:38:37 +0000
committerRomain Derie <rde@odoo.com>2018-12-19 18:23:36 +0000
commitf7df35540b8404a14e8d29af80b8e8642a4c1bdb (patch)
tree3863f5d566a5dbdcf01b742cf45b20242d52e1d9
parenteb19016ba35d4dc4723edcc244fd412680b5ff2f (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.py7
-rw-r--r--addons/website/models/ir_ui_view.py35
-rw-r--r--addons/website/models/website.py41
-rw-r--r--addons/website/tests/test_views.py31
-rw-r--r--addons/website_theme_install/tests/__init__.py4
-rw-r--r--addons/website_theme_install/tests/test_views.py151
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")