[18.0][FIX] stock_secondary_unit: Keep the count exact across the whole route - #2640
Open
carlosdauden wants to merge 5 commits into
Open
carlosdauden wants to merge 5 commits into
carlosdauden wants to merge 5 commits into
Conversation
carlosdauden
force-pushed
the
18.0-FIX-stock_secondary_unit_independent_route_integrity
branch
from
September 14, 2026 06:23
5fd588c to
e7c2af5
Compare
carlos-lopez-tecnativa
suggested changes
Sep 14, 2026
carlos-lopez-tecnativa
left a comment
Contributor
There was a problem hiding this comment.
Please add the external PR to test-requirements.txt in a temporary commit so it can be tested correctly on Runboat.
Author
|
Changes done |
carlosdauden
force-pushed
the
18.0-FIX-stock_secondary_unit_independent_route_integrity
branch
from
September 16, 2026 11:33
8e3bb75 to
a0099fc
Compare
Partner sells fish by weight but some orders are placed by piece count instead (e.g. "40 pieces" for 40 dinner guests) - the count must survive exactly through every hop of the route and every backorder, never be recomputed from the weight actually processed. Adds a new "secondary_priority" dependency_type (the weight is still estimated from the count on entry, like "dependent", but the count is never itself recomputed back from the weight, like "independent") and makes every quantity-touching path respect it for both this type and the existing "independent" one, gated behind a shared COUNT_PRESERVING_DEPENDENCY_TYPES constant: - `_prepare_move_split_vals()`: the backorder created on a partial validation gets exactly demand-minus-done pieces, never a weight-proportional estimate; the "done" move's own count is synced to what was actually processed instead of staying at the stale original demand. Uses `_write()` + `invalidate_recordset()` to avoid retriggering the primary-quantity compute and clobbering the just-set real weight with a factor-based guess. - `StockMoveLine._compute_quantity()`: once a line's count has been measured, a later write to a dependency no longer overwrites it - "secondary_priority" needs this same protection "independent" already had, since on a move LINE the target field is what was actually counted, not a demand estimate. - `StockRule._push_prepare_move_copy_values()` (new): a route mixing pull and push rules (e.g. pick_ship) creates its second leg via `copy()` only after the first leg validates - core copies the DONE quantity forward, never the demand, so the pushed move must do the same for the secondary unit instead of losing the count to `copy=False` and falling back to a weight-derived guess. - `_merge_moves_fields()`: sums the count across merging moves (e.g. a backorder's pushed leg merging into an existing move) instead of silently keeping whichever move happened to survive the merge. - `secondary_uom_qty_done` (new stored field): core has both a demand (`product_uom_qty`) and a done aggregate (`quantity`, computed from `move_line_ids.quantity`) on stock.move: the secondary unit only ever had the demand half. Mirrors the done half the same way, so the above no longer need to re-derive "how many pieces were actually done" inline every time. Tests cover the backorder split for both dependency types, the move-line protection, and the pick_ship push/merge scenario with and without a backorder.
…nt propagation
Two more real gaps found via git archaeology against the sibling
version projects, on top of what's already on this branch:
- _merge_moves(): absorbing a negative move (e.g. a return) into a
positive one is a separate code path in core from the ordinary
positive-move merge - it only rebalances product_uom_qty
(stock.move._merge_moves' own neg_qty_moves loop), never calling
_merge_moves_fields(). This existed in 15.0, was lost in the 16.0/17.0
migration. Snapshots each negative move's secondary_uom_qty by its
merge key before the real merge runs, then applies the matching
rebalancing to whichever move survives - exact addition for
COUNT_PRESERVING_DEPENDENCY_TYPES, a factor-based recompute
otherwise. Verified live against a throwaway savepoint before writing
the permanent test.
- _prepare_procurement_values(): a move-triggered pull-chain
procurement (2-step reception, internal MTO) had no way to carry the
secondary unit forward at all - this existed in 15.0 too. Only has an
externally visible effect when a _get_custom_move_fields() override
elsewhere (e.g. sale_stock_secondary_unit's) whitelists these keys,
same coupling that already existed in 15.0.
Plus a collateral bug fix, found during the same investigation, not a
version regression: _get_aggregated_product_quantities() used `=`
instead of `+=` for secondary_uom_qty. Core's own aggregation key
purposely ignores lots/SNs ("these are expected to already be properly
grouped by line"), so several move lines commonly share one key (e.g.
different lots of the same product) - the last one processed was
silently overwriting instead of accumulating with earlier ones,
understating the count shown on the delivery slip report whenever that
happens.
Also adds a regression test for a historical bug (TT59838, "secondary
uom qty accumulated when only product_uom_qty is changed") that the
existing merge_extra guard in _merge_moves_fields() already prevents,
but was never directly asserted.
Rewrote readme/DESCRIPTION.md and readme/USAGE.md covering every real use case this session built or recovered: backorder split preservation, push+merge survival on pick_ship routes, negative-move (return) merge absorption, pull-chain procurement propagation, the secondary_uom_qty_done aggregate, and the delivery-slip aggregation fix. README.rst/index.html regenerated from the fragments via pre-commit.
Pin product_secondary_unit to OCA/product-attribute#2398 so Runboat and CI can test the "secondary_priority" dependency_type this module needs before that PR merges. Remove once it does.
carlosdauden
force-pushed
the
18.0-FIX-stock_secondary_unit_independent_route_integrity
branch
from
September 17, 2026 18:36
a0099fc to
34bce49
Compare
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
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.
Partner sells fish by weight but some orders are placed by piece
count instead (e.g. "40 pieces" for 40 dinner guests) - the count must
survive exactly through every hop of the route and every backorder,
never be recomputed from the weight actually processed.
Adds a new "secondary_priority" dependency_type (the weight is still
estimated from the count on entry, like "dependent", but the count is
never itself recomputed back from the weight, like "independent") and
makes every quantity-touching path respect it for both this type and
the existing "independent" one, gated behind a shared
COUNT_PRESERVING_DEPENDENCY_TYPES constant:
_prepare_move_split_vals(): the backorder created on a partialvalidation gets exactly demand-minus-done pieces, never a
weight-proportional estimate; the "done" move's own count is synced
to what was actually processed instead of staying at the stale
original demand. Uses
_write()+invalidate_recordset()to avoidretriggering the primary-quantity compute and clobbering the just-set
real weight with a factor-based guess.
StockMoveLine._compute_quantity(): once a line's count has beenmeasured, a later write to a dependency no longer overwrites it -
"secondary_priority" needs this same protection "independent" already
had, since on a move LINE the target field is what was actually
counted, not a demand estimate.
StockRule._push_prepare_move_copy_values()(new): a route mixingpull and push rules (e.g. pick_ship) creates its second leg via
copy()only after the first leg validates - core copies the DONEquantity forward, never the demand, so the pushed move must do the
same for the secondary unit instead of losing the count to
copy=Falseand falling back to a weight-derived guess._merge_moves_fields(): sums the count across merging moves (e.g. abackorder's pushed leg merging into an existing move) instead of
silently keeping whichever move happened to survive the merge.
secondary_uom_qty_done(new stored field): core has both a demand(
product_uom_qty) and a done aggregate (quantity, computed frommove_line_ids.quantity) on stock.move: the secondary unit only everhad the demand half. Mirrors the done half the same way, so the
above no longer need to re-derive "how many pieces were actually
done" inline every time.
Tests cover the backorder split for both dependency types, the
move-line protection, and the pick_ship push/merge scenario with and
without a backorder.
Depends on:
@Tecnativa
ping @sergio-teruel @carlos-lopez-tecnativa @CarlosRoca13