From e3c0779df80d909318d911e7aa279edf19e653a1 Mon Sep 17 00:00:00 2001 From: Thierry Ducrest Date: Tue, 17 Oct 2023 11:29:38 +0200 Subject: [PATCH 1/5] sf_base: add decorator to gracefully handle exceptions --- shopfloor_base/utils.py | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/shopfloor_base/utils.py b/shopfloor_base/utils.py index b58bf2ef76..befac317e9 100644 --- a/shopfloor_base/utils.py +++ b/shopfloor_base/utils.py @@ -5,6 +5,7 @@ import os from functools import wraps +from odoo.exceptions import ValidationError from odoo.modules.module import load_information_from_description_file from odoo.tools.config import config as odoo_config @@ -33,6 +34,31 @@ def wrapped(*args, **kwargs): return _ensure_model +def catch_errors(response_error): + """Decorator for service endpoints to gracefully handle some exceptions. + + The decorator is passed a function with the same arguments than the function + handlling the endpoint, plus a keyword argument called `message`. + """ + + def _catch_errors(fn): + @wraps(fn) + def wrapper(*args, **kwargs): + try: + res = fn(*args, **kwargs) + except ValidationError as ex: + message = { + "message_type": "error", + "body": ex.name, + } + return response_error(*args, **kwargs, message=message) + return res + + return wrapper + + return _catch_errors + + APP_VERSIONS = {} From 6ac2d67049431586df6b41c50554adcd58277839 Mon Sep 17 00:00:00 2001 From: Thierry Ducrest Date: Tue, 17 Oct 2023 12:07:00 +0200 Subject: [PATCH 2/5] shpfloor: use catch_error decorator in zp, spt --- shopfloor/services/single_pack_transfer.py | 11 +++++++- shopfloor/services/zone_picking.py | 25 +++++++++++++++---- shopfloor/tests/test_single_pack_transfer.py | 18 ------------- .../tests/test_single_pack_transfer_base.py | 18 +++++++++++++ 4 files changed, 48 insertions(+), 24 deletions(-) diff --git a/shopfloor/services/single_pack_transfer.py b/shopfloor/services/single_pack_transfer.py index 64e2bfd3ec..39eaa5bc02 100644 --- a/shopfloor/services/single_pack_transfer.py +++ b/shopfloor/services/single_pack_transfer.py @@ -6,6 +6,7 @@ from odoo.addons.base_rest.components.service import to_int from odoo.addons.component.core import Component +from odoo.addons.shopfloor_base.utils import catch_errors class SinglePackTransfer(Component): @@ -205,10 +206,18 @@ def _create_package_level(self, package): def _is_move_state_valid(self, moves): return all(move.state != "cancel" for move in moves) + def _validate_catch_error( + self, package_level_id, location_barcode, confirmation=False, message=None + ): + package_level = self.env["stock.package_level"].browse(package_level_id) + return self._response_for_scan_location( + package_level, message=message, confirmation_required=confirmation + ) + + @catch_errors(_validate_catch_error) def validate(self, package_level_id, location_barcode, confirmation=False): """Validate the transfer""" search = self._actions_for("search") - package_level = self.env["stock.package_level"].browse(package_level_id) if not package_level.exists(): return self._response_for_start( diff --git a/shopfloor/services/zone_picking.py b/shopfloor/services/zone_picking.py index d73283f13b..4c55e94fc2 100644 --- a/shopfloor/services/zone_picking.py +++ b/shopfloor/services/zone_picking.py @@ -10,6 +10,7 @@ from odoo.addons.base_rest.components.service import to_bool, to_int from odoo.addons.component.core import Component +from odoo.addons.shopfloor_base.utils import catch_errors from ..exceptions import ConcurentWorkOnTransfer from ..utils import to_float @@ -1075,6 +1076,21 @@ def _set_destination_update_quantity(self, move_line, quantity, barcode): return response return response + def _set_destination_catch_error( + self, + move_line_id, + barcode, + quantity, + confirmation=False, + handle_complete_mix_pack=False, + message=None, + ): + move_line = self.env["stock.move.line"].browse(move_line_id) + return self._response_for_set_line_destination( + move_line, message=message, confirmation_required=confirmation + ) + + @catch_errors(_set_destination_catch_error) # flake8: noqa: C901 def set_destination( self, @@ -1535,17 +1551,16 @@ def set_destination_all(self, barcode, confirmation=False): message = None buffer_lines = self._find_buffer_move_lines() if location: - error = None location_dest = buffer_lines.mapped("location_dest_id") # check if move lines share the same destination if len(location_dest) != 1: - error = self.msg_store.lines_different_dest_location() + message = self.msg_store.lines_different_dest_location() # check if the scanned location is allowed moves = buffer_lines.mapped("move_id") if not self.is_dest_location_valid(moves, location): - error = self.msg_store.location_not_allowed() - if error: - return self._set_destination_all_response(buffer_lines, message=error) + message = self.msg_store.location_not_allowed() + if message: + return self._set_destination_all_response(buffer_lines, message=message) # check if the destination location is not the expected one # - OK if the scanned destination is a child of the current # destination set on buffer lines diff --git a/shopfloor/tests/test_single_pack_transfer.py b/shopfloor/tests/test_single_pack_transfer.py index 08bb5bca41..7ab3c85156 100644 --- a/shopfloor/tests/test_single_pack_transfer.py +++ b/shopfloor/tests/test_single_pack_transfer.py @@ -2,8 +2,6 @@ # Copyright 2020 Akretion (http://www.akretion.com) # License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl.html). -from odoo.tests.common import Form - from .test_single_pack_transfer_base import SinglePackTransferCommonBase @@ -46,22 +44,6 @@ def setUpClassBaseData(cls, *args, **kwargs): lines=[(cls.product_a, 1), (cls.product_b, 1)] ) - @classmethod - def _create_initial_move(cls, lines): - """Create the move to satisfy the pre-condition before /start""" - picking_form = Form(cls.env["stock.picking"]) - picking_form.picking_type_id = cls.picking_type - picking_form.location_id = cls.stock_location - picking_form.location_dest_id = cls.shelf2 - for line in lines: - with picking_form.move_ids_without_package.new() as move: - move.product_id = line[0] - move.product_uom_qty = line[1] - picking = picking_form.save() - picking.action_confirm() - picking.action_assign() - return picking - def _simulate_started(self, package): """Replicate what the /start endpoint would do on the given package. diff --git a/shopfloor/tests/test_single_pack_transfer_base.py b/shopfloor/tests/test_single_pack_transfer_base.py index 2fd243f50e..e1f2ea3c17 100644 --- a/shopfloor/tests/test_single_pack_transfer_base.py +++ b/shopfloor/tests/test_single_pack_transfer_base.py @@ -1,6 +1,8 @@ # Copyright 2020 Camptocamp SA (http://www.camptocamp.com) # Copyright 2020 Akretion (http://www.akretion.com) # License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl.html). +from odoo.tests.common import Form + from .common import CommonCase @@ -28,3 +30,19 @@ def setUp(self): self.service = self.get_service( "single_pack_transfer", menu=self.menu, profile=self.profile ) + + @classmethod + def _create_initial_move(cls, lines): + """Create the move to satisfy the pre-condition before /start""" + picking_form = Form(cls.env["stock.picking"]) + picking_form.picking_type_id = cls.picking_type + picking_form.location_id = cls.stock_location + picking_form.location_dest_id = cls.shelf2 + for line in lines: + with picking_form.move_ids_without_package.new() as move: + move.product_id = line[0] + move.product_uom_qty = line[1] + picking = picking_form.save() + picking.action_confirm() + picking.action_assign() + return picking From c0b26dce578fa9aff4ca00f716b8483331812d75 Mon Sep 17 00:00:00 2001 From: Thierry Ducrest Date: Tue, 17 Oct 2023 12:07:25 +0200 Subject: [PATCH 3/5] Add shopfloor_location_package_restriction Finally it only contains tests so could be rename or better removed. --- .../shopfloor_location_package_restriction | 1 + .../setup.py | 6 ++ .../__init__.py | 0 .../__manifest__.py | 18 +++++ .../readme/CONTRIBUTORS.rst | 1 + .../readme/DESCRIPTION.rst | 7 ++ .../tests/__init__.py | 2 + ...test_single_pack_transfer_force_package.py | 69 +++++++++++++++++++ .../tests/test_zone_picking_force_package.py | 50 ++++++++++++++ 9 files changed, 154 insertions(+) create mode 120000 setup/shopfloor_location_package_restriction/odoo/addons/shopfloor_location_package_restriction create mode 100644 setup/shopfloor_location_package_restriction/setup.py create mode 100644 shopfloor_location_package_restriction/__init__.py create mode 100644 shopfloor_location_package_restriction/__manifest__.py create mode 100644 shopfloor_location_package_restriction/readme/CONTRIBUTORS.rst create mode 100644 shopfloor_location_package_restriction/readme/DESCRIPTION.rst create mode 100644 shopfloor_location_package_restriction/tests/__init__.py create mode 100644 shopfloor_location_package_restriction/tests/test_single_pack_transfer_force_package.py create mode 100644 shopfloor_location_package_restriction/tests/test_zone_picking_force_package.py diff --git a/setup/shopfloor_location_package_restriction/odoo/addons/shopfloor_location_package_restriction b/setup/shopfloor_location_package_restriction/odoo/addons/shopfloor_location_package_restriction new file mode 120000 index 0000000000..2104d7dd9b --- /dev/null +++ b/setup/shopfloor_location_package_restriction/odoo/addons/shopfloor_location_package_restriction @@ -0,0 +1 @@ +../../../../shopfloor_location_package_restriction \ No newline at end of file diff --git a/setup/shopfloor_location_package_restriction/setup.py b/setup/shopfloor_location_package_restriction/setup.py new file mode 100644 index 0000000000..28c57bb640 --- /dev/null +++ b/setup/shopfloor_location_package_restriction/setup.py @@ -0,0 +1,6 @@ +import setuptools + +setuptools.setup( + setup_requires=['setuptools-odoo'], + odoo_addon=True, +) diff --git a/shopfloor_location_package_restriction/__init__.py b/shopfloor_location_package_restriction/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/shopfloor_location_package_restriction/__manifest__.py b/shopfloor_location_package_restriction/__manifest__.py new file mode 100644 index 0000000000..53b8f6adf7 --- /dev/null +++ b/shopfloor_location_package_restriction/__manifest__.py @@ -0,0 +1,18 @@ +# Copyright 2023 Camptocamp SA +# License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl.html) + +{ + "name": "Shopfloor Location Package Restriction", + "summary": "Glue module between shopfloor and location package restriction", + "version": "14.0.1.0.0", + "category": "Inventory", + "website": "https://github.com/OCA/wms", + "author": "Camptocamp, Odoo Community Association (OCA)", + "license": "AGPL-3", + "depends": [ + "shopfloor", + # OCA/stock-logistics-warehouse + "stock_location_package_restriction", + ], + "auto_install": True, +} diff --git a/shopfloor_location_package_restriction/readme/CONTRIBUTORS.rst b/shopfloor_location_package_restriction/readme/CONTRIBUTORS.rst new file mode 100644 index 0000000000..0dd376faec --- /dev/null +++ b/shopfloor_location_package_restriction/readme/CONTRIBUTORS.rst @@ -0,0 +1 @@ +* Thierry Ducrest diff --git a/shopfloor_location_package_restriction/readme/DESCRIPTION.rst b/shopfloor_location_package_restriction/readme/DESCRIPTION.rst new file mode 100644 index 0000000000..6410f8969b --- /dev/null +++ b/shopfloor_location_package_restriction/readme/DESCRIPTION.rst @@ -0,0 +1,7 @@ +Glue module between `shopfloor` and `stock_location_package_restriction`. +It allows to send proper error message to the frontend instead of a stack +trace error being displayed. + +Finally his only use is to test. + +Should be removed. diff --git a/shopfloor_location_package_restriction/tests/__init__.py b/shopfloor_location_package_restriction/tests/__init__.py new file mode 100644 index 0000000000..a4486cba47 --- /dev/null +++ b/shopfloor_location_package_restriction/tests/__init__.py @@ -0,0 +1,2 @@ +from . import test_single_pack_transfer_force_package +from . import test_zone_picking_force_package diff --git a/shopfloor_location_package_restriction/tests/test_single_pack_transfer_force_package.py b/shopfloor_location_package_restriction/tests/test_single_pack_transfer_force_package.py new file mode 100644 index 0000000000..dcc64eede5 --- /dev/null +++ b/shopfloor_location_package_restriction/tests/test_single_pack_transfer_force_package.py @@ -0,0 +1,69 @@ +# Copyright 2023 Camptocamp SA +# License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl) + +from odoo.addons.shopfloor.tests.test_single_pack_transfer_base import ( + SinglePackTransferCommonBase, +) + + +class TestSinglePackTransferForcePackage(SinglePackTransferCommonBase): + @classmethod + def setUpClass(cls): + super().setUpClass() + # Prepare the pack and related picking has started + cls.pack_a = cls.env["stock.quant.package"].create( + {"location_id": cls.stock_location.id} + ) + cls.quant_a = ( + cls.env["stock.quant"] + .sudo() + .create( + { + "product_id": cls.product_a.id, + "location_id": cls.shelf1.id, + "quantity": 1, + "package_id": cls.pack_a.id, + } + ) + ) + cls.picking = cls._create_initial_move(lines=[(cls.product_a, 1)]) + cls.move_line = cls.picking.move_line_ids + cls.package_level = cls.move_line.package_level_id + cls.package_level.is_done = True + cls.picking.move_line_ids.qty_done = 1 + # Add restriction on destination location + cls.shelf2.sudo().package_restriction = "singlepackage" + # Add a package on the destination location + cls.pack_1 = cls.env["stock.quant.package"].create( + {"location_id": cls.shelf2.id} + ) + cls._update_qty_in_location( + cls.shelf2, + cls.product_a, + 1, + package=cls.pack_1, + ) + + def test_scan_location_has_restrictions(self): + """ """ + response = self.service.dispatch( + "validate", + params={ + "package_level_id": self.package_level.id, + "location_barcode": self.shelf2.barcode, + }, + ) + message = { + "message_type": "error", + "body": ( + f"Only one package is allowed on the location " + f"{self.shelf2.display_name}.You cannot add " + f"the {self.move_line.package_id.name}, there is already {self.pack_1.name}." + ), + } + self.assert_response( + response, + next_state="scan_location", + data=self.ANY, + message=message, + ) diff --git a/shopfloor_location_package_restriction/tests/test_zone_picking_force_package.py b/shopfloor_location_package_restriction/tests/test_zone_picking_force_package.py new file mode 100644 index 0000000000..567d989a0b --- /dev/null +++ b/shopfloor_location_package_restriction/tests/test_zone_picking_force_package.py @@ -0,0 +1,50 @@ +# Copyright 2023 Camptocamp SA +# License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl) + +from odoo.addons.shopfloor.tests.test_zone_picking_base import ZonePickingCommonCase + + +class TestZonePickingForcePackage(ZonePickingCommonCase): + def setUp(self): + super().setUp() + self.service.work.current_picking_type = self.picking1.picking_type_id + + def test_set_destination_location_package_restriction(self): + """Check error restriction on location is properly forwarded to frontend.""" + # Add a restriction on the location + self.packing_location.sudo().package_restriction = "singlepackage" + # Add a first package on the location + self.pack_1 = self.env["stock.quant.package"].create( + {"location_id": self.packing_location.id} + ) + self._update_qty_in_location( + self.packing_location, self.product_a, 1, package=self.pack_1 + ) + picking_type = self.picking1.picking_type_id + move_line = self.picking1.move_lines.move_line_ids + move_line.qty_done = move_line.product_uom_qty + response = self.service.dispatch( + "set_destination", + params={ + "move_line_id": move_line.id, + "barcode": self.packing_location.barcode, + "quantity": move_line.product_uom_qty, + "confirmation": False, + }, + ) + message = { + "message_type": "error", + "body": ( + f"Only one package is allowed on the location " + f"{self.packing_location.display_name}.You cannot add " + f"the {move_line.package_id.name}, there is already {self.pack_1.name}." + ), + } + self.assert_response_set_line_destination( + response, + self.zone_location, + picking_type, + move_line, + message=message, + qty_done=10.0, + ) From 96812b3a6d9ad62ed5bfd62a75c0ba5c13f4bf6b Mon Sep 17 00:00:00 2001 From: Thierry Ducrest Date: Mon, 30 Oct 2023 11:37:05 +0100 Subject: [PATCH 4/5] fixup! sf_base: add decorator to gracefully handle exceptions --- shopfloor_base/utils.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/shopfloor_base/utils.py b/shopfloor_base/utils.py index befac317e9..a3e0aeb396 100644 --- a/shopfloor_base/utils.py +++ b/shopfloor_base/utils.py @@ -43,15 +43,18 @@ def catch_errors(response_error): def _catch_errors(fn): @wraps(fn) - def wrapper(*args, **kwargs): + def wrapper(self, *args, **kwargs): + savepoint = self._actions_for("savepoint").new() try: - res = fn(*args, **kwargs) + res = fn(self, *args, **kwargs) except ValidationError as ex: + savepoint.rollback() message = { "message_type": "error", "body": ex.name, } - return response_error(*args, **kwargs, message=message) + return response_error(self, *args, **kwargs, message=message) + savepoint.release() return res return wrapper From 041e7e537bb910cf9b70cb662b53ca2479d11ff7 Mon Sep 17 00:00:00 2001 From: Thierry Ducrest Date: Mon, 30 Oct 2023 13:14:15 +0100 Subject: [PATCH 5/5] fixup! fixup! sf_base: add decorator to gracefully handle exceptions --- shopfloor_base/utils.py | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/shopfloor_base/utils.py b/shopfloor_base/utils.py index a3e0aeb396..fbff6ae162 100644 --- a/shopfloor_base/utils.py +++ b/shopfloor_base/utils.py @@ -5,7 +5,7 @@ import os from functools import wraps -from odoo.exceptions import ValidationError +from odoo.exceptions import UserError, ValidationError from odoo.modules.module import load_information_from_description_file from odoo.tools.config import config as odoo_config @@ -34,24 +34,27 @@ def wrapped(*args, **kwargs): return _ensure_model -def catch_errors(response_error): +def catch_errors(response_error, exceptions_to_catch=None): """Decorator for service endpoints to gracefully handle some exceptions. The decorator is passed a function with the same arguments than the function handlling the endpoint, plus a keyword argument called `message`. """ + if exceptions_to_catch is None: + exceptions_to_catch = (ValidationError, UserError) + def _catch_errors(fn): @wraps(fn) def wrapper(self, *args, **kwargs): savepoint = self._actions_for("savepoint").new() try: res = fn(self, *args, **kwargs) - except ValidationError as ex: + except exceptions_to_catch as ex: savepoint.rollback() message = { "message_type": "error", - "body": ex.name, + "body": str(ex), } return response_error(self, *args, **kwargs, message=message) savepoint.release()