Skip to content
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

Open
wants to merge 1 commit into
base: 14.0
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions stock_available_to_promise_release/models/stock_location_route.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,16 @@
class Route(models.Model):
_inherit = "stock.location.route"

allow_unrelease_return_done_move = fields.Boolean(
string="Reverse done transfer on cancellation",
default=False,
help=(
"If checked, unreleasing the delivery may create a new inverse "
"internal operation on the last done pulled transfer. "
"Otherwise, you won't be able to unrelease as soon as one of "
"the pulled transfer is done"
),
)
available_to_promise_defer_pull = fields.Boolean(
string="Release based on Available to Promise",
default=False,
Expand Down
102 changes: 92 additions & 10 deletions stock_available_to_promise_release/models/stock_move.py
Original file line number Diff line number Diff line change
Expand Up @@ -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__)

Expand Down Expand Up @@ -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 (
Expand Down Expand Up @@ -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):
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
def _return_quantity_in_stock(self, quantity_to_return):
def _return_quantity_in_stock(self, quantity_to_return):
"""Return a quantity from a list of moves.
The quantity to return is in the product uom"""

picking = self.picking_id
picking.ensure_one()
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
picking.ensure_one()
picking.ensure_one()
move.product_id.ensure_one()


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
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
return_type = picking.picking_type_id.return_picking_type_id
return_type = picking.picking_type_id.return_picking_type_id
if not return_type:
raise UserError(_(f"The operation {picking.name} is done and cannot be returned"))

Copy link
Contributor

Choose a reason for hiding this comment

The 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,
Copy link
Contributor

Choose a reason for hiding this comment

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

This is not necessary. Otherwise, it should be

Suggested change
"original_location_id": picking.location_dest_id.id,
"original_location_id": picking.location_id.id,

Copy link
Contributor

Choose a reason for hiding this comment

The 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":
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if not move.state == "done":
if move.state != "done":

continue
move_qty = min(quantity_to_return - returned_quantity, move.quantity_done)
Copy link
Contributor

Choose a reason for hiding this comment

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

Process real quantities and not uom quantities. You can use product_qty as the move is done

Suggested change
move_qty = min(quantity_to_return - returned_quantity, move.quantity_done)
move_qty = min(quantity_to_return - returned_quantity, move.product_qty)

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:
Copy link
Contributor

Choose a reason for hiding this comment

The 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

Expand All @@ -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")
Copy link
Contributor

Choose a reason for hiding this comment

The 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
Copy link
Contributor

Choose a reason for hiding this comment

The 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
Copy link
Contributor

Choose a reason for hiding this comment

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

No need for mapped

Suggested change
moves_to_cancel.mapped("picking_id").ids
moves_to_cancel.picking_id.ids

)
# 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()
Copy link
Contributor

Choose a reason for hiding this comment

The 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
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if float_is_zero(qty_to_return_at_step, precision_rounding=rounding):
continue
if float_is_zero(qty_to_return_at_step, precision_rounding=rounding):
continue
elif not move.rule_id.allow_unrelease_return_done_move:
raise UserError(_(f"You cannot unrelease the move {move.name} because some origin moves ', '.join(done_moves.mapped("name"))} are done"))

Copy link
Contributor

Choose a reason for hiding this comment

The 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
Copy link
Contributor

Choose a reason for hiding this comment

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

I would move this before the block just after the if float_is_zero
qty_returned_for_move += qty_to_return_at_step

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})
Expand All @@ -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
Expand Down
4 changes: 4 additions & 0 deletions stock_available_to_promise_release/models/stock_rule.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,10 @@ class StockRule(models.Model):
related="route_id.available_to_promise_defer_pull", store=True
)

allow_unrelease_return_done_move = fields.Boolean(
related="route_id.allow_unrelease_return_done_move", store=True
)

no_backorder_at_release = fields.Boolean(
related="route_id.no_backorder_at_release", store=True
)
Expand Down
16 changes: 13 additions & 3 deletions stock_available_to_promise_release/tests/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -128,8 +128,18 @@ def _out_picking(cls, pickings):
return pickings.filtered(lambda r: r.picking_type_code == "outgoing")

@classmethod
def _deliver(cls, picking):
def _get_backorder_for_pickings(cls, pickings):
return cls.env["stock.picking"].search([("backorder_id", "in", pickings.ids)])

@classmethod
def _deliver(cls, picking, product_qty=None):
picking.action_assign()
for line in picking.mapped("move_lines.move_line_ids"):
line.qty_done = line.product_uom_qty
if product_qty:
lines = picking.move_lines.move_line_ids
for product, qty in product_qty:
line = lines.filtered(lambda m: m.product_id == product)
line.qty_done = qty
else:
for line in picking.mapped("move_lines.move_line_ids"):
line.qty_done = line.product_uom_qty
picking._action_done()
136 changes: 136 additions & 0 deletions stock_available_to_promise_release/tests/test_unrelease_cancel.py
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")
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
<field name="arch" type="xml">
<xpath expr="//group/field[@name='company_id']" position="after">
<field name="available_to_promise_defer_pull" />
<field name="allow_unrelease_return_done_move" />
<field name="no_backorder_at_release" />
</xpath>
</field>
Expand Down
Loading