diff --git a/spp_base_common/README.rst b/spp_base_common/README.rst index 9a77499d1..6fa4869f7 100644 --- a/spp_base_common/README.rst +++ b/spp_base_common/README.rst @@ -120,6 +120,21 @@ Dependencies Changelog ========= +19.0.2.0.3 +~~~~~~~~~~ + +- fix: make the menu-icon decoration run by ``ir.module.module.next()`` + best-effort (#383). The pass now runs in its own savepoint and any + ``psycopg2.Error`` raised while decorating is logged and skipped + instead of propagating out of the module operation and leaving the + transaction aborted, so this hook is never the cause of a failed + install or upgrade; missing menu xmlids use + ``raise_if_not_found=False``. The caller's own pending writes are + flushed before the guard and stay the caller's responsibility. Odoo 19 + only calls ``next()`` from the immediate install/upgrade buttons; the + registry-load exposure of the same shape lives in + ``spp_hide_menus_base`` and is tracked in #526. + 19.0.2.0.2 ~~~~~~~~~~ diff --git a/spp_base_common/__manifest__.py b/spp_base_common/__manifest__.py index 09e67451d..550c135b5 100644 --- a/spp_base_common/__manifest__.py +++ b/spp_base_common/__manifest__.py @@ -5,7 +5,7 @@ { "name": "OpenSPP Base (Common)", "category": "OpenSPP/Core", - "version": "19.0.2.0.2", + "version": "19.0.2.0.3", "sequence": 1, "author": "OpenSPP.org", "website": "https://github.com/OpenSPP/OpenSPP2", diff --git a/spp_base_common/models/ir_module_module.py b/spp_base_common/models/ir_module_module.py index a9e90221c..de94c9236 100644 --- a/spp_base_common/models/ir_module_module.py +++ b/spp_base_common/models/ir_module_module.py @@ -1,5 +1,7 @@ import logging +import psycopg2 + from odoo import models _logger = logging.getLogger(__name__) @@ -73,16 +75,37 @@ class IrModuleModule(models.Model): } def update_menu_icons(self): + """Point the root menus of known third-party apps at OpenSPP icons. + + Purely cosmetic and best-effort: a database error while decorating + must never abort the module operation that triggered it, so the + whole pass runs in its own savepoint and is skipped on failure. + + The caller's pending ORM writes are flushed first so that only the + decoration itself is covered by the guard; a failure in the caller's + own writes stays the caller's error. Retryable errors (serialization + failures, deadlocks) are deliberately swallowed too: retrying the + module operation would rebuild the registry for a cosmetic write. + """ + self.env.cr.flush() + try: + with self.env.cr.savepoint(): + self._write_menu_icons() + except psycopg2.Error: + _logger.warning( + "Skipping the OpenSPP app menu icon update because the database reported an error; " + "the menus keep their current icons and the module operation continues", + exc_info=True, + ) + + def _write_menu_icons(self): for module in self.search([]): icon_info = self.ICON_MAP.get(module.name) - if icon_info: - try: - menu = self.env.ref(icon_info["menu_xml_id"]) - except ValueError: - menu = False - - if menu: - menu.write({"web_icon": icon_info["icon"]}) + if not icon_info: + continue + menu = self.env.ref(icon_info["menu_xml_id"], raise_if_not_found=False) + if menu: + menu.write({"web_icon": icon_info["icon"]}) def next(self): # Call your icon update logic first diff --git a/spp_base_common/readme/HISTORY.md b/spp_base_common/readme/HISTORY.md index 2812444f2..62038fd7c 100644 --- a/spp_base_common/readme/HISTORY.md +++ b/spp_base_common/readme/HISTORY.md @@ -1,3 +1,7 @@ +### 19.0.2.0.3 + +- fix: make the menu-icon decoration run by `ir.module.module.next()` best-effort (#383). The pass now runs in its own savepoint and any `psycopg2.Error` raised while decorating is logged and skipped instead of propagating out of the module operation and leaving the transaction aborted, so this hook is never the cause of a failed install or upgrade; missing menu xmlids use `raise_if_not_found=False`. The caller's own pending writes are flushed before the guard and stay the caller's responsibility. Odoo 19 only calls `next()` from the immediate install/upgrade buttons; the registry-load exposure of the same shape lives in `spp_hide_menus_base` and is tracked in #526. + ### 19.0.2.0.2 - test: add a regression test guarding the PDF backend selected by `odoo.tools.pdf`. The Docker image accidentally shipped legacy PyPDF2 3.x next to pypdf; Odoo prefers PyPDF2 when importable, and its removed 1.x API (`numPages`/`getPage`) crashes multi-record PDF printing with a `DeprecationError` (OP#1168). The fix is in `docker/Dockerfile` (`--no-deps` on the Odoo editable install); this test fails on any image that regresses. diff --git a/spp_base_common/static/description/index.html b/spp_base_common/static/description/index.html index eabec4c9a..23970a1ec 100644 --- a/spp_base_common/static/description/index.html +++ b/spp_base_common/static/description/index.html @@ -495,6 +495,22 @@

Changelog

+

19.0.2.0.3

+ +
+

19.0.2.0.2

-
+

19.0.2.0.1

-
+

19.0.2.0.0

  • Initial migration to OpenSPP2
    • diff --git a/spp_base_common/tests/test_ir_module_module.py b/spp_base_common/tests/test_ir_module_module.py index 7d473ed1d..28e660309 100644 --- a/spp_base_common/tests/test_ir_module_module.py +++ b/spp_base_common/tests/test_ir_module_module.py @@ -1,4 +1,29 @@ +import contextlib +import functools +from unittest.mock import patch + +import psycopg2 +import psycopg2.extensions + from odoo.tests import TransactionCase +from odoo.tools import mute_logger + +HOOK_LOGGER = "odoo.addons.spp_base_common.models.ir_module_module" +DISCUSS_ICON = "spp_base_common,static/description/icon-Discuss-White-line.png" + + +def _failing_lookup_for(menu_xml_id, original_lookup): + """Build an ``ir.model.data._xmlid_lookup`` stand-in that aborts the + transaction for one menu and behaves normally for every other xmlid, so the + other ``next()`` overrides in the MRO keep working.""" + + @functools.wraps(original_lookup) + def _xmlid_lookup(model, xmlid): + if xmlid == menu_xml_id: + model.env.cr.execute("SELECT 1 FROM spp_table_that_does_not_exist") + return original_lookup(model, xmlid) + + return _xmlid_lookup class TestIRModuleModule(TransactionCase): @@ -14,7 +39,102 @@ def test_01_update_menu_icons(self): # Verify that the icon was updated self.survey_module.next() menu = self.env.ref("mail.menu_root_discuss") - self.assertEqual( - menu.web_icon, - "spp_base_common,static/description/icon-Discuss-White-line.png", + self.assertEqual(menu.web_icon, DISCUSS_ICON) + + def test_02_missing_menu_xmlid_is_skipped(self): + """An ICON_MAP entry whose menu does not exist neither raises nor blocks the others.""" + discuss_menu = self.env.ref("mail.menu_root_discuss") + discuss_menu.write({"web_icon": False}) + broken_entry = { + "base": { + "menu_xml_id": "spp_base_common.menu_that_does_not_exist", + "icon": "spp_base_common,static/description/icon-fast-api.png", + } + } + + with patch.dict(self.IrModule.ICON_MAP, broken_entry): + self.IrModule.update_menu_icons() + + self.assertEqual(discuss_menu.web_icon, DISCUSS_ICON) + + def test_03_database_error_inside_hook_is_logged_and_skipped(self): + """A SQL failure while decorating is logged, and the module operation still completes.""" + # Fail on a menu only this hook looks up, so the sibling next() override in + # spp_hide_menus_base (which shares every real menu xmlid) is not affected. + test_menu_xml_id = "spp_base_common.test_383_menu" + icon_entry = {"base": {"menu_xml_id": test_menu_xml_id, "icon": DISCUSS_ICON}} + IrModelData = type(self.env["ir.model.data"]) + failing_lookup = _failing_lookup_for(test_menu_xml_id, IrModelData._xmlid_lookup) + discuss_menu = self.env.ref("mail.menu_root_discuss") + discuss_menu.write({"web_icon": False}) + + with ( + patch.dict(self.IrModule.ICON_MAP, icon_entry), + patch.object(IrModelData, "_xmlid_lookup", failing_lookup), + mute_logger("odoo.sql_db"), + self.assertLogs(HOOK_LOGGER, level="WARNING") as captured, + ): + action = self.survey_module.next() + + # next() returned its action instead of raising (its type depends on open todos). + self.assertIsInstance(action, dict) + self.assertIn("type", action) + self.assertTrue( + any("menu icon" in message for message in captured.output), + f"expected a skipped-decoration warning, got {captured.output}", ) + # The transaction is still usable: the failure was contained in a savepoint. + self.env.cr.execute("SELECT 1") + self.assertEqual(self.env.cr.fetchone(), (1,)) + # ...and the pass is atomic: the Discuss icon written before the failure was rolled back. + self.assertFalse(discuss_menu.web_icon) + + def test_04_hook_survives_already_aborted_transaction(self): + """The hook must not raise when the cursor is already poisoned before it starts.""" + # Odoo's assertRaises runs its body in a savepoint and rolls it back, which + # would un-poison the cursor; a bare try/except keeps the transaction aborted. + self.env.cr.execute("SAVEPOINT test_383_poisoned_cursor") + try: + with mute_logger("odoo.sql_db"), contextlib.suppress(psycopg2.Error): + self.env.cr.execute("SELECT 1 FROM spp_table_that_does_not_exist") + self.assertEqual( + self.env.cr.connection.get_transaction_status(), + psycopg2.extensions.TRANSACTION_STATUS_INERROR, + "precondition: the transaction must be aborted before the hook runs", + ) + + with mute_logger("odoo.sql_db"), self.assertLogs(HOOK_LOGGER, level="WARNING"): + self.IrModule.update_menu_icons() + finally: + self.env.cr.execute("ROLLBACK TO SAVEPOINT test_383_poisoned_cursor") + self.env.cr.execute("RELEASE SAVEPOINT test_383_poisoned_cursor") + + def test_05_callers_pending_writes_are_not_swallowed(self): + """A failure flushing the caller's own pending writes is the caller's error. + + The guard covers only the decoration pass. Pending ORM writes queued by the + caller are flushed before the guard, so their failure surfaces where it + belongs instead of being logged as a menu icon problem. + """ + discuss_menu = self.env.ref("mail.menu_root_discuss") + self.env.cr.execute("SAVEPOINT test_383_pending_write") + try: + discuss_menu.write({"web_icon": False}) # queued, not flushed yet + with mute_logger("odoo.sql_db"), contextlib.suppress(psycopg2.Error): + self.env.cr.execute("SELECT 1 FROM spp_table_that_does_not_exist") + self.assertEqual( + self.env.cr.connection.get_transaction_status(), + psycopg2.extensions.TRANSACTION_STATUS_INERROR, + "precondition: the transaction must be aborted before the hook runs", + ) + + raised = None + with mute_logger("odoo.sql_db"), self.assertNoLogs(HOOK_LOGGER, level="WARNING"): + try: + self.IrModule.update_menu_icons() + except psycopg2.Error as exc: + raised = exc + self.assertIsInstance(raised, psycopg2.errors.InFailedSqlTransaction) + finally: + self.env.cr.execute("ROLLBACK TO SAVEPOINT test_383_pending_write") + self.env.cr.execute("RELEASE SAVEPOINT test_383_pending_write")