From e707c810e3f893bdddb7a3abe94110b4287651db Mon Sep 17 00:00:00 2001 From: Guewen Baconnier Date: Thu, 29 Mar 2018 16:19:08 +0200 Subject: [PATCH] Fix computation of stock available unreserved The computation of the unreserved available amount using the StockQuant._get_available_quantity was wrong as soon as more than one quant was found for the same product. It can easily happen when you have sublocations and a quant in each location. The reason is that the algorithm was: 1. searching for all the quants for a given product 2. calling StockQuant._get_available_quantity for each quant 3. _get_available_quantity is an @api.model method, which itself will search for all quants for the product and the given location and children Which means that if you have these locations: Stock Stock > Bin A Stock > Bin B And these quants: 1. Product: Product A Location: Bin A Quantity: 60 Reserved: 0 2. Product: Product A Location: Bin B Quantity: 10 Reserved: 0 Instead of 70, the result was 140. (One loop for each quant, each loop recomputing the total quantity in _get_available_quantity, all summed togethed, for each new quant, an additional sum would be added). Ultimately, the _get_available_quantity method does the sum of (quantity - quantity reserved). This commit uses the same logic than the 10.0 branch, it finds the quants contextually using ProductProduct._get_domain_locations and get the available quantity as the sum of (quantity - quantity reserved). We can't really use StockQuant._get_available_quantity because this one expects a location, while here we don't necessarily know it. I removed _product_available_not_res_hook which seems to have no purpose, it does not receive the result of the computation and its own result is unused. --- stock_available_unreserved/models/product.py | 42 +++++++++---------- .../tests/test_stock_available_unreserved.py | 35 ++++++++++++++++ 2 files changed, 55 insertions(+), 22 deletions(-) diff --git a/stock_available_unreserved/models/product.py b/stock_available_unreserved/models/product.py index 867917fd9..5a400832e 100644 --- a/stock_available_unreserved/models/product.py +++ b/stock_available_unreserved/models/product.py @@ -6,6 +6,7 @@ from odoo import api, fields, models from odoo.addons import decimal_precision as dp +from odoo.tools.float_utils import float_round UNIT = dp.get_precision('Product Unit of Measure') @@ -56,16 +57,10 @@ class ProductProduct(models.Model): compute='_compute_qty_available_not_reserved', ) - @api.multi - def _product_available_not_res_hook(self, quants): - """Hook used to introduce possible variations""" - return False - @api.multi def _prepare_domain_available_not_reserved(self): domain_quant = [ ('product_id', 'in', self.ids), - ('contains_unreserved', '=', True), ] domain_quant_locations = self._get_domain_locations()[0] domain_quant.extend(domain_quant_locations) @@ -77,23 +72,26 @@ class ProductProduct(models.Model): res = {} domain_quant = self._prepare_domain_available_not_reserved() - quants = self.env['stock.quant'].with_context(lang=False).search( + quants = self.env['stock.quant'].with_context(lang=False).read_group( domain_quant, - ) - # TODO: this should probably be refactored performance-wise - for prod in self: - vals = {} - prod_quant = quants.filtered(lambda x: x.product_id == prod) - quantity = sum(prod_quant.mapped( - lambda x: x._get_available_quantity( - x.product_id, - x.location_id - ) - )) - vals['qty_available_not_res'] = quantity - res[prod.id] = vals - self._product_available_not_res_hook(quants) - + ['product_id', 'location_id', 'quantity', 'reserved_quantity'], + ['product_id', 'location_id'], + lazy=False) + product_sums = {} + for quant in quants: + # create a dictionary with the total value per products + product_sums.setdefault(quant['product_id'][0], 0.) + product_sums[quant['product_id'][0]] += ( + quant['quantity'] - quant['reserved_quantity'] + ) + for product in self.with_context(prefetch_fields=False, lang=''): + available_not_res = float_round( + product_sums.get(product.id, 0.0), + precision_rounding=product.uom_id.rounding + ) + res[product.id] = { + 'qty_available_not_res': available_not_res, + } return res @api.multi diff --git a/stock_available_unreserved/tests/test_stock_available_unreserved.py b/stock_available_unreserved/tests/test_stock_available_unreserved.py index 865139a2c..19dade613 100644 --- a/stock_available_unreserved/tests/test_stock_available_unreserved.py +++ b/stock_available_unreserved/tests/test_stock_available_unreserved.py @@ -19,6 +19,23 @@ class TestStockLogisticsWarehouse(SavepointCase): cls.stock_location = cls.env.ref('stock.stock_location_stock') cls.customer_location = cls.env.ref('stock.stock_location_customers') cls.uom_unit = cls.env.ref('product.product_uom_unit') + cls.main_company = cls.env.ref('base.main_company') + + cls.bin_a = cls.env['stock.location'].create({ + 'usage': 'internal', + 'name': 'Bin A', + 'location_id': cls.stock_location.id, + 'company_id': cls.main_company.id + }) + + cls.bin_b = cls.env['stock.location'].create({ + 'usage': 'internal', + 'name': 'Bin B', + 'location_id': cls.stock_location.id, + 'company_id': cls.main_company.id + }) + + cls.env['stock.location']._parent_store_compute() # Create product template cls.templateAB = cls.templateObj.create({ @@ -140,3 +157,21 @@ class TestStockLogisticsWarehouse(SavepointCase): self.compare_qty_available_not_res(self.templateAB, 3) self.templateAB.action_open_quants_unreserved() + + def test_more_than_one_quant(self): + self.env['stock.quant'].create( + {'location_id': self.stock_location.id, + 'company_id': self.main_company.id, + 'product_id': self.productA.id, + 'quantity': 10.0}) + self.env['stock.quant'].create( + {'location_id': self.bin_a.id, + 'company_id': self.main_company.id, + 'product_id': self.productA.id, + 'quantity': 10.0}) + self.env['stock.quant'].create( + {'location_id': self.bin_b.id, + 'company_id': self.main_company.id, + 'product_id': self.productA.id, + 'quantity': 60.0}) + self.compare_qty_available_not_res(self.productA, 80)