[FIX] fields: in to_column(), returning self.column is generally not correct

Consider the following example:

    class Foo(models.Model):
        _name = 'foo'
        _columns = {
            'state': fields.selection([('a', 'A')]),
        }

    class Bar(models.Model):
        _inherit = 'foo'
        state = fields.Selection(selection_add=[('b', 'B')])

The attribute 'column' of the field does not have the full selection list,
therefore the column object cannot not be reused, even a copy of it.  The
solution is to systematically recreate the column from the field's final
specification, except for function fields that have no sensible way for being
recreated.

diff --git a/openerp/addons/test_inherit/__openerp__.py b/openerp/addons/test_inherit/__openerp__.py
index 35e6990..0254d6c 100644 (file)
--- a/openerp/addons/test_inherit/__openerp__.py
+++ b/openerp/addons/test_inherit/__openerp__.py
@@ -8,7 +8,10 @@
     'maintainer': 'OpenERP SA',
     'website': 'http://www.openerp.com',
     'depends': ['base'],
-    'data': ['ir.model.access.csv'],
+    'data': [
+        'ir.model.access.csv',
+        'demo_data.xml',
+    ],
     'installable': True,
     'auto_install': False,
 }

diff --git a/openerp/addons/test_inherit/demo_data.xml b/openerp/addons/test_inherit/demo_data.xml
new file mode 100644 (file)
index 0000000..c5262ee
--- /dev/null
+++ b/openerp/addons/test_inherit/demo_data.xml
@@ -0,0 +1,23 @@
+<openerp>
+    <data>
+        <record id="mother_a" model="test.inherit.mother">
+            <field name="name">Mother A</field>
+            <field name="state">a</field>
+        </record>
+
+        <record id="mother_b" model="test.inherit.mother">
+            <field name="name">Mother B</field>
+            <field name="state">b</field>
+        </record>
+
+        <record id="mother_c" model="test.inherit.mother">
+            <field name="name">Mother C</field>
+            <field name="state">c</field>
+        </record>
+
+        <record id="mother_d" model="test.inherit.mother">
+            <field name="name">Mother D</field>
+            <field name="state">d</field>
+        </record>
+    </data>
+</openerp>

diff --git a/openerp/addons/test_inherit/models.py b/openerp/addons/test_inherit/models.py
index a02b366..0896e64 100644 (file)
--- a/openerp/addons/test_inherit/models.py
+++ b/openerp/addons/test_inherit/models.py
@@ -8,10 +8,10 @@ class mother(models.Model):
     _columns = {
         # check interoperability of field inheritance with old-style fields
         'name': osv.fields.char('Name', required=True),
+        'state': osv.fields.selection([('a', 'A'), ('b', 'B')], string='State'),
     }
 
     surname = fields.Char(compute='_compute_surname')
-    state = fields.Selection([('a', 'A'), ('b', 'B')])
 
     @api.one
     @api.depends('name')

diff --git a/openerp/addons/test_inherit/tests/test_inherit.py b/openerp/addons/test_inherit/tests/test_inherit.py
index bbd6ef0..c1b6206 100644 (file)
--- a/openerp/addons/test_inherit/tests/test_inherit.py
+++ b/openerp/addons/test_inherit/tests/test_inherit.py
@@ -43,10 +43,12 @@ class test_inherits(common.TransactionCase):
     def test_selection_extension(self):
         """ check that attribute selection_add=... extends selection on fields. """
         mother = self.env['test.inherit.mother']
-        field = mother._fields['state']
 
-        # the extra values are added
-        self.assertEqual(field.selection, [('a', 'A'), ('b', 'B'), ('c', 'C'), ('d', 'D')])
+        # the extra values are added, both in the field and the column
+        self.assertEqual(mother._fields['state'].selection,
+                         [('a', 'A'), ('b', 'B'), ('c', 'C'), ('d', 'D')])
+        self.assertEqual(mother._columns['state'].selection,
+                         [('a', 'A'), ('b', 'B'), ('c', 'C'), ('d', 'D')])
 
 
 # vim:expandtab:smartindent:tabstop=4:softtabstop=4:shiftwidth=4:

diff --git a/openerp/fields.py b/openerp/fields.py
index 3e0b2a3..e72b6db 100644 (file)
--- a/openerp/fields.py
+++ b/openerp/fields.py
@@ -592,11 +592,9 @@ class Field(object):
     def to_column(self):
         """ return a low-level field object corresponding to `self` """
         assert self.store
-        if self.column:
-            # some columns are registry-dependent, like float fields (digits);
-            # duplicate them to avoid sharing between registries
-            return copy(self.column)
 
+        # some columns are registry-dependent, like float fields (digits);
+        # duplicate them to avoid sharing between registries
         _logger.debug("Create fields._column for Field %s", self)
         args = {}
         for attr, prop in self.column_attrs:
@@ -610,6 +608,11 @@ class Field(object):
             args['relation'] = self.comodel_name
             return fields.property(**args)
 
+        if isinstance(self.column, fields.function):
+            # it is too tricky to recreate a function field, so for that case,
+            # we make a stupid (and possibly incorrect) copy of the column
+            return copy(self.column)
+
         return getattr(fields, self.type)(**args)
 
     # properties used by to_column() to create a column instance
@@ -1182,12 +1185,11 @@ class Selection(Field):
         field = self.related_field
         self.selection = lambda model: field._description_selection(model.env)
 
-    def _setup_regular(self, env):
-        super(Selection, self)._setup_regular(env)
-        # determine selection (applying extensions)
-        cls = type(env[self.model_name])
+    def set_class_name(self, cls, name):
+        super(Selection, self).set_class_name(cls, name)
+        # determine selection (applying 'selection_add' extensions)
         selection = None
-        for field in resolve_all_mro(cls, self.name, reverse=True):
+        for field in resolve_all_mro(cls, name, reverse=True):
             if isinstance(field, type(self)):
                 # We cannot use field.selection or field.selection_add here
                 # because those attributes are overridden by `set_class_name`.
@@ -1348,13 +1350,11 @@ class Many2one(_Relational):
     def __init__(self, comodel_name=None, string=None, **kwargs):
         super(Many2one, self).__init__(comodel_name=comodel_name, string=string, **kwargs)
 
-    def _setup_regular(self, env):
-        super(Many2one, self)._setup_regular(env)
-
-        # self.inverse_fields is populated by the corresponding One2many field
-
+    def set_class_name(self, cls, name):
+        super(Many2one, self).set_class_name(cls, name)
         # determine self.delegate
-        self.delegate = self.name in env[self.model_name]._inherits.values()
+        if not self.delegate:
+            self.delegate = name in cls._inherits.values()
 
     _column_ondelete = property(attrgetter('ondelete'))
     _column_auto_join = property(attrgetter('auto_join'))

diff --git a/openerp/models.py b/openerp/models.py
index 35e06db..929b634 100644 (file)
--- a/openerp/models.py
+++ b/openerp/models.py
@@ -2271,28 +2271,13 @@ class BaseModel(object):
                 if val is not False:
                     cr.execute(update_query, (ss[1](val), key))
 
-    def _check_selection_field_value(self, cr, uid, field, value, context=None):
-        """Raise except_orm if value is not among the valid values for the selection field"""
-        if self._columns[field]._type == 'reference':
-            val_model, val_id_str = value.split(',', 1)
-            val_id = False
-            try:
-                val_id = long(val_id_str)
-            except ValueError:
-                pass
-            if not val_id:
-                raise except_orm(_('ValidateError'),
-                                 _('Invalid value for reference field "%s.%s" (last part must be a non-zero integer): "%s"') % (self._table, field, value))
-            val = val_model
-        else:
-            val = value
-        if isinstance(self._columns[field].selection, (tuple, list)):
-            if val in dict(self._columns[field].selection):
-                return
-        elif val in dict(self._columns[field].selection(self, cr, uid, context=context)):
-            return
-        raise except_orm(_('ValidateError'),
-                         _('The value "%s" for the field "%s.%s" is not in the selection') % (value, self._name, field))
+    @api.model
+    def _check_selection_field_value(self, field, value):
+        """ Check whether value is among the valid values for the given
+            selection/reference field, and raise an exception if not.
+        """
+        field = self._fields[field]
+        field.convert_to_cache(value, self)
 
     def _check_removed_columns(self, cr, log=False):
         # iterate on the database columns to drop the NOT NULL constraints