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
This commit is contained in:
Oliver 2024-05-20 12:51:56 +10:00 committed by GitHub
parent b26640fb36
commit c540b12c97
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
12 changed files with 75 additions and 18 deletions

View File

@ -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.

View File

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

View File

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

View File

@ -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.

View File

@ -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.

View File

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

View File

@ -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

View File

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

View File

@ -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.

View File

@ -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" %}

View File

@ -497,7 +497,7 @@ export function ApiForm({
{/* Form Fields */}
<Stack gap="sm">
{(!isValid || nonFieldErrors.length > 0) && (
<Alert radius="sm" color="red" title={t`Form Errors Exist`}>
<Alert radius="sm" color="red" title={t`Error`}>
{nonFieldErrors.length > 0 && (
<Stack gap="xs">
{nonFieldErrors.map((message) => (

View File

@ -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',