[FIX] product_replenishment_cost_mrp: compute BoM based cost record by record - #936
Open
fw-bot-adhoc wants to merge 1 commit into
Open
[FIX] product_replenishment_cost_mrp: compute BoM based cost record by record#936fw-bot-adhoc wants to merge 1 commit into
fw-bot-adhoc wants to merge 1 commit into
Conversation
…y record A product whose replenishment cost type is "Based on BoM" reads the replenishment cost of its components, and a component can be based on a BoM as well. The fields were not declared recursive, so the ORM computed them in batches and protected the whole batch: reading the field on a record of the batch that was not computed yet returns 0 with no error, and the parent silently lost the cost of that component. It only shows up when no component of the chain is left outside the batch to resolve it. That is why the product form is always right, the contextual action fails only when the whole chain is selected, and the scheduled action, which walks the whole catalog, is always wrong. The resulting value is not even stable between runs: it depends on which products fall in the same batch and on the order they are read, which differs between product.product and product.template. Declaring the fields recursive makes the ORM compute them one record at a time, so the protection covers a single record and reading a component computes it instead of returning zero. The declaration lives in this module and not in product_replenishment_cost because the recursion is introduced here: with no BoM based cost, no product reads the field of another one, and the bases that do not manufacture do not pay for it. Measured on a synthetic catalog of 1000 products based on BoM, 500 of them built over a sub assembly: without the fix every parent was undervalued, 5000 instead of 15000 in total, and the fix costs 14% more time, 6.13s to 6.99s on the best of three runs. X-original-commit: fd3fd04
Contributor
|
|
Author
|
@les-adhoc @mav-adhoc this PR targets 19.0 and is the last of the forward-port chain. To merge the full chain, use
More info at https://github.com/odoo/odoo/wiki/Mergebot#forward-port |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Un producto con costo de reposición "Basado en LdM" lee el costo de reposición de sus componentes, y un componente puede ser a su vez "Basado en LdM". Los campos no estaban declarados
recursive, así que el ORM los computaba de a lotes y protegía el lote entero: leer el campo sobre un registro del lote que todavía no se computó devuelve 0 sin error, y el padre pierde en silencio el costo de ese componente.Solo se manifiesta cuando no queda ningún componente de la cadena fuera del lote que lo resuelva. Por eso la ficha del producto siempre da bien, la acción contextual falla solo si se selecciona toda la cadena, y la acción planificada —que recorre el catálogo entero— falla siempre. El valor tampoco es estable entre corridas: depende de qué productos caen en el mismo lote y del orden en que se leen, que difiere entre
product.product(pordefault_code) yproduct.template(porname).El cambio
Declarar
recursive=Truesobrereplenishment_costyreplenishment_base_cost_on_currency(comparten compute, la protección los agrupa). El ORM pasa a computarlos de a un registro: la protección cubre un solo record y la lectura del componente lo computa en vez de devolver cero.La declaración va en este módulo y no en
product_replenishment_costporque la recursión la introduce acá: sin costo basado en LdM ningún producto lee el campo de otro, y las bases que no fabrican no pagan el costo.Alternativas descartadas
Test plan
Dos tests nuevos en
test_replenishment_cost_bom.py, ambos sobre un terminado (30) construido sobre un sub armado (20) más un insumo (10). Hay que iterar el recordset:browse()resetea el prefetch set y computa de a uno, que es justamente el escenario que no falla.test_multi_level_bom_computed_as_batch— lectura del campo con toda la cadena en el mismo lote.test_update_cost_computed_as_batch— la acción que graba el costo contable sobre esa misma selección.Sin el fix ambos dan
10.0 != 30.0. Con el fix pasan los 5 del módulo, más los 16 deproduct_replenishment_cost.Medición
Catálogo sintético de 2000 productos, 1000 de ellos "Basado en LdM" (500 sub armados y 500 terminados construidos sobre esos sub armados). Leyendo los 1000 juntos, que es lo que hace la acción planificada:
+14% de tiempo. Sobre los 2000 productos mezclados el overhead baja a +10%, porque los productos con costo manual o de proveedor no cambian de comportamiento.
Nota para el reviewer
_bom_find, en otra parte del archivo. Este se para solo sobre18.0.tests/__init__.pyytests/test_replenishment_cost_bom.pyen su rebase: esa cobertura ya queda incluida acá.bump.Forward-Port-Of: #933