-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
[14.0][IMP] base_exception: Better general Handling of Sub-Exceptions #3365
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
raumschmiede-joshuaL
wants to merge
2
commits into
OCA:14.0
Choose a base branch
from
raumschmiede-joshuaL:14.0-sub-exceptions
base: 14.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 all commits
Commits
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,6 @@ | ||
| # Copyright 2011 Raphaël Valyi, Renato Lima, Guewen Baconnier, Sodexis | ||
| # Copyright 2017 Akretion (http://www.akretion.com) | ||
| # Copyright 2025 Raumschmiede GmbH | ||
| # Mourad EL HADJ MIMOUNE <[email protected]> | ||
| # Copyright 2020 Hibou Corp. | ||
| # Copyright 2023 ACSONE SA/NV (http://acsone.eu) | ||
|
|
@@ -91,6 +92,22 @@ def _popup_exceptions(self): | |
| def _get_popup_action(self): | ||
| return self.env.ref("base_exception.action_exception_rule_confirm") | ||
|
|
||
| def _add_detected_exceptions_to_self(self): | ||
| return self != self._get_main_records() | ||
|
|
||
| def _detect_exceptions(self, rule): | ||
| records = super()._detect_exceptions(rule) | ||
| # If _get_main_records returns self, it adds the exceptions to itself in | ||
| # detect_exceptions. If _get_main_records does not return self, it adds the | ||
| # exceptions here | ||
| if not self._add_detected_exceptions_to_self(): | ||
| return records | ||
|
|
||
| (self - records).exception_ids = [(3, rule.id)] | ||
| records.exception_ids = [(4, rule.id)] | ||
|
|
||
| return records | ||
|
|
||
| def _check_exception(self): | ||
| """Check exceptions | ||
|
|
||
|
|
||
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 |
|---|---|---|
|
|
@@ -3,6 +3,7 @@ | |
| # Mourad EL HADJ MIMOUNE <[email protected]> | ||
| # Copyright 2020 Hibou Corp. | ||
| # Copyright 2023 ACSONE SA/NV (http://acsone.eu) | ||
| # Copyright 2025 Raumschmiede GmbH | ||
| # License AGPL-3.0 or later (https://www.gnu.org/licenses/agpl.html). | ||
|
|
||
| import logging | ||
|
|
@@ -28,6 +29,13 @@ def _get_main_records(self): | |
| """ | ||
| return self | ||
|
|
||
| def _get_sub_exception_field_names(self): | ||
| """ | ||
| Use this to check exceptions on underlying records and the | ||
| detected exceptions need to be added to the current record | ||
| """ | ||
| return [] | ||
|
|
||
| def _rule_domain(self): | ||
| """Filter exception.rules. | ||
| By default, only the rules with the correct model | ||
|
|
@@ -52,7 +60,9 @@ def detect_exceptions(self): | |
| records_with_rule_in_exceptions = main_records.filtered( | ||
| lambda r, rule_id=rule_info.id: rule_id in r.exception_ids.ids | ||
| ) | ||
| records_with_exception = self._detect_exceptions(rule_info) | ||
| records_with_exception = self._detect_exceptions( | ||
| rule_info | ||
| )._get_main_records() | ||
| to_remove = records_with_rule_in_exceptions - records_with_exception | ||
| to_add = records_with_exception - records_with_rule_in_exceptions | ||
| # we expect to always work on the same model type | ||
|
|
@@ -64,23 +74,18 @@ def detect_exceptions(self): | |
| rules_to_add[rule_info.id] |= to_add | ||
| if records_with_exception: | ||
| all_exception_ids.append(rule_info.id) | ||
| # Cumulate all the records to attach to the rule | ||
| # before linking. We don't want to call "rule.write()" | ||
| # which would: | ||
| # * write on write_date so lock the exception.rule | ||
| # * trigger the recomputation of "main_exception_id" on | ||
| # all the sale orders related to the rule, locking them all | ||
| # and preventing concurrent writes | ||
| # Reversing the write by writing on SaleOrder instead of | ||
| # ExceptionRule fixes the 2 kinds of unexpected locks. | ||
| # It should not result in more queries than writing on ExceptionRule: | ||
| # 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)]}) | ||
|
|
||
| # Detect exceptions on underlying sub-records if needed. If the sub-records' | ||
| # model defines the current model in _get_main_records, | ||
| # the exceptions detected are added to the current record | ||
| for field in self._get_sub_exception_field_names(): | ||
| all_exception_ids += self.mapped(field).detect_exceptions() | ||
|
|
||
| return all_exception_ids | ||
|
|
||
| @api.model | ||
|
|
||
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 |
|---|---|---|
|
|
@@ -14,3 +14,4 @@ | |
|
|
||
| * Kevin Khao <[email protected]> | ||
| * Laurent Mignon <[email protected]> | ||
| * Joshua Lauer <[email protected]> | ||
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
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
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.
Uh oh!
There was an error while loading. Please reload this page.
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.
Here a small minor request.
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.
Is it correct that since you are writing the rules now on the records, the logic in
detect_exceptionsinbase.exception.methodis not used anymore. Or only if the returned main record isn't the same as where it was executed from?Isn't this quite bad because the checks and writes in
detect_exceptionsare done twice?Isn't it possible to have the writing in just one place?
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.
detect_exceptionsinbase.exception.methodis still needed.If a model inherits from
base.exception.method, it has noexception_ids, that's why it must return another record in_get_main_records. The detected rules inbase.exception.methodare written to that other model. Like it is done right now inaccount_move_exception.If a model inherits from
base.exception, there are 2 ways:_get_main_records, like it is done in my PR forsale_exception. But as it returns another record, it will not write the detected rules on the sale order lines, only on the sales orders returned by_get_main_records. That's why this override here is needed_get_main_records: Then the detected rules are added to self inbase.exception.methodand this here has no effect as it is always FalseThere 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.
Ok. Since you are only adding it if the main record isn't the same. So if the exception is on sale.order and it is detected on sale.order it will add it in
detect_exceptions.