stock_move_location: Remove update of assigned moves location

The update of the source location of a move is not a good idea, as
shown in the test case that is modified by this commit and explained
below.

In general, a stock.move will be linked to a stock.picking whose
location_id and location_dest_id are defined by the picking type.
As such, the stock.move will inherit these locations and it is the
assignation of the stock.move that will create a stock.move.line
where the location (in case of outgoing picking) or the destination
location (in case of incoming picking through putaway) will be a
sub location of the respective location defined on the move.

So, if we want to empty a stock.location and move all its content
to another stock.location, we do not want to update the location
of the reserved moves, since it can break the reservation mechanism
of Odoo, as the assignation of a move will always look into the
children locations of the move, and it is not supposed to be updated
by any internal transfer, to what could be a children location.

Then, if internal transfers were planned from the location that is
being emptied, but are supposed to be executed after the location
has been emptied, there is probably an issue with the planning and
the sequence in which both transfers are being executed.
Since the internal transfer being updated in the test case was also
moving from a shelf location to its parent (where technically the goods
are already, since they are in a child location), we should prefer not
breaking the standard workflow of Odoo instead of supporting a corner
case with an internal transfer.

I guess we can revisit my assumption if we have a good reason to update
the stock location, but the one that was tested doesn't seem to be
good enough to justify breaking a standard mechanism.

stock_move_location/tests/test_common.py

@@ -2,7 +2,7 @@
 # Copyright 2018 Camptocamp SA
 # License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl)
 
-from odoo.tests import common
+from odoo.tests import Form, common
 
 
 class TestsCommon(common.SavepointCase):
@@ -104,6 +104,11 @@ class TestsCommon(common.SavepointCase):
             }
         )
 
+    def _create_picking(self, picking_type):
+        with Form(self.env["stock.picking"]) as picking_form:
+            picking_form.picking_type_id = picking_type
+        return picking_form.save()
+
     def _create_putaway_for_product(self, product, loc_in, loc_out):
         putaway = self.env["stock.putaway.rule"].create(
             {

stock_move_location/tests/test_move_location.py

@@ -133,35 +133,76 @@ class TestMoveLocation(TestsCommon):
             putaway_line.destination_location_id, self.internal_loc_2_shelf
         )
 
-    def test_inmediate_transfer_reserved_quantity(self):
+    def test_delivery_order_assignation_after_transfer(self):
         """
-        Unreserve quantities in old location and reserve the same items on
-        new location
+        Make sure using the wizard doesn't break assignation on delivery orders
         """
+        delivery_order_type = self.env.ref("stock.picking_type_out")
+        internal_transfer_type = self.env.ref("stock.picking_type_internal")
+        wh_stock_shelf_1 = self.env.ref("stock.stock_location_components")
+        wh_stock_shelf_2 = self.env.ref("stock.stock_location_14")
+        wh_stock_shelf_3 = wh_stock_shelf_1.copy({"name": "Shelf 3"})
+
         # Create some quants
         self.set_product_amount(
-            self.product_lots, self.internal_loc_1, 100, lot_id=self.lot1
+            self.product_lots, wh_stock_shelf_1, 100, lot_id=self.lot1
         )
-        # Reserve some quantities
-        stock_move = self.env["stock.move"].create(
+
+        # Create and assign a delivery picking to reserve some quantities
+        delivery_picking = self._create_picking(delivery_order_type)
+        delivery_move = self.env["stock.move"].create(
             {
-                "name": "Move for test",
+                "name": "Delivery move",
                 "product_id": self.product_lots.id,
                 "product_uom_qty": 20.0,
                 "product_uom": self.product_lots.uom_id.id,
-                "location_id": self.internal_loc_1.id,
-                "location_dest_id": self.internal_loc_2.id,
+                "location_id": delivery_picking.location_id.id,
+                "location_dest_id": delivery_picking.location_dest_id.id,
+                "picking_id": delivery_picking.id,
             }
         )
-        stock_move._action_confirm()
-        stock_move._action_assign()
-        # Move all quantities to other location
-        wizard = self._create_wizard(self.internal_loc_1, self.internal_loc_2_shelf)
+        delivery_picking.action_confirm()
+        self.assertEqual(delivery_picking.state, "confirmed")
+        delivery_picking.action_assign()
+        self.assertEqual(delivery_picking.state, "assigned")
+
+        # Move all quantities to other location using module's wizard
+        wizard = self._create_wizard(wh_stock_shelf_1, wh_stock_shelf_2)
         wizard.onchange_origin_location()
         wizard.action_move_location()
-        # The old reserved quantities must be in new location after confirm wizard
-        self.assertEqual(len(stock_move.move_line_ids), 1)
-        self.assertEqual(stock_move.move_line_ids.product_uom_qty, 20.0)
-        self.assertEqual(
-            stock_move.move_line_ids.location_id, self.internal_loc_2_shelf
+        self.assertEqual(delivery_picking.state, "assigned")
+
+        # Do a planned transfer to move quantities to other location
+        #  without using module's wizard
+        internal_picking = self._create_picking(internal_transfer_type)
+        internal_picking.write(
+            {"location_id": wh_stock_shelf_2, "location_dest_id": wh_stock_shelf_3.id}
         )
+        self.env["stock.move"].create(
+            {
+                "name": "Internal move",
+                "product_id": self.product_lots.id,
+                "product_uom_qty": 100.0,
+                "product_uom": self.product_lots.uom_id.id,
+                "location_id": internal_picking.location_id.id,
+                "location_dest_id": internal_picking.location_dest_id.id,
+                "picking_id": internal_picking.id,
+            }
+        )
+        # Unreserve quantity on the delivery to allow moving the quantity
+        delivery_picking.do_unreserve()
+        self.assertEqual(delivery_picking.state, "confirmed")
+        internal_picking.action_confirm()
+        internal_picking.action_assign()
+        internal_picking.move_line_ids.qty_done = (
+            internal_picking.move_line_ids.product_uom_qty
+        )
+        internal_picking.button_validate()
+        self.assertEqual(internal_picking.state, "done")
+        # Assign the delivery must work
+        delivery_picking.action_assign()
+        self.assertEqual(delivery_picking.state, "assigned")
+        # The old reserved quantities must be in new location after confirm wizard
+        self.assertEqual(len(delivery_move.move_line_ids), 1)
+        self.assertEqual(delivery_move.move_line_ids.product_uom_qty, 20.0)
+        self.assertEqual(delivery_move.move_line_ids.location_id, wh_stock_shelf_3)

stock_move_location/wizard/stock_move_location.py

@@ -238,8 +238,6 @@ class StockMoveLocationWizard(models.TransientModel):
             moves_to_unreserve = move_lines.mapped("move_id")
             # Unreserve in old location
             moves_to_unreserve._do_unreserve()
-            # Change location in move with the new one
-            moves_to_unreserve.write({"location_id": line.destination_location_id.id})
             moves_to_reassign |= moves_to_unreserve
         return moves_to_reassign