From ba8f99c0a64560dae5ed31f38fcaa6c60a34a7d8 Mon Sep 17 00:00:00 2001
From: "Adrien Widart (awt)" <awt@odoo.com>
Date: Thu, 14 Sep 2023 13:53:46 +0200
Subject: [PATCH] [FIX] mrp: create PBM SM from confirmed MO

To reproduce the issue:
1. In Settings, enable "Multi Routes"
2. Edit the warehouse:
   - Manufacture: 2 steps
3. Create three storable products P1, P2, P3
4. Create and confirm a MO for 1 x P1 with 1 x P2
   - It should create the PBM picking
5. Add a component line for P3
6. (Because the MO is locked, the user can not edit the 'to consume'
   quantity, so) Unlock the MO
7. Set the 'to consume' qty to 1
8. Save the MO

Error: P3 is not added to PBM picking. Worst: suppose the user does
the internal transfer and then checks the availability of P3, the
line will still be unreserved

Step 6, when unlocking the MO, it also triggers a save. As a result,
the SM for P3 is created with its demand defined to 0. When adding
such SM, we do several things through this method call:
https://github.com/odoo/odoo/blob/be0b61cbaf3d3b7082aca8f96dcf8a6ee7885fea/addons/mrp/models/mrp_production.py#L776
- We will adapt its procure method (here, because of 2-steps
  manufacturing, it will be MTO)
- We will confirm the new SM -> we will run a procurement for a zero
  quantity -> it will not generate any new SM

Then, when updating the SM quantity (step 7), nothing will run a new
procurement.

Moreover, this also explains why trying to reserve the SM does not
work: it's an MTO one, but it does not have any `move_orig_ids`, so
it is not possible to assign it.

Solutions:
- It should be possible to edit the 'to consume' qty of a new SM,
  even on a locked and confirmed MO
- A procurement should be executed when updating the demand of an SM
  from 0 to >0. From 16.1, a procurement will always be executed
  each time the quantity changed (see [1]). Here, we want to limit
  the impact/risk of the fix

[1] 1f4fb64a197729b709bba8524b6bc59892a2f099

OPW-3253204

closes odoo/odoo#135478

Signed-off-by: William Henrotin (whe) <whe@odoo.com>
---
 addons/mrp/models/stock_move.py           | 24 ++++++++++++----
 addons/mrp/tests/test_procurement.py      | 35 +++++++++++++++++++++++
 addons/mrp/views/mrp_production_views.xml |  2 +-
 3 files changed, 55 insertions(+), 6 deletions(-)

diff --git a/addons/mrp/models/stock_move.py b/addons/mrp/models/stock_move.py
index 5205a2c81b45..5e9d36c9773d 100644
--- a/addons/mrp/models/stock_move.py
+++ b/addons/mrp/models/stock_move.py
@@ -243,11 +243,25 @@ class StockMove(models.Model):
         return defaults
 
     def write(self, vals):
-        if 'product_uom_qty' in vals and 'move_line_ids' in vals:
-            # first update lines then product_uom_qty as the later will unreserve
-            # so possibly unlink lines
-            move_line_vals = vals.pop('move_line_ids')
-            super().write({'move_line_ids': move_line_vals})
+        if 'product_uom_qty' in vals:
+            if 'move_line_ids' in vals:
+                # first update lines then product_uom_qty as the later will unreserve
+                # so possibly unlink lines
+                move_line_vals = vals.pop('move_line_ids')
+                super().write({'move_line_ids': move_line_vals})
+            procurement_requests = []
+            for move in self:
+                if move.raw_material_production_id.state != 'confirmed' \
+                        or not float_is_zero(move.product_uom_qty, precision_rounding=move.product_uom.rounding) \
+                        or move.procure_method != 'make_to_order':
+                    continue
+                values = move._prepare_procurement_values()
+                origin = move._prepare_procurement_origin()
+                procurement_requests.append(self.env['procurement.group'].Procurement(
+                    move.product_id, vals['product_uom_qty'], move.product_uom,
+                    move.location_id, move.rule_id and move.rule_id.name or "/",
+                    origin, move.company_id, values))
+            self.env['procurement.group'].run(procurement_requests)
         return super().write(vals)
 
     def _action_assign(self):
diff --git a/addons/mrp/tests/test_procurement.py b/addons/mrp/tests/test_procurement.py
index f2408edf653f..3931009d2363 100644
--- a/addons/mrp/tests/test_procurement.py
+++ b/addons/mrp/tests/test_procurement.py
@@ -751,3 +751,38 @@ class TestProcurement(TestMrpCommon):
             {'product_qty': 1, 'bom_id': bom01.id, 'picking_type_id': manu_operation01.id, 'location_dest_id': stock_location01.id},
             {'product_qty': 2, 'bom_id': bom02.id, 'picking_type_id': manu_operation02.id, 'location_dest_id': stock_location02.id},
         ])
+
+    def test_pbm_and_additionnal_components(self):
+        """
+        2-steps manufacturring.
+        When adding a new component to a confirmed MO, it should add an SM in
+        the PBM picking. Also, it should be possible to define the to-consume
+        qty of the new line even if the MO is locked
+        """
+        warehouse = self.env['stock.warehouse'].search([('company_id', '=', self.env.company.id)], limit=1)
+        warehouse.manufacture_steps = 'pbm'
+
+        mo_form = Form(self.env['mrp.production'])
+        mo_form.bom_id = self.bom_4
+        mo = mo_form.save()
+        mo.action_confirm()
+
+        if not mo.is_locked:
+            mo.action_toggle_is_locked()
+
+        with Form(mo) as mo_form:
+            with mo_form.move_raw_ids.new() as raw_line:
+                raw_line.product_id = self.product_2
+                raw_line.product_uom_qty = 2.0
+
+        move_vals = mo._get_move_raw_values(self.product_10, 0, self.product_2.uom_id)
+        mo.move_raw_ids = [(0, 0, move_vals)]
+        mo.move_raw_ids[-1].product_uom_qty = 10.0
+
+        expected_vals = [
+            {'product_id': self.product_1.id, 'product_uom_qty': 1.0},
+            {'product_id': self.product_2.id, 'product_uom_qty': 2.0},
+            {'product_id': self.product_10.id, 'product_uom_qty': 10.0},
+        ]
+        self.assertRecordValues(mo.move_raw_ids, expected_vals)
+        self.assertRecordValues(mo.picking_ids.move_lines, expected_vals)
diff --git a/addons/mrp/views/mrp_production_views.xml b/addons/mrp/views/mrp_production_views.xml
index 07c0d19886d5..34067f06b766 100644
--- a/addons/mrp/views/mrp_production_views.xml
+++ b/addons/mrp/views/mrp_production_views.xml
@@ -295,7 +295,7 @@
                                     <field name="location_dest_id" domain="[('id', 'child_of', parent.location_dest_id)]" invisible="1"/>
                                     <field name="state" invisible="1" force_save="1"/>
                                     <field name="should_consume_qty" invisible="1"/>
-                                    <field name="product_uom_qty" widget="mrp_should_consume" force_save="1" string="To Consume" attrs="{'readonly': ['&amp;', ('parent.state', '!=', 'draft'), '|', '&amp;', ('parent.state', 'not in', ('confirmed', 'progress', 'to_close')), ('parent.is_planned', '!=', True), ('parent.is_locked', '=', True)]}" width="1"/>
+                                    <field name="product_uom_qty" widget="mrp_should_consume" force_save="1" string="To Consume" attrs="{'readonly': ['&amp;', ('parent.state', '!=', 'draft'), '|', '&amp;', ('parent.state', 'not in', ('confirmed', 'progress', 'to_close')), ('parent.is_planned', '!=', True), '&amp;', ('state', '!=', 'draft'), ('parent.is_locked', '=', True)]}" width="1"/>
                                     <field name="product_uom" attrs="{'readonly': [('state', '!=', 'draft'), ('id', '!=', False)]}" options="{'no_open': True, 'no_create': True}" groups="uom.group_uom"/>
                                     <field name="product_type" invisible="1"/>
                                     <field name="product_qty" invisible="1" readonly="1"/>
-- 
GitLab