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

Conversation

mmequignon
Copy link
Member

@mmequignon mmequignon commented Jan 13, 2025

This PR adds a new option on stock routes, allowing to unrelease a delivery even if some internal operations (e.g. pick) are already done, creating a return move to the stock location.

Copy link
Contributor

@jbaudoux jbaudoux left a comment

Choose a reason for hiding this comment

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

Need to adapt README as well

stock_available_to_promise_release/models/stock_move.py Outdated Show resolved Hide resolved
stock_available_to_promise_release/models/stock_move.py Outdated Show resolved Hide resolved
stock_available_to_promise_release/models/stock_move.py Outdated Show resolved Hide resolved
return_type = picking.picking_type_id.return_picking_type_id
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?

@jbaudoux
Copy link
Contributor

cc @rousseldenis @lmignon

Copy link
Contributor

@jbaudoux jbaudoux left a comment

Choose a reason for hiding this comment

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

Some minor remarks

}
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":

for move in self:
if not 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)

}
product_return_moves.append((0, 0, return_move_vals))
returned_quantity += move_qty
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

@@ -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"""

@@ -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()
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_qty = moves_to_return._return_quantity_in_stock(
qty_to_return_at_step
)
qty_returned_for_move += returned_qty
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

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

# 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

Comment on lines +688 to +689
if float_is_zero(qty_to_return_at_step, precision_rounding=rounding):
continue
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


returned_quantity = 0
# create return
return_type = picking.picking_type_id.return_picking_type_id
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

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