From 1db43edebb0db598869f0bcef9739c0d9f4df0d9 Mon Sep 17 00:00:00 2001 From: "Tiffany Chang (tic)" Date: Tue, 8 Aug 2023 12:13:24 +0200 Subject: [PATCH] [FIX] mrp: correctly set consumption warning values Previous fix odoo/odoo#121602 did not correctly handle the case when not all of the qty to manufacture is manufactured (the qtys to change to were miscalculated in this case). Additionally, it missed fixing a few more use cases when setting the qtys to match the wizard's lines/qtys: - if the UoM of a MO's component line is changed => the correct qty was not correctly converted into the move.product_uom's qty (now it is) - if a component's move is deleted before the MO is confirmed => the move (i.e. the missing component) was not correctly added back into the MO (now it is) - if there are 2 MO component moves with the same product => both were set to the same "correct qty" value (now we only set the first move to that qty, others are set to 0 since we have no way of knowing how to distribute the qtys otherwise) Also, since an UserError needed to be added in case of a missing comp move for a tracked product, existing error logic has been updated to list all applicable products and the message has been improved to be more helpful. Note that the fix for saas-16.4 and earlier is slightly different from this fix due to the change in how stock move original demand qtys no longer change like they used to (see: odoo/odoo#130342) and because a backorder bug was also exposed by this change. closes odoo/odoo#135796 Task: 3456604 Signed-off-by: Arnold Moyaux (arm) Signed-off-by: Tiffany Chang (tic) --- addons/mrp/i18n/mrp.pot | 16 ++- addons/mrp/tests/test_order.py | 131 +++++++++++++++++-- addons/mrp/wizard/mrp_consumption_warning.py | 53 +++++--- 3 files changed, 165 insertions(+), 35 deletions(-) diff --git a/addons/mrp/i18n/mrp.pot b/addons/mrp/i18n/mrp.pot index d525d6dbcbc..f52c2404070 100644 --- a/addons/mrp/i18n/mrp.pot +++ b/addons/mrp/i18n/mrp.pot @@ -5546,6 +5546,15 @@ msgstr "" msgid "Validate" msgstr "" +#. module: mrp +#. odoo-python +#: code:addons/addons/mrp/wizard/mrp_consumption_warning.py:0 +#: code:addons/mrp/wizard/mrp_consumption_warning.py:0 +#, python-format +msgid "" +"Values cannot be set and validated because a Lot/Serial Number needs to be specified for a tracked product that is having its consumed amount increased:\n" +"- " + #. module: mrp #. odoo-javascript #: code:addons/mrp/static/src/components/bom_overview_control_panel/mrp_bom_overview_control_panel.xml:0 @@ -6194,13 +6203,6 @@ msgstr "" msgid "You need to provide a lot for the finished product." msgstr "" -#. module: mrp -#. odoo-python -#: code:addons/mrp/wizard/mrp_consumption_warning.py:0 -#, python-format -msgid "You need to supply Lot/Serial Number" -msgstr "" - #. module: mrp #. odoo-python #: code:addons/mrp/models/mrp_production.py:0 diff --git a/addons/mrp/tests/test_order.py b/addons/mrp/tests/test_order.py index c4bd05fdc7f..513020ba129 100644 --- a/addons/mrp/tests/test_order.py +++ b/addons/mrp/tests/test_order.py @@ -3378,25 +3378,130 @@ class TestMrpOrder(TestMrpCommon): def test_consumption_action_set_qty_and_validate(self): """ - Check `To consume` and `consumed` qty should be updated as per the consumption warning + Check `To Consume` and `Consumed` qty are correctly updated to match the consumption warning values + under 4 use cases: + scenario 1: + - bom is changed after MO is created => action_set_qty = match BoM + scenario 2 (combined 2 use cases since they shouldn't affect each other): + - a component move is deleted before MO is confirmed => action_set_qty = add missing BoM component + - a component's UoM is changed after MO is created => action_set_qty = match BoM qty, but leave UoM unchanged (i.e. correctly convert) + scenario 3: + - a component has 2 moves in a MO => action_set_qty = set the 1st move to the correct qty, set 2nd move to 0 + (i.e. no way to know how to distribute qty_done across these moves since warning aggregates qty by product) """ - mo, bom, _p_final, _p1, _p2 = self.generate_mo(consumption='warning', qty_final=10) + mo, bom, p_final, p1, p2 = self.generate_mo(consumption='warning', qty_final=10, qty_base_1=12, qty_base_2=20) + + #### scenario 1 - change BoM after MO created #### mo_form = Form(mo) - mo_form.qty_producing = 10.0 + mo_form.qty_producing = 4 mo = mo_form.save() - - bom.bom_line_ids[0].product_qty = 3 - - self.assertEqual(mo.move_raw_ids[0].product_uom_qty, 10) - self.assertEqual(mo.move_raw_ids[0].quantity_done, 10) + # mo.move_raw_ids[0] = p2 => 20 qty_base, mo.move_raw_ids[1] = p1 => 12 qty_base + self.assertEqual(mo.move_raw_ids[0].product_uom_qty, 200, "current MO To Consume qty should match expected qty to produce") + self.assertEqual(mo.move_raw_ids[0].quantity_done, 80, "current MO Consumed qty should match expected qty to produce") + self.assertEqual(mo.move_raw_ids[1].product_uom_qty, 120, "current MO To Consume qty should match expected qty produced") + self.assertEqual(mo.move_raw_ids[1].quantity_done, 48, "current MO Consumed qty should match expected qty produced") + # bom changes won't auto-update MO, it will only show diff in consumption warning + bom.bom_line_ids[0].product_qty = 10 + self.assertEqual(mo.move_raw_ids[0].product_uom_qty, 200) + self.assertEqual(mo.move_raw_ids[0].quantity_done, 80) action = mo.button_mark_done() warning = Form(self.env['mrp.consumption.warning'].with_context(**action['context'])) consumption = warning.save() - self.assertEqual(consumption.mrp_consumption_warning_line_ids.product_consumed_qty_uom, 10) - self.assertEqual(consumption.mrp_consumption_warning_line_ids.product_expected_qty_uom, 30) - consumption.action_set_qty() - self.assertEqual(mo.move_raw_ids[0].product_uom_qty, 30) - self.assertEqual(mo.move_raw_ids[0].quantity_done, 30) + self.assertEqual(consumption.mrp_consumption_warning_line_ids.product_consumed_qty_uom, 80, "qty consumed incorrectly passed to wizard") + self.assertEqual(consumption.mrp_consumption_warning_line_ids.product_expected_qty_uom, 40, "expected qty should match current BoM qty for qty being produced") + action = consumption.action_set_qty() + backorder = Form(self.env['mrp.production.backorder'].with_context(**action['context'])) + backorder.save().action_backorder() + self.assertEqual(mo.move_raw_ids[0].product_uom_qty, 80, "current bom expected qty should remain unchanged") + self.assertEqual(mo.move_raw_ids[0].quantity_done, 40, "current bom expected qty was not applied as qty to be done") + self.assertEqual(mo.move_raw_ids[1].product_uom_qty, 48, "line without consumption issue was incorrectly changed") + self.assertEqual(mo.move_raw_ids[1].quantity_done, 48, "line without consumption issue was incorrectly changed") + self.assertEqual(mo.state, 'done') + # double check that backorder qtys are also correct + mo_backorder = mo.procurement_group_id.mrp_production_ids[-1] + self.assertEqual(mo_backorder.move_raw_ids[0].product_uom_qty, 120, "backorder values are based on original MO, not current bom") + self.assertEqual(mo_backorder.move_raw_ids[1].product_uom_qty, 72, "backorder values incorrectly calculated") + + #### scenario 2 - that removing a line in the MO + changing the uom of a line #### + mo2_form = Form(self.env['mrp.production']) + mo2_form.product_id = p_final + mo2_form.bom_id = bom + mo2_form.product_qty = 5.0 + mo2 = mo2_form.save() + for move in mo2.move_raw_ids: + if move.product_id == p2: + move.unlink() + else: + # p1 = qty_base_1 = 12 => now 12 dozens instead of units + move.product_uom = self.env.ref('uom.product_uom_dozen') + mo2.action_confirm() + mo2_form = Form(mo2) + mo2_form.qty_producing = 4 + mo2 = mo2_form.save() + self.assertEqual(len(mo2.move_raw_ids), 1, "current MO should still have 1 component from its BoM deleted") + self.assertEqual(mo2.move_raw_ids[0].product_uom_qty, 60, "current MO To Consume qty should match manually set expected qty produced") + self.assertEqual(mo2.move_raw_ids[0].quantity_done, 48, "current MO Consumed qty should match expected qty to produce based on manually set value") + + action = mo2.button_mark_done() + warning = Form(self.env['mrp.consumption.warning'].with_context(**action['context'])) + consumption = warning.save() + self.assertEqual(len(consumption.mrp_consumption_warning_line_ids), 2, "deleted move should also show as an consumption line diff from BoM") + # mrp_consumption_warning_line_ids[1] = p1 => 12 unit qty_base, mrp_consumption_warning_line_ids[0] = p2 => 10 unit qty_base + self.assertEqual(consumption.mrp_consumption_warning_line_ids[0].product_consumed_qty_uom, 0, "missing line was not correctly passed to wizard") + self.assertEqual(consumption.mrp_consumption_warning_line_ids[0].product_expected_qty_uom, 40, "expected qty should match current BoM qty for qty being produced") + self.assertEqual(consumption.mrp_consumption_warning_line_ids[1].product_consumed_qty_uom, 576, "qty consumed was not correctly converted to product's uom before passing to wizard") + self.assertEqual(consumption.mrp_consumption_warning_line_ids[1].product_expected_qty_uom, 48, "expected qty should match current BoM qty for qty being produced") + action = consumption.action_set_qty() + backorder2 = Form(self.env['mrp.production.backorder'].with_context(**action['context'])) + backorder2.save().action_backorder() + # expect 2 moves: 1 for the originally missing product p2 with qty demand/done = 40 + # 1 for the overused product p1 one with qty demand/done = 48/12 = 4 dozens + self.assertEqual(len(mo2.move_raw_ids), 2, "missing line was not correctly added") + for move in mo2.move_raw_ids: + if move.product_id == p2: + self.assertEqual(move.product_uom_qty, 40, "missing line values were not correctly added") + self.assertEqual(move.quantity_done, 40, "missing line values were not correctly added") + else: + self.assertEqual(move.product_uom_qty, 48, "expected qty should be unchanged") + self.assertEqual(move.quantity_done, 4, "expected qty was not applied as qty to be done (UoM was possibly not correctly converted)") + self.assertEqual(mo2.state, 'done') + + #### scenario 3 - repeated comp move #### + # bom.bom_line_ids[0]/product_id = p2 + bom.bom_line_ids[0].unlink() + mo3 = self.env['mrp.production'].create({ + 'product_id': p_final.id, + 'bom_id': bom.id, + 'product_qty': 1, + 'product_uom_id': p_final.uom_id.id, + }) + mo3_form = Form(mo3) + with mo3_form.move_raw_ids.new() as line: + line.product_id = p1 + line.product_uom_qty = 5 + mo3 = mo3_form.save() + mo3.action_confirm() + self.assertEqual(len(mo3.move_raw_ids), 2, "there should be 2 comp lines") + self.assertEqual(len(mo3.move_raw_ids.product_id), 1, "comp lines should have same product") + mo3_form = Form(mo3) + mo3_form.qty_producing = 1 + mo3 = mo3_form.save() + self.assertEqual(mo3.move_raw_ids[0].product_uom_qty, 12, "BoM created comp move does not match expected To Consume qty") + self.assertEqual(mo3.move_raw_ids[0].quantity_done, 12, "BoM created comp move does not match expected Consumed qty") + self.assertEqual(mo3.move_raw_ids[1].product_uom_qty, 5, "Manually added comp move does not match original To Consume qty") + self.assertEqual(mo3.move_raw_ids[1].quantity_done, 5, "Manually added comp move was not Consumed") + action = mo3.button_mark_done() + warning = Form(self.env['mrp.consumption.warning'].with_context(**action['context'])) + consumption = warning.save() + self.assertEqual(len(consumption.mrp_consumption_warning_line_ids), 1, "warning lines should be grouped by product") + self.assertEqual(consumption.mrp_consumption_warning_line_ids[0].product_expected_qty_uom, 12, "BoM expected qty not correctly passed to wizard") + self.assertEqual(consumption.mrp_consumption_warning_line_ids[0].product_consumed_qty_uom, 17, "total Consumed qty not correctly passed to wizard") + action = consumption.action_set_qty() + self.assertEqual(mo3.move_raw_ids[0].product_uom_qty, 12, "BoM created comp move does not match expected To Consume qty") + self.assertEqual(mo3.move_raw_ids[0].quantity_done, 12, "BoM created comp move does not match expected Consumed qty") + self.assertEqual(mo3.move_raw_ids[1].product_uom_qty, 5, "Manually added comp move To Consume qty should be unchanged") + self.assertEqual(mo3.move_raw_ids[1].quantity_done, 0, "Extra line Consumed qty not correctly zero-ed") + self.assertEqual(mo3.state, 'done') def test_exceeded_consumed_qty_and_duplicated_lines(self): """ diff --git a/addons/mrp/wizard/mrp_consumption_warning.py b/addons/mrp/wizard/mrp_consumption_warning.py index b6b8e5b86a2..a7dfbf650d0 100644 --- a/addons/mrp/wizard/mrp_consumption_warning.py +++ b/addons/mrp/wizard/mrp_consumption_warning.py @@ -3,7 +3,7 @@ from odoo import _, fields, models, api from odoo.exceptions import UserError -from odoo.tools import float_compare, float_round +from odoo.tools import float_compare, float_is_zero class MrpConsumptionWarning(models.TransientModel): @@ -36,23 +36,46 @@ class MrpConsumptionWarning(models.TransientModel): return self.mrp_production_ids.with_context(ctx, skip_consumption=True).button_mark_done() def action_set_qty(self): - self.mrp_production_ids.action_assign() + missing_move_vals = [] + problem_tracked_products = self.env['product.product'] for production in self.mrp_production_ids: - for move in production.move_raw_ids: - rounding = move.product_uom.rounding - for line in self.mrp_consumption_warning_line_ids: + for line in self.mrp_consumption_warning_line_ids: + if line.mrp_production_id != production: + continue + for move in production.move_raw_ids: if line.product_id != move.product_id: continue - if float_compare(line.product_expected_qty_uom, move.product_uom_qty, precision_rounding=rounding) != 0: - move.product_uom_qty = line.product_uom_id._compute_quantity(line.product_expected_qty_uom, move.product_uom) - if float_compare(move.quantity_done, move.should_consume_qty, precision_rounding=rounding) == 0: - continue - new_qty = float_round((production.qty_producing - production.qty_produced) * move.unit_factor, precision_rounding=move.product_uom.rounding) - if move.has_tracking in ('lot', 'serial'): - if not (production.use_auto_consume_components_lots and - float_compare(move.reserved_availability, new_qty, precision_rounding=move.product_uom.rounding) >= 0): - raise UserError(_('You need to supply Lot/Serial Number')) - move.quantity_done = new_qty + qty_expected = line.product_uom_id._compute_quantity(line.product_expected_qty_uom, move.product_uom) + qty_compare_result = float_compare(qty_expected, move.quantity_done, precision_rounding=move.product_uom.rounding) + if qty_compare_result != 0: + if (move.has_tracking in ('lot', 'serial') + and not production.use_auto_consume_components_lots + and qty_compare_result > 0): + problem_tracked_products |= line.product_id + break + move.quantity_done = qty_expected + # in case multiple lines with same product => set others to 0 since we have no way to know how to distribute the qty done + line.product_expected_qty_uom = 0 + # move was deleted before confirming MO or force deleted somehow + if not float_is_zero(line.product_expected_qty_uom, precision_rounding=line.product_uom_id.rounding): + if line.product_id.tracking in ('lot', 'serial') and not line.mrp_production_id.use_auto_consume_components_lots: + problem_tracked_products |= line.product_id + continue + missing_move_vals.append({ + 'product_id': line.product_id.id, + 'product_uom': line.product_uom_id.id, + 'product_uom_qty': line.product_expected_qty_uom, + 'quantity_done': line.product_expected_qty_uom, + 'raw_material_production_id': line.mrp_production_id.id, + 'additional': True, + }) + if problem_tracked_products: + raise UserError( + _("Values cannot be set and validated because a Lot/Serial Number needs to be specified for a tracked product that is having its consumed amount increased:\n- ") + + "\n- ".join(problem_tracked_products.mapped('name')) + ) + if missing_move_vals: + self.env['stock.move'].create(missing_move_vals) return self.action_confirm() def action_cancel(self):