availability of prodlot check when assigning a prodlot to a move: non optimal location_id control; patch included

[Raphaël Valyi - http://www.akretion.com]

Hi,

Currently,
When selecting a prodlot_id in the move form view, when showing the many2one prodlot popup, the available stock quantity by prodlot is correctly displayed.
BUT
Once a prodlot_id chosen, there is an availability control performed, with method onchange_lot_id in stock/stock.py.
and then if you look at the SQL request performed, you'll notice that this onchange_lot_id won't look for an availability of the prodlot only inside the selected location but rather for ALL internal location.
Meaning that if you have the given prodlot is available somewhere in some internal location BUT NOT inside the currently selected location, then the check won't tell you that the product isn't really available for that specific location.

This is because that the stock_production_lot._get_stock method is called without a location_id key in its context this time, contrary to when it's called in the many2one popup.

We think that a simple patch might be apply to have the location_id passed properly from the onchange_lot_id check method to the _get_stock context argument. See attached patch.

Hope this helps,

Raphaël Valyi

[Husen Daudi (husendaudi)]

Thanks Raphaël
Fixed in trunk revision 2238

[Raphaël Valyi - http://www.akretion.com (rvalyi)]

Guys,

I don't see my patch anymore both in 5-0 branch and trunk, so I assume it's broken again.
Could you explain why this has been reverted? Thank you.

[Jay Vora (Serpent Consulting Services) (jayvora)]

Hi,
Can you please confirm if the error still appears?

I see that your patch is not patched currently.

Thanks.

[Christophe Simonis (OpenERP) (kangol)]

fixed by 2342 <email address hidden>

[Raphaël Valyi - http://www.akretion.com (rvalyi)]

Hello Christophe,

unfortunately your commit is not correct and worse it generate a new bug (regression) every time you try to make a picking or a move.

Indeed, we talked about merging the loc_id param with the context param. But we were mistaken because of wrong parameter names!!!
Indeed, the param called 'context' in stock_move#onchange_lot_id is actually a list of ids and should have been called ids instead.
Thus we get an error like "list has no method copy()" when running your code.

So instead I propose to apply my new patch over that, it revert your commit, apply my former patch which was correct and also rename the "context" arguments into "ids" in order to avoid such future mistake.

Please commit that patch as soon as possible because the new bug is really annoying I think.

Raphaël Valyi

[Fabien (Open ERP) (fp-tinyerp)]

I fixed this this morning, without having seen your patch. So it's a little bit different. See also this:
http://openerp-code.blogspot.com/2009/06/stock-stockmove-onchangelotid.html

[Raphaël Valyi - http://www.akretion.com (rvalyi)]

Fabien,

I understand your fix and I think that the right fix in absolute. So I agree
200% this should be committed... ...on trunk
Still, did you really grep to know if you were not breaking any known 5-0
subclasser?

Luckily that change is not impacting too much things this time...
Actually I think the irony is that the only public module impacted is
trunk-addons-community/product_hardware_revision
for... ... Anevia.

But that one is all right, I'm in favor of evolution rather than stagnation
and I'll make sure I correct the module to follow your API change.
Still, please in the future, think twice before changing an API on 5-0 and
if doing it, 3 greps in addons, extra-addons and community addons quicly
give you a short list of possibly broken modules.

I'm a bit harsh, but that's getting better, I'm sure your team will finally
come up with something professionnal and hopefully everybody, including Tiny
win time in the long run. I'm all in favor of dynamic languages like Python,
but it force us to much more test and rigor than static languages that won't
even compile in such API changes cases, Python will never complain before
runtime so discipline is very much required for people to trust your ERP;

Thanks for the fix.

Raphaël Valyi.

On Tue, Jun 9, 2009 at 11:59 PM, Fabien (Open ERP) <email address hidden> wrote:

> I fixed this this morning, without having seen your patch. So it's a little
> bit different. See also this:
> http://openerp-code.blogspot.com/2009/06/stock-stockmove-onchangelotid.html
>
> ** Changed in: openerp
> Status: Confirmed => Fix Released
>
> --
> availability of prodlot check when assigning a prodlot to a move: non
> optimal location_id control; patch included
> https://bugs.launchpad.net/bugs/338329
> You received this bug notification because you are a direct subscriber
> of the bug.
>
> Status in Enterprise Management Software: Fix Released
>
> Bug description:
> Hi,
>
> Currently,
> When selecting a prodlot_id in the move form view, when showing the
> many2one prodlot popup, the available stock quantity by prodlot is correctly
> displayed.
> BUT
> Once a prodlot_id chosen, there is an availability control performed, with
> method onchange_lot_id in stock/stock.py.
> and then if you look at the SQL request performed, you'll notice that this
> onchange_lot_id won't look for an availability of the prodlot only inside
> the selected location but rather for ALL internal location.
> Meaning that if you have the given prodlot is available somewhere in some
> internal location BUT NOT inside the currently selected location, then the
> check won't tell you that the product isn't really available for that
> specific location.
>
> This is because that the stock_production_lot._get_stock method is called
> without a location_id key in its context this time, contrary to when it's
> called in the many2one popup.
>
> We think that a simple patch might be apply to have the location_id passed
> properly from the onchange_lot_id check method to the _get_stock context
> argument. See attached patch.
>
> Hope this helps,
>
> Raphaël Valyi
>