Merge lp:~openerp-dev/openobject-server/trunk-field-completion-xmo into lp:openobject-server

Proposed by Xavier (Open ERP)
Status: Needs review
Proposed branch: lp:~openerp-dev/openobject-server/trunk-field-completion-xmo
Merge into: lp:openobject-server
Diff against target: 275 lines (+228/-0)
8 files modified
openerp/osv/orm.py (+49/-0)
openerp/tests/addons/test_complete_field/__init__.py (+2/-0)
openerp/tests/addons/test_complete_field/__openerp__.py (+13/-0)
openerp/tests/addons/test_complete_field/ir.model.access.csv (+3/-0)
openerp/tests/addons/test_complete_field/models.py (+25/-0)
openerp/tests/addons/test_complete_field/tests/__init__.py (+9/-0)
openerp/tests/addons/test_complete_field/tests/test_basic_assumptions.py (+24/-0)
openerp/tests/addons/test_complete_field/tests/test_completion.py (+103/-0)
To merge this branch: bzr merge lp:~openerp-dev/openobject-server/trunk-field-completion-xmo
Reviewer Review Type Date Requested Status
OpenERP Core Team Pending
Review via email: mp+198236@code.launchpad.net

Description of the change

Currently, m2o field autocompletion in search view uses generic name_search. Because it's generic, for related models with lots of records but few linked from current model records, most completion results are completely useless (they don't match records being filtered) and may be actively harmful and confusing (name collisions/similarity).

complete_field (currently only for m2o, may or may not be extended to other field types in the future) performs a search in the current model instead, and only then dereferences (and reduplicates) the m2o field values.

The current proposal is more of a POC, as the whole implementation is done during search + read and thus fairly inefficient, it's more of a behavioral/UI test thing.

Possible improvements:
* batch search/read in order not to pull the whole database at once (yeah, the whole database)
* reimplement logic in SQL, will require extracting part of search and read/name_get, but aside from that it should be possible to do the whole thing in a single SQL query (?) with very little massaging afterwards

To post a comment you must log in.

Unmerged revisions

5011. By Xavier (Open ERP)

[ADD] new method complete_field specifically for fetching completion of m2o fields

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'openerp/osv/orm.py'
2--- openerp/osv/orm.py 2013-11-27 15:25:37 +0000
3+++ openerp/osv/orm.py 2013-12-09 12:08:45 +0000
4@@ -2450,6 +2450,55 @@
5 res = self.name_get(cr, access_rights_uid, ids, context)
6 return res
7
8+ def complete_field(self, cr, uid, field, key,
9+ parent_domain=None, field_domain=None,
10+ operator='ilike', limit=9, context=None):
11+ """ Completes the m2o field ``field`` to match the provided value
12+ ``key``. Only returns record references linked from a record of the
13+ current model.
14+
15+ :param str field: field to complete
16+ :param str key: ilike value for the field to complete
17+ :param parent_domain: filter to apply on current model records
18+ :param field_domain: filter to apply on acceptable ``field`` records
19+ :param str operator: matching operator between ``field`` and ``key``,
20+ defaults to ``ilike``
21+ :param int limit: number of results to return, defaults to 9
22+ :returns: name_get/name_search-ish results, list of (id, name)
23+ """
24+ column = self._all_columns[field].column
25+
26+ if column._type != 'many2one':
27+ raise ValueError(
28+ "Completed field must be a many2one, got %s" % column._type)
29+ if not isinstance(column, fields.many2one):
30+ raise TypeError("Completed fields must be actual many2one, got " %
31+ type(column).__name__)
32+
33+ domain = [(field, operator, key)]
34+ if field_domain:
35+ for section in field_domain:
36+ if isinstance(section, basestring):
37+ domain.append(section)
38+ else:
39+ # prefix each field of the section with the name of the
40+ # field: (foo, =, 3) => ($field.foo, =, 3) this way field's
41+ # own values can be filtered from parent
42+ f, o, v = section
43+ domain.append(("%s.%s" % (field, f), o, v))
44+ if parent_domain:
45+ domain.extend(parent_domain)
46+
47+ ids = self.search(cr, uid, domain, context=context)
48+ records = self.read(cr, uid, ids, fields=[field], context=context)
49+
50+ s = set()
51+ return list(itertools.islice(
52+ (item for item in (record[field] for record in records)
53+ # ordered unique results
54+ if item not in s and not s.add(item)),
55+ None, limit))
56+
57 def read_string(self, cr, uid, id, langs, fields=None, context=None):
58 res = {}
59 res2 = {}
60
61=== added directory 'openerp/tests/addons/test_complete_field'
62=== added file 'openerp/tests/addons/test_complete_field/__init__.py'
63--- openerp/tests/addons/test_complete_field/__init__.py 1970-01-01 00:00:00 +0000
64+++ openerp/tests/addons/test_complete_field/__init__.py 2013-12-09 12:08:45 +0000
65@@ -0,0 +1,2 @@
66+# -*- coding: utf-8 -*-
67+import models
68
69=== added file 'openerp/tests/addons/test_complete_field/__openerp__.py'
70--- openerp/tests/addons/test_complete_field/__openerp__.py 1970-01-01 00:00:00 +0000
71+++ openerp/tests/addons/test_complete_field/__openerp__.py 2013-12-09 12:08:45 +0000
72@@ -0,0 +1,13 @@
73+# -*- coding: utf-8 -*-
74+{
75+ 'name': 'test field completion',
76+ 'version': '0.1',
77+ 'category': 'Tests',
78+ 'description': """""",
79+ 'author': 'OpenERP SA',
80+ 'maintainer': 'OpenERP SA',
81+ 'website': 'http://www.openerp.com',
82+ 'depends': ['base'],
83+ 'installable': True,
84+ 'auto_install': False,
85+}
86
87=== added file 'openerp/tests/addons/test_complete_field/ir.model.access.csv'
88--- openerp/tests/addons/test_complete_field/ir.model.access.csv 1970-01-01 00:00:00 +0000
89+++ openerp/tests/addons/test_complete_field/ir.model.access.csv 2013-12-09 12:08:45 +0000
90@@ -0,0 +1,3 @@
91+"id","name","model_id:id","group_id:id","perm_read","perm_write","perm_create","perm_unlink"
92+access_test_complete_field_model,access_test_complete_field_model,model_test_complete_field_model,,1,1,1,1
93+ access_test_complete_field_sub,access_test_complete_field_sub,model_test_complete_field_sub,,1,1,1,1
94
95=== added file 'openerp/tests/addons/test_complete_field/models.py'
96--- openerp/tests/addons/test_complete_field/models.py 1970-01-01 00:00:00 +0000
97+++ openerp/tests/addons/test_complete_field/models.py 2013-12-09 12:08:45 +0000
98@@ -0,0 +1,25 @@
99+# -*- coding: utf-8 -*-
100+from itertools import count
101+
102+from openerp.osv import orm, fields
103+
104+class TestModel(orm.Model):
105+ _name = 'test_complete_field.model'
106+ _columns = {
107+ 'boolean': fields.boolean(),
108+ 'integer': fields.integer(),
109+ 'float': fields.float(),
110+ 'char': fields.char(),
111+ 'selection': fields.selection(zip(count(), ['a', 'b', 'c'])),
112+ 'fn': fields.function(lambda *args, **kwargs: None,type='many2one'),
113+
114+ 'many2one': fields.many2one('test_complete_field.sub'),
115+ }
116+
117+class TestModelSub(orm.Model):
118+ _name = 'test_complete_field.sub'
119+
120+ _columns = {
121+ 'name': fields.char(),
122+ 'tag': fields.integer(),
123+ }
124
125=== added directory 'openerp/tests/addons/test_complete_field/tests'
126=== added file 'openerp/tests/addons/test_complete_field/tests/__init__.py'
127--- openerp/tests/addons/test_complete_field/tests/__init__.py 1970-01-01 00:00:00 +0000
128+++ openerp/tests/addons/test_complete_field/tests/__init__.py 2013-12-09 12:08:45 +0000
129@@ -0,0 +1,9 @@
130+# -*- coding: utf-8 -*-
131+
132+from . import test_basic_assumptions, test_completion
133+
134+fast_suite = []
135+checks = [
136+ test_basic_assumptions,
137+ test_completion,
138+]
139
140=== added file 'openerp/tests/addons/test_complete_field/tests/test_basic_assumptions.py'
141--- openerp/tests/addons/test_complete_field/tests/test_basic_assumptions.py 1970-01-01 00:00:00 +0000
142+++ openerp/tests/addons/test_complete_field/tests/test_basic_assumptions.py 2013-12-09 12:08:45 +0000
143@@ -0,0 +1,24 @@
144+# -*- coding: utf-8 -*-
145+
146+from openerp.tests import common
147+
148+class TestBasicAssumptions(common.TransactionCase):
149+
150+ def setUp(self):
151+ super(TestBasicAssumptions, self).setUp()
152+ self.Model = self.registry('test_complete_field.model')
153+
154+ def test_allowed_types(self):
155+ """ Only m2o fields can be completed at this point """
156+ for t in 'boolean', 'integer', 'float', 'char', 'selection':
157+ with self.assertRaises(ValueError):
158+ self.Model.complete_field(self.cr, self.uid, t, 'thing')
159+
160+
161+ def test_no_functions(self):
162+ """ uncertain how function fields are supposed to behave, forbid them """
163+ with self.assertRaises(TypeError):
164+ self.Model.complete_field(self.cr, self.uid, 'fn', 'thing')
165+
166+ def test_executing_no_fault(self):
167+ self.Model.complete_field(self.cr, self.uid, 'many2one', 'thing')
168
169=== added file 'openerp/tests/addons/test_complete_field/tests/test_completion.py'
170--- openerp/tests/addons/test_complete_field/tests/test_completion.py 1970-01-01 00:00:00 +0000
171+++ openerp/tests/addons/test_complete_field/tests/test_completion.py 2013-12-09 12:08:45 +0000
172@@ -0,0 +1,103 @@
173+# -*- coding: utf-8 -*-
174+import operator
175+
176+from openerp.tests import common
177+
178+class TestBasicCompletion(common.TransactionCase):
179+ def setUp(self):
180+ super(TestBasicCompletion, self).setUp()
181+
182+ Parent = self.registry('test_complete_field.model')
183+ Model = self.registry('test_complete_field.sub')
184+ items = [{'name': name, 'tag': tag} for name, tag in [
185+ ('foo', 1),
186+ ('bar', 1),
187+ ('baz', 2),
188+ ('qux', 3),
189+ ('quux', 4),
190+ ('corge', 4),
191+ ('grault', 4),
192+ ('garply', 4),
193+ ('waldo', 5),
194+ ('fred', 5),
195+ ('plugh', 5),
196+ ('xyzzy', 6),
197+ ('thud', 7),
198+ ]]
199+ for values in items:
200+ id = Model.create(self.cr, self.uid, values)
201+ if values['tag'] > 2:
202+ Parent.create(self.cr, self.uid, {
203+ 'many2one': id,
204+ 'boolean': bool(values['tag'] % 2),
205+ })
206+
207+ def complete(self, value, **kwargs):
208+ return self.registry('test_complete_field.model').complete_field(
209+ self.cr, self.uid, 'many2one', value, **kwargs)
210+
211+ def test_get_all(self):
212+ results = self.complete('')
213+ self.assertEqual(len(results), 9,
214+ "by default completion should provide 9 items tops")
215+
216+ results_unlimited = self.complete('', limit=None)
217+ self.assertEqual(len(results_unlimited), 10,
218+ "should only return m2os linked to a parent record")
219+
220+ def test_complete_value(self):
221+ results = self.complete('qu')
222+ self.assertEqual(map(operator.itemgetter(1), results), [
223+ 'qux', 'quux'
224+ ])
225+
226+ def test_filter_parent(self):
227+ result = self.complete('', parent_domain=[('boolean', '=', True)])
228+ self.assertEqual(map(operator.itemgetter(1), result), [
229+ 'qux', 'waldo', 'fred', 'plugh', 'thud'
230+ ])
231+
232+ def test_filter_field(self):
233+ result = self.complete('', field_domain=[('tag', '=', 5)])
234+ self.assertEqual(map(operator.itemgetter(1), result), [
235+ 'waldo', 'fred', 'plugh',
236+ ])
237+
238+ def test_filter_all_the_things(self):
239+ result = self.complete('u',
240+ field_domain=[('tag', '>', 4)],
241+ parent_domain=[('boolean', '=', True)])
242+ self.assertEqual(map(operator.itemgetter(1), result), [
243+ 'plugh', 'thud'
244+ ])
245+
246+class TestUniqueResults(common.TransactionCase):
247+ def test_redundancies(self):
248+ m2o = self.registry('test_complete_field.sub').create(self.cr, self.uid, {
249+ 'name': 'foo',
250+ 'tag': 42,
251+ })
252+ Model = self.registry('test_complete_field.model')
253+ for i in xrange(42):
254+ Model.create(self.cr, self.uid, {'many2one': m2o})
255+
256+ results = Model.complete_field(self.cr, self.uid, 'many2one', '')
257+ self.assertEqual(results, [(m2o, 'foo')])
258+
259+ def test_sufficient_results(self):
260+ """ Limiting should be performed after uniquification, not before """
261+ Sub = self.registry('test_complete_field.sub')
262+ m2o1 = Sub.create(self.cr, self.uid, {'name': 'foo', 'tag': 1})
263+ m2o2 = Sub.create(self.cr, self.uid, {'name': 'bar', 'tag': 2})
264+
265+ Model = self.registry('test_complete_field.model')
266+ for i in xrange(42):
267+ # pad with a bunch of identical objects
268+ Model.create(self.cr, self.uid, {'many2one': m2o1})
269+ Model.create(self.cr, self.uid, {'many2one': m2o2})
270+
271+ results = Model.complete_field(self.cr, self.uid, 'many2one', '')
272+ self.assertEqual(results, [
273+ (m2o1, 'foo'),
274+ (m2o2, 'bar'),
275+ ])