using _inherit and _name for a class is not modular

Bug #996816 reported by qdp (OpenERP)
156
This bug affects 28 people
Affects Status Importance Assigned to Milestone
Odoo Server (MOVED TO GITHUB)
Confirmed
Low
OpenERP Publisher's Warranty Team

Bug Description

If you define a class that use _inherit and _name, to make a copy of the parent class, you expect that any modification made by a tier module is applied to both the parent and the copy classes. But it's not the case and here are the steps to help you to figure it out:

1°) My aim is to use a copy of the stock.picking class for incoming shipments, it will allow me to simplify the code a lot (especially in the views definition). So i defined a class stock.picking.in as follow (stock/stock.py):
class stock_picking_in(osv.osv):
    _inherit= 'stock.picking'
    _name='stock.picking.in'
    _table='stock_picking' #(here the copy model is using the same table as the parent, but it's not relevant for the bug description, and the same apply if the table is different)
    ...

2°) in purchase module, i changed the one2many field on the purchase order that referred to the stock_picking table. What i need to do is to reference the class stock.picking.in, instead of the stock.picking. So i defined it as follow (purchase/purchase.py, line 176):
       'picking_ids': fields.one2many('stock.picking.in', 'purchase_id', 'Picking List', readonly=True, help="This is the list of incoming shipments that have been generated for this purchase order."),

3°) the purchase module already include some code to add the field purchase_id on the stock.picking class and table (purchase/stock.py):
 37 class stock_picking(osv.osv):
 38 _inherit = 'stock.picking'
 39 _columns = {
 40 'purchase_id': fields.many2one('purchase.order', 'Purchase Order',
 41 ondelete='set null', select=True),
 42 }

So i thought it would be ok like this... But, no it isn't. At the purchase module installation a traceback is telling me that the field purchase_id doesn't exists on the stock.picking.in model. The addition of it on stock.picking didn't propagate to my class.

This is really problematic because it means that using _inherit and _name on the same class is not modular.

*Note that, in the meanwhile this bug is solved (if it will be) i this used a workaround: i just copied the add of the purchase_id field on my new stock.picking.in class too, in this way (purchase/stock.py):
129 # Redefinition of the new field in order to update the model stock.picking.in in the orm
132 class stock_picking_in(osv.osv):
133 _inherit = 'stock.picking.in'
134 _columns = {
135 'purchase_id': fields.many2one('purchase.order', 'Purchase Order',
136 ondelete='set null', select=True),
137 }
So yeah, it works but it's not convenient. If we want to use more and more this feature we should improve its modularity. Please be kind and remove this hack[*] whenever the framework support this feature.

Thanks,
Quentin

[*]: the same applies for the sale module, of course. Lines to remove when it's fixed are in sale/stock.py
189 # Redefinition of the new field in order to update the model stock.picking.out in the orm
192 class stock_picking_out(osv.osv):
193 _inherit = 'stock.picking.out'
194 _columns = {
195 'sale_id': fields.many2one('sale.order', 'Sale Order',
196 ondelete='set null', select=True),
197 }

Tags: maintenance

Related branches

Revision history for this message
Amit Parik (amit-parik) wrote :

Hello qdp,

I have checked your scenario but I think this is not a bug it's behaviour of framework.

You have created a new table(object) through _inherit with another name (stock_picking_in) and main object is stock_picking. Then you have inherit the stock.picking object again (for adding purchase_id field) because you have to set a one2many field picking_ids on purchase order. But here you have assigned a object stock_picking_in (on one2many field). That's it's purchase_id required on stock_picking_in because after inherits the stock.picking it's never affected on stock_picking_in

The reason when mail stock.picking is loaded (stock.py) at that time purchase_id doesn't exist on stock.picking. Then we have _inherit this object with new name, So It will create a new table which consider all the field of stock.picking and add extra-field (Which you have assigned on stock.picking.in) , now both object are loaded. After that on purchase/stock.py you have inherit stock_picking main object and add purchase_id, So It will affect only stock.picking object not the stock.picking.in (which you want). That's why purchase_id doesn't found on stock.picking.in and you have to add it on stock.picking.in.

One most important thing which is the sequence (time of your table creation) of module loading. If you first inherit the stock.picking with adding a purchase_id after that you have create a new table through _inherit with another name this time you are able to see purchase_id on both object.

I am giving you a one small example, I think which is more helpfull.

Class test_test(osv.osv):
_name = test.test
 _columns = {
     'number': fields.integer(string='Number', readonly=True, help="List OF Address"),
     'name': fields.char('Name', size=64, required=True, select=True, help="unique number of the purchase order,computed automatically when the purchase order is created"),
        'test' : fields.many2one('res.users', 'Manange by', readonly=True),
     }

Revision history for this message
Amit Parik (amit-parik) wrote :

Hello qdp,

Sorry for the the inconvenience. My keyboard was stuck that's why I am not able to completed the comment.

let me continue.......... please ignore above example. I am writing here again!

I am giving you a one small example, I think which is more helpfull.

Class test_test(osv.osv):
    _name = test.test
    _columns = {
     'name': fields.char('Name', size=64),

     }

Class test_test_new(osv.osv):
    _name = test.test.new
    _inherit = 'test.test'
  _columns = {
        'number'
}

Revision history for this message
Amit Parik (amit-parik) wrote :

class test_test_new(osv.osv):
    _inherit = 'test.test'
    _name = 'test.test.new'
    _columns = {
        'addnew': fields.char('New', size=64),
        'partner' : fields.many2one('res.partner', 'Partner'),
        }
test_test_new()

class test_test(osv.osv):
 _inherit = 'test.test'
 _name = 'test.test'
 _columns = {
     'user': fields.many2one('res.user','User'),
     }
test_test()

So by suing this example if you check your print your table test_test_new then you didn't see the User field (which is the behaviour because test_test_new is loaded first then after we added a user field). If you change the seq of class like first main object then inherit main object and then load the test.test.new. You can see the User field (on test.test.new ).

*If we have update all object as you suggest then we have to check all the object list which are already inherited then we have to add this new field on all inherited object, I think which doesn't make sense. If I inherit a one object 10 times and then for all this 10 object we have to add new field.

That's I don't think we can consider this as a bug.

Please share your views

Thanks and waiting for your reply! Sorry for the inconvenience faced on comment!

Changed in openobject-server:
status: New → Incomplete
Revision history for this message
qdp (OpenERP) (qdp) wrote :

Yes Amit,

you just described the issue as i did. Imagine that this:

class test_test(osv.osv):
 _inherit = 'test.test'
 _name = 'test.test'
 _columns = {
     'user': fields.many2one('res.user','User'),
     }
test_test()

is done in another module: you can't choose the sequence of class loading without changing the dependencies of modules, and you can't know all the modules that inherit a class. You're stuck.

I'm not saying it is a blocking and important bug (so far, as it's not used often), but it's a limitation of the current framework and we surely need to fix this issue if we want to use this feature more and more[1].

Quentin
[1]: BTW, this issue is linked to this bug: https://bugs.launchpad.net/openobject-addons/+bug/995986

Revision history for this message
Amit Parik (amit-parik) wrote :

Hello Quentin (qpd),

Yes, I also agree with your reply that we need to fix this issue because currently we have faced the lots of issue due to this limitations of our framework. That's why we have to fix this type of issue from addons side. Would you please check following bugs.

1) https://bugs.launchpad.net/openobject-addons/+bug/982751
2) https://bugs.launchpad.net/openobject-addons/+bug/996838
3)https://bugs.launchpad.net/openobject-addons/+bug/985461 (same as you suggested lp:995989)

Currently I am setting this as a "Triaged" and assign to framework team they will take the appropriate decision on this issue.
Also I am sending this to framework-experts mailing list and ask their suggestion for this (which is better for us.)

As you said this is not a blocking and important bug, I am setting this as a "Low".

@Framework Experts : Would you please share your views on this.

Thanks for your valueable time!

Changed in openobject-server:
assignee: nobody → OpenERP's Framework R&D (openerp-dev-framework)
importance: Undecided → Low
status: Incomplete → Triaged
Revision history for this message
Amit Parik (amit-parik) wrote :

After the Experts suggestion and discussion we will set appropriate "Status" of this bug.

Thanks!

Revision history for this message
hifly (csnlca) wrote : Re: [Openerp-expert-framework] [Bug 996816] Re: using _inherit and _name for a class is not modular
Download full text (4.8 KiB)

Hi,

About 2:
 in purchase module, i changed the one2many field on the purchase order
that referred to the stock_picking table. What i need to do is to reference
the class stock.picking.in, instead of the stock.picking.

I think, (not sure) you should add a new class in a new module depends on
you new module named stock_ext which contains the stock.picking.in and
stock.picking.out definition.
or you add a new class in the module stock_ext directly.
in new class, you should inherit purchase_order and redefine the
picking_ids field.
and as the same, you should redefine the picking_ids field of sale_order.

You can try it.

Andy

2012/5/12 Amit Parik (OpenERP) <email address hidden>

> After the Experts suggestion and discussion we will set appropriate
> "Status" of this bug.
>
> Thanks!
>
> --
> You received this bug notification because you are a member of OpenERP
> Framework Experts, which is subscribed to the bug report.
> https://bugs.launchpad.net/bugs/996816
>
> Title:
> using _inherit and _name for a class is not modular
>
> Status in OpenERP Server:
> Triaged
>
> Bug description:
> If you define a class that use _inherit and _name, to make a copy of
> the parent class, you expect that any modification made by a tier
> module is applied to both the parent and the copy classes. But it's
> not the case and here are the steps to help you to figure it out:
>
> 1°) My aim is to use a copy of the stock.picking class for incoming
> shipments, it will allow me to simplify the code a lot (especially in the
> views definition). So i defined a class stock.picking.in as follow
> (stock/stock.py):
> class stock_picking_in(osv.osv):
> _inherit= 'stock.picking'
> _name='stock.picking.in'
> _table='stock_picking' #(here the copy model is using the same
> table as the parent, but it's not relevant for the bug description, and the
> same apply if the table is different)
> ...
>
> 2°) in purchase module, i changed the one2many field on the purchase
> order that referred to the stock_picking table. What i need to do is to
> reference the class stock.picking.in, instead of the stock.picking. So i
> defined it as follow (purchase/purchase.py, line 176):
> 'picking_ids': fields.one2many('stock.picking.in', 'purchase_id',
> 'Picking List', readonly=True, help="This is the list of incoming shipments
> that have been generated for this purchase order."),
>
> 3°) the purchase module already include some code to add the field
> purchase_id on the stock.picking class and table (purchase/stock.py):
> 37 class stock_picking(osv.osv):
> 38 _inherit = 'stock.picking'
> 39 _columns = {
> 40 'purchase_id': fields.many2one('purchase.order', 'Purchase
> Order',
> 41 ondelete='set null', select=True),
> 42 }
>
> So i thought it would be ok like this... But, no it isn't. At the
> purchase module installation a traceback is telling me that the field
> purchase_id doesn't exists on the stock.picking.in model. The addition
> of it on stock.picking didn't propagate to my class.
>
> This is really problematic because it means that using _inherit and
> _name on the same class is not modular.
>
...

Read more...

Revision history for this message
Olivier Dony (Odoo) (odo-openerp) wrote :

I agree with Quentin, this is an annoying issue affecting the flexibility and consistency of OpenERP when inheriting by prototype (using _inherit with a different _name).
As we've seen there are many bugs that come from this: all those bugs with missing columns and SQL errors that depend on the order of module installation.

We want OpenERP module installation to work regardless of the order in which unrelated modules are installed, and due to this issue we do not have the guarantee that it will work at the moment.

Amit, we have fixed some of the current bug reports on addons side, but in every case it was a dirty hack, usually by copying the column from the parent, or introducing "fake" dependencies. This is not good at all, and proves we need to find a solution at the framework level.

One possible technical solution would be for the system to refresh all "cousin" (different _name) models when a new _inheriting model is loaded, but I think that requires quite a lot of low-level hacking in the module loading code. In order to refresh/reload a given "cousin" model we'd have to go though its inheritance tree, and refresh all ancestors, possibly requiring the original Python class to be available in the process. And this would have to be done during module install/upgrade as well, in order to properly update the database schema.
So I'm not quite sure of the technical solution to adopt, but we definitely need to solve this.

Changed in openobject-server:
status: Triaged → Confirmed
Revision history for this message
Antoine (antoine-suj) wrote :

I'm wrote on this bug report because I use mail module.
And when I use it through plugin, plugin_thunderbird module, mail.thread try to retrieve all models (in message_capable_models method) inherit mail.thread with this condition :
-> if 'mail.thread' in getattr(model, '_inherit', []):

But It doesn't get all models just models...
Depend of instance order...

Revision history for this message
Alexandre Fayolle - camptocamp (alexandre-fayolle-c2c) wrote :

I reported which seems to be related to this https://bugs.launchpad.net/openobject-addons/+bug/1169998

Revision history for this message
Olivier Dony (Odoo) (odo-openerp) wrote :

FYI, there's a work-in-progress branch that addresses this issue in trunk, improving the module loading mechanics and introducing at the same time a "base.model" that can be inherited like a regular model if you want to alter the core API methods (allowing "meta" modules like audit_trail or base_action_rule to plug themselves in the ORM without having to monkey-patch or wrap the core CRUD methods)

Have a look here if you're interested: lp:~openerp-dev/openobject-server/trunk-base-model-thu

Revision history for this message
Vo Minh Thu (thu) wrote :

I didn't know about this bug report when I started the following branch, but I think it is exactly what was asked for.

https://code.launchpad.net/~openerp-dev/openobject-server/trunk-base-model-thu/+merge/159245

Revision history for this message
Stefan Rijnhart (Opener) (stefan-opener) wrote :

Removing from ocb-addons as there is no merge proposal.

no longer affects: ocb-addons
no longer affects: ocb-addons/7.0
Revision history for this message
Lionel Sausin - Initiatives/Numérigraphe (ls-initiatives) wrote :

Now that Vo Minh Thu left OpenERP SA, is there someone working on improving this?
(Not complaining, just asking).

Revision history for this message
Raphaël Valyi - http://www.akretion.com (rvalyi) wrote :

following on Github here in v8:
https://github.com/odoo/odoo/issues/1359

Changed in openobject-server:
assignee: OpenERP's Framework R&D (openerp-dev-framework) → OpenERP Publisher's Warranty Team (openerp-opw)
tags: added: maintenance
Revision history for this message
qdp (OpenERP) (qdp) wrote :

maintenance and OPW? really?

For your information, the place where we (odoo) were facing this problem is not existing anymore in v8, since the new WMS remove the extra stock.picking.XXX class.

To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Duplicates of this bug

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.