add stock_account_avco_negative_fix
This commit is contained in:
parent
10a288f8d5
commit
b07082f70d
9 changed files with 302 additions and 4 deletions
|
|
@ -16,8 +16,10 @@ _logger = logging.getLogger(__name__)
|
||||||
class StockMove(models.Model):
|
class StockMove(models.Model):
|
||||||
_inherit = "stock.move"
|
_inherit = "stock.move"
|
||||||
|
|
||||||
def product_price_update_before_done(self):
|
def product_price_update_before_done(self, forced_qty=None):
|
||||||
super(StockMove, self).product_price_update_before_done()
|
super(StockMove, self).product_price_update_before_done(
|
||||||
|
forced_qty=forced_qty
|
||||||
|
)
|
||||||
for move in self.filtered(lambda move: move.location_id.usage == "supplier"):
|
for move in self.filtered(lambda move: move.location_id.usage == "supplier"):
|
||||||
if (
|
if (
|
||||||
move.product_id.last_purchase_price_compute_type
|
move.product_id.last_purchase_price_compute_type
|
||||||
|
|
|
||||||
|
|
@ -15,8 +15,10 @@ _logger = logging.getLogger(__name__)
|
||||||
class StockMove(models.Model):
|
class StockMove(models.Model):
|
||||||
_inherit = "stock.move"
|
_inherit = "stock.move"
|
||||||
|
|
||||||
def product_price_update_before_done(self):
|
def product_price_update_before_done(self, forced_qty=None):
|
||||||
res = super(StockMove, self).product_price_update_before_done()
|
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"):
|
for move in self.filtered(lambda move: move.location_id.usage == "supplier"):
|
||||||
if (
|
if (
|
||||||
move.product_id.last_purchase_price_received_compute_type
|
move.product_id.last_purchase_price_received_compute_type
|
||||||
|
|
|
||||||
75
stock_account_avco_negative_fix/README.md
Normal file
75
stock_account_avco_negative_fix/README.md
Normal file
|
|
@ -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
|
||||||
1
stock_account_avco_negative_fix/__init__.py
Normal file
1
stock_account_avco_negative_fix/__init__.py
Normal file
|
|
@ -0,0 +1 @@
|
||||||
|
from . import models
|
||||||
13
stock_account_avco_negative_fix/__manifest__.py
Normal file
13
stock_account_avco_negative_fix/__manifest__.py
Normal file
|
|
@ -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,
|
||||||
|
}
|
||||||
1
stock_account_avco_negative_fix/models/__init__.py
Normal file
1
stock_account_avco_negative_fix/models/__init__.py
Normal file
|
|
@ -0,0 +1 @@
|
||||||
|
from . import stock_move
|
||||||
95
stock_account_avco_negative_fix/models/stock_move.py
Normal file
95
stock_account_avco_negative_fix/models/stock_move.py
Normal file
|
|
@ -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
|
||||||
1
stock_account_avco_negative_fix/tests/__init__.py
Normal file
1
stock_account_avco_negative_fix/tests/__init__.py
Normal file
|
|
@ -0,0 +1 @@
|
||||||
|
from . import test_avco_negative_fix
|
||||||
108
stock_account_avco_negative_fix/tests/test_avco_negative_fix.py
Normal file
108
stock_account_avco_negative_fix/tests/test_avco_negative_fix.py
Normal file
|
|
@ -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)
|
||||||
Loading…
Add table
Add a link
Reference in a new issue