From e2df05532763c79c9555676f4a40a8ec43378c78 Mon Sep 17 00:00:00 2001 From: Jairo Llopis Date: Tue, 18 Oct 2016 12:53:13 +0200 Subject: [PATCH] [8.0][FIX][base_export_manager] Fix "Expected singleton" bug. (#521) [FIX][base_export_manager] Fix "Expected singleton" bug. If you had a field that got translated in more than 1 addon, you'd possibly getto this error: File "/opt/odoo/0079_ahk_openerp/oca/base_export_manager/models/ir_exports_line.py", line 105, in _compute_label field.name)), File "/opt/odoo/common/openerp/v8/openerp/fields.py", line 825, in __get__ record.ensure_one() File "/opt/odoo/common/openerp/v8/openerp/models.py", line 5355, in ensure_one raise except_orm("ValueError", "Expected singleton: %s" % self) except_orm: ('ValueError', 'Expected singleton: ir.translation(4899, 703976)') With this patch, now we let Odoo return the translated string by using its standard method to do so, so we have to care for less. * Move installation outside a data file. This makes the whole installation to be able to roll back if something goes wrong, instead of entering an error loop. * Include envorionment in its manager. * Add 4th field * Move to api.multi, refactoring some stuff. - Add some comments in complex parts. - Rename `onchange_name` to `_onchange_name` (guidelines). - Make `_compute_name`'s try block shorter and easier to understand. * Allow R/W of name directly in model. * Update tests to cover new behaviors. --- base_export_manager/README.rst | 3 +- base_export_manager/__init__.py | 1 + base_export_manager/data/ir_exports_data.xml | 6 - base_export_manager/hooks.py | 20 +++ .../migrations/8.0.2.1.0/post-migrate.py | 9 + base_export_manager/models/ir_exports_line.py | 157 +++++++++++------- base_export_manager/tests/__init__.py | 3 +- .../tests/test_ir_exports_line.py | 53 +++++- base_export_manager/views/ir_exports.xml | 28 ++-- 9 files changed, 190 insertions(+), 90 deletions(-) delete mode 100644 base_export_manager/data/ir_exports_data.xml create mode 100644 base_export_manager/hooks.py create mode 100644 base_export_manager/migrations/8.0.2.1.0/post-migrate.py diff --git a/base_export_manager/README.rst b/base_export_manager/README.rst index 07454a9f0..be8bf9c81 100644 --- a/base_export_manager/README.rst +++ b/base_export_manager/README.rst @@ -44,7 +44,8 @@ To manage export profiles, you need to: * Choose a name. * Choose a model (table in the database). * Choose the fields to export. - * If you choose a related field, you can choose also up to 3 levels of subfields. + * If you choose a related field, you can choose also up to 4 levels of + subfields. * You can drag & drop to reorder the fields. To use one of those profiles, you need to: diff --git a/base_export_manager/__init__.py b/base_export_manager/__init__.py index 13d8b17b5..ba332de8d 100644 --- a/base_export_manager/__init__.py +++ b/base_export_manager/__init__.py @@ -3,3 +3,4 @@ # License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl.html). from . import models +from .hooks import post_init_hook diff --git a/base_export_manager/data/ir_exports_data.xml b/base_export_manager/data/ir_exports_data.xml deleted file mode 100644 index c04761a89..000000000 --- a/base_export_manager/data/ir_exports_data.xml +++ /dev/null @@ -1,6 +0,0 @@ - - - - - - diff --git a/base_export_manager/hooks.py b/base_export_manager/hooks.py new file mode 100644 index 000000000..5718758cc --- /dev/null +++ b/base_export_manager/hooks.py @@ -0,0 +1,20 @@ +# -*- coding: utf-8 -*- +# Copyright 2016 Jairo Llopis +# License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl). +from openerp import api, SUPERUSER_ID + + +def post_init_hook(cr, registry): + """Loaded after installing the module. + + ``ir.exports.line.name`` was before a char field, and now it is a computed + char field with stored values. We have to inverse it to avoid database + inconsistencies. + """ + with api.Environment.manage(): + env = api.Environment(cr, SUPERUSER_ID, {}) + env["ir.exports.line"].search([ + ("field1_id", "=", False), + ("export_id", "!=", False), + ("name", "!=", False), + ])._inverse_name() diff --git a/base_export_manager/migrations/8.0.2.1.0/post-migrate.py b/base_export_manager/migrations/8.0.2.1.0/post-migrate.py new file mode 100644 index 000000000..146b535b8 --- /dev/null +++ b/base_export_manager/migrations/8.0.2.1.0/post-migrate.py @@ -0,0 +1,9 @@ +# -*- coding: utf-8 -*- +# Copyright 2016 Jairo Llopis +# License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl.html). +from openerp.addons.base_export_manager import post_init_hook + + +def migrate(cr, version): + """When updating, now you need the post_init_hook.""" + post_init_hook(cr, None) diff --git a/base_export_manager/models/ir_exports_line.py b/base_export_manager/models/ir_exports_line.py index d4c268099..581c39f68 100644 --- a/base_export_manager/models/ir_exports_line.py +++ b/base_export_manager/models/ir_exports_line.py @@ -30,6 +30,10 @@ class IrExportsLine(models.Model): "ir.model.fields", "Third field", domain="[('model_id', '=', model3_id)]") + field4_id = fields.Many2one( + "ir.model.fields", + "Fourth field", + domain="[('model_id', '=', model4_id)]") model1_id = fields.Many2one( "ir.model", "First model", @@ -44,6 +48,10 @@ class IrExportsLine(models.Model): "ir.model", "Third model", compute="_compute_model3_id") + model4_id = fields.Many2one( + "ir.model", + "Fourth model", + compute="_compute_model4_id") sequence = fields.Integer() label = fields.Char( compute="_compute_label") @@ -54,98 +62,117 @@ class IrExportsLine(models.Model): return self.env.context.get("default_model1_id", False) @api.multi - @api.depends("field1_id", "field2_id", "field3_id") + @api.depends("field1_id", "field2_id", "field3_id", "field4_id") def _compute_name(self): """Get the name from the selected fields.""" - for s in self: - s.name = "/".join((s.field_n(num).name - for num in range(1, 4) - if s.field_n(num))) + for one in self: + name = "/".join((one.field_n(num).name for num in range(1, 5) + if one.field_n(num))) + if name != one.name: + one.name = name @api.multi @api.depends("field1_id") def _compute_model2_id(self): """Get the related model for the second field.""" - ir_model = self.env["ir.model"] - for s in self: - s.model2_id = ( - s.field1_id.ttype and - "2" in s.field1_id.ttype and - ir_model.search([("model", "=", s.field1_id.relation)])) + IrModel = self.env["ir.model"] + for one in self: + one.model2_id = ( + one.field1_id.ttype and + "2" in one.field1_id.ttype and + IrModel.search([("model", "=", one.field1_id.relation)])) @api.multi @api.depends("field2_id") def _compute_model3_id(self): """Get the related model for the third field.""" - ir_model = self.env["ir.model"] - for s in self: - s.model3_id = ( - s.field2_id.ttype and - "2" in s.field2_id.ttype and - ir_model.search([("model", "=", s.field2_id.relation)])) + IrModel = self.env["ir.model"] + for one in self: + one.model3_id = ( + one.field2_id.ttype and + "2" in one.field2_id.ttype and + IrModel.search([("model", "=", one.field2_id.relation)])) + + @api.multi + @api.depends("field3_id") + def _compute_model4_id(self): + """Get the related model for the third field.""" + IrModel = self.env["ir.model"] + for one in self: + one.model4_id = ( + one.field3_id.ttype and + "2" in one.field3_id.ttype and + IrModel.search([("model", "=", one.field3_id.relation)])) @api.multi @api.depends('name') def _compute_label(self): """Column label in a user-friendly format and language.""" - translations = self.env["ir.translation"] - for s in self: + for one in self: parts = list() - for num in range(1, 4): - field = s.field_n(num) + for num in range(1, 5): + field = one.field_n(num) if not field: break - # Translate label if possible - parts.append( - translations.search([ - ("type", "=", "field"), - ("lang", "=", self.env.context.get("lang")), - ("name", "=", "%s,%s" % (s.model_n(num).model, - field.name)), - ]).value or - field.display_name) - s.label = ("%s (%s)" % ("/".join(parts), s.name) - if parts and s.name else False) + try: + parts.append( + one.env[one.model_n(num).model]._fields[field.name] + .get_description(one.env)["string"]) + except KeyError: + # No human-readable string available, so empty this + return + one.label = ("%s (%s)" % ("/".join(parts), one.name) + if parts and one.name else False) @api.multi def _inverse_name(self): """Get the fields from the name.""" - for s in self: - # Field names can have up to only 3 indentation levels - parts = s.name.split("/", 2) - - for num in range(1, 4): - try: - # Fail in excessive subfield level - field_name = parts[num - 1] - except IndexError: - # Remove subfield on failure - s[s.field_n(num, True)] = False - else: - model = s.model_n(num) - s[s.field_n(num, True)] = self._get_field_id( - model, field_name) + for one in self: + # Field names can have up to only 4 indentation levels + parts = one.name.split("/") + if len(parts) > 4: + raise exceptions.ValidationError( + _("It's not allowed to have more than 4 levels depth: " + "%s") % one.name) + for num in range(1, 5): + if num > len(parts): + # Empty subfield in this case + one[one.field_n(num, True)] = False + continue + field_name = parts[num - 1] + model = one.model_n(num) + # You could get to failing constraint while populating the + # fields, so we skip the uniqueness check and manually check + # the full constraint after the loop + one.with_context(skip_check=True)[one.field_n(num, True)] = ( + one._get_field_id(model, field_name)) + one._check_name() @api.multi - @api.constrains("field1_id", "field2_id", "field3_id") + @api.constrains("field1_id", "field2_id", "field3_id", "field4_id") def _check_name(self): - for rec_id in self: - if not rec_id.label: + for one in self: + if not one.label: raise exceptions.ValidationError( - _("Field '%s' does not exist") % rec_id.name) - lines = self.search([('export_id', '=', rec_id.export_id.id), - ('name', '=', rec_id.name)]) - if len(lines) > 1: - raise exceptions.ValidationError( - _("Field '%s' already exists") % rec_id.name) + _("Field '%s' does not exist") % one.name) + if not one.env.context.get('skip_check'): + lines = one.search([('export_id', '=', one.export_id.id), + ('name', '=', one.name)]) + if len(lines) > 1: + raise exceptions.ValidationError( + _("Field '%s' already exists") % one.name) - @api.model - def _install_base_export_manager(self): - """Populate ``field*_id`` fields.""" - self.search([("export_id", "=", False)]).unlink() - self.search([("field1_id", "=", False), - ("name", "!=", False)])._inverse_name() + @api.multi + @api.onchange('name') + def _onchange_name(self): + if self.name: + self._inverse_name() + else: + self.field1_id = False + self.field2_id = False + self.field3_id = False + self.field4_id = False @api.model def _get_field_id(self, model, name): @@ -157,9 +184,13 @@ class IrExportsLine(models.Model): :param str name: Technical name of the field, like ``child_ids``. """ - return self.env["ir.model.fields"].search( + field = self.env["ir.model.fields"].search( [("name", "=", name), ("model_id", "=", model.id)]) + if not field.exists(): + raise exceptions.ValidationError( + _("Field '%s' not found in model '%s'") % (name, model.model)) + return field @api.multi def field_n(self, n, only_name=False): diff --git a/base_export_manager/tests/__init__.py b/base_export_manager/tests/__init__.py index 242cd85cd..5a46641eb 100644 --- a/base_export_manager/tests/__init__.py +++ b/base_export_manager/tests/__init__.py @@ -3,4 +3,5 @@ # Copyright 2015 Antiun Ingeniería S.L. - Jairo Llopis # License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl.html). -from . import test_ir_exports, test_ir_exports_line +from . import test_ir_exports +from . import test_ir_exports_line diff --git a/base_export_manager/tests/test_ir_exports_line.py b/base_export_manager/tests/test_ir_exports_line.py index acfb3cc3a..bcfa3cb5b 100644 --- a/base_export_manager/tests/test_ir_exports_line.py +++ b/base_export_manager/tests/test_ir_exports_line.py @@ -1,5 +1,6 @@ # -*- coding: utf-8 -*- # Copyright 2015 Antiun Ingenieria S.L. - Javier Iniesta +# Copyright 2016 Pedro M. Baeza # License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl.html). from openerp.tests.common import TransactionCase @@ -7,12 +8,19 @@ from openerp.exceptions import ValidationError class TestIrExportsLineCase(TransactionCase): - def setUp(self): super(TestIrExportsLineCase, self).setUp() m_ir_exports = self.env['ir.exports'] self.export = m_ir_exports.create({'name': 'Partner Test', 'resource': 'res.partner'}) + self.partner_model = self.env['ir.model'].search( + [('model', '=', 'res.partner')]) + self.field_parent_id = self.env['ir.model.fields'].search( + [('name', '=', 'parent_id'), + ('model_id', '=', self.partner_model.id)]) + self.field_name = self.env['ir.model.fields'].search( + [('name', '=', 'name'), + ('model_id', '=', self.partner_model.id)]) def test_check_name(self): m_ir_exports_line = self.env['ir.exports.line'] @@ -34,3 +42,46 @@ class TestIrExportsLineCase(TransactionCase): with self.assertRaises(ValidationError): m_ir_exports_line.create({'name': '', 'export_id': self.export.id}) + + def test_model_default_by_context(self): + """Fields inherit the model_id by context.""" + line = self.env["ir.exports.line"].with_context( + default_model1_id=self.export.model_id.id).create({ + "name": "name", + "export_id": self.export.id, + }) + self.assertEqual(line.model1_id, self.export.model_id) + + def test_inverse_name(self): + line = self.env['ir.exports.line'].create({ + 'export_id': self.export.id, + 'name': 'parent_id/parent_id/parent_id/name', + }) + self.assertEqual(line.model1_id, self.partner_model) + self.assertEqual(line.model2_id, self.partner_model) + self.assertEqual(line.field1_id, self.field_parent_id) + self.assertEqual(line.field2_id, self.field_parent_id) + self.assertEqual(line.field3_id, self.field_parent_id) + self.assertEqual(line.field4_id, self.field_name) + + def test_compute_name(self): + line = self.env['ir.exports.line'].create({ + 'export_id': self.export.id, + 'field1_id': self.field_parent_id.id, + 'field2_id': self.field_parent_id.id, + 'field3_id': self.field_parent_id.id, + 'field4_id': self.field_name.id, + }) + self.assertEqual(line.name, 'parent_id/parent_id/parent_id/name') + + def test_write_name_same_root(self): + self.env['ir.exports.line'].create({ + 'export_id': self.export.id, + 'name': 'parent_id', + }) + line = self.env['ir.exports.line'].create({ + 'export_id': self.export.id, + 'name': 'name', + }) + # This should end without errors + line.name = 'parent_id/name' diff --git a/base_export_manager/views/ir_exports.xml b/base_export_manager/views/ir_exports.xml index 106948f7e..c252d47cd 100644 --- a/base_export_manager/views/ir_exports.xml +++ b/base_export_manager/views/ir_exports.xml @@ -47,33 +47,25 @@ + + options="{'no_open': True, 'no_create': True}"/> + attrs="{'readonly': [('model2_id', '=', False)]}" + options="{'no_open': True, 'no_create': True}"/> + attrs="{'readonly': [('model3_id', '=', False)]}" + options="{'no_open': True, 'no_create': True}"/> +