[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:
Arnold Moyaux
2018-03-27 17:36:44 +02:00
parent f22c5794c4
commit b33d5af4a7
4 changed files with 69 additions and 12 deletions
+2 -2
View File
@@ -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 """
+1 -1
View File
@@ -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
+22 -9
View File
@@ -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()
+44
View File
@@ -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