diff --git a/product_sale_price_from_pricelist/models/stock_move.py b/product_sale_price_from_pricelist/models/stock_move.py index 9865db0..86cbc33 100644 --- a/product_sale_price_from_pricelist/models/stock_move.py +++ b/product_sale_price_from_pricelist/models/stock_move.py @@ -16,8 +16,10 @@ _logger = logging.getLogger(__name__) class StockMove(models.Model): _inherit = "stock.move" - def product_price_update_before_done(self): - super(StockMove, self).product_price_update_before_done() + def product_price_update_before_done(self, forced_qty=None): + super(StockMove, self).product_price_update_before_done( + forced_qty=forced_qty + ) for move in self.filtered(lambda move: move.location_id.usage == "supplier"): if ( move.product_id.last_purchase_price_compute_type diff --git a/product_update_price_last_purchase/models/stock_move.py b/product_update_price_last_purchase/models/stock_move.py index 424c563..09b3e1d 100644 --- a/product_update_price_last_purchase/models/stock_move.py +++ b/product_update_price_last_purchase/models/stock_move.py @@ -15,8 +15,10 @@ _logger = logging.getLogger(__name__) class StockMove(models.Model): _inherit = "stock.move" - def product_price_update_before_done(self): - res = super(StockMove, self).product_price_update_before_done() + def product_price_update_before_done(self, forced_qty=None): + res = super(StockMove, self).product_price_update_before_done( + forced_qty=forced_qty + ) for move in self.filtered(lambda move: move.location_id.usage == "supplier"): if ( move.product_id.last_purchase_price_received_compute_type diff --git a/stock_account_avco_negative_fix/README.md b/stock_account_avco_negative_fix/README.md new file mode 100644 index 0000000..3251a7e --- /dev/null +++ b/stock_account_avco_negative_fix/README.md @@ -0,0 +1,75 @@ +# Stock Account AVCO Negative Fix + +Corrige un bug del core de Odoo 16 en la valoración de coste promedio (AVCO) +que puede dejar el `standard_price` de un producto en un valor negativo (o +cero) cuando existe stock negativo (`quantity_svl < 0`) y se recibe una +compra que compensa solo parcialmente ese stock. + +## Descripción del problema + +El método `product_price_update_before_done()` de +`stock_account/models/stock_move.py` calcula el nuevo coste medio ponderado +de un producto AVCO con la fórmula: + +``` +new_std_price = (amount_unit * qty_svl + price_unit_in * qty) / (qty_svl + qty) +``` + +Cuando `(qty_svl + qty)` es un valor positivo muy pequeño (el stock negativo +casi se compensa con la compra recibida, pero no del todo), el numerador +puede resultar negativo y el `standard_price` calculado se vuelve negativo, +corrompiendo la valoración de inventario en los recálculos posteriores. + +Bug reportado en upstream sin corrección oficial: +https://github.com/odoo/odoo/issues/187169 + +## Solución + +Este módulo **no** copia ni sobrescribe el algoritmo del core. En su lugar, +hereda `stock.move` y aplica una estrategia de post-procesado con `super()` +sobre `_action_done()`, basada en una **precondición** evaluada antes de que +nada más intervenga: + +1. Antes de llamar a `super()`, para cada movimiento entrante AVCO relevante + (`move._is_in()` y `cost_method == 'average'`), comprueba si el producto + tenía `quantity_svl <= 0` **antes** de este movimiento (la precondición + bajo la cual el core puede degenerar a un coste negativo o cero). Si se + cumple, captura el `price_unit` entrante (`move._get_price_unit()`). +2. Llama a `super()._action_done()` para que el core ejecute su lógica + completa sin modificaciones (media ponderada AVCO, rama FIFO y el + `_run_fifo_vacuum()` final). +3. Después de `super()`, para cada producto marcado como "en riesgo" en el + paso 1, reescribe **incondicionalmente** su `standard_price` con el + `price_unit` capturado —sin comprobar el valor que haya quedado escrito—, + replicando el mismo patrón de escritura del core (`disable_auto_svl=True`, + `sudo()`) para no generar SVLs de revaluación espurios. + +**¿Por qué precondición incondicional y no comprobar el signo final?** Una +primera versión de este fix comprobaba el signo de `standard_price` +*después* de `super()`. Se detectó que ese criterio es insuficiente: otros +módulos que reaccionan durante `_action_done()` (por ejemplo, el OCA +`product_cost_price_avco_sync`, que sincroniza el coste AVCO al reaccionar a +`stock.valuation.layer.write()` disparado por `_run_fifo_vacuum()`) pueden +reescribir `standard_price` a un valor positivo pero distinto del esperado, +haciendo que la comprobación `<= 0` nunca se cumpla aunque la situación de +riesgo sí se haya dado. Al decidir **antes** de que empiece la cadena y +aplicar la corrección de forma incondicional, el fix es robusto frente a +cualquier módulo que reescriba `standard_price` durante el proceso, ahora o +en el futuro. + +Este enfoque evita duplicar el algoritmo del core (resiliente a cambios de +upstream) y es compatible con cualquier otro módulo, presente o futuro, que +también herede `product_price_update_before_done`/`_action_done` sobre +`stock.move`, ya que la cadena de `super()` se respeta íntegramente. + +## Alcance + +- Solo actúa sobre productos con `cost_method = 'average'`. +- Productos FIFO o de coste estándar no se ven afectados. +- No introduce modelos, vistas ni campos nuevos. + +## Créditos + +### Autores + +- Criptomart diff --git a/stock_account_avco_negative_fix/__init__.py b/stock_account_avco_negative_fix/__init__.py new file mode 100644 index 0000000..0650744 --- /dev/null +++ b/stock_account_avco_negative_fix/__init__.py @@ -0,0 +1 @@ +from . import models diff --git a/stock_account_avco_negative_fix/__manifest__.py b/stock_account_avco_negative_fix/__manifest__.py new file mode 100644 index 0000000..73606b0 --- /dev/null +++ b/stock_account_avco_negative_fix/__manifest__.py @@ -0,0 +1,13 @@ +# Copyright 2026 Criptomart +# License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl). +{ + "name": "Stock Account AVCO Negative Fix", + "version": "16.0.1.0.0", + "summary": "Prevent negative AVCO cost on incoming moves with negative stock", + "license": "AGPL-3", + "author": "Criptomart", + "website": "https://github.com/OCA/stock-logistics-workflow", + "depends": ["stock_account"], + "data": [], + "installable": True, +} diff --git a/stock_account_avco_negative_fix/models/__init__.py b/stock_account_avco_negative_fix/models/__init__.py new file mode 100644 index 0000000..6bda2d2 --- /dev/null +++ b/stock_account_avco_negative_fix/models/__init__.py @@ -0,0 +1 @@ +from . import stock_move diff --git a/stock_account_avco_negative_fix/models/stock_move.py b/stock_account_avco_negative_fix/models/stock_move.py new file mode 100644 index 0000000..c81ccf0 --- /dev/null +++ b/stock_account_avco_negative_fix/models/stock_move.py @@ -0,0 +1,95 @@ +from odoo import models +from odoo.tools import float_compare + + +class StockMove(models.Model): + _inherit = "stock.move" + + def _action_done(self, cancel_backorder=False): + """Fix negative AVCO ``standard_price`` after a move is done. + + Odoo core (``stock_account/models/stock_move.py``, + ``product_price_update_before_done``) computes the new average cost + of an AVCO product as a weighted average between the current + valuation (``quantity_svl`` / ``standard_price``) and the incoming + move. When the product has non-positive stock (``quantity_svl <= 0``) + and the incoming quantity only partially compensates it, the + resulting weighted average can become negative or zero, corrupting + the product cost for every subsequent outgoing valuation. + + This is a known, unfixed upstream bug: see + https://github.com/odoo/odoo/issues/187169 + + Rather than copying/overriding the core algorithm (which would break + compatibility with any OCA module -- present or future -- also + inheriting ``product_price_update_before_done``), we post-process the + final result of the whole ``_action_done`` flow: + + - *Before* calling ``super()``, we detect the AVCO products that are + "at risk": incoming moves (``move._is_in()``) of a product with + ``cost_method == 'average'`` whose stock valuation quantity + (``quantity_svl``) is already ``<= 0`` *before* this move is + processed. This is exactly the precondition under which the core's + weighted-average formula can degenerate to a negative or zero + result. We capture, for each such ``(company, product)``, the + incoming move's own ``price_unit`` (``move._get_price_unit()``). + - We let ``super()._action_done()`` run its full logic unmodified + (AVCO weighted average, FIFO branch, and the ``_run_fifo_vacuum()`` + step that runs at the very end of ``_action_done``). + - *After* ``super()`` has fully completed, for every "at risk" + ``(company, product)`` captured above, we unconditionally rewrite + ``standard_price`` with the captured incoming ``price_unit`` (not + the product's configured purchase price). + + Note this fix is intentionally based on the *precondition* + (``quantity_svl <= 0`` before the move) rather than on inspecting the + *final* ``standard_price`` sign. An earlier version of this fix only + rewrote ``standard_price`` when it was left ``<= 0`` after + ``super()``, but that check could be silently bypassed: some module + reacting to the ``_run_fifo_vacuum()`` step (e.g. the OCA module + ``product_cost_price_avco_sync``, which hooks into + ``stock.valuation.layer.write()``) can rewrite ``standard_price`` to + a *different but still positive* value before our check ran, + preventing the fix from ever triggering even though the scenario + that causes the original bug did occur. Keying off the precondition + instead makes the fix robust regardless of what any other module + does afterwards, without depending on assumptions about whether the + vacuum runs in production (in practice, ``product_cost_price_avco_sync`` + disables the vacuum outside of tests for performance reasons, since + it is only meaningful for FIFO -- but the fix should not rely on + that being permanently true). + """ + at_risk_price_unit = {} + for move in self.filtered( + lambda m: m._is_in() + and m.with_company(m.company_id).product_id.cost_method == "average" + ): + company = move.company_id + product = move.product_id.with_company(company) + quantity_svl = product.sudo().quantity_svl + rounding = product.uom_id.rounding + if float_compare(quantity_svl, 0.0, precision_rounding=rounding) <= 0: + at_risk_price_unit[(company.id, product.id)] = move._get_price_unit() + + res = super()._action_done(cancel_backorder=cancel_backorder) + + if not at_risk_price_unit: + return res + + companies = self.env["res.company"].browse( + {key[0] for key in at_risk_price_unit} + ) + products = self.env["product.product"].browse( + {key[1] for key in at_risk_price_unit} + ) + company_by_id = {company.id: company for company in companies} + product_by_id = {product.id: product for product in products} + + for (company_id, product_id), price_unit in at_risk_price_unit.items(): + company = company_by_id[company_id] + product = product_by_id[product_id] + product.with_company(company.id).with_context( + disable_auto_svl=True + ).sudo().write({"standard_price": price_unit}) + + return res diff --git a/stock_account_avco_negative_fix/tests/__init__.py b/stock_account_avco_negative_fix/tests/__init__.py new file mode 100644 index 0000000..64b554e --- /dev/null +++ b/stock_account_avco_negative_fix/tests/__init__.py @@ -0,0 +1 @@ +from . import test_avco_negative_fix diff --git a/stock_account_avco_negative_fix/tests/test_avco_negative_fix.py b/stock_account_avco_negative_fix/tests/test_avco_negative_fix.py new file mode 100644 index 0000000..a3f7f07 --- /dev/null +++ b/stock_account_avco_negative_fix/tests/test_avco_negative_fix.py @@ -0,0 +1,108 @@ +from odoo.tests import tagged +from odoo.tests.common import TransactionCase + + +@tagged("post_install", "-at_install") +class TestAvcoNegativeFix(TransactionCase): + @classmethod + def setUpClass(cls): + super().setUpClass() + cls.supplier_location = cls.env.ref("stock.stock_location_suppliers") + cls.customer_location = cls.env.ref("stock.stock_location_customers") + cls.stock_location = cls.env.ref("stock.stock_location_stock") + cls.picking_type_in = cls.env.ref("stock.picking_type_in") + cls.picking_type_out = cls.env.ref("stock.picking_type_out") + cls.categ_avco = cls.env["product.category"].create( + {"name": "Test AVCO category", "property_cost_method": "average"} + ) + cls.categ_fifo = cls.env["product.category"].create( + {"name": "Test FIFO category", "property_cost_method": "fifo"} + ) + + def _create_product(self, categ, standard_price): + return self.env["product.product"].create( + { + "name": "Test product %s" % categ.name, + "type": "product", + "categ_id": categ.id, + "standard_price": standard_price, + } + ) + + def _validate_move(self, product, qty, location_id, location_dest_id, price_unit=None): + move_vals = { + "name": "test move", + "product_id": product.id, + "product_uom_qty": qty, + "product_uom": product.uom_id.id, + "location_id": location_id.id, + "location_dest_id": location_dest_id.id, + } + if price_unit is not None: + move_vals["price_unit"] = price_unit + move = self.env["stock.move"].create(move_vals) + move._action_confirm() + move.quantity_done = qty + move._action_done() + return move + + def _sell_without_stock(self, product, qty): + return self._validate_move( + product, qty, self.stock_location, self.customer_location + ) + + def _receive(self, product, qty, price_unit): + return self._validate_move( + product, qty, self.supplier_location, self.stock_location, price_unit + ) + + def test_negative_result_is_corrected(self): + """TC-01 (RF-01): incoming move on a product with quantity_svl <= 0 + (risk precondition) unconditionally gets standard_price set to the + incoming move's price unit, reproducing product_id=8611.""" + product = self._create_product(self.categ_avco, 6.71) + self._sell_without_stock(product, 1.056) + self.assertAlmostEqual(product.quantity_svl, -1.056, places=3) + self.assertAlmostEqual(product.standard_price, 6.71, places=2) + + self._receive(product, 1.0, 7.56) + + self.assertGreater(product.standard_price, 0.0) + self.assertAlmostEqual(product.standard_price, 7.56, places=2) + + def test_zero_result_is_corrected(self): + """TC-02 (RF-02): quantity_svl exactly 0 before the incoming move is + also a risk precondition; standard_price is set to the incoming + move's price unit.""" + product = self._create_product(self.categ_avco, 6.0) + self._sell_without_stock(product, 2.0) + self.assertAlmostEqual(product.quantity_svl, -2.0, places=3) + self.assertAlmostEqual(product.standard_price, 6.0, places=2) + + self._receive(product, 3.0, 4.0) + + self.assertNotAlmostEqual(product.standard_price, 0.0, places=2) + self.assertAlmostEqual(product.standard_price, 4.0, places=2) + + def test_positive_result_is_untouched(self): + """TC-03 (RF-03): with quantity_svl > 0 before the incoming move + (no risk precondition), the post-processing fix does not + intervene at all and the core's weighted average is left as-is.""" + product = self._create_product(self.categ_avco, 1.0) + self._receive(product, 10.0, 10.0) + self.assertAlmostEqual(product.quantity_svl, 10.0, places=3) + self.assertAlmostEqual(product.standard_price, 10.0, places=2) + + self._receive(product, 10.0, 20.0) + + self.assertAlmostEqual(product.standard_price, 15.0, places=2) + + def test_non_avco_product_not_affected(self): + """TC-04 (RF-04): the fix only applies to cost_method == 'average' + products; FIFO products follow the core's unmodified behaviour.""" + product = self._create_product(self.categ_fifo, 5.0) + self.assertEqual(product.cost_method, "fifo") + + self._receive(product, 10.0, 8.0) + + self.assertAlmostEqual(product.standard_price, 8.0, places=2)