Skip to content

[14.0][PERF] dynamic routing #722

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
OCA-git-bot merged 2 commits into from
Aug 22, 2023
Merged

[14.0][PERF] dynamic routing #722

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
dad83cf
[PERF] dynamic routing
jbaudoux Aug 12, 2023
1ea674e
stock_dynamic_routing: add test
jbaudoux Aug 17, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion stock_dynamic_routing/__manifest__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@
{
"name": "Stock Dynamic Routing",
"summary": "Dynamic routing of stock moves",
"author": "Camptocamp, Odoo Community Association (OCA)",
"author": "Camptocamp, BCIM, Odoo Community Association (OCA)",
"maintainers": ["jbaudoux"],
"website": "https://github.com/OCA/wms",
"category": "Warehouse Management",
"version": "14.0.1.0.2",
Expand Down
19 changes: 17 additions & 2 deletions stock_dynamic_routing/models/stock_move.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,16 @@
# Copyright 2019-2020 Camptocamp (https://www.camptocamp.com)
# Copyright 2021 Jacques-Etienne Baudoux (BCIM) <je@bcim.be>
# License AGPL-3.0 or later (https://www.gnu.org/licenses/agpl)
import logging
import uuid
from collections import OrderedDict, defaultdict, namedtuple

from psycopg2 import sql

from odoo import models

_logger = logging.getLogger(__name__)


class StockMove(models.Model):
_inherit = "stock.move"
Expand Down Expand Up @@ -118,7 +121,8 @@ def _prepare_routing_pull(self):
self.env.cr.execute(
sql.SQL("SAVEPOINT {}").format(sql.Identifier(savepoint_name))
)
super()._action_assign()
_logger.debug("Prepare pull re-routing")
super(StockMove, self.with_context(bypass_entire_pack=True))._action_assign()

moves_routing = self._routing_compute_rules()
if not any(
Expand All @@ -131,8 +135,10 @@ def _prepare_routing_pull(self):
self.env.cr.execute(
sql.SQL("RELEASE SAVEPOINT {}").format(sql.Identifier(savepoint_name))
)
self.mapped("picking_id")._check_entire_pack()
return {}

_logger.debug("Rollback computation for applying pull re-routing")
# rollback _action_assign, it'll be called again after the routing
self.env.clear()
# pylint: disable=sql-injection
Expand Down Expand Up @@ -380,6 +386,7 @@ def _routing_pull_switch_source(self):
new move but switch the source location of the current move. This
might trigger a new routing on the destination move.
"""
next_move_in_chain_ids = []
for move in self:
origmoves_by_location = OrderedDict()
for orig_move in move.move_orig_ids:
Expand All @@ -400,7 +407,15 @@ def _routing_pull_switch_source(self):
# we have a different routing
move.move_orig_ids -= orig_moves
split_move.move_orig_ids = orig_moves
split_move.location_id = location_id
if split_move.location_id != location_id:
split_move.with_context(
__applying_routing_rule=True
).location_id = location_id
next_move_in_chain_ids.append(split_move.id)
# Apply dynamic routing on next waiting moves in the chain
self.browse(next_move_in_chain_ids).filtered(
lambda r: r.state == "waiting"
)._chain_apply_routing()

def _apply_routing_rule_push(self, routing_details):
"""Apply push dynamic routing
Expand Down
6 changes: 6 additions & 0 deletions stock_dynamic_routing/models/stock_picking.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,12 @@ class StockPicking(models.Model):
" canceled because it was left empty after a dynamic routing.",
)

def _check_entire_pack(self):
# This can be dropped in v16 as part of odoo standard
if self.env.context.get("bypass_entire_pack"):
return
return super()._check_entire_pack()

@api.depends("canceled_by_routing")
def _compute_state(self):
super()._compute_state()
Expand Down
51 changes: 51 additions & 0 deletions stock_dynamic_routing/tests/test_routing_pull.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1057,3 +1057,54 @@ def test_mix_routing_reservation_same_location(self):
{"location_id": self.location_hb_1_2.id, "product_uom_qty": 7},
],
)

def test_route_waiting_moves(self):
"""Routing of waiting moves.

When the initial move is rerouted, the waiting moves in the chain
are also rerouted in cascade even if the destination location of the
initial move is not changed.
"""
# make a routing that does not change locations
self.pick_type_routing_op.write(
{
"default_location_src_id": self.wh.pick_type_id.default_location_src_id,
"default_location_dest_id": self.wh.pick_type_id.default_location_dest_id,
}
)
self.routing.location_id = (
self.pick_type_routing_op.default_location_src_id.id,
)
out_type_routing = self.wh.out_type_id.copy(
{"name": "OUTP Routing", "sequence_code": "WH/OUTP"}
)
self.env["stock.routing"].create(
{
"location_id": out_type_routing.default_location_src_id.id,
"picking_type_id": self.wh.out_type_id.id,
"rule_ids": [
(0, 0, {"method": "pull", "picking_type_id": out_type_routing.id})
],
}
)
pick_picking, customer_picking = self._create_pick_ship(
self.wh, [(self.product1, 10)]
)
self._update_product_qty_in_location(self.location_shelf_1, self.product1, 20.0)
pick_picking.action_assign()
new_cust_picking = self.env["stock.picking"].search(
[("picking_type_id", "=", out_type_routing.id)]
)

self.assertEqual(len(new_cust_picking), 1)
self.assertRecordValues(
new_cust_picking,
[
{
"state": "waiting",
"location_id": self.wh.wh_output_stock_loc_id.id,
"location_dest_id": self.customer_loc.id,
"picking_type_id": out_type_routing.id,
}
],
)
5 changes: 3 additions & 2 deletions stock_move_source_relocate/__manifest__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,11 @@
"name": "Stock Move Source Relocation",
"summary": "Change source location of unavailable moves",
"version": "14.0.1.1.0",
"development_status": "Alpha",
"development_status": "Beta",
"category": "Warehouse Management",
"website": "https://github.com/OCA/wms",
"author": "Camptocamp, Odoo Community Association (OCA)",
"author": "Camptocamp, BCIM, Odoo Community Association (OCA)",
"maintainers": ["jbaudoux"],
"license": "AGPL-3",
"application": False,
"installable": True,
Expand Down
23 changes: 21 additions & 2 deletions stock_move_source_relocate/models/stock_move.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,14 @@
# Copyright 2020 Camptocamp SA
# Copyright 2023 Michael Tietz (MT Software) <mtietz@mt-software.de>
# Copyright 2023 Jacques-Etienne Baudoux (BCIM) <je@bcim.be>
# License AGPL-3.0 or later (https://www.gnu.org/licenses/agpl)
import logging

from odoo import models
from odoo.tools.float_utils import float_compare

_logger = logging.getLogger(__name__)


class StockMove(models.Model):
_inherit = "stock.move"
Expand All @@ -18,22 +22,33 @@ def _action_assign(self):
unconfirmed_moves = unconfirmed_moves.filtered(
lambda m: m.state in ["confirmed", "partially_available"]
)
unconfirmed_moves._apply_source_relocate()
if unconfirmed_moves:
unconfirmed_moves._apply_source_relocate()
Comment on lines +25 to +26

@sebalix sebalix Aug 22, 2023

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does it change something to check the recordset here?

Should we instead do this check in the _apply_source_relocate method, if self is empty return.

@jbaudoux jbaudoux Aug 22, 2023

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No the method should not be called if not necessary.
Otherwise, all methods in odoo would start with if not self: return which is ugly

@sebalix sebalix Aug 22, 2023

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, here except the debug log the method will do nothing. And as the Odoo API tends to be "functional-programming" in some way it's OK to call methods on an object without thinking if it's empty or not.


def _apply_source_relocate(self):
# Read the `reserved_availability` field of the moves out of the loop
# to prevent unwanted cache invalidation when actually reserving.
reserved_availability = {move: move.reserved_availability for move in self}
roundings = {move: move.product_id.uom_id.rounding for move in self}
relocated_ids = []
_logger.debug(
"Try to relocate moves of operation type (%s)"
% ", ".join(self.picking_type_id.mapped("name"))
)
for move in self:
# We don't need to ignore moves with "_should_bypass_reservation()
# is True" because they are reserved at this point.
relocation = self.env["stock.source.relocate"]._rule_for_move(move)
if not relocation or relocation.relocate_location_id == move.location_id:
continue
move._apply_source_relocate_rule(
relocated = move._apply_source_relocate_rule(
relocation, reserved_availability, roundings
)
if relocated:
relocated_ids.append(relocated.id)
if relocated_ids:
_logger.debug("Relocated moves %s" % relocated_ids)
self.browse(relocated_ids)._after_apply_source_relocate_rule()

def _apply_source_relocate_rule(self, relocation, reserved_availability, roundings):
self.ensure_one()
Expand Down Expand Up @@ -63,3 +78,7 @@ def _apply_source_relocate_rule(self, relocation, reserved_availability, roundin
new_move.location_id = relocation.relocate_location_id
self._action_assign()
return new_move

def _after_apply_source_relocate_rule(self):
# Hook for stock_dynamic_routing

@sebalix sebalix Aug 22, 2023

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure we need to mention this module in stock_move_source_relocate? It's the job of the glue module (or anything else) to use this hook.

return
1 change: 1 addition & 0 deletions stock_move_source_relocate/readme/CONTRIBUTORS.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
* Guewen Baconnier <guewen.baconnier@camptocamp.com>
* Jacques-Etienne Baudoux (BCIM) <je@bcim.be>
* `Trobz <https://trobz.com>`_:
* Dung Tran <dungtd@trobz.com>
5 changes: 3 additions & 2 deletions stock_move_source_relocate_dynamic_routing/__manifest__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@
{
"name": "Stock Source Relocate - Dynamic Routing",
"summary": "Glue module",
"author": "Camptocamp, Odoo Community Association (OCA)",
"author": "Camptocamp, BCIM, Odoo Community Association (OCA)",
"maintainers": ["jbaudoux"],
"website": "https://github.com/OCA/wms",
"category": "Warehouse Management",
"version": "14.0.1.0.1",
Expand All @@ -12,5 +13,5 @@
"data": ["views/stock_routing_views.xml", "views/stock_source_relocate_views.xml"],
"auto_install": True,
"installable": True,
"development_status": "Alpha",
"development_status": "Beta",
}
12 changes: 11 additions & 1 deletion stock_move_source_relocate_dynamic_routing/models/stock_move.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,12 @@
# Copyright 2020 Camptocamp SA
# Copyright 2023 Jacques-Etienne Baudoux (BCIM) <je@bcim.be>
# License AGPL-3.0 or later (https://www.gnu.org/licenses/agpl)
import logging

from odoo import models

_logger = logging.getLogger(__name__)


class StockMove(models.Model):
_inherit = "stock.move"
Expand All @@ -17,5 +21,11 @@ def _apply_source_relocate_rule(self, relocation, reserved_availability, roundin
)._apply_source_relocate_rule(relocation, reserved_availability, roundings)
# restore the previous context without "__applying_routing_rule", otherwise
# it wouldn't properly apply the routing in chain in the further moves
relocated.with_context(self.env.context)._chain_apply_routing()
relocated = relocated.with_context(self.env.context)
return relocated

def _after_apply_source_relocate_rule(self):
super()._after_apply_source_relocate_rule()
result = self._chain_apply_routing()
_logger.debug("Dynamic routing applied on relocated moves %s" % self.ids)
return result
1 change: 1 addition & 0 deletions stock_move_source_relocate_dynamic_routing/readme/CONTRIBUTORS.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
* Guewen Baconnier <guewen.baconnier@camptocamp.com>
* Jacques-Etienne Baudoux (BCIM) <je@bcim.be>