From 633873365d6e0a4d048d48633843f7814298b4cb Mon Sep 17 00:00:00 2001
From: Oliver <oliver.henry.walters@gmail.com>
Date: Tue, 9 Jul 2024 00:45:47 +1000
Subject: [PATCH] BOM delete fix (#7586)

* Fix order of operations for BOM item bulk delete

* Improve error messaging

* Add "validate_delete" method
---
 src/backend/InvenTree/InvenTree/api.py | 23 ++++++++++++++++++++++-
 src/backend/InvenTree/part/api.py      |  4 +---
 src/backend/InvenTree/part/models.py   |  8 ++++----
 3 files changed, 27 insertions(+), 8 deletions(-)

diff --git a/src/backend/InvenTree/InvenTree/api.py b/src/backend/InvenTree/InvenTree/api.py
index b4fa3f298d..cba3727c9e 100644
--- a/src/backend/InvenTree/InvenTree/api.py
+++ b/src/backend/InvenTree/InvenTree/api.py
@@ -311,8 +311,26 @@ class BulkDeleteMixin:
     - Speed (single API call and DB query)
     """
 
+    def validate_delete(self, queryset, request) -> None:
+        """Perform validation right before deletion.
+
+        Arguments:
+            queryset: The queryset to be deleted
+            request: The request object
+
+        Returns:
+            None
+
+        Raises:
+            ValidationError: If the deletion should not proceed
+        """
+        pass
+
     def filter_delete_queryset(self, queryset, request):
-        """Provide custom filtering for the queryset *before* it is deleted."""
+        """Provide custom filtering for the queryset *before* it is deleted.
+
+        The default implementation does nothing, just returns the queryset.
+        """
         return queryset
 
     def delete(self, request, *args, **kwargs):
@@ -371,6 +389,9 @@ class BulkDeleteMixin:
             if filters:
                 queryset = queryset.filter(**filters)
 
+            # Run a final validation step (should raise an error if the deletion should not proceed)
+            self.validate_delete(queryset, request)
+
             n_deleted = queryset.count()
             queryset.delete()
 
diff --git a/src/backend/InvenTree/part/api.py b/src/backend/InvenTree/part/api.py
index 74522b0268..d8b6fbd190 100644
--- a/src/backend/InvenTree/part/api.py
+++ b/src/backend/InvenTree/part/api.py
@@ -1875,14 +1875,12 @@ class BomList(BomMixin, DataExportViewMixin, ListCreateDestroyAPIView):
         'pricing_updated': 'sub_part__pricing_data__updated',
     }
 
-    def filter_delete_queryset(self, queryset, request):
+    def validate_delete(self, queryset, request) -> None:
         """Ensure that there are no 'locked' items."""
         for bom_item in queryset:
             # Note: Calling check_part_lock may raise a ValidationError
             bom_item.check_part_lock(bom_item.part)
 
-        return super().filter_delete_queryset(queryset, request)
-
 
 class BomDetail(BomMixin, RetrieveUpdateDestroyAPI):
     """API endpoint for detail view of a single BomItem object."""
diff --git a/src/backend/InvenTree/part/models.py b/src/backend/InvenTree/part/models.py
index b7cb80c101..561c200bd2 100644
--- a/src/backend/InvenTree/part/models.py
+++ b/src/backend/InvenTree/part/models.py
@@ -4101,16 +4101,16 @@ class BomItem(
         """
         # TODO: Perhaps control this with a global setting?
 
-        msg = _('BOM item cannot be modified - assembly is locked')
-
         if assembly.locked:
-            raise ValidationError(msg)
+            raise ValidationError(_('BOM item cannot be modified - assembly is locked'))
 
         # If this BOM item is inherited, check all variants of the assembly
         if self.inherited:
             for part in assembly.get_descendants(include_self=False):
                 if part.locked:
-                    raise ValidationError(msg)
+                    raise ValidationError(
+                        _('BOM item cannot be modified - variant assembly is locked')
+                    )
 
     # A link to the parent part
     # Each part will get a reverse lookup field 'bom_items'