diff --git a/base_exception/README.rst b/base_exception/README.rst index 673390c682a..22cad868be4 100644 --- a/base_exception/README.rst +++ b/base_exception/README.rst @@ -91,6 +91,7 @@ Contributors - Kevin Khao - Laurent Mignon - Do Anh Duy +- Akim Juillerat Other credits ------------- diff --git a/base_exception/__manifest__.py b/base_exception/__manifest__.py index bfb1682204e..26b47796533 100644 --- a/base_exception/__manifest__.py +++ b/base_exception/__manifest__.py @@ -17,6 +17,9 @@ "depends": ["base_setup"], "maintainers": ["hparfr", "sebastienbeau"], "license": "AGPL-3", + "external_dependencies": { + "python": ["decorator"], + }, "data": [ "security/base_exception_security.xml", "security/ir.model.access.csv", @@ -24,4 +27,9 @@ "views/base_exception_view.xml", ], "installable": True, + "assets": { + "web.assets_backend": [ + "base_exception/static/src/js/base_exception.esm.js", + ], + }, } diff --git a/base_exception/exceptions.py b/base_exception/exceptions.py new file mode 100644 index 00000000000..8124b9830dd --- /dev/null +++ b/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 diff --git a/base_exception/models/base_exception.py b/base_exception/models/base_exception.py index 0c485401688..1504214a2a7 100644 --- a/base_exception/models/base_exception.py +++ b/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): diff --git a/base_exception/models/base_exception_method.py b/base_exception/models/base_exception_method.py index d5926468658..040eb9fced2 100644 --- a/base_exception/models/base_exception_method.py +++ b/base_exception/models/base_exception_method.py @@ -4,14 +4,19 @@ # Copyright 2020 Hibou Corp. # Copyright 2023 ACSONE SA/NV (http://acsone.eu) # License AGPL-3.0 or later (https://www.gnu.org/licenses/agpl.html). +import json import logging from collections import defaultdict -from odoo import api, models +from odoo import Command, api, models +from odoo.api import Environment from odoo.exceptions import UserError from odoo.fields import Domain +from odoo.tools import config from odoo.tools.safe_eval import safe_eval +from ..exceptions import BaseExceptionError + _logger = logging.getLogger(__name__) @@ -84,12 +89,77 @@ 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 + # The registry is not ready yet when modules are being installed or + # updated: the ongoing transaction holds exclusive locks on the tables + # it has just modified (ALTER TABLE ...), so any query made through a + # second connection would wait forever on those locks (and the locks + # cannot be released, as the current thread is the one waiting). + # Use the current cursor in that case, as already done when running + # tests. + test_mode = ( + config["test_enable"] or not self.env.registry.ready + ) and not self.env.context.get("test_base_exception") + # Resolve the main records (e.g. the sale order behind a sale order + # line) using the current cursor, which sees records created earlier + # in this same transaction even before they are committed. + main_records = self._get_main_records() + # 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) + if not test_mode + else self.env + ) + for rule_id, records in rules_to_remove.items(): + records.with_env(new_env).write( + {"exception_ids": [Command.unlink(rule_id)]} + ) + for rule_id, records in rules_to_add.items(): + records.with_env(new_env).write( + {"exception_ids": [Command.link(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. + # Re-derive main_records through new_env rather than re-running + # self.with_env(new_env)._get_main_records(): when self are + # records just created earlier in the ongoing (not yet committed) + # transaction (e.g. a line added while editing a confirmed sale + # order), new_cr is a genuinely separate DB connection that + # cannot see them yet, and _get_main_records() traversal + # (e.g. sale.order.line -> order_id) would raise MissingError. + # main_records itself was already resolved above through the + # current cursor, so only rebinding it to new_env is needed here. + main_records_new_env = main_records.with_env(new_env) + if ( + rules_to_add + or main_records_new_env._must_raise_exception_after_detection() + ): + raise_exception = True + if raise_exception: + raise BaseExceptionError( + json.dumps(self._detect_exception_get_exc_class_values()) + ) return all_exception_ids + def _detect_exception_get_exc_class_values(self): + return { + "src_model": self._name, + "target_model": self._name, + } + + def _must_raise_exception_after_detection(self): + main_records = self._get_main_records() + all_ignore_exception = all( + main_records.filtered("exception_ids").mapped("ignore_exception") + ) + any_blocking_exception = any( + rule.is_blocking for rule in main_records.exception_ids + ) + return not all_ignore_exception or any_blocking_exception + @api.model def _exception_rule_eval_context(self, rec): return { diff --git a/base_exception/readme/CONTRIBUTORS.md b/base_exception/readme/CONTRIBUTORS.md index 2df68f2fd76..53a0d8cdecf 100644 --- a/base_exception/readme/CONTRIBUTORS.md +++ b/base_exception/readme/CONTRIBUTORS.md @@ -12,3 +12,4 @@ - Kevin Khao \<\> - Laurent Mignon \<\> - Do Anh Duy \<\> +- Akim Juillerat \<\> diff --git a/base_exception/static/description/index.html b/base_exception/static/description/index.html index 624e3395557..80b3b4e2eb8 100644 --- a/base_exception/static/description/index.html +++ b/base_exception/static/description/index.html @@ -438,6 +438,7 @@

Contributors

  • Kevin Khao <kevin.khao@akretion.com>
  • Laurent Mignon <laurent.mignon@acsone.eu>
  • Do Anh Duy <duyda@trobz.com>
    • +
  • Akim Juillerat <akim.juillerat@camptocamp.com>
    • diff --git a/base_exception/static/src/js/base_exception.esm.js b/base_exception/static/src/js/base_exception.esm.js new file mode 100644 index 00000000000..3ae4c083176 --- /dev/null +++ b/base_exception/static/src/js/base_exception.esm.js @@ -0,0 +1,44 @@ +import {registry} from "@web/core/registry"; + +/* 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); +} + +function baseExceptionErrorHandler(env, uncaughtError, originalError) { + const controller = env.services.action.currentController; + if ( + originalError.exceptionName === + "odoo.addons.base_exception.exceptions.BaseExceptionError" + ) { + const excData = JSON.parse(originalError.data.message); + if (excData.target_model === controller.props.resModel) { + env.services.action.doAction({ + type: "ir.actions.client", + tag: "popup_exception", + }); + } else { + env.services.action.doAction({ + type: "ir.actions.client", + tag: "soft_reload", + }); + } + return true; + } +} + +registry.category("actions").add("popup_exception", popUpException); +registry + .category("error_handlers") + .add("base_exception_error", baseExceptionErrorHandler, {sequence: 0}); diff --git a/base_exception/tests/common.py b/base_exception/tests/common.py new file mode 100644 index 00000000000..47a928a09de --- /dev/null +++ b/base_exception/tests/common.py @@ -0,0 +1,40 @@ +# Copyright 2026 Camptocamp SA +# License AGPL-3.0 or later (https://www.gnu.org/licenses/agpl) + +try: + from decorator import decoratorx as decorator +except ImportError: + from decorator import decorator + +from contextlib import contextmanager +from unittest.mock import patch + +from ..exceptions import BaseExceptionError + + +@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 mock_base_exception_method_env(self): + return func(self) diff --git a/base_exception/tests/purchase_test.py b/base_exception/tests/purchase_test.py index ae1ce7e66d6..72809c780f0 100644 --- a/base_exception/tests/purchase_test.py +++ b/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"}) diff --git a/base_exception/tests/test_base_exception.py b/base_exception/tests/test_base_exception.py index dac21d1e6a7..829f64ef6ab 100644 --- a/base_exception/tests/test_base_exception.py +++ b/base_exception/tests/test_base_exception.py @@ -2,15 +2,27 @@ # Copyright 2020 Hibou Corp. # License AGPL-3.0 or later (https://www.gnu.org/licenses/agpl.html). +from unittest.mock import patch + +from odoo import SUPERUSER_ID +from odoo.api import Environment from odoo.exceptions import UserError, ValidationError from odoo.orm.model_classes import add_to_registry from odoo.tests import TransactionCase +from ..exceptions import BaseExceptionError +from .common import ( + mock_base_exception_method_env, + patch_base_exception_method_env, + swallow_base_exception_error, +) + class TestBaseException(TransactionCase): @classmethod def setUpClass(cls): super().setUpClass() + cls.env = cls.env(context=dict(cls.env.context, test_base_exception=True)) cls.originExceptionRuleClasses = cls.registry["exception.rule"]._base_classes__ from . import purchase_test @@ -69,6 +81,7 @@ def setUpClass(cls): def restore_exception_rule(cls): cls.registry["exception.rule"]._base_classes__ = cls.originExceptionRuleClasses + @patch_base_exception_method_env def test_valid(self): self.partner.write({"zip": "00000"}) self.exception_rule.active = False @@ -79,12 +92,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( { @@ -97,6 +114,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( { @@ -109,6 +128,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): @@ -134,6 +155,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() @@ -143,9 +165,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 @@ -164,3 +190,45 @@ 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() + 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 + mock_base_exception_method_env(self, env=new_env), + self.assertRaises(BaseExceptionError), + ): + self.po.button_detect_and_confirm() + # 1. Entering assertRaises will create a first savepoint using self.env.cr. + # 2. 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. + # 3. As the with block of assertRaises is exited a rollback to the first + # savepoint will be triggered, what invalidates the second savepoint. + # + # 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 + # Ensure write from base.exception.method.detect_exceptions was called + # with the new env that must be committed as the main env is the one to + # be rollbacked. + 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") diff --git a/requirements.txt b/requirements.txt index f63d53a4c42..016321d27ad 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,6 +1,7 @@ # generated from manifests external_dependencies cryptography dataclasses +decorator numpy odoorpc openupgradelib