[12.0] Customer language + company_name handling #474
Conversation
|
@lmignon I'll have to support partner titles as well: to me it makes sense to keep it in the core as we are talking about basic core fields for odoo. What do you think? Draft here #476 Otherwise we could move everything into a separated module like FTR, I added support for partner_firstname in a separated module here #475 |
shopinvader/services/customer.py
Outdated
| def _prepare_params(self, params): | ||
| for key in self._empty_fields_to_drop(): |
There was a problem hiding this comment.
@simahawk I don't understand why empty fields should be dropped. The required attribute into the cerberus scheme means that the field must be (or not) present into the json whatever their values. The attribute nullablecan be used to say if a field can be set to null....This allows the following usages:
{'my_field': null} -> reset the value to null for my_field
{} -> nothing to change for my field
{'my_field': 'val1'l} -> set value 'val1' for my_field
There was a problem hiding this comment.
but AFAIS if you pass an empty string this is not popped out which means you gonna have an empty string where you expect none.
Maybe using a custom coerce is better?
There was a problem hiding this comment.
If you don't want to modify a field, don't put it into the json...
There was a problem hiding this comment.
but the client might send it anyway (eg: an hidden field). I just want to be defensive in such cases. 🤷♂️
There was a problem hiding this comment.
it's the responsibility of the client to sent or not the field. Otherwise how will it be possible to reset these fields?
There was a problem hiding this comment.
I removed the hack, in any case if we have issues we can think about it later. Thanks for your feedback :)
Codecov Report
@@ Coverage Diff @@
## 12.0 #474 +/- ##
=======================================
Coverage 91.26% 91.26%
=======================================
Files 104 104
Lines 2898 2898
=======================================
Hits 2645 2645
Misses 253 253
Continue to review full report at Codecov.
|
simahawk
force-pushed
the
12-customer-attrs
branch
from
f949a87 to
be23cf8
Compare
|
/ocabot merge patch |
|
Hey, thanks for contributing! Proceeding to merge this for you. |
|
Congratulations, your PR was merged at 5890a73. Thanks a lot for contributing to shopinvader. ❤️ |
Addresses #473.
I've found that
company_nameas well can be supported by default harmlessly.