From dc54341dc0393a754f7e8f2e288705e4e18d10b2 Mon Sep 17 00:00:00 2001 From: Adrien Widart Date: Thu, 25 Nov 2021 09:52:32 +0000 Subject: [PATCH] [FIX] mrp_subcontracting_account: always return PU of subcontracted SM When using AVCO and subcontracting without any additional cost, the valuation of the finished product is incorrect To reproduce the issue: 1. Create a Product Category PC: - Costing Method: AVCO 2. Create two products P_compo and P_finished: - Both: - Type: Storable - Category: PC - P_compo: - Cost: 10 - Routes: Resupply Subcontractor 3. Update P_compo's quantity: 2 4. Create a Bill of Materials BO: - Product: P_finished - Type: Subcontracting - Subcontractor: a partner P - Component: 1 x P_compo 5. In Inventory, create a planned Receipt R: - From: P - Operations: 1 x P_finished 6. Mark R as To Do 7. Process the delivery of P_compo 8. Validate R 9. Repeat steps 5-8 10. Open the Inventory Valuation Error: There are two valuation lines for P_finished: one line has a value of $10, which is correct (this is the cost of the component). However, the value of the second line is $20, it should be $10 too. Here are a part of the values used to generate the related MO: https://github.com/odoo/odoo/blob/2d12fb8fb94c0f2acade7222cfedbec34114a8e9/addons/mrp_subcontracting_account/models/stock_picking.py#L21-L25 In the above case, an extra cost is defined on the MO and is based on the unit price of the subcontracted SM. However, `_get_price_unit` will return an incorrect value: https://github.com/odoo/odoo/blob/251be6b943ea8c3f274bb0863d0af3f7c6b8d10d/addons/stock_account/models/stock_move.py#L39 After step 8, the standard price of P_finished is $10. Also, in the above case, there isn't any subcontracting cost, so there isn't any unit price defined on the SM. Therefore, `_get_price_unit` returns the standard price of P_finished ($10). As a result, when computing the value of the finished stock move: https://github.com/odoo/odoo/blob/4fc2ec31861d4602357f130066ec3507b96d8dc8/addons/mrp_account/models/mrp_production.py#L39-L42 It uses the extra cost of the MO + the component cost. This explains where the $20 come from. OPW-2641339 closes odoo/odoo#80889 X-original-commit: 0567536f8b66c71f6860041926542f88026793b1 Signed-off-by: William Henrotin (whe) Signed-off-by: Adrien Widart --- .../models/__init__.py | 1 + .../models/stock_move.py | 12 +++++++++++ .../tests/test_subcontracting_account.py | 20 +++++++++++++++++++ addons/stock_account/models/stock_move.py | 6 +++++- 4 files changed, 38 insertions(+), 1 deletion(-) create mode 100644 addons/mrp_subcontracting_account/models/stock_move.py diff --git a/addons/mrp_subcontracting_account/models/__init__.py b/addons/mrp_subcontracting_account/models/__init__.py index aee2f0054db..c65666fb491 100644 --- a/addons/mrp_subcontracting_account/models/__init__.py +++ b/addons/mrp_subcontracting_account/models/__init__.py @@ -3,3 +3,4 @@ from . import stock_picking from . import product_product +from . import stock_move diff --git a/addons/mrp_subcontracting_account/models/stock_move.py b/addons/mrp_subcontracting_account/models/stock_move.py new file mode 100644 index 00000000000..d57c2c94df8 --- /dev/null +++ b/addons/mrp_subcontracting_account/models/stock_move.py @@ -0,0 +1,12 @@ +# -*- coding: utf-8 -*- +# Part of Odoo. See LICENSE file for full copyright and licensing details. + +from odoo import models + + +class StockMove(models.Model): + _inherit = 'stock.move' + + def _should_force_price_unit(self): + self.ensure_one() + return self.is_subcontract or super()._should_force_price_unit() diff --git a/addons/mrp_subcontracting_account/tests/test_subcontracting_account.py b/addons/mrp_subcontracting_account/tests/test_subcontracting_account.py index 7a1b38ec87f..60a1da5d0c7 100644 --- a/addons/mrp_subcontracting_account/tests/test_subcontracting_account.py +++ b/addons/mrp_subcontracting_account/tests/test_subcontracting_account.py @@ -68,6 +68,26 @@ class TestAccountSubcontractingFlows(TestMrpSubcontractingCommon): self.assertEqual(picking_receipt.move_ids.stock_valuation_layer_ids.value, 0) self.assertEqual(picking_receipt.move_ids.product_id.value_svl, 60) + # Do the same without any additionnal cost + 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 = self.finished + move.product_uom_qty = 1 + picking_receipt = picking_form.save() + picking_receipt.move_ids.price_unit = 0 + + picking_receipt.action_confirm() + picking_receipt.move_ids.quantity_done = 1.0 + picking_receipt._action_done() + + mo = picking_receipt._get_subcontract_production() + # In this case, since there isn't any additionnal cost, the total cost of the subcontracting + # is the sum of the components' costs: 10 + 20 = 30 + self.assertEqual(mo.move_finished_ids.stock_valuation_layer_ids.value, 30) + self.assertEqual(picking_receipt.move_ids.product_id.value_svl, 90) + def test_subcontracting_account_backorder(self): """ This test uses tracked (serial and lot) component and tracked (serial) finished product The original subcontracting production order will be split into 4 backorders. This test diff --git a/addons/stock_account/models/stock_move.py b/addons/stock_account/models/stock_move.py index 82eec648948..7ba1144f46c 100644 --- a/addons/stock_account/models/stock_move.py +++ b/addons/stock_account/models/stock_move.py @@ -34,6 +34,10 @@ class StockMove(models.Model): self.analytic_account_line_id.unlink() return super()._action_cancel() + def _should_force_price_unit(self): + self.ensure_one() + return False + def _get_price_unit(self): """ Returns the unit price to value this stock move """ self.ensure_one() @@ -42,7 +46,7 @@ class StockMove(models.Model): # If the move is a return, use the original move's price unit. if self.origin_returned_move_id and self.origin_returned_move_id.sudo().stock_valuation_layer_ids: price_unit = self.origin_returned_move_id.sudo().stock_valuation_layer_ids[-1].unit_cost - return not float_is_zero(price_unit, precision) and price_unit or self.product_id.standard_price + return price_unit if not float_is_zero(price_unit, precision) or self._should_force_price_unit() else self.product_id.standard_price @api.model def _get_valued_types(self):