From 2e32cc5aa35044e8f0e7c299c8612a8095d9f479 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Theys?= Date: Fri, 28 Dec 2018 12:14:39 +0000 Subject: [PATCH] [FIX] base: remove copies of views when removing module If a module has a view that has been copied (copy on write, web editor, customize show, manual copy, ...), currently the copied view will not be deleted if the module of the original view is removed. If the copy is not used anymore, it is not a problem (but still polluting the database), but if the copy is still used (ex. it is an inherit of a view still in use), then it will crash if that copy references other data (views, fields, ...) that were in the module that is now removed. Eg. The view `website_sale_comparison.product_add_to_compare` is inheriting `website_sale.product` and calls `website_sale_comparison.add_to_compare`. If the view `website_sale_comparison.product_add_to_compare` is copied (when using the web editor on the product page -> copy on write is making a copy of the `product` view and all its tree), then the product page on website will crash when the module `website_sale_comparison` is removed because `product_add_to_compare` will call the deleted view `add_to_compare`. The solution is to also remove copied views when removing a module. Instead of just using the xml id, we should look for the `key` starting with the module name currently removed, because copied views don't have an external id. task-1920005 PR: #29753 --- .../website_sale_comparison/tests/__init__.py | 4 ++ .../tests/test_website_sale_comparison.py | 39 +++++++++++++++++++ odoo/addons/base/models/ir_module.py | 16 ++++++++ 3 files changed, 59 insertions(+) create mode 100644 addons/website_sale_comparison/tests/__init__.py create mode 100644 addons/website_sale_comparison/tests/test_website_sale_comparison.py diff --git a/addons/website_sale_comparison/tests/__init__.py b/addons/website_sale_comparison/tests/__init__.py new file mode 100644 index 00000000000..0a54e54e5cf --- /dev/null +++ b/addons/website_sale_comparison/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_website_sale_comparison diff --git a/addons/website_sale_comparison/tests/test_website_sale_comparison.py b/addons/website_sale_comparison/tests/test_website_sale_comparison.py new file mode 100644 index 00000000000..5913b00bb40 --- /dev/null +++ b/addons/website_sale_comparison/tests/test_website_sale_comparison.py @@ -0,0 +1,39 @@ +# -*- coding: utf-8 -*- +# Part of Odoo. See LICENSE file for full copyright and licensing details. + +import odoo.tests + + +@odoo.tests.tagged('-at_install', 'post_install') +class TestWebsiteSaleComparison(odoo.tests.TransactionCase): + def test_01_website_sale_comparison_remove(self): + """ This tour makes sure the product page still works after the module + `website_sale_comparison` has been removed. + + Technically it tests the removal of copied views by the base method + `_remove_copied_views`. The problematic view that has to be removed is + `product_add_to_compare` because it has a reference to `add_to_compare`. + """ + + Website0 = self.env['website'].with_context(website_id=None) + Website1 = self.env['website'].with_context(website_id=1) + + # Retrieve the generic view + product = Website0.viewref('website_sale.product') + # Trigger COW to create specific views of the whole tree + product.with_context(website_id=1).write({'name': 'Trigger COW'}) + + # Verify initial state: the specific views exist + self.assertEquals(Website1.viewref('website_sale.product').website_id.id, 1) + self.assertEquals(Website1.viewref('website_sale_comparison.product_add_to_compare').website_id.id, 1) + + # Remove the module (use `module_uninstall` because it is enough to test + # what we want here, no need/can't use `button_immediate_uninstall` + # because it would commit the test transaction) + website_sale_comparison = self.env['ir.module.module'].search([('name', '=', 'website_sale_comparison')]) + website_sale_comparison.module_uninstall() + + # Check that the generic view is correctly removed + self.assertFalse(Website0.viewref('website_sale_comparison.product_add_to_compare', raise_if_not_found=False)) + # Check that the specific view is correctly removed + self.assertFalse(Website1.viewref('website_sale_comparison.product_add_to_compare', raise_if_not_found=False)) diff --git a/odoo/addons/base/models/ir_module.py b/odoo/addons/base/models/ir_module.py index 350b4d7a489..a270a46dfb1 100644 --- a/odoo/addons/base/models/ir_module.py +++ b/odoo/addons/base/models/ir_module.py @@ -25,6 +25,7 @@ import lxml.html import odoo from odoo import api, fields, models, modules, tools, _ from odoo.exceptions import AccessDenied, UserError +from odoo.osv import expression from odoo.tools.parse_version import parse_version from odoo.tools.misc import topological_sort from odoo.http import request @@ -456,9 +457,24 @@ class Module(models.Model): """ modules_to_remove = self.mapped('name') self.env['ir.model.data']._module_data_uninstall(modules_to_remove) + self._remove_copied_views() self.write({'state': 'uninstalled', 'latest_version': False}) return True + @api.multi + def _remove_copied_views(self): + """ Remove the copies of the views installed by the modules in `self`. + + Those copies do not have an external id so they will not be cleaned by + `_module_data_uninstall`. This is why we rely on `key` instead. + + It is important to remove these copies because using them will crash if + they rely on data that don't exist anymore if the module is removed. + """ + domain = expression.OR([[('key', '=like', m.name + '.%')] for m in self]) + orphans = self.env['ir.ui.view'].with_context(active_test=False).search(domain) + orphans.unlink() + @api.multi @api.returns('self') def downstream_dependencies(self, known_deps=None,