[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

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

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):

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%
@@ -506,6 +501,42 @@ class Website(models.Model):
     def is_public_user(self):
         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']

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")

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

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")