From 271433118e174139fead18110756370cdc511999 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Cifuentes?= Date: Fri, 12 Apr 2024 14:35:24 +0200 Subject: [PATCH] [FIX] stock_dynamic_routing: Prevent several splits when first source is under stock --- stock_dynamic_routing/models/stock_move.py | 95 +++++++++++++++---- .../tests/test_routing_pull.py | 43 +++++++++ 2 files changed, 122 insertions(+), 16 deletions(-) diff --git a/stock_dynamic_routing/models/stock_move.py b/stock_dynamic_routing/models/stock_move.py index e6f91731c5c..bc44648d9f6 100644 --- a/stock_dynamic_routing/models/stock_move.py +++ b/stock_dynamic_routing/models/stock_move.py @@ -20,6 +20,31 @@ class StockMove(models.Model): "rule push_original_destination", ) + def _create_savepoint(self): + savepoint_name = uuid.uuid1().hex + self.env.flush_all() + # pylint: disable=sql-injection + self.env.cr.execute( + sql.SQL("SAVEPOINT {}").format(sql.Identifier(savepoint_name)) + ) + return savepoint_name + + def _release_savepoint(self, savepoint_name): + self.env.flush_all() + # pylint: disable=sql-injection + self.env.cr.execute( + sql.SQL("RELEASE SAVEPOINT {}").format(sql.Identifier(savepoint_name)) + ) + + def _rollback_to_savepoint(self, savepoint_name): + self.env.clear() + # pylint: disable=sql-injection + self.env.cr.execute( + sql.SQL("ROLLBACK TO SAVEPOINT {}").format(sql.Identifier(savepoint_name)) + ) + + + def _no_routing_details(self): return self.RoutingDetails( rule=self.env["stock.routing.rule"].browse(), @@ -112,12 +137,7 @@ def _prepare_routing_pull(self): if not self: return self - savepoint_name = uuid.uuid1().hex - self.env.flush_all() - # pylint: disable=sql-injection - self.env.cr.execute( - sql.SQL("SAVEPOINT {}").format(sql.Identifier(savepoint_name)) - ) + savepoint_name = self._create_savepoint() super()._action_assign() moves_routing = self._routing_compute_rules() @@ -126,19 +146,11 @@ def _prepare_routing_pull(self): ): # no routing to apply, so the reservations done by _action_assign # are valid and we can resolve to a normal flow - self.env.flush_all() - # pylint: disable=sql-injection - self.env.cr.execute( - sql.SQL("RELEASE SAVEPOINT {}").format(sql.Identifier(savepoint_name)) - ) + self._release_savepoint(savepoint_name) return {} # rollback _action_assign, it'll be called again after the routing - self.env.clear() - # pylint: disable=sql-injection - self.env.cr.execute( - sql.SQL("ROLLBACK TO SAVEPOINT {}").format(sql.Identifier(savepoint_name)) - ) + self._rollback_to_savepoint(savepoint_name) return moves_routing def _routing_compute_rules(self): @@ -194,6 +206,32 @@ def _routing_compute_rules(self): moves_routing[move][routing_details] += missing_reserved_quantity return moves_routing + def _get_excluded_locations(self, routing_quantities): + """Given a routing_quantities object, returns all the given locations and + their children in a recordset of the stock.location model. + """ + exclude_locations_ids = [ + routing_details.rule.location_src_id.id + for routing_details, qty in routing_quantities.items() + if routing_details.rule._name == "stock.routing.rule" + ] + locations = self.env["stock.location"].browse(exclude_locations_ids) + parent_locations = locations + child_locations = parent_locations.mapped("child_ids") + location_child = self.env["stock.location"] + while child_locations: + location_child |= child_locations + parent_locations = child_locations + child_locations = parent_locations.mapped("child_ids") + + exclude_locations_ids += location_child.ids + location_recs = self.env["stock.location"].search( + [ + ("id", "in", exclude_locations_ids), + ] + ) + return location_recs + def _routing_splits(self, moves_routing): """Split moves according to routing rules @@ -218,6 +256,31 @@ def _routing_splits(self, moves_routing): # these. new_move_vals = move._split(qty) if new_move_vals: + # In case the move is split, to ensure that the new move, + # which is going to search in the same location hierarchy, + # does not reserve stock stored in unused locations for + # dynamic routing and does not generate unnecessary splits, + # we try to reserve the existing move. + # We check if the reservation has been made in a dynamically + # routed location, and if it has, + # we rollback to undo any possible splits. + + savepoint_name = self._create_savepoint() + move.with_context( + exclude_apply_dynamic_routing=True, + )._action_assign() + locations_excluded = self._get_excluded_locations( + routing_quantities + ) + rollback = False + for move_line in move.move_line_ids: + if move_line.location_id in locations_excluded: + rollback = True + break + if rollback: + self._rollback_to_savepoint(savepoint_name) + else: + self._release_savepoint(savepoint_name) new_move = self.env["stock.move"].create(new_move_vals) new_move._action_confirm(merge=False) else: diff --git a/stock_dynamic_routing/tests/test_routing_pull.py b/stock_dynamic_routing/tests/test_routing_pull.py index fc76a6d8d7b..55ab1f23acb 100644 --- a/stock_dynamic_routing/tests/test_routing_pull.py +++ b/stock_dynamic_routing/tests/test_routing_pull.py @@ -639,6 +639,49 @@ def test_partial_qty(self): self.assert_src_highbay(routing_move) self.assert_dest_handover(routing_move) + def test_partial_availability_stock_and_higbay(self): + # In this case, we have stock in a non-routed stock location to partially + # fulfill the demand (10/30), and the rest in a routed location. + # The stock in the stock location is entered first, so it will be used first. + # The remaining demand will be fulfilled from the routed location, + # which has sufficient stock. + # We should only split the initial PICK picking, r + # esulting in the following after routing: + # +--------------------------------------------------------------------+ + # | HO/xxxx Assigned | + # | Stock → Stock/Handover | + # | 20x Product Highbay/Bay1/Bin1 → Stock/Handover (available) move_ho | + # +--------------------------------------------------------------------+ + # + # +-------------------------------------------------------------------+ + # | PICK/xxxx Waiting | + # | Stock → Output | + # | 20x Product Stock/Handover → Output (waiting) move_a2 (split) | + # | 10x Product Stock/Shelf1 → Output (available) move_a1 | + # +-------------------------------------------------------------------+ + # + # +-------------------------------------------------+ + # | OUT/xxxx Waiting | + # | Output → Customer | + # | 30x Product Output → Customer (waiting) move_b | + # +-------------------------------------------------+ + pick_picking, customer_picking = self._create_pick_ship( + self.wh, [(self.product1, 30)] + ) + move_a = pick_picking.move_ids + + self._update_product_qty_in_location( + self.location_shelf_1, move_a.product_id, 10 + ) + self._update_product_qty_in_location( + self.location_hb_1_2, move_a.product_id, 100 + ) + pick_picking.action_assign() + self.assertEqual(len(pick_picking.move_ids), 2) + routing_move = pick_picking.move_ids.filtered(lambda m: m.id != move_a.id) + self.assertEqual(move_a.product_uom_qty, 10) + self.assertEqual(routing_move.product_uom_qty, 20) + def test_change_dest_move_source(self): # Change the picking type destination so the move goes to a location # which is a parent destination of the routing destination (move will