From ecf66e650716cddb73f23537236dae9c0a43c68a Mon Sep 17 00:00:00 2001 From: Adrien Widart Date: Tue, 25 Jan 2022 10:22:33 +0000 Subject: [PATCH] [FIX] mrp_subcontracting: process delivery with several backorders In case of a MRP subcontracting, it is not possible to process the delivery with several backorders. To reproduce the issue: 1. Create two products P_compo, P_finished - Both storable - P_compo must have the route "Resupply Subcontractor on Order" 2. Update P_compo's quantity: 5 3. Create a BoM: - Product: P_finished - BoM type: Subcontracting - Subcontractors: a partner P - Components: 1 x P_compo 4. In Inventory, create a planned transfer T: - Operation Type: Receipt - Receive From: P - Operations: 5 x P_finished 5. Mark as Todo 6. Inventory > Resupply Subcontractor, find the delivery of P_compo for P and process it 7. Back to T, set the done quantity to 1.25 8. Validate T (with backorder) 9. On the backorder BO1, set the done quantity to 1.0 10. Validate BO1 (with backorder) 11. Open the second backorder BO2 and validate it Error: a Validation Error is displayed "You can not enter negative quantities.", which doesn't make sense The issue comes from the inner method `_get_available_move_lines` in `_action_assign`. When validating a picking, at some point, we are here, at the beginning of `/mrp_subcontracting._action_done`: https://github.com/odoo/odoo/blob/6d6b2c41adfa842c82252373b3dbd1ed0a5ebc92/addons/mrp_subcontracting/models/stock_picking.py#L36-L37 To understand what happens next, we need to note that, in this method, the associated MO will be marked as done later (on line 76). Back to the current line (L37), we are calling `super` which leads to the problematic `_get_available_move_lines`. Suppose we are on step 10 (we are validating the first backorder), we have https://github.com/odoo/odoo/blob/58a9f57c0827a51e46ef0f52a3d48ba343667fe0/addons/stock/models/stock_move.py#L1390-L1391 `move_orig_ids` contains two stock moves, each one associated to one MO. However, as said above, the second MO is not yet marked as done, this will be done later on in `/mrp_subcontracting._action_done`. Therefore, `move_lines_in` is only defined with the first SM, from the first MO, with a quantity of `1.25` Further in `_get_available_move_lines`, we have: https://github.com/odoo/odoo/blob/58a9f57c0827a51e46ef0f52a3d48ba343667fe0/addons/stock/models/stock_move.py#L1403-L1405 `move.move_orig_ids.mapped('move_dest_ids') - move` gives two SM, and here is the issue: the second one is already processed, so both are kept in `move_lines_out_done` and the sum of their quantity is equal to `1.25 + 1 = 2.25` Therefore, at the end of the method, when computing the difference, it gives a negative difference. This negative value will be used to create a SML and later on, it will trigger the validation error. Once the above issue is fixed, there is another "invisible" issue that need to be fixed: when generating several backorders on the deliveries, only the first one will trigger the creation of a new MO. Suppose we are back in `/mrp_subcontracting._action_done` (still on step 10 in the above case), in the first for-loop: https://github.com/odoo/odoo/blob/6d6b2c41adfa842c82252373b3dbd1ed0a5ebc92/addons/mrp_subcontracting/models/stock_picking.py#L39-L43 `move._get_subcontract_production()` returns the two MO (from the initial picking and from the first backorder). However, since the first one has already been processed, the if-condition is validated. Therefore, this for-loop is done and the second MO is not processed (the quantities are not recorded and its field `subcontracting_has_been_recorded` is not defined to `True`). As a result, still in `/mrp_subcontracting._action_done`, we are now in the second for-loop: https://github.com/odoo/odoo/blob/6d6b2c41adfa842c82252373b3dbd1ed0a5ebc92/addons/mrp_subcontracting/models/stock_picking.py#L71-L72 `_subcontracting_filter_to_done` removes the two MOs because the first one is done and the second one has not its field `subcontracting_has_been_recorded` set to `True`. The condition in the first for-loop is not accurate enough. OPW-2731658 closes odoo/odoo#83658 X-original-commit: 1b740ae148acfc832b124122763817b08ea110ab Signed-off-by: Arnold Moyaux Signed-off-by: Adrien Widart --- .../models/stock_picking.py | 9 +++- .../tests/test_subcontracting.py | 49 +++++++++++++++++++ addons/stock/models/stock_move.py | 5 +- 3 files changed, 59 insertions(+), 4 deletions(-) diff --git a/addons/mrp_subcontracting/models/stock_picking.py b/addons/mrp_subcontracting/models/stock_picking.py index f49a4b19ad1..c1f0803b43c 100644 --- a/addons/mrp_subcontracting/models/stock_picking.py +++ b/addons/mrp_subcontracting/models/stock_picking.py @@ -38,9 +38,14 @@ class StockPicking(models.Model): for move in self.move_ids.filtered(lambda move: move.is_subcontract): # Auto set qty_producing/lot_producing_id of MO wasn't recorded # manually (if the flexible + record_component or has tracked component) - if any(production._has_been_recorded() for production in move._get_subcontract_production()): + productions = move._get_subcontract_production() + recorded_productions = productions.filtered(lambda p: p._has_been_recorded()) + recorded_qty = sum(recorded_productions.mapped('qty_producing')) + sm_done_qty = sum(productions._get_subcontract_move().mapped('quantity_done')) + rounding = self.env['decimal.precision'].precision_get('Product Unit of Measure') + if float_compare(recorded_qty, sm_done_qty, precision_digits=rounding) >= 0: continue - production = move._get_subcontract_production() + production = productions - recorded_productions if not production: continue if len(production) > 1: diff --git a/addons/mrp_subcontracting/tests/test_subcontracting.py b/addons/mrp_subcontracting/tests/test_subcontracting.py index 27cf0e53a8c..36494fc4b0d 100644 --- a/addons/mrp_subcontracting/tests/test_subcontracting.py +++ b/addons/mrp_subcontracting/tests/test_subcontracting.py @@ -590,6 +590,55 @@ class TestSubcontractingFlows(TestMrpSubcontractingCommon): self.assertEqual(subcontracting_values['bom_cost'], 10) self.assertEqual(report_values['lines']['total'], 20) # 10 For subcontracting + 5 for comp1 + 5 for subcontracting of comp2_bom + def test_several_backorders(self): + def process_picking(picking, qty): + picking.move_ids.quantity_done = qty + action = picking.button_validate() + if isinstance(action, dict): + wizard = Form(self.env[action['res_model']].with_context(action['context'])).save() + wizard.process() + + resupply_route = self.env['stock.route'].search([('name', '=', 'Resupply Subcontractor on Order')]) + finished, component = self.env['product.product'].create([{ + 'name': 'Finished Product', + 'type': 'product', + }, { + 'name': 'Component', + 'type': 'product', + 'route_ids': [(4, resupply_route.id)], + }]) + + bom = self.env['mrp.bom'].create({ + 'product_tmpl_id': finished.product_tmpl_id.id, + 'product_qty': 1.0, + 'type': 'subcontract', + 'subcontractor_ids': [(4, self.subcontractor_partner1.id)], + 'bom_line_ids': [(0, 0, {'product_id': component.id, 'product_qty': 1.0})], + }) + + picking_form = Form(self.env['stock.picking']) + picking_form.picking_type_id = self.env.ref('stock.picking_type_in') + picking_form.partner_id = self.subcontractor_partner1 + with picking_form.move_ids_without_package.new() as move: + move.product_id = finished + move.product_uom_qty = 5 + picking = picking_form.save() + picking.action_confirm() + + supply_picking = self.env['mrp.production'].search([('bom_id', '=', bom.id)]).picking_ids + process_picking(supply_picking, 5) + + process_picking(picking, 1.25) + + backorder01 = picking.backorder_ids + process_picking(backorder01, 1) + + backorder02 = backorder01.backorder_ids + process_picking(backorder02, 0) + self.assertEqual(backorder02.move_ids.quantity_done, 2.75) + + self.assertEqual(self.env['mrp.production'].search_count([('bom_id', '=', bom.id)]), 3) + class TestSubcontractingTracking(TransactionCase): diff --git a/addons/stock/models/stock_move.py b/addons/stock/models/stock_move.py index 745b8276dcc..a439872d01b 100644 --- a/addons/stock/models/stock_move.py +++ b/addons/stock/models/stock_move.py @@ -1416,7 +1416,7 @@ class StockMove(models.Model): move_lines_out_reserved = (reserved_moves_out_siblings | moves_out_siblings_to_consider).mapped('move_line_ids') def _keys_out_groupby(ml): - return (ml.location_id.id, ml.lot_id.id, ml.package_id.id, ml.owner_id.id) + return (ml.location_id, ml.lot_id, ml.package_id, ml.owner_id) grouped_move_lines_out = {} for k, g in groupby(move_lines_out_done, key=_keys_out_groupby): @@ -1428,7 +1428,8 @@ class StockMove(models.Model): grouped_move_lines_out[k] = sum(self.env['stock.move.line'].concat(*g).mapped('reserved_qty')) available_move_lines = {key: grouped_move_lines_in[key] - grouped_move_lines_out.get(key, 0) for key in grouped_move_lines_in} # pop key if the quantity available amount to 0 - return dict((k, v) for k, v in available_move_lines.items() if v) + rounding = move.product_id.uom_id.rounding + return dict((k, v) for k, v in available_move_lines.items() if float_compare(v, 0, precision_rounding=rounding) > 0) StockMove = self.env['stock.move'] assigned_moves_ids = OrderedSet()