Skip to content
Snippets Groups Projects
Commit 89814281 authored by Adrien Widart's avatar Adrien Widart
Browse files

[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#80373

Signed-off-by: default avatarWilliam Henrotin (whe) <whe@odoo.com>
parent 0cfe0b58
No related branches found
No related tags found
No related merge requests found
......@@ -2,3 +2,4 @@
# Part of Odoo. See LICENSE file for full copyright and licensing details.
from . import stock_picking
from . import stock_move
# -*- 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()
......@@ -22,7 +22,7 @@ class TestAccountSubcontractingFlows(TestMrpSubcontractingCommon):
self.customer_location = self.env.ref('stock.stock_location_customers')
self.supplier_location = self.env.ref('stock.stock_location_suppliers')
self.uom_unit = self.env.ref('uom.product_uom_unit')
self.env.ref('product.product_category_all').property_cost_method = 'fifo'
self.env.ref('product.product_category_all').property_cost_method = 'average'
self.env.ref('product.product_category_all').property_valuation = 'real_time'
# IN 10@10 comp1 10@20 comp2
......@@ -75,3 +75,16 @@ class TestAccountSubcontractingFlows(TestMrpSubcontractingCommon):
self.assertEqual(picking_receipt.move_lines.stock_valuation_layer_ids.value, 60)
self.assertEqual(picking_receipt.move_lines.product_id.value_svl, 60)
self.assertEqual(picking_receipt.move_lines.stock_valuation_layer_ids.account_move_id.amount_total, 60)
# Do the same without any additionnal cost
picking_receipt = picking_receipt.copy()
picking_receipt.move_lines.price_unit = 0
picking_receipt.action_confirm()
picking_receipt.move_lines.quantity_done = 1.0
picking_receipt.action_done()
# 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(picking_receipt.move_lines.stock_valuation_layer_ids.value, 30)
self.assertEqual(picking_receipt.move_lines.product_id.value_svl, 90)
......@@ -28,6 +28,10 @@ class StockMove(models.Model):
action_data['domain'] = [('id', 'in', self.account_move_ids.ids)]
return action_data
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()
......@@ -36,7 +40,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):
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment