From ce355c978686753c784c1a985975c9fabc7730bd Mon Sep 17 00:00:00 2001 From: Arnold Moyaux Date: Tue, 12 Mar 2019 16:11:03 +0000 Subject: [PATCH] [FIX] mrp: wrong reserved quantity in produce wizard Issue 1: Usecase to reproduce: BOM: 1 prod final F -> 2 unit of components C - Create a MO of 2 units - Reserve 4 units of C - Produce 1 unit of F (produce wizard) - Save - Produce 1 unit of F - Update quantity to 2 The reserved quantity in the wizard is 4 although there is 2 units already done on the first operation. So it should remain 2. It happens because the produce wizard did not take into account the 2 units already done on the first stock move line. This commit fix by using ml.product_qty - ml.qty_done each time it's needed. Issue 2: Usecase to reproduce: SAME BOM - Create a MO of 2 units - Reserve - update quantity to 3 - It creates a new line with to consume 2, reserved 0 - update quantity to 4 - It update the line 2 0 to 4 to consume 4 reserved So we ended with 8 units of C reserved. It happens because the system match the workorder line with the stock move line one by one without a global view on what is reserved or not. So the second workorder line found a move line with 4 reserved but 0 done and did not check the first workorder line that already use the 4 pieces reserved. So it considers it as available and take the reservation a second time. It's fixed by an iteration over workorder line instead of move line and keep the used reserved quantity in a dict by lot. Issue 3: Usecase to reproduce: SAME BOM - Create a MO of 2 - Produce 2 - Update quantity to 3 New line with to consume 2 reserved 0 - Update quantity to 4 New line with to consume 2 reserved 0 It's bad since we would like a line 4 0 as in saas-12.1 It happens because _update_workorder_lines will always create a new line with remaining quantity if it could'nt find one with reservation to update. However the line could exist without reservation so it should update it instead of creating a new one. It's fixed by adding the remaining quantity in the first workorder line without lot (if the tracking is serial it should continue to create a new line). --- addons/mrp/models/mrp_abstract_workorder.py | 56 ++++++++++++--------- addons/mrp/tests/test_order.py | 8 +-- 2 files changed, 35 insertions(+), 29 deletions(-) diff --git a/addons/mrp/models/mrp_abstract_workorder.py b/addons/mrp/models/mrp_abstract_workorder.py index 0738156cc7a..87cd28f1854 100644 --- a/addons/mrp/models/mrp_abstract_workorder.py +++ b/addons/mrp/models/mrp_abstract_workorder.py @@ -1,6 +1,8 @@ # -*- coding: utf-8 -*- # Part of Odoo. See LICENSE file for full copyright and licensing details. +from collections import defaultdict + from odoo import api, fields, models, _ from odoo.addons import decimal_precision as dp from odoo.exceptions import UserError @@ -81,19 +83,30 @@ class MrpAbstractWorkorder(models.AbstractModel): qty_todo = 0 else: # Search among wo lines which one could be updated - for move_line in move_raw.move_line_ids: - # Get workorder lines that match reservation. - candidates = move_workorder_lines._find_candidate(move_line) - for candidate in candidates: - if float_compare(qty_todo, 0, precision_rounding=rounding) <= 0: - break - qty_to_add = move_line.product_uom_qty - candidate.qty_done - line_values['to_update'][candidate] = { - 'qty_done': candidate.qty_done + qty_to_add, - 'qty_to_consume': candidate.qty_to_consume + qty_to_add, - 'qty_reserved': candidate.qty_reserved + qty_to_add, + qty_reserved_wl = defaultdict(float) + for workorder_line in move_workorder_lines.sorted(key=lambda wl: wl.qty_reserved, reverse=True): + rounding = workorder_line.product_uom_id.rounding + if float_compare(qty_todo, 0, precision_rounding=rounding) <= 0: + break + move_lines = workorder_line._get_move_lines() + qty_reserved_wl[workorder_line.lot_id] += workorder_line.qty_reserved + qty_reserved_remaining = sum(move_lines.mapped('product_uom_qty')) - sum(move_lines.mapped('qty_done')) - qty_reserved_wl[workorder_line.lot_id] + if float_compare(qty_reserved_remaining, 0, precision_rounding=rounding) > 0: + qty_to_add = min(qty_reserved_remaining, qty_todo) + line_values['to_update'][workorder_line] = { + 'qty_done': workorder_line.qty_done + qty_to_add, + 'qty_to_consume': workorder_line.qty_to_consume + qty_to_add, + 'qty_reserved': workorder_line.qty_reserved + qty_to_add, } qty_todo -= qty_to_add + qty_reserved_wl[workorder_line.lot_id] += qty_to_add + + if not workorder_line.qty_reserved and not workorder_line.lot_id and workorder_line.product_tracking != 'serial': + line_values['to_update'][workorder_line] = { + 'qty_done': workorder_line.qty_done + qty_todo, + 'qty_to_consume': workorder_line.qty_to_consume + qty_todo, + } + qty_todo = 0 # if there are still qty_todo, create new wo lines if float_compare(qty_todo, 0.0, precision_rounding=rounding) > 0: @@ -122,18 +135,18 @@ class MrpAbstractWorkorder(models.AbstractModel): line.lot_id == move_line.lot_id ) if linked_wo_line: - if float_compare(sum(linked_wo_line.mapped('qty_to_consume')), move_line.product_uom_qty, precision_rounding=move.product_uom.rounding) < 0: - to_consume_in_line = min(qty_to_consume, move_line.product_uom_qty - sum(linked_wo_line.mapped('qty_to_consume'))) + if float_compare(sum(linked_wo_line.mapped('qty_to_consume')), move_line.product_uom_qty - move_line.qty_done, precision_rounding=move.product_uom.rounding) < 0: + to_consume_in_line = min(qty_to_consume, move_line.product_uom_qty - move_line.qty_done - sum(linked_wo_line.mapped('qty_to_consume'))) else: continue else: - to_consume_in_line = min(qty_to_consume, move_line.product_uom_qty) + to_consume_in_line = min(qty_to_consume, move_line.product_uom_qty - move_line.qty_done) line = { 'move_id': move.id, 'product_id': move.product_id.id, 'product_uom_id': is_tracked and move.product_id.uom_id.id or move.product_uom.id, 'qty_to_consume': to_consume_in_line, - 'qty_reserved': move_line.product_uom_qty, + 'qty_reserved': to_consume_in_line, 'lot_id': move_line.lot_id.id, 'qty_done': to_consume_in_line } @@ -345,16 +358,9 @@ class MrpAbstractWorkorderLine(models.AbstractModel): return vals_list - def _find_candidate(self, move_line): - """ Method used in order to return move lines that match reservation. - The purpose is to update exisiting workorder line regarding the - reservation of the raw moves. - """ - rounding = move_line.product_uom_id.rounding - return self.filtered(lambda line: - line.lot_id == move_line.lot_id and - float_compare(line.qty_done, move_line.product_uom_qty, precision_rounding=rounding) < 0 and - line.product_id == move_line.product_id) + def _get_move_lines(self): + return self.move_id.move_line_ids.filtered(lambda ml: + ml.lot_id == self.lot_id and ml.product_id == self.product_id) # To be implemented in specific model def _get_final_lot(self): diff --git a/addons/mrp/tests/test_order.py b/addons/mrp/tests/test_order.py index 45c6e7278e2..d7421bacea1 100644 --- a/addons/mrp/tests/test_order.py +++ b/addons/mrp/tests/test_order.py @@ -620,9 +620,9 @@ class TestMrpOrder(TestMrpCommon): product_produce.do_produce() ml_p1 = mo.move_raw_ids.filtered(lambda x: x.product_id == p1).mapped('move_line_ids') - self.assertEqual(len(ml_p1), 4) + self.assertEqual(len(ml_p1), 3) for ml in ml_p1: - self.assertIn(ml.qty_done, [1.0, 2.0], 'Quantity done should be 1.0, 2.0 or 3.0') + self.assertIn(ml.qty_done, [1.0, 2.0, 3.0], 'Quantity done should be 1.0, 2.0 or 3.0') self.assertEqual(sum(ml_p1.mapped('qty_done')), 6.0, 'Total qty consumed should be 6.0') self.assertEqual(sum(ml_p1.mapped('product_uom_qty')), 5.0, 'Total qty reserved should be 5.0') @@ -691,10 +691,10 @@ class TestMrpOrder(TestMrpCommon): 'active_ids': [mo.id], })) produce_form.qty_producing = 3 - self.assertEqual(len(produce_form.workorder_line_ids._records), 4, 'Update the produce quantity should change the components quantity.') + self.assertEqual(len(produce_form.workorder_line_ids._records), 2, 'Update the produce quantity should change the components quantity.') self.assertEqual(sum([x['qty_done'] for x in produce_form.workorder_line_ids._records]), 15, 'Update the produce quantity should change the components quantity.') produce_form.qty_producing = 4 - self.assertEqual(len(produce_form.workorder_line_ids._records), 6, 'Update the produce quantity should change the components quantity.') + self.assertEqual(len(produce_form.workorder_line_ids._records), 2, 'Update the produce quantity should change the components quantity.') self.assertEqual(sum([x['qty_done'] for x in produce_form.workorder_line_ids._records]), 20, 'Update the produce quantity should change the components quantity.') produce_form.qty_producing = 1 self.assertEqual(len(produce_form.workorder_line_ids._records), 2, 'Update the produce quantity should change the components quantity.')