Skip to content
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.

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

[14.0][IMP] shopfloor: add decorator to gracefully handle exceptions on response #768

Open
wants to merge 5 commits into
base: 14.0
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 6 additions & 0 deletions setup/shopfloor_location_package_restriction/setup.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
import setuptools

setuptools.setup(
setup_requires=['setuptools-odoo'],
odoo_addon=True,
)
11 changes: 10 additions & 1 deletion shopfloor/services/single_pack_transfer.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down Expand Up @@ -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(
Expand Down
25 changes: 20 additions & 5 deletions shopfloor/services/zone_picking.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand Down
18 changes: 0 additions & 18 deletions shopfloor/tests/test_single_pack_transfer.py
Original file line number Diff line number Diff line change
Expand Up @@ -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


Expand Down Expand Up @@ -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.

Expand Down
18 changes: 18 additions & 0 deletions shopfloor/tests/test_single_pack_transfer_base.py
Original file line number Diff line number Diff line change
@@ -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


Expand Down Expand Up @@ -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
32 changes: 32 additions & 0 deletions shopfloor_base/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import os
from functools import wraps

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

Expand Down Expand Up @@ -33,6 +34,37 @@ def wrapped(*args, **kwargs):
return _ensure_model


def catch_errors(response_error, exceptions_to_catch=None):
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
def catch_errors(response_error, exceptions_to_catch=None):
def with_savepoint(response_error, exceptions_to_catch=None):

I would prefer this name, not blocking

"""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 exceptions_to_catch as ex:
savepoint.rollback()
message = {
"message_type": "error",
"body": str(ex),
}
return response_error(self, *args, **kwargs, message=message)
savepoint.release()
return res

return wrapper

return _catch_errors


APP_VERSIONS = {}


Expand Down
Empty file.
18 changes: 18 additions & 0 deletions shopfloor_location_package_restriction/__manifest__.py
Original file line number Diff line number Diff line change
@@ -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,
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
* Thierry Ducrest <thierry.ducrest@camptocamp.com>
7 changes: 7 additions & 0 deletions shopfloor_location_package_restriction/readme/DESCRIPTION.rst
Original file line number Diff line number Diff line change
@@ -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.
2 changes: 2 additions & 0 deletions shopfloor_location_package_restriction/tests/__init__.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
from . import test_single_pack_transfer_force_package
from . import test_zone_picking_force_package
Original file line number Diff line number Diff line change
@@ -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,
)
Original file line number Diff line number Diff line change
@@ -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,
)
Loading