Merge lp:~openerp-dev/openobject-server/trunk-extended-view-inheritance-xmo into lp:openobject-server
- trunk-extended-view-inheritance-xmo
- Merge into trunk
Status: | Merged |
---|---|
Merge reported by: | Christophe Simonis (OpenERP) |
Merged at revision: | not available |
Proposed branch: | lp:~openerp-dev/openobject-server/trunk-extended-view-inheritance-xmo |
Merge into: | lp:openobject-server |
Diff against target: |
584 lines (+351/-36) 5 files modified
openerp/addons/base/ir/ir_ui_view.py (+57/-12) openerp/addons/base/tests/test_views.py (+281/-10) openerp/import_xml.rng (+8/-1) openerp/osv/orm.py (+3/-13) openerp/tools/convert.py (+2/-0) |
To merge this branch: | bzr merge lp:~openerp-dev/openobject-server/trunk-extended-view-inheritance-xmo |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Christophe Matthieu (OpenERP) | Pending | ||
Olivier Dony (Odoo) | Pending | ||
Review via email: mp+218256@code.launchpad.net |
Commit message
Description of the change
Task 8149:
Add a ``mode`` (change name?) field to views, can be either primary or extension.
If mode=primary and inherit_id is set, the inherited view is fully resolved (all inheritance applied) in its own module before resolving the current view's inheritance.
This is a change from the historical cross-model inheritance, which would take the root view (model b) and apply *only* the inherited views for the current model (a), and would ignore any inheritance dealing with the root view's model.
Olivier Dony (Odoo) (odo-openerp) wrote : | # |
- 5205. By Xavier (Open ERP)
-
[MERGE] from trunk
- 5206. By Xavier (Open ERP)
-
[FIX] typos
- 5207. By Xavier (Open ERP)
-
[FIX] forgot to add primary attribute support to <template> in schema
Xavier (Open ERP) (xmo-deactivatedaccount) wrote : | # |
> - shouldn't the import_xml.rng schema be modified to explicitly mention @primary for <template>?
Yep, forgot about it
> - minor typos:
Fixed
Xavier (Open ERP) (xmo-deactivatedaccount) wrote : | # |
> - not sure if I missed it when reading the TestViewCombined testcase, but we theoretically have a valid case when we fork a second primary view within the same model (e.g. a4 could be forked off a1 with primary=true), leading to a case similar to test_cross_
Mmm probably, but the behavior is currently undefined: does an explicit @primary create an inheritance boundary? That is, right now an in-model @primary should have no effect, should it have the effect of creating an inheritance subtree which does *not* apply to its parent? AKA should get_inheriting_
> - we do have existing views in addons that are using the old/implicit version of this hack (such as the stock.picking.
Maybe, that can probably be fixed separately, the old hack should still work I think.
Xavier (Open ERP) (xmo-deactivatedaccount) wrote : | # |
> - could we add a small @help for the new `mode` field to make its meaning crystal clear (you know, with the view_type/view_mode conspiracy ;-))
done
> - wouldn't _check_mode() be simpler to implement as a pure SQL constraint (_sql_constraints)?
and done
- 5208. By Xavier (Open ERP)
-
[IMP] add some more explanations for the behavior of the mode attribute on views
- 5209. By Xavier (Open ERP)
-
[IMP] replace check_mode python-level constraint by an SQL CHECK constraint
- 5210. By Xavier (Open ERP)
-
[IMP] make mode handling more regular
Before this commit, @mode=primary would be sorta-ignored[0] if the current
view and its parent had the same model: the current view would *still* get
applied (as an extension) when asking OpenERP for its parent. This commit
makes mode=primary views behave regularly, they are *never* applied when
asking for their parent, only when asking for them or their children.This allows "forking" views, and using extended views in some contexts without
breaking or duplicating the original view[0] there was actually a problem when asking for the current view directly,
first its parent would be resolved by applying it, then it would be
applied to resolve itself, the view would thus get applied twice (oops)
Xavier (Open ERP) (xmo-deactivatedaccount) wrote : | # |
Fixed @primary within a model
Christophe Simonis (OpenERP) (kangol) wrote : | # |
Merged in git
Preview Diff
1 | === modified file 'openerp/addons/base/ir/ir_ui_view.py' |
2 | --- openerp/addons/base/ir/ir_ui_view.py 2014-05-01 15:26:04 +0000 |
3 | +++ openerp/addons/base/ir/ir_ui_view.py 2014-05-06 14:48:27 +0000 |
4 | @@ -117,8 +117,23 @@ |
5 | 'groups_id': fields.many2many('res.groups', 'ir_ui_view_group_rel', 'view_id', 'group_id', |
6 | string='Groups', help="If this field is empty, the view applies to all users. Otherwise, the view applies to the users of those groups only."), |
7 | 'model_ids': fields.one2many('ir.model.data', 'res_id', domain=[('model','=','ir.ui.view')], auto_join=True), |
8 | + |
9 | + 'mode': fields.selection( |
10 | + [('primary', "Base view"), ('extension', "Extension View")], |
11 | + string="View inheritance mode", required=True, |
12 | + help="""Only applies if this view inherits from an other one (inherit_id is not False/Null). |
13 | + |
14 | +* if extension (default), if this view is requested the closest primary view |
15 | + is looked up (via inherit_id), then all views inheriting from it with this |
16 | + view's model are applied |
17 | +* if primary, the closest primary view is fully resolved (even if it uses a |
18 | + different model than this one), then this view's inheritance specs |
19 | + (<xpath/>) are applied, and the result is used as if it were this view's |
20 | + actual arch. |
21 | +"""), |
22 | } |
23 | _defaults = { |
24 | + 'mode': 'primary', |
25 | 'priority': 16, |
26 | } |
27 | _order = "priority,name" |
28 | @@ -167,8 +182,14 @@ |
29 | return False |
30 | return True |
31 | |
32 | + _sql_constraints = [ |
33 | + ('inheritance_mode', |
34 | + "CHECK (mode != 'extension' OR inherit_id IS NOT NULL)", |
35 | + "Invalid inheritance mode: if the mode is 'extension', the view must" |
36 | + " extend an other view"), |
37 | + ] |
38 | _constraints = [ |
39 | - (_check_xml, 'Invalid view definition', ['arch']) |
40 | + (_check_xml, 'Invalid view definition', ['arch']), |
41 | ] |
42 | |
43 | def _auto_init(self, cr, context=None): |
44 | @@ -177,6 +198,12 @@ |
45 | if not cr.fetchone(): |
46 | cr.execute('CREATE INDEX ir_ui_view_model_type_inherit_id ON ir_ui_view (model, inherit_id)') |
47 | |
48 | + def _compute_defaults(self, cr, uid, values, context=None): |
49 | + if 'inherit_id' in values: |
50 | + values.setdefault( |
51 | + 'mode', 'extension' if values['inherit_id'] else 'primary') |
52 | + return values |
53 | + |
54 | def create(self, cr, uid, values, context=None): |
55 | if 'type' not in values: |
56 | if values.get('inherit_id'): |
57 | @@ -185,10 +212,13 @@ |
58 | values['type'] = etree.fromstring(values['arch']).tag |
59 | |
60 | if not values.get('name'): |
61 | - values['name'] = "%s %s" % (values['model'], values['type']) |
62 | + values['name'] = "%s %s" % (values.get('model'), values['type']) |
63 | |
64 | self.read_template.clear_cache(self) |
65 | - return super(view, self).create(cr, uid, values, context) |
66 | + return super(view, self).create( |
67 | + cr, uid, |
68 | + self._compute_defaults(cr, uid, values, context=context), |
69 | + context=context) |
70 | |
71 | def write(self, cr, uid, ids, vals, context=None): |
72 | if not isinstance(ids, (list, tuple)): |
73 | @@ -203,7 +233,10 @@ |
74 | self.pool.get('ir.ui.view.custom').unlink(cr, uid, custom_view_ids) |
75 | |
76 | self.read_template.clear_cache(self) |
77 | - ret = super(view, self).write(cr, uid, ids, vals, context) |
78 | + ret = super(view, self).write( |
79 | + cr, uid, ids, |
80 | + self._compute_defaults(cr, uid, vals, context=context), |
81 | + context) |
82 | |
83 | # if arch is modified views become noupdatable |
84 | if 'arch' in vals and not context.get('install_mode', False): |
85 | @@ -224,7 +257,7 @@ |
86 | # default view selection |
87 | def default_view(self, cr, uid, model, view_type, context=None): |
88 | """ Fetches the default view for the provided (model, view_type) pair: |
89 | - view with no parent (inherit_id=Fase) with the lowest priority. |
90 | + primary view with the lowest priority. |
91 | |
92 | :param str model: |
93 | :param int view_type: |
94 | @@ -234,7 +267,7 @@ |
95 | domain = [ |
96 | ['model', '=', model], |
97 | ['type', '=', view_type], |
98 | - ['inherit_id', '=', False], |
99 | + ['mode', '=', 'primary'], |
100 | ] |
101 | ids = self.search(cr, uid, domain, limit=1, order='priority', context=context) |
102 | if not ids: |
103 | @@ -260,15 +293,18 @@ |
104 | |
105 | user_groups = frozenset(self.pool.get('res.users').browse(cr, 1, uid, context).groups_id) |
106 | |
107 | - check_view_ids = context and context.get('check_view_ids') or (0,) |
108 | - conditions = [['inherit_id', '=', view_id], ['model', '=', model]] |
109 | + conditions = [ |
110 | + ['inherit_id', '=', view_id], |
111 | + ['model', '=', model], |
112 | + ['mode', '=', 'extension'], |
113 | + ] |
114 | if self.pool._init: |
115 | # Module init currently in progress, only consider views from |
116 | # modules whose code is already loaded |
117 | conditions.extend([ |
118 | '|', |
119 | ['model_ids.module', 'in', tuple(self.pool._init_modules)], |
120 | - ['id', 'in', check_view_ids], |
121 | + ['id', 'in', context and context.get('check_view_ids') or (0,)], |
122 | ]) |
123 | view_ids = self.search(cr, uid, conditions, context=context) |
124 | |
125 | @@ -424,7 +460,7 @@ |
126 | if context is None: context = {} |
127 | if root_id is None: |
128 | root_id = source_id |
129 | - sql_inherit = self.pool.get('ir.ui.view').get_inheriting_views_arch(cr, uid, source_id, model, context=context) |
130 | + sql_inherit = self.pool['ir.ui.view'].get_inheriting_views_arch(cr, uid, source_id, model, context=context) |
131 | for (specs, view_id) in sql_inherit: |
132 | specs_tree = etree.fromstring(specs.encode('utf-8')) |
133 | if context.get('inherit_branding'): |
134 | @@ -447,7 +483,7 @@ |
135 | |
136 | # if view_id is not a root view, climb back to the top. |
137 | base = v = self.browse(cr, uid, view_id, context=context) |
138 | - while v.inherit_id: |
139 | + while v.mode != 'primary': |
140 | v = v.inherit_id |
141 | root_id = v.id |
142 | |
143 | @@ -457,7 +493,16 @@ |
144 | |
145 | # read the view arch |
146 | [view] = self.read(cr, uid, [root_id], fields=fields, context=context) |
147 | - arch_tree = etree.fromstring(view['arch'].encode('utf-8')) |
148 | + view_arch = etree.fromstring(view['arch'].encode('utf-8')) |
149 | + if not v.inherit_id: |
150 | + arch_tree = view_arch |
151 | + else: |
152 | + parent_view = self.read_combined( |
153 | + cr, uid, v.inherit_id.id, fields=fields, context=context) |
154 | + arch_tree = etree.fromstring(parent_view['arch']) |
155 | + self.apply_inheritance_specs( |
156 | + cr, uid, arch_tree, view_arch, parent_view['id'], context=context) |
157 | + |
158 | |
159 | if context.get('inherit_branding'): |
160 | arch_tree.attrib.update({ |
161 | |
162 | === modified file 'openerp/addons/base/tests/test_views.py' |
163 | --- openerp/addons/base/tests/test_views.py 2014-04-08 11:49:36 +0000 |
164 | +++ openerp/addons/base/tests/test_views.py 2014-05-06 14:48:27 +0000 |
165 | @@ -1,12 +1,16 @@ |
166 | # -*- encoding: utf-8 -*- |
167 | from functools import partial |
168 | +import itertools |
169 | |
170 | import unittest2 |
171 | |
172 | from lxml import etree as ET |
173 | from lxml.builder import E |
174 | |
175 | +from psycopg2 import IntegrityError |
176 | + |
177 | from openerp.tests import common |
178 | +import openerp.tools |
179 | |
180 | Field = E.field |
181 | |
182 | @@ -14,9 +18,15 @@ |
183 | def setUp(self): |
184 | super(ViewCase, self).setUp() |
185 | self.addTypeEqualityFunc(ET._Element, self.assertTreesEqual) |
186 | + self.Views = self.registry('ir.ui.view') |
187 | + |
188 | + def browse(self, id, context=None): |
189 | + return self.Views.browse(self.cr, self.uid, id, context=context) |
190 | + def create(self, value, context=None): |
191 | + return self.Views.create(self.cr, self.uid, value, context=context) |
192 | |
193 | def assertTreesEqual(self, n1, n2, msg=None): |
194 | - self.assertEqual(n1.tag, n2.tag) |
195 | + self.assertEqual(n1.tag, n2.tag, msg) |
196 | self.assertEqual((n1.text or '').strip(), (n2.text or '').strip(), msg) |
197 | self.assertEqual((n1.tail or '').strip(), (n2.tail or '').strip(), msg) |
198 | |
199 | @@ -24,8 +34,8 @@ |
200 | # equality (!?!?!?!) |
201 | self.assertEqual(dict(n1.attrib), dict(n2.attrib), msg) |
202 | |
203 | - for c1, c2 in zip(n1, n2): |
204 | - self.assertTreesEqual(c1, c2, msg) |
205 | + for c1, c2 in itertools.izip_longest(n1, n2): |
206 | + self.assertEqual(c1, c2, msg) |
207 | |
208 | |
209 | class TestNodeLocator(common.TransactionCase): |
210 | @@ -374,13 +384,6 @@ |
211 | """ Applies a sequence of modificator archs to a base view |
212 | """ |
213 | |
214 | -class TestViewCombined(ViewCase): |
215 | - """ |
216 | - Test fallback operations of View.read_combined: |
217 | - * defaults mapping |
218 | - * ? |
219 | - """ |
220 | - |
221 | class TestNoModel(ViewCase): |
222 | def test_create_view_nomodel(self): |
223 | View = self.registry('ir.ui.view') |
224 | @@ -628,6 +631,7 @@ |
225 | def _insert_view(self, **kw): |
226 | """Insert view into database via a query to passtrough validation""" |
227 | kw.pop('id', None) |
228 | + kw.setdefault('mode', 'extension' if kw.get('inherit_id') else 'primary') |
229 | |
230 | keys = sorted(kw.keys()) |
231 | fields = ','.join('"%s"' % (k.replace('"', r'\"'),) for k in keys) |
232 | @@ -804,3 +808,270 @@ |
233 | E.button(name="action_next", type="object", string="New button")), |
234 | string="Replacement title", version="7.0" |
235 | )) |
236 | + |
237 | +class ViewModeField(ViewCase): |
238 | + """ |
239 | + This should probably, eventually, be folded back into other test case |
240 | + classes, integrating the test (or not) of the mode field to regular cases |
241 | + """ |
242 | + |
243 | + def testModeImplicitValue(self): |
244 | + """ mode is auto-generated from inherit_id: |
245 | + * inherit_id -> mode=extension |
246 | + * not inherit_id -> mode=primary |
247 | + """ |
248 | + view = self.browse(self.create({ |
249 | + 'inherit_id': None, |
250 | + 'arch': '<qweb/>' |
251 | + })) |
252 | + self.assertEqual(view.mode, 'primary') |
253 | + |
254 | + view2 = self.browse(self.create({ |
255 | + 'inherit_id': view.id, |
256 | + 'arch': '<qweb/>' |
257 | + })) |
258 | + self.assertEqual(view2.mode, 'extension') |
259 | + |
260 | + @openerp.tools.mute_logger('openerp.sql_db') |
261 | + def testModeExplicit(self): |
262 | + view = self.browse(self.create({ |
263 | + 'inherit_id': None, |
264 | + 'arch': '<qweb/>' |
265 | + })) |
266 | + view2 = self.browse(self.create({ |
267 | + 'inherit_id': view.id, |
268 | + 'mode': 'primary', |
269 | + 'arch': '<qweb/>' |
270 | + })) |
271 | + self.assertEqual(view.mode, 'primary') |
272 | + |
273 | + with self.assertRaises(IntegrityError): |
274 | + self.create({ |
275 | + 'inherit_id': None, |
276 | + 'mode': 'extension', |
277 | + 'arch': '<qweb/>' |
278 | + }) |
279 | + |
280 | + @openerp.tools.mute_logger('openerp.sql_db') |
281 | + def testPurePrimaryToExtension(self): |
282 | + """ |
283 | + A primary view with inherit_id=None can't be converted to extension |
284 | + """ |
285 | + view_pure_primary = self.browse(self.create({ |
286 | + 'inherit_id': None, |
287 | + 'arch': '<qweb/>' |
288 | + })) |
289 | + with self.assertRaises(IntegrityError): |
290 | + view_pure_primary.write({'mode': 'extension'}) |
291 | + |
292 | + def testInheritPrimaryToExtension(self): |
293 | + """ |
294 | + A primary view with an inherit_id can be converted to extension |
295 | + """ |
296 | + base = self.create({'inherit_id': None, 'arch': '<qweb/>'}) |
297 | + view = self.browse(self.create({ |
298 | + 'inherit_id': base, |
299 | + 'mode': 'primary', |
300 | + 'arch': '<qweb/>' |
301 | + })) |
302 | + |
303 | + view.write({'mode': 'extension'}) |
304 | + |
305 | + def testDefaultExtensionToPrimary(self): |
306 | + """ |
307 | + An extension view can be converted to primary |
308 | + """ |
309 | + base = self.create({'inherit_id': None, 'arch': '<qweb/>'}) |
310 | + view = self.browse(self.create({ |
311 | + 'inherit_id': base, |
312 | + 'arch': '<qweb/>' |
313 | + })) |
314 | + |
315 | + view.write({'mode': 'primary'}) |
316 | + |
317 | +class TestDefaultView(ViewCase): |
318 | + def testDefaultViewBase(self): |
319 | + self.create({ |
320 | + 'inherit_id': False, |
321 | + 'priority': 10, |
322 | + 'mode': 'primary', |
323 | + 'arch': '<qweb/>', |
324 | + }) |
325 | + v2 = self.create({ |
326 | + 'inherit_id': False, |
327 | + 'priority': 1, |
328 | + 'mode': 'primary', |
329 | + 'arch': '<qweb/>', |
330 | + }) |
331 | + |
332 | + default = self.Views.default_view(self.cr, self.uid, False, 'qweb') |
333 | + self.assertEqual( |
334 | + default, v2, |
335 | + "default_view should get the view with the lowest priority for " |
336 | + "a (model, view_type) pair" |
337 | + ) |
338 | + |
339 | + def testDefaultViewPrimary(self): |
340 | + v1 = self.create({ |
341 | + 'inherit_id': False, |
342 | + 'priority': 10, |
343 | + 'mode': 'primary', |
344 | + 'arch': '<qweb/>', |
345 | + }) |
346 | + self.create({ |
347 | + 'inherit_id': False, |
348 | + 'priority': 5, |
349 | + 'mode': 'primary', |
350 | + 'arch': '<qweb/>', |
351 | + }) |
352 | + v3 = self.create({ |
353 | + 'inherit_id': v1, |
354 | + 'priority': 1, |
355 | + 'mode': 'primary', |
356 | + 'arch': '<qweb/>', |
357 | + }) |
358 | + |
359 | + default = self.Views.default_view(self.cr, self.uid, False, 'qweb') |
360 | + self.assertEqual( |
361 | + default, v3, |
362 | + "default_view should get the view with the lowest priority for " |
363 | + "a (model, view_type) pair in all the primary tables" |
364 | + ) |
365 | + |
366 | +class TestViewCombined(ViewCase): |
367 | + """ |
368 | + * When asked for a view, instead of looking for the closest parent with |
369 | + inherit_id=False look for mode=primary |
370 | + * If root.inherit_id, resolve the arch for root.inherit_id (?using which |
371 | + model?), then apply root's inheritance specs to it |
372 | + * Apply inheriting views on top |
373 | + """ |
374 | + |
375 | + def setUp(self): |
376 | + super(TestViewCombined, self).setUp() |
377 | + |
378 | + self.a1 = self.create({ |
379 | + 'model': 'a', |
380 | + 'arch': '<qweb><a1/></qweb>' |
381 | + }) |
382 | + self.a2 = self.create({ |
383 | + 'model': 'a', |
384 | + 'inherit_id': self.a1, |
385 | + 'priority': 5, |
386 | + 'arch': '<xpath expr="//a1" position="after"><a2/></xpath>' |
387 | + }) |
388 | + self.a3 = self.create({ |
389 | + 'model': 'a', |
390 | + 'inherit_id': self.a1, |
391 | + 'arch': '<xpath expr="//a1" position="after"><a3/></xpath>' |
392 | + }) |
393 | + # mode=primary should be an inheritance boundary in both direction, |
394 | + # even within a model it should not extend the parent |
395 | + self.a4 = self.create({ |
396 | + 'model': 'a', |
397 | + 'inherit_id': self.a1, |
398 | + 'mode': 'primary', |
399 | + 'arch': '<xpath expr="//a1" position="after"><a4/></xpath>', |
400 | + }) |
401 | + |
402 | + self.b1 = self.create({ |
403 | + 'model': 'b', |
404 | + 'inherit_id': self.a3, |
405 | + 'mode': 'primary', |
406 | + 'arch': '<xpath expr="//a1" position="after"><b1/></xpath>' |
407 | + }) |
408 | + self.b2 = self.create({ |
409 | + 'model': 'b', |
410 | + 'inherit_id': self.b1, |
411 | + 'arch': '<xpath expr="//a1" position="after"><b2/></xpath>' |
412 | + }) |
413 | + |
414 | + self.c1 = self.create({ |
415 | + 'model': 'c', |
416 | + 'inherit_id': self.a1, |
417 | + 'mode': 'primary', |
418 | + 'arch': '<xpath expr="//a1" position="after"><c1/></xpath>' |
419 | + }) |
420 | + self.c2 = self.create({ |
421 | + 'model': 'c', |
422 | + 'inherit_id': self.c1, |
423 | + 'priority': 5, |
424 | + 'arch': '<xpath expr="//a1" position="after"><c2/></xpath>' |
425 | + }) |
426 | + self.c3 = self.create({ |
427 | + 'model': 'c', |
428 | + 'inherit_id': self.c2, |
429 | + 'priority': 10, |
430 | + 'arch': '<xpath expr="//a1" position="after"><c3/></xpath>' |
431 | + }) |
432 | + |
433 | + self.d1 = self.create({ |
434 | + 'model': 'd', |
435 | + 'inherit_id': self.b1, |
436 | + 'mode': 'primary', |
437 | + 'arch': '<xpath expr="//a1" position="after"><d1/></xpath>' |
438 | + }) |
439 | + |
440 | + def read_combined(self, id): |
441 | + return self.Views.read_combined( |
442 | + self.cr, self.uid, |
443 | + id, ['arch'], |
444 | + context={'check_view_ids': self.Views.search(self.cr, self.uid, [])} |
445 | + ) |
446 | + |
447 | + def test_basic_read(self): |
448 | + arch = self.read_combined(self.a1)['arch'] |
449 | + self.assertEqual( |
450 | + ET.fromstring(arch), |
451 | + E.qweb( |
452 | + E.a1(), |
453 | + E.a3(), |
454 | + E.a2(), |
455 | + ), arch) |
456 | + |
457 | + def test_read_from_child(self): |
458 | + arch = self.read_combined(self.a3)['arch'] |
459 | + self.assertEqual( |
460 | + ET.fromstring(arch), |
461 | + E.qweb( |
462 | + E.a1(), |
463 | + E.a3(), |
464 | + E.a2(), |
465 | + ), arch) |
466 | + |
467 | + def test_read_from_child_primary(self): |
468 | + arch = self.read_combined(self.a4)['arch'] |
469 | + self.assertEqual( |
470 | + ET.fromstring(arch), |
471 | + E.qweb( |
472 | + E.a1(), |
473 | + E.a4(), |
474 | + E.a3(), |
475 | + E.a2(), |
476 | + ), arch) |
477 | + |
478 | + def test_cross_model_simple(self): |
479 | + arch = self.read_combined(self.c2)['arch'] |
480 | + self.assertEqual( |
481 | + ET.fromstring(arch), |
482 | + E.qweb( |
483 | + E.a1(), |
484 | + E.c3(), |
485 | + E.c2(), |
486 | + E.c1(), |
487 | + E.a3(), |
488 | + E.a2(), |
489 | + ), arch) |
490 | + |
491 | + def test_cross_model_double(self): |
492 | + arch = self.read_combined(self.d1)['arch'] |
493 | + self.assertEqual( |
494 | + ET.fromstring(arch), |
495 | + E.qweb( |
496 | + E.a1(), |
497 | + E.d1(), |
498 | + E.b2(), |
499 | + E.b1(), |
500 | + E.a3(), |
501 | + E.a2(), |
502 | + ), arch) |
503 | |
504 | === modified file 'openerp/import_xml.rng' |
505 | --- openerp/import_xml.rng 2014-01-16 09:17:16 +0000 |
506 | +++ openerp/import_xml.rng 2014-05-06 14:48:27 +0000 |
507 | @@ -217,7 +217,14 @@ |
508 | <rng:optional><rng:attribute name="priority"/></rng:optional> |
509 | <rng:choice> |
510 | <rng:group> |
511 | - <rng:optional><rng:attribute name="inherit_id"/></rng:optional> |
512 | + <rng:optional> |
513 | + <rng:attribute name="inherit_id"/> |
514 | + <rng:optional> |
515 | + <rng:attribute name="primary"> |
516 | + <rng:value>True</rng:value> |
517 | + </rng:attribute> |
518 | + </rng:optional> |
519 | + </rng:optional> |
520 | <rng:optional><rng:attribute name="inherit_option_id"/></rng:optional> |
521 | <rng:optional><rng:attribute name="groups"/></rng:optional> |
522 | </rng:group> |
523 | |
524 | === modified file 'openerp/osv/orm.py' |
525 | --- openerp/osv/orm.py 2014-04-16 14:34:31 +0000 |
526 | +++ openerp/osv/orm.py 2014-05-06 14:48:27 +0000 |
527 | @@ -729,7 +729,6 @@ |
528 | _all_columns = {} |
529 | |
530 | _table = None |
531 | - _invalids = set() |
532 | _log_create = False |
533 | _sql_constraints = [] |
534 | _protected = ['read', 'write', 'create', 'default_get', 'perm_read', 'unlink', 'fields_get', 'fields_view_get', 'search', 'name_get', 'distinct_field_get', 'name_search', 'copy', 'import_data', 'search_count', 'exists'] |
535 | @@ -1543,9 +1542,6 @@ |
536 | |
537 | yield dbid, xid, converted, dict(extras, record=stream.index) |
538 | |
539 | - def get_invalid_fields(self, cr, uid): |
540 | - return list(self._invalids) |
541 | - |
542 | def _validate(self, cr, uid, ids, context=None): |
543 | context = context or {} |
544 | lng = context.get('lang') |
545 | @@ -1566,12 +1562,9 @@ |
546 | # Check presence of __call__ directly instead of using |
547 | # callable() because it will be deprecated as of Python 3.0 |
548 | if hasattr(msg, '__call__'): |
549 | - tmp_msg = msg(self, cr, uid, ids, context=context) |
550 | - if isinstance(tmp_msg, tuple): |
551 | - tmp_msg, params = tmp_msg |
552 | - translated_msg = tmp_msg % params |
553 | - else: |
554 | - translated_msg = tmp_msg |
555 | + translated_msg = msg(self, cr, uid, ids, context=context) |
556 | + if isinstance(translated_msg, tuple): |
557 | + translated_msg = translated_msg[0] % translated_msg[1] |
558 | else: |
559 | translated_msg = trans._get_source(cr, uid, self._name, 'constraint', lng, msg) |
560 | if extra_error: |
561 | @@ -1579,11 +1572,8 @@ |
562 | error_msgs.append( |
563 | _("The field(s) `%s` failed against a constraint: %s") % (', '.join(fields), translated_msg) |
564 | ) |
565 | - self._invalids.update(fields) |
566 | if error_msgs: |
567 | raise except_orm('ValidateError', '\n'.join(error_msgs)) |
568 | - else: |
569 | - self._invalids.clear() |
570 | |
571 | def default_get(self, cr, uid, fields_list, context=None): |
572 | """ |
573 | |
574 | === modified file 'openerp/tools/convert.py' |
575 | --- openerp/tools/convert.py 2014-04-24 13:14:05 +0000 |
576 | +++ openerp/tools/convert.py 2014-05-06 14:48:27 +0000 |
577 | @@ -893,6 +893,8 @@ |
578 | record.append(Field(name="groups_id", eval="[(6, 0, ["+', '.join(grp_lst)+"])]")) |
579 | if el.attrib.pop('page', None) == 'True': |
580 | record.append(Field(name="page", eval="True")) |
581 | + if el.get('primary') == 'True': |
582 | + record.append(Field('primary', name='mode')) |
583 | |
584 | return self._tag_record(cr, record, data_node) |
585 |
Note: moved to ~openerp-dev namespace for runbot testing, you probably need to sync with trunk to have the necessary *Bundle stuff for latest trunk compatibility
Looks good to me, some minor remarks:
- could we add a small @help for the new `mode` field to make its meaning crystal clear (you know, with the view_type/view_mode conspiracy ;-)) model_simple( ) except not cross-model ;-) Should we test it too? {in,out} views based on stock.picking's views, shouldn't we turn this to explicit with primary=True, and get rid of the explicit view_id references where they're not needed anymore (as primary views will be selected by as default views). aryToExtension"
- shouldn't the import_xml.rng schema be modified to explicitly mention @primary for <template>?
- wouldn't _check_mode() be simpler to implement as a pure SQL constraint (_sql_constraints)?
- not sure if I missed it when reading the TestViewCombined testcase, but we theoretically have a valid case when we fork a second primary view within the same model (e.g. a4 could be forked off a1 with primary=true), leading to a case similar to test_cross_
- we do have existing views in addons that are using the old/implicit version of this hack (such as the stock.picking.
- minor typos:
+ l.223 "mode=extendion"
+ l.268 "def testInheritPirm
Thanks for the _invalids cleanup too!