[trunk/7.0] multicompany: error message when creating a stock.move

[Alexandre Fayolle - camptocamp]

Steps to reproduce:

* log on a 7.0 instance on runbot as admin
* create a new user with login "test_warehouse" belonging to a child company of "you company", make him warehouse manager, let him access only that company (don't forget to change his password)
* log out, log on as test_warehouse
* go the Warehouse > internal pickings and click on create
* add a new item

What I get:

* an unexpected error message box, which I can remove (by clicking "OK") but which is *very* annoying when it shows up 50x each day

See screenshot.

This error is caused by trying to get the name of a stock location on which the user has no access.

[Alexandre Fayolle - camptocamp (alexandre-fayolle-c2c)]

Further investigation shows that the methods _default_location_destination and _default_location_source are at fault : these return a company independent location found with a xml_id and this location is related to base.main_company on which my user has no read access.

[Alexandre Fayolle - camptocamp (alexandre-fayolle-c2c)]

in #3 what I mean by "company independent" is that the location is hard coded, and does not take the user into account.

[Alexandre Fayolle - camptocamp (alexandre-fayolle-c2c)]

3rd method with same bug: on_change_move_type (also in stock.move)

[Alexandre Fayolle - camptocamp (alexandre-fayolle-c2c)]

probably same bug waiting in mrp/mrp.py

[Olivier Dony (Odoo) (odo-openerp)]

When resolving "default values" that could be company-dependent, the logic should be to perform an extra search() on any resolved ID to make sure the user can access the record. For 7.0 this has to be done manually[1], whereas in trunk ir.model.data has a new method for that purpose: check_object_refererence()[2].

In this particular case it seems we can do even better: we could perform a search() on stock.location for the kind of location that is desired, depending on the context. If more than one location is found we can preferably pick the "default value" if it is part of the results (which means the user can access it too!), otherwise we can just pick the first one. If no location of the right type is found we should not return any (i.e. `False`).

What do you think?

[1] This bugfix for example: http://bazaar.launchpad.net/~openerp/openobject-addons/7.0/revision/9069#hr_timesheet/hr_timesheet.py
[2] http://bazaar.launchpad.net/~openerp/openobject-server/trunk/view/head:/openerp/addons/base/ir/ir_model.py#L872

[Alexandre Fayolle - camptocamp (alexandre-fayolle-c2c)]

actually mrp uses get_object which does check for read permission.

[Alexandre Fayolle - camptocamp (alexandre-fayolle-c2c)]

@odo-openerp: first thing to note is that get_object_reference performs no check at all, so IMO it is wrong to use it to look for stuff with can have some ir_rules attached. So "quick and dirty patch" I'm working on for v7 uses get_object (and handles the exceptions).

This is consistent with what is done in stock for stock.warehouse (and I'm adding an extra check for the possible ValueError which can be raised in that method)

[Martin Trigaux (OpenERP) (mat-openerp)]

Hello guys,

I have merged Alexandre's patch with few modifications (some of the checks were already done in another revision).
Thanks for the patch and report

revno: 9381 [merge]
revision-id: <email address hidden>