From e39af99476345963f3fac64194786855b59751e6 Mon Sep 17 00:00:00 2001 From: "Adrien Widart (awt)" Date: Thu, 12 Jan 2023 15:51:34 +0000 Subject: [PATCH] [FIX] mrp_subcontracting_dropshipping,*: dropship valued product *:purchase_stock,stock_account To reproduce the issue: 1. In Settings, enable 'Multi-Step Routes' 2. Create a product category PC: - Method: FIFO - Valuation: Auto 3. Create two products P_compo, P_finished - Both: - Type: Storable - Category: PC - With a positive cost - P_finished: - Vendors: a new vendor V 4. Create and confirm a SO with one order line: - Product: P_finished - Route: Dropship 5. Confirm the PO and process the dropship 6. Accounting > Accounting > Miscellaneous > Journal Items Error: There are a lot of lines and most are incorrect When marking the dropship SM as done, we check how we should value it: https://github.com/odoo/odoo/blob/18450973eaf3ca048cdcd2046a8cba5f5c0f98b0/addons/stock_account/models/stock_move.py#L244-L252 Here is one issue: `_is_dropshipped` returns `True`, which is correct because the source is the subcontractor location and the destination a customer: https://github.com/odoo/odoo/blob/8276a9b51b7c5fc0ae6dd557ae88a9c31f1d1962/addons/mrp_subcontracting_dropshipping/models/stock_move.py#L14-L20 However, `_is_out` will return `True` too. To get this value, we check if the source and destination locations have to be valued: https://github.com/odoo/odoo/blob/18450973eaf3ca048cdcd2046a8cba5f5c0f98b0/addons/stock_account/models/stock_move.py#L99-L100 Again, since the source location is the subcontractor one (which is an internal one) and the destination location is the customer one, we will consider that we have to value this SM. As a result, we will apply `_create_out_svl` and `_create_dropshipped_svl` on the same SM (this explains some incorrect AMLs are generated). Then, there is another issue. In `_create_dropshipped_svl`, we create an in-SVL and an out-SVL for P_finished. However, when processing the MO, an in-SVL will also be generated for P_finished. The issue comes from `_create_dropshipped_svl`: it creates an in-SVL but the source location is not outside from the company, so it should not create the incoming layer. Finally, when creating the SVL in `_create_dropshipped_svl`, we use the unit price of the SM to define the value of the SVL. This is an issue: suppose the unit price of the POL is $10 and the costs of P_compo is $1, the unit price of the SM is then $10 (and so does the value of the SVL) but the actual value of the delivered P_finished is $11. Howver, when calling `_action_done` on a SM, we first process it and only then we will process the MO: https://github.com/odoo/odoo/blob/829369d3ca0530f1aad0599fbb1598810a928cc3/addons/mrp_subcontracting/models/stock_picking.py#L33-L36 So, in `_create_dropshipped_svl`, we can't know the actual cost of P_finished. This is the reason why we have to create a compensation layer once the MO is processed (i.e. in the override from `mrp_subcontracting_dropshipping`) Similar issues will happen if we return the dropshipped/subcontracted product. OPW-3078499 closes odoo/odoo#110390 X-original-commit: 200aac56771fd1ef759f73b277a6808d96ee5f31 Signed-off-by: William Henrotin (whe) Signed-off-by: Adrien Widart --- .../models/stock_picking.py | 32 +++++ .../tests/__init__.py | 1 + .../tests/test_anglo_saxon_valuation.py | 134 ++++++++++++++++++ addons/purchase_stock/models/stock_move.py | 2 +- addons/stock_account/models/stock_move.py | 40 +++--- 5 files changed, 190 insertions(+), 19 deletions(-) create mode 100644 addons/mrp_subcontracting_dropshipping/tests/test_anglo_saxon_valuation.py diff --git a/addons/mrp_subcontracting_dropshipping/models/stock_picking.py b/addons/mrp_subcontracting_dropshipping/models/stock_picking.py index 4ae776d4e5d..82ec1c0d8e4 100644 --- a/addons/mrp_subcontracting_dropshipping/models/stock_picking.py +++ b/addons/mrp_subcontracting_dropshipping/models/stock_picking.py @@ -2,6 +2,7 @@ # Part of Odoo. See LICENSE file for full copyright and licensing details. from odoo import models +from odoo.tools import float_compare class StockPicking(models.Model): @@ -20,6 +21,37 @@ class StockPicking(models.Model): def _action_done(self): res = super()._action_done() self.move_ids.move_dest_ids._action_assign() + + # If needed, create a compensation layer, so we add the MO cost to the dropship one + svls = self.env['stock.valuation.layer'] + for move in self.move_ids: + if not (move.is_subcontract and move._is_dropshipped() and move.state == 'done'): + continue + + dropship_svls = move.stock_valuation_layer_ids + if not dropship_svls: + continue + + subcontract_svls = move.move_orig_ids.stock_valuation_layer_ids + subcontract_value = sum(subcontract_svls.mapped('value')) + dropship_value = abs(sum(dropship_svls.mapped('value'))) + diff = subcontract_value - dropship_value + if float_compare(diff, 0, precision_rounding=move.company_id.currency_id.rounding) <= 0: + continue + + svl_vals = move._prepare_common_svl_vals() + svl_vals.update({ + 'remaining_value': 0, + 'remaining_qty': 0, + 'value': -diff, + 'quantity': 0, + 'unit_cost': 0, + 'stock_valuation_layer_id': dropship_svls[0].id, + 'stock_move_id': move.id, + }) + svls |= self.env['stock.valuation.layer'].create(svl_vals) + svls._validate_accounting_entries() + return res def _prepare_subcontract_mo_vals(self, subcontract_move, bom): diff --git a/addons/mrp_subcontracting_dropshipping/tests/__init__.py b/addons/mrp_subcontracting_dropshipping/tests/__init__.py index 813f502e006..69dd037dae2 100644 --- a/addons/mrp_subcontracting_dropshipping/tests/__init__.py +++ b/addons/mrp_subcontracting_dropshipping/tests/__init__.py @@ -3,3 +3,4 @@ from . import test_purchase_subcontracting from . import test_sale_dropshipping +from . import test_anglo_saxon_valuation diff --git a/addons/mrp_subcontracting_dropshipping/tests/test_anglo_saxon_valuation.py b/addons/mrp_subcontracting_dropshipping/tests/test_anglo_saxon_valuation.py new file mode 100644 index 00000000000..0623d848f6a --- /dev/null +++ b/addons/mrp_subcontracting_dropshipping/tests/test_anglo_saxon_valuation.py @@ -0,0 +1,134 @@ +# -*- coding: utf-8 -*- +# Part of Odoo. See LICENSE file for full copyright and licensing details. + +from odoo.addons.stock_account.tests.test_anglo_saxon_valuation_reconciliation_common import ValuationReconciliationTestCommon +from odoo.tests import tagged, Form + + +@tagged('post_install', '-at_install') +class TestSubcontractingDropshippingValuation(ValuationReconciliationTestCommon): + + @classmethod + def setUpClass(cls, chart_template_ref=None): + super().setUpClass(chart_template_ref=chart_template_ref) + + categ_form = Form(cls.env['product.category']) + categ_form.name = 'fifo auto' + categ_form.parent_id = cls.env.ref('product.product_category_all') + categ_form.property_cost_method = 'fifo' + categ_form.property_valuation = 'real_time' + cls.categ_fifo_auto = categ_form.save() + + (cls.product_a | cls.product_b).type = 'product' + + cls.bom_a = cls.env['mrp.bom'].create({ + 'product_tmpl_id': cls.product_a.product_tmpl_id.id, + 'type': 'subcontract', + 'subcontractor_ids': [(6, 0, cls.partner_a.ids)], + 'bom_line_ids': [ + (0, 0, {'product_id': cls.product_b.id, 'product_qty': 1.0}), + ], + }) + + def test_valuation_subcontracted_and_dropshipped(self): + """ + Product: + - FIFO + Auto + - Subcontracted + Purchase 2 from Subcontractor to a customer (dropship). + Then return 1 to subcontractor and one to stock + It should generate the correct valuations AMLs + """ + # pylint: disable=bad-whitespace + all_amls_ids = self.env['account.move.line'].search_read([], ['id']) + + grp_multi_loc = self.env.ref('stock.group_stock_multi_locations') + self.env.user.write({'groups_id': [(4, grp_multi_loc.id)]}) + + (self.product_a | self.product_b).categ_id = self.categ_fifo_auto + self.product_b.standard_price = 10 + + dropship_picking_type = self.env['stock.picking.type'].search([ + ('company_id', '=', self.env.company.id), + ('default_location_src_id.usage', '=', 'supplier'), + ('default_location_dest_id.usage', '=', 'customer'), + ], limit=1, order='sequence') + + po = self.env['purchase.order'].create({ + "partner_id": self.partner_a.id, + "picking_type_id": dropship_picking_type.id, + "dest_address_id": self.partner_b.id, + "order_line": [(0, 0, { + 'product_id': self.product_a.id, + 'name': self.product_a.name, + 'product_qty': 2.0, + 'price_unit': 100, + 'taxes_id': False, + })], + }) + po.button_confirm() + + delivery = po.picking_ids + res = delivery.button_validate() + Form(self.env['stock.immediate.transfer'].with_context(res['context'])).save().process() + + stock_in_acc_id = self.categ_fifo_auto.property_stock_account_input_categ_id.id + stock_out_acc_id = self.categ_fifo_auto.property_stock_account_output_categ_id.id + stock_valu_acc_id = self.categ_fifo_auto.property_stock_valuation_account_id.id + + amls = self.env['account.move.line'].search([('id', 'not in', all_amls_ids)]) + all_amls_ids += amls.ids + self.assertRecordValues(amls, [ + # Compensation of dropshipping value + {'account_id': stock_valu_acc_id, 'product_id': self.product_a.id, 'debit': 0.0, 'credit': 20.0}, + {'account_id': stock_out_acc_id, 'product_id': self.product_a.id, 'debit': 20.0, 'credit': 0.0}, + # Receipt from subcontractor + {'account_id': stock_in_acc_id, 'product_id': self.product_a.id, 'debit': 0.0, 'credit': 220.0}, + {'account_id': stock_valu_acc_id, 'product_id': self.product_a.id, 'debit': 220.0, 'credit': 0.0}, + # Delivery to subcontractor + {'account_id': stock_valu_acc_id, 'product_id': self.product_b.id, 'debit': 0.0, 'credit': 20.0}, + {'account_id': stock_out_acc_id, 'product_id': self.product_b.id, 'debit': 20.0, 'credit': 0.0}, + # Initial dropshipped value + {'account_id': stock_valu_acc_id, 'product_id': self.product_a.id, 'debit': 0.0, 'credit': 200.0}, + {'account_id': stock_out_acc_id, 'product_id': self.product_a.id, 'debit': 200.0, 'credit': 0.0}, + ]) + + # return to subcontracting location + sbc_location = self.env.company.subcontracting_location_id + return_form = Form(self.env['stock.return.picking'].with_context(active_id=delivery.id, active_model='stock.picking')) + return_form.location_id = sbc_location + with return_form.product_return_moves.edit(0) as line: + line.quantity = 1 + return_wizard = return_form.save() + return_id, _ = return_wizard._create_returns() + return_picking = self.env['stock.picking'].browse(return_id) + return_picking.move_ids.quantity_done = 1 + return_picking.button_validate() + + amls = self.env['account.move.line'].search([('id', 'not in', all_amls_ids)]) + all_amls_ids += amls.ids + self.assertRecordValues(amls, [ + {'account_id': stock_valu_acc_id, 'product_id': self.product_a.id, 'debit': 0.0, 'credit': 110.0}, + {'account_id': stock_in_acc_id, 'product_id': self.product_a.id, 'debit': 110.0, 'credit': 0.0}, + ]) + + # return to stock location + warehouse = self.env['stock.warehouse'].search([('company_id', '=', self.env.company.id)], limit=1) + stock_location = warehouse.lot_stock_id + stock_location.return_location = True + return_form = Form(self.env['stock.return.picking'].with_context(active_id=delivery.id, active_model='stock.picking')) + return_form.location_id = stock_location + with return_form.product_return_moves.edit(0) as line: + line.quantity = 1 + return_wizard = return_form.save() + return_id, _ = return_wizard._create_returns() + return_picking = self.env['stock.picking'].browse(return_id) + return_picking.move_ids.quantity_done = 1 + return_picking.button_validate() + + amls = self.env['account.move.line'].search([('id', 'not in', all_amls_ids)]) + all_amls_ids += amls.ids + self.assertRecordValues(amls, [ + {'account_id': stock_out_acc_id, 'product_id': self.product_a.id, 'debit': 0.0, 'credit': 110.0}, + {'account_id': stock_valu_acc_id, 'product_id': self.product_a.id, 'debit': 110.0, 'credit': 0.0}, + ]) diff --git a/addons/purchase_stock/models/stock_move.py b/addons/purchase_stock/models/stock_move.py index 52b565bbac9..71a29fcd737 100644 --- a/addons/purchase_stock/models/stock_move.py +++ b/addons/purchase_stock/models/stock_move.py @@ -34,7 +34,7 @@ class StockMove(models.Model): def _get_price_unit(self): """ Returns the unit price for the move""" self.ensure_one() - if not self.purchase_line_id or not self.product_id.id: + if self.origin_returned_move_id or not self.purchase_line_id or not self.product_id.id: return super(StockMove, self)._get_price_unit() price_unit_prec = self.env['decimal.precision'].precision_get('Product Price') line = self.purchase_line_id diff --git a/addons/stock_account/models/stock_move.py b/addons/stock_account/models/stock_move.py index 864b34cdf66..c5949178bea 100644 --- a/addons/stock_account/models/stock_move.py +++ b/addons/stock_account/models/stock_move.py @@ -46,6 +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: layers = self.origin_returned_move_id.sudo().stock_valuation_layer_ids + layers |= layers.stock_valuation_layer_ids quantity = sum(layers.mapped("quantity")) return layers.currency_id.round(sum(layers.mapped("value")) / quantity) if not float_is_zero(quantity, precision_rounding=layers.uom_id.rounding) else 0 return price_unit if not float_is_zero(price_unit, precision) or self._should_force_price_unit() else self.product_id.standard_price @@ -86,7 +87,7 @@ class StockMove(models.Model): :rtype: bool """ self.ensure_one() - if self._get_in_move_lines(): + if self._get_in_move_lines() and not self._is_dropshipped_returned(): return True return False @@ -114,7 +115,7 @@ class StockMove(models.Model): :rtype: bool """ self.ensure_one() - if self._get_out_move_lines(): + if self._get_out_move_lines() and not self._is_dropshipped(): return True return False @@ -206,23 +207,26 @@ class StockMove(models.Model): common_vals = dict(move._prepare_common_svl_vals(), remaining_qty=0) - # create the in - in_vals = { - 'unit_cost': unit_cost, - 'value': unit_cost * quantity, - 'quantity': quantity, - } - in_vals.update(common_vals) - svl_vals_list.append(in_vals) + # create the in if it does not come from a valued location (eg subcontract -> customer) + if not move.location_id._should_be_valued(): + in_vals = { + 'unit_cost': unit_cost, + 'value': unit_cost * quantity, + 'quantity': quantity, + } + in_vals.update(common_vals) + svl_vals_list.append(in_vals) + + # create the out if it does not go to a valued location (eg customer -> subcontract) + if not move.location_dest_id._should_be_valued(): + out_vals = { + 'unit_cost': unit_cost, + 'value': unit_cost * quantity * -1, + 'quantity': quantity * -1, + } + out_vals.update(common_vals) + svl_vals_list.append(out_vals) - # create the out - out_vals = { - 'unit_cost': unit_cost, - 'value': unit_cost * quantity * -1, - 'quantity': quantity * -1, - } - out_vals.update(common_vals) - svl_vals_list.append(out_vals) return self.env['stock.valuation.layer'].sudo().create(svl_vals_list) def _create_dropshipped_returned_svl(self, forced_quantity=None):