From c2bfba7ff29c9b521a2bb60a3bc5ffb9691d07b6 Mon Sep 17 00:00:00 2001 From: Adrien Widart Date: Fri, 31 Dec 2021 10:30:49 +0000 Subject: [PATCH] [FIX] purchase_{stock,mrp}: compute price difference of a kit In automated-AVCO configuration, buying a kit at a higher price than its cost can create inconsistencies in the accounting. To reproduce the issue: (Need account_accountant. Use demo data) 1. Create a product category PC: - Costing Method: AVCO - Inventory Valuation: Automated - Set up the Price Difference Account PDA 2. Create 3 products P_kit, P_compo01, P_compo02 - Type: Storable - Category: PC - P_compo01: - Cost: 10 - P_compo02: - Cost: 20 3. Create a bill of materials: - Product: P_kit - Type: Kit - Components: - 1 x P_compo01 - 1 x P_compo02 4. On P_kit's form, "Compute Price from BoM": - The cost should be $30 5. Create a purchase order PO with one line: - Product: P_kit - Quantity: 1 - Unit Price: 100 6. Confirm PO and process the receipt 7. Create and Post the bill Error: There is an error in the journal items of the bill: the value for PDA is $85 When posting the bill, for each account move line, the module computes the stock valuation of the associated product and the price difference. To do so, it sums the valuation of all related outgoing stock moves and divides by the quantity to get the value per unit, then it compares with the unit price used on the PO's line. Here is the issue: in case of a kit, there is one outgoing move per component while the PO's line is linked to the kit itself. Therefore, in the above case, it uses the outgoing moves of P_compo01 and P_compo02, adds up their value ($10 + $20 = $30) and then divides by the total quantity (one P_compo01 and one P_compo02, thus $30 / 2 = $15). This is the reason why it considers that the unit value of P_kit equals $15. Then, since the unit price on the PO's line is $100, it gets a price difference value equal to $85. When comparing the unit value of the kit and its unit price, the unit value should not be divided by the quantity of components ($30 should not be divided by 2). Moreover, when buying such a kit at $100, the surplus ($70) should be distributed among each component. However, it is difficult to define a rule to correctly weight this distribution. Therefore, this surplus will be considered as a price difference. OPW-2566546 closes odoo/odoo#82463 X-original-commit: 20888055d4271bf3cf9e7bc3d42150a1f72e4495 Signed-off-by: Arnold Moyaux Signed-off-by: Adrien Widart --- addons/purchase_mrp/models/stock_move.py | 14 ++++ addons/purchase_mrp/tests/__init__.py | 1 + .../tests/test_anglo_saxon_valuation.py | 77 +++++++++++++++++++ .../purchase_stock/models/account_invoice.py | 21 +---- addons/purchase_stock/models/stock.py | 24 +++++- 5 files changed, 117 insertions(+), 20 deletions(-) create mode 100644 addons/purchase_mrp/tests/test_anglo_saxon_valuation.py diff --git a/addons/purchase_mrp/models/stock_move.py b/addons/purchase_mrp/models/stock_move.py index 849a2354d7b..abcbdb026f0 100644 --- a/addons/purchase_mrp/models/stock_move.py +++ b/addons/purchase_mrp/models/stock_move.py @@ -12,3 +12,17 @@ class StockMove(models.Model): if self.purchase_line_id: vals['purchase_line_id'] = self.purchase_line_id.id return vals + + def _get_valuation_price_and_qty(self, related_aml, to_curr): + valuation_price_unit_total, valuation_total_qty = super()._get_valuation_price_and_qty(related_aml, to_curr) + boms = self.env['mrp.bom']._bom_find(related_aml.product_id, company_id=related_aml.company_id.id, bom_type='phantom') + if related_aml.product_id in boms: + kit_bom = boms[related_aml.product_id] + order_qty = related_aml.product_id.uom_id._compute_quantity(related_aml.quantity, kit_bom.product_uom_id) + filters = { + 'incoming_moves': lambda m: m.location_id.usage == 'supplier' and (not m.origin_returned_move_id or (m.origin_returned_move_id and m.to_refund)), + 'outgoing_moves': lambda m: m.location_id.usage != 'supplier' and m.to_refund + } + valuation_total_qty = self._compute_kit_quantities(related_aml.product_id, order_qty, kit_bom, filters) + valuation_total_qty = kit_bom.product_uom_id._compute_quantity(valuation_total_qty, related_aml.product_id.uom_id) + return valuation_price_unit_total, valuation_total_qty diff --git a/addons/purchase_mrp/tests/__init__.py b/addons/purchase_mrp/tests/__init__.py index 45619da7f61..5ba019d85f1 100644 --- a/addons/purchase_mrp/tests/__init__.py +++ b/addons/purchase_mrp/tests/__init__.py @@ -2,3 +2,4 @@ # Part of Odoo. See LICENSE file for full copyright and licensing details. from . import test_purchase_mrp_flow +from . import test_anglo_saxon_valuation diff --git a/addons/purchase_mrp/tests/test_anglo_saxon_valuation.py b/addons/purchase_mrp/tests/test_anglo_saxon_valuation.py new file mode 100644 index 00000000000..53695146fe0 --- /dev/null +++ b/addons/purchase_mrp/tests/test_anglo_saxon_valuation.py @@ -0,0 +1,77 @@ +# -*- coding: utf-8 -*- +# Part of Odoo. See LICENSE file for full copyright and licensing details. + +from odoo.fields import Date +from odoo.tests import Form, tagged +from odoo.tests.common import TransactionCase +from odoo.addons.stock_account.tests.test_stockvaluation import _create_accounting_data + + +@tagged('post_install', '-at_install') +class TestAngloSaxonValuationPurchaseMRP(TransactionCase): + + @classmethod + def setUpClass(cls): + super(TestAngloSaxonValuationPurchaseMRP, cls).setUpClass() + cls.vendor01 = cls.env['res.partner'].create({'name': "Super Vendor"}) + + cls.stock_input_account, cls.stock_output_account, cls.stock_valuation_account, cls.expense_account, cls.stock_journal = _create_accounting_data(cls.env) + cls.avco_category = cls.env['product.category'].create({ + 'name': 'AVCO', + 'property_cost_method': 'average', + 'property_valuation': 'real_time', + 'property_stock_account_input_categ_id': cls.stock_input_account.id, + 'property_stock_account_output_categ_id': cls.stock_output_account.id, + 'property_stock_journal': cls.stock_journal.id, + 'property_stock_valuation_account_id': cls.stock_valuation_account.id, + }) + + def test_kit_anglo_saxo_price_diff(self): + """ + Suppose an automated-AVCO configuration and a Price Difference Account defined on + the product category. When buying a kit of that category at a higher price than its + cost, the difference should be published on the Price Difference Account + """ + price_diff_account = self.env['account.account'].create({ + 'name': 'Super Price Difference Account', + 'code': 'SPDA', + 'user_type_id': self.env.ref('account.data_account_type_current_assets').id, + 'reconcile': True, + }) + self.avco_category.property_account_creditor_price_difference_categ = price_diff_account + + kit, compo01, compo02 = self.env['product.product'].create([{ + 'name': name, + 'standard_price': price, + 'type': 'product', + 'categ_id': self.avco_category.id, + } for name, price in [('Kit', 0), ('Compo 01', 10), ('Compo 02', 20)]]) + + self.env['mrp.bom'].create({ + 'product_tmpl_id': kit.product_tmpl_id.id, + 'type': 'phantom', + 'bom_line_ids': [(0, 0, { + 'product_id': p.id, + 'product_qty': 1, + }) for p in [compo01, compo02]] + }) + kit.button_bom_cost() + + po_form = Form(self.env['purchase.order']) + po_form.partner_id = self.vendor01 + with po_form.order_line.new() as pol_form: + pol_form.product_id = kit + pol_form.price_unit = 100 + po = po_form.save() + po.button_confirm() + + action = po.picking_ids.button_validate() + wizard = Form(self.env[action['res_model']].with_context(action['context'])).save() + wizard.process() + + action = po.action_create_invoice() + invoice = self.env['account.move'].browse(action['res_id']) + invoice.invoice_date = Date.today() + invoice.action_post() + price_diff_aml = invoice.line_ids.filtered(lambda l: l.account_id == price_diff_account) + self.assertEqual(price_diff_aml.balance, 70, "Should be the purchase price minus the kit cost (i.e. 100 - 30)") diff --git a/addons/purchase_stock/models/account_invoice.py b/addons/purchase_stock/models/account_invoice.py index 7f3fab2b3aa..df307aa99bd 100644 --- a/addons/purchase_stock/models/account_invoice.py +++ b/addons/purchase_stock/models/account_invoice.py @@ -2,8 +2,7 @@ # Part of Odoo. See LICENSE file for full copyright and licensing details. from odoo import api, fields, models, _ -from odoo.tools.float_utils import float_compare, float_is_zero -from odoo.exceptions import UserError +from odoo.tools.float_utils import float_compare class AccountMove(models.Model): @@ -72,23 +71,7 @@ class AccountMove(models.Model): valuation_stock_moves = valuation_stock_moves.filtered(lambda stock_move: stock_move._is_in()) if valuation_stock_moves: - valuation_price_unit_total = 0 - valuation_total_qty = 0 - for val_stock_move in valuation_stock_moves: - # In case val_stock_move is a return move, its valuation entries have been made with the - # currency rate corresponding to the original stock move - valuation_date = val_stock_move.origin_returned_move_id.date or val_stock_move.date - svl = val_stock_move.with_context(active_test=False).mapped('stock_valuation_layer_ids').filtered(lambda l: l.quantity) - layers_qty = sum(svl.mapped('quantity')) - layers_values = sum(svl.mapped('value')) - valuation_price_unit_total += line.company_currency_id._convert( - layers_values, move.currency_id, - move.company_id, valuation_date, round=False, - ) - valuation_total_qty += layers_qty - - if float_is_zero(valuation_total_qty, precision_rounding=line.product_uom_id.rounding or line.product_id.uom_id.rounding): - raise UserError(_('Odoo is not able to generate the anglo saxon entries. The total valuation of %s is zero.') % line.product_id.display_name) + valuation_price_unit_total, valuation_total_qty = valuation_stock_moves._get_valuation_price_and_qty(line, move.currency_id) valuation_price_unit = valuation_price_unit_total / valuation_total_qty valuation_price_unit = line.product_id.uom_id._compute_price(valuation_price_unit, line.product_uom_id) diff --git a/addons/purchase_stock/models/stock.py b/addons/purchase_stock/models/stock.py index 4a75d32fe7d..48e6e61ad0e 100644 --- a/addons/purchase_stock/models/stock.py +++ b/addons/purchase_stock/models/stock.py @@ -2,9 +2,11 @@ # Part of Odoo. See LICENSE file for full copyright and licensing details. from odoo import api, fields, models, _ -from odoo.tools.float_utils import float_round +from odoo.tools.float_utils import float_round, float_is_zero +from odoo.exceptions import UserError from dateutil.relativedelta import relativedelta + class StockPicking(models.Model): _inherit = 'stock.picking' @@ -110,6 +112,26 @@ class StockMove(models.Model): res = super()._get_source_document() return self.purchase_line_id.order_id or res + def _get_valuation_price_and_qty(self, related_aml, to_curr): + valuation_price_unit_total = 0 + valuation_total_qty = 0 + for val_stock_move in self: + # In case val_stock_move is a return move, its valuation entries have been made with the + # currency rate corresponding to the original stock move + valuation_date = val_stock_move.origin_returned_move_id.date or val_stock_move.date + svl = val_stock_move.with_context(active_test=False).mapped('stock_valuation_layer_ids').filtered( + lambda l: l.quantity) + layers_qty = sum(svl.mapped('quantity')) + layers_values = sum(svl.mapped('value')) + valuation_price_unit_total += related_aml.company_currency_id._convert( + layers_values, to_curr, related_aml.company_id, valuation_date, round=False, + ) + valuation_total_qty += layers_qty + if float_is_zero(valuation_total_qty, precision_rounding=related_aml.product_uom_id.rounding or related_aml.product_id.uom_id.rounding): + raise UserError( + _('Odoo is not able to generate the anglo saxon entries. The total valuation of %s is zero.') % related_aml.product_id.display_name) + return valuation_price_unit_total, valuation_total_qty + class StockWarehouse(models.Model): _inherit = 'stock.warehouse'