Skip to content
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

[IMP] *: translation methods cleaning #38859

Merged
merged 7 commits into from Nov 19, 2019

Conversation

@mart-e
Copy link
Contributor

mart-e commented Oct 16, 2019

Extract some cleaning wanted in #30759 but not related to language packs

  • amount_to_text should work only on activated languages
  • deprecate load_lang and rely on _create_lang or _activate_lang
  • trans_load_data and _load_module_terms no longer silently activate languages
  • pass explicitly parameters instead of relying on context content
  • replace some IrTranslation._load_module_terms(['base'], ['fr_FR']) by BaseModule._update_translations(['fr_FR']) for higher level methods
  • create TranslationExporter class to clarify the trans_export method (and clean dead code)

Task-id: 2088290
Pad: https://pad.odoo.com/p/r.f6f789f8711d21314bd902972153ea9d

@robodoo robodoo added the seen 🙂 label Oct 16, 2019
@C3POdoo C3POdoo added the RD label Oct 16, 2019
@mart-e mart-e force-pushed the odoo-dev:master-language-usage-clean-mat branch 6 times, most recently from 8cbb907 to d35d95b Oct 16, 2019
odoo/addons/base/models/res_lang.py Outdated Show resolved Hide resolved
odoo/addons/base/models/res_lang.py Outdated Show resolved Hide resolved
@mart-e mart-e force-pushed the odoo-dev:master-language-usage-clean-mat branch 5 times, most recently from d8951fd to ac0f96d Oct 16, 2019
@robodoo robodoo added the CI 🤖 label Oct 16, 2019
@mart-e mart-e force-pushed the odoo-dev:master-language-usage-clean-mat branch from e84e34b to 41bf43c Oct 17, 2019
@robodoo robodoo removed the CI 🤖 label Oct 17, 2019
@mart-e mart-e force-pushed the odoo-dev:master-language-usage-clean-mat branch 3 times, most recently from 1cbc47d to 4fd2dbe Oct 17, 2019
@mart-e mart-e requested a review from rco-odoo Oct 17, 2019
@mart-e mart-e force-pushed the odoo-dev:master-language-usage-clean-mat branch 2 times, most recently from 09feb2b to fca23cf Oct 17, 2019
@robodoo robodoo added the CI 🤖 label Oct 17, 2019
Copy link
Member

rco-odoo left a comment

A big step to better code!

odoo/addons/base/models/ir_translation.py Outdated Show resolved Hide resolved
odoo/addons/base/models/res_lang.py Outdated Show resolved Hide resolved
odoo/addons/base/models/res_lang.py Outdated Show resolved Hide resolved
odoo/addons/base/models/res_lang.py Show resolved Hide resolved
odoo/addons/base/wizard/base_import_language.py Outdated Show resolved Hide resolved
odoo/tools/translate.py Outdated Show resolved Hide resolved
@mart-e mart-e force-pushed the odoo-dev:master-language-usage-clean-mat branch from fca23cf to d7e8b63 Oct 22, 2019
@robodoo robodoo removed the CI 🤖 label Oct 22, 2019
@mart-e mart-e force-pushed the odoo-dev:master-language-usage-clean-mat branch 3 times, most recently from edb97d1 to c90ad94 Oct 22, 2019
@mart-e mart-e force-pushed the odoo-dev:master-language-usage-clean-mat branch from 0cfbc91 to cf11ab2 Nov 7, 2019
@robodoo robodoo added CI 🤖 and removed CI 🤖 labels Nov 7, 2019
@mart-e mart-e force-pushed the odoo-dev:master-language-usage-clean-mat branch from cf11ab2 to 51de41d Nov 8, 2019
@robodoo robodoo removed the CI 🤖 label Nov 8, 2019
@robodoo robodoo added the CI 🤖 label Nov 8, 2019
mart-e added 4 commits Oct 16, 2019
Only activated languages should be used in "amount to text" features.
If a language code of a not-used language is used, it should be
ignored for consistency with the rest of the interface.
load_lang was a kind of hybrid method trying to active or creating a
language if not found. This was error prone.
Instead rely on two methods with clear purpose:
ResLang._create_lang(lang, lang_name=None)
  - create a new res.lang entry using the locale of the server
    return the res.lang record to match the API of _activate_lang

ResLang._active_lang(code)
  - activate the given code lang

Most of the time, _active_lang is what is expected

tools.trans_load_data and IrTranslation._load_module_terms no longer
activate the language if not active.
Loading the translations should be explicit on an activated language,
it is too error prone to silently activate/create a language if not
found.
Remove lang_name from trans_load_data as no longer needed.
Instead of relying on the context content, pass explicit values for
overwrite and create_empty_translations
applu this to trans_load and trans_load_data
Adapt the test that was trying to create empty translations.
To keep up with the new API
Note we still do not have a good solution for the problem mentionned
below.
@mart-e mart-e force-pushed the odoo-dev:master-language-usage-clean-mat branch from 51de41d to 01c6f76 Nov 19, 2019
@robodoo robodoo removed the CI 🤖 label Nov 19, 2019
@mart-e mart-e force-pushed the odoo-dev:master-language-usage-clean-mat branch from 01c6f76 to 81339c5 Nov 19, 2019
@mart-e

This comment has been minimized.

Copy link
Contributor Author

mart-e commented Nov 19, 2019

@robodoo r+ merge

@robodoo robodoo added the r+ 👌 label Nov 19, 2019
Instead, updating the translations of a module can be done directly
on the ir.module.module record
Remove one call to _update_translation by the actual creation of the
language
@robodoo

This comment has been minimized.

Copy link
Contributor

robodoo commented Nov 19, 2019

Merge method set to merge directly, using the PR as merge commit message

mart-e added 2 commits Oct 17, 2019
Instead of previous long methods, use a class to clarify what the
export actually does.
The TranslationModuleReader is written to copy the API of the
TranslationFileWriter.
This way, exporting translations is reduced to:
1. create a reader that will fetch all module translations (either
   from db or from static files)
2. create a writer in a specific format (po or csv)
3. export the content from the reader to the writer

Simplify the writer by deducing modules from exported translations
instead of fetching it again in a oneliner (this way can benefit from
yield operations)

Remove the 'all_installed' possibility in modules as it was not
working (creating query with 2 WHERE clause).

The new methode _get_translatable_records works on a per model basis.
This will allow a big performance gain as the previous code was
making a .exists() for each record individually.
In the future, this method could be removed as the main goal is to
test the presence of the rare attribute _translate=False.
It was misleading as only forced for translations of type 'code' but
for the other translations, it was retrieved from the imported file
(the comment in a .po file or column in a .csv)

This will allow another optimisation in the next commit, moving to a
TranslationModuleReader instance
@mart-e mart-e force-pushed the odoo-dev:master-language-usage-clean-mat branch from 81339c5 to 95ed217 Nov 19, 2019
@mart-e

This comment has been minimized.

Copy link
Contributor Author

mart-e commented Nov 19, 2019

robodoo r+ again, my bad

@robodoo robodoo added the CI 🤖 label Nov 19, 2019
@robodoo

This comment has been minimized.

Copy link
Contributor

robodoo commented Nov 19, 2019

Linked pull request(s) odoo/enterprise#6210 not ready. Linked PRs are not staged until all of them are ready.

robodoo added a commit that referenced this pull request Nov 19, 2019
Extract some cleaning wanted in #30759 but not related to language packs

- `amount_to_text` should work only on activated languages
- deprecate `load_lang` and rely on `_create_lang` or `_activate_lang`
- `trans_load_data` and `_load_module_terms` no longer silently activate languages
- pass explicitly parameters instead of relying on context content
- replace some `IrTranslation._load_module_terms(['base'], ['fr_FR'])` by `BaseModule._update_translations(['fr_FR'])` for higher level methods
- create `TranslationExporter` class to clarify the `trans_export` method (and clean dead code)

closes #38859

Task-id: 2088290
Pad: https://pad.odoo.com/p/r.f6f789f8711d21314bd902972153ea9d
Signed-off-by: Martin Trigaux (mat) <mat@odoo.com>
@robodoo robodoo merged commit 95ed217 into odoo:master Nov 19, 2019
2 checks passed
2 checks passed
ci/runbot runbot build 692927-38859-95ed21 (runtime 10s)
Details
legal/cla Martin Trigaux Odoo CLA signature check
Details
@robodoo robodoo added merged 🎉 and removed merging 👷 labels Nov 19, 2019
@robodoo

This comment has been minimized.

Copy link
Contributor

robodoo commented Nov 19, 2019

Merged at f591ec1, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.