[16.0][3863][ADD] crm_exception #56
Conversation
AungKoKoLin1997
force-pushed
the
3863-add-crm_exception
branch
from
7c7f9cf to
b9bc1a4
Compare
AungKoKoLin1997
requested review from
kanda999,
yostashiro and
rinaldifirdaus
There was a problem hiding this comment.
IMO, we can go ahead with this design. as workaround, the client should prepare multiple exception for each field and give the warning message on exception name such "Please fill the Phone field", so that the users will acknowledge of which field that is required to be filled.
crm_exception/models/crm_lead.py
Outdated
| def crm_check_exception(self): | ||
| self._check_exception() | ||
|
|
||
| def _check_crm_lead_check_exception(self, vals): | ||
| check_exceptions = any( | ||
| field in vals for field in self._fields_trigger_check_exception() | ||
| ) | ||
| if check_exceptions: | ||
| self.crm_check_exception() |
There was a problem hiding this comment.
| def crm_check_exception(self): | |
| self._check_exception() | |
| def _check_crm_lead_check_exception(self, vals): | |
| check_exceptions = any( | |
| field in vals for field in self._fields_trigger_check_exception() | |
| ) | |
| if check_exceptions: | |
| self.crm_check_exception() | |
| def _check_crm_lead_check_exception(self, vals): | |
| check_exceptions = any( | |
| field in vals for field in self._fields_trigger_check_exception() | |
| ) | |
| if check_exceptions: | |
| self._check_exception() |
crm_exception/__manifest__.py
Outdated
| # Copyright 2023 Quartile Limited | ||
| # License AGPL-3.0 or later (https://www.gnu.org/licenses/agpl). | ||
| { | ||
| "name": "Crm Exception", |
There was a problem hiding this comment.
| "name": "Crm Exception", | |
| "name": "CRM Exception", |
crm_exception/__manifest__.py
Outdated
| { | ||
| "name": "Crm Exception", | ||
| "version": "16.0.1.0.0", | ||
| "category": "Tools", |
There was a problem hiding this comment.
| "category": "Tools", | |
| "category": "Customer Relationship Management", |
crm_exception/readme/DESCRIPTION.rst
Outdated
| @@ -0,0 +1 @@ | |||
| This module allows you attach several customizable exceptions to your opportunity. | |||
| <xpath expr="//field[@name='tag_ids']" position="after"> | ||
| <field | ||
| name="ignore_exception" | ||
| states="purchase" |
There was a problem hiding this comment.
Propose an alternative or simply remove it.
| if check_exceptions: | ||
| self.crm_check_exception() | ||
|
|
||
| def write(self, vals): |
There was a problem hiding this comment.
@AungKoKoLin1997 Any reason to use write() instead of @api.constrains()? Shouldn't the latter make the code more concise?
There was a problem hiding this comment.
The main purpose is to avoid hardcoding a trigger field list in @api.constrains and ensure flexibility for potential future additions to the _fields_trigger_check_exception. May be rare case but I think it is also good for design.
There was a problem hiding this comment.
@AungKoKoLin1997 Isn't that practically the same as the trigger fields are hardcoded anyway in _fields_trigger_check_exception() or am I missing something?
There was a problem hiding this comment.
Yes. The trigger fields are hardcoded in _fields_trigger_check_exception() and we may extend this function and can add new more fields from another module if we will need some adjustments in the future.
There was a problem hiding this comment.
@AungKoKoLin1997 Can you provide the reason why we are now not supporting the ignore_exception option?
I misunderstood. I will revert it. |
|
Closing this PR. I will create in OCA repo. |
I will consider creating PR in OCA after the design is approved.
3863