From 9da06bc19bdb647ce230411b1e2b227fbd33415e Mon Sep 17 00:00:00 2001 From: vava-odoo Date: Wed, 10 Jan 2024 10:20:19 +0100 Subject: [PATCH] [FIX] base_import_module: raise UserError if import fails MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Before this commit, when an error occurred during the installation of a data module, there was no displayed error nor a rollback to remove data already installed from the module. This commit makes sure that, if an error occurs, it is displayed through a UserError that will do a rollback to remove every record from the module. closes odoo/odoo#148823 Signed-off-by: Rémy Voet (ryv) --- .../i18n/base_import_module.pot | 11 ++++++ addons/base_import_module/models/ir_module.py | 15 +++----- addons/base_import_module/tests/__init__.py | 2 +- .../tests/test_import_module.py | 37 +++++++++++++++++-- 4 files changed, 52 insertions(+), 13 deletions(-) diff --git a/addons/base_import_module/i18n/base_import_module.pot b/addons/base_import_module/i18n/base_import_module.pot index 323e0d115e0..9b8f52139e1 100644 --- a/addons/base_import_module/i18n/base_import_module.pot +++ b/addons/base_import_module/i18n/base_import_module.pot @@ -91,6 +91,17 @@ msgstr "" msgid "Display Name" msgstr "" +#. module: base_import_module +#. odoo-python +#: code:addons/base_import_module/models/ir_module.py:0 +#, python-format +msgid "" +"Error while importing module '%(module)s'.\n" +"\n" +" %(error_message)s \n" +"\n" +msgstr "" + #. module: base_import_module #. odoo-python #: code:addons/base_import_module/models/ir_module.py:0 diff --git a/addons/base_import_module/models/ir_module.py b/addons/base_import_module/models/ir_module.py index 58ad7950ba4..1288fcd7010 100644 --- a/addons/base_import_module/models/ir_module.py +++ b/addons/base_import_module/models/ir_module.py @@ -207,8 +207,6 @@ class IrModule(models.Model): if not zipfile.is_zipfile(module_file): raise UserError(_('Only zip files are supported.')) - success = [] - errors = dict() module_names = [] with zipfile.ZipFile(module_file, "r") as z: for zf in z.filelist: @@ -252,15 +250,14 @@ class IrModule(models.Model): try: # assert mod_name.startswith('theme_') path = opj(module_dir, mod_name) - if self.sudo()._import_module(mod_name, path, force=force, with_demo=with_demo): - success.append(mod_name) + self.sudo()._import_module(mod_name, path, force=force, with_demo=with_demo) except Exception as e: _logger.exception('Error while importing module') - errors[mod_name] = exception_to_unicode(e) - r = ["Successfully imported module '%s'" % mod for mod in success] - for mod, error in errors.items(): - r.append("Error while importing module '%s'.\n\n %s \n Make sure those modules are installed and try again." % (mod, error)) - return '\n'.join(r), module_names + raise UserError(_( + "Error while importing module '%(module)s'.\n\n %(error_message)s \n\n", + module=mod_name, error_message=exception_to_unicode(e), + )) + return "", module_names def module_uninstall(self): # Delete an ir_module_module record completely if it was an imported diff --git a/addons/base_import_module/tests/__init__.py b/addons/base_import_module/tests/__init__.py index 0a0c7d51b6a..7099ed13380 100644 --- a/addons/base_import_module/tests/__init__.py +++ b/addons/base_import_module/tests/__init__.py @@ -1,5 +1,5 @@ # -*- coding: utf-8 -*- # Part of Odoo. See LICENSE file for full copyright and licensing details. -from . import test_import_module + from . import test_cloc from . import test_import_module diff --git a/addons/base_import_module/tests/test_import_module.py b/addons/base_import_module/tests/test_import_module.py index 8adb6595934..31b5a37af62 100644 --- a/addons/base_import_module/tests/test_import_module.py +++ b/addons/base_import_module/tests/test_import_module.py @@ -15,6 +15,7 @@ from unittest.mock import patch from odoo import release from odoo.addons import __path__ as __addons_path__ +from odoo.exceptions import UserError from odoo.tools import mute_logger @@ -77,9 +78,39 @@ class TestImportModule(odoo.tests.TransactionCase): files = [ ('foo/__manifest__.py', b"foo") ] - with mute_logger("odoo.addons.base_import_module.models.ir_module"): - result = self.import_zipfile(files) - self.assertIn("Error while importing module 'foo'", result[0]) + error_message = "Error while importing module 'foo'" + with ( + mute_logger("odoo.addons.base_import_module.models.ir_module"), + self.assertRaises(UserError, msg=error_message), + ): + self.import_zipfile(files) + + def test_import_zip_invalid_data(self): + """Assert no data remains in the db if module import fails""" + files = [ + ('foo/__manifest__.py', b"{'data': ['foo.xml', 'bar.xml']}"), + ('foo/foo.xml', b""" + + + foo + + + """), + # typo in model to throw an error + ('foo/bar.xml', b""" + + + bar + + + """), + ] + with ( + mute_logger("odoo.addons.base_import_module.models.ir_module"), + self.assertRaises(UserError), + ): + self.import_zipfile(files) + self.assertFalse(self.env.ref('foo.foo', raise_if_not_found=False)) def test_import_zip_data_not_in_manifest(self): """Assert a data file not mentioned in the manifest is not imported"""