Perform a "full_clean" on serialized model

- DRF does not by deault run validate_unique on the model
- Need to check if we are "creating" or "updating" a model
- Catch and re-throw errors in the correct format
- Unit tests

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)
+        data = super().run_validation(data)
 
         # Now ensure the underlying model is correct
+
+        if not hasattr(self, 'instance') or self.instance is None:
+            # No instance exists (we are creating a new one)
             instance = self.Meta.model(**data)
-        instance.clean()
+        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
 

InvenTree/part/test_api.py

@@ -1,3 +1,4 @@
+from django.db import models
 from rest_framework import status
 
 from django.urls import reverse
@@ -293,6 +294,169 @@ 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...

InvenTree/part/test_part.py

@@ -109,13 +109,14 @@ class PartTest(TestCase):
 
         try:
             part.save()
-            assert(False)
+            self.assertTrue(False)
         except:
             pass
 
         self.assertEqual(Part.objects.count(), n + 1)
 
-        Part.objects.create(
+        # But we should be able to create a part with a different revision
+        part_2 = Part.objects.create(
             category=cat,
             name='part',
             description='description',
@@ -125,6 +126,12 @@ class PartTest(TestCase):
 
         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/')

InvenTree/stock/migrations/0061_auto_20210511_0911.py

@@ -199,6 +199,7 @@ def update_history(apps, schema_editor):
                 update_count += 1
 
 
+    if update_count > 0:
         print(f"\n==========================\nUpdated {update_count} StockItemHistory entries")
 
 

InvenTree/stock/migrations/0064_auto_20210621_1724.py

@@ -26,6 +26,7 @@ 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)
 
+    if items.count() > 0:
         print(f"Found {items.count()} stock items with missing purchase price information")
 
     update_count = 0
@@ -56,6 +57,7 @@ def extract_purchase_price(apps, schema_editor):
 
                     break
 
+    if update_count > 0:
         print(f"Updated pricing for {update_count} stock items")
 
 def reverse_operation(apps, schema_editor):