diff --git a/module_auto_update/__manifest__.py b/module_auto_update/__manifest__.py index db65d269a..57a5846db 100644 --- a/module_auto_update/__manifest__.py +++ b/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, ' diff --git a/module_auto_update/models/module.py b/module_auto_update/models/module.py index 4d9ccec59..273d72588 100644 --- a/module_auto_update/models/module.py +++ b/module_auto_update/models/module.py @@ -30,11 +30,16 @@ class Module(models.Model): ).split(",") for r in self: - r.checksum_dir = dirhash( - get_module_path(r.name), - 'sha1', - excluded_extensions=exclude, - ) + 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) diff --git a/module_auto_update/tests/test_module.py b/module_auto_update/tests/test_module.py index 5d499fc27..bd25ed07a 100644 --- a/module_auto_update/tests/test_module.py +++ b/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', diff --git a/module_auto_update/tests/test_module_upgrade.py b/module_auto_update/tests/test_module_upgrade.py index edc24fd8e..0f908d5e0 100644 --- a/module_auto_update/tests/test_module_upgrade.py +++ b/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, - ) - self.env['base.module.upgrade'].upgrade_module() - update_list_mock.assert_called_once_with() - self.env['ir.module.module']._revert_method('update_list') + """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() + get_module_list_mock.assert_called_once_with() + finally: + self.env['base.module.upgrade']._revert_method('get_module_list') diff --git a/module_auto_update/wizards/module_upgrade.py b/module_auto_update/wizards/module_upgrade.py index e9b69e07c..188030dd8 100644 --- a/module_auto_update/wizards/module_upgrade.py +++ b/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()