[FIX] stock: allow to validate a picking with 2 extra moves
Use case to reproduce: - Create a picking containing 2 moves that have a quantity_done greater than reserved quantity - Validate the picking with or without backorder Traceback due to a record that do not exist anymore. It happens due to 2 functionality that have a wrong behavior when used together: - merge_move function that will try to merge moves with same characteristics inside a same picking. - action_done functionality that will create an extra move if the quantity done for a move is greater than reserved quantity. (the extra move is used in order to propagate changes) With our usecase: - MOVE A, reserved_qty: 10, qty_done: 20 - MOVE B, reserved_qty: 10, qty_done: 15 action_done on move A will create a new move thus we will have - MOVE A, reserved_qty: 10, qty_done: 20 - A bis, reserved_qty: 10, qty_done: 0 - MOVE B, reserved_qty: 10, qty_done: 15 then merge move will not only merge move A and A bis but will also merge B which result with inconsenstencies and traceback since system will try to process move B after A - MOVE A,A',B reserved_qty:30, qty_done:35 (the 5 extra qty is not propagated) This commit adds a kwarg in action_confirm that is propagated to merge_move this kwargs is a record set taht will limit the moves that can be used for the merge in order to merge extra move in original move opw-1825264
This commit is contained in:
@@ -137,12 +137,12 @@ class StockMove(models.Model):
|
||||
If you want to cancel this MO, please change the consumed quantities to 0.'))
|
||||
return super(StockMove, self)._action_cancel()
|
||||
|
||||
def _action_confirm(self, merge=True):
|
||||
def _action_confirm(self, merge=True, merge_into=False):
|
||||
moves = self.env['stock.move']
|
||||
for move in self:
|
||||
moves |= move.action_explode()
|
||||
# we go further with the list of ids potentially changed by action_explode
|
||||
return super(StockMove, moves)._action_confirm(merge=merge)
|
||||
return super(StockMove, moves)._action_confirm(merge=merge, merge_into=merge_into)
|
||||
|
||||
def action_explode(self):
|
||||
""" Explodes pickings """
|
||||
|
||||
@@ -28,7 +28,7 @@ class StockMove(models.Model):
|
||||
|
||||
def _action_done(self):
|
||||
result = super(StockMove, self)._action_done()
|
||||
for line in self.mapped('sale_line_id'):
|
||||
for line in result.mapped('sale_line_id'):
|
||||
line.qty_delivered = line._get_delivered_qty()
|
||||
return result
|
||||
|
||||
|
||||
@@ -535,7 +535,7 @@ class StockMove(models.Model):
|
||||
move.product_uom.id, move.restrict_partner_id.id, move.scrapped, move.origin_returned_move_id.id
|
||||
]
|
||||
|
||||
def _merge_moves(self):
|
||||
def _merge_moves(self, merge_into=False):
|
||||
""" This method will, for each move in `self`, go up in their linked picking and try to
|
||||
find in their existing moves a candidate into which we can merge the move.
|
||||
:return: Recordset of moves passed to this method. If some of the passed moves were merged
|
||||
@@ -543,12 +543,19 @@ class StockMove(models.Model):
|
||||
"""
|
||||
distinct_fields = self._prepare_merge_moves_distinct_fields()
|
||||
|
||||
candidate_moves_list = []
|
||||
if not merge_into:
|
||||
for picking in self.mapped('picking_id'):
|
||||
candidate_moves_list.append(picking.move_lines)
|
||||
else:
|
||||
candidate_moves_list.append(merge_into | self)
|
||||
|
||||
# Move removed after merge
|
||||
moves_to_unlink = self.env['stock.move']
|
||||
moves_to_merge = []
|
||||
for picking in self.mapped('picking_id'):
|
||||
for candidate_moves in candidate_moves_list:
|
||||
# First step find move to merge.
|
||||
for k, g in groupby(sorted(picking.move_lines, key=self._prepare_merge_move_sort_method), key=itemgetter(*distinct_fields)):
|
||||
for k, g in groupby(sorted(candidate_moves, key=self._prepare_merge_move_sort_method), key=itemgetter(*distinct_fields)):
|
||||
moves = self.env['stock.move'].concat(*g).filtered(lambda m: m.state not in ('done', 'cancel', 'draft'))
|
||||
# If we have multiple records we will merge then in a single one.
|
||||
if len(moves) > 1:
|
||||
@@ -697,7 +704,7 @@ class StockMove(models.Model):
|
||||
'location_dest_id': self.location_dest_id.id,
|
||||
}
|
||||
|
||||
def _action_confirm(self, merge=True):
|
||||
def _action_confirm(self, merge=True, merge_into=False):
|
||||
""" Confirms stock move or put it in waiting if it's linked to another move.
|
||||
:param: merge: According to this boolean, a newly confirmed move will be merged
|
||||
in another move of the same picking sharing its characteristics.
|
||||
@@ -737,7 +744,7 @@ class StockMove(models.Model):
|
||||
moves._assign_picking()
|
||||
self._push_apply()
|
||||
if merge:
|
||||
return self._merge_moves()
|
||||
return self._merge_moves(merge_into=merge_into)
|
||||
return self
|
||||
|
||||
def _prepare_procurement_values(self):
|
||||
@@ -973,7 +980,7 @@ class StockMove(models.Model):
|
||||
The rationale for the creation of an extra move is the application of a potential push
|
||||
rule that will handle the extra quantities.
|
||||
"""
|
||||
extra_move = self.env['stock.move']
|
||||
extra_move = self
|
||||
rounding = self.product_uom.rounding
|
||||
# moves created after the picking is assigned do not have `product_uom_qty`, but we shouldn't create extra moves for them
|
||||
if float_compare(self.quantity_done, self.product_uom_qty, precision_rounding=rounding) > 0:
|
||||
@@ -983,7 +990,11 @@ class StockMove(models.Model):
|
||||
precision_rounding=self.product_uom.rounding,
|
||||
rounding_method ='UP')
|
||||
extra_move_vals = self._prepare_extra_move_vals(extra_move_quantity)
|
||||
extra_move = self.copy(default=extra_move_vals)._action_confirm()
|
||||
extra_move = self.copy(default=extra_move_vals)
|
||||
if extra_move.picking_id:
|
||||
extra_move = extra_move._action_confirm(merge_into=self)
|
||||
else:
|
||||
extra_move = extra_move._action_confirm()
|
||||
|
||||
# link it to some move lines. We don't need to do it for move since they should be merged.
|
||||
if self.exists() and not self.picking_id:
|
||||
@@ -1022,7 +1033,9 @@ class StockMove(models.Model):
|
||||
for move in moves:
|
||||
if move.state == 'cancel' or move.quantity_done <= 0:
|
||||
continue
|
||||
moves_todo |= move
|
||||
# extra move will not be merged in mrp
|
||||
if not move.picking_id:
|
||||
moves_todo |= move
|
||||
moves_todo |= move._create_extra_move()
|
||||
|
||||
# Split moves where necessary and move quants
|
||||
@@ -1052,7 +1065,7 @@ class StockMove(models.Model):
|
||||
.filtered(lambda p: p.quant_ids and len(p.quant_ids) > 1):
|
||||
if len(result_package.quant_ids.mapped('location_id')) > 1:
|
||||
raise UserError(_('You should not put the contents of a package in different locations.'))
|
||||
picking = self and self[0].picking_id or False
|
||||
picking = moves_todo and moves_todo[0].picking_id or False
|
||||
moves_todo.write({'state': 'done', 'date': fields.Datetime.now()})
|
||||
moves_todo.mapped('move_dest_ids')._action_assign()
|
||||
|
||||
|
||||
@@ -963,6 +963,50 @@ class TestSinglePicking(TestStockCommon):
|
||||
self.assertEqual(sum(move1.move_line_ids.mapped('qty_done')), 2.0)
|
||||
self.assertEqual(move1.state, 'done')
|
||||
|
||||
def test_extra_move_4(self):
|
||||
""" Create a picking with similar moves (created after
|
||||
confirmation). Action done should propagate all the extra
|
||||
quantity and only merge extra moves in their original moves.
|
||||
"""
|
||||
delivery = self.env['stock.picking'].create({
|
||||
'location_id': self.stock_location,
|
||||
'location_dest_id': self.customer_location,
|
||||
'partner_id': self.partner_delta_id,
|
||||
'picking_type_id': self.picking_type_out,
|
||||
})
|
||||
self.MoveObj.create({
|
||||
'name': self.productA.name,
|
||||
'product_id': self.productA.id,
|
||||
'product_uom_qty': 5,
|
||||
'quantity_done': 10,
|
||||
'product_uom': self.productA.uom_id.id,
|
||||
'picking_id': delivery.id,
|
||||
'location_id': self.stock_location,
|
||||
'location_dest_id': self.customer_location,
|
||||
})
|
||||
stock_location = self.env['stock.location'].browse(self.stock_location)
|
||||
self.env['stock.quant']._update_available_quantity(self.productA, stock_location, 5)
|
||||
delivery.action_confirm()
|
||||
delivery.action_assign()
|
||||
|
||||
delivery.write({
|
||||
'move_lines': [(0, 0, {
|
||||
'name': self.productA.name,
|
||||
'product_id': self.productA.id,
|
||||
'product_uom_qty': 0,
|
||||
'quantity_done': 10,
|
||||
'state': 'assigned',
|
||||
'product_uom': self.productA.uom_id.id,
|
||||
'picking_id': delivery.id,
|
||||
'location_id': self.stock_location,
|
||||
'location_dest_id': self.customer_location,
|
||||
})]
|
||||
})
|
||||
delivery.action_done()
|
||||
self.assertEqual(len(delivery.move_lines), 2, 'Move should not be merged together')
|
||||
for move in delivery.move_lines:
|
||||
self.assertEqual(move.quantity_done, move.product_uom_qty, 'Initial demand should be equals to quantity done')
|
||||
|
||||
def test_recheck_availability_1(self):
|
||||
""" Check the good behavior of check availability. I create a DO for 2 unit with
|
||||
only one in stock. After the first check availability, I should have 1 reserved
|
||||
|
||||
Reference in New Issue
Block a user