From d08b4407be3cf9b404087056a413fbdc9eecbd9d Mon Sep 17 00:00:00 2001 From: Simon Lejeune Date: Mon, 11 Sep 2017 15:55:21 +0200 Subject: [PATCH] [REF] stock: `_get_available_qty` and tracked products Suppose you have a product tracked by serial number. In your inventory, you have two quants: -1 for serial1, +1 for serial2. If you create a move for 1 product and try to run `action_assign`, it didn't work before this patch because stock.move's `_update_reserved_quantity` is guarded by a call to an unstricted `_get_available_quantity` (unstricted meaning considering all quants regardless of their lot_id/package_id/owner_id). This call resulted in an available quantity of 0, thus the stock move could not be reserved To fix this issue, we patched `_get_available_quantity` so that it rightly considers products of different lot_id as different products, i.e. it returns an available quantity of +1 in the initial scenario. Of course, we consider all quants of the same lot_id, i.e. if you have -1 for serial1 and +1 for serial2, the available quantity amounts to 0. In another scenario, let's say we have only -1 serial1 in stock, if we run `_get_available_quantity`, the method should return 0: nothing is reservable. We continue this reasonment by returning 0 everytime the available quantity is negative, tracked or not. This is probably what a method named `_get_available_quantity` should have done since the beginning. There's only a single case where we need to know the quantity even if it's negative: it's just after we move a quant to its destination location. If this quant was tracked and resulted in a negative quant (i.e. we moved something we didn't have), we try to compensate with an untracked quant. In this special case, we do not want to ignore the negative quants when calling `_get_available_quantity`. That's why we introduce a new argument to this method. We could have added a new method `_get_quantity` for clarity sake. --- addons/mrp/tests/test_unbuild.py | 8 ++--- addons/stock/models/stock_quant.py | 26 +++++++++++---- addons/stock/tests/test_move.py | 52 ++++++++++++++++++++++++------ addons/stock/tests/test_move2.py | 6 ++-- addons/stock/tests/test_quant.py | 9 ++++-- 5 files changed, 76 insertions(+), 25 deletions(-) diff --git a/addons/mrp/tests/test_unbuild.py b/addons/mrp/tests/test_unbuild.py index 1a184895c93..f035f2e31b9 100644 --- a/addons/mrp/tests/test_unbuild.py +++ b/addons/mrp/tests/test_unbuild.py @@ -113,7 +113,7 @@ class TestUnbuild(TestMrpCommon): }).action_unbuild() # Check quantity in stock after last unbuild. - self.assertEqual(self.env['stock.quant']._get_available_quantity(p_final, self.stock_location), -5, 'You should have negative quantity for final product in stock') + self.assertEqual(self.env['stock.quant']._get_available_quantity(p_final, self.stock_location, allow_negative=True), -5, 'You should have negative quantity for final product in stock') self.assertEqual(self.env['stock.quant']._get_available_quantity(p1, self.stock_location), 120, 'You should have 80 products in stock') self.assertEqual(self.env['stock.quant']._get_available_quantity(p2, self.stock_location), 10, 'You should have consumed all the 5 product in stock') @@ -194,7 +194,7 @@ class TestUnbuild(TestMrpCommon): 'product_uom_id': self.uom_unit.id, }).action_unbuild() - self.assertEqual(self.env['stock.quant']._get_available_quantity(p_final, self.stock_location, lot_id=lot), -5, 'You should have negative quantity for final product in stock') + self.assertEqual(self.env['stock.quant']._get_available_quantity(p_final, self.stock_location, lot_id=lot, allow_negative=True), -5, 'You should have negative quantity for final product in stock') self.assertEqual(self.env['stock.quant']._get_available_quantity(p1, self.stock_location), 120, 'You should have 80 products in stock') self.assertEqual(self.env['stock.quant']._get_available_quantity(p2, self.stock_location), 10, 'You should have consumed all the 5 product in stock') @@ -279,7 +279,7 @@ class TestUnbuild(TestMrpCommon): 'product_uom_id': self.uom_unit.id, }).action_unbuild() - self.assertEqual(self.env['stock.quant']._get_available_quantity(p_final, self.stock_location), -5, 'You should have negative quantity for final product in stock') + self.assertEqual(self.env['stock.quant']._get_available_quantity(p_final, self.stock_location, allow_negative=True), -5, 'You should have negative quantity for final product in stock') self.assertEqual(self.env['stock.quant']._get_available_quantity(p1, self.stock_location, lot_id=lot), 120, 'You should have 80 products in stock') self.assertEqual(self.env['stock.quant']._get_available_quantity(p2, self.stock_location), 10, 'You should have consumed all the 5 product in stock') @@ -378,7 +378,7 @@ class TestUnbuild(TestMrpCommon): 'product_uom_id': self.uom_unit.id, }).action_unbuild() - self.assertEqual(self.env['stock.quant']._get_available_quantity(p_final, self.stock_location, lot_id=lot_final), -5, 'You should have negative quantity for final product in stock') + self.assertEqual(self.env['stock.quant']._get_available_quantity(p_final, self.stock_location, lot_id=lot_final, allow_negative=True), -5, 'You should have negative quantity for final product in stock') self.assertEqual(self.env['stock.quant']._get_available_quantity(p1, self.stock_location, lot_id=lot_1), 120, 'You should have 80 products in stock') self.assertEqual(self.env['stock.quant']._get_available_quantity(p2, self.stock_location, lot_id=lot_2), 10, 'You should have consumed all the 5 product in stock') diff --git a/addons/stock/models/stock_quant.py b/addons/stock/models/stock_quant.py index 767162c1e92..26eddffce30 100644 --- a/addons/stock/models/stock_quant.py +++ b/addons/stock/models/stock_quant.py @@ -126,7 +126,7 @@ class StockQuant(models.Model): return self.search(domain, order=removal_strategy_order) @api.model - def _get_available_quantity(self, product_id, location_id, lot_id=None, package_id=None, owner_id=None, strict=False): + def _get_available_quantity(self, product_id, location_id, lot_id=None, package_id=None, owner_id=None, strict=False, allow_negative=False): """ Return the available quantity, i.e. the sum of `quantity` minus the sum of `reserved_quantity`, for the set of quants sharing the combination of `product_id, location_id` if `strict` is set to False or sharing the *exact same characteristics* @@ -146,7 +146,23 @@ class StockQuant(models.Model): """ self = self.sudo() quants = self._gather(product_id, location_id, lot_id=lot_id, package_id=package_id, owner_id=owner_id, strict=strict) - return sum(quants.mapped('quantity')) - sum(quants.mapped('reserved_quantity')) + if product_id.tracking == 'none': + available_quantity = sum(quants.mapped('quantity')) - sum(quants.mapped('reserved_quantity')) + if allow_negative: + return available_quantity + else: + return available_quantity if available_quantity >= 0.0 else 0.0 + else: + availaible_quantities = {lot_id: 0.0 for lot_id in list(set(quants.mapped('lot_id'))) + ['untracked']} + for quant in quants: + if not quant.lot_id: + availaible_quantities['untracked'] += quant.quantity - quant.reserved_quantity + else: + availaible_quantities[quant.lot_id] += quant.quantity - quant.reserved_quantity + if allow_negative: + return sum(availaible_quantities.values()) + else: + return sum([available_quantity for available_quantity in availaible_quantities.values() if available_quantity > 0]) @api.model def _update_available_quantity(self, product_id, location_id, quantity, lot_id=None, package_id=None, owner_id=None, in_date=None): @@ -206,7 +222,7 @@ class StockQuant(models.Model): 'owner_id': owner_id and owner_id.id, 'in_date': in_date, }) - return self._get_available_quantity(product_id, location_id, lot_id=lot_id, package_id=package_id, owner_id=owner_id, strict=False), fields.Datetime.from_string(in_date) + return self._get_available_quantity(product_id, location_id, lot_id=lot_id, package_id=package_id, owner_id=owner_id, strict=False, allow_negative=True), fields.Datetime.from_string(in_date) @api.model def _update_reserved_quantity(self, product_id, location_id, quantity, lot_id=None, package_id=None, owner_id=None, strict=False): @@ -223,9 +239,7 @@ class StockQuant(models.Model): """ self = self.sudo() quants = self._gather(product_id, location_id, lot_id=lot_id, package_id=package_id, owner_id=owner_id, strict=strict) - - quants_quantity = sum(quants.mapped('quantity')) - available_quantity = quants_quantity - sum(quants.mapped('reserved_quantity')) + available_quantity = self._get_available_quantity(product_id, location_id, lot_id=lot_id, package_id=package_id, owner_id=owner_id, strict=strict) if quantity > 0 and quantity > available_quantity: raise UserError(_('It is not possible to reserve more products than you have in stock.')) elif quantity < 0 and abs(quantity) > sum(quants.mapped('reserved_quantity')): diff --git a/addons/stock/tests/test_move.py b/addons/stock/tests/test_move.py index 55611dc9925..7576d068b0c 100644 --- a/addons/stock/tests/test_move.py +++ b/addons/stock/tests/test_move.py @@ -74,7 +74,8 @@ class StockMove(TransactionCase): move1.action_done() self.assertEqual(move1.state, 'done') # no quants are created in the supplier location - self.assertEqual(self.env['stock.quant']._get_available_quantity(self.product1, self.supplier_location), -100.0) + self.assertEqual(self.env['stock.quant']._get_available_quantity(self.product1, self.supplier_location), 0.0) + self.assertEqual(self.env['stock.quant']._get_available_quantity(self.product1, self.supplier_location, allow_negative=True), -100.0) self.assertEqual(self.env['stock.quant']._get_available_quantity(self.product1, self.stock_location), 100.0) self.assertEqual(len(self.env['stock.quant']._gather(self.product1, self.supplier_location)), 1.0) self.assertEqual(len(self.env['stock.quant']._gather(self.product1, self.stock_location)), 1.0) @@ -114,8 +115,10 @@ class StockMove(TransactionCase): self.assertEqual(move_line.product_qty, 0) # change reservation to 0 for done move self.assertEqual(move1.state, 'done') - # no quants are created in the supplier location - self.assertEqual(self.env['stock.quant']._get_available_quantity(self.product3, self.supplier_location), -5.0) + self.assertEqual(self.env['stock.quant']._get_available_quantity(self.product3, self.supplier_location), 0.0) + supplier_quants = self.env['stock.quant']._gather(self.product3, self.supplier_location) + self.assertEqual(sum(supplier_quants.mapped('quantity')), -5.0) + self.assertEqual(self.env['stock.quant']._get_available_quantity(self.product3, self.stock_location), 5.0) self.assertEqual(len(self.env['stock.quant']._gather(self.product3, self.supplier_location)), 1.0) quants = self.env['stock.quant']._gather(self.product3, self.stock_location) @@ -165,7 +168,9 @@ class StockMove(TransactionCase): self.assertEqual(move1.state, 'done') # Quant balance should result with 5 quant in supplier and stock - self.assertEqual(self.env['stock.quant']._get_available_quantity(self.product2, self.supplier_location), -5.0) + self.assertEqual(self.env['stock.quant']._get_available_quantity(self.product2, self.supplier_location), 0.0) + supplier_quants = self.env['stock.quant']._gather(self.product2, self.supplier_location) + self.assertEqual(sum(supplier_quants.mapped('quantity')), -5.0) self.assertEqual(self.env['stock.quant']._get_available_quantity(self.product2, self.stock_location), 5.0) self.assertEqual(len(self.env['stock.quant']._gather(self.product2, self.supplier_location)), 5.0) @@ -299,7 +304,7 @@ class StockMove(TransactionCase): self.assertEqual(len(move1.move_line_ids), 2) def test_mixed_tracking_reservation_2(self): - """ Send products tracked by lot to a customer. In your stock, there two tracked and + """ Send products tracked by lot to a customer. In your stock, there are two tracked and mulitple untracked quants. There should be as many move lines as there are quants reserved. Edit the reserve move lines to set them to new serial numbers, the reservation should stay. Validate and the final quantity in stock should be 0, not negative. @@ -315,7 +320,6 @@ class StockMove(TransactionCase): self.env['stock.quant']._update_available_quantity(self.product2, self.stock_location, 2) self.env['stock.quant']._update_available_quantity(self.product2, self.stock_location, 1, lot_id=lot1) self.env['stock.quant']._update_available_quantity(self.product2, self.stock_location, 1, lot_id=lot2) - self.assertEqual(self.env['stock.quant']._get_available_quantity(self.product2, self.stock_location), 4.0) # creation move1 = self.env['stock.move'].create({ @@ -685,6 +689,30 @@ class StockMove(TransactionCase): self.assertEqual(len(self.env['stock.quant']._gather(self.product1, self.stock_location)), 1.0) self.assertEqual(move1.availability, 50.0) + def test_availability_3(self): + lot1 = self.env['stock.production.lot'].create({ + 'name': 'lot1', + 'product_id': self.product2.id, + }) + lot2 = self.env['stock.production.lot'].create({ + 'name': 'lot2', + 'product_id': self.product2.id, + }) + self.env['stock.quant']._update_available_quantity(self.product2, self.stock_location, -1.0, lot_id=lot1) + self.env['stock.quant']._update_available_quantity(self.product2, self.stock_location, 1.0, lot_id=lot2) + move1 = self.env['stock.move'].create({ + 'name': 'test_availability_3', + 'location_id': self.stock_location.id, + 'location_dest_id': self.customer_location.id, + 'product_id': self.product2.id, + 'product_uom': self.uom_unit.id, + 'product_uom_qty': 1.0, + }) + move1.action_confirm() + move1.action_assign() + self.assertEqual(move1.state, 'assigned') + self.assertEqual(move1.reserved_availability, 1.0) + def test_unreserve_1(self): """ Check that unreserving a stock move sets the products reserved as available and set the state back to confirmed. @@ -1932,7 +1960,8 @@ class StockMove(TransactionCase): self.assertEqual(self.env['stock.quant']._get_available_quantity(self.product1, self.stock_location), 0.0) self.assertEqual(self.env['stock.quant']._get_available_quantity(self.product1, shelf1_location), 1.0) - self.assertEqual(self.env['stock.quant']._get_available_quantity(self.product1, shelf2_location), -1.0) + self.assertEqual(self.env['stock.quant']._get_available_quantity(self.product1, shelf2_location), 0.0) + self.assertEqual(self.env['stock.quant']._get_available_quantity(self.product1, shelf2_location, allow_negative=True), -1.0) def test_edit_done_move_line_7(self): """ Test that editing a done stock move line linked to an untracked product correctly and @@ -2027,8 +2056,10 @@ class StockMove(TransactionCase): # edit once done, we actually moved 2 products move1.move_line_ids.qty_done = 2 - self.assertEqual(self.env['stock.quant']._get_available_quantity(self.product1, shelf1_location), -1.0) - self.assertEqual(self.env['stock.quant']._get_available_quantity(self.product1, self.stock_location), -1.0) + self.assertEqual(self.env['stock.quant']._get_available_quantity(self.product1, shelf1_location), 0.0) + self.assertEqual(self.env['stock.quant']._get_available_quantity(self.product1, shelf1_location, allow_negative=True), -1.0) + self.assertEqual(self.env['stock.quant']._get_available_quantity(self.product1, self.stock_location), 0.0) + self.assertEqual(self.env['stock.quant']._get_available_quantity(self.product1, self.stock_location, allow_negative=True), -1.0) self.assertEqual(move1.product_uom_qty, 2.0) def test_edit_done_move_line_9(self): @@ -2243,7 +2274,8 @@ class StockMove(TransactionCase): self.assertEqual(picking.move_lines.move_line_ids.qty_done, 10.0) self.assertEqual(picking.move_lines.move_line_ids.product_qty, 0.0) # Check quants data - self.assertEqual(self.env['stock.quant']._get_available_quantity(self.product1, self.stock_location), -10.0) + self.assertEqual(self.env['stock.quant']._get_available_quantity(self.product1, self.stock_location), 0.0) + self.assertEqual(self.env['stock.quant']._get_available_quantity(self.product1, self.stock_location, allow_negative=True), -10.0) self.assertEqual(len(self.env['stock.quant']._gather(self.product1, self.stock_location)), 1.0) def test_immediate_validate_4(self): diff --git a/addons/stock/tests/test_move2.py b/addons/stock/tests/test_move2.py index 3f24334d195..f0fbd786d53 100644 --- a/addons/stock/tests/test_move2.py +++ b/addons/stock/tests/test_move2.py @@ -622,7 +622,8 @@ class TestSinglePicking(TestStockCommon): delivery_order.move_lines[0].move_line_ids[0].qty_done = 2 self.assertEqual(self.env['stock.quant']._get_available_quantity(self.productA, pack_location), 0.0) delivery_order.with_context(debug=True).do_transfer() - self.assertEqual(self.env['stock.quant']._get_available_quantity(self.productA, pack_location), -1.0) + self.assertEqual(self.env['stock.quant']._get_available_quantity(self.productA, pack_location), 0.0) + self.assertEqual(self.env['stock.quant']._get_available_quantity(self.productA, pack_location, allow_negative=True), -1.0) extra_move = delivery_order.move_lines - move1 extra_move_line = extra_move.move_line_ids[0] @@ -677,7 +678,8 @@ class TestSinglePicking(TestStockCommon): delivery_order.move_lines[0].move_line_ids[0].qty_done = 3 self.assertEqual(self.env['stock.quant']._get_available_quantity(self.productA, pack_location), 0.0) delivery_order.do_transfer() - self.assertEqual(self.env['stock.quant']._get_available_quantity(self.productA, pack_location), -2.0) + self.assertEqual(self.env['stock.quant']._get_available_quantity(self.productA, pack_location), 0.0) + self.assertEqual(self.env['stock.quant']._get_available_quantity(self.productA, pack_location, allow_negative=True), -2.0) extra_move = delivery_order.move_lines - move1 extra_move_line = extra_move.move_line_ids[0] diff --git a/addons/stock/tests/test_quant.py b/addons/stock/tests/test_quant.py index 0dd7af02db7..f28fa3c4ef2 100644 --- a/addons/stock/tests/test_quant.py +++ b/addons/stock/tests/test_quant.py @@ -131,7 +131,8 @@ class StockQuant(TransactionCase): 'quantity': 5.0, 'reserved_quantity': 0.0, }) - self.assertEqual(self.env['stock.quant']._get_available_quantity(product1, stock_location), -5.0) + self.assertEqual(self.env['stock.quant']._get_available_quantity(product1, stock_location), 0.0) + self.assertEqual(self.env['stock.quant']._get_available_quantity(product1, stock_location, allow_negative=True), -5.0) def test_get_available_quantity_7(self): """ Quantity availability with only one tracked quant in a location. @@ -153,7 +154,8 @@ class StockQuant(TransactionCase): 'reserved_quantity': 20.0, 'lot_id': lot1.id, }) - self.assertEqual(self.env['stock.quant']._get_available_quantity(product1, stock_location, lot_id=lot1), -10.0) + self.assertEqual(self.env['stock.quant']._get_available_quantity(product1, stock_location, lot_id=lot1), 0.0) + self.assertEqual(self.env['stock.quant']._get_available_quantity(product1, stock_location, lot_id=lot1, allow_negative=True), -10.0) def test_get_available_quantity_8(self): """ Quantity availability with a consumable product. @@ -282,7 +284,8 @@ class StockQuant(TransactionCase): 'type': 'product', }) self.env['stock.quant']._update_available_quantity(product1, stock_location, -1.0) - self.assertEqual(self.env['stock.quant']._get_available_quantity(product1, stock_location), -1.0) + self.assertEqual(self.env['stock.quant']._get_available_quantity(product1, stock_location), 0.0) + self.assertEqual(self.env['stock.quant']._get_available_quantity(product1, stock_location, allow_negative=True), -1.0) def test_decrease_available_quantity_2(self): """ Decrease the available quantity when multiple quants are already in a location.