[FIX] {sale_,}stock: reverse orig/dest moves if is return

In a multistep configuration, decreasing the quantity on a line of a
confirmed sale order may create some inconsistencies.

To reproduce the issue:
1. In Settings, enable "Multi-Step Routes"
2. Edit the existing warehouse:
    - Outgoing Shipments: 3 steps
3. Create a storable product P
4. Set the on-hand qty to 5
5. Create and confirm a sale order SO with 5 x P
6. Process the PICK/PACK pickings
7. Edit the SO:
    - Decrease the qty: 3 x P
8. In the SO's deliveries, open the return of the pack step (Output ->
Packing)
    - Error: Nothing changes, there isn't any reserved quantity.
However, there are currently 5 x P at the Output location, so the
reservation should work
9. Inventory > Report > Stock Moves, list all SM related to P
    - Error: The origin/destination moves of the pack's return (Output
-> Packing) are incorrect too: the origins contain the pick's return
(Packing -> Stock) while it should not contain anything and the
destination moves contain the ship move (Output -> Customer) while it
should contain the pick's return (Packing -> Stock)

`SM_ship`: SM used to deliver from Output to Customers
`SM_ret_ship`: SM used to return P from Customers to Output
`SM_ret_pack`: SM used to return P from Output to Packing
`SM_ret_pick`: SM used to return P from Packing to Stock

When confirming a move, we transform the move if its demand is negative:
https://github.com/odoo/odoo/blob/53b424d9e43b07fe7aa84ff00035fb7772ef6291/addons/stock/models/stock_move.py#L1239-L1247
We reverse the source/destination locations but we don't do the same
with the origin/destination moves. That is an issue: before reversing,
we have:
| SM          | Qty | From    | To        | Origin      | Destination |
|-------------|-----|---------|-----------|-------------|-------------|
| SM_ret_pick | -2  | Stock   | Packing   | /           | SM_ret_pack |
| SM_ret_pack | -2  | Packing | Output    | SM_ret_pick | SM_ret_ship |
| SM_ret_ship | -2  | Output  | Customers | SM_ret_pack | /           |

Therefore, reversing the source/destination locations without updating
the origin/destination moves does not make sense: we should reverse them
too depending on their direction (positive/negative) and their
source/destination location.

Also, before trying to transform the moves, we try to merge them:
https://github.com/odoo/odoo/blob/53b424d9e43b07fe7aa84ff00035fb7772ef6291/addons/stock/models/stock_move.py#L1236-L1237
This is relevant for `SM_ret_ship`: we find an existing SM (`SM_ship`,
qty=5) that can absorb the negative demand of `SM_ret_ship` (qty=-2). In
such case, we add the origin/destination moves of the negative SM to the
positive one. However, in the above case, it will create an
inconsistency: after the fix, at that point, `SM_ret_pick` and
`SM_ret_pack` have been correctly reversed, so we have:
| SM          | Qty | From    | To        | Origin      | Destination |
|-------------|-----|---------|-----------|-------------|-------------|
| SM_ret_pick | 2   | Packing | Stock     | SM_ret_pack | /           |
| SM_ret_pack | 2   | Output  | Packing   | SM_ret_pack | SM_ret_pick |
| SM_ret_ship | -2  | Output  | Customers | /           | SM_ret_pack |
| SM_ship     | 5   | Output  | Customers | (SM_pack)   | /           |

Therefore, adding `SM_ret_pack` to the destination moves of `SM_ship` is
incorrect. (Note that, without the fix, `SM_ret_pack` would be added to
the origins of `SM_ship`, which would be incorrect too)

Finally, the reservation (step 8) does't work because of two reasons:
- If there are some `move_orig_ids`, we use the done value of these SM
to assign the current SM. However, as explained above, there is an issue
with this field
- If there isn't any `move_orig_ids`, there is an issue with the procure
method. Its value is `make_to_order`: we skip the reservation for such a
value:
https://github.com/odoo/odoo/blob/53b424d9e43b07fe7aa84ff00035fb7772ef6291/addons/stock/models/stock_move.py#L1499-L1501

OPW-2889679

closes odoo/odoo#95263

closes odoo/odoo#95926

X-original-commit: fb94c140a91aca26a56c5162d61b00b4a4b0d85b
Signed-off-by: William Henrotin (whe) <whe@odoo.com>
Signed-off-by: Adrien Widart <awt@odoo.com>

addons/sale_stock/tests/test_sale_stock.py

@@ -1205,3 +1205,55 @@ class TestSaleStock(TestSaleCommon, ValuationReconciliationTestCommon):
         self.assertEqual(so.order_line[1].product_id, self.product_b)
         self.assertEqual(so.order_line[1].qty_delivered, 1)
         self.assertEqual(so.order_line[1].product_uom.id, uom_km_id)
+
+    def test_return_with_mto_and_multisteps(self):
+        """
+        Suppose a product P and a 3-steps delivery.
+        Sell 5 x P, process pick & pack pickings and then decrease the qty on
+        the SO line:
+        - the ship picking should be updated
+        - there should be a return R1 for the pack picking
+        - there should be a return R2 for the pick picking
+        - it should be possible to reserve R1
+        """
+        warehouse = self.env['stock.warehouse'].search([('company_id', '=', self.env.company.id)], limit=1)
+        warehouse.delivery_steps = 'pick_pack_ship'
+        stock_location = warehouse.lot_stock_id
+        pack_location, out_location, custo_location = warehouse.delivery_route_id.rule_ids.location_dest_id
+
+        product = self.env['product.product'].create({
+            'name': 'SuperProduct',
+            'type': 'product',
+        })
+
+        self.env['stock.quant']._update_available_quantity(product, stock_location, 5)
+
+        so_form = Form(self.env['sale.order'])
+        so_form.partner_id = self.partner_a
+        with so_form.order_line.new() as line:
+            line.product_id = product
+            line.product_uom_qty = 5
+        so = so_form.save()
+        so.action_confirm()
+
+        pick_picking, pack_picking, _ = so.picking_ids
+        (pick_picking + pack_picking).move_ids.quantity_done = 5
+        (pick_picking + pack_picking).button_validate()
+
+        with Form(so) as so_form:
+            with so_form.order_line.edit(0) as line:
+                line.product_uom_qty = 3
+
+        move_lines = so.picking_ids.move_ids.sorted('id')
+        ship_sm, pack_sm, pick_sm, ret_pack_sm, ret_pick_sm = move_lines
+        self.assertRecordValues(move_lines, [
+            {'location_id': out_location.id, 'location_dest_id': custo_location.id, 'move_orig_ids': pack_sm.ids, 'move_dest_ids': []},
+            {'location_id': pack_location.id, 'location_dest_id': out_location.id, 'move_orig_ids': pick_sm.ids, 'move_dest_ids': ship_sm.ids},
+            {'location_id': stock_location.id, 'location_dest_id': pack_location.id, 'move_orig_ids': [], 'move_dest_ids': pack_sm.ids},
+            {'location_id': out_location.id, 'location_dest_id': pack_location.id, 'move_orig_ids': [], 'move_dest_ids': ret_pick_sm.ids},
+            {'location_id': pack_location.id, 'location_dest_id': stock_location.id, 'move_orig_ids': ret_pack_sm.ids, 'move_dest_ids': []},
+        ])
+
+        ret_pack_sm.picking_id.action_assign()
+        self.assertEqual(ret_pack_sm.state, 'assigned')
+        self.assertEqual(ret_pack_sm.move_line_ids.reserved_uom_qty, 2)

addons/stock/models/stock_move.py

@@ -894,8 +894,8 @@ class StockMove(models.Model):
                 if float_compare(pos_move.product_uom_qty, abs(neg_move.product_uom_qty), precision_rounding=pos_move.product_uom.rounding) >= 0:
                     pos_move.product_uom_qty += neg_move.product_uom_qty
                     pos_move.write({
-                        'move_dest_ids': [Command.link(m.id) for m in neg_move.mapped('move_dest_ids')],
-                        'move_orig_ids': [Command.link(m.id) for m in neg_move.mapped('move_orig_ids')],
+                        'move_dest_ids': [Command.link(m.id) for m in neg_move.mapped('move_dest_ids') if m.location_id == pos_move.location_dest_id],
+                        'move_orig_ids': [Command.link(m.id) for m in neg_move.mapped('move_orig_ids') if m.location_dest_id == pos_move.location_id],
                     })
                     merged_moves |= pos_move
                     moves_to_unlink |= neg_move
@@ -1256,9 +1256,21 @@ class StockMove(models.Model):
             move.product_uom_qty, 0, precision_rounding=move.product_uom.rounding) < 0)
         for move in neg_r_moves:
             move.location_id, move.location_dest_id = move.location_dest_id, move.location_id
+            orig_move_ids, dest_move_ids = [], []
+            for m in move.move_orig_ids | move.move_dest_ids:
+                from_loc, to_loc = m.location_id, m.location_dest_id
+                if float_compare(m.product_uom_qty, 0, precision_rounding=m.product_uom.rounding) < 0:
+                    from_loc, to_loc = to_loc, from_loc
+                if to_loc == move.location_id:
+                    orig_move_ids += m.ids
+                elif move.location_dest_id == from_loc:
+                    dest_move_ids += m.ids
+            move.move_orig_ids, move.move_dest_ids = [(6, 0, orig_move_ids)], [(6, 0, dest_move_ids)]
             move.product_uom_qty *= -1
             if move.picking_type_id.return_picking_type_id:
                 move.picking_type_id = move.picking_type_id.return_picking_type_id
+            # We are returning some products, we must take them in the source location
+            move.procure_method = 'make_to_stock'
         neg_r_moves._assign_picking()
 
         # call `_action_assign` on every confirmed move which location_id bypasses the reservation + those expected to be auto-assigned