stock function fields of stock.location compute wrong quanties

[Omar (Pexego)]

Hi,

stock_real, stock_virtual, stock_real_value and stock_virtual_value function fields of stock.location, compute wrong stock quantities if you don't pass in context the product, because in its function _product_value when obtaining the products whose get the stock, it gets products from currently location origin moves + currently location destination moves, then it will have duplicate products if in this location had stock entries and outputs of same product. Check this logic, I see this issue showing these fields in stock.locations tree view. I attach a patch.

Regards

[Amit Parik (amit-parik)]

Hello Omar,

I have checked your issue and patch.
But I did not get where you faced the problem or in which condition you have faced the wrong quantity.

So would you please provide more information on this or provide proper example on this.

Thanks and waiting for your reply!

[Omar (Pexego) (omar7r)]

Hi amit,

Please, test it: Extend stock_location tree view, with these fields: stock_real, stock_virtual, stock_real_value and stock_virtual_value, and see the result if you have several stock moves then, comprare it with stock quantities in product list view or in Stock in location view.

Regards.

[Amit Parik (amit-parik)]

Hello Omar,

Thanks for your quick reply!
I have again checked your issue and I did not find wrong stock quantities computation in stock location.

I have checked with several stock moves, then I have opened the stock location via product->"Stock by location" act_window and see that the calculated quantity which is as normal.
I have also checked with your patch and print the both the values, the only difference I have found that in your patch's method, I have not seen the duplicate product id in products_by_location. But in our code I have been see the same product id in more than 1 times.

But I did not faced the wrong value of real stock and virtual stock.

So would you please provide us the proper example or video on this.

Thanks and waiting for your reply!

[Omar (Pexego) (omar7r)]

Hi Amit,

But you mustn't test "Stock by location", you must test to add: stock_real, stock_virtual, stock_real_value and stock_virtual_value fields in stock location tree view, because in "Stock by location" adds values in context to filter quantity get method, but opening tree view with these fields, it hasn't values in context to filter quantity get method, and sum all quantities for all products in location. If you have output moves and input moves of same product in location openerp's source duplicates the quantities sum(output qty + input qty) with my patch you avoid this behavior.
In my last message I said that you compare tree quantities against "Stock by location".

Regards,

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

Hello Omar,

You are right, the current function fields on stock.location do not properly compute the values in case the product_id is not passed in the context. And your patch will fix it indeed, so we could apply it.
However these fields are really not meant to be used without the filter on product, because the computation can be very expensive. The product_id passed in the context acts as a 'flag' to enable the function fields, and will normally be passed only when accessing the 'stock by location' views of a product. See bug 777121: in trunk an explicit check was introduced to completely disable the computation when product_id is not provided.

So we can apply your patch in trunk, but it will not work for what you want to do, due to the patch for bug 777121. Instead, perhaps this could be improved by creating 2 functions: one that will compute all the time properly, and another one that calls the first one only when a product_id is passed in context. For normal views we would use the function field that works only when product_id is in context, and people who want the full (expensive!) version can use the other field.
What do you think?

PS: for 6.0 we can't change that, the policy is not to avoid changing core code if it's not necessary to use official modules. But it's the OPW team that decides for 6.0, so you can try to report it via OPW, and they could apply your patch if they think it's safe enough.

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

Agree with you Olivier, thanks for catching it and avoiding it to regress
again as it has very bad consequences in a loaded production.

On Wed, Jul 13, 2011 at 12:46 PM, Olivier Dony (OpenERP) <
<email address hidden>> wrote:

> Hello Omar,
>
> You are right, the current function fields on stock.location do not
> properly compute the values in case the product_id is not passed in the
> context. And your patch will fix it indeed, so we could apply it.
> However these fields are really not meant to be used without the filter on
> product, because the computation can be very expensive. The product_id
> passed in the context acts as a 'flag' to enable the function fields, and
> will normally be passed only when accessing the 'stock by location' views of
> a product. See bug 777121: in trunk an explicit check was introduced to
> completely disable the computation when product_id is not provided.
>
> So we can apply your patch in trunk, but it will not work for what you want
> to do, due to the patch for bug 777121. Instead, perhaps this could be
> improved by creating 2 functions: one that will compute all the time
> properly, and another one that calls the first one only when a product_id is
> passed in context. For normal views we would use the function field that
> works only when product_id is in context, and people who want the full
> (expensive!) version can use the other field.
> What do you think?
>
> PS: for 6.0 we can't change that, the policy is not to avoid changing
> core code if it's not necessary to use official modules. But it's the
> OPW team that decides for 6.0, so you can try to report it via OPW, and
> they could apply your patch if they think it's safe enough.
>
> ** Changed in: openobject-addons
> Importance: Undecided => Wishlist
>
> ** Changed in: openobject-addons
> Status: New => Confirmed
>
> ** Changed in: openobject-addons
> Assignee: (unassigned) => OpenERP R&D Addons Team 2
> (openerp-dev-addons2)
>
> --
> You received this bug notification because you are a member of OpenERP
> Drivers, which is subscribed to OpenERP Addons.
> https://bugs.launchpad.net/bugs/800301
>
> Title:
> stock function fields of stock.location compute wrong quanties
>
> Status in OpenERP Modules (addons):
> Confirmed
>
> Bug description:
> Hi,
>
> stock_real, stock_virtual, stock_real_value and stock_virtual_value
> function fields of stock.location, compute wrong stock quantities if
> you don't pass in context the product, because in its function
> _product_value when obtaining the products whose get the stock, it
> gets products from currently location origin moves + currently
> location destination moves, then it will have duplicate products if in
> this location had stock entries and outputs of same product. Check
> this logic, I see this issue showing these fields in stock.locations
> tree view. I attach a patch.
>
> Regards
>
> To manage notifications about this bug go to:
> https://bugs.launchpad.net/openobject-addons/+bug/800301/+subscriptions
>

[Omar (Pexego) (omar7r)]

Hi Olivier,

Agree with you, if in trunk this is impossible by default, it hasn't sense to apply my patch. But:
This fields without product are expensive in cost, but I add these fields in one implementation because I use mixture locations for alcohol factories, then it's important they see how many stock are in the mixture location and how alcohol there are. With trunk patch this operation will be most expensive because I will go around all products in locations and sum his quantities.

In my opinion you must apply my patch and hide by default stock fields if there isn't product in context like rvalyi say in his bug but don't remove this functionality because it can be useful in occasions.