Skip to content

Commit

Permalink
[FIX] stock: add an automated way to fix unreserve issue
Browse files Browse the repository at this point in the history
It happens that the `stock.quant.reserved_quantity` and `stock.move.line.product_qty`
for the same parameters are not equals (due to some bugs in the past)

It could implies that it doesn't exist enough reserved quantity on the
quant when a user try to unreserve a stock.move.line resulting in the
UserError 'It is not possible to unreserve more products of %s than you
have in stock'

However on current stable version (13.0 and 14.0) a lot of PR already
fixed a lot of issue as:
#69377
#66347
#61758
#54355
#43180
#70975
for example (there is more than that)

So the idea is to provide a way to unlock the database manually for
stable but not in the new version to be able to analyse them and find
possible bugs responssible for the desynchronization.

The server action check all the quants and their relative move line
to check if match correctly. If not it will remove the reservation
from both. It could remove the reservation from some `pickings` and
`stock.move`

related to #62139

closes #79180

Signed-off-by: William Henrotin (whe) <whe@odoo.com>
  • Loading branch information
amoyaux committed Dec 9, 2021
1 parent 7f30ef4 commit d99e173
Show file tree
Hide file tree
Showing 5 changed files with 198 additions and 9 deletions.
118 changes: 117 additions & 1 deletion addons/stock/data/stock_data.xml
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,123 @@
<field name="name">Last In First Out (LIFO)</field>
<field name="method">lifo</field>
</record>

<record id="stock_quant_stock_move_line_desynchronization" model="ir.actions.server">
<field name="name">Correct inconsistencies for reservation</field>
<field name="model_id" ref="base.model_ir_actions_server"/>
<field name="state">code</field>
<field name="code">
quants = env['stock.quant'].sudo().search([])

move_line_ids = []
move_line_to_recompute_ids = []

logging = ''

for quant in quants:

move_lines = env['stock.move.line'].search([
('product_id', '=', quant.product_id.id),
('location_id', '=', quant.location_id.id),
('lot_id', '=', quant.lot_id.id),
('package_id', '=', quant.package_id.id),
('owner_id', '=', quant.owner_id.id),
('product_qty', '!=', 0),
])

move_line_ids += move_lines.ids
reserved_on_move_lines = sum(move_lines.mapped('product_qty'))
move_line_str = str.join(', ', [str(move_line_id) for move_line_id in move_lines.ids])

if quant.location_id.should_bypass_reservation():
# If a quant is in a location that should bypass the reservation, its `reserved_quantity` field
# should be 0.
if quant.reserved_quantity != 0:
logging += "Problematic quant found: %s (quantity: %s, reserved_quantity: %s)\n" % (quant.id, quant.quantity, quant.reserved_quantity)
logging += "its `reserved_quantity` field is not 0 while its location should bypass the reservation\n"
if move_lines:
logging += "These move lines are reserved on it: %s (sum of the reservation: %s)\n" % (move_line_str, reserved_on_move_lines)
else:
logging += "no move lines are reserved on it, you can safely reset its `reserved_quantity` to 0\n"
logging += '******************\n'
quant.write({'reserved_quantity': 0})
else:
# If a quant is in a reservable location, its `reserved_quantity` should be exactly the sum
# of the `product_qty` of all the partially_available / assigned move lines with the same
# characteristics.

if quant.reserved_quantity == 0:
if move_lines:
logging += "Problematic quant found: %s (quantity: %s, reserved_quantity: %s)\n" % (quant.id, quant.quantity, quant.reserved_quantity)
logging += "its `reserved_quantity` field is 0 while these move lines are reserved on it: %s (sum of the reservation: %s)\n" % (move_line_str, reserved_on_move_lines)
logging += '******************\n'
move_lines.with_context(bypass_reservation_update=True).sudo().write({'product_uom_qty': 0})
move_line_to_recompute_ids += move_lines.ids
elif quant.reserved_quantity &lt; 0:
logging += "Problematic quant found: %s (quantity: %s, reserved_quantity: %s)\n" % (quant.id, quant.quantity, quant.reserved_quantity)
logging += "its `reserved_quantity` field is negative while it should not happen\n"
quant.write({'reserved_quantity': 0})
if move_lines:
logging += "These move lines are reserved on it: %s (sum of the reservation: %s)\n" % (move_line_str, reserved_on_move_lines)
move_lines.with_context(bypass_reservation_update=True).sudo().write({'product_uom_qty': 0})
move_line_to_recompute_ids += move_lines.ids
logging += '******************\n'
else:
if reserved_on_move_lines != quant.reserved_quantity:
logging += "Problematic quant found: %s (quantity: %s, reserved_quantity: %s)\n" % (quant.id, quant.quantity, quant.reserved_quantity)
logging += "its `reserved_quantity` does not reflect the move lines reservation\n"
logging += "These move lines are reserved on it: %s (sum of the reservation: %s)\n" % (move_line_str, reserved_on_move_lines)
logging += '******************\n'
move_lines.with_context(bypass_reservation_update=True).sudo().write({'product_uom_qty': 0})
move_line_to_recompute_ids += move_lines.ids
quant.write({'reserved_quantity': 0})
else:
if any(move_line.product_qty &lt; 0 for move_line in
move_lines):
logging += "Problematic quant found: %s (quantity: %s, reserved_quantity: %s)\n" % (quant.id, quant.quantity, quant.reserved_quantity)
logging += "its `reserved_quantity` correctly reflects the move lines reservation but some are negatives\n"
logging += "These move lines are reserved on it: %s (sum of the reservation: %s)\n" % (move_line_str, reserved_on_move_lines)
logging += '******************\n'
move_lines.with_context(bypass_reservation_update=True).sudo().write({'product_uom_qty': 0})
move_line_to_recompute_ids += move_lines.ids
quant.write({'reserved_quantity': 0})

move_lines = env['stock.move.line'].search([('product_id.type', '=',
'product'), ('product_qty', '!=', 0), ('id', 'not in',
move_line_ids)])

move_lines_to_unreserve = []

for move_line in move_lines:
if not move_line.location_id.should_bypass_reservation():
logging += "Problematic move line found: %s (reserved_quantity: %s)\n" % (move_line.id, move_line.product_qty)
logging += "There is no exiting quants despite its `reserved_quantity`\n"
logging += '******************\n'
move_lines_to_unreserve.append(move_line.id)
move_line_to_recompute_ids.append(move_line.id)

if len(move_lines_to_unreserve) &gt; 0:
env.cr.execute("""
UPDATE stock_move_line SET product_uom_qty = 0, product_qty = 0 WHERE id in %s ;
"""
% (tuple(move_lines_to_unreserve), ))

if logging:
env['ir.logging'].sudo().create({
'name': 'Unreserve stock.quant and stock.move.line',
'type': 'server',
'level': 'DEBUG',
'dbname': env.cr.dbname,
'message': logging,
'func': '_update_reserved_quantity',
'path': 'addons/stock/models/stock_quant.py',
'line': '0',
})

if move_line_to_recompute_ids:
env['stock.move.line'].browse(move_line_to_recompute_ids).move_id._recompute_state()

</field>
</record>
<!--
Resource: stock.location
-->
Expand Down
12 changes: 10 additions & 2 deletions addons/stock/i18n/stock.pot
Original file line number Diff line number Diff line change
Expand Up @@ -2820,14 +2820,22 @@ msgstr ""
#: code:addons/stock/models/stock_quant.py:0
#, python-format
msgid ""
"It is not possible to reserve more products of %s than you have in stock."
"It is not possible to unreserve more products of %s than you have in stock."
"The correction could unreserve some operations with problematics products."
msgstr ""

#. module: stock
#: code:addons/stock/models/stock_quant.py:0
#, python-format
msgid ""
"It is not possible to unreserve more products of %s than you have in stock."
"It is not possible to unreserve more products of %s than you have in stock. Contact an administrator."
msgstr ""

#. module: stock
#: code:addons/stock/models/stock_quant.py:0
#, python-format
msgid ""
"Try our automated action to fix it"
msgstr ""

#. module: stock
Expand Down
13 changes: 11 additions & 2 deletions addons/stock/models/stock_quant.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
from psycopg2 import OperationalError, Error

from odoo import api, fields, models, _
from odoo.exceptions import UserError, ValidationError
from odoo.exceptions import RedirectWarning, UserError, ValidationError
from odoo.osv import expression
from odoo.tools.float_utils import float_compare, float_is_zero, float_round

Expand Down Expand Up @@ -473,7 +473,16 @@ def _update_reserved_quantity(self, product_id, location_id, quantity, lot_id=No
# if we want to unreserve
available_quantity = sum(quants.mapped('reserved_quantity'))
if float_compare(abs(quantity), available_quantity, precision_rounding=rounding) > 0:
raise UserError(_('It is not possible to unreserve more products of %s than you have in stock.') % product_id.display_name)
action_fix_unreserve = self.env.ref(
'stock.stock_quant_stock_move_line_desynchronization', raise_if_not_found=False)
if action_fix_unreserve and self.user_has_groups('base.group_system'):
raise RedirectWarning(
_("""It is not possible to unreserve more products of %s than you have in stock.
The correction could unreserve some operations with problematics products.""") % product_id.display_name,
action_fix_unreserve.id,
_('Automated action to fix it'))
else:
raise UserError(_('It is not possible to unreserve more products of %s than you have in stock. Contact an administrator.') % product_id.display_name)
else:
return reserved_quants

Expand Down
6 changes: 3 additions & 3 deletions addons/stock/tests/test_quant.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

from odoo.exceptions import ValidationError
from odoo.tests.common import SavepointCase
from odoo.exceptions import AccessError, UserError
from odoo.exceptions import AccessError, RedirectWarning, UserError


class StockQuant(SavepointCase):
Expand Down Expand Up @@ -431,7 +431,7 @@ def test_increase_decrease_reserved_quantity_1(self):
with self.assertRaises(UserError):
self.env['stock.quant']._update_reserved_quantity(self.product, self.stock_location, 1.0)
self.assertEqual(self.env['stock.quant']._get_available_quantity(self.product, self.stock_location), 0.0)
with self.assertRaises(UserError):
with self.assertRaises(RedirectWarning):
self.env['stock.quant']._update_reserved_quantity(self.product, self.stock_location, -1.0, strict=True)
self.assertEqual(self.env['stock.quant']._get_available_quantity(self.product, self.stock_location), 0.0)

Expand Down Expand Up @@ -476,7 +476,7 @@ def test_mix_tracked_untracked_1(self):
self.assertEqual(self.env['stock.quant']._get_available_quantity(self.product_serial, self.stock_location, strict=True), 1.0)
self.assertEqual(self.env['stock.quant']._get_available_quantity(self.product_serial, self.stock_location, lot_id=lot1), 2.0)

with self.assertRaises(UserError):
with self.assertRaises(RedirectWarning):
self.env['stock.quant']._update_reserved_quantity(self.product_serial, self.stock_location, -1.0, strict=True)

self.assertEqual(self.env['stock.quant']._get_available_quantity(self.product_serial, self.stock_location), 2.0)
Expand Down
58 changes: 57 additions & 1 deletion addons/stock/tests/test_robustness.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
# -*- coding: utf-8 -*-
# Part of Odoo. See LICENSE file for full copyright and licensing details.

from odoo.exceptions import UserError, ValidationError
from odoo.exceptions import RedirectWarning, UserError, ValidationError
from odoo.tests.common import SavepointCase


Expand Down Expand Up @@ -221,3 +221,59 @@ def test_lot_id_product_id_mix(self):
'location_dest_id': move2.location_dest_id.id,
})]})

def test_unreserve_error(self):
self.env['stock.quant']._update_available_quantity(
self.product1, self.stock_location, 5)

move = self.env['stock.move'].create({
'name': 'test_lot_id_product_id_mix_move_1',
'location_id': self.stock_location.id,
'location_dest_id': self.customer_location.id,
'product_id': self.product1.id,
'product_uom': self.uom_unit.id,
'product_uom_qty': 5.0,
})
move._action_confirm()
move._action_assign()
quant = self.env['stock.quant']._gather(
self.product1, self.stock_location, strict=True)
self.env.cr.execute("UPDATE stock_quant set reserved_quantity=0 WHERE id=%s", (quant.id,))
quant.invalidate_cache()
server_action = self.env.ref(
'stock.stock_quant_stock_move_line_desynchronization', raise_if_not_found=False)
if server_action:
with self.assertRaises(RedirectWarning):
move._do_unreserve()
else:
with self.assertRaises(UserError):
move._do_unreserve()

def test_unreserve_fix(self):
self.env['stock.quant']._update_available_quantity(
self.product1, self.stock_location, 5)

move = self.env['stock.move'].create({
'name': 'test_lot_id_product_id_mix_move_1',
'location_id': self.stock_location.id,
'location_dest_id': self.customer_location.id,
'product_id': self.product1.id,
'product_uom': self.uom_unit.id,
'product_uom_qty': 5.0,
})

move._action_confirm()
move._action_assign()

quant = self.env['stock.quant']._gather(
self.product1, self.stock_location, strict=True)
self.env.cr.execute(
"UPDATE stock_quant set reserved_quantity=0 WHERE id=%s", (quant.id,))
quant.invalidate_cache()
server_action = self.env.ref(
'stock.stock_quant_stock_move_line_desynchronization', raise_if_not_found=False)
if not server_action:
return
server_action.run()
self.assertEqual(move.reserved_availability, 0)
self.assertEqual(move.state, 'confirmed')
self.assertEqual(quant.reserved_quantity, 0)

0 comments on commit d99e173

Please sign in to comment.