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

[grindtildeath]

This aims to solve issues when modules not depending on whatever implementation of base_exception, override the same function that triggers the detection of exceptions.

Before this commit, any changes done in such overrides could end up being committed to the database if the MRO did execute such function before the function implementing base_exception that avoids to call super in case an exception is detected. (eg sale.order. action_confirm in sale_exception)

With this commit, in case there is any newly detected exception, or a record with exception that is not ignored, or a blocking exception linked to a record, exception changes will be committed in DB while a specific Exception type will be raised to rollback any changes done in the ongoing transaction. Such an exception will be handled in the UI to refresh the exception_ids field so that the user knows why the action was not completed.

Replaces and fixes:

• [18.0][POC] base_exception: Rollback transaction triggering exception rule #3476
• [16]sale_exception: Subscription behavior when confirming an upsell sale-workflow#2667
• [16.0][FIX] sale_expection: don't compute loyalty program if there is… sale-workflow#3672
• [FIX] sale_exception: add a register_hook to prevent other action_con… sale-workflow#3767
• [18.0][FIX] sale_exception: Rollback confirmation upon exception sale-workflow#3885
• [18.0][FIX] sale_exception: handle exception rollback in action_confirm method sale-workflow#3952

Dependent PRs:

• [18.0][IMP] sale_exception: Use hook to popup exception after rollback sale-workflow#4255
• [18.0][IMP] sale_exception: Use hook to popup exception after rollback purchase-workflow#3012

[OCA-git-bot]

Hi @sebastienbeau, @hparfr,
some modules you are maintaining are being modified, check this out!

[ivantodorovich]

Suggested change

	with Registry(self.env.cr.dbname).cursor() as new_cr:
	with self.env.registry.cursor() as new_cr:

no need to initialize a new registry

[hparfr]

ping @florian-dacosta

[grindtildeath]

ping @jok-adhoc @lav-adhoc @lef-adhoc @maq-adhoc @rousseldenis @florian-dacosta

After spending hours on this issue, IMO this solves the core issue we have with non dependent module having their changes done in action_confirm committed after an exception was detected since the transaction was not fully rolled back.

I then changed the way the exception popup can be displayed and it seems to work smoothly with no side effects and no need of glue modules.

Could you guys please have a look, eventually test this (with their dependent PRs) and let me know what do you think? 🙏

[rousseldenis]

LGTM.

Great! @grindtildeath Thanks for this, this seems indeed cleaner to solve the historical issue we have with this.

[florian-dacosta]

Hello @grindtildeath Thanks for this PR.
Code review looks fine. There is a pre-commit error, I had this recently and did change the name of the file by .esm.js to solve it. (Not sure it is the right solution).

But I did a test and it is not working as expected. On a demo database with only sale_management and sale_exception installed, I did add the following line in the method action_confirm of sale_management (before the super) : self.write({"client_order_ref": "TEST"}).

sale-workflow.webm

As you can see in the video, after clicking on the confirm button of the sale order, I have the new exception and the client order ref field did not change, all good on this part.
But, I did not have any window with the exceptions + all button are disabled. (including the little arrow to go to partner form for instance, not only the header buttons).

Can you check this issue please ?

[grindtildeath]

@florian-dacosta Thanks for your feedback.

I reworked the JS part to make it cleaner and simpler, it seems to solve your issue and works seamlessly on my side. Would be good if you can test again.

[ivantodorovich]

CI fails due to pre-commit

[florian-dacosta]

@grindtildeath
It seems to work pretty well now, but there is still an issue.
I test with sale_exception, and if I run the cron to test the exceptions on draft order, manually (with button method_direct_trigger), I have this error
AttributeError: The method 'ir.cron.action_popup_exceptions' does not exist

This is because the button on ir.cron do trigger the BaseExceptionError.

[grindtildeath]

@florian-dacosta Tanks for the feedback, must be fixed now

[florian-dacosta]

Thanks for the fix @grindtildeath
It seems to work quite well now. The last thing that bother me a little, is that an exception is raised even when not needed.
Indeed, when you confirm a sale order, you want the exception to revert everything, but when it is the cron that "test exceptions on draft orders", you don"'t need to revert anything the raise is not needed.

It does not seem to be a big problem since the exceptions are commited anyway, but
It still gives a warning : 2026-04-14 07:44:57,926 31 WARNING db odoo.http: Exceptions detected on model: sale.order
while it is a "false alarm" since it is totally normal to set/remove exeption with the cron.

Also, I guess we may have situation with the inverse issue we try to fix here.
Imagine you override some test_all_draft_orders on sale order for instance, and you expect your code to change some stuff after the update of all exceptions, it won't happen because of the raise, while you would not expect a raise since the method detect_exceptions succeed in doing what it has been designed for...
overriding detect_exceptions directly could also lead to this.
We want to raise only because detect_exceptions is called during the action_confirm, but not necessarly for other cases...

I am not blocking anything as for now, the gain seems better than the potential loss, but I think it may be an issue.
What do you think ?

[simahawk]

LGTM. As for version bump I would go for major.

[simahawk]

Suggested change

	) or any(rule.is_blocking for rule in self.mapped("exception_ids"))
	) or any(rule.is_blocking for rule in self.exception_ids)

[simahawk]

If I get it right when looking at If you register the exception into any of these registries, you won't need this patch:

const errorHandlerRegistry = registry.category("error_handlers");
const errorDialogRegistry = registry.category("error_dialogs");
const errorNotificationRegistry = registry.category("error_notifications");

The second one is probably the right one.

Not a blocker but maybe we can add a TODO here 😉

[simahawk]

@florian-dacosta If I get your explanation right, it sounds like a side effect that is not fully in control.

To allow devs to still store some information before the exception is raised we could add an hook and pass to it the new env. This way you can control it in a very explicit way.

Not a blocker. maybe we can add a TODO in the roadmap.

[florian-dacosta]

@florian-dacosta If I get your explanation right, it sounds like a side effect that is not fully in control.

To allow devs to still store some information before the exception is raised we could add an hook and pass to it the new env. This way you can control it in a very explicit way.

Not a blocker. maybe we can add a TODO in the roadmap.

I was more thinking on a context to get rid of the raise.
So, by default (no context), the error is raise and the rollback is done (required when the detect exceptions is part of the action_confirm for instance.
And for a method like sale.order.test_all_draft_orders which only goal is to check exceptions, you know the raise is not necessary, you could call it with the special context.

Well anyway, not a blocker like I said, maybe it is anticipating an issue we won't ever have. I have no usecase in mind, just thought about the possible side effects.