From 8ca10a8f39c6e6b8647a695aa543f008f01ba034 Mon Sep 17 00:00:00 2001 From: "Tiffany Chang (tic)" Date: Fri, 20 Aug 2021 14:33:37 +0000 Subject: [PATCH] [FIX] stock: ensure safe UoM change for done smls We fix 2 related UoM issues: 1. Fix quant inconsistency from changing the UoM of Done stock.move.lines Steps to reproduce: - Enable "Storage Locations" setting - Create a new "Storable Product" and create a receipt for 1 unit of it - Validate the 1 unit receieved - Open "Detailed Operations" of the move and change the stock.move.line UoM to dozen. Expected result: 13 on hand Actual result: 1 on hand To fix this: - prevent users from editing the UoM after the picking is done (i.e. unless adding a new stock.move.line and not saving). - update the write on done logic so stock.move.line UoM changes are considering and will update the quant correctly (in case of RPC or direct write). 2. Prevent changing UoM of Done stock.move to prevent inconsistent field values within stock.move and confusion for users Steps to reproduce: - Complete a picking (incoming is easiest to see) with a new product (i.e. 0 qty) having 1 unit done. - Unlock picking and add a new stock.move with 1 unit done and save. - Edit the just added stock.move's UoM from Units to Dozen. - Check the quantity on hand / Done qty of stock.move after leaving and returning to form. Expected result: 13 On Hand Actual Result: 2 On Hand and the "Done" qty in the picking is 0.0083 (i.e. 1/12 of a dozen) To fix this: - prevent users from editing the UoM after the picking is done (unless adding a new stock.move and not saving) - if a Done stock.move UoM is uodated, a UserError occurs because there is no straightforward way to ensure the quant is updated correctly since is handled within the move.line (i.e. has no visibility to its move's uom change => changing only UoM and not qty done will result in no quant update) closes odoo/odoo#75823 Signed-off-by: Arnold Moyaux --- addons/stock/models/stock_move.py | 2 ++ addons/stock/models/stock_move_line.py | 8 ++++-- addons/stock/tests/test_move.py | 33 ++++++++++++++++++++++ addons/stock/views/stock_move_views.xml | 9 ++++-- addons/stock/views/stock_picking_views.xml | 4 +-- 5 files changed, 48 insertions(+), 8 deletions(-) diff --git a/addons/stock/models/stock_move.py b/addons/stock/models/stock_move.py index e71589db219c4..d53f4e637a3fd 100644 --- a/addons/stock/models/stock_move.py +++ b/addons/stock/models/stock_move.py @@ -447,6 +447,8 @@ def write(self, vals): # messages according to the state of the stock.move records. receipt_moves_to_reassign = self.env['stock.move'] move_to_recompute_state = self.env['stock.move'] + if 'product_uom' in vals and any(move.state == 'done' for move in self): + raise UserError(_('You cannot change the UoM for a stock move that has been set to \'Done\'.')) if 'product_uom_qty' in vals: move_to_unreserve = self.env['stock.move'] for move in self.filtered(lambda m: m.state not in ('done', 'draft') and m.picking_id): diff --git a/addons/stock/models/stock_move_line.py b/addons/stock/models/stock_move_line.py index 2ff92c44acff0..1542fcc974f2c 100644 --- a/addons/stock/models/stock_move_line.py +++ b/addons/stock/models/stock_move_line.py @@ -255,7 +255,8 @@ def write(self, vals): ('lot_id', 'stock.production.lot'), ('package_id', 'stock.quant.package'), ('result_package_id', 'stock.quant.package'), - ('owner_id', 'res.partner') + ('owner_id', 'res.partner'), + ('product_uom_id', 'uom.uom') ] updates = {} for key, model in triggers: @@ -316,7 +317,7 @@ def write(self, vals): mls = mls.filtered(lambda ml: not float_is_zero(ml.qty_done - vals['qty_done'], precision_rounding=ml.product_uom_id.rounding)) for ml in mls: # undo the original move line - qty_done_orig = ml.move_id.product_uom._compute_quantity(ml.qty_done, ml.move_id.product_id.uom_id, rounding_method='HALF-UP') + qty_done_orig = ml.product_uom_id._compute_quantity(ml.qty_done, ml.move_id.product_id.uom_id, rounding_method='HALF-UP') in_date = Quant._update_available_quantity(ml.product_id, ml.location_dest_id, -qty_done_orig, lot_id=ml.lot_id, package_id=ml.result_package_id, owner_id=ml.owner_id)[1] Quant._update_available_quantity(ml.product_id, ml.location_id, qty_done_orig, lot_id=ml.lot_id, @@ -331,7 +332,8 @@ def write(self, vals): package_id = updates.get('package_id', ml.package_id) result_package_id = updates.get('result_package_id', ml.result_package_id) owner_id = updates.get('owner_id', ml.owner_id) - quantity = ml.move_id.product_uom._compute_quantity(qty_done, ml.move_id.product_id.uom_id, rounding_method='HALF-UP') + product_uom_id = updates.get('product_uom_id', ml.product_uom_id) + quantity = product_uom_id._compute_quantity(qty_done, ml.move_id.product_id.uom_id, rounding_method='HALF-UP') if not ml._should_bypass_reservation(location_id): ml._free_reservation(product_id, location_id, quantity, lot_id=lot_id, package_id=package_id, owner_id=owner_id) if not float_is_zero(quantity, precision_digits=precision): diff --git a/addons/stock/tests/test_move.py b/addons/stock/tests/test_move.py index 086da647d6db8..2a0f692e01c9f 100644 --- a/addons/stock/tests/test_move.py +++ b/addons/stock/tests/test_move.py @@ -3368,6 +3368,39 @@ def test_edit_done_move_line_13(self): self.assertEqual(self.env['stock.quant']._get_available_quantity(self.product_lot, self.stock_location, lot_id=lot1, package_id=package1), 0.0) self.assertEqual(self.env['stock.quant']._get_available_quantity(self.product_lot, self.stock_location, lot_id=lot2, package_id=package1), 1.0) + def test_edit_done_move_line_14(self): + """ Test that editing a done stock move line with a different UoM from its stock move correctly + updates the quant when its qty and/or its UoM is edited. Also check that we don't allow editing + a done stock move's UoM. + """ + move1 = self.env['stock.move'].create({ + 'name': 'test_edit_moveline', + 'location_id': self.supplier_location.id, + 'location_dest_id': self.stock_location.id, + 'product_id': self.product.id, + 'product_uom': self.uom_unit.id, + 'product_uom_qty': 12.0, + }) + move1._action_confirm() + move1._action_assign() + move1.move_line_ids.product_uom_id = self.uom_dozen + move1.move_line_ids.qty_done = 1 + move1._action_done() + self.assertEqual(self.env['stock.quant']._get_available_quantity(self.product, self.stock_location), 12.0) + + move1.move_line_ids.qty_done = 2 + self.assertEqual(self.env['stock.quant']._get_available_quantity(self.product, self.stock_location), 24.0) + self.assertEqual(move1.product_uom_qty, 24.0) + self.assertEqual(move1.product_qty, 24.0) + + move1.move_line_ids.product_uom_id = self.uom_unit + self.assertEqual(self.env['stock.quant']._get_available_quantity(self.product, self.stock_location), 2.0) + self.assertEqual(move1.product_uom_qty, 2.0) + self.assertEqual(move1.product_qty, 2.0) + + with self.assertRaises(UserError): + move1.product_uom = self.uom_dozen + def test_immediate_validate_1(self): """ In a picking with a single available move, clicking on validate without filling any quantities should open a wizard asking to process all the reservation (so, the whole move). diff --git a/addons/stock/views/stock_move_views.xml b/addons/stock/views/stock_move_views.xml index 8050fd1a81196..6c891ea7564cf 100644 --- a/addons/stock/views/stock_move_views.xml +++ b/addons/stock/views/stock_move_views.xml @@ -95,7 +95,7 @@ - + @@ -276,7 +276,10 @@ - + @@ -304,7 +307,7 @@ - + diff --git a/addons/stock/views/stock_picking_views.xml b/addons/stock/views/stock_picking_views.xml index e60dc12d9ec8c..f1c3a12b52f0c 100644 --- a/addons/stock/views/stock_picking_views.xml +++ b/addons/stock/views/stock_picking_views.xml @@ -345,7 +345,7 @@ - +