From de71a61a8b4825cecb882a63324725e817ac6d40 Mon Sep 17 00:00:00 2001 From: gsinghpal Date: Sun, 19 Apr 2026 00:51:32 -0400 Subject: [PATCH] fix(fusion_accounting_migration): add menu + tighten safety-guard test coverage Addresses code review feedback on Task 17: - Add menuitem so 'Fusion Accounting -> Migrate from Enterprise' is reachable (the UserError guidance now actually works). Placed at top level since parenting under fusion_accounting_ai.menu_fusion_accounting_root would require adding that module as a hard dep, which is wrong semantically (migration should not require AI). Both menuitems carry the admin group so the menu stays hidden from users who can't open the wizard anyway. - Update the UserError wording to "Fusion Accounting -> Migrate from Enterprise" (no longer "Settings -> ...") to match the actual menu location; 'migration' is preserved per the test's assertIn check. - Add skipTest guard to test_uninstall_not_blocked_when_migration_completed so it doesn't pass vacuously on Community-only CI (the guard's `if not installed: continue` would otherwise return True regardless of the flag value, giving a false green). - Move GUARDED_MODULES import to top of wizards/migration_wizard.py (no circular-import risk -- models/ir_module_module.py doesn't import from wizards/). - Expand docstrings on button_immediate_uninstall and module_uninstall overrides to note they may both fire in a single UI uninstall call and that the guard is idempotent (pure read + raise). Made-with: Cursor --- .../models/ir_module_module.py | 22 ++++++++++++++----- .../tests/test_safety_guard.py | 15 +++++++++++-- .../wizards/migration_wizard.py | 3 ++- .../wizards/migration_wizard_views.xml | 18 +++++++++++++++ 4 files changed, 50 insertions(+), 8 deletions(-) diff --git a/fusion_accounting_migration/models/ir_module_module.py b/fusion_accounting_migration/models/ir_module_module.py index b5fccddf..28968435 100644 --- a/fusion_accounting_migration/models/ir_module_module.py +++ b/fusion_accounting_migration/models/ir_module_module.py @@ -6,8 +6,8 @@ guard checks an ir.config_parameter flag named: fusion_accounting.migration..completed If the flag is False/unset and the module is currently installed, the guard -raises UserError pointing the user to Settings -> Fusion Accounting -> -Migrate from Enterprise. +raises UserError pointing the user to the top-level +Fusion Accounting -> Migrate from Enterprise menu. The migration wizard sets the flag to True after a successful migration run for that module. @@ -51,7 +51,7 @@ class IrModuleModule(models.Model): raise UserError(_( "Cannot uninstall %s: the Fusion Accounting migration " "for this module has not run yet. Please open\n" - " Settings -> Fusion Accounting -> Migrate from Enterprise\n" + " Fusion Accounting -> Migrate from Enterprise\n" "and run the migration before uninstalling. Once the " "migration has completed, the safety guard will allow " "uninstall.\n\n" @@ -62,11 +62,23 @@ class IrModuleModule(models.Model): return True def button_immediate_uninstall(self): - """Override to invoke the safety guard before allowing uninstall.""" + """Override to invoke the safety guard before allowing uninstall. + + Both this and ``module_uninstall`` below can fire in a single UI + uninstall call (button_immediate_uninstall -> module_uninstall). The + guard is a pure read + raise, so re-running it is idempotent: on the + happy path it just re-confirms; on the blocked path the first call + already raised and the second is never reached. + """ self._fusion_check_uninstall_guard(self.mapped('name')) return super().button_immediate_uninstall() def module_uninstall(self): - """Override the lower-level uninstall path too (CLI / API uninstall).""" + """Override the lower-level uninstall path too (CLI / API uninstall). + + See ``button_immediate_uninstall`` above -- both overrides may run in + the same UI uninstall; the guard is idempotent so double-invocation + is safe. + """ self._fusion_check_uninstall_guard(self.mapped('name')) return super().module_uninstall() diff --git a/fusion_accounting_migration/tests/test_safety_guard.py b/fusion_accounting_migration/tests/test_safety_guard.py index 67532c33..5f301664 100644 --- a/fusion_accounting_migration/tests/test_safety_guard.py +++ b/fusion_accounting_migration/tests/test_safety_guard.py @@ -7,11 +7,22 @@ class TestSafetyGuard(TransactionCase): """Verify the safety guard blocks Enterprise uninstall when migration hasn't run.""" def test_uninstall_not_blocked_when_migration_completed(self): - """If the per-module migration flag is set, uninstall is allowed.""" + """If the per-module migration flag is set, uninstall is allowed. + + Skip if account_accountant isn't installed -- otherwise the guard's + `if not installed: continue` short-circuit would make this test pass + vacuously without exercising the flag-check branch. + """ + Module = self.env['ir.module.module'].sudo() + if not Module.search_count([ + ('name', '=', 'account_accountant'), + ('state', '=', 'installed'), + ]): + self.skipTest("account_accountant not installed in this DB") self.env['ir.config_parameter'].sudo().set_param( 'fusion_accounting.migration.account_accountant.completed', 'True' ) - guard = self.env['ir.module.module']._fusion_check_uninstall_guard(['account_accountant']) + guard = Module._fusion_check_uninstall_guard(['account_accountant']) self.assertTrue(guard, "Guard should pass when migration flag is set") def test_uninstall_blocked_when_migration_pending(self): diff --git a/fusion_accounting_migration/wizards/migration_wizard.py b/fusion_accounting_migration/wizards/migration_wizard.py index cdde55da..7ca98363 100644 --- a/fusion_accounting_migration/wizards/migration_wizard.py +++ b/fusion_accounting_migration/wizards/migration_wizard.py @@ -10,6 +10,8 @@ the bank-rec verification check. Phase 6 will add asset migration, etc. from odoo import _, api, fields, models +from ..models.ir_module_module import GUARDED_MODULES + class FusionMigrationWizard(models.TransientModel): _name = "fusion.migration.wizard" @@ -34,7 +36,6 @@ class FusionMigrationWizard(models.TransientModel): @api.depends_context('uid') def _compute_detected(self): Mod = self.env['ir.module.module'].sudo() - from ..models.ir_module_module import GUARDED_MODULES installed = Mod.search([ ('name', 'in', list(GUARDED_MODULES)), ('state', '=', 'installed'), diff --git a/fusion_accounting_migration/wizards/migration_wizard_views.xml b/fusion_accounting_migration/wizards/migration_wizard_views.xml index dd27259b..17446e9e 100644 --- a/fusion_accounting_migration/wizards/migration_wizard_views.xml +++ b/fusion_accounting_migration/wizards/migration_wizard_views.xml @@ -25,4 +25,22 @@ form new + + + +