From 66ef641f33d4173fc55fe1016a292a2ebf55bdd3 Mon Sep 17 00:00:00 2001 From: Martin Trigaux Date: Wed, 3 Jul 2019 12:59:19 +0000 Subject: [PATCH 1/2] [REF] base: remove field source of ir.translation Instead, make sure the field src is always up to date Add tests Changes in _write: - Replace _set_ids (to be deprecated) call by _upsert_translations as it works in batch - Call _upsert_translations for any language, including en_US In case an English translation already existed for a record, only the master value (on the reference record) was updated but the user kept seeing the translation value (was revealed 73a7534bfc). - Read src_trans without language Similar as above, if an English translation already existed, the translation was used for the result of the read and not the new value that has just been inserted into the database - Add _set_source method When updating a master record of a translated field, the src field must be updated, including in different languages. Before it was ok that the src field was out of date as the source was computed Changes in copy_translation: - set src as the new value without lang update the comment to reflect reality since 489494e7339 src will contain the English version if no changes were made and it will contain the modified value if copy was overriden Changes in upsert_translations: - Do not force a module, comment, state Only src, res_id, name, value and lang must be given. Optional values will no longer be set to null if not given --- addons/transifex/models/ir_translation.py | 4 +- addons/website_gengo/controllers/main.py | 2 +- odoo/addons/base/models/ir_translation.py | 74 ++++++------------- odoo/addons/base/tests/test_translate.py | 65 ++++++++++++++++ .../base/views/ir_translation_views.xml | 8 +- odoo/models.py | 23 ++++-- 6 files changed, 112 insertions(+), 64 deletions(-) diff --git a/addons/transifex/models/ir_translation.py b/addons/transifex/models/ir_translation.py index 51edae39206..6c9e84755d7 100644 --- a/addons/transifex/models/ir_translation.py +++ b/addons/transifex/models/ir_translation.py @@ -63,7 +63,7 @@ class IrTranslation(models.Model): project_modules[module] = tx_project for translation in self: - if not translation.module or not translation.source or translation.lang == 'en_US': + if not translation.module or not translation.src or translation.lang == 'en_US': # custom or source term translation.transifex_url = False continue @@ -85,6 +85,6 @@ class IrTranslation(models.Model): 'lang': lang_code, 'module': translation.module, 'src': "text:'" + werkzeug.url_quote_plus( - translation.source[:50].replace("\n", "").replace("'", "") + translation.src[:50].replace("\n", "").replace("'", "") ) + "'", } diff --git a/addons/website_gengo/controllers/main.py b/addons/website_gengo/controllers/main.py index 56eeec5ff5c..ceb380671a5 100644 --- a/addons/website_gengo/controllers/main.py +++ b/addons/website_gengo/controllers/main.py @@ -14,7 +14,7 @@ class WebsiteGengo(http.Controller): result = {"done": 0} gengo_translation_ids = request.env['ir.translation'].search([('id', 'in', translated_ids), ('gengo_translation', '!=', False)]) for trans in gengo_translation_ids: - result['done'] += len(trans.source.split()) + result['done'] += len(trans.src.split()) return result @http.route('/website/check_gengo_set', type='json', auth='user', website=True) diff --git a/odoo/addons/base/models/ir_translation.py b/odoo/addons/base/models/ir_translation.py index b05cffcc1d9..8c5906a19a4 100644 --- a/odoo/addons/base/models/ir_translation.py +++ b/odoo/addons/base/models/ir_translation.py @@ -171,8 +171,6 @@ class IrTranslation(models.Model): lang = fields.Selection(selection='_get_languages', string='Language', validate=False) type = fields.Selection(TRANSLATION_TYPE, string='Type', index=True) src = fields.Text(string='Internal Source') # stored in database, kept for backward compatibility - source = fields.Text(string='Source term', compute='_compute_source', - inverse='_inverse_source', search='_search_source') value = fields.Text(string='Translation Value') module = fields.Char(index=True, help="Module this term belongs to") @@ -196,55 +194,6 @@ class IrTranslation(models.Model): langs = self.env['res.lang'].search([]) return [(lang.code, lang.name) for lang in langs] - @api.depends('type', 'name', 'res_id') - def _compute_source(self): - ''' Get source name for the translation. If object type is model, return - the value stored in db. Otherwise, return value store in src field. - ''' - for record in self: - record.source = record.src - if record.type != 'model': - continue - model_name, field_name = record.name.split(',') - if model_name not in self.env: - continue - model = self.env[model_name] - field = model._fields.get(field_name) - if field is None: - continue - if not callable(field.translate): - # Pass context without lang, need to read real stored field, not translation - try: - result = model.browse(record.res_id).with_context(lang=None).read([field_name]) - except AccessError: - # because we can read self but not the record, - # that means we would get an access error when accessing the translations - # so instead we defer the access right to the "check" method - result = [{field_name: _("Cannot be translated; record not accessible.")}] - record.source = result[0][field_name] if result else False - - def _inverse_source(self): - ''' When changing source term of a translation, change its value in db - for the associated object, and the src field. - ''' - self.ensure_one() - if self.type == 'model': - model_name, field_name = self.name.split(',') - model = self.env[model_name] - field = model._fields[field_name] - if not callable(field.translate): - # Make a context without language information, because we want - # to write on the value stored in db and not on the one - # associated with the current language. Also not removing lang - # from context trigger an error when lang is different. - model.browse(self.res_id).with_context(lang=None).write({field_name: self.source}) - if self.src != self.source: - self.write({'src': self.source}) - - def _search_source(self, operator, value): - ''' the source term is stored on 'src' field ''' - return [('src', operator, value)] - def _auto_init(self): res = super(IrTranslation, self)._auto_init() # Add separate md5 index on src (no size limit on values, and good performance). @@ -329,6 +278,22 @@ class IrTranslation(models.Model): ]) return len(ids) + def _set_source(self, name, ids, src): + """ Update the translation source of records. + + :param name: a string defined as "," + :param ids: the ids of the given records + :param src: the source of the translation + """ + self._cr.execute("""UPDATE ir_translation + SET src=%s + WHERE type=%s AND name=%s AND res_id IN %s + RETURNING id""", + (src, 'model', name, tuple(ids))) + existing_ids = [row[0] for row in self._cr.fetchall()] + # invalidate src for updated translations + self.invalidate_cache(fnames=['src'], ids=existing_ids) + @api.model def _get_source_query(self, name, types, lang, source, res_id): if source: @@ -645,6 +610,8 @@ class IrTranslation(models.Model): This method is used for creations of translations where the given ``vals_list`` is trusted to be the right values and potential conflicts should be updated to the new given value. + Mandatory values: name, lang, res_id, src, type + The other keys are ignored during update if not present """ rows_by_type = defaultdict(list) for vals in vals_list: @@ -661,7 +628,10 @@ class IrTranslation(models.Model): ON CONFLICT (type, lang, name, res_id) WHERE type='model' DO UPDATE SET (name, lang, res_id, src, type, value, module, state, comments) = (EXCLUDED.name, EXCLUDED.lang, EXCLUDED.res_id, EXCLUDED.src, EXCLUDED.type, - EXCLUDED.value, EXCLUDED.module, EXCLUDED.state, EXCLUDED.comments) + EXCLUDED.value, + COALESCE(EXCLUDED.module, ir_translation.module), + COALESCE(EXCLUDED.state, ir_translation.state), + COALESCE(EXCLUDED.comments, ir_translation.comments)) WHERE EXCLUDED.value IS NOT NULL AND EXCLUDED.value != ''; """.format(", ".join(["%s"] * len(rows_by_type['model']))) self.env.cr.execute(query, rows_by_type['model']) diff --git a/odoo/addons/base/tests/test_translate.py b/odoo/addons/base/tests/test_translate.py index 1477d41e788..f4a3cb29c94 100644 --- a/odoo/addons/base/tests/test_translate.py +++ b/odoo/addons/base/tests/test_translate.py @@ -275,6 +275,13 @@ class TestTranslation(TransactionCase): category_fr = category.with_context({'lang': 'fr_FR'}) self.assertEqual(category_fr.name, 'Clients', "Did not found translation for initial value") + translation_fr = self.env['ir.translation'].search([ + ('name', '=', 'res.partner.category,name'), + ('res_id', '=', category.id), + ('lang', '=', 'fr_FR'), + ]) + self.assertEqual(translation_fr.src, 'Customers', "Did not set English version as source") + def test_103_duplicate_record_fr(self): category = self.customers.with_context({'lang': 'fr_FR'}).copy({'name': 'Clients (copie)'}) @@ -284,6 +291,13 @@ class TestTranslation(TransactionCase): category_fr = category.with_context({'lang': 'fr_FR'}) self.assertEqual(category_fr.name, 'Clients (copie)', "Did not used default value for translated value") + translation_fr = self.env['ir.translation'].search([ + ('name', '=', 'res.partner.category,name'), + ('res_id', '=', category.id), + ('lang', '=', 'fr_FR'), + ]) + self.assertEqual(translation_fr.src, 'Clients (copie)', "Did not set new name as source") + def test_104_orderby_translated_field(self): """ Test search ordered by a translated field. """ # create a category with a French translation @@ -328,6 +342,57 @@ class TestTranslation(TransactionCase): self.assertEqual(len(translations), 1, "Translations were not duplicated after `translate_fields` call") self.assertEqual(translations.value, "Apprenti", "The first translation must stay") + def test_106_en_us_translation(self): + """ Test synchronizing translations with duplicated source """ + # create a category with a French translation + cheese = self.env['res.partner.category'].create({'name': 'Cheese'}) + + self.env['ir.translation'].translate_fields('res.partner.category', cheese.id, 'name') + + translations = self.env['ir.translation'].search([('name', '=', 'res.partner.category,name'), ('res_id', '=', cheese.id)], order='lang') + self.assertEqual(len(translations), 2) + self.assertRecordValues(translations, + [{'lang': 'en_US', 'src': 'Cheese', 'value': 'Cheese'}, + {'lang': 'fr_FR', 'src': 'Cheese', 'value': 'Cheese'}]) + + # Translate in both language + translations[0].value = 'The Cheese' + translations[1].value = 'Fromage' + + # lang=None bypass translation system + self.assertEqual(cheese.with_context(lang=None).name, 'Cheese') + self.assertEqual(cheese.with_context(lang='fr_FR').name, 'Fromage') + self.assertEqual(cheese.with_context(lang='en_US').name, 'The Cheese') + + # set a new master value + cheese.with_context(lang='en_US').write({'name': 'Delicious Cheese'}) + + # every src must be updated + self.assertEqual(cheese.with_context(lang=None).name, 'Delicious Cheese') + self.assertRecordValues(translations, + [{'lang': 'en_US', 'src': 'Delicious Cheese', 'value': 'Delicious Cheese'}, + {'lang': 'fr_FR', 'src': 'Delicious Cheese', 'value': 'Fromage'}]) + + self.assertEqual(cheese.with_context(lang=None).name, 'Delicious Cheese') + self.assertEqual(cheese.with_context(lang='fr_FR').name, 'Fromage') + self.assertEqual(cheese.with_context(lang='en_US').name, 'Delicious Cheese') + + def test_107_duplicate_record_en(self): + category = self.customers.with_context({'lang': 'en_US'}).copy() + + category_no = category.with_context({}) + self.assertEqual(category_no.name, 'Customers', "Duplication did not set untranslated value") + + category_fr = category.with_context({'lang': 'fr_FR'}) + self.assertEqual(category_fr.name, 'Clients', "Did not found translation for initial value") + + translation_fr = self.env['ir.translation'].search([ + ('name', '=', 'res.partner.category,name'), + ('res_id', '=', category.id), + ('lang', '=', 'fr_FR'), + ]) + self.assertEqual(translation_fr.src, 'Customers', "Did not set English version as source") + class TestXMLTranslation(TransactionCase): def setUp(self): diff --git a/odoo/addons/base/views/ir_translation_views.xml b/odoo/addons/base/views/ir_translation_views.xml index f1c373fcc96..772fa1d297b 100644 --- a/odoo/addons/base/views/ir_translation_views.xml +++ b/odoo/addons/base/views/ir_translation_views.xml @@ -11,7 +11,7 @@ string="Web-only translations" domain="[('comments', 'like', 'openerp-web')]"/> - + @@ -38,7 +38,7 @@ - + @@ -55,7 +55,7 @@ ir.translation - + @@ -71,7 +71,7 @@ - + diff --git a/odoo/models.py b/odoo/models.py index 7b4a2ac283d..01c728a65d2 100644 --- a/odoo/models.py +++ b/odoo/models.py @@ -3410,6 +3410,9 @@ Fields: # val is not a translation: update the table val = field.convert_to_column(val, self, vals) columns.append((name, field.column_format, val)) + tname = "%s,%s" % (self._name, name) + if field.translate is True and not single_lang: + self.env['ir.translation']._set_source(tname, self.ids, val) updated.append(name) else: other_fields.append(field) @@ -3440,6 +3443,7 @@ Fields: + '\n\n({} {}, {} {})'.format(_('Records:'), sub_ids[:6], _('User:'), self._uid) ) + translation_values = [] for name in updated: field = self._fields[name] if callable(field.translate): @@ -3449,15 +3453,23 @@ Fields: elif has_translation and field.translate: # The translated value of a field has been modified. - src_trans = self.read([name])[0][name] + src_trans = self.with_context(lang=None).read([name])[0][name] if not src_trans: # Insert value to DB src_trans = vals[name] self.with_context(lang=None).write({name: src_trans}) tname = "%s,%s" % (self._name, name) val = field.convert_to_column(vals[name], self, vals) - self.env['ir.translation']._set_ids( - tname, 'model', self.env.lang, self.ids, val, src_trans) + translation_values += [dict( + src=src_trans, + value=val, + name=tname, + lang=self.env.lang, + type='model', + state='translated', + res_id=res_id) for res_id in self.ids] + + self.env['ir.translation']._upsert_translations(translation_values) # mark fields to recompute; do this before setting other fields, because # the latter can require the value of computed fields, e.g., a one2many @@ -4283,11 +4295,12 @@ Fields: vals_list = [] for vals in Translation.search_read(domain): del vals['id'] - del vals['source'] # remove source to avoid triggering _set_src del vals['module'] # duplicated vals is not linked to any module vals['res_id'] = target_id + if not callable(field.translate): + vals['src'] = new_wo_lang[name] if vals['lang'] == old.env.lang and field.translate is True: - # force a source if the new_val was not changed by copy override + # update master record if the new_val was not changed by copy override if new_val == old[name]: new_wo_lang[name] = old_wo_lang[name] vals['src'] = old_wo_lang[name] From 18d9c2cab2426d337e53b9c1e10a3aa103649936 Mon Sep 17 00:00:00 2001 From: Martin Trigaux Date: Thu, 25 Jul 2019 09:52:25 +0000 Subject: [PATCH 2/2] [FIX] base: single lang behaviour If a db is in single lang (en_US or not) but still has translations, the translation should be updated in addition to updating the referenced record Add tests to formalise the expected behaviour: If one language only (en_US or not), when writing on a translatable field: - record field should updated - value of potential existing en_US translation should be updated - src of potential existing translations should be updated - no new translation should be created If en_US and fr_FR, when writing on a translatable field in en_US: - record field should updated - value of potential existing en_US translation should be updated - src of potential existing translations should be updated - new en_US translation should be created if was not present If en_US and fr_FR, when writing on a translatable field in fr_FR: - record field should not be updated - value of potential existing fr_FR translation should be updated - src of potential existing translations should not be updated - new fr_FR translation should be created if was not present Adapt test_new_api test get_installed is ormcached, just putting active = True is not enough --- odoo/addons/base/models/ir_translation.py | 24 ++++++ odoo/addons/base/tests/test_translate.py | 86 +++++++++++++++++++ .../test_new_api/tests/test_new_fields.py | 3 +- odoo/models.py | 13 ++- 4 files changed, 121 insertions(+), 5 deletions(-) diff --git a/odoo/addons/base/models/ir_translation.py b/odoo/addons/base/models/ir_translation.py index 8c5906a19a4..f460050a7e9 100644 --- a/odoo/addons/base/models/ir_translation.py +++ b/odoo/addons/base/models/ir_translation.py @@ -649,6 +649,30 @@ class IrTranslation(models.Model): """.format(", ".join(["%s"] * len(rows_by_type['model_terms']))) self.env.cr.execute(query, rows_by_type['model_terms']) + def _update_translations(self, vals_list): + """ Update translations of type 'model' or 'model_terms'. + + This method is used for update of translations where the given + ``vals_list`` is trusted to be the right values + No new translation will be created + """ + grouped_rows = {} + for vals in vals_list: + key = (vals['lang'], vals['type'], vals['name']) + grouped_rows.setdefault(key, [vals['value'], vals['src'], vals['state'], []]) + grouped_rows[key][3].append(vals['res_id']) + + for where, values in grouped_rows.items(): + self._cr.execute( + """ UPDATE ir_translation + SET value=%s, + src=%s, + state=%s + WHERE lang=%s AND type=%s AND name=%s AND res_id in %s + """, + (values[0], values[1], values[2], where[0], where[1], where[2], tuple(values[3])) + ) + @api.model def translate_fields(self, model, id, field=None): """ Open a view for translating the field(s) of the record (model, id). """ diff --git a/odoo/addons/base/tests/test_translate.py b/odoo/addons/base/tests/test_translate.py index f4a3cb29c94..f04b1035b08 100644 --- a/odoo/addons/base/tests/test_translate.py +++ b/odoo/addons/base/tests/test_translate.py @@ -393,6 +393,92 @@ class TestTranslation(TransactionCase): ]) self.assertEqual(translation_fr.src, 'Customers', "Did not set English version as source") +class TestTranslationWrite(TransactionCase): + + def setUp(self): + super().setUp() + self.category = self.env['res.partner.category'].create({'name': 'Reblochon'}) + + def test_01_en(self): + langs = self.env['res.lang'].get_installed() + self.assertEqual([('en_US', 'English (US)')], langs, "Test did not started with expected languages") + + self.category.with_context(lang='en_US').write({'name': 'English Name'}) + name = self.category.with_context(lang=None).read(['name']) + self.assertEqual(name[0]['name'], "English Name", "Reference field not updated") + translations = self.env['ir.translation'].search([ + ('name', '=', 'res.partner.category,name'), + ('res_id', '=', self.category.id), + ('lang', '=', 'en_US'), + ]) + self.assertEqual(len(translations), 0, "No English translation should be created when writing in English") + + def test_02_en_translated(self): + langs = self.env['res.lang'].get_installed() + self.assertEqual([('en_US', 'English (US)')], langs, "Test did not started with expected languages") + translation = self.env['ir.translation'].create({ + 'type': 'model', + 'name': 'res.partner.category,name', + 'lang': 'en_US', + 'res_id': self.category.id, + 'src': 'Reblochon', + 'value': 'Translated Name', + 'state': 'translated', + }) + + self.category.with_context(lang='en_US').write({'name': 'English Name'}) + translation_value = translation.read(['value']) + self.assertEqual(translation_value[0]['value'], "English Name", "Existing translation was not updated") + + source_name = self.category.with_context(lang=None).read(['name']) + self.assertEqual(source_name[0]['name'], "English Name", "Reference field not updated") + + def test_03_fr_single(self): + self.env['res.lang'].load_lang('fr_FR') + self.env['res.users'].with_context(active_test=False).search([]).write({'lang': 'fr_FR'}) + self.env.ref('base.lang_en').active = False + + langs = self.env['res.lang'].get_installed() + self.assertEqual([('fr_FR', 'French / Français')], langs, "Test did not started with expected languages") + + self.category.with_context(lang='fr_FR').write({'name': 'French Name'}) + source_name = self.category.with_context(lang=None).read(['name']) + self.assertEqual(source_name[0]['name'], "French Name", "Reference field not updated") + translations = self.env['ir.translation'].search([ + ('name', '=', 'res.partner.category,name'), + ('res_id', '=', self.category.id), + ('lang', '=', 'fr_FR'), + ]) + self.assertEqual(len(translations), 0, "No French translation should be created when writing in French") + + def test_04_fr_multi(self): + self.env['res.lang'].load_lang('fr_FR') + + langs = self.env['res.lang'].get_installed() + self.assertEqual([('en_US', 'English (US)'), ('fr_FR', 'French / Français')], langs, + "Test did not started with expected languages") + self.env['ir.translation'].create({ + 'type': 'model', + 'name': 'res.partner.category,name', + 'lang': 'en_US', + 'res_id': self.category.id, + 'src': 'Reblochon', + 'value': 'Translated Name', + 'state': 'translated', + }) + + self.category.with_context(lang='fr_FR').write({'name': 'French Name'}) + self.category.with_context(lang='en_US').write({'name': 'English Name'}) + translations = self.env['ir.translation'].search([ + ('name', '=', 'res.partner.category,name'), + ('res_id', '=', self.category.id), + ], order='lang') + self.assertRecordValues(translations, [ + {'src': 'English Name', 'value': 'English Name', 'lang': 'en_US'}, + {'src': 'English Name', 'value': 'French Name', 'lang': 'fr_FR'} + ]) + + class TestXMLTranslation(TransactionCase): def setUp(self): diff --git a/odoo/addons/test_new_api/tests/test_new_fields.py b/odoo/addons/test_new_api/tests/test_new_fields.py index b8f254e512f..d372f6f23ae 100644 --- a/odoo/addons/test_new_api/tests/test_new_fields.py +++ b/odoo/addons/test_new_api/tests/test_new_fields.py @@ -1230,8 +1230,7 @@ class TestFields(common.TransactionCase): email = self.env.ref('test_new_api.emailmessage_0_0') self.assertEqual(email.message, message) - french = self.env['res.lang']._lang_get('fr_FR') - french.active = True + self.env['res.lang'].load_lang('fr_FR') def count(msg): # return the number of translations of msg.label diff --git a/odoo/models.py b/odoo/models.py index 01c728a65d2..5ffde4cd104 100644 --- a/odoo/models.py +++ b/odoo/models.py @@ -3397,6 +3397,12 @@ Fields: other_fields = [] # list of non-column fields single_lang = len(self.env['res.lang'].get_installed()) <= 1 has_translation = self.env.lang and self.env.lang != 'en_US' + # when there is only one language, update existing translations but + # do not create new ones + if single_lang: + process_translations = self.env['ir.translation']._update_translations + else: + process_translations = self.env['ir.translation']._upsert_translations for name, val in vals.items(): field = self._fields[name] @@ -3411,7 +3417,7 @@ Fields: val = field.convert_to_column(val, self, vals) columns.append((name, field.column_format, val)) tname = "%s,%s" % (self._name, name) - if field.translate is True and not single_lang: + if field.translate is True and self.env.lang: self.env['ir.translation']._set_source(tname, self.ids, val) updated.append(name) else: @@ -3451,7 +3457,7 @@ Fields: # synchronize translated terms when possible. self.env['ir.translation']._sync_terms_translations(field, self) - elif has_translation and field.translate: + elif self.env.lang and field.translate: # The translated value of a field has been modified. src_trans = self.with_context(lang=None).read([name])[0][name] if not src_trans: @@ -3469,7 +3475,8 @@ Fields: state='translated', res_id=res_id) for res_id in self.ids] - self.env['ir.translation']._upsert_translations(translation_values) + if translation_values: + process_translations(translation_values) # mark fields to recompute; do this before setting other fields, because # the latter can require the value of computed fields, e.g., a one2many