Skip to content

[16.0] [FIX] stock_move_location: prevent picking type reset on origin change#2553

Open
alvaro-gmz wants to merge 1 commit intoOCA:16.0from
factorlibre:16.0-fix-stock_move_location-reset-fields
Open

[16.0] [FIX] stock_move_location: prevent picking type reset on origin change#2553
alvaro-gmz wants to merge 1 commit intoOCA:16.0from
factorlibre:16.0-fix-stock_move_location-reset-fields

Conversation

@alvaro-gmz
Copy link
Copy Markdown

@alvaro-gmz alvaro-gmz commented Apr 10, 2026

Description

The move location wizard (wiz.stock.move.location) had two related issues that caused user-entered data to be silently overwritten or lost:

  1. The operation type (picking_type_id) was reset whenever the user changed the origin location or saved the wizard (e.g. clicking "Immediate Transfer" or "Planned Transfer"), discarding any manual selection the user had made.
  2. The destination location could be missing on wizard lines when they were built from the origin onchange before the user had set the destination, causing a NOT NULL constraint error on stock.move.location_dest_id when the transfer was executed.

Root cause

1. Picking type reset

picking_type_id = fields.Many2one(
    compute="_compute_picking_type_id",
    comodel_name="stock.picking.type",
    readonly=False,
    store=True,
)

@api.depends("origin_location_id")
def _compute_picking_type_id(self):
    ...
    rec.picking_type_id = picking_type.id

The combination of compute + store=True + readonly=False with a dependency on origin_location_id means the user could edit the field in the UI, but any write to the record (including button clicks that save the wizard) re-triggered the compute and silently overwrote the manual selection.

Before this behaviour was introduced, the module used a plain default that returned the first internal picking type of the company. The computed variant was added to auto-pick a type based on the origin location, but the side effect of losing the user's input was never intended.

2. Destination missing on lines

When the origin location changes, onchange_origin_location calls _reset_stock_move_location_lines, which builds lines using self.destination_location_id. If the user sets the origin before the destination, the lines end up with no destination. The line-level onchange for the destination propagates the value to the existing lines afterwards, but there is no safeguard when the flow gets the wizard into a state where at least one line reaches the picking creation step without a destination.

Fix

  • Revert picking_type_id to a regular Many2one field with a simple default=_get_default_picking_type_id returning the first internal or outgoing picking type of the company. This matches the original module behaviour before the compute-based change (stable user selection) while keeping the support for outgoing picking types added later in the compute variant.
  • Add a safeguard in action_move_location that copies self.destination_location_id to any line lacking one before the picking is created, so a transfer no longer fails on a NOT NULL constraint when lines were built before the destination was set.
  • Add regression tests that cover both scenarios.

Existing tests that instantiate the wizard via create(...) keep working thanks to the default, and all flows that already set destination_location_id on the lines via onchange are unaffected by the safeguard.

Steps to reproduce

1. Picking type reset

  1. Go to Inventory > Operations > Move from location...
  2. Select an origin location.
  3. Change the "Operation Type" field to a different picking type of your choice.
  4. Click "Planned Transfer" (or change the origin location).
  5. Before the fix: the operation type is reset to the one computed from the origin, discarding the user's selection.
  6. After the fix: the operation type keeps the user's selection.

2. Destination missing

  1. Go to Inventory > Operations > Move from location...
  2. Select an origin location (lines are created without destination at this point).
  3. Select a destination location.
  4. Click "Planned Transfer".
  5. Before the fix: IntegrityError: null value in column "location_dest_id" of relation "stock_move" violates not-null constraint.
  6. After the fix: the picking is created with the expected destination.

Changes

  • stock_move_location/wizard/stock_move_location.py:
    • Restore picking_type_id to a regular field with default=_get_default_picking_type_id (first internal or outgoing picking type).
    • Remove the compute, helper and onchange introduced by the computed variant.
    • Add a safeguard in action_move_location to fill destination_location_id on lines missing it.
  • stock_move_location/tests/test_move_location.py:
    • Add test_picking_type_preserved_on_origin_change to assert the user's manual picking type is not overwritten when the origin location changes.
    • Add test_destination_propagated_to_lines_without_dest to assert the safeguard fills the destination on lines lacking one before creating the picking.

@alvaro-gmz alvaro-gmz force-pushed the 16.0-fix-stock_move_location-reset-fields branch 2 times, most recently from 9d67cb5 to 462c226 Compare April 13, 2026 10:37
…tination

The picking_type_id field was defined as a stored computed field
depending on origin_location_id. Every write to the wizard triggered
the compute and overwrote the user's manual selection, so the
operation type was reset when changing origin or clicking
"Immediate/Planned Transfer".

Additionally, lines created from the onchange on origin could end up
with an empty destination_location_id (the user hadn't set it yet at
that point), leading to a NOT NULL constraint error when creating the
stock.move on transfer execution.

Changes:
- Restore picking_type_id as a regular Many2one field with a simple
  default (first internal or outgoing picking type for the company),
  which matches the behaviour the module had before the compute-based
  variant was introduced. The user's manual selection is now preserved
  throughout the whole wizard lifecycle and the outgoing picking types
  added by the ACSONE improvement are still supported as default.
- Add a safeguard in action_move_location that propagates the
  wizard's destination_location_id to any line that lacks one before
  creating the picking, so "Planned Transfer" no longer fails when
  lines were built before the destination was set.
- Add regression tests covering both fixes.
@alvaro-gmz alvaro-gmz force-pushed the 16.0-fix-stock_move_location-reset-fields branch from 462c226 to 552e929 Compare April 13, 2026 10:56
@alvaro-gmz alvaro-gmz marked this pull request as ready for review April 15, 2026 13:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants