[FIX] models: display_name and name_get mismatch

- display_name uses name_get and not the other way around:
name_get should not call _compute_display_name, _compute_display_name should call name_get.
The previous behaviour was not backward-compatible with the old api.
All the models redefining name_get would have 2 different behaviors between name_get and display_name.

- Do not set an inverse function to display_name:
In most cases, writing on display_name writes on _rec_name (if any, not mandatory).
If the display_name computation is redefined, we need to redefine as well the inverse method to avoid unexpected behaviour
This required to also modify tests in base_import as readonly fields are avoided.

- Remove search method on display_name:
For the same reason as for the first point, it could be good that searching on display_name use name_search (and not the other way around).
However doing this would be very inefficiant (need to do the search, without limit, extract the ids of the name_get result just to generate
a subdomain ('id', 'in', [...]). As in most cases it would anyway mean to search on the _rec_name it's better to directly do so.

- Changing label to avoid mismatch:
In view displaying the list of fields or when a match is made on the label of a field (e.g. when importing csv file,
matching is made on both label and technical name), the fact that display_name field has '
Calling it 'Display Name' will avoid most errors.

- remove display_name definition from website_forum_doc,ir_model:
These fields are doing the same thing as the display_name of the new api, we can remove them.
We need to keep the one for res.partner as it's a stored field.
This commit is contained in:
Martin Trigaux 2014-07-16 10:35:52 +02:00 committed by Olivier Dony
parent bcd9456db5
commit f138aa2608
7 changed files with 58 additions and 75 deletions

View File

@ -1005,15 +1005,18 @@ class account_invoice(models.Model):
#TODO: implement messages system #TODO: implement messages system
return True return True
@api.one @api.multi
def _compute_display_name(self): def name_get(self):
TYPES = { TYPES = {
'out_invoice': _('Invoice'), 'out_invoice': _('Invoice'),
'in_invoice': _('Supplier Invoice'), 'in_invoice': _('Supplier Invoice'),
'out_refund': _('Refund'), 'out_refund': _('Refund'),
'in_refund': _('Supplier Refund'), 'in_refund': _('Supplier Refund'),
} }
self.display_name = "%s %s" % (self.number or TYPES[self.type], self.name or '') result = []
for inv in self:
result.append((inv.id, "%s %s" % (inv.number or TYPES[inv.type], inv.name or '')))
return result
@api.model @api.model
def name_search(self, name, args=None, operator='ilike', limit=100): def name_search(self, name, args=None, operator='ilike', limit=100):

View File

@ -11,18 +11,10 @@ ID_FIELD = {
'required': False, 'required': False,
'fields': [], 'fields': [],
} }
DISPLAY_NAME_FIELD = {
'id': 'display_name',
'name': 'display_name',
'string': "Name",
'required': False,
'fields': [],
}
def make_field(name='value', string='unknown', required=False, fields=[]): def make_field(name='value', string='unknown', required=False, fields=[]):
return [ return [
ID_FIELD, ID_FIELD,
DISPLAY_NAME_FIELD,
{'id': name, 'name': name, 'string': string, 'required': required, 'fields': fields}, {'id': name, 'name': name, 'string': string, 'required': required, 'fields': fields},
] ]
@ -50,7 +42,7 @@ class test_basic_fields(BaseImportCase):
def test_readonly(self): def test_readonly(self):
""" Readonly fields should be filtered out""" """ Readonly fields should be filtered out"""
self.assertEqualFields(self.get_fields('char.readonly'), [ID_FIELD, DISPLAY_NAME_FIELD]) self.assertEqualFields(self.get_fields('char.readonly'), [ID_FIELD])
def test_readonly_states(self): def test_readonly_states(self):
""" Readonly fields with states should not be filtered out""" """ Readonly fields with states should not be filtered out"""
@ -59,12 +51,12 @@ class test_basic_fields(BaseImportCase):
def test_readonly_states_noreadonly(self): def test_readonly_states_noreadonly(self):
""" Readonly fields with states having nothing to do with """ Readonly fields with states having nothing to do with
readonly should still be filtered out""" readonly should still be filtered out"""
self.assertEqualFields(self.get_fields('char.noreadonly'), [ID_FIELD, DISPLAY_NAME_FIELD]) self.assertEqualFields(self.get_fields('char.noreadonly'), [ID_FIELD])
def test_readonly_states_stillreadonly(self): def test_readonly_states_stillreadonly(self):
""" Readonly fields with readonly states leaving them readonly """ Readonly fields with readonly states leaving them readonly
always... filtered out""" always... filtered out"""
self.assertEqualFields(self.get_fields('char.stillreadonly'), [ID_FIELD, DISPLAY_NAME_FIELD]) self.assertEqualFields(self.get_fields('char.stillreadonly'), [ID_FIELD])
def test_m2o(self): def test_m2o(self):
""" M2O fields should allow import of themselves (name_get), """ M2O fields should allow import of themselves (name_get),
@ -92,7 +84,6 @@ class test_o2m(BaseImportCase):
def test_shallow(self): def test_shallow(self):
self.assertEqualFields(self.get_fields('o2m'), make_field(fields=[ self.assertEqualFields(self.get_fields('o2m'), make_field(fields=[
ID_FIELD, ID_FIELD,
DISPLAY_NAME_FIELD,
# FIXME: should reverse field be ignored? # FIXME: should reverse field be ignored?
{'id': 'parent_id', 'name': 'parent_id', 'string': 'unknown', 'required': False, 'fields': [ {'id': 'parent_id', 'name': 'parent_id', 'string': 'unknown', 'required': False, 'fields': [
{'id': 'parent_id', 'name': 'id', 'string': 'External ID', 'required': False, 'fields': []}, {'id': 'parent_id', 'name': 'id', 'string': 'External ID', 'required': False, 'fields': []},
@ -250,7 +241,6 @@ class test_preview(TransactionCase):
# Order depends on iteration order of fields_get # Order depends on iteration order of fields_get
self.assertItemsEqual(result['fields'], [ self.assertItemsEqual(result['fields'], [
ID_FIELD, ID_FIELD,
DISPLAY_NAME_FIELD,
{'id': 'name', 'name': 'name', 'string': 'Name', 'required':False, 'fields': []}, {'id': 'name', 'name': 'name', 'string': 'Name', 'required':False, 'fields': []},
{'id': 'somevalue', 'name': 'somevalue', 'string': 'Some Value', 'required':True, 'fields': []}, {'id': 'somevalue', 'name': 'somevalue', 'string': 'Some Value', 'required':True, 'fields': []},
{'id': 'othervalue', 'name': 'othervalue', 'string': 'Other Variable', 'required':False, 'fields': []}, {'id': 'othervalue', 'name': 'othervalue', 'string': 'Other Variable', 'required':False, 'fields': []},

View File

@ -185,12 +185,15 @@ class event_event(models.Model):
for reg in self.registration_ids for reg in self.registration_ids
) )
@api.one @api.multi
@api.depends('name', 'date_begin', 'date_end') @api.depends('name', 'date_begin', 'date_end')
def _compute_display_name(self): def name_get(self):
dates = [dt.split(' ')[0] for dt in [self.date_begin, self.date_end] if dt] result = []
dates = sorted(set(dates)) for event in self:
self.display_name = '%s (%s)' % (self.name, ' - '.join(dates)) dates = [dt.split(' ')[0] for dt in [event.date_begin, event.date_end] if dt]
dates = sorted(set(dates))
result.append((event.id, '%s (%s)' % (event.name, ' - '.join(dates))))
return result
@api.one @api.one
@api.constrains('seats_max', 'seats_available') @api.constrains('seats_max', 'seats_available')

View File

@ -24,13 +24,13 @@ class Documentation(osv.Model):
res.append((record['id'], name)) res.append((record['id'], name))
return res return res
# TODO master remove me
def _name_get_fnc(self, cr, uid, ids, prop, unknow_none, context=None): def _name_get_fnc(self, cr, uid, ids, prop, unknow_none, context=None):
res = self.name_get(cr, uid, ids, context=context) res = self.name_get(cr, uid, ids, context=context)
return dict(res) return dict(res)
_columns = { _columns = {
'sequence': fields.integer('Sequence'), 'sequence': fields.integer('Sequence'),
'display_name': fields.function(_name_get_fnc, type="char", string='Full Name'),
'name': fields.char('Name', required=True, translate=True), 'name': fields.char('Name', required=True, translate=True),
'introduction': fields.html('Introduction', translate=True), 'introduction': fields.html('Introduction', translate=True),
'parent_id': fields.many2one('forum.documentation.toc', 'Parent Table Of Content', ondelete='cascade'), 'parent_id': fields.many2one('forum.documentation.toc', 'Parent Table Of Content', ondelete='cascade'),

View File

@ -132,10 +132,15 @@ class ir_model(osv.osv):
('obj_name_uniq', 'unique (model)', 'Each model must be unique!'), ('obj_name_uniq', 'unique (model)', 'Each model must be unique!'),
] ]
def _search_display_name(self, operator, value): # overridden to allow searching both on model name (model field)
# overridden to allow searching both on model name (model field) and # and model description (name field)
# model description (name field) def _name_search(self, cr, uid, name='', args=None, operator='ilike', context=None, limit=100, name_get_uid=None):
return ['|', ('model', operator, value), ('name', operator, value)] if args is None:
args = []
domain = args + ['|', ('model', operator, name), ('name', operator, name)]
return self.name_get(cr, name_get_uid or uid,
super(ir_model, self).search(cr, uid, domain, limit=limit, context=context),
context=context)
def _drop_table(self, cr, uid, ids, context=None): def _drop_table(self, cr, uid, ids, context=None):
for model in self.browse(cr, uid, ids, context): for model in self.browse(cr, uid, ids, context):
@ -806,20 +811,19 @@ class ir_model_data(osv.osv):
""" """
_name = 'ir.model.data' _name = 'ir.model.data'
_order = 'module,model,name' _order = 'module,model,name'
def _display_name_get(self, cr, uid, ids, prop, unknow_none, context=None): def name_get(self, cr, uid, ids, context=None):
result = {} result = {}
result2 = {} result2 = []
for res in self.browse(cr, uid, ids, context=context): for res in self.browse(cr, uid, ids, context=context):
if res.id: if res.id:
result.setdefault(res.model, {}) result.setdefault(res.model, {})
result[res.model][res.res_id] = res.id result[res.model][res.res_id] = res.id
result2[res.id] = False
for model in result: for model in result:
try: try:
r = dict(self.pool[model].name_get(cr, uid, result[model].keys(), context=context)) r = dict(self.pool[model].name_get(cr, uid, result[model].keys(), context=context))
for key,val in result[model].items(): for key,val in result[model].items():
result2[val] = r.get(key, False) result2.append((val, r.get(key, False)))
except: except:
# some object have no valid name_get implemented, we accept this # some object have no valid name_get implemented, we accept this
pass pass
@ -836,7 +840,6 @@ class ir_model_data(osv.osv):
help="External Key/Identifier that can be used for " help="External Key/Identifier that can be used for "
"data integration with third-party systems"), "data integration with third-party systems"),
'complete_name': fields.function(_complete_name_get, type='char', string='Complete ID'), 'complete_name': fields.function(_complete_name_get, type='char', string='Complete ID'),
'display_name': fields.function(_display_name_get, type='char', string='Record Name'),
'model': fields.char('Model Name', required=True, select=1), 'model': fields.char('Model Name', required=True, select=1),
'module': fields.char('Module', required=True, select=1), 'module': fields.char('Module', required=True, select=1),
'res_id': fields.integer('Record ID', select=1, 'res_id': fields.integer('Record ID', select=1,

View File

@ -192,7 +192,7 @@ class res_partner_bank(osv.osv):
def name_get(self, cr, uid, ids, context=None): def name_get(self, cr, uid, ids, context=None):
if not len(ids): if not len(ids):
return [] return []
bank_dicts = self.read(cr, uid, ids, context=context) bank_dicts = self.read(cr, uid, ids, self.fields_get_keys(cr, uid, context=context), context=context)
return self._prepare_name_get(cr, uid, bank_dicts, context=context) return self._prepare_name_get(cr, uid, bank_dicts, context=context)
def onchange_company_id(self, cr, uid, ids, company_id, context=None): def onchange_company_id(self, cr, uid, ids, company_id, context=None):

View File

@ -500,9 +500,8 @@ class BaseModel(object):
# this field 'id' must override any other column or field # this field 'id' must override any other column or field
cls._add_field('id', fields.Id(automatic=True)) cls._add_field('id', fields.Id(automatic=True))
add('display_name', fields.Char(string='Name', add('display_name', fields.Char(string='Display Name', automatic=True,
compute='_compute_display_name', inverse='_inverse_display_name', compute='_compute_display_name'))
search='_search_display_name', automatic=True))
if cls._log_access: if cls._log_access:
add('create_uid', fields.Many2one('res.users', string='Created by', automatic=True)) add('create_uid', fields.Many2one('res.users', string='Created by', automatic=True))
@ -1648,30 +1647,8 @@ class BaseModel(object):
@api.depends(lambda self: (self._rec_name,) if self._rec_name else ()) @api.depends(lambda self: (self._rec_name,) if self._rec_name else ())
def _compute_display_name(self): def _compute_display_name(self):
name = self._rec_name for i, got_name in enumerate(self.name_get()):
if name in self._fields: self[i].display_name = got_name[1]
convert = self._fields[name].convert_to_display_name
for record in self:
record.display_name = convert(record[name])
else:
for record in self:
record.display_name = "%s,%s" % (record._name, record.id)
def _inverse_display_name(self):
name = self._rec_name
if name in self._fields and not self._fields[name].relational:
for record in self:
record[name] = record.display_name
else:
_logger.warning("Cannot inverse field display_name on %s", self._name)
def _search_display_name(self, operator, value):
name = self._rec_name
if name in self._fields:
return [(name, operator, value)]
else:
_logger.warning("Cannot search field display_name on %s", self._name)
return [(0, '=', 1)]
@api.multi @api.multi
def name_get(self): def name_get(self):
@ -1682,11 +1659,15 @@ class BaseModel(object):
:return: list of pairs ``(id, text_repr)`` for all records :return: list of pairs ``(id, text_repr)`` for all records
""" """
result = [] result = []
for record in self: name = self._rec_name
try: if name in self._fields:
result.append((record.id, record.display_name)) convert = self._fields[name].convert_to_display_name
except MissingError: for record in self:
pass result.append((record.id, convert(record[name])))
else:
for record in self:
result.append((record.id, "%s,%s" % (record._name, record.id)))
return result return result
@api.model @api.model
@ -1702,13 +1683,12 @@ class BaseModel(object):
:rtype: tuple :rtype: tuple
:return: the :meth:`~.name_get` pair value of the created record :return: the :meth:`~.name_get` pair value of the created record
""" """
# Shortcut the inverse function of 'display_name' with self._rec_name. if self._rec_name:
# This is useful when self._rec_name is a required field: in that case, record = self.create({self._rec_name: name})
# create() creates a record without the field, and inverse display_name return record.name_get()[0]
# afterwards. else:
field_name = self._rec_name if self._rec_name else 'display_name' _logger.warning("Cannot execute name_create, no _rec_name defined on %s", self._name)
record = self.create({field_name: name}) return False
return (record.id, record.display_name)
@api.model @api.model
def name_search(self, name='', args=None, operator='ilike', limit=100): def name_search(self, name='', args=None, operator='ilike', limit=100):
@ -1734,8 +1714,10 @@ class BaseModel(object):
:return: list of pairs ``(id, text_repr)`` for all matching records. :return: list of pairs ``(id, text_repr)`` for all matching records.
""" """
args = list(args or []) args = list(args or [])
if not (name == '' and operator == 'ilike'): if not self._rec_name:
args += [('display_name', operator, name)] _logger.warning("Cannot execute name_search, no _rec_name defined on %s", self._name)
elif not (name == '' and operator == 'ilike'):
args += [(self._rec_name, operator, name)]
return self.search(args, limit=limit).name_get() return self.search(args, limit=limit).name_get()
def _name_search(self, cr, user, name='', args=None, operator='ilike', context=None, limit=100, name_get_uid=None): def _name_search(self, cr, user, name='', args=None, operator='ilike', context=None, limit=100, name_get_uid=None):
@ -1743,8 +1725,10 @@ class BaseModel(object):
# for the name_get part to solve some access rights issues # for the name_get part to solve some access rights issues
args = list(args or []) args = list(args or [])
# optimize out the default criterion of ``ilike ''`` that matches everything # optimize out the default criterion of ``ilike ''`` that matches everything
if not (name == '' and operator == 'ilike'): if not self._rec_name:
args += [('display_name', operator, name)] _logger.warning("Cannot execute name_search, no _rec_name defined on %s", self._name)
elif not (name == '' and operator == 'ilike'):
args += [(self._rec_name, operator, name)]
access_rights_uid = name_get_uid or user access_rights_uid = name_get_uid or user
ids = self._search(cr, user, args, limit=limit, context=context, access_rights_uid=access_rights_uid) ids = self._search(cr, user, args, limit=limit, context=context, access_rights_uid=access_rights_uid)
res = self.name_get(cr, access_rights_uid, ids, context) res = self.name_get(cr, access_rights_uid, ids, context)