From b115f606720cf8fcda595fb3f89e5b08a46d737d Mon Sep 17 00:00:00 2001
From: "Tiffany Chang (tic)" <tic@odoo.com>
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#75891
X-original-commit: 8ca10a8
Signed-off-by: Arnold Moyaux <amoyaux@users.noreply.github.com>
---
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 3e927a14fd78..f610fdbf7f4b 100644
--- a/addons/stock/models/stock_move.py
+++ b/addons/stock/models/stock_move.py
@@ -554,6 +554,8 @@ class StockMove(models.Model):
# 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 ef92f758aaea..d0277281cdc2 100644
--- a/addons/stock/models/stock_move_line.py
+++ b/addons/stock/models/stock_move_line.py
@@ -271,7 +271,8 @@ class StockMoveLine(models.Model):
('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:
@@ -332,7 +333,7 @@ class StockMoveLine(models.Model):
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,
@@ -347,7 +348,8 @@ class StockMoveLine(models.Model):
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 eb52068ee8b0..ba3e27a39b40 100644
--- a/addons/stock/tests/test_move.py
+++ b/addons/stock/tests/test_move.py
@@ -3379,6 +3379,39 @@ class StockMove(SavepointCase):
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 e85a6f375832..fd34f2e3ada0 100644
--- a/addons/stock/views/stock_move_views.xml
+++ b/addons/stock/views/stock_move_views.xml
@@ -75,7 +75,7 @@
<field name="product_uom_qty" string="Demand" attrs="{'readonly': [('is_initial_demand_editable', '=', False)]}"/>
<field name="reserved_availability" string="Reserved"/>
<field name="quantity_done" string="Done" attrs="{'readonly': [('is_quantity_done_editable', '=', False)]}"/>
- <field name="product_uom" attrs="{'readonly': [('state', '!=', 'draft'), ('additional', '=', False)]}" options="{'no_open': True, 'no_create': True}" string="Unit of Measure" groups="uom.group_uom"/>
+ <field name="product_uom" attrs="{'readonly': [('state', '!=', 'draft'), ('id', '!=', False)]}" options="{'no_open': True, 'no_create': True}" string="Unit of Measure" groups="uom.group_uom"/>
</tree>
</field>
</record>
@@ -234,7 +234,10 @@
<field name="is_locked" invisible="1"/>
<field name="picking_code" invisible="1"/>
<field name="qty_done" attrs="{'readonly': ['|', '&', ('state', '=', 'done'), ('is_locked', '=', True), '&', ('package_level_id', '!=', False), ('parent.picking_type_entire_packs', '=', True)]}"/>
- <field name="product_uom_id" options="{'no_open': True, 'no_create': True}" attrs="{'readonly': ['|', ('product_uom_qty', '!=', 0.0), '&', ('package_level_id', '!=', False), ('parent.picking_type_entire_packs', '=', True)]}" string="Unit of Measure" groups="uom.group_uom"/>
+ <field name="product_uom_id" options="{'no_open': True, 'no_create': True}" string="Unit of Measure" groups="uom.group_uom"
+ attrs="{'readonly': ['|', '|', ('product_uom_qty', '!=', 0.0),
+ '&', ('package_level_id', '!=', False), ('parent.picking_type_entire_packs', '=', True),
+ '&', ('state', '=', 'done'), ('id', '!=', False)]}"/>
</tree>
</field>
</record>
@@ -263,7 +266,7 @@
<field name="product_uom_qty" readonly="1" attrs="{'column_invisible': ['|',('parent.immediate_transfer', '=', True),('parent.picking_type_code','=','incoming')]}" optional="show"/>
<field name="is_locked" invisible="1"/>
<field name="qty_done" attrs="{'readonly': [('state', 'in', ('done', 'cancel')), ('is_locked', '=', True)]}" force_save="1"/>
- <field name="product_uom_id" force_save="1" attrs="{'readonly': [('state', '!=', 'draft')]}" groups="uom.group_uom"/>
+ <field name="product_uom_id" force_save="1" attrs="{'readonly': [('state', '!=', 'draft'), ('id', '!=', False)]}" groups="uom.group_uom"/>
</tree>
</field>
</record>
diff --git a/addons/stock/views/stock_picking_views.xml b/addons/stock/views/stock_picking_views.xml
index 749b2d919cdf..28d3304c5ad5 100644
--- a/addons/stock/views/stock_picking_views.xml
+++ b/addons/stock/views/stock_picking_views.xml
@@ -383,7 +383,7 @@
<field name="forecast_expected_date" invisible="1" />
<field name="forecast_availability" string="Reserved" attrs="{'column_invisible': ['|', ('parent.picking_type_code', '!=', 'outgoing'), ('parent.state','=', 'done')]}" widget="forecast_widget"/>
<field name="quantity_done" string="Done" attrs="{'readonly': [('is_quantity_done_editable', '=', False)]}"/>
- <field name="product_uom" attrs="{'readonly': [('state', '!=', 'draft'), ('additional', '=', False)]}" options="{'no_open': True, 'no_create': True}" string="Unit of Measure" groups="uom.group_uom"/>
+ <field name="product_uom" attrs="{'readonly': [('state', '!=', 'draft'), ('id', '!=', False)]}" options="{'no_open': True, 'no_create': True}" string="Unit of Measure" groups="uom.group_uom"/>
<field name="lot_ids" widget="many2many_tags"
groups="stock.group_production_lot"
attrs="{'invisible': ['|', ('show_details_visible', '=', False), ('has_tracking', '!=', 'serial')]}"
@@ -418,7 +418,7 @@
<field name="forecast_expected_date" invisible="1"/>
<field name="forecast_availability" string="Reserved" attrs="{'invisible': ['|', ('parent.picking_type_code', '!=', 'outgoing'), ('parent.state','=', 'done')]}" widget="forecast_widget"/>
<field name="quantity_done" string="Done" attrs="{'readonly': [('is_quantity_done_editable', '=', False)]}"/>
- <field name="product_uom" attrs="{'readonly': [('state', '!=', 'draft'), ('additional', '=', False)]}" options="{'no_open': True, 'no_create': True}" string="Unit of Measure" groups="uom.group_uom"/>
+ <field name="product_uom" attrs="{'readonly': [('state', '!=', 'draft'), ('id', '!=', False)]}" options="{'no_open': True, 'no_create': True}" string="Unit of Measure" groups="uom.group_uom"/>
<field name="description_picking" string="Description"/>
</group>
</form>
--
GitLab