[FIX] base_import_module: raise UserError if import fails

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) <ryv@odoo.com>

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

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

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

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"""
+                <data>
+                    <record id="foo" model="res.partner">
+                        <field name="name">foo</field>
+                    </record>
+                </data>
+            """),
+            # typo in model to throw an error
+            ('foo/bar.xml', b"""
+                <data>
+                    <record id="bar" model="res.prtner">
+                        <field name="name">bar</field>
+                    </record>
+                </data>
+            """),
+        ]
+        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"""