[14.0][IMP] base_exception: Better general Handling of Sub-Exceptions #3365
Conversation
|
Hi @sebastienbeau, @hparfr, |
|
Will add the 2nd commit after the tests for the 1st commit are done to be sure I committed the right things |
This ensures that the model on which the exceptions are detected get the exceptions assigned by default. Previously it was only possible if this def was overridden manually
7a71af2 to
78e44aa
Compare
|
@hparfr, if a model inherits from That's why I want to move it from |
| if not self._add_detected_exceptions_to_self(): | ||
| return records | ||
|
|
||
| (self - records).exception_ids = [(3, rule.id)] | ||
| records.exception_ids = [(4, rule.id)] |
There was a problem hiding this comment.
Here a small minor request.
| if not self._add_detected_exceptions_to_self(): | |
| return records | |
| (self - records).exception_ids = [(3, rule.id)] | |
| records.exception_ids = [(4, rule.id)] | |
| if self._add_detected_exceptions_to_self(): | |
| (self - records).exception_ids = [(3, rule.id)] | |
| records.exception_ids = [(4, rule.id)] |
There was a problem hiding this comment.
Is it correct that since you are writing the rules now on the records, the logic in detect_exceptions in base.exception.method is 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_exceptions are done twice?
Isn't it possible to have the writing in just one place?
There was a problem hiding this comment.
detect_exceptions in base.exception.method is still needed.
If a model inherits from base.exception.method, it has no exception_ids, that's why it must return another record in _get_main_records. The detected rules in base.exception.method are written to that other model. Like it is done right now in account_move_exception.
If a model inherits from base.exception, there are 2 ways:
- It overrides
_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 - It does not override
_get_main_records: Then the detected rules are added to self inbase.exception.methodand this here has no effect as it is always False
There was a problem hiding this comment.
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.
Hi,
I moved the new def
_get_sub_exception_field_namesfrom PR #3354 to here.This here uses the commits of the other PR. I will do a rebase after the other was merged.
I implemented a way to add exceptions to the record on which the exceptions where detected on in case
_get_main_recordsreturns something else thanself, likeself.order_id.That commit breakssale_exceptionbecause I added_get_main_recordstodetect_exceptionsso that it is not necessary anymore to do this in an overridden_detect_exceptions.EDIT: I checked
sale_exceptionagain to update PR OCA/sale-workflow#3861. Seems that it is not breaking the module. The overridden_detect_exceptionsinsale.order.linereturns the same as the overridden_get_main_records. After that_get_main_recordsis called again on the returned sales orders. So only one useless_get_main_recordscall.I removed the comment in
detect_exceptionsbecause it refered to the_reverse_fieldand the Many2many field on the exception rule. As this does not exist anymore, the comment is also not needed anymore