From a2e57f1b3d54c68dea814f29ed55cef713727a65 Mon Sep 17 00:00:00 2001
From: Guewen Baconnier <guewen.baconnier@camptocamp.com>
Date: Fri, 29 May 2020 08:53:59 +0200
Subject: [PATCH] cluster picking: fix reservation inconsistency

When we pick a good and pick less than the reserved quantity,
the endpoint splits the move line in 2 parts: one with the qty_done
set to the quantity passed to the endpoint, another move line for
the remaining.
When the copy of the move line is created, the reserved quantity
on the move line doesn't change, but when the quantity is reduced on
the current line, the reserved quantity on the quant is automatically
reduced accordingly.
We don't want this, because we didn't reduced the total reserved
quantity, only moved it to another move line.

Before:
-------

* We have a quant of 40 product A in Shelf 1
* We reserve a move line of 20 product A in Shelf 1, the reserved
  quantity of the quant is 20
* We start the batch transfer, using the barcode scanner app, we
  scan the product and update the quantity to pick 13 products only
* A new line of 7 is created, quant's reserved quantity is still 20
* Quantity of the current move line is set to 13, the reserved quantity
  of the quant is now 13

Result: we have a desynchronization between the move lines (20 reserved)
and the quants (13 reserved). If we try to unreserve a move line, we
have an error: "It is not possible to unreserve more products of %s than
you have in stock."

After:
------

Same steps as before, but we ignore the update of the quant at the last
step.

Result: both the move lines and quants have 20 units reserved.
---
 shopfloor/services/cluster_picking.py        |  7 ++++-
 shopfloor/tests/test_cluster_picking_scan.py | 29 ++++++++++++++------
 2 files changed, 26 insertions(+), 10 deletions(-)

diff --git a/shopfloor/services/cluster_picking.py b/shopfloor/services/cluster_picking.py
index 881b56931e..e40d981491 100644
--- a/shopfloor/services/cluster_picking.py
+++ b/shopfloor/services/cluster_picking.py
@@ -601,7 +601,12 @@ def scan_destination_pack(self, picking_batch_id, move_line_id, barcode, quantit
             # contained less items than expected)
             remaining = move_line.product_uom_qty - quantity
             new_line = move_line.copy({"product_uom_qty": remaining, "qty_done": 0})
-            move_line.product_uom_qty = quantity
+            # if we didn't bypass reservation update, the quant reservation
+            # would be reduced as much as the deduced quantity, which is wrong
+            # as we only moved the quantity to a new move line
+            move_line.with_context(
+                bypass_reservation_update=True
+            ).product_uom_qty = quantity
 
         search = self.actions_for("search")
         bin_package = search.package_from_scan(barcode)
diff --git a/shopfloor/tests/test_cluster_picking_scan.py b/shopfloor/tests/test_cluster_picking_scan.py
index b67471d560..11909cccf0 100644
--- a/shopfloor/tests/test_cluster_picking_scan.py
+++ b/shopfloor/tests/test_cluster_picking_scan.py
@@ -467,6 +467,15 @@ def test_scan_destination_pack_quantity_more(self):
     def test_scan_destination_pack_quantity_less(self):
         """Pick less units than expected"""
         line = self.one_line_picking.move_line_ids
+        quant = self.env["stock.quant"].search(
+            [
+                ("location_id", "=", line.location_id.id),
+                ("product_id", "=", line.product_id.id),
+            ]
+        )
+        quant.ensure_one()
+        self.assertRecordValues(quant, [{"quantity": 40.0, "reserved_quantity": 20.0}])
+
         # when we pick less quantity than expected, the line is split
         # and the user is proposed to pick the next line for the remaining
         # quantity
@@ -479,16 +488,7 @@ def test_scan_destination_pack_quantity_less(self):
                 "quantity": line.product_uom_qty - 3,
             },
         )
-
-        self.assertRecordValues(
-            line,
-            [{"qty_done": 7, "result_package_id": self.bin1.id, "product_uom_qty": 7}],
-        )
         new_line = self.one_line_picking.move_line_ids - line
-        self.assertRecordValues(
-            new_line,
-            [{"qty_done": 0, "result_package_id": False, "product_uom_qty": 3}],
-        )
 
         self.assert_response(
             response,
@@ -502,6 +502,17 @@ def test_scan_destination_pack_quantity_less(self):
             },
         )
 
+        self.assertRecordValues(
+            line,
+            [{"qty_done": 7, "result_package_id": self.bin1.id, "product_uom_qty": 7}],
+        )
+        self.assertRecordValues(
+            new_line,
+            [{"qty_done": 0, "result_package_id": False, "product_uom_qty": 3}],
+        )
+        # the reserved quantity on the quant must stay the same
+        self.assertRecordValues(quant, [{"quantity": 40.0, "reserved_quantity": 20.0}])
+
     def test_scan_destination_pack_zero_check(self):
         """Location will be emptied, have to go to zero check"""
         line = self.one_line_picking.move_line_ids