Skip to content

[18.0][FIX] stock_secondary_unit: Keep the count exact across the whole route - #2640

Open
carlosdauden wants to merge 5 commits into
OCA:18.0from
Tecnativa:18.0-FIX-stock_secondary_unit_independent_route_integrity
Open

carlosdauden wants to merge 5 commits into
OCA:18.0from
Tecnativa:18.0-FIX-stock_secondary_unit_independent_route_integrity

Conversation

@carlosdauden

@carlosdauden carlosdauden commented Sep 7, 2026

Copy link
Copy Markdown

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.

Depends on:

@Tecnativa

ping @sergio-teruel @carlos-lopez-tecnativa @CarlosRoca13

@OCA-git-bot OCA-git-bot added series:18.0 mod:stock_secondary_unit Module stock_secondary_unit labels Sep 7, 2026
@carlosdauden
carlosdauden force-pushed the 18.0-FIX-stock_secondary_unit_independent_route_integrity branch from 5fd588c to e7c2af5 Compare September 14, 2026 06:23

@carlos-lopez-tecnativa carlos-lopez-tecnativa left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add the external PR to test-requirements.txt in a temporary commit so it can be tested correctly on Runboat.

@carlosdauden

Copy link
Copy Markdown
Author

Changes done

Comment thread stock_secondary_unit/models/stock_move.py
@carlosdauden
carlosdauden force-pushed the 18.0-FIX-stock_secondary_unit_independent_route_integrity branch from 8e3bb75 to a0099fc Compare September 16, 2026 11:33
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
carlosdauden force-pushed the 18.0-FIX-stock_secondary_unit_independent_route_integrity branch from a0099fc to 34bce49 Compare September 17, 2026 18:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

mod:stock_secondary_unit Module stock_secondary_unit series:18.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants