Code review comment for lp:~openerp-dev/openobject-server/trunk-extended-view-inheritance-xmo

Revision history for this message
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_model_simple() except not cross-model ;-) Should we test it too?

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_views_arch only look for extension views?

> - 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).

Maybe, that can probably be fixed separately, the old hack should still work I think.

« Back to merge proposal