diff --git a/InvenTree/InvenTree/serializers.py b/InvenTree/InvenTree/serializers.py index fa7674723c..2bc89ef637 100644 --- a/InvenTree/InvenTree/serializers.py +++ b/InvenTree/InvenTree/serializers.py @@ -6,12 +6,15 @@ Serializers used in various InvenTree apps # -*- coding: utf-8 -*- from __future__ import unicode_literals -from rest_framework import serializers - import os from django.conf import settings from django.contrib.auth.models import User +from django.core.exceptions import ValidationError as DjangoValidationError + +from rest_framework import serializers +from rest_framework.fields import empty +from rest_framework.exceptions import ValidationError class UserSerializer(serializers.ModelSerializer): @@ -39,18 +42,34 @@ class InvenTreeModelSerializer(serializers.ModelSerializer): but also ensures that the underlying model class data are checked on validation. """ - def validate(self, data): + def run_validation(self, data=empty): """ Perform serializer validation. In addition to running validators on the serializer fields, this class ensures that the underlying model is also validated. """ - # Run any native validation checks first (may throw an ValidationError) - data = super(serializers.ModelSerializer, self).validate(data) + # Run any native validation checks first (may raise a ValidationError) + data = super().run_validation(data) # Now ensure the underlying model is correct - instance = self.Meta.model(**data) - instance.clean() + + if not hasattr(self, 'instance') or self.instance is None: + # No instance exists (we are creating a new one) + instance = self.Meta.model(**data) + else: + # Instance already exists (we are updating!) + instance = self.instance + + # Update instance fields + for attr, value in data.items(): + setattr(instance, attr, value) + + # Run a 'full_clean' on the model. + # Note that by default, DRF does *not* perform full model validation! + try: + instance.full_clean() + except (ValidationError, DjangoValidationError) as exc: + raise ValidationError(detail=serializers.as_serializer_error(exc)) return data diff --git a/InvenTree/build/migrations/0029_auto_20210601_1525.py b/InvenTree/build/migrations/0029_auto_20210601_1525.py index c5ea04b5c9..e8b2d58947 100644 --- a/InvenTree/build/migrations/0029_auto_20210601_1525.py +++ b/InvenTree/build/migrations/0029_auto_20210601_1525.py @@ -40,7 +40,8 @@ def assign_bom_items(apps, schema_editor): except BomItem.DoesNotExist: pass - print(f"Assigned BomItem for {count_valid}/{count_total} entries") + if count_total > 0: + print(f"Assigned BomItem for {count_valid}/{count_total} entries") def unassign_bom_items(apps, schema_editor): diff --git a/InvenTree/company/migrations/0026_auto_20201110_1011.py b/InvenTree/company/migrations/0026_auto_20201110_1011.py index c6be37b967..20ec7d2f6f 100644 --- a/InvenTree/company/migrations/0026_auto_20201110_1011.py +++ b/InvenTree/company/migrations/0026_auto_20201110_1011.py @@ -71,7 +71,8 @@ def migrate_currencies(apps, schema_editor): count += 1 - print(f"Updated {count} SupplierPriceBreak rows") + if count > 0: + print(f"Updated {count} SupplierPriceBreak rows") def reverse_currencies(apps, schema_editor): """ diff --git a/InvenTree/company/test_api.py b/InvenTree/company/test_api.py index c43280c76c..d5cb573f47 100644 --- a/InvenTree/company/test_api.py +++ b/InvenTree/company/test_api.py @@ -119,7 +119,9 @@ class ManufacturerTest(InvenTreeAPITestCase): data = { 'MPN': 'MPN-TEST-123', } + response = self.client.patch(url, data, format='json') + self.assertEqual(response.status_code, status.HTTP_200_OK) self.assertEqual(response.data['MPN'], 'MPN-TEST-123') diff --git a/InvenTree/part/fixtures/part.yaml b/InvenTree/part/fixtures/part.yaml index 508a1577bb..3c24690efc 100644 --- a/InvenTree/part/fixtures/part.yaml +++ b/InvenTree/part/fixtures/part.yaml @@ -6,7 +6,7 @@ name: 'M2x4 LPHS' description: 'M2x4 low profile head screw' category: 8 - link: www.acme.com/parts/m2x4lphs + link: http://www.acme.com/parts/m2x4lphs tree_id: 0 purchaseable: True level: 0 @@ -56,6 +56,7 @@ fields: name: 'C_22N_0805' description: '22nF capacitor in 0805 package' + purchaseable: true category: 3 tree_id: 0 level: 0 diff --git a/InvenTree/part/migrations/0056_auto_20201110_1125.py b/InvenTree/part/migrations/0056_auto_20201110_1125.py index b382dded71..862f9411c8 100644 --- a/InvenTree/part/migrations/0056_auto_20201110_1125.py +++ b/InvenTree/part/migrations/0056_auto_20201110_1125.py @@ -71,7 +71,8 @@ def migrate_currencies(apps, schema_editor): count += 1 - print(f"Updated {count} SupplierPriceBreak rows") + if count > 0: + print(f"Updated {count} SupplierPriceBreak rows") def reverse_currencies(apps, schema_editor): """ diff --git a/InvenTree/part/migrations/0068_part_unique_part.py b/InvenTree/part/migrations/0068_part_unique_part.py new file mode 100644 index 0000000000..2e87fc7b2a --- /dev/null +++ b/InvenTree/part/migrations/0068_part_unique_part.py @@ -0,0 +1,17 @@ +# Generated by Django 3.2.4 on 2021-06-21 23:10 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('part', '0067_partinternalpricebreak'), + ] + + operations = [ + migrations.AddConstraint( + model_name='part', + constraint=models.UniqueConstraint(fields=('name', 'IPN', 'revision'), name='unique_part'), + ), + ] diff --git a/InvenTree/part/models.py b/InvenTree/part/models.py index 8aa370a7ae..8ddf049216 100644 --- a/InvenTree/part/models.py +++ b/InvenTree/part/models.py @@ -321,6 +321,9 @@ class Part(MPTTModel): verbose_name = _("Part") verbose_name_plural = _("Parts") ordering = ['name', ] + constraints = [ + UniqueConstraint(fields=['name', 'IPN', 'revision'], name='unique_part') + ] class MPTTMeta: # For legacy reasons the 'variant_of' field is used to indicate the MPTT parent @@ -379,7 +382,7 @@ class Part(MPTTModel): logger.info(f"Deleting unused image file '{previous.image}'") previous.image.delete(save=False) - self.clean() + self.full_clean() super().save(*args, **kwargs) @@ -642,23 +645,6 @@ class Part(MPTTModel): 'IPN': _('Duplicate IPN not allowed in part settings'), }) - # Part name uniqueness should be case insensitive - try: - parts = Part.objects.exclude(id=self.id).filter( - name__iexact=self.name, - IPN__iexact=self.IPN, - revision__iexact=self.revision) - - if parts.exists(): - msg = _("Part must be unique for name, IPN and revision") - raise ValidationError({ - "name": msg, - "IPN": msg, - "revision": msg, - }) - except Part.DoesNotExist: - pass - def clean(self): """ Perform cleaning operations for the Part model @@ -671,8 +657,6 @@ class Part(MPTTModel): super().clean() - self.validate_unique() - if self.trackable: for part in self.get_used_in().all(): diff --git a/InvenTree/part/test_api.py b/InvenTree/part/test_api.py index 0f5f59d3a3..176149c880 100644 --- a/InvenTree/part/test_api.py +++ b/InvenTree/part/test_api.py @@ -1,8 +1,10 @@ +# -*- coding: utf-8 -*- + from rest_framework import status from django.urls import reverse -from part.models import Part +from part.models import Part, PartCategory from stock.models import StockItem from company.models import Company @@ -230,6 +232,18 @@ class PartAPITest(InvenTreeAPITestCase): response = self.client.get(url, data={'part': 10004}) self.assertEqual(len(response.data), 7) + # Try to post a new object (missing description) + response = self.client.post( + url, + data={ + 'part': 10000, + 'test_name': 'My very first test', + 'required': False, + } + ) + + self.assertEqual(response.status_code, 400) + # Try to post a new object (should succeed) response = self.client.post( url, @@ -237,6 +251,7 @@ class PartAPITest(InvenTreeAPITestCase): 'part': 10000, 'test_name': 'New Test', 'required': True, + 'description': 'a test description' }, format='json', ) @@ -248,7 +263,8 @@ class PartAPITest(InvenTreeAPITestCase): url, data={ 'part': 10004, - 'test_name': " newtest" + 'test_name': " newtest", + 'description': 'dafsdf', }, format='json', ) @@ -293,6 +309,171 @@ class PartAPITest(InvenTreeAPITestCase): self.assertEqual(len(data['results']), n) +class PartDetailTests(InvenTreeAPITestCase): + """ + Test that we can create / edit / delete Part objects via the API + """ + + fixtures = [ + 'category', + 'part', + 'location', + 'bom', + 'test_templates', + ] + + roles = [ + 'part.change', + 'part.add', + 'part.delete', + 'part_category.change', + 'part_category.add', + ] + + def setUp(self): + super().setUp() + + def test_part_operations(self): + n = Part.objects.count() + + # Create a part + response = self.client.post( + reverse('api-part-list'), + { + 'name': 'my test api part', + 'description': 'a part created with the API', + 'category': 1, + } + ) + + self.assertEqual(response.status_code, 201) + + pk = response.data['pk'] + + # Check that a new part has been added + self.assertEqual(Part.objects.count(), n + 1) + + part = Part.objects.get(pk=pk) + + self.assertEqual(part.name, 'my test api part') + + # Edit the part + url = reverse('api-part-detail', kwargs={'pk': pk}) + + # Let's change the name of the part + + response = self.client.patch(url, { + 'name': 'a new better name', + }) + + self.assertEqual(response.status_code, 200) + self.assertEqual(response.data['pk'], pk) + self.assertEqual(response.data['name'], 'a new better name') + + part = Part.objects.get(pk=pk) + + # Name has been altered + self.assertEqual(part.name, 'a new better name') + + # Part count should not have changed + self.assertEqual(Part.objects.count(), n + 1) + + # Now, try to set the name to the *same* value + # 2021-06-22 this test is to check that the "duplicate part" checks don't do strange things + response = self.client.patch(url, { + 'name': 'a new better name', + }) + + self.assertEqual(response.status_code, 200) + + # Try to remove the part + response = self.client.delete(url) + + self.assertEqual(response.status_code, 204) + + # Part count should have reduced + self.assertEqual(Part.objects.count(), n) + + def test_duplicates(self): + """ + Check that trying to create 'duplicate' parts results in errors + """ + + # Create a part + response = self.client.post(reverse('api-part-list'), { + 'name': 'part', + 'description': 'description', + 'IPN': 'IPN-123', + 'category': 1, + 'revision': 'A', + }) + + self.assertEqual(response.status_code, 201) + + n = Part.objects.count() + + # Check that we cannot create a duplicate in a different category + response = self.client.post(reverse('api-part-list'), { + 'name': 'part', + 'description': 'description', + 'IPN': 'IPN-123', + 'category': 2, + 'revision': 'A', + }) + + self.assertEqual(response.status_code, 400) + + # Check that only 1 matching part exists + parts = Part.objects.filter( + name='part', + description='description', + IPN='IPN-123' + ) + + self.assertEqual(parts.count(), 1) + + # A new part should *not* have been created + self.assertEqual(Part.objects.count(), n) + + # But a different 'revision' *can* be created + response = self.client.post(reverse('api-part-list'), { + 'name': 'part', + 'description': 'description', + 'IPN': 'IPN-123', + 'category': 2, + 'revision': 'B', + }) + + self.assertEqual(response.status_code, 201) + self.assertEqual(Part.objects.count(), n + 1) + + # Now, check that we cannot *change* an existing part to conflict + pk = response.data['pk'] + + url = reverse('api-part-detail', kwargs={'pk': pk}) + + # Attempt to alter the revision code + response = self.client.patch( + url, + { + 'revision': 'A', + }, + format='json', + ) + + self.assertEqual(response.status_code, 400) + + # But we *can* change it to a unique revision code + response = self.client.patch( + url, + { + 'revision': 'C', + } + ) + + self.assertEqual(response.status_code, 200) + + class PartAPIAggregationTest(InvenTreeAPITestCase): """ Tests to ensure that the various aggregation annotations are working correctly... @@ -319,6 +500,8 @@ class PartAPIAggregationTest(InvenTreeAPITestCase): # Add a new part self.part = Part.objects.create( name='Banana', + description='This is a banana', + category=PartCategory.objects.get(pk=1), ) # Create some stock items associated with the part diff --git a/InvenTree/part/test_part.py b/InvenTree/part/test_part.py index 2bc24c3a99..e30c80549f 100644 --- a/InvenTree/part/test_part.py +++ b/InvenTree/part/test_part.py @@ -11,7 +11,7 @@ from django.core.exceptions import ValidationError import os -from .models import Part, PartTestTemplate +from .models import Part, PartCategory, PartTestTemplate from .models import rename_part_image, match_part_names from .templatetags import inventree_extras @@ -78,6 +78,61 @@ class PartTest(TestCase): p = Part.objects.get(pk=100) self.assertEqual(str(p), "BOB | Bob | A2 - Can we build it?") + def test_duplicate(self): + """ + Test that we cannot create a "duplicate" Part + """ + + n = Part.objects.count() + + cat = PartCategory.objects.get(pk=1) + + Part.objects.create( + category=cat, + name='part', + description='description', + IPN='IPN', + revision='A', + ) + + self.assertEqual(Part.objects.count(), n + 1) + + part = Part( + category=cat, + name='part', + description='description', + IPN='IPN', + revision='A', + ) + + with self.assertRaises(ValidationError): + part.validate_unique() + + try: + part.save() + self.assertTrue(False) + except: + pass + + self.assertEqual(Part.objects.count(), n + 1) + + # But we should be able to create a part with a different revision + part_2 = Part.objects.create( + category=cat, + name='part', + description='description', + IPN='IPN', + revision='B', + ) + + self.assertEqual(Part.objects.count(), n + 2) + + # Now, check that we cannot *change* part_2 to conflict + part_2.revision = 'A' + + with self.assertRaises(ValidationError): + part_2.validate_unique() + def test_metadata(self): self.assertEqual(self.r1.name, 'R_2K2_0805') self.assertEqual(self.r1.get_absolute_url(), '/part/3/') @@ -277,21 +332,24 @@ class PartSettingsTest(TestCase): """ # Create a part - Part.objects.create(name='Hello', description='A thing', IPN='IPN123') + Part.objects.create(name='Hello', description='A thing', IPN='IPN123', revision='A') # Attempt to create a duplicate item (should fail) with self.assertRaises(ValidationError): - Part.objects.create(name='Hello', description='A thing', IPN='IPN123') + part = Part(name='Hello', description='A thing', IPN='IPN123', revision='A') + part.validate_unique() # Attempt to create item with duplicate IPN (should be allowed by default) Part.objects.create(name='Hello', description='A thing', IPN='IPN123', revision='B') # And attempt again with the same values (should fail) with self.assertRaises(ValidationError): - Part.objects.create(name='Hello', description='A thing', IPN='IPN123', revision='B') + part = Part(name='Hello', description='A thing', IPN='IPN123', revision='B') + part.validate_unique() # Now update the settings so duplicate IPN values are *not* allowed InvenTreeSetting.set_setting('PART_ALLOW_DUPLICATE_IPN', False, self.user) with self.assertRaises(ValidationError): - Part.objects.create(name='Hello', description='A thing', IPN='IPN123', revision='C') + part = Part(name='Hello', description='A thing', IPN='IPN123', revision='C') + part.full_clean() diff --git a/InvenTree/stock/migrations/0061_auto_20210511_0911.py b/InvenTree/stock/migrations/0061_auto_20210511_0911.py index 0ab37250c8..ba0ecc5207 100644 --- a/InvenTree/stock/migrations/0061_auto_20210511_0911.py +++ b/InvenTree/stock/migrations/0061_auto_20210511_0911.py @@ -199,7 +199,8 @@ def update_history(apps, schema_editor): update_count += 1 - print(f"\n==========================\nUpdated {update_count} StockItemHistory entries") + if update_count > 0: + print(f"\n==========================\nUpdated {update_count} StockItemHistory entries") def reverse_update(apps, schema_editor): diff --git a/InvenTree/stock/migrations/0064_auto_20210621_1724.py b/InvenTree/stock/migrations/0064_auto_20210621_1724.py index 71314f366c..361ad02c80 100644 --- a/InvenTree/stock/migrations/0064_auto_20210621_1724.py +++ b/InvenTree/stock/migrations/0064_auto_20210621_1724.py @@ -26,7 +26,8 @@ def extract_purchase_price(apps, schema_editor): # Find all the StockItem objects without a purchase_price which point to a PurchaseOrder items = StockItem.objects.filter(purchase_price=None).exclude(purchase_order=None) - print(f"Found {items.count()} stock items with missing purchase price information") + if items.count() > 0: + print(f"Found {items.count()} stock items with missing purchase price information") update_count = 0 @@ -56,7 +57,8 @@ def extract_purchase_price(apps, schema_editor): break - print(f"Updated pricing for {update_count} stock items") + if update_count > 0: + print(f"Updated pricing for {update_count} stock items") def reverse_operation(apps, schema_editor): """