From 7bf6980b05d81304a52d794277420fbcc556244a Mon Sep 17 00:00:00 2001 From: Oliver Walters Date: Mon, 6 Jun 2022 10:15:33 +1000 Subject: [PATCH 1/4] Updates for the PartRelated model - Deleting a part also deletes the relationships - Add unique_together requirement - Bug fixes - Added unit tests --- .../migrations/0078_auto_20220606_0024.py | 28 +++++++++ InvenTree/part/models.py | 60 +++++++------------ InvenTree/part/test_part.py | 51 +++++++++++++++- 3 files changed, 99 insertions(+), 40 deletions(-) create mode 100644 InvenTree/part/migrations/0078_auto_20220606_0024.py diff --git a/InvenTree/part/migrations/0078_auto_20220606_0024.py b/InvenTree/part/migrations/0078_auto_20220606_0024.py new file mode 100644 index 000000000000..385eb591ce5b --- /dev/null +++ b/InvenTree/part/migrations/0078_auto_20220606_0024.py @@ -0,0 +1,28 @@ +# Generated by Django 3.2.13 on 2022-06-06 00:24 + +from django.db import migrations, models +import django.db.models.deletion + + +class Migration(migrations.Migration): + + dependencies = [ + ('part', '0077_alter_bomitem_unique_together'), + ] + + operations = [ + migrations.AlterField( + model_name='partrelated', + name='part_1', + field=models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='related_parts_1', to='part.part', verbose_name='Part 1'), + ), + migrations.AlterField( + model_name='partrelated', + name='part_2', + field=models.ForeignKey(help_text='Select Related Part', on_delete=django.db.models.deletion.CASCADE, related_name='related_parts_2', to='part.part', verbose_name='Part 2'), + ), + migrations.AlterUniqueTogether( + name='partrelated', + unique_together={('part_1', 'part_2')}, + ), + ] diff --git a/InvenTree/part/models.py b/InvenTree/part/models.py index 912541fbc028..ebe9f882d6f8 100644 --- a/InvenTree/part/models.py +++ b/InvenTree/part/models.py @@ -2037,27 +2037,20 @@ def get_conversion_options(self): return filtered_parts def get_related_parts(self): - """Return list of tuples for all related parts. - - Includes: - - first value is PartRelated object - - second value is matching Part object - """ - related_parts = [] + """Return a set of all related parts for this part""" + related_parts = set() related_parts_1 = self.related_parts_1.filter(part_1__id=self.pk) related_parts_2 = self.related_parts_2.filter(part_2__id=self.pk) - related_parts.append() - for related_part in related_parts_1: # Add to related parts list - related_parts.append(related_part.part_2) + related_parts.add(related_part.part_2) for related_part in related_parts_2: # Add to related parts list - related_parts.append(related_part.part_1) + related_parts.add(related_part.part_1) return related_parts @@ -2829,44 +2822,35 @@ def get_api_url(): class PartRelated(models.Model): """Store and handle related parts (eg. mating connector, crimps, etc.).""" + class Meta: + """Metaclass defines extra model properties""" + unique_together = ('part_1', 'part_2') + part_1 = models.ForeignKey(Part, related_name='related_parts_1', - verbose_name=_('Part 1'), on_delete=models.DO_NOTHING) + verbose_name=_('Part 1'), on_delete=models.CASCADE) part_2 = models.ForeignKey(Part, related_name='related_parts_2', - on_delete=models.DO_NOTHING, + on_delete=models.CASCADE, verbose_name=_('Part 2'), help_text=_('Select Related Part')) def __str__(self): """Return a string representation of this Part-Part relationship""" return f'{self.part_1} <--> {self.part_2}' - def validate(self, part_1, part_2): - """Validate that the two parts relationship is unique.""" - validate = True - - parts = Part.objects.all() - related_parts = PartRelated.objects.all() - - # Check if part exist and there are not the same part - if (part_1 in parts and part_2 in parts) and (part_1.pk != part_2.pk): - # Check if relation exists already - for relation in related_parts: - if (part_1 == relation.part_1 and part_2 == relation.part_2) \ - or (part_1 == relation.part_2 and part_2 == relation.part_1): - validate = False - break - else: - validate = False - - return validate + def save(self, *args, **kwargs): + """Enforce a 'clean' operation when saving a PartRelated instance""" + self.clean() + self.validate_unique() + super().save(*args, **kwargs) def clean(self): """Overwrite clean method to check that relation is unique.""" - validate = self.validate(self.part_1, self.part_2) - if not validate: - error_message = _('Error creating relationship: check that ' - 'the part is not related to itself ' - 'and that the relationship is unique') + super().clean() + + if self.part_1 == self.part_2: + raise ValidationError(_("Part relationship cannot be created between a part and itself")) - raise ValidationError(error_message) + # Check for inverse relationship + if PartRelated.objects.filter(part_1=self.part_2, part_2=self.part_1).exists(): + raise ValidationError(_("Duplicate relationship already exists")) diff --git a/InvenTree/part/test_part.py b/InvenTree/part/test_part.py index fea5cf601276..97e690c53534 100644 --- a/InvenTree/part/test_part.py +++ b/InvenTree/part/test_part.py @@ -15,8 +15,8 @@ from InvenTree import version from InvenTree.helpers import InvenTreeTestCase -from .models import (Part, PartCategory, PartCategoryStar, PartStar, - PartTestTemplate, rename_part_image) +from .models import (Part, PartCategory, PartCategoryStar, PartRelated, + PartStar, PartTestTemplate, rename_part_image) from .templatetags import inventree_extras @@ -280,6 +280,53 @@ def test_metadata(self): self.assertEqual(len(p.metadata.keys()), 4) + def test_related(self): + """Unit tests for the PartRelated model""" + + # Create a part relationship + PartRelated.objects.create(part_1=self.r1, part_2=self.r2) + self.assertEqual(PartRelated.objects.count(), 1) + + # Creating a duplicate part relationship should fail + with self.assertRaises(ValidationError): + PartRelated.objects.create(part_1=self.r1, part_2=self.r2) + + # Creating an 'inverse' duplicate relationship should also fail + with self.assertRaises(ValidationError): + PartRelated.objects.create(part_1=self.r2, part_2=self.r1) + + # Try to add a self-referential relationship + with self.assertRaises(ValidationError): + PartRelated.objects.create(part_1=self.r2, part_2=self.r2) + + # Test relation lookup for each part + r1_relations = self.r1.get_related_parts() + self.assertEqual(len(r1_relations), 1) + self.assertIn(self.r2, r1_relations) + + r2_relations = self.r2.get_related_parts() + self.assertEqual(len(r2_relations), 1) + self.assertIn(self.r1, r2_relations) + + # Delete a part, ensure the relationship also gets deleted + self.r1.delete() + + self.assertEqual(PartRelated.objects.count(), 0) + self.assertEqual(len(self.r2.get_related_parts()), 0) + + # Add multiple part relationships to self.r2 + for p in Part.objects.all().exclude(pk=self.r2.pk): + PartRelated.objects.create(part_1=p, part_2=self.r2) + + n = Part.objects.count() - 1 + + self.assertEqual(PartRelated.objects.count(), n) + self.assertEqual(len(self.r2.get_related_parts()), n) + + # Deleting r2 should remove *all* relationships + self.r2.delete() + self.assertEqual(PartRelated.objects.count(), 0) + class TestTemplateTest(TestCase): """Unit test for the TestTemplate class""" From 5db1c11b2295e22078bb5f843e4c65fe91825e90 Mon Sep 17 00:00:00 2001 From: Oliver Walters Date: Mon, 6 Jun 2022 10:46:57 +1000 Subject: [PATCH 2/4] Adds JS function to delete a part instance --- InvenTree/part/templates/part/part_base.html | 14 +++--- InvenTree/templates/js/translated/part.js | 46 ++++++++++++++++++++ 2 files changed, 53 insertions(+), 7 deletions(-) diff --git a/InvenTree/part/templates/part/part_base.html b/InvenTree/part/templates/part/part_base.html index bcf98bef9c1f..e940d46926e1 100644 --- a/InvenTree/part/templates/part/part_base.html +++ b/InvenTree/part/templates/part/part_base.html @@ -559,13 +559,13 @@
{% if roles.part.delete %} $("#part-delete").click(function() { - launchModalForm( - "{% url 'part-delete' part.id %}", - { - redirect: {% if part.category %}"{% url 'category-detail' part.category.id %}"{% else %}"{% url 'part-index' %}"{% endif %}, - no_post: {% if part.active %}true{% else %}false{% endif %}, - } - ); + deletePart({{ part.pk }}, { + {% if part.category %} + redirect: '{% url "category-detail" part.category.pk %}', + {% else %} + redirect: '{% url "part-index" %}', + {% endif %} + }); }); {% endif %} diff --git a/InvenTree/templates/js/translated/part.js b/InvenTree/templates/js/translated/part.js index dfc143977d81..4c7a593f5ee4 100644 --- a/InvenTree/templates/js/translated/part.js +++ b/InvenTree/templates/js/translated/part.js @@ -21,6 +21,7 @@ */ /* exported + deletePart, duplicateBom, duplicatePart, editCategory, @@ -395,6 +396,51 @@ function duplicatePart(pk, options={}) { } +// Launch form to delete a part +function deletePart(pk, options={}) { + + inventreeGet(`/api/part/${pk}/`, {}, { + success: function(part) { + if (part.active) { + showAlertDialog( + '{% trans "Active Part" %}', + '{% trans "Part cannot be deleted as it is currently active" %}', + { + alert_style: 'danger', + } + ); + return; + } + + var thumb = thumbnailImage(part.thumbnail || part.image); + + var html = ` +
+

${thumb} ${part.full_name} - ${part.description}

+ + {% trans "Deleting this part cannot be reversed" %} +
    +
  • {% trans "Any stock items for this part will be deleted" %}
    • +
  • {% trans "This part will be removed from any Bills of Material" %}
    • +
  • {% trans "All manufacturer and supplier information for this part will be deleted" %}
    • +
`; + + constructForm( + `/api/part/${pk}/`, + { + method: 'DELETE', + title: '{% trans "Delete Part" %}', + preFormContent: html, + onSuccess: function(response) { + handleFormSuccess(response, options); + } + } + ) + + } + }); +} + /* Toggle the 'starred' status of a part. * Performs AJAX queries and updates the display on the button. * From 6b9cd3f9f9b7ef3e7f2f790f282dc3cac663c896 Mon Sep 17 00:00:00 2001 From: Oliver Walters Date: Mon, 6 Jun 2022 10:48:06 +1000 Subject: [PATCH 3/4] Remove legacy delete view --- .../part/templates/part/partial_delete.html | 78 ------------------- InvenTree/part/urls.py | 1 - InvenTree/part/views.py | 17 ---- 3 files changed, 96 deletions(-) delete mode 100644 InvenTree/part/templates/part/partial_delete.html diff --git a/InvenTree/part/templates/part/partial_delete.html b/InvenTree/part/templates/part/partial_delete.html deleted file mode 100644 index cd83f8df4f9b..000000000000 --- a/InvenTree/part/templates/part/partial_delete.html +++ /dev/null @@ -1,78 +0,0 @@ -{% extends "modal_form.html" %} -{% load i18n %} - -{% block pre_form_content %} - -{% if part.active %} - -
- {% blocktrans with full_name=part.full_name %}Part '{{full_name}}' cannot be deleted as it is still marked as active. -
Disable the "Active" part attribute and re-try. - {% endblocktrans %} -
- -{% else %} - -
- {% blocktrans with full_name=part.full_name %}Are you sure you want to delete part '{{full_name}}'?{% endblocktrans %} -
- -{% if part.used_in_count %} -
-

{% blocktrans with count=part.used_in_count %}This part is used in BOMs for {{count}} other parts. If you delete this part, the BOMs for the following parts will be updated{% endblocktrans %}: -

    - {% for child in part.used_in.all %} -
  • {{ child.part.full_name }} - {{ child.part.description }}
    • - {% endfor %} -

    -{% endif %} - -{% if part.stock_items.all|length > 0 %} -
    -

    {% blocktrans with count=part.stock_items.all|length %}There are {{count}} stock entries defined for this part. If you delete this part, the following stock entries will also be deleted:{% endblocktrans %} -

      - {% for stock in part.stock_items.all %} -
    • {{ stock }}
      • - {% endfor %} -
    -

    -{% endif %} - -{% if part.manufacturer_parts.all|length > 0 %} -
    -

    {% blocktrans with count=part.manufacturer_parts.all|length %}There are {{count}} manufacturers defined for this part. If you delete this part, the following manufacturer parts will also be deleted:{% endblocktrans %} -

      - {% for spart in part.manufacturer_parts.all %} -
    • {% if spart.manufacturer %}{{ spart.manufacturer.name }} - {% endif %}{{ spart.MPN }}
      • - {% endfor %} -
    -

    -{% endif %} - -{% if part.supplier_parts.all|length > 0 %} -
    -

    {% blocktrans with count=part.supplier_parts.all|length %}There are {{count}} suppliers defined for this part. If you delete this part, the following supplier parts will also be deleted:{% endblocktrans %} -

      - {% for spart in part.supplier_parts.all %} - {% if spart.supplier %} -
    • {{ spart.supplier.name }} - {{ spart.SKU }}
      • - {% endif %} - {% endfor %} -
    -

    -{% endif %} - -{% if part.serials.all|length > 0 %} -
    -

    {% blocktrans with count=part.serials.all|length full_name=part.full_name %}There are {{count}} unique parts tracked for '{{full_name}}'. Deleting this part will permanently remove this tracking information.{% endblocktrans %}

    -{% endif %} - -{% endif %} - -{% endblock %} - -{% block form %} -{% if not part.active %} -{{ block.super }} -{% endif %} -{% endblock %} diff --git a/InvenTree/part/urls.py b/InvenTree/part/urls.py index 9c70b364c970..ac34225c0081 100644 --- a/InvenTree/part/urls.py +++ b/InvenTree/part/urls.py @@ -11,7 +11,6 @@ from . import views part_detail_urls = [ - re_path(r'^delete/?', views.PartDelete.as_view(), name='part-delete'), re_path(r'^bom-download/?', views.BomDownload.as_view(), name='bom-download'), re_path(r'^pricing/', views.PartPricing.as_view(), name='part-pricing'), diff --git a/InvenTree/part/views.py b/InvenTree/part/views.py index d67e72f4cc4f..a57f6905bd79 100644 --- a/InvenTree/part/views.py +++ b/InvenTree/part/views.py @@ -760,23 +760,6 @@ def get_data(self): } -class PartDelete(AjaxDeleteView): - """View to delete a Part object.""" - - model = Part - ajax_template_name = 'part/partial_delete.html' - ajax_form_title = _('Confirm Part Deletion') - context_object_name = 'part' - - success_url = '/part/' - - def get_data(self): - """Returns custom message once the part deletion has been performed""" - return { - 'danger': _('Part was deleted'), - } - - class PartPricing(AjaxView): """View for inspecting part pricing information.""" From c2f905de2aee8b46cbec9582b792a3dca1acb67d Mon Sep 17 00:00:00 2001 From: Oliver Walters Date: Mon, 6 Jun 2022 10:57:48 +1000 Subject: [PATCH 4/4] JS linting --- InvenTree/templates/js/translated/part.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/InvenTree/templates/js/translated/part.js b/InvenTree/templates/js/translated/part.js index 4c7a593f5ee4..36ff76d50352 100644 --- a/InvenTree/templates/js/translated/part.js +++ b/InvenTree/templates/js/translated/part.js @@ -435,8 +435,7 @@ function deletePart(pk, options={}) { handleFormSuccess(response, options); } } - ) - + ); } }); }