From 4ee5df76607c80c94a4aea832dcd2f73ad8e5ee1 Mon Sep 17 00:00:00 2001 From: Adrien Widart Date: Wed, 17 Aug 2022 11:27:26 +0000 Subject: [PATCH] [FIX] sale_mrp: decrease kit quantity after the delivery When decreasing the SOL qty, if the SOL product is a kit and if the kit has already been delivered+returned, an additional and useless return will be created To reproduce the issue: 1. Create two products consumable products P_kit, P_compo 2. Create a bill of materials: - Product: P_kit - Type: Kit - Components: 1 x P_compo 3. Create and confirm a sale order SO with 4 x P_kit 4. Process the delivery 5. Process a return with 1 x P_compo 6. Edit SO: - Quantity: 3 (instead of 4) Error: once the SO is saved, a second (and useless) return will be created When updating the SOL quantity, we try to adapt the pickings. To do so, we first get the quantity that is currently in delivery, then we compute the difference between that quantity and the new SOL qty. Eventually, we create a procurement based on that difference. https://github.com/odoo/odoo/blob/e11dfa341baf82fedee753502aef5ef2972f75cb/addons/sale_stock/models/sale_order.py#L553 https://github.com/odoo/odoo/blob/e11dfa341baf82fedee753502aef5ef2972f75cb/addons/sale_stock/models/sale_order.py#L573-L578 In case of a kit, the "in delivery" quantity is too simply computed: we return the old SOL qty (see diff). So, in the above case, we have: - In delivery : 4 - New sol qty : 3 - Difference : -1 That's the reason why a new return is created, to fulfill the difference. We could apply a smarter computation of the "in delivery" qty thanks to a mix between the computation of the delivered quantity of a kit: https://github.com/odoo/odoo/blob/e85de8f2d342ac1e69d8eb4020dbb2d4c69ad90b/addons/sale_mrp/models/sale.py#L111-L118 and the way we compute the "in delivery" quantity for a non-kit product: https://github.com/odoo/odoo/blob/e11dfa341baf82fedee753502aef5ef2972f75cb/addons/sale_stock/models/sale_order.py#L501-L504 https://github.com/odoo/odoo/blob/e11dfa341baf82fedee753502aef5ef2972f75cb/addons/sale_stock/models/sale_order.py#L511-L515 Note: if the kit has several components, and if the user does not return an entire kit, a similar issue will occur. That's the reason why this commit solution is smarter than the current code but is not the smartest. For such a use case, we would need a bigger solution/refactoring (handle the components one by one instead of the kit for the quantities and the procurements) OPW-2917209 closes odoo/odoo#98899 X-original-commit: 9649e1031df60c5ba60424e9fbcf7187f6783b5c Signed-off-by: Tiffany Chang Signed-off-by: Adrien Widart --- addons/sale_mrp/models/sale_order_line.py | 14 ++-- addons/sale_mrp/tests/test_sale_mrp_flow.py | 80 +++++++++++++++++++++ 2 files changed, 90 insertions(+), 4 deletions(-) diff --git a/addons/sale_mrp/models/sale_order_line.py b/addons/sale_mrp/models/sale_order_line.py index 9a1b499798c..85cbca703f2 100644 --- a/addons/sale_mrp/models/sale_order_line.py +++ b/addons/sale_mrp/models/sale_order_line.py @@ -109,10 +109,16 @@ class SaleOrderLine(models.Model): def _get_qty_procurement(self, previous_product_uom_qty=False): self.ensure_one() # Specific case when we change the qty on a SO for a kit product. - # We don't try to be too smart and keep a simple approach: we compare the quantity before - # and after update, and return the difference. We don't take into account what was already - # sent, or any other exceptional case. + # We don't try to be too smart and keep a simple approach: we use the quantity of entire + # kits that are currently in delivery bom = self.env['mrp.bom']._bom_find(self.product_id, bom_type='phantom')[self.product_id] if bom: - return previous_product_uom_qty and previous_product_uom_qty.get(self.id, 0.0) or self.qty_delivered + moves = self.move_ids.filtered(lambda r: r.state != 'cancel' and not r.scrapped) + filters = { + 'incoming_moves': lambda m: m.location_dest_id.usage == 'customer' and (not m.origin_returned_move_id or (m.origin_returned_move_id and m.to_refund)), + 'outgoing_moves': lambda m: m.location_dest_id.usage != 'customer' and m.to_refund + } + order_qty = self.product_uom._compute_quantity(self.product_uom_qty, bom.product_uom_id) + qty = moves._compute_kit_quantities(self.product_id, order_qty, bom, filters) + return bom.product_uom_id._compute_quantity(qty, self.product_uom) return super(SaleOrderLine, self)._get_qty_procurement(previous_product_uom_qty=previous_product_uom_qty) diff --git a/addons/sale_mrp/tests/test_sale_mrp_flow.py b/addons/sale_mrp/tests/test_sale_mrp_flow.py index b001b679dd4..e1b596a7fc0 100644 --- a/addons/sale_mrp/tests/test_sale_mrp_flow.py +++ b/addons/sale_mrp/tests/test_sale_mrp_flow.py @@ -43,6 +43,12 @@ class TestSaleMrpFlow(ValuationReconciliationTestCommon): cls.uom_unit.write({ 'name': 'Test-Unit', 'rounding': 0.01}) + cls.uom_ten = cls.UoM.create({ + 'name': 'Test-Ten', + 'category_id': cls.categ_unit.id, + 'factor_inv': 10, + 'uom_type': 'bigger', + 'rounding': 0.001}) cls.uom_dozen = cls.UoM.create({ 'name': 'Test-DozenA', 'category_id': cls.categ_unit.id, @@ -1848,3 +1854,77 @@ class TestSaleMrpFlow(ValuationReconciliationTestCommon): price = line.product_id.with_company(line.company_id)._compute_average_price(0, line.product_uom_qty, line.move_ids) self.assertEqual(price, 10) + + def test_kit_decrease_sol_qty(self): + """ + Create and confirm a SO with a qty. Increasing/Decreasing the SOL qty + should update the qty on the delivery. Then, process the delivery, make + a return and adapt the SOL qty -> there should not be any new picking + """ + stock_location = self.company_data['default_warehouse'].lot_stock_id + custo_location = self.env.ref('stock.stock_location_customers') + + grp_uom = self.env.ref('uom.group_uom') + self.env.user.write({'groups_id': [(4, grp_uom.id)]}) + + # 100 kit_3 = 100 x compo_f + 200 x compo_g + self.env['stock.quant']._update_available_quantity(self.component_f, stock_location, 100) + self.env['stock.quant']._update_available_quantity(self.component_g, stock_location, 200) + + 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 = self.kit_3 + line.product_uom_qty = 7 + line.product_uom = self.uom_ten + so = so_form.save() + so.action_confirm() + + delivery = so.picking_ids + self.assertRecordValues(delivery.move_ids, [ + {'product_id': self.component_f.id, 'product_uom_qty': 70}, + {'product_id': self.component_g.id, 'product_uom_qty': 140}, + ]) + + # Decrease + with Form(so) as so_form: + with so_form.order_line.edit(0) as line: + line.product_uom_qty = 6 + self.assertRecordValues(delivery.move_ids, [ + {'product_id': self.component_f.id, 'product_uom_qty': 60}, + {'product_id': self.component_g.id, 'product_uom_qty': 120}, + ]) + + # Increase + with Form(so) as so_form: + with so_form.order_line.edit(0) as line: + line.product_uom_qty = 10 + self.assertRecordValues(delivery.move_ids, [ + {'product_id': self.component_f.id, 'product_uom_qty': 100}, + {'product_id': self.component_g.id, 'product_uom_qty': 200}, + ]) + + delivery.action_set_quantities_to_reservation() + delivery.button_validate() + + # Return 2 [uom_ten] x kit_3 + return_wizard_form = Form(self.env['stock.return.picking'].with_context(active_ids=delivery.ids, active_id=delivery.id, active_model='stock.picking')) + return_wizard = return_wizard_form.save() + return_wizard.product_return_moves[0].quantity = 20 + return_wizard.product_return_moves[1].quantity = 40 + action = return_wizard.create_returns() + return_picking = self.env['stock.picking'].browse(action['res_id']) + return_picking.action_set_quantities_to_reservation() + return_picking.button_validate() + + # Adapt the SOL qty according to the delivered one + with Form(so) as so_form: + with so_form.order_line.edit(0) as line: + line.product_uom_qty = 8 + + self.assertRecordValues(so.picking_ids.move_ids, [ + {'product_id': self.component_f.id, 'location_dest_id': custo_location.id, 'quantity_done': 100, 'state': 'done'}, + {'product_id': self.component_g.id, 'location_dest_id': custo_location.id, 'quantity_done': 200, 'state': 'done'}, + {'product_id': self.component_f.id, 'location_dest_id': stock_location.id, 'quantity_done': 20, 'state': 'done'}, + {'product_id': self.component_g.id, 'location_dest_id': stock_location.id, 'quantity_done': 40, 'state': 'done'}, + ])