[7.0] adding custom fields to stock.picking.out / stock.picking.in should either work or be prevented

Bug #1169998 reported by Alexandre Fayolle - camptocamp
70
This bug affects 12 people
Affects Status Importance Assigned to Milestone
Odoo Addons (MOVED TO GITHUB)
Fix Released
Wishlist
OpenERP R&D Addons Team 2

Bug Description

In the MP (that I approved...) https://code.launchpad.net/~openerp-dev/openobject-addons/7.0-opw-590100-ado/+merge/154917 a regression was introduced :

Before that MP, it was possible to add custom fields to stock.picking.in and stock.picking.out, and to search these models with a domain specific to this custom field.

Now, this crashes because we delegate the searching to stock.picking which does not have that specific field. The read method is also affected.

Among other side effects, the customized views using that new field will fail to load.

Steps to reproduce on runbot:

* as admin/admin, using the web interface, add a Field to stock.picking.out (e.g a boolean field called reproduce_bug)
* visit the Delivery Orders view, and create a custom search on reproduce_bug is True

Expected: no lines (because no such delivery orders have been created

Actual: Error message saying that stock.picking has no field reproduce_bug.

Server Traceback (most recent call last):
  File "/home/odoo/runbot/static/openerp-dev-7-0-7248/server/openerp/addons/web/session.py", line 90, in send
    return openerp.netsvc.dispatch_rpc(service_name, method, args)
  File "/home/odoo/runbot/static/openerp-dev-7-0-7248/server/openerp/netsvc.py", line 293, in dispatch_rpc
    result = ExportService.getService(service_name).dispatch(method, params)
  File "/home/odoo/runbot/static/openerp-dev-7-0-7248/server/openerp/service/web_services.py", line 626, in dispatch
    res = fn(db, uid, *params)
  File "/home/odoo/runbot/static/openerp-dev-7-0-7248/server/openerp/osv/osv.py", line 188, in execute_kw
    return self.execute(db, uid, obj, method, *args, **kw or {})
  File "/home/odoo/runbot/static/openerp-dev-7-0-7248/server/openerp/osv/osv.py", line 131, in wrapper
    return f(self, dbname, *args, **kwargs)
  File "/home/odoo/runbot/static/openerp-dev-7-0-7248/server/openerp/osv/osv.py", line 197, in execute
    res = self.execute_cr(cr, uid, obj, method, *args, **kw)
  File "/home/odoo/runbot/static/openerp-dev-7-0-7248/server/openerp/addons/audittrail/audittrail.py", line 514, in execute_cr
    return fct_src(cr, uid, model, method, *args, **kw)
  File "/home/odoo/runbot/static/openerp-dev-7-0-7248/server/openerp/osv/osv.py", line 185, in execute_cr
    return getattr(object, method)(cr, uid, *args, **kw)
  File "/home/odoo/runbot/static/openerp-dev-7-0-7248/server/openerp/addons/stock/stock.py", line 3005, in search
    return self.pool.get('stock.picking').search(cr, user, args, offset, limit, order, context, count)
  File "/home/odoo/runbot/static/openerp-dev-7-0-7248/server/openerp/osv/orm.py", line 2354, in search
    return self._search(cr, user, args, offset=offset, limit=limit, order=order, context=context, count=count)
  File "/home/odoo/runbot/static/openerp-dev-7-0-7248/server/openerp/osv/orm.py", line 4843, in _search
    query = self._where_calc(cr, user, args, context=context)
  File "/home/odoo/runbot/static/openerp-dev-7-0-7248/server/openerp/osv/orm.py", line 4674, in _where_calc
    e = expression.expression(cr, user, domain, self, context)
  File "/home/odoo/runbot/static/openerp-dev-7-0-7248/server/openerp/osv/expression.py", line 641, in __init__
    self.parse(cr, uid, context=context)
  File "/home/odoo/runbot/static/openerp-dev-7-0-7248/server/openerp/osv/expression.py", line 805, in parse
    raise ValueError("Invalid field %r in leaf %r" % (left, str(leaf)))
ValueError: Invalid field 'x_reproduce_bug' in leaf "<osv.ExtendedLeaf: ('x_reproduce_bug', '=', True) on stock_picking (ctx: )>"

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

Workaround: when writing custom modules, never add custom fields directly on stock.picking.in or stock.picking.out, always do so in stock.picking.

description: updated
summary: - 7.0 : crash when searching on stock.picking.out / stock.picking.in
- specific fields
+ 7.0 : delivery module is no longer installable
summary: - 7.0 : delivery module is no longer installable
+ 7.0 : .0 : crash when searching on stock.picking.out / stock.picking.in
+ specific fields
description: updated
description: updated
Revision history for this message
Alexandre Fayolle - camptocamp (alexandre-fayolle-c2c) wrote : Re: 7.0 : .0 : crash when searching on stock.picking.out / stock.picking.in specific fields

I thought the delivery module would exhibit the bug, but that module adds the new columns on stock.picking and stock.picking.out

There is also a note about this bug report https://bugs.launchpad.net/openobject-server/+bug/996816

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

Nice catch! Indeed, this is a consequence of using a rather unusual ORM trick to have these 2 "virtual" models on top of the real stock.picking model. It is related to bug 996816, but the problem here goes the other way around: propagation of new fields on the virtual models to the real model, vs real model to virtual ones in bug 996816 (hence I removed the duplicate link)

Regardless of the framework issue, the intention behind the stock.picking.* models is only to have two virtual copies of stock.picking in order to have a clear separation of menus and views, while still sharing the same database table.
So when writing custom modules it really only makes sense to add fields to stock.picking. Some official modules (a.o delivery) do inherit directly stock.picking.{in,out} on top of stock.picking but this is only a workaround for bug 996816, the goal is only to inherit stock.picking.

For custom fields added in the UI, the workaround of adding them exclusively on stock.picking is correct and the right way to go, even if that is not clear in the UI.

Now if we can find a simple fix for this issue in the stock module (without having to inherit ir.model[.fields]) let's go for it, otherwise this issue would be better dealt with at the framework level, and we have yet to see how complicated this improvement is and decide if it's worth it.

Thanks for the detailed bug report!

Changed in openobject-addons:
assignee: nobody → OpenERP R&D Addons Team 2 (openerp-dev-addons2)
importance: Undecided → Wishlist
milestone: none → 7.0
status: New → Confirmed
summary: - 7.0 : .0 : crash when searching on stock.picking.out / stock.picking.in
- specific fields
+ [7.0] adding custom fields to stock.picking.out / stock.picking.in
+ should either work or be prevented
Revision history for this message
Guewen Baconnier @ Camptocamp (gbaconnier-c2c) wrote :

I just had an issue, my concern was to add a one2many in stock.picking.out.

Attempt 1:
Added the one2many field in stock.picking.out
Added the field in an inherit of the view stock.view_picking_out_form

Result: Impossible to browse the field

Attempt 2:
Added the one2many field in stock.picking
Added the field in an inherit of the view stock.view_picking_out_form

Result: browse of the field ok, impossible to upgrade the module as the view parser fails to find the field in stock.picking.out

Attempt 3:
Added the one2many field in stock.picking AND stock.picking.out
Added the field in an inherit of the view stock.view_picking_out_form

Result: OK, though this is nasty workaround

If anyone has a better solution, I would be pleased to hear it.

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

@odo-openerp: maybe a visible notice could be added in the code of stock/stock.py. Question https://answers.launchpad.net/openobject-addons/+question/229387 shows that this is hardly known among partners and documenting this in delivery for instance is not the appropriate place at all.

tags: added: maintenance
Revision history for this message
Carlos Contreras (carlos-realsystems) wrote :

Guewen Baconnier @ Camptocamp (gbaconnier-c2c)

you need to inherit from stock.picking.out and add attribute _table="stock.picking" in that way you will see your fields on stock.picking fields available on your view.

Revision history for this message
Paulius Sladkevičius @ hbee (komsas) wrote :

@Carlos Contreras

Your suggestion don't solve inheritance issue, still Guewen Baconnier "Attempt 3" is only one nasty hack that works.

BTW, I've faced issue that function/related field don't work when you try to add it on the stock.picking.out/in without defining it on the stock.picking.

Revision history for this message
David BEAL (ak) (davidbeal) wrote :

I tested the third scenario of guewen. It works, it's ok.

But you may prefer this solution :

class StockPicking(orm.Model):
    _inherit = 'stock.picking'

    _columns = {
        'new_field': fields.many2one(
            'other.model',
            'New field'),
    }

class StockPickingOut(orm.Model):
    _inherit = 'stock.picking.out'

    def __init__(self, pool, cr):
        super(StockPickingOut, self).__init__(pool, cr)
        self._columns['new_field'] = self.pool['stock.picking']._columns['new_field']

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

Closed in 8.0, the stock.picking.* classes have been removed.

Changed in openobject-addons:
status: Confirmed → Fix Released
milestone: 7.0 → 8.0
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.