From cb618e9a99ea3b86fdeed0295168a1e9fd7cddc3 Mon Sep 17 00:00:00 2001 From: Arnold Moyaux <arm@odoo.com> Date: Thu, 31 Jan 2019 14:49:54 +0000 Subject: [PATCH] [IMP] stock: _assign_picking in batch Only search once for batch of moves that should be assign to the same picking and assign them all at once. Since the moves are processed in batch for a picking commit 8fe8ca9 is not needed anymore. closes odoo/odoo#30699 --- addons/delivery/models/stock_move.py | 9 ++++ addons/procurement_jit/sale.py | 4 +- addons/sale_mrp/models/sale_mrp.py | 6 +-- addons/sale_stock/models/sale_order.py | 31 +++++------- addons/sale_stock/models/stock.py | 13 +++-- addons/sale_stock/tests/test_sale_stock.py | 12 ++++- addons/stock/models/stock_move.py | 57 ++++++++++++---------- addons/stock_dropshipping/models/sale.py | 5 +- 8 files changed, 78 insertions(+), 59 deletions(-) diff --git a/addons/delivery/models/stock_move.py b/addons/delivery/models/stock_move.py index 691c1c3dd42d..805aaf5bfa71 100644 --- a/addons/delivery/models/stock_move.py +++ b/addons/delivery/models/stock_move.py @@ -15,3 +15,12 @@ class StockMove(models.Model): def _cal_move_weight(self): for move in self.filtered(lambda moves: moves.product_id.weight > 0.00): move.weight = (move.product_qty * move.product_id.weight) + + def _get_new_picking_values(self): + vals = super(StockMove, self)._get_new_picking_values() + vals['carrier_id'] = self.mapped('sale_line_id.order_id.carrier_id').id + return vals + + def _key_assign_picking(self): + keys = super(StockMove, self)._key_assign_picking() + return keys + (self.sale_line_id.order_id.carrier_id,) diff --git a/addons/procurement_jit/sale.py b/addons/procurement_jit/sale.py index 9ad3cf50b1de..c6e638ee721b 100644 --- a/addons/procurement_jit/sale.py +++ b/addons/procurement_jit/sale.py @@ -8,8 +8,8 @@ class SaleOrderLine(models.Model): _inherit = "sale.order.line" @api.multi - def _action_launch_stock_rule(self): - res = super(SaleOrderLine, self)._action_launch_stock_rule() + def _action_launch_stock_rule(self, previous_product_uom_qty=False): + res = super(SaleOrderLine, self)._action_launch_stock_rule(previous_product_uom_qty=previous_product_uom_qty) orders = list(set(x.order_id for x in self)) for order in orders: reassign = order.picking_ids.filtered(lambda x: x.state=='confirmed' or (x.state in ['waiting', 'assigned'] and not x.printed)) diff --git a/addons/sale_mrp/models/sale_mrp.py b/addons/sale_mrp/models/sale_mrp.py index 144a1c93a86c..d80fdf068a77 100644 --- a/addons/sale_mrp/models/sale_mrp.py +++ b/addons/sale_mrp/models/sale_mrp.py @@ -91,7 +91,7 @@ class SaleOrderLine(models.Model): warning = {'warning': warning_mess} return warning - def _get_qty_procurement(self): + def _get_qty_procurement(self, previous_product_uom_qty=False): self.ensure_one() # Specific case when we change the qty on a SO for a kit product. # We don't try to be too smart and keep a simple approach: we compare the quantity before @@ -99,8 +99,8 @@ class SaleOrderLine(models.Model): # sent, or any other exceptional case. bom = self.env['mrp.bom']._bom_find(product=self.product_id, bom_type='phantom') if bom and 'previous_product_uom_qty' in self.env.context: - return self.env.context['previous_product_uom_qty'].get(self.id, 0.0) - return super(SaleOrderLine, self)._get_qty_procurement() + return previous_product_uom_qty and previous_product_uom_qty.get(self.id, 0.0) + return super(SaleOrderLine, self)._get_qty_procurement(previous_product_uom_qty=previous_product_uom_qty) class AccountInvoiceLine(models.Model): diff --git a/addons/sale_stock/models/sale_order.py b/addons/sale_stock/models/sale_order.py index 45974190d631..fc636b8177a5 100644 --- a/addons/sale_stock/models/sale_order.py +++ b/addons/sale_stock/models/sale_order.py @@ -60,17 +60,24 @@ class SaleOrder(models.Model): def write(self, values): if values.get('order_line') and self.state == 'sale': for order in self: - pre_order_line_qty = {order_line: order_line.product_uom_qty for order_line in order.mapped('order_line')} + pre_order_line_qty = {order_line: order_line.product_uom_qty for order_line in order.mapped('order_line') if not order_line.is_expense} res = super(SaleOrder, self).write(values) if values.get('order_line') and self.state == 'sale': for order in self: to_log = {} + order_lines_to_run = self.env['sale.order.line'] for order_line in order.order_line: - if pre_order_line_qty.get(order_line, False) and float_compare(order_line.product_uom_qty, pre_order_line_qty[order_line], order_line.product_uom.rounding) < 0: + if order_line not in pre_order_line_qty: + order_lines_to_run |= order_line + elif float_compare(order_line.product_uom_qty, pre_order_line_qty[order_line], order_line.product_uom.rounding) < 0: to_log[order_line] = (order_line.product_uom_qty, pre_order_line_qty[order_line]) + elif float_compare(order_line.product_uom_qty, pre_order_line_qty[order_line], order_line.product_uom.rounding) > 0: + order_lines_to_run |= order_line if to_log: documents = self.env['stock.picking']._log_activity_get_documents(to_log, 'move_ids', 'UP') order._log_decrease_ordered_quantity(documents) + if order_lines_to_run: + order_lines_to_run._action_launch_stock_rule(pre_order_line_qty) return res @api.multi @@ -155,7 +162,6 @@ class SaleOrder(models.Model): self.env['stock.picking']._log_activity(_render_note_exception_quantity_so, documents) - class SaleOrderLine(models.Model): _inherit = 'sale.order.line' @@ -199,19 +205,6 @@ class SaleOrderLine(models.Model): lines.filtered(lambda line: line.state == 'sale')._action_launch_stock_rule() return lines - @api.multi - def write(self, values): - lines = self.env['sale.order.line'] - if 'product_uom_qty' in values: - precision = self.env['decimal.precision'].precision_get('Product Unit of Measure') - lines = self.filtered( - lambda r: r.state == 'sale' and not r.is_expense and float_compare(r.product_uom_qty, values['product_uom_qty'], precision_digits=precision) == -1) - previous_product_uom_qty = {line.id: line.product_uom_qty for line in lines} - res = super(SaleOrderLine, self).write(values) - if lines: - lines.with_context(previous_product_uom_qty=previous_product_uom_qty)._action_launch_stock_rule() - return res - @api.depends('order_id.state') def _compute_invoice_status(self): super(SaleOrderLine, self)._compute_invoice_status() @@ -341,7 +334,7 @@ class SaleOrderLine(models.Model): }) return values - def _get_qty_procurement(self): + def _get_qty_procurement(self, previous_product_uom_qty=False): self.ensure_one() qty = 0.0 for move in self.move_ids.filtered(lambda r: r.state != 'cancel'): @@ -352,7 +345,7 @@ class SaleOrderLine(models.Model): return qty @api.multi - def _action_launch_stock_rule(self): + def _action_launch_stock_rule(self, previous_product_uom_qty=False): """ Launch procurement group run method with required/custom fields genrated by a sale order line. procurement group will launch '_run_pull', '_run_buy' or '_run_manufacture' @@ -363,7 +356,7 @@ class SaleOrderLine(models.Model): for line in self: if line.state != 'sale' or not line.product_id.type in ('consu','product'): continue - qty = line._get_qty_procurement() + qty = line._get_qty_procurement(previous_product_uom_qty) if float_compare(qty, line.product_uom_qty, precision_digits=precision) >= 0: continue diff --git a/addons/sale_stock/models/stock.py b/addons/sale_stock/models/stock.py index f622c61820e1..36c2795e61dd 100644 --- a/addons/sale_stock/models/stock.py +++ b/addons/sale_stock/models/stock.py @@ -38,11 +38,14 @@ class StockMove(models.Model): def _assign_picking_post_process(self, new=False): super(StockMove, self)._assign_picking_post_process(new=new) - if new and self.sale_line_id and self.sale_line_id.order_id: - self.picking_id.message_post_with_view( - 'mail.message_origin_link', - values={'self': self.picking_id, 'origin': self.sale_line_id.order_id}, - subtype_id=self.env.ref('mail.mt_note').id) + if new: + picking_id = self.mapped('picking_id') + sale_order_ids = self.mapped('sale_line_id.order_id') + for sale_order_id in sale_order_ids: + picking_id.message_post_with_view( + 'mail.message_origin_link', + values={'self': picking_id, 'origin': sale_order_id}, + subtype_id=self.env.ref('mail.mt_note').id) class ProcurementGroup(models.Model): diff --git a/addons/sale_stock/tests/test_sale_stock.py b/addons/sale_stock/tests/test_sale_stock.py index 0be82e262d0a..7782ecdc7f1c 100644 --- a/addons/sale_stock/tests/test_sale_stock.py +++ b/addons/sale_stock/tests/test_sale_stock.py @@ -396,14 +396,22 @@ class TestSaleStock(TestSale): self.assertEqual(move1.product_qty, 12) # edit the so line, sell 2 dozen, the move should now be 24 units - so1.order_line.product_uom_qty = 2 + so1.write({ + 'order_line': [ + (1, so1.order_line.id, {'product_uom_qty': 2}), + ] + }) self.assertEqual(move1.product_uom_qty, 24) self.assertEqual(move1.product_uom.id, uom_unit.id) self.assertEqual(move1.product_qty, 24) # force the propagation of the uom, sell 3 dozen self.env['ir.config_parameter'].sudo().set_param('stock.propagate_uom', '1') - so1.order_line.product_uom_qty = 3 + so1.write({ + 'order_line': [ + (1, so1.order_line.id, {'product_uom_qty': 3}), + ] + }) move2 = so1.picking_ids.move_lines.filtered(lambda m: m.product_uom.id == uom_dozen.id) self.assertEqual(move2.product_uom_qty, 1) self.assertEqual(move2.product_uom.id, uom_dozen.id) diff --git a/addons/stock/models/stock_move.py b/addons/stock/models/stock_move.py index d1104bdb0848..1a6eff0447b4 100644 --- a/addons/stock/models/stock_move.py +++ b/addons/stock/models/stock_move.py @@ -675,6 +675,10 @@ class StockMove(models.Model): } } + def _key_assign_picking(self): + self.ensure_one() + return self.group_id, self.location_id, self.location_dest_id, self.picking_type_id + def _search_picking_for_assignation(self): self.ensure_one() picking = self.env['stock.picking'].search([ @@ -692,11 +696,16 @@ class StockMove(models.Model): type (moves should already have them identical). Otherwise, create a new picking to assign them to. """ Picking = self.env['stock.picking'] - for move in self: - recompute = False - picking = move._search_picking_for_assignation() + grouped_moves = groupby(sorted(self, key=lambda m: [f.id for f in m._key_assign_picking()]), key=lambda m: [m._key_assign_picking()]) + for group, moves in grouped_moves: + moves = self.env['stock.move'].concat(*list(moves)) + new_picking = False + # Could pass the arguments contained in group but they are the same + # for each move that why moves[0] is acceptable + picking = moves[0]._search_picking_for_assignation() if picking: - if picking.partner_id.id != move.partner_id.id or picking.origin != move.origin: + if any(picking.partner_id.id != m.partner_id.id or + picking.origin != m.origin for m in moves): # If a picking is found, we'll append `move` to its move list and thus its # `partner_id` and `ref` field will refer to multiple records. In this # case, we chose to wipe them. @@ -705,34 +714,32 @@ class StockMove(models.Model): 'origin': False, }) else: - recompute = True - picking = Picking.create(move._get_new_picking_values()) - - move.write({'picking_id': picking.id}) - move._assign_picking_post_process(new=recompute) - # If this method is called in batch by a write on a one2many and - # at some point had to create a picking, some next iterations could - # try to find back the created picking. As we look for it by searching - # on some computed fields, we have to force a recompute, else the - # record won't be found. - if recompute: - move.recompute() + new_picking = True + picking = Picking.create(moves._get_new_picking_values()) + + moves.write({'picking_id': picking.id}) + moves._assign_picking_post_process(new=new_picking) return True def _assign_picking_post_process(self, new=False): pass def _get_new_picking_values(self): - """ Prepares a new picking for this move as it could not be assigned to - another picking. This method is designed to be inherited. """ + """ return create values for new picking that will be linked with group + of moves in self. + """ + origins = self.filtered(lambda m: m.origin).mapped('origin') + origin = len(origins) == 1 and origins[0] or False + partners = self.mapped('partner_id') + partner = len(partners) == 1 and partners.id or False return { - 'origin': self.origin, - 'company_id': self.company_id.id, - 'move_type': self.group_id and self.group_id.move_type or 'direct', - 'partner_id': self.partner_id.id, - 'picking_type_id': self.picking_type_id.id, - 'location_id': self.location_id.id, - 'location_dest_id': self.location_dest_id.id, + 'origin': origin, + 'company_id': self.mapped('company_id').id, + 'move_type': self.mapped('group_id').move_type or 'direct', + 'partner_id': partner, + 'picking_type_id': self.mapped('picking_type_id').id, + 'location_id': self.mapped('location_id').id, + 'location_dest_id': self.mapped('location_dest_id').id, } def _should_be_assigned(self): diff --git a/addons/stock_dropshipping/models/sale.py b/addons/stock_dropshipping/models/sale.py index b527701a0498..5e23612a0981 100644 --- a/addons/stock_dropshipping/models/sale.py +++ b/addons/stock_dropshipping/models/sale.py @@ -10,7 +10,7 @@ class SaleOrderLine(models.Model): purchase_line_ids = fields.One2many('purchase.order.line', 'sale_line_id') @api.multi - def _get_qty_procurement(self): + def _get_qty_procurement(self, previous_product_uom_qty): # People without purchase rights should be able to do this operation purchase_lines_sudo = self.sudo().purchase_line_ids if not self.move_ids.filtered(lambda r: r.state != 'cancel') and purchase_lines_sudo.filtered(lambda r: r.state != 'cancel'): @@ -19,5 +19,4 @@ class SaleOrderLine(models.Model): qty += po_line.product_uom._compute_quantity(po_line.product_qty, self.product_uom, rounding_method='HALF-UP') return qty else: - return super(SaleOrderLine, self)._get_qty_procurement() - + return super(SaleOrderLine, self)._get_qty_procurement(previous_product_uom_qty=previous_product_uom_qty) -- GitLab