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 ;-))
- 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_model_simple() except not cross-model ;-) Should we test it too?
- we do have existing views in addons that are using the old/implicit version of this hack (such as the stock.picking.{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).
- minor typos:
+ l.223 "mode=extendion"
+ l.268 "def testInheritPirmaryToExtension"
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!