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

translating terms directly in the view (wizard) #31090

Closed
wants to merge 13 commits into from

Conversation

jso-odoo
Copy link
Contributor

Description of the issue/feature this PR addresses:

Current behavior before PR:

  • Field translation was not user friendly.

Desired behavior after PR is merged:

  • Field translation will be user friendly and user can edit the translation value also from the tree view.

--
I confirm I have signed the CLA and read the PR guidelines at www.odoo.com/submit-pr

@robodoo robodoo added the CI 🤖 Robodoo has seen passing statuses label Feb 14, 2019
@C3POdoo C3POdoo added the RD research & development, internal work label Feb 14, 2019
@robodoo robodoo removed the CI 🤖 Robodoo has seen passing statuses label Feb 14, 2019
@robodoo robodoo added the CI 🤖 Robodoo has seen passing statuses label Feb 14, 2019
@robodoo robodoo added CI 🤖 Robodoo has seen passing statuses and removed CI 🤖 Robodoo has seen passing statuses labels Feb 15, 2019
@robodoo robodoo removed the CI 🤖 Robodoo has seen passing statuses label Feb 15, 2019
@robodoo robodoo added CI 🤖 Robodoo has seen passing statuses and removed CI 🤖 Robodoo has seen passing statuses labels Feb 15, 2019
@robodoo robodoo added CI 🤖 Robodoo has seen passing statuses and removed CI 🤖 Robodoo has seen passing statuses labels Feb 25, 2019
@robodoo robodoo removed the CI 🤖 Robodoo has seen passing statuses label Mar 27, 2019
@robodoo robodoo added the CI 🤖 Robodoo has seen passing statuses label Mar 27, 2019
@wtaferner
Copy link
Contributor

@mart-e
Do you think it would be possible to introduce/backport this in/to 11.0? Basically it seems that it is by now a usability feature which might be applicable on older versions too with a calculated risk.

@mart-e
Copy link
Contributor

mart-e commented May 2, 2019

@wtaferner this code is adding a new model so definitely not possible to backport in stable. Maybe some parts can be taken in a new module.

jso-odoo and others added 4 commits May 2, 2019 16:46
… on translated field will open wizard with editable listview, where user will have two field(to have simple view), Language and Translation Value, added two transient model to meet to specification without changing JS code
@mart-e mart-e force-pushed the master-translate-term-jso branch from 5ef73ec to 9c9d99e Compare May 2, 2019 14:47
@robodoo robodoo removed the CI 🤖 Robodoo has seen passing statuses label May 2, 2019
mart-e
mart-e previously requested changes May 2, 2019
Copy link
Contributor

@mart-e mart-e left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The result looks good, it greatly improve the recording and update of translations 👍
However, the code still seems very complex. It looks like you are duplicating code in the wizard that suspiciously looks a lot like the one in translate_fields. Can't we reuse some information we already got?

if field:
fld = record._fields[field]
if not fld.related:
domain += ['|', ('name', '=', "%s,%s" % (fld.model_name, fld.name)),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use expression.AND and expression.OR when creating domains with conditional clauses, it avoids errors

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

when I said to use expression, it was to avoid the += 😉 (I will correct)

<field name="arch" type="xml">
<form string="Translations">
<field name="translation_lines" nolabel="1">
<tree string="Translations" editable="top" create="false" delete="false">
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why preventing to delete? It actually is an easy way to get rid of old translations.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mart-e this is o2m field and records of translations loaded in new TransientModel with default_get, even if we allow delete, we can not delete as it is in o2m using default_get, we still not have database ids for those records, so we simply remove delete button

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@msh-odoo so if I want to remove a bad translation (but I don't know how to fix it), how should I proceed? Put an empty string?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mart-e Well, I have one way to delete translation but I am not sure we should go with it
Allow user to delete translation from wizard o2m, when Confirm button clicked, we will generate default lines again in confirm button method and match with Wizard record's o2m lines, whatever missing considered as deleted, what do you think? this is the only way currently I have

odoo/addons/base/wizard/base_translation_wizard.py Outdated Show resolved Hide resolved
odoo/addons/base/wizard/base_translation_wizard.py Outdated Show resolved Hide resolved

domain = self._prepare_domain()
translations = IrTranslation.search(domain)
res['translation_lines'] = [[0, False, {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

detail: we usually a list of tuples for commands instead of a list of list

odoo/addons/base/models/ir_translation.py Outdated Show resolved Hide resolved
@robodoo robodoo added the CI 🤖 Robodoo has seen passing statuses label May 2, 2019
In this commit,
- changed the conditional domains with expression.OR
- changed method name
- improved naming conventions
@robodoo robodoo added CI 🤖 Robodoo has seen passing statuses and removed CI 🤖 Robodoo has seen passing statuses labels May 7, 2019
@robodoo robodoo added CI 🤖 Robodoo has seen passing statuses and removed CI 🤖 Robodoo has seen passing statuses labels May 7, 2019
@robodoo robodoo added CI 🤖 Robodoo has seen passing statuses and removed CI 🤖 Robodoo has seen passing statuses labels May 7, 2019
@mart-e mart-e dismissed their stale review May 17, 2019 12:44

cleaning and merging


@api.model
def default_get(self, fields_list):
res = super(TranslationWizard, self).default_get(fields_list)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in python 3, super().default_get(fields_list) is enough

@mart-e
Copy link
Contributor

mart-e commented Jul 2, 2019

Replaced by #33623

@mart-e mart-e closed this Jul 2, 2019
@mart-e mart-e deleted the master-translate-term-jso branch July 2, 2019 13:02
@robodoo robodoo added closed 💔 and removed CI 🤖 Robodoo has seen passing statuses labels Jul 2, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
RD research & development, internal work
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants