From 653409f7659e37655fd2a8c0c7b071079390fdd8 Mon Sep 17 00:00:00 2001 From: Nicolas Delbovier Date: Tue, 21 Apr 2026 14:05:18 +0200 Subject: [PATCH 1/3] [REF] shopfloor_reception: use core `is_dest_location_valid` Replaces the custom location validation logic in shopfloor_reception with the standard 'is_dest_location_valid' method from the base shopfloor module. This avoids code duplication and ensures consistent behavior across the application. --- shopfloor_reception/services/reception.py | 79 ++++++----------- .../tests/test_set_destination.py | 88 ++++++++++++------- 2 files changed, 80 insertions(+), 87 deletions(-) diff --git a/shopfloor_reception/services/reception.py b/shopfloor_reception/services/reception.py index ba9e5ec8f01..fe813b70418 100644 --- a/shopfloor_reception/services/reception.py +++ b/shopfloor_reception/services/reception.py @@ -709,12 +709,7 @@ def _set_package_on_move_line(self, picking, line, package): if not pack_location: line.result_package_id = package return None - ( - move_dest_location_ok, - pick_type_dest_location_ok, - ) = self._check_location_ok(pack_location, line, picking) - if not (move_dest_location_ok or pick_type_dest_location_ok): - # Package location is not a child of the move destination + if not self.is_dest_location_valid(line.move_id, pack_location): message = self.msg_store.dest_location_not_allowed() return self._response_for_set_quantity(picking, line, message=message) quantity = line.qty_done @@ -740,11 +735,7 @@ def _set_quantity__by_package(self, picking, selected_line, package): return self._response_for_set_destination(picking, selected_line) def _set_quantity__by_location(self, picking, selected_line, location): - move_dest_location_ok, pick_type_dest_location_ok = self._check_location_ok( - location, selected_line, picking - ) - if not (move_dest_location_ok or pick_type_dest_location_ok): - # Scanned location isn't a child of the move's dest location + if not self.is_dest_location_valid(selected_line.move_id, location): message = self.msg_store.dest_location_not_allowed() return self._response_for_set_quantity( picking, selected_line, message=message @@ -759,24 +750,6 @@ def _set_quantity__by_lot(self, picking, selected_line, barcode): selected_line.qty_done += 1 return self._response_for_set_quantity(picking, selected_line) - def _check_location_ok(self, location, selected_line, picking): - if location.usage == "view": - return (False, False) - - move_dest_location = selected_line.location_dest_id - pick_type_dest_location = picking.picking_type_id.default_location_dest_id - - move_dest_location_ok = location.parent_path.startswith( - move_dest_location.parent_path - ) - pick_type_dest_location_ok = location.parent_path.startswith( - pick_type_dest_location.parent_path - ) - if move_dest_location_ok or pick_type_dest_location_ok: - return (move_dest_location_ok, pick_type_dest_location_ok) - - return (False, False) - def _use_handlers(self, handlers, *args, **kwargs): for handler in handlers: response = handler(*args, **kwargs) @@ -1469,8 +1442,13 @@ def _auto_post_line(self, selected_line): move._recompute_state() new_move.extract_and_action_done() + def is_dest_location_valid(self, moves, location): + if location.usage == "view": + return False + return super().is_dest_location_valid(moves, location) + def set_destination( - self, picking_id, selected_line_id, location_name, confirmation=False + self, picking_id, selected_line_id, location_name, confirmation="" ): """Set the destination on the move line. @@ -1478,10 +1456,10 @@ def set_destination( location_name: The name of the location transitions: - - set_destination: Warning: User scanned a child location of the picking type. + - set_destination: Warning: User scanned a valid but unexpected location. Ask for confirmation - set_destination: Error: User tried to scan a non-valid location - - select_move: User scanned a child location of the move's dest location + - select_move: User scanned a valid location """ picking = self.env["stock.picking"].browse(picking_id) selected_line = self.env["stock.move.line"].browse(selected_line_id) @@ -1495,37 +1473,30 @@ def set_destination( return self._response_for_set_destination( picking, selected_line, message=message ) - search = self._actions_for("search") - location = search.location_from_scan(location_name) + location = self._actions_for("search").location_from_scan(location_name) if not location: return self._response_for_set_destination( picking, selected_line, message=self.msg_store.no_location_found() ) - move_dest_location_ok, pick_type_dest_location_ok = self._check_location_ok( - location, selected_line, picking - ) - if not (move_dest_location_ok or pick_type_dest_location_ok): + if not self.is_dest_location_valid(selected_line.move_id, location): return self._response_for_set_destination( picking, selected_line, message=self.msg_store.dest_location_not_allowed(), ) - if move_dest_location_ok: - # If location is a child of move's dest location, assign it without asking - selected_line.location_dest_id = location - elif pick_type_dest_location_ok: - # If location is a child of picking types's dest location, - # ask for confirmation before assigning - if not confirmation: - return self._response_for_set_destination( - picking, - selected_line, - message=self.msg_store.place_in_location_ask_confirmation( - location.name - ), - ) - selected_line.location_dest_id = location + if confirmation != location_name and self.is_dest_location_to_confirm( + selected_line.location_dest_id, location + ): + return self._response_for_set_destination( + picking, + selected_line, + message=self.msg_store.place_in_location_ask_confirmation( + location.name + ), + ) + selected_line.location_dest_id = location + response = self._post_line(selected_line) if response: return response @@ -1690,7 +1661,7 @@ def set_destination(self): "required": True, }, "location_name": {"required": True, "type": "string"}, - "confirmation": {"type": "boolean"}, + "confirmation": {"type": "string"}, } def select_dest_package(self): diff --git a/shopfloor_reception/tests/test_set_destination.py b/shopfloor_reception/tests/test_set_destination.py index e026ecc4a3b..f6a725f57ee 100644 --- a/shopfloor_reception/tests/test_set_destination.py +++ b/shopfloor_reception/tests/test_set_destination.py @@ -9,76 +9,98 @@ class TestSetDestination(CommonCase): def setUpClassBaseData(cls): super().setUpClassBaseData() cls.packing_location.sudo().active = True - cls.location_dest = cls.env.ref("stock.stock_location_stock") - - @classmethod - def _change_line_dest(cls, line): - # Modify the location dest on the move, so we have different children - # for move's dest_location and pick type's dest_location - line.location_dest_id = cls.location_dest + cls.sub_input_location = ( + cls.env["stock.location"] + .sudo() + .create( + { + "name": "Test Reception Shelf", + "location_id": cls.input_location.id, + } + ) + ) + cls.sub_input_location_2 = ( + cls.env["stock.location"] + .sudo() + .create( + { + "name": "Test Reception Shelf 2", + "location_id": cls.input_location.id, + } + ) + ) + cls.sub_stock_location = ( + cls.env["stock.location"] + .sudo() + .create( + { + "name": "Test Reception Shelf", + "location_id": cls.stock_location.id, + } + ) + ) def test_scan_location_child_of_dest_location(self): picking = self._create_picking() selected_move_line = picking.move_line_ids.filtered( lambda l: l.product_id == self.product_a ) - self._change_line_dest(selected_move_line) + self.assertTrue( + self.sub_input_location.is_sublocation_of(picking.location_dest_id) + ) + self.assertNotEqual( + self.sub_input_location, selected_move_line.location_dest_id + ) + response = self.service.dispatch( "set_destination", params={ "picking_id": picking.id, "selected_line_id": selected_move_line.id, - "location_name": self.shelf2.name, + "location_name": self.sub_input_location.name, }, ) - self.assertEqual(selected_move_line.location_dest_id, self.shelf2) + self.assertEqual(selected_move_line.location_dest_id, self.sub_input_location) self.assert_response( response, next_state="select_move", data=self._data_for_select_move(picking) ) - def test_scan_location_child_of_pick_type_dest_location(self): + def test_scan_location_valid_but_unexpected(self): picking = self._create_picking() selected_move_line = picking.move_line_ids.filtered( lambda l: l.product_id == self.product_a ) - self._change_line_dest(selected_move_line) + + selected_move_line.location_dest_id = self.sub_input_location + # `sub_input_location_2` is valid but not the expected location response = self.service.dispatch( "set_destination", params={ "picking_id": picking.id, "selected_line_id": selected_move_line.id, - "location_name": self.dispatch_location.name, + "location_name": self.sub_input_location_2.name, }, ) - - # location is a child of the picking type's location. destination location - # hasn't been set - self.assertNotEqual(selected_move_line.location_dest_id, self.dispatch_location) - # But a confirmation has been asked - data = self.data.picking(picking) - self.assert_response( + self.assertMessage( response, - next_state="set_destination", - data={ - "picking": data, - "selected_move_line": self.data.move_lines(selected_move_line), - }, - message={ - "message_type": "warning", - "body": f"Place it in {self.dispatch_location.name}?", - }, + self.msg_store.place_in_location_ask_confirmation( + self.sub_input_location_2.name + ), + ) + self.assertNotEqual( + selected_move_line.location_dest_id, self.sub_input_location_2 ) - # Send the same message with confirmation=True to confirm + response = self.service.dispatch( "set_destination", params={ "picking_id": picking.id, "selected_line_id": selected_move_line.id, - "location_name": self.dispatch_location.name, - "confirmation": True, + "location_name": self.sub_input_location_2.name, + "confirmation": self.sub_input_location_2.name, }, ) - self.assertEqual(selected_move_line.location_dest_id, self.dispatch_location) + self.assertEqual(selected_move_line.location_dest_id, self.sub_input_location_2) self.assert_response( response, next_state="select_move", data=self._data_for_select_move(picking) ) From 2f32dd381a2331f4f05e5db1c77075414724e434 Mon Sep 17 00:00:00 2001 From: Nicolas Delbovier Date: Tue, 21 Apr 2026 14:43:50 +0200 Subject: [PATCH 2/3] [FIX] shopfloor_reception: set_destination - pass confirmation to front --- shopfloor_reception/services/reception.py | 7 ++++++- shopfloor_reception/tests/test_recover.py | 2 ++ shopfloor_reception/tests/test_select_move.py | 1 + shopfloor_reception/tests/test_set_destination.py | 1 + shopfloor_reception/tests/test_set_quantity.py | 5 +++++ shopfloor_reception/tests/test_set_quantity_action.py | 2 ++ 6 files changed, 17 insertions(+), 1 deletion(-) diff --git a/shopfloor_reception/services/reception.py b/shopfloor_reception/services/reception.py index fe813b70418..514b083ab01 100644 --- a/shopfloor_reception/services/reception.py +++ b/shopfloor_reception/services/reception.py @@ -908,12 +908,15 @@ def _response_for_set_quantity( ) return self._align_display_product_uom_qty(line, response) - def _response_for_set_destination(self, picking, line, message=None): + def _response_for_set_destination( + self, picking, line, message=None, confirmation=None + ): return self._response( next_state="set_destination", data={ "selected_move_line": self._data_for_move_lines(line), "picking": self.data.picking(picking), + "confirmation": confirmation, }, message=message, ) @@ -1494,6 +1497,7 @@ def set_destination( message=self.msg_store.place_in_location_ask_confirmation( location.name ), + confirmation=location_name, ) selected_line.location_dest_id = location @@ -1851,6 +1855,7 @@ def _schema_set_destination(self): "schema": {"type": "dict", "schema": self.schemas.move_line()}, }, "picking": {"type": "dict", "schema": self.schemas.picking()}, + "confirmation": {"type": "string", "nullable": True}, } @property diff --git a/shopfloor_reception/tests/test_recover.py b/shopfloor_reception/tests/test_recover.py index 622366cc096..ab51dbdf03e 100644 --- a/shopfloor_reception/tests/test_recover.py +++ b/shopfloor_reception/tests/test_recover.py @@ -102,6 +102,7 @@ def test_recover(self): data={ "picking": picking_data, "selected_move_line": move_line_data, + "confirmation": None, }, ) # Scan the line again, we should end up with the exact same result @@ -116,6 +117,7 @@ def test_recover(self): data={ "picking": picking_data, "selected_move_line": move_line_data, + "confirmation": None, }, message=self.recover_msg, ) diff --git a/shopfloor_reception/tests/test_select_move.py b/shopfloor_reception/tests/test_select_move.py index ad0ded79d9e..63dc7b9bd60 100644 --- a/shopfloor_reception/tests/test_select_move.py +++ b/shopfloor_reception/tests/test_select_move.py @@ -103,6 +103,7 @@ def test_scan_product_partial(self): data={ "picking": data, "selected_move_line": self.data.move_lines(selected_move_line), + "confirmation": None, }, ) diff --git a/shopfloor_reception/tests/test_set_destination.py b/shopfloor_reception/tests/test_set_destination.py index f6a725f57ee..ba2738b0d13 100644 --- a/shopfloor_reception/tests/test_set_destination.py +++ b/shopfloor_reception/tests/test_set_destination.py @@ -125,6 +125,7 @@ def test_scan_location_not_child_of_dest_locations(self): data={ "picking": data, "selected_move_line": self.data.move_lines(selected_move_line), + "confirmation": None, }, message={"message_type": "error", "body": "You cannot place it here"}, ) diff --git a/shopfloor_reception/tests/test_set_quantity.py b/shopfloor_reception/tests/test_set_quantity.py index 9b4da52973e..5d7d626e53c 100644 --- a/shopfloor_reception/tests/test_set_quantity.py +++ b/shopfloor_reception/tests/test_set_quantity.py @@ -285,6 +285,7 @@ def test_scan_package_without_location(self): data={ "picking": data, "selected_move_line": self.data.move_lines(selected_move_line), + "confirmation": None, }, ) @@ -407,6 +408,7 @@ def test_scan_new_package(self): data={ "picking": picking_data, "selected_move_line": self.data.move_lines(selected_move_line), + "confirmation": None, }, ) @@ -667,6 +669,7 @@ def test_split_move_line(self): data={ "picking": picking_data, "selected_move_line": self.data.move_lines(selected_move_line), + "confirmation": None, }, ) # there should be 3 lines now @@ -742,6 +745,7 @@ def test_concurrent_update_2(self): data={ "picking": picking_data, "selected_move_line": self.data.move_lines(move_line_user_1), + "confirmation": None, }, ) @@ -845,6 +849,7 @@ def test_move_states(self): data={ "picking": data, "selected_move_line": self.data.move_lines(move_line_user_2), + "confirmation": None, }, ) self.assertEqual(move_product_a.quantity_done, 1.0) diff --git a/shopfloor_reception/tests/test_set_quantity_action.py b/shopfloor_reception/tests/test_set_quantity_action.py index cceb476a373..f1eb6fd3f72 100644 --- a/shopfloor_reception/tests/test_set_quantity_action.py +++ b/shopfloor_reception/tests/test_set_quantity_action.py @@ -56,6 +56,7 @@ def test_process_with_new_package(self): data={ "picking": data, "selected_move_line": self.data.move_lines(self.selected_move_line), + "confirmation": None, }, ) self.assertTrue(self.selected_move_line.result_package_id) @@ -76,6 +77,7 @@ def test_process_without_package(self): data={ "picking": data, "selected_move_line": self.data.move_lines(self.selected_move_line), + "confirmation": None, }, ) self.assertFalse(self.selected_move_line.result_package_id) From d1b41a06b3c3634980b91811bf1c6f8907a37a9e Mon Sep 17 00:00:00 2001 From: Nicolas Delbovier Date: Tue, 21 Apr 2026 14:36:40 +0200 Subject: [PATCH 3/3] [FIX] shopfloor_reception_mobile: trigger confirmation on set_destination --- .../static/src/scenario/reception_states.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/shopfloor_reception_mobile/static/src/scenario/reception_states.js b/shopfloor_reception_mobile/static/src/scenario/reception_states.js index 9ff37f360a9..624c085e0e5 100644 --- a/shopfloor_reception_mobile/static/src/scenario/reception_states.js +++ b/shopfloor_reception_mobile/static/src/scenario/reception_states.js @@ -239,8 +239,7 @@ export const reception_states = function () { picking_id: this.state.data.picking.id, selected_line_id: this.line_being_handled.id, location_name: location.text, - // FIXME if it is always set to true, it is not really used ? - confirmation: true, + confirmation: this.state.data.confirmation || "", }) ); },