[16.0][FIX] shopfloor_reception/mobile: set_destination validation flow

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)
@@ -935,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,
         )
@@ -1469,19 +1445,24 @@ 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.
 
         input:
             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 +1476,31 @@ 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
+                ),
+                confirmation=location_name,
+            )
+        selected_line.location_dest_id = location
+
         response = self._post_line(selected_line)
         if response:
             return response
@@ -1690,7 +1665,7 @@ def set_destination(self):
                 "required": True,
             },
             "location_name": {"required": True, "type": "string"},
-            "confirmation": {"type": "boolean"},
+            "confirmation": {"type": "string"},
         }
 
     def select_dest_package(self):
@@ -1880,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

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,
         )

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,
             },
         )
 

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)
         )
@@ -103,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"},
         )

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)

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)

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 || "",
                     })
                 );
             },