From 65d413426dc18645ff353d0e0412721f5390781e Mon Sep 17 00:00:00 2001
From: Thierry Ducrest <thierry.ducrest@camptocamp.com>
Date: Fri, 13 Oct 2023 11:56:22 +0200
Subject: [PATCH] fixup! shopfloor: zp, spt refactor to help iheritance

---
 shopfloor/services/service.py              | 16 ++++++++++++++++
 shopfloor/services/single_pack_transfer.py | 18 +++---------------
 shopfloor/services/zone_picking.py         |  9 ++-------
 3 files changed, 21 insertions(+), 22 deletions(-)

diff --git a/shopfloor/services/service.py b/shopfloor/services/service.py
index bf10f67907..cb1189659f 100644
--- a/shopfloor/services/service.py
+++ b/shopfloor/services/service.py
@@ -85,6 +85,22 @@ def is_dest_location_valid(self, moves, location):
             moves.picking_id.location_dest_id, func=all
         ) or location.is_sublocation_of(moves.location_dest_id, func=all)
 
+    def validate_dest_location(self, moves, location):
+        """Validate the destination for given moves.
+
+        The previous function `is_dest_location_valid` has a limited use
+        because it only returns a Boolean for the check.
+        Returning an error message allows inheriting modules to commnuicate
+        their own error.
+
+        Returns a an error message if not valid.
+        """
+        if not location:
+            return self.msg_store.no_location_found()
+        if not self.is_dest_location_valid(moves, location):
+            return self.msg_store.dest_location_not_allowed()
+        return ""
+
     def is_dest_location_to_confirm(self, location_dest_id, location):
         """Check the destination location requires confirmation
 
diff --git a/shopfloor/services/single_pack_transfer.py b/shopfloor/services/single_pack_transfer.py
index 3e0e792678..c0f3e497e5 100644
--- a/shopfloor/services/single_pack_transfer.py
+++ b/shopfloor/services/single_pack_transfer.py
@@ -205,18 +205,6 @@ def _create_package_level(self, package):
     def _is_move_state_valid(self, moves):
         return all(move.state != "cancel" for move in moves)
 
-    def _validate_destination_location(self, package_level, location):
-        moves = package_level.move_line_ids.move_id
-        if not location:
-            return self._response_for_scan_location(
-                package_level, message=self.msg_store.no_location_found()
-            )
-        if not self.is_dest_location_valid(moves, location):
-            return self._response_for_scan_location(
-                package_level, message=self.msg_store.dest_location_not_allowed()
-            )
-        return None
-
     def validate(self, package_level_id, location_barcode, confirmation=False):
         """Validate the transfer"""
         search = self._actions_for("search")
@@ -236,9 +224,9 @@ def validate(self, package_level_id, location_barcode, confirmation=False):
             )
 
         scanned_location = search.location_from_scan(location_barcode)
-        response = self._validate_destination_location(package_level, scanned_location)
-        if response:
-            return response
+        message = self.validate_dest_location(moves, scanned_location)
+        if message:
+            return self._response_for_scan_location(package_level, message=message)
 
         if not confirmation and self.is_dest_location_to_confirm(
             package_level.location_dest_id, scanned_location
diff --git a/shopfloor/services/zone_picking.py b/shopfloor/services/zone_picking.py
index 071d9f6f1c..649e833efd 100644
--- a/shopfloor/services/zone_picking.py
+++ b/shopfloor/services/zone_picking.py
@@ -899,7 +899,7 @@ def _set_destination_location(
         # if `confirmation is True
         # Ask confirmation to the user if the scanned location is not in the
         # expected ones but is valid (in picking type's default destination)
-        message = self._validate_destination(location, move_line.move_id)
+        message = self.validate_dest_location(move_line.move_id, location)
         if message:
             response = self._response_for_set_line_destination(
                 move_line,
@@ -1543,7 +1543,7 @@ def set_destination_all(self, barcode, confirmation=False):
             # check if the scanned location is allowed
             moves = buffer_lines.mapped("move_id")
             if not message:
-                message = self._validate_destination(location, moves)
+                message = self.validate_dest_location(moves, location)
 
             if message:
                 return self._set_destination_all_response(buffer_lines, message=message)
@@ -1572,11 +1572,6 @@ def set_destination_all(self, barcode, confirmation=False):
             message = self.msg_store.no_location_found()
         return self._set_destination_all_response(buffer_lines, message=message)
 
-    def _validate_destination(self, location, moves):
-        if not self.is_dest_location_valid(moves, location):
-            return self.msg_store.dest_location_not_allowed()
-        return None
-
     def _write_destination_on_lines(self, lines, location):
         self._lock_lines(lines)
         lines.location_dest_id = location