-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
[18.0][FIX] base_exception: Rollback transaction if we detect exceptions #3590
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
grindtildeath
wants to merge
8
commits into
OCA:18.0
Choose a base branch
from
camptocamp:18.0-fix-base_exception_rollback_transaction_fix_ui
base: 18.0
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 3 commits
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
61eed8a
[FIX] base_exception: Rollback transaction if we detect exceptions
grindtildeath 9a1c94c
Fix tests
grindtildeath 08266fe
[REF] base_exception: Handle the exception popup at client level
grindtildeath e1b81f8
[REF] base_exception: Use a client action to reload smoothly
grindtildeath 1a2e3c6
fixup! [REF] base_exception: Use a client action to reload smoothly
grindtildeath 9857533
fixup! fixup! [REF] base_exception: Use a client action to reload smo…
grindtildeath c008459
[TEST] base_exception: Test exception rollbacking
grindtildeath e9c8a9d
fixup! fixup! fixup! [REF] base_exception: Use a client action to rel…
grindtildeath File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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 | ||
|
Comment on lines
+99
to
+104
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe extract this to a method |
||
| if raise_exception: | ||
| raise BaseExceptionError("Exceptions detected") | ||
| 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 { | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,80 @@ | ||
| import {patch} from "@web/core/utils/patch"; | ||
| import {rpc} from "@web/core/network/rpc"; | ||
| import {FormController} from "@web/views/form/form_controller"; | ||
|
|
||
| // keep track of the current FormController by storing it | ||
| const activeForm = { | ||
| controller: null, | ||
| }; | ||
|
|
||
| patch(FormController.prototype, { | ||
| setup() { | ||
| super.setup(); | ||
| activeForm.controller = this; | ||
| }, | ||
| willUnmount() { | ||
| if (activeForm.controller === this) { | ||
| activeForm.controller = null; | ||
| } | ||
| super.willUnmount(); | ||
| }, | ||
| }); | ||
|
|
||
| function getActiveRecordInfo(controller) { | ||
| const model = controller.model; | ||
| const root = model?.root; | ||
| return { | ||
| root, | ||
| resModel: root?.resModel, | ||
| resId: root?.resId, | ||
| }; | ||
| } | ||
|
|
||
| async function popUpException() { | ||
| const controller = activeForm.controller; | ||
| const orm = controller.env.services.orm; | ||
|
|
||
| const {resModel, resId} = getActiveRecordInfo(controller); | ||
| if (!resModel || !resId) return false; | ||
| const actionService = controller.env.services.action; | ||
| const action = await orm.call(resModel, "action_popup_exceptions", [[resId]]); | ||
| if (!action) return false; | ||
|
|
||
| await actionService.doAction(action); | ||
|
|
||
| return true; | ||
| } | ||
|
|
||
| async function refreshExceptionIdsField() { | ||
| const controller = activeForm.controller; | ||
| if (!controller) return false; | ||
|
|
||
| const {root, resModel, resId} = getActiveRecordInfo(controller); | ||
| if (!resModel || !resId) return false; | ||
| const orm = controller.env.services.orm; | ||
|
|
||
| // Read the latest value for just that field | ||
| await orm.read(resModel, [resId], ["exception_ids"]); | ||
| // Reload the record; OWL will re-render the field | ||
| await root.load(); | ||
|
|
||
| return true; | ||
| } | ||
|
|
||
| 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" | ||
| ) { | ||
| await refreshExceptionIdsField(); | ||
| await popUpException(); | ||
| } else { | ||
| throw error; | ||
| } | ||
| } | ||
| }, | ||
| }); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
to avoid all the
with_enveverytime you read/write, initialize a records_env variable.also you can use self.env(cr=) like this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually we have many sets of records:
For each one we need to use the new cursor, so I guess it makes sense to create the new_env this way. AFAIU
self.env(cr=new_cr)would just do the same, but we need to reuse it many times anyway