[16.0][ADD] stock_picking_operation_loss_quantity#1394
[16.0][ADD] stock_picking_operation_loss_quantity#1394rousseldenis wants to merge 21 commits intoOCA:16.0from
Conversation
368c3d7 to
dff9c13
Compare
|
depends on #1330 |
255be27 to
bf41a93
Compare
|
What this module needs is to make the user access inventory easily through the Loss picking menu |
3bd8305 to
56858c3
Compare
|
There hasn't been any activity on this pull request in the past 4 months, so it has been marked as stale and it will be closed automatically if no further activity occurs in the next 30 days. |
56858c3 to
8b3802c
Compare
|
There hasn't been any activity on this pull request in the past 4 months, so it has been marked as stale and it will be closed automatically if no further activity occurs in the next 30 days. |
|
ping @rousseldenis This one is used in our project and in production for years... |
|
There hasn't been any activity on this pull request in the past 4 months, so it has been marked as stale and it will be closed automatically if no further activity occurs in the next 30 days. |
…on inventory When an inventory is validated, we need to cancel any remaining pending moves created to make the quantity no more available in case of loss declaration.
…ancel the loss move When user want to apply inventory, be less restrictive on inventory_mode check as the values should have been checked in stock module.
Update the warehouse loss sequence creation to use the specific code defined in the loss picking type. If no code is defined, it falls back to the default "LOSS" string.
…izards + clean code - Relocate the loss declaration code from external wizard to the stock.move.line model - Remove useless grouping logic (the code is always called on a single line through UI) - Rename variables in for better clarity - Remove code that was never reached in unit tests and that seemed dead - Clean and improve unit tests (add quants verifications)
…ines to same loss picking
8b3802c to
2fa0d9f
Compare
… do not get merged
…shold Introduce a 'Loss Auto-Clear Threshold' setting on the warehouse. When a product/location combination (quant) reaches this number of consecutive loss declarations, the system automatically reserves the remaining quantity. This prevents redundant manual declarations for "ghost" stock and improves warehouse efficiency by stopping future workers from attempting to pick non-existent products.
…a loss on loss pickings
…e lines to count number of loss declaration on loss picking Each loss declaration creates a single move on the loss picking but it is not sure that every move has a single move line
…s confirmed and linked to a move line when using `loss_auto_clear_threshold=1`
…at auto-clearing parent location also clears child location
lmignon
left a comment
There was a problem hiding this comment.
@nicolas-delbovier-acsone Thank you for the improvements. However, I do have some concerns about the usability of the approach. See my comment.
| new_loss_move_vals = { | ||
| "name": self.product_id.display_name, | ||
| "product_id": self.product_id.id, | ||
| "product_uom_qty": unprocessed_qty, # This is the "demand" | ||
| "product_uom": self.product_uom_id.id, | ||
| "location_id": self.location_id.id, | ||
| "location_dest_id": loss_pick_type.default_location_dest_id.id, | ||
| "lot_ids": [Command.set(self.lot_id.ids)] if self.lot_id else False, | ||
| } | ||
|
|
There was a problem hiding this comment.
@nicolas-delbovier-acsone For flexibility and readability this code should be put into a _prepare_loss_move_vals(self, qty) method `
| search_domain = [ | ||
| ("reserved_uom_qty", ">", 0.0), | ||
| ("product_id", "=", self.product_id.id), | ||
| ("location_id", "=", self.location_id.id), | ||
| ("lot_id", "=", self.lot_id.id), | ||
| ("package_id", "=", self.package_id.id), | ||
| ("owner_id", "=", self.owner_id.id), | ||
| ("state", "not in", ("done", "cancel")), | ||
| ("picking_type_id", "=", loss_pick_type.id), | ||
| ( | ||
| "location_dest_id", | ||
| "=", | ||
| loss_pick_type.default_location_dest_id.id, | ||
| ), | ||
| ] | ||
| similar_loss_lines = self.env["stock.move.line"].search(search_domain) |
There was a problem hiding this comment.
IMO This code should also be put into a dedicated method and the domain also provided by a specific method
| } | ||
| ) | ||
| new_loss_move._action_confirm(merge=False) | ||
|
|
There was a problem hiding this comment.
I can see a problem with this approach. In the end, if the product hasn’t been found, we’re left with n identical moves plus a final move with the remaining quantities in the location. For the operator, who only sees the transactions, this will pose a problem. In my view, it would be preferable to merge the moves and retain the number of loss declarations already made for the same product/batch in the same location.
| "picking_type_id": loss_pick_type.id, | ||
| "location_id": self.location_id.id, | ||
| "location_dest_id": loss_pick_type.default_location_dest_id.id, |
There was a problem hiding this comment.
IMO the information required to create the picking should be provided by a prepare method _prepare_loss_picking_vals ....
Refactor the module to improve extensibility and readability by extracting search domains and write/create values into dedicated methods.
…MS ambiguity Rename `_release_unprocessed_qty` to `_unreserve_unprocessed_qty`. The term 'release' can be confused with WMS-specific logic. Using 'unreserve' better aligns with standard Odoo stock terminology and clarifies that the function handles the unreservation of quantities.
…mber of loss declaration instead of relying on number of moves on the picking
…ged in loss picking
lmignon
left a comment
There was a problem hiding this comment.
@nicolas-delbovier-acsone Some little comments about methods and variable naming but it looks good 😏
| ), | ||
| ] | ||
|
|
||
| def _find_loss_picking(self): |
There was a problem hiding this comment.
@nicolas-delbovier-acsone You search for a stock.move.line not for a picking. This method should be renamed.
There was a problem hiding this comment.
No, I am indeed looking for a picking.
I look for a picking of type "LOSS" that contains moves that match with my new one I want to create:
def _find_loss_picking(self):
similar_loss_lines = self.env["stock.move.line"].search(
self._find_loss_picking_moves_domain()
)
loss_picking = first(similar_loss_lines.picking_id) # <- LOOK HERE
return loss_picking| ) | ||
|
|
||
| # Search for an already existing LOSS picking for this quant | ||
| loss_picking = self._find_loss_picking() |
There was a problem hiding this comment.
This variable name is confusing loss_picking seems to refer to a picking but in fact it's a stock.move.line....
There was a problem hiding this comment.
It is a picking, see my comment above
Depends on: