[IMP] stock: fix and batch push_apply
- Fix the issue of the added test. - Also batch the `push_apply` + `_action_confirm` by doing confirm in BFS instead of DFS. Improve a lot the performance when the _action_confirm is on batch. closes odoo/odoo#61567 Signed-off-by: Arnold Moyaux <amoyaux@users.noreply.github.com>
This commit is contained in:
committed by
Arnold Moyaux
parent
a313db6fab
commit
9ab123034a
@@ -14,6 +14,7 @@ from dateutil import relativedelta
|
||||
from odoo import SUPERUSER_ID, _, api, fields, models
|
||||
from odoo.exceptions import UserError
|
||||
from odoo.osv import expression
|
||||
from odoo.tools import OrderedSet
|
||||
from odoo.tools.float_utils import float_compare, float_is_zero, float_repr, float_round
|
||||
from odoo.tools.misc import format_date
|
||||
|
||||
@@ -719,6 +720,7 @@ class StockMove(models.Model):
|
||||
return True
|
||||
|
||||
def _push_apply(self):
|
||||
new_moves = []
|
||||
for move in self:
|
||||
# if the move is already chained, there is no need to check push rules
|
||||
if move.move_dest_ids:
|
||||
@@ -729,12 +731,15 @@ class StockMove(models.Model):
|
||||
# first priority goes to the preferred routes defined on the move itself (e.g. coming from a SO line)
|
||||
warehouse_id = move.warehouse_id or move.picking_id.picking_type_id.warehouse_id
|
||||
if move.location_dest_id.company_id == self.env.company:
|
||||
rules = self.env['procurement.group']._search_rule(move.route_ids, move.product_id, warehouse_id, domain)
|
||||
rule = self.env['procurement.group']._search_rule(move.route_ids, move.product_id, warehouse_id, domain)
|
||||
else:
|
||||
rules = self.sudo().env['procurement.group']._search_rule(move.route_ids, move.product_id, warehouse_id, domain)
|
||||
rule = self.sudo().env['procurement.group']._search_rule(move.route_ids, move.product_id, warehouse_id, domain)
|
||||
# Make sure it is not returning the return
|
||||
if rules and (not move.origin_returned_move_id or move.origin_returned_move_id.location_dest_id.id != rules.location_id.id):
|
||||
rules._run_push(move)
|
||||
if rule and (not move.origin_returned_move_id or move.origin_returned_move_id.location_dest_id.id != rule.location_id.id):
|
||||
new_move = rule._run_push(move)
|
||||
if new_move:
|
||||
new_moves.append(new_move)
|
||||
return self.env['stock.move'].concat(*new_moves)
|
||||
|
||||
def _merge_moves_fields(self):
|
||||
""" This method will return a dict of stock move’s values that represent the values of all moves in `self` merged. """
|
||||
@@ -763,7 +768,7 @@ class StockMove(models.Model):
|
||||
return [
|
||||
move.product_id.id, move.price_unit, move.procure_method, move.location_id, move.location_dest_id,
|
||||
move.product_uom.id, move.restrict_partner_id.id, move.scrapped, move.origin_returned_move_id.id,
|
||||
move.package_level_id.id, move.propagate_cancel, move.description_picking
|
||||
move.package_level_id.id, move.propagate_cancel, move.description_picking or ""
|
||||
]
|
||||
|
||||
def _clean_merged(self):
|
||||
@@ -1070,27 +1075,25 @@ class StockMove(models.Model):
|
||||
:param: merge: According to this boolean, a newly confirmed move will be merged
|
||||
in another move of the same picking sharing its characteristics.
|
||||
"""
|
||||
move_create_proc = self.env['stock.move']
|
||||
move_to_confirm = self.env['stock.move']
|
||||
move_waiting = self.env['stock.move']
|
||||
|
||||
to_assign = {}
|
||||
# Use OrderedSet of id (instead of recordset + |= ) for performance
|
||||
move_create_proc, move_to_confirm, move_waiting = OrderedSet(), OrderedSet(), OrderedSet()
|
||||
to_assign = defaultdict(OrderedSet)
|
||||
for move in self:
|
||||
if move.state != 'draft':
|
||||
continue
|
||||
# if the move is preceeded, then it's waiting (if preceeding move is done, then action_assign has been called already and its state is already available)
|
||||
# if the move is preceded, then it's waiting (if preceding move is done, then action_assign has been called already and its state is already available)
|
||||
if move.move_orig_ids:
|
||||
move_waiting |= move
|
||||
move_waiting.add(move.id)
|
||||
else:
|
||||
if move.procure_method == 'make_to_order':
|
||||
move_create_proc |= move
|
||||
move_create_proc.add(move.id)
|
||||
else:
|
||||
move_to_confirm |= move
|
||||
move_to_confirm.add(move.id)
|
||||
if move._should_be_assigned():
|
||||
key = (move.group_id.id, move.location_id.id, move.location_dest_id.id)
|
||||
if key not in to_assign:
|
||||
to_assign[key] = self.env['stock.move']
|
||||
to_assign[key] |= move
|
||||
to_assign[key].add(move.id)
|
||||
|
||||
move_create_proc, move_to_confirm, move_waiting = self.browse(move_create_proc), self.browse(move_to_confirm), self.browse(move_waiting)
|
||||
|
||||
# create procurements for make to order moves
|
||||
procurement_requests = []
|
||||
@@ -1107,15 +1110,20 @@ class StockMove(models.Model):
|
||||
(move_waiting | move_create_proc).write({'state': 'waiting'})
|
||||
|
||||
# assign picking in batch for all confirmed move that share the same details
|
||||
for moves in to_assign.values():
|
||||
moves._assign_picking()
|
||||
self._push_apply()
|
||||
for moves_ids in to_assign.values():
|
||||
self.browse(moves_ids)._assign_picking()
|
||||
new_push_moves = self._push_apply()
|
||||
self._check_company()
|
||||
moves = self
|
||||
if merge:
|
||||
moves = self._merge_moves(merge_into=merge_into)
|
||||
|
||||
# call `_action_assign` on every confirmed move which location_id bypasses the reservation
|
||||
moves.filtered(lambda move: not move.picking_id.immediate_transfer and move._should_bypass_reservation() and move.state == 'confirmed')._action_assign()
|
||||
|
||||
if new_push_moves:
|
||||
new_push_moves._action_confirm()
|
||||
|
||||
return moves
|
||||
|
||||
def _prepare_procurement_values(self):
|
||||
|
||||
@@ -164,6 +164,7 @@ class StockRule(models.Model):
|
||||
Care this function is not call by method run. It is called explicitely
|
||||
in stock_move.py inside the method _push_apply
|
||||
"""
|
||||
self.ensure_one()
|
||||
new_date = fields.Datetime.to_string(move.date + relativedelta(days=self.delay))
|
||||
if self.auto == 'transparent':
|
||||
old_dest_location = move.location_dest_id
|
||||
@@ -171,7 +172,7 @@ class StockRule(models.Model):
|
||||
# avoid looping if a push rule is not well configured; otherwise call again push_apply to see if a next step is defined
|
||||
if self.location_id != old_dest_location:
|
||||
# TDE FIXME: should probably be done in the move model IMO
|
||||
move._push_apply()
|
||||
return move._push_apply()[:1]
|
||||
else:
|
||||
new_move_vals = self._push_prepare_move_copy_values(move, new_date)
|
||||
new_move = move.sudo().copy(new_move_vals)
|
||||
@@ -179,7 +180,7 @@ class StockRule(models.Model):
|
||||
new_move.write({'procure_method': 'make_to_stock'})
|
||||
if not new_move.location_id.should_bypass_reservation():
|
||||
move.write({'move_dest_ids': [(4, new_move.id)]})
|
||||
new_move._action_confirm()
|
||||
return new_move
|
||||
|
||||
def _push_prepare_move_copy_values(self, move_to_copy, new_date):
|
||||
company_id = self.company_id.id
|
||||
|
||||
@@ -1703,6 +1703,70 @@ class TestSinglePicking(TestStockCommon):
|
||||
self.assertEqual(len(qc_move), 1)
|
||||
self.assertTrue(qc_move.move_orig_ids == input_move, 'Move between QC and stock should only have the input move as origin')
|
||||
|
||||
def test_merge_chained_moves_multi_confirm(self):
|
||||
""" Imagine multiple step delivery. A receipt picking for the same product should by add to
|
||||
a existing picking from input to QC and another from QC to stock.
|
||||
This existing picking is confirm in the same time (not possible in stock, but can be with batch picking)
|
||||
and have some move to merge.
|
||||
"""
|
||||
warehouse = self.env['stock.warehouse'].create({
|
||||
'name': 'TEST WAREHOUSE',
|
||||
'code': 'TEST1',
|
||||
'reception_steps': 'three_steps',
|
||||
})
|
||||
receipt1 = self.env['stock.picking'].create({
|
||||
'location_id': self.supplier_location,
|
||||
'location_dest_id': warehouse.wh_input_stock_loc_id.id,
|
||||
'picking_type_id': warehouse.in_type_id.id,
|
||||
})
|
||||
move_receipt_1 = self.MoveObj.create({
|
||||
'name': self.productA.name,
|
||||
'product_id': self.productA.id,
|
||||
'product_uom_qty': 5,
|
||||
'product_uom': self.productA.uom_id.id,
|
||||
'picking_id': receipt1.id,
|
||||
'location_id': self.supplier_location,
|
||||
'location_dest_id': warehouse.wh_input_stock_loc_id.id,
|
||||
})
|
||||
receipt2 = self.env['stock.picking'].create({
|
||||
'location_id': warehouse.wh_input_stock_loc_id.id,
|
||||
'location_dest_id': warehouse.wh_qc_stock_loc_id.id,
|
||||
'picking_type_id': warehouse.int_type_id.id,
|
||||
})
|
||||
move1_receipt_2 = self.MoveObj.create({
|
||||
'name': self.productB.name,
|
||||
'product_id': self.productB.id,
|
||||
'product_uom_qty': 1,
|
||||
'product_uom': self.productB.uom_id.id,
|
||||
'picking_id': receipt2.id,
|
||||
'location_id': warehouse.wh_input_stock_loc_id.id,
|
||||
'location_dest_id': warehouse.wh_qc_stock_loc_id.id,
|
||||
})
|
||||
move2_receipt_2 = self.MoveObj.create({
|
||||
'name': self.productB.name,
|
||||
'product_id': self.productB.id,
|
||||
'product_uom_qty': 2,
|
||||
'product_uom': self.productB.uom_id.id,
|
||||
'picking_id': receipt2.id,
|
||||
'location_id': warehouse.wh_input_stock_loc_id.id,
|
||||
'location_dest_id': warehouse.wh_qc_stock_loc_id.id,
|
||||
})
|
||||
(receipt1 | receipt2).action_confirm()
|
||||
|
||||
# Check following move has been created
|
||||
self.assertTrue(move_receipt_1.move_dest_ids, 'No move created from push rules')
|
||||
self.assertTrue((move1_receipt_2 | move2_receipt_2).exists().move_dest_ids, 'No move created from push rules')
|
||||
self.assertEqual(len((move1_receipt_2 | move2_receipt_2).exists()), 1, 'Move has been merged with the other one')
|
||||
self.assertEqual(move_receipt_1.move_dest_ids.picking_id, receipt2, 'Dest Move of receipt1 should be in the receipt2')
|
||||
|
||||
# Check no move is still in draft
|
||||
self.assertTrue("draft" not in (receipt1 | receipt2).move_lines.mapped("state"))
|
||||
|
||||
# Check the content of the pickings
|
||||
self.assertEqual(receipt1.move_lines.mapped("product_uom_qty"), [5])
|
||||
self.assertEqual(receipt2.move_lines.filtered(lambda m: m.product_id == self.productB).mapped("product_uom_qty"), [3])
|
||||
self.assertEqual(receipt2.move_lines.filtered(lambda m: m.product_id == self.productA).mapped("product_uom_qty"), [5])
|
||||
|
||||
def test_empty_moves_validation_1(self):
|
||||
""" Use button validate on a picking that contains only moves
|
||||
without initial demand and without quantity done should be
|
||||
|
||||
Reference in New Issue
Block a user