[18.0][FIX] base_exception: Rollback transaction if we detect exceptions

base_exception/__manifest__.py

@@ -24,4 +24,9 @@
         "views/base_exception_view.xml",
     ],
     "installable": True,
+    "assets": {
+        "web.assets_backend": [
+            "base_exception/static/src/js/base_exception.esm.js",
+        ],
+    },
 }

base_exception/exceptions.py

@@ -0,0 +1,8 @@
+# Copyright 2025 Camptocamp SA
+# License AGPL-3.0 or later (https://www.gnu.org/licenses/agpl)
+
+from odoo.exceptions import UserError
+
+
+class BaseExceptionError(UserError):
+    pass

base_exception/models/base_exception.py

@@ -66,17 +66,22 @@ def _compute_exceptions_summary(self):
             else:
                 rec.exceptions_summary = False
 
+    def _must_popup_exception(self):
+        """Hook to redefine if the exception pop up must be shown"""
+        return False
+
+    def action_popup_exceptions(self):
+        if self._must_popup_exception():
+            return self._popup_exceptions()
+        return {"type": "ir.actions.client", "tag": "soft_reload"}
+
     def _popup_exceptions(self):
         """This method is used to show the popup action view.
         Used in several dependent modules."""
-        record = self._get_popup_action()
-        action = record.sudo().read()[0]
-        action = {
-            field: value
-            for field, value in action.items()
-            if field in record._get_readable_fields()
-        }
-        action.update(
+        # TODO: When migrating, use _for_xml_id instead of this
+        action = self._get_popup_action()
+        action_dict = action.sudo()._get_action_dict()
+        action_dict.update(
             {
                 "context": {
                     "active_id": self.ids[0],
@@ -85,7 +90,7 @@ def _popup_exceptions(self):
                 }
             }
         )
-        return action
+        return action_dict
 
     @api.model
     def _get_popup_action(self):

base_exception/models/base_exception_method.py

@@ -8,10 +8,13 @@
 from collections import defaultdict
 
 from odoo import _, api, models
+from odoo.api import Environment
 from odoo.exceptions import UserError
 from odoo.osv import expression
 from odoo.tools.safe_eval import safe_eval
 
+from ..exceptions import BaseExceptionError
+
 _logger = logging.getLogger(__name__)
 
 
@@ -84,12 +87,30 @@ def detect_exceptions(self):
         # the "to remove" part generates one DELETE per rule on the relation
         # table
         # and the "to add" part generates one INSERT (with unnest) per rule.
-        for rule_id, records in rules_to_remove.items():
-            records.write({"exception_ids": [(3, rule_id)]})
-        for rule_id, records in rules_to_add.items():
-            records.write({"exception_ids": [(4, rule_id)]})
+        raise_exception = False
+        # Write exceptions in a new transaction to be committed so that we can
+        #  rollback the ongoing one while keeping the exceptions stored
+        with self.env.registry.cursor() as new_cr:
+            new_env = Environment(new_cr, self.env.uid, self.env.context)
+            for rule_id, records in rules_to_remove.items():
+                records.with_env(new_env).write({"exception_ids": [(3, rule_id)]})
+            for rule_id, records in rules_to_add.items():
+                records.with_env(new_env).write({"exception_ids": [(4, rule_id)]})
+            # In case we have new exception, or exceptions that were not ignored yet, or
+            #  blocking exceptions, we need to raise an exception to rollback the
+            #  ongoing transaction
+            self_new_env = self.with_env(new_env)
+            if rules_to_add or self_new_env._must_raise_exception_after_detection():
+                raise_exception = True
+        if raise_exception:
+            raise BaseExceptionError(f"Exceptions detected on model: {self._name}")
         return all_exception_ids
 
+    def _must_raise_exception_after_detection(self):
+        return not all(
+            rec.ignore_exception for rec in self if rec.exception_ids
+        ) or any(rule.is_blocking for rule in self.mapped("exception_ids"))
+
     @api.model
     def _exception_rule_eval_context(self, rec):
         return {

base_exception/readme/CONTRIBUTORS.md

@@ -12,3 +12,4 @@
 - Kevin Khao \<<kevin.khao@akretion.com>\>
 - Laurent Mignon \<<laurent.mignon@acsone.eu>\>
 - Do Anh Duy \<<duyda@trobz.com>\>
+- Akim Juillerat \<<akim.juillerat@camptocamp.com>\>

base_exception/static/src/js/base_exception.esm.js

@@ -0,0 +1,40 @@
+import {patch} from "@web/core/utils/patch";
+import {registry} from "@web/core/registry";
+import {rpc} from "@web/core/network/rpc";
+
+/* eslint-disable no-unused-vars */
+async function popUpException(env, _action) {
+    /* eslint-enable no-unused-vars */
+    const controller = env.services.action.currentController;
+    const orm = env.services.orm;
+    const resId = controller.currentState?.resId;
+    const resModel = controller.props.resModel;
+    if (!resModel || !resId) return;
+    const popupAction = await orm.call(resModel, "action_popup_exceptions", [[resId]]);
+    if (!popupAction) return;
+    // Do a soft reload before displaying the popup to display the exception
+    // on the Form view
+    await env.services.action.restore(controller.jsId);
+    await env.services.action.doAction(popupAction);
+}
+
+patch(rpc, {
+    async _rpc(url, params = {}, settings = {}) {
+        try {
+            return await super._rpc(url, params, settings);
+        } catch (error) {
+            if (
+                error.exceptionName ===
+                "odoo.addons.base_exception.exceptions.BaseExceptionError"
+            ) {
+                if (error.data.message.split(":")[1].trim() === params.model) {
+                    return {type: "ir.actions.client", tag: "popup_exception"};
+                }
+                return {type: "ir.actions.client", tag: "soft_reload"};
+            }
+            throw error;
+        }
+    },
+});
+
+registry.category("actions").add("popup_exception", popUpException);

base_exception/tests/purchase_test.py

@@ -66,6 +66,11 @@ def button_confirm(self):
         self.write({"state": "purchase"})
         return True
 
+    def button_detect_and_confirm(self):
+        if self.detect_exceptions():
+            return
+        return self.button_confirm()
+
     def button_cancel(self):
         self.write({"state": "cancel"})
 

base_exception/tests/test_base_exception.py

@@ -2,11 +2,23 @@
 # Copyright 2020 Hibou Corp.
 # License AGPL-3.0 or later (https://www.gnu.org/licenses/agpl.html).
 
+try:
+    from decorator import decoratorx as decorator
+except ImportError:
+    from decorator import decorator
+
+from contextlib import contextmanager
+from unittest.mock import patch
+
 from odoo_test_helper import FakeModelLoader
 
+from odoo import SUPERUSER_ID
+from odoo.api import Environment
 from odoo.exceptions import UserError, ValidationError
 from odoo.tests import TransactionCase
 
+from ..exceptions import BaseExceptionError
+
 
 class TestBaseException(TransactionCase):
     def setUp(self):
@@ -47,10 +59,36 @@ def setUp(self):
             }
         )
 
+    @decorator
+    def swallow_base_exception_error(func, self):
+        def wrapper(*args, **kwargs):
+            try:
+                return func(*args, **kwargs)
+            except BaseExceptionError:
+                return None
+
+        return wrapper
+
+    @contextmanager
+    def mock_base_exception_method_env(self, env=None):
+        if env is None:
+            env = self.env
+        with patch(
+            "odoo.addons.base_exception.models.base_exception_method.Environment"
+        ) as mocked_env:
+            mocked_env.return_value = env
+            yield
+
+    @decorator
+    def patch_base_exception_method_env(func, self):
+        with self.mock_base_exception_method_env():
+            return func(self)
+
     def tearDown(self):
         self.loader.restore_registry()
         return super().tearDown()
 
+    @patch_base_exception_method_env
     def test_valid(self):
         self.partner.write({"zip": "00000"})
         self.exception_rule.active = False
@@ -61,12 +99,16 @@ def test_exception_rule_confirm(self):
         self.exception_rule_confirm.action_confirm()
         self.assertFalse(self.exception_rule_confirm.exception_ids)
 
+    @patch_base_exception_method_env
+    @swallow_base_exception_error
     def test_fail_by_py(self):
         with self.assertRaises(ValidationError):
             self.po.button_confirm()
         self.po.with_context(raise_exception=False).button_confirm()
         self.assertTrue(self.po.exception_ids)
 
+    @patch_base_exception_method_env
+    @swallow_base_exception_error
     def test_fail_by_domain(self):
         self.exception_rule.write(
             {
@@ -79,6 +121,8 @@ def test_fail_by_domain(self):
         self.po.with_context(raise_exception=False).button_confirm()
         self.assertTrue(self.po.exception_ids)
 
+    @patch_base_exception_method_env
+    @swallow_base_exception_error
     def test_fail_by_method(self):
         self.exception_rule.write(
             {
@@ -91,6 +135,8 @@ def test_fail_by_method(self):
         self.po.with_context(raise_exception=False).button_confirm()
         self.assertTrue(self.po.exception_ids)
 
+    @patch_base_exception_method_env
+    @swallow_base_exception_error
     def test_ignorable_exception(self):
         # Block because of exception during validation
         with self.assertRaises(ValidationError):
@@ -116,6 +162,7 @@ def test_purchase_check_button_draft(self):
         self.po.button_draft()
         self.assertEqual(self.po.state, "draft")
 
+    @patch_base_exception_method_env
     def test_purchase_check_button_confirm(self):
         self.partner.write({"zip": "00000"})
         self.po.button_confirm()
@@ -125,9 +172,13 @@ def test_purchase_check_button_cancel(self):
         self.po.button_cancel()
         self.assertEqual(self.po.state, "cancel")
 
+    @patch_base_exception_method_env
+    @swallow_base_exception_error
     def test_detect_exceptions(self):
         self.po.detect_exceptions()
 
+    @patch_base_exception_method_env
+    @swallow_base_exception_error
     def test_blocking_exception(self):
         self.exception_rule.is_blocking = True
         # Block because of exception during validation
@@ -146,3 +197,42 @@ def test_blocking_exception(self):
         self.po.with_context(raise_exception=False).button_confirm()
         self.assertTrue(self.po.exception_ids)
         self.assertTrue(self.po.exceptions_summary)
+
+    def test_rollback_main_transaction(self):
+        # Get new TestCursor
+        self.registry.enter_test_mode(self.cr)
+        self.addCleanup(self.registry.leave_test_mode)
+        with (
+            self.registry.cursor() as new_cr,
+            patch(
+                "odoo.addons.base_exception.models.base_exception.BaseExceptionModel._check_exception"
+            ) as mocked_check_exception,
+        ):
+            mocked_check_exception.return_value = None
+            new_env = Environment(new_cr, SUPERUSER_ID, {"module": "base_exception"})
+            with (
+                # Use new_env created here instead of the one in base_exception_method
+                self.mock_base_exception_method_env(env=new_env),
+                self.assertRaises(BaseExceptionError),
+            ):
+                self.po.button_detect_and_confirm()
+            # Entering assertRaises will create a first savepoint using self.env.cr.
+            # Then, when write is triggered through new_cr in
+            # base.exception.method.detect_exceptions, a second savepoint will be
+            # created using new_cr, and an odoo.sql_db.Savepoint object will be stored
+            # on new_cr._savepoint for this second savepoint.
+            # As the with block of assertRaises is exited a rollback to the first
+            # savepoint will be triggered.
+            # However, the Savepoint object for the second savepoint will not be
+            # removed from new_cr._savepoint, but as both self.env.cr and new_cr
+            # use the same psycopg2 cursor object behind the scene, the second
+            # savepoint does not exist anymore in the database.
+            # This situation would actually trigger a "savepoint does not exist"
+            # psycopg2 exception when trying to release or rollback the savepoint
+            # when closing the cursor. Therefore, we can safely remove the reference
+            # to that object to avoid this error when exiting the test.
+            new_cr._savepoint = None
+            self.assertFalse(self.po.exception_ids)
+            self.assertTrue(self.po.with_env(new_env).exception_ids)
+            self.assertNotEqual(self.po.state, "purchase")
+            self.assertNotEqual(self.po.with_env(new_env).state, "purchase")