From c540b12c97bd9478a75d5fd9d92d4f3f89fb64ab Mon Sep 17 00:00:00 2001 From: Oliver Date: Mon, 20 May 2024 12:51:56 +1000 Subject: [PATCH] Prevent deletion of part which is used in an assembly (#7260) * Prevent deletion of part which is used in an assembly * add 'validate_model_deletion' option to ValidationMixin plugun * Add global setting to control part delete behaviour * Update settings location * Unit test updates * Further unit test updates * Fix typos --- src/backend/InvenTree/InvenTree/models.py | 14 +++++++++++++ src/backend/InvenTree/common/models.py | 6 ++++++ src/backend/InvenTree/company/tests.py | 6 +++++- src/backend/InvenTree/part/api.py | 15 ------------- src/backend/InvenTree/part/models.py | 21 +++++++++++++++++++ src/backend/InvenTree/part/test_api.py | 4 +++- src/backend/InvenTree/part/test_bom_item.py | 2 ++ src/backend/InvenTree/part/test_part.py | 4 ++++ .../base/integration/ValidationMixin.py | 17 +++++++++++++++ .../templates/InvenTree/settings/part.html | 1 + src/frontend/src/components/forms/ApiForm.tsx | 2 +- .../pages/Index/Settings/SystemSettings.tsx | 1 + 12 files changed, 75 insertions(+), 18 deletions(-) diff --git a/src/backend/InvenTree/InvenTree/models.py b/src/backend/InvenTree/InvenTree/models.py index 6317c673d2..2ceda41f2f 100644 --- a/src/backend/InvenTree/InvenTree/models.py +++ b/src/backend/InvenTree/InvenTree/models.py @@ -122,6 +122,20 @@ class PluginValidationMixin(DiffMixin): self.run_plugin_validation() super().save(*args, **kwargs) + def delete(self): + """Run plugin validation on model delete. + + Allows plugins to prevent model instances from being deleted. + + Note: Each plugin may raise a ValidationError to prevent deletion. + """ + from plugin.registry import registry + + for plugin in registry.with_mixin('validation'): + plugin.validate_model_deletion(self) + + super().delete() + class MetadataMixin(models.Model): """Model mixin class which adds a JSON metadata field to a model, for use by any (and all) plugins. diff --git a/src/backend/InvenTree/common/models.py b/src/backend/InvenTree/common/models.py index 76dccae4d4..d5f18234e8 100644 --- a/src/backend/InvenTree/common/models.py +++ b/src/backend/InvenTree/common/models.py @@ -1436,6 +1436,12 @@ class InvenTreeSetting(BaseInvenTreeSetting): 'validator': bool, 'default': True, }, + 'PART_ALLOW_DELETE_FROM_ASSEMBLY': { + 'name': _('Allow Deletion from Assembly'), + 'description': _('Allow deletion of parts which are used in an assembly'), + 'validator': bool, + 'default': False, + }, 'PART_IPN_REGEX': { 'name': _('IPN Regex'), 'description': _('Regular expression pattern for matching Part IPN'), diff --git a/src/backend/InvenTree/company/tests.py b/src/backend/InvenTree/company/tests.py index a789b951d4..351fee268d 100644 --- a/src/backend/InvenTree/company/tests.py +++ b/src/backend/InvenTree/company/tests.py @@ -286,7 +286,11 @@ class ManufacturerPartSimpleTest(TestCase): def test_delete(self): """Test deletion of a ManufacturerPart.""" - Part.objects.get(pk=self.part.id).delete() + part = Part.objects.get(pk=self.part.id) + part.active = False + part.save() + part.delete() + # Check that ManufacturerPart was deleted self.assertEqual(ManufacturerPart.objects.count(), 3) diff --git a/src/backend/InvenTree/part/api.py b/src/backend/InvenTree/part/api.py index df0a3451f4..12b25b723a 100644 --- a/src/backend/InvenTree/part/api.py +++ b/src/backend/InvenTree/part/api.py @@ -1446,21 +1446,6 @@ class PartChangeCategory(CreateAPI): class PartDetail(PartMixin, RetrieveUpdateDestroyAPI): """API endpoint for detail view of a single Part object.""" - def destroy(self, request, *args, **kwargs): - """Delete a Part instance via the API. - - - If the part is 'active' it cannot be deleted - - It must first be marked as 'inactive' - """ - part = Part.objects.get(pk=int(kwargs['pk'])) - # Check if inactive - if not part.active: - # Delete - return super(PartDetail, self).destroy(request, *args, **kwargs) - # Return 405 error - message = 'Part is active: cannot delete' - return Response(status=status.HTTP_405_METHOD_NOT_ALLOWED, data=message) - def update(self, request, *args, **kwargs): """Custom update functionality for Part instance. diff --git a/src/backend/InvenTree/part/models.py b/src/backend/InvenTree/part/models.py index b0e020efa3..c53f556fb5 100644 --- a/src/backend/InvenTree/part/models.py +++ b/src/backend/InvenTree/part/models.py @@ -448,6 +448,27 @@ class Part( return context + def delete(self, **kwargs): + """Custom delete method for the Part model. + + Prevents deletion of a Part if any of the following conditions are met: + + - The part is still active + - The part is used in a BOM for a different part. + """ + if self.active: + raise ValidationError(_('Cannot delete this part as it is still active')) + + if not common.models.InvenTreeSetting.get_setting( + 'PART_ALLOW_DELETE_FROM_ASSEMBLY', cache=False + ): + if BomItem.objects.filter(sub_part=self).exists(): + raise ValidationError( + _('Cannot delete this part as it is used in an assembly') + ) + + super().delete() + def save(self, *args, **kwargs): """Overrides the save function for the Part model. diff --git a/src/backend/InvenTree/part/test_api.py b/src/backend/InvenTree/part/test_api.py index f65d06a425..769d39f02c 100644 --- a/src/backend/InvenTree/part/test_api.py +++ b/src/backend/InvenTree/part/test_api.py @@ -1408,7 +1408,7 @@ class PartDetailTests(PartAPITestBase): response = self.delete(url) # As the part is 'active' we cannot delete it - self.assertEqual(response.status_code, 405) + self.assertEqual(response.status_code, 400) # So, let's make it not active response = self.patch(url, {'active': False}, expected_code=200) @@ -2586,6 +2586,8 @@ class PartInternalPriceBreakTest(InvenTreeAPITestCase): p.active = False p.save() + InvenTreeSetting.set_setting('PART_ALLOW_DELETE_FROM_ASSEMBLY', True) + response = self.delete(reverse('api-part-detail', kwargs={'pk': 1})) self.assertEqual(response.status_code, 204) diff --git a/src/backend/InvenTree/part/test_bom_item.py b/src/backend/InvenTree/part/test_bom_item.py index 7d5a8293ff..86d4578f4a 100644 --- a/src/backend/InvenTree/part/test_bom_item.py +++ b/src/backend/InvenTree/part/test_bom_item.py @@ -187,6 +187,8 @@ class BomItemTest(TestCase): self.assertEqual(bom_item.substitutes.count(), 4) for sub in subs: + sub.active = False + sub.save() sub.delete() # The substitution links should have been automatically removed diff --git a/src/backend/InvenTree/part/test_part.py b/src/backend/InvenTree/part/test_part.py index 57f91af817..9c70ea23ca 100644 --- a/src/backend/InvenTree/part/test_part.py +++ b/src/backend/InvenTree/part/test_part.py @@ -336,6 +336,8 @@ class PartTest(TestCase): self.assertIn(self.r1, r2_relations) # Delete a part, ensure the relationship also gets deleted + self.r1.active = False + self.r1.save() self.r1.delete() self.assertEqual(PartRelated.objects.count(), countbefore) @@ -351,6 +353,8 @@ class PartTest(TestCase): self.assertEqual(len(self.r2.get_related_parts()), n) # Deleting r2 should remove *all* newly created relationships + self.r2.active = False + self.r2.save() self.r2.delete() self.assertEqual(PartRelated.objects.count(), countbefore) diff --git a/src/backend/InvenTree/plugin/base/integration/ValidationMixin.py b/src/backend/InvenTree/plugin/base/integration/ValidationMixin.py index 285069b9f2..d282de64d0 100644 --- a/src/backend/InvenTree/plugin/base/integration/ValidationMixin.py +++ b/src/backend/InvenTree/plugin/base/integration/ValidationMixin.py @@ -49,6 +49,23 @@ class ValidationMixin: """Raise a ValidationError with the given message.""" raise ValidationError(message) + def validate_model_deletion(self, instance): + """Run custom validation when a model instance is being deleted. + + This method is called when a model instance is being deleted. + It allows the plugin to raise a ValidationError if the instance cannot be deleted. + + Arguments: + instance: The model instance to validate + + Returns: + None or True (refer to class docstring) + + Raises: + ValidationError if the instance cannot be deleted + """ + return None + def validate_model_instance(self, instance, deltas=None): """Run custom validation on a database model instance. diff --git a/src/backend/InvenTree/templates/InvenTree/settings/part.html b/src/backend/InvenTree/templates/InvenTree/settings/part.html index 00d606c5b9..dae8368b7f 100644 --- a/src/backend/InvenTree/templates/InvenTree/settings/part.html +++ b/src/backend/InvenTree/templates/InvenTree/settings/part.html @@ -15,6 +15,7 @@ {% include "InvenTree/settings/setting.html" with key="PART_IPN_REGEX" %} {% include "InvenTree/settings/setting.html" with key="PART_ALLOW_DUPLICATE_IPN" %} {% include "InvenTree/settings/setting.html" with key="PART_ALLOW_EDIT_IPN" %} + {% include "InvenTree/settings/setting.html" with key="PART_ALLOW_DELETE_FROM_ASSEMBLY" %} {% include "InvenTree/settings/setting.html" with key="PART_NAME_FORMAT" %} {% include "InvenTree/settings/setting.html" with key="PART_SHOW_RELATED" icon="fa-random" %} {% include "InvenTree/settings/setting.html" with key="PART_CREATE_INITIAL" icon="fa-boxes" %} diff --git a/src/frontend/src/components/forms/ApiForm.tsx b/src/frontend/src/components/forms/ApiForm.tsx index 88eadd0ce2..2ef68b4281 100644 --- a/src/frontend/src/components/forms/ApiForm.tsx +++ b/src/frontend/src/components/forms/ApiForm.tsx @@ -497,7 +497,7 @@ export function ApiForm({ {/* Form Fields */} {(!isValid || nonFieldErrors.length > 0) && ( - + {nonFieldErrors.length > 0 && ( {nonFieldErrors.map((message) => ( diff --git a/src/frontend/src/pages/Index/Settings/SystemSettings.tsx b/src/frontend/src/pages/Index/Settings/SystemSettings.tsx index 3208caaa09..904869bb7e 100644 --- a/src/frontend/src/pages/Index/Settings/SystemSettings.tsx +++ b/src/frontend/src/pages/Index/Settings/SystemSettings.tsx @@ -173,6 +173,7 @@ export default function SystemSettings() { 'PART_IPN_REGEX', 'PART_ALLOW_DUPLICATE_IPN', 'PART_ALLOW_EDIT_IPN', + 'PART_ALLOW_DELETE_FROM_ASSEMBLY', 'PART_NAME_FORMAT', 'PART_SHOW_RELATED', 'PART_CREATE_INITIAL',