[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

@@ -29,6 +29,7 @@ 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.
   * You can drag & drop to reorder the fields.
@@ -49,6 +50,8 @@ Known issues / Roadmap
 ======================
 
 * Translated labels are not used in final exported file.
+* If your database has dangling export profiles, you could get an error when
+  installing. You will have to manually clean it before.
 
 Bug Tracker
 ===========

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

base_export_manager/__openerp__.py

@@ -5,12 +5,11 @@
 {
     'name': "Manage model export profiles",
     'category': 'Personalization',
-    'version': '8.0.2.0.1',
+    'version': '8.0.2.1.0',
     'depends': [
         'web',
     ],
     'data': [
-        'data/ir_exports_data.xml',
         'views/assets.xml',
         'views/ir_exports_view.xml',
     ],
@@ -23,4 +22,5 @@
     'website': 'http://www.antiun.com',
     'license': 'AGPL-3',
     'installable': True,
+    'post_init_hook': 'post_init_hook',
 }

base_export_manager/data/ir_exports_data.xml

@@ -1,8 +0,0 @@
-<?xml version="1.0" encoding="utf-8"?>
-<openerp>
-<data>
-
-<function model="ir.exports.line" name="_install_base_export_manager"/>
-
-</data>
-</openerp>

base_export_manager/hooks.py

@@ -0,0 +1,20 @@
+# -*- coding: utf-8 -*-
+# Copyright 2016 Jairo Llopis <jairo.llopis@tecnativa.com>
+# 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()

base_export_manager/migrations/8.0.2.1.0/post-migrate.py

@@ -0,0 +1,9 @@
+# -*- coding: utf-8 -*-
+# Copyright 2016 Jairo Llopis <jairo.llopis@tecnativa.com>
+# 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)

base_export_manager/models/ir_exports_line.py

@@ -1,6 +1,7 @@
 # -*- coding: utf-8 -*-
 # © 2015 Antiun Ingeniería S.L. - Antonio Espinosa
 # Copyright 2015-2016 Jairo Llopis <jairo.llopis@tecnativa.com>
+# Copyright 2016 Pedro M. Baeza <pedro.baeza@tecnativa.com>
 # License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl.html).
 
 from openerp import models, fields, api, exceptions
@@ -13,7 +14,6 @@ class IrExportsLine(models.Model):
 
     name = fields.Char(
         required=False,
-        readonly=True,
         store=True,
         compute="_compute_name",
         inverse="_inverse_name",
@@ -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,103 +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 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()
 
-            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)
-
-    @api.one
-    @api.constrains("field1_id", "field2_id", "field3_id")
+    @api.multi
+    @api.constrains("field1_id", "field2_id", "field3_id", "field4_id")
     def _check_name(self):
-        if not self.label:
-            raise exceptions.ValidationError(
-                _("Field '%s' does not exist") % self.name)
-        lines = self.search([('export_id', '=', self.export_id.id),
-                             ('name', '=', self.name)])
-        if len(lines) > 1:
-            raise exceptions.ValidationError(
-                _("Field '%s' already exists") % self.name)
+        for one in self:
+            if not one.label:
+                raise exceptions.ValidationError(
+                    _("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()
-        lines = self.search(
-            [("field1_id", "=", False), ("name", "!=", False)])
-        for line in lines:
-            try:
-                line._inverse_name()
-            except:
-                # Prevent possible inexisting fields
-                pass
+    @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):
@@ -162,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):

base_export_manager/static/src/js/main.js

@@ -2,7 +2,7 @@
  * License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl.html). */
 
 // Check jQuery available
-if (typeof jQuery === 'undefined') { throw new Error('Requires jQuery') }
+if (typeof jQuery === 'undefined') { throw new Error('Requires jQuery'); }
 
 +function ($) {
     'use strict';
@@ -21,12 +21,12 @@ if (typeof jQuery === 'undefined') { throw new Error('Requires jQuery') }
             },
             add_field: function(field_id, string) {
                 var field_list = this.$el.find('#fields_list');
-                if (this.$el.find("#fields_list option[value='" + field_id + "']")
-                        && !this.$el.find("#fields_list option[value='" + field_id + "']").length) {
+                if (this.$el.find("#fields_list option[value='" + field_id + "']") &&
+                        !this.$el.find("#fields_list option[value='" + field_id + "']").length) {
                     field_list.append(new Option(string + ' (' + field_id + ')', field_id));
                 }
             },
         });
-    }
+    };
 
 }(jQuery);

base_export_manager/tests/__init__.py

@@ -3,4 +3,5 @@
 # © 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

base_export_manager/tests/test_ir_exports_line.py

@@ -1,5 +1,6 @@
 # -*- coding: utf-8 -*-
 # © 2015 Antiun Ingenieria S.L. - Javier Iniesta
+# Copyright 2016 Pedro M. Baeza <pedro.baeza@tecnativa.com>
 # 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'

base_export_manager/views/ir_exports_view.xml

@@ -47,33 +47,25 @@
                         <field name="model1_id" invisible="True"/>
                         <field name="model2_id" invisible="True"/>
                         <field name="model3_id" invisible="True"/>
+                        <field name="model4_id" invisible="True"/>
                         <field name="label"/>
                         <field name="name"/>
                         <field
                             name="field1_id"
                             required="True"
-                            options="{
-                                'no_open': True,
-                                'no_create': True,
-                            }"/>
+                            options="{'no_open': True, 'no_create': True}"/>
                         <field
                             name="field2_id"
-                            attrs="{
-                                'readonly': [('model2_id', '=', False)],
-                            }"
-                            options="{
-                                'no_open': True,
-                                'no_create': True,
-                            }"/>
+                            attrs="{'readonly': [('model2_id', '=', False)]}"
+                            options="{'no_open': True, 'no_create': True}"/>
                         <field
                             name="field3_id"
-                            attrs="{
-                                'readonly': [('model3_id', '=', False)],
-                            }"
-                            options="{
-                                'no_open': True,
-                                'no_create': True,
-                            }"/>
+                            attrs="{'readonly': [('model3_id', '=', False)]}"
+                            options="{'no_open': True, 'no_create': True}"/>
+                        <field
+                            name="field4_id"
+                            attrs="{'readonly': [('model3_id', '=', False)]}"
+                            options="{'no_open': True, 'no_create': True}"/>
                     </tree>
                 </field>
             </group>