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
This commit is contained in:
@@ -6,8 +6,8 @@ guard checks an ir.config_parameter flag named:
|
||||
fusion_accounting.migration.<module_name>.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()
|
||||
|
||||
@@ -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):
|
||||
|
||||
@@ -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'),
|
||||
|
||||
@@ -25,4 +25,22 @@
|
||||
<field name="view_mode">form</field>
|
||||
<field name="target">new</field>
|
||||
</record>
|
||||
|
||||
<!--
|
||||
Top-level "Fusion Accounting" menu so the UserError guidance
|
||||
("Fusion Accounting -> Migrate from Enterprise") is actually reachable.
|
||||
Placed at top level (no parent) because the migration is a one-time
|
||||
admin task; making it visible during switchover is the point.
|
||||
`groups` hides the menu from non-admins (mirroring the ACL on the wizard).
|
||||
-->
|
||||
<menuitem id="menu_fusion_migration_root"
|
||||
name="Fusion Accounting"
|
||||
sequence="95"
|
||||
groups="fusion_accounting_core.group_fusion_accounting_admin"/>
|
||||
<menuitem id="menu_fusion_migration_wizard"
|
||||
name="Migrate from Enterprise"
|
||||
parent="menu_fusion_migration_root"
|
||||
action="action_fusion_migration_wizard"
|
||||
sequence="10"
|
||||
groups="fusion_accounting_core.group_fusion_accounting_admin"/>
|
||||
</odoo>
|
||||
|
||||
Reference in New Issue
Block a user