[16.0] [ADD] account_chart_update_multilang - new module #1957
Conversation
|
I'll address the test error on Monday. I have no clue of why is failing at the moment. |
edlopen
force-pushed
the
16.0-add-account_chart_update_multilang
branch
from
b605539
to
36534e1
Compare
edlopen
force-pushed
the
16.0-add-account_chart_update_multilang
branch
2 times, most recently
from
b6ffa87
to
6bfeb3d
Compare
| @api.model | ||
| @tools.ormcache("name") |
There was a problem hiding this comment.
This is not an api.model function because it does need a record.
Besides, caching it won't save much time because the things it does are quite cheap.
| return self.env["res.lang"].search([("code", "!=", self.lang)]).mapped("code") | ||
|
|
||
| def _update_other_langs(self, templates): | ||
| for _, tpl_xmlid in templates.get_external_id().items(): |
There was a problem hiding this comment.
| for _, tpl_xmlid in templates.get_external_id().items(): | |
| for tpl_xmlid in templates.get_external_id().values(): |
| to_include = self.fields_to_include(template._name) | ||
| for key in template._fields: | ||
| if not template._fields[key].translate or key not in to_include: | ||
| continue |
There was a problem hiding this comment.
suggestion: here you're iterating over all fields and then checking if they are in a list, which in turn loops behind the scenes to do that check.
It seems flipping the logic makes more sense:
| to_include = self.fields_to_include(template._name) | |
| for key in template._fields: | |
| if not template._fields[key].translate or key not in to_include: | |
| continue | |
| to_include = self.fields_to_include(template._name) | |
| for key in to_include: | |
| if not template._fields[key].translate: | |
| continue |
| template_trans = getattr(template.with_context(lang=lang), key) | ||
| real_trans = getattr(real.with_context(lang=lang), key) | ||
| if template_trans != real_trans: | ||
| res[key] = template[key] |
There was a problem hiding this comment.
suggestion: Don't use getattr if you can avoid it, for security reasons. It was explained in some OXP security talk but I can't remember the year... In short, it makes sure the thing you're accessing is a field and not a method, property or something else.
| template_trans = getattr(template.with_context(lang=lang), key) | |
| real_trans = getattr(real.with_context(lang=lang), key) | |
| if template_trans != real_trans: | |
| res[key] = template[key] | |
| template_trans = template.with_context(lang=lang)[key] | |
| real_trans = real.with_context(lang=lang)[key] | |
| if template_trans != real_trans: | |
| res[key] = template[key] |
|
I've diagnosed a bit more the problem of not being able to translate notes in fiscal positions. It's due to how the html translation algorithm works. Pushing a wip fix. Could you set this as draft please? |
yajo
force-pushed
the
16.0-add-account_chart_update_multilang
branch
from
e8c7b58
to
0806aaf
Compare
yajo
force-pushed
the
16.0-add-account_chart_update_multilang
branch
from
0806aaf
to
ff8e9ef
Compare
The algorithm to translate them is done by terms, which is a bit clunky for our use case. However, since `account.fiscal.position.template`'s note field is Text, not Html, and that's the one we care about, we can predict that the Html version will just wrap it with a <p> tag. Well, in short, it's ugly but works. MT-7597
yajo
force-pushed
the
16.0-add-account_chart_update_multilang
branch
from
ff8e9ef
to
e9dd1a3
Compare
|
Last push should fix the problem. |
This new module extends the functionality of account_chart_update so that if the tax or fiscal position template has translated fields, it is passed to the final tax or fiscal position when applying the chart of accounts.
https://www.loom.com/share/ddc5e524a5f540e3ae680dc7f40d0090?sid=872b10d1-9a61-4d52-8452-91546a77ccbe
Although I am working to improve, please bear with me because of my English.
@Shide @yajo @rafaelbn please review, if you can.
@moduon MT-7597