-
-
Notifications
You must be signed in to change notification settings - Fork 196
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[14.0] stock_available_to_promise_release: Allow unrelease processed qties #971
base: 14.0
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -9,7 +9,7 @@ | |||||||||||||
from odoo import _, api, fields, models | ||||||||||||||
from odoo.exceptions import UserError | ||||||||||||||
from odoo.osv import expression | ||||||||||||||
from odoo.tools import date_utils, float_compare, float_round, groupby | ||||||||||||||
from odoo.tools import date_utils, float_compare, float_is_zero, float_round, groupby | ||||||||||||||
|
||||||||||||||
_logger = logging.getLogger(__name__) | ||||||||||||||
|
||||||||||||||
|
@@ -100,8 +100,14 @@ def _is_unrelease_allowed_on_origin_moves(self, origin_moves): | |||||||||||||
# The picking is printed, we can't unrelease the move | ||||||||||||||
# because the processing of the origin moves is started. | ||||||||||||||
return False | ||||||||||||||
# If allow_unrelease_on_cancel is not checked, only release assigned/waiting | ||||||||||||||
# moves. If checked, also unrelease done moves. | ||||||||||||||
if self.rule_id.allow_unrelease_return_done_move: | ||||||||||||||
unreleasable_states = ("cancel",) | ||||||||||||||
else: | ||||||||||||||
unreleasable_states = ("done", "cancel") | ||||||||||||||
origin_moves = origin_moves.filtered( | ||||||||||||||
lambda m: m.state not in ("done", "cancel") | ||||||||||||||
lambda m: m.state not in unreleasable_states | ||||||||||||||
) | ||||||||||||||
origin_qty_todo = sum(origin_moves.mapped("product_qty")) | ||||||||||||||
return ( | ||||||||||||||
|
@@ -591,6 +597,39 @@ def _get_chained_moves_iterator(self, chain_field): | |||||||||||||
yield moves | ||||||||||||||
moves = moves.mapped(chain_field) | ||||||||||||||
|
||||||||||||||
def _return_quantity_in_stock(self, quantity_to_return): | ||||||||||||||
picking = self.picking_id | ||||||||||||||
picking.ensure_one() | ||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||
|
||||||||||||||
returned_quantity = 0 | ||||||||||||||
# create return | ||||||||||||||
return_type = picking.picking_type_id.return_picking_type_id | ||||||||||||||
jbaudoux marked this conversation as resolved.
Show resolved
Hide resolved
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. without using f-string to not give code in the translations |
||||||||||||||
wiz_values = { | ||||||||||||||
"picking_id": picking.id, | ||||||||||||||
"original_location_id": picking.location_dest_id.id, | ||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is not necessary. Otherwise, it should be
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @mmequignon Can you drop this line? |
||||||||||||||
"location_id": return_type.default_location_dest_id.id, | ||||||||||||||
} | ||||||||||||||
product_return_moves = [] | ||||||||||||||
for move in self: | ||||||||||||||
if not move.state == "done": | ||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||
continue | ||||||||||||||
move_qty = min(quantity_to_return - returned_quantity, move.quantity_done) | ||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Process real quantities and not uom quantities. You can use
Suggested change
|
||||||||||||||
return_move_vals = { | ||||||||||||||
"product_id": move.product_id.id, | ||||||||||||||
"quantity": move_qty, | ||||||||||||||
"uom_id": move.product_id.uom_id.id, | ||||||||||||||
"move_id": move.id, | ||||||||||||||
} | ||||||||||||||
product_return_moves.append((0, 0, return_move_vals)) | ||||||||||||||
returned_quantity += move_qty | ||||||||||||||
jbaudoux marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||
if returned_quantity == quantity_to_return: | ||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. use float compare |
||||||||||||||
break | ||||||||||||||
if product_return_moves: | ||||||||||||||
wiz_values["product_return_moves"] = product_return_moves | ||||||||||||||
return_wiz = self.env["stock.return.picking"].create(wiz_values) | ||||||||||||||
return_wiz.create_returns() | ||||||||||||||
return returned_quantity | ||||||||||||||
|
||||||||||||||
def unrelease(self, safe_unrelease=False): | ||||||||||||||
"""Unrelease unreleasavbe moves | ||||||||||||||
|
||||||||||||||
|
@@ -605,24 +644,66 @@ def unrelease(self, safe_unrelease=False): | |||||||||||||
impacted_picking_ids = set() | ||||||||||||||
|
||||||||||||||
for move in moves_to_unrelease: | ||||||||||||||
rounding = move.product_id.uom_id.rounding | ||||||||||||||
# When a move is returned, it is going straight to WH/Stock, | ||||||||||||||
# skipping all intermediate zones (pick/pack). | ||||||||||||||
# That is why we need to keep track of qty returned along the way. | ||||||||||||||
# We do not want to return the same goods at each step. | ||||||||||||||
# At a given step (pick/pack/ship), qty to return is | ||||||||||||||
# move.product_uom_qty - cancelled_qty_at_step - already returned qties | ||||||||||||||
qty_to_unrelease = move.product_qty | ||||||||||||||
qty_returned_for_move = 0 | ||||||||||||||
mmequignon marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||
iterator = move._get_chained_moves_iterator("move_orig_ids") | ||||||||||||||
moves_to_cancel = self.env["stock.move"] | ||||||||||||||
moves_to_cancel_for_move = self.env["stock.move"] | ||||||||||||||
# backup procure_method as when you don't propagate cancel, the | ||||||||||||||
# destination move is forced to make_to_stock | ||||||||||||||
procure_method = move.procure_method | ||||||||||||||
next(iterator) # skip the current move | ||||||||||||||
for origin_moves in iterator: | ||||||||||||||
done_moves = origin_moves.filtered(lambda m: m.state == "done") | ||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Move this variable down where it is used |
||||||||||||||
origin_moves = origin_moves.filtered( | ||||||||||||||
mmequignon marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||
lambda m: m.state not in ("done", "cancel") | ||||||||||||||
) | ||||||||||||||
qty_cancelled_at_step = 0 | ||||||||||||||
if origin_moves: | ||||||||||||||
origin_moves = move._split_origins(origin_moves) | ||||||||||||||
impacted_picking_ids.update(origin_moves.mapped("picking_id").ids) | ||||||||||||||
qty_to_split = qty_to_unrelease - qty_returned_for_move | ||||||||||||||
# TODO find new name | ||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You can drop this TODO, you fixed it already |
||||||||||||||
moves_to_cancel = move._split_origins( | ||||||||||||||
origin_moves, qty=qty_to_split | ||||||||||||||
) | ||||||||||||||
impacted_picking_ids.update( | ||||||||||||||
moves_to_cancel.mapped("picking_id").ids | ||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No need for mapped
Suggested change
|
||||||||||||||
) | ||||||||||||||
# avoid to propagate cancel to the original move | ||||||||||||||
origin_moves.write({"propagate_cancel": False}) | ||||||||||||||
moves_to_cancel.write({"propagate_cancel": False}) | ||||||||||||||
# origin_moves._action_cancel() | ||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. remove this obsolete commented line |
||||||||||||||
moves_to_cancel |= origin_moves | ||||||||||||||
moves_to_cancel._action_cancel() | ||||||||||||||
moves_to_cancel_for_move |= moves_to_cancel | ||||||||||||||
qty_cancelled_at_step = sum(moves_to_cancel.mapped("product_qty")) | ||||||||||||||
# checking that for the current step (pick/pack/ship) | ||||||||||||||
# move.product_uom_qty == step.cancelled_qty + move.returned_quanty | ||||||||||||||
# If not the case, we have to move back goods in stock. | ||||||||||||||
qty_to_return_at_step = ( | ||||||||||||||
qty_to_unrelease - qty_cancelled_at_step - qty_returned_for_move | ||||||||||||||
) | ||||||||||||||
if float_is_zero(qty_to_return_at_step, precision_rounding=rounding): | ||||||||||||||
continue | ||||||||||||||
Comment on lines
+688
to
+689
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. without using f-string to not give code in the translations |
||||||||||||||
# Multiple pickings can satisfy a move | ||||||||||||||
# -> len(move.move_orig_ids.picking_id) > 1 | ||||||||||||||
# Group done_moves per picking, and create returns | ||||||||||||||
groups_iterator = groupby(done_moves, key=lambda m: m.picking_id) | ||||||||||||||
mmequignon marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||
for __, moves_list in groups_iterator: | ||||||||||||||
moves_to_return = self.browse([move.id for move in moves_list]) | ||||||||||||||
returned_qty = moves_to_return._return_quantity_in_stock( | ||||||||||||||
qty_to_return_at_step | ||||||||||||||
) | ||||||||||||||
qty_returned_for_move += returned_qty | ||||||||||||||
mmequignon marked this conversation as resolved.
Show resolved
Hide resolved
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would move this before the block just after the |
||||||||||||||
qty_to_return_at_step -= returned_qty | ||||||||||||||
if float_is_zero( | ||||||||||||||
qty_to_return_at_step, precision_rounding=rounding | ||||||||||||||
): | ||||||||||||||
break | ||||||||||||||
|
||||||||||||||
mmequignon marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||
moves_to_cancel_for_move._action_cancel() | ||||||||||||||
# restore the procure_method overwritten by _action_cancel() | ||||||||||||||
move.procure_method = procure_method | ||||||||||||||
moves_to_unrelease.write({"need_release": True}) | ||||||||||||||
|
@@ -637,14 +718,15 @@ def unrelease(self, safe_unrelease=False): | |||||||||||||
picking.message_post(body=body) | ||||||||||||||
picking.last_release_date = False | ||||||||||||||
|
||||||||||||||
def _split_origins(self, origins): | ||||||||||||||
def _split_origins(self, origins, qty=None): | ||||||||||||||
"""Split the origins of the move according to the quantity into the | ||||||||||||||
move and the quantity in the origin moves. | ||||||||||||||
|
||||||||||||||
Return the origins for the move's quantity. | ||||||||||||||
""" | ||||||||||||||
self.ensure_one() | ||||||||||||||
qty = self.product_qty | ||||||||||||||
if not qty: | ||||||||||||||
qty = self.product_qty | ||||||||||||||
# Unreserve goods before the split | ||||||||||||||
origins._do_unreserve() | ||||||||||||||
rounding = self.product_uom.rounding | ||||||||||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,136 @@ | ||
# Copyright 2025 Camptocamp SA | ||
# License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl) | ||
from datetime import datetime | ||
|
||
from .common import PromiseReleaseCommonCase | ||
|
||
|
||
class TestAvailableToPromiseReleaseCancel(PromiseReleaseCommonCase): | ||
@classmethod | ||
def setUpClass(cls): | ||
super().setUpClass() | ||
|
||
cls.wh.delivery_steps = "pick_pack_ship" | ||
cls._update_qty_in_location(cls.loc_bin1, cls.product1, 15.0) | ||
|
||
delivery_route = cls.wh.delivery_route_id | ||
ship_rule = delivery_route.rule_ids.filtered( | ||
lambda r: r.location_id == cls.loc_customer | ||
) | ||
cls.loc_output = ship_rule.location_src_id | ||
pack_rule = delivery_route.rule_ids.filtered( | ||
lambda r: r.location_id == cls.loc_output | ||
) | ||
cls.loc_pack = pack_rule.location_src_id | ||
pick_rule = delivery_route.rule_ids.filtered( | ||
lambda r: r.location_id == cls.loc_pack | ||
) | ||
cls.pick_type = pick_rule.picking_type_id | ||
cls.pack_type = pack_rule.picking_type_id | ||
|
||
cls.picking_chain = cls._create_picking_chain( | ||
cls.wh, [(cls.product1, 10)], date=datetime(2019, 9, 2, 16, 0) | ||
) | ||
cls.ship_picking = cls._out_picking(cls.picking_chain) | ||
cls.pack_picking = cls._prev_picking(cls.ship_picking) | ||
cls.pick_picking = cls._prev_picking(cls.pack_picking) | ||
|
||
# Why is this not working when creating picking after enabling this setting? | ||
delivery_route.write( | ||
{ | ||
"available_to_promise_defer_pull": True, | ||
"allow_unrelease_return_done_move": True, | ||
} | ||
) | ||
cls.ship_picking.release_available_to_promise() | ||
cls.cleanup_type = cls.env["stock.picking.type"].create( | ||
{ | ||
"name": "Cancel Cleanup", | ||
"default_location_dest_id": cls.loc_stock.id, | ||
"sequence_code": "CCP", | ||
"code": "internal", | ||
} | ||
) | ||
cls.pick_type.return_picking_type_id = cls.cleanup_type | ||
cls.pack_type.return_picking_type_id = cls.cleanup_type | ||
|
||
@classmethod | ||
def _get_cleanup_picking(cls): | ||
return cls.env["stock.picking"].search( | ||
[("picking_type_id", "=", cls.cleanup_type.id)] | ||
) | ||
|
||
def test_unrelease_picked(self): | ||
# In this case, we should get 1 return picking from | ||
# WH/PACK to WH/STOCK | ||
self._deliver(self.pick_picking) | ||
self.ship_picking.unrelease() | ||
self.assertTrue(self.ship_picking.need_release) | ||
self.assertEqual(self.pack_picking.state, "cancel") | ||
self.assertEqual(self.pick_picking.state, "done") | ||
cancel_picking = self._get_cleanup_picking() | ||
self.assertEqual(len(cancel_picking), 1) | ||
self.assertEqual(cancel_picking.location_id, self.loc_pack) | ||
self.assertEqual(cancel_picking.location_dest_id, self.loc_stock) | ||
|
||
def test_unrelease_packed(self): | ||
# In this case, we should get 1 return picking from | ||
# WH/OUT to WH/STOCK | ||
self._deliver(self.pick_picking) | ||
self._deliver(self.pack_picking) | ||
self.ship_picking.unrelease() | ||
self.assertTrue(self.ship_picking.need_release) | ||
self.assertEqual(self.pack_picking.state, "done") | ||
self.assertEqual(self.pick_picking.state, "done") | ||
cancel_picking = self._get_cleanup_picking() | ||
self.assertEqual(len(cancel_picking), 1) | ||
self.assertEqual(cancel_picking.location_id, self.loc_output) | ||
self.assertEqual(cancel_picking.location_dest_id, self.loc_stock) | ||
|
||
def test_unrelease_picked_partial(self): | ||
qty_picked = [(self.product1, 5.0)] | ||
self._deliver(self.pick_picking, product_qty=qty_picked) | ||
pick_backorder = self._get_backorder_for_pickings(self.pick_picking) | ||
self.assertTrue(pick_backorder) | ||
self.ship_picking.unrelease() | ||
self.assertTrue(self.ship_picking.need_release) | ||
self.assertEqual(self.pack_picking.state, "cancel") | ||
self.assertEqual(self.pick_picking.state, "done") | ||
cancel_picking = self._get_cleanup_picking() | ||
# In the end, we cancelled 5 units for the pick backorder, and returned | ||
# 5 units from pack -> stock | ||
self.assertEqual(pick_backorder.state, "cancel") | ||
self.assertEqual(cancel_picking.location_id, self.loc_pack) | ||
self.assertEqual(cancel_picking.location_dest_id, self.loc_stock) | ||
self.assertEqual(cancel_picking.move_lines.product_uom_qty, 5.0) | ||
|
||
def test_unrelease_packed_partial(self): | ||
self._deliver(self.pick_picking) | ||
qty_packed = [(self.product1, 5.0)] | ||
self._deliver(self.pack_picking, product_qty=qty_packed) | ||
pack_backorder = self._get_backorder_for_pickings(self.pack_picking) | ||
self.assertTrue(pack_backorder) | ||
self.ship_picking.unrelease() | ||
self.assertTrue(self.ship_picking.need_release) | ||
self.assertEqual(self.pack_picking.state, "done") | ||
self.assertEqual(self.pick_picking.state, "done") | ||
cancel_pickings = self._get_cleanup_picking() | ||
self.assertEqual(len(cancel_pickings), 2) | ||
# In the end, we cancelled 5 units for the pack backorder, returned | ||
# 5 units from pack -> stock, and 5 units from output -> stock | ||
pack_cancel = cancel_pickings.filtered(lambda p: p.location_id == self.loc_pack) | ||
ship_cancel = cancel_pickings.filtered( | ||
lambda p: p.location_id == self.loc_output | ||
) | ||
self.assertEqual(pack_cancel.move_lines.product_uom_qty, 5.0) | ||
self.assertEqual(ship_cancel.move_lines.product_uom_qty, 5.0) | ||
|
||
def test_unrelease_shipped(self): | ||
self._deliver(self.pick_picking) | ||
self._deliver(self.pack_picking) | ||
self._deliver(self.ship_picking) | ||
self.ship_picking.unrelease() | ||
# Did nothing | ||
self.assertEqual(self.ship_picking.state, "done") | ||
self.assertEqual(self.pack_picking.state, "done") | ||
self.assertEqual(self.pick_picking.state, "done") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.