Merge pull request #928 from LasLabs/hotfix/10.0/module_auto_update-forward_port_imp

[REF][module_auto_update] Forward port v9 improvements

module_auto_update/__manifest__.py

@@ -5,7 +5,7 @@
 {
     'name': 'Module Auto Update',
     'summary': 'Automatically update Odoo modules',
-    'version': '10.0.1.0.0',
+    'version': '10.0.1.0.1',
     'category': 'Extra Tools',
     'website': 'https://odoo-community.org/',
     'author': 'LasLabs, '

module_auto_update/models/module.py

@@ -30,11 +30,16 @@ class Module(models.Model):
         ).split(",")
 
         for r in self:
+            try:
                 r.checksum_dir = dirhash(
                     get_module_path(r.name),
                     'sha1',
                     excluded_extensions=exclude,
                 )
+            except TypeError:
+                _logger.debug(
+                    "Cannot compute dir hash for %s, module not found",
+                    r.display_name)
 
     def _store_checksum_installed(self, vals):
         if self.env.context.get('retain_checksum_installed'):
@@ -46,6 +51,13 @@ class Module(models.Model):
             elif vals.get('state') == 'uninstalled':
                 self.write({'checksum_installed': False})
 
+    @api.multi
+    def button_uninstall(self):
+        return super(
+            Module,
+            self.with_context(module_uninstall=True),
+        ).button_uninstall()
+
     @api.multi
     def button_uninstall_cancel(self):
         return super(
@@ -66,16 +78,6 @@ class Module(models.Model):
         res._store_checksum_installed(vals)
         return res
 
-    @api.model
-    def update_list(self):
-        res = super(Module, self).update_list()
-        installed_modules = self.search([('state', '=', 'installed')])
-        upgradeable_modules = installed_modules.filtered(
-            lambda r: r.checksum_dir != r.checksum_installed,
-        )
-        upgradeable_modules.write({'state': "to upgrade"})
-        return res
-
     @api.multi
     def write(self, vals):
         res = super(Module, self).write(vals)

module_auto_update/tests/test_module.py

@@ -3,6 +3,7 @@
 # License LGPL-3.0 or later (http://www.gnu.org/licenses/lgpl).
 
 import logging
+import os
 import tempfile
 
 import mock
@@ -21,6 +22,10 @@ except ImportError:
 model = 'odoo.addons.module_auto_update.models.module'
 
 
+class EndTestException(Exception):
+    pass
+
+
 class TestModule(TransactionCase):
 
     def setUp(self):
@@ -35,6 +40,7 @@ class TestModule(TransactionCase):
             'sha1',
             excluded_extensions=['pyc', 'pyo'],
         )
+        self.own_writeable = os.access(self.own_dir_path, os.W_OK)
 
     @mock.patch('%s.get_module_path' % model)
     def create_test_module(self, vals, get_module_path_mock):
@@ -52,6 +58,8 @@ class TestModule(TransactionCase):
     def test_compute_checksum_dir_ignore_excluded(self):
         """It should exclude .pyc/.pyo extensions from checksum
         calculations"""
+        if not self.own_writeable:
+            self.skipTest("Own directory not writeable")
         with tempfile.NamedTemporaryFile(
                 suffix='.pyc', dir=self.own_dir_path):
             self.assertEqual(
@@ -62,6 +70,8 @@ class TestModule(TransactionCase):
     def test_compute_checksum_dir_recomputes_when_file_added(self):
         """It should return a different value when a non-.pyc/.pyo file is
         added to the module directory"""
+        if not self.own_writeable:
+            self.skipTest("Own directory not writeable")
         with tempfile.NamedTemporaryFile(
                 suffix='.py', dir=self.own_dir_path):
             self.assertNotEqual(
@@ -116,6 +126,41 @@ class TestModule(TransactionCase):
             'overwrite',
         )
 
+    @mock.patch('%s.get_module_path' % model)
+    def test_button_uninstall_no_recompute(self, module_path_mock):
+        """It should not attempt update on `button_uninstall`."""
+        module_path_mock.return_value = self.own_dir_path
+        vals = {
+            'name': 'module_auto_update_test_module',
+            'state': 'installed',
+        }
+        test_module = self.create_test_module(vals)
+        test_module.checksum_installed = 'test'
+        uninstall_module = self.env['ir.module.module'].search([
+            ('name', '=', 'web'),
+        ])
+        uninstall_module.button_uninstall()
+        self.assertNotEqual(
+            test_module.state, 'to upgrade',
+            'Auto update logic was triggered during uninstall.',
+        )
+
+    def test_button_immediate_uninstall_no_recompute(self):
+        """It should not attempt update on `button_immediate_uninstall`."""
+
+        uninstall_module = self.env['ir.module.module'].search([
+            ('name', '=', 'web'),
+        ])
+
+        try:
+            mk = mock.MagicMock()
+            uninstall_module._patch_method('button_uninstall', mk)
+            mk.side_effect = EndTestException
+            with self.assertRaises(EndTestException):
+                uninstall_module.button_immediate_uninstall()
+        finally:
+            uninstall_module._revert_method('button_uninstall')
+
     def test_button_uninstall_cancel(self):
         """It should preserve checksum_installed when cancelling uninstall"""
         self.own_module.write({'state': 'to remove'})
@@ -154,31 +199,33 @@ class TestModule(TransactionCase):
         )
 
     @mock.patch('%s.get_module_path' % model)
-    def test_update_list(self, get_module_path_mock):
+    def test_get_module_list(self, module_path_mock):
         """It should change the state of modules with different
         checksum_dir and checksum_installed to 'to upgrade'"""
-        get_module_path_mock.return_value = self.own_dir_path
+        module_path_mock.return_value = self.own_dir_path
         vals = {
             'name': 'module_auto_update_test_module',
             'state': 'installed',
         }
         test_module = self.create_test_module(vals)
         test_module.checksum_installed = 'test'
-        self.env['ir.module.module'].update_list()
+        self.env['base.module.upgrade'].get_module_list()
         self.assertEqual(
             test_module.state, 'to upgrade',
             'List update does not mark upgradeable modules "to upgrade"',
         )
 
-    def test_update_list_only_changes_installed(self):
+    @mock.patch('%s.get_module_path' % model)
+    def test_get_module_list_only_changes_installed(self, module_path_mock):
         """It should not change the state of a module with a former state
         other than 'installed' to 'to upgrade'"""
+        module_path_mock.return_value = self.own_dir_path
         vals = {
             'name': 'module_auto_update_test_module',
             'state': 'uninstalled',
         }
         test_module = self.create_test_module(vals)
-        self.env['ir.module.module'].update_list()
+        self.env['base.module.upgrade'].get_module_list()
         self.assertNotEqual(
             test_module.state, 'to upgrade',
             'List update changed state of an uninstalled module',

module_auto_update/tests/test_module_upgrade.py

@@ -31,12 +31,14 @@ class TestModuleUpgrade(TransactionCase):
 
     @mock.patch.object(Registry, 'new')
     def test_upgrade_module(self, new_mock):
-        """It should call update_list method on ir.module.module"""
-        update_list_mock = mock.MagicMock()
-        self.env['ir.module.module']._patch_method(
-            'update_list',
-            update_list_mock,
+        """Calls get_module_list when upgrading in api.model mode"""
+        get_module_list_mock = mock.MagicMock()
+        try:
+            self.env['base.module.upgrade']._patch_method(
+                'get_module_list',
+                get_module_list_mock,
             )
             self.env['base.module.upgrade'].upgrade_module()
-        update_list_mock.assert_called_once_with()
-        self.env['ir.module.module']._revert_method('update_list')
+            get_module_list_mock.assert_called_once_with()
+        finally:
+            self.env['base.module.upgrade']._revert_method('get_module_list')

module_auto_update/wizards/module_upgrade.py

@@ -8,6 +8,17 @@ from odoo import api, models
 class ModuleUpgrade(models.TransientModel):
     _inherit = 'base.module.upgrade'
 
+    @api.model
+    def get_module_list(self):
+        if not self.env.context.get('module_uninstall'):
+            Module = self.env["ir.module.module"]
+            installed_modules = Module.search([('state', '=', 'installed')])
+            upgradeable_modules = installed_modules.filtered(
+                lambda r: r.checksum_dir != r.checksum_installed,
+            )
+            upgradeable_modules.button_upgrade()
+        return super(ModuleUpgrade, self).get_module_list()
+
     @api.multi
     def upgrade_module_cancel(self):
         return super(
@@ -17,5 +28,7 @@ class ModuleUpgrade(models.TransientModel):
 
     @api.multi
     def upgrade_module(self):
-        self.env['ir.module.module'].update_list()
-        super(ModuleUpgrade, self).upgrade_module()
+        # Compute updates by checksum when called in @api.model fashion
+        if not self:
+            self.get_module_list()
+        return super(ModuleUpgrade, self).upgrade_module()