From 85bbe18f53869e7a1ffea68ae94aff23555a5c33 Mon Sep 17 00:00:00 2001 From: "Cuong, Nguyen Minh Tran Manh" Date: Thu, 30 Jul 2026 13:21:10 +0700 Subject: [PATCH] [FIX] stock_account: migrate product.value#value to a unit cost 19.0 replaced stock.valuation.layer with product.value, and pre-migration maps them with rename_models()/rename_tables(). That renames the table in place, so every row id and column value survives untouched -- but the two models give the `value` column different meanings: - 17.0 stock.valuation.layer#value is a *total* value delta, with the per-unit price in the separate unit_cost column. - 19.0 product.value#value is a *unit cost* for records not linked to a move. Those are what product.product#_get_last_product_value() reads to determine standard_price. So a move-less record keeps a total value where 19.0 reads a unit cost. Since a total value is the unit cost multiplied by the quantity of the layer, the resulting standard_price can be off by orders of magnitude, or negative where the layer was a decrease. Nothing fails loudly: the wrong cost simply propagates into COGS. Records linked to a move keep total-value semantics and are consumed correctly by stock_move_value(), so they are left alone; the new step runs last because it rewrites the values stock_move_value() reads. The unit cost is taken from unit_cost, which the rename leaves behind as an orphan column, wherever 17.0 populated it. For manual revaluations 17.0 wrote unit_cost NULL and quantity 0, so there it is derived the way 17.0 itself derived it in product.product._prepare_valuation_layer_field_values(): the running sum(value) / sum(quantity) over the product's layers up to that record's (date, id). Evaluating it per record reproduces the historical cost at that record's own date, which matters because _get_last_product_value() selects by date. Being an accumulation over the layers rather than a per-record field, it does not depend on the cost method, so it covers fifo -- where a revaluation is spread over the remaining_value of the layers still in stock and never lands in a per-product unit cost -- as well as average. The accumulation sums the 17.0 total values, so those are snapshot into a temporary table before the first pass starts overwriting them with unit costs. A record whose product had no quantity on hand at that point has no unit cost to derive; those keep their value and are logged rather than guessed at, since a wrong standard_price is silent and ends up in the accounts. Checked on an upgraded 17.0 database: every move-less record was resolved from one of the two sources, the derived values matched the costs 17.0 had recorded independently, and no move-linked or 19.0-native record was modified. The fifo path was exercised with layers generated through the revaluation wizard and a costing-method change on a fifo category. --- .../stock_account/19.0.1.1/post-migration.py | 118 ++++++++++++++++++ 1 file changed, 118 insertions(+) diff --git a/openupgrade_scripts/scripts/stock_account/19.0.1.1/post-migration.py b/openupgrade_scripts/scripts/stock_account/19.0.1.1/post-migration.py index c90e05afd159..ad252f87384c 100644 --- a/openupgrade_scripts/scripts/stock_account/19.0.1.1/post-migration.py +++ b/openupgrade_scripts/scripts/stock_account/19.0.1.1/post-migration.py @@ -94,6 +94,122 @@ def stock_move_value(env): ) +def product_value_unit_cost(env): + """ + Convert product.value#value from a total value to a unit cost where 19.0 + expects one. + + 17.0 stock.valuation.layer#value held the *total* value of the layer, with + the per-unit price in the separate unit_cost column. 19.0 + product.value#value is a unit cost for records not linked to a move: those + are what product.product#_get_last_product_value() reads to determine + standard_price. Records linked to a move keep total-value semantics and are + consumed by stock_move_value() above, so they are left alone -- which is + also why this has to run after it. + + Without this, a move-less record keeps a total value where 19.0 reads a + unit cost, silently corrupting standard_price and every COGS entry derived + from it. + + Two sources, in order of directness: + + 1. unit_cost, which pre-migration's rename_tables() leaves in place as an + orphan column. Exact wherever 17.0 populated it. + 2. For manual revaluations 17.0 wrote unit_cost NULL and quantity 0, so the + unit cost is derived the same way 17.0 itself did in + product.product#_prepare_valuation_layer_field_values(): the running + sum(value) / sum(quantity) over the product's layers up to that point. + Evaluating it per record reproduces the historical cost at each record's + own date, which matters because _get_last_product_value() selects by + date. + + Being an accumulation over the layers rather than a per-layer field, (2) is + cost-method independent and so covers fifo -- where a revaluation is spread + over the remaining_value of the layers still in stock and never lands in a + per-product unit cost -- as well as average. + """ + # The accumulation in (2) sums the 17.0 total values, so snapshot them + # before (1) starts overwriting them with unit costs. + openupgrade.logged_query( + env.cr, + """ + CREATE TEMPORARY TABLE openupgrade_17_layer_value AS + SELECT id, product_id, company_id, date, value, quantity + FROM product_value + """, + ) + openupgrade.logged_query( + env.cr, + """ + CREATE INDEX openupgrade_17_layer_value_idx + ON openupgrade_17_layer_value (product_id, company_id, date, id) + """, + ) + + openupgrade.logged_query( + env.cr, + """ + UPDATE product_value + SET value = unit_cost + WHERE move_id IS NULL + AND quantity IS NOT NULL + AND unit_cost IS NOT NULL + """, + ) + openupgrade.logged_query( + env.cr, + """ + WITH computed AS ( + SELECT target.id, + sum(layer.value) / sum(layer.quantity) AS unit_cost + FROM product_value target + JOIN openupgrade_17_layer_value layer + ON layer.product_id = target.product_id + AND layer.company_id = target.company_id + AND (layer.date, layer.id) <= (target.date, target.id) + WHERE target.move_id IS NULL + AND target.quantity IS NOT NULL + AND target.unit_cost IS NULL + GROUP BY target.id + HAVING sum(layer.quantity) <> 0 + ) + UPDATE product_value + SET value = computed.unit_cost + FROM computed + WHERE product_value.id = computed.id + """, + ) + + # A product with no quantity on hand at that point has no meaningful unit + # cost to derive. Leave those alone and say so: a wrong standard_price is + # silent and ends up in the accounts, so guessing is worse than reporting. + env.cr.execute( + """ + SELECT target.id, target.description + FROM product_value target + LEFT JOIN LATERAL ( + SELECT sum(layer.quantity) AS quantity + FROM openupgrade_17_layer_value layer + WHERE layer.product_id = target.product_id + AND layer.company_id = target.company_id + AND (layer.date, layer.id) <= (target.date, target.id) + ) accumulated ON TRUE + WHERE target.move_id IS NULL + AND target.quantity IS NOT NULL + AND target.unit_cost IS NULL + AND coalesce(accumulated.quantity, 0) = 0 + """ + ) + for value_id, description in env.cr.fetchall(): + openupgrade.logger.warning( + "stock_account: product.value %s keeps its 17.0 total value because " + "the product had no quantity on hand to derive a unit cost from; " + "review it manually as it feeds standard_price. Description: %s", + value_id, + description, + ) + + @openupgrade.migrate() def migrate(env, version): product_value_product_id(env) @@ -101,3 +217,5 @@ def migrate(env, version): product_category_property_valuation(env) stock_location_valuation_account_id(env) stock_move_value(env) + # Must stay last: it rewrites the values stock_move_value() reads. + product_value_unit_cost(env)