test: cover CEL ace editor mode and hide-menu upgrade re-hide

Add a guardrail asserting the CEL Expressions view's ace fields use a
CodeEditor-valid mode (regression test for the 'text' mode crash that
broke the tab), plus unit and integration coverage for
spp_hide_menus_base._reapply_hide() and the hide_menus() re-hide branch
that runs after a module upgrade resets a menu's group_ids.

Author: kneckinator

spp_approval/tests/__init__.py

@@ -4,3 +4,4 @@
 from . import test_approval_review
 from . import test_approval_security
 from . import test_cel_evaluator_security
+from . import test_cel_view

spp_approval/tests/test_cel_view.py

@@ -0,0 +1,56 @@
+# Part of OpenSPP. See LICENSE file for full copyright and licensing details.
+"""Guardrail for the CEL Expressions view's ace editor configuration.
+
+Odoo 19's ``CodeEditor`` OWL component validates its ``mode`` prop against
+a fixed allow-list:
+
+    addons/web/static/src/core/code_editor/code_editor.js
+        static MODES = ["javascript", "xml", "qweb", "scss", "python"];
+
+A field declaring ``widget="ace" options="{'mode': '<invalid>'}"`` crashes
+the client the moment the editor mounts (the field is only rendered when
+its ``use_cel_*`` toggle is enabled). The original view used
+``'mode': 'text'`` — not in MODES — which is exactly that crash. This test
+parses the view arch and fails if any ace field declares a mode the
+component would reject, so the regression can't silently come back.
+"""
+
+import ast
+
+from lxml import etree
+
+from odoo.tests import TransactionCase, tagged
+
+# Mirror of CodeEditor.MODES (see module docstring). Kept here as a literal
+# because the allow-list lives in JS and isn't importable from Python.
+VALID_ACE_MODES = {"javascript", "xml", "qweb", "scss", "python"}
+
+
+@tagged("post_install", "-at_install")
+class TestCelViewAceMode(TransactionCase):
+    """The CEL Expressions form must only use ace modes CodeEditor accepts."""
+
+    def test_cel_ace_fields_use_valid_mode(self):
+        view = self.env.ref("spp_approval.view_spp_approval_definition_form_cel")
+        arch = etree.fromstring(view.arch)
+
+        ace_fields = arch.xpath("//field[@widget='ace']")
+        self.assertTrue(
+            ace_fields,
+            "Expected at least one widget='ace' field in the CEL view — did the view change?",
+        )
+
+        for field in ace_fields:
+            options = field.get("options")
+            self.assertTrue(
+                options,
+                f"ace field {field.get('name')!r} must declare options with a mode",
+            )
+            mode = ast.literal_eval(options).get("mode")
+            self.assertIn(
+                mode,
+                VALID_ACE_MODES,
+                f"ace field {field.get('name')!r} uses mode {mode!r}, which "
+                f"CodeEditor rejects (valid: {sorted(VALID_ACE_MODES)}). "
+                "This crashes the client when the field is rendered.",
+            )

spp_hide_menus_base/tests/test_hide_menu.py

@@ -9,6 +9,7 @@
 flow.
 """
 
+from odoo import Command
 from odoo.tests import TransactionCase, tagged
 
 
@@ -149,6 +150,101 @@ def test_hide_menus_is_idempotent(self):
         for record in after_second:
             self.assertEqual(record.state, "hide")
 
+    def test_reapply_hide_after_simulated_upgrade_reset(self):
+        """_reapply_hide() re-applies the hide group after a module upgrade
+        reset the menu's group_ids via XML (noupdate="0").
+
+        A real module upgrade can't run inside a test transaction, so we
+        simulate its effect: hide the menu, then overwrite group_ids the way
+        an XML data reload would, dropping the hide group. _reapply_hide()
+        must detect the stale state and restore the hidden configuration.
+        """
+        hide_group = self.env.ref("spp_hide_menus_base.group_hide_menus_user")
+        record = self.env["spp.hide.menu"].create({"menu_id": self.menu.id, "xml_id": "test.hide_menu_target"})
+        record.hide_menu()
+        self.assertIn(hide_group, self.menu.group_ids, "precondition: menu should be hidden")
+
+        # Simulate the upgrade resetting group_ids to the module's XML default
+        # (some real group, without the hide group).
+        reset_groups = self.env.ref("base.group_user")
+        self.menu.write({"group_ids": [Command.set([reset_groups.id])]})
+        self.assertNotIn(hide_group, self.menu.group_ids, "precondition: reset must drop the hide group")
+
+        record._reapply_hide()
+
+        self.assertIn(
+            hide_group,
+            self.menu.group_ids,
+            "_reapply_hide() should restore the hide group after an upgrade reset",
+        )
+        # The reset groups become the new restore snapshot so show_menu()
+        # returns the menu to its real post-upgrade default.
+        self.assertEqual(record.default_group_ids, reset_groups)
+
+    def test_reapply_hide_noop_when_already_hidden(self):
+        """_reapply_hide() is a no-op when the hide group is still present.
+
+        If no upgrade reset happened, the guard (hide_group not in
+        group_ids) is False, so neither group_ids nor the saved snapshot
+        should change.
+        """
+        hide_group = self.env.ref("spp_hide_menus_base.group_hide_menus_user")
+        record = self.env["spp.hide.menu"].create({"menu_id": self.menu.id, "xml_id": "test.hide_menu_target"})
+        record.hide_menu()
+        groups_before = self.menu.group_ids
+        snapshot_before = record.default_group_ids
+        self.assertIn(hide_group, groups_before, "precondition: menu should be hidden")
+
+        record._reapply_hide()
+
+        self.assertEqual(self.menu.group_ids, groups_before)
+        self.assertEqual(record.default_group_ids, snapshot_before)
+
+    def test_hide_menus_reapplies_after_reset(self):
+        """``hide_menus()`` re-hides an already-hidden menu whose group_ids
+        were reset, exercising the ``elif ... state == "hide"`` branch.
+
+        This is the end-to-end shape of the upgrade bug: a menu is hidden,
+        a later module upgrade resets its group_ids, and the next
+        install/upgrade pass through hide_menus() must put the hide group
+        back.
+        """
+        IrModuleModule = self.env["ir.module.module"]
+        HideMenu = self.env["spp.hide.menu"]
+        hide_group = self.env.ref("spp_hide_menus_base.group_hide_menus_user")
+
+        # Find one resolvable MENU_APP entry to drive the real entry point.
+        target = None
+        for module_name, info in IrModuleModule.MENU_APP.items():
+            menu = self.env.ref(info["menu_xml_id"], raise_if_not_found=False)
+            module = IrModuleModule.search([("name", "=", module_name)], limit=1)
+            if menu and module:
+                target = menu
+                break
+        if target is None:
+            self.skipTest("No MENU_APP entries are resolvable in this test DB")
+
+        # First pass creates the record and hides the menu.
+        HideMenu.search([]).unlink()
+        IrModuleModule.hide_menus()
+        record = HideMenu.search([("menu_id", "=", target.id)], limit=1)
+        self.assertTrue(record, "hide_menus() should have created a hide record")
+        self.assertEqual(record.state, "hide")
+        self.assertIn(hide_group, target.group_ids)
+
+        # Simulate an upgrade resetting the menu's group_ids via XML.
+        reset_groups = self.env.ref("base.group_user")
+        target.write({"group_ids": [Command.set([reset_groups.id])]})
+        self.assertNotIn(hide_group, target.group_ids)
+
+        # Second pass must re-hide via the state == "hide" branch.
+        IrModuleModule.hide_menus()
+        self.assertIn(
+            hide_group,
+            target.group_ids,
+            "hide_menus() should re-hide a menu whose group_ids were reset on upgrade",
+        )
+
     def test_hide_menus_skips_unknown_modules(self):
         """An ir.module.module record whose name isn't in MENU_APP must be
         ignored by hide_menus() — no spp.hide.menu record is created for it.